5259 sorted tags, security labels and profiles (#5260)

* 5259 sorted tags, security labels and profiles

* fixed typos and augmented a test case for sorter
This commit is contained in:
Emre Dincturk 2023-09-01 14:12:26 -04:00 committed by GitHub
parent 33be707238
commit 862c0487f6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 858 additions and 3 deletions

View File

@ -0,0 +1,7 @@
---
type: change
issue: 5229
title: "Previously, when using INLINE tag storage mode, a superfluous version of a resource would be created as a result
of an update request which didn't have a real logical change to the resource but only changed the order of existing
items in tag, security label or profile collections. This change prevents this behaviour. Also on resource retrieval,
these meta collections are sorted alphabetically, based on (security, code) pair for tags and security labels."

View File

@ -7,3 +7,20 @@ We have replaced the synchronous mechanism with a two stage process. Events are
database upon completion of the transaction and subsequently submitted to the broker by a scheduled task. database upon completion of the transaction and subsequently submitted to the broker by a scheduled task.
This new asynchronous submission mechanism will introduce a slight delay in event publishing. It is our view that such This new asynchronous submission mechanism will introduce a slight delay in event publishing. It is our view that such
delay is largely compensated by the capability to retry submission upon failure which will eliminate event losses. delay is largely compensated by the capability to retry submission upon failure which will eliminate event losses.
There are some potentially breaking changes:
* On resource retrieval and before storage, tags, security label and profile collections in resource meta will be
sorted in lexicographical order. The order of the elements for Coding types (i.e. tags and security labels) is defined
by the (security, code) pair of each element. This normally should not break any clients because these properties are
sets according to the FHIR specification, and hence the order of the elements in these collections should not matter.
Also with this change the following side effects can be observed:
- If using INLINE tag storage mode, the first update request to a resource which has tags, security
labels or profiles could create a superfluous resource version if the update request does not really introduce any
change to the resource. This is because the persisted tags, security labels, and profile may not be sorted in
lexicographical order, and this would be interpreted as a new resource version since the tags would be sorted
before storage after this change. If the update request actually changes the resource, there is no concern here.
Also, subsequent updates will not create an additional version because of ordering of the meta properties anymore.
- These meta collections are sorted in place by the storage layer before persisting the resource, so any piece of
code that is calling storage layer directly should not be passing in unmodifiable collections, as it would
result in an error.

View File

@ -184,6 +184,8 @@ import ca.uhn.fhir.rest.server.interceptor.consent.IConsentContextServices;
import ca.uhn.fhir.rest.server.interceptor.partition.RequestTenantPartitionInterceptor; import ca.uhn.fhir.rest.server.interceptor.partition.RequestTenantPartitionInterceptor;
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
import ca.uhn.fhir.subscription.api.IResourceModifiedMessagePersistenceSvc; import ca.uhn.fhir.subscription.api.IResourceModifiedMessagePersistenceSvc;
import ca.uhn.fhir.util.IMetaTagSorter;
import ca.uhn.fhir.util.MetaTagSorterAlphabetical;
import ca.uhn.hapi.converters.canonical.VersionCanonicalizer; import ca.uhn.hapi.converters.canonical.VersionCanonicalizer;
import org.hl7.fhir.common.hapi.validation.support.UnknownCodeSystemWarningValidationSupport; import org.hl7.fhir.common.hapi.validation.support.UnknownCodeSystemWarningValidationSupport;
import org.hl7.fhir.utilities.graphql.IGraphQLStorageServices; import org.hl7.fhir.utilities.graphql.IGraphQLStorageServices;
@ -904,4 +906,9 @@ public class JpaConfig {
return new ResourceModifiedMessagePersistenceSvcImpl( return new ResourceModifiedMessagePersistenceSvcImpl(
theFhirContext, theIResourceModifiedDao, theDaoRegistry, theHapiTransactionService); theFhirContext, theIResourceModifiedDao, theDaoRegistry, theHapiTransactionService);
} }
@Bean
public IMetaTagSorter metaTagSorter() {
return new MetaTagSorterAlphabetical();
}
} }

View File

@ -2087,6 +2087,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
break; break;
} }
} }
myMetaTagSorter.sort(retVal);
return retVal; return retVal;
} }

View File

