Skip to content

Commit 163565b

Browse files
l46kokcopybara-github
authored andcommitted
Null assignability fix for repeated and map fields
PiperOrigin-RevId: 876590274
1 parent 3fd7b96 commit 163565b

3 files changed

Lines changed: 83 additions & 4 deletions

File tree

common/src/main/java/dev/cel/common/internal/ProtoAdapter.java

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,7 @@ public Optional<Object> adaptValueToFieldType(
222222
throw new IllegalArgumentException("Unsupported field type");
223223
}
224224

225-
String typeFullName = fieldDescriptor.getMessageType().getFullName();
226-
if (!WellKnownProto.ANY_VALUE.typeName().equals(typeFullName)
227-
&& !WellKnownProto.JSON_VALUE.typeName().equals(typeFullName)) {
225+
if (!isAnyOrJsonValue(fieldDescriptor)) {
228226
return Optional.empty();
229227
}
230228
}
@@ -242,7 +240,11 @@ public Optional<Object> adaptValueToFieldType(
242240
getDefaultValueForMaybeMessage(keyDescriptor),
243241
valueDescriptor.getLiteType(),
244242
getDefaultValueForMaybeMessage(valueDescriptor));
243+
boolean isValueAnyOrJson = isAnyOrJsonValue(valueDescriptor);
245244
for (Map.Entry entry : ((Map<?, ?>) fieldValue).entrySet()) {
245+
if (!isValueAnyOrJson && entry.getValue() instanceof NullValue) {
246+
continue;
247+
}
246248
mapEntries.add(
247249
protoMapEntry.toBuilder()
248250
.setKey(keyConverter.backwardConverter().convert(entry.getKey()))
@@ -252,15 +254,36 @@ public Optional<Object> adaptValueToFieldType(
252254
return Optional.of(mapEntries);
253255
}
254256
if (fieldDescriptor.isRepeated()) {
257+
List<?> listValue = (List<?>) fieldValue;
258+
if (listValue.contains(NullValue.NULL_VALUE)) {
259+
if (!isAnyOrJsonValue(fieldDescriptor)) {
260+
List<Object> filteredList = new ArrayList<>(listValue.size());
261+
for (Object elem : listValue) {
262+
if (!(elem instanceof NullValue)) {
263+
filteredList.add(elem);
264+
}
265+
}
266+
listValue = filteredList;
267+
}
268+
}
255269
return Optional.of(
256270
AdaptingTypes.adaptingList(
257-
(List<?>) fieldValue, fieldToValueConverter(fieldDescriptor).reverse()));
271+
listValue, fieldToValueConverter(fieldDescriptor).reverse()));
258272
}
259273

260274
return Optional.of(
261275
fieldToValueConverter(fieldDescriptor).backwardConverter().convert(fieldValue));
262276
}
263277

278+
private boolean isAnyOrJsonValue(FieldDescriptor fieldDescriptor) {
279+
String typeFullName =
280+
fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE
281+
? fieldDescriptor.getMessageType().getFullName()
282+
: "";
283+
return WellKnownProto.ANY_VALUE.typeName().equals(typeFullName)
284+
|| WellKnownProto.JSON_VALUE.typeName().equals(typeFullName);
285+
}
286+
264287
@SuppressWarnings("rawtypes")
265288
private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) {
266289
switch (fieldDescriptor.getType()) {

runtime/src/test/resources/nullAssignability.baseline

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,32 @@ Source: has(TestAllTypes{single_timestamp: null}.single_timestamp)
3333
bindings: {}
3434
result: false
3535

36+
Source: TestAllTypes{repeated_timestamp: [timestamp(1), null]}.repeated_timestamp == [timestamp(1)]
37+
=====>
38+
bindings: {}
39+
result: true
40+
41+
Source: TestAllTypes{map_bool_timestamp: {true: null, false: timestamp(1)}}.map_bool_timestamp == {false: timestamp(1)}
42+
=====>
43+
bindings: {}
44+
result: true
45+
46+
Source: TestAllTypes{repeated_any: [1, null]}.repeated_any == [1, null]
47+
=====>
48+
bindings: {}
49+
result: true
50+
51+
Source: TestAllTypes{map_bool_any: {true: null, false: 1}}.map_bool_any == {true: null, false: 1}
52+
=====>
53+
bindings: {}
54+
result: true
55+
56+
Source: TestAllTypes{repeated_value: [google.protobuf.Value{bool_value: true}, null]}.repeated_value == [true, null]
57+
=====>
58+
bindings: {}
59+
result: true
60+
61+
Source: TestAllTypes{map_bool_value: {true: null, false: google.protobuf.Value{bool_value: true}}}.map_bool_value == {true: null, false: true}
62+
=====>
63+
bindings: {}
64+
result: true

testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2145,6 +2145,33 @@ public void nullAssignability() throws Exception {
21452145

21462146
source = "has(TestAllTypes{single_timestamp: null}.single_timestamp)";
21472147
runTest();
2148+
2149+
source =
2150+
"TestAllTypes{repeated_timestamp: [timestamp(1), null]}.repeated_timestamp =="
2151+
+ " [timestamp(1)]";
2152+
runTest();
2153+
2154+
source =
2155+
"TestAllTypes{map_bool_timestamp: {true: null, false: timestamp(1)}}.map_bool_timestamp =="
2156+
+ " {false: timestamp(1)}";
2157+
runTest();
2158+
2159+
source = "TestAllTypes{repeated_any: [1, null]}.repeated_any == [1, null]";
2160+
runTest();
2161+
2162+
source =
2163+
"TestAllTypes{map_bool_any: {true: null, false: 1}}.map_bool_any == {true: null, false: 1}";
2164+
runTest();
2165+
2166+
source =
2167+
"TestAllTypes{repeated_value: [google.protobuf.Value{bool_value: true},"
2168+
+ " null]}.repeated_value == [true, null]";
2169+
runTest();
2170+
2171+
source =
2172+
"TestAllTypes{map_bool_value: {true: null, false: google.protobuf.Value{bool_value:"
2173+
+ " true}}}.map_bool_value == {true: null, false: true}";
2174+
runTest();
21482175
}
21492176

21502177
@Test

0 commit comments

Comments
 (0)