Skip to content

Commit 62c1f11

Browse files
l46kokcopybara-github
authored andcommitted
Null assignability fix for repeated and map fields
PiperOrigin-RevId: 882683464
1 parent a5a1ef1 commit 62c1f11

File tree

3 files changed

+102
-5
lines changed

3 files changed

+102
-5
lines changed

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

Lines changed: 46 additions & 5 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 (!isFieldAnyOrJson(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 = isFieldAnyOrJson(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,54 @@ public Optional<Object> adaptValueToFieldType(
252254
return Optional.of(mapEntries);
253255
}
254256
if (fieldDescriptor.isRepeated()) {
257+
List<?> listValue = (List<?>) fieldValue;
258+
259+
if (!isFieldAnyOrJson(fieldDescriptor)) {
260+
listValue = filterOutNullValues(listValue);
261+
}
262+
255263
return Optional.of(
256-
AdaptingTypes.adaptingList(
257-
(List<?>) fieldValue, fieldToValueConverter(fieldDescriptor).reverse()));
264+
AdaptingTypes.adaptingList(listValue, fieldToValueConverter(fieldDescriptor).reverse()));
258265
}
259266

260267
return Optional.of(
261268
fieldToValueConverter(fieldDescriptor).backwardConverter().convert(fieldValue));
262269
}
263270

271+
private static List<?> filterOutNullValues(List<?> originalList) {
272+
List<Object> filteredList = null;
273+
274+
for (int i = 0; i < originalList.size(); i++) {
275+
Object elem = originalList.get(i);
276+
277+
if (elem instanceof NullValue) {
278+
if (filteredList == null) {
279+
filteredList = new ArrayList<>(originalList.size() - 1);
280+
if (i > 0) {
281+
filteredList.addAll(originalList.subList(0, i));
282+
}
283+
}
284+
} else if (filteredList != null) {
285+
filteredList.add(elem);
286+
}
287+
}
288+
289+
// Return the original list if no nulls were found to avoid unnecessary allocations
290+
return filteredList != null ? filteredList : originalList;
291+
}
292+
293+
private static boolean isFieldAnyOrJson(FieldDescriptor fieldDescriptor) {
294+
if (!fieldDescriptor.getType().equals(FieldDescriptor.Type.MESSAGE)) {
295+
return false;
296+
}
297+
298+
String typeFullName = fieldDescriptor.getMessageType().getFullName();
299+
300+
return WellKnownProto.getByTypeName(typeFullName)
301+
.map(wkp -> wkp.equals(WellKnownProto.ANY_VALUE) || wkp.equals(WellKnownProto.JSON_VALUE))
302+
.orElse(false);
303+
}
304+
264305
@SuppressWarnings("rawtypes")
265306
private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) {
266307
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
@@ -2162,6 +2162,33 @@ public void nullAssignability() throws Exception {
21622162

21632163
source = "has(TestAllTypes{single_timestamp: null}.single_timestamp)";
21642164
runTest();
2165+
2166+
source =
2167+
"TestAllTypes{repeated_timestamp: [timestamp(1), null]}.repeated_timestamp =="
2168+
+ " [timestamp(1)]";
2169+
runTest();
2170+
2171+
source =
2172+
"TestAllTypes{map_bool_timestamp: {true: null, false: timestamp(1)}}.map_bool_timestamp =="
2173+
+ " {false: timestamp(1)}";
2174+
runTest();
2175+
2176+
source = "TestAllTypes{repeated_any: [1, null]}.repeated_any == [1, null]";
2177+
runTest();
2178+
2179+
source =
2180+
"TestAllTypes{map_bool_any: {true: null, false: 1}}.map_bool_any == {true: null, false: 1}";
2181+
runTest();
2182+
2183+
source =
2184+
"TestAllTypes{repeated_value: [google.protobuf.Value{bool_value: true},"
2185+
+ " null]}.repeated_value == [true, null]";
2186+
runTest();
2187+
2188+
source =
2189+
"TestAllTypes{map_bool_value: {true: null, false: google.protobuf.Value{bool_value:"
2190+
+ " true}}}.map_bool_value == {true: null, false: true}";
2191+
runTest();
21652192
}
21662193

21672194
@Test

0 commit comments

Comments
 (0)