@ -54,6 +54,7 @@ import ca.uhn.fhir.parser.DataFormatException;
import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.parser.IParser;
import ca.uhn.fhir.parser.LenientErrorHandler; import ca.uhn.fhir.parser.LenientErrorHandler;
import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.util.IMetaTagSorter;
import ca.uhn.fhir.util.MetaUtil; import ca.uhn.fhir.util.MetaUtil;
import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.Validate;
import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IAnyResource;
@ -98,6 +99,9 @@ public class JpaStorageResourceParser implements IJpaStorageResourceParser {
@Autowired @Autowired
private ExternallyStoredResourceServiceRegistry myExternallyStoredResourceServiceRegistry; private ExternallyStoredResourceServiceRegistry myExternallyStoredResourceServiceRegistry;
@Autowired
IMetaTagSorter myMetaTagSorter;
@Override @Override
public IBaseResource toResource(IBasePersistedResource theEntity, boolean theForHistoryOperation) { public IBaseResource toResource(IBasePersistedResource theEntity, boolean theForHistoryOperation) {
RuntimeResourceDefinition type = myFhirContext.getResourceDefinition(theEntity.getResourceType()); RuntimeResourceDefinition type = myFhirContext.getResourceDefinition(theEntity.getResourceType());
@ -229,6 +233,9 @@ public class JpaStorageResourceParser implements IJpaStorageResourceParser {
// 7. Add partition information // 7. Add partition information
populateResourcePartitionInformation(theEntity, retVal); populateResourcePartitionInformation(theEntity, retVal);
// 8. sort tags, security labels and profiles
myMetaTagSorter.sort(retVal.getMeta());
return retVal; return retVal;
} }

View File

@ -0,0 +1,100 @@
package ca.uhn.fhir.jpa.dao.r4;
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
import ca.uhn.fhir.storage.test.TagTestCasesUtil;
import org.hl7.fhir.r4.model.Meta;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import java.util.List;
import static ca.uhn.fhir.test.utilities.TagTestUtil.createMeta;
import static ca.uhn.fhir.test.utilities.TagTestUtil.generateAllCodingPairs;
public class FhirResourceDaoR4TagsOrderTest extends BaseJpaR4Test {
private TagTestCasesUtil myTagTestCasesUtil;
@Override
@BeforeEach
protected void before() throws Exception {
super.before();
myTagTestCasesUtil = new TagTestCasesUtil(myPatientDao, mySystemDao, mySrd, true);
}
@ParameterizedTest
@EnumSource(JpaStorageSettings.TagStorageModeEnum.class)
public void testCreateResource_ExpectToRetrieveTagsSorted(JpaStorageSettings.TagStorageModeEnum theTagStorageMode) {
myStorageSettings.setTagStorageMode(theTagStorageMode);
// TODO: In inline mode, $meta endpoint doesn't return tags, see https://github.com/hapifhir/hapi-fhir/issues/5206
// When this issue is fixed, the following line could be removed so that we check $meta for Inline mode as well
myTagTestCasesUtil.setMetaOperationSupported(theTagStorageMode != JpaStorageSettings.TagStorageModeEnum.INLINE);
myTagTestCasesUtil.createResourceWithTagsAndExpectToRetrieveThemSorted();
}
@ParameterizedTest
@EnumSource(
// running this test for tag storage modes other than INLINE mode, since INLINE mode replaces the tags and security labels
// on update rather than adding them to the existing set. The INLINE mode has its own test below.
value = JpaStorageSettings.TagStorageModeEnum.class,
names = {"INLINE"},
mode = EnumSource.Mode.EXCLUDE)
public void testUpdateResource_ShouldNotIncreaseVersionBecauseOfTagOrder_NonInlineModes(JpaStorageSettings.TagStorageModeEnum theTagStorageMode) {
myStorageSettings.setTagStorageMode(theTagStorageMode);
myTagTestCasesUtil.updateResourceWithExistingTagsButInDifferentOrderAndExpectVersionToRemainTheSame_NonInlineModes();
}
@Test
public void testUpdateResource_ShouldNotIncreaseVersionBecauseOfTagOrder_InlineMode() {
myStorageSettings.setTagStorageMode(JpaStorageSettings.TagStorageModeEnum.INLINE);
myTagTestCasesUtil.updateResourceWithExistingTagsButInDifferentOrderAndExpectVersionToRemainTheSame_InlineMode();
}
@ParameterizedTest
@EnumSource(
// running this test for tag storage modes other than INLINE mode, since INLINE mode replaces the tags and security labels
// on update rather than adding them to the existing set. The INLINE mode has its own test below.
value = JpaStorageSettings.TagStorageModeEnum.class,
names = {"INLINE"},
mode = EnumSource.Mode.EXCLUDE)
public void testUpdateResource_ExpectToRetrieveTagsSorted_NonInlineModes(JpaStorageSettings.TagStorageModeEnum theTagStorageMode) {
myStorageSettings.setTagStorageMode(theTagStorageMode);
myTagTestCasesUtil.updateResourceWithTagsAndExpectToRetrieveTagsSorted_NonInlineModes();
}
@Test
public void testUpdateResource_ExpectToRetrieveTagsSorted_InlineMode() {
myStorageSettings.setTagStorageMode(JpaStorageSettings.TagStorageModeEnum.INLINE);
// TODO: In inline mode, $meta endpoint doesn't return tags, see https://github.com/hapifhir/hapi-fhir/issues/5206
// When this issue is fixed, the following line could be removed so that we check $meta for Inline mode as well
myTagTestCasesUtil.setMetaOperationSupported(false);
Meta metaInputOnCreate = createMeta(
// generateAllCodingPairs creates a list that has 6 codings in this case in this order:
// (sys2, c), (sys2, b), (sys2, a), (sys1, c), (sys1, b), (sys1, a)
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //security
List.of("c", "b", "a") // profile
);
// meta input for update (adding new tags)
Meta metaInputOnUpdate = createMeta(
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("cc", "bb", "aa")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("cc", "bb", "aa")), //security
List.of("cc", "bb", "aa") //profile
);
// inline mode replaces the tags completely on update, so only new tags are expected after update
Meta expectedMetaAfterUpdate = createMeta(
generateAllCodingPairs(List.of("sys1", "sys2"), List.of("aa", "bb", "cc")), //tag (replaced & sorted)
generateAllCodingPairs(List.of("sys1", "sys2"), List.of("aa", "bb", "cc")), //security (replaced & sorted)
List.of("aa", "bb", "cc") //profile (replaced & sorted)
);
myTagTestCasesUtil.updateResourceAndVerifyMeta(metaInputOnCreate, metaInputOnUpdate, expectedMetaAfterUpdate, false);
}
}

View File

@ -43,6 +43,8 @@ import ca.uhn.fhir.jpa.sp.SearchParamPresenceSvcImpl;
import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import ca.uhn.fhir.util.ClasspathUtil; import ca.uhn.fhir.util.ClasspathUtil;
import ca.uhn.fhir.util.IMetaTagSorter;
import ca.uhn.fhir.util.MetaTagSorterAlphabetical;
import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.StopWatch;
import ca.uhn.fhir.validation.IInstanceValidatorModule; import ca.uhn.fhir.validation.IInstanceValidatorModule;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
@ -149,6 +151,7 @@ public class GiantTransactionPerfTest {
private IIdHelperService myIdHelperService; private IIdHelperService myIdHelperService;
@Mock @Mock
private IJpaStorageResourceParser myJpaStorageResourceParser; private IJpaStorageResourceParser myJpaStorageResourceParser;
private IMetaTagSorter myMetaTagSorter;
@AfterEach @AfterEach
public void afterEach() { public void afterEach() {
@ -175,6 +178,8 @@ public class GiantTransactionPerfTest {
myPartitionSettings = new PartitionSettings(); myPartitionSettings = new PartitionSettings();
myMetaTagSorter = new MetaTagSorterAlphabetical();
myHapiTransactionService = new HapiTransactionService(); myHapiTransactionService = new HapiTransactionService();
myHapiTransactionService.setTransactionManager(myTransactionManager); myHapiTransactionService.setTransactionManager(myTransactionManager);
myHapiTransactionService.setInterceptorBroadcaster(myInterceptorSvc); myHapiTransactionService.setInterceptorBroadcaster(myInterceptorSvc);
@ -267,6 +272,7 @@ public class GiantTransactionPerfTest {
myEobDao.setPartitionSettingsForUnitTest(myPartitionSettings); myEobDao.setPartitionSettingsForUnitTest(myPartitionSettings);
myEobDao.setJpaStorageResourceParserForUnitTest(myJpaStorageResourceParser); myEobDao.setJpaStorageResourceParserForUnitTest(myJpaStorageResourceParser);
myEobDao.setExternallyStoredResourceServiceRegistryForUnitTest(new ExternallyStoredResourceServiceRegistry()); myEobDao.setExternallyStoredResourceServiceRegistryForUnitTest(new ExternallyStoredResourceServiceRegistry());
myEobDao.setMyMetaTagSorter(myMetaTagSorter);
myEobDao.start(); myEobDao.start();
myDaoRegistry.setResourceDaos(Lists.newArrayList(myEobDao)); myDaoRegistry.setResourceDaos(Lists.newArrayList(myEobDao));

View File

@ -0,0 +1,316 @@
/*-
* #%L
* hapi-fhir-storage-test-utilities
* %%
* Copyright (C) 2014 - 2023 Smile CDR, Inc.
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package ca.uhn.fhir.storage.test;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
import ca.uhn.fhir.jpa.api.dao.IFhirSystemDao;
import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import org.hl7.fhir.instance.model.api.IBaseMetaType;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Meta;
import org.hl7.fhir.r4.model.Patient;
import java.util.List;
import static ca.uhn.fhir.test.utilities.TagTestUtil.assertCodingsEqualAndInOrder;
import static ca.uhn.fhir.test.utilities.TagTestUtil.createMeta;
import static ca.uhn.fhir.test.utilities.TagTestUtil.generateAllCodingPairs;
import static ca.uhn.fhir.test.utilities.TagTestUtil.toStringList;
import static org.junit.jupiter.api.Assertions.assertEquals;
/**
* Contains some test case helper functions for testing the storage of meta properties: tag, security and profile
*/
public class TagTestCasesUtil {
private IFhirResourceDao<Patient> myPatientDao;
private IFhirSystemDao<Bundle, Meta> mySystemDao;
private RequestDetails myRequestDetails;
private boolean myMetaOperationSupported;
public TagTestCasesUtil(IFhirResourceDao<Patient> thePatientDao, IFhirSystemDao<Bundle, Meta> theSystemDao, RequestDetails theRequestDetails, boolean theMetaOperationSupported) {
this.myPatientDao = thePatientDao;
this.mySystemDao = theSystemDao;
this.myRequestDetails = theRequestDetails;
this.myMetaOperationSupported = theMetaOperationSupported;
}
/**
* Creates a resource with the given Meta and reads the resource back and asserts that the resource
* has the specified meta properties for tag, security and profile
*/
public IBaseResource createResourceAndVerifyMeta (Meta theMetaInputOnCreate, Meta theExpectedMetaAfterCreate){
DaoMethodOutcome createOutcome = createPatient(theMetaInputOnCreate);
IIdType versionlessPatientId = createOutcome.getResource().getIdElement().toVersionless();
Patient patient = myPatientDao.read(versionlessPatientId, myRequestDetails);
verifyMeta(theExpectedMetaAfterCreate, patient.getMeta());
if (myMetaOperationSupported) {
//test meta get operation with a specific id
Meta meta = myPatientDao.metaGetOperation(Meta.class, versionlessPatientId, myRequestDetails);
verifyMeta(theExpectedMetaAfterCreate, meta);
//test meta get operation for the resource type (without specific id)
//note: this, and the following system level metaGet operation, assume that the tags created
//by this function are the only tags in the system, which is true for all test cases that use this function
//currently, but if changes these checks could be relaxed
//to check for contains in order rather than equality of the tag lists, or metaGet operations that aren't
//specific to a particular resource id could be separated into its own test function
meta = myPatientDao.metaGetOperation(Meta.class, myRequestDetails);
verifyMeta(theExpectedMetaAfterCreate, meta);
//test meta operation for system
meta = mySystemDao.metaGetOperation(myRequestDetails);
verifyMeta(theExpectedMetaAfterCreate, meta);
}
//ensure version endpoint also returns tags as expected
IIdType versionId = new IdType(String.format("%s/_history/1", patient.getIdElement().toVersionless()));
patient = myPatientDao.read(versionId, myRequestDetails);
verifyMeta(theExpectedMetaAfterCreate, patient.getMeta());
return patient;
}
/**
* Creates a resource with the given meta properties, then updates the resource with the specified meta properties, then
* reads the resource back and asserts that the resource has the specified properties for tag, security and profile
*/
public IBaseResource updateResourceAndVerifyMeta(Meta theMetaInputOnCreate, Meta theMetaInputOnUpdate, Meta theExpectedMetaAfterUpdate, boolean theExpectNop) {
DaoMethodOutcome createOutcome = createPatient(theMetaInputOnCreate);
IIdType versionlessPatientId = createOutcome.getId().toVersionless();
DaoMethodOutcome updateOutcome = updatePatient(versionlessPatientId, theMetaInputOnUpdate);
assertEquals(theExpectNop, updateOutcome.isNop());
Patient patient = myPatientDao.read(versionlessPatientId, myRequestDetails);
verifyMeta(theExpectedMetaAfterUpdate, patient.getMeta());
if (myMetaOperationSupported) {
Meta meta = myPatientDao.metaGetOperation(Meta.class, versionlessPatientId, myRequestDetails);
verifyMeta(theExpectedMetaAfterUpdate, meta);
}
//ensure version endpoint also returns tags as expected
IIdType versionId = new IdType(String.format("%s/_history/2", patient.getIdElement().toVersionless()));
patient = myPatientDao.read(versionId, myRequestDetails);
verifyMeta(theExpectedMetaAfterUpdate, patient.getMeta());
return patient;
}
public IBaseResource updateResourceAndVerifyVersion(IIdType theResourceId, Meta theMetaInputOnUpdate, String theExpectedVersion) {
updatePatient(theResourceId, theMetaInputOnUpdate);
Patient patient = myPatientDao.read(theResourceId, myRequestDetails);
assertEquals(theExpectedVersion, patient.getMeta().getVersionId());
return patient;
}
/**
* Verifies that tag order doesn't cause a version change for non-inline modes, for which the update behavior is to
* take the union of existing and new tags.
* The verification consists of 3 parts:
* - Part 1: Create a resource with tags and update the resource with same tags in different order, expect version
* to remain at 1.
* - Part 2: Update the resource with a different set of tags, which would add the new set to the existing set and
* increment the version to 2. Then update the resource again with the all the tags the resource current has but in
* different order, and expect the version to remain at 2. This part ensures that the storage is able to determine
* whether the version should be incremented or not after new tags are added to a resource with
* subsequent updates (as opposed to adding tags during resource creation which Part 1 verifies).
* - Part 3: Update the resource with a subset of the tags it currently has but in a different order and expect
* the version to remain the same.
*/
public void updateResourceWithExistingTagsButInDifferentOrderAndExpectVersionToRemainTheSame_NonInlineModes(){
// Part 1: Create with tags
Meta metaInputOnCreate = createMeta(
// generateAllCodingPairs creates a list that has 6 codings in this case in this order:
// (sys2, c), (sys2, b), (sys2, a), (sys1, c), (sys1, b), (sys1, a)
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //security
List.of("c", "b", "a") // profile
);
DaoMethodOutcome createOutcome = createPatient(metaInputOnCreate);
IIdType versionlessPatientId = createOutcome.getId().toVersionless();
// use the same input on update as the creation but order everything differently
Meta metaInputOnUpdate = createMeta(
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("b", "c", "a")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("b", "c", "a")), //security
List.of("b", "c", "a") // profile
);
//update and assert version remains the same (1)
updateResourceAndVerifyVersion(versionlessPatientId, metaInputOnUpdate, "1");
// Part 2: update the resource with a completely different set of tags, which will be added to the existing
// set by the storage, the resource will have all of a,b,c,aa,bb,cc as tags after the update
metaInputOnUpdate = createMeta(
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("aa", "bb", "cc")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("aa", "bb", "cc")), //security
List.of("b", "c", "a") // profile
);
// expect the version to be incremented
updateResourceAndVerifyVersion(versionlessPatientId, metaInputOnUpdate, "2");
// update with all tags the resource has in different order
metaInputOnUpdate = createMeta(
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "bb", "aa", "b", "cc", "c")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "bb", "aa", "b", "cc", "c")), //security
List.of("b", "c", "a") // profile
);
// expect version to remain same before
updateResourceAndVerifyVersion(versionlessPatientId, metaInputOnUpdate, "2");
// Part 3: update with a subset of existing tags in random order
metaInputOnUpdate = createMeta(
generateAllCodingPairs(List.of("sys2"), List.of("bb", "c", "a")), //tag
generateAllCodingPairs(List.of("sys2"), List.of("bb", "c", "a")), //security
List.of("b", "c", "a") // profile
);
// expect version to remain same before
updateResourceAndVerifyVersion(versionlessPatientId, metaInputOnUpdate, "2");
}
/**
* Verifies that tag order doesn't cause version to increase for inline mode where the update behavior is to
* replace the tags completely. This only executes Part 1 of the nonInlineMode test above
*/
public void updateResourceWithExistingTagsButInDifferentOrderAndExpectVersionToRemainTheSame_InlineMode(){
Meta metaInputOnCreate = createMeta(
// generateAllCodingPairs creates a list that has 6 codings in this case in this order:
// (sys2, c), (sys2, b), (sys2, a), (sys1, c), (sys1, b), (sys1, a)
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //security
List.of("c", "b", "a") // profile
);
DaoMethodOutcome createOutcome = createPatient(metaInputOnCreate);
IIdType versionlessPatientId = createOutcome.getId().toVersionless();
// use the same input on update as the creation but order everything differently
Meta metaInputOnUpdate = createMeta(
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("b", "c", "a")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("b", "c", "a")), //security
List.of("b", "c", "a") // profile
);
//update and assert version remains the same (1)
updateResourceAndVerifyVersion(versionlessPatientId, metaInputOnUpdate, "1");
}
public void createResourceWithTagsAndExpectToRetrieveThemSorted() {
Meta metaInputOnCreate = createMeta(
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //security
List.of("c", "b", "a") // profile
);
//expect properties to be alphabetically sorted
Meta expectedMetaAfterCreate = createMeta(
generateAllCodingPairs(List.of("sys1", "sys2"), List.of("a", "b", "c")), //tag (sorted)
generateAllCodingPairs(List.of("sys1", "sys2"), List.of("a", "b", "c")), //security (sorted)
List.of("a", "b", "c") //profile (sorted)
);
createResourceAndVerifyMeta(metaInputOnCreate, expectedMetaAfterCreate);
}
public void updateResourceWithTagsAndExpectToRetrieveTagsSorted_NonInlineModes() {
// meta input for initial creation
Meta metaInputOnCreate = createMeta(
// generateAllCodingPairs creates a list that has 6 codings in this case in this order:
// (sys2, c), (sys2, b), (sys2, a), (sys1, c), (sys1, b), (sys1, a)
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a")), //security
List.of("c", "b", "a") // profile
);
// meta input for update (adding new tags)
Meta metaInputOnUpdate = createMeta(
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("cc", "bb", "aa")), //tag
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("cc", "bb", "aa")), //security
List.of("cc", "bb", "aa") //profile
);
// the new tags & security must be added to the existing set and must be in alphabetical order
// the profile will be completely replaced
Meta expectedMetaAfterUpdate = createMeta(
generateAllCodingPairs(List.of("sys1", "sys2"), List.of("a", "aa", "b", "bb", "c", "cc")), //tag (added & sorted)
generateAllCodingPairs(List.of("sys1", "sys2"), List.of("a", "aa", "b", "bb", "c", "cc")), //security (added & sorted)
List.of("aa", "bb", "cc") //profile (replaced & sorted)
);
IBaseResource resource = updateResourceAndVerifyMeta(metaInputOnCreate, metaInputOnUpdate, expectedMetaAfterUpdate, false);
// expect the resource version to be 2, since the meta is updated
assertEquals("2", resource.getMeta().getVersionId());
//ensure version endpoint also returns tags sorted
IIdType version2Id = new IdType(String.format("%s/_history/2", resource.getIdElement().toVersionless()));
resource = myPatientDao.read(version2Id, myRequestDetails);
verifyMeta(expectedMetaAfterUpdate, resource.getMeta());
}
private DaoMethodOutcome createPatient(Meta theMeta) {
Patient inputPatient = new Patient();
inputPatient.setMeta(theMeta);
return myPatientDao.create(inputPatient, myRequestDetails);
}
private DaoMethodOutcome updatePatient(IIdType thePatientId, Meta theMeta) {
Patient inputPatient = new Patient();
inputPatient.setId(thePatientId);
inputPatient.setMeta(theMeta);
return myPatientDao.update(inputPatient, myRequestDetails);
}
private void verifyMeta(IBaseMetaType theExpectedMeta, IBaseMetaType theActualMeta) {
assertCodingsEqualAndInOrder(theExpectedMeta.getTag(), theActualMeta.getTag());
assertCodingsEqualAndInOrder(theExpectedMeta.getSecurity(), theActualMeta.getSecurity());
assertEquals(toStringList(theExpectedMeta.getProfile()), toStringList(theActualMeta.getProfile()));
}
public void setMetaOperationSupported(boolean theMetaOperationSupported) {
this.myMetaOperationSupported = theMetaOperationSupported;
}
}

