Properly honor enableStandardEnvironment flag when constructing runtime

Fixes #255 PiperOrigin-RevId: 610809925
google · Feb 27, 2024 · ec8f3cf · ec8f3cf
1 parent a370d05
commit ec8f3cf
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 23 deletions.
diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java
@@ -227,10 +227,8 @@ public CelRuntimeLegacyImpl build() {
 
       DynamicProto dynamicProto = DynamicProto.create(runtimeTypeFactory);
 
-      DefaultDispatcher dispatcher = DefaultDispatcher.create(options, dynamicProto);
-      if (standardEnvironmentEnabled) {
-        StandardFunctions.add(dispatcher, dynamicProto, options);
-      }
+      DefaultDispatcher dispatcher =
+          DefaultDispatcher.create(options, dynamicProto, standardEnvironmentEnabled);
 
       ImmutableMap<String, CelFunctionBinding> functionBindingMap =
           ImmutableMap.copyOf(functionBindings);
@@ -306,6 +304,8 @@ private Builder(Builder builder) {
       this.options = builder.options;
       this.extensionRegistry = builder.extensionRegistry;
       this.customTypeFactory = builder.customTypeFactory;
+      this.standardEnvironmentEnabled = builder.standardEnvironmentEnabled;
+      this.celValueProvider = builder.celValueProvider;
       // The following needs to be deep copied as they are collection builders
       this.fileTypes = deepCopy(builder.fileTypes);
       this.celRuntimeLibraries = deepCopy(builder.celRuntimeLibraries);

diff --git a/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java b/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java
@@ -43,46 +43,46 @@
 @ThreadSafe
 @Internal
 public final class DefaultDispatcher implements Dispatcher, Registrar {
-  @SuppressWarnings("unused")
-  private final CelOptions celOptions;
-
   /**
-   * @deprecated use {@link DefaultDispatcher(CelOptions)} instead.
+   * Creates a new dispatcher with all standard functions.
+   *
+   * @deprecated Migrate to fluent APIs. See {@link CelRuntimeFactory}.
    */
   @Deprecated
-  public DefaultDispatcher(ImmutableSet<ExprFeatures> features) {
-    this(CelOptions.fromExprFeatures(features));
-  }
-
-  public DefaultDispatcher(CelOptions celOptions) {
-    this.celOptions = celOptions;
+  public static DefaultDispatcher create() {
+    return create(CelOptions.LEGACY);
   }
 
   /**
    * Creates a new dispatcher with all standard functions.
    *
-   * @deprecated use {@link #create(CelOptions)} instead.
+   * @deprecated Migrate to fluent APIs. See {@link CelRuntimeFactory}.
    */
   @Deprecated
   public static DefaultDispatcher create(ImmutableSet<ExprFeatures> features) {
     return create(CelOptions.fromExprFeatures(features));
   }
 
+  /**
+   * Creates a new dispatcher with all standard functions.
+   *
+   * @deprecated Migrate to fluent APIs. See {@link CelRuntimeFactory}.
+   */
+  @Deprecated
   public static DefaultDispatcher create(CelOptions celOptions) {
     DynamicProto dynamicProto = DynamicProto.create(DefaultMessageFactory.INSTANCE);
-    return create(celOptions, dynamicProto);
+    return create(celOptions, dynamicProto, true);
   }
 
-  public static DefaultDispatcher create(CelOptions celOptions, DynamicProto dynamicProto) {
-    DefaultDispatcher dispatcher = new DefaultDispatcher(celOptions);
-    StandardFunctions.add(dispatcher, dynamicProto, celOptions);
+  public static DefaultDispatcher create(
+      CelOptions celOptions, DynamicProto dynamicProto, boolean enableStandardEnvironment) {
+    DefaultDispatcher dispatcher = new DefaultDispatcher();
+    if (enableStandardEnvironment) {
+      StandardFunctions.add(dispatcher, dynamicProto, celOptions);
+    }
     return dispatcher;
   }
 
-  public static DefaultDispatcher create() {
-    return create(CelOptions.LEGACY);
-  }
-
   /** Internal representation of an overload. */
   @Immutable
   private static final class Overload {
@@ -237,4 +237,6 @@ public Dispatcher.ImmutableCopy immutableCopy() {
       return this;
     }
   }
+
+  private DefaultDispatcher() {}
 }
diff --git a/runtime/src/test/java/dev/cel/runtime/CelRuntimeTest.java b/runtime/src/test/java/dev/cel/runtime/CelRuntimeTest.java
@@ -15,6 +15,7 @@
 package dev.cel.runtime;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
 
 import com.google.api.expr.v1alpha1.Constant;
 import com.google.api.expr.v1alpha1.Expr;
@@ -398,4 +399,19 @@ public void trace_withVariableResolver() throws Exception {
 
     assertThat(result).isEqualTo("hello");
   }
+
+  @Test
+  public void standardEnvironmentDisabledForRuntime_throws() throws Exception {
+    CelCompiler celCompiler =
+        CelCompilerFactory.standardCelCompilerBuilder().setStandardEnvironmentEnabled(true).build();
+    CelRuntime celRuntime =
+        CelRuntimeFactory.standardCelRuntimeBuilder().setStandardEnvironmentEnabled(false).build();
+    CelAbstractSyntaxTree ast = celCompiler.compile("size('hello')").getAst();
+
+    CelEvaluationException e =
+        assertThrows(CelEvaluationException.class, () -> celRuntime.createProgram(ast).eval());
+    assertThat(e)
+        .hasMessageThat()
+        .contains("Unknown overload id 'size_string' for function 'size'");
+  }
 }