Skip to content

Commit a1b9a78

Browse files
committed
fix: handle missing @reference entities gracefully (#607)
- Add null checks in ReferenceDeserializer to prevent NPE when referenced entities are missing - Add null check in JSONOperationsImpl.get() to handle null results from Redis - Log warnings when referenced entities cannot be found - Filter out missing references from collections instead of throwing NPE - Add tests to verify proper handling of missing single and collection references
1 parent 1577b26 commit a1b9a78

File tree

3 files changed

+79
-4
lines changed

3 files changed

+79
-4
lines changed

redis-om-spring/src/main/java/com/redis/om/spring/ops/json/JSONOperationsImpl.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ public Long del(K key, Path2 path) {
6161
@Override
6262
public String get(K key) {
6363
var result = client.clientForJSON().jsonGet(key.toString(), Path2.ROOT_PATH);
64-
if (result instanceof JSONArray jsonArray) {
64+
if (result == null) {
65+
return null;
66+
} else if (result instanceof JSONArray jsonArray) {
6567
return !jsonArray.isEmpty() ? jsonArray.get(0).toString() : null;
6668
} else if (result instanceof LinkedTreeMap<?, ?> linkedTreeMap) {
6769
return getGson().toJson(linkedTreeMap);

redis-om-spring/src/main/java/com/redis/om/spring/serialization/gson/ReferenceDeserializer.java

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,18 @@ public Object deserialize(JsonElement json, Type typeOfT, JsonDeserializationCon
106106
}
107107
if (referenceJSON == null) {
108108
referenceJSON = ops.get(referenceKey);
109-
if (referenceCache != null && shouldCache(type)) {
109+
if (referenceJSON != null && referenceCache != null && shouldCache(type)) {
110110
referenceCache.put(referenceKey, referenceJSON);
111111
}
112112
}
113+
114+
// Handle missing reference gracefully
115+
if (referenceJSON == null) {
116+
logger.warn(String.format("Referenced entity with key '%s' not found for type %s", referenceKey, type
117+
.getName()));
118+
return null;
119+
}
120+
113121
jsonObject = gson.fromJson(referenceJSON, JsonObject.class);
114122
reference = deserializeEntity(jsonObject, context);
115123
} else if (json.isJsonObject()) {
@@ -139,8 +147,19 @@ public Object deserialize(JsonElement json, Type typeOfT, JsonDeserializationCon
139147
} else {
140148
values = ops.mget(keys);
141149
}
142-
((Collection) reference).addAll(values.stream().map(raw -> gson.fromJson(raw, JsonObject.class)).map(
143-
jo -> deserializeEntity(jo, context)).toList());
150+
// Filter out null values (missing references) and log warnings
151+
List<Object> deserializedReferences = new ArrayList<>();
152+
for (int i = 0; i < values.size(); i++) {
153+
String raw = values.get(i);
154+
if (raw != null) {
155+
JsonObject jo = gson.fromJson(raw, JsonObject.class);
156+
deserializedReferences.add(deserializeEntity(jo, context));
157+
} else {
158+
logger.warn(String.format("Referenced entity with key '%s' not found for type %s", keys[i], type
159+
.getName()));
160+
}
161+
}
162+
((Collection) reference).addAll(deserializedReferences);
144163
}
145164
}
146165

tests/src/test/java/com/redis/om/spring/annotations/document/ReferenceTest.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.redis.om.spring.annotations.document;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
45

56
import java.util.*;
67
import java.util.stream.Collectors;
@@ -286,4 +287,57 @@ void testExtremeReferencesWithFindAll() {
286287
assertThat(tmr.getRef10()).isNotNull();
287288
}
288289
}
290+
291+
@Test
292+
void testNPEWithMissingReference() {
293+
// First create a city with a valid state reference
294+
var usa = countryRepository.save(Country.of("USA"));
295+
var fl = stateRepository.save(State.of("FL", "Florida", usa));
296+
var miami = cityRepository.save(City.of("Miami", fl));
297+
298+
// Verify the city was saved correctly
299+
var savedCity = cityRepository.findById("Miami");
300+
assertThat(savedCity).isPresent();
301+
assertThat(savedCity.get().getState().getId()).isEqualTo("FL");
302+
303+
// Now delete the state that the city references
304+
stateRepository.deleteById("FL");
305+
306+
// Verify the state is gone
307+
var deletedState = stateRepository.findById("FL");
308+
assertThat(deletedState).isEmpty();
309+
310+
// After the fix, this should no longer throw NPE
311+
// Instead, it should return the city with null state reference
312+
var cityWithMissingRef = cityRepository.findById("Miami");
313+
assertThat(cityWithMissingRef).isPresent();
314+
assertThat(cityWithMissingRef.get().getId()).isEqualTo("Miami");
315+
assertThat(cityWithMissingRef.get().getState()).isNull();
316+
}
317+
318+
@Test
319+
void testCollectionWithMissingReferences() {
320+
// Create states and a collection referencing them
321+
var usa = countryRepository.save(Country.of("USA"));
322+
var ny = stateRepository.save(State.of("NY", "New York", usa));
323+
var nj = stateRepository.save(State.of("NJ", "New Jersey", usa));
324+
var ct = stateRepository.save(State.of("CT", "Connecticut", usa));
325+
326+
var statesCollection = statesRepository.save(States.of("Tri-State Area", Set.of(ny, nj, ct)));
327+
328+
// Verify the collection was saved correctly
329+
var saved = statesRepository.findById("Tri-State Area");
330+
assertThat(saved).isPresent();
331+
assertThat(saved.get().getStates()).hasSize(3);
332+
333+
// Delete one of the referenced states
334+
stateRepository.deleteById("NJ");
335+
336+
// Retrieving the collection should not throw NPE
337+
// It should only contain the existing states
338+
var collectionWithMissingRef = statesRepository.findById("Tri-State Area");
339+
assertThat(collectionWithMissingRef).isPresent();
340+
assertThat(collectionWithMissingRef.get().getStates()).hasSize(2);
341+
assertThat(collectionWithMissingRef.get().getStates()).extracting("id").containsExactlyInAnyOrder("NY", "CT");
342+
}
289343
}

0 commit comments

Comments
 (0)