View File

@ -59,6 +59,7 @@ import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
import ca.uhn.fhir.rest.server.util.ResourceSearchParams; import ca.uhn.fhir.rest.server.util.ResourceSearchParams;
import ca.uhn.fhir.util.BundleUtil; import ca.uhn.fhir.util.BundleUtil;
import ca.uhn.fhir.util.FhirTerser; import ca.uhn.fhir.util.FhirTerser;
import ca.uhn.fhir.util.IMetaTagSorter;
import ca.uhn.fhir.util.OperationOutcomeUtil; import ca.uhn.fhir.util.OperationOutcomeUtil;
import ca.uhn.fhir.util.ResourceReferenceInfo; import ca.uhn.fhir.util.ResourceReferenceInfo;
import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.StopWatch;
@ -115,11 +116,19 @@ public abstract class BaseStorageDao {
@Autowired @Autowired
protected JpaStorageSettings myStorageSettings; protected JpaStorageSettings myStorageSettings;
@Autowired
protected IMetaTagSorter myMetaTagSorter;
@VisibleForTesting @VisibleForTesting
public void setSearchParamRegistry(ISearchParamRegistry theSearchParamRegistry) { public void setSearchParamRegistry(ISearchParamRegistry theSearchParamRegistry) {
mySearchParamRegistry = theSearchParamRegistry; mySearchParamRegistry = theSearchParamRegistry;
} }
@VisibleForTesting
public void setMyMetaTagSorter(IMetaTagSorter theMetaTagSorter) {
myMetaTagSorter = theMetaTagSorter;
}
/** /**
* May be overridden by subclasses to validate resources prior to storage * May be overridden by subclasses to validate resources prior to storage
* *
@ -153,6 +162,8 @@ public abstract class BaseStorageDao {
} }
performAutoVersioning(theResource, thePerformIndexing); performAutoVersioning(theResource, thePerformIndexing);
myMetaTagSorter.sort(theResource.getMeta());
} }
/** /**

View File

@ -0,0 +1,26 @@
/*-
* #%L
* HAPI FHIR Storage api
* %%
* Copyright (C) 2014 - 2023 Smile CDR, Inc.
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package ca.uhn.fhir.util;
import org.hl7.fhir.instance.model.api.IBaseMetaType;
public interface IMetaTagSorter {
void sort(IBaseMetaType theMeta);
}

View File

@ -0,0 +1,59 @@
/*-
* #%L
* HAPI FHIR Storage api
* %%
* Copyright (C) 2014 - 2023 Smile CDR, Inc.
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package ca.uhn.fhir.util;
import org.hl7.fhir.instance.model.api.IBaseCoding;
import org.hl7.fhir.instance.model.api.IBaseMetaType;
import org.hl7.fhir.instance.model.api.IPrimitiveType;
import java.util.Comparator;
import java.util.List;
/**
* Contains methods to sort resource meta fields that are sets (i.e., tags, security labels and profiles) in alphabetical order.
* It sorts the Coding type sets (tags and security labels) based on the (system, code) pair.
* The system field has higher priority on sorting than the code field so the Coding set will be sorted first by system
* and then by code for each system.
*/
public class MetaTagSorterAlphabetical implements IMetaTagSorter {
private final Comparator<String> nullFirstStringComparator = Comparator.nullsFirst(Comparator.naturalOrder());
private final Comparator<IBaseCoding> myCodingAlphabeticalComparator = Comparator.comparing(
IBaseCoding::getSystem, nullFirstStringComparator)
.thenComparing(IBaseCoding::getCode, nullFirstStringComparator);
private final Comparator<IPrimitiveType<String>> myPrimitiveStringAlphabeticalComparator =
Comparator.comparing(IPrimitiveType::getValue, nullFirstStringComparator);
public void sortCodings(List<? extends IBaseCoding> theCodings) {
theCodings.sort(myCodingAlphabeticalComparator);
}
public void sortPrimitiveStrings(List<? extends IPrimitiveType<String>> theList) {
theList.sort(myPrimitiveStringAlphabeticalComparator);
}
public void sort(IBaseMetaType theMeta) {
sortCodings(theMeta.getTag());
sortCodings(theMeta.getSecurity());
sortPrimitiveStrings(theMeta.getProfile());
}
}

View File

@ -0,0 +1,192 @@
package ca.uhn.fhir.util;
import org.hl7.fhir.r4.model.CanonicalType;
import org.hl7.fhir.r4.model.Meta;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.stream.Stream;
import static ca.uhn.fhir.test.utilities.TagTestUtil.toCanonicalTypeList;
import static ca.uhn.fhir.test.utilities.TagTestUtil.createMeta;
import static ca.uhn.fhir.test.utilities.TagTestUtil.toStringList;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Named.named;
import org.hl7.fhir.r4.model.Coding;
import org.junit.jupiter.api.BeforeEach;
import static ca.uhn.fhir.test.utilities.TagTestUtil.assertCodingsEqualAndInOrder;
import static ca.uhn.fhir.test.utilities.TagTestUtil.createCoding;
import static ca.uhn.fhir.test.utilities.TagTestUtil.generateAllCodingPairs;
class MetaTagSorterAlphabeticalTest {
private MetaTagSorterAlphabetical myTagSorter;
@BeforeEach
public void beforeEach() {
this.myTagSorter = new MetaTagSorterAlphabetical();
}
private static Stream<Arguments> provideTestCodings() {
return Stream.of(
Arguments.of(
//the description of the test case
named("the system is sorted before the code",
// the Input
List.of(createCoding("sys2", "code1"), createCoding("sys1", "code2"))),
// the expected result
List.of(createCoding("sys1", "code2"), createCoding("sys2", "code1"))
),
Arguments.of(
//the description of the test case
named("code determines the order if system are the same",
// the Input
List.of(createCoding("sys", "code2"), createCoding("sys", "code1"))),
// the expected result
List.of(createCoding("sys", "code1"), createCoding("sys", "code2"))
),
Arguments.of(
//the description of the test case
named("null system is less than non-null system",
// the Input
List.of(createCoding("sys", "code1"), createCoding(null, "code2"))),
// the expected result
List.of(createCoding(null, "code2"), createCoding("sys", "code1"))
),
Arguments.of(
//the description of the test case
named("null code is less than a non-null code",
// the Input
List.of(createCoding("sys", "code"), createCoding("sys", null))),
// the expected result
List.of(createCoding("sys", null), createCoding("sys", "code"))
),
Arguments.of(
//the description of the test case
named("works if both system and code are null",
// the Input
List.of(createCoding(null, null).setDisplay("display1"),
createCoding(null, null).setDisplay("display2"))),
// the expected result
List.of(createCoding(null, null).setDisplay("display1"),
createCoding(null, null).setDisplay("display2"))
),
Arguments.of(
//the description of the test case
named("works on a singleton list",
// the Input
List.of(createCoding("sys", "code"))),
// the expected result
List.of(createCoding("sys", "code"))
),
Arguments.of(
//the description of the test case
named("works on an empty list",
// the Input
Collections.EMPTY_LIST),
// the expected result
Collections.EMPTY_LIST
),
Arguments.of(
//the description of the test case
named("more than 2 tags",
// the Input
generateAllCodingPairs(List.of("sys2", "sys1"), List.of("c", "b", "a"))),
// the expected result
generateAllCodingPairs(List.of("sys1", "sys2"), List.of("a", "b", "c"))
)
);
}
@ParameterizedTest(name = "{index}: {0}")
@MethodSource("provideTestCodings")
public void testSortCodings(List<Coding> theInput, List<Coding> theExpected) {
// Copy over the input tags into a new list since List.of creates immutable lists
List<Coding> toBeSorted = new ArrayList<>(theInput);
myTagSorter.sortCodings(toBeSorted);
assertCodingsEqualAndInOrder(theExpected, toBeSorted);
}
private static Stream<Arguments> provideTestPrimitiveStrings() {
return Stream.of(
Arguments.of(
//the description of the test case
named("two sorted alphabetically",
// the Input
List.of("b", "a")),
// the expected result
List.of("a","b")
),
Arguments.of(
//the description of the test case
named("null is less than non-null value",
// the Input
Arrays.asList("a", null)),
// the expected result
Arrays.asList(null, "a")
),
Arguments.of(
//the description of the test case
named("works on a singleton list",
// the Input
List.of("x")),
// the expected result
List.of("x")
),
Arguments.of(
//the description of the test case
named("works on an empty list",
// the Input
Collections.EMPTY_LIST),
// the expected result
Collections.EMPTY_LIST
),
Arguments.of(
//the description of the test case
named("more than 2 in the list",
// the Input
List.of("c", "b", "a")),
// the expected result
List.of("a", "b", "c")
)
);
}
@ParameterizedTest(name = "{index}: {0}")
@MethodSource("provideTestPrimitiveStrings")
public void testSortPrimitiveStringTypes(List<String> theInput, List<String> theExpected) {
List<CanonicalType> toBeSorted = toCanonicalTypeList(theInput);
myTagSorter.sortPrimitiveStrings(toBeSorted);
assertEquals(theExpected, toStringList(toBeSorted));
}
@Test
public void testSort() {
List<Coding> testCoding = List.of(createCoding("s", "2"), createCoding("s", "1"));
List<String> profiles = List.of("2", "1");
Meta meta = createMeta(testCoding, testCoding, profiles);
myTagSorter.sort(meta);
List<Coding> expectedCoding = List.of(createCoding("s", "1"), createCoding("s", "2"));
List<String> expectedProfile = List.of("1", "2");
assertCodingsEqualAndInOrder(expectedCoding, meta.getTag());
assertCodingsEqualAndInOrder(expectedCoding, meta.getSecurity());
assertEquals(expectedProfile, toStringList(meta.getProfile()));
}
}

View File

@ -193,7 +193,7 @@ public abstract class BaseResource extends BaseElement implements IResource {
for (IdDt next : profilesList) { for (IdDt next : profilesList) {
retVal.add(next); retVal.add(next);
} }
return Collections.unmodifiableList(retVal); return retVal;
} }
@Override @Override
@ -209,7 +209,7 @@ public abstract class BaseResource extends BaseElement implements IResource {
next.getCodeElement().getValue()) next.getCodeElement().getValue())
.setDisplay(next.getDisplayElement().getValue())); .setDisplay(next.getDisplayElement().getValue()));
} }
return Collections.unmodifiableList(retVal); return retVal;
} }
@Override @Override
@ -232,7 +232,7 @@ public abstract class BaseResource extends BaseElement implements IResource {
for (Tag next : tagList) { for (Tag next : tagList) {
retVal.add(next); retVal.add(next);
} }
return Collections.unmodifiableList(retVal); return retVal;
} }
@Override @Override

