From 0c5b64cce7d75aa49b5be771f68df9cf48b67706 Mon Sep 17 00:00:00 2001 From: jmarchionatto <60409882+jmarchionatto@users.noreply.github.com> Date: Thu, 20 Oct 2022 13:47:18 -0400 Subject: [PATCH] Change IG loader check for existence of SearchParameter resources to use same strategy as validator to avoid first one not finding resource and validator finding it later, producing a duplication exception. (#4164) Co-authored-by: juan.marchionatto --- .../4162-error-loading-us-core-ig.yaml | 5 ++ .../jpa/packages/PackageInstallerSvcImpl.java | 32 +++++++- .../searchparam/config/SearchParamConfig.java | 8 ++ .../util/SearchParameterHelper.java | 64 +++++++++++++++ .../util/SearchParameterHelperTest.java | 77 +++++++++++++++++++ 5 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4162-error-loading-us-core-ig.yaml create mode 100644 hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/util/SearchParameterHelper.java create mode 100644 hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/util/SearchParameterHelperTest.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4162-error-loading-us-core-ig.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4162-error-loading-us-core-ig.yaml new file mode 100644 index 00000000000..add9518dbc1 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4162-error-loading-us-core-ig.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 4162 +title: "Loading us-core IG was raising `UnprocessableEntityException: HAPI-2131: Can't process submitted SearchParameter as it is overlapping an existing one`. + This problem has been fixed." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java index 2cbc6f1e774..d9b8cba3508 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java @@ -20,7 +20,6 @@ package ca.uhn.fhir.jpa.packages; * #L% */ -import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.context.BaseRuntimeChildDefinition; import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition; import ca.uhn.fhir.context.BaseRuntimeElementDefinition; @@ -28,6 +27,7 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.context.support.IValidationSupport; import ca.uhn.fhir.context.support.ValidationSupportContext; +import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; @@ -38,6 +38,7 @@ import ca.uhn.fhir.jpa.model.entity.NpmPackageVersionEntity; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistryController; +import ca.uhn.fhir.jpa.searchparam.util.SearchParameterHelper; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.param.TokenParam; @@ -113,6 +114,9 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { private ISearchParamRegistryController mySearchParamRegistryController; @Autowired private PartitionSettings myPartitionSettings; + @Autowired + private SearchParameterHelper mySearchParameterHelper; + /** * Constructor */ @@ -492,6 +496,8 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { } else if (resource.getClass().getSimpleName().equals("Subscription")) { String id = extractIdFromSubscription(resource); return SearchParameterMap.newSynchronous().add("_id", new TokenParam(id)); + } else if (resource.getClass().getSimpleName().equals("SearchParameter")) { + return buildSearchParameterMapForSearchParameter(resource); } else if (resourceHasUrlElement(resource)) { String url = extractUniqueUrlFromMetadataResource(resource); return SearchParameterMap.newSynchronous().add("url", new UriParam(url)); @@ -501,6 +507,30 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { } } + + /** + * Strategy is to build a SearchParameterMap same way the SearchParamValidatingInterceptor does, to make sure that + * the loader search detects existing resources and routes process to 'update' path, to avoid treating it as a new + * upload which validator later rejects as duplicated. + * To achieve this, we try canonicalizing the SearchParameter first (as the validator does) and if that is not possible + * we cascade to building the map from 'url' or 'identifier'. + */ + private SearchParameterMap buildSearchParameterMapForSearchParameter(IBaseResource theResource) { + Optional spmFromCanonicalized = mySearchParameterHelper.buildSearchParameterMapFromCanonical(theResource); + if (spmFromCanonicalized.isPresent()) { + return spmFromCanonicalized.get(); + } + + if (resourceHasUrlElement(theResource)) { + String url = extractUniqueUrlFromMetadataResource(theResource); + return SearchParameterMap.newSynchronous().add("url", new UriParam(url)); + } else { + TokenParam identifierToken = extractIdentifierFromOtherResourceTypes(theResource); + return SearchParameterMap.newSynchronous().add("identifier", identifierToken); + } + } + + private String extractUniqeIdFromNamingSystem(IBaseResource resource) { FhirTerser terser = myFhirContext.newTerser(); IBase uniqueIdComponent = (IBase) terser.getSingleValueOrNull(resource, "uniqueId"); diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/config/SearchParamConfig.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/config/SearchParamConfig.java index 92cf0349eca..dab9416b674 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/config/SearchParamConfig.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/config/SearchParamConfig.java @@ -42,6 +42,7 @@ import ca.uhn.fhir.jpa.searchparam.matcher.IndexedSearchParamExtractor; import ca.uhn.fhir.jpa.searchparam.matcher.SearchParamMatcher; import ca.uhn.fhir.jpa.searchparam.registry.SearchParamRegistryImpl; import ca.uhn.fhir.jpa.searchparam.registry.SearchParameterCanonicalizer; +import ca.uhn.fhir.jpa.searchparam.util.SearchParameterHelper; import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; @@ -131,4 +132,11 @@ public class SearchParamConfig { ResourceChangeListenerCache registeredResourceChangeListener(String theResourceName, IResourceChangeListener theResourceChangeListener, SearchParameterMap theSearchParameterMap, long theRemoteRefreshIntervalMs) { return new ResourceChangeListenerCache(theResourceName, theResourceChangeListener, theSearchParameterMap, theRemoteRefreshIntervalMs); } + + @Bean + @Lazy + SearchParameterHelper searchParameterHelper(FhirContext theFhirContext) { + return new SearchParameterHelper(searchParameterCanonicalizer(theFhirContext)); + } + } diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/util/SearchParameterHelper.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/util/SearchParameterHelper.java new file mode 100644 index 00000000000..abc6ae5a723 --- /dev/null +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/util/SearchParameterHelper.java @@ -0,0 +1,64 @@ +package ca.uhn.fhir.jpa.searchparam.util; + +import ca.uhn.fhir.context.RuntimeSearchParam; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.jpa.searchparam.registry.SearchParameterCanonicalizer; +import ca.uhn.fhir.rest.param.TokenAndListParam; +import ca.uhn.fhir.rest.param.TokenOrListParam; +import ca.uhn.fhir.rest.param.TokenParam; +import org.hl7.fhir.instance.model.api.IBaseResource; + +import java.util.List; +import java.util.Optional; + +import static org.apache.commons.lang3.StringUtils.isNotBlank; + +public class SearchParameterHelper { + private final SearchParameterCanonicalizer mySearchParameterCanonicalizer; + + public SearchParameterHelper(SearchParameterCanonicalizer theSearchParameterCanonicalizer) { + mySearchParameterCanonicalizer = theSearchParameterCanonicalizer; + } + + + public Optional buildSearchParameterMapFromCanonical(IBaseResource theRuntimeSearchParam) { + RuntimeSearchParam canonicalSearchParam = mySearchParameterCanonicalizer.canonicalizeSearchParameter(theRuntimeSearchParam); + if (canonicalSearchParam == null) { + return Optional.empty(); + } + + SearchParameterMap retVal = new SearchParameterMap(); + + String theCode = canonicalSearchParam.getName(); + List theBases = List.copyOf(canonicalSearchParam.getBase()); + + TokenAndListParam codeParam = new TokenAndListParam().addAnd(new TokenParam(theCode)); + TokenAndListParam basesParam = toTokenAndList(theBases); + + retVal.add("code", codeParam); + retVal.add("base", basesParam); + + return Optional.of(retVal); + } + + + private TokenAndListParam toTokenAndList(List theBases) { + TokenAndListParam retVal = new TokenAndListParam(); + + if (theBases != null) { + + TokenOrListParam tokenOrListParam = new TokenOrListParam(); + retVal.addAnd(tokenOrListParam); + + for (String next : theBases) { + if (isNotBlank(next)) { + tokenOrListParam.addOr(new TokenParam(next)); + } + } + } + + return retVal.getValuesAsQueryTokens().isEmpty() ? null : retVal; + } + + +} diff --git a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/util/SearchParameterHelperTest.java b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/util/SearchParameterHelperTest.java new file mode 100644 index 00000000000..ac20e600d79 --- /dev/null +++ b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/util/SearchParameterHelperTest.java @@ -0,0 +1,77 @@ +package ca.uhn.fhir.jpa.searchparam.util; + +import ca.uhn.fhir.context.RuntimeSearchParam; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.jpa.searchparam.registry.SearchParameterCanonicalizer; +import ca.uhn.fhir.model.api.IQueryParameterType; +import ca.uhn.fhir.rest.param.TokenParam; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class SearchParameterHelperTest { + + @Mock + private SearchParameterCanonicalizer mockedCanonicalizer; + @Mock + private IBaseResource mockedSearchParam; + @Mock + private RuntimeSearchParam mockedRuntimeSearchParam; + + private SearchParameterHelper myTestedHelper; + + @BeforeEach + void setUp() { + myTestedHelper = new SearchParameterHelper(mockedCanonicalizer); + } + + @Test + void whenParamNonCanonicalizableReturnsEmpty() { + when(mockedCanonicalizer.canonicalizeSearchParameter(mockedSearchParam)).thenReturn(null); + + Optional result = myTestedHelper.buildSearchParameterMapFromCanonical(mockedSearchParam); + + assertTrue(result.isEmpty()); + } + + @Test + void whenParamCanonicalizableReturnsFromCanonical() { + String codeParamValue = "code-param-value"; + String baseParamValue = "base-param-value"; + + when(mockedCanonicalizer.canonicalizeSearchParameter(mockedSearchParam)).thenReturn(mockedRuntimeSearchParam); + when(mockedRuntimeSearchParam.getName()).thenReturn(codeParamValue); + when(mockedRuntimeSearchParam.getBase()).thenReturn(Set.of(baseParamValue)); + + Optional result = myTestedHelper.buildSearchParameterMapFromCanonical(mockedSearchParam); + + assertTrue(result.isPresent()); + SearchParameterMap spMap = result.get(); + assertEquals(2, spMap.size()); + + List> codeParam = spMap.get("code"); + assertEquals(1, codeParam.size()); + assertEquals(1, codeParam.get(0).size()); + assertTrue(codeParam.get(0).get(0) instanceof TokenParam); + TokenParam codeTokenParam = (TokenParam) codeParam.get(0).get(0); + assertEquals(codeParamValue, codeTokenParam.getValue()); + + List> baseParam = spMap.get("base"); + assertEquals(1, baseParam.size()); + assertEquals(1, baseParam.get(0).size()); + assertTrue(baseParam.get(0).get(0) instanceof TokenParam); + TokenParam baseTokenParam = (TokenParam) baseParam.get(0).get(0); + assertEquals(baseParamValue, baseTokenParam.getValue()); + } +}