5290 - Feature Request - Implement toggle to prevent conditional updates from invalidating match criteria. (#5291)

* Test skeleton, TODO IMPLEMENTER added all over the place

* Add test and TODO IMPLEMENTER

* Add jpa storage property to allow preventing conditional updates to invalidate match criteria

* spotless

* Implement review suggestions

---------

Co-authored-by: juan.marchionatto <juan.marchionatto@smilecdr.com>
This commit is contained in:
Tadgh 2023-09-19 07:01:33 -07:00 committed by GitHub
parent 67e421649b
commit 49a28efd53
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 255 additions and 57 deletions

View File

@ -36,6 +36,8 @@ public final class HapiSystemProperties {
static final String TEST_MODE = "test"; static final String TEST_MODE = "test";
static final String UNIT_TEST_MODE = "unit_test_mode"; static final String UNIT_TEST_MODE = "unit_test_mode";
static final long DEFAULT_TEST_SYSTEM_PROP_VALIDATION_RESOURCE_CACHES_MS = 10 * DateUtils.MILLIS_PER_SECOND; static final long DEFAULT_TEST_SYSTEM_PROP_VALIDATION_RESOURCE_CACHES_MS = 10 * DateUtils.MILLIS_PER_SECOND;
static final String PREVENT_INVALIDATING_CONDITIONAL_MATCH_CRITERIA =
"hapi.storage.prevent_invalidating_conditional_match_criteria";
private HapiSystemProperties() {} private HapiSystemProperties() {}
@ -158,4 +160,9 @@ public final class HapiSystemProperties {
public static boolean isSuppressHapiFhirVersionLogEnabled() { public static boolean isSuppressHapiFhirVersionLogEnabled() {
return Boolean.parseBoolean(System.getProperty(SUPPRESS_HAPI_FHIR_VERSION_LOG)); return Boolean.parseBoolean(System.getProperty(SUPPRESS_HAPI_FHIR_VERSION_LOG));
} }
public static boolean isPreventInvalidatingConditionalMatchCriteria() {
return Boolean.parseBoolean(System.getProperty(
HapiSystemProperties.PREVENT_INVALIDATING_CONDITIONAL_MATCH_CRITERIA, Boolean.FALSE.toString()));
}
} }

View File

@ -0,0 +1,4 @@
---
type: add
issue: 5290
title: "Added storage property to prevent conditional updates from invalidating match criteria."

View File

@ -179,3 +179,10 @@ Clients may want to disable this setting for performance reasons as it populates
Setting this property explicitly to false disables the feature: [Non Resource DB History](/apidocs/hapi-fhir-storage/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.html#isNonResourceDbHistoryEnabled()) Setting this property explicitly to false disables the feature: [Non Resource DB History](/apidocs/hapi-fhir-storage/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.html#isNonResourceDbHistoryEnabled())
# Prevent Conditional Updates to Invalidate Match Criteria
JPA Server prevents conditional updated to invalidate match criteria for first version of resources.
This setting, disabled by default, allows to configure the same behaviour for later versions.
Setting this property explicitly to true enables the feature: [Prevent Conditional Updates Invalidating Match Criteria](/apidocs/hapi-fhir-storage/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.html#isPreventInvalidatingConditionalMatchCriteria())

View File

@ -1036,7 +1036,8 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
myDaoSearchParamSynchronizer = theDaoSearchParamSynchronizer; myDaoSearchParamSynchronizer = theDaoSearchParamSynchronizer;
} }
private void verifyMatchUrlForConditionalCreate( private void verifyMatchUrlForConditionalCreateOrUpdate(
CreateOrUpdateByMatch theCreateOrUpdate,
IBaseResource theResource, IBaseResource theResource,
String theIfNoneExist, String theIfNoneExist,
ResourceIndexedSearchParams theParams, ResourceIndexedSearchParams theParams,
@ -1044,13 +1045,19 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
// Make sure that the match URL was actually appropriate for the supplied resource // Make sure that the match URL was actually appropriate for the supplied resource
InMemoryMatchResult outcome = InMemoryMatchResult outcome =
myInMemoryResourceMatcher.match(theIfNoneExist, theResource, theParams, theRequestDetails); myInMemoryResourceMatcher.match(theIfNoneExist, theResource, theParams, theRequestDetails);
if (outcome.supported() && !outcome.matched()) { if (outcome.supported() && !outcome.matched()) {
throw new InvalidRequestException( String errorMsg = getConditionalCreateOrUpdateErrorMsg(theCreateOrUpdate);
Msg.code(929) throw new InvalidRequestException(Msg.code(929) + errorMsg);
+ "Failed to process conditional create. The supplied resource did not satisfy the conditional URL.");
} }
} }
private String getConditionalCreateOrUpdateErrorMsg(CreateOrUpdateByMatch theCreateOrUpdate) {
return String.format(
"Failed to process conditional %s. " + "The supplied resource did not satisfy the conditional URL.",
theCreateOrUpdate.name().toLowerCase());
}
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@Override @Override
public ResourceTable updateEntity( public ResourceTable updateEntity(
@ -1173,17 +1180,8 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
} }
if (changed.isChanged()) { if (changed.isChanged()) {
checkConditionalMatch(
// Make sure that the match URL was actually appropriate for the supplied entity, theUpdateVersion, theResource, thePerformIndexing, newParams, theRequest);
// resource. We only do this for version 1 right now since technically it
// is possible (and legal) for someone to be using a conditional update
// to match a resource and then update it in a way that it no longer
// matches. We could certainly make this configurable though in the
// future.
if (entity.getVersion() <= 1L && entity.getCreatedByMatchUrl() != null && thePerformIndexing) {
verifyMatchUrlForConditionalCreate(
theResource, entity.getCreatedByMatchUrl(), newParams, theRequest);
}
if (CURRENTLY_REINDEXING.get(theResource) != Boolean.TRUE) { if (CURRENTLY_REINDEXING.get(theResource) != Boolean.TRUE) {
entity.setUpdated(theTransactionDetails.getTransactionDate()); entity.setUpdated(theTransactionDetails.getTransactionDate());
@ -1333,6 +1331,52 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
return entity; return entity;
} }
/**
* Make sure that the match URL was actually appropriate for the supplied
* resource, if so configured, or do it only for first version, since technically it
* is possible (and legal) for someone to be using a conditional update
* to match a resource and then update it in a way that it no longer
* matches.
*/
private void checkConditionalMatch(
ResourceTable theEntity,
boolean theUpdateVersion,
IBaseResource theResource,
boolean thePerformIndexing,
ResourceIndexedSearchParams theNewParams,
RequestDetails theRequest) {
if (!thePerformIndexing) {
return;
}
if (theEntity.getCreatedByMatchUrl() == null && theEntity.getUpdatedByMatchUrl() == null) {
return;
}
// version is not updated at this point, but could be pending for update, which we consider here
long pendingVersion = theEntity.getVersion();
if (theUpdateVersion && !theEntity.isVersionUpdatedInCurrentTransaction()) {
pendingVersion++;
}
if (myStorageSettings.isPreventInvalidatingConditionalMatchCriteria() || pendingVersion <= 1L) {
String createOrUpdateUrl;
CreateOrUpdateByMatch createOrUpdate;
if (theEntity.getCreatedByMatchUrl() != null) {
createOrUpdateUrl = theEntity.getCreatedByMatchUrl();
createOrUpdate = CreateOrUpdateByMatch.CREATE;
} else {
createOrUpdateUrl = theEntity.getUpdatedByMatchUrl();
createOrUpdate = CreateOrUpdateByMatch.UPDATE;
}
verifyMatchUrlForConditionalCreateOrUpdate(
createOrUpdate, theResource, createOrUpdateUrl, theNewParams, theRequest);
}
}
public IBasePersistedResource updateHistoryEntity( public IBasePersistedResource updateHistoryEntity(
RequestDetails theRequest, RequestDetails theRequest,
T theResource, T theResource,
@ -1609,6 +1653,8 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
// Notify IServerOperationInterceptors about pre-action call // Notify IServerOperationInterceptors about pre-action call
notifyInterceptors(theRequestDetails, theResource, theOldResource, theTransactionDetails, true); notifyInterceptors(theRequestDetails, theResource, theOldResource, theTransactionDetails, true);
entity.setUpdatedByMatchUrl(theMatchUrl);
// Perform update // Perform update
ResourceTable savedEntity = updateEntity( ResourceTable savedEntity = updateEntity(
theRequestDetails, theRequestDetails,
@ -1990,4 +2036,9 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
public static void setValidationDisabledForUnitTest(boolean theValidationDisabledForUnitTest) { public static void setValidationDisabledForUnitTest(boolean theValidationDisabledForUnitTest) {
ourValidationDisabledForUnitTest = theValidationDisabledForUnitTest; ourValidationDisabledForUnitTest = theValidationDisabledForUnitTest;
} }
private enum CreateOrUpdateByMatch {
CREATE,
UPDATE
}
} }

View File

@ -2212,6 +2212,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
IIdType resourceId; IIdType resourceId;
RestOperationTypeEnum update = RestOperationTypeEnum.UPDATE; RestOperationTypeEnum update = RestOperationTypeEnum.UPDATE;
if (isNotBlank(theMatchUrl)) { if (isNotBlank(theMatchUrl)) {
// Validate that the supplied resource matches the conditional.
Set<JpaPid> match = myMatchResourceUrlService.processMatchUrl( Set<JpaPid> match = myMatchResourceUrlService.processMatchUrl(
theMatchUrl, myResourceType, theTransactionDetails, theRequest, theResource); theMatchUrl, myResourceType, theTransactionDetails, theRequest, theResource);
if (match.size() > 1) { if (match.size() > 1) {

View File

@ -420,6 +420,9 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas
@Transient @Transient
private volatile String myCreatedByMatchUrl; private volatile String myCreatedByMatchUrl;
@Transient
private volatile String myUpdatedByMatchUrl;
/** /**
* Constructor * Constructor
*/ */
@ -1007,6 +1010,18 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas
myCreatedByMatchUrl = theCreatedByMatchUrl; myCreatedByMatchUrl = theCreatedByMatchUrl;
} }
public String getUpdatedByMatchUrl() {
return myUpdatedByMatchUrl;
}
public void setUpdatedByMatchUrl(String theUpdatedByMatchUrl) {
myUpdatedByMatchUrl = theUpdatedByMatchUrl;
}
public boolean isVersionUpdatedInCurrentTransaction() {
return myVersionUpdatedInCurrentTransaction;
}
public void setLuceneIndexData(ExtendedHSearchIndexData theLuceneIndexData) { public void setLuceneIndexData(ExtendedHSearchIndexData theLuceneIndexData) {
myLuceneIndexData = theLuceneIndexData; myLuceneIndexData = theLuceneIndexData;
} }

View File

@ -40,7 +40,9 @@ import org.hl7.fhir.r4.model.Organization;
import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.Resource; import org.hl7.fhir.r4.model.Resource;
import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import javax.persistence.Id; import javax.persistence.Id;
@ -48,6 +50,7 @@ import java.util.ArrayList;
import java.util.Date; import java.util.Date;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.TimeZone; import java.util.TimeZone;
import java.util.UUID; import java.util.UUID;
@ -63,6 +66,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
@ -120,15 +124,15 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
p.getMeta().addTag("system", "coding", "display"); p.getMeta().addTag("system", "coding", "display");
myMemoryCacheService.invalidateAllCaches(); myMemoryCacheService.invalidateAllCaches();
myPatientDao.create(p, new SystemRequestDetails()); myPatientDao.create(p, mySrd);
//inject conflicting. //inject conflicting.
myTagDefinitionDao.saveAndFlush(def); myTagDefinitionDao.saveAndFlush(def);
myMemoryCacheService.invalidateAllCaches(); myMemoryCacheService.invalidateAllCaches();
myPatientDao.create(p, new SystemRequestDetails()); myPatientDao.create(p, mySrd);
myMemoryCacheService.invalidateAllCaches(); myMemoryCacheService.invalidateAllCaches();
myPatientDao.create(p, new SystemRequestDetails()); myPatientDao.create(p, mySrd);
} }
@ -139,35 +143,35 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
Patient p = new Patient(); Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(methodName + "2"); p.addIdentifier().setSystem("urn:system").setValue(methodName + "2");
IIdType id = myPatientDao.create(p).getId().toUnqualified(); IIdType id = myPatientDao.create(p, mySrd).getId().toUnqualified();
p = new Patient(); p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(methodName + "2"); p.addIdentifier().setSystem("urn:system").setValue(methodName + "2");
p.setActive(true); p.setActive(true);
IIdType id2 = myPatientDao.create(p, "Patient?identifier=urn:system|" + methodName + "2").getId().toUnqualified(); IIdType id2 = myPatientDao.create(p, "Patient?identifier=urn:system|" + methodName + "2", mySrd).getId().toUnqualified();
assertEquals(id.getValue(), id2.getValue()); assertEquals(id.getValue(), id2.getValue());
p = new Patient(); p = new Patient();
p.setId(id); p.setId(id);
p.addIdentifier().setSystem("urn:system").setValue(methodName + "2"); p.addIdentifier().setSystem("urn:system").setValue(methodName + "2");
p.setActive(false); p.setActive(false);
myPatientDao.update(p).getId(); myPatientDao.update(p, mySrd);
p.setActive(true); p.setActive(true);
id2 = myPatientDao.update(p, "Patient?identifier=urn:system|" + methodName + "2").getId().toUnqualified(); id2 = myPatientDao.update(p, "Patient?identifier=urn:system|" + methodName + "2", mySrd).getId().toUnqualified();
assertEquals(id.getIdPart(), id2.getIdPart()); assertEquals(id.getIdPart(), id2.getIdPart());
assertEquals("3", id2.getVersionIdPart()); assertEquals("3", id2.getVersionIdPart());
Patient newPatient = myPatientDao.read(id); Patient newPatient = myPatientDao.read(id, mySrd);
assertEquals("1", newPatient.getIdElement().getVersionIdPart()); assertEquals("1", newPatient.getIdElement().getVersionIdPart());
newPatient = myPatientDao.read(id.toVersionless()); newPatient = myPatientDao.read(id.toVersionless(), mySrd);
assertEquals("3", newPatient.getIdElement().getVersionIdPart()); assertEquals("3", newPatient.getIdElement().getVersionIdPart());
myPatientDao.delete(id.toVersionless()); myPatientDao.delete(id.toVersionless(), mySrd);
try { try {
myPatientDao.read(id.toVersionless()); myPatientDao.read(id.toVersionless(), mySrd);
fail(); fail();
} catch (ResourceGoneException e) { } catch (ResourceGoneException e) {
// nothing // nothing
@ -175,6 +179,92 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
} }
@Nested
public class TestConditionalResourceMustMatchConditionForUpdate {
private final Patient myPatient = new Patient();
@BeforeEach
void setUp() {
myPatient.setId("existing-patient");
myPatient.addIdentifier().setSystem("http://kookaburra.text/id").setValue("kookaburra1");
myPatientDao.update(myPatient, mySrd);
}
@AfterEach
void tearDown() {
myStorageSettings.setPreventInvalidatingConditionalMatchCriteria(false);
}
@Nested
public class ForFirstVersion {
// For first version must fail validation no matter the state of PreventInvalidatingConditionalMatchCriteria
@Test
public void withPreventInvalidatingConditionalMatchCriteria_true_mustThrow() {
//Note this should always default to false to preserve existing behaviour
myStorageSettings.setPreventInvalidatingConditionalMatchCriteria(true);
Patient p2 = new Patient();
p2.addIdentifier().setSystem("http://kookaburra.text/id").setValue("kookaburra1");
InvalidRequestException thrown = assertThrows(InvalidRequestException.class,
() -> myPatientDao.update(p2,
"Patient?identifier=http://kookaburra.text/id|kookaburra2", mySrd));
assertThat(thrown.getMessage(), endsWith(
"Failed to process conditional create. The supplied resource did not satisfy the conditional URL."));
}
@Test
public void withPreventInvalidatingConditionalMatchCriteria_false_mustThrow() {
//Note this should always default to false to preserve existing behaviour
assertFalse(myStorageSettings.isPreventInvalidatingConditionalMatchCriteria());
Patient p2 = new Patient();
p2.addIdentifier().setSystem("http://kookaburra.text/id").setValue("kookaburra1");
InvalidRequestException thrown = assertThrows(InvalidRequestException.class,
() -> myPatientDao.update(p2,
"Patient?identifier=http://kookaburra.text/id|kookaburra2", mySrd));
assertThat(thrown.getMessage(), endsWith(
"Failed to process conditional create. The supplied resource did not satisfy the conditional URL."));
}
}
@Nested
public class ForOtherThanFirstVersion {
// For other than first version must fail validation only when PreventInvalidatingConditionalMatchCriteria is true
@Test
public void withPreventInvalidatingConditionalMatchCriteria_false_mustWork() {
//Note this should always default to false to preserve existing behaviour
assertFalse(myStorageSettings.isPreventInvalidatingConditionalMatchCriteria());
Patient p2 = new Patient();
p2.addIdentifier().setSystem("http://kookaburra.text/id").setValue("kookaburra2");
myPatientDao.update(p2, "Patient?identifier=http://kookaburra.text/id|kookaburra1", mySrd);
}
@Test
public void withPreventInvalidatingConditionalMatchCriteria_true_mustThrow() {
myStorageSettings.setPreventInvalidatingConditionalMatchCriteria(true); //Note this should always default to false to preserve existing behaviour
Patient p2 = new Patient();
p2.addIdentifier().setSystem("http://kookaburra.text/id").setValue("kookaburra2");
InvalidRequestException thrown = assertThrows(InvalidRequestException.class,
() -> myPatientDao.update(p2,
"Patient?identifier=http://kookaburra.text/id|kookaburra1", mySrd));
assertThat(thrown.getMessage(), endsWith(
"Failed to process conditional update. The supplied resource did not satisfy the conditional URL."));
}
}
}
@Test @Test
public void testUpdateConditionalOnEmailParameterWithPlusSymbol() { public void testUpdateConditionalOnEmailParameterWithPlusSymbol() {
IBundleProvider outcome; IBundleProvider outcome;
@ -184,21 +274,20 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
p.addTelecom() p.addTelecom()
.setSystem(ContactPoint.ContactPointSystem.EMAIL) .setSystem(ContactPoint.ContactPointSystem.EMAIL)
.setValue("help-im+a@bug.com"); .setValue("help-im+a@bug.com");
myPatientDao.update(p, "Patient?email=help-im+a@bug.com"); myPatientDao.update(p, "Patient?email=help-im+a@bug.com", mySrd);
myCaptureQueriesListener.logSelectQueries(); myCaptureQueriesListener.logSelectQueries();
outcome = myPatientDao.search(SearchParameterMap.newSynchronous()); outcome = myPatientDao.search(SearchParameterMap.newSynchronous(), mySrd);
assertEquals(1, outcome.sizeOrThrowNpe()); assertEquals(1, outcome.sizeOrThrowNpe());
p = new Patient(); p = new Patient();
p.addTelecom() p.addTelecom()
.setSystem(ContactPoint.ContactPointSystem.EMAIL) .setSystem(ContactPoint.ContactPointSystem.EMAIL)
.setValue("help-im+a@bug.com"); .setValue("help-im+a@bug.com");
myPatientDao.update(p, "Patient?email=help-im+a@bug.com"); myPatientDao.update(p, "Patient?email=help-im+a@bug.com", mySrd);
outcome = myPatientDao.search(SearchParameterMap.newSynchronous()); outcome = myPatientDao.search(SearchParameterMap.newSynchronous(), mySrd);
assertEquals(1, outcome.sizeOrThrowNpe()); assertEquals(1, outcome.sizeOrThrowNpe());
} }
@Test @Test
@ -210,19 +299,19 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
p.addTelecom() p.addTelecom()
.setSystem(ContactPoint.ContactPointSystem.EMAIL) .setSystem(ContactPoint.ContactPointSystem.EMAIL)
.setValue("help-im+a@bug.com"); .setValue("help-im+a@bug.com");
myPatientDao.update(p, "Patient?email=help-im%2Ba@bug.com"); myPatientDao.update(p, "Patient?email=help-im%2Ba@bug.com", mySrd);
myCaptureQueriesListener.logSelectQueries(); myCaptureQueriesListener.logSelectQueries();
outcome = myPatientDao.search(SearchParameterMap.newSynchronous()); outcome = myPatientDao.search(SearchParameterMap.newSynchronous(), mySrd);
assertEquals(1, outcome.sizeOrThrowNpe()); assertEquals(1, outcome.sizeOrThrowNpe());
p = new Patient(); p = new Patient();
p.addTelecom() p.addTelecom()
.setSystem(ContactPoint.ContactPointSystem.EMAIL) .setSystem(ContactPoint.ContactPointSystem.EMAIL)
.setValue("help-im+a@bug.com"); .setValue("help-im+a@bug.com");
myPatientDao.update(p, "Patient?email=help-im%2Ba@bug.com"); myPatientDao.update(p, "Patient?email=help-im%2Ba@bug.com", mySrd);
outcome = myPatientDao.search(SearchParameterMap.newSynchronous()); outcome = myPatientDao.search(SearchParameterMap.newSynchronous(), mySrd);
assertEquals(1, outcome.sizeOrThrowNpe()); assertEquals(1, outcome.sizeOrThrowNpe());
} }
@ -236,7 +325,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
Patient p = new Patient(); Patient p = new Patient();
p.addIdentifier().setSystem("sys1").setValue("val1"); p.addIdentifier().setSystem("sys1").setValue("val1");
p.addName().setFamily("FAMILY1"); p.addName().setFamily("FAMILY1");
IIdType id = myPatientDao.create(p).getId().toUnqualifiedVersionless(); IIdType id = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless();
runInTransaction(() -> { runInTransaction(() -> {
myEntityManager.createQuery("UPDATE ResourceIndexedSearchParamString s SET s.myHashIdentity = null").executeUpdate(); myEntityManager.createQuery("UPDATE ResourceIndexedSearchParamString s SET s.myHashIdentity = null").executeUpdate();
@ -252,12 +341,12 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
p.setId(id); p.setId(id);
p.addIdentifier().setSystem("sys2").setValue("val2"); p.addIdentifier().setSystem("sys2").setValue("val2");
p.addName().setFamily("FAMILY2"); p.addName().setFamily("FAMILY2");
myPatientDao.update(p); myPatientDao.update(p, mySrd);
SearchParameterMap map = new SearchParameterMap(); SearchParameterMap map = new SearchParameterMap();
map.setLoadSynchronous(true); map.setLoadSynchronous(true);
map.add(Patient.SP_FAMILY, new StringParam("FAMILY2")); map.add(Patient.SP_FAMILY, new StringParam("FAMILY2"));
Patient newPatient = (Patient) myPatientDao.search(map).getResources(0, 1).get(0); Patient newPatient = (Patient) myPatientDao.search(map, mySrd).getResources(0, 1).get(0);
assertEquals("FAMILY2", newPatient.getName().get(0).getFamily()); assertEquals("FAMILY2", newPatient.getName().get(0).getFamily());
} }
@ -266,7 +355,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
IIdType id = runInTransaction(() -> { IIdType id = runInTransaction(() -> {
Patient p = new Patient(); Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue("2"); p.addIdentifier().setSystem("urn:system").setValue("2");
return myPatientDao.create(p).getId().toUnqualified(); return myPatientDao.create(p, mySrd).getId().toUnqualified();
}); });
String createTime = runInTransaction(() -> { String createTime = runInTransaction(() -> {
@ -286,7 +375,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
Patient p = new Patient(); Patient p = new Patient();
p.setId(id.getIdPart()); p.setId(id.getIdPart());
p.addIdentifier().setSystem("urn:system").setValue("2"); p.addIdentifier().setSystem("urn:system").setValue("2");
myPatientDao.update(p).getResource(); myPatientDao.update(p, mySrd);
}); });
runInTransaction(() -> { runInTransaction(() -> {
@ -415,7 +504,6 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
public void testHardMetaCapIsEnforcedOnCreate() { public void testHardMetaCapIsEnforcedOnCreate() {
myStorageSettings.setResourceMetaCountHardLimit(3); myStorageSettings.setResourceMetaCountHardLimit(3);
IIdType id;
{ {
Patient patient = new Patient(); Patient patient = new Patient();
patient.getMeta().addTag().setSystem("http://foo").setCode("1"); patient.getMeta().addTag().setSystem("http://foo").setCode("1");
@ -424,7 +512,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
patient.getMeta().addTag().setSystem("http://foo").setCode("4"); patient.getMeta().addTag().setSystem("http://foo").setCode("4");
patient.setActive(true); patient.setActive(true);
try { try {
id = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); myPatientDao.create(patient, mySrd);
fail(); fail();
} catch (UnprocessableEntityException e) { } catch (UnprocessableEntityException e) {
assertEquals(Msg.code(932) + "Resource contains 4 meta entries (tag/profile/security label), maximum is 3", e.getMessage()); assertEquals(Msg.code(932) + "Resource contains 4 meta entries (tag/profile/security label), maximum is 3", e.getMessage());
@ -466,33 +554,33 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
Patient p = new Patient(); Patient p = new Patient();
p.setActive(true); p.setActive(true);
p.setId("Patient/A"); p.setId("Patient/A");
String id = myPatientDao.update(p).getId().getValue(); String id = myPatientDao.update(p, mySrd).getId().getValue();
assertThat(id, endsWith("Patient/A/_history/1")); assertThat(id, endsWith("Patient/A/_history/1"));
// Second time should not result in an update // Second time should not result in an update
p = new Patient(); p = new Patient();
p.setActive(true); p.setActive(true);
p.setId("Patient/A"); p.setId("Patient/A");
id = myPatientDao.update(p).getId().getValue(); id = myPatientDao.update(p, mySrd).getId().getValue();
assertThat(id, endsWith("Patient/A/_history/1")); assertThat(id, endsWith("Patient/A/_history/1"));
// And third time should not result in an update // And third time should not result in an update
p = new Patient(); p = new Patient();
p.setActive(true); p.setActive(true);
p.setId("Patient/A"); p.setId("Patient/A");
id = myPatientDao.update(p).getId().getValue(); id = myPatientDao.update(p, mySrd).getId().getValue();
assertThat(id, endsWith("Patient/A/_history/1")); assertThat(id, endsWith("Patient/A/_history/1"));
myPatientDao.read(new IdType("Patient/A")); myPatientDao.read(new IdType("Patient/A"), mySrd);
myPatientDao.read(new IdType("Patient/A/_history/1")); myPatientDao.read(new IdType("Patient/A/_history/1"), mySrd);
try { try {
myPatientDao.read(new IdType("Patient/A/_history/2")); myPatientDao.read(new IdType("Patient/A/_history/2"), mySrd);
fail(); fail();
} catch (ResourceNotFoundException e) { } catch (ResourceNotFoundException e) {
// good // good
} }
try { try {
myPatientDao.read(new IdType("Patient/A/_history/3")); myPatientDao.read(new IdType("Patient/A/_history/3"), mySrd);
fail(); fail();
} catch (ResourceNotFoundException e) { } catch (ResourceNotFoundException e) {
// good // good
@ -502,7 +590,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
p = new Patient(); p = new Patient();
p.setActive(false); p.setActive(false);
p.setId("Patient/A"); p.setId("Patient/A");
id = myPatientDao.update(p).getId().getValue(); id = myPatientDao.update(p, mySrd).getId().getValue();
assertThat(id, endsWith("Patient/A/_history/2")); assertThat(id, endsWith("Patient/A/_history/2"));
} }
@ -554,7 +642,8 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
IBundleProvider historyBundle = myPatientDao.history(outcome.getId(), null, null, null, mySrd); IBundleProvider historyBundle = myPatientDao.history(outcome.getId(), null, null, null, mySrd);
assertEquals(2, historyBundle.size().intValue()); assertNotNull(historyBundle);
assertEquals(2, Objects.requireNonNull(historyBundle.size()).intValue());
List<IBaseResource> history = historyBundle.getResources(0, 2); List<IBaseResource> history = historyBundle.getResources(0, 2);
@ -600,7 +689,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
Patient p = new Patient(); Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(methodName + "2"); p.addIdentifier().setSystem("urn:system").setValue(methodName + "2");
myPatientDao.create(p, mySrd).getId(); myPatientDao.create(p, mySrd);
InstantDt start = InstantDt.withCurrentTime(); InstantDt start = InstantDt.withCurrentTime();
ourLog.info("First time: {}", start.getValueAsString()); ourLog.info("First time: {}", start.getValueAsString());
@ -634,7 +723,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
Patient p = new Patient(); Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(methodName + "2"); p.addIdentifier().setSystem("urn:system").setValue(methodName + "2");
myPatientDao.create(p, mySrd).getId(); myPatientDao.create(p, mySrd);
InstantDt start = InstantDt.withCurrentTime(); InstantDt start = InstantDt.withCurrentTime();
ourLog.info("First time: {}", start.getValueAsString()); ourLog.info("First time: {}", start.getValueAsString());
@ -685,7 +774,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
Patient p = new Patient(); Patient p = new Patient();
try { try {
myPatientDao.update(p); myPatientDao.update(p, mySrd);
} catch (InvalidRequestException e) { } catch (InvalidRequestException e) {
assertEquals(Msg.code(987) + "Can not update resource of type Patient as it has no ID", e.getMessage()); assertEquals(Msg.code(987) + "Can not update resource of type Patient as it has no ID", e.getMessage());
} }
@ -700,7 +789,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
Patient p = new Patient(); Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(methodName + "2"); p.addIdentifier().setSystem("urn:system").setValue(methodName + "2");
myPatientDao.create(p, mySrd).getId(); myPatientDao.create(p, mySrd);
InstantDt start = InstantDt.withCurrentTime(); InstantDt start = InstantDt.withCurrentTime();
Thread.sleep(100); Thread.sleep(100);
@ -781,7 +870,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
Patient p2 = new Patient(); Patient p2 = new Patient();
p2.addIdentifier().setSystem("urn:system").setValue("testUpdateMaintainsSearchParamsDstu2BBB"); p2.addIdentifier().setSystem("urn:system").setValue("testUpdateMaintainsSearchParamsDstu2BBB");
p2.addName().setFamily("Tester").addGiven("testUpdateMaintainsSearchParamsDstu2BBB"); p2.addName().setFamily("Tester").addGiven("testUpdateMaintainsSearchParamsDstu2BBB");
myPatientDao.create(p2, mySrd).getId(); myPatientDao.create(p2, mySrd);
List<JpaPid> ids = myPatientDao.searchForIds(new SearchParameterMap(Patient.SP_GIVEN, new StringParam("testUpdateMaintainsSearchParamsDstu2AAA")), null); List<JpaPid> ids = myPatientDao.searchForIds(new SearchParameterMap(Patient.SP_GIVEN, new StringParam("testUpdateMaintainsSearchParamsDstu2AAA")), null);
assertEquals(1, ids.size()); assertEquals(1, ids.size());
@ -1152,7 +1241,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
p.setId(UUID.randomUUID().toString()); p.setId(UUID.randomUUID().toString());
p.addName().setFamily("FAM"); p.addName().setFamily("FAM");
try { try {
myPatientDao.update(p); myPatientDao.update(p, mySrd);
fail(); fail();
} catch (ResourceNotFoundException e) { } catch (ResourceNotFoundException e) {
assertThat(e.getMessage(), matchesPattern(Msg.code(959) + "No resource exists on this server resource with ID.*, and client-assigned IDs are not enabled.")); assertThat(e.getMessage(), matchesPattern(Msg.code(959) + "No resource exists on this server resource with ID.*, and client-assigned IDs are not enabled."));

View File

@ -107,6 +107,9 @@ public class JpaStorageSettings extends StorageSettings {
* Child Configurations * Child Configurations
*/ */
private static final Integer DEFAULT_INTERNAL_SYNCHRONOUS_SEARCH_SIZE = 10000; private static final Integer DEFAULT_INTERNAL_SYNCHRONOUS_SEARCH_SIZE = 10000;
private static final boolean DEFAULT_PREVENT_INVALIDATING_CONDITIONAL_MATCH_CRITERIA = false;
/** /**
* Do not change default of {@code 0}! * Do not change default of {@code 0}!
* *
@ -331,6 +334,16 @@ public class JpaStorageSettings extends StorageSettings {
*/ */
private boolean myResourceHistoryDbEnabled = true; private boolean myResourceHistoryDbEnabled = true;
/**
* This setting allows preventing a conditional update to invalidate the match criteria.
* <p/>
* By default, this is disabled unless explicitly enabled.
*
* @since 6.8.2
*/
private boolean myPreventInvalidatingConditionalMatchCriteria =
DEFAULT_PREVENT_INVALIDATING_CONDITIONAL_MATCH_CRITERIA;
/** /**
* Constructor * Constructor
*/ */
@ -354,6 +367,9 @@ public class JpaStorageSettings extends StorageSettings {
if (HapiSystemProperties.isUnitTestModeEnabled()) { if (HapiSystemProperties.isUnitTestModeEnabled()) {
setJobFastTrackingEnabled(true); setJobFastTrackingEnabled(true);
} }
if (HapiSystemProperties.isPreventInvalidatingConditionalMatchCriteria()) {
setPreventInvalidatingConditionalMatchCriteria(true);
}
} }
/** /**
@ -2372,6 +2388,14 @@ public class JpaStorageSettings extends StorageSettings {
myNonResourceDbHistoryEnabled = theNonResourceDbHistoryEnabled; myNonResourceDbHistoryEnabled = theNonResourceDbHistoryEnabled;
} }
public void setPreventInvalidatingConditionalMatchCriteria(boolean theCriteria) {
myPreventInvalidatingConditionalMatchCriteria = theCriteria;
}
public boolean isPreventInvalidatingConditionalMatchCriteria() {
return myPreventInvalidatingConditionalMatchCriteria;
}
public enum StoreMetaSourceInformationEnum { public enum StoreMetaSourceInformationEnum {
NONE(false, false), NONE(false, false),
SOURCE_URI(true, false), SOURCE_URI(true, false),