diff --git a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel index 4a5f6dcc7..20d49c476 100644 --- a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel @@ -60,7 +60,6 @@ INTERPRABLE_SOURCES = [ # keep sorted DISPATCHER_SOURCES = [ "DefaultDispatcher.java", - "Dispatcher.java", ] java_library( @@ -125,7 +124,6 @@ java_library( ":resolved_overload_internal", "//:auto_value", "//common:error_codes", - "//common/annotations", "//runtime:function_resolver", "@maven//:com_google_code_findbugs_annotations", "@maven//:com_google_errorprone_error_prone_annotations", @@ -146,7 +144,6 @@ cel_android_library( ":resolved_overload_internal_android", "//:auto_value", "//common:error_codes", - "//common/annotations", "@maven//:com_google_code_findbugs_annotations", "@maven//:com_google_errorprone_error_prone_annotations", "@maven_android//:com_google_guava_guava", diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java index 4f58d44db..1b7071505 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java @@ -287,12 +287,12 @@ public CelRuntimeLegacyImpl build() { functionBindingsBuilder.putAll(customFunctionBindings); - DefaultDispatcher dispatcher = DefaultDispatcher.create(); + DefaultDispatcher.Builder dispatcherBuilder = DefaultDispatcher.newBuilder(); functionBindingsBuilder .buildOrThrow() .forEach( (String overloadId, CelFunctionBinding func) -> - dispatcher.add( + dispatcherBuilder.addOverload( overloadId, func.getArgTypes(), func.isStrict(), func.getDefinition())); RuntimeTypeProvider runtimeTypeProvider; @@ -313,7 +313,7 @@ public CelRuntimeLegacyImpl build() { new DefaultInterpreter( DescriptorTypeResolver.create(), runtimeTypeProvider, - dispatcher.immutableCopy(), + dispatcherBuilder.build(), options); return new CelRuntimeLegacyImpl( diff --git a/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java b/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java index 6228dd253..2643ddf75 100644 --- a/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java +++ b/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java @@ -14,41 +14,29 @@ package dev.cel.runtime; +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.auto.value.AutoBuilder; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableMap; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.Immutable; -import javax.annotation.concurrent.ThreadSafe; -import com.google.errorprone.annotations.concurrent.GuardedBy; import dev.cel.common.CelErrorCode; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -/** - * Default implementation of {@link Dispatcher}. - * - *

Should be final, do not mock; mocking {@link Dispatcher} instead. - */ -@ThreadSafe -final class DefaultDispatcher implements Dispatcher { - public static DefaultDispatcher create() { - return new DefaultDispatcher(); - } +/** Default implementation of dispatcher. */ +@Immutable +final class DefaultDispatcher implements CelFunctionResolver { - @GuardedBy("this") - private final Map overloads = new HashMap<>(); - - public synchronized void add( - String overloadId, List> argTypes, boolean isStrict, CelFunctionOverload overload) { - overloads.put(overloadId, CelResolvedOverload.of(overloadId, overload, isStrict, argTypes)); - } + private final ImmutableMap overloads; @Override - public synchronized Optional findOverload( + public Optional findOverload( String functionName, List overloadIds, Object[] args) throws CelEvaluationException { - return DefaultDispatcher.findOverload(functionName, overloadIds, overloads, args); + return findOverload(functionName, overloadIds, overloads, args); } /** Finds the overload that matches the given function name, overload IDs, and arguments. */ @@ -87,31 +75,36 @@ public static Optional findOverload( return Optional.ofNullable(match); } - @Override - public synchronized Dispatcher.ImmutableCopy immutableCopy() { - return new ImmutableCopy(overloads); + static Builder newBuilder() { + return new AutoBuilder_DefaultDispatcher_Builder(); } - @Immutable - private static final class ImmutableCopy implements Dispatcher.ImmutableCopy { - private final ImmutableMap overloads; + @AutoBuilder(ofClass = DefaultDispatcher.class) + abstract static class Builder { - private ImmutableCopy(Map overloads) { - this.overloads = ImmutableMap.copyOf(overloads); - } + abstract ImmutableMap overloads(); - @Override - public Optional findOverload( - String functionName, List overloadIds, Object[] args) - throws CelEvaluationException { - return DefaultDispatcher.findOverload(functionName, overloadIds, overloads, args); - } + abstract ImmutableMap.Builder overloadsBuilder(); + + @CanIgnoreReturnValue + Builder addOverload( + String overloadId, + List> argTypes, + boolean isStrict, + CelFunctionOverload overload) { + checkNotNull(overloadId); + checkNotNull(argTypes); + checkNotNull(overload); - @Override - public Dispatcher.ImmutableCopy immutableCopy() { + overloadsBuilder() + .put(overloadId, CelResolvedOverload.of(overloadId, overload, isStrict, argTypes)); return this; } + + abstract DefaultDispatcher build(); } - private DefaultDispatcher() {} + DefaultDispatcher(ImmutableMap overloads) { + this.overloads = overloads; + } } diff --git a/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java b/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java index b471d972e..f40960e74 100644 --- a/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java +++ b/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java @@ -64,7 +64,7 @@ final class DefaultInterpreter implements Interpreter { private final TypeResolver typeResolver; private final RuntimeTypeProvider typeProvider; - private final Dispatcher dispatcher; + private final DefaultDispatcher dispatcher; private final CelOptions celOptions; /** @@ -102,7 +102,7 @@ static IntermediateResult create(Object value) { public DefaultInterpreter( TypeResolver typeResolver, RuntimeTypeProvider typeProvider, - Dispatcher dispatcher, + DefaultDispatcher dispatcher, CelOptions celOptions) { this.typeResolver = checkNotNull(typeResolver); this.typeProvider = checkNotNull(typeProvider); @@ -120,7 +120,7 @@ public Interpretable createInterpretable(CelAbstractSyntaxTree ast) { static final class DefaultInterpretable implements Interpretable, UnknownTrackingInterpretable { private final TypeResolver typeResolver; private final RuntimeTypeProvider typeProvider; - private final Dispatcher.ImmutableCopy dispatcher; + private final DefaultDispatcher dispatcher; private final Metadata metadata; private final CelAbstractSyntaxTree ast; private final CelOptions celOptions; @@ -128,12 +128,12 @@ static final class DefaultInterpretable implements Interpretable, UnknownTrackin DefaultInterpretable( TypeResolver typeResolver, RuntimeTypeProvider typeProvider, - Dispatcher dispatcher, + DefaultDispatcher dispatcher, CelAbstractSyntaxTree ast, CelOptions celOptions) { this.typeResolver = checkNotNull(typeResolver); this.typeProvider = checkNotNull(typeProvider); - this.dispatcher = checkNotNull(dispatcher).immutableCopy(); + this.dispatcher = checkNotNull(dispatcher); this.ast = checkNotNull(ast); this.metadata = new DefaultMetadata(ast); this.celOptions = checkNotNull(celOptions); diff --git a/runtime/src/main/java/dev/cel/runtime/Dispatcher.java b/runtime/src/main/java/dev/cel/runtime/Dispatcher.java deleted file mode 100644 index e7bc6163f..000000000 --- a/runtime/src/main/java/dev/cel/runtime/Dispatcher.java +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2022 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.runtime; - -import com.google.errorprone.annotations.Immutable; -import javax.annotation.concurrent.ThreadSafe; -import dev.cel.common.annotations.Internal; - -/** - * An object which implements dispatching of function calls. - * - *

CEL Library Internals. Do Not Use. - */ -@ThreadSafe -@Internal -interface Dispatcher extends CelFunctionResolver { - - /** - * Returns an {@link ImmutableCopy} from current instance. - * - * @see ImmutableCopy - */ - ImmutableCopy immutableCopy(); - - /** - * An {@link Immutable} copy of a {@link Dispatcher}. Currently {@link DefaultDispatcher} - * implementation implements both {@link Dispatcher} and {@link Registrar} and cannot be annotated - * as {@link Immutable}. - * - *

Should consider to provide Registrar.dumpAsDispatcher and Registrar.dumpAsAsyncDispatcher - * instead of letting DefaultDispatcher or AsyncDispatcher to implement both Registrar and - * Dispatcher. But it requires a global refactoring. - */ - @Immutable - interface ImmutableCopy extends Dispatcher {} -} diff --git a/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java b/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java index aba73aed4..e1415eabd 100644 --- a/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java @@ -176,22 +176,19 @@ public CelLiteRuntime build() { functionBindingsBuilder.putAll(customFunctionBindings); - DefaultDispatcher dispatcher = DefaultDispatcher.create(); + DefaultDispatcher.Builder dispatcherBuilder = DefaultDispatcher.newBuilder(); functionBindingsBuilder .buildOrThrow() .forEach( (String overloadId, CelFunctionBinding func) -> - dispatcher.add( - overloadId, - func.getArgTypes(), - func.isStrict(), - (args) -> func.getDefinition().apply(args))); + dispatcherBuilder.addOverload( + overloadId, func.getArgTypes(), func.isStrict(), func.getDefinition())); Interpreter interpreter = new DefaultInterpreter( TypeResolver.create(), CelValueRuntimeTypeProvider.newInstance(celValueProvider), - dispatcher, + dispatcherBuilder.build(), celOptions); return new LiteRuntimeImpl( diff --git a/runtime/src/test/java/dev/cel/runtime/DefaultInterpreterTest.java b/runtime/src/test/java/dev/cel/runtime/DefaultInterpreterTest.java index e9ac6b5a3..b70bffdb6 100644 --- a/runtime/src/test/java/dev/cel/runtime/DefaultInterpreterTest.java +++ b/runtime/src/test/java/dev/cel/runtime/DefaultInterpreterTest.java @@ -74,14 +74,15 @@ public Object adapt(String messageName, Object message) { } }; CelAbstractSyntaxTree ast = celCompiler.compile("[1].all(x, [2].all(y, error()))").getAst(); - DefaultDispatcher dispatcher = DefaultDispatcher.create(); - dispatcher.add( + DefaultDispatcher.Builder dispatcherBuilder = DefaultDispatcher.newBuilder(); + dispatcherBuilder.addOverload( "error", ImmutableList.of(long.class), /* isStrict= */ true, (args) -> new IllegalArgumentException("Always throws")); DefaultInterpreter defaultInterpreter = - new DefaultInterpreter(new TypeResolver(), emptyProvider, dispatcher, CelOptions.DEFAULT); + new DefaultInterpreter( + new TypeResolver(), emptyProvider, dispatcherBuilder.build(), CelOptions.DEFAULT); DefaultInterpretable interpretable = (DefaultInterpretable) defaultInterpreter.createInterpretable(ast);