From 08ab677902c7b040ff4b2a3fe4f6f5d1a991f905 Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Fri, 30 Oct 2026 16:29:59 -0700 Subject: [PATCH] Align container equality, membership checks, and async memoization with IEEE 754. PiperOrigin-RevId: 1010874460 --- .../main/java/dev/cel/common/CelOptions.java | 9 + .../java/dev/cel/common/CelOptionsTest.java | 1 + .../test/java/dev/cel/conformance/BUILD.bazel | 11 +- .../runtime/ProtoMessageRuntimeEquality.java | 22 +- .../java/dev/cel/runtime/RuntimeEquality.java | 178 ++++++++------ .../cel/runtime/planner/AsyncCallRecord.java | 9 +- .../ProtoMessageRuntimeEqualityTest.java | 31 ++- .../dev/cel/runtime/RuntimeEqualityTest.java | 228 +++++++++++++++--- .../runtime/planner/AsyncCallRecordTest.java | 31 ++- .../planner/ProgramPlannerAsyncTest.java | 24 +- 10 files changed, 384 insertions(+), 160 deletions(-) diff --git a/common/src/main/java/dev/cel/common/CelOptions.java b/common/src/main/java/dev/cel/common/CelOptions.java index 3e4dfd54b..e1063e567 100644 --- a/common/src/main/java/dev/cel/common/CelOptions.java +++ b/common/src/main/java/dev/cel/common/CelOptions.java @@ -120,6 +120,8 @@ public enum ProtoUnsetFieldOptions { public abstract boolean enableTimestampOverflowCheck(); + public abstract boolean enableIeee754ContainerEquality(); + public abstract int maxRegexProgramSize(); public abstract Builder toBuilder(); @@ -166,6 +168,7 @@ public static Builder newBuilder() { .fromProtoUnsetFieldOption(ProtoUnsetFieldOptions.BIND_DEFAULT) .enableComprehension(true) .enableTimestampOverflowCheck(true) + .enableIeee754ContainerEquality(true) .maxRegexProgramSize(-1); } @@ -525,6 +528,12 @@ public abstract static class Builder { */ public abstract Builder enableTimestampOverflowCheck(boolean value); + /** + * Enforces IEEE 754 NaN inequality on identical container references instead of + * short-circuiting on {@code x == y} (enabled by default). + */ + public abstract Builder enableIeee754ContainerEquality(boolean value); + public abstract CelOptions build(); } } diff --git a/common/src/test/java/dev/cel/common/CelOptionsTest.java b/common/src/test/java/dev/cel/common/CelOptionsTest.java index 53eb4fed4..d551f1d01 100644 --- a/common/src/test/java/dev/cel/common/CelOptionsTest.java +++ b/common/src/test/java/dev/cel/common/CelOptionsTest.java @@ -35,5 +35,6 @@ public void current_defaults() { // Defaults that aren't represented in deprecated CelOptions assertThat(CelOptions.current().build().enableUnknownTracking()).isFalse(); assertThat(CelOptions.current().build().enablePrattParser()).isFalse(); + assertThat(CelOptions.current().build().enableIeee754ContainerEquality()).isTrue(); } } diff --git a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel index 28e1abbb3..8a455005f 100644 --- a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel +++ b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel @@ -138,16 +138,7 @@ _ALL_TESTS = [ ] _TESTS_TO_SKIP_SPEC_IMPORT = [ - # Skip until duration/timestamp range, container NaN/negative-zero equality, and proto wrapper validation are updated for spec parity. - "comparisons/bound/bound_list_nan_eq_self_false", - "comparisons/bound/bound_list_nan_ne_self_true", - "comparisons/bound/bound_map_nan_eq_self_false", - "comparisons/bound/bound_map_nan_ne_self_true", - "comparisons/in_list_literal/nan_not_in_list", - "comparisons/in_list_literal/nested_list_nan_not_in_list", - "comparisons/in_list_literal/nested_list_neg_zero_in_list", - "comparisons/in_list_literal/nested_map_nan_not_in_list", - "comparisons/in_list_literal/nested_map_neg_zero_in_list", + # Skip until duration/timestamp range and proto wrapper validation are updated for spec parity. "timestamps/duration_conversions/from_string_max", "timestamps/duration_conversions/from_string_min", "timestamps/duration_conversions/toString_duration_max", diff --git a/runtime/src/main/java/dev/cel/runtime/ProtoMessageRuntimeEquality.java b/runtime/src/main/java/dev/cel/runtime/ProtoMessageRuntimeEquality.java index 1c82e7fd9..5a75b0079 100644 --- a/runtime/src/main/java/dev/cel/runtime/ProtoMessageRuntimeEquality.java +++ b/runtime/src/main/java/dev/cel/runtime/ProtoMessageRuntimeEquality.java @@ -16,11 +16,11 @@ import com.google.errorprone.annotations.Immutable; import com.google.protobuf.Message; +import com.google.protobuf.MessageLiteOrBuilder; import dev.cel.common.CelOptions; import dev.cel.common.annotations.Internal; import dev.cel.common.internal.DynamicProto; import dev.cel.common.internal.ProtoEquality; -import java.util.Objects; /** * ProtoMessageRuntimeEquality contains methods for performing CEL related equality checks, @@ -41,23 +41,11 @@ public static ProtoMessageRuntimeEquality create( } @Override - public boolean objectEquals(Object x, Object y) { - if (celOptions.disableCelStandardEquality()) { - return Objects.equals(x, y); + boolean protoEquals(MessageLiteOrBuilder x, MessageLiteOrBuilder y) { + if (x instanceof Message && y instanceof Message) { + return protoEquality.equals((Message) x, (Message) y); } - - if (celOptions.enableProtoDifferencerEquality()) { - x = runtimeHelpers.adaptValue(x); - y = runtimeHelpers.adaptValue(y); - if (x instanceof Message) { - if (!(y instanceof Message)) { - return false; - } - return protoEquality.equals((Message) x, (Message) y); - } - } - - return super.objectEquals(x, y); + return super.protoEquals(x, y); } @Override diff --git a/runtime/src/main/java/dev/cel/runtime/RuntimeEquality.java b/runtime/src/main/java/dev/cel/runtime/RuntimeEquality.java index c8cffe115..bc39be47f 100644 --- a/runtime/src/main/java/dev/cel/runtime/RuntimeEquality.java +++ b/runtime/src/main/java/dev/cel/runtime/RuntimeEquality.java @@ -24,10 +24,9 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; -import java.util.Set; +import java.util.RandomAccess; /** RuntimeEquality contains methods for performing CEL related equality checks. */ @Immutable @@ -45,10 +44,14 @@ public static RuntimeEquality create(RuntimeHelpers runtimeHelper, CelOptions ce /** Determine whether the {@code list} contains the given {@code value}. */ public boolean inList(List list, A value) { - if (list.contains(value)) { - return true; - } - if (value instanceof Number) { + if (list instanceof RandomAccess) { + int size = list.size(); + for (int i = 0; i < size; i++) { + if (objectEquals(list.get(i), value)) { + return true; + } + } + } else { for (A elem : list) { if (objectEquals(elem, value)) { return true; @@ -83,6 +86,9 @@ public Optional findInMap(Map map, Object index) { if (index instanceof MessageLiteOrBuilder) { index = runtimeHelpers.adaptProtoToValue((MessageLiteOrBuilder) index); } + if (isNan(index)) { + return Optional.empty(); + } Object v = map.get(index); if (v != null) { return Optional.of(v); @@ -135,100 +141,116 @@ public Optional findInMap(Map map, Object index) { * comparable even if they are not of the same type, where type differences are usually trivially * false. */ - @SuppressWarnings({"rawtypes", "unchecked", "ReferenceEquality"}) + @SuppressWarnings("ReferenceEquality") public boolean objectEquals(Object x, Object y) { if (celOptions.disableCelStandardEquality()) { return Objects.equals(x, y); } - if (x instanceof Optional) { - if (!(y instanceof Optional)) { - return false; - } - Optional xOpt = (Optional) x; - Optional yOpt = (Optional) y; - return xOpt.isPresent() == yOpt.isPresent() - && (!xOpt.isPresent() || objectEquals(xOpt.get(), yOpt.get())); + if (x == y) { + return !containsNan(x); } - if (x == y && !isNan(x)) { - return true; + if (x instanceof Optional) { + return y instanceof Optional && optionalEquals((Optional) x, (Optional) y); } x = runtimeHelpers.adaptValue(x); y = runtimeHelpers.adaptValue(y); - if (x instanceof Number) { - if (!(y instanceof Number)) { - return false; - } - return ComparisonFunctions.numericEquals((Number) x, (Number) y); + if (ComparisonFunctions.isNumeric(x)) { + return y instanceof Number && ComparisonFunctions.numericEquals((Number) x, (Number) y); } - if (celOptions.enableProtoDifferencerEquality()) { - if (x instanceof MessageLiteOrBuilder) { - if (!(y instanceof MessageLiteOrBuilder)) { - return false; - } - // TODO: Implement when CelLiteDescriptor is available - throw new UnsupportedOperationException( - "Proto Differencer equality is not supported for MessageLite."); - } + if (celOptions.enableProtoDifferencerEquality() && x instanceof MessageLiteOrBuilder) { + return y instanceof MessageLiteOrBuilder + && protoEquals((MessageLiteOrBuilder) x, (MessageLiteOrBuilder) y); } if (x instanceof Iterable) { - if (!(y instanceof Iterable)) { + return y instanceof Iterable && iterableEquals((Iterable) x, (Iterable) y); + } + if (x instanceof Map) { + return y instanceof Map && mapEquals((Map) x, (Map) y); + } + return Objects.equals(x, y); + } + + boolean protoEquals(MessageLiteOrBuilder x, MessageLiteOrBuilder y) { + // TODO: Implement when CelLiteDescriptor is available + throw new UnsupportedOperationException( + "Proto Differencer equality is not supported for MessageLite."); + } + + private boolean optionalEquals(Optional x, Optional y) { + return x.isPresent() == y.isPresent() && (!x.isPresent() || objectEquals(x.get(), y.get())); + } + + private boolean iterableEquals(Iterable x, Iterable y) { + if (areRandomAccessLists(x, y)) { + List xList = (List) x; + List yList = (List) y; + int size = xList.size(); + if (size != yList.size()) { return false; } - Iterable xIter = (Iterable) x; - Iterable yIter = (Iterable) y; - Iterator yElems = yIter.iterator(); - IllegalArgumentException e = null; - for (Object xElem : xIter) { - if (!yElems.hasNext()) { + for (int i = 0; i < size; i++) { + if (!objectEquals(xList.get(i), yList.get(i))) { return false; } - try { - if (!objectEquals(xElem, yElems.next())) { - return false; - } - } catch (IllegalArgumentException iae) { - e = iae; + } + } else { + Iterator yElems = y.iterator(); + for (Object xElem : x) { + if (!yElems.hasNext() || !objectEquals(xElem, yElems.next())) { + return false; } } if (yElems.hasNext()) { return false; } - if (e != null) { - throw e; - } - return true; } - if (x instanceof Map) { - if (!(y instanceof Map)) { - return false; - } - Map xMap = (Map) x; - Map yMap = (Map) y; - if (xMap.size() != yMap.size()) { + return true; + } + + private boolean mapEquals(Map x, Map y) { + if (x.size() != y.size()) { + return false; + } + for (Map.Entry xEntry : x.entrySet()) { + Optional yVal = findInMap(y, xEntry.getKey()); + // Use isPresent() rather than isEmpty() to stay backwards compatible with Java 8. + if (!yVal.isPresent() || !objectEquals(xEntry.getValue(), yVal.get())) { return false; } - IllegalArgumentException e = null; - Set entrySet = xMap.entrySet(); - for (Map.Entry xEntry : entrySet) { - Optional yVal = findInMap(yMap, xEntry.getKey()); - // Use isPresent() rather than isEmpty() to stay backwards compatible with Java 8. - if (!yVal.isPresent()) { - return false; - } - try { - if (!objectEquals(xEntry.getValue(), yVal.get())) { - return false; - } - } catch (IllegalArgumentException iae) { - e = iae; + } + return true; + } + + private boolean containsNan(Object v) { + if (v instanceof Optional) { + Optional opt = (Optional) v; + return opt.isPresent() && containsNan(opt.get()); + } + if (!celOptions.enableIeee754ContainerEquality() + && (v instanceof Iterable || v instanceof Map)) { + return false; + } + v = runtimeHelpers.adaptValue(v); + if (v instanceof Double) { + return ((Double) v).isNaN(); + } + if (celOptions.enableProtoDifferencerEquality() && v instanceof MessageLiteOrBuilder) { + return !protoEquals((MessageLiteOrBuilder) v, (MessageLiteOrBuilder) v); + } + if (v instanceof Iterable) { + return !iterableEquals((Iterable) v, (Iterable) v); + } + if (v instanceof Map) { + for (Map.Entry entry : ((Map) v).entrySet()) { + if (entry.getValue() == null + || containsNan(entry.getKey()) + || containsNan(entry.getValue())) { + return true; } } - if (e != null) { - throw e; - } - return true; + return false; } - return Objects.equals(x, y); + return false; } /** @@ -294,6 +316,14 @@ private static boolean isNan(Object value) { return value instanceof Number && Double.isNaN(((Number) value).doubleValue()); } + private static boolean areRandomAccessLists(Iterable x, Iterable y) { + // NOMUTANTS -- RandomAccess fast path is allocation-free; Iterator fallback is equivalent. + return x instanceof List + && x instanceof RandomAccess + && y instanceof List + && y instanceof RandomAccess; + } + RuntimeEquality(RuntimeHelpers runtimeHelpers, CelOptions celOptions) { this.runtimeHelpers = runtimeHelpers; this.celOptions = celOptions; diff --git a/runtime/src/main/java/dev/cel/runtime/planner/AsyncCallRecord.java b/runtime/src/main/java/dev/cel/runtime/planner/AsyncCallRecord.java index 7223cfbb1..1ea636030 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/AsyncCallRecord.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/AsyncCallRecord.java @@ -21,6 +21,7 @@ import dev.cel.runtime.CelAsyncCall; import dev.cel.runtime.CelAsyncFunctionOverload; import dev.cel.runtime.RuntimeEquality; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import org.jspecify.annotations.Nullable; @@ -96,9 +97,6 @@ static int hashCall(long exprId, String overloadId, Object[] args) { /** * Returns whether this record tracks a call to the same expression node, function, overload, and * arguments. - * - *

Arguments are compared under CEL equality, except that NaN compares equal to itself so that - * a node re-evaluated with a NaN argument can find its existing record. */ boolean matches( long exprId, @@ -115,7 +113,10 @@ boolean matches( for (int i = 0; i < this.args.length; i++) { Object arg = this.args[i]; Object otherArg = args[i]; - if (!runtimeEquality.objectEquals(arg, otherArg) && !(isNan(arg) && isNan(otherArg))) { + // Treat Java-equal containers and scalar NaNs as matching so NaN args memoize across passes. + if (!Objects.equals(arg, otherArg) + && !runtimeEquality.objectEquals(arg, otherArg) + && !(isNan(arg) && isNan(otherArg))) { return false; } } diff --git a/runtime/src/test/java/dev/cel/runtime/ProtoMessageRuntimeEqualityTest.java b/runtime/src/test/java/dev/cel/runtime/ProtoMessageRuntimeEqualityTest.java index 3f41cc5f5..58cb11aea 100644 --- a/runtime/src/test/java/dev/cel/runtime/ProtoMessageRuntimeEqualityTest.java +++ b/runtime/src/test/java/dev/cel/runtime/ProtoMessageRuntimeEqualityTest.java @@ -279,6 +279,18 @@ public abstract static class Builder { @Parameters public static List data() { + AttributeContext nanAttributeContext = + AttributeContext.newBuilder() + .setRequest( + Request.newBuilder() + .setAuth( + Auth.newBuilder() + .setClaims( + Struct.newBuilder() + .putFields( + "custom", + Value.newBuilder().setNumberValue(Double.NaN).build())))) + .build(); return Arrays.asList( new Object[][] { // Boolean tests. @@ -638,19 +650,12 @@ public static List data() { // Message.equals() treats NaN values as equal. Message differencer treats NaN values // as inequal (the same behavior as the C++ implementation). { - AttributeContext.newBuilder() - .setRequest( - Request.newBuilder() - .setAuth( - Auth.newBuilder() - .setClaims( - Struct.newBuilder() - .putFields( - "custom", - Value.newBuilder() - .setNumberValue(Double.NaN) - .build())))) - .build(), + nanAttributeContext, + nanAttributeContext, + Result.proto(/* equalsOutcome= */ true, /* diffOutcome= */ false), + }, + { + nanAttributeContext, AttributeContext.newBuilder() .setRequest( Request.newBuilder() diff --git a/runtime/src/test/java/dev/cel/runtime/RuntimeEqualityTest.java b/runtime/src/test/java/dev/cel/runtime/RuntimeEqualityTest.java index 78caeca01..de729eb1a 100644 --- a/runtime/src/test/java/dev/cel/runtime/RuntimeEqualityTest.java +++ b/runtime/src/test/java/dev/cel/runtime/RuntimeEqualityTest.java @@ -20,10 +20,19 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.primitives.UnsignedLong; +import com.google.protobuf.MessageLite; +import com.google.protobuf.MessageLiteOrBuilder; +import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import dev.cel.common.CelOptions; +import dev.cel.common.internal.DefaultDescriptorPool; +import dev.cel.common.internal.DefaultMessageFactory; +import dev.cel.common.internal.DynamicProto; import dev.cel.common.values.ProtoMessageLiteValueProvider; import dev.cel.expr.conformance.proto2.TestAllTypes; +import java.util.Collections; +import java.util.LinkedList; +import java.util.Map; import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @@ -31,47 +40,183 @@ @RunWith(TestParameterInjector.class) public final class RuntimeEqualityTest { + private static final RuntimeEquality RUNTIME_EQUALITY = + RuntimeEquality.create(RuntimeHelpers.create(), CelOptions.DEFAULT); + + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum EqualPairTestCase { + INTS(1, 1), + INT_AND_LONG(2, 2L), + INT_AND_DOUBLE(3, 3.0), + INT_AND_ULONG(4, UnsignedLong.valueOf(4)), + POSITIVE_AND_NEGATIVE_ZERO(0.0d, -0.0d), + NEGATIVE_ZERO_AND_LONG_ZERO(-0.0d, 0L), + NEGATIVE_ZERO_AND_ULONG_ZERO(-0.0d, UnsignedLong.ZERO), + HETEROGENEOUS_LISTS( + ImmutableList.of(1, 2, 3), ImmutableList.of(1.0, 2L, UnsignedLong.valueOf(3))), + HETEROGENEOUS_LINKED_LISTS( + new LinkedList<>(ImmutableList.of(1, 2, 3)), + new LinkedList<>(ImmutableList.of(1.0, 2L, UnsignedLong.valueOf(3)))), + HETEROGENEOUS_MAPS( + ImmutableMap.of("a", 1, "b", 2), ImmutableMap.of("a", 1L, "b", UnsignedLong.valueOf(2))), + EMPTY_OPTIONALS(Optional.empty(), Optional.empty()), + PRESENT_HETEROGENEOUS_OPTIONALS(Optional.of(1), Optional.of(1.0)), + SHORTS((short) 200, (short) 200); + + private final Object x; + private final Object y; + + EqualPairTestCase(Object x, Object y) { + this.x = x; + this.y = y; + } + } + + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum IdenticalNanReferenceTestCase { + DOUBLE(Double.NaN), + FLOAT(Float.NaN), + OPTIONAL(Optional.of(Double.NaN)), + LIST(ImmutableList.of(Double.NaN)), + LINKED_LIST(new LinkedList<>(ImmutableList.of(Double.NaN))), + MAP_VALUE(ImmutableMap.of("k", Double.NaN)), + MAP_DOUBLE_NAN_KEY(ImmutableMap.of(Double.NaN, "v")), + MAP_FLOAT_NAN_KEY(ImmutableMap.of(Float.NaN, "v")); + + private final Object value; + + IdenticalNanReferenceTestCase(Object value) { + this.value = value; + } + } + + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum UnequalContainerPairTestCase { + LIST_ELEMENTS(ImmutableList.of(1L), ImmutableList.of(2L)), + LIST_SIZES(ImmutableList.of(1L), ImmutableList.of(1L, 2L)), + LINKED_LIST_ELEMENTS( + new LinkedList<>(ImmutableList.of(1L)), new LinkedList<>(ImmutableList.of(2L))), + LINKED_LIST_SIZES( + new LinkedList<>(ImmutableList.of(1L)), new LinkedList<>(ImmutableList.of(1L, 2L))), + MAP_VALUES(ImmutableMap.of("a", 1L), ImmutableMap.of("a", 2L)), + MAP_SIZES(ImmutableMap.of("a", 1L), ImmutableMap.of("a", 1L, "b", 2L)); + + private final Object x; + private final Object y; + + UnequalContainerPairTestCase(Object x, Object y) { + this.x = x; + this.y = y; + } + } + + @Test + public void objectEquals_and_hashCode(@TestParameter EqualPairTestCase testCase) { + assertThat(RUNTIME_EQUALITY.objectEquals(testCase.x, testCase.y)).isTrue(); + assertThat(RUNTIME_EQUALITY.objectEquals(testCase.y, testCase.x)).isTrue(); + assertThat(RUNTIME_EQUALITY.hashCode(testCase.x)) + .isEqualTo(RUNTIME_EQUALITY.hashCode(testCase.y)); + } + @Test - public void objectEquals_and_hashCode() { - RuntimeEquality runtimeEquality = - RuntimeEquality.create(RuntimeHelpers.create(), CelOptions.DEFAULT); - assertEqualityAndHashCode(runtimeEquality, 1, 1); - assertEqualityAndHashCode(runtimeEquality, 2, 2L); - assertEqualityAndHashCode(runtimeEquality, 3, 3.0); - assertEqualityAndHashCode(runtimeEquality, 4, UnsignedLong.valueOf(4)); - assertEqualityAndHashCode(runtimeEquality, 0.0d, -0.0d); - assertEqualityAndHashCode( - runtimeEquality, - ImmutableList.of(1, 2, 3), - ImmutableList.of(1.0, 2L, UnsignedLong.valueOf(3))); - assertEqualityAndHashCode( - runtimeEquality, - ImmutableMap.of("a", 1, "b", 2), - ImmutableMap.of("a", 1L, "b", UnsignedLong.valueOf(2))); - assertEqualityAndHashCode(runtimeEquality, Optional.empty(), Optional.empty()); - assertEqualityAndHashCode(runtimeEquality, Optional.of(1), Optional.of(1.0)); - } - - private void assertEqualityAndHashCode(RuntimeEquality runtimeEquality, Object obj1, Object obj2) { - assertThat(runtimeEquality.objectEquals(obj1, obj2)).isTrue(); - assertThat(runtimeEquality.hashCode(obj1)).isEqualTo(runtimeEquality.hashCode(obj2)); + public void objectEquals_unequalContainers_returnsFalse( + @TestParameter UnequalContainerPairTestCase testCase) { + assertThat(RUNTIME_EQUALITY.objectEquals(testCase.x, testCase.y)).isFalse(); + assertThat(RUNTIME_EQUALITY.objectEquals(testCase.y, testCase.x)).isFalse(); + } + + @Test + public void objectEquals_unsupportedNumberSubtypes_returnsFalse() { + assertThat(RUNTIME_EQUALITY.objectEquals((short) 1, 1L)).isFalse(); + assertThat(RUNTIME_EQUALITY.objectEquals(1L, (short) 1)).isFalse(); + } + + @Test + public void inList_nan_returnsFalse() { + assertThat(RUNTIME_EQUALITY.inList(ImmutableList.of(Double.NaN), Double.NaN)).isFalse(); + assertThat( + RUNTIME_EQUALITY.inList( + ImmutableList.of(ImmutableList.of(Double.NaN)), ImmutableList.of(Double.NaN))) + .isFalse(); + } + + @Test + public void inList_nonRandomAccessList() { + LinkedList linkedList = new LinkedList<>(ImmutableList.of(1L, "a", 0L)); + + assertThat(RUNTIME_EQUALITY.inList(linkedList, -0.0d)).isTrue(); + assertThat(RUNTIME_EQUALITY.inList(linkedList, Double.NaN)).isFalse(); + } + + @Test + public void inMap_nanKey_returnsFalse() { + ImmutableMap doubleNanMap = ImmutableMap.of(Double.NaN, "v"); + ImmutableMap floatNanMap = ImmutableMap.of(Float.NaN, "v"); + + assertThat(RUNTIME_EQUALITY.inMap(doubleNanMap, Double.NaN)).isFalse(); + assertThat(RUNTIME_EQUALITY.inMap(floatNanMap, Float.NaN)).isFalse(); + assertThat(RUNTIME_EQUALITY.objectEquals(doubleNanMap, ImmutableMap.of(Double.NaN, "v"))) + .isFalse(); + } + + @Test + public void objectEquals_nullMapValue_returnsFalse() { + Map nullValMap = Collections.singletonMap("k", null); + + assertThat(RUNTIME_EQUALITY.objectEquals(nullValMap, nullValMap)).isFalse(); + } + + @Test + public void objectEquals_protoMessageLite_throwsSymmetrically() { + ProtoMessageRuntimeEquality protoEquality = + ProtoMessageRuntimeEquality.create( + DynamicProto.create(DefaultMessageFactory.create(DefaultDescriptorPool.INSTANCE)), + CelOptions.current().enableProtoDifferencerEquality(true).build()); + MessageLiteOrBuilder liteOnly = + new MessageLiteOrBuilder() { + @Override + public MessageLite getDefaultInstanceForType() { + return null; + } + + @Override + public boolean isInitialized() { + return true; + } + }; + TestAllTypes fullMessage = TestAllTypes.getDefaultInstance(); + + assertThrows( + UnsupportedOperationException.class, + () -> protoEquality.objectEquals(liteOnly, fullMessage)); + assertThrows( + UnsupportedOperationException.class, + () -> protoEquality.objectEquals(fullMessage, liteOnly)); + assertThrows( + UnsupportedOperationException.class, () -> protoEquality.objectEquals(liteOnly, liteOnly)); + } + + @Test + public void inList_negativeZero_returnsTrue() { + assertThat( + RUNTIME_EQUALITY.inList( + ImmutableList.of(ImmutableList.of(0L)), ImmutableList.of(-0.0))) + .isTrue(); } @Test public void objectEquals_messageLite_throws() { - RuntimeEquality runtimeEquality = - RuntimeEquality.create(RuntimeHelpers.create(), CelOptions.DEFAULT); TestAllTypes.Builder builder = TestAllTypes.newBuilder(); TestAllTypes defaultInstance = TestAllTypes.getDefaultInstance(); UnsupportedOperationException builderThrown = assertThrows( UnsupportedOperationException.class, - () -> runtimeEquality.objectEquals(builder, defaultInstance)); + () -> RUNTIME_EQUALITY.objectEquals(builder, defaultInstance)); UnsupportedOperationException stringThrown = assertThrows( UnsupportedOperationException.class, - () -> runtimeEquality.objectEquals("not_a_message", defaultInstance)); + () -> RUNTIME_EQUALITY.objectEquals("not_a_message", defaultInstance)); assertThat(builderThrown).hasMessageThat().isEqualTo("Not implemented yet"); assertThat(stringThrown).hasMessageThat().isEqualTo("Not implemented yet"); @@ -79,8 +224,6 @@ public void objectEquals_messageLite_throws() { @Test public void objectEquals_wireMessageLite_throws() { - RuntimeEquality runtimeEquality = - RuntimeEquality.create(RuntimeHelpers.create(), CelOptions.DEFAULT); Object wireMsg1 = ProtoMessageLiteValueProvider.newInstance() .protoCelValueConverter() @@ -93,24 +236,35 @@ public void objectEquals_wireMessageLite_throws() { UnsupportedOperationException messagesThrown = assertThrows( UnsupportedOperationException.class, - () -> runtimeEquality.objectEquals(wireMsg1, wireMsg2)); + () -> RUNTIME_EQUALITY.objectEquals(wireMsg1, wireMsg2)); UnsupportedOperationException stringThrown = assertThrows( UnsupportedOperationException.class, - () -> runtimeEquality.objectEquals("not_a_message", wireMsg1)); + () -> RUNTIME_EQUALITY.objectEquals("not_a_message", wireMsg1)); assertThat(messagesThrown).hasMessageThat().isEqualTo("Message equality is not supported"); assertThat(stringThrown).hasMessageThat().isEqualTo("Message equality is not supported"); } @Test - public void objectEquals_nanWithIdenticalReference_returnsFalse() { - RuntimeEquality runtimeEquality = - RuntimeEquality.create(RuntimeHelpers.create(), CelOptions.DEFAULT); + public void objectEquals_nanWithIdenticalReference_returnsFalse( + @TestParameter IdenticalNanReferenceTestCase testCase) { + assertThat(RUNTIME_EQUALITY.objectEquals(testCase.value, testCase.value)).isFalse(); + } + + @Test + public void objectEquals_ieee754ContainerEqualityDisabled_shortCircuitsIdenticalContainers() { + RuntimeEquality optOutEquality = + RuntimeEquality.create( + RuntimeHelpers.create(), + CelOptions.current().enableIeee754ContainerEquality(false).build()); + ImmutableList nanList = ImmutableList.of(Double.NaN); + ImmutableMap nanMap = ImmutableMap.of("k", Double.NaN); Double nan = Double.NaN; - Optional optNan = Optional.of(nan); - assertThat(runtimeEquality.objectEquals(nan, nan)).isFalse(); - assertThat(runtimeEquality.objectEquals(optNan, optNan)).isFalse(); + assertThat(optOutEquality.objectEquals(nanList, nanList)).isTrue(); + assertThat(optOutEquality.objectEquals(nanMap, nanMap)).isTrue(); + assertThat(optOutEquality.objectEquals(nanList, ImmutableList.of(1.0d))).isFalse(); + assertThat(optOutEquality.objectEquals(nan, nan)).isFalse(); } } diff --git a/runtime/src/test/java/dev/cel/runtime/planner/AsyncCallRecordTest.java b/runtime/src/test/java/dev/cel/runtime/planner/AsyncCallRecordTest.java index 0dacb48ca..a7eda9b85 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/AsyncCallRecordTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/AsyncCallRecordTest.java @@ -24,11 +24,13 @@ import com.google.common.util.concurrent.SettableFuture; import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; +import com.google.testing.junit.testparameterinjector.TestParameters; import dev.cel.common.CelOptions; import dev.cel.common.values.NullValue; import dev.cel.runtime.CelAsyncFunctionOverload; import dev.cel.runtime.RuntimeEquality; import dev.cel.runtime.RuntimeHelpers; +import java.util.Optional; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -212,11 +214,32 @@ public void matches_floatNanArgs_returnsTrue() { } @Test - public void matches_nanVsNonNan_returnsFalse() { - AsyncCallRecord record = recordWithArgs(Double.NaN); + public void matches_nanContainerArgs_returnsTrue() { + AsyncCallRecord record = + recordWithArgs( + ImmutableList.of(Double.NaN), + ImmutableMap.of("k", Double.NaN), + Optional.of(Double.NaN)); + + assertThat( + matches( + record, + 10L, + "myFunc", + "myFunc_overload", + ImmutableList.of(Double.NaN), + ImmutableMap.of("k", Double.NaN), + Optional.of(Double.NaN))) + .isTrue(); + } + + @Test + @TestParameters("{recorded: NaN, candidate: 0.0}") + @TestParameters("{recorded: 0.0, candidate: NaN}") + public void matches_nanVsNonNan_returnsFalse(double recorded, double candidate) { + AsyncCallRecord record = recordWithArgs(recorded); - assertThat(matches(record, 10L, "myFunc", "myFunc_overload", 0.0d)).isFalse(); - assertThat(matches(record, 10L, "myFunc", "myFunc_overload", 1.0d)).isFalse(); + assertThat(matches(record, 10L, "myFunc", "myFunc_overload", candidate)).isFalse(); } @Test diff --git a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerAsyncTest.java b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerAsyncTest.java index 8a7fd8a95..074d4a149 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerAsyncTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerAsyncTest.java @@ -146,7 +146,7 @@ public final class ProgramPlannerAsyncTest { newFunctionDeclaration( "asyncListSize", newGlobalOverload( - "asyncListSize_list", SimpleType.INT, ListType.create(SimpleType.INT))), + "asyncListSize_list", SimpleType.INT, ListType.create(SimpleType.DYN))), newFunctionDeclaration( "asyncMapSize", newGlobalOverload( @@ -1131,6 +1131,28 @@ public void evalAsync_memoization_normalizesDoubleNanAcrossPasses() throws Excep assertThat(callCounter.get()).isEqualTo(1); } + @Test + @SuppressWarnings("Immutable") // Test only + public void evalAsync_memoization_nanListArgsAcrossPasses() throws Exception { + AtomicInteger callCounter = new AtomicInteger(); + Program program = + createProgram( + "asyncListSize([dx]) + asyncSquare(asyncSquare(2))", + ASYNC_SQUARE_INT, + CelFunctionBinding.fromAsync( + "asyncListSize_list", + List.class, + arg -> { + callCounter.incrementAndGet(); + return immediateFuture((long) arg.size()); + })); + + Object result = program.evalAsync(ImmutableMap.of("dx", Double.NaN)).get(5, SECONDS); + + assertThat(result).isEqualTo(17L); + assertThat(callCounter.get()).isEqualTo(1); + } + @Test @SuppressWarnings("Immutable") // Test only public void evalAsync_memoization_normalizesNegativeZeroAcrossPasses() throws Exception {