From 731ce14b0454d6992bb3970187e377e5a2c0f318 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Mon, 29 Jan 2024 16:40:50 -0800 Subject: [PATCH] Add cel.block as an extension library PiperOrigin-RevId: 602536654 --- bundle/src/main/java/dev/cel/bundle/Cel.java | 4 +- .../main/java/dev/cel/bundle/CelFactory.java | 7 +- .../src/main/java/dev/cel/bundle/CelImpl.java | 38 ++- .../src/test/java/dev/cel/bundle/BUILD.bazel | 2 + .../test/java/dev/cel/bundle/CelImplTest.java | 25 ++ .../main/java/dev/cel/checker/CelChecker.java | 2 + .../dev/cel/checker/CelCheckerLegacyImpl.java | 99 +++++-- .../src/main/java/dev/cel/checker/Env.java | 4 +- .../checker/ProtoTypeMaskTypeProvider.java | 6 +- .../src/test/java/dev/cel/checker/BUILD.bazel | 1 + .../cel/checker/CelCheckerLegacyImplTest.java | 106 ++++++++ .../dev/cel/checker/CelCompilerImplTest.java | 45 ++++ .../dev/cel/checker/CelFunctionDeclTest.java | 10 +- .../ProtoTypeMaskTypeProviderTest.java | 31 ++- .../java/dev/cel/common/CelFunctionDecl.java | 12 +- .../main/java/dev/cel/common/CelOptions.java | 1 - .../java/dev/cel/common/CelOverloadDecl.java | 2 +- .../java/dev/cel/compiler/CelCompiler.java | 2 + .../dev/cel/compiler/CelCompilerImpl.java | 15 ++ .../dev/cel/optimizer/optimizers/BUILD.bazel | 3 + .../optimizers/SubexpressionOptimizer.java | 16 ++ .../dev/cel/optimizer/optimizers/BUILD.bazel | 3 + .../SubexpressionOptimizerTest.java | 252 ++++++++++++++++++ .../main/java/dev/cel/parser/CelParser.java | 2 + .../java/dev/cel/parser/CelParserImpl.java | 44 ++- .../dev/cel/parser/CelParserImplTest.java | 40 +++ .../main/java/dev/cel/runtime/CelRuntime.java | 2 + .../dev/cel/runtime/CelRuntimeLegacyImpl.java | 62 ++++- .../dev/cel/runtime/DefaultInterpreter.java | 26 +- .../cel/runtime/CelRuntimeLegacyImplTest.java | 60 +++++ 30 files changed, 854 insertions(+), 68 deletions(-) create mode 100644 checker/src/test/java/dev/cel/checker/CelCheckerLegacyImplTest.java create mode 100644 checker/src/test/java/dev/cel/checker/CelCompilerImplTest.java diff --git a/bundle/src/main/java/dev/cel/bundle/Cel.java b/bundle/src/main/java/dev/cel/bundle/Cel.java index 964b21a5c..677a4af2c 100644 --- a/bundle/src/main/java/dev/cel/bundle/Cel.java +++ b/bundle/src/main/java/dev/cel/bundle/Cel.java @@ -20,4 +20,6 @@ /** Cel interface for parse, type-check, and evaluation of CEL programs. */ @ThreadSafe -public interface Cel extends CelCompiler, CelRuntime {} +public interface Cel extends CelCompiler, CelRuntime { + CelBuilder toCelBuilder(); +} diff --git a/bundle/src/main/java/dev/cel/bundle/CelFactory.java b/bundle/src/main/java/dev/cel/bundle/CelFactory.java index c69c4a3a5..a2080bc25 100644 --- a/bundle/src/main/java/dev/cel/bundle/CelFactory.java +++ b/bundle/src/main/java/dev/cel/bundle/CelFactory.java @@ -17,8 +17,10 @@ import dev.cel.checker.CelCheckerLegacyImpl; import dev.cel.common.CelOptions; import dev.cel.compiler.CelCompiler; +import dev.cel.compiler.CelCompilerImpl; import dev.cel.parser.CelParserImpl; import dev.cel.runtime.CelRuntime; +import dev.cel.runtime.CelRuntimeLegacyImpl; /** Helper class to configure the entire CEL stack in a common interface. */ public final class CelFactory { @@ -33,7 +35,10 @@ private CelFactory() {} * evaluation are enabled by default. */ public static CelBuilder standardCelBuilder() { - return CelImpl.newBuilder(CelParserImpl.newBuilder(), CelCheckerLegacyImpl.newBuilder()) + return CelImpl.newBuilder( + CelCompilerImpl.newBuilder( + CelParserImpl.newBuilder(), CelCheckerLegacyImpl.newBuilder()), + CelRuntimeLegacyImpl.newBuilder()) .setOptions(CelOptions.current().build()) .setStandardEnvironmentEnabled(true); } diff --git a/bundle/src/main/java/dev/cel/bundle/CelImpl.java b/bundle/src/main/java/dev/cel/bundle/CelImpl.java index 20086b114..ad062d113 100644 --- a/bundle/src/main/java/dev/cel/bundle/CelImpl.java +++ b/bundle/src/main/java/dev/cel/bundle/CelImpl.java @@ -45,7 +45,6 @@ import dev.cel.common.values.CelValueProvider; import dev.cel.compiler.CelCompiler; import dev.cel.compiler.CelCompilerBuilder; -import dev.cel.compiler.CelCompilerImpl; import dev.cel.compiler.CelCompilerLibrary; import dev.cel.parser.CelMacro; import dev.cel.parser.CelParserBuilder; @@ -53,7 +52,6 @@ import dev.cel.runtime.CelEvaluationException; import dev.cel.runtime.CelRuntime; import dev.cel.runtime.CelRuntimeBuilder; -import dev.cel.runtime.CelRuntimeLegacyImpl; import dev.cel.runtime.CelRuntimeLibrary; import java.util.Arrays; import java.util.function.Function; @@ -102,6 +100,31 @@ public void accept(EnvVisitor envVisitor) { } } + @Override + public CelBuilder toCelBuilder() { + return newBuilder(toCompilerBuilder(), toRuntimeBuilder()); + } + + @Override + public CelParserBuilder toParserBuilder() { + return compiler.get().toParserBuilder(); + } + + @Override + public CelCheckerBuilder toCheckerBuilder() { + return compiler.get().toCheckerBuilder(); + } + + @Override + public CelCompilerBuilder toCompilerBuilder() { + return compiler.get().toCompilerBuilder(); + } + + @Override + public CelRuntimeBuilder toRuntimeBuilder() { + return runtime.get().toRuntimeBuilder(); + } + /** Combines a prebuilt {@link CelCompiler} and {@link CelRuntime} into {@link CelImpl}. */ static CelImpl combine(CelCompiler compiler, CelRuntime runtime) { return new CelImpl(Suppliers.memoize(() -> compiler), Suppliers.memoize(() -> runtime)); @@ -112,8 +135,9 @@ static CelImpl combine(CelCompiler compiler, CelRuntime runtime) { * *

By default, {@link CelOptions#DEFAULT} are enabled, as is the CEL standard environment. */ - static CelBuilder newBuilder(CelParserBuilder parserBuilder, CelCheckerBuilder checkerBuilder) { - return new CelImpl.Builder(parserBuilder, checkerBuilder); + static CelBuilder newBuilder( + CelCompilerBuilder compilerBuilder, CelRuntimeBuilder celRuntimeBuilder) { + return new CelImpl.Builder(compilerBuilder, celRuntimeBuilder); } /** Builder class for CelImpl instances. */ @@ -122,9 +146,9 @@ public static final class Builder implements CelBuilder { private final CelCompilerBuilder compilerBuilder; private final CelRuntimeBuilder runtimeBuilder; - private Builder(CelParserBuilder parserBuilder, CelCheckerBuilder checkerBuilder) { - this.compilerBuilder = CelCompilerImpl.newBuilder(parserBuilder, checkerBuilder); - this.runtimeBuilder = CelRuntimeLegacyImpl.newBuilder(); + private Builder(CelCompilerBuilder celCompilerBuilder, CelRuntimeBuilder celRuntimeBuilder) { + this.compilerBuilder = celCompilerBuilder; + this.runtimeBuilder = celRuntimeBuilder; } @Override diff --git a/bundle/src/test/java/dev/cel/bundle/BUILD.bazel b/bundle/src/test/java/dev/cel/bundle/BUILD.bazel index 68ecfa5b2..cccfc8eed 100644 --- a/bundle/src/test/java/dev/cel/bundle/BUILD.bazel +++ b/bundle/src/test/java/dev/cel/bundle/BUILD.bazel @@ -12,6 +12,7 @@ java_library( "//:auto_value", "//:java_truth", "//bundle:cel", + "//checker", "//checker:checker_legacy_environment", "//checker:proto_type_mask", "//common", @@ -31,6 +32,7 @@ java_library( "//common/types:type_providers", "//compiler", "//compiler:compiler_builder", + "//parser", "//parser:macro", "//runtime", "//runtime:unknown_attributes", diff --git a/bundle/src/test/java/dev/cel/bundle/CelImplTest.java b/bundle/src/test/java/dev/cel/bundle/CelImplTest.java index f844246c3..2b3cf30bb 100644 --- a/bundle/src/test/java/dev/cel/bundle/CelImplTest.java +++ b/bundle/src/test/java/dev/cel/bundle/CelImplTest.java @@ -50,6 +50,7 @@ import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import com.google.testing.junit.testparameterinjector.TestParameters; +import dev.cel.checker.CelCheckerLegacyImpl; import dev.cel.checker.DescriptorTypeProvider; import dev.cel.checker.ProtoTypeMask; import dev.cel.checker.TypeProvider; @@ -77,6 +78,8 @@ import dev.cel.common.types.StructTypeReference; import dev.cel.compiler.CelCompiler; import dev.cel.compiler.CelCompilerFactory; +import dev.cel.compiler.CelCompilerImpl; +import dev.cel.parser.CelParserImpl; import dev.cel.parser.CelStandardMacro; import dev.cel.runtime.CelAttribute; import dev.cel.runtime.CelAttribute.Qualifier; @@ -86,6 +89,7 @@ import dev.cel.runtime.CelRuntime.CelFunctionBinding; import dev.cel.runtime.CelRuntime.Program; import dev.cel.runtime.CelRuntimeFactory; +import dev.cel.runtime.CelRuntimeLegacyImpl; import dev.cel.runtime.CelUnknownSet; import dev.cel.runtime.CelVariableResolver; import dev.cel.runtime.UnknownContext; @@ -1812,6 +1816,27 @@ public boolean isAssignableFrom(CelType other) { assertThat(result).isEqualTo("5"); } + @Test + public void toBuilder_isImmutable() { + CelBuilder celBuilder = CelFactory.standardCelBuilder(); + CelImpl celImpl = (CelImpl) celBuilder.build(); + + CelImpl.Builder newCelBuilder = (CelImpl.Builder) celImpl.toCelBuilder(); + CelParserImpl.Builder newParserBuilder = (CelParserImpl.Builder) celImpl.toParserBuilder(); + CelCheckerLegacyImpl.Builder newCheckerBuilder = + (CelCheckerLegacyImpl.Builder) celImpl.toCheckerBuilder(); + CelCompilerImpl.Builder newCompilerBuilder = + (CelCompilerImpl.Builder) celImpl.toCompilerBuilder(); + CelRuntimeLegacyImpl.Builder newRuntimeBuilder = + (CelRuntimeLegacyImpl.Builder) celImpl.toRuntimeBuilder(); + + assertThat(newCelBuilder).isNotEqualTo(celBuilder); + assertThat(newParserBuilder).isNotEqualTo(celImpl.toParserBuilder()); + assertThat(newCheckerBuilder).isNotEqualTo(celImpl.toCheckerBuilder()); + assertThat(newCompilerBuilder).isNotEqualTo(celImpl.toCompilerBuilder()); + assertThat(newRuntimeBuilder).isNotEqualTo(celImpl.toRuntimeBuilder()); + } + private static TypeProvider aliasingProvider(ImmutableMap typeAliases) { return new TypeProvider() { @Override diff --git a/checker/src/main/java/dev/cel/checker/CelChecker.java b/checker/src/main/java/dev/cel/checker/CelChecker.java index 5a6e9c28b..4022212cb 100644 --- a/checker/src/main/java/dev/cel/checker/CelChecker.java +++ b/checker/src/main/java/dev/cel/checker/CelChecker.java @@ -28,4 +28,6 @@ public interface CelChecker { *

Check validates the type-agreement of the parsed {@code CelAbstractSyntaxTree}. */ CelValidationResult check(CelAbstractSyntaxTree ast); + + CelCheckerBuilder toCheckerBuilder(); } diff --git a/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java b/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java index ccc9c2955..4ed263c11 100644 --- a/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java +++ b/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java @@ -20,6 +20,7 @@ import dev.cel.expr.Decl; import dev.cel.expr.Type; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -62,8 +63,8 @@ public final class CelCheckerLegacyImpl implements CelChecker, EnvVisitable { private final CelOptions celOptions; private final String container; - private final ImmutableList identDeclarations; - private final ImmutableList functionDeclarations; + private final ImmutableSet identDeclarations; + private final ImmutableSet functionDeclarations; private final Optional expectedResultType; @SuppressWarnings("Immutable") @@ -71,6 +72,10 @@ public final class CelCheckerLegacyImpl implements CelChecker, EnvVisitable { private final boolean standardEnvironmentEnabled; + // Builder is mutable by design. APIs must make defensive copies in and out of this class. + @SuppressWarnings("Immutable") + private final Builder checkerBuilder; + @Override public CelValidationResult check(CelAbstractSyntaxTree ast) { CelSource source = ast.getSource(); @@ -88,6 +93,11 @@ public CelValidationResult check(CelAbstractSyntaxTree ast) { return new CelValidationResult(checkedAst, ImmutableList.of()); } + @Override + public CelCheckerBuilder toCheckerBuilder() { + return new Builder(checkerBuilder); + } + @Override public void accept(EnvVisitor envVisitor) { Errors errors = new Errors("", ""); @@ -132,9 +142,9 @@ public static CelCheckerBuilder newBuilder() { /** Builder class for the legacy {@code CelChecker} implementation. */ public static final class Builder implements CelCheckerBuilder { - private final ImmutableList.Builder identDeclarations; - private final ImmutableList.Builder functionDeclarations; - private final ImmutableList.Builder protoTypeMasks; + private final ImmutableSet.Builder identDeclarations; + private final ImmutableSet.Builder functionDeclarations; + private final ImmutableSet.Builder protoTypeMasks; private final ImmutableSet.Builder messageTypes; private final ImmutableSet.Builder fileTypes; private final ImmutableSet.Builder celCheckerLibraries; @@ -316,6 +326,38 @@ public CelCheckerBuilder addLibraries(Iterable libr return this; } + // The following getters exist for asserting immutability for collections held by this builder, + // and shouldn't be exposed to the public. + @VisibleForTesting + ImmutableSet.Builder getFunctionDecls() { + return this.functionDeclarations; + } + + @VisibleForTesting + ImmutableSet.Builder getIdentDecls() { + return this.identDeclarations; + } + + @VisibleForTesting + ImmutableSet.Builder getProtoTypeMasks() { + return this.protoTypeMasks; + } + + @VisibleForTesting + ImmutableSet.Builder getMessageTypes() { + return this.messageTypes; + } + + @VisibleForTesting + ImmutableSet.Builder getFileTypes() { + return this.fileTypes; + } + + @VisibleForTesting + ImmutableSet.Builder getCheckerLibraries() { + return this.celCheckerLibraries; + } + @Override @CheckReturnValue public CelCheckerLegacyImpl build() { @@ -348,13 +390,13 @@ public CelCheckerLegacyImpl build() { // Configure the declaration set, and possibly alter the type provider if ProtoDecl values // are provided as they may prevent the use of certain field selection patterns against the // proto. - ImmutableList identDeclarationSet = identDeclarations.build(); - ImmutableList protoTypeMaskSet = protoTypeMasks.build(); + ImmutableSet identDeclarationSet = identDeclarations.build(); + ImmutableSet protoTypeMaskSet = protoTypeMasks.build(); if (!protoTypeMaskSet.isEmpty()) { ProtoTypeMaskTypeProvider protoTypeMaskTypeProvider = new ProtoTypeMaskTypeProvider(messageTypeProvider, protoTypeMaskSet); identDeclarationSet = - ImmutableList.builder() + ImmutableSet.builder() .addAll(identDeclarationSet) .addAll(protoTypeMaskTypeProvider.computeDeclsFromProtoTypeMasks()) .build(); @@ -375,29 +417,55 @@ public CelCheckerLegacyImpl build() { functionDeclarations.build(), Optional.fromNullable(expectedResultType), legacyProvider, - standardEnvironmentEnabled); + standardEnvironmentEnabled, + this); } private Builder() { this.celOptions = CelOptions.newBuilder().build(); - this.identDeclarations = ImmutableList.builder(); - this.functionDeclarations = ImmutableList.builder(); + this.identDeclarations = ImmutableSet.builder(); + this.functionDeclarations = ImmutableSet.builder(); this.fileTypes = ImmutableSet.builder(); this.messageTypes = ImmutableSet.builder(); - this.protoTypeMasks = ImmutableList.builder(); + this.protoTypeMasks = ImmutableSet.builder(); this.celCheckerLibraries = ImmutableSet.builder(); this.container = ""; } + + private Builder(Builder builder) { + // The following properties are either immutable or simple primitives, thus can be assigned + // directly. + this.celOptions = builder.celOptions; + this.celTypeProvider = builder.celTypeProvider; + this.container = builder.container; + this.customTypeProvider = builder.customTypeProvider; + this.expectedResultType = builder.expectedResultType; + this.standardEnvironmentEnabled = builder.standardEnvironmentEnabled; + // The following needs to be deep copied as they are collection builders + this.functionDeclarations = deepCopy(builder.functionDeclarations); + this.identDeclarations = deepCopy(builder.identDeclarations); + this.fileTypes = deepCopy(builder.fileTypes); + this.messageTypes = deepCopy(builder.messageTypes); + this.protoTypeMasks = deepCopy(builder.protoTypeMasks); + this.celCheckerLibraries = deepCopy(builder.celCheckerLibraries); + } + + private static ImmutableSet.Builder deepCopy(ImmutableSet.Builder builderToCopy) { + ImmutableSet.Builder newBuilder = ImmutableSet.builder(); + newBuilder.addAll(builderToCopy.build()); + return newBuilder; + } } private CelCheckerLegacyImpl( CelOptions celOptions, String container, - ImmutableList identDeclarations, - ImmutableList functionDeclarations, + ImmutableSet identDeclarations, + ImmutableSet functionDeclarations, Optional expectedResultType, TypeProvider typeProvider, - boolean standardEnvironmentEnabled) { + boolean standardEnvironmentEnabled, + Builder checkerBuilder) { this.celOptions = celOptions; this.container = container; this.identDeclarations = identDeclarations; @@ -405,6 +473,7 @@ private CelCheckerLegacyImpl( this.expectedResultType = expectedResultType; this.typeProvider = typeProvider; this.standardEnvironmentEnabled = standardEnvironmentEnabled; + this.checkerBuilder = new Builder(checkerBuilder); } private static ImmutableList errorsToIssues(Errors errors) { diff --git a/checker/src/main/java/dev/cel/checker/Env.java b/checker/src/main/java/dev/cel/checker/Env.java index 17d563dc8..8ce80f488 100644 --- a/checker/src/main/java/dev/cel/checker/Env.java +++ b/checker/src/main/java/dev/cel/checker/Env.java @@ -958,7 +958,7 @@ private static CelFunctionDecl sanitizeFunction(CelFunctionDecl func) { } CelFunctionDecl.Builder funcBuilder = func.toBuilder(); - ImmutableList.Builder overloadsBuilder = new ImmutableList.Builder<>(); + ImmutableSet.Builder overloadsBuilder = new ImmutableSet.Builder<>(); for (CelOverloadDecl overloadDecl : funcBuilder.overloads()) { CelOverloadDecl.Builder overloadBuilder = overloadDecl.toBuilder(); CelType resultType = overloadBuilder.build().resultType(); @@ -966,7 +966,7 @@ private static CelFunctionDecl sanitizeFunction(CelFunctionDecl func) { overloadBuilder.setResultType(getWellKnownType(resultType)); } - ImmutableList.Builder parameterTypeBuilder = ImmutableList.builder(); + ImmutableSet.Builder parameterTypeBuilder = ImmutableSet.builder(); for (CelType paramType : overloadBuilder.parameterTypes()) { if (isWellKnownType(paramType)) { parameterTypeBuilder.add(getWellKnownType(paramType)); diff --git a/checker/src/main/java/dev/cel/checker/ProtoTypeMaskTypeProvider.java b/checker/src/main/java/dev/cel/checker/ProtoTypeMaskTypeProvider.java index 07325db00..61511c38d 100644 --- a/checker/src/main/java/dev/cel/checker/ProtoTypeMaskTypeProvider.java +++ b/checker/src/main/java/dev/cel/checker/ProtoTypeMaskTypeProvider.java @@ -44,10 +44,10 @@ public final class ProtoTypeMaskTypeProvider implements CelTypeProvider { @SuppressWarnings("Immutable") private final ImmutableMap allTypes; - private final ImmutableList protoTypeMasks; + private final ImmutableSet protoTypeMasks; ProtoTypeMaskTypeProvider( - CelTypeProvider delegateProvider, ImmutableList protoTypeMasks) { + CelTypeProvider delegateProvider, ImmutableSet protoTypeMasks) { this.protoTypeMasks = protoTypeMasks; this.allTypes = computeVisibleFieldsMap(delegateProvider, protoTypeMasks); } @@ -89,7 +89,7 @@ ImmutableList computeDeclsFromProtoTypeMasks() { } private static ImmutableMap computeVisibleFieldsMap( - CelTypeProvider delegateProvider, ImmutableList protoTypeMasks) { + CelTypeProvider delegateProvider, ImmutableSet protoTypeMasks) { Map> fieldMap = new HashMap<>(); for (ProtoTypeMask typeMask : protoTypeMasks) { Optional rootType = delegateProvider.findType(typeMask.getTypeName()); diff --git a/checker/src/test/java/dev/cel/checker/BUILD.bazel b/checker/src/test/java/dev/cel/checker/BUILD.bazel index 1498a6b9b..338dcbb10 100644 --- a/checker/src/test/java/dev/cel/checker/BUILD.bazel +++ b/checker/src/test/java/dev/cel/checker/BUILD.bazel @@ -14,6 +14,7 @@ java_library( "//:auto_value", "//checker", "//checker:cel_ident_decl", + "//checker:checker_builder", "//checker:checker_legacy_environment", "//checker:proto_expr_visitor", "//checker:proto_type_mask", diff --git a/checker/src/test/java/dev/cel/checker/CelCheckerLegacyImplTest.java b/checker/src/test/java/dev/cel/checker/CelCheckerLegacyImplTest.java new file mode 100644 index 000000000..1f96367f3 --- /dev/null +++ b/checker/src/test/java/dev/cel/checker/CelCheckerLegacyImplTest.java @@ -0,0 +1,106 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dev.cel.checker; + +import static com.google.common.truth.Truth.assertThat; + +import dev.cel.common.CelFunctionDecl; +import dev.cel.common.CelOverloadDecl; +import dev.cel.common.CelVarDecl; +import dev.cel.common.types.SimpleType; +import dev.cel.compiler.CelCompilerFactory; +import dev.cel.testing.testdata.proto2.TestAllTypesProto.TestAllTypes; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class CelCheckerLegacyImplTest { + + @Test + public void toCheckerBuilder_isNewInstance() { + CelCheckerBuilder celCheckerBuilder = CelCompilerFactory.standardCelCheckerBuilder(); + CelCheckerLegacyImpl celChecker = (CelCheckerLegacyImpl) celCheckerBuilder.build(); + + CelCheckerLegacyImpl.Builder newCheckerBuilder = + (CelCheckerLegacyImpl.Builder) celChecker.toCheckerBuilder(); + + assertThat(newCheckerBuilder).isNotEqualTo(celCheckerBuilder); + } + + @Test + public void toCheckerBuilder_isImmutable() { + CelCheckerBuilder originalCheckerBuilder = CelCompilerFactory.standardCelCheckerBuilder(); + CelCheckerLegacyImpl celChecker = (CelCheckerLegacyImpl) originalCheckerBuilder.build(); + originalCheckerBuilder.addLibraries(new CelCheckerLibrary() {}); + + CelCheckerLegacyImpl.Builder newCheckerBuilder = + (CelCheckerLegacyImpl.Builder) celChecker.toCheckerBuilder(); + + assertThat(newCheckerBuilder.getCheckerLibraries().build()).isEmpty(); + } + + @Test + public void toCheckerBuilder_collectionProperties_copied() { + CelCheckerBuilder celCheckerBuilder = + CelCompilerFactory.standardCelCheckerBuilder() + .addFunctionDeclarations( + CelFunctionDecl.newFunctionDeclaration( + "test", CelOverloadDecl.newGlobalOverload("test_id", SimpleType.INT))) + .addVarDeclarations(CelVarDecl.newVarDeclaration("ident", SimpleType.INT)) + .addMessageTypes(TestAllTypes.getDescriptor()) + .addFileTypes(TestAllTypes.getDescriptor().getFile()) + .addProtoTypeMasks( + ProtoTypeMask.ofAllFields("dev.cel.testing.testdata.proto2.TestAllTypes")) + .addLibraries(new CelCheckerLibrary() {}); + CelCheckerLegacyImpl celChecker = (CelCheckerLegacyImpl) celCheckerBuilder.build(); + + CelCheckerLegacyImpl.Builder newCheckerBuilder = + (CelCheckerLegacyImpl.Builder) celChecker.toCheckerBuilder(); + + assertThat(newCheckerBuilder.getFunctionDecls().build()).hasSize(1); + assertThat(newCheckerBuilder.getIdentDecls().build()).hasSize(1); + assertThat(newCheckerBuilder.getProtoTypeMasks().build()).hasSize(1); + assertThat(newCheckerBuilder.getMessageTypes().build()).hasSize(1); + assertThat(newCheckerBuilder.getFileTypes().build()).hasSize(1); + assertThat(newCheckerBuilder.getCheckerLibraries().build()).hasSize(1); + } + + @Test + public void toCheckerBuilder_collectionProperties_areImmutable() { + CelCheckerBuilder celCheckerBuilder = CelCompilerFactory.standardCelCheckerBuilder(); + CelCheckerLegacyImpl celChecker = (CelCheckerLegacyImpl) celCheckerBuilder.build(); + CelCheckerLegacyImpl.Builder newCheckerBuilder = + (CelCheckerLegacyImpl.Builder) celChecker.toCheckerBuilder(); + + // Mutate the original builder containing collections + celCheckerBuilder.addFunctionDeclarations( + CelFunctionDecl.newFunctionDeclaration( + "test", CelOverloadDecl.newGlobalOverload("test_id", SimpleType.INT))); + celCheckerBuilder.addVarDeclarations(CelVarDecl.newVarDeclaration("ident", SimpleType.INT)); + celCheckerBuilder.addMessageTypes(TestAllTypes.getDescriptor()); + celCheckerBuilder.addFileTypes(TestAllTypes.getDescriptor().getFile()); + celCheckerBuilder.addProtoTypeMasks( + ProtoTypeMask.ofAllFields("dev.cel.testing.testdata.proto2.TestAllTypes")); + celCheckerBuilder.addLibraries(new CelCheckerLibrary() {}); + + assertThat(newCheckerBuilder.getFunctionDecls().build()).isEmpty(); + assertThat(newCheckerBuilder.getIdentDecls().build()).isEmpty(); + assertThat(newCheckerBuilder.getProtoTypeMasks().build()).isEmpty(); + assertThat(newCheckerBuilder.getMessageTypes().build()).isEmpty(); + assertThat(newCheckerBuilder.getFileTypes().build()).isEmpty(); + assertThat(newCheckerBuilder.getCheckerLibraries().build()).isEmpty(); + } +} diff --git a/checker/src/test/java/dev/cel/checker/CelCompilerImplTest.java b/checker/src/test/java/dev/cel/checker/CelCompilerImplTest.java new file mode 100644 index 000000000..b3abc4c6b --- /dev/null +++ b/checker/src/test/java/dev/cel/checker/CelCompilerImplTest.java @@ -0,0 +1,45 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dev.cel.checker; + +import static com.google.common.truth.Truth.assertThat; + +import dev.cel.common.CelFunctionDecl; +import dev.cel.common.CelOverloadDecl; +import dev.cel.common.types.SimpleType; +import dev.cel.compiler.CelCompilerBuilder; +import dev.cel.compiler.CelCompilerFactory; +import dev.cel.compiler.CelCompilerImpl; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class CelCompilerImplTest { + + @Test + public void toCompilerBuilder_isImmutable() { + CelCompilerBuilder celCompilerBuilder = CelCompilerFactory.standardCelCompilerBuilder(); + CelCompilerImpl celCompiler = (CelCompilerImpl) celCompilerBuilder.build(); + celCompilerBuilder.addFunctionDeclarations( + CelFunctionDecl.newFunctionDeclaration( + "test", CelOverloadDecl.newGlobalOverload("test_id", SimpleType.INT))); + + CelCompilerImpl.Builder newCompilerBuilder = + (CelCompilerImpl.Builder) celCompiler.toCompilerBuilder(); + + assertThat(newCompilerBuilder).isNotEqualTo(celCompilerBuilder); + } +} diff --git a/checker/src/test/java/dev/cel/checker/CelFunctionDeclTest.java b/checker/src/test/java/dev/cel/checker/CelFunctionDeclTest.java index fe386d8c7..662761f41 100644 --- a/checker/src/test/java/dev/cel/checker/CelFunctionDeclTest.java +++ b/checker/src/test/java/dev/cel/checker/CelFunctionDeclTest.java @@ -22,6 +22,7 @@ import dev.cel.common.CelFunctionDecl; import dev.cel.common.CelOverloadDecl; import dev.cel.common.types.SimpleType; +import java.util.Iterator; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -38,7 +39,7 @@ public void declareGlobalFunction_success() { assertThat(functionDecl.name()).isEqualTo("testGlobalFunction"); assertThat(functionDecl.overloads()).hasSize(1); - CelOverloadDecl overloadDecl = functionDecl.overloads().get(0); + CelOverloadDecl overloadDecl = functionDecl.overloads().iterator().next(); assertThat(overloadDecl.overloadId()).isEqualTo("overloadId"); assertThat(overloadDecl.isInstanceFunction()).isFalse(); assertThat(overloadDecl.resultType()).isEqualTo(SimpleType.BOOL); @@ -54,7 +55,7 @@ public void declareMemberFunction_success() { assertThat(functionDecl.name()).isEqualTo("testMemberFunction"); assertThat(functionDecl.overloads()).hasSize(1); - CelOverloadDecl overloadDecl = functionDecl.overloads().get(0); + CelOverloadDecl overloadDecl = functionDecl.overloads().iterator().next(); assertThat(overloadDecl.overloadId()).isEqualTo("overloadId"); assertThat(overloadDecl.isInstanceFunction()).isTrue(); assertThat(overloadDecl.resultType()).isEqualTo(SimpleType.TIMESTAMP); @@ -74,13 +75,14 @@ public void declareFunction_withBuilder_success() { assertThat(functionDecl.name()).isEqualTo("testFunction"); assertThat(functionDecl.overloads()).hasSize(2); - CelOverloadDecl memberOverloadDecl = functionDecl.overloads().get(0); + Iterator iterator = functionDecl.overloads().iterator(); + CelOverloadDecl memberOverloadDecl = iterator.next(); assertThat(memberOverloadDecl.overloadId()).isEqualTo("memberOverloadId"); assertThat(memberOverloadDecl.isInstanceFunction()).isTrue(); assertThat(memberOverloadDecl.resultType()).isEqualTo(SimpleType.INT); assertThat(memberOverloadDecl.parameterTypes()).containsExactly(SimpleType.UINT); - CelOverloadDecl globalOverloadDecl = functionDecl.overloads().get(1); + CelOverloadDecl globalOverloadDecl = iterator.next(); assertThat(globalOverloadDecl.overloadId()).isEqualTo("globalOverloadId"); assertThat(globalOverloadDecl.isInstanceFunction()).isFalse(); assertThat(globalOverloadDecl.resultType()).isEqualTo(SimpleType.STRING); diff --git a/checker/src/test/java/dev/cel/checker/ProtoTypeMaskTypeProviderTest.java b/checker/src/test/java/dev/cel/checker/ProtoTypeMaskTypeProviderTest.java index 31d0fb221..bf41c1e20 100644 --- a/checker/src/test/java/dev/cel/checker/ProtoTypeMaskTypeProviderTest.java +++ b/checker/src/test/java/dev/cel/checker/ProtoTypeMaskTypeProviderTest.java @@ -19,7 +19,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.protobuf.FieldMask; import com.google.rpc.context.AttributeContext; @@ -52,7 +51,7 @@ public void protoTypeMaskProvider_badFieldMask() { () -> new ProtoTypeMaskTypeProvider( celTypeProvider, - ImmutableList.of( + ImmutableSet.of( ProtoTypeMask.of( "google.rpc.context.AttributeContext", FieldMask.newBuilder().addPaths("missing").build())))); @@ -62,16 +61,16 @@ public void protoTypeMaskProvider_badFieldMask() { public void lookupFieldNames_undeclaredMessageType() { CelTypeProvider celTypeProvider = new ProtoMessageTypeProvider(); ProtoTypeMaskTypeProvider protoTypeMaskProvider = - new ProtoTypeMaskTypeProvider(celTypeProvider, ImmutableList.of()); + new ProtoTypeMaskTypeProvider(celTypeProvider, ImmutableSet.of()); assertThat(protoTypeMaskProvider.findType(ATTRIBUTE_CONTEXT_TYPE)).isEmpty(); } @Test public void lookupFieldNames_noProtoDecls() { CelTypeProvider celTypeProvider = - new ProtoMessageTypeProvider(ImmutableList.of(AttributeContext.getDescriptor())); + new ProtoMessageTypeProvider(ImmutableSet.of(AttributeContext.getDescriptor())); ProtoTypeMaskTypeProvider protoTypeMaskProvider = - new ProtoTypeMaskTypeProvider(celTypeProvider, ImmutableList.of()); + new ProtoTypeMaskTypeProvider(celTypeProvider, ImmutableSet.of()); ProtoMessageType protoType = assertTypeFound(protoTypeMaskProvider, ATTRIBUTE_CONTEXT_TYPE); assertThat(protoType.fields().stream().map(f -> f.name()).collect(toImmutableList())) .containsExactly( @@ -91,10 +90,10 @@ public void lookupFieldNames_noProtoDecls() { @Test public void lookupFieldNames_fullProtoDecl() { CelTypeProvider celTypeProvider = - new ProtoMessageTypeProvider(ImmutableList.of(AttributeContext.getDescriptor())); + new ProtoMessageTypeProvider(ImmutableSet.of(AttributeContext.getDescriptor())); ProtoTypeMaskTypeProvider protoTypeMaskProvider = new ProtoTypeMaskTypeProvider( - celTypeProvider, ImmutableList.of(ProtoTypeMask.ofAllFields(ATTRIBUTE_CONTEXT_TYPE))); + celTypeProvider, ImmutableSet.of(ProtoTypeMask.ofAllFields(ATTRIBUTE_CONTEXT_TYPE))); ProtoMessageType protoType = assertTypeFound(protoTypeMaskProvider, ATTRIBUTE_CONTEXT_TYPE); assertTypeHasFields( protoType, @@ -114,11 +113,11 @@ public void lookupFieldNames_fullProtoDecl() { @Test public void lookupFieldNames_partialProtoDecl() { CelTypeProvider celTypeProvider = - new ProtoMessageTypeProvider(ImmutableList.of(AttributeContext.getDescriptor())); + new ProtoMessageTypeProvider(ImmutableSet.of(AttributeContext.getDescriptor())); ProtoTypeMaskTypeProvider protoTypeMaskProvider = new ProtoTypeMaskTypeProvider( celTypeProvider, - ImmutableList.of( + ImmutableSet.of( ProtoTypeMask.of( "google.rpc.context.AttributeContext", FieldMask.newBuilder() @@ -169,7 +168,7 @@ public void computeDecls() { ProtoTypeMaskTypeProvider protoTypeMaskProvider = new ProtoTypeMaskTypeProvider( celTypeProvider, - ImmutableList.of( + ImmutableSet.of( ProtoTypeMask.of( "google.rpc.context.AttributeContext", FieldMask.newBuilder() @@ -191,11 +190,11 @@ public void computeDecls() { @Test public void lookupFieldType() { CelTypeProvider celTypeProvider = - new ProtoMessageTypeProvider(ImmutableList.of(AttributeContext.getDescriptor())); + new ProtoMessageTypeProvider(ImmutableSet.of(AttributeContext.getDescriptor())); ProtoTypeMaskTypeProvider protoTypeMaskProvider = new ProtoTypeMaskTypeProvider( celTypeProvider, - ImmutableList.of( + ImmutableSet.of( ProtoTypeMask.of( "google.rpc.context.AttributeContext", FieldMask.newBuilder() @@ -228,11 +227,11 @@ public void lookupFieldType() { @Test public void lookupFieldType_notExposedField() { CelTypeProvider celTypeProvider = - new ProtoMessageTypeProvider(ImmutableList.of(AttributeContext.getDescriptor())); + new ProtoMessageTypeProvider(ImmutableSet.of(AttributeContext.getDescriptor())); ProtoTypeMaskTypeProvider protoTypeMaskProvider = new ProtoTypeMaskTypeProvider( celTypeProvider, - ImmutableList.of( + ImmutableSet.of( ProtoTypeMask.of( "google.rpc.context.AttributeContext", FieldMask.newBuilder().addPaths("resource.name").build()))); @@ -243,11 +242,11 @@ public void lookupFieldType_notExposedField() { @Test public void lookupType_notExposed() { CelTypeProvider celTypeProvider = - new ProtoMessageTypeProvider(ImmutableList.of(AttributeContext.getDescriptor())); + new ProtoMessageTypeProvider(ImmutableSet.of(AttributeContext.getDescriptor())); ProtoTypeMaskTypeProvider protoTypeMaskProvider = new ProtoTypeMaskTypeProvider( celTypeProvider, - ImmutableList.of( + ImmutableSet.of( ProtoTypeMask.of( "google.rpc.context.AttributeContext", FieldMask.newBuilder().addPaths("resource.name").build()))); diff --git a/common/src/main/java/dev/cel/common/CelFunctionDecl.java b/common/src/main/java/dev/cel/common/CelFunctionDecl.java index 68b5ff406..12beb53d7 100644 --- a/common/src/main/java/dev/cel/common/CelFunctionDecl.java +++ b/common/src/main/java/dev/cel/common/CelFunctionDecl.java @@ -20,7 +20,7 @@ import dev.cel.expr.Decl; import dev.cel.expr.Decl.FunctionDecl; import com.google.auto.value.AutoValue; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.CheckReturnValue; import com.google.errorprone.annotations.Immutable; @@ -36,7 +36,7 @@ public abstract class CelFunctionDecl { public abstract String name(); /** Required. List of function overloads. Must contain at least one overload. */ - public abstract ImmutableList overloads(); + public abstract ImmutableSet overloads(); /** Builder for configuring the {@link CelFunctionDecl}. */ @AutoValue.Builder @@ -46,12 +46,12 @@ public abstract static class Builder { /** Sets the function name {@link #name()} */ public abstract Builder setName(String name); - public abstract ImmutableList overloads(); + public abstract ImmutableSet overloads(); - public abstract ImmutableList.Builder overloadsBuilder(); + public abstract ImmutableSet.Builder overloadsBuilder(); @CanIgnoreReturnValue - public abstract Builder setOverloads(ImmutableList overloads); + public abstract Builder setOverloads(ImmutableSet overloads); /** Adds one or more function overloads */ @CanIgnoreReturnValue @@ -77,7 +77,7 @@ public Builder addOverloads(Iterable overloads) { /** Create a new builder to construct a {@code CelFunctionDecl} instance. */ public static Builder newBuilder() { - return new AutoValue_CelFunctionDecl.Builder().setOverloads(ImmutableList.of()); + return new AutoValue_CelFunctionDecl.Builder().setOverloads(ImmutableSet.of()); } /** Constructs a function declaration with any number of {@link CelOverloadDecl} */ diff --git a/common/src/main/java/dev/cel/common/CelOptions.java b/common/src/main/java/dev/cel/common/CelOptions.java index f4082f918..eaa812e2b 100644 --- a/common/src/main/java/dev/cel/common/CelOptions.java +++ b/common/src/main/java/dev/cel/common/CelOptions.java @@ -89,7 +89,6 @@ public abstract class CelOptions { public abstract boolean enableCelValue(); - public abstract int comprehensionMaxIterations(); public abstract Builder toBuilder(); diff --git a/common/src/main/java/dev/cel/common/CelOverloadDecl.java b/common/src/main/java/dev/cel/common/CelOverloadDecl.java index 247f86bc1..30bcac5a8 100644 --- a/common/src/main/java/dev/cel/common/CelOverloadDecl.java +++ b/common/src/main/java/dev/cel/common/CelOverloadDecl.java @@ -93,7 +93,7 @@ public abstract static class Builder { * Sets the parameter types {@link #parameterTypes()}. Note that this will override any * parameter types added via the accumulator methods {@link #addParameterTypes}. */ - public abstract Builder setParameterTypes(ImmutableList value); + public abstract Builder setParameterTypes(ImmutableSet value); public abstract CelType resultType(); diff --git a/compiler/src/main/java/dev/cel/compiler/CelCompiler.java b/compiler/src/main/java/dev/cel/compiler/CelCompiler.java index dd4d5dd10..602646ab4 100644 --- a/compiler/src/main/java/dev/cel/compiler/CelCompiler.java +++ b/compiler/src/main/java/dev/cel/compiler/CelCompiler.java @@ -57,4 +57,6 @@ default CelValidationResult compile(String expression, String description) { throw new IllegalStateException("this method must only be called when !hasError()", ex); } } + + CelCompilerBuilder toCompilerBuilder(); } diff --git a/compiler/src/main/java/dev/cel/compiler/CelCompilerImpl.java b/compiler/src/main/java/dev/cel/compiler/CelCompilerImpl.java index 5a99054a6..ac81f5d0c 100644 --- a/compiler/src/main/java/dev/cel/compiler/CelCompilerImpl.java +++ b/compiler/src/main/java/dev/cel/compiler/CelCompilerImpl.java @@ -81,6 +81,21 @@ public void accept(EnvVisitor envVisitor) { } } + @Override + public CelCompilerBuilder toCompilerBuilder() { + return newBuilder(toParserBuilder(), toCheckerBuilder()); + } + + @Override + public CelCheckerBuilder toCheckerBuilder() { + return checker.toCheckerBuilder(); + } + + @Override + public CelParserBuilder toParserBuilder() { + return parser.toParserBuilder(); + } + /** Combines a prebuilt {@link CelParser} and {@link CelChecker} into {@link CelCompilerImpl}. */ static CelCompilerImpl combine(CelParser parser, CelChecker checker) { return new CelCompilerImpl(parser, checker); diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel b/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel index 10096a73d..77b3d289b 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel @@ -42,8 +42,11 @@ java_library( "//bundle:cel", "//checker:checker_legacy_environment", "//common", + "//common:compiler_common", "//common/ast", "//common/navigation", + "//common/types", + "//common/types:type_providers", "//optimizer:ast_optimizer", "//optimizer:mutable_ast", "//parser:operator", diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/SubexpressionOptimizer.java b/optimizer/src/main/java/dev/cel/optimizer/optimizers/SubexpressionOptimizer.java index 8d057c74a..e36a0f8d9 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/SubexpressionOptimizer.java +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/SubexpressionOptimizer.java @@ -19,12 +19,15 @@ import static java.util.Arrays.stream; import com.google.auto.value.AutoValue; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Streams; import dev.cel.bundle.Cel; import dev.cel.checker.Standard; import dev.cel.common.CelAbstractSyntaxTree; +import dev.cel.common.CelFunctionDecl; +import dev.cel.common.CelOverloadDecl; import dev.cel.common.CelSource; import dev.cel.common.ast.CelExpr; import dev.cel.common.ast.CelExpr.CelIdent; @@ -32,6 +35,9 @@ import dev.cel.common.navigation.CelNavigableAst; import dev.cel.common.navigation.CelNavigableExpr; import dev.cel.common.navigation.CelNavigableExpr.TraversalOrder; +import dev.cel.common.types.CelType; +import dev.cel.common.types.ListType; +import dev.cel.common.types.SimpleType; import dev.cel.optimizer.CelAstOptimizer; import dev.cel.optimizer.MutableAst; import dev.cel.parser.Operator; @@ -64,6 +70,8 @@ public class SubexpressionOptimizer implements CelAstOptimizer { new SubexpressionOptimizer(SubexpressionOptimizerOptions.newBuilder().build()); private static final String BIND_IDENTIFIER_PREFIX = "@r"; private static final String MANGLED_COMPREHENSION_IDENTIFIER_PREFIX = "@c"; + private static final String CEL_BLOCK_FUNCTION = "cel.@block"; + private static final String BLOCK_INDEX_PREFIX = "@index"; private static final ImmutableSet CSE_ALLOWED_FUNCTIONS = Streams.concat( stream(Operator.values()).map(Operator::getFunction), @@ -325,6 +333,14 @@ private CelExpr normalizeForEquality(CelExpr celExpr) { return mutableAst.clearExprIds(celExpr); } + @VisibleForTesting + static CelFunctionDecl newCelBlockFunctionDecl(CelType resultType) { + return CelFunctionDecl.newFunctionDeclaration( + CEL_BLOCK_FUNCTION, + CelOverloadDecl.newGlobalOverload( + "cel_block_list", resultType, ListType.create(SimpleType.DYN), resultType)); + } + /** Options to configure how Common Subexpression Elimination behave. */ @AutoValue public abstract static class SubexpressionOptimizerOptions { diff --git a/optimizer/src/test/java/dev/cel/optimizer/optimizers/BUILD.bazel b/optimizer/src/test/java/dev/cel/optimizer/optimizers/BUILD.bazel index a4f8143f8..c907ef1f7 100644 --- a/optimizer/src/test/java/dev/cel/optimizer/optimizers/BUILD.bazel +++ b/optimizer/src/test/java/dev/cel/optimizer/optimizers/BUILD.bazel @@ -13,11 +13,13 @@ java_library( "//common:compiler_common", "//common:options", "//common/ast", + "//common/navigation", "//common/resources/testdata/proto3:test_all_types_java_proto", "//common/types", "//extensions", "//extensions:optional_library", "//optimizer", + "//optimizer:mutable_ast", "//optimizer:optimization_exception", "//optimizer:optimizer_builder", "//optimizer/optimizers:common_subexpression_elimination", @@ -25,6 +27,7 @@ java_library( "//parser:macro", "//parser:operator", "//parser:unparser", + "//runtime", "@maven//:com_google_guava_guava", "@maven//:com_google_testparameterinjector_test_parameter_injector", "@maven//:junit_junit", diff --git a/optimizer/src/test/java/dev/cel/optimizer/optimizers/SubexpressionOptimizerTest.java b/optimizer/src/test/java/dev/cel/optimizer/optimizers/SubexpressionOptimizerTest.java index 9b516bdd5..70b12f536 100644 --- a/optimizer/src/test/java/dev/cel/optimizer/optimizers/SubexpressionOptimizerTest.java +++ b/optimizer/src/test/java/dev/cel/optimizer/optimizers/SubexpressionOptimizerTest.java @@ -29,8 +29,15 @@ import dev.cel.common.CelAbstractSyntaxTree; import dev.cel.common.CelFunctionDecl; import dev.cel.common.CelOptions; +import dev.cel.common.CelOverloadDecl; +import dev.cel.common.CelValidationException; +import dev.cel.common.CelVarDecl; import dev.cel.common.ast.CelConstant; import dev.cel.common.ast.CelExpr; +import dev.cel.common.ast.CelExpr.ExprKind.Kind; +import dev.cel.common.navigation.CelNavigableAst; +import dev.cel.common.navigation.CelNavigableExpr; +import dev.cel.common.types.ListType; import dev.cel.common.types.OptionalType; import dev.cel.common.types.SimpleType; import dev.cel.common.types.StructTypeReference; @@ -39,14 +46,19 @@ import dev.cel.optimizer.CelOptimizationException; import dev.cel.optimizer.CelOptimizer; import dev.cel.optimizer.CelOptimizerFactory; +import dev.cel.optimizer.MutableAst; import dev.cel.optimizer.optimizers.SubexpressionOptimizer.SubexpressionOptimizerOptions; import dev.cel.parser.CelStandardMacro; import dev.cel.parser.CelUnparser; import dev.cel.parser.CelUnparserFactory; import dev.cel.parser.Operator; +import dev.cel.runtime.CelRuntime; +import dev.cel.runtime.CelRuntime.CelFunctionBinding; +import dev.cel.runtime.CelRuntimeFactory; import dev.cel.testing.testdata.proto3.TestAllTypesProto.NestedTestAllTypes; import dev.cel.testing.testdata.proto3.TestAllTypesProto.TestAllTypes; import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.Test; import org.junit.runner.RunWith; @@ -55,6 +67,37 @@ public class SubexpressionOptimizerTest { private static final Cel CEL = newCelBuilder().build(); + private static final Cel CEL_FOR_EVALUATING_BLOCK = + CelFactory.standardCelBuilder() + .setStandardMacros(CelStandardMacro.STANDARD_MACROS) + .addFunctionDeclarations( + // These are test only declarations, as the actual function is made internal using @ + // symbol. + // If the main function declaration needs updating, be sure to update the test + // declaration as well. + CelFunctionDecl.newFunctionDeclaration( + "cel.block", + CelOverloadDecl.newGlobalOverload( + "block_test_only_overload", + SimpleType.DYN, + ListType.create(SimpleType.DYN), + SimpleType.DYN)), + SubexpressionOptimizer.newCelBlockFunctionDecl(SimpleType.DYN), + CelFunctionDecl.newFunctionDeclaration( + "get_true", + CelOverloadDecl.newGlobalOverload("get_true_overload", SimpleType.BOOL))) + // Similarly, this is a test only decl (index0 -> @index0) + .addVarDeclarations( + CelVarDecl.newVarDeclaration("index0", SimpleType.DYN), + CelVarDecl.newVarDeclaration("index1", SimpleType.DYN), + CelVarDecl.newVarDeclaration("index2", SimpleType.DYN), + CelVarDecl.newVarDeclaration("@index0", SimpleType.DYN), + CelVarDecl.newVarDeclaration("@index1", SimpleType.DYN), + CelVarDecl.newVarDeclaration("@index2", SimpleType.DYN)) + .addMessageTypes(TestAllTypes.getDescriptor()) + .addVar("msg", StructTypeReference.create(TestAllTypes.getDescriptor().getFullName())) + .build(); + private static final CelOptimizer CEL_OPTIMIZER = CelOptimizerFactory.standardCelOptimizerBuilder(CEL) .addAstOptimizers( @@ -659,4 +702,213 @@ public void iterationLimitReached_throws() throws Exception { .optimize(ast)); assertThat(e).hasMessageThat().isEqualTo("Optimization failure: Max iteration count reached."); } + + private enum BlockTestCase { + BOOL_LITERAL("cel.block([true, false], index0 || index1)"), + STRING_CONCAT("cel.block(['a' + 'b', index0 + 'c'], index1 + 'd') == 'abcd'"), + + BLOCK_WITH_EXISTS_TRUE("cel.block([[1, 2, 3], [3, 4, 5].exists(e, e in index0)], index1)"), + BLOCK_WITH_EXISTS_FALSE("cel.block([[1, 2, 3], ![4, 5].exists(e, e in index0)], index1)"), + ; + + private final String source; + + BlockTestCase(String source) { + this.source = source; + } + } + + @Test + public void block_success(@TestParameter BlockTestCase testCase) throws Exception { + CelAbstractSyntaxTree ast = compileUsingInternalFunctions(testCase.source); + + Object evaluatedResult = CEL_FOR_EVALUATING_BLOCK.createProgram(ast).eval(); + + assertThat(evaluatedResult).isNotNull(); + } + + @Test + @SuppressWarnings("Immutable") // Test only + public void lazyEval_blockIndexNeverReferenced() throws Exception { + AtomicInteger invocation = new AtomicInteger(); + CelRuntime celRuntime = + CelRuntimeFactory.standardCelRuntimeBuilder() + .addMessageTypes(TestAllTypes.getDescriptor()) + .addFunctionBindings( + CelFunctionBinding.from( + "get_true_overload", + ImmutableList.of(), + arg -> { + invocation.getAndIncrement(); + return true; + })) + .build(); + CelAbstractSyntaxTree ast = + compileUsingInternalFunctions( + "cel.block([get_true()], has(msg.single_int64) ? index0 : false)"); + + boolean result = + (boolean) + celRuntime + .createProgram(ast) + .eval(ImmutableMap.of("msg", TestAllTypes.getDefaultInstance())); + + assertThat(result).isFalse(); + assertThat(invocation.get()).isEqualTo(0); + } + + @Test + @SuppressWarnings("Immutable") // Test only + public void lazyEval_blockIndexEvaluatedOnlyOnce() throws Exception { + AtomicInteger invocation = new AtomicInteger(); + CelRuntime celRuntime = + CelRuntimeFactory.standardCelRuntimeBuilder() + .addMessageTypes(TestAllTypes.getDescriptor()) + .addFunctionBindings( + CelFunctionBinding.from( + "get_true_overload", + ImmutableList.of(), + arg -> { + invocation.getAndIncrement(); + return true; + })) + .build(); + CelAbstractSyntaxTree ast = + compileUsingInternalFunctions("cel.block([get_true()], index0 && index0 && index0)"); + + boolean result = (boolean) celRuntime.createProgram(ast).eval(); + + assertThat(result).isTrue(); + assertThat(invocation.get()).isEqualTo(1); + } + + @Test + @SuppressWarnings("Immutable") // Test only + public void lazyEval_multipleBlockIndices_inResultExpr() throws Exception { + AtomicInteger invocation = new AtomicInteger(); + CelRuntime celRuntime = + CelRuntimeFactory.standardCelRuntimeBuilder() + .addMessageTypes(TestAllTypes.getDescriptor()) + .addFunctionBindings( + CelFunctionBinding.from( + "get_true_overload", + ImmutableList.of(), + arg -> { + invocation.getAndIncrement(); + return true; + })) + .build(); + CelAbstractSyntaxTree ast = + compileUsingInternalFunctions( + "cel.block([get_true(), get_true(), get_true()], index0 && index0 && index1 && index1" + + " && index2 && index2)"); + + boolean result = (boolean) celRuntime.createProgram(ast).eval(); + + assertThat(result).isTrue(); + assertThat(invocation.get()).isEqualTo(3); + } + + @Test + @SuppressWarnings("Immutable") // Test only + public void lazyEval_multipleBlockIndices_cascaded() throws Exception { + AtomicInteger invocation = new AtomicInteger(); + CelRuntime celRuntime = + CelRuntimeFactory.standardCelRuntimeBuilder() + .addMessageTypes(TestAllTypes.getDescriptor()) + .addFunctionBindings( + CelFunctionBinding.from( + "get_true_overload", + ImmutableList.of(), + arg -> { + invocation.getAndIncrement(); + return true; + })) + .build(); + CelAbstractSyntaxTree ast = + compileUsingInternalFunctions("cel.block([get_true(), index0, index1], index2)"); + + boolean result = (boolean) celRuntime.createProgram(ast).eval(); + + assertThat(result).isTrue(); + assertThat(invocation.get()).isEqualTo(1); + } + + @Test + @TestParameters("{source: 'cel.block([])'}") + @TestParameters("{source: 'cel.block([1])'}") + @TestParameters("{source: 'cel.block(1, 2)'}") + @TestParameters("{source: 'cel.block(1, [1])'}") + public void block_invalidArguments_throws(String source) { + CelValidationException e = + assertThrows(CelValidationException.class, () -> compileUsingInternalFunctions(source)); + + assertThat(e).hasMessageThat().contains("found no matching overload for 'cel.block'"); + } + + @Test + public void blockIndex_invalidArgument_throws() { + CelValidationException e = + assertThrows( + CelValidationException.class, + () -> compileUsingInternalFunctions("cel.block([1], index)")); + + assertThat(e).hasMessageThat().contains("undeclared reference"); + } + + /** + * Converts AST containing cel.block related test functions to internal functions (e.g: cel.block + * -> cel.@block) + */ + private static CelAbstractSyntaxTree compileUsingInternalFunctions(String expression) + throws CelValidationException { + MutableAst mutableAst = MutableAst.newInstance(1000); + CelAbstractSyntaxTree astToModify = CEL_FOR_EVALUATING_BLOCK.compile(expression).getAst(); + while (true) { + CelExpr celExpr = + CelNavigableAst.fromAst(astToModify) + .getRoot() + .allNodes() + .filter(node -> node.getKind().equals(Kind.CALL)) + .map(CelNavigableExpr::expr) + .filter(expr -> expr.call().function().equals("cel.block")) + .findAny() + .orElse(null); + if (celExpr == null) { + break; + } + astToModify = + mutableAst.replaceSubtree( + astToModify, + celExpr.toBuilder() + .setCall(celExpr.call().toBuilder().setFunction("cel.@block").build()) + .build(), + celExpr.id()); + } + + while (true) { + CelExpr celExpr = + CelNavigableAst.fromAst(astToModify) + .getRoot() + .allNodes() + .filter(node -> node.getKind().equals(Kind.IDENT)) + .map(CelNavigableExpr::expr) + .filter(expr -> expr.ident().name().startsWith("index")) + .findAny() + .orElse(null); + if (celExpr == null) { + break; + } + String internalIdentName = "@" + celExpr.ident().name(); + astToModify = + mutableAst.replaceSubtree( + astToModify, + celExpr.toBuilder() + .setIdent(celExpr.ident().toBuilder().setName(internalIdentName).build()) + .build(), + celExpr.id()); + } + + return CEL_FOR_EVALUATING_BLOCK.check(astToModify).getAst(); + } } diff --git a/parser/src/main/java/dev/cel/parser/CelParser.java b/parser/src/main/java/dev/cel/parser/CelParser.java index 53879d11b..da1b7ba2a 100644 --- a/parser/src/main/java/dev/cel/parser/CelParser.java +++ b/parser/src/main/java/dev/cel/parser/CelParser.java @@ -52,4 +52,6 @@ default CelValidationResult parse(String expression) { */ @CheckReturnValue CelValidationResult parse(CelSource source); + + CelParserBuilder toParserBuilder(); } diff --git a/parser/src/main/java/dev/cel/parser/CelParserImpl.java b/parser/src/main/java/dev/cel/parser/CelParserImpl.java index 5d9944397..f630f7137 100644 --- a/parser/src/main/java/dev/cel/parser/CelParserImpl.java +++ b/parser/src/main/java/dev/cel/parser/CelParserImpl.java @@ -16,6 +16,7 @@ import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -50,6 +51,10 @@ public final class CelParserImpl implements CelParser { // Specific options for limits on parsing power. private final CelOptions options; + // Builder is mutable by design. APIs must make defensive copies in and out of this class. + @SuppressWarnings("Immutable") + private final Builder parserBuilder; + /** Creates a new {@link Builder}. */ public static CelParserBuilder newBuilder() { return new Builder().setOptions(CelOptions.DEFAULT); @@ -65,6 +70,11 @@ public CelValidationResult parse(CelSource source) { return Parser.parse(this, source, getOptions()); } + @Override + public CelParserBuilder toParserBuilder() { + return new Builder(parserBuilder); + } + Optional findMacro(String key) { return Optional.ofNullable(macros.get(key)); } @@ -136,6 +146,23 @@ public CelOptions getOptions() { return this.options; } + // The following getters exist for asserting immutability for collections held by this builder, + // and shouldn't be exposed to the public. + @VisibleForTesting + List getStandardMacros() { + return this.standardMacros; + } + + @VisibleForTesting + Map getMacros() { + return this.macros; + } + + @VisibleForTesting + ImmutableSet.Builder getParserLibraries() { + return this.celParserLibraries; + } + @Override @CheckReturnValue public CelParserImpl build() { @@ -149,7 +176,7 @@ public CelParserImpl build() { standardMacros.stream() .map(CelStandardMacro::getDefinition) .forEach(celMacro -> builder.put(celMacro.getKey(), celMacro)); - return new CelParserImpl(builder.buildOrThrow(), checkNotNull(options)); + return new CelParserImpl(builder.buildOrThrow(), checkNotNull(options), this); } private Builder() { @@ -157,10 +184,23 @@ private Builder() { this.celParserLibraries = ImmutableSet.builder(); this.standardMacros = new ArrayList<>(); } + + private Builder(Builder builder) { + // The following properties are either immutable or simple primitives, thus can be assigned + // directly. + this.options = builder.options; + // The following needs to be deep copied as they are collection builders + this.macros = new HashMap<>(builder.macros); + this.standardMacros = new ArrayList<>(builder.standardMacros); + this.celParserLibraries = ImmutableSet.builder(); + this.celParserLibraries.addAll(builder.celParserLibraries.build()); + } } - private CelParserImpl(ImmutableMap macros, CelOptions options) { + private CelParserImpl( + ImmutableMap macros, CelOptions options, Builder parserBuilder) { this.macros = macros; this.options = options; + this.parserBuilder = new Builder(parserBuilder); } } diff --git a/parser/src/test/java/dev/cel/parser/CelParserImplTest.java b/parser/src/test/java/dev/cel/parser/CelParserImplTest.java index 7c01c3611..f12fef292 100644 --- a/parser/src/test/java/dev/cel/parser/CelParserImplTest.java +++ b/parser/src/test/java/dev/cel/parser/CelParserImplTest.java @@ -271,4 +271,44 @@ public void parse_macroArgumentContainsSyntaxError_throws(String expression) { assertThat(parseResult.getErrorString()).containsMatch("ERROR: .*mismatched input ','"); assertThrows(CelValidationException.class, parseResult::getAst); } + + @Test + public void toParserBuilder_isNewInstance() { + CelParserBuilder celParserBuilder = CelParserFactory.standardCelParserBuilder(); + CelParserImpl celParser = (CelParserImpl) celParserBuilder.build(); + + CelParserImpl.Builder newParserBuilder = (CelParserImpl.Builder) celParser.toParserBuilder(); + + assertThat(newParserBuilder).isNotEqualTo(celParserBuilder); + } + + @Test + public void toParserBuilder_isImmutable() { + CelParserBuilder originalParserBuilder = CelParserFactory.standardCelParserBuilder(); + CelParserImpl celParser = (CelParserImpl) originalParserBuilder.build(); + originalParserBuilder.addLibraries(new CelParserLibrary() {}); + + CelParserImpl.Builder newParserBuilder = (CelParserImpl.Builder) celParser.toParserBuilder(); + + assertThat(newParserBuilder.getParserLibraries().build()).isEmpty(); + } + + @Test + public void toParserBuilder_collectionProperties_copied() { + CelParserBuilder celParserBuilder = + CelParserFactory.standardCelParserBuilder() + .setStandardMacros(CelStandardMacro.STANDARD_MACROS) + .addMacros( + CelMacro.newGlobalMacro( + "test", 1, (a, b, c) -> Optional.of(CelExpr.newBuilder().build()))) + .addLibraries(new CelParserLibrary() {}); + CelParserImpl celParser = (CelParserImpl) celParserBuilder.build(); + + CelParserImpl.Builder newParserBuilder = (CelParserImpl.Builder) celParser.toParserBuilder(); + + assertThat(newParserBuilder.getStandardMacros()) + .hasSize(CelStandardMacro.STANDARD_MACROS.size()); + assertThat(newParserBuilder.getMacros()).hasSize(1); + assertThat(newParserBuilder.getParserLibraries().build()).hasSize(1); + } } diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntime.java b/runtime/src/main/java/dev/cel/runtime/CelRuntime.java index 01260fb79..2b28d015b 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntime.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntime.java @@ -37,6 +37,8 @@ public interface CelRuntime { @CanIgnoreReturnValue Program createProgram(CelAbstractSyntaxTree ast) throws CelEvaluationException; + CelRuntimeBuilder toRuntimeBuilder(); + /** Creates an evaluable {@code Program} instance which is thread-safe and immutable. */ @AutoValue @Immutable diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java index 5f8f3f259..19059506a 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java @@ -17,6 +17,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -43,6 +44,8 @@ import dev.cel.common.values.CelValueProvider; import dev.cel.common.values.ProtoMessageValueProvider; import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; import java.util.function.Function; import org.jspecify.nullness.Nullable; @@ -60,12 +63,21 @@ public final class CelRuntimeLegacyImpl implements CelRuntime { private final Interpreter interpreter; private final CelOptions options; + // Builder is mutable by design. APIs must guarantee a new instance to be returned. + // CEL-Internal-4 + private final Builder runtimeBuilder; + @Override public CelRuntime.Program createProgram(CelAbstractSyntaxTree ast) { checkState(ast.isChecked(), "programs must be created from checked expressions"); return CelRuntime.Program.from(interpreter.createInterpretable(ast), options); } + @Override + public CelRuntimeBuilder toRuntimeBuilder() { + return new Builder(runtimeBuilder); + } + /** Create a new builder for constructing a {@code CelRuntime} instance. */ public static CelRuntimeBuilder newBuilder() { return new Builder(); @@ -75,7 +87,7 @@ public static CelRuntimeBuilder newBuilder() { public static final class Builder implements CelRuntimeBuilder { private final ImmutableSet.Builder fileTypes; - private final ImmutableMap.Builder functionBindings; + private final HashMap functionBindings; private final ImmutableSet.Builder celRuntimeLibraries; @SuppressWarnings("unused") @@ -99,7 +111,7 @@ public CelRuntimeBuilder addFunctionBindings(CelFunctionBinding... bindings) { @Override public CelRuntimeBuilder addFunctionBindings(Iterable bindings) { - bindings.forEach(o -> functionBindings.put(o.getOverloadId(), o)); + bindings.forEach(o -> functionBindings.putIfAbsent(o.getOverloadId(), o)); return this; } @@ -168,6 +180,23 @@ public CelRuntimeBuilder setExtensionRegistry(ExtensionRegistry extensionRegistr return this; } + // The following getters exist for asserting immutability for collections held by this builder, + // and shouldn't be exposed to the public. + @VisibleForTesting + Map getFunctionBindings() { + return this.functionBindings; + } + + @VisibleForTesting + ImmutableSet.Builder getRuntimeLibraries() { + return this.celRuntimeLibraries; + } + + @VisibleForTesting + ImmutableSet.Builder getFileTypes() { + return this.fileTypes; + } + /** Build a new {@code CelRuntimeLegacyImpl} instance from the builder config. */ @Override public CelRuntimeLegacyImpl build() { @@ -203,7 +232,8 @@ public CelRuntimeLegacyImpl build() { StandardFunctions.add(dispatcher, dynamicProto, options); } - ImmutableMap functionBindingMap = functionBindings.buildOrThrow(); + ImmutableMap functionBindingMap = + ImmutableMap.copyOf(functionBindings); functionBindingMap.forEach( (String overloadId, CelFunctionBinding func) -> dispatcher.add( @@ -238,7 +268,7 @@ public CelRuntimeLegacyImpl build() { } return new CelRuntimeLegacyImpl( - new DefaultInterpreter(runtimeTypeProvider, dispatcher, options), options); + new DefaultInterpreter(runtimeTypeProvider, dispatcher, options), options, this); } private static CelDescriptorPool newDescriptorPool( @@ -264,15 +294,35 @@ private static ProtoMessageFactory maybeCombineMessageFactory( private Builder() { this.options = CelOptions.newBuilder().build(); this.fileTypes = ImmutableSet.builder(); - this.functionBindings = ImmutableMap.builder(); + this.functionBindings = new HashMap<>(); this.celRuntimeLibraries = ImmutableSet.builder(); this.extensionRegistry = ExtensionRegistry.getEmptyRegistry(); this.customTypeFactory = null; } + + private Builder(Builder builder) { + // The following properties are either immutable or simple primitives, thus can be assigned + // directly. + this.options = builder.options; + this.extensionRegistry = builder.extensionRegistry; + this.customTypeFactory = builder.customTypeFactory; + // The following needs to be deep copied as they are collection builders + this.fileTypes = deepCopy(builder.fileTypes); + this.celRuntimeLibraries = deepCopy(builder.celRuntimeLibraries); + this.functionBindings = new HashMap<>(builder.functionBindings); + } + + private static ImmutableSet.Builder deepCopy(ImmutableSet.Builder builderToCopy) { + ImmutableSet.Builder newBuilder = ImmutableSet.builder(); + newBuilder.addAll(builderToCopy.build()); + return newBuilder; + } } - private CelRuntimeLegacyImpl(Interpreter interpreter, CelOptions options) { + private CelRuntimeLegacyImpl( + Interpreter interpreter, CelOptions options, Builder runtimeBuilder) { this.interpreter = interpreter; this.options = options; + this.runtimeBuilder = new Builder(runtimeBuilder); } } diff --git a/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java b/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java index f27882eda..c198734be 100644 --- a/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java +++ b/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java @@ -34,7 +34,6 @@ import dev.cel.common.ast.CelExpr.CelCreateList; import dev.cel.common.ast.CelExpr.CelCreateMap; import dev.cel.common.ast.CelExpr.CelCreateStruct; -import dev.cel.common.ast.CelExpr.CelIdent; import dev.cel.common.ast.CelExpr.CelSelect; import dev.cel.common.ast.CelExpr.ExprKind; import dev.cel.common.ast.CelReference; @@ -194,7 +193,7 @@ private IntermediateResult evalInternal(ExecutionFrame frame, CelExpr expr) result = IntermediateResult.create(evalConstant(frame, expr, expr.constant())); break; case IDENT: - result = evalIdent(frame, expr, expr.ident()); + result = evalIdent(frame, expr); break; case SELECT: result = evalSelect(frame, expr, expr.select()); @@ -257,7 +256,7 @@ private Object evalConstant( } } - private IntermediateResult evalIdent(ExecutionFrame frame, CelExpr expr, CelIdent unusedIdent) + private IntermediateResult evalIdent(ExecutionFrame frame, CelExpr expr) throws InterpreterException { CelReference reference = ast.getReferenceOrThrow(expr.id()); if (reference.value().isPresent()) { @@ -371,6 +370,12 @@ private IntermediateResult evalCall(ExecutionFrame frame, CelExpr expr, CelCall return result.get(); } break; + case "cel_block_list": + return evalCelBlock(frame, expr, callExpr); + // case "cel_index_int": + // long index = callExpr.args().get(0).constant().int64Value(); + // String blockIdent = "cel.@block" + index; + // return resolveIdent(frame, CelExpr.ofIdentExpr(expr.id(), blockIdent), blockIdent); default: break; } @@ -846,6 +851,21 @@ private IntermediateResult evalComprehension( frame.popScope(); return result; } + + private IntermediateResult evalCelBlock( + ExecutionFrame frame, CelExpr unusedExpr, CelCall blockCall) throws InterpreterException { + CelCreateList exprList = blockCall.args().get(0).createList(); + ImmutableMap.Builder blockList = ImmutableMap.builder(); + for (int index = 0; index < exprList.elements().size(); index++) { + // Register the block indices as lazily evaluated expressions stored as unique identifiers. + blockList.put( + "@index" + index, + IntermediateResult.create(new LazyExpression(exprList.elements().get(index)))); + } + frame.pushScope(blockList.buildOrThrow()); + + return evalInternal(frame, blockCall.args().get(1)); + } } /** Contains a CelExpr that is to be lazily evaluated. */ diff --git a/runtime/src/test/java/dev/cel/runtime/CelRuntimeLegacyImplTest.java b/runtime/src/test/java/dev/cel/runtime/CelRuntimeLegacyImplTest.java index 02fd0ae7c..2ce5e4d0b 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelRuntimeLegacyImplTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelRuntimeLegacyImplTest.java @@ -19,6 +19,8 @@ import dev.cel.common.CelException; import dev.cel.compiler.CelCompiler; import dev.cel.compiler.CelCompilerFactory; +import dev.cel.runtime.CelRuntime.CelFunctionBinding; +import dev.cel.testing.testdata.proto3.TestAllTypesProto.TestAllTypes; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -35,4 +37,62 @@ public void evalException() throws CelException { CelEvaluationException e = Assert.assertThrows(CelEvaluationException.class, program::eval); assertThat(e).hasCauseThat().isInstanceOf(ArithmeticException.class); } + + @Test + public void toRuntimeBuilder_isNewInstance() { + CelRuntimeBuilder celRuntimeBuilder = CelRuntimeFactory.standardCelRuntimeBuilder(); + CelRuntimeLegacyImpl celRuntime = (CelRuntimeLegacyImpl) celRuntimeBuilder.build(); + + CelRuntimeLegacyImpl.Builder newRuntimeBuilder = + (CelRuntimeLegacyImpl.Builder) celRuntime.toRuntimeBuilder(); + + assertThat(newRuntimeBuilder).isNotEqualTo(celRuntimeBuilder); + } + + @Test + public void toRuntimeBuilder_isImmutable() { + CelRuntimeBuilder originalRuntimeBuilder = CelRuntimeFactory.standardCelRuntimeBuilder(); + CelRuntimeLegacyImpl celRuntime = (CelRuntimeLegacyImpl) originalRuntimeBuilder.build(); + originalRuntimeBuilder.addLibraries(runtimeBuilder -> {}); + + CelRuntimeLegacyImpl.Builder newRuntimeBuilder = + (CelRuntimeLegacyImpl.Builder) celRuntime.toRuntimeBuilder(); + + assertThat(newRuntimeBuilder.getRuntimeLibraries().build()).isEmpty(); + } + + @Test + public void toRuntimeBuilder_collectionProperties_copied() { + CelRuntimeBuilder celRuntimeBuilder = CelRuntimeFactory.standardCelRuntimeBuilder(); + celRuntimeBuilder.addMessageTypes(TestAllTypes.getDescriptor()); + celRuntimeBuilder.addFileTypes(TestAllTypes.getDescriptor().getFile()); + celRuntimeBuilder.addFunctionBindings(CelFunctionBinding.from("test", Integer.class, arg -> 1)); + celRuntimeBuilder.addLibraries(runtimeBuilder -> {}); + CelRuntimeLegacyImpl celRuntime = (CelRuntimeLegacyImpl) celRuntimeBuilder.build(); + + CelRuntimeLegacyImpl.Builder newRuntimeBuilder = + (CelRuntimeLegacyImpl.Builder) celRuntime.toRuntimeBuilder(); + + assertThat(newRuntimeBuilder.getFunctionBindings()).hasSize(1); + assertThat(newRuntimeBuilder.getRuntimeLibraries().build()).hasSize(1); + assertThat(newRuntimeBuilder.getFileTypes().build()).hasSize(6); + } + + @Test + public void toRuntimeBuilder_collectionProperties_areImmutable() { + CelRuntimeBuilder celRuntimeBuilder = CelRuntimeFactory.standardCelRuntimeBuilder(); + CelRuntimeLegacyImpl celRuntime = (CelRuntimeLegacyImpl) celRuntimeBuilder.build(); + CelRuntimeLegacyImpl.Builder newRuntimeBuilder = + (CelRuntimeLegacyImpl.Builder) celRuntime.toRuntimeBuilder(); + + // Mutate the original builder containing collections + celRuntimeBuilder.addMessageTypes(TestAllTypes.getDescriptor()); + celRuntimeBuilder.addFileTypes(TestAllTypes.getDescriptor().getFile()); + celRuntimeBuilder.addFunctionBindings(CelFunctionBinding.from("test", Integer.class, arg -> 1)); + celRuntimeBuilder.addLibraries(runtimeBuilder -> {}); + + assertThat(newRuntimeBuilder.getFunctionBindings()).isEmpty(); + assertThat(newRuntimeBuilder.getRuntimeLibraries().build()).isEmpty(); + assertThat(newRuntimeBuilder.getFileTypes().build()).isEmpty(); + } }