From bfd6b2c2e20e9203f0d91029aef19cbb098a1763 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Fri, 16 Oct 2026 19:22:45 +0200 Subject: [PATCH 1/3] Optimize collection and protobuf value handling Reduce wrapper allocation in collection traversal, membership, and generated protobuf field access. Preserve live list sizes, iterator cursor semantics, custom collection contracts, and dynamic-message fallback. Keep hashes consistent across CEL numeric types, normalize unsigned protobuf fields without sign extension, and preserve Java map equality and extension compatibility. Add collection, iterator, generated-field, and public API regression coverage plus benchmarks. --- .../interpreter/CollectionOperationBench.java | 376 ++++++++++++++++++ .../cel/interpreter/FieldAccessBench.java | 266 +++++++++++++ .../cel/common/types/BaseIteratorT.java | 52 +++ .../projectnessie/cel/common/types/BoolT.java | 3 +- .../cel/common/types/BytesT.java | 5 + .../cel/common/types/DoubleT.java | 21 +- .../projectnessie/cel/common/types/IntT.java | 23 +- .../cel/common/types/IterableT.java | 7 +- .../cel/common/types/IteratorT.java | 172 ++++++-- .../projectnessie/cel/common/types/ListT.java | 339 ++++++++++++---- .../projectnessie/cel/common/types/MapT.java | 323 ++++++++++++--- .../projectnessie/cel/common/types/NullT.java | 3 +- .../cel/common/types/OptionalT.java | 33 +- .../cel/common/types/StringT.java | 29 +- .../projectnessie/cel/common/types/Types.java | 18 + .../projectnessie/cel/common/types/UintT.java | 17 +- .../cel/common/types/pb/FieldDescription.java | 268 +++++++++++-- .../types/pb/GeneratedFieldAccessor.java | 190 ++++++++- .../cel/common/types/pb/PbObjectT.java | 16 + .../common/types/pb/ProtoTypeRegistry.java | 120 +++++- .../cel/common/types/traits/Indexer.java | 16 + .../cel/common/types/traits/Sizer.java | 15 + .../cel/common/types/traits/Trait.java | 6 +- .../projectnessie/cel/extension/MathLib.java | 4 +- .../cel/extension/StringsLib.java | 9 +- .../cel/interpreter/AstPruner.java | 7 +- .../cel/interpreter/AttributeFactory.java | 8 + .../cel/interpreter/Interpretable.java | 115 ++++-- .../interpreter/InterpretableDecorator.java | 3 +- .../cel/common/types/BytesTest.java | 4 +- .../cel/common/types/DoubleTest.java | 5 + .../cel/common/types/IteratorTTest.java | 286 +++++++++++++ .../cel/common/types/ListTest.java | 13 +- .../cel/common/types/MapTest.java | 195 +++++++++ .../common/types/PrimitiveArrayListTest.java | 106 +++++ .../cel/common/types/StringTest.java | 1 + .../common/types/pb/FieldDescriptionTest.java | 145 +++++++ .../types/pb/GeneratedFieldAccessorTest.java | 153 ++++++- .../cel/interpreter/AttributesTest.java | 42 ++ .../SetMembershipOptimizationTest.java | 110 +++++ .../IteratorPublicApiCompileTest.java | 135 +++++++ .../publicapi/MapPublicApiCompileTest.java | 31 ++ .../cel/types/jackson/JacksonObjectT.java | 2 +- .../cel/types/jackson3/JacksonObjectT.java | 2 +- 44 files changed, 3343 insertions(+), 351 deletions(-) create mode 100644 core/src/jmh/java/org/projectnessie/cel/interpreter/CollectionOperationBench.java create mode 100644 core/src/jmh/java/org/projectnessie/cel/interpreter/FieldAccessBench.java create mode 100644 core/src/main/java/org/projectnessie/cel/common/types/BaseIteratorT.java create mode 100644 core/src/test/java/org/projectnessie/cel/common/types/IteratorTTest.java create mode 100644 core/src/test/java/org/projectnessie/cel/common/types/PrimitiveArrayListTest.java create mode 100644 core/src/test/java/org/projectnessie/cel/interpreter/SetMembershipOptimizationTest.java create mode 100644 core/src/test/java/org/projectnessie/cel/publicapi/IteratorPublicApiCompileTest.java create mode 100644 core/src/test/java/org/projectnessie/cel/publicapi/MapPublicApiCompileTest.java diff --git a/core/src/jmh/java/org/projectnessie/cel/interpreter/CollectionOperationBench.java b/core/src/jmh/java/org/projectnessie/cel/interpreter/CollectionOperationBench.java new file mode 100644 index 000000000..aebf5dd47 --- /dev/null +++ b/core/src/jmh/java/org/projectnessie/cel/interpreter/CollectionOperationBench.java @@ -0,0 +1,376 @@ +/* + * Copyright (C) 2021 The Authors of CEL-Java + * + * 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 + * + * http://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 org.projectnessie.cel.interpreter; + +import static org.projectnessie.cel.Env.newEnv; +import static org.projectnessie.cel.EnvOption.declarations; +import static org.projectnessie.cel.EnvOption.types; +import static org.projectnessie.cel.ProgramOption.evalOptions; +import static org.projectnessie.cel.Util.mapOf; +import static org.projectnessie.cel.common.types.IntT.intOf; + +import dev.cel.expr.conformance.proto3.TestAllTypes; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.projectnessie.cel.Env; +import org.projectnessie.cel.Env.AstIssuesTuple; +import org.projectnessie.cel.EvalOption; +import org.projectnessie.cel.Program; +import org.projectnessie.cel.checker.Decls; +import org.projectnessie.cel.common.types.IteratorT; +import org.projectnessie.cel.common.types.MapT; +import org.projectnessie.cel.common.types.pb.ProtoTypeRegistry; +import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.common.types.traits.Lister; +import org.projectnessie.cel.common.types.traits.Mapper; + +@Warmup(iterations = 1, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 5, time = 300, timeUnit = TimeUnit.MILLISECONDS) +@Fork(1) +@Threads(1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +public class CollectionOperationBench { + private static final String TEST_ALL_TYPES = "cel.expr.conformance.proto3.TestAllTypes"; + + @State(Scope.Benchmark) + public static class DirectListState { + @Param({"containsLast", "containsMiss", "equal", "iterate", "indexScan"}) + public String operation; + + @Param({"intArray", "longArray", "doubleArray", "javaList"}) + public String representation; + + @Param({"16", "1024"}) + public int size; + + Lister left; + Val right; + Val needle; + + @Setup + public void init() { + ProtoTypeRegistry adapter = ProtoTypeRegistry.newRegistry(); + Object leftNative = listValue(representation, size, 50_000); + Object rightNative = listValue(representation, size, 50_000); + left = (Lister) adapter.nativeToValue(leftNative); + right = adapter.nativeToValue(rightNative); + needle = intOf(operation.equals("containsMiss") ? -50_000 : 50_000L + size - 1); + if (representation.equals("doubleArray")) { + needle = adapter.nativeToValue(needle.intValue() * 1.0d); + } + } + } + + @Benchmark + public Val directList(DirectListState state) { + return switch (state.operation) { + case "containsLast", "containsMiss" -> state.left.contains(state.needle); + case "equal" -> state.left.equal(state.right); + case "iterate" -> consumeIterator(state.left.iterator()); + case "indexScan" -> consumeIndexed(state.left); + default -> throw new IllegalArgumentException(state.operation); + }; + } + + @State(Scope.Benchmark) + public static class ListProgramState { + @Param({"containsLast", "containsMiss", "equal", "existsLate"}) + public String operation; + + @Param({"intArray", "longArray", "javaList"}) + public String representation; + + @Param({"16", "1024"}) + public int size; + + Program program; + Map vars; + + @Setup + public void init() { + String expression; + switch (operation) { + case "containsLast", "containsMiss" -> expression = "target in items"; + case "equal" -> expression = "items == other"; + case "existsLate" -> expression = "items.exists(i, i == target)"; + default -> throw new IllegalArgumentException(operation); + } + program = + program( + expression, + Decls.newVar("items", Decls.newListType(Decls.Int)), + Decls.newVar("other", Decls.newListType(Decls.Int)), + Decls.newVar("target", Decls.Int)); + long target = operation.equals("containsMiss") ? -50_000 : 50_000L + size - 1; + vars = + mapOf( + "items", + listValue(representation, size, 50_000), + "other", + listValue(representation, size, 50_000), + "target", + target); + } + } + + @Benchmark + public Object listProgram(ListProgramState state) { + return state.program.eval(state.vars); + } + + @State(Scope.Benchmark) + public static class DirectMapState { + @Param({"adapt", "contains", "find", "size", "equal", "equalWrapped", "equalCross", "iterate"}) + public String operation; + + @Param({"16", "1024"}) + public int size; + + ProtoTypeRegistry adapter; + Map nativeMap; + Mapper map; + Val other; + Mapper wrappedMap; + Val wrappedOther; + Val key; + + @Setup + public void init() { + adapter = ProtoTypeRegistry.newRegistry(); + nativeMap = stringMap(size); + map = (Mapper) adapter.nativeToValue(nativeMap); + other = adapter.nativeToValue(stringMap(size)); + wrappedMap = wrappedStringMap(adapter, size); + wrappedOther = wrappedStringMap(adapter, size); + key = adapter.nativeToValue("key-" + (size - 1)); + } + } + + @Benchmark + public Val directMap(DirectMapState state) { + return switch (state.operation) { + case "adapt" -> state.adapter.nativeToValue(state.nativeMap); + case "contains" -> state.map.contains(state.key); + case "find" -> state.map.find(state.key); + case "size" -> state.map.size(); + case "equal" -> state.map.equal(state.other); + case "equalWrapped" -> state.wrappedMap.equal(state.wrappedOther); + case "equalCross" -> state.map.equal(state.wrappedOther); + case "iterate" -> consumeIterator(state.map.iterator()); + default -> throw new IllegalArgumentException(state.operation); + }; + } + + @State(Scope.Benchmark) + public static class MapProgramState { + @Param({"contains", "size", "index", "equal"}) + public String operation; + + @Param({"16", "1024"}) + public int size; + + Program program; + Map vars; + + @Setup + public void init() { + String expression = + switch (operation) { + case "contains" -> "key in attrs"; + case "size" -> "size(attrs) == expectedSize"; + case "index" -> "attrs[key] == target"; + case "equal" -> "attrs == other"; + default -> throw new IllegalArgumentException(operation); + }; + program = + program( + expression, + Decls.newVar("attrs", Decls.newMapType(Decls.String, Decls.Int)), + Decls.newVar("other", Decls.newMapType(Decls.String, Decls.Int)), + Decls.newVar("key", Decls.String), + Decls.newVar("target", Decls.Int), + Decls.newVar("expectedSize", Decls.Int)); + vars = + mapOf( + "attrs", + stringMap(size), + "other", + stringMap(size), + "key", + "key-" + (size - 1), + "target", + (long) size - 1, + "expectedSize", + (long) size); + } + } + + @Benchmark + public Object mapProgram(MapProgramState state) { + return state.program.eval(state.vars); + } + + @State(Scope.Benchmark) + public static class ProtoMapProgramState { + @Param({"contains", "lookup", "lookupRepeated", "equal"}) + public String operation; + + @Param({"16", "1024"}) + public int size; + + Program program; + Map vars; + + @Setup + public void init() { + String expression = + switch (operation) { + case "contains" -> "key in msg.map_string_uint64"; + case "lookup" -> "msg.map_string_uint64[key] == target"; + case "lookupRepeated" -> + "msg.map_string_uint64[key] == target && msg.map_string_uint64[key] == target"; + case "equal" -> "msg.map_string_uint64 == other.map_string_uint64"; + default -> throw new IllegalArgumentException(operation); + }; + Env env = + newEnv( + types(TestAllTypes.getDefaultInstance()), + declarations( + Decls.newVar("msg", Decls.newObjectType(TEST_ALL_TYPES)), + Decls.newVar("other", Decls.newObjectType(TEST_ALL_TYPES)), + Decls.newVar("key", Decls.String), + Decls.newVar("target", Decls.Uint))); + program = program(env, expression); + TestAllTypes message = protoMessage(size); + vars = + mapOf( + "msg", + message, + "other", + protoMessage(size), + "key", + "key-" + (size - 1), + "target", + (long) size - 1); + } + } + + @Benchmark + public Object protoMapProgram(ProtoMapProgramState state) { + return state.program.eval(state.vars); + } + + private static Val consumeIterator(IteratorT iterator) { + Val value = intOf(0); + while (iterator.hasNext().booleanValue()) { + value = iterator.next(); + } + return value; + } + + private static Val consumeIndexed(Lister list) { + Val value = intOf(0); + int size = list.nativeSize(); + for (int i = 0; i < size; i++) { + value = list.nativeGetAt(i); + } + return value; + } + + private static Object listValue(String representation, int size, int offset) { + return switch (representation) { + case "intArray" -> { + int[] values = new int[size]; + for (int i = 0; i < size; i++) { + values[i] = offset + i; + } + yield values; + } + case "longArray" -> { + long[] values = new long[size]; + for (int i = 0; i < size; i++) { + values[i] = offset + i; + } + yield values; + } + case "doubleArray" -> { + double[] values = new double[size]; + for (int i = 0; i < size; i++) { + values[i] = offset + i; + } + yield values; + } + case "javaList" -> { + Long[] values = new Long[size]; + for (int i = 0; i < size; i++) { + values[i] = (long) offset + i; + } + yield java.util.Arrays.asList(values); + } + default -> throw new IllegalArgumentException(representation); + }; + } + + private static Map stringMap(int size) { + Map values = new HashMap<>(size * 4 / 3 + 1); + for (long i = 0; i < size; i++) { + values.put("key-" + i, i); + } + return values; + } + + private static Mapper wrappedStringMap(ProtoTypeRegistry adapter, int size) { + Map values = new HashMap<>(size * 4 / 3 + 1); + for (long i = 0; i < size; i++) { + values.put(adapter.nativeToValue("key-" + i), adapter.nativeToValue(i)); + } + return (Mapper) MapT.newWrappedMap(adapter, values); + } + + private static TestAllTypes protoMessage(int size) { + TestAllTypes.Builder builder = TestAllTypes.newBuilder(); + for (int i = 0; i < size; i++) { + builder.putMapStringUint64("key-" + i, i); + } + return builder.build(); + } + + private static Program program(String expression, com.google.api.expr.v1alpha1.Decl... decls) { + return program(newEnv(declarations(decls)), expression); + } + + private static Program program(Env env, String expression) { + AstIssuesTuple ast = env.compile(expression); + if (ast.hasIssues()) { + throw ast.getIssues().err(); + } + return env.program(ast.getAst(), evalOptions(EvalOption.OptOptimize)); + } +} diff --git a/core/src/jmh/java/org/projectnessie/cel/interpreter/FieldAccessBench.java b/core/src/jmh/java/org/projectnessie/cel/interpreter/FieldAccessBench.java new file mode 100644 index 000000000..c3dee8dcb --- /dev/null +++ b/core/src/jmh/java/org/projectnessie/cel/interpreter/FieldAccessBench.java @@ -0,0 +1,266 @@ +/* + * Copyright (C) 2021 The Authors of CEL-Java + * + * 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 + * + * http://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 org.projectnessie.cel.interpreter; + +import static org.projectnessie.cel.common.types.StringT.stringOf; +import static org.projectnessie.cel.common.types.pb.ProtoTypeRegistry.newRegistry; +import static org.projectnessie.cel.interpreter.Activation.newActivation; +import static org.projectnessie.cel.interpreter.AttributeFactory.newAttributeFactory; + +import com.google.api.expr.v1alpha1.Type; +import com.google.protobuf.DynamicMessage; +import com.google.protobuf.Int32Value; +import dev.cel.expr.conformance.proto3.TestAllTypes; +import dev.cel.expr.conformance.proto3.TestAllTypes.NestedMessage; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.projectnessie.cel.common.containers.Container; +import org.projectnessie.cel.common.types.StringT; +import org.projectnessie.cel.common.types.pb.PbObjectT; +import org.projectnessie.cel.common.types.ref.FieldType; +import org.projectnessie.cel.common.types.ref.TypeRegistry; +import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.interpreter.AttributeFactory.NamespacedAttribute; +import org.projectnessie.cel.interpreter.Interpretable.EvalAttr; +import org.projectnessie.cel.interpreter.Interpretable.EvalTestOnly; + +@Warmup(iterations = 5, time = 200, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 10, time = 200, timeUnit = TimeUnit.MILLISECONDS) +@Fork(3) +@Threads(1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +public class FieldAccessBench { + + @State(Scope.Benchmark) + public static class FieldAccessState { + final TestAllTypes message; + final DynamicMessage dynamicMessage; + final PbObjectT object; + final PbObjectT dynamicObject; + final PbObjectT absentObject; + final StringT int32Name = stringOf("single_int32"); + final StringT optionalBoolName = stringOf("optional_bool"); + final StringT nestedMessageName = stringOf("single_nested_message"); + final StringT uint32Name = stringOf("single_uint32"); + final StringT enumName = stringOf("standalone_enum"); + final StringT wrapperName = stringOf("single_int32_wrapper"); + final StringT repeatedName = stringOf("repeated_int32"); + final StringT mapName = stringOf("map_string_string"); + final Activation activation; + final NamespacedAttribute int32Attribute; + final NamespacedAttribute nestedInt32Attribute; + final Interpretable presence; + final FieldType int32; + final FieldType optionalBool; + final FieldType nestedMessage; + final FieldType uint32; + final FieldType enumField; + final FieldType wrapper; + final FieldType repeated; + final FieldType map; + + public FieldAccessState() { + Map mapValues = new HashMap<>(); + for (int i = 0; i < 16; i++) { + mapValues.put("key-" + i, "value-" + i); + } + + TestAllTypes.Builder builder = + TestAllTypes.newBuilder() + .setSingleInt32(50_000) + .setSingleInt64(0x51a7_19b3_42c5L) + .setSingleFloat(12_345.25f) + .setSingleDouble(12_345.25d) + .setOptionalBool(false) + .setSingleNestedMessage(NestedMessage.newBuilder().setBb(50_000)) + .setSingleUint32(-1) + .setSingleUint64(-1L) + .setStandaloneEnumValue(12_345) + .setSingleInt32Wrapper(Int32Value.of(50_000)) + .putAllMapStringString(mapValues); + for (int i = 0; i < 16; i++) { + builder.addRepeatedInt32(50_000 + i); + } + message = builder.build(); + dynamicMessage = + DynamicMessage.newBuilder(message.getDescriptorForType()).mergeFrom(message).build(); + + TypeRegistry registry = newRegistry(TestAllTypes.getDefaultInstance()); + object = (PbObjectT) registry.nativeToValue(message); + dynamicObject = (PbObjectT) registry.nativeToValue(dynamicMessage); + absentObject = (PbObjectT) registry.nativeToValue(TestAllTypes.getDefaultInstance()); + + String messageType = message.getDescriptorForType().getFullName(); + int32 = registry.findFieldType(messageType, "single_int32"); + optionalBool = registry.findFieldType(messageType, "optional_bool"); + nestedMessage = registry.findFieldType(messageType, "single_nested_message"); + uint32 = registry.findFieldType(messageType, "single_uint32"); + enumField = registry.findFieldType(messageType, "standalone_enum"); + wrapper = registry.findFieldType(messageType, "single_int32_wrapper"); + repeated = registry.findFieldType(messageType, "repeated_int32"); + map = registry.findFieldType(messageType, "map_string_string"); + + Type outerType = registry.findType(messageType); + Type nestedType = registry.findType("cel.expr.conformance.proto3.TestAllTypes.NestedMessage"); + AttributeFactory attributes = + newAttributeFactory(Container.defaultContainer, registry, registry); + + int32Attribute = attributes.absoluteAttribute(1, "message"); + int32Attribute.addQualifier(attributes.newQualifier(outerType, 2, "single_int32")); + + nestedInt32Attribute = attributes.absoluteAttribute(3, "message"); + nestedInt32Attribute.addQualifier( + attributes.newQualifier(outerType, 4, "single_nested_message")); + nestedInt32Attribute.addQualifier(attributes.newQualifier(nestedType, 5, "bb")); + + EvalAttr presenceOperand = new EvalAttr(registry, attributes.absoluteAttribute(6, "message")); + presence = new EvalTestOnly(7, presenceOperand, optionalBoolName, optionalBool); + activation = newActivation(Map.of("message", message)); + } + } + + @Benchmark + public int directGeneratedInt32(FieldAccessState state) { + return state.message.getSingleInt32(); + } + + @Benchmark + public Object rawGeneratedInt32(FieldAccessState state) { + return state.int32.getFrom.getFrom(state.message); + } + + @Benchmark + public Object rawDynamicFallbackInt32(FieldAccessState state) { + return state.int32.getFrom.getFrom(state.dynamicMessage); + } + + @Benchmark + public Val objectGeneratedInt32(FieldAccessState state) { + return state.object.get(state.int32Name); + } + + @Benchmark + public Val objectDynamicInt32(FieldAccessState state) { + return state.dynamicObject.get(state.int32Name); + } + + @Benchmark + public Object checkedTerminalInt32(FieldAccessState state) { + return state.int32Attribute.resolve(state.activation); + } + + @Benchmark + public Object checkedNestedInt32(FieldAccessState state) { + return state.nestedInt32Attribute.resolve(state.activation); + } + + @Benchmark + public boolean rawPresencePresent(FieldAccessState state) { + return state.optionalBool.isSet.isSet(state.message); + } + + @Benchmark + public boolean rawPresenceAbsent(FieldAccessState state) { + return state.optionalBool.isSet.isSet(TestAllTypes.getDefaultInstance()); + } + + @Benchmark + public Val checkedPresence(FieldAccessState state) { + return state.presence.eval(state.activation); + } + + @Benchmark + public Val objectPresencePresent(FieldAccessState state) { + return state.object.isSet(state.optionalBoolName); + } + + @Benchmark + public Val objectPresenceAbsent(FieldAccessState state) { + return state.absentObject.isSet(state.optionalBoolName); + } + + @Benchmark + public Object rawNestedMessage(FieldAccessState state) { + return state.nestedMessage.getFrom.getFrom(state.message); + } + + @Benchmark + public Val objectNestedMessage(FieldAccessState state) { + return state.object.get(state.nestedMessageName); + } + + @Benchmark + public Object rawUnsigned(FieldAccessState state) { + return state.uint32.getFrom.getFrom(state.message); + } + + @Benchmark + public Val objectUnsigned(FieldAccessState state) { + return state.object.get(state.uint32Name); + } + + @Benchmark + public Object rawEnum(FieldAccessState state) { + return state.enumField.getFrom.getFrom(state.message); + } + + @Benchmark + public Val objectEnum(FieldAccessState state) { + return state.object.get(state.enumName); + } + + @Benchmark + public Object rawWrapper(FieldAccessState state) { + return state.wrapper.getFrom.getFrom(state.message); + } + + @Benchmark + public Val objectWrapper(FieldAccessState state) { + return state.object.get(state.wrapperName); + } + + @Benchmark + public Object rawRepeated(FieldAccessState state) { + return state.repeated.getFrom.getFrom(state.message); + } + + @Benchmark + public Val objectRepeated(FieldAccessState state) { + return state.object.get(state.repeatedName); + } + + @Benchmark + public Object rawMap(FieldAccessState state) { + return state.map.getFrom.getFrom(state.message); + } + + @Benchmark + public Val objectMap(FieldAccessState state) { + return state.object.get(state.mapName); + } +} diff --git a/core/src/main/java/org/projectnessie/cel/common/types/BaseIteratorT.java b/core/src/main/java/org/projectnessie/cel/common/types/BaseIteratorT.java new file mode 100644 index 000000000..9a1d84d4b --- /dev/null +++ b/core/src/main/java/org/projectnessie/cel/common/types/BaseIteratorT.java @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2026 The Authors of CEL-Java + * + * 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 + * + * http://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 org.projectnessie.cel.common.types; + +import org.projectnessie.cel.common.types.ref.BaseVal; + +/** Shared Java object and primitive-access behavior for project-owned iterator cursors. */ +abstract class BaseIteratorT extends BaseVal implements IteratorT { + + @Override + public final boolean booleanValue() { + return IteratorT.super.booleanValue(); + } + + @Override + public final long intValue() { + return IteratorT.super.intValue(); + } + + @Override + public final double doubleValue() { + return IteratorT.super.doubleValue(); + } + + @Override + public final boolean equals(Object other) { + return this == other; + } + + @Override + public final int hashCode() { + return System.identityHashCode(this); + } + + @Override + public final String toString() { + return "iterator"; + } +} diff --git a/core/src/main/java/org/projectnessie/cel/common/types/BoolT.java b/core/src/main/java/org/projectnessie/cel/common/types/BoolT.java index 0f1eb4c13..fd80c5bff 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/BoolT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/BoolT.java @@ -18,7 +18,6 @@ import static org.projectnessie.cel.common.types.Err.newTypeConversionError; import static org.projectnessie.cel.common.types.Err.noSuchOverload; import static org.projectnessie.cel.common.types.IntT.intOfCompare; -import static org.projectnessie.cel.common.types.StringT.stringOf; import com.google.protobuf.Any; import com.google.protobuf.BoolValue; @@ -94,7 +93,7 @@ public T convertToNative(Class typeDesc) { @Override public Val convertToType(Type typeVal) { return switch (typeVal.typeEnum()) { - case String -> stringOf(Boolean.toString(b)); + case String -> b ? StringT.TRUE : StringT.FALSE; case Bool -> this; case Type -> BoolType; default -> newTypeConversionError(BoolType, typeVal); diff --git a/core/src/main/java/org/projectnessie/cel/common/types/BytesT.java b/core/src/main/java/org/projectnessie/cel/common/types/BytesT.java index 7cdeff37d..0a2690c88 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/BytesT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/BytesT.java @@ -176,6 +176,11 @@ public Val size() { return IntT.intOf(b.length); } + @Override + public int nativeSize() { + return b.length; + } + /** Type implements the ref.Val interface method. */ @Override public Type type() { diff --git a/core/src/main/java/org/projectnessie/cel/common/types/DoubleT.java b/core/src/main/java/org/projectnessie/cel/common/types/DoubleT.java index 3510d9afc..e720c12e6 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/DoubleT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/DoubleT.java @@ -58,7 +58,25 @@ public final class DoubleT extends BaseVal Trait.NegatorType, Trait.SubtractorType); + public static final DoubleT DoubleNegOne = new DoubleT(-1d); + public static final DoubleT DoubleNegZero = new DoubleT(-0d); + public static final DoubleT DoubleZero = new DoubleT(0d); + public static final DoubleT DoubleOne = new DoubleT(1d); + public static DoubleT doubleOf(double d) { + var raw = Double.doubleToRawLongBits(d); + if (raw == 0L) { + return DoubleZero; + } + if (raw == 0x8000000000000000L) { + return DoubleNegZero; + } + if (raw == 0x3ff0000000000000L) { + return DoubleOne; + } + if (raw == 0xbff0000000000000L) { + return DoubleNegOne; + } return new DoubleT(d); } @@ -270,7 +288,6 @@ public boolean equals(Object o) { @Override public int hashCode() { - // Used to allow heterogeneous numeric map keys - return (int) d; + return Types.numericHashCode(d); } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/IntT.java b/core/src/main/java/org/projectnessie/cel/common/types/IntT.java index 7c4dc00be..9608fd491 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/IntT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/IntT.java @@ -38,6 +38,7 @@ import com.google.protobuf.Int64Value; import com.google.protobuf.Value; import java.time.Instant; +import java.util.stream.IntStream; import org.projectnessie.cel.common.types.Overflow.OverflowException; import org.projectnessie.cel.common.types.ref.BaseVal; import org.projectnessie.cel.common.types.ref.Type; @@ -68,11 +69,14 @@ public final class IntT extends BaseVal Trait.NegatorType, Trait.SubtractorType); + private static final IntT[] CONST = + IntStream.rangeClosed(-10, 10).mapToObj(IntT::new).toArray(IntT[]::new); + /** Int constants used for comparison results. IntZero is the zero-value for Int */ - public static final IntT IntZero = new IntT(0); + public static final IntT IntZero = CONST[10]; - public static final IntT IntOne = new IntT(1); - public static final IntT IntNegOne = new IntT(-1); + public static final IntT IntOne = CONST[11]; + public static final IntT IntNegOne = CONST[9]; /** maxIntJSON is defined as the Number.MAX_SAFE_INTEGER value per EcmaScript 6. */ public static final long maxIntJSON = (1L << 53) - 1; @@ -97,14 +101,8 @@ public static IntT intOfCompare(int compareToResult) { } public static IntT intOf(long i) { - if (i == 0L) { - return IntZero; - } - if (i == 1L) { - return IntOne; - } - if (i == -1L) { - return IntNegOne; + if (i >= -10 && i <= 10) { + return CONST[(int) i + 10]; } return new IntT(i); } @@ -369,7 +367,6 @@ public boolean equals(Object o) { @Override public int hashCode() { - // Used to allow heterogeneous numeric map keys - return (int) i; + return Types.numericHashCode(i); } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/IterableT.java b/core/src/main/java/org/projectnessie/cel/common/types/IterableT.java index 3109dbe6b..9d2cf4f06 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/IterableT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/IterableT.java @@ -17,6 +17,11 @@ public interface IterableT { - /** Iterator returns a new iterator view of the struct. */ + /** + * Returns a new one-shot cursor over this aggregate. + * + *

Traversal order, source-mutation behavior, and traversal failures are defined by the + * implementing aggregate. The returned cursor is not thread-safe; callers must serialize access. + */ IteratorT iterator(); } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/IteratorT.java b/core/src/main/java/org/projectnessie/cel/common/types/IteratorT.java index bfa6ebb5e..7da5729ba 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/IteratorT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/IteratorT.java @@ -20,25 +20,159 @@ import java.util.Iterator; import java.util.NoSuchElementException; -import org.projectnessie.cel.common.types.ref.BaseVal; +import java.util.Objects; import org.projectnessie.cel.common.types.ref.Type; import org.projectnessie.cel.common.types.ref.TypeAdapter; import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.common.types.traits.Trait; -/** Iterator permits safe traversal over the contents of an aggregate type. */ +/** + * Stateful, one-shot cursor used by the low-level interpreter to traverse an aggregate value. + * + *

This interface extends {@link Val} so cursors can pass through interpreter operations. An + * iterator is not a first-class CEL value: CEL expressions cannot name or construct one, and only + * traversal, runtime type inspection, and the diagnostic behavior defined here are supported. + * + *

Iterators are not thread-safe or reusable. Callers must serialize access and must not mutate + * the source aggregate while traversing it. Traversal order, source-mutation behavior, and + * non-exhaustion failures are defined by the source aggregate or wrapped Java iterator. + * + *

Direct implementations need only implement {@link #hasNext()} and {@link #next()}. A class + * that also extends a superclass with concrete {@link Val} methods must account for Java's + * superclass-before-interface default-method precedence. In particular, an implementation extending + * {@link org.projectnessie.cel.common.types.ref.BaseVal} must override its primitive accessors and + * its payload-based {@link Object#equals(Object)}, {@link Object#hashCode()}, and {@link + * Object#toString()} methods. + */ public interface IteratorT extends Val { + /** + * Runtime type used for low-level iterator trait dispatch. + * + *

This object-kind type is not a CEL language type and is not supported as an expression-level + * conversion target. + */ + Type IteratorType = TypeT.newObjectTypeValue("iterator", Trait.IteratorType); + /** + * Wraps a Java iterator as a CEL interpreter cursor. + * + *

Elements already implementing {@link Val} are returned unchanged; other elements, including + * {@code null}, are converted through {@code adapter}. A Java {@link NoSuchElementException} is + * represented by the CEL no-more-elements error. Other delegate and adapter failures propagate + * unchanged. + * + * @param adapter adapter used for native elements + * @param iterator Java iterator to wrap + * @return a new one-shot cursor + * @throws NullPointerException if either argument is {@code null} + */ static IteratorT javaIterator(TypeAdapter adapter, Iterator iterator) { - return new JavaIteratorT(adapter, iterator); + return new JavaIteratorT( + Objects.requireNonNull(adapter, "adapter"), Objects.requireNonNull(iterator, "iterator")); } - /** HasNext returns true if there are unvisited elements in the Iterator. */ + /** + * Reports whether an unvisited element remains without advancing the cursor. + * + * @return a CEL boolean value + */ Val hasNext(); - /** Next returns the next element. */ + /** + * Advances the cursor once and returns the next element. + * + * @return the next value, or a CEL no-more-elements error after exhaustion + */ Val next(); - final class JavaIteratorT extends BaseVal implements IteratorT { + /** + * Returns the stable low-level iterator runtime type without advancing the cursor. + * + * @return {@link #IteratorType} + */ + @Override + default Type type() { + return IteratorType; + } + + /** + * Returns no data payload because an iterator is interpreter control state. + * + * @return always {@code null} + */ + @Override + default Object value() { + return null; + } + + /** + * Returns a CEL no-such-overload error without observing or advancing either cursor. + * + * @param other value presented for comparison + * @return a CEL error + */ + @Override + default Val equal(Val other) { + return Err.noSuchOverload(this, "equal", other); + } + + /** + * Returns a CEL conversion error because iterators are not first-class CEL values. + * + * @param typeValue requested CEL type + * @return a CEL error + */ + @Override + default Val convertToType(Type typeValue) { + return Err.newTypeConversionError(IteratorType, typeValue); + } + + /** + * Native conversion is unsupported because an iterator has no CEL data payload. + * + * @param typeDesc requested native type + * @param requested native type + * @return never returns normally + * @throws UnsupportedOperationException always + */ + @Override + @SuppressWarnings("removal") + default T convertToNative(Class typeDesc) { + throw new UnsupportedOperationException("type conversion not supported for 'iterator'"); + } + + /** + * Returns no primitive boolean because an iterator is interpreter control state. + * + * @throws UnsupportedOperationException always + */ + @Override + default boolean booleanValue() { + throw new UnsupportedOperationException("iterator cannot be used as boolean"); + } + + /** + * Returns no primitive integer because an iterator is interpreter control state. + * + * @throws UnsupportedOperationException always + */ + @Override + default long intValue() { + throw new UnsupportedOperationException("iterator cannot be used as integer"); + } + + /** + * Returns no primitive double because an iterator is interpreter control state. + * + * @throws UnsupportedOperationException always + */ + @Override + default double doubleValue() { + throw new UnsupportedOperationException("iterator cannot be used as double"); + } + + /** Built-in cursor returned by {@link #javaIterator(TypeAdapter, Iterator)}. */ + final class JavaIteratorT extends BaseIteratorT { private final TypeAdapter adapter; private final Iterator iterator; @@ -65,31 +199,5 @@ public Val next() { } return adapter.nativeToValue(n); } - - @Override - @SuppressWarnings("removal") - public T convertToNative(Class typeDesc) { - throw new UnsupportedOperationException(); - } - - @Override - public Val convertToType(Type typeValue) { - throw new UnsupportedOperationException(); - } - - @Override - public Val equal(Val other) { - throw new UnsupportedOperationException(); - } - - @Override - public Type type() { - throw new UnsupportedOperationException(); - } - - @Override - public Object value() { - throw new UnsupportedOperationException(); - } } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/ListT.java b/core/src/main/java/org/projectnessie/cel/common/types/ListT.java index 7a0abb74d..ae54cbe9d 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/ListT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/ListT.java @@ -45,6 +45,13 @@ import org.projectnessie.cel.common.types.traits.Lister; import org.projectnessie.cel.common.types.traits.Trait; +/** + * Base class and factories for CEL list values. + * + *

Factories adapt elements lazily where possible. Array-backed lists retain their source array, + * and {@link #newGenericList(TypeAdapter, List)} retains a live Java-list view. Callers must keep + * retained sources stable while a CEL operation or program evaluation consumes them. + */ public abstract class ListT extends BaseVal implements Lister { /** ListType singleton. */ public static final Type ListType = @@ -79,6 +86,12 @@ public static Val newGenericArrayList(TypeAdapter adapter, Object[] value) { return new GenericListT(adapter, value); } + /** + * Returns a CEL list backed by a live view of {@code value}. + * + *

Mutations completed between CEL operations are visible through the returned value. The + * caller must not mutate the list while a CEL operation or program evaluation is consuming it. + */ public static Val newGenericList(TypeAdapter adapter, List value) { return new ListBackedListT(adapter, value); } @@ -106,26 +119,26 @@ public Type type() { abstract static class BaseListT extends ListT { protected final TypeAdapter adapter; - protected final long size; - BaseListT(TypeAdapter adapter, long size) { + BaseListT(TypeAdapter adapter) { this.adapter = adapter; - this.size = size; } final Val concatValues(int thisSize, Lister other) { // Concatenation is CEL evaluation; outbound conversion belongs to result export. - int otherSize = (int) other.size().intValue(); + int otherSize = other.nativeSize(); Val[] values = new Val[thisSize + otherSize]; for (int i = 0; i < thisSize; i++) { - values[i] = get(intOf(i)); + values[i] = getUnchecked(i); } for (int i = 0; i < otherSize; i++) { - values[thisSize + i] = other.get(intOf(i)); + values[thisSize + i] = elementAt(other, i); } return new ValListT(adapter, values, true); } + abstract int currentSize(); + @SuppressWarnings("removal") @Override public T convertToNative(Class typeDesc) { @@ -177,9 +190,9 @@ private Value toPbValue(TypeAdapter adapter) { private ListValue toPbListValue(TypeAdapter adapter) { ListValue.Builder list = ListValue.newBuilder(); - int s = (int) size; + int s = nativeSize(); for (int i = 0; i < s; i++) { - Val v = get(intOf(i)); + Val v = getUnchecked(i); Value e = adapter.valueToNative(v, Value.class); list.addValues(e); } @@ -192,7 +205,7 @@ private List toJavaList(TypeAdapter adapter) { @SuppressWarnings({"rawtypes", "unchecked"}) private Object toJavaArray(TypeAdapter adapter, Class typeDesc) { - int s = (int) size; + int s = nativeSize(); Class compType = typeDesc.getComponentType(); if (compType == Enum.class) { // Note: cannot create `Enum` values of the right type here. @@ -201,7 +214,7 @@ private Object toJavaArray(TypeAdapter adapter, Class typeDesc) { Object array = Array.newInstance(compType, s); for (int i = 0; i < s; i++) { - Val v = get(intOf(i)); + Val v = getUnchecked(i); Object e = adapter.valueToNative(v, compType); Array.set(array, i, e); } @@ -219,25 +232,42 @@ public Val convertToType(Type typeValue) { @Override public IteratorT iterator() { - return new ArrayListIteratorT(); + return new ArrayListIteratorT(nativeSize()); + } + + abstract Val getUnchecked(int index); + + static Val elementAt(Lister list, int index) { + return list instanceof BaseListT baseList + ? baseList.getUnchecked(index) + : list.nativeGetAt(index); + } + + @Override + public Val nativeGetAt(int index) { + int currentSize = nativeSize(); + if (index < 0 || index >= currentSize) { + return newErr( + "invalid_argument: index '%d' out of range in list of size '%d'", index, currentSize); + } + return getUnchecked(index); } @Override public Val equal(Val other) { - if (other.type() != ListType) { + if (!(other instanceof ListT o)) { return False; } - ListT o = (ListT) other; - if (size != o.size().intValue()) { + int currentSize = nativeSize(); + if (currentSize != o.nativeSize()) { return False; } - for (long i = 0; i < size; i++) { - IntT idx = intOf(i); - Val e1 = get(idx); + for (int i = 0; i < currentSize; i++) { + Val e1 = getUnchecked(i); if (isError(e1)) { return e1; } - Val e2 = o.get(idx); + Val e2 = elementAt(o, i); if (isError(e2)) { return e2; } @@ -256,8 +286,9 @@ public Val equal(Val other) { @Override public Val contains(Val value) { - for (long i = 0; i < size; i++) { - Val elem = get(intOf(i)); + int currentSize = nativeSize(); + for (int i = 0; i < currentSize; i++) { + Val elem = getUnchecked(i); if (value.equal(elem) == True) { return True; } @@ -267,7 +298,12 @@ public Val contains(Val value) { @Override public Val size() { - return intOf(size); + return intOf(nativeSize()); + } + + @Override + public final int nativeSize() { + return currentSize(); } @Override @@ -275,17 +311,18 @@ public boolean equals(Object o) { if (this == o) { return true; } - if (!(o instanceof Val)) { + if (!(o instanceof Val val)) { return false; } - return equal((Val) o) == True; + return equal(val) == True; } @Override public int hashCode() { int result = 1; - for (long i = 0; i < size; i++) { - result = 31 * result + get(intOf(i)).hashCode(); + int currentSize = nativeSize(); + for (int i = 0; i < currentSize; i++) { + result = 31 * result + getUnchecked(i).hashCode(); } return result; } @@ -305,17 +342,23 @@ int checkedIndex(Val index, int size) { throw new InvalidIndexException( valOrErr(index, "unsupported index type '%s' in list", index.type())); } - int i = (int) index.intValue(); - if (i < 0 || i >= size) { + long longIndex = index.intValue(); + if (longIndex < 0 || longIndex >= size) { // Note: the conformance tests assert on 'invalid_argument' throw new InvalidIndexException( - newErr("invalid_argument: index '%d' out of range in list of size '%d'", i, size)); + newErr( + "invalid_argument: index '%d' out of range in list of size '%d'", longIndex, size)); } - return i; + return (int) longIndex; } - private final class ArrayListIteratorT extends BaseVal implements IteratorT { - private long index; + private final class ArrayListIteratorT extends BaseIteratorT { + private final int size; + private int index; + + private ArrayListIteratorT(int size) { + this.size = size; + } @Override public Val hasNext() { @@ -325,36 +368,10 @@ public Val hasNext() { @Override public Val next() { if (index < size) { - return get(intOf(index++)); + return getUnchecked(index++); } return noMoreElements(); } - - @Override - @SuppressWarnings("removal") - public T convertToNative(Class typeDesc) { - throw new UnsupportedOperationException("IMPLEMENT ME??"); - } - - @Override - public Val convertToType(Type typeValue) { - throw new UnsupportedOperationException("IMPLEMENT ME??"); - } - - @Override - public Val equal(Val other) { - throw new UnsupportedOperationException("IMPLEMENT ME??"); - } - - @Override - public Type type() { - throw new UnsupportedOperationException("IMPLEMENT ME??"); - } - - @Override - public Object value() { - throw new UnsupportedOperationException("IMPLEMENT ME??"); - } } } @@ -375,10 +392,15 @@ static final class GenericListT extends BaseListT { private final Object[] array; GenericListT(TypeAdapter adapter, Object[] array) { - super(adapter, array.length); + super(adapter); this.array = array; } + @Override + int currentSize() { + return array.length; + } + @Override public Object value() { return array; @@ -401,7 +423,12 @@ public Val get(Val index) { return e.error; } - return adapter.nativeToValue(array[i]); + return getUnchecked(i); + } + + @Override + Val getUnchecked(int index) { + return adapter.nativeToValue(array[index]); } @Override @@ -412,7 +439,7 @@ public String toString() { + ", adapter=" + adapter + ", size=" - + size + + nativeSize() + '}'; } } @@ -421,10 +448,15 @@ static final class ListBackedListT extends BaseListT { private final List list; ListBackedListT(TypeAdapter adapter, List list) { - super(adapter, list.size()); + super(adapter); this.list = list; } + @Override + int currentSize() { + return list.size(); + } + @Override public Object value() { return list; @@ -442,12 +474,17 @@ public Val add(Val other) { public Val get(Val index) { int i; try { - i = checkedIndex(index, list.size()); + i = checkedIndex(index, nativeSize()); } catch (InvalidIndexException e) { return e.error; } - return adapter.nativeToValue(list.get(i)); + return getUnchecked(i); + } + + @Override + Val getUnchecked(int index) { + return adapter.nativeToValue(list.get(index)); } } @@ -461,11 +498,16 @@ static final class ValListT extends BaseListT { } ValListT(TypeAdapter adapter, Val[] array, boolean preserveUnsignedRepresentation) { - super(adapter, array.length); + super(adapter); this.array = array; this.preserveUnsignedRepresentation = preserveUnsignedRepresentation; } + @Override + int currentSize() { + return array.length; + } + @Override public Object value() { Object[] nativeArray = new Object[array.length]; @@ -486,16 +528,16 @@ public Val add(Val other) { boolean preserveUnsigned = preserveUnsignedRepresentation || other instanceof ValListT values && values.preserveUnsignedRepresentation; - if (other instanceof ValListT) { - Val[] otherArray = ((ValListT) other).array; + if (other instanceof ValListT valListT) { + Val[] otherArray = valListT.array; Val[] newArray = Arrays.copyOf(array, array.length + otherArray.length); System.arraycopy(otherArray, 0, newArray, array.length, otherArray.length); return new ValListT(adapter, newArray, preserveUnsigned); } else { - int otherSIze = (int) otherLister.size().intValue(); - Val[] newArray = Arrays.copyOf(array, array.length + otherSIze); - for (int i = 0; i < otherSIze; i++) { - newArray[array.length + i] = otherLister.get(intOf(i)); + int otherSize = otherLister.nativeSize(); + Val[] newArray = Arrays.copyOf(array, array.length + otherSize); + for (int i = 0; i < otherSize; i++) { + newArray[array.length + i] = elementAt(otherLister, i); } return new ValListT(adapter, newArray, preserveUnsigned); } @@ -509,7 +551,12 @@ public Val get(Val index) { } catch (InvalidIndexException e) { return e.error; } - return array[i]; + return getUnchecked(i); + } + + @Override + Val getUnchecked(int index) { + return array[index]; } @Override @@ -520,14 +567,14 @@ public String toString() { + ", adapter=" + adapter + ", size=" - + size + + nativeSize() + '}'; } } abstract static class PrimitiveArrayListT extends BaseListT { - PrimitiveArrayListT(TypeAdapter adapter, long size) { - super(adapter, size); + PrimitiveArrayListT(TypeAdapter adapter) { + super(adapter); } @Override @@ -535,14 +582,14 @@ public Val add(Val other) { if (!(other instanceof Lister otherLister)) { return noSuchOverload(this, "add", other); } - int thisSize = (int) size; - int otherSize = (int) otherLister.size().intValue(); + int thisSize = nativeSize(); + int otherSize = otherLister.nativeSize(); Val[] newArray = new Val[thisSize + otherSize]; for (int i = 0; i < thisSize; i++) { - newArray[i] = get(intOf(i)); + newArray[i] = getUnchecked(i); } for (int i = 0; i < otherSize; i++) { - newArray[thisSize + i] = otherLister.get(intOf(i)); + newArray[thisSize + i] = elementAt(otherLister, i); } return new ValListT(adapter, newArray); } @@ -552,10 +599,15 @@ static final class IntArrayListT extends PrimitiveArrayListT { private final int[] array; IntArrayListT(TypeAdapter adapter, int[] array) { - super(adapter, array.length); + super(adapter); this.array = array; } + @Override + int currentSize() { + return array.length; + } + @Override public Object value() { return array; @@ -569,7 +621,49 @@ public Val get(Val index) { } catch (InvalidIndexException e) { return e.error; } - return intOf(array[i]); + return getUnchecked(i); + } + + @Override + Val getUnchecked(int index) { + return intOf(array[index]); + } + + @Override + public Val contains(Val value) { + if (value instanceof IntT intValue) { + long needle = intValue.intValue(); + if (needle >= Integer.MIN_VALUE && needle <= Integer.MAX_VALUE) { + int intNeedle = (int) needle; + for (int element : array) { + if (element == intNeedle) { + return True; + } + } + return False; + } + return False; + } + return super.contains(value); + } + + @Override + public Val equal(Val other) { + if (other instanceof IntArrayListT intList) { + return boolOf(Arrays.equals(array, intList.array)); + } + if (other instanceof LongArrayListT longList) { + if (array.length != longList.array.length) { + return False; + } + for (int i = 0; i < array.length; i++) { + if (array[i] != longList.array[i]) { + return False; + } + } + return True; + } + return super.equal(other); } } @@ -577,10 +671,15 @@ static final class LongArrayListT extends PrimitiveArrayListT { private final long[] array; LongArrayListT(TypeAdapter adapter, long[] array) { - super(adapter, array.length); + super(adapter); this.array = array; } + @Override + int currentSize() { + return array.length; + } + @Override public Object value() { return array; @@ -594,7 +693,37 @@ public Val get(Val index) { } catch (InvalidIndexException e) { return e.error; } - return intOf(array[i]); + return getUnchecked(i); + } + + @Override + Val getUnchecked(int index) { + return intOf(array[index]); + } + + @Override + public Val contains(Val value) { + if (value instanceof IntT intValue) { + long needle = intValue.intValue(); + for (long element : array) { + if (element == needle) { + return True; + } + } + return False; + } + return super.contains(value); + } + + @Override + public Val equal(Val other) { + if (other instanceof LongArrayListT longList) { + return boolOf(Arrays.equals(array, longList.array)); + } + if (other instanceof IntArrayListT intList) { + return intList.equal(this); + } + return super.equal(other); } } @@ -602,10 +731,15 @@ static final class DoubleArrayListT extends PrimitiveArrayListT { private final double[] array; DoubleArrayListT(TypeAdapter adapter, double[] array) { - super(adapter, array.length); + super(adapter); this.array = array; } + @Override + int currentSize() { + return array.length; + } + @Override public Object value() { return array; @@ -619,7 +753,42 @@ public Val get(Val index) { } catch (InvalidIndexException e) { return e.error; } - return doubleOf(array[i]); + return getUnchecked(i); + } + + @Override + Val getUnchecked(int index) { + return doubleOf(array[index]); + } + + @Override + public Val contains(Val value) { + if (value instanceof DoubleT doubleValue) { + double needle = doubleValue.doubleValue(); + for (double element : array) { + if (element == needle) { + return True; + } + } + return False; + } + return super.contains(value); + } + + @Override + public Val equal(Val other) { + if (other instanceof DoubleArrayListT doubleList) { + if (array.length != doubleList.array.length) { + return False; + } + for (int i = 0; i < array.length; i++) { + if (array[i] != doubleList.array[i]) { + return False; + } + } + return True; + } + return super.equal(other); } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/MapT.java b/core/src/main/java/org/projectnessie/cel/common/types/MapT.java index 2ffcf3ac8..5e1894a1b 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/MapT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/MapT.java @@ -22,13 +22,13 @@ import static org.projectnessie.cel.common.types.Err.newTypeConversionError; import static org.projectnessie.cel.common.types.TypeT.TypeType; import static org.projectnessie.cel.common.types.Types.boolOf; +import static org.projectnessie.cel.common.types.Util.isUnknownOrError; import com.google.protobuf.Any; import com.google.protobuf.Struct; import com.google.protobuf.Value; import java.util.HashMap; import java.util.Map; -import java.util.Objects; import org.projectnessie.cel.common.types.ref.BaseVal; import org.projectnessie.cel.common.types.ref.Type; import org.projectnessie.cel.common.types.ref.TypeAdapter; @@ -40,6 +40,14 @@ import org.projectnessie.cel.common.types.traits.Sizer; import org.projectnessie.cel.common.types.traits.Trait; +/** + * Base class and factories for CEL map values. + * + *

Map keys are CEL values and obey CEL equality. Factory ownership differs by input: already + * adapted maps can remain live views, while ordinary Java maps are re-keyed into an internal map + * and retain their raw values for lazy adaptation. Source keys and equality/hash-relevant objects + * must remain stable while the resulting map is used. + */ public abstract class MapT extends BaseVal implements Mapper, Container, Indexer, IterableT, Sizer { /** MapType singleton. */ public static final Type MapType = @@ -65,6 +73,11 @@ public T toNative(TypeAdapter adapter, Class targetType) { return convertToNative(targetType); } + /** + * Creates a CEL map from either already adapted entries or ordinary Java entries. + * + * @return the map value, or a CEL error for null or CEL-equivalent duplicate keys + */ @SuppressWarnings("unchecked") public static Val newMaybeWrappedMap(TypeAdapter adapter, Map value) { boolean alreadyWrapped = true; @@ -81,19 +94,20 @@ public static Val newMaybeWrappedMap(TypeAdapter adapter, Map value) { return newWrappedMap(adapter, (Map) value); } - Map newMap = new HashMap<>(value.size() * 4 / 3 + 1); + Map newMap = new HashMap<>(value.size() * 4 / 3 + 1); for (Map.Entry entry : value.entrySet()) { Val k = adapter.nativeToValue(entry.getKey()); - Val v = adapter.nativeToValue(entry.getValue()); if (k.type().typeEnum() == TypeEnum.Null) { return newErr("unsupported key type"); } - if (newMap.putIfAbsent(k, v) != null) { + int previousSize = newMap.size(); + newMap.put(k, entry.getValue()); + if (newMap.size() == previousSize) { // Prevent duplicate keys, error out. return newErr("Failed with repeated key"); } } - return newWrappedMap(adapter, newMap); + return new NativeMapT(adapter, newMap); } public static boolean isSupportedLiteralKeyType(Val key) { @@ -108,6 +122,110 @@ public Type type() { return MapType; } + protected final Val mapEqual(Val other) { + if (!(other instanceof MapT o)) { + return False; + } + if (nativeSize() != o.nativeSize()) { + return False; + } + + Iterable entries = mapEntries(); + if (entries != null) { + for (Object entry : entries) { + Val key = mapEntryKey(entry); + Val value = mapEntryValue(entry); + Val oVal = o.find(key); + if (oVal == null) { + return False; + } + Val equal = mapValueEqual(value, oVal); + if (equal != True) { + return equal; + } + } + return True; + } + + IteratorT myIter = iterator(); + while (myIter.hasNext() == True) { + Val key = myIter.next(); + + Val val = get(key); + Val oVal = o.find(key); + if (oVal == null) { + return False; + } + Val eq = mapValueEqual(val, oVal); + if (eq == True) { + continue; + } + return eq; + } + return True; + } + + /** Returns backing entries when this implementation can traverse them directly. */ + protected Iterable mapEntries() { + return null; + } + + /** Adapts the key of an entry returned by {@link #mapEntries()}. */ + protected Val mapEntryKey(Object entry) { + throw new UnsupportedOperationException(); + } + + /** Adapts the value of an entry returned by {@link #mapEntries()}. */ + protected Val mapEntryValue(Object entry) { + throw new UnsupportedOperationException(); + } + + protected static Val mapValueEqual(Val value, Val otherValue) { + if (isError(value)) { + return value; + } + if (isError(otherValue)) { + return otherValue; + } + + Val equal = value.equal(otherValue); + if (equal == True || equal == False) { + return equal; + } + if (!value.type().equals(otherValue.type())) { + return False; + } + equal = value.equal(otherValue); + return equal instanceof Err ? equal : boolOf(equal == True); + } + + @Override + public boolean equals(Object other) { + return this == other || other instanceof MapT o && mapEqual(o) == True; + } + + @Override + public int hashCode() { + int hash = 0; + Iterable entries = mapEntries(); + if (entries != null) { + for (Object entry : entries) { + Val key = mapEntryKey(entry); + Val value = mapEntryValue(entry); + hash += key.hashCode() ^ (value != null ? value.hashCode() : 0); + } + return hash; + } + + IteratorT keys = iterator(); + while (keys.hasNext() == True) { + Val key = keys.next(); + Val value = find(key); + hash += key.hashCode() ^ (value != null ? value.hashCode() : 0); + } + return hash; + } + static final class ValMapT extends MapT { private final TypeAdapter adapter; @@ -195,49 +313,7 @@ public IteratorT iterator() { @Override public Val equal(Val other) { - // TODO this is expensive :( - if (!(other instanceof MapT o)) { - return False; - } - if (!size().equal(o.size()).booleanValue()) { - return False; - } - IteratorT myIter = iterator(); - while (myIter.hasNext() == True) { - Val key = myIter.next(); - - Val val = get(key); - Val oVal = o.find(key); - if (oVal == null) { - return False; - } - if (isError(val)) { - return val; - } - if (isError(oVal)) { - return val; - } - - Val eq = val.equal(oVal); - if (eq == True) { - continue; - } - if (eq == False) { - return False; - } - - if (!val.type().equals(oVal.type())) { - return False; - } - eq = val.equal(oVal); - if (eq instanceof Err) { - return eq; - } - if (eq != True) { - return False; - } - } - return True; + return mapEqual(other); } @Override @@ -250,6 +326,9 @@ public Object value() { @Override public Val contains(Val value) { + if (isUnknownOrError(value)) { + return value; + } return boolOf(find(value) != null); } @@ -263,6 +342,26 @@ public Val size() { return IntT.intOf(map.size()); } + @Override + public int nativeSize() { + return map.size(); + } + + @Override + protected Iterable mapEntries() { + return map.entrySet(); + } + + @Override + protected Val mapEntryKey(Object entry) { + return (Val) ((Map.Entry) entry).getKey(); + } + + @Override + protected Val mapEntryValue(Object entry) { + return (Val) ((Map.Entry) entry).getValue(); + } + @Override public Val find(Val key) { // Note: no special handling for heterogenous numeric map keys needed, the Val type @@ -271,25 +370,133 @@ public Val find(Val key) { } @Override - public boolean equals(Object o) { - if (this == o) { - return true; + public String toString() { + return "JavaMapT{" + "adapter=" + adapter + ", map=" + map + '}'; + } + } + + static final class NativeMapT extends MapT { + private final TypeAdapter adapter; + private final Map map; + + NativeMapT(TypeAdapter adapter, Map map) { + this.adapter = adapter; + this.map = map; + } + + @SuppressWarnings("removal") + @Override + public T convertToNative(Class typeDesc) { + return toNative(adapter, typeDesc); + } + + @Override + public T toNative(TypeAdapter conversionAdapter, Class typeDesc) { + return conversionAdapter.valueToNative(newWrappedMap(adapter, adaptedMap()), typeDesc); + } + + @Override + public Val convertToType(Type typeValue) { + if (typeValue == MapType) { + return this; + } + if (typeValue == TypeType) { + return MapType; + } + return newTypeConversionError(MapType, typeValue); + } + + @Override + public IteratorT iterator() { + return IteratorT.javaIterator(adapter, map.keySet().iterator()); + } + + @Override + public Val equal(Val other) { + if (other instanceof NativeMapT nativeOther) { + if (map.size() != nativeOther.map.size()) { + return False; + } + for (Map.Entry entry : map.entrySet()) { + Object otherRawValue = nativeOther.map.get(entry.getKey()); + if (otherRawValue == null && !nativeOther.map.containsKey(entry.getKey())) { + return False; + } + Val equal = + mapValueEqual( + adapter.nativeToValue(entry.getValue()), + nativeOther.adapter.nativeToValue(otherRawValue)); + if (equal != True) { + return equal; + } + } + return True; } - if (o == null || getClass() != o.getClass()) { - return false; + return mapEqual(other); + } + + @Override + public Object value() { + Map nativeMap = new HashMap<>(map.size() * 4 / 3 + 1); + map.forEach( + (key, rawValue) -> nativeMap.put(key.value(), adapter.nativeToValue(rawValue).value())); + return nativeMap; + } + + @Override + public Val contains(Val value) { + if (isUnknownOrError(value)) { + return value; } - ValMapT valMapT = (ValMapT) o; - return Objects.equals(map, valMapT.map); + Object rawValue = map.get(value); + return boolOf(rawValue != null || map.containsKey(value)); + } + + @Override + public Val get(Val index) { + return find(index); + } + + @Override + public Val size() { + return IntT.intOf(map.size()); + } + + @Override + public int nativeSize() { + return map.size(); + } + + @Override + protected Iterable mapEntries() { + return map.entrySet(); } @Override - public int hashCode() { - return Objects.hash(super.hashCode(), map); + protected Val mapEntryKey(Object entry) { + return (Val) ((Map.Entry) entry).getKey(); + } + + @Override + protected Val mapEntryValue(Object entry) { + return adapter.nativeToValue(((Map.Entry) entry).getValue()); + } + + @Override + public Val find(Val key) { + Object rawValue = map.get(key); + return rawValue != null || map.containsKey(key) ? adapter.nativeToValue(rawValue) : null; + } + + private Map adaptedMap() { + Map adapted = new HashMap<>(map.size() * 4 / 3 + 1); + map.forEach((key, rawValue) -> adapted.put(key, adapter.nativeToValue(rawValue))); + return adapted; } @Override public String toString() { - return "JavaMapT{" + "adapter=" + adapter + ", map=" + map + '}'; + return "NativeMapT{" + "adapter=" + adapter + ", map=" + map + '}'; } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/NullT.java b/core/src/main/java/org/projectnessie/cel/common/types/NullT.java index 10c56a3c0..423b56bc1 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/NullT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/NullT.java @@ -19,7 +19,6 @@ import static org.projectnessie.cel.common.types.BoolT.True; import static org.projectnessie.cel.common.types.Err.newTypeConversionError; import static org.projectnessie.cel.common.types.Err.noSuchOverload; -import static org.projectnessie.cel.common.types.StringT.stringOf; import com.google.protobuf.Any; import com.google.protobuf.Value; @@ -83,7 +82,7 @@ public T convertToNative(Class typeDesc) { @Override public Val convertToType(Type typeValue) { return switch (typeValue.typeEnum()) { - case String -> stringOf("null"); + case String -> StringT.NULL; case Null -> this; case Type -> NullType; default -> newTypeConversionError(NullType, typeValue); diff --git a/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java b/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java index f816c7e32..f1b1a6425 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java @@ -137,15 +137,15 @@ public Val isSet(Val field) { if (!present) { return False; } - if (value instanceof OptionalT) { - return ((OptionalT) value).isSet(field); + if (value instanceof OptionalT optionalT) { + return optionalT.isSet(field); } - if (value instanceof FieldTester) { - Val present = ((FieldTester) value).isSet(field); + if (value instanceof FieldTester fieldTester) { + Val present = fieldTester.isSet(field); return isMissingAccess(present) ? False : present; } - if (value instanceof Container) { - return ((Container) value).contains(field); + if (value instanceof Container container) { + return container.contains(field); } return noSuchOverload(value, "has", field); } @@ -201,19 +201,18 @@ private static boolean isZeroValue(Val value) { value.value() instanceof ZonedDateTime timestamp && timestamp.toInstant().equals(Instant.EPOCH); case String, Bytes, List, Map -> - value.type().hasTrait(Trait.SizerType) && ((Sizer) value).size().equal(IntZero) == True; - case Object -> - value.value() instanceof Message && ((Message) value.value()).getAllFields().isEmpty(); + value instanceof Sizer sizer && sizer.size().equal(IntZero) == True; + case Object -> value.value() instanceof Message message && message.getAllFields().isEmpty(); default -> false; }; } private static Val optionalAccess(Val operand, Val index) { - if (operand instanceof OptionalT) { - return ((OptionalT) operand).get(index); + if (operand instanceof OptionalT optionalT) { + return optionalT.get(index); } - if (operand instanceof FieldTester && index.type().typeEnum() == TypeEnum.String) { - Val present = ((FieldTester) operand).isSet(index); + if (operand instanceof FieldTester fieldTester && index.type().typeEnum() == TypeEnum.String) { + Val present = fieldTester.isSet(index); if (present == False) { return none(); } @@ -221,12 +220,12 @@ private static Val optionalAccess(Val operand, Val index) { return isMissingAccess(present) ? none() : present; } } - if (operand instanceof Mapper) { - Val value = ((Mapper) operand).find(index); + if (operand instanceof Mapper mapper) { + Val value = mapper.find(index); return value == null ? none() : of(value); } - if (operand instanceof Indexer) { - Val value = ((Indexer) operand).get(index); + if (operand instanceof Indexer indexer) { + Val value = indexer.get(index); if (isMissingAccess(value)) { return none(); } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/StringT.java b/core/src/main/java/org/projectnessie/cel/common/types/StringT.java index c4c22af7c..3587bec82 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/StringT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/StringT.java @@ -70,6 +70,10 @@ public final class StringT extends BaseVal implements Adder, Comparer, Matcher, Overloads.StartsWith, StringT::stringStartsWith); + static final StringT TRUE = stringOf("true"); + static final StringT FALSE = stringOf("false"); + static final StringT NULL = stringOf("null"); + public static StringT stringOf(String s) { return new StringT(s); } @@ -83,10 +87,10 @@ private StringT(String s) { /** Add implements traits.Adder.Add. */ @Override public Val add(Val other) { - if (!(other instanceof StringT)) { + if (!(other instanceof StringT o)) { return noSuchOverload(this, "add", other); } - return new StringT(s + ((StringT) other).s); + return new StringT(s + o.s); } /** ConvertToNative implements ref.Val.ConvertToNative. */ @@ -184,11 +188,11 @@ public Val equal(Val other) { /** Match implements traits.Matcher.Match. */ @Override public Val match(Val pattern) { - if (!(pattern instanceof StringT)) { + if (!(pattern instanceof StringT o)) { return noSuchOverload(this, "match", pattern); } try { - Pattern p = Pattern.compile(((StringT) pattern).s); + Pattern p = Pattern.compile(o.s); java.util.regex.Matcher m = p.matcher(s); return boolOf(m.find()); } catch (Exception e) { @@ -214,6 +218,11 @@ public Val size() { return intOf(s.codePointCount(0, s.length())); } + @Override + public int nativeSize() { + return s.codePointCount(0, s.length()); + } + /** Type implements ref.Val.Type. */ @Override public Type type() { @@ -244,23 +253,23 @@ public int hashCode() { } static Val stringContains(String s, Val sub) { - if (!(sub instanceof StringT)) { + if (!(sub instanceof StringT str)) { return noSuchOverload(StringType, "contains", sub); } - return boolOf(s.contains(((StringT) sub).s)); + return boolOf(s.contains(str.s)); } static Val stringEndsWith(String s, Val suf) { - if (!(suf instanceof StringT)) { + if (!(suf instanceof StringT str)) { return noSuchOverload(StringType, "endsWith", suf); } - return boolOf(s.endsWith(((StringT) suf).s)); + return boolOf(s.endsWith(str.s)); } static Val stringStartsWith(String s, Val pre) { - if (!(pre instanceof StringT)) { + if (!(pre instanceof StringT str)) { return noSuchOverload(StringType, "startsWith", pre); } - return boolOf(s.startsWith(((StringT) pre).s)); + return boolOf(s.startsWith(str.s)); } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/Types.java b/core/src/main/java/org/projectnessie/cel/common/types/Types.java index 601ca50d6..b2a3e4b28 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/Types.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/Types.java @@ -59,4 +59,22 @@ public static Type getTypeByName(String typeName) { public static BoolT boolOf(boolean b) { return b ? BoolT.True : BoolT.False; } + + static int numericHashCode(long value) { + return numericHashCode((double) value); + } + + static int unsignedNumericHashCode(long value) { + // Round the unsigned magnitude once, keeping a sticky low bit before scaling back up. + double doubleValue = + value >= 0 ? (double) value : (double) ((value >>> 1) | (value & 1L)) * 2.0d; + return numericHashCode(doubleValue); + } + + static int numericHashCode(double value) { + // CEL numeric equality compares signed and unsigned integers after conversion to double when + // either operand is a double. Use the same canonical representation for Java hash-based maps. + // Canonicalize signed zero because CEL considers -0.0 and 0.0 equal. + return Double.hashCode(value == 0.0d ? 0.0d : value); + } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/UintT.java b/core/src/main/java/org/projectnessie/cel/common/types/UintT.java index 2d2f8ee76..569abb980 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/UintT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/UintT.java @@ -36,6 +36,7 @@ import com.google.protobuf.UInt64Value; import com.google.protobuf.Value; import java.math.BigInteger; +import java.util.stream.IntStream; import org.projectnessie.cel.common.ULong; import org.projectnessie.cel.common.types.Overflow.OverflowException; import org.projectnessie.cel.common.types.ref.BaseVal; @@ -65,16 +66,19 @@ public final class UintT extends BaseVal Trait.MultiplierType, Trait.SubtractorType); + private static final UintT[] CONST = + IntStream.rangeClosed(0, 10).mapToObj(UintT::new).toArray(UintT[]::new); + /** Uint constants */ - public static final UintT UintZero = new UintT(0); + public static final UintT UintZero = CONST[0]; public static UintT uintOf(ULong i) { return uintOf(i.longValue()); } public static UintT uintOf(long i) { - if (i == 0L) { - return UintZero; + if (i >= 0L && i <= 10) { + return CONST[(int) i]; } return new UintT(i); } @@ -326,17 +330,16 @@ public boolean equals(Object o) { if (this == o) { return true; } - if (!(o instanceof Val)) { + if (!(o instanceof Val val)) { return false; } // Defer to CEL's equal functionality to allow heterogeneous numeric map keys - return equal((Val) o).booleanValue(); + return equal(val).booleanValue(); } @Override public int hashCode() { - // Used to allow heterogeneous numeric map keys - return (int) i; + return Types.unsignedNumericHashCode(i); } public String toString() { diff --git a/core/src/main/java/org/projectnessie/cel/common/types/pb/FieldDescription.java b/core/src/main/java/org/projectnessie/cel/common/types/pb/FieldDescription.java index 1af5b41c1..4bce1405b 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/pb/FieldDescription.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/pb/FieldDescription.java @@ -15,11 +15,14 @@ */ package org.projectnessie.cel.common.types.pb; +import static org.projectnessie.cel.common.types.BoolT.False; import static org.projectnessie.cel.common.types.BoolT.True; import static org.projectnessie.cel.common.types.Err.newTypeConversionError; +import static org.projectnessie.cel.common.types.Err.noMoreElements; import static org.projectnessie.cel.common.types.IntT.intOf; import static org.projectnessie.cel.common.types.TypeT.TypeType; import static org.projectnessie.cel.common.types.Types.boolOf; +import static org.projectnessie.cel.common.types.Util.isUnknownOrError; import static org.projectnessie.cel.common.types.pb.PbTypeDescription.reflectTypeOf; import static org.projectnessie.cel.common.types.pb.PbTypeDescription.unwrapDynamic; @@ -38,6 +41,7 @@ import java.lang.reflect.Array; import java.util.AbstractList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -45,10 +49,12 @@ import org.projectnessie.cel.common.types.IteratorT; import org.projectnessie.cel.common.types.MapT; import org.projectnessie.cel.common.types.ref.TypeAdapter; +import org.projectnessie.cel.common.types.ref.TypeEnum; import org.projectnessie.cel.common.types.ref.Val; /** FieldDescription holds metadata related to fields declared within a type. */ public final class FieldDescription extends Description { + private static final Object NO_NATIVE_MAP_KEY = new Object(); /** KeyType holds the key FieldDescription for map fields. */ final FieldDescription keyType; @@ -239,6 +245,9 @@ public Object getFrom(Db db, Object target) { if (fieldVal instanceof EnumValue) { return (long) ((EnumValue) fieldVal).getNumber(); } + if (fd.isRepeated() && fieldVal instanceof List) { + return fieldVal; + } if (fieldVal instanceof Message) { return maybeUnwrapDynamic(db, (Message) fieldVal); } @@ -306,7 +315,14 @@ public String name() { return desc.getName(); } - /** ReflectType returns the Golang reflect.Type for this field. */ + /** + * Returns the Java representation used when converting a CEL value for this field. + * + *

Map fields return {@link Map}; repeated fields return an array class; scalar fields return + * their boxed class. Ordinary message fields use {@link DynamicMessage}; recognized well-known + * message fields use their canonical generated class. Generated optimized field access is + * selected separately from this descriptor-level representation. + */ public Class reflectType() { boolean r = desc.isRepeated(); if (r && desc.isMapField()) { @@ -418,6 +434,39 @@ public Object getField(Object target, TypeAdapter adapter) { return getValueFromField(fd, message); } + Object adaptGeneratedValue(Object value, TypeAdapter adapter) { + if (desc.isMapField() && value instanceof Map) { + return new ProtoMapT(adapter, desc, (Map) value); + } + if (desc.isRepeated()) { + FieldDescriptor.Type type = desc.getType(); + if (value instanceof List + && (type == FieldDescriptor.Type.UINT32 + || type == FieldDescriptor.Type.UINT64 + || type == FieldDescriptor.Type.FIXED32 + || type == FieldDescriptor.Type.FIXED64)) { + return new UnsignedLongList(desc, (List) value); + } + return value; + } + return normalizeUnsignedValue(desc, value); + } + + boolean isWrapper() { + return isWellKnownType(desc); + } + + boolean generatedValueNeedsAdaptation() { + if (desc.isMapField()) { + return true; + } + FieldDescriptor.Type type = desc.getType(); + return type == FieldDescriptor.Type.UINT32 + || type == FieldDescriptor.Type.UINT64 + || type == FieldDescriptor.Type.FIXED32 + || type == FieldDescriptor.Type.FIXED64; + } + public static Object getValueFromField(FieldDescriptor desc, Message message) { if (!desc.isRepeated() && isWellKnownType(desc) && !message.hasField(desc)) { @@ -426,14 +475,7 @@ public static Object getValueFromField(FieldDescriptor desc, Message message) { Object v = message.getField(desc); if (!desc.isMapField() && !desc.isRepeated()) { - FieldDescriptor.Type type = desc.getType(); - if (v != null - && (type == FieldDescriptor.Type.UINT32 - || type == FieldDescriptor.Type.UINT64 - || type == FieldDescriptor.Type.FIXED32 - || type == FieldDescriptor.Type.FIXED64)) { - v = ULong.valueOf(((Number) v).longValue()); - } + v = normalizeUnsignedValue(desc, v); } else if (desc.isMapField()) { // TODO protobuf-java inefficiency // protobuf-java does NOT have a generic way to retrieve the underlying map, but instead @@ -482,7 +524,7 @@ public static Object getValueFromField(FieldDescriptor desc, Message message) { || type == FieldDescriptor.Type.UINT64 || type == FieldDescriptor.Type.FIXED32 || type == FieldDescriptor.Type.FIXED64)) { - v = new UnsignedLongList((List) v); + v = new UnsignedLongList(desc, (List) v); } } return v; @@ -508,15 +550,17 @@ private FieldDescriptor fieldDescriptorFor(Message message) { } private static final class UnsignedLongList extends AbstractList { + private final FieldDescriptor descriptor; private final List repeated; - private UnsignedLongList(List repeated) { + private UnsignedLongList(FieldDescriptor descriptor, List repeated) { + this.descriptor = descriptor; this.repeated = repeated; } @Override public ULong get(int index) { - return ULong.valueOf(((Number) repeated.get(index)).longValue()); + return (ULong) normalizeUnsignedValue(descriptor, repeated.get(index)); } @Override @@ -528,14 +572,25 @@ public int size() { private static final class ProtoMapT extends MapT { private final TypeAdapter adapter; private final List entries; + private final Map map; private final FieldDescriptor keyDesc; private final FieldDescriptor valueDesc; private volatile Map indexedEntries; + private volatile Map canonicalEntries; private volatile boolean scanned; private ProtoMapT(TypeAdapter adapter, FieldDescriptor field, List entries) { this.adapter = adapter; this.entries = entries; + this.map = null; + this.keyDesc = field.getMessageType().findFieldByNumber(1); + this.valueDesc = field.getMessageType().findFieldByNumber(2); + } + + private ProtoMapT(TypeAdapter adapter, FieldDescriptor field, Map map) { + this.adapter = adapter; + this.entries = null; + this.map = map; this.keyDesc = field.getMessageType().findFieldByNumber(1); this.valueDesc = field.getMessageType().findFieldByNumber(2); } @@ -565,12 +620,36 @@ public Val convertToType(org.projectnessie.cel.common.types.ref.Type typeValue) @Override public IteratorT iterator() { - return IteratorT.javaIterator(adapter, toJavaMap().keySet().iterator()); + return new EntryKeyIterator(); } @Override public Val equal(Val other) { - return MapT.newMaybeWrappedMap(adapter, toJavaMap()).equal(other); + if (other instanceof ProtoMapT protoOther + && map != null + && protoOther.map != null + && keyDesc.getType() == protoOther.keyDesc.getType()) { + if (nativeSize() != protoOther.nativeSize()) { + return False; + } + for (Map.Entry entry : map.entrySet()) { + Object key = entry.getKey(); + Object otherRawValue = protoOther.map.get(key); + if (otherRawValue == null && !protoOther.map.containsKey(key)) { + return False; + } + Val equal = + mapValueEqual( + adapter.nativeToValue(normalizeUnsignedValue(valueDesc, entry.getValue())), + protoOther.adapter.nativeToValue( + normalizeUnsignedValue(protoOther.valueDesc, otherRawValue))); + if (equal != True) { + return equal; + } + } + return True; + } + return mapEqual(other); } @Override @@ -580,6 +659,15 @@ public Object value() { @Override public Val contains(Val value) { + if (isUnknownOrError(value)) { + return value; + } + if (map != null) { + Object nativeKey = nativeMapKey(value); + if (nativeKey != NO_NATIVE_MAP_KEY) { + return boolOf(map.containsKey(nativeKey)); + } + } return boolOf(find(value) != null); } @@ -590,28 +678,91 @@ public Val get(Val index) { @Override public Val size() { - return intOf(toJavaMap().size()); + return intOf(entryCount()); + } + + @Override + public int nativeSize() { + return entryCount(); + } + + @Override + protected Iterable mapEntries() { + return allEntries(); + } + + @Override + protected Val mapEntryKey(Object entry) { + return adapter.nativeToValue(nativeEntryKey(entry)); + } + + @Override + protected Val mapEntryValue(Object entry) { + return adapter.nativeToValue(nativeEntryValue(entry)); } @Override public Val find(Val key) { + if (map != null) { + Object nativeKey = nativeMapKey(key); + if (nativeKey != NO_NATIVE_MAP_KEY) { + Object value = map.get(nativeKey); + return value != null || map.containsKey(nativeKey) + ? adapter.nativeToValue(normalizeUnsignedValue(valueDesc, value)) + : null; + } + } Map index = indexedEntries; if (index != null) { return index.get(key); } - if (!scanned || entries.size() <= 1) { + if (!scanned || entryCount() <= 1) { scanned = true; return scan(key); } return indexedEntries().get(key); } + private Object nativeMapKey(Val key) { + switch (keyDesc.getType()) { + case BOOL: + return key.type().typeEnum() == TypeEnum.Bool ? key.booleanValue() : NO_NATIVE_MAP_KEY; + case STRING: + return key.type().typeEnum() == TypeEnum.String ? key.value() : NO_NATIVE_MAP_KEY; + case INT32: + case SINT32: + case SFIXED32: + if (key.type().typeEnum() != TypeEnum.Int) { + return NO_NATIVE_MAP_KEY; + } + long int32 = key.intValue(); + return int32 >= Integer.MIN_VALUE && int32 <= Integer.MAX_VALUE + ? (int) int32 + : NO_NATIVE_MAP_KEY; + case INT64: + case SINT64: + case SFIXED64: + return key.type().typeEnum() == TypeEnum.Int ? key.intValue() : NO_NATIVE_MAP_KEY; + case UINT32: + case FIXED32: + if (key.type().typeEnum() != TypeEnum.Uint) { + return NO_NATIVE_MAP_KEY; + } + long uint32 = key.intValue(); + return Long.compareUnsigned(uint32, 0xffff_ffffL) <= 0 ? (int) uint32 : NO_NATIVE_MAP_KEY; + case UINT64: + case FIXED64: + return key.type().typeEnum() == TypeEnum.Uint ? key.intValue() : NO_NATIVE_MAP_KEY; + default: + return NO_NATIVE_MAP_KEY; + } + } + private Val scan(Val key) { - for (int i = entries.size() - 1; i >= 0; i--) { - Object entry = entries.get(i); - Val candidate = adapter.nativeToValue(mapEntryKey(entry)); + for (Object entry : allEntries()) { + Val candidate = adapter.nativeToValue(nativeEntryKey(entry)); if (candidate.equal(key) == True) { - return adapter.nativeToValue(mapEntryValue(entry)); + return adapter.nativeToValue(nativeEntryValue(entry)); } } return null; @@ -625,10 +776,10 @@ private Map indexedEntries() { synchronized (this) { index = indexedEntries; if (index == null) { - index = new HashMap<>(entries.size() * 4 / 3 + 1); - for (Object entry : entries) { - Val key = adapter.nativeToValue(mapEntryKey(entry)); - Val value = adapter.nativeToValue(mapEntryValue(entry)); + index = new HashMap<>(entryCount() * 4 / 3 + 1); + for (Object entry : allEntries()) { + Val key = adapter.nativeToValue(nativeEntryKey(entry)); + Val value = adapter.nativeToValue(nativeEntryValue(entry)); index.put(key, value); } indexedEntries = index; @@ -638,22 +789,25 @@ private Map indexedEntries() { } private Map toJavaMap() { - Map map = new HashMap<>(entries.size() * 4 / 3 + 1); - for (Object entry : entries) { - map.put(mapEntryKey(entry), mapEntryValue(entry)); + Map javaMap = new HashMap<>(entryCount() * 4 / 3 + 1); + for (Object entry : allEntries()) { + javaMap.put(nativeEntryKey(entry), nativeEntryValue(entry)); } - return map; + return javaMap; } - private Object mapEntryKey(Object entry) { + private Object nativeEntryKey(Object entry) { return normalizeUnsignedValue(entryKeyDescriptor(entry), rawMapEntryValue(entry, 1)); } - private Object mapEntryValue(Object entry) { + private Object nativeEntryValue(Object entry) { return normalizeUnsignedValue(entryValueDescriptor(entry), rawMapEntryValue(entry, 2)); } private FieldDescriptor entryKeyDescriptor(Object entry) { + if (entry instanceof Map.Entry) { + return keyDesc; + } if (entry instanceof DynamicMessage) { List fields = ((DynamicMessage) entry).getDescriptorForType().getFields(); if (fields.size() == 2) { @@ -664,6 +818,9 @@ private FieldDescriptor entryKeyDescriptor(Object entry) { } private FieldDescriptor entryValueDescriptor(Object entry) { + if (entry instanceof Map.Entry) { + return valueDesc; + } if (entry instanceof DynamicMessage) { List fields = ((DynamicMessage) entry).getDescriptorForType().getFields(); if (fields.size() == 2) { @@ -674,6 +831,9 @@ private FieldDescriptor entryValueDescriptor(Object entry) { } private Object rawMapEntryValue(Object entry, int fieldNumber) { + if (entry instanceof Map.Entry mapEntry) { + return fieldNumber == 1 ? mapEntry.getKey() : mapEntry.getValue(); + } if (entry instanceof MapEntry mapEntry) { return fieldNumber == 1 ? mapEntry.getKey() : mapEntry.getValue(); } @@ -686,6 +846,54 @@ private Object rawMapEntryValue(Object entry, int fieldNumber) { throw new IllegalArgumentException( String.format("Unexpected %s (%s) in list of map fields", entry.getClass(), entry)); } + + private int entryCount() { + return entries != null ? canonicalEntries().size() : map.size(); + } + + private Iterable allEntries() { + return entries != null ? canonicalEntries().entrySet() : map.entrySet(); + } + + private Map canonicalEntries() { + Map canonical = canonicalEntries; + if (canonical != null) { + return canonical; + } + synchronized (this) { + canonical = canonicalEntries; + if (canonical == null) { + canonical = new LinkedHashMap<>(entries.size() * 4 / 3 + 1); + for (Object entry : entries) { + canonical.put(nativeEntryKey(entry), nativeEntryValue(entry)); + } + canonicalEntries = canonical; + } + return canonical; + } + } + + private final class EntryKeyIterator implements IteratorT { + private final java.util.Iterator iterator = allEntries().iterator(); + + @Override + public Val hasNext() { + return boolOf(iterator.hasNext()); + } + + @Override + public Val next() { + if (iterator.hasNext()) { + return adapter.nativeToValue(nativeEntryKey(iterator.next())); + } + return noMoreElements(); + } + + @Override + public String toString() { + return "iterator"; + } + } } private static Object normalizeUnsignedValue(FieldDescriptor desc, Object value) { diff --git a/core/src/main/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessor.java b/core/src/main/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessor.java index f65cde96f..38fbee860 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessor.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessor.java @@ -16,13 +16,17 @@ package org.projectnessie.cel.common.types.pb; import com.google.protobuf.ByteString; +import com.google.protobuf.Descriptors.EnumValueDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.DynamicMessage; import com.google.protobuf.Message; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; +import java.util.List; +import java.util.Map; import org.projectnessie.cel.common.types.ref.FieldGetter; +import org.projectnessie.cel.common.types.ref.FieldTester; final class GeneratedFieldAccessor { @@ -31,24 +35,59 @@ final class GeneratedFieldAccessor { private GeneratedFieldAccessor() {} static FieldGetter create(PbTypeDescription type, FieldDescription field) { + return create(type, field, false); + } + + static FieldGetter createForObject(PbTypeDescription type, FieldDescription field) { + return create(type, field, true); + } + + private static FieldGetter create( + PbTypeDescription type, FieldDescription field, boolean allowOrdinaryMessages) { FieldDescriptor descriptor = field.descriptor(); - Class returnType = generatedReturnType(descriptor); Message zero = type.zero(); Class messageClass = type.reflectType(); - if (returnType == null - || descriptor.isExtension() - || descriptor.isRepeated() + if (descriptor.isExtension() || zero instanceof DynamicMessage || !messageClass.isInstance(zero)) { return null; } String suffix = accessorSuffix(descriptor.getName()); - for (String methodName : new String[] {"get" + suffix, "get" + suffix + '_'}) { + String methodName; + Class returnType; + if (descriptor.isMapField()) { + FieldDescriptor valueDescriptor = descriptor.getMessageType().findFieldByNumber(2); + methodName = + valueDescriptor.getType() == FieldDescriptor.Type.ENUM + ? "get" + suffix + "ValueMap" + : "get" + suffix + "Map"; + returnType = Map.class; + } else if (descriptor.isRepeated()) { + methodName = + descriptor.getType() == FieldDescriptor.Type.ENUM + ? "get" + suffix + "ValueList" + : "get" + suffix + "List"; + returnType = List.class; + } else if (descriptor.getType() == FieldDescriptor.Type.ENUM) { + if (isNullValue(descriptor.getEnumType().getFullName())) { + return null; + } + methodName = "get" + suffix + "Value"; + returnType = int.class; + } else { + methodName = "get" + suffix; + returnType = generatedReturnType(descriptor, zero, allowOrdinaryMessages); + } + if (returnType == null) { + return null; + } + + for (String candidate : new String[] {methodName, methodName + '_'}) { try { MethodHandle getter = MethodHandles.publicLookup() - .findVirtual(messageClass, methodName, MethodType.methodType(returnType)); + .findVirtual(messageClass, candidate, MethodType.methodType(returnType)); if (!matchesField(getter, zero, descriptor)) { continue; } @@ -61,44 +100,149 @@ static FieldGetter create(PbTypeDescription type, FieldDescription field) { return null; } - private static Class generatedReturnType(FieldDescriptor descriptor) { + static FieldTester createTester(PbTypeDescription type, FieldDescription field) { + FieldDescriptor descriptor = field.descriptor(); + Message zero = type.zero(); + Class messageClass = type.reflectType(); + if (descriptor.isExtension() + || descriptor.isRepeated() + || !descriptor.hasPresence() + || zero instanceof DynamicMessage + || !messageClass.isInstance(zero)) { + return null; + } + + String methodName = "has" + accessorSuffix(descriptor.getName()); + for (String candidate : new String[] {methodName, methodName + '_'}) { + try { + MethodHandle tester = + MethodHandles.publicLookup() + .findVirtual(messageClass, candidate, MethodType.methodType(boolean.class)); + if (!matchesPresence(tester, zero, descriptor)) { + continue; + } + MethodHandle objectTester = + tester.asType(MethodType.methodType(boolean.class, Object.class)); + return target -> invokeBoolean(objectTester, target); + } catch (IllegalAccessException | NoSuchMethodException | RuntimeException e) { + // Generated-name collisions and inaccessible generated classes use descriptor access. + } + } + return null; + } + + private static Class generatedReturnType( + FieldDescriptor descriptor, Message zero, boolean allowOrdinaryMessages) { return switch (descriptor.getType()) { case BOOL -> boolean.class; case STRING -> String.class; case BYTES -> ByteString.class; - case INT32, SINT32, SFIXED32 -> int.class; - case INT64, SINT64, SFIXED64 -> long.class; + case INT32, SINT32, SFIXED32, UINT32, FIXED32 -> int.class; + case INT64, SINT64, SFIXED64, UINT64, FIXED64 -> long.class; case FLOAT -> float.class; case DOUBLE -> double.class; + case MESSAGE -> { + Object value = + allowOrdinaryMessages + || Checked.CheckedWellKnowns.containsKey( + descriptor.getMessageType().getFullName()) + ? zero.getField(descriptor) + : null; + yield value instanceof Message ? value.getClass() : null; + } default -> null; }; } private static boolean matchesField( MethodHandle getter, Message zero, FieldDescriptor descriptor) { - Object expected = validationValue(descriptor); - Message probe = zero.newBuilderForType().setField(descriptor, expected).buildPartial(); try { - return expected.equals(getter.invoke(probe)); + Message.Builder builder = zero.newBuilderForType(); + if (descriptor.isMapField()) { + Message.Builder entry = builder.newBuilderForField(descriptor); + for (FieldDescriptor entryField : descriptor.getMessageType().getFields()) { + Object value = validationValue(entryField, entry); + if (value == null) { + return false; + } + entry.setField(entryField, value); + } + builder.addRepeatedField(descriptor, entry.buildPartial()); + return ((Map) getter.invoke(builder.buildPartial())).size() == 1; + } + + Object expected = validationValue(descriptor, builder); + if (expected == null) { + return false; + } + if (descriptor.isRepeated()) { + builder.addRepeatedField(descriptor, expected); + List actual = (List) getter.invoke(builder.buildPartial()); + if (actual.size() != 1) { + return false; + } + Object actualValue = actual.get(0); + return descriptor.getType() == FieldDescriptor.Type.ENUM + ? actualValue.equals(((EnumValueDescriptor) expected).getNumber()) + : expected.equals(actualValue); + } + + builder.setField(descriptor, expected); + Object actual = getter.invoke(builder.buildPartial()); + return descriptor.getType() == FieldDescriptor.Type.ENUM + ? actual.equals(((EnumValueDescriptor) expected).getNumber()) + : expected.equals(actual); } catch (Throwable e) { return false; } } - private static Object validationValue(FieldDescriptor descriptor) { + private static boolean matchesPresence( + MethodHandle tester, Message zero, FieldDescriptor descriptor) { + try { + if ((boolean) tester.invoke(zero)) { + return false; + } + Message.Builder builder = zero.newBuilderForType(); + builder.setField(descriptor, builder.getField(descriptor)); + return (boolean) tester.invoke(builder.buildPartial()); + } catch (Throwable e) { + return false; + } + } + + private static Object validationValue( + FieldDescriptor descriptor, Message.Builder containingBuilder) { return switch (descriptor.getType()) { case BOOL -> true; case STRING -> "cel_generated_field_accessor"; case BYTES -> ByteString.copyFromUtf8("cel_generated_field_accessor"); - case INT32, SINT32, SFIXED32 -> 0x51a7; - case INT64, SINT64, SFIXED64 -> 0x51a7_19b3_42c5L; + case INT32, SINT32, SFIXED32, UINT32, FIXED32 -> 0x51a7; + case INT64, SINT64, SFIXED64, UINT64, FIXED64 -> 0x51a7_19b3_42c5L; case FLOAT -> 123.25f; case DOUBLE -> 123.25d; - default -> - throw new IllegalArgumentException("Unsupported field type " + descriptor.getType()); + case ENUM -> { + List values = descriptor.getEnumType().getValues(); + yield values.get(Math.min(1, values.size() - 1)); + } + case MESSAGE -> nonDefaultMessage(containingBuilder.newBuilderForField(descriptor)); + default -> null; }; } + private static Message nonDefaultMessage(Message.Builder builder) { + for (FieldDescriptor field : builder.getDescriptorForType().getFields()) { + if (field.isRepeated() || field.getType() == FieldDescriptor.Type.MESSAGE) { + continue; + } + Object value = validationValue(field, builder); + if (value != null) { + return builder.setField(field, value).buildPartial(); + } + } + return null; + } + private static String accessorSuffix(String fieldName) { StringBuilder result = new StringBuilder(fieldName.length()); boolean capitalizeNext = true; @@ -120,6 +264,10 @@ private static String accessorSuffix(String fieldName) { return result.toString(); } + private static boolean isNullValue(String enumTypeName) { + return enumTypeName.equals("google.protobuf.NullValue"); + } + private static Object invoke(MethodHandle getter, Object target) { try { return (Object) getter.invokeExact(target); @@ -127,4 +275,12 @@ private static Object invoke(MethodHandle getter, Object target) { throw new RuntimeException(e); } } + + private static boolean invokeBoolean(MethodHandle tester, Object target) { + try { + return (boolean) tester.invokeExact(target); + } catch (Throwable e) { + throw new RuntimeException(e); + } + } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/pb/PbObjectT.java b/core/src/main/java/org/projectnessie/cel/common/types/pb/PbObjectT.java index df771cbdc..b5dd28d0b 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/pb/PbObjectT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/pb/PbObjectT.java @@ -32,6 +32,7 @@ import com.google.protobuf.Value; import org.projectnessie.cel.common.types.ObjectT; import org.projectnessie.cel.common.types.StringT; +import org.projectnessie.cel.common.types.ref.FieldType; import org.projectnessie.cel.common.types.ref.Type; import org.projectnessie.cel.common.types.ref.TypeAdapter; import org.projectnessie.cel.common.types.ref.Val; @@ -63,6 +64,10 @@ public Val isSet(Val field) { return noSuchOverload(this, "isSet", field); } String protoFieldStr = (String) field.value(); + FieldType fieldType = fieldType(protoFieldStr, true); + if (fieldType != null) { + return boolOf(fieldType.isSet.isSet(value)); + } FieldDescription fd = fieldDescription(protoFieldStr); if (fd == null) { return noSuchField(protoFieldStr); @@ -76,6 +81,10 @@ public Val get(Val index) { return noSuchOverload(this, "get", index); } String protoFieldStr = (String) index.value(); + FieldType fieldType = fieldType(protoFieldStr, false); + if (fieldType != null) { + return adapter.nativeToValue(fieldType.getFrom.getFrom(value)); + } FieldDescription fd = fieldDescription(protoFieldStr); if (fd == null) { return noSuchField(protoFieldStr); @@ -232,6 +241,13 @@ private FieldDescription fieldDescription(String fieldName) { return ((ProtoTypeRegistry) adapter).findFieldDescription(typeDesc().name(), fieldName); } + private FieldType fieldType(String fieldName, boolean presenceTest) { + if (adapter instanceof ProtoTypeRegistry registry) { + return registry.findFieldTypeForObjectAccess(typeDesc().name(), fieldName, presenceTest); + } + return null; + } + @SuppressWarnings("unchecked") private T buildFrom(Class typeDesc) { try { diff --git a/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java b/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java index ee5908bae..87b548cef 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java @@ -96,6 +96,7 @@ import org.projectnessie.cel.common.types.UintT; import org.projectnessie.cel.common.types.UnknownT; import org.projectnessie.cel.common.types.ref.FieldGetter; +import org.projectnessie.cel.common.types.ref.FieldTester; import org.projectnessie.cel.common.types.ref.FieldType; import org.projectnessie.cel.common.types.ref.TypeAdapterSupport; import org.projectnessie.cel.common.types.ref.TypeRegistry; @@ -106,7 +107,7 @@ public final class ProtoTypeRegistry implements TypeRegistry { private static final ProtoTypeRegistry DEFAULT_REGISTRY = newDefaultRegistry(); private final Map revTypeMap; - private final Map fieldTypeCache; + private final Map> fieldTypeCache; private final Db pbdb; private ProtoTypeRegistry( @@ -217,10 +218,34 @@ public Val enumValue(String enumName) { return intOf(enumVal.value()); } + /** + * Resolves checked type and runtime access metadata for a protobuf field. + * + *

The returned metadata supports both generated and descriptor-based access according to the + * representation currently bound to this registry. + * + * @param messageType fully qualified registered protobuf message name + * @param fieldName protobuf source field name or qualified extension name + * @return field metadata, or {@code null} if the message type or field is unknown + */ @Override public FieldType findFieldType(String messageType, String fieldName) { - String cacheKey = messageType + '\n' + fieldName; - return fieldTypeCache.computeIfAbsent(cacheKey, key -> loadFieldType(messageType, fieldName)); + Map messageFields = fieldTypeCache.get(messageType); + if (messageFields == null) { + Map newFields = new ConcurrentHashMap<>(); + Map existing = fieldTypeCache.putIfAbsent(messageType, newFields); + messageFields = existing != null ? existing : newFields; + } + FieldType fieldType = messageFields.get(fieldName); + if (fieldType != null) { + return fieldType; + } + FieldType loaded = loadFieldType(messageType, fieldName); + if (loaded == null) { + return null; + } + FieldType existing = messageFields.putIfAbsent(fieldName, loaded); + return existing != null ? existing : loaded; } private FieldType loadFieldType(String messageType, String fieldName) { @@ -228,18 +253,70 @@ private FieldType loadFieldType(String messageType, String fieldName) { if (field == null) { return null; } - FieldGetter getter = target -> field.getField(target, this); + FieldTester descriptorTester = field::hasField; + FieldGetter descriptorGetter = target -> field.getField(target, this); PbTypeDescription type = pbdb.describeType(messageType); FieldGetter generatedGetter = type != null ? GeneratedFieldAccessor.create(type, field) : null; - if (generatedGetter != null) { + FieldGetter objectGetter = generatedGetter; + if (objectGetter == null && type != null) { + objectGetter = GeneratedFieldAccessor.createForObject(type, field); + } + FieldTester generatedTester = + type != null ? GeneratedFieldAccessor.createTester(type, field) : null; + FieldTester tester = descriptorTester; + if (generatedTester != null) { Class generatedType = type.reflectType(); - getter = + tester = target -> generatedType.isInstance(target) - ? generatedGetter.getFrom(target) - : field.getField(target, this); + ? generatedTester.isSet(target) + : field.hasField(target); + } + FieldGetter getter = + generatedGetter != null + ? bindGeneratedGetter(type, field, generatedGetter, tester) + : descriptorGetter; + FieldGetter optimizedObjectGetter = + objectGetter != null ? bindGeneratedGetter(type, field, objectGetter, tester) : null; + return new ProtoFieldType( + field.checkedType(), tester, getter, generatedTester, optimizedObjectGetter); + } + + private FieldGetter bindGeneratedGetter( + PbTypeDescription type, + FieldDescription field, + FieldGetter generatedGetter, + FieldTester tester) { + Class generatedType = type.reflectType(); + if (field.isWrapper()) { + return target -> + generatedType.isInstance(target) + ? !tester.isSet(target) + ? com.google.protobuf.NullValue.NULL_VALUE + : generatedGetter.getFrom(target) + : field.getField(target, this); + } + if (field.generatedValueNeedsAdaptation()) { + return target -> + generatedType.isInstance(target) + ? field.adaptGeneratedValue(generatedGetter.getFrom(target), this) + : field.getField(target, this); + } + return target -> + generatedType.isInstance(target) + ? generatedGetter.getFrom(target) + : field.getField(target, this); + } + + FieldType findFieldTypeForObjectAccess( + String messageType, String fieldName, boolean presenceTest) { + FieldType fieldType = findFieldType(messageType, fieldName); + if (!(fieldType instanceof ProtoFieldType protoFieldType)) { + return null; } - return new FieldType(field.checkedType(), field::hasField, getter); + return presenceTest + ? protoFieldType.objectPresenceFieldType + : protoFieldType.objectGetterFieldType; } FieldDescription findFieldDescription(String messageType, String fieldName) { @@ -351,10 +428,10 @@ && isNullPrunedMessageField(fieldDesc) private Object toNativeRepeatedFieldValue(Lister value, FieldDescriptor fieldDesc) { Class elementType = messageNativeType(fieldDesc); - int size = (int) value.size().intValue(); + int size = value.nativeSize(); List converted = new ArrayList<>(size); for (int i = 0; i < size; i++) { - Val element = value.get(intOf(i)); + Val element = value.nativeGetAt(i); if (element == NullT.NullValue && isNullPrunedMessageField(fieldDesc)) { continue; } @@ -413,7 +490,7 @@ private Object toProtoMapStructure(FieldDescription field, MapT value) { FieldDescriptor valueType = field.valueType.descriptor(); WireFormat.FieldType keyFieldType = WireFormat.FieldType.valueOf(keyType.getType().name()); WireFormat.FieldType valueFieldType = WireFormat.FieldType.valueOf(valueType.getType().name()); - List> entries = new ArrayList<>((int) value.size().intValue()); + List> entries = new ArrayList<>(value.nativeSize()); IteratorT iterator = value.iterator(); while (iterator.hasNext() == True) { @@ -558,6 +635,7 @@ public void registerDescriptor(FileDescriptor fileDesc) { /** RegisterMessage registers a protocol buffer message and its dependencies. */ public void registerMessage(Message message) { FileDescription fd = pbdb.registerMessage(message); + fieldTypeCache.remove(typeNameFromMessage(message)); registerAllTypes(fd); } @@ -663,6 +741,24 @@ public String toString() { return "ProtoTypeRegistry{" + "revTypeMap.size=" + revTypeMap.size() + ", pbdb=" + pbdb + '}'; } + private static final class ProtoFieldType extends FieldType { + private final FieldType objectPresenceFieldType; + private final FieldType objectGetterFieldType; + + private ProtoFieldType( + Type type, + FieldTester isSet, + FieldGetter getFrom, + FieldTester generatedTester, + FieldGetter objectGetter) { + super(type, isSet, getFrom); + this.objectPresenceFieldType = + generatedTester != null ? new FieldType(type, isSet, getFrom) : null; + this.objectGetterFieldType = + objectGetter != null ? new FieldType(type, isSet, objectGetter) : null; + } + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/core/src/main/java/org/projectnessie/cel/common/types/traits/Indexer.java b/core/src/main/java/org/projectnessie/cel/common/types/traits/Indexer.java index 6132eb687..76c853d6b 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/traits/Indexer.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/traits/Indexer.java @@ -15,10 +15,26 @@ */ package org.projectnessie.cel.common.types.traits; +import static org.projectnessie.cel.common.types.IntT.intOf; + import org.projectnessie.cel.common.types.ref.Val; /** Indexer permits random access of elements by index 'a[b()]'. */ public interface Indexer { /** Get the value at the specified index or error. */ Val get(Val index); + + /** + * Returns the value at a native integer index. + * + *

The default delegates to {@link #get(Val)} with a CEL int. Implementations may override it + * to avoid constructing that index, but must preserve the same result, error, and unknown + * semantics. + * + * @param index zero-based Java index + * @return the selected value, or the same absence, CEL error, or unknown result as {@code get} + */ + default Val nativeGetAt(int index) { + return get(intOf(index)); + } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/traits/Sizer.java b/core/src/main/java/org/projectnessie/cel/common/types/traits/Sizer.java index 86fb786ca..8bb6aef5f 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/traits/Sizer.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/traits/Sizer.java @@ -21,4 +21,19 @@ public interface Sizer { /** Size returns the number of elements or length of the value. */ Val size(); + + /** + * Returns the same size as a Java {@code int}. + * + *

The default converts {@link #size()}; implementations may override it to avoid constructing + * a CEL integer but must preserve the same numeric result. + * + * @return the non-negative size + * @throws ArithmeticException if the size cannot be represented as a Java {@code int} + * @throws RuntimeException if {@link #size()} returns a value that cannot be converted to an + * integer + */ + default int nativeSize() { + return Math.toIntExact(size().intValue()); + } } diff --git a/core/src/main/java/org/projectnessie/cel/common/types/traits/Trait.java b/core/src/main/java/org/projectnessie/cel/common/types/traits/Trait.java index e22d4d4c1..a5587de33 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/traits/Trait.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/traits/Trait.java @@ -37,7 +37,11 @@ public enum Trait { /** IterableType types can be iterated over in comprehensions. */ IterableType, - /** IteratorType types support iterator semantics. */ + /** + * IteratorType marks low-level interpreter cursor values. + * + *

It is used for internal traversal dispatch and does not define a CEL language type. + */ IteratorType, /** MatcherType types support pattern matching via 'matches' method. */ diff --git a/core/src/main/java/org/projectnessie/cel/extension/MathLib.java b/core/src/main/java/org/projectnessie/cel/extension/MathLib.java index 427d29647..cdef0f4e5 100644 --- a/core/src/main/java/org/projectnessie/cel/extension/MathLib.java +++ b/core/src/main/java/org/projectnessie/cel/extension/MathLib.java @@ -325,10 +325,10 @@ private static Val minMax(Val[] values, boolean greatest) { private static List candidates(Val[] values) { if (values.length == 1 && values[0] instanceof Lister list) { - int size = (int) list.size().intValue(); + int size = list.nativeSize(); List elements = new ArrayList<>(size); for (int i = 0; i < size; i++) { - elements.add(list.get(intOf(i))); + elements.add(list.nativeGetAt(i)); } return elements; } diff --git a/core/src/main/java/org/projectnessie/cel/extension/StringsLib.java b/core/src/main/java/org/projectnessie/cel/extension/StringsLib.java index 3b8d0b3ef..b6124e1e5 100644 --- a/core/src/main/java/org/projectnessie/cel/extension/StringsLib.java +++ b/core/src/main/java/org/projectnessie/cel/extension/StringsLib.java @@ -17,7 +17,6 @@ import static java.math.RoundingMode.HALF_EVEN; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.projectnessie.cel.common.types.IntT.intOf; import java.math.BigDecimal; import java.util.*; @@ -748,7 +747,7 @@ private static Val format(Val pattern, Val args) { } private static String formatPattern(String pattern, Sizer argsSizer, Indexer argsIndexer) { - int argCount = Math.toIntExact(argsSizer.size().intValue()); + int argCount = argsSizer.nativeSize(); int argIndex = 0; StringBuilder out = new StringBuilder(pattern.length()); for (int i = 0; i < pattern.length(); i++) { @@ -784,7 +783,7 @@ private static String formatPattern(String pattern, Sizer argsSizer, Indexer arg if (argIndex >= argCount) { throw new FormatException("index %d out of range", argIndex); } - Val arg = argsIndexer.get(intOf(argIndex++)); + Val arg = argsIndexer.nativeGetAt(argIndex++); out.append(formatValue(ch, precision, arg)); } return out.toString(); @@ -919,13 +918,13 @@ private static String renderScientificClause(Val value, int precision) { private static String renderList(Val value) { Sizer sizer = (Sizer) value; Indexer indexer = (Indexer) value; - int size = Math.toIntExact(sizer.size().intValue()); + int size = sizer.nativeSize(); StringBuilder out = new StringBuilder("["); for (int i = 0; i < size; i++) { if (i > 0) { out.append(", "); } - out.append(renderStringClause(indexer.get(intOf(i)))); + out.append(renderStringClause(indexer.nativeGetAt(i))); } return out.append(']').toString(); } diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/AstPruner.java b/core/src/main/java/org/projectnessie/cel/interpreter/AstPruner.java index 52b7dc341..00d2a5421 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/AstPruner.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/AstPruner.java @@ -20,7 +20,6 @@ // common subexpressions. import static org.projectnessie.cel.common.types.BoolT.True; -import static org.projectnessie.cel.common.types.IntT.intOf; import static org.projectnessie.cel.common.types.UnknownT.isUnknown; import static org.projectnessie.cel.common.types.Util.isUnknownOrError; @@ -122,10 +121,10 @@ Expr maybeCreateLiteral(long id, Val v) { // Attempt to build a list literal. if (v instanceof Lister list) { - int sz = (int) list.size().intValue(); + int sz = list.nativeSize(); List elemExprs = new ArrayList<>(sz); for (int i = 0; i < sz; i++) { - Val elem = list.get(intOf(i)); + Val elem = list.nativeGetAt(i); if (isUnknownOrError(elem)) { return null; } @@ -144,7 +143,7 @@ Expr maybeCreateLiteral(long id, Val v) { // Create a map literal if possible. if (v instanceof Mapper mp) { IteratorT it = mp.iterator(); - List entries = new ArrayList<>((int) mp.size().intValue()); + List entries = new ArrayList<>(mp.nativeSize()); while (it.hasNext() == True) { Val key = it.next(); Val val = mp.get(key); diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/AttributeFactory.java b/core/src/main/java/org/projectnessie/cel/interpreter/AttributeFactory.java index 44618e0ee..05c2b0ce9 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/AttributeFactory.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/AttributeFactory.java @@ -45,11 +45,13 @@ import org.projectnessie.cel.common.ULong; import org.projectnessie.cel.common.containers.Container; import org.projectnessie.cel.common.types.Err; +import org.projectnessie.cel.common.types.IntT; import org.projectnessie.cel.common.types.ref.FieldType; import org.projectnessie.cel.common.types.ref.TypeAdapter; import org.projectnessie.cel.common.types.ref.TypeProvider; import org.projectnessie.cel.common.types.ref.Val; import org.projectnessie.cel.common.types.traits.Indexer; +import org.projectnessie.cel.common.types.traits.Lister; import org.projectnessie.cel.common.types.traits.Mapper; import org.projectnessie.cel.interpreter.AttributePattern.QualifierValueEquator; @@ -1428,6 +1430,12 @@ static Val refResolve(TypeAdapter adapter, Val idx, Object obj) { } return elem; } + if (celVal instanceof Lister list && idx instanceof IntT) { + long intIndex = idx.intValue(); + if (intIndex >= Integer.MIN_VALUE && intIndex <= Integer.MAX_VALUE) { + return list.nativeGetAt((int) intIndex); + } + } if (celVal instanceof Indexer indexer) { return indexer.get(idx); } diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java b/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java index 9a0d534e7..59a11d5dc 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java @@ -39,7 +39,6 @@ import java.util.Objects; import java.util.Set; import org.projectnessie.cel.common.operators.Operator; -import org.projectnessie.cel.common.types.Err; import org.projectnessie.cel.common.types.IterableT; import org.projectnessie.cel.common.types.IteratorT; import org.projectnessie.cel.common.types.ListT; @@ -166,8 +165,8 @@ final class EvalIdent extends AbstractEval implements Coster { /** Eval implements the Interpretable interface method. */ @Override public Val eval(Activation ctx) { - if (ctx instanceof PartialActivation) { - for (AttributePattern pattern : ((PartialActivation) ctx).unknownAttributePatterns()) { + if (ctx instanceof PartialActivation partialActivation) { + for (AttributePattern pattern : partialActivation.unknownAttributePatterns()) { if (pattern.variableMatches(name)) { return unknownOf(id); } @@ -231,11 +230,11 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { } Val obj = op.eval(ctx); - if (obj instanceof FieldTester) { - return ((FieldTester) obj).isSet(field); + if (obj instanceof FieldTester tester) { + return tester.isSet(field); } - if (obj instanceof Container) { - return ((Container) obj).contains(field); + if (obj instanceof Container container) { + return container.contains(field); } return valOrErr(obj, "invalid type for field selection."); } @@ -1560,17 +1559,19 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { } IteratorT it = ((IterableT) foldRange).iterator(); long index = 0L; + var isLister = foldRange instanceof Lister; + var mapper = (foldRange instanceof Mapper m) ? m : null; while (it.hasNext() == True) { // Modify the iter var in the fold activation. Val next = it.next(); Activation loopCtx = iterCtx; if (iterCtx2 != null) { - if (foldRange instanceof Lister) { + if (isLister) { iterCtx.val = intOf(index); iterCtx2.val = next; - } else if (foldRange instanceof Mapper) { + } else if (mapper != null) { iterCtx.val = next; - iterCtx2.val = ((Mapper) foldRange).get(next); + iterCtx2.val = mapper.get(next); } else { return valOrErr( foldRange, "got '%s', expected list or map type", foldRange.getClass().getName()); @@ -1607,11 +1608,16 @@ public Cost cost() { if (!foldRange.type().hasTrait(Trait.IterableType)) { return Cost.Unknown; } - long rangeCnt = 0L; - IteratorT it = ((IterableT) foldRange).iterator(); - while (it.hasNext() == True) { - it.next(); - rangeCnt++; + long rangeCnt; + if (foldRange instanceof Sizer sizer) { + rangeCnt = sizer.nativeSize(); + } else { + rangeCnt = 0L; + IteratorT it = ((IterableT) foldRange).iterator(); + while (it.hasNext() == True) { + it.next(); + rangeCnt++; + } } Cost a = estimateCost(accu); Cost c = estimateCost(cond); @@ -1699,16 +1705,18 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { List values = new ArrayList<>(listCapacity(foldRange)); IteratorT it = ((IterableT) foldRange).iterator(); long index = 0L; + var isLister = foldRange instanceof Lister; + var mapper = (foldRange instanceof Mapper m) ? m : null; while (it.hasNext() == True) { Val next = it.next(); Activation loopCtx = iterCtx; if (iterCtx2 != null) { - if (foldRange instanceof Lister) { + if (isLister) { iterCtx.val = intOf(index); iterCtx2.val = next; - } else if (foldRange instanceof Mapper) { + } else if (mapper != null) { iterCtx.val = next; - iterCtx2.val = ((Mapper) foldRange).get(next); + iterCtx2.val = mapper.get(next); } else { return valOrErr( foldRange, "got '%s', expected list or map type", foldRange.getClass().getName()); @@ -1740,7 +1748,7 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { private int listCapacity(Val foldRange) { if (foldRange.type().hasTrait(Trait.SizerType)) { - long size = ((Sizer) foldRange).size().intValue(); + long size = ((Sizer) foldRange).nativeSize(); if (size > 0 && size <= Integer.MAX_VALUE) { return (int) size; } @@ -1759,11 +1767,16 @@ public Cost cost() { if (!foldRange.type().hasTrait(Trait.IterableType)) { return Cost.Unknown; } - long rangeCnt = 0L; - IteratorT it = ((IterableT) foldRange).iterator(); - while (it.hasNext() == True) { - it.next(); - rangeCnt++; + long rangeCnt; + if (foldRange instanceof Sizer sizer) { + rangeCnt = sizer.nativeSize(); + } else { + rangeCnt = 0L; + IteratorT it = ((IterableT) foldRange).iterator(); + while (it.hasNext() == True) { + it.next(); + rangeCnt++; + } } return range.add(result.multiply(rangeCnt)); } @@ -1834,19 +1847,21 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { Map values = new HashMap<>(mapCapacity(foldRange)); IteratorT it = ((IterableT) foldRange).iterator(); long index = 0L; + var isLister = foldRange instanceof Lister; + var mapper = (foldRange instanceof Mapper m) ? m : null; while (it.hasNext() == True) { Val next = it.next(); Val key; Activation loopCtx = iterCtx; if (iterCtx2 != null) { - if (foldRange instanceof Lister) { + if (isLister) { key = intOf(index); iterCtx.val = key; iterCtx2.val = next; - } else if (foldRange instanceof Mapper) { + } else if (mapper != null) { key = next; iterCtx.val = key; - iterCtx2.val = ((Mapper) foldRange).get(next); + iterCtx2.val = mapper.get(next); } else { return valOrErr( foldRange, "got '%s', expected list or map type", foldRange.getClass().getName()); @@ -1879,7 +1894,7 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { private int mapCapacity(Val foldRange) { if (foldRange.type().hasTrait(Trait.SizerType)) { - long size = ((Sizer) foldRange).size().intValue(); + long size = ((Sizer) foldRange).nativeSize(); if (size > 0 && size <= Integer.MAX_VALUE) { long capacity = size * 4 / 3 + 1; return capacity <= Integer.MAX_VALUE ? (int) capacity : Integer.MAX_VALUE; @@ -1899,11 +1914,16 @@ public Cost cost() { if (!foldRange.type().hasTrait(Trait.IterableType)) { return Cost.Unknown; } - long rangeCnt = 0L; - IteratorT it = ((IterableT) foldRange).iterator(); - while (it.hasNext() == True) { - it.next(); - rangeCnt++; + long rangeCnt; + if (foldRange instanceof Sizer sizer) { + rangeCnt = sizer.nativeSize(); + } else { + rangeCnt = 0L; + IteratorT it = ((IterableT) foldRange).iterator(); + while (it.hasNext() == True) { + it.next(); + rangeCnt++; + } } return range.add(result.multiply(rangeCnt)); } @@ -1955,6 +1975,12 @@ final class EvalSetMembership extends AbstractEval implements Coster { @Override public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { Val val = arg.eval(ctx); + if (isUnknownOrError(val)) { + return val; + } + if (valueSet.isEmpty()) { + return False; + } if (!val.type().typeName().equals(argTypeName)) { return noSuchOverload(null, Operator.In.id, val); } @@ -2564,18 +2590,20 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { iterCtx2.name = iterVar2; } IteratorT it = ((IterableT) foldRange).iterator(); + var isLister = foldRange instanceof Lister; + var mapper = (foldRange instanceof Mapper m) ? m : null; long index = 0L; while (it.hasNext() == True) { // Modify the iter var in the fold activation. Val next = it.next(); Activation loopCtx = iterCtx; if (iterCtx2 != null) { - if (foldRange instanceof Lister) { + if (isLister) { iterCtx.val = intOf(index); iterCtx2.val = next; - } else if (foldRange instanceof Mapper) { + } else if (mapper != null) { iterCtx.val = next; - iterCtx2.val = ((Mapper) foldRange).get(next); + iterCtx2.val = mapper.get(next); } else { return valOrErr( foldRange, "got '%s', expected list or map type", foldRange.getClass().getName()); @@ -2609,11 +2637,16 @@ public Cost cost() { if (!foldRange.type().hasTrait(Trait.IterableType)) { return Cost.Unknown; } - long rangeCnt = 0L; - IteratorT it = ((IterableT) foldRange).iterator(); - while (it.hasNext() == True) { - it.next(); - rangeCnt++; + long rangeCnt; + if (foldRange instanceof Sizer sizer) { + rangeCnt = sizer.nativeSize(); + } else { + rangeCnt = 0L; + IteratorT it = ((IterableT) foldRange).iterator(); + while (it.hasNext() == True) { + it.next(); + rangeCnt++; + } } Cost a = estimateCost(accu); diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretableDecorator.java b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretableDecorator.java index dfc43d6b3..6af0a60b2 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretableDecorator.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretableDecorator.java @@ -15,7 +15,6 @@ */ package org.projectnessie.cel.interpreter; -import static org.projectnessie.cel.common.types.BoolT.False; import static org.projectnessie.cel.common.types.BoolT.True; import static org.projectnessie.cel.common.types.Err.throwErrorAsIllegalStateException; import static org.projectnessie.cel.common.types.IntT.IntZero; @@ -208,7 +207,7 @@ static Interpretable maybeOptimizeSetMembership(Interpretable i, InterpretableCa // always be convertible to a `traits.Lister` type. Lister list = (Lister) l.value(); if (list.size() == IntZero) { - return newConstValue(inlist.id(), False); + return new EvalSetMembership(inlist, lhs, null, Set.of()); } IteratorT it = list.iterator(); Type typ = null; diff --git a/core/src/test/java/org/projectnessie/cel/common/types/BytesTest.java b/core/src/test/java/org/projectnessie/cel/common/types/BytesTest.java index 5494ea72d..0234b4190 100644 --- a/core/src/test/java/org/projectnessie/cel/common/types/BytesTest.java +++ b/core/src/test/java/org/projectnessie/cel/common/types/BytesTest.java @@ -143,7 +143,9 @@ void bytesConvertToType() { @Test void bytesSize() { - assertThat(bytesOf("1234567890").size().equal(intOf(10))).isSameAs(True); + BytesT value = bytesOf("1234567890"); + assertThat(value.size().equal(intOf(10))).isSameAs(True); + assertThat(value.nativeSize()).isEqualTo(10); } @Test diff --git a/core/src/test/java/org/projectnessie/cel/common/types/DoubleTest.java b/core/src/test/java/org/projectnessie/cel/common/types/DoubleTest.java index f544394d9..0e8bb47f1 100644 --- a/core/src/test/java/org/projectnessie/cel/common/types/DoubleTest.java +++ b/core/src/test/java/org/projectnessie/cel/common/types/DoubleTest.java @@ -73,6 +73,11 @@ void doubleConvertToNative_Zeros() { DoubleT n0 = doubleOf(-0.0d); assertThat(p0.equal(n0)).isSameAs(True); assertThat(n0.compare(p0)).isSameAs(IntZero); // "-0.0 < 0.0" --> False + + Double p0Obj = (Double) p0.value(); + Double n0Obj = (Double) n0.value(); + assertThat(Double.doubleToRawLongBits(p0Obj)).isEqualTo(Double.doubleToRawLongBits(0.0d)); + assertThat(Double.doubleToRawLongBits(n0Obj)).isEqualTo(Double.doubleToRawLongBits(-0.0d)); } @Test diff --git a/core/src/test/java/org/projectnessie/cel/common/types/IteratorTTest.java b/core/src/test/java/org/projectnessie/cel/common/types/IteratorTTest.java new file mode 100644 index 000000000..6ed45c1f7 --- /dev/null +++ b/core/src/test/java/org/projectnessie/cel/common/types/IteratorTTest.java @@ -0,0 +1,286 @@ +/* + * Copyright (C) 2026 The Authors of CEL-Java + * + * 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 + * + * http://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 org.projectnessie.cel.common.types; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.projectnessie.cel.common.types.BoolT.BoolType; +import static org.projectnessie.cel.common.types.BoolT.False; +import static org.projectnessie.cel.common.types.BoolT.True; +import static org.projectnessie.cel.common.types.IntT.intOf; +import static org.projectnessie.cel.common.types.IteratorT.IteratorType; +import static org.projectnessie.cel.common.types.NullT.NullValue; +import static org.projectnessie.cel.common.types.StringT.stringOf; +import static org.projectnessie.cel.common.types.TypeT.TypeType; + +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; +import org.projectnessie.cel.common.types.ref.BaseVal; +import org.projectnessie.cel.common.types.ref.TypeAdapter; +import org.projectnessie.cel.common.types.ref.TypeEnum; +import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.common.types.traits.Trait; + +class IteratorTTest { + + @Test + void exposesIteratorRuntimeType() { + IteratorT iterator = + IteratorT.javaIterator(value -> stringOf((String) value), List.of().iterator()); + + assertThat(iterator).isInstanceOf(BaseVal.class); + assertThat(iterator.type()).isSameAs(IteratorType); + assertThat(IteratorType.typeName()).isEqualTo("iterator"); + assertThat(IteratorType.typeEnum()).isEqualTo(TypeEnum.Object); + for (Trait trait : Trait.values()) { + assertThat(IteratorType.hasTrait(trait)) + .as(trait.name()) + .isEqualTo(trait == Trait.IteratorType); + } + } + + @Test + void traversesAndAdaptsNativeElementsOnce() { + AtomicInteger adaptations = new AtomicInteger(); + TypeAdapter adapter = + value -> { + adaptations.incrementAndGet(); + return stringOf((String) value); + }; + CountingIterator delegate = new CountingIterator(List.of("one", "two")); + IteratorT iterator = IteratorT.javaIterator(adapter, delegate); + + assertThat(iterator.hasNext()).isSameAs(True); + assertThat(iterator.hasNext()).isSameAs(True); + assertThat(delegate.nextCalls).isZero(); + assertThat(adaptations).hasValue(0); + + assertThat(iterator.next()).isEqualTo(stringOf("one")); + assertThat(delegate.nextCalls).isEqualTo(1); + assertThat(adaptations).hasValue(1); + assertThat(iterator.next()).isEqualTo(stringOf("two")); + assertThat(delegate.nextCalls).isEqualTo(2); + assertThat(adaptations).hasValue(2); + assertThat(iterator.hasNext()).isSameAs(False); + + assertNoMoreElements(iterator.next()); + assertNoMoreElements(iterator.next()); + assertThat(delegate.nextCalls).isEqualTo(4); + assertThat(adaptations).hasValue(2); + } + + @Test + void returnsValElementsWithoutAdapting() { + AtomicInteger adaptations = new AtomicInteger(); + Val value = intOf(42); + IteratorT iterator = + IteratorT.javaIterator( + ignored -> { + adaptations.incrementAndGet(); + return stringOf("unexpected"); + }, + List.of(value).iterator()); + + assertThat(iterator.next()).isSameAs(value); + assertThat(adaptations).hasValue(0); + } + + @Test + void adaptsNativeNullElement() { + AtomicInteger adaptations = new AtomicInteger(); + IteratorT iterator = + IteratorT.javaIterator( + value -> { + assertThat(value).isNull(); + adaptations.incrementAndGet(); + return NullValue; + }, + Collections.singletonList((Object) null).iterator()); + + assertThat(iterator.next()).isSameAs(NullValue); + assertThat(adaptations).hasValue(1); + } + + @Test + @SuppressWarnings("removal") + void diagnosticsAreStableAndNonConsuming() { + AtomicInteger adaptations = new AtomicInteger(); + CountingIterator delegate = new CountingIterator(List.of("value")); + IteratorT iterator = + IteratorT.javaIterator( + value -> { + adaptations.incrementAndGet(); + return stringOf((String) value); + }, + delegate); + CountingIterator otherDelegate = new CountingIterator(List.of("other")); + IteratorT other = IteratorT.javaIterator(value -> stringOf((String) value), otherDelegate); + int identityHash = iterator.hashCode(); + + assertThat(iterator.type()).isSameAs(IteratorType); + assertThat(iterator.value()).isNull(); + assertCelError(iterator.equal(other), "no such overload: iterator.equal(iterator)"); + assertCelError(iterator.equal(stringOf("other")), "no such overload: iterator.equal(string)"); + assertCelError( + iterator.convertToType(IteratorType), + "type conversion error from 'iterator' to 'iterator'"); + assertCelError( + iterator.convertToType(TypeType), "type conversion error from 'iterator' to 'type'"); + assertCelError( + iterator.convertToType(BoolType), "type conversion error from 'iterator' to 'bool'"); + assertThatThrownBy(() -> iterator.convertToNative(Object.class)) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("type conversion not supported for 'iterator'"); + assertThatThrownBy(() -> iterator.convertToNative(Val.class)) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("type conversion not supported for 'iterator'"); + assertThatThrownBy(() -> iterator.convertToNative(IteratorT.class)) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("type conversion not supported for 'iterator'"); + assertThatThrownBy(iterator::booleanValue) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("iterator cannot be used as boolean"); + assertThatThrownBy(iterator::intValue) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("iterator cannot be used as integer"); + assertThatThrownBy(iterator::doubleValue) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("iterator cannot be used as double"); + + assertThat(iterator).isSameAs(iterator).isNotSameAs(other); + assertThat(iterator.equals(other)).isFalse(); + assertThat(iterator.hashCode()).isEqualTo(identityHash); + assertThat(iterator).hasToString("iterator"); + assertThat(delegate.nextCalls).isZero(); + assertThat(otherDelegate.nextCalls).isZero(); + assertThat(adaptations).hasValue(0); + + assertThat(iterator.next()).isEqualTo(stringOf("value")); + assertThat(iterator.hashCode()).isEqualTo(identityHash); + assertThat(iterator).hasToString("iterator"); + } + + @Test + void validatesFactoryArguments() { + Iterator delegate = List.of().iterator(); + TypeAdapter adapter = value -> stringOf(String.valueOf(value)); + + assertThatThrownBy(() -> IteratorT.javaIterator(null, delegate)) + .isInstanceOf(NullPointerException.class) + .hasMessage("adapter"); + assertThatThrownBy(() -> IteratorT.javaIterator(adapter, null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("iterator"); + } + + @Test + void mapsDelegateExhaustionAndPropagatesOtherFailures() { + IteratorT exhausted = + IteratorT.javaIterator( + value -> stringOf(String.valueOf(value)), + new Iterator<>() { + @Override + public boolean hasNext() { + return true; + } + + @Override + public Object next() { + throw new NoSuchElementException("delegate exhausted"); + } + }); + assertNoMoreElements(exhausted.next()); + + IllegalStateException delegateFailure = new IllegalStateException("delegate failed"); + IteratorT failingDelegate = + IteratorT.javaIterator( + value -> stringOf(String.valueOf(value)), + new Iterator<>() { + @Override + public boolean hasNext() { + return true; + } + + @Override + public Object next() { + throw delegateFailure; + } + }); + assertThatThrownBy(failingDelegate::next).isSameAs(delegateFailure); + } + + @Test + void propagatesAdapterFailureAfterAdvancing() { + AtomicInteger nextCalls = new AtomicInteger(); + IllegalArgumentException adapterFailure = new IllegalArgumentException("adapter failed"); + IteratorT iterator = + IteratorT.javaIterator( + value -> { + throw adapterFailure; + }, + new Iterator<>() { + @Override + public boolean hasNext() { + return true; + } + + @Override + public Object next() { + nextCalls.incrementAndGet(); + return "value"; + } + }); + + assertThatThrownBy(iterator::next).isSameAs(adapterFailure); + assertThat(nextCalls).hasValue(1); + } + + private static void assertNoMoreElements(Val value) { + assertCelError(value, "no more elements"); + } + + private static void assertCelError(Val value, String message) { + assertThat(value).matches(Err::isError).hasToString(message); + } + + private static final class CountingIterator implements Iterator { + private final List values; + private int index; + private int nextCalls; + + private CountingIterator(List values) { + this.values = values; + } + + @Override + public boolean hasNext() { + return index < values.size(); + } + + @Override + public Object next() { + nextCalls++; + if (!hasNext()) { + throw new NoSuchElementException(); + } + return values.get(index++); + } + } +} diff --git a/core/src/test/java/org/projectnessie/cel/common/types/ListTest.java b/core/src/test/java/org/projectnessie/cel/common/types/ListTest.java index de6a823b4..c1c8996af 100644 --- a/core/src/test/java/org/projectnessie/cel/common/types/ListTest.java +++ b/core/src/test/java/org/projectnessie/cel/common/types/ListTest.java @@ -22,6 +22,7 @@ import static org.projectnessie.cel.common.types.DoubleT.doubleOf; import static org.projectnessie.cel.common.types.IntT.IntType; import static org.projectnessie.cel.common.types.IntT.intOf; +import static org.projectnessie.cel.common.types.IteratorT.IteratorType; import static org.projectnessie.cel.common.types.ListT.ListType; import static org.projectnessie.cel.common.types.ListT.newGenericArrayList; import static org.projectnessie.cel.common.types.ListT.newStringArrayList; @@ -47,6 +48,7 @@ import org.projectnessie.cel.common.types.traits.Indexer; import org.projectnessie.cel.common.types.traits.Lister; import org.projectnessie.cel.common.types.traits.Sizer; +import org.projectnessie.cel.common.types.traits.Trait; @TestInstance(Lifecycle.PER_CLASS) public abstract class ListTest { @@ -237,6 +239,7 @@ Lister checkList(TestData tc, Val listVal) { // Sizer.size() int size = tc.sourceSize(); assertThat(list.size()).isEqualTo(intOf(tc.sourceSize())); + assertThat(list.nativeSize()).isEqualTo(size); for (int i = 0; i < size; i++) { Object src = tc.sourceGet(i); @@ -244,6 +247,7 @@ Lister checkList(TestData tc, Val listVal) { // Indexer.get() Val elem = list.get(intOf(i)); + assertThat(list.nativeGetAt(i)).isEqualTo(elem); Object nat = elem.convertToNative((src instanceof Val) ? (Class) Val.class : src.getClass()); assertThat(src).isOfAnyClassIn(nat.getClass()); @@ -268,7 +272,12 @@ Lister checkList(TestData tc, Val listVal) { // IterableT.iterate() IteratorT iter = list.iterator(); - assertThat(iter).isNotNull(); + int iteratorHash = iter.hashCode(); + assertThat(iter).isNotNull().hasToString("iterator"); + assertThat(iter.equals(iter)).isTrue(); + assertThat(iter.equals(list.iterator())).isFalse(); + assertThat(iter.type()).isSameAs(IteratorType); + assertThat(iter.type().hasTrait(Trait.IteratorType)).isTrue(); List collected = new ArrayList<>(); for (int index = 0; iter.hasNext() == True; index++) { Val next = iter.next(); @@ -285,6 +294,8 @@ Lister checkList(TestData tc, Val listVal) { assertThat(iter.hasNext()).isSameAs(False); assertThat(iter.next()).matches(Err::isError); } + assertThat(iter.hashCode()).isEqualTo(iteratorHash); + assertThat(iter).hasToString("iterator"); // Adder.add() assertThat(list.add(NullValue)).matches(Err::isError); diff --git a/core/src/test/java/org/projectnessie/cel/common/types/MapTest.java b/core/src/test/java/org/projectnessie/cel/common/types/MapTest.java index 3d5d02e64..780ed91c0 100644 --- a/core/src/test/java/org/projectnessie/cel/common/types/MapTest.java +++ b/core/src/test/java/org/projectnessie/cel/common/types/MapTest.java @@ -17,29 +17,67 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.projectnessie.cel.common.types.BoolT.False; import static org.projectnessie.cel.common.types.BoolT.True; import static org.projectnessie.cel.common.types.DoubleT.doubleOf; +import static org.projectnessie.cel.common.types.Err.newErr; import static org.projectnessie.cel.common.types.IntT.intOf; +import static org.projectnessie.cel.common.types.IteratorT.IteratorType; +import static org.projectnessie.cel.common.types.MapT.newMaybeWrappedMap; import static org.projectnessie.cel.common.types.MapT.newWrappedMap; +import static org.projectnessie.cel.common.types.NullT.NullValue; import static org.projectnessie.cel.common.types.StringT.stringOf; import static org.projectnessie.cel.common.types.Types.boolOf; import static org.projectnessie.cel.common.types.UintT.uintOf; +import static org.projectnessie.cel.common.types.UnknownT.unknownOf; import static org.projectnessie.cel.common.types.pb.ProtoTypeRegistry.newRegistry; import com.google.common.collect.ImmutableMap; import com.google.protobuf.Struct; +import java.math.BigInteger; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.projectnessie.cel.common.types.pb.DefaultTypeAdapter; import org.projectnessie.cel.common.types.ref.TypeAdapter; import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.common.types.traits.Trait; public class MapTest { + @Test + void unsignedHashesMatchDoubleEqualityAtRoundingBoundaries() { + for (long center : new long[] {Long.MIN_VALUE, 0xc0ecc38b5dee8521L, -4L}) { + for (int delta = -3; delta <= 3; delta++) { + long bits = center + delta; + Val unsigned = uintOf(bits); + Val floating = doubleOf(new BigInteger(Long.toUnsignedString(bits)).doubleValue()); + assertThat(unsigned.equal(floating)).isSameAs(True); + assertThat(unsigned.hashCode()) + .as(Long.toUnsignedString(bits)) + .isEqualTo(floating.hashCode()); + MapT map = (MapT) newWrappedMap(newRegistry(), Map.of(unsigned, stringOf("found"))); + assertThat(map.find(floating)).isEqualTo(stringOf("found")); + } + } + } + + @Test + void javaMapEqualityIsReflexiveForNanValues() { + TypeAdapter adapter = DefaultTypeAdapter.Instance; + for (Val map : + List.of( + MapT.newMaybeWrappedMap(adapter, Map.of("nan", Double.NaN)), + newWrappedMap(adapter, Map.of(stringOf("nan"), doubleOf(Double.NaN))))) { + assertThat(map.equal(map)).isSameAs(BoolT.False); + assertThat(map.equals(map)).isTrue(); + } + } + @Test void nativeMapConversionUsesAdapterRecursively() { TypeAdapter adapter = @@ -83,6 +121,153 @@ void nativeMapConversionPreservesNullAndTypeSemantics() { .hasMessage("type conversion not supported for 'type'"); } + @Test + void numericHashCodeMatchesHeterogeneousCelEquality() { + long[] values = {0L, 1L, 1L << 40, 1L << 53, (1L << 53) + 1, Long.MAX_VALUE}; + for (long value : values) { + Val intValue = intOf(value); + Val uintValue = uintOf(value); + Val doubleValue = doubleOf(value); + + assertThat(intValue).isEqualTo(uintValue); + assertThat(intValue.hashCode()).isEqualTo(uintValue.hashCode()); + assertThat(intValue).isEqualTo(doubleValue); + assertThat(intValue.hashCode()).isEqualTo(doubleValue.hashCode()); + } + + assertThat(doubleOf(-0.0d)).isEqualTo(doubleOf(0.0d)); + assertThat(doubleOf(-0.0d).hashCode()).isEqualTo(doubleOf(0.0d).hashCode()); + } + + @Test + void largeHeterogeneousNumericKeys() { + long key = 1L << 40; + MapT map = + (MapT) + newWrappedMap( + newRegistry(), new HashMap<>(Map.of(intOf(key), stringOf("large value")))); + + assertThat(map.find(intOf(key))).isEqualTo(stringOf("large value")); + assertThat(map.find(uintOf(key))).isEqualTo(stringOf("large value")); + assertThat(map.find(doubleOf(key))).isEqualTo(stringOf("large value")); + + Map duplicateKeys = new HashMap<>(); + duplicateKeys.put(key, "signed"); + duplicateKeys.put((double) key, "double"); + assertThat(newMaybeWrappedMap(newRegistry(), duplicateKeys)).matches(Err::isError); + } + + @Test + void containsPropagatesErrorAndUnknownKeys() { + MapT map = + (MapT) newWrappedMap(newRegistry(), new HashMap<>(Map.of(stringOf("key"), intOf(1)))); + Val error = newErr("key failed"); + Val unknown = unknownOf(42); + + assertThat(map.contains(error)).isSameAs(error); + assertThat(map.contains(unknown)).isSameAs(unknown); + } + + @Test + void equalityPropagatesErrorFromOtherMap() { + Val error = newErr("value failed"); + MapT left = + (MapT) newWrappedMap(newRegistry(), new HashMap<>(Map.of(stringOf("key"), intOf(1)))); + MapT right = (MapT) newWrappedMap(newRegistry(), new HashMap<>(Map.of(stringOf("key"), error))); + + assertThat(left.equal(right)).isSameAs(error); + } + + @Test + void nativeMapEqualityRetainsCelValueSemantics() { + MapT integer = (MapT) newMaybeWrappedMap(DefaultTypeAdapter.Instance, Map.of("key", 50_000L)); + MapT floatingPoint = + (MapT) newMaybeWrappedMap(DefaultTypeAdapter.Instance, Map.of("key", 50_000.0d)); + MapT positiveZero = (MapT) newMaybeWrappedMap(DefaultTypeAdapter.Instance, Map.of("key", 0.0d)); + MapT negativeZero = + (MapT) newMaybeWrappedMap(DefaultTypeAdapter.Instance, Map.of("key", -0.0d)); + MapT nan = (MapT) newMaybeWrappedMap(DefaultTypeAdapter.Instance, Map.of("key", Double.NaN)); + MapT nanCopy = + (MapT) newMaybeWrappedMap(DefaultTypeAdapter.Instance, Map.of("key", Double.NaN)); + Val error = newErr("value failed"); + MapT erroneous = (MapT) newMaybeWrappedMap(DefaultTypeAdapter.Instance, Map.of("key", error)); + + assertThat(integer.equal(floatingPoint)).isSameAs(True); + assertThat(integer).isEqualTo(floatingPoint); + assertThat(positiveZero.equal(negativeZero)).isSameAs(True); + assertThat(positiveZero).isEqualTo(negativeZero); + assertThat(nan.equal(nanCopy)).isSameAs(False); + assertThat(nan).isNotEqualTo(nanCopy); + assertThat(integer.equal(erroneous)).isSameAs(error); + } + + @Test + void nativeMapSnapshotsStructureAndAdaptsValuesOnDemand() { + Object selectedValue = Long.valueOf(50_000L); + AtomicInteger selectedValueAdaptations = new AtomicInteger(); + TypeAdapter adapter = + value -> { + if (value == selectedValue) { + selectedValueAdaptations.incrementAndGet(); + } + return DefaultTypeAdapter.Instance.nativeToValue(value); + }; + Map source = new HashMap<>(); + source.put("selected", selectedValue); + source.put("other", 60_000L); + + MapT map = (MapT) newMaybeWrappedMap(adapter, source); + + assertThat(selectedValueAdaptations).hasValue(0); + assertThat(map.size()).isEqualTo(intOf(2)); + assertThat(map.contains(stringOf("selected"))).isSameAs(True); + assertThat(selectedValueAdaptations).hasValue(0); + + source.put("selected", 70_000L); + source.put("added", 80_000L); + assertThat(map.size()).isEqualTo(intOf(2)); + assertThat(map.find(stringOf("added"))).isNull(); + assertThat(map.find(stringOf("selected"))).isEqualTo(intOf(50_000)); + assertThat(selectedValueAdaptations).hasValue(1); + } + + @Test + void nativeMapDistinguishesNullValueFromMissingKey() { + Map source = new HashMap<>(); + source.put("null", null); + MapT map = (MapT) newMaybeWrappedMap(DefaultTypeAdapter.Instance, source); + + assertThat(map.contains(stringOf("null"))).isSameAs(True); + assertThat(map.find(stringOf("null"))).isSameAs(NullValue); + assertThat(map.contains(stringOf("missing"))).isSameAs(BoolT.False); + assertThat(map.find(stringOf("missing"))).isNull(); + } + + @Test + @SuppressWarnings("removal") + void nativeMapConversionsAdaptCapturedValues() { + MapT map = + (MapT) + newMaybeWrappedMap( + DefaultTypeAdapter.Instance, Map.of("one", 1L, "nested", Map.of("two", 2L))); + + assertThat(map.value()).isEqualTo(Map.of("one", 1L, "nested", Map.of("two", 2L))); + assertThat(map.convertToNative(Map.class)) + .isEqualTo(Map.of("one", 1L, "nested", Map.of("two", 2L))); + assertThat(map.convertToNative(Struct.class).getFieldsMap()).containsOnlyKeys("one", "nested"); + } + + @Test + void mapJavaEqualityAndHashCodeWorkAcrossRepresentations() { + MapT nativeMap = (MapT) newMaybeWrappedMap(DefaultTypeAdapter.Instance, Map.of("key", 50_000L)); + MapT valMap = + (MapT) newWrappedMap(DefaultTypeAdapter.Instance, Map.of(stringOf("key"), intOf(50_000L))); + + assertThat(nativeMap).isEqualTo(valMap); + assertThat(valMap).isEqualTo(nativeMap); + assertThat(nativeMap.hashCode()).isEqualTo(valMap.hashCode()); + } + @Test @SuppressWarnings("removal") void heterogenousKeys() { @@ -137,12 +322,22 @@ void heterogenousKeys() { assertThat(celMap.find(intOf(42))).isNull(); IteratorT iter = celMap.iterator(); + int iteratorHash = iter.hashCode(); + assertThat(iter.type()).isSameAs(IteratorType); + assertThat(iter.type().hasTrait(Trait.IteratorType)).isTrue(); + assertThat(iter).hasToString("iterator"); + assertThat(iter.equals(iter)).isTrue(); + assertThat(iter.equals(celMap.iterator())).isFalse(); Map mapFromIter = new HashMap<>(); while (iter.hasNext() == True) { Val key = iter.next(); mapFromIter.put(key, celMap.find(key)); } assertThat(mapFromIter).hasSize(javaMap.size()).containsAllEntriesOf(javaMap); + assertThat(iter.next()).matches(Err::isError).hasToString("no more elements"); + assertThat(iter.next()).matches(Err::isError).hasToString("no more elements"); + assertThat(iter.hashCode()).isEqualTo(iteratorHash); + assertThat(iter).hasToString("iterator"); assertThat(celMap.convertToNative(Map.class)) .isEqualTo( diff --git a/core/src/test/java/org/projectnessie/cel/common/types/PrimitiveArrayListTest.java b/core/src/test/java/org/projectnessie/cel/common/types/PrimitiveArrayListTest.java new file mode 100644 index 000000000..9ab19f624 --- /dev/null +++ b/core/src/test/java/org/projectnessie/cel/common/types/PrimitiveArrayListTest.java @@ -0,0 +1,106 @@ +/* + * Copyright (C) 2021 The Authors of CEL-Java + * + * 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 + * + * http://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 org.projectnessie.cel.common.types; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.projectnessie.cel.common.types.BoolT.False; +import static org.projectnessie.cel.common.types.BoolT.True; +import static org.projectnessie.cel.common.types.DoubleT.doubleOf; +import static org.projectnessie.cel.common.types.IntT.intOf; +import static org.projectnessie.cel.common.types.ListT.newDoubleArrayList; +import static org.projectnessie.cel.common.types.ListT.newIntArrayList; +import static org.projectnessie.cel.common.types.ListT.newLongArrayList; +import static org.projectnessie.cel.common.types.ListT.newValArrayList; +import static org.projectnessie.cel.common.types.UintT.uintOf; + +import org.junit.jupiter.api.Test; +import org.projectnessie.cel.common.types.pb.DefaultTypeAdapter; +import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.common.types.traits.Lister; + +class PrimitiveArrayListTest { + + @Test + void exactAndHeterogeneousMembership() { + Lister intList = list(newIntArrayList(DefaultTypeAdapter.Instance, new int[] {50_000, 50_001})); + Lister longList = + list(newLongArrayList(DefaultTypeAdapter.Instance, new long[] {50_000, Long.MAX_VALUE})); + Lister doubleList = + list(newDoubleArrayList(DefaultTypeAdapter.Instance, new double[] {-0.0d, 50_000.0d})); + + assertThat(intList.contains(intOf(50_001))).isSameAs(True); + assertThat(intList.contains(uintOf(50_001))).isSameAs(True); + assertThat(intList.contains(doubleOf(50_001))).isSameAs(True); + assertThat(intList.contains(intOf(Long.MAX_VALUE))).isSameAs(False); + assertThat(longList.contains(intOf(Long.MAX_VALUE))).isSameAs(True); + assertThat(doubleList.contains(doubleOf(0.0d))).isSameAs(True); + assertThat(doubleList.contains(doubleOf(Double.NaN))).isSameAs(False); + } + + @Test + void exactAndCrossIntegerEquality() { + Val ints = newIntArrayList(DefaultTypeAdapter.Instance, new int[] {50_000, 50_001}); + Val equalInts = newIntArrayList(DefaultTypeAdapter.Instance, new int[] {50_000, 50_001}); + Val longs = newLongArrayList(DefaultTypeAdapter.Instance, new long[] {50_000, 50_001}); + Val different = newLongArrayList(DefaultTypeAdapter.Instance, new long[] {50_000, 50_002}); + + assertThat(ints.equal(equalInts)).isSameAs(True); + assertThat(ints.equal(longs)).isSameAs(True); + assertThat(longs.equal(ints)).isSameAs(True); + assertThat(ints.equal(different)).isSameAs(False); + } + + @Test + void doubleEqualityPreservesNanAndSignedZeroSemantics() { + Val signedZero = newDoubleArrayList(DefaultTypeAdapter.Instance, new double[] {-0.0d}); + Val positiveZero = newDoubleArrayList(DefaultTypeAdapter.Instance, new double[] {0.0d}); + Val nan = newDoubleArrayList(DefaultTypeAdapter.Instance, new double[] {Double.NaN}); + + assertThat(signedZero.equal(positiveZero)).isSameAs(True); + assertThat(nan.equal(nan)).isSameAs(False); + } + + @Test + void iterationAndPublicIndexBehaviorRemainUnchanged() { + Lister values = + list(newIntArrayList(DefaultTypeAdapter.Instance, new int[] {50_000, 50_001, 50_002})); + IteratorT iterator = values.iterator(); + + assertThat(iterator.next()).isEqualTo(intOf(50_000)); + assertThat(iterator.next()).isEqualTo(intOf(50_001)); + assertThat(iterator.next()).isEqualTo(intOf(50_002)); + assertThat(iterator.hasNext()).isSameAs(False); + assertThat(values.get(intOf(-1))).matches(Err::isError); + assertThat(values.get(doubleOf(0.5d))).matches(Err::isError); + assertThat(values.nativeGetAt(-1)).matches(Err::isError); + assertThat(values.nativeGetAt(values.nativeSize())).matches(Err::isError); + assertThat(values.nativeGetAt(1)).isEqualTo(intOf(50_001)); + } + + @Test + void genericEqualityFallbackRemainsAvailable() { + Val primitive = newIntArrayList(DefaultTypeAdapter.Instance, new int[] {50_000, 50_001}); + Val generic = + newValArrayList(DefaultTypeAdapter.Instance, new Val[] {intOf(50_000), intOf(50_001)}); + + assertThat(primitive.equal(generic)).isSameAs(True); + assertThat(generic.equal(primitive)).isSameAs(True); + } + + private static Lister list(Val value) { + return (Lister) value; + } +} diff --git a/core/src/test/java/org/projectnessie/cel/common/types/StringTest.java b/core/src/test/java/org/projectnessie/cel/common/types/StringTest.java index 5a203defa..6b57535df 100644 --- a/core/src/test/java/org/projectnessie/cel/common/types/StringTest.java +++ b/core/src/test/java/org/projectnessie/cel/common/types/StringTest.java @@ -232,6 +232,7 @@ void stringSize() { assertThat(stringOf("").size()).isSameAs(IntZero); assertThat(stringOf("hello world").size()).isEqualTo(intOf(11)); assertThat(stringOf("\u65e5\u672c\u8a9e").size()).isEqualTo(intOf(3)); + assertThat(stringOf("\u65e5\u672c\u8a9e").nativeSize()).isEqualTo(3); } @Test diff --git a/core/src/test/java/org/projectnessie/cel/common/types/pb/FieldDescriptionTest.java b/core/src/test/java/org/projectnessie/cel/common/types/pb/FieldDescriptionTest.java index f20a56bff..62099f224 100644 --- a/core/src/test/java/org/projectnessie/cel/common/types/pb/FieldDescriptionTest.java +++ b/core/src/test/java/org/projectnessie/cel/common/types/pb/FieldDescriptionTest.java @@ -16,8 +16,10 @@ package org.projectnessie.cel.common.types.pb; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.projectnessie.cel.common.types.BoolT.False; import static org.projectnessie.cel.common.types.BoolT.True; +import static org.projectnessie.cel.common.types.IntT.intOf; import static org.projectnessie.cel.common.types.StringT.stringOf; import static org.projectnessie.cel.common.types.UintT.uintOf; import static org.projectnessie.cel.common.types.pb.Db.newDb; @@ -40,16 +42,111 @@ import java.time.Instant; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import org.projectnessie.cel.common.ULong; +import org.projectnessie.cel.common.types.BoolT; +import org.projectnessie.cel.common.types.Err; +import org.projectnessie.cel.common.types.IntT; +import org.projectnessie.cel.common.types.IteratorT; +import org.projectnessie.cel.common.types.MapT; import org.projectnessie.cel.common.types.TimestampT; import org.projectnessie.cel.common.types.traits.Mapper; public class FieldDescriptionTest { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void unsignedReadersAgreeForGeneratedAndDynamicMessages(boolean dynamic) throws Exception { + TestAllTypes generated = + TestAllTypes.newBuilder() + .setSingleUint32(-1) + .setSingleFixed32(-1) + .setSingleUint64(-1L) + .setSingleFixed64(-1L) + .addRepeatedUint32(Integer.MIN_VALUE) + .addRepeatedUint32(-1) + .addRepeatedFixed32(Integer.MIN_VALUE) + .addRepeatedFixed32(-1) + .addRepeatedUint64(Long.MIN_VALUE) + .addRepeatedUint64(-1L) + .addRepeatedFixed64(Long.MIN_VALUE) + .addRepeatedFixed64(-1L) + .build(); + Message message = + dynamic + ? DynamicMessage.parseFrom(TestAllTypes.getDescriptor(), generated.toByteString()) + : generated; + ProtoTypeRegistry registry = ProtoTypeRegistry.newRegistry(generated); + for (String name : + List.of("single_uint32", "single_fixed32", "single_uint64", "single_fixed64")) { + long expected = name.endsWith("32") ? 0xffffffffL : -1L; + FieldDescriptor descriptor = message.getDescriptorForType().findFieldByName(name); + assertThat(FieldDescription.getValueFromField(descriptor, message)) + .as(name) + .isEqualTo(ULong.valueOf(expected)); + assertThat( + registry + .findFieldType(descriptor.getContainingType().getFullName(), name) + .getFrom + .getFrom(message)) + .as(name) + .isEqualTo(ULong.valueOf(expected)); + } + for (String name : + List.of("repeated_uint32", "repeated_fixed32", "repeated_uint64", "repeated_fixed64")) { + List expected = + name.endsWith("32") + ? List.of(ULong.valueOf(0x80000000L), ULong.valueOf(0xffffffffL)) + : List.of(ULong.valueOf(Long.MIN_VALUE), ULong.valueOf(-1L)); + FieldDescriptor descriptor = message.getDescriptorForType().findFieldByName(name); + assertThat(FieldDescription.getValueFromField(descriptor, message)) + .as(name) + .isEqualTo(expected); + assertThat( + registry + .findFieldType(descriptor.getContainingType().getFullName(), name) + .getFrom + .getFrom(message)) + .as(name) + .isEqualTo(expected); + } + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void protobufMapIteratorsFollowTheIteratorProtocol(boolean dynamic) throws Exception { + TestAllTypes generated = TestAllTypes.newBuilder().putMapStringString("key", "value").build(); + Message message = + dynamic + ? DynamicMessage.parseFrom(TestAllTypes.getDescriptor(), generated.toByteString()) + : generated; + ProtoTypeRegistry registry = ProtoTypeRegistry.newRegistry(generated); + MapT map = + (MapT) + registry + .findFieldType(TestAllTypes.getDescriptor().getFullName(), "map_string_string") + .getFrom + .getFrom(message); + IteratorT iterator = map.iterator(); + assertThat(iterator.type()).isSameAs(IteratorT.IteratorType); + assertThat(iterator.value()).isNull(); + assertThat(iterator.equal(iterator)).matches(Err::isError); + assertThat(iterator.convertToType(IntT.IntType)).matches(Err::isError); + assertThatThrownBy(iterator::intValue) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("iterator cannot be used as integer"); + assertThat(iterator.equals(iterator)).isTrue(); + assertThat(iterator.equals(map.iterator())).isFalse(); + assertThat(iterator.next()).isEqualTo(stringOf("key")); + assertThat(iterator.hasNext()).isSameAs(BoolT.False); + assertThat(iterator.next()).matches(Err::isError); + } + @Test void fieldDescription() { Db pbdb = newDb(); @@ -109,6 +206,12 @@ static GetFromTestCase[] getFromTestCases() { new GetFromTestCase() .field("single_nested_message") .want(NestedMessage.newBuilder().setBb(123).build()), + new GetFromTestCase() + .field("repeated_nested_message") + .want( + Arrays.asList( + NestedMessage.newBuilder().setBb(456).build(), + NestedMessage.newBuilder().setBb(789).build())), new GetFromTestCase().field("standalone_enum").want(NestedEnum.BAR.getValueDescriptor()), new GetFromTestCase().field("single_value").want("hello world"), new GetFromTestCase() @@ -135,6 +238,8 @@ void getFrom(GetFromTestCase tc) { builder.setSingleInt32Wrapper(Int32Value.of(42)); builder.setStandaloneEnum(NestedEnum.BAR); builder.setSingleNestedMessage(NestedMessage.newBuilder().setBb(123)); + builder.addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(456)); + builder.addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(789)); builder.setSingleValue(Value.newBuilder().setStringValue("hello world")); builder.setSingleStruct( Struct.newBuilder() @@ -154,6 +259,27 @@ void getFrom(GetFromTestCase tc) { assertThat(got).isEqualTo(tc.want); } + @Test + void getFromRepeatedMessageFieldOnDynamicMessage() { + Db pbdb = newDb(); + TestAllTypes generated = + TestAllTypes.newBuilder() + .addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(456)) + .addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(789)) + .build(); + pbdb.registerMessage(generated); + PbTypeDescription type = pbdb.describeType(generated.getDescriptorForType().getFullName()); + FieldDescription field = type.fieldByName("repeated_nested_message"); + DynamicMessage dynamic = + DynamicMessage.newBuilder(generated.getDescriptorForType()).mergeFrom(generated).build(); + + List values = (List) field.getFrom(pbdb, dynamic); + + assertThat(values).hasSize(2).allMatch(Message.class::isInstance); + Message first = (Message) values.get(0); + assertThat(first.getField(first.getDescriptorForType().findFieldByName("bb"))).isEqualTo(456); + } + @Test void getFromUnsignedMapEntries() { Db pbdb = newDb(); @@ -204,14 +330,33 @@ void getFieldProtoMapSupportsRepeatedLookup() { Mapper map = (Mapper) field.getField(msg, registry); assertThat(map.find(uintOf(2)).equal(stringOf("two"))).isSameAs(True); + assertThat(map.find(intOf(2)).equal(stringOf("two"))).isSameAs(True); assertThat(map.find(uintOf(-1L)).equal(stringOf("large"))).isSameAs(True); assertThat(map.find(uintOf(1)).equal(stringOf("one"))).isSameAs(True); assertThat(map.find(uintOf(42))).isNull(); assertThat(map.contains(uintOf(2))).isSameAs(True); assertThat(map.contains(uintOf(42))).isSameAs(False); + + DynamicMessage dynamicMessage = + DynamicMessage.newBuilder(msg.getDescriptorForType()).mergeFrom(msg).build(); + Mapper dynamicMap = (Mapper) field.getField(dynamicMessage, registry); + assertThat(dynamicMap.find(uintOf(2)).equal(stringOf("two"))).isSameAs(True); + assertThat(((MapT) map).equal((MapT) dynamicMap)).isSameAs(True); + assertThat(dynamicMap).isEqualTo(map); + assertThat(dynamicMap.hashCode()).isEqualTo(map.hashCode()); + + TestAllTypes equalMessage = msg.toBuilder().build(); + Mapper equalGeneratedMap = (Mapper) field.getField(equalMessage, registry); + assertThat(((MapT) map).equal((MapT) equalGeneratedMap)).isSameAs(True); + assertThat(map).isEqualTo(equalGeneratedMap); + + TestAllTypes differentMessage = msg.toBuilder().putMapUint64String(2L, "different").build(); + Mapper differentGeneratedMap = (Mapper) field.getField(differentMessage, registry); + assertThat(((MapT) map).equal((MapT) differentGeneratedMap)).isSameAs(False); } @Test + @SuppressWarnings("removal") void dynamicProtoMapUsesTheLastDuplicateEntry() { FieldDescriptor mapField = TestAllTypes.getDescriptor().findFieldByName("map_string_int64"); FieldDescriptor keyField = mapField.getMessageType().findFieldByNumber(1); diff --git a/core/src/test/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessorTest.java b/core/src/test/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessorTest.java index 3cf535df6..40a7ed9ea 100644 --- a/core/src/test/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessorTest.java +++ b/core/src/test/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessorTest.java @@ -16,17 +16,27 @@ package org.projectnessie.cel.common.types.pb; import static org.assertj.core.api.Assertions.assertThat; +import static org.projectnessie.cel.common.types.BoolT.True; +import static org.projectnessie.cel.common.types.IntT.intOf; +import static org.projectnessie.cel.common.types.StringT.stringOf; +import static org.projectnessie.cel.common.types.UintT.uintOf; import static org.projectnessie.cel.common.types.pb.Db.newDb; import com.google.protobuf.ByteString; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.DynamicMessage; +import com.google.protobuf.Int32Value; +import com.google.protobuf.NullValue; import dev.cel.expr.conformance.proto3.TestAllTypes; import dev.cel.expr.conformance.proto3.TestAllTypes.NestedEnum; import java.util.List; import org.junit.jupiter.api.Test; +import org.projectnessie.cel.common.ULong; +import org.projectnessie.cel.common.types.MapT; import org.projectnessie.cel.common.types.ref.FieldGetter; +import org.projectnessie.cel.common.types.ref.FieldTester; import org.projectnessie.cel.common.types.ref.FieldType; +import org.projectnessie.cel.common.types.traits.Lister; class GeneratedFieldAccessorTest { @@ -73,7 +83,7 @@ void bindsSupportedGeneratedScalarGetters() { } @Test - void excludesFieldsThatNeedSpecializedNormalization() { + void bindsFieldsWithSpecializedNormalization() { Db db = newDb(); db.registerMessage(TestAllTypes.getDefaultInstance()); PbTypeDescription type = db.describeType(TestAllTypes.getDescriptor().getFullName()); @@ -82,20 +92,127 @@ void excludesFieldsThatNeedSpecializedNormalization() { List.of( "single_uint32", "standalone_enum", - "single_nested_message", "single_int32_wrapper", "repeated_int32", - "map_string_string")) { + "map_string_string", + "single_any", + "single_duration", + "single_timestamp")) { assertThat(GeneratedFieldAccessor.create(type, type.fieldByName(fieldName))) + .as(fieldName) + .isNotNull(); + } + + assertThat(GeneratedFieldAccessor.create(type, type.fieldByName("optional_null_value"))) + .isNull(); + assertThat(GeneratedFieldAccessor.create(type, type.fieldByName("repeated_null_value"))) + .isNotNull(); + assertThat(GeneratedFieldAccessor.create(type, type.fieldByName("single_nested_message"))) + .isNull(); + assertThat( + GeneratedFieldAccessor.createForObject(type, type.fieldByName("single_nested_message"))) + .isNotNull(); + } + + @Test + void bindsOnlyGeneratedPresenceMethods() { + Db db = newDb(); + db.registerMessage(TestAllTypes.getDefaultInstance()); + PbTypeDescription type = db.describeType(TestAllTypes.getDescriptor().getFullName()); + TestAllTypes present = + TestAllTypes.newBuilder() + .setOptionalBool(false) + .setSingleNestedMessage(TestAllTypes.NestedMessage.getDefaultInstance()) + .setSingleInt32Wrapper(Int32Value.getDefaultInstance()) + .build(); + + for (String fieldName : + List.of("optional_bool", "single_nested_message", "single_int32_wrapper")) { + FieldTester tester = GeneratedFieldAccessor.createTester(type, type.fieldByName(fieldName)); + assertThat(tester).as(fieldName).isNotNull(); + assertThat(tester.isSet(TestAllTypes.getDefaultInstance())).as(fieldName).isFalse(); + assertThat(tester.isSet(present)).as(fieldName).isTrue(); + } + + for (String fieldName : + List.of("single_int32", "standalone_enum", "repeated_int32", "map_string_string")) { + assertThat(GeneratedFieldAccessor.createTester(type, type.fieldByName(fieldName))) .as(fieldName) .isNull(); } } + @Test + void registryNormalizesGeneratedFieldValues() { + TestAllTypes message = + TestAllTypes.newBuilder() + .setSingleUint32(-1) + .setSingleUint64(Long.MIN_VALUE) + .setStandaloneEnumValue(12_345) + .setSingleNestedMessage(TestAllTypes.NestedMessage.newBuilder().setBb(50_000)) + .setSingleInt32Wrapper(Int32Value.of(50_000)) + .addRepeatedUint32(-1) + .addRepeatedNestedEnumValue(12_345) + .addRepeatedNullValue(NullValue.NULL_VALUE) + .putMapStringString("key", "value") + .putMapUint32Uint64(-1, Long.MIN_VALUE) + .putMapStringEnumValue("unknown", 12_345) + .putMapBoolNullValue(true, NullValue.NULL_VALUE) + .build(); + ProtoTypeRegistry registry = ProtoTypeRegistry.newRegistry(TestAllTypes.getDefaultInstance()); + String typeName = message.getDescriptorForType().getFullName(); + + assertThat(registry.findFieldType(typeName, "single_uint32").getFrom.getFrom(message)) + .isEqualTo(ULong.valueOf(0xffffffffL)); + assertThat(registry.findFieldType(typeName, "single_uint64").getFrom.getFrom(message)) + .isEqualTo(ULong.valueOf(Long.MIN_VALUE)); + assertThat(registry.findFieldType(typeName, "standalone_enum").getFrom.getFrom(message)) + .isEqualTo(12_345); + assertThat(registry.findFieldType(typeName, "single_nested_message").getFrom.getFrom(message)) + .isEqualTo(message.getSingleNestedMessage()); + assertThat(registry.findFieldType(typeName, "single_int32_wrapper").getFrom.getFrom(message)) + .isEqualTo(Int32Value.of(50_000)); + assertThat( + registry + .findFieldType(typeName, "single_int32_wrapper") + .getFrom + .getFrom(TestAllTypes.getDefaultInstance())) + .isEqualTo(NullValue.NULL_VALUE); + assertThat(registry.findFieldType(typeName, "repeated_uint32").getFrom.getFrom(message)) + .isEqualTo(List.of(ULong.valueOf(0xffffffffL))); + assertThat(registry.findFieldType(typeName, "repeated_nested_enum").getFrom.getFrom(message)) + .isEqualTo(List.of(12_345)); + + Object mapValue = + registry.findFieldType(typeName, "map_string_string").getFrom.getFrom(message); + assertThat(mapValue).isInstanceOf(MapT.class); + assertThat(((MapT) mapValue).get(stringOf("key"))).isEqualTo(stringOf("value")); + MapT unsignedMap = + (MapT) registry.findFieldType(typeName, "map_uint32_uint64").getFrom.getFrom(message); + assertThat(unsignedMap.get(uintOf(0xffffffffL))).isEqualTo(uintOf(Long.MIN_VALUE)); + MapT enumMap = + (MapT) registry.findFieldType(typeName, "map_string_enum").getFrom.getFrom(message); + assertThat(enumMap.get(stringOf("unknown"))).isEqualTo(intOf(12_345)); + MapT nullMap = + (MapT) registry.findFieldType(typeName, "map_bool_null_value").getFrom.getFrom(message); + assertThat(nullMap.get(True)).isEqualTo(intOf(0)); + + PbObjectT object = (PbObjectT) registry.nativeToValue(message); + assertThat(object.get(stringOf("single_uint32"))).isEqualTo(uintOf(0xffffffffL)); + assertThat(object.get(stringOf("standalone_enum"))).isEqualTo(intOf(12_345)); + assertThat(object.get(stringOf("single_int32_wrapper"))).isEqualTo(intOf(50_000)); + assertThat(((Lister) object.get(stringOf("repeated_null_value"))).get(intOf(0))) + .isEqualTo(intOf(0)); + } + @Test void generatedRegistryGetterFallsBackForDynamicMessages() { TestAllTypes generated = - TestAllTypes.newBuilder().setSingleInt32(42).setStandaloneEnum(NestedEnum.BAR).build(); + TestAllTypes.newBuilder() + .setSingleInt32(42) + .setStandaloneEnum(NestedEnum.BAR) + .setSingleNestedMessage(TestAllTypes.NestedMessage.newBuilder().setBb(50_000)) + .build(); DynamicMessage dynamic = DynamicMessage.newBuilder(generated.getDescriptorForType()).mergeFrom(generated).build(); ProtoTypeRegistry registry = ProtoTypeRegistry.newRegistry(TestAllTypes.getDefaultInstance()); @@ -110,6 +227,16 @@ void generatedRegistryGetterFallsBackForDynamicMessages() { FieldDescriptor enumDescriptor = generated.getDescriptorForType().findFieldByName("standalone_enum"); assertThat(enumField.getFrom.getFrom(dynamic)).isEqualTo(dynamic.getField(enumDescriptor)); + + PbObjectT generatedObject = (PbObjectT) registry.nativeToValue(generated); + PbObjectT dynamicObject = (PbObjectT) registry.nativeToValue(dynamic); + assertThat(generatedObject.get(stringOf("single_nested_message"))) + .isEqualTo(registry.nativeToValue(generated.getSingleNestedMessage())); + assertThat(dynamicObject.get(stringOf("single_nested_message"))) + .isEqualTo( + registry.nativeToValue( + dynamic.getField( + generated.getDescriptorForType().findFieldByName("single_nested_message")))); } @Test @@ -136,5 +263,23 @@ void dynamicTypeDescriptionDoesNotBindGeneratedGetter() { PbTypeDescription type = db.describeType(zero.getDescriptorForType().getFullName()); assertThat(GeneratedFieldAccessor.create(type, type.fieldByName("single_int32"))).isNull(); + assertThat(GeneratedFieldAccessor.createTester(type, type.fieldByName("optional_bool"))) + .isNull(); + } + + @Test + void generatedRegistrationInvalidatesDescriptorOnlyFieldCache() { + DynamicMessage dynamicZero = DynamicMessage.getDefaultInstance(TestAllTypes.getDescriptor()); + ProtoTypeRegistry registry = ProtoTypeRegistry.newEmptyRegistry(); + registry.registerMessage(dynamicZero); + String typeName = dynamicZero.getDescriptorForType().getFullName(); + + FieldType descriptorOnly = registry.findFieldType(typeName, "single_int32"); + registry.registerMessage(TestAllTypes.getDefaultInstance()); + FieldType generated = registry.findFieldType(typeName, "single_int32"); + + assertThat(generated).isNotSameAs(descriptorOnly); + assertThat(generated.getFrom.getFrom(TestAllTypes.newBuilder().setSingleInt32(50_000).build())) + .isEqualTo(50_000); } } diff --git a/core/src/test/java/org/projectnessie/cel/interpreter/AttributesTest.java b/core/src/test/java/org/projectnessie/cel/interpreter/AttributesTest.java index 67122412e..5001a589d 100644 --- a/core/src/test/java/org/projectnessie/cel/interpreter/AttributesTest.java +++ b/core/src/test/java/org/projectnessie/cel/interpreter/AttributesTest.java @@ -23,16 +23,20 @@ import static org.projectnessie.cel.common.containers.Container.newContainer; import static org.projectnessie.cel.common.types.BoolT.False; import static org.projectnessie.cel.common.types.BoolT.True; +import static org.projectnessie.cel.common.types.DoubleT.doubleOf; +import static org.projectnessie.cel.common.types.Err.isError; import static org.projectnessie.cel.common.types.Err.newErr; import static org.projectnessie.cel.common.types.IntT.IntType; import static org.projectnessie.cel.common.types.IntT.intOf; import static org.projectnessie.cel.common.types.StringT.stringOf; +import static org.projectnessie.cel.common.types.UintT.uintOf; import static org.projectnessie.cel.common.types.UnknownT.unknownOf; import static org.projectnessie.cel.common.types.pb.ProtoTypeRegistry.newRegistry; import static org.projectnessie.cel.interpreter.Activation.emptyActivation; import static org.projectnessie.cel.interpreter.Activation.newActivation; import static org.projectnessie.cel.interpreter.Activation.newPartialActivation; import static org.projectnessie.cel.interpreter.AttributeFactory.newAttributeFactory; +import static org.projectnessie.cel.interpreter.AttributeFactory.refResolve; import static org.projectnessie.cel.interpreter.AttributePattern.newAttributePattern; import static org.projectnessie.cel.interpreter.AttributePattern.newPartialAttributeFactory; import static org.projectnessie.cel.interpreter.Coster.Cost.estimateCost; @@ -47,6 +51,8 @@ import com.google.protobuf.Any; import dev.cel.expr.conformance.proto3.TestAllTypes; import dev.cel.expr.conformance.proto3.TestAllTypes.NestedMessage; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Proxy; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -64,6 +70,7 @@ import org.projectnessie.cel.common.types.pb.DefaultTypeAdapter; import org.projectnessie.cel.common.types.ref.TypeRegistry; import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.common.types.traits.Lister; import org.projectnessie.cel.interpreter.AttributeFactory.Attribute; import org.projectnessie.cel.interpreter.AttributeFactory.NamespacedAttribute; import org.projectnessie.cel.interpreter.AttributeFactory.Qualifier; @@ -73,6 +80,41 @@ class AttributesTest { + @Test + void refResolvePreservesCustomListIndexTypes() { + Lister list = + (Lister) + Proxy.newProxyInstance( + getClass().getClassLoader(), + new Class[] {Lister.class}, + (proxy, method, arguments) -> { + if (method.getName().equals("get")) { + return arguments[0]; + } + if (method.isDefault()) { + return InvocationHandler.invokeDefault(proxy, method, arguments); + } + throw new AssertionError(method); + }); + for (Val index : List.of(intOf(42), uintOf(42), doubleOf(42))) { + Val result = refResolve(value -> list, index, list); + assertThat(result.type()).isSameAs(index.type()); + } + } + + @Test + void refResolveUsesNativeListIndicesWithoutChangingIndexSemantics() { + TypeRegistry registry = newRegistry(); + int[] values = {2, 42}; + + assertThat(refResolve(registry, intOf(1), values)).isEqualTo(intOf(42)); + assertThat(refResolve(registry, uintOf(1), values)).isEqualTo(intOf(42)); + assertThat(refResolve(registry, doubleOf(1), values)).isEqualTo(intOf(42)); + assertThat(refResolve(registry, doubleOf(0.5), values)).matches(v -> isError(v)); + assertThat(refResolve(registry, intOf((long) Integer.MAX_VALUE + 1), values)) + .matches(v -> isError(v)); + } + @Test void attributesAbsoluteAttr() { TypeRegistry reg = newRegistry(); diff --git a/core/src/test/java/org/projectnessie/cel/interpreter/SetMembershipOptimizationTest.java b/core/src/test/java/org/projectnessie/cel/interpreter/SetMembershipOptimizationTest.java new file mode 100644 index 000000000..e24aa5f2a --- /dev/null +++ b/core/src/test/java/org/projectnessie/cel/interpreter/SetMembershipOptimizationTest.java @@ -0,0 +1,110 @@ +/* + * Copyright (C) 2021 The Authors of CEL-Java + * + * 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 + * + * http://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 org.projectnessie.cel.interpreter; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.projectnessie.cel.common.types.BoolT.False; +import static org.projectnessie.cel.common.types.Err.newErr; +import static org.projectnessie.cel.common.types.IntT.intOf; +import static org.projectnessie.cel.common.types.ListT.newValArrayList; +import static org.projectnessie.cel.common.types.UnknownT.unknownOf; +import static org.projectnessie.cel.interpreter.Activation.emptyActivation; +import static org.projectnessie.cel.interpreter.Interpretable.newConstValue; + +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; +import org.projectnessie.cel.common.types.Overloads; +import org.projectnessie.cel.common.types.pb.DefaultTypeAdapter; +import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.interpreter.Interpretable.InterpretableCall; + +class SetMembershipOptimizationTest { + + @Test + void emptySetStillEvaluatesLeftOperand() { + AtomicInteger evaluations = new AtomicInteger(); + Interpretable lhs = countingValue(intOf(42), evaluations); + Interpretable optimized = optimize(lhs, new Val[0]); + + assertThat(optimized.eval(emptyActivation())).isSameAs(False); + assertThat(evaluations).hasValue(1); + } + + @Test + void optimizedMembershipPropagatesErrorAndUnknown() { + Val error = newErr("left failed"); + Val unknown = unknownOf(42); + + assertThat(optimize(newConstValue(1, error), new Val[] {intOf(1)}).eval(emptyActivation())) + .isSameAs(error); + assertThat(optimize(newConstValue(1, unknown), new Val[] {intOf(1)}).eval(emptyActivation())) + .isSameAs(unknown); + } + + private static Interpretable optimize(Interpretable lhs, Val[] values) { + Interpretable rhs = newConstValue(2, newValArrayList(DefaultTypeAdapter.Instance, values)); + return InterpretableDecorator.maybeOptimizeSetMembership( + new MembershipCall(lhs, rhs), new MembershipCall(lhs, rhs)); + } + + private static Interpretable countingValue(Val value, AtomicInteger evaluations) { + return new Interpretable() { + @Override + public long id() { + return 1; + } + + @Override + public Val eval(Activation activation) { + evaluations.incrementAndGet(); + return value; + } + }; + } + + private static final class MembershipCall implements InterpretableCall { + private final Interpretable[] args; + + private MembershipCall(Interpretable lhs, Interpretable rhs) { + this.args = new Interpretable[] {lhs, rhs}; + } + + @Override + public long id() { + return 3; + } + + @Override + public Val eval(Activation activation) { + throw new UnsupportedOperationException(); + } + + @Override + public String function() { + return "@in"; + } + + @Override + public String overloadID() { + return Overloads.InList; + } + + @Override + public Interpretable[] args() { + return args; + } + } +} diff --git a/core/src/test/java/org/projectnessie/cel/publicapi/IteratorPublicApiCompileTest.java b/core/src/test/java/org/projectnessie/cel/publicapi/IteratorPublicApiCompileTest.java new file mode 100644 index 000000000..58c7b5140 --- /dev/null +++ b/core/src/test/java/org/projectnessie/cel/publicapi/IteratorPublicApiCompileTest.java @@ -0,0 +1,135 @@ +/* + * Copyright (C) 2026 The Authors of CEL-Java + * + * 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 + * + * http://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 org.projectnessie.cel.publicapi; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.projectnessie.cel.common.types.BoolT.False; +import static org.projectnessie.cel.common.types.Err.noMoreElements; + +import org.junit.jupiter.api.Test; +import org.projectnessie.cel.common.types.IteratorT; +import org.projectnessie.cel.common.types.ref.BaseVal; +import org.projectnessie.cel.common.types.ref.Val; + +/** Compile-time fixture for direct external implementations of the public iterator protocol. */ +class IteratorPublicApiCompileTest { + + @Test + void directImplementationNeedsOnlyTraversalMethods() { + IteratorT iterator = new ExternalIterator(); + IteratorT other = new ExternalIterator(); + int identityHash = iterator.hashCode(); + + assertThat(iterator.type()).isSameAs(IteratorT.IteratorType); + assertThat(iterator.value()).isNull(); + assertThat(iterator.hasNext()).isSameAs(False); + assertThat(iterator.next()).matches(org.projectnessie.cel.common.types.Err::isError); + assertThat(iterator.equals(iterator)).isTrue(); + assertThat(iterator.equals(other)).isFalse(); + assertThat(iterator.hashCode()).isEqualTo(identityHash); + } + + @Test + void externalOverrideStillWins() { + IteratorT iterator = new OverridingIterator(); + + assertThat(iterator.value()).isEqualTo("external"); + } + + @Test + void baseValImplementationCanSelectIteratorPrimitiveDiagnostics() { + IteratorT iterator = new ExternalBaseValIterator(); + IteratorT other = new ExternalBaseValIterator(); + int identityHash = iterator.hashCode(); + + assertThat(iterator.type()).isSameAs(IteratorT.IteratorType); + assertThatThrownBy(iterator::booleanValue) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("iterator cannot be used as boolean"); + assertThatThrownBy(iterator::intValue) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("iterator cannot be used as integer"); + assertThatThrownBy(iterator::doubleValue) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("iterator cannot be used as double"); + assertThat(iterator.equals(iterator)).isTrue(); + assertThat(iterator.equals(other)).isFalse(); + assertThat(iterator.hashCode()).isEqualTo(identityHash); + assertThat(iterator).hasToString("iterator"); + } + + private static class ExternalIterator implements IteratorT { + @Override + public Val hasNext() { + return False; + } + + @Override + public Val next() { + return noMoreElements(); + } + } + + private static final class OverridingIterator extends ExternalIterator { + @Override + public Object value() { + return "external"; + } + } + + private static final class ExternalBaseValIterator extends BaseVal implements IteratorT { + @Override + public Val hasNext() { + return False; + } + + @Override + public Val next() { + return noMoreElements(); + } + + @Override + public boolean booleanValue() { + return IteratorT.super.booleanValue(); + } + + @Override + public long intValue() { + return IteratorT.super.intValue(); + } + + @Override + public double doubleValue() { + return IteratorT.super.doubleValue(); + } + + @Override + public boolean equals(Object other) { + return this == other; + } + + @Override + public int hashCode() { + return System.identityHashCode(this); + } + + @Override + public String toString() { + return "iterator"; + } + } +} diff --git a/core/src/test/java/org/projectnessie/cel/publicapi/MapPublicApiCompileTest.java b/core/src/test/java/org/projectnessie/cel/publicapi/MapPublicApiCompileTest.java new file mode 100644 index 000000000..acd94a482 --- /dev/null +++ b/core/src/test/java/org/projectnessie/cel/publicapi/MapPublicApiCompileTest.java @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2026 The Authors of CEL-Java + * + * 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 + * + * http://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 org.projectnessie.cel.publicapi; + +import org.projectnessie.cel.common.types.MapT; + +/** Compile-time fixture preserving external maps' existing Java equality overrides. */ +abstract class MapPublicApiCompileTest extends MapT { + @Override + public boolean equals(Object other) { + return this == other; + } + + @Override + public int hashCode() { + return System.identityHashCode(this); + } +} diff --git a/jackson/src/main/java/org/projectnessie/cel/types/jackson/JacksonObjectT.java b/jackson/src/main/java/org/projectnessie/cel/types/jackson/JacksonObjectT.java index 0267bd69a..feb619751 100644 --- a/jackson/src/main/java/org/projectnessie/cel/types/jackson/JacksonObjectT.java +++ b/jackson/src/main/java/org/projectnessie/cel/types/jackson/JacksonObjectT.java @@ -78,7 +78,7 @@ public Val get(Val index) { } @Override - @SuppressWarnings("removal") + @SuppressWarnings({"removal", "unchecked"}) public T convertToNative(Class typeDesc) { if (typeDesc.isAssignableFrom(value.getClass())) { return (T) value; diff --git a/jackson3/src/main/java/org/projectnessie/cel/types/jackson3/JacksonObjectT.java b/jackson3/src/main/java/org/projectnessie/cel/types/jackson3/JacksonObjectT.java index 7018a4f50..cf7e098dc 100644 --- a/jackson3/src/main/java/org/projectnessie/cel/types/jackson3/JacksonObjectT.java +++ b/jackson3/src/main/java/org/projectnessie/cel/types/jackson3/JacksonObjectT.java @@ -78,7 +78,7 @@ public Val get(Val index) { } @Override - @SuppressWarnings("removal") + @SuppressWarnings({"removal", "unchecked"}) public T convertToNative(Class typeDesc) { if (typeDesc.isAssignableFrom(value.getClass())) { return (T) value; From 9c87311fdd540384e204c249d0ccc19da0a1f818 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Tue, 27 Oct 2026 15:48:24 +0100 Subject: [PATCH 2/3] Preserve Java map equality contracts Compare map values using Java equality while retaining CEL equality for evaluation. Restrict cross-implementation Java equality to the built-in native and wrapped representations so custom maps can retain identity semantics. Add regressions for unknown expression IDs, hash-based lookup, and symmetric equality with empty and populated custom maps. --- .../projectnessie/cel/common/types/MapT.java | 24 +++- .../cel/common/types/MapTest.java | 109 ++++++++++++++++++ 2 files changed, 130 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/projectnessie/cel/common/types/MapT.java b/core/src/main/java/org/projectnessie/cel/common/types/MapT.java index 5e1894a1b..cb5c3ebfe 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/MapT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/MapT.java @@ -29,6 +29,7 @@ import com.google.protobuf.Value; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import org.projectnessie.cel.common.types.ref.BaseVal; import org.projectnessie.cel.common.types.ref.Type; import org.projectnessie.cel.common.types.ref.TypeAdapter; @@ -123,6 +124,10 @@ public Type type() { } protected final Val mapEqual(Val other) { + return mapEqual(other, false); + } + + private Val mapEqual(Val other, boolean javaEquality) { if (!(other instanceof MapT o)) { return False; } @@ -139,7 +144,7 @@ protected final Val mapEqual(Val other) { if (oVal == null) { return False; } - Val equal = mapValueEqual(value, oVal); + Val equal = javaEquality ? boolOf(Objects.equals(value, oVal)) : mapValueEqual(value, oVal); if (equal != True) { return equal; } @@ -156,7 +161,7 @@ protected final Val mapEqual(Val other) { if (oVal == null) { return False; } - Val eq = mapValueEqual(val, oVal); + Val eq = javaEquality ? boolOf(Objects.equals(val, oVal)) : mapValueEqual(val, oVal); if (eq == True) { continue; } @@ -201,7 +206,20 @@ protected static Val mapValueEqual(Val value, Val otherValue) { @Override public boolean equals(Object other) { - return this == other || other instanceof MapT o && mapEqual(o) == True; + if (this == other) { + return true; + } + if (!(other instanceof MapT o)) { + return false; + } + // Built-in representations share Java equality. External implementations may override it. + if (getClass() != o.getClass() + && !((this instanceof ValMapT || this instanceof NativeMapT) + && (o instanceof ValMapT || o instanceof NativeMapT))) { + return false; + } + // CEL equality can ignore Java identity data, such as an unknown value's expression ID. + return mapEqual(o, true) == True; } @Override diff --git a/core/src/test/java/org/projectnessie/cel/common/types/MapTest.java b/core/src/test/java/org/projectnessie/cel/common/types/MapTest.java index 780ed91c0..1a06bf865 100644 --- a/core/src/test/java/org/projectnessie/cel/common/types/MapTest.java +++ b/core/src/test/java/org/projectnessie/cel/common/types/MapTest.java @@ -37,12 +37,14 @@ import java.math.BigInteger; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.projectnessie.cel.common.types.pb.DefaultTypeAdapter; +import org.projectnessie.cel.common.types.ref.Type; import org.projectnessie.cel.common.types.ref.TypeAdapter; import org.projectnessie.cel.common.types.ref.Val; import org.projectnessie.cel.common.types.traits.Trait; @@ -78,6 +80,113 @@ void javaMapEqualityIsReflexiveForNanValues() { } } + @Test + void javaMapEqualityDistinguishesUnknownExpressionIds() { + TypeAdapter adapter = DefaultTypeAdapter.Instance; + List maps = + List.of( + (MapT) newWrappedMap(adapter, Map.of(stringOf("key"), unknownOf(1))), + (MapT) newMaybeWrappedMap(adapter, Map.of("key", unknownOf(1)))); + List different = + List.of( + (MapT) newWrappedMap(adapter, Map.of(stringOf("key"), unknownOf(2))), + (MapT) newMaybeWrappedMap(adapter, Map.of("key", unknownOf(2)))); + for (MapT map : maps) { + for (MapT same : maps) { + assertThat(map.equals(same)).isTrue(); + assertThat(map.hashCode()).isEqualTo(same.hashCode()); + assertThat(new HashSet<>(List.of(map))).contains(same); + } + for (MapT other : different) { + assertThat(map.equal(other)).isSameAs(True); + assertThat(map.equals(other)).isFalse(); + assertThat(other.equals(map)).isFalse(); + } + } + } + + @Test + void javaMapEqualityRespectsCustomIdentityEquality() { + TypeAdapter adapter = DefaultTypeAdapter.Instance; + for (Map entries : List.of(Map.of(), Map.of("key", 1L))) { + MapT nativeMap = (MapT) newMaybeWrappedMap(adapter, entries); + Map wrapped = new HashMap<>(); + entries.forEach((key, value) -> wrapped.put(stringOf(key), intOf(value))); + MapT valMap = (MapT) newWrappedMap(adapter, wrapped); + MapT custom = new IdentityMap(valMap); + for (MapT builtin : List.of(nativeMap, valMap)) { + assertThat(builtin.equal(custom)).isSameAs(True); + assertThat(builtin.equals(custom)).isFalse(); + assertThat(custom.equals(builtin)).isFalse(); + } + assertThat(custom.equals(custom)).isTrue(); + } + } + + private static final class IdentityMap extends MapT { + private final MapT delegate; + + private IdentityMap(MapT delegate) { + this.delegate = delegate; + } + + @Override + public boolean equals(Object other) { + return this == other; + } + + @Override + public int hashCode() { + return System.identityHashCode(this); + } + + @Override + @SuppressWarnings("removal") + public T convertToNative(Class type) { + throw new UnsupportedOperationException(); + } + + @Override + public Val convertToType(Type type) { + return delegate.convertToType(type); + } + + @Override + public Val equal(Val other) { + return delegate.equal(other); + } + + @Override + public Object value() { + return delegate.value(); + } + + @Override + public Val find(Val key) { + return delegate.find(key); + } + + @Override + public Val get(Val key) { + return delegate.get(key); + } + + @Override + public Val contains(Val key) { + return delegate.contains(key); + } + + @Override + public Val size() { + return delegate.size(); + } + + @Override + public IteratorT iterator() { + return delegate.iterator(); + } + } + @Test void nativeMapConversionUsesAdapterRecursively() { TypeAdapter adapter = From 5367cf060c50783cbd5a8c22ab0e77ed786e7b07 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Wed, 28 Oct 2026 09:04:49 +0100 Subject: [PATCH 3/3] Preserve empty repeated protobuf wrapper fields Limit absent-wrapper null substitution to singular fields. Generated getters must retain empty lists for repeated wrappers, matching descriptor access. Cover all nine wrapper types with empty and populated generated and dynamic messages through registry and object getters. --- .../common/types/pb/ProtoTypeRegistry.java | 2 +- .../types/pb/GeneratedFieldAccessorTest.java | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java b/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java index 87b548cef..a2324fd8c 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java @@ -288,7 +288,7 @@ private FieldGetter bindGeneratedGetter( FieldGetter generatedGetter, FieldTester tester) { Class generatedType = type.reflectType(); - if (field.isWrapper()) { + if (field.isWrapper() && !field.descriptor().isRepeated()) { return target -> generatedType.isInstance(target) ? !tester.isSet(target) diff --git a/core/src/test/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessorTest.java b/core/src/test/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessorTest.java index 40a7ed9ea..b52f8923c 100644 --- a/core/src/test/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessorTest.java +++ b/core/src/test/java/org/projectnessie/cel/common/types/pb/GeneratedFieldAccessorTest.java @@ -26,11 +26,14 @@ import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.DynamicMessage; import com.google.protobuf.Int32Value; +import com.google.protobuf.Message; import com.google.protobuf.NullValue; import dev.cel.expr.conformance.proto3.TestAllTypes; import dev.cel.expr.conformance.proto3.TestAllTypes.NestedEnum; import java.util.List; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.projectnessie.cel.common.ULong; import org.projectnessie.cel.common.types.MapT; import org.projectnessie.cel.common.types.ref.FieldGetter; @@ -205,6 +208,46 @@ void registryNormalizesGeneratedFieldValues() { .isEqualTo(intOf(0)); } + @ParameterizedTest + @ValueSource( + strings = { + "repeated_int64_wrapper", "repeated_int32_wrapper", "repeated_double_wrapper", + "repeated_float_wrapper", "repeated_uint64_wrapper", "repeated_uint32_wrapper", + "repeated_string_wrapper", "repeated_bool_wrapper", "repeated_bytes_wrapper" + }) + void repeatedWrapperGettersPreserveLists(String fieldName) { + TestAllTypes empty = TestAllTypes.getDefaultInstance(); + FieldDescriptor descriptor = empty.getDescriptorForType().findFieldByName(fieldName); + Message wrapper = empty.newBuilderForType().newBuilderForField(descriptor).build(); + TestAllTypes populated = empty.toBuilder().addRepeatedField(descriptor, wrapper).build(); + ProtoTypeRegistry registry = ProtoTypeRegistry.newRegistry(empty); + String typeName = empty.getDescriptorForType().getFullName(); + FieldType field = registry.findFieldType(typeName, fieldName); + FieldType objectField = registry.findFieldTypeForObjectAccess(typeName, fieldName, false); + assertThat(objectField).isNotNull(); + + for (TestAllTypes generated : List.of(empty, populated)) { + int expectedSize = generated.getRepeatedFieldCount(descriptor); + DynamicMessage dynamic = + DynamicMessage.newBuilder(generated.getDescriptorForType()).mergeFrom(generated).build(); + for (Message target : List.of(generated, dynamic)) { + for (FieldType getter : List.of(field, objectField)) { + Object value = getter.getFrom.getFrom(target); + assertThat(value).isInstanceOf(List.class); + assertThat((List) value).hasSize(expectedSize); + assertThat(getter.isSet.isSet(target)).isEqualTo(expectedSize != 0); + } + PbObjectT object = (PbObjectT) registry.nativeToValue(target); + assertThat(object.get(stringOf(fieldName))).isInstanceOf(Lister.class); + Lister list = (Lister) object.get(stringOf(fieldName)); + assertThat(list.size()).isEqualTo(intOf(expectedSize)); + if (expectedSize != 0) { + assertThat(list.get(intOf(0))).isEqualTo(registry.nativeToValue(wrapper)); + } + } + } + } + @Test void generatedRegistryGetterFallsBackForDynamicMessages() { TestAllTypes generated =