Skip to content

Commit f1e6c33

Browse files
committed
Fix Nullpointer Exception in ReferenceResolve
1 parent 5ff2214 commit f1e6c33

File tree

10 files changed

+250
-367
lines changed

10 files changed

+250
-367
lines changed

src/main/java/de/medizininformatikinitiative/torch/config/AppConfig.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import de.medizininformatikinitiative.torch.util.Redaction;
3434
import de.medizininformatikinitiative.torch.util.ReferenceExtractor;
3535
import de.medizininformatikinitiative.torch.util.ReferenceHandler;
36+
import de.medizininformatikinitiative.torch.util.ResourceGroupValidator;
3637
import de.medizininformatikinitiative.torch.util.ResourceReader;
3738
import de.medizininformatikinitiative.torch.util.ResultFileManager;
3839
import de.numcodex.sq2cql.Translator;
@@ -126,8 +127,13 @@ public ProcessedGroupFactory attributeGroupProcessor(CompartmentManager manager)
126127

127128

128129
@Bean
129-
ReferenceHandler referenceHandler(DataStore dataStore, ProfileMustHaveChecker mustHaveChecker, CompartmentManager compartmentManager, ConsentValidator consentValidator) {
130-
return new ReferenceHandler(mustHaveChecker);
130+
ReferenceHandler referenceHandler(ResourceGroupValidator resourceGroupValidator) {
131+
return new ReferenceHandler(resourceGroupValidator);
132+
}
133+
134+
@Bean
135+
ResourceGroupValidator resourceGroupValidator(ProfileMustHaveChecker profileMustHaveChecker) {
136+
return new ResourceGroupValidator(profileMustHaveChecker);
131137
}
132138

133139
@Bean

src/main/java/de/medizininformatikinitiative/torch/exceptions/ResourceTypeMissmatchException.java

Lines changed: 0 additions & 8 deletions
This file was deleted.

src/main/java/de/medizininformatikinitiative/torch/service/ReferenceResolver.java

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,7 @@ public Mono<ResourceBundle> resolveCoreBundle(ResourceBundle coreBundle, Map<Str
6767
.collect(Collectors.toSet()))
6868
.expand(currentGroupSet ->
6969
processResourceGroups(currentGroupSet, null, coreBundle, false, groupMap)
70-
.onErrorResume(e -> {
71-
return Mono.empty(); // Skip this resource group on error
72-
}))
70+
.onErrorResume(e -> Mono.empty()))
7371
.then(Mono.just(coreBundle));
7472
}
7573

@@ -144,26 +142,20 @@ public Mono<Set<ResourceGroup>> processResourceGroups(
144142
return bundleLoader.fetchUnknownResources(referencesGroupedByResourceGroup, patientBundle, coreBundle, applyConsent)
145143
.thenMany(
146144
Flux.fromIterable(referencesGroupedByResourceGroup.entrySet())
145+
.filter(entry -> entry.getKey() != null && entry.getValue() != null && !entry.getValue().isEmpty())
147146
.concatMap(entry ->
148-
{
149-
try {
150-
return referenceHandler.handleReferences(
151-
entry.getValue(),
152-
patientBundle,
153-
coreBundle,
154-
groupMap
155-
);
156-
} catch (MustHaveViolatedException e) {
157-
return Flux.empty();
158-
}
159-
}
147+
Flux.fromIterable(referenceHandler.handleReferences(
148+
entry.getValue(),
149+
patientBundle,
150+
coreBundle,
151+
groupMap
152+
))
160153
)
161154
)
162155
.collect(Collectors.toSet())
163156
.flatMap(set -> set.isEmpty() ? Mono.empty() : Mono.just(set));
164157
}
165158

