Fix resource version regression caused by Hibernate 5.6 (#3325)
* failing test * failing test * Fixes * Updated note to self * Improve pre-caching of resource IDs * Work on tests * Test fixes * Add changelog Co-authored-by: Ken Stevens <khstevens@gmail.com>
This commit is contained in:
James Agnew
GitHub
parent
ed2ea91f56
commit
715a4c4602
|
|
@ -0,0 +1,9 @@
|
|||
---
|
||||
type: fix
|
||||
issue: 3325
|
||||
title: "In HAPI FHIR 5.6.0, a regression meant that JPA server FHIR transactions containing two identical
|
||||
conditional operations (for example, a transaction with two conditional create operations where the
|
||||
conditional URL was identical for both) could result in resources being saved in an inconsistent state.
|
||||
This scenario will now be result in the server returning an HTTP 400 error instead. Note that there is
|
||||
no meaning defined in the FHIR specification for duplicate entries in a FHIR transaction bundle, so it
|
||||
has never been recommended to do this."
|
||||
|
|
@ -36,7 +36,6 @@ import ca.uhn.fhir.jpa.model.config.PartitionSettings;
|
|||
import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource;
|
||||
import ca.uhn.fhir.jpa.model.entity.BaseHasResource;
|
||||
import ca.uhn.fhir.jpa.model.entity.BaseTag;
|
||||
import ca.uhn.fhir.jpa.model.entity.ForcedId;
|
||||
import ca.uhn.fhir.jpa.model.entity.IBaseResourceEntity;
|
||||
import ca.uhn.fhir.jpa.model.entity.PartitionablePartitionId;
|
||||
import ca.uhn.fhir.jpa.model.entity.ResourceEncodingEnum;
|
||||
|
|
@ -274,28 +273,6 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the newly created forced ID. If the entity already had a forced ID, or if
|
||||
* none was created, returns null.
|
||||
*/
|
||||
protected ForcedId createForcedIdIfNeeded(ResourceTable theEntity, IIdType theId, boolean theCreateForPureNumericIds) {
|
||||
ForcedId retVal = null;
|
||||
if (theId.isEmpty() == false && theId.hasIdPart() && theEntity.getForcedId() == null) {
|
||||
if (theCreateForPureNumericIds || !IdHelperService.isValidPid(theId)) {
|
||||
retVal = new ForcedId();
|
||||
retVal.setResourceType(theEntity.getResourceType());
|
||||
retVal.setForcedId(theId.getIdPart());
|
||||
retVal.setResource(theEntity);
|
||||
retVal.setPartitionId(theEntity.getPartitionId());
|
||||
theEntity.setForcedId(retVal);
|
||||
}
|
||||
} else if (theEntity.getForcedId() != null) {
|
||||
retVal = theEntity.getForcedId();
|
||||
}
|
||||
|
||||
return retVal;
|
||||
}
|
||||
|
||||
private void extractTagsHapi(TransactionDetails theTransactionDetails, IResource theResource, ResourceTable theEntity, Set<ResourceTag> allDefs) {
|
||||
TagList tagList = ResourceMetadataKeyEnum.TAG_LIST.get(theResource);
|
||||
if (tagList != null) {
|
||||
|
|
|
|||
|
|
@ -301,21 +301,6 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
|
|||
}
|
||||
}
|
||||
|
||||
boolean serverAssignedId;
|
||||
if (isNotBlank(theResource.getIdElement().getIdPart())) {
|
||||
if (theResource.getUserData(JpaConstants.RESOURCE_ID_SERVER_ASSIGNED) == Boolean.TRUE) {
|
||||
createForcedIdIfNeeded(entity, theResource.getIdElement(), true);
|
||||
serverAssignedId = true;
|
||||
} else {
|
||||
validateResourceIdCreation(theResource, theRequest);
|
||||
boolean createForPureNumericIds = getConfig().getResourceClientIdStrategy() != DaoConfig.ClientIdStrategyEnum.ALPHANUMERIC;
|
||||
createForcedIdIfNeeded(entity, theResource.getIdElement(), createForPureNumericIds);
|
||||
serverAssignedId = false;
|
||||
}
|
||||
} else {
|
||||
serverAssignedId = true;
|
||||
}
|
||||
|
||||
// Notify interceptors
|
||||
if (theRequest != null) {
|
||||
ActionRequestDetails requestDetails = new ActionRequestDetails(theRequest, getContext(), theResource);
|
||||
|
|
@ -330,59 +315,53 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
|
|||
.add(TransactionDetails.class, theTransactionDetails);
|
||||
doCallHooks(theTransactionDetails, theRequest, Pointcut.STORAGE_PRESTORAGE_RESOURCE_CREATED, hookParams);
|
||||
|
||||
String resourceIdBeforeStorage = theResource.getIdElement().getIdPart();
|
||||
boolean resourceHadIdBeforeStorage = isNotBlank(resourceIdBeforeStorage);
|
||||
boolean resourceIdWasServerAssigned = theResource.getUserData(JpaConstants.RESOURCE_ID_SERVER_ASSIGNED) == Boolean.TRUE;
|
||||
if (resourceHadIdBeforeStorage && !resourceIdWasServerAssigned) {
|
||||
validateResourceIdCreation(theResource, theRequest);
|
||||
}
|
||||
|
||||
// Perform actual DB update
|
||||
ResourceTable updatedEntity = updateEntity(theRequest, theResource, entity, null, thePerformIndexing, false, theTransactionDetails, false, thePerformIndexing);
|
||||
|
||||
IIdType id = myFhirContext.getVersion().newIdType().setValue(updatedEntity.getIdDt().toUnqualifiedVersionless().getValue());
|
||||
// Store the resource forced ID if necessary
|
||||
ResourcePersistentId persistentId = new ResourcePersistentId(updatedEntity.getResourceId());
|
||||
theTransactionDetails.addResolvedResourceId(id, persistentId);
|
||||
if (entity.getForcedId() != null) {
|
||||
myIdHelperService.addResolvedPidToForcedId(persistentId, theRequestPartitionId, updatedEntity.getResourceType(), updatedEntity.getForcedId().getForcedId(), updatedEntity.getDeleted());
|
||||
if (resourceHadIdBeforeStorage) {
|
||||
if (resourceIdWasServerAssigned) {
|
||||
boolean createForPureNumericIds = true;
|
||||
createForcedIdIfNeeded(entity, resourceIdBeforeStorage, createForPureNumericIds, persistentId, theRequestPartitionId);
|
||||
} else {
|
||||
boolean createForPureNumericIds = getConfig().getResourceClientIdStrategy() != DaoConfig.ClientIdStrategyEnum.ALPHANUMERIC;
|
||||
createForcedIdIfNeeded(entity, resourceIdBeforeStorage, createForPureNumericIds, persistentId, theRequestPartitionId);
|
||||
}
|
||||
|
||||
theResource.setId(entity.getIdDt());
|
||||
|
||||
if (serverAssignedId) {
|
||||
} else {
|
||||
switch (getConfig().getResourceClientIdStrategy()) {
|
||||
case NOT_ALLOWED:
|
||||
case ALPHANUMERIC:
|
||||
break;
|
||||
case ANY:
|
||||
ForcedId forcedId = createForcedIdIfNeeded(updatedEntity, theResource.getIdElement(), true);
|
||||
if (forcedId != null) {
|
||||
|
||||
/*
|
||||
* As of Hibernate 5.6.2, assigning the forced ID to the
|
||||
* resource table causes an extra update to happen, even
|
||||
* though the ResourceTable entity isn't actually changed
|
||||
* (there is a @OneToOne reference on ResourceTable to the
|
||||
* ForcedId table, but the actual column is on the ForcedId
|
||||
* table so it doesn't actually make sense to update the table
|
||||
* when this is set). But to work around that we clear this
|
||||
* here.
|
||||
*
|
||||
* If you get rid of the following line (maybe possible
|
||||
* in a future version of Hibernate) try running the tests
|
||||
* in FhirResourceDaoR4QueryCountTest
|
||||
* JA 20220126
|
||||
*/
|
||||
updatedEntity.setForcedId(null);
|
||||
updatedEntity.setTransientForcedId(forcedId.getForcedId());
|
||||
|
||||
myForcedIdDao.save(forcedId);
|
||||
}
|
||||
boolean createForPureNumericIds = true;
|
||||
createForcedIdIfNeeded(updatedEntity, theResource.getIdElement().getIdPart(), createForPureNumericIds, persistentId, theRequestPartitionId);
|
||||
// for client ID mode ANY, we will always have a forced ID. If we ever
|
||||
// stop populating the transient forced ID be warned that we use it
|
||||
// (and expect it to be set correctly) farther below.
|
||||
assert updatedEntity.getTransientForcedId() != null;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
ResourcePersistentId resourcePersistentId = new ResourcePersistentId(entity.getResourceId());
|
||||
resourcePersistentId.setAssociatedResourceId(entity.getIdType(myFhirContext));
|
||||
// Populate the resource with it's actual final stored ID from the entity
|
||||
theResource.setId(entity.getIdDt());
|
||||
|
||||
theTransactionDetails.addResolvedResourceId(resourcePersistentId.getAssociatedResourceId(), resourcePersistentId);
|
||||
// Pre-cache the resource ID
|
||||
persistentId.setAssociatedResourceId(entity.getIdType(myFhirContext));
|
||||
myIdHelperService.addResolvedPidToForcedId(persistentId, theRequestPartitionId, getResourceName(), entity.getTransientForcedId(), null);
|
||||
theTransactionDetails.addResolvedResourceId(persistentId.getAssociatedResourceId(), persistentId);
|
||||
|
||||
// Pre-cache the match URL
|
||||
if (theIfNoneExist != null) {
|
||||
myMatchResourceUrlService.matchUrlResolved(theTransactionDetails, getResourceName(), theIfNoneExist, resourcePersistentId);
|
||||
myMatchResourceUrlService.matchUrlResolved(theTransactionDetails, getResourceName(), theIfNoneExist, persistentId);
|
||||
}
|
||||
|
||||
// Update the version/last updated in the resource so that interceptors get
|
||||
|
|
@ -411,16 +390,44 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
|
|||
String msg = getContext().getLocalizer().getMessageSanitized(BaseStorageDao.class, "successfulCreate", outcome.getId(), w.getMillisAndRestart());
|
||||
outcome.setOperationOutcome(createInfoOperationOutcome(msg));
|
||||
|
||||
String forcedId = null;
|
||||
if (updatedEntity.getForcedId() != null) {
|
||||
forcedId = updatedEntity.getForcedId().getForcedId();
|
||||
}
|
||||
myIdHelperService.addResolvedPidToForcedId(persistentId, theRequestPartitionId, getResourceName(), forcedId, null);
|
||||
|
||||
ourLog.debug(msg);
|
||||
return outcome;
|
||||
}
|
||||
|
||||
private void createForcedIdIfNeeded(ResourceTable theEntity, String theResourceId, boolean theCreateForPureNumericIds, ResourcePersistentId thePersistentId, RequestPartitionId theRequestPartitionId) {
|
||||
if (isNotBlank(theResourceId) && theEntity.getForcedId() == null) {
|
||||
if (theCreateForPureNumericIds || !IdHelperService.isValidPid(theResourceId)) {
|
||||
ForcedId forcedId = new ForcedId();
|
||||
forcedId.setResourceType(theEntity.getResourceType());
|
||||
forcedId.setForcedId(theResourceId);
|
||||
forcedId.setResource(theEntity);
|
||||
forcedId.setPartitionId(theEntity.getPartitionId());
|
||||
|
||||
/*
|
||||
* As of Hibernate 5.6.2, assigning the forced ID to the
|
||||
* resource table causes an extra update to happen, even
|
||||
* though the ResourceTable entity isn't actually changed
|
||||
* (there is a @OneToOne reference on ResourceTable to the
|
||||
* ForcedId table, but the actual column is on the ForcedId
|
||||
* table so it doesn't actually make sense to update the table
|
||||
* when this is set). But to work around that we avoid
|
||||
* actually assigning ResourceTable#myForcedId here.
|
||||
*
|
||||
* It's conceivable they may fix this in the future, or
|
||||
* they may not.
|
||||
*
|
||||
* If you want to try assigning the forced it to the resource
|
||||
* entity (by calling ResourceTable#setForcedId) try running
|
||||
* the tests FhirResourceDaoR4QueryCountTest to verify that
|
||||
* nothing has broken as a result.
|
||||
* JA 20220121
|
||||
*/
|
||||
theEntity.setTransientForcedId(forcedId.getForcedId());
|
||||
myForcedIdDao.save(forcedId);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void validateResourceIdCreation(T theResource, RequestDetails theRequest) {
|
||||
DaoConfig.ClientIdStrategyEnum strategy = getConfig().getResourceClientIdStrategy();
|
||||
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ import ca.uhn.fhir.model.dstu2.valueset.IssueTypeEnum;
|
|||
import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
|
||||
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
|
||||
import org.hl7.fhir.exceptions.FHIRException;
|
||||
import org.hl7.fhir.instance.model.api.IBase;
|
||||
import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
|
||||
|
|
@ -118,6 +119,11 @@ public class TransactionProcessorVersionAdapterDstu2 implements ITransactionProc
|
|||
return theEntry.getFullUrl();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void setFullUrl(Bundle.Entry theEntry, String theFullUrl) {
|
||||
theEntry.setFullUrl(theFullUrl);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getEntryIfNoneExist(Bundle.Entry theEntry) {
|
||||
return theEntry.getRequest().getIfNoneExist();
|
||||
|
|
|
|||
|
|
@ -114,6 +114,12 @@ public class TransactionProcessorVersionAdapterR5 implements ITransactionProcess
|
|||
return theEntry.getFullUrl();
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public void setFullUrl(Bundle.BundleEntryComponent theEntry, String theFullUrl) {
|
||||
theEntry.setFullUrl(theFullUrl);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getEntryIfNoneExist(Bundle.BundleEntryComponent theEntry) {
|
||||
return theEntry.getRequest().getIfNoneExist();
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import ca.uhn.fhir.jpa.entity.TermValueSet;
|
|||
import ca.uhn.fhir.jpa.entity.TermValueSetConcept;
|
||||
import ca.uhn.fhir.jpa.entity.TermValueSetConceptDesignation;
|
||||
import ca.uhn.fhir.jpa.model.entity.ForcedId;
|
||||
import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
|
||||
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
|
||||
import ca.uhn.fhir.jpa.model.util.JpaConstants;
|
||||
import ca.uhn.fhir.jpa.partition.IPartitionLookupSvc;
|
||||
|
|
@ -595,7 +596,7 @@ public abstract class BaseJpaTest extends BaseTest {
|
|||
|
||||
protected int logAllResourceVersions() {
|
||||
return runInTransaction(() -> {
|
||||
List<ResourceTable> resources = myResourceTableDao.findAll();
|
||||
List<ResourceHistoryTable> resources = myResourceHistoryTableDao.findAll();
|
||||
ourLog.info("Resources Versions:\n * {}", resources.stream().map(t -> t.toString()).collect(Collectors.joining("\n * ")));
|
||||
return resources.size();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -427,10 +427,12 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest {
|
|||
request.addEntry().setResource(p).getRequest().setMethod(HTTPVerbEnum.POST).setIfNoneExist("Patient?identifier=urn%3Asystem%7C" + methodName);
|
||||
|
||||
myCaptureQueriesListener.clear();
|
||||
try {
|
||||
mySystemDao.transaction(mySrd, request);
|
||||
myCaptureQueriesListener.logSelectQueriesForCurrentThread();
|
||||
|
||||
assertEquals(1, logAllResources());
|
||||
fail();
|
||||
} catch (InvalidRequestException e) {
|
||||
assertEquals("Unable to process Transaction - Request contains multiple anonymous entries (Bundle.entry.fullUrl not populated) with conditional URL: \"Patient?identifier=urn%3Asystem%7CtestTransactionCreateWithDuplicateMatchUrl01\". Does transaction request contain duplicates?", e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamQuantity;
|
|||
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamQuantityNormalized;
|
||||
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamString;
|
||||
import ca.uhn.fhir.jpa.model.entity.ResourceLink;
|
||||
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
|
||||
import ca.uhn.fhir.jpa.model.util.UcumServiceUtil;
|
||||
import ca.uhn.fhir.jpa.partition.SystemRequestDetails;
|
||||
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
|
||||
|
|
@ -36,6 +37,7 @@ import org.hl7.fhir.r4.model.Quantity;
|
|||
import org.hl7.fhir.r4.model.Reference;
|
||||
import org.hl7.fhir.r4.model.SampledData;
|
||||
import org.hl7.fhir.r4.model.SearchParameter;
|
||||
import org.hl7.fhir.r4.model.StructureDefinition;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.slf4j.Logger;
|
||||
|
|
@ -547,6 +549,38 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test {
|
|||
ourLog.info("ID2: {}", id2);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCreateAndSearchWithUuidResourceStrategy() {
|
||||
myDaoConfig.setResourceServerIdStrategy(DaoConfig.IdStrategyEnum.UUID);
|
||||
myDaoConfig.setResourceClientIdStrategy(DaoConfig.ClientIdStrategyEnum.ANY);
|
||||
|
||||
StructureDefinition sd = new StructureDefinition();
|
||||
sd.setUrl("http://foo.com");
|
||||
DaoMethodOutcome result = myStructureDefinitionDao.create(sd);
|
||||
assertTrue(result.getCreated());
|
||||
StructureDefinition readSd = myStructureDefinitionDao.read(result.getId());
|
||||
assertEquals("http://foo.com", readSd.getUrl());
|
||||
|
||||
logAllResources();
|
||||
logAllResourceVersions();
|
||||
|
||||
runInTransaction(()->{
|
||||
List<ResourceTable> resources = myResourceTableDao.findAll();
|
||||
assertEquals(1, resources.size());
|
||||
assertEquals(1, resources.get(0).getVersion());
|
||||
|
||||
List<ResourceHistoryTable> resourceVersions = myResourceHistoryTableDao.findAll();
|
||||
assertEquals(1, resourceVersions.size());
|
||||
assertEquals(1, resourceVersions.get(0).getVersion());
|
||||
});
|
||||
|
||||
SearchParameterMap map = SearchParameterMap.newSynchronous();
|
||||
|
||||
myCaptureQueriesListener.clear();
|
||||
IBundleProvider bundle = myStructureDefinitionDao.search(map);
|
||||
myCaptureQueriesListener.logSelectQueriesForCurrentThread();
|
||||
assertEquals(1, bundle.size());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTransactionCreateWithUuidResourceStrategy() {
|
||||
|
|
|
|||
|
|
@ -938,6 +938,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
|
|||
assertEquals(1, myCaptureQueriesListener.countUpdateQueries());
|
||||
assertEquals(0, myCaptureQueriesListener.countDeleteQueries());
|
||||
runInTransaction(() -> assertEquals(4, myResourceTableDao.count()));
|
||||
logAllResources();
|
||||
|
||||
// Run it again - This time even the match URL should be cached
|
||||
|
||||
|
|
|
|||
|
|
@ -49,7 +49,6 @@ import org.hl7.fhir.r4.model.Bundle.BundleEntryResponseComponent;
|
|||
import org.hl7.fhir.r4.model.Bundle.BundleType;
|
||||
import org.hl7.fhir.r4.model.Bundle.HTTPVerb;
|
||||
import org.hl7.fhir.r4.model.CanonicalType;
|
||||
import org.hl7.fhir.r4.model.CarePlan;
|
||||
import org.hl7.fhir.r4.model.Coding;
|
||||
import org.hl7.fhir.r4.model.Communication;
|
||||
import org.hl7.fhir.r4.model.Condition;
|
||||
|
|
@ -1737,6 +1736,39 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest {
|
|||
assertThat(responseTypes.toString(), responseTypes, contains("Practitioner", "Observation", "Observation"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTransactionWithDuplicateConditionalUpdateOnlyCreatesOne() {
|
||||
Bundle inputBundle = new Bundle();
|
||||
inputBundle.setType(Bundle.BundleType.TRANSACTION);
|
||||
|
||||
Encounter enc1 = new Encounter();
|
||||
enc1.addIdentifier().setSystem("urn:foo").setValue("12345");
|
||||
enc1.getClass_().setDisplay("ENC1");
|
||||
inputBundle
|
||||
.addEntry()
|
||||
.setResource(enc1)
|
||||
.getRequest()
|
||||
.setMethod(HTTPVerb.PUT)
|
||||
.setUrl("Encounter?identifier=urn:foo|12345");
|
||||
Encounter enc2 = new Encounter();
|
||||
enc2.addIdentifier().setSystem("urn:foo").setValue("12345");
|
||||
enc2.getClass_().setDisplay("ENC2");
|
||||
inputBundle
|
||||
.addEntry()
|
||||
.setResource(enc2)
|
||||
.getRequest()
|
||||
.setMethod(HTTPVerb.PUT)
|
||||
.setUrl("Encounter?identifier=urn:foo|12345");
|
||||
|
||||
try {
|
||||
mySystemDao.transaction(mySrd, inputBundle);
|
||||
fail();
|
||||
} catch (InvalidRequestException e) {
|
||||
assertEquals("Unable to process Transaction - Request contains multiple anonymous entries (Bundle.entry.fullUrl not populated) with conditional URL: \"Encounter?identifier=urn:foo|12345\". Does transaction request contain duplicates?", e.getMessage());
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void testTransactionCreateInlineMatchUrlWithTwoMatches() {
|
||||
|
|
@ -1982,9 +2014,11 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest {
|
|||
p.addIdentifier().setSystem("urn:system").setValue(methodName);
|
||||
request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setIfNoneExist("Patient?identifier=urn%3Asystem%7C" + methodName);
|
||||
|
||||
try {
|
||||
mySystemDao.transaction(mySrd, request);
|
||||
assertEquals(1, logAllResources());
|
||||
assertEquals(1, logAllResourceVersions());
|
||||
} catch (InvalidRequestException e) {
|
||||
assertEquals("Unable to process Transaction - Request contains multiple anonymous entries (Bundle.entry.fullUrl not populated) with conditional URL: \"Patient?identifier=urn%3Asystem%7CtestTransactionCreateWithDuplicateMatchUrl01\". Does transaction request contain duplicates?", e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -2663,37 +2697,12 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest {
|
|||
.setMethod(HTTPVerb.POST)
|
||||
.setIfNoneExist("Encounter?identifier=urn:foo|12345");
|
||||
|
||||
try {
|
||||
mySystemDao.transaction(mySrd, inputBundle);
|
||||
assertEquals(1, logAllResources());
|
||||
assertEquals(1, logAllResourceVersions());
|
||||
fail();
|
||||
} catch (InvalidRequestException e) {
|
||||
assertEquals("Unable to process Transaction - Request contains multiple anonymous entries (Bundle.entry.fullUrl not populated) with conditional URL: \"Encounter?identifier=urn:foo|12345\". Does transaction request contain duplicates?", e.getMessage());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTransactionDoubleConditionalUpdateOnlyCreatesOne() {
|
||||
Bundle inputBundle = new Bundle();
|
||||
inputBundle.setType(Bundle.BundleType.TRANSACTION);
|
||||
|
||||
Encounter enc1 = new Encounter();
|
||||
enc1.addIdentifier().setSystem("urn:foo").setValue("12345");
|
||||
inputBundle
|
||||
.addEntry()
|
||||
.setResource(enc1)
|
||||
.getRequest()
|
||||
.setMethod(HTTPVerb.PUT)
|
||||
.setUrl("Encounter?identifier=urn:foo|12345");
|
||||
Encounter enc2 = new Encounter();
|
||||
enc2.addIdentifier().setSystem("urn:foo").setValue("12345");
|
||||
inputBundle
|
||||
.addEntry()
|
||||
.setResource(enc2)
|
||||
.getRequest()
|
||||
.setMethod(HTTPVerb.PUT)
|
||||
.setUrl("Encounter?identifier=urn:foo|12345");
|
||||
|
||||
mySystemDao.transaction(mySrd, inputBundle);
|
||||
|
||||
assertEquals(1, logAllResources());
|
||||
assertEquals(1, logAllResourceVersions());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -23,6 +23,8 @@ package ca.uhn.fhir.jpa.model.entity;
|
|||
import ca.uhn.fhir.model.primitive.IdDt;
|
||||
import ca.uhn.fhir.rest.api.Constants;
|
||||
import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId;
|
||||
import org.apache.commons.lang3.builder.ToStringBuilder;
|
||||
import org.apache.commons.lang3.builder.ToStringStyle;
|
||||
import org.hibernate.annotations.OptimisticLock;
|
||||
|
||||
import javax.persistence.CascadeType;
|
||||
|
|
@ -57,9 +59,7 @@ import java.util.Collection;
|
|||
@Index(name = "IDX_RESVER_DATE", columnList = "RES_UPDATED")
|
||||
})
|
||||
public class ResourceHistoryTable extends BaseHasResource implements Serializable {
|
||||
|
||||
public static final String IDX_RESVER_ID_VER = "IDX_RESVER_ID_VER";
|
||||
|
||||
/**
|
||||
* @see ResourceEncodingEnum
|
||||
*/
|
||||
|
|
@ -74,32 +74,24 @@ public class ResourceHistoryTable extends BaseHasResource implements Serializabl
|
|||
@GeneratedValue(strategy = GenerationType.AUTO, generator = "SEQ_RESOURCE_HISTORY_ID")
|
||||
@Column(name = "PID")
|
||||
private Long myId;
|
||||
|
||||
@ManyToOne(fetch = FetchType.LAZY)
|
||||
@JoinColumn(name = "RES_ID", nullable = false, updatable = false, foreignKey = @ForeignKey(name = "FK_RESOURCE_HISTORY_RESOURCE"))
|
||||
private ResourceTable myResourceTable;
|
||||
|
||||
@Column(name = "RES_ID", nullable = false, updatable = false, insertable = false)
|
||||
private Long myResourceId;
|
||||
|
||||
@Column(name = "RES_TYPE", length = ResourceTable.RESTYPE_LEN, nullable = false)
|
||||
private String myResourceType;
|
||||
|
||||
@Column(name = "RES_VER", nullable = false)
|
||||
private Long myResourceVersion;
|
||||
|
||||
@OneToMany(mappedBy = "myResourceHistory", cascade = CascadeType.ALL, fetch = FetchType.LAZY, orphanRemoval = true)
|
||||
private Collection<ResourceHistoryTag> myTags;
|
||||
|
||||
@Column(name = "RES_TEXT", length = Integer.MAX_VALUE - 1, nullable = true)
|
||||
@Lob()
|
||||
@OptimisticLock(excluded = true)
|
||||
private byte[] myResource;
|
||||
|
||||
@Column(name = "RES_TEXT_VC", length = RES_TEXT_VC_MAX_LENGTH, nullable = true)
|
||||
@OptimisticLock(excluded = true)
|
||||
private String myResourceTextVc;
|
||||
|
||||
@Column(name = "RES_ENCODING", nullable = false, length = ENCODING_COL_LENGTH)
|
||||
@Enumerated(EnumType.STRING)
|
||||
@OptimisticLock(excluded = true)
|
||||
|
|
@ -107,10 +99,23 @@ public class ResourceHistoryTable extends BaseHasResource implements Serializabl
|
|||
@OneToOne(mappedBy = "myResourceHistoryTable", cascade = {CascadeType.REMOVE})
|
||||
private ResourceHistoryProvenanceEntity myProvenance;
|
||||
|
||||
/**
|
||||
* Constructor
|
||||
*/
|
||||
public ResourceHistoryTable() {
|
||||
super();
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
|
||||
.append("resourceId", myResourceId)
|
||||
.append("resourceType", myResourceType)
|
||||
.append("resourceVersion", myResourceVersion)
|
||||
.append("pid", myId)
|
||||
.toString();
|
||||
}
|
||||
|
||||
public String getResourceTextVc() {
|
||||
return myResourceTextVc;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -664,6 +664,7 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas
|
|||
ToStringBuilder b = new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE);
|
||||
b.append("pid", myId);
|
||||
b.append("resourceType", myResourceType);
|
||||
b.append("version", myVersion);
|
||||
if (getPartitionId() != null) {
|
||||
b.append("partitionId", getPartitionId().getPartitionId());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -98,6 +98,7 @@ import org.hl7.fhir.instance.model.api.IBaseReference;
|
|||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
import org.hl7.fhir.instance.model.api.IIdType;
|
||||
import org.hl7.fhir.instance.model.api.IPrimitiveType;
|
||||
import org.hl7.fhir.r4.model.IdType;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
|
|
@ -154,8 +155,6 @@ public abstract class BaseTransactionProcessor {
|
|||
@Autowired
|
||||
private IInterceptorBroadcaster myInterceptorBroadcaster;
|
||||
@Autowired
|
||||
private MatchResourceUrlService myMatchResourceUrlService;
|
||||
@Autowired
|
||||
private HapiTransactionService myHapiTransactionService;
|
||||
@Autowired
|
||||
private DaoConfig myDaoConfig;
|
||||
|
|
@ -477,19 +476,18 @@ public abstract class BaseTransactionProcessor {
|
|||
* we try to handle the resource with the placeholder first.
|
||||
*/
|
||||
Set<String> placeholderIds = new HashSet<>();
|
||||
final List<IBase> entries = requestEntries;
|
||||
for (IBase nextEntry : entries) {
|
||||
for (IBase nextEntry : requestEntries) {
|
||||
String fullUrl = myVersionAdapter.getFullUrl(nextEntry);
|
||||
if (isNotBlank(fullUrl) && fullUrl.startsWith(URN_PREFIX)) {
|
||||
placeholderIds.add(fullUrl);
|
||||
}
|
||||
}
|
||||
entries.sort(new TransactionSorter(placeholderIds));
|
||||
requestEntries.sort(new TransactionSorter(placeholderIds));
|
||||
|
||||
// perform all writes
|
||||
prepareThenExecuteTransactionWriteOperations(theRequestDetails, theActionName,
|
||||
transactionDetails, transactionStopWatch,
|
||||
response, originalRequestOrder, entries);
|
||||
response, originalRequestOrder, requestEntries);
|
||||
|
||||
// perform all gets
|
||||
// (we do these last so that the gets happen on the final state of the DB;
|
||||
|
|
@ -711,11 +709,11 @@ public abstract class BaseTransactionProcessor {
|
|||
|
||||
/**
|
||||
* Searches for duplicate conditional creates and consolidates them.
|
||||
*
|
||||
* @param theEntries
|
||||
*/
|
||||
private void consolidateDuplicateConditionals(List<IBase> theEntries) {
|
||||
private void consolidateDuplicateConditionals(RequestDetails theRequestDetails, String theActionName, List<IBase> theEntries) {
|
||||
final Set<String> keysWithNoFullUrl = new HashSet<>();
|
||||
final HashMap<String, String> keyToUuid = new HashMap<>();
|
||||
|
||||
for (int index = 0, originalIndex = 0; index < theEntries.size(); index++, originalIndex++) {
|
||||
IBase nextReqEntry = theEntries.get(index);
|
||||
IBaseResource resource = myVersionAdapter.getResource(nextReqEntry);
|
||||
|
|
@ -724,33 +722,64 @@ public abstract class BaseTransactionProcessor {
|
|||
String entryFullUrl = myVersionAdapter.getFullUrl(nextReqEntry);
|
||||
String requestUrl = myVersionAdapter.getEntryRequestUrl(nextReqEntry);
|
||||
String ifNoneExist = myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry);
|
||||
String key = verb + "|" + requestUrl + "|" + ifNoneExist;
|
||||
|
||||
// Conditional UPDATE
|
||||
boolean consolidateEntry = false;
|
||||
if ("PUT".equals(verb)) {
|
||||
if (isNotBlank(entryFullUrl) && isNotBlank(requestUrl)) {
|
||||
boolean consolidateEntryCandidate = false;
|
||||
String conditionalUrl;
|
||||
switch (verb) {
|
||||
case "PUT":
|
||||
conditionalUrl = requestUrl;
|
||||
if (isNotBlank(requestUrl)) {
|
||||
int questionMarkIndex = requestUrl.indexOf('?');
|
||||
if (questionMarkIndex >= 0 && requestUrl.length() > (questionMarkIndex + 1)) {
|
||||
consolidateEntry = true;
|
||||
}
|
||||
consolidateEntryCandidate = true;
|
||||
}
|
||||
}
|
||||
break;
|
||||
|
||||
// Conditional CREATE
|
||||
if ("POST".equals(verb)) {
|
||||
if (isNotBlank(entryFullUrl) && isNotBlank(requestUrl) && isNotBlank(ifNoneExist)) {
|
||||
if (!entryFullUrl.equals(requestUrl)) {
|
||||
consolidateEntry = true;
|
||||
case "POST":
|
||||
conditionalUrl = ifNoneExist;
|
||||
if (isNotBlank(ifNoneExist)) {
|
||||
if (isBlank(entryFullUrl) || !entryFullUrl.equals(requestUrl)) {
|
||||
consolidateEntryCandidate = true;
|
||||
}
|
||||
}
|
||||
break;
|
||||
|
||||
default:
|
||||
continue;
|
||||
}
|
||||
|
||||
if (consolidateEntry) {
|
||||
if (isNotBlank(conditionalUrl) && !conditionalUrl.contains("?")) {
|
||||
conditionalUrl = myContext.getResourceType(resource) + "?" + conditionalUrl;
|
||||
}
|
||||
|
||||
String key = verb + "|" + conditionalUrl;
|
||||
if (consolidateEntryCandidate) {
|
||||
if (isBlank(entryFullUrl)) {
|
||||
if (isNotBlank(conditionalUrl)) {
|
||||
if (!keysWithNoFullUrl.add(key)) {
|
||||
throw new InvalidRequestException(
|
||||
"Unable to process " + theActionName + " - Request contains multiple anonymous entries (Bundle.entry.fullUrl not populated) with conditional URL: \"" + UrlUtil.sanitizeUrlPart(conditionalUrl) + "\". Does transaction request contain duplicates?");
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if (!keyToUuid.containsKey(key)) {
|
||||
keyToUuid.put(key, entryFullUrl);
|
||||
} else {
|
||||
ourLog.info("Discarding transaction bundle entry {} as it contained a duplicate conditional {}", originalIndex, verb);
|
||||
String msg = "Discarding transaction bundle entry " + originalIndex + " as it contained a duplicate conditional " + verb;
|
||||
ourLog.info(msg);
|
||||
// Interceptor broadcast: JPA_PERFTRACE_INFO
|
||||
if (CompositeInterceptorBroadcaster.hasHooks(Pointcut.JPA_PERFTRACE_WARNING, myInterceptorBroadcaster, theRequestDetails)) {
|
||||
StorageProcessingMessage message = new StorageProcessingMessage().setMessage(msg);
|
||||
HookParams params = new HookParams()
|
||||
.add(RequestDetails.class, theRequestDetails)
|
||||
.addIfMatchesType(ServletRequestDetails.class, theRequestDetails)
|
||||
.add(StorageProcessingMessage.class, message);
|
||||
CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.JPA_PERFTRACE_INFO, params);
|
||||
}
|
||||
|
||||
theEntries.remove(index);
|
||||
index--;
|
||||
String existingUuid = keyToUuid.get(key);
|
||||
|
|
@ -760,6 +789,7 @@ public abstract class BaseTransactionProcessor {
|
|||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Iterates over all entries, and if it finds any which have references which match the fullUrl of the entry that was consolidated out
|
||||
|
|
@ -869,7 +899,7 @@ public abstract class BaseTransactionProcessor {
|
|||
/*
|
||||
* Look for duplicate conditional creates and consolidate them
|
||||
*/
|
||||
consolidateDuplicateConditionals(theEntries);
|
||||
consolidateDuplicateConditionals(theRequest, theActionName, theEntries);
|
||||
|
||||
/*
|
||||
* Loop through the request and process any entries of type
|
||||
|
|
|
|||
|
|
@ -56,6 +56,8 @@ public interface ITransactionProcessorVersionAdapter<BUNDLE extends IBaseBundle,
|
|||
|
||||
String getFullUrl(BUNDLEENTRY theEntry);
|
||||
|
||||
void setFullUrl(BUNDLEENTRY theEntry, String theFullUrl);
|
||||
|
||||
String getEntryIfNoneExist(BUNDLEENTRY theEntry);
|
||||
|
||||
String getEntryRequestUrl(BUNDLEENTRY theEntry);
|
||||
|
|
@ -75,4 +77,5 @@ public interface ITransactionProcessorVersionAdapter<BUNDLE extends IBaseBundle,
|
|||
void setRequestVerb(BUNDLEENTRY theEntry, String theVerb);
|
||||
|
||||
void setRequestUrl(BUNDLEENTRY theEntry, String theUrl);
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -130,6 +130,11 @@ public class TransactionProcessorVersionAdapterDstu3 implements ITransactionProc
|
|||
return theEntry.getFullUrl();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void setFullUrl(Bundle.BundleEntryComponent theEntry, String theFullUrl) {
|
||||
theEntry.setFullUrl(theFullUrl);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getEntryIfNoneExist(Bundle.BundleEntryComponent theEntry) {
|
||||
return theEntry.getRequest().getIfNoneExist();
|
||||
|
|
|
|||
|
|
@ -114,6 +114,12 @@ public class TransactionProcessorVersionAdapterR4 implements ITransactionProcess
|
|||
return theEntry.getFullUrl();
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public void setFullUrl(Bundle.BundleEntryComponent theEntry, String theFullUrl) {
|
||||
theEntry.setFullUrl(theFullUrl);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getEntryIfNoneExist(Bundle.BundleEntryComponent theEntry) {
|
||||
return theEntry.getRequest().getIfNoneExist();
|
||||
|
|
|
|||
|
|
@ -178,6 +178,10 @@ public class MemoryCacheService {
|
|||
TAG_DEFINITION(TagDefinitionCacheKey.class),
|
||||
RESOURCE_LOOKUP(String.class),
|
||||
FORCED_ID_TO_PID(String.class),
|
||||
/**
|
||||
* Key type: {@literal Long}
|
||||
* Value type: {@literal Optional<String>}
|
||||
*/
|
||||
PID_TO_FORCED_ID(Long.class),
|
||||
CONCEPT_TRANSLATION(TranslationQuery.class),
|
||||
MATCH_URL(String.class),
|
||||
|
|
|
|||
Loading…
Reference in New Issue