Skip to content

Commit c23630b

Browse files
l46kokcopybara-github
authored andcommitted
Fix planner runtime to respect the ordering of user provided type provider
PiperOrigin-RevId: 992491032
1 parent 872fc15 commit c23630b

4 files changed

Lines changed: 19 additions & 19 deletions

File tree

‎bundle/src/test/java/dev/cel/bundle/CelImplTest.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1982,10 +1982,13 @@ public void program_nativeTypeUnknownsEnabled_asCallArguments() throws Exception
19821982

19831983
@Test
19841984
public void program_comprehensionDisabled_throws() throws Exception {
1985-
// TODO: Planner ExecutionFrame ignores CelOptions.enableComprehension(false).
19861985
Cel cel =
1987-
legacyCelBuilderWithMacros()
1988-
.setOptions(CelOptions.current().enableComprehension(false).build())
1986+
plannerCelBuilderWithMacros()
1987+
.setOptions(
1988+
CelOptions.current()
1989+
.enableHeterogeneousNumericComparisons(true)
1990+
.enableComprehension(false)
1991+
.build())
19891992
.build();
19901993
CelAbstractSyntaxTree ast = cel.compile("['foo', 'bar'].map(x, x)").getAst();
19911994

‎runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -551,13 +551,14 @@ public CelRuntime build() {
551551
.setResolveTypeDependencies(options().resolveTypeDependencies())
552552
.build();
553553

554-
CelTypeProvider combinedTypeProvider =
555-
new CelTypeProvider.CombinedCelTypeProvider(
556-
DefaultTypeProvider.getInstance(), messageTypeProvider);
554+
ImmutableList.Builder<CelTypeProvider> typeProviders = ImmutableList.builder();
555+
typeProviders.add(DefaultTypeProvider.getInstance());
557556
if (typeProvider() != null) {
558-
combinedTypeProvider =
559-
new CelTypeProvider.CombinedCelTypeProvider(combinedTypeProvider, typeProvider());
557+
typeProviders.add(typeProvider());
560558
}
559+
typeProviders.add(messageTypeProvider);
560+
CelTypeProvider combinedTypeProvider =
561+
new CelTypeProvider.CombinedCelTypeProvider(typeProviders.build());
561562

562563
DescriptorTypeResolver descriptorTypeResolver =
563564
DescriptorTypeResolver.create(combinedTypeProvider, celValueConverter);

‎runtime/src/main/java/dev/cel/runtime/planner/ExecutionFrame.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ static ExecutionFrame create(
4646
@Nullable CelEvaluationListener listener) {
4747
return new ExecutionFrame(
4848
functionResolver,
49-
celOptions.comprehensionMaxIterations(),
49+
getComprehensionMaxIterations(celOptions),
5050
partialVars,
5151
listener,
5252
/* asyncTracker= */ null);
@@ -61,12 +61,16 @@ static ExecutionFrame createForAsync(
6161
checkNotNull(asyncTracker, "asyncTracker");
6262
return new ExecutionFrame(
6363
functionResolver,
64-
celOptions.comprehensionMaxIterations(),
64+
getComprehensionMaxIterations(celOptions),
6565
partialVars,
6666
listener,
6767
asyncTracker);
6868
}
6969

70+
private static int getComprehensionMaxIterations(CelOptions celOptions) {
71+
return celOptions.enableComprehension() ? celOptions.comprehensionMaxIterations() : 0;
72+
}
73+
7074
Optional<CelResolvedOverload> findOverload(
7175
String functionName, Collection<String> overloadIds, Object[] args)
7276
throws CelEvaluationException {

‎runtime/src/test/java/dev/cel/runtime/DescriptorTypeResolverTest.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import dev.cel.common.values.OpaqueValue;
3333
import dev.cel.expr.conformance.proto3.TestAllTypes;
3434
import dev.cel.extensions.CelOptionalLibrary;
35-
import java.util.Optional;
3635
import org.junit.Test;
3736
import org.junit.runner.RunWith;
3837

@@ -45,16 +44,9 @@ public class DescriptorTypeResolverTest {
4544
private static final Cel CEL =
4645
CelFactory.plannerCelBuilder()
4746
.setTypeProvider(PROTO_MESSAGE_TYPE_PROVIDER)
48-
// TODO: Replace setValueProvider with
49-
// addMessageTypes(TestAllTypes.getDescriptor()) once CelRuntimeImpl prioritizes custom
50-
// CelTypeProvider over its internal messageTypeProvider.
51-
.setValueProvider(
52-
(structType, fields) ->
53-
structType.equals(TestAllTypes.getDescriptor().getFullName())
54-
? Optional.of(TestAllTypes.getDefaultInstance())
55-
: Optional.empty())
5647
.addCompilerLibraries(CelOptionalLibrary.INSTANCE)
5748
.addRuntimeLibraries(CelOptionalLibrary.INSTANCE)
49+
.addMessageTypes(TestAllTypes.getDescriptor())
5850
.setContainer(CelContainer.ofName(TestAllTypes.getDescriptor().getFullName()))
5951
.build();
6052

0 commit comments

Comments
 (0)