Skip to content

Commit 9ea5c6a

Browse files
l46kokcopybara-github
authored andcommitted
Properly honor enableStandardEnvironment flag when constructing runtime
Fixes #255 PiperOrigin-RevId: 610809925
1 parent 09b934a commit 9ea5c6a

File tree

3 files changed

+41
-23
lines changed

3 files changed

+41
-23
lines changed

runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,8 @@ public CelRuntimeLegacyImpl build() {
227227

228228
DynamicProto dynamicProto = DynamicProto.create(runtimeTypeFactory);
229229

230-
DefaultDispatcher dispatcher = DefaultDispatcher.create(options, dynamicProto);
231-
if (standardEnvironmentEnabled) {
232-
StandardFunctions.add(dispatcher, dynamicProto, options);
233-
}
230+
DefaultDispatcher dispatcher =
231+
DefaultDispatcher.create(options, dynamicProto, standardEnvironmentEnabled);
234232

235233
ImmutableMap<String, CelFunctionBinding> functionBindingMap =
236234
ImmutableMap.copyOf(functionBindings);
@@ -306,6 +304,8 @@ private Builder(Builder builder) {
306304
this.options = builder.options;
307305
this.extensionRegistry = builder.extensionRegistry;
308306
this.customTypeFactory = builder.customTypeFactory;
307+
this.standardEnvironmentEnabled = builder.standardEnvironmentEnabled;
308+
this.celValueProvider = builder.celValueProvider;
309309
// The following needs to be deep copied as they are collection builders
310310
this.fileTypes = deepCopy(builder.fileTypes);
311311
this.celRuntimeLibraries = deepCopy(builder.celRuntimeLibraries);

runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java

+21-19
Original file line numberDiff line numberDiff line change
@@ -43,46 +43,46 @@
4343
@ThreadSafe
4444
@Internal
4545
public final class DefaultDispatcher implements Dispatcher, Registrar {
46-
@SuppressWarnings("unused")
47-
private final CelOptions celOptions;
48-
4946
/**
50-
* @deprecated use {@link DefaultDispatcher(CelOptions)} instead.
47+
* Creates a new dispatcher with all standard functions.
48+
*
49+
* @deprecated Migrate to fluent APIs. See {@link CelRuntimeFactory}.
5150
*/
5251
@Deprecated
53-
public DefaultDispatcher(ImmutableSet<ExprFeatures> features) {
54-
this(CelOptions.fromExprFeatures(features));
55-
}
56-
57-
public DefaultDispatcher(CelOptions celOptions) {
58-
this.celOptions = celOptions;
52+
public static DefaultDispatcher create() {
53+
return create(CelOptions.LEGACY);
5954
}
6055

6156
/**
6257
* Creates a new dispatcher with all standard functions.
6358
*
64-
* @deprecated use {@link #create(CelOptions)} instead.
59+
* @deprecated Migrate to fluent APIs. See {@link CelRuntimeFactory}.
6560
*/
6661
@Deprecated
6762
public static DefaultDispatcher create(ImmutableSet<ExprFeatures> features) {
6863
return create(CelOptions.fromExprFeatures(features));
6964
}
7065

66+
/**
67+
* Creates a new dispatcher with all standard functions.
68+
*
69+
* @deprecated Migrate to fluent APIs. See {@link CelRuntimeFactory}.
70+
*/
71+
@Deprecated
7172
public static DefaultDispatcher create(CelOptions celOptions) {
7273
DynamicProto dynamicProto = DynamicProto.create(DefaultMessageFactory.INSTANCE);
73-
return create(celOptions, dynamicProto);
74+
return create(celOptions, dynamicProto, true);
7475
}
7576

76-
public static DefaultDispatcher create(CelOptions celOptions, DynamicProto dynamicProto) {
77-
DefaultDispatcher dispatcher = new DefaultDispatcher(celOptions);
78-
StandardFunctions.add(dispatcher, dynamicProto, celOptions);
77+
public static DefaultDispatcher create(
78+
CelOptions celOptions, DynamicProto dynamicProto, boolean enableStandardEnvironment) {
79+
DefaultDispatcher dispatcher = new DefaultDispatcher();
80+
if (enableStandardEnvironment) {
81+
StandardFunctions.add(dispatcher, dynamicProto, celOptions);
82+
}
7983
return dispatcher;
8084
}
8185

82-
public static DefaultDispatcher create() {
83-
return create(CelOptions.LEGACY);
84-
}
85-
8686
/** Internal representation of an overload. */
8787
@Immutable
8888
private static final class Overload {
@@ -237,4 +237,6 @@ public Dispatcher.ImmutableCopy immutableCopy() {
237237
return this;
238238
}
239239
}
240+
241+
private DefaultDispatcher() {}
240242
}

runtime/src/test/java/dev/cel/runtime/CelRuntimeTest.java

+16
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package dev.cel.runtime;
1616

1717
import static com.google.common.truth.Truth.assertThat;
18+
import static org.junit.Assert.assertThrows;
1819

1920
import com.google.api.expr.v1alpha1.Constant;
2021
import com.google.api.expr.v1alpha1.Expr;
@@ -398,4 +399,19 @@ public void trace_withVariableResolver() throws Exception {
398399

399400
assertThat(result).isEqualTo("hello");
400401
}
402+
403+
@Test
404+
public void standardEnvironmentDisabledForRuntime_throws() throws Exception {
405+
CelCompiler celCompiler =
406+
CelCompilerFactory.standardCelCompilerBuilder().setStandardEnvironmentEnabled(true).build();
407+
CelRuntime celRuntime =
408+
CelRuntimeFactory.standardCelRuntimeBuilder().setStandardEnvironmentEnabled(false).build();
409+
CelAbstractSyntaxTree ast = celCompiler.compile("size('hello')").getAst();
410+
411+
CelEvaluationException e =
412+
assertThrows(CelEvaluationException.class, () -> celRuntime.createProgram(ast).eval());
413+
assertThat(e)
414+
.hasMessageThat()
415+
.contains("Unknown overload id 'size_string' for function 'size'");
416+
}
401417
}

0 commit comments

Comments
 (0)