166-
167159
/**
168160
* Extracts for every ResourceGroup the ReferenceWrappers and collects them ordered by
169161
*
@@ -179,9 +171,9 @@ public Map<ResourceGroup, List<ReferenceWrapper>> loadReferencesByResourceGroup(
179171
ResourceBundle coreBundle,
180172
Map<String, AnnotatedAttributeGroup> groupMap) {
181173

182-
return resourceGroups.parallelStream()
174+
return resourceGroups.stream()
183175
.map(resourceGroup -> processResourceGroup(resourceGroup, patientBundle, coreBundle, groupMap))
184-
.filter(entry -> !entry.getValue().isEmpty())
176+
.filter(entry -> entry.getKey() != null && entry.getValue() != null && !entry.getValue().isEmpty())
185177
.collect(Collectors.toMap(
186178
Map.Entry::getKey,
187179
Map.Entry::getValue,
@@ -219,11 +211,7 @@ private Map.Entry<ResourceGroup, List<ReferenceWrapper>> processResourceGroup(
219211
? patientBundle.bundle().get(resourceGroup.resourceId())
220212
: coreBundle.get(resourceGroup.resourceId());
221213

222-
if (resource.isPresent()) {
223-
return extractReferences(resourceGroup, resource.get(), groupMap, processingBundle);
224-
} else {
225-
return handleMissingResource(resourceGroup, processingBundle);
226-
}
214+
return resource.map(value -> extractReferences(resourceGroup, value, groupMap, processingBundle)).orElseGet(() -> handleMissingResource(resourceGroup, processingBundle));
227215
}
228216

229217
/**
@@ -253,9 +241,7 @@ private Map.Entry<ResourceGroup, List<ReferenceWrapper>> extractReferences(
253241
List<ReferenceWrapper> extracted = referenceExtractor.extract(resource, groupMap, resourceGroup.groupId());
254242
return Map.entry(resourceGroup, extracted);
255243
} catch (MustHaveViolatedException e) {
256-
synchronized (processingBundle) {
257-
processingBundle.addResourceGroupValidity(resourceGroup, false);
258-
}
244+
processingBundle.addResourceGroupValidity(resourceGroup, false);
259245
return Map.entry(resourceGroup, Collections.emptyList());
260246
}
261247
}
@@ -270,11 +256,8 @@ private Map.Entry<ResourceGroup, List<ReferenceWrapper>> extractReferences(
270256
private Map.Entry<ResourceGroup, List<ReferenceWrapper>> handleMissingResource(
271257
ResourceGroup resourceGroup,
272258
ResourceBundle processingBundle) {
273-
274-
synchronized (processingBundle) {
275-
logger.warn("Empty resource marked as valid for group {}", resourceGroup);
276-
processingBundle.addResourceGroupValidity(resourceGroup, false);
277-
}
259+
logger.warn("Empty resource marked as valid for group {}", resourceGroup);
260+
processingBundle.addResourceGroupValidity(resourceGroup, false);
278261
return Map.entry(resourceGroup, Collections.emptyList());
279262
}
280263

src/main/java/de/medizininformatikinitiative/torch/util/ReferenceHandler.java

Lines changed: 53 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,12 @@
77
import de.medizininformatikinitiative.torch.model.management.ResourceAttribute;
88
import de.medizininformatikinitiative.torch.model.management.ResourceBundle;
99
import de.medizininformatikinitiative.torch.model.management.ResourceGroup;
10-
import org.hl7.fhir.r4.model.Resource;
1110
import org.slf4j.Logger;
1211
import org.slf4j.LoggerFactory;
13-
import reactor.core.publisher.Flux;
1412

1513
import javax.annotation.Nullable;
1614
import java.util.ArrayList;
15+
import java.util.Collections;
1716
import java.util.List;
1817
import java.util.Map;
1918
import java.util.Objects;
@@ -22,24 +21,27 @@
2221

2322
public class ReferenceHandler {
2423
private static final Logger logger = LoggerFactory.getLogger(ReferenceHandler.class);
25-
private final ProfileMustHaveChecker profileMustHaveChecker;
24+
private final ResourceGroupValidator resourceGroupValidator;
2625

2726

28-
public ReferenceHandler(ProfileMustHaveChecker profileMustHaveChecker) {
29-
this.profileMustHaveChecker = profileMustHaveChecker;
27+
public ReferenceHandler(ResourceGroupValidator resourceGroupValidator) {
28+
this.resourceGroupValidator = resourceGroupValidator;
3029

3130
}
3231

33-
private static Flux<List<ResourceGroup>> checkReferenceViolatesMustHave(ReferenceWrapper referenceWrapper, List<ResourceGroup> list, ResourceBundle processingBundle) {
32+
private static List<ResourceGroup> checkReferenceViolatesMustHave(ReferenceWrapper referenceWrapper, List<ResourceGroup> list, ResourceBundle processingBundle) throws MustHaveViolatedException {
3433
ResourceAttribute referenceAttribute = referenceWrapper.toResourceAttributeGroup();
3534
if (referenceWrapper.refAttribute().mustHave() && list.isEmpty()) {
3635
processingBundle.setResourceAttributeInValid(referenceAttribute);
37-
return Flux.error(new MustHaveViolatedException(
36+
throw new MustHaveViolatedException(
3837
"MustHave condition violated: No valid references were resolved for " + referenceWrapper.references()
39-
));
38+
);
39+
}
40+
if (referenceWrapper.references().isEmpty()) {
41+
return List.of();
4042
}
4143
processingBundle.setResourceAttributeValid(referenceAttribute);
42-
return Flux.just(list);
44+
return list;
4345
}
4446

4547
/**
@@ -49,47 +51,57 @@ private static Flux<List<ResourceGroup>> checkReferenceViolatesMustHave(Referenc
4951
* @param groupMap cache containing all known attributeGroups
5052
* @return newly added ResourceGroups to be processed
5153
*/
52-
public Flux<ResourceGroup> handleReferences(List<ReferenceWrapper> references,
54+
public List<ResourceGroup> handleReferences(List<ReferenceWrapper> references,
5355
@Nullable PatientResourceBundle patientBundle,
5456
ResourceBundle coreBundle,
55-
Map<String, AnnotatedAttributeGroup> groupMap) throws MustHaveViolatedException {
57+
Map<String, AnnotatedAttributeGroup> groupMap) {
5658
ResourceBundle processingBundle = (patientBundle != null) ? patientBundle.bundle() : coreBundle;
5759
ResourceGroup parentGroup = new ResourceGroup(references.getFirst().resourceId(), references.getFirst().groupId());
5860

59-
List<ReferenceWrapper> unprocessedReferences = filterUnprocessedReferences(references, processingBundle);
60-
Set<ResourceGroup> knownGroups = processingBundle.getKnownResourceGroups();
61-
return Flux.fromIterable(unprocessedReferences)
62-
.concatMap(ref -> handleReference(ref, patientBundle, coreBundle, groupMap).doOnNext(
63-
resourceGroupList -> {
64-
ResourceAttribute referenceAttribute = ref.toResourceAttributeGroup();
65-
resourceGroupList.forEach(resourceGroup -> processingBundle.addAttributeToChild(referenceAttribute, resourceGroup));
61+
try {
62+
List<ReferenceWrapper> unprocessedReferences = filterUnprocessedReferences(references, processingBundle);
63+
Set<ResourceGroup> knownGroups = processingBundle.getKnownResourceGroups();
64+
return unprocessedReferences.stream()
65+
// map each reference to a list of ResourceGroups
66+
.map(ref -> {
67+
List<ResourceGroup> resourceGroupList;
68+
try {
69+
resourceGroupList = handleReference(ref, patientBundle, coreBundle, groupMap);
70+
} catch (MustHaveViolatedException e) {
71+
processingBundle.addResourceGroupValidity(parentGroup, false);
72+
return Collections.<ResourceGroup>emptyList();
6673
}
67-
))
68-
.collectList()
69-
.flatMapMany(results -> Flux.fromIterable(results.stream()
70-
.flatMap(List::stream)
71-
.toList()))
72-
.filter(group -> !knownGroups.contains(group))
73-
.onErrorResume(MustHaveViolatedException.class, e -> {
74-
processingBundle.addResourceGroupValidity(parentGroup, false);
75-
logger.warn("MustHaveViolatedException occurred. Stopping resource processing: {}", e.getMessage());
76-
return Flux.empty();
77-
});
74+
ResourceAttribute referenceAttribute = ref.toResourceAttributeGroup();
75+
// side effect: add attribute to each resource group
76+
resourceGroupList.forEach(rg -> processingBundle.addAttributeToChild(referenceAttribute, rg));
77+
return resourceGroupList;
78+
})
79+
.flatMap(List::stream)
80+
.filter(group -> !knownGroups.contains(group))
81+
.toList();
82+
} catch (MustHaveViolatedException e) {
83+
processingBundle.addResourceGroupValidity(parentGroup, false);
84+
logger.warn("MustHaveViolatedException occurred. Stopping resource processing: {}", e.getMessage());
85+
return List.of();
86+
}
7887
}
7988

8089
/**
8190
* Handles a ReferenceWrapper by resolving its references and updating the processing bundle.
8291
*
92+
* <p>
93+
* Checks if referenceAttribute
94+
*
8395
* @param referenceWrapper The reference wrapper to handle.
8496
* @param patientBundle The patient bundle being updated, if present.
8597
* @param coreBundle to be updated and queried, that contains a centrally shared concurrent HashMap.
8698
* @param groupMap Map of attribute groups for validation.
8799
* @return A Flux emitting a list of ResourceGroups corresponding to the resolved references.
88100
*/
89-
public Flux<List<ResourceGroup>> handleReference(ReferenceWrapper referenceWrapper,
90-
@Nullable PatientResourceBundle patientBundle,
91-
ResourceBundle coreBundle,
92-
Map<String, AnnotatedAttributeGroup> groupMap) {
101+
public List<ResourceGroup> handleReference(ReferenceWrapper referenceWrapper,
102+
@Nullable PatientResourceBundle patientBundle,
103+
ResourceBundle coreBundle,
104+
Map<String, AnnotatedAttributeGroup> groupMap) throws MustHaveViolatedException {
93105

94106
ResourceBundle processingBundle = patientBundle != null ? patientBundle.bundle() : coreBundle;
95107

@@ -104,45 +116,14 @@ public Flux<List<ResourceGroup>> handleReference(ReferenceWrapper referenceWrapp
104116
})
105117
.filter(Objects::nonNull)
106118
.filter(Optional::isPresent)
107-
.flatMap(resource -> collectValidGroups(referenceWrapper, groupMap, resource.get(), processingBundle).stream())
119+
.flatMap(resource -> resourceGroupValidator.collectValidGroups(referenceWrapper, groupMap, resource.get(), processingBundle).stream())
108120
.toList();
109121

110122
// Now run your must-have validation and wrap in Flux
111123
return checkReferenceViolatesMustHave(referenceWrapper, allValidGroups, processingBundle);
112124
}
113125

114126

115-
/**
116-
* Collects all valid resourceGroups for the currently processed ResourceBundle.
117-
* <p> For a given reference and resource checks if already a valid group in processingBundle.
118-
* If resourceGroups not assigned yet, executes filter, musthave (Without References) and profile checks.
119-
*
120-
* @param groupMap known attribute groups
121-
* @param resource Resource to be checked
122-
* @param processingBundle bundle that is currently processed
123-
* @return ResourceGroup if previously unknown and assignable to the group.
124-
*/
125-
private List<ResourceGroup> collectValidGroups(ReferenceWrapper referenceWrapper, Map<String, AnnotatedAttributeGroup> groupMap, Resource resource, ResourceBundle processingBundle) {
126-
return referenceWrapper.refAttribute().linkedGroups().stream()
127-
.map(groupId -> {
128-
ResourceGroup resourceGroup = new ResourceGroup(ResourceUtils.getRelativeURL(resource), groupId);
129-
Boolean isValid = processingBundle.isValidResourceGroup(resourceGroup);
130-
if (isValid == null) {
131-
AnnotatedAttributeGroup group = groupMap.get(groupId);
132-
boolean fulfilled = profileMustHaveChecker.fulfilled(resource, group);
133-
if (group.compiledFilter() != null) {
134-
fulfilled = fulfilled && group.compiledFilter().test(resource);
135-
}
136-
logger.trace("Group {} for Reference: {}", groupId, fulfilled);
137-
isValid = fulfilled;
138-
processingBundle.addResourceGroupValidity(resourceGroup, isValid);
139-
}
140-
return isValid ? resourceGroup : null;
141-
})
142-
.filter(Objects::nonNull)
143-
.toList();
144-
}
145-
146127
/**
147128
* Iterates over the references to find unprocessed reference wrappers.
148129
*
@@ -163,19 +144,15 @@ private List<ReferenceWrapper> filterUnprocessedReferences(List<ReferenceWrapper
163144

164145
Boolean isValid = processingBundle.resourceAttributeValidity().get(resourceAttribute);
165146
processingBundle.addAttributeToParent(resourceAttribute, parentGroup);
166-
if (Boolean.TRUE.equals(isValid)) {
167-
// Already valid, skip
147+
if (isValid == null) {
148+
uncheckedReferences.add(reference);
168149
} else {
169-
if (Boolean.FALSE.equals(isValid)) {
170-
if (reference.refAttribute().mustHave()) {
171-
processingBundle.addResourceGroupValidity(parentGroup, false);
172-
throw new MustHaveViolatedException(
173-
"Must-have attribute violated for reference: " + reference + " in group: " + parentGroup);
174-
}
175-
176-
} else {
177-
uncheckedReferences.add(reference);
150+
if (!isValid && reference.refAttribute().mustHave()) {
151+
processingBundle.addResourceGroupValidity(parentGroup, false);
152+
throw new MustHaveViolatedException(
153+
"Must-have attribute violated for reference: " + reference + " in group: " + parentGroup);
178154
}
155+
179156
}
180157
}
181158
return uncheckedReferences;

0 commit comments

Comments
 (0)