From 564beb7211a17a64cc6c3c14f2359afee565b3f5 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 13 Sep 2023 05:55:02 -0400 Subject: [PATCH] Allow absolute links for refs with an identifier (#5295) * Allow absolute links for refs with an identifier * Add changelog * Update hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5295-fix-absolute-refs-with-identifier.yaml Co-authored-by: Kevin Dougan SmileCDR <72025369+KevinDougan-SmileCDR@users.noreply.github.com> * Remove redundant class * Review comments --------- Co-authored-by: Kevin Dougan SmileCDR <72025369+KevinDougan-SmileCDR@users.noreply.github.com> --- ...295-fix-absolute-refs-with-identifier.yaml | 6 +++++ .../SearchParamExtractorService.java | 17 ++++++++++-- .../r5/FhirSystemDaoTransactionR5Test.java | 14 ++++++++++ .../test/resources/docref-test-bundle.json | 27 +++++++++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5295-fix-absolute-refs-with-identifier.yaml create mode 100644 hapi-fhir-jpaserver-test-r5/src/test/resources/docref-test-bundle.json diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5295-fix-absolute-refs-with-identifier.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5295-fix-absolute-refs-with-identifier.yaml new file mode 100644 index 00000000000..80b40e9c0fb --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5295-fix-absolute-refs-with-identifier.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 5295 +title: "A regression in the HAPI FHIR 6.6.0 JPA server meant that absolute resource + references which also contained an identifier were rejected even if the server + was configured to allow absolute references. This has been corrected." diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java index 3b39ccc7df6..01e288499c4 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java @@ -64,6 +64,7 @@ import org.apache.commons.lang3.StringUtils; import org.hl7.fhir.instance.model.api.IBaseReference; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.IdType; import org.springframework.beans.factory.annotation.Autowired; import java.util.ArrayList; @@ -692,8 +693,20 @@ public class SearchParamExtractorService { return; } - final boolean hasNoIdentifier = !nextReference.hasIdentifier(); - final String baseUrl = hasNoIdentifier ? nextId.getBaseUrl() : null; + String baseUrl = nextId.getBaseUrl(); + + // If this is a conditional URL, the part after the question mark + // can include URLs (e.g. token system URLs) and these really confuse + // the IdType parser because a conditional URL isn't actually a valid + // FHIR ID. So in order to truly determine whether we're dealing with + // an absolute reference, we strip the query part and reparse + // the reference. + int questionMarkIndex = nextId.getValue().indexOf('?'); + if (questionMarkIndex != -1) { + IdType preQueryId = new IdType(nextId.getValue().substring(0, questionMarkIndex - 1)); + baseUrl = preQueryId.getBaseUrl(); + } + String typeString = nextId.getResourceType(); if (isBlank(typeString)) { String msg = "Invalid resource reference found at path[" + path + "] - Does not contain resource type - " diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirSystemDaoTransactionR5Test.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirSystemDaoTransactionR5Test.java index 0b9448af79c..8e4e4398463 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirSystemDaoTransactionR5Test.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirSystemDaoTransactionR5Test.java @@ -20,12 +20,14 @@ import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import javax.annotation.Nonnull; +import java.io.IOException; import java.util.UUID; import static org.apache.commons.lang3.StringUtils.countMatches; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.in; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -40,6 +42,7 @@ public class FhirSystemDaoTransactionR5Test extends BaseJpaR5Test { myStorageSettings.setMatchUrlCacheEnabled(defaults.isMatchUrlCacheEnabled()); myStorageSettings.setDeleteEnabled(defaults.isDeleteEnabled()); myStorageSettings.setInlineResourceTextBelowSize(defaults.getInlineResourceTextBelowSize()); + myStorageSettings.setAllowExternalReferences(defaults.isAllowExternalReferences()); } @@ -503,6 +506,17 @@ public class FhirSystemDaoTransactionR5Test extends BaseJpaR5Test { } + @Test + public void testExternalReference() throws IOException { + myStorageSettings.setAllowExternalReferences(true); + + Bundle input = loadResourceFromClasspath(Bundle.class, "docref-test-bundle.json"); + Bundle output = mySystemDao.transaction(mySrd, input); + assertEquals(1, output.getEntry().size()); + } + + + @Test public void testConditionalDeleteAndConditionalUpdateOnSameResource_MultipleMatchesAlreadyExist() { diff --git a/hapi-fhir-jpaserver-test-r5/src/test/resources/docref-test-bundle.json b/hapi-fhir-jpaserver-test-r5/src/test/resources/docref-test-bundle.json new file mode 100644 index 00000000000..8a7a66f30ea --- /dev/null +++ b/hapi-fhir-jpaserver-test-r5/src/test/resources/docref-test-bundle.json @@ -0,0 +1,27 @@ +{ + "resourceType": "Bundle", + "type": "transaction", + "entry": [ + { + "fullUrl": "Provenance/1.2.40.0.13.1.1.584109161.20230630175432428.38899", + "resource": { + "resourceType": "Provenance", + "id": "1.2.40.0.13.1.1.584109161.20230630175432428.38899", + "entity": [ + { + "what": { + "reference": "https://somehost:8443/mhd/r4/responder/DocumentReference/urn:oid:1.2.40.0.13.1.1.584109161.20230630175432428.38899", + "identifier": { + "value": "urn:oid:1.2.40.0.13.1.1.584109161.20230630175432428.38899" + } + } + } + ] + }, + "request": { + "method": "PUT", + "url": "Provenance/1.2.40.0.13.1.1.584109161.20230630175432428.38899" + } + } + ] +}