Skip to content

Commit e06705c

Browse files
YoofiTT96YoofiTT96
authored
fix(calm-hub): guard against null versions document in resource stores (finos#2561)
* fix(calm-hub): guard against null versions document in resource stores When a resource document exists without a versions sub-document (e.g. hand-edited store, partial migration), all get* and write* methods in the Mongo and Nitrite architecture/pattern/timeline stores would NPE. Add null checks on the versions document in every affected method, throwing the appropriate *NotFoundException or *VersionNotFoundException rather than propagating a NullPointerException as HTTP 500. Also add instanceof String guards in the Nitrite stores before casting versionObj, covering the case where the stored value is not a String. All store implementations now behave consistently: a null versions document throws *NotFoundException from getVersions and *VersionNotFoundException from getForVersion across both Mongo and Nitrite backends. Inline null checks were preferred over a helper method because each method throws a different checked exception type, and Java checked exceptions do not compose cleanly with functional interfaces — a helper would require an awkward multi-throws signature or an unchecked wrapper that hides intent. The three-line pattern is idiomatic Java here. Fixes finos#2498. * test(calm-hub): cover null-versions guards in Nitrite pattern and timeline write paths Add three tests mirroring testUpdateArchitectureForVersion_whenVersionsDocumentIsNull: - testCreatePatternForVersion_whenVersionsDocumentIsNull_throwsPatternNotFoundException - testUpdatePatternForVersion_whenVersionsDocumentIsNull_throwsPatternNotFoundException - testUpdateTimelineForVersion_whenVersionsDocumentIsNull_throwsTimelineNotFoundException Note: NitriteTimelineStore.getTimelineVersions previously returned an empty list (200) when the versions sub-document was absent; it now throws TimelineNotFoundException (404) for consistency with all other store implementations. This is an intentional behavior change.
1 parent c49d5cd commit e06705c

12 files changed

Lines changed: 411 additions & 10 deletions

‎calm-hub/src/main/java/org/finos/calm/store/mongo/MongoArchitectureStore.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ public List<String> getArchitectureVersions(Architecture architecture) throws Na
136136
if (architecture.getId() == architectureDoc.getInteger("architectureId")) {
137137
// Extract the versions map from the matching pattern
138138
Document versions = (Document) architectureDoc.get("versions");
139+
if (versions == null) {
140+
throw new ArchitectureNotFoundException();
141+
}
139142
Set<String> versionKeys = versions.keySet();
140143

141144
//Convert from Mongo representation
@@ -176,6 +179,9 @@ public String getArchitectureForVersion(Architecture architecture) throws Namesp
176179
if (architecture.getId() == architectureDoc.getInteger("architectureId")) {
177180
// Retrieve the versions map from the matching pattern
178181
Document versions = (Document) architectureDoc.get("versions");
182+
if (versions == null) {
183+
throw new ArchitectureVersionNotFoundException();
184+
}
179185

180186
// Return the pattern JSON blob for the specified version
181187
Document versionDoc = (Document) versions.get(architecture.getMongoVersion());

‎calm-hub/src/main/java/org/finos/calm/store/mongo/MongoPatternStore.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ public List<String> getPatternVersions(Pattern pattern) throws NamespaceNotFound
125125
if (pattern.getId() == patternDoc.getInteger("patternId")) {
126126
// Extract the versions map from the matching pattern
127127
Document versions = (Document) patternDoc.get("versions");
128+
if (versions == null) {
129+
throw new PatternNotFoundException();
130+
}
128131
Set<String> versionKeys = versions.keySet();
129132

130133
//Convert from Mongo representation
@@ -165,6 +168,9 @@ public String getPatternForVersion(Pattern pattern) throws NamespaceNotFoundExce
165168
if (pattern.getId() == patternDoc.getInteger("patternId")) {
166169
// Retrieve the versions map from the matching pattern
167170
Document versions = (Document) patternDoc.get("versions");
171+
if (versions == null) {
172+
throw new PatternVersionNotFoundException();
173+
}
168174

169175
// Return the pattern JSON blob for the specified version
170176
Document versionDoc = (Document) versions.get(pattern.getMongoVersion());

‎calm-hub/src/main/java/org/finos/calm/store/mongo/MongoTimelineStore.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@ public List<String> getTimelineVersions(Timeline timeline) throws NamespaceNotFo
117117
if (timeline.getId() == timelineDoc.getInteger("timelineId")) {
118118
// Extract the versions map from the matching timeline
119119
Document versions = (Document) timelineDoc.get("versions");
120+
if (versions == null) {
121+
throw new TimelineNotFoundException();
122+
}
120123
Set<String> versionKeys = versions.keySet();
121124

122125
// Convert from Mongo representation
@@ -157,6 +160,9 @@ public String getTimelineForVersion(Timeline timeline) throws NamespaceNotFoundE
157160
if (timeline.getId() == timelineDoc.getInteger("timelineId")) {
158161
// Retrieve the versions map from the matching timeline
159162
Document versions = (Document) timelineDoc.get("versions");
163+
if (versions == null) {
164+
throw new TimelineVersionNotFoundException();
165+
}
160166

161167
// Return the timeline JSON blob for the specified version
162168
Document versionDoc = (Document) versions.get(timeline.getMongoVersion());

‎calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteArchitectureStore.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ public List<String> getArchitectureVersions(Architecture architecture) throws Na
162162
if (architecture.getId() == architectureDoc.get(ARCHITECTURE_ID_FIELD, Integer.class)) {
163163
// Extract the versions map from the matching architecture
164164
Document versions = architectureDoc.get(VERSIONS_FIELD, Document.class);
165+
if (versions == null) {
166+
throw new ArchitectureNotFoundException();
167+
}
165168
Set<String> versionKeys = versions.getFields();
166169

167170
// Convert from Nitrite representation
@@ -205,20 +208,21 @@ public String getArchitectureForVersion(Architecture architecture) throws Namesp
205208
if (architecture.getId() == architectureDoc.get(ARCHITECTURE_ID_FIELD, Integer.class)) {
206209
// Retrieve the versions map from the matching architecture
207210
Document versions = architectureDoc.get(VERSIONS_FIELD, Document.class);
211+
if (versions == null) {
212+
throw new ArchitectureVersionNotFoundException();
213+
}
208214

209215
// Return the architecture JSON blob for the specified version
210216
String mongoVersion = architecture.getMongoVersion();
211217
Object versionObj = versions.get(mongoVersion);
212218
LOG.info("VersionDoc: [{}], Mongo Version: [{}]", versions, mongoVersion);
213219

214-
if (versionObj == null) {
220+
if (!(versionObj instanceof String)) {
215221
LOG.warn("Version '{}' not found for architecture {} in namespace '{}'",
216222
architecture.getDotVersion(), architecture.getId(), architecture.getNamespace());
217223
throw new ArchitectureVersionNotFoundException();
218224
}
219225

220-
// In NitriteDB, we're storing the JSON as a string directly
221-
// No need to convert to JSON string
222226
return (String) versionObj;
223227
}
224228
}
@@ -285,6 +289,9 @@ private void writeArchitectureToNitrite(Architecture architecture) throws Archit
285289
if (architectureDoc.get(ARCHITECTURE_ID_FIELD, Integer.class) == architecture.getId()) {
286290
// Found the architecture, update its version
287291
Document versions = architectureDoc.get(VERSIONS_FIELD, Document.class);
292+
if (versions == null) {
293+
throw new ArchitectureNotFoundException();
294+
}
288295
versions.put(architecture.getMongoVersion(), architecture.getArchitectureJson());
289296
architectureDoc.put(NAME_FIELD, architecture.getName());
290297
architectureDoc.put(DESCRIPTION_FIELD, architecture.getDescription());

‎calm-hub/src/main/java/org/finos/calm/store/nitrite/NitritePatternStore.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ public List<String> getPatternVersions(Pattern pattern) throws NamespaceNotFound
181181
}
182182

183183
Document versions = patternDoc.get(VERSIONS_FIELD, Document.class);
184+
if (versions == null) {
185+
throw new PatternNotFoundException();
186+
}
184187
Set<String> fieldNames = versions.getFields();
185188
List<String> versionList = new ArrayList<>();
186189
for (String fieldName : fieldNames) {
@@ -206,6 +209,9 @@ public String getPatternForVersion(Pattern pattern) throws NamespaceNotFoundExce
206209
}
207210

208211
Document versions = patternDoc.get(VERSIONS_FIELD, Document.class);
212+
if (versions == null) {
213+
throw new PatternVersionNotFoundException();
214+
}
209215
String patternJson = versions.get(pattern.getMongoVersion(), String.class);
210216

211217
if (patternJson == null) {
@@ -243,6 +249,9 @@ public Pattern createPatternForVersion(Pattern pattern) throws NamespaceNotFound
243249
}
244250

245251
Document versions = patternDoc.get(VERSIONS_FIELD, Document.class);
252+
if (versions == null) {
253+
throw new PatternNotFoundException();
254+
}
246255
if (versions.containsKey(pattern.getMongoVersion())) {
247256
LOG.warn("Version '{}' already exists for pattern {} in namespace '{}'",
248257
pattern.getMongoVersion(), pattern.getId(), pattern.getNamespace());
@@ -307,6 +316,9 @@ public Pattern updatePatternForVersion(Pattern pattern) throws NamespaceNotFound
307316
}
308317

309318
Document versions = patternDoc.get(VERSIONS_FIELD, Document.class);
319+
if (versions == null) {
320+
throw new PatternNotFoundException();
321+
}
310322
versions.put(pattern.getMongoVersion(), pattern.getPatternJson());
311323
patternDoc.put(VERSIONS_FIELD, versions);
312324

‎calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteTimelineStore.java‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,15 @@ public List<String> getTimelineVersions(Timeline timeline) throws NamespaceNotFo
162162
if (timeline.getId() == timelineDoc.get(TIMELINE_ID_FIELD, Integer.class)) {
163163
// Extract the versions map from the matching timeline
164164
Document versions = timelineDoc.get(VERSIONS_FIELD, Document.class);
165+
if (versions == null) {
166+
throw new TimelineNotFoundException();
167+
}
165168

166169
// Convert from Nitrite representation
167170
List<String> resourceVersions = new ArrayList<>();
168-
if (versions != null) {
169-
Set<String> versionKeys = versions.getFields();
170-
for (String versionKey : versionKeys) {
171-
resourceVersions.add(versionKey.replace('-', '.'));
172-
}
171+
Set<String> versionKeys = versions.getFields();
172+
for (String versionKey : versionKeys) {
173+
resourceVersions.add(versionKey.replace('-', '.'));
173174
}
174175
return resourceVersions;
175176
}
@@ -204,19 +205,21 @@ public String getTimelineForVersion(Timeline timeline) throws NamespaceNotFoundE
204205
if (timeline.getId() == timelineDoc.get(TIMELINE_ID_FIELD, Integer.class)) {
205206
// Retrieve the versions map from the matching timeline
206207
Document versions = timelineDoc.get(VERSIONS_FIELD, Document.class);
208+
if (versions == null) {
209+
throw new TimelineVersionNotFoundException();
210+
}
207211

208212
// Return the timeline JSON blob for the specified version
209213
String mongoVersion = timeline.getMongoVersion();
210214
Object versionObj = versions.get(mongoVersion);
211215
LOG.info("VersionDoc: [{}], Mongo Version: [{}]", versions, mongoVersion);
212216

213-
if (versionObj == null) {
217+
if (!(versionObj instanceof String)) {
214218
LOG.warn("Version '{}' not found for timeline {} in namespace '{}'",
215219
timeline.getDotVersion(), timeline.getId(), timeline.getNamespace());
216220
throw new TimelineVersionNotFoundException();
217221
}
218222

219-
// In NitriteDB, we're storing the JSON as a string directly
220223
return (String) versionObj;
221224
}
222225
}
@@ -280,6 +283,9 @@ private void writeTimelineToNitrite(Timeline timeline) throws TimelineNotFoundEx
280283
if (timelineDoc.get(TIMELINE_ID_FIELD, Integer.class) == timeline.getId()) {
281284
// Found the timeline, update its version
282285
Document versions = timelineDoc.get(VERSIONS_FIELD, Document.class);
286+
if (versions == null) {
287+
throw new TimelineNotFoundException();
288+
}
283289
versions.put(timeline.getMongoVersion(), timeline.getTimelineJson());
284290
timelineDoc.put(VERSIONS_FIELD, versions);
285291
// Defensive: the REST layer enforces @NotBlank on name/description via CreateTimelineRequest,

‎calm-hub/src/test/java/org/finos/calm/store/mongo/TestMongoArchitectureStoreShould.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,38 @@ void get_architecture_version_for_invalid_pattern_throws_exception() {
273273
verify(findIterable).projection(Projections.fields(Projections.include("architectures")));
274274
}
275275

276+
@Test
277+
void throw_architecture_not_found_when_versions_document_is_missing_on_get_versions() {
278+
Document architectureWithNoVersions = new Document("namespace", NAMESPACE)
279+
.append("architectures", List.of(new Document("architectureId", 42)));
280+
FindIterable<Document> findIterable = Mockito.mock(DocumentFindIterable.class);
281+
when(namespaceStore.namespaceExists(anyString())).thenReturn(true);
282+
when(architectureCollection.find(any(Bson.class))).thenReturn(findIterable);
283+
when(findIterable.projection(any(Bson.class))).thenReturn(findIterable);
284+
when(findIterable.first()).thenReturn(architectureWithNoVersions);
285+
286+
Architecture architecture = new Architecture.ArchitectureBuilder().setNamespace(NAMESPACE).setId(42).build();
287+
288+
assertThrows(ArchitectureNotFoundException.class,
289+
() -> mongoArchitectureStore.getArchitectureVersions(architecture));
290+
}
291+
292+
@Test
293+
void throw_architecture_version_not_found_when_versions_document_is_missing_on_get_for_version() {
294+
Document architectureWithNoVersions = new Document("namespace", NAMESPACE)
295+
.append("architectures", List.of(new Document("architectureId", 42)));
296+
FindIterable<Document> findIterable = Mockito.mock(DocumentFindIterable.class);
297+
when(namespaceStore.namespaceExists(anyString())).thenReturn(true);
298+
when(architectureCollection.find(any(Bson.class))).thenReturn(findIterable);
299+
when(findIterable.projection(any(Bson.class))).thenReturn(findIterable);
300+
when(findIterable.first()).thenReturn(architectureWithNoVersions);
301+
302+
Architecture architecture = new Architecture.ArchitectureBuilder().setNamespace(NAMESPACE).setId(42).setVersion("1.0.0").build();
303+
304+
assertThrows(ArchitectureVersionNotFoundException.class,
305+
() -> mongoArchitectureStore.getArchitectureForVersion(architecture));
306+
}
307+
276308
@Test
277309
void get_architecture_versions_for_valid_architecture_returns_list_of_versions() throws ArchitectureNotFoundException, NamespaceNotFoundException {
278310
mockSetupArchitectureDocumentWithVersions();

‎calm-hub/src/test/java/org/finos/calm/store/mongo/TestMongoPatternStoreShould.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,38 @@ void get_pattern_version_for_invalid_pattern_throws_exception() {
253253
verify(findIterable).projection(Projections.fields(Projections.include("patterns")));
254254
}
255255

256+
@Test
257+
void throw_pattern_not_found_when_versions_document_is_missing_on_get_versions() {
258+
Document patternWithNoVersions = new Document("namespace", "finos")
259+
.append("patterns", List.of(new Document("patternId", 42)));
260+
DocumentFindIterable findIterable = Mockito.mock(DocumentFindIterable.class);
261+
when(namespaceStore.namespaceExists(anyString())).thenReturn(true);
262+
when(patternCollection.find(any(Bson.class))).thenReturn(findIterable);
263+
when(findIterable.projection(any(Bson.class))).thenReturn(findIterable);
264+
when(findIterable.first()).thenReturn(patternWithNoVersions);
265+
266+
Pattern pattern = new Pattern.PatternBuilder().setNamespace("finos").setId(42).build();
267+
268+
assertThrows(PatternNotFoundException.class,
269+
() -> mongoPatternStore.getPatternVersions(pattern));
270+
}
271+
272+
@Test
273+
void throw_pattern_version_not_found_when_versions_document_is_missing_on_get_for_version() {
274+
Document patternWithNoVersions = new Document("namespace", "finos")
275+
.append("patterns", List.of(new Document("patternId", 42)));
276+
DocumentFindIterable findIterable = Mockito.mock(DocumentFindIterable.class);
277+
when(namespaceStore.namespaceExists(anyString())).thenReturn(true);
278+
when(patternCollection.find(any(Bson.class))).thenReturn(findIterable);
279+
when(findIterable.projection(any(Bson.class))).thenReturn(findIterable);
280+
when(findIterable.first()).thenReturn(patternWithNoVersions);
281+
282+
Pattern pattern = new Pattern.PatternBuilder().setNamespace("finos").setId(42).setVersion("1.0.0").build();
283+
284+
assertThrows(PatternVersionNotFoundException.class,
285+
() -> mongoPatternStore.getPatternForVersion(pattern));
286+
}
287+
256288
@Test
257289
void get_pattern_versions_for_valid_pattern_returns_list_of_versions() throws PatternNotFoundException, NamespaceNotFoundException {
258290
mockSetupPatternDocumentWithVersions();

‎calm-hub/src/test/java/org/finos/calm/store/mongo/TestMongoTimelineStoreShould.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,38 @@ void get_timeline_version_for_invalid_timeline_throws_exception() {
249249
verify(findIterable).projection(Projections.fields(Projections.include("timelines")));
250250
}
251251

252+
@Test
253+
void throw_timeline_not_found_when_versions_document_is_missing_on_get_versions() {
254+
Document timelineWithNoVersions = new Document("namespace", NAMESPACE)
255+
.append("timelines", List.of(new Document("timelineId", 42)));
256+
FindIterable<Document> findIterable = Mockito.mock(DocumentFindIterable.class);
257+
when(namespaceStore.namespaceExists(anyString())).thenReturn(true);
258+
when(timelineCollection.find(any(Bson.class))).thenReturn(findIterable);
259+
when(findIterable.projection(any(Bson.class))).thenReturn(findIterable);
260+
when(findIterable.first()).thenReturn(timelineWithNoVersions);
261+
262+
Timeline timeline = new Timeline.TimelineBuilder().setNamespace(NAMESPACE).setId(42).build();
263+
264+
assertThrows(TimelineNotFoundException.class,
265+
() -> mongoTimelineStore.getTimelineVersions(timeline));
266+
}
267+
268+
@Test
269+
void throw_timeline_version_not_found_when_versions_document_is_missing_on_get_for_version() {
270+
Document timelineWithNoVersions = new Document("namespace", NAMESPACE)
271+
.append("timelines", List.of(new Document("timelineId", 42)));
272+
FindIterable<Document> findIterable = Mockito.mock(DocumentFindIterable.class);
273+
when(namespaceStore.namespaceExists(anyString())).thenReturn(true);
274+
when(timelineCollection.find(any(Bson.class))).thenReturn(findIterable);
275+
when(findIterable.projection(any(Bson.class))).thenReturn(findIterable);
276+
when(findIterable.first()).thenReturn(timelineWithNoVersions);
277+
278+
Timeline timeline = new Timeline.TimelineBuilder().setNamespace(NAMESPACE).setId(42).setVersion("1.0.0").build();
279+
280+
assertThrows(TimelineVersionNotFoundException.class,
281+
() -> mongoTimelineStore.getTimelineForVersion(timeline));
282+
}
283+
252284
@Test
253285
void get_timeline_versions_for_valid_timeline_returns_list_of_versions() throws TimelineNotFoundException, NamespaceNotFoundException {
254286
mockSetupTimelineDocumentWithVersions();

0 commit comments

Comments
 (0)