Skip to content

Commit ffe0dd3

Browse files
authored
Fix CEL bindings for maps (#278)
This fixes the variable binding to `this` for maps. Previously, the binding was using a map of the custom value abstraction `ObjectValue` that we use in protovalidate-java. This `Value` class has no relation at all to CEL-Java's `Val` object. So, using it as a variable binding does not work because CEL-Java doesn't know how to interpret it. This adds an additional `mapValueAsObject` function to return a map of `Object`s for the binding. Note this not ideal, but should be sufficient for now. Further context in the comments.
1 parent 04f235f commit ffe0dd3

File tree

2 files changed

+35
-17
lines changed

2 files changed

+35
-17
lines changed

conformance/expected-failures.yaml

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +0,0 @@
1-
custom_rules:
2-
- field_expression/map/enum/invalid
3-
- field_expression/map/enum/valid
4-
- field_expression/map/message/invalid
5-
- field_expression/map/message/valid
6-
- field_expression/map/bool/valid
7-
- field_expression/map/bool/invalid
8-
- field_expression/map/string/valid
9-
- field_expression/map/string/invalid
10-
- field_expression/map/int32/valid
11-
- field_expression/map/int32/invalid
12-
- field_expression/map/uint32/valid
13-
- field_expression/map/uint32/invalid
14-
- field_expression/map/int64/valid
15-
- field_expression/map/int64/invalid
16-
- field_expression/map/uint64/valid
17-
- field_expression/map/uint64/invalid

src/main/java/build/buf/protovalidate/ObjectValue.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ public Message messageValue() {
6464
@Override
6565
public <T> T value(Class<T> clazz) {
6666
Descriptors.FieldDescriptor.Type type = fieldDescriptor.getType();
67+
if (fieldDescriptor.isMapField()) {
68+
return clazz.cast(mapValueAsObject());
69+
}
6770
if (!fieldDescriptor.isRepeated()
6871
&& (type == Descriptors.FieldDescriptor.Type.UINT32
6972
|| type == Descriptors.FieldDescriptor.Type.UINT64
@@ -93,6 +96,38 @@ public List<Value> repeatedValue() {
9396
return out;
9497
}
9598

99+
// TODO - This should be refactored at some point.
100+
//
101+
// This is essentially the same functionality as `mapValue` except that it
102+
// returns a Map of Objects rather than a Map of protovalidate-java Values.
103+
// It is used for binding to a CEL variable (i.e. `this`).
104+
// Trying to bind a Map of Values to a CEL variable does not work because
105+
// CEL-Java doesn't know how to interpret that proprietary Value object.
106+
//
107+
// Ideally, we should be using CEL-Java's org.projectnessie.cel.common.types.ref.Val
108+
// type instead of our own custom Value abstraction. However, since we are evaluating
109+
// Java CEL implementations, we should probably wait until that decision is made before
110+
// making such a large refactor. This should suffice as a stopgap until then.
111+
private Map<Object, Object> mapValueAsObject() {
112+
List<AbstractMessage> input =
113+
value instanceof List
114+
? (List<AbstractMessage>) value
115+
: Collections.singletonList((AbstractMessage) value);
116+
117+
Descriptors.FieldDescriptor keyDesc = fieldDescriptor.getMessageType().findFieldByNumber(1);
118+
Descriptors.FieldDescriptor valDesc = fieldDescriptor.getMessageType().findFieldByNumber(2);
119+
Map<Object, Object> out = new HashMap<>(input.size());
120+
121+
for (AbstractMessage entry : input) {
122+
Object keyValue = entry.getField(keyDesc);
123+
Object valValue = entry.getField(valDesc);
124+
125+
out.put(keyValue, valValue);
126+
}
127+
128+
return out;
129+
}
130+
96131
@Override
97132
public Map<Value, Value> mapValue() {
98133
List<AbstractMessage> input =

0 commit comments

Comments
 (0)