From 0e314a93826b5f1c905f755fd01b2117dcd19687 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Mon, 22 Feb 2021 15:09:59 -0500 Subject: [PATCH 1/8] fix searchparam not found failure at startup (#2408) * reproduced and fixed issue reported by Adi * undoing rearrange --- .../registry/SearchParamRegistryImpl.java | 17 +++++++++--- .../registry/SearchParamRegistryImplTest.java | 27 ++++++++++++++++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImpl.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImpl.java index cb3a8ffbc16..cce2377ea21 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImpl.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImpl.java @@ -34,6 +34,7 @@ import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.searchparam.JpaRuntimeSearchParam; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.util.SearchParameterUtil; import ca.uhn.fhir.util.StopWatch; import com.google.common.annotations.VisibleForTesting; @@ -47,19 +48,19 @@ import org.springframework.beans.factory.annotation.Autowired; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.isBlank; public class SearchParamRegistryImpl implements ISearchParamRegistry, IResourceChangeListener { private static final Logger ourLog = LoggerFactory.getLogger(SearchParamRegistryImpl.class); private static final int MAX_MANAGED_PARAM_COUNT = 10000; - private static long REFRESH_INTERVAL = DateUtils.MILLIS_PER_HOUR; + private static final long REFRESH_INTERVAL = DateUtils.MILLIS_PER_HOUR; @Autowired private ModelConfig myModelConfig; @@ -74,7 +75,7 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry, IResourceC private volatile ReadOnlySearchParamCache myBuiltInSearchParams; private volatile IPhoneticEncoder myPhoneticEncoder; - private volatile JpaSearchParamCache myJpaSearchParamCache = new JpaSearchParamCache(); + private final JpaSearchParamCache myJpaSearchParamCache = new JpaSearchParamCache(); private volatile RuntimeSearchParamCache myActiveSearchParams; @Autowired @@ -282,7 +283,15 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry, IResourceC @Override public void handleInit(Collection theResourceIds) { - List searchParams = theResourceIds.stream().map(id -> mySearchParamProvider.read(id)).collect(Collectors.toList()); + List searchParams = new ArrayList<>(); + for (IIdType id : theResourceIds) { + try { + IBaseResource searchParam = mySearchParamProvider.read(id); + searchParams.add(searchParam); + } catch (ResourceNotFoundException e) { + ourLog.warn("SearchParameter {} not found. Excluding from list of active search params.", id); + } + } initializeActiveSearchParams(searchParams); } diff --git a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImplTest.java b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImplTest.java index 3ff4b285b7c..0deeb0b7a9e 100644 --- a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImplTest.java +++ b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImplTest.java @@ -17,8 +17,11 @@ import ca.uhn.fhir.jpa.searchparam.MatchUrlService; import ca.uhn.fhir.jpa.searchparam.matcher.InMemoryMatchResult; import ca.uhn.fhir.jpa.searchparam.matcher.InMemoryResourceMatcher; import ca.uhn.fhir.jpa.searchparam.matcher.SearchParamMatcher; +import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.server.SimpleBundleProvider; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; +import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.hl7.fhir.r4.model.Enumerations; import org.hl7.fhir.r4.model.SearchParameter; @@ -62,10 +65,10 @@ public class SearchParamRegistryImplTest { private static final ReadOnlySearchParamCache ourBuiltInSearchParams = ReadOnlySearchParamCache.fromFhirContext(ourFhirContext); public static final int TEST_SEARCH_PARAMS = 3; - private static List ourEntities; - private static ResourceVersionMap ourResourceVersionMap; + private static final List ourEntities; + private static final ResourceVersionMap ourResourceVersionMap; private static int ourLastId; - private static int ourBuiltinPatientSearchParamCount; + private static final int ourBuiltinPatientSearchParamCount; static { ourEntities = new ArrayList<>(); @@ -172,6 +175,24 @@ public class SearchParamRegistryImplTest { mySearchParamRegistry.resetForUnitTest(); } + @Test + void handleInit() { + assertEquals(25, mySearchParamRegistry.getActiveSearchParams("Patient").size()); + + IdDt idBad = new IdDt("SearchParameter/bad"); + when(mySearchParamProvider.read(idBad)).thenThrow(new ResourceNotFoundException("id bad")); + + IdDt idGood = new IdDt("SearchParameter/good"); + SearchParameter goodSearchParam = buildSearchParameter(Enumerations.PublicationStatus.ACTIVE); + when(mySearchParamProvider.read(idGood)).thenReturn(goodSearchParam); + + List idList = new ArrayList<>(); + idList.add(idBad); + idList.add(idGood); + mySearchParamRegistry.handleInit(idList); + assertEquals(26, mySearchParamRegistry.getActiveSearchParams("Patient").size()); + } + @Test public void testRefreshAfterExpiry() { mySearchParamRegistry.requestRefresh(); From 18b61f21d0ca4298a6ec5899e28c3469e7c190f0 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 22 Feb 2021 18:23:03 -0500 Subject: [PATCH 2/8] re-add only relevant code --- .../fhir/docs/server_jpa_mdm/mdm_rules.md | 53 +++++++++++++++++- .../mdm/rules/config/MdmRuleValidator.java | 55 ++++++++++++++----- .../mdm/rules/json/MdmFieldMatchJson.java | 14 ++++- .../rules/svc/MdmResourceFieldMatcher.java | 20 ++++++- .../mdm/rules/svc/MdmResourceMatcherSvc.java | 3 +- .../rules/config/MdmRuleValidatorTest.java | 30 ++++++++++ .../mdm/rules/svc/BaseMdmRulesR4Test.java | 2 + .../svc/MdmResourceFieldMatcherR4Test.java | 17 ------ .../mdm/rules/svc/ResourceMatcherR4Test.java | 7 ++- 9 files changed, 159 insertions(+), 42 deletions(-) diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_mdm/mdm_rules.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_mdm/mdm_rules.md index aa1320cd352..3619be3ca12 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_mdm/mdm_rules.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_mdm/mdm_rules.md @@ -51,7 +51,7 @@ Here is an example of a full HAPI MDM rules json document: { "name": "firstname-meta", "resourceType": "Patient", - "resourcePath": "name.given", + "fhirPath": "name.given.first()", "matcher": { "algorithm": "METAPHONE" } @@ -173,8 +173,8 @@ Here is a matcher matchField that uses the SOUNDEX matcher to determine whether ```json { - "name": "familyname-soundex", - "resourceType": "*", + "name": "familyname-soundex", + "resourceType": "*", "resourcePath": "name.family", "matcher": { "algorithm": "SOUNDEX" @@ -196,6 +196,53 @@ Here is a matcher matchField that only matches when two family names are identic } ``` +While it is often suitable to use the `resourcePath` field to indicate the location of the data to be matched, occasionally you will need more direct control over precisely which fields are matched. When performing string matching, the matcher will indiscriminately try to match all elements of the left resource to all elements of the right resource. For example, consider the following two patients and matcher. + +```json +{ + "resourceType": "Patient", + "name": [{ + "given": ["Frank", "John"] + }] +} +``` + +```json +{ + "resourceType": "Patient", + "name": [{ + "given": ["John", "Frank"] + }] +} +``` + +```json +{ + "name": "firstname-meta", + "resourceType": "Patient", + "resourcePath": "name.given", + "matcher": { + "algorithm": "METAPHONE" + } +} +``` + +In this example, these two patients would match, as the matcher will compare all elements of `["John", "Frank"]` to all elements of `["Frank", "John"]` and find that there are matches. This is when you would want to use a FHIRPath matcher, as FHIRPath expressions give you more direct control. This following example shows a matcher that would cause these two patient's not to match to each other. + +```json +{ + "name": "firstname-meta-fhirpath", + "resourceType": "Patient", + "fhirPath": "name.given[0]", + "matcher": { + "algorithm": "METAPHONE" + } +} +``` +Since FHIRPath expressions support indexing it is possible to directly indicate that you would only like to compare the first element of each resource. + + + Special identifier matching is also available if you need to match on a particular identifier system: ```json { diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidator.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidator.java index 5d67cb68448..959058cadbd 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidator.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidator.java @@ -23,6 +23,7 @@ package ca.uhn.fhir.mdm.rules.config; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.fhirpath.IFhirPath; import ca.uhn.fhir.mdm.api.MdmConstants; import ca.uhn.fhir.mdm.api.IMdmRuleValidator; import ca.uhn.fhir.mdm.rules.json.MdmFieldMatchJson; @@ -33,7 +34,9 @@ import ca.uhn.fhir.mdm.rules.json.MdmSimilarityJson; import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.rest.server.util.ISearchParamRetriever; import ca.uhn.fhir.util.FhirTerser; +import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.Patient; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -51,16 +54,14 @@ public class MdmRuleValidator implements IMdmRuleValidator { private final FhirContext myFhirContext; private final ISearchParamRetriever mySearchParamRetriever; - private final Class myPatientClass; - private final Class myPractitionerClass; private final FhirTerser myTerser; + private final IFhirPath myFhirPath; @Autowired public MdmRuleValidator(FhirContext theFhirContext, ISearchParamRetriever theSearchParamRetriever) { myFhirContext = theFhirContext; - myPatientClass = theFhirContext.getResourceDefinition("Patient").getImplementingClass(); - myPractitionerClass = theFhirContext.getResourceDefinition("Practitioner").getImplementingClass(); myTerser = myFhirContext.newTerser(); + myFhirPath = myFhirContext.newFhirPath(); mySearchParamRetriever = theSearchParamRetriever; } @@ -158,20 +159,44 @@ public class MdmRuleValidator implements IMdmRuleValidator { } private void validateFieldPathForType(String theResourceType, MdmFieldMatchJson theFieldMatch) { - ourLog.debug(" validating resource {} for {} ", theResourceType, theFieldMatch.getResourcePath()); + ourLog.debug("Validating resource {} for {} ", theResourceType, theFieldMatch.getResourcePath()); - try { - RuntimeResourceDefinition resourceDefinition = myFhirContext.getResourceDefinition(theResourceType); - Class implementingClass = resourceDefinition.getImplementingClass(); - String path = theResourceType + "." + theFieldMatch.getResourcePath(); - myTerser.getDefinition(implementingClass, path); - } catch (DataFormatException | ConfigurationException | ClassCastException e) { - throw new ConfigurationException("MatchField " + + if (theFieldMatch.getFhirPath() != null && theFieldMatch.getResourcePath() != null) { + throw new ConfigurationException("MatchField [" + theFieldMatch.getName() + - " resourceType " + + "] resourceType [" + theFieldMatch.getResourceType() + - " has invalid path '" + theFieldMatch.getResourcePath() + "'. " + - e.getMessage()); + "] has defined both a resourcePath and a fhirPath. You must define one of the two."); + } + + if (theFieldMatch.getResourcePath() == null && theFieldMatch.getFhirPath() == null) { + throw new ConfigurationException("MatchField [" + + theFieldMatch.getName() + + "] resourceType [" + + theFieldMatch.getResourceType() + + "] has defined neither a resourcePath or a fhirPath. You must define one of the two."); + } + + if (theFieldMatch.getResourcePath() != null) { + try { //Try to validate the struture definition path + RuntimeResourceDefinition resourceDefinition = myFhirContext.getResourceDefinition(theResourceType); + Class implementingClass = resourceDefinition.getImplementingClass(); + String path = theResourceType + "." + theFieldMatch.getResourcePath(); + myTerser.getDefinition(implementingClass, path); + } catch (DataFormatException | ConfigurationException | ClassCastException e) { + //Fallback to attempting to FHIRPath evaluate it. + throw new ConfigurationException("MatchField " + + theFieldMatch.getName() + + " resourceType " + + theFieldMatch.getResourceType() + + " has invalid path '" + theFieldMatch.getResourcePath() + "'. " + e.getMessage()); + } + } else { //Try to validate the FHIRPath + try { + myFhirPath.parse(theResourceType + "." + theFieldMatch.getFhirPath()); + } catch (Exception e) { + throw new ConfigurationException("MatchField [" + theFieldMatch.getName() + "] resourceType [" + theFieldMatch.getResourceType() + "] has failed FHIRPath evaluation. " + e.getMessage()); + } } } diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/json/MdmFieldMatchJson.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/json/MdmFieldMatchJson.java index aba025c0bcd..ac7f331e080 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/json/MdmFieldMatchJson.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/json/MdmFieldMatchJson.java @@ -44,9 +44,12 @@ public class MdmFieldMatchJson implements IModelJson { @JsonProperty(value = "resourceType", required = true) String myResourceType; - @JsonProperty(value = "resourcePath", required = true) + @JsonProperty(value = "resourcePath", required = false) String myResourcePath; + @JsonProperty(value = "fhirPath", required = false) + String myFhirPath; + @JsonProperty(value = "matcher", required = false) MdmMatcherJson myMatcher; @@ -112,4 +115,13 @@ public class MdmFieldMatchJson implements IModelJson { } throw new InternalErrorException("Field Match " + myName + " has neither a matcher nor a similarity."); } + + public String getFhirPath() { + return myFhirPath; + } + + public MdmFieldMatchJson setFhirPath(String theFhirPath) { + myFhirPath = theFhirPath; + return this; + } } diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceFieldMatcher.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceFieldMatcher.java index 2c8807b907e..121d49c4c03 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceFieldMatcher.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceFieldMatcher.java @@ -21,6 +21,7 @@ package ca.uhn.fhir.mdm.rules.svc; */ import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.fhirpath.IFhirPath; import ca.uhn.fhir.mdm.api.MdmMatchEvaluation; import ca.uhn.fhir.mdm.rules.json.MdmFieldMatchJson; import ca.uhn.fhir.mdm.rules.json.MdmRulesJson; @@ -43,16 +44,20 @@ public class MdmResourceFieldMatcher { private final MdmFieldMatchJson myMdmFieldMatchJson; private final String myResourceType; private final String myResourcePath; + private final String myFhirPath; private final MdmRulesJson myMdmRulesJson; private final String myName; + private final boolean myIsFhirPathExpression; public MdmResourceFieldMatcher(FhirContext theFhirContext, MdmFieldMatchJson theMdmFieldMatchJson, MdmRulesJson theMdmRulesJson) { myFhirContext = theFhirContext; myMdmFieldMatchJson = theMdmFieldMatchJson; myResourceType = theMdmFieldMatchJson.getResourceType(); myResourcePath = theMdmFieldMatchJson.getResourcePath(); + myFhirPath = theMdmFieldMatchJson.getFhirPath(); myName = theMdmFieldMatchJson.getName(); myMdmRulesJson = theMdmRulesJson; + myIsFhirPathExpression = myFhirPath != null; } /** @@ -71,9 +76,18 @@ public class MdmResourceFieldMatcher { validate(theLeftResource); validate(theRightResource); - FhirTerser terser = myFhirContext.newTerser(); - List leftValues = terser.getValues(theLeftResource, myResourcePath, IBase.class); - List rightValues = terser.getValues(theRightResource, myResourcePath, IBase.class); + List leftValues; + List rightValues; + + if (myIsFhirPathExpression) { + IFhirPath fhirPath = myFhirContext.newFhirPath(); + leftValues = fhirPath.evaluate(theLeftResource, myFhirPath, IBase.class); + rightValues = fhirPath.evaluate(theRightResource, myFhirPath, IBase.class); + } else { + FhirTerser fhirTerser = myFhirContext.newTerser(); + leftValues = fhirTerser.getValues(theLeftResource, myResourcePath, IBase.class); + rightValues = fhirTerser.getValues(theRightResource, myResourcePath, IBase.class); + } return match(leftValues, rightValues); } diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceMatcherSvc.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceMatcherSvc.java index 5e8c17727b0..37bae2c95cd 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceMatcherSvc.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceMatcherSvc.java @@ -144,7 +144,8 @@ public class MdmResourceMatcherSvc { return retVal; } - private boolean isValidResourceType(String theResourceType, String theFieldComparatorType) { + + private boolean isValidResourceType(String theResourceType, String theFieldComparatorType) { return ( theFieldComparatorType.equalsIgnoreCase(MdmConstants.ALL_RESOURCE_SEARCH_PARAM_TYPE) || theFieldComparatorType.equalsIgnoreCase(theResourceType) diff --git a/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidatorTest.java b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidatorTest.java index f12ba8c35ae..514d4da0eb6 100644 --- a/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidatorTest.java +++ b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidatorTest.java @@ -71,6 +71,36 @@ public class MdmRuleValidatorTest extends BaseR4Test { } } + @Test + public void testMatcherBadFhirPath() throws IOException { + try { + setMdmRuleJson("bad-rules-bad-fhirpath.json"); + fail(); + } catch (ConfigurationException e) { + assertThat(e.getMessage(), startsWith("MatchField [given-name] resourceType [Patient] has failed FHIRPath evaluation. Error in ?? at 1, 1: The name blurst is not a valid function name")); + } + } + + @Test + public void testBadRulesMissingBothPaths() throws IOException { + try { + setMdmRuleJson("bad-rules-no-path.json"); + fail(); + } catch (ConfigurationException e) { + assertThat(e.getMessage(), startsWith("MatchField [given-name] resourceType [Patient] has defined neither a resourcePath or a fhirPath. You must define one of the two.")); + } + } + + @Test + public void testBadRulesBothPathsFilled() throws IOException { + try { + setMdmRuleJson("bad-rules-both-paths.json"); + fail(); + } catch (ConfigurationException e) { + assertThat(e.getMessage(), startsWith("MatchField [given-name] resourceType [Patient] has defined both a resourcePath and a fhirPath. You must define one of the two.")); + } + } + @Test public void testMatcherBadSearchParam() throws IOException { try { diff --git a/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/BaseMdmRulesR4Test.java b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/BaseMdmRulesR4Test.java index e7ab67d3b52..02b456673f4 100644 --- a/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/BaseMdmRulesR4Test.java +++ b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/BaseMdmRulesR4Test.java @@ -16,6 +16,7 @@ import java.util.Arrays; public abstract class BaseMdmRulesR4Test extends BaseR4Test { public static final String PATIENT_GIVEN = "patient-given"; + public static final String PATIENT_GIVEN_FIRST = "patient-given-first"; public static final String PATIENT_FAMILY = "patient-last"; public static final double NAME_THRESHOLD = 0.8; @@ -36,6 +37,7 @@ public abstract class BaseMdmRulesR4Test extends BaseR4Test { .setResourceType("Patient") .setResourcePath("name.given") .setSimilarity(new MdmSimilarityJson().setAlgorithm(MdmSimilarityEnum.COSINE).setMatchThreshold(NAME_THRESHOLD)); + myBothNameFields = String.join(",", PATIENT_GIVEN, PATIENT_FAMILY); } diff --git a/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceFieldMatcherR4Test.java b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceFieldMatcherR4Test.java index fea95e21926..c00841ca708 100644 --- a/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceFieldMatcherR4Test.java +++ b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/MdmResourceFieldMatcherR4Test.java @@ -30,7 +30,6 @@ public class MdmResourceFieldMatcherR4Test extends BaseMdmRulesR4Test { @BeforeEach public void before() { super.before(); - myComparator = new MdmResourceFieldMatcher(ourFhirContext, myGivenNameMatchField, myMdmRulesJson); myJohn = buildJohn(); myJohny = buildJohny(); @@ -91,22 +90,6 @@ public class MdmResourceFieldMatcherR4Test extends BaseMdmRulesR4Test { } } - @Test - public void testBadPath() { - try { - MdmFieldMatchJson matchField = new MdmFieldMatchJson() - .setName("patient-foo") - .setResourceType("Patient") - .setResourcePath("foo") - .setSimilarity(new MdmSimilarityJson().setAlgorithm(MdmSimilarityEnum.COSINE).setMatchThreshold(NAME_THRESHOLD)); - MdmResourceFieldMatcher comparator = new MdmResourceFieldMatcher(ourFhirContext, matchField, myMdmRulesJson); - comparator.match(myJohn, myJohny); - fail(); - } catch (DataFormatException e) { - assertThat(e.getMessage(), startsWith("Unknown child name 'foo' in element Patient")); - } - } - @Test public void testMatch() { assertTrue(myComparator.match(myJohn, myJohny).match); diff --git a/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/ResourceMatcherR4Test.java b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/ResourceMatcherR4Test.java index 7301b2bcbbd..0b105cf1892 100644 --- a/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/ResourceMatcherR4Test.java +++ b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/ResourceMatcherR4Test.java @@ -7,12 +7,15 @@ import ca.uhn.fhir.mdm.rules.json.MdmFieldMatchJson; import ca.uhn.fhir.mdm.rules.json.MdmMatcherJson; import ca.uhn.fhir.mdm.rules.json.MdmRulesJson; import ca.uhn.fhir.mdm.rules.matcher.MdmMatcherEnum; +import ca.uhn.fhir.util.StopWatch; import org.hl7.fhir.r4.model.HumanName; import org.hl7.fhir.r4.model.Patient; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -56,8 +59,8 @@ public class ResourceMatcherR4Test extends BaseMdmRulesR4Test { @Test public void testMetaphoneMatchResult() { MdmResourceMatcherSvc matcherSvc = buildMatcher(buildNamePhoneRules(MdmMatcherEnum.METAPHONE)); - MdmMatchOutcome result = matcherSvc.match(myLeft, myRight); - assertMatchResult(MdmMatchResultEnum.MATCH, 7L, 3.0, false, false, result); + MdmMatchOutcome result = matcherSvc.match(myLeft, myRight); + assertMatchResult(MdmMatchResultEnum.MATCH, 7L, 3.0, false, false, result); } @Test From 0bcf421fb20f7db1fab4e91516662cc43158c20b Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 22 Feb 2021 18:32:27 -0500 Subject: [PATCH 3/8] add forgotten files --- .../resources/bad-rules-bad-fhirpath.json | 18 ++++++++++++++++++ .../test/resources/bad-rules-both-paths.json | 19 +++++++++++++++++++ .../src/test/resources/bad-rules-no-path.json | 17 +++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 hapi-fhir-server-mdm/src/test/resources/bad-rules-bad-fhirpath.json create mode 100644 hapi-fhir-server-mdm/src/test/resources/bad-rules-both-paths.json create mode 100644 hapi-fhir-server-mdm/src/test/resources/bad-rules-no-path.json diff --git a/hapi-fhir-server-mdm/src/test/resources/bad-rules-bad-fhirpath.json b/hapi-fhir-server-mdm/src/test/resources/bad-rules-bad-fhirpath.json new file mode 100644 index 00000000000..b68948e0e9a --- /dev/null +++ b/hapi-fhir-server-mdm/src/test/resources/bad-rules-bad-fhirpath.json @@ -0,0 +1,18 @@ +{ + "version": "1", + "mdmTypes": ["Patient", "Practitioner", "Medication"], + "candidateSearchParams" : [], + "candidateFilterSearchParams" : [], + "matchFields" : [ { + "name" : "given-name", + "resourceType" : "Patient", + "fhirPath" : "name.given.blurst()", + "matcher" : { + "algorithm": "STRING", + "exact" : true + } + }], + "matchResultMap" : { + "given-name" : "POSSIBLE_MATCH" + } +} diff --git a/hapi-fhir-server-mdm/src/test/resources/bad-rules-both-paths.json b/hapi-fhir-server-mdm/src/test/resources/bad-rules-both-paths.json new file mode 100644 index 00000000000..5e243ca1083 --- /dev/null +++ b/hapi-fhir-server-mdm/src/test/resources/bad-rules-both-paths.json @@ -0,0 +1,19 @@ +{ + "version": "1", + "mdmTypes": ["Patient", "Practitioner", "Medication"], + "candidateSearchParams" : [], + "candidateFilterSearchParams" : [], + "matchFields" : [ { + "name" : "given-name", + "resourceType" : "Patient", + "resourcePath" : "name.first", + "fhirPath" : "name.given.first()", + "matcher" : { + "algorithm": "STRING", + "exact" : true + } + }], + "matchResultMap" : { + "given-name" : "POSSIBLE_MATCH" + } +} diff --git a/hapi-fhir-server-mdm/src/test/resources/bad-rules-no-path.json b/hapi-fhir-server-mdm/src/test/resources/bad-rules-no-path.json new file mode 100644 index 00000000000..1e962048b04 --- /dev/null +++ b/hapi-fhir-server-mdm/src/test/resources/bad-rules-no-path.json @@ -0,0 +1,17 @@ +{ + "version": "1", + "mdmTypes": ["Patient", "Practitioner", "Medication"], + "candidateSearchParams" : [], + "candidateFilterSearchParams" : [], + "matchFields" : [ { + "name" : "given-name", + "resourceType" : "Patient", + "matcher" : { + "algorithm": "STRING", + "exact" : true + } + }], + "matchResultMap" : { + "given-name" : "POSSIBLE_MATCH" + } +} From 04b36d9e0a8960c90292eb7891626f63a8e06aac Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 22 Feb 2021 18:47:34 -0500 Subject: [PATCH 4/8] Readd forgotten file --- .../svc/FhirPathResourceMatcherR4Test.java | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/FhirPathResourceMatcherR4Test.java diff --git a/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/FhirPathResourceMatcherR4Test.java b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/FhirPathResourceMatcherR4Test.java new file mode 100644 index 00000000000..b482fcd71ca --- /dev/null +++ b/hapi-fhir-server-mdm/src/test/java/ca/uhn/fhir/mdm/rules/svc/FhirPathResourceMatcherR4Test.java @@ -0,0 +1,116 @@ +package ca.uhn.fhir.mdm.rules.svc; + +import ca.uhn.fhir.context.RuntimeSearchParam; +import ca.uhn.fhir.mdm.api.MdmMatchOutcome; +import ca.uhn.fhir.mdm.api.MdmMatchResultEnum; +import ca.uhn.fhir.mdm.rules.json.MdmFieldMatchJson; +import ca.uhn.fhir.mdm.rules.json.MdmMatcherJson; +import ca.uhn.fhir.mdm.rules.json.MdmRulesJson; +import ca.uhn.fhir.mdm.rules.matcher.MdmMatcherEnum; +import org.hl7.fhir.r4.model.HumanName; +import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class FhirPathResourceMatcherR4Test extends BaseMdmRulesR4Test { + private static final String MATCH_FIELDS = PATIENT_GIVEN_FIRST + "," + PATIENT_GIVEN; + private Patient myLeft; + private Patient myRight; + + @Override + @BeforeEach + public void before() { + super.before(); + when(mySearchParamRetriever.getActiveSearchParam("Patient", "birthdate")).thenReturn(mock(RuntimeSearchParam.class)); + when(mySearchParamRetriever.getActiveSearchParam("Patient", "identifier")).thenReturn(mock(RuntimeSearchParam.class)); + when(mySearchParamRetriever.getActiveSearchParam("Patient", "active")).thenReturn(mock(RuntimeSearchParam.class)); + + { + myLeft = new Patient(); + HumanName name = myLeft.addName(); + name.addGiven("Gary"); + name.addGiven("John"); + myLeft.setId("Patient/1"); + } + { + myRight = new Patient(); + HumanName name = myRight.addName(); + name.addGiven("John"); + name.addGiven("Gary"); + myRight.setId("Patient/2"); + } + } + + + @Test + public void testFhirPathOrderedMatches() { + MdmResourceMatcherSvc matcherSvc = buildMatcher(buildOrderedGivenNameRules(MdmMatcherEnum.STRING)); + + myLeft = new Patient(); + HumanName name = myLeft.addName(); + name.addGiven("Gary"); + name.addGiven("John"); + myLeft.setId("Patient/1"); + + myRight = new Patient(); + HumanName name2 = myRight.addName(); + name2.addGiven("John"); + name2.addGiven("Gary"); + myRight.setId("Patient/2"); + + MdmMatchOutcome result = matcherSvc.match(myLeft, myRight); + assertMatchResult(MdmMatchResultEnum.NO_MATCH, 0L, 0.0, false, false, result); + + myRight = new Patient(); + name = myRight.addName(); + name.addGiven("John"); + name.addGiven("Gary"); + myRight.setId("Patient/2"); + + myLeft = new Patient(); + name2 = myLeft.addName(); + name2.addGiven("Frank"); + name2.addGiven("Gary"); + myLeft.setId("Patient/1"); + + result = matcherSvc.match(myLeft, myRight); + assertMatchResult(MdmMatchResultEnum.POSSIBLE_MATCH, 1L, 1.0, false, false, result); + + } + + @Test + public void testStringMatchResult() { + MdmResourceMatcherSvc matcherSvc = buildMatcher(buildOrderedGivenNameRules(MdmMatcherEnum.STRING)); + MdmMatchOutcome result = matcherSvc.match(myLeft, myRight); + assertMatchResult(MdmMatchResultEnum.NO_MATCH, 0L, 0.0, false, false, result); + } + + protected MdmRulesJson buildOrderedGivenNameRules(MdmMatcherEnum theMatcherEnum) { + MdmFieldMatchJson firstGivenNameMatchField = new MdmFieldMatchJson() + .setName(PATIENT_GIVEN_FIRST) + .setResourceType("Patient") + .setFhirPath("name.given.first()") + .setMatcher(new MdmMatcherJson().setAlgorithm(theMatcherEnum)); + + MdmFieldMatchJson secondGivenNameMatchField = new MdmFieldMatchJson() + .setName(PATIENT_GIVEN) + .setResourceType("Patient") + .setFhirPath("name.given[1]") + .setMatcher(new MdmMatcherJson().setAlgorithm(theMatcherEnum)); + + MdmRulesJson retval = new MdmRulesJson(); + retval.setVersion("test version"); + retval.addMatchField(secondGivenNameMatchField); + retval.addMatchField(firstGivenNameMatchField); + retval.setMdmTypes(Arrays.asList("Patient")); + retval.putMatchResult(MATCH_FIELDS, MdmMatchResultEnum.MATCH); + retval.putMatchResult(PATIENT_GIVEN_FIRST, MdmMatchResultEnum.POSSIBLE_MATCH); + retval.putMatchResult(PATIENT_GIVEN, MdmMatchResultEnum.POSSIBLE_MATCH); + return retval; + } +} From b108e43600fe8c17f86b638fd9827ab2c2e7f9f1 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 22 Feb 2021 20:49:09 -0500 Subject: [PATCH 5/8] Allow partition date for non-partitionable resources (#2407) * Allow partition date for searchparam resources * Cleanup * Test fix * Documentation update * Docs tweak --- hapi-deployable-pom/pom.xml | 24 --------- .../interceptor/model/RequestPartitionId.java | 12 ++++- .../ca/uhn/fhir/i18n/hapi-messages.properties | 2 +- ...w-partition-date-for-nonpartitionable.yaml | 6 +++ .../server_jpa_partitioning/partitioning.md | 19 +++++++ .../partition/RequestPartitionHelperSvc.java | 44 +++++++++-------- .../PartitioningInterceptorR4Test.java | 49 +++++++++++++++++++ .../provider/r4/MultitenantServerR4Test.java | 15 ++++++ .../fhir/test/utilities/ITestDataBuilder.java | 2 +- pom.xml | 2 +- 10 files changed, 126 insertions(+), 49 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2407-allow-partition-date-for-nonpartitionable.yaml diff --git a/hapi-deployable-pom/pom.xml b/hapi-deployable-pom/pom.xml index d2c17731195..4eb78edfb61 100644 --- a/hapi-deployable-pom/pom.xml +++ b/hapi-deployable-pom/pom.xml @@ -37,31 +37,7 @@ - - - - org.ow2.asm - asm-all - 5.0.4 - - org.basepom.maven diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/model/RequestPartitionId.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/model/RequestPartitionId.java index 410e995d9f5..f8518e3a3f0 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/model/RequestPartitionId.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/model/RequestPartitionId.java @@ -226,6 +226,11 @@ public class RequestPartitionId { return fromPartitionIds(Collections.singletonList(null)); } + @Nonnull + public static RequestPartitionId defaultPartition(@Nullable LocalDate thePartitionDate) { + return fromPartitionIds(Collections.singletonList(null), thePartitionDate); + } + @Nonnull public static RequestPartitionId fromPartitionId(@Nullable Integer thePartitionId) { return fromPartitionIds(Collections.singletonList(thePartitionId)); @@ -238,7 +243,12 @@ public class RequestPartitionId { @Nonnull public static RequestPartitionId fromPartitionIds(@Nonnull Collection thePartitionIds) { - return new RequestPartitionId(null, toListOrNull(thePartitionIds), null); + return fromPartitionIds(thePartitionIds, null); + } + + @Nonnull + public static RequestPartitionId fromPartitionIds(@Nonnull Collection thePartitionIds, @Nullable LocalDate thePartitionDate) { + return new RequestPartitionId(null, toListOrNull(thePartitionIds), thePartitionDate); } @Nonnull diff --git a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties index fdff9a69951..571dd26a636 100644 --- a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties +++ b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties @@ -164,7 +164,7 @@ ca.uhn.fhir.jpa.patch.JsonPatchUtils.failedToApplyPatch=Failed to apply JSON pat ca.uhn.fhir.jpa.graphql.JpaStorageServices.invalidGraphqlArgument=Unknown GraphQL argument "{0}". Value GraphQL argument for this type are: {1} -ca.uhn.fhir.jpa.partition.RequestPartitionHelperSvc.blacklistedResourceTypeForPartitioning=Resource type {0} can not be partitioned +ca.uhn.fhir.jpa.partition.RequestPartitionHelperSvc.nonDefaultPartitionSelectedForNonPartitionable=Resource type {0} can not be partitioned ca.uhn.fhir.jpa.partition.RequestPartitionHelperSvc.unknownPartitionId=Unknown partition ID: {0} ca.uhn.fhir.jpa.partition.RequestPartitionHelperSvc.unknownPartitionName=Unknown partition name: {0} diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2407-allow-partition-date-for-nonpartitionable.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2407-allow-partition-date-for-nonpartitionable.yaml new file mode 100644 index 00000000000..b8942a6f558 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2407-allow-partition-date-for-nonpartitionable.yaml @@ -0,0 +1,6 @@ +--- +type: add +issue: 2407 +title: "When using the JPA server in partitioned mode with a partition interceptor, the interceptor is now called even for + resource types that can not be placed in a non-default partition (e.g. SearchParameter, CodeSystem, etc.). The interceptor + may return null or default in this case, but can include a non-null partition date if needed." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_partitioning/partitioning.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_partitioning/partitioning.md index b7806c5713b..2bb299f0bcd 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_partitioning/partitioning.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_partitioning/partitioning.md @@ -82,6 +82,25 @@ A hook against the [`Pointcut.STORAGE_PARTITION_IDENTIFY_READ`](/hapi-fhir/apido As of HAPI FHIR 5.3.0, the *Identify Partition for Read* hook method may return multiple partition names or IDs. If more than one partition is identified, the server will search in all identified partitions. +## Non-Partitionable Resources + +Some resource types can not be placed in any partition other than the DEFAULT partition. When a resource of one of these types is being created, the *STORAGE_PARTITION_IDENTIFY_CREATE* pointcut is invoked, but the hook method must return [defaultPartition()](https://hapifhir.io/hapi-fhir/apidocs/hapi-fhir-base/ca/uhn/fhir/interceptor/model/RequestPartitionId.html#defaultPartition()). A partition date may optionally be included. + +The following resource types may not be placed in any partition except the default partition: + +* CapabilityStatement +* CodeSystem +* CompartmentDefinition +* ConceptMap +* NamingSystem +* OperationDefinition +* Questionnaire +* SearchParameter +* StructureDefinition +* StructureMap +* Subscription +* ValueSet + ## Examples See [Partition Interceptor Examples](./partition_interceptor_examples.html) for various samples of how partitioning interceptors can be set up. diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java index ebd15af2a7e..37da051117a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java @@ -50,7 +50,7 @@ import static ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster.hasHooks; public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { - private final HashSet myPartitioningBlacklist; + private final HashSet myNonPartitionableResourceNames; @Autowired private IInterceptorBroadcaster myInterceptorBroadcaster; @@ -62,25 +62,25 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { private PartitionSettings myPartitionSettings; public RequestPartitionHelperSvc() { - myPartitioningBlacklist = new HashSet<>(); + myNonPartitionableResourceNames = new HashSet<>(); // Infrastructure - myPartitioningBlacklist.add("Subscription"); - myPartitioningBlacklist.add("SearchParameter"); + myNonPartitionableResourceNames.add("Subscription"); + myNonPartitionableResourceNames.add("SearchParameter"); // Validation and Conformance - myPartitioningBlacklist.add("StructureDefinition"); - myPartitioningBlacklist.add("Questionnaire"); - myPartitioningBlacklist.add("CapabilityStatement"); - myPartitioningBlacklist.add("CompartmentDefinition"); - myPartitioningBlacklist.add("OperationDefinition"); + myNonPartitionableResourceNames.add("StructureDefinition"); + myNonPartitionableResourceNames.add("Questionnaire"); + myNonPartitionableResourceNames.add("CapabilityStatement"); + myNonPartitionableResourceNames.add("CompartmentDefinition"); + myNonPartitionableResourceNames.add("OperationDefinition"); // Terminology - myPartitioningBlacklist.add("ConceptMap"); - myPartitioningBlacklist.add("CodeSystem"); - myPartitioningBlacklist.add("ValueSet"); - myPartitioningBlacklist.add("NamingSystem"); - myPartitioningBlacklist.add("StructureMap"); + myNonPartitionableResourceNames.add("ConceptMap"); + myNonPartitionableResourceNames.add("CodeSystem"); + myNonPartitionableResourceNames.add("ValueSet"); + myNonPartitionableResourceNames.add("NamingSystem"); + myNonPartitionableResourceNames.add("StructureMap"); } @@ -97,7 +97,7 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { if (myPartitionSettings.isPartitioningEnabled()) { // Handle system requests - if ((theRequest == null && myPartitioningBlacklist.contains(theResourceType))) { + if ((theRequest == null && myNonPartitionableResourceNames.contains(theResourceType))) { return RequestPartitionId.defaultPartition(); } @@ -128,10 +128,6 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { RequestPartitionId requestPartitionId; if (myPartitionSettings.isPartitioningEnabled()) { - // Handle system requests - if ((theRequest == null && myPartitioningBlacklist.contains(theResourceType))) { - return RequestPartitionId.defaultPartition(); - } // Interceptor call: STORAGE_PARTITION_IDENTIFY_CREATE HookParams params = new HookParams() @@ -140,6 +136,12 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { .addIfMatchesType(ServletRequestDetails.class, theRequest); requestPartitionId = (RequestPartitionId) doCallHooksAndReturnObject(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PARTITION_IDENTIFY_CREATE, params); + // Handle system requests + boolean nonPartitionableResource = myNonPartitionableResourceNames.contains(theResourceType); + if (nonPartitionableResource && requestPartitionId == null) { + requestPartitionId = RequestPartitionId.defaultPartition(); + } + String resourceName = myFhirContext.getResourceType(theResource); validateSinglePartitionForCreate(requestPartitionId, resourceName, Pointcut.STORAGE_PARTITION_IDENTIFY_CREATE); @@ -271,8 +273,8 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { if ((theRequestPartitionId.hasPartitionIds() && !theRequestPartitionId.getPartitionIds().contains(null)) || (theRequestPartitionId.hasPartitionNames() && !theRequestPartitionId.getPartitionNames().contains(JpaConstants.DEFAULT_PARTITION_NAME))) { - if (myPartitioningBlacklist.contains(theResourceName)) { - String msg = myFhirContext.getLocalizer().getMessageSanitized(RequestPartitionHelperSvc.class, "blacklistedResourceTypeForPartitioning", theResourceName); + if (myNonPartitionableResourceNames.contains(theResourceName)) { + String msg = myFhirContext.getLocalizer().getMessageSanitized(RequestPartitionHelperSvc.class, "nonDefaultPartitionSelectedForNonPartitionable", theResourceName); throw new UnprocessableEntityException(msg); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/PartitioningInterceptorR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/PartitioningInterceptorR4Test.java index 156f2528082..b2fe10e4e25 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/PartitioningInterceptorR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/PartitioningInterceptorR4Test.java @@ -10,10 +10,13 @@ import ca.uhn.fhir.jpa.entity.PartitionEntity; import ca.uhn.fhir.jpa.interceptor.ex.PartitionInterceptorReadAllPartitions; import ca.uhn.fhir.jpa.interceptor.ex.PartitionInterceptorReadPartitionsBasedOnScopes; import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.partition.IPartitionLookupSvc; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import com.google.common.collect.Sets; import org.apache.commons.lang3.StringUtils; @@ -21,6 +24,7 @@ import org.apache.commons.lang3.Validate; import org.hamcrest.Matchers; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.StructureDefinition; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -82,6 +86,51 @@ public class PartitioningInterceptorR4Test extends BaseJpaR4SystemTest { } + @Test + public void testCreateNonPartionableResourceWithPartitionDate() { + myPartitionInterceptor.addCreatePartition(RequestPartitionId.defaultPartition(LocalDate.of(2021, 2, 22))); + + StructureDefinition sd = new StructureDefinition(); + sd.setUrl("http://foo"); + myStructureDefinitionDao.create(sd); + + runInTransaction(()->{ + List resources = myResourceTableDao.findAll(); + assertEquals(1, resources.size()); + assertEquals(null, resources.get(0).getPartitionId().getPartitionId()); + assertEquals(22, resources.get(0).getPartitionId().getPartitionDate().getDayOfMonth()); + }); + } + + @Test + public void testCreateNonPartionableResourceWithNullPartitionReturned() { + myPartitionInterceptor.addCreatePartition(null); + + StructureDefinition sd = new StructureDefinition(); + sd.setUrl("http://foo"); + myStructureDefinitionDao.create(sd); + + runInTransaction(()->{ + List resources = myResourceTableDao.findAll(); + assertEquals(1, resources.size()); + assertEquals(null, resources.get(0).getPartitionId()); + }); + } + + @Test + public void testCreateNonPartionableResourceWithDisallowedPartitionReturned() { + myPartitionInterceptor.addCreatePartition(RequestPartitionId.fromPartitionName("FOO")); + + StructureDefinition sd = new StructureDefinition(); + sd.setUrl("http://foo"); + try { + myStructureDefinitionDao.create(sd); + fail(); + } catch (UnprocessableEntityException e) { + assertEquals("Resource type StructureDefinition can not be partitioned", e.getMessage()); + } + } + /** * Should fail if no interceptor is registered for the READ pointcut */ diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java index 2e33c901ee0..94289a2c532 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java @@ -118,6 +118,21 @@ public class MultitenantServerR4Test extends BaseMultitenantResourceProviderR4Te } + @Test + public void testCreateAndRead_NonPartitionableResource_DefaultTenant() { + + // Create patients + + IIdType idA = createResource("NamingSystem", withTenant(JpaConstants.DEFAULT_PARTITION_NAME), withStatus("draft")); + + runInTransaction(() -> { + ResourceTable resourceTable = myResourceTableDao.findById(idA.getIdPartAsLong()).orElseThrow(() -> new IllegalStateException()); + assertNull(resourceTable.getPartitionId()); + }); + + } + + @Test public void testCreate_InvalidTenant() { diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/ITestDataBuilder.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/ITestDataBuilder.java index a5daef31adb..ee503727c3e 100644 --- a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/ITestDataBuilder.java +++ b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/ITestDataBuilder.java @@ -144,7 +144,7 @@ public interface ITestDataBuilder { return createResource("Organization", theModifiers); } - default IIdType createResource(String theResourceType, Consumer[] theModifiers) { + default IIdType createResource(String theResourceType, Consumer... theModifiers) { IBaseResource resource = buildResource(theResourceType, theModifiers); if (isNotBlank(resource.getIdElement().getValue())) { diff --git a/pom.xml b/pom.xml index 6a0ec668486..e65670c96e1 100644 --- a/pom.xml +++ b/pom.xml @@ -1958,7 +1958,7 @@ org.codehaus.mojo animal-sniffer-maven-plugin - 1.19 + 1.20 org.codehaus.mojo From 8c4d522f11373d37252312adc39e24f7862a64cf Mon Sep 17 00:00:00 2001 From: hdconradi Date: Tue, 23 Feb 2021 19:46:10 +0100 Subject: [PATCH 6/8] Remove the trailing ,' from some of the schemata (#2413) Co-authored-by: Heinz-Dieter Conradi --- .../org/hl7/fhir/r4/model/profile/profiles-others.xml | 2 +- .../org/hl7/fhir/r4/model/profile/profiles-resources.xml | 4 ++-- .../org/hl7/fhir/r4/model/schema/cqf-questionnaire.sch | 2 +- .../org/hl7/fhir/r4/model/schema/cqif-questionnaire.sch | 2 +- .../org/hl7/fhir/r4/model/schema/fhir-invariants.sch | 2 +- .../resources/org/hl7/fhir/r4/model/schema/questionnaire.sch | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/profile/profiles-others.xml b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/profile/profiles-others.xml index 3ae8cf7c17f..34dafbc4188 100644 --- a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/profile/profiles-others.xml +++ b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/profile/profiles-others.xml @@ -16389,7 +16389,7 @@ - + diff --git a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/profile/profiles-resources.xml b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/profile/profiles-resources.xml index 28dd40efcad..9e7978121f9 100644 --- a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/profile/profiles-resources.xml +++ b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/profile/profiles-resources.xml @@ -285286,7 +285286,7 @@ - + @@ -287200,7 +287200,7 @@ - + diff --git a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/cqf-questionnaire.sch b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/cqf-questionnaire.sch index 24583657185..13de9a27be9 100644 --- a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/cqf-questionnaire.sch +++ b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/cqf-questionnaire.sch @@ -35,7 +35,7 @@ Read-only can't be specified for "display" items (inherited) Initial values can't be specified for groups or display items (inherited) Required and repeat aren't permitted for display items (inherited) - Only 'choice' and 'open-choice' items can have answerValueSet (inherited) + Only 'choice' and 'open-choice' items can have answerValueSet (inherited) A question cannot have both answerOption and answerValueSet (inherited) Display items cannot have a "code" asserted (inherited) Maximum length can only be declared for simple question types (inherited) diff --git a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/cqif-questionnaire.sch b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/cqif-questionnaire.sch index 66aeeb498f0..92ed60ffee1 100644 --- a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/cqif-questionnaire.sch +++ b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/cqif-questionnaire.sch @@ -41,7 +41,7 @@ Read-only can't be specified for "display" items (inherited) Initial values can't be specified for groups or display items (inherited) Required and repeat aren't permitted for display items (inherited) - Only 'choice' items can have answerValueSet (inherited) + Only 'choice' items can have answerValueSet (inherited) A question cannot have both answerOption and answerValueSet (inherited) Display items cannot have a "code" asserted (inherited) Maximum length can only be declared for simple question types (inherited) diff --git a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/fhir-invariants.sch b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/fhir-invariants.sch index 05a2fb327f4..d1f9d1f5bcd 100644 --- a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/fhir-invariants.sch +++ b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/fhir-invariants.sch @@ -10667,7 +10667,7 @@ que-9: Read-only can't be specified for "display" items que-8: Initial values can't be specified for groups or display items que-6: Required and repeat aren't permitted for display items - que-5: Only 'choice' and 'open-choice' items can have answerValueSet + que-5: Only 'choice' and 'open-choice' items can have answerValueSet que-4: A question cannot have both answerOption and answerValueSet que-3: Display items cannot have a "code" asserted que-10: Maximum length can only be declared for simple question types diff --git a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/questionnaire.sch b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/questionnaire.sch index d17fdc08d90..e2b2c91a595 100644 --- a/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/questionnaire.sch +++ b/hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/schema/questionnaire.sch @@ -78,7 +78,7 @@ que-9: Read-only can't be specified for "display" items que-8: Initial values can't be specified for groups or display items que-6: Required and repeat aren't permitted for display items - que-5: Only 'choice' and 'open-choice' items can have answerValueSet + que-5: Only 'choice' and 'open-choice' items can have answerValueSet que-4: A question cannot have both answerOption and answerValueSet que-3: Display items cannot have a "code" asserted que-10: Maximum length can only be declared for simple question types From b2b046215437c108f1b4545c40cabe119bf1c4f9 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 23 Feb 2021 18:04:53 -0500 Subject: [PATCH 7/8] Skip loading a FHIRPath object if we are older than STU3 (#2416) * Skip loading a FHIRPath object if we are older than STU3 * Change log level --- .../fhir/mdm/rules/config/MdmRuleValidator.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidator.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidator.java index 959058cadbd..27a84c92d87 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidator.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/rules/config/MdmRuleValidator.java @@ -22,6 +22,7 @@ package ca.uhn.fhir.mdm.rules.config; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.fhirpath.IFhirPath; import ca.uhn.fhir.mdm.api.MdmConstants; @@ -61,7 +62,12 @@ public class MdmRuleValidator implements IMdmRuleValidator { public MdmRuleValidator(FhirContext theFhirContext, ISearchParamRetriever theSearchParamRetriever) { myFhirContext = theFhirContext; myTerser = myFhirContext.newTerser(); - myFhirPath = myFhirContext.newFhirPath(); + if (myFhirContext.getVersion().getVersion().isEqualOrNewerThan(FhirVersionEnum.DSTU3)) { + myFhirPath = myFhirContext.newFhirPath(); + } else { + ourLog.debug("Skipping FHIRPath validation as DSTU2 does not support FHIR"); + myFhirPath = null; + } mySearchParamRetriever = theSearchParamRetriever; } @@ -193,7 +199,11 @@ public class MdmRuleValidator implements IMdmRuleValidator { } } else { //Try to validate the FHIRPath try { - myFhirPath.parse(theResourceType + "." + theFieldMatch.getFhirPath()); + if (myFhirPath != null) { + myFhirPath.parse(theResourceType + "." + theFieldMatch.getFhirPath()); + } else { + ourLog.debug("Can't validate FHIRPath expression due to a lack of IFhirPath object."); + } } catch (Exception e) { throw new ConfigurationException("MatchField [" + theFieldMatch.getName() + "] resourceType [" + theFieldMatch.getResourceType() + "] has failed FHIRPath evaluation. " + e.getMessage()); } From 8f474d11b8b82dd6168ad6196a13cf43821b170d Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Wed, 24 Feb 2021 05:55:55 -0500 Subject: [PATCH 8/8] Disable inline match URLs on public server --- .../src/main/java/ca/uhn/fhirtest/config/TestR4Config.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestR4Config.java b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestR4Config.java index dc20bd902c7..db7f0b7192d 100644 --- a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestR4Config.java +++ b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestR4Config.java @@ -66,7 +66,7 @@ public class TestR4Config extends BaseJavaConfigR4 { retVal.setWebsocketContextPath("/websocketR4"); retVal.setAllowContainsSearches(true); retVal.setAllowMultipleDelete(true); - retVal.setAllowInlineMatchUrlReferences(true); + retVal.setAllowInlineMatchUrlReferences(false); retVal.setAllowExternalReferences(true); retVal.getTreatBaseUrlsAsLocal().add("http://hapi.fhir.org/baseR4"); retVal.getTreatBaseUrlsAsLocal().add("https://hapi.fhir.org/baseR4");