From 65d9401787b5147e8ad51486891d8455e65ca81c Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 30 Apr 2025 13:02:33 -0400 Subject: [PATCH 1/3] bindings --- .../build/buf/protovalidate/ObjectValue.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/main/java/build/buf/protovalidate/ObjectValue.java b/src/main/java/build/buf/protovalidate/ObjectValue.java index dcd2fa9b..748b5a99 100644 --- a/src/main/java/build/buf/protovalidate/ObjectValue.java +++ b/src/main/java/build/buf/protovalidate/ObjectValue.java @@ -64,6 +64,9 @@ public Message messageValue() { @Override public T value(Class clazz) { Descriptors.FieldDescriptor.Type type = fieldDescriptor.getType(); + if (fieldDescriptor.isMapField()) { + return clazz.cast(getMapBinding()); + } if (!fieldDescriptor.isRepeated() && (type == Descriptors.FieldDescriptor.Type.UINT32 || type == Descriptors.FieldDescriptor.Type.UINT64 @@ -93,6 +96,30 @@ public List repeatedValue() { return out; } + // TODO - This is essentially the same functionality as `mapValue` except that it + // returns a Map of Objects rather than a Map of protovalidate-java Value. + // Trying to bind to a variable (i.e. `this`) using a Map of Values does not work + // because CEL-Java doesn't know how to interpret a + private Map getMapBinding() { + List input = + value instanceof List + ? (List) value + : Collections.singletonList((AbstractMessage) value); + + Descriptors.FieldDescriptor keyDesc = fieldDescriptor.getMessageType().findFieldByNumber(1); + Descriptors.FieldDescriptor valDesc = fieldDescriptor.getMessageType().findFieldByNumber(2); + Map out = new HashMap<>(input.size()); + + for (AbstractMessage entry : input) { + Object keyValue = entry.getField(keyDesc); + Object valValue = entry.getField(valDesc); + + out.put(keyValue, valValue); + } + + return out; + } + @Override public Map mapValue() { List input = From fc560d159ec3fe964e86414c55b77536f09d6a85 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 30 Apr 2025 17:39:42 -0400 Subject: [PATCH 2/3] Tests --- src/main/java/build/buf/protovalidate/ObjectValue.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/ObjectValue.java b/src/main/java/build/buf/protovalidate/ObjectValue.java index 748b5a99..d1158be2 100644 --- a/src/main/java/build/buf/protovalidate/ObjectValue.java +++ b/src/main/java/build/buf/protovalidate/ObjectValue.java @@ -98,9 +98,9 @@ public List repeatedValue() { // TODO - This is essentially the same functionality as `mapValue` except that it // returns a Map of Objects rather than a Map of protovalidate-java Value. - // Trying to bind to a variable (i.e. `this`) using a Map of Values does not work - // because CEL-Java doesn't know how to interpret a - private Map getMapBinding() { + // Trying to bind a Map of Values to a CEL variable (i.e. `this`) does not work + // because CEL-Java doesn't know how to interpret that proprietary Value object. + private Map mapValueAsObject() { List input = value instanceof List ? (List) value From 63bea0f54a56348bcf60ce706dd4130285c4dfe8 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Thu, 1 May 2025 10:18:26 -0400 Subject: [PATCH 3/3] Docs --- conformance/expected-failures.yaml | 17 ----------------- .../build/buf/protovalidate/ObjectValue.java | 18 +++++++++++++----- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/conformance/expected-failures.yaml b/conformance/expected-failures.yaml index 17daf1bf..e69de29b 100644 --- a/conformance/expected-failures.yaml +++ b/conformance/expected-failures.yaml @@ -1,17 +0,0 @@ -custom_rules: - - field_expression/map/enum/invalid - - field_expression/map/enum/valid - - field_expression/map/message/invalid - - field_expression/map/message/valid - - field_expression/map/bool/valid - - field_expression/map/bool/invalid - - field_expression/map/string/valid - - field_expression/map/string/invalid - - field_expression/map/int32/valid - - field_expression/map/int32/invalid - - field_expression/map/uint32/valid - - field_expression/map/uint32/invalid - - field_expression/map/int64/valid - - field_expression/map/int64/invalid - - field_expression/map/uint64/valid - - field_expression/map/uint64/invalid diff --git a/src/main/java/build/buf/protovalidate/ObjectValue.java b/src/main/java/build/buf/protovalidate/ObjectValue.java index d1158be2..af37251f 100644 --- a/src/main/java/build/buf/protovalidate/ObjectValue.java +++ b/src/main/java/build/buf/protovalidate/ObjectValue.java @@ -65,7 +65,7 @@ public Message messageValue() { public T value(Class clazz) { Descriptors.FieldDescriptor.Type type = fieldDescriptor.getType(); if (fieldDescriptor.isMapField()) { - return clazz.cast(getMapBinding()); + return clazz.cast(mapValueAsObject()); } if (!fieldDescriptor.isRepeated() && (type == Descriptors.FieldDescriptor.Type.UINT32 @@ -96,10 +96,18 @@ public List repeatedValue() { return out; } - // TODO - This is essentially the same functionality as `mapValue` except that it - // returns a Map of Objects rather than a Map of protovalidate-java Value. - // Trying to bind a Map of Values to a CEL variable (i.e. `this`) does not work - // because CEL-Java doesn't know how to interpret that proprietary Value object. + // TODO - This should be refactored at some point. + // + // This is essentially the same functionality as `mapValue` except that it + // returns a Map of Objects rather than a Map of protovalidate-java Values. + // It is used for binding to a CEL variable (i.e. `this`). + // Trying to bind a Map of Values to a CEL variable does not work because + // CEL-Java doesn't know how to interpret that proprietary Value object. + // + // Ideally, we should be using CEL-Java's org.projectnessie.cel.common.types.ref.Val + // type instead of our own custom Value abstraction. However, since we are evaluating + // Java CEL implementations, we should probably wait until that decision is made before + // making such a large refactor. This should suffice as a stopgap until then. private Map mapValueAsObject() { List input = value instanceof List