From 0094c2a5a9a5c256b16e75ea25f3b67111b49a12 Mon Sep 17 00:00:00 2001 From: Hauke Hund Date: Mon, 30 Sep 2024 17:39:42 +0200 Subject: [PATCH] moved questionnaire canonical reference check, code cleanup, more tests ReferenceExtractorImpl now also extracts canonical references. Moved the QuestionnaireResponse.qestionnaire canonical reference check from QuestionnaireResponseAuthorizationRule to the general reference check infrastructure, enabling creation of Questionnaire and QuestionnaireResponse resources in the same transaction bundle regardless of order. Currently only Task.instantiatesCanonical and QuestionnaireResponse.qestionnaire canonical references are enforced. --- .../fhir/service/ReferenceExtractorImpl.java | 122 ++++++++++--- .../dsf/fhir/service/ResourceReference.java | 51 ++++-- .../AbstractAuthorizationRule.java | 6 - ...uestionnaireResponseAuthorizationRule.java | 27 +-- .../dao/command/ReferencesHelperImpl.java | 5 +- .../dsf/fhir/service/ReferenceResolver.java | 45 ++++- .../fhir/service/ReferenceResolverImpl.java | 162 ++++++++++-------- .../impl/AbstractResourceServiceImpl.java | 5 +- .../QuestionnaireResponseIntegrationTest.java | 10 +- ...sQuestionnaireResponseIntegrationTest.java | 13 ++ 10 files changed, 295 insertions(+), 151 deletions(-) diff --git a/dsf-fhir/dsf-fhir-rest-adapter/src/main/java/dev/dsf/fhir/service/ReferenceExtractorImpl.java b/dsf-fhir/dsf-fhir-rest-adapter/src/main/java/dev/dsf/fhir/service/ReferenceExtractorImpl.java index 9af14321d..d6dc077a9 100644 --- a/dsf-fhir/dsf-fhir-rest-adapter/src/main/java/dev/dsf/fhir/service/ReferenceExtractorImpl.java +++ b/dsf-fhir/dsf-fhir-rest-adapter/src/main/java/dev/dsf/fhir/service/ReferenceExtractorImpl.java @@ -10,6 +10,7 @@ import org.hl7.fhir.r4.model.Attachment; import org.hl7.fhir.r4.model.BackboneElement; import org.hl7.fhir.r4.model.Binary; +import org.hl7.fhir.r4.model.CanonicalType; import org.hl7.fhir.r4.model.CarePlan; import org.hl7.fhir.r4.model.CareTeam; import org.hl7.fhir.r4.model.ClaimResponse; @@ -80,18 +81,25 @@ public class ReferenceExtractorImpl implements ReferenceExtractor private Function toResourceReferenceFromReference(String referenceLocation, Class... referenceTypes) { - return ref -> new ResourceReference(referenceLocation, ref, referenceTypes); + return reference -> new ResourceReference(referenceLocation, reference, referenceTypes); } private Function toResourceReferenceFromRelatedArtifact( String relatedArtifactLocation) { - return rel -> new ResourceReference(relatedArtifactLocation, rel); + return relatedArtifact -> new ResourceReference(relatedArtifactLocation, relatedArtifact); } private Function toResourceReferenceFromAttachment(String attachmentLocation) { - return rel -> new ResourceReference(attachmentLocation, rel); + return attachment -> new ResourceReference(attachmentLocation, attachment); + } + + @SafeVarargs + private Function toResourceReferenceFromCanonical(String canonicalLocation, + Class... referenceTypes) + { + return canonical -> new ResourceReference(canonicalLocation, canonical, referenceTypes); } @SafeVarargs @@ -260,6 +268,50 @@ private Stream getReferences(E ba .map(toResourceReferenceFromReference(referenceLocation, referenceTypes)) : Stream.empty(); } + @SafeVarargs + private Stream getCanonical(R resource, Predicate hasCanonical, + Function getCanonical, String canonicalLocation, + Class... canonicalTypes) + { + return hasCanonical.test(resource) ? Stream.of(getCanonical.apply(resource)) + .map(toResourceReferenceFromCanonical(canonicalLocation, canonicalTypes)) : Stream.empty(); + } + + @SafeVarargs + private Stream getCanonicals(R resource, Predicate hasCanonical, + Function> getCanonicals, String canonicalLocation, + Class... canonicalTypes) + { + return hasCanonical.test(resource) ? Stream.of(getCanonicals.apply(resource)).flatMap(List::stream) + .map(toResourceReferenceFromCanonical(canonicalLocation, canonicalTypes)) : Stream.empty(); + } + + @SafeVarargs + private Stream getCanonical(E backboneElement, + Predicate hasCanonical, Function getCanonical, String canonicalLocation, + Class... canonicalTypes) + { + return hasCanonical.test(backboneElement) ? Stream.of(getCanonical.apply(backboneElement)) + .map(toResourceReferenceFromCanonical(canonicalLocation, canonicalTypes)) : Stream.empty(); + } + + @SafeVarargs + private Stream getBackboneElementsCanonical( + R resource, Predicate hasBackboneElements, Function> getBackboneElements, + Predicate hasCanonical, Function getCanonical, String canonicalLocation, + Class... canonicalTypes) + { + if (hasBackboneElements.test(resource)) + { + List backboneElements = getBackboneElements.apply(resource); + return backboneElements.stream() + .map(e -> getCanonical(e, hasCanonical, getCanonical, canonicalLocation, canonicalTypes)) + .flatMap(Function.identity()); + } + else + return Stream.empty(); + } + private Stream getExtensionReferences(DomainResource resource) { var extensions = resource.getExtension().stream().filter(e -> e.getValue() instanceof Reference) @@ -447,9 +499,14 @@ public Stream getReferences(CodeSystem resource) if (resource == null) return Stream.empty(); + var supplements = getCanonical(resource, CodeSystem::hasSupplementsElement, CodeSystem::getSupplementsElement, + "CodeSystem.supplements", CodeSystem.class); + var valueSet = getCanonical(resource, CodeSystem::hasValueSetElement, CodeSystem::getValueSetElement, + "CodeSystem.valueSet", ValueSet.class); + var extensionReferences = getExtensionReferences(resource); - return extensionReferences; + return concat(valueSet, supplements, extensionReferences); } @Override @@ -588,6 +645,8 @@ public Stream getReferences(Measure resource) if (resource == null) return Stream.empty(); + var library = getCanonicals(resource, Measure::hasLibrary, Measure::getLibrary, "Measure.library", + Library.class); var subject = getReference(resource, Measure::hasSubjectReference, Measure::getSubjectReference, "Measure.subject", Group.class); var relatedArtifacts = getRelatedArtifacts(resource, Measure::hasRelatedArtifact, Measure::getRelatedArtifact, @@ -595,7 +654,7 @@ public Stream getReferences(Measure resource) var extensionReferences = getExtensionReferences(resource); - return concat(subject, relatedArtifacts, extensionReferences); + return concat(library, subject, relatedArtifacts, extensionReferences); } @Override @@ -604,6 +663,8 @@ public Stream getReferences(MeasureReport resource) if (resource == null) return Stream.empty(); + var measure = getCanonical(resource, MeasureReport::hasMeasureElement, MeasureReport::getMeasureElement, + "MeasureReport.measure", Measure.class); var subject = getReference(resource, MeasureReport::hasSubject, MeasureReport::getSubject, "MeasureReport.subject", Patient.class, Practitioner.class, PractitionerRole.class, Location.class, Device.class, RelatedPerson.class, Group.class); @@ -627,7 +688,8 @@ public Stream getReferences(MeasureReport resource) var extensionReferences = getExtensionReferences(resource); - return concat(subject, reporter, subjectResults1, subjectResults2, evaluatedResource, extensionReferences); + return concat(measure, subject, reporter, subjectResults1, subjectResults2, evaluatedResource, + extensionReferences); } @Override @@ -785,20 +847,24 @@ public Stream getReferences(Questionnaire resource) if (resource == null) return Stream.empty(); + var derivedFrom = getCanonicals(resource, Questionnaire::hasDerivedFrom, Questionnaire::getDerivedFrom, + "Questionnaire.derivedFrom", Questionnaire.class); var enableWhen = getBackboneElements2Reference(resource, Questionnaire::hasItem, Questionnaire::getItem, Questionnaire.QuestionnaireItemComponent::hasEnableWhen, Questionnaire.QuestionnaireItemComponent::getEnableWhen, Questionnaire.QuestionnaireItemEnableWhenComponent::hasAnswerReference, Questionnaire.QuestionnaireItemEnableWhenComponent::getAnswerReference, "Questionnaire.item.enableWhen.answerReference"); - var answerOption = getBackboneElements2Reference(resource, Questionnaire::hasItem, Questionnaire::getItem, Questionnaire.QuestionnaireItemComponent::hasAnswerOption, Questionnaire.QuestionnaireItemComponent::getAnswerOption, Questionnaire.QuestionnaireItemAnswerOptionComponent::hasValueReference, Questionnaire.QuestionnaireItemAnswerOptionComponent::getValueReference, "Questionnaire.item.answerOption.valueReference"); - + var answerValueSet = getBackboneElementsCanonical(resource, Questionnaire::hasItem, Questionnaire::getItem, + Questionnaire.QuestionnaireItemComponent::hasAnswerValueSetElement, + Questionnaire.QuestionnaireItemComponent::getAnswerValueSetElement, "Questionnaire.item.answerValueSet", + ValueSet.class); var initial = getBackboneElements2Reference(resource, Questionnaire::hasItem, Questionnaire::getItem, Questionnaire.QuestionnaireItemComponent::hasInitial, Questionnaire.QuestionnaireItemComponent::getInitial, @@ -808,7 +874,7 @@ public Stream getReferences(Questionnaire resource) var extensionReferences = getExtensionReferences(resource); - return concat(enableWhen, answerOption, initial, extensionReferences); + return concat(derivedFrom, enableWhen, answerOption, answerValueSet, initial, extensionReferences); } @Override @@ -820,26 +886,24 @@ public Stream getReferences(QuestionnaireResponse resource) var author = getReference(resource, QuestionnaireResponse::hasAuthor, QuestionnaireResponse::getAuthor, "QuestionnaireResponse.author", Device.class, Organization.class, Patient.class, Practitioner.class, PractitionerRole.class, RelatedPerson.class); - var basedOn = getReferences(resource, QuestionnaireResponse::hasBasedOn, QuestionnaireResponse::getBasedOn, "QuestionnaireResponse.basedOn", CarePlan.class, ServiceRequest.class); - var encounter = getReference(resource, QuestionnaireResponse::hasEncounter, QuestionnaireResponse::getEncounter, "QuestionnaireResponse.encounter", Encounter.class); - var partOf = getReferences(resource, QuestionnaireResponse::hasPartOf, QuestionnaireResponse::getPartOf, "QuestionnaireResponse.partOf", Observation.class, Procedure.class); - + var questionnaire = getCanonical(resource, QuestionnaireResponse::hasQuestionnaireElement, + QuestionnaireResponse::getQuestionnaireElement, "QuestionnaireResponse.questionnaire", + Questionnaire.class); var source = getReference(resource, QuestionnaireResponse::hasSource, QuestionnaireResponse::getSource, "QuestionnaireResponse.source", Patient.class, Practitioner.class, PractitionerRole.class, RelatedPerson.class); - var subject = getReference(resource, QuestionnaireResponse::hasSubject, QuestionnaireResponse::getSubject, "QuestionnaireResponse.subject"); var extensionReferences = getExtensionReferences(resource); - return concat(author, basedOn, encounter, partOf, source, subject, extensionReferences); + return concat(author, basedOn, encounter, partOf, questionnaire, source, subject, extensionReferences); } @Override @@ -876,9 +940,13 @@ public Stream getReferences(StructureDefinition resource) if (resource == null) return Stream.empty(); + var baseDefinition = getCanonical(resource, StructureDefinition::hasBaseDefinitionElement, + StructureDefinition::getBaseDefinitionElement, "StructureDefinition.baseDefinition", + StructureDefinition.class); + var extensionReferences = getExtensionReferences(resource); - return extensionReferences; + return concat(baseDefinition, extensionReferences); } @Override @@ -899,23 +967,25 @@ public Stream getReferences(Task resource) return Stream.empty(); var basedOns = getReferences(resource, Task::hasBasedOn, Task::getBasedOn, "Task.basedOn"); - var partOfs = getReferences(resource, Task::hasPartOf, Task::getPartOf, "Task.partOf", Task.class); - var focus = getReference(resource, Task::hasFocus, Task::getFocus, "Task.focus"); - var forRef = getReference(resource, Task::hasFor, Task::getFor, "Task.for"); var encounter = getReference(resource, Task::hasEncounter, Task::getEncounter, "Task.encounter", Encounter.class); - var requester = getReference(resource, Task::hasRequester, Task::getRequester, "Task.requester", Device.class, - Organization.class, Patient.class, Practitioner.class, PractitionerRole.class, RelatedPerson.class); + var focus = getReference(resource, Task::hasFocus, Task::getFocus, "Task.focus"); + var forRef = getReference(resource, Task::hasFor, Task::getFor, "Task.for"); + var instantiatesCanonical = getCanonical(resource, Task::hasInstantiatesCanonicalElement, + Task::getInstantiatesCanonicalElement, "Task.instantiatesCanonical", ActivityDefinition.class); + var insurance = getReferences(resource, Task::hasInsurance, Task::getInsurance, "Task.insurance", + Coverage.class, ClaimResponse.class); + var location = getReference(resource, Task::hasLocation, Task::getLocation, "Task.location", Location.class); var owner = getReference(resource, Task::hasOwner, Task::getOwner, "Task.owner", Practitioner.class, PractitionerRole.class, Organization.class, CareTeam.class, HealthcareService.class, Patient.class, Device.class, RelatedPerson.class); - var location = getReference(resource, Task::hasLocation, Task::getLocation, "Task.location", Location.class); + var partOfs = getReferences(resource, Task::hasPartOf, Task::getPartOf, "Task.partOf", Task.class); var reasonReference = getReference(resource, Task::hasReasonReference, Task::getReasonReference, "Task.reasonReference"); - var insurance = getReferences(resource, Task::hasInsurance, Task::getInsurance, "Task.insurance", - Coverage.class, ClaimResponse.class); var relevanteHistories = getReferences(resource, Task::hasRelevantHistory, Task::getRelevantHistory, "Task.relevantHistory", Provenance.class); + var requester = getReference(resource, Task::hasRequester, Task::getRequester, "Task.requester", Device.class, + Organization.class, Patient.class, Practitioner.class, PractitionerRole.class, RelatedPerson.class); var restrictionRecipiets = getBackboneElementReferences(resource, Task::hasRestriction, Task::getRestriction, Task.TaskRestrictionComponent::hasRecipient, Task.TaskRestrictionComponent::getRecipient, "Task.restriction.recipient", Patient.class, Practitioner.class, PractitionerRole.class, @@ -925,8 +995,8 @@ public Stream getReferences(Task resource) var outputReferences = getOutputReferences(resource); var extensionReferences = getExtensionReferences(resource); - return concat(basedOns, partOfs, focus, forRef, encounter, requester, owner, location, reasonReference, - insurance, relevanteHistories, restrictionRecipiets, inputReferences, outputReferences, + return concat(basedOns, encounter, focus, forRef, instantiatesCanonical, insurance, location, owner, partOfs, + reasonReference, relevanteHistories, requester, restrictionRecipiets, inputReferences, outputReferences, extensionReferences); } diff --git a/dsf-fhir/dsf-fhir-rest-adapter/src/main/java/dev/dsf/fhir/service/ResourceReference.java b/dsf-fhir/dsf-fhir-rest-adapter/src/main/java/dev/dsf/fhir/service/ResourceReference.java index 7dccd6930..d4015defd 100644 --- a/dsf-fhir/dsf-fhir-rest-adapter/src/main/java/dev/dsf/fhir/service/ResourceReference.java +++ b/dsf-fhir/dsf-fhir-rest-adapter/src/main/java/dev/dsf/fhir/service/ResourceReference.java @@ -1,7 +1,6 @@ package dev.dsf.fhir.service; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.EnumSet; @@ -11,6 +10,7 @@ import java.util.regex.Pattern; import org.hl7.fhir.r4.model.Attachment; +import org.hl7.fhir.r4.model.CanonicalType; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.RelatedArtifact; @@ -143,42 +143,54 @@ public enum ReferenceType /** * unknown url in Attachment */ - ATTACHMENT_UNKNOWN_URL + ATTACHMENT_UNKNOWN_URL, + /** + * canoncial reference + */ + CANONICAL } private final String location; private final Reference reference; private final RelatedArtifact relatedArtifact; private final Attachment attachment; + private final CanonicalType canonical; private final List> referenceTypes = new ArrayList<>(); @SafeVarargs public ResourceReference(String location, Reference reference, Class... referenceTypes) { - this(location, reference, null, null, Arrays.asList(referenceTypes)); + this(location, reference, null, null, null, List.of(referenceTypes)); } public ResourceReference(String location, RelatedArtifact relatedArtifact) { - this(location, null, relatedArtifact, null, Collections.emptyList()); + this(location, null, relatedArtifact, null, null, Collections.emptyList()); } public ResourceReference(String location, Attachment attachment) { - this(location, null, null, attachment, Collections.emptyList()); + this(location, null, null, attachment, null, Collections.emptyList()); + } + + @SafeVarargs + public ResourceReference(String location, CanonicalType canonical, Class... referenceTypes) + { + this(location, null, null, null, canonical, List.of(referenceTypes)); } private ResourceReference(String location, Reference reference, RelatedArtifact relatedArtifact, - Attachment attachment, Collection> referenceTypes) + Attachment attachment, CanonicalType canonical, Collection> referenceTypes) { this.location = location; - if (reference == null && relatedArtifact == null && attachment == null) - throw new IllegalArgumentException("Either reference, relatedArtifact or attachment expected"); + if (reference == null && relatedArtifact == null && attachment == null && canonical == null) + throw new IllegalArgumentException("Either reference, relatedArtifact, attachment or canonical expected"); this.reference = reference; this.relatedArtifact = relatedArtifact; this.attachment = attachment; + this.canonical = canonical; if (referenceTypes != null) this.referenceTypes.addAll(referenceTypes); @@ -214,6 +226,16 @@ public Attachment getAttachment() return attachment; } + public boolean hasCanonical() + { + return canonical != null; + } + + public CanonicalType getCanonical() + { + return canonical; + } + public String getValue() { if (hasReference()) @@ -222,8 +244,10 @@ else if (hasRelatedArtifact()) return relatedArtifact.getUrl(); else if (hasAttachment()) return attachment.getUrl(); + else if (hasCanonical()) + return canonical.getValue(); else - throw new IllegalArgumentException("reference, related artefact or attachment not set"); + throw new IllegalArgumentException("reference, related artefact, attachment or canonical not set"); } public List> getReferenceTypes() @@ -336,8 +360,15 @@ else if (reference.hasType() && reference.hasIdentifier() && reference.getIdenti return ReferenceType.UNKNOWN; } + else if (canonical != null) + { + if (canonical.hasValue()) + return ReferenceType.CANONICAL; + else + return ReferenceType.UNKNOWN; + } else - throw new IllegalStateException("Either reference or relatedArtifact expected"); + throw new IllegalStateException("Either reference, related artefact, attachment or canonical expected"); } public String getLocation() diff --git a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/authorization/AbstractAuthorizationRule.java b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/authorization/AbstractAuthorizationRule.java index b2343cc27..4e978a6e3 100644 --- a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/authorization/AbstractAuthorizationRule.java +++ b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/authorization/AbstractAuthorizationRule.java @@ -335,12 +335,6 @@ protected final Optional createIfLiteralInternalOrLogicalRefe return Optional.empty(); } - protected final Optional resolveReference(Connection connection, Identity identity, - Optional reference) - { - return reference.flatMap(ref -> referenceResolver.resolveReference(identity, ref, connection)); - } - @Override public Optional reasonPermanentDeleteAllowed(Identity identity, R oldResource) { diff --git a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/authorization/QuestionnaireResponseAuthorizationRule.java b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/authorization/QuestionnaireResponseAuthorizationRule.java index dd3b3273f..ef86fe655 100644 --- a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/authorization/QuestionnaireResponseAuthorizationRule.java +++ b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/authorization/QuestionnaireResponseAuthorizationRule.java @@ -1,7 +1,6 @@ package dev.dsf.fhir.authorization; import java.sql.Connection; -import java.sql.SQLException; import java.util.ArrayList; import java.util.EnumSet; import java.util.List; @@ -9,7 +8,6 @@ import java.util.Optional; import java.util.stream.Collectors; -import org.hl7.fhir.r4.model.Questionnaire; import org.hl7.fhir.r4.model.QuestionnaireResponse; import org.hl7.fhir.r4.model.QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent; import org.hl7.fhir.r4.model.QuestionnaireResponse.QuestionnaireResponseItemComponent; @@ -90,12 +88,8 @@ private Optional newResourceOk(Connection connection, QuestionnaireRespo getItemAndValidate(newResource, CODESYSTEM_DSF_BPMN_USER_TASK_VALUE_USER_TASK_ID, errors); - String questionnaireUrlAndVersion = newResource.getQuestionnaire(); - if (!questionnaireExists(connection, questionnaireUrlAndVersion)) - { - errors.add( - "Questionnaire ressource referenced via canonical QuestionnaireResponse.questionnaire does not exist"); - } + if (!newResource.hasQuestionnaire()) + errors.add("QuestionnaireResponse.questionnaire missing"); if (errors.isEmpty()) return Optional.empty(); @@ -150,23 +144,6 @@ private Optional getItemAndValidate(QuestionnaireResponse newResource, S return Optional.of(value.getValue()); } - private boolean questionnaireExists(Connection connection, String questionnaireUrlAndVersion) - { - try - { - Optional questionnaire = daoProvider.getQuestionnaireDao() - .readByUrlAndVersionWithTransaction(connection, questionnaireUrlAndVersion); - - return questionnaire.isPresent(); - } - catch (SQLException e) - { - logger.warn("Could not check questionnaire with url|version '{}' for questionnaire-response - {}", - questionnaireUrlAndVersion, e.getMessage()); - throw new RuntimeException(e); - } - } - @Override public Optional reasonReadAllowed(Connection connection, Identity identity, QuestionnaireResponse existingResource) diff --git a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/ReferencesHelperImpl.java b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/ReferencesHelperImpl.java index 21fea87ae..1890fafb4 100644 --- a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/ReferencesHelperImpl.java +++ b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/ReferencesHelperImpl.java @@ -200,7 +200,7 @@ public void checkReferences(Map idTranslationTable, Connection c Predicate checkReference) throws WebApplicationException { referenceExtractor.getReferences(resource).filter(checkReference) - .filter(ref -> referenceResolver.referenceCanBeChecked(ref, connection)).forEach(ref -> + .filter(ref -> referenceResolver.referenceCanBeResolved(ref, connection)).forEach(ref -> { Optional outcome = checkReference(ref, connection); if (outcome.isPresent()) @@ -224,6 +224,9 @@ private Optional checkReference(ResourceReference reference, C case LOGICAL -> referenceResolver.checkLogicalReference(identity, resource, reference, connection, index); + case CANONICAL -> + referenceResolver.checkCanonicalReference(identity, resource, reference, connection, index); + // unknown URLs to non FHIR servers in related artifacts must not be checked case RELATED_ARTEFACT_UNKNOWN_URL, ATTACHMENT_UNKNOWN_URL -> Optional.empty(); diff --git a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/service/ReferenceResolver.java b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/service/ReferenceResolver.java index d601c234f..048d9e8ca 100755 --- a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/service/ReferenceResolver.java +++ b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/service/ReferenceResolver.java @@ -35,15 +35,6 @@ public interface ReferenceResolver */ Optional resolveReference(Identity identity, ResourceReference reference, Connection connection); - /** - * @param reference - * not null - * @param connection - * not null - * @return true if the {@link ResourceReference} can be checked - */ - boolean referenceCanBeChecked(ResourceReference reference, Connection connection); - /** * @param resource * not null @@ -160,4 +151,40 @@ Optional checkLogicalReference(Identity identity, Resource res Optional checkLogicalReference(Identity identity, Resource resource, ResourceReference resourceReference, Connection connection, Integer bundleIndex) throws IllegalArgumentException; + + /** + * @param identity + * not null + * @param resource + * not null + * @param reference + * not null + * @param connection + * not null + * @return {@link Optional#empty()} if the reference check was successful + * @throws IllegalArgumentException + * if the reference is not of type {@link ResourceReference.ReferenceType#CANONICAL} + * @see ResourceReference#getType(String) + */ + Optional checkCanonicalReference(Identity identity, Resource resource, + ResourceReference reference, Connection connection) throws IllegalArgumentException; + + /** + * @param identity + * not null + * @param resource + * not null + * @param reference + * not null + * @param connection + * not null + * @param bundleIndex + * may be null + * @return {@link Optional#empty()} if the reference check was successful + * @throws IllegalArgumentException + * if the reference is not of type {@link ResourceReference.ReferenceType#CANONICAL} + * @see ResourceReference#getType(String) + */ + Optional checkCanonicalReference(Identity identity, Resource resource, + ResourceReference reference, Connection connection, Integer bundleIndex) throws IllegalArgumentException; } diff --git a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/service/ReferenceResolverImpl.java b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/service/ReferenceResolverImpl.java index b8f59320c..3e5a64795 100755 --- a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/service/ReferenceResolverImpl.java +++ b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/service/ReferenceResolverImpl.java @@ -2,7 +2,6 @@ import java.sql.Connection; import java.util.Arrays; -import java.util.Collections; import java.util.EnumSet; import java.util.List; import java.util.Map; @@ -41,6 +40,9 @@ public class ReferenceResolverImpl implements ReferenceResolver, InitializingBea { private static final Logger logger = LoggerFactory.getLogger(ReferenceResolverImpl.class); + private static final String QUESTIONNAIRE_RESPONSE_QUESTIONNAIRE = "QuestionnaireResponse.questionnaire"; + private static final String TASK_INSTANTIATES_CANONICAL = "Task.instantiatesCanonical"; + private final String serverBase; private final DaoProvider daoProvider; private final ResponseGenerator responseGenerator; @@ -72,12 +74,6 @@ public void afterPropertiesSet() throws Exception @Override public boolean referenceCanBeResolved(ResourceReference reference, Connection connection) - { - return referenceCanBeChecked(reference, connection); - } - - @Override - public boolean referenceCanBeChecked(ResourceReference reference, Connection connection) { Objects.requireNonNull(reference, "reference"); Objects.requireNonNull(connection, "connection"); @@ -85,36 +81,19 @@ public boolean referenceCanBeChecked(ResourceReference reference, Connection con return switch (reference.getType(serverBase)) { case LITERAL_EXTERNAL, RELATED_ARTEFACT_LITERAL_EXTERNAL_URL, ATTACHMENT_LITERAL_EXTERNAL_URL -> - literalExternalReferenceCanBeCheckedAndResolved(reference); + clientProvider.endpointExists(reference.getServerBase(serverBase)); - case LOGICAL -> logicalReferenceCanBeCheckedAndResolved(reference, connection); + case LOGICAL -> exceptionHandler.handleSqlException( + () -> daoProvider.getNamingSystemDao().existsWithUniqueIdUriEntryResolvable(connection, + reference.getReference().getIdentifier().getSystem())); + + case CANONICAL -> QUESTIONNAIRE_RESPONSE_QUESTIONNAIRE.equals(reference.getLocation()) + || TASK_INSTANTIATES_CANONICAL.equals(reference.getLocation()); default -> true; }; } - private boolean logicalReferenceCanBeCheckedAndResolved(ResourceReference reference, Connection connection) - { - if (!ReferenceType.LOGICAL.equals(reference.getType(serverBase))) - throw new IllegalArgumentException("Not a logical reference"); - - return exceptionHandler.handleSqlException( - () -> daoProvider.getNamingSystemDao().existsWithUniqueIdUriEntryResolvable(connection, - reference.getReference().getIdentifier().getSystem())); - } - - private boolean literalExternalReferenceCanBeCheckedAndResolved(ResourceReference reference) - { - if (!EnumSet.of(ReferenceType.LITERAL_EXTERNAL, ReferenceType.RELATED_ARTEFACT_LITERAL_EXTERNAL_URL, - ReferenceType.ATTACHMENT_LITERAL_EXTERNAL_URL).contains(reference.getType(serverBase))) - { - throw new IllegalArgumentException( - "Not a literal external reference, related artifact literal external url or attachment literal external url"); - } - - return clientProvider.endpointExists(reference.getServerBase(serverBase)); - } - @Override public Optional resolveReference(Identity identity, ResourceReference reference, Connection connection) { @@ -131,6 +110,7 @@ public Optional resolveReference(Identity identity, ResourceReference case CONDITIONAL, RELATED_ARTEFACT_CONDITIONAL_URL, ATTACHMENT_CONDITIONAL_URL -> resolveConditionalReference(identity, reference, connection); case LOGICAL -> resolveLogicalReference(identity, reference, connection); + default -> throw new IllegalArgumentException("Reference of type " + type + " not supported"); }; } @@ -240,7 +220,9 @@ private Optional resolveConditionalReference(Identity identity, Resour Connection connection) { Objects.requireNonNull(reference, "reference"); - throwIfReferenceTypeUnexpected(reference.getType(serverBase), ReferenceType.CONDITIONAL, + + ReferenceType referenceType = reference.getType(serverBase); + throwIfReferenceTypeUnexpected(referenceType, ReferenceType.CONDITIONAL, ReferenceType.RELATED_ARTEFACT_CONDITIONAL_URL, ReferenceType.ATTACHMENT_CONDITIONAL_URL); String referenceValue = reference.getValue(); @@ -271,7 +253,7 @@ private Optional resolveConditionalReference(Identity identity, Resour return Optional.empty(); } - return search(identity, connection, d, reference, condition.getQueryParams(), true); + return search(identity, connection, d, reference, condition.getQueryParams(), referenceType); } } @@ -302,14 +284,14 @@ private Optional resolveLogicalReference(Identity identity, ResourceRe } Identifier targetIdentifier = reference.getReference().getIdentifier(); - return search(identity, connection, d, reference, Map.of("identifier", - Collections.singletonList(targetIdentifier.getSystem() + "|" + targetIdentifier.getValue())), true); + return search(identity, connection, d, reference, + Map.of("identifier", List.of(targetIdentifier.getSystem() + "|" + targetIdentifier.getValue())), + ReferenceType.LOGICAL); } } private Optional search(Identity identity, Connection connection, ResourceDao referenceTargetDao, - ResourceReference resourceReference, Map> queryParameters, - boolean logicalNotConditional) + ResourceReference resourceReference, Map> queryParameters, ReferenceType referenceType) { if (Arrays.stream(SearchQuery.STANDARD_PARAMETERS).anyMatch(queryParameters::containsKey)) { @@ -333,11 +315,17 @@ private Optional search(Identity identity, Connection connection, Reso String unsupportedQueryParametersString = unsupportedQueryParameters.stream() .map(SearchQueryParameterError::toString).collect(Collectors.joining("; ")); - logger.warn("{} reference {} at {} in resource contains unsupported queryparameter{} {}", - logicalNotConditional ? "Logical" : "Conditional", queryParameters, resourceReference.getLocation(), - unsupportedQueryParameters.size() != 1 ? "s" : "", unsupportedQueryParametersString); - - return Optional.empty(); + if (EnumSet.of(ReferenceType.CONDITIONAL, ReferenceType.RELATED_ARTEFACT_CONDITIONAL_URL, + ReferenceType.ATTACHMENT_CONDITIONAL_URL).contains(referenceType)) + { + logger.warn("Conditional reference {} at {} in resource contains unsupported queryparameter{} {}", + queryParameters, resourceReference.getLocation(), + unsupportedQueryParameters.size() != 1 ? "s" : "", unsupportedQueryParametersString); + return Optional.empty(); + } + else + throw new IllegalStateException("Unable to search for " + referenceTargetDao.getResourceTypeName() + + ": Unsupported query parameters"); } PartialResult result = exceptionHandler.handleSqlException(() -> @@ -348,39 +336,26 @@ private Optional search(Identity identity, Connection connection, Reso return referenceTargetDao.searchWithTransaction(connection, query); }); - if (result.getTotal() <= 0) - { - if (logicalNotConditional) - logger.warn("Reference target by identifier '{}|{}' of reference at {} in resource", - resourceReference.getReference().getIdentifier().getSystem(), - resourceReference.getReference().getIdentifier().getValue(), resourceReference.getLocation()); - else - logger.warn("Reference target by condition '{}' of reference at {} in resource", - UriComponentsBuilder.newInstance().path(referenceTargetDao.getResourceTypeName()) - .replaceQueryParams(CollectionUtils.toMultiValueMap(queryParameters)).toUriString(), - resourceReference.getLocation()); - - return Optional.empty(); - } - else if (result.getTotal() == 1) - { + if (result.getTotal() == 1) return Optional.of(result.getPartialResult().get(0)); - } - else // if (result.getOverallCount() > 1) + + else { int overallCount = result.getTotal(); - if (logicalNotConditional) - logger.warn( - "Found {} matches for reference target by identifier '{}|{}' of reference at {} in resource", - overallCount, resourceReference.getReference().getIdentifier().getSystem(), - resourceReference.getReference().getIdentifier().getValue(), resourceReference.getLocation()); - else - logger.warn("Found {} matches for reference target by condition '{}' of reference at {} in resource", - overallCount, + if (ReferenceType.LOGICAL.equals(referenceType)) + logger.warn("Found {} matches for reference at {} with identifier '{}|{}'", overallCount, + resourceReference.getLocation(), resourceReference.getReference().getIdentifier().getSystem(), + resourceReference.getReference().getIdentifier().getValue()); + else if (EnumSet.of(ReferenceType.CONDITIONAL, ReferenceType.RELATED_ARTEFACT_CONDITIONAL_URL, + ReferenceType.ATTACHMENT_CONDITIONAL_URL).contains(referenceType)) + logger.warn("Found {} matches for reference at {} with condition '{}'", overallCount, + resourceReference.getLocation(), UriComponentsBuilder.newInstance().path(referenceTargetDao.getResourceTypeName()) - .replaceQueryParams(CollectionUtils.toMultiValueMap(queryParameters)).toUriString(), - resourceReference.getLocation()); + .replaceQueryParams(CollectionUtils.toMultiValueMap(queryParameters)).toUriString()); + else if (ReferenceType.CANONICAL.equals(referenceType)) + logger.warn("Found {} matches for reference at {} with url '{}'", overallCount, + resourceReference.getLocation(), resourceReference.getCanonical().getValue()); return Optional.empty(); } @@ -558,8 +533,9 @@ public Optional checkLogicalReference(Identity identity, Resou Identifier targetIdentifier = reference.getReference().getIdentifier(); // Resource target = - return search(identity, resource, bundleIndex, connection, d, reference, Map.of("identifier", - Collections.singletonList(targetIdentifier.getSystem() + "|" + targetIdentifier.getValue())), true); + return search(identity, resource, bundleIndex, connection, d, reference, + Map.of("identifier", List.of(targetIdentifier.getSystem() + "|" + targetIdentifier.getValue())), + true); // resourceReference.getReference().setIdentifier(null).setReferenceElement( // new IdType(target.getResourceType().name(), target.getIdElement().getIdPart())); @@ -627,4 +603,46 @@ else if (result.getTotal() == 1) resource, resourceReference, result.getTotal())); } } + + @Override + public Optional checkCanonicalReference(Identity identity, Resource resource, + ResourceReference reference, Connection connection) throws IllegalArgumentException + { + return checkCanonicalReference(identity, resource, reference, connection, null); + } + + @Override + public Optional checkCanonicalReference(Identity identity, Resource resource, + ResourceReference reference, Connection connection, Integer bundleIndex) throws IllegalArgumentException + { + Objects.requireNonNull(identity, "identity"); + Objects.requireNonNull(resource, "resource"); + Objects.requireNonNull(reference, "reference"); + Objects.requireNonNull(connection, "connection"); + throwIfReferenceTypeUnexpected(reference.getType(serverBase), ReferenceType.CANONICAL); + + Optional> referenceDao = switch (reference.getLocation()) + { + case QUESTIONNAIRE_RESPONSE_QUESTIONNAIRE -> Optional.of(daoProvider.getQuestionnaireDao()); + case TASK_INSTANTIATES_CANONICAL -> Optional.of(daoProvider.getActivityDefinitionDao()); + + default -> Optional.empty(); + }; + + if (referenceDao.isEmpty()) + { + logger.debug( + "Canonical reference check only implemented for QuestionnaireResponse.questionnaire and Task.instantiatesCanonical, not checking {}", + reference.getLocation()); + return Optional.empty(); + } + + Optional referencedResource = referenceDao.flatMap(dao -> search(identity, connection, dao, reference, + Map.of("url", List.of(reference.getCanonical().getValue())), ReferenceType.CANONICAL)); + + if (referencedResource.isPresent()) + return Optional.empty(); + else + return Optional.of(responseGenerator.referenceTargetNotFoundLocally(bundleIndex, resource, reference)); + } } \ No newline at end of file diff --git a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/webservice/impl/AbstractResourceServiceImpl.java b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/webservice/impl/AbstractResourceServiceImpl.java index 1778674d7..a97e8c735 100755 --- a/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/webservice/impl/AbstractResourceServiceImpl.java +++ b/dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/webservice/impl/AbstractResourceServiceImpl.java @@ -243,7 +243,7 @@ private void checkReferences(Resource resource, Connection connection, Predicate throws WebApplicationException { referenceExtractor.getReferences(resource).filter(checkReference) - .filter(ref -> referenceResolver.referenceCanBeChecked(ref, connection)).forEach(ref -> + .filter(ref -> referenceResolver.referenceCanBeResolved(ref, connection)).forEach(ref -> { Optional outcome = checkReference(resource, connection, ref); if (outcome.isPresent()) @@ -268,6 +268,9 @@ private Optional checkReference(Resource resource, Connection case LOGICAL -> referenceResolver.checkLogicalReference(getCurrentIdentity(), resource, reference, connection); + case CANONICAL -> + referenceResolver.checkCanonicalReference(getCurrentIdentity(), resource, reference, connection); + // unknown URLs to non FHIR servers in related artifacts must not be checked case RELATED_ARTEFACT_UNKNOWN_URL, ATTACHMENT_UNKNOWN_URL -> Optional.empty(); diff --git a/dsf-fhir/dsf-fhir-server/src/test/java/dev/dsf/fhir/integration/QuestionnaireResponseIntegrationTest.java b/dsf-fhir/dsf-fhir-server/src/test/java/dev/dsf/fhir/integration/QuestionnaireResponseIntegrationTest.java index 84966dddd..d93a490ed 100644 --- a/dsf-fhir/dsf-fhir-server/src/test/java/dev/dsf/fhir/integration/QuestionnaireResponseIntegrationTest.java +++ b/dsf-fhir/dsf-fhir-server/src/test/java/dev/dsf/fhir/integration/QuestionnaireResponseIntegrationTest.java @@ -41,7 +41,7 @@ public void testCreateValidByLocalUser() throws Exception } @Test - public void testCreateNotAllowedByLocalUser() throws Exception + public void testCreateNotAllowedByLocalUserStatusCompleted() throws Exception { QuestionnaireResponse questionnaireResponse = createQuestionnaireResponse(); questionnaireResponse.setStatus(QuestionnaireResponseStatus.COMPLETED); @@ -49,6 +49,14 @@ public void testCreateNotAllowedByLocalUser() throws Exception expectForbidden(() -> getWebserviceClient().create(questionnaireResponse)); } + @Test + public void testCreateNotAllowedByLocalUserQuestionnaireDoesNotExists() throws Exception + { + QuestionnaireResponse questionnaireResponse = createQuestionnaireResponse(); + + expectForbidden(() -> getWebserviceClient().create(questionnaireResponse)); + } + @Test public void testCreateNotAllowedByRemoteUser() throws Exception { diff --git a/dsf-fhir/dsf-fhir-server/src/test/java/dev/dsf/fhir/integration/QuestionnaireVsQuestionnaireResponseIntegrationTest.java b/dsf-fhir/dsf-fhir-server/src/test/java/dev/dsf/fhir/integration/QuestionnaireVsQuestionnaireResponseIntegrationTest.java index 38c010c1f..b1dc6d7bb 100644 --- a/dsf-fhir/dsf-fhir-server/src/test/java/dev/dsf/fhir/integration/QuestionnaireVsQuestionnaireResponseIntegrationTest.java +++ b/dsf-fhir/dsf-fhir-server/src/test/java/dev/dsf/fhir/integration/QuestionnaireVsQuestionnaireResponseIntegrationTest.java @@ -186,4 +186,17 @@ public void testPostQuestionnaireAndCorrespondingQuestionnaireResponseInTransact assertTrue(getWebserviceClient().postBundle(bundle).getEntry().stream() .allMatch(entry -> entry.getResponse().getStatus().equals("201 Created"))); } + + @Test + public void testPostQuestionnaireResponseInTransactionBundleQuestionnaireDoesNotExistForbidden() throws Exception + { + QuestionnaireResponse questionnaireResponse = createQuestionnaireResponse("1.5.3"); + + Bundle bundle = new Bundle(); + bundle.setType(Bundle.BundleType.TRANSACTION); + bundle.addEntry().setResource(questionnaireResponse).setFullUrl("urn:uuid:" + UUID.randomUUID().toString()) + .getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl(ResourceType.QuestionnaireResponse.name()); + + expectForbidden(() -> getWebserviceClient().postBundle(bundle)); + } } \ No newline at end of file