View File

@ -0,0 +1,106 @@
/*-
* #%L
* HAPI FHIR Test Utilities
* %%
* Copyright (C) 2014 - 2023 Smile CDR, Inc.
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package ca.uhn.fhir.test.utilities;
import org.hl7.fhir.instance.model.api.IBaseCoding;
import org.hl7.fhir.instance.model.api.IPrimitiveType;
import org.hl7.fhir.r4.model.CanonicalType;
import org.hl7.fhir.r4.model.Coding;
import org.hl7.fhir.r4.model.Meta;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class TagTestUtil {
/**
* generates a list that contains of all possible Coding pairs from the given system and code values in the iteration order of the lists.
* For example: generateAllCodings(["s1", "s2"], ["c1", "c2"]) creates a coding list that contains 4 codings in this order:
* [("s1", "c1"), ("s1", "c2"), ("s2", "c1"), ("s2", "c2")].
* @param theSystems
* @param theCodes
* @return
*/
public static List<Coding> generateAllCodingPairs(List<String> theSystems, List<String> theCodes) {
List<Coding> result = new ArrayList<>();
for (String system: theSystems) {
for (String code: theCodes) {
result.add(createCoding(system, code));
}
}
return result;
}
/**
* asserts that the two coding list contain equal codings (in the same order)
* @param theExpectedCodings
* @param theActualCodings
*/
public static void assertCodingsEqualAndInOrder(List<? extends IBaseCoding> theExpectedCodings, List<? extends IBaseCoding> theActualCodings) {
assertEquals(theExpectedCodings.size(), theActualCodings.size());
for (int index = 0; index < theExpectedCodings.size(); index++) {
final IBaseCoding expectedCoding = theExpectedCodings.get(index);
final IBaseCoding actualCoding = theActualCodings.get(index);
assertAll(
() -> assertEquals(expectedCoding.getSystem(), actualCoding.getSystem()),
() -> assertEquals(expectedCoding.getCode(), actualCoding.getCode()),
() -> assertEquals(expectedCoding.getDisplay(), actualCoding.getDisplay()),
() -> assertEquals(expectedCoding.getVersion(), actualCoding.getVersion()),
() -> assertEquals(expectedCoding.getUserSelected(), actualCoding.getUserSelected())
);
}
}
public static Coding createCoding(String theSystem, String theCode) {
return createCoding(null, false, theCode, null, theSystem);
}
public static Coding createCoding(String theVersion, boolean theUserSelected, String theCode, String theDisplay, String theSystem) {
final Coding coding = new Coding();
coding.setVersion(theVersion);
coding.setUserSelected(theUserSelected);
coding.setCode(theCode);
coding.setDisplay(theDisplay);
coding.setSystem(theSystem);
return coding;
}
public static Meta createMeta(List<Coding> theTags, List<Coding> theSecurityLabels, List<String> theProfiles) {
Meta meta = new Meta();
meta.setTag(new ArrayList<>(theTags));
meta.setSecurity(new ArrayList<>(theSecurityLabels));
meta.setProfile(toCanonicalTypeList(theProfiles));
return meta;
}
public static List<CanonicalType> toCanonicalTypeList(List<String> theStrings) {
return theStrings.stream().map(s -> new CanonicalType(s)).collect(Collectors.toList());
}
public static List<String> toStringList(List<? extends IPrimitiveType<String>> thePrimitiveTypes) {
return thePrimitiveTypes.stream().map(c -> c.getValue()).collect(Collectors.toList());
}
}