From 43631d4937064c62255268b268d494b1e16850d0 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sat, 8 May 2021 18:09:50 -0400 Subject: [PATCH] Account for NOPs in Auto-Versioned References (#2643) * Account for NOPs in auto-versioned references * Add docs * Add tests --- .../2643-fix-incorrect-autoversion-nop.yaml | 8 + .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 5 + .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 2 +- .../jpa/dao/BaseTransactionProcessor.java | 292 +++++++++++------- ...irResourceDaoR4VersionedReferenceTest.java | 184 ++++++++++- .../fhir/jpa/migrate/taskdef/BaseTask.java | 6 +- 6 files changed, 372 insertions(+), 125 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2643-fix-incorrect-autoversion-nop.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2643-fix-incorrect-autoversion-nop.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2643-fix-incorrect-autoversion-nop.yaml new file mode 100644 index 00000000000..6a50c46d6ce --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2643-fix-incorrect-autoversion-nop.yaml @@ -0,0 +1,8 @@ +--- +type: fix +issue: 2643 +title: "When using Auto-Version references in the JPA server, if an auto-versioned reference + within a FHIR transaction pointed to a resource that did not actually change during the + transaction (e.g. an update/PUT where the resource body was unchanged from the existing + version), the reference would point to an incremented version number even though none existed. + This has been corrected." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 0ecf54258bf..afc8bfb5cbd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1142,6 +1142,11 @@ public abstract class BaseHapiFhirDao extends BaseStora mySearchParamWithInlineReferencesExtractor.populateFromResource(newParams, theTransactionDetails, entity, theResource, existingParams, theRequest); changed = populateResourceIntoEntity(theTransactionDetails, theRequest, theResource, entity, true); + + if (theForceUpdate) { + changed.setChanged(true); + } + if (changed.isChanged()) { entity.setUpdated(theTransactionDetails.getTransactionDate()); if (theResource instanceof IResource) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 11445d37467..76b1ec91de7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -1537,7 +1537,7 @@ public abstract class BaseHapiFhirResourceDao extends B String id = theResource.getIdElement().getValue(); Runnable onRollback = () -> theResource.getIdElement().setValue(id); - // Execute the update in a retriable transasction + // Execute the update in a retriable transaction return myTransactionService.execute(theRequest, tx -> doUpdate(theResource, theMatchUrl, thePerformIndexing, theForceUpdateVersion, theRequest, theTransactionDetails), onRollback); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java index d9e16ce0805..9b3609d3a02 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java @@ -922,118 +922,7 @@ public abstract class BaseTransactionProcessor { * Perform ID substitutions and then index each resource we have saved */ - FhirTerser terser = myContext.newTerser(); - theTransactionStopWatch.startTask("Index " + theIdToPersistedOutcome.size() + " resources"); - int i = 0; - for (DaoMethodOutcome nextOutcome : theIdToPersistedOutcome.values()) { - - if (i++ % 250 == 0) { - ourLog.debug("Have indexed {} entities out of {} in transaction", i, theIdToPersistedOutcome.values().size()); - } - - if (nextOutcome.isNop()) { - continue; - } - - IBaseResource nextResource = nextOutcome.getResource(); - if (nextResource == null) { - continue; - } - - // References - Set referencesToVersion = BaseStorageDao.extractReferencesToAutoVersion(myContext, myModelConfig, nextResource); - List allRefs = terser.getAllResourceReferences(nextResource); - for (ResourceReferenceInfo nextRef : allRefs) { - IBaseReference resourceReference = nextRef.getResourceReference(); - IIdType nextId = resourceReference.getReferenceElement(); - IIdType newId = null; - if (!nextId.hasIdPart()) { - if (resourceReference.getResource() != null) { - IIdType targetId = resourceReference.getResource().getIdElement(); - if (targetId.getValue() == null) { - // This means it's a contained resource - continue; - } else if (theIdSubstitutions.containsValue(targetId)) { - newId = targetId; - } else { - throw new InternalErrorException("References by resource with no reference ID are not supported in DAO layer"); - } - } else { - continue; - } - } - if (newId != null || theIdSubstitutions.containsKey(nextId)) { - if (newId == null) { - newId = theIdSubstitutions.get(nextId); - } - ourLog.debug(" * Replacing resource ref {} with {}", nextId, newId); - if (referencesToVersion.contains(resourceReference)) { - resourceReference.setReference(newId.getValue()); - } else { - resourceReference.setReference(newId.toVersionless().getValue()); - } - } else if (nextId.getValue().startsWith("urn:")) { - throw new InvalidRequestException("Unable to satisfy placeholder ID " + nextId.getValue() + " found in element named '" + nextRef.getName() + "' within resource of type: " + nextResource.getIdElement().getResourceType()); - } else { - if (referencesToVersion.contains(resourceReference)) { - DaoMethodOutcome outcome = theIdToPersistedOutcome.get(nextId); - if (!outcome.isNop() && !Boolean.TRUE.equals(outcome.getCreated())) { - resourceReference.setReference(nextId.getValue()); - } - } - } - } - - // URIs - Class> uriType = (Class>) myContext.getElementDefinition("uri").getImplementingClass(); - List> allUris = terser.getAllPopulatedChildElementsOfType(nextResource, uriType); - for (IPrimitiveType nextRef : allUris) { - if (nextRef instanceof IIdType) { - continue; // No substitution on the resource ID itself! - } - IIdType nextUriString = newIdType(nextRef.getValueAsString()); - if (theIdSubstitutions.containsKey(nextUriString)) { - IIdType newId = theIdSubstitutions.get(nextUriString); - ourLog.debug(" * Replacing resource ref {} with {}", nextUriString, newId); - nextRef.setValueAsString(newId.toVersionless().getValue()); - } else { - ourLog.debug(" * Reference [{}] does not exist in bundle", nextUriString); - } - } - - IPrimitiveType deletedInstantOrNull; - if (nextResource instanceof IAnyResource) { - deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get((IAnyResource) nextResource); - } else { - deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get((IResource) nextResource); - } - Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null; - - IFhirResourceDao dao = myDaoRegistry.getResourceDao(nextResource.getClass()); - IJpaDao jpaDao = (IJpaDao) dao; - - IBasePersistedResource updateOutcome = null; - if (updatedEntities.contains(nextOutcome.getEntity())) { - updateOutcome = jpaDao.updateInternal(theRequest, nextResource, true, false, nextOutcome.getEntity(), nextResource.getIdElement(), nextOutcome.getPreviousResource(), theTransactionDetails); - } else if (!nonUpdatedEntities.contains(nextOutcome.getId())) { - updateOutcome = jpaDao.updateEntity(theRequest, nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, false, theTransactionDetails, false, true); - } - - // Make sure we reflect the actual final version for the resource. - if (updateOutcome != null) { - IIdType newId = updateOutcome.getIdDt(); - for (IIdType nextEntry : entriesToProcess.values()) { - if (nextEntry.getResourceType().equals(newId.getResourceType())) { - if (nextEntry.getIdPart().equals(newId.getIdPart())) { - if (!nextEntry.hasVersionIdPart() || !nextEntry.getVersionIdPart().equals(newId.getVersionIdPart())) { - nextEntry.setParts(nextEntry.getBaseUrl(), nextEntry.getResourceType(), nextEntry.getIdPart(), newId.getVersionIdPart()); - } - } - } - } - } - - } + resolveReferencesThenSaveAndIndexResources(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, theTransactionStopWatch, entriesToProcess, nonUpdatedEntities, updatedEntities); theTransactionStopWatch.endCurrentTask(); theTransactionStopWatch.startTask("Flush writes to database"); @@ -1042,8 +931,6 @@ public abstract class BaseTransactionProcessor { theTransactionStopWatch.endCurrentTask(); - - /* * Double check we didn't allow any duplicates we shouldn't have */ @@ -1093,6 +980,183 @@ public abstract class BaseTransactionProcessor { } } + /** + * This method replaces any placeholder references in the + * source transaction Bundle with their actual targets, then stores the resource contents and indexes + * in the database. This is trickier than you'd think because of a couple of possibilities during the + * save: + * * There may be resources that have not changed (e.g. an update/PUT with a resource body identical + * to what is already in the database) + * * There may be resources with auto-versioned references, meaning we're replacing certain references + * in the resource with a versioned references, referencing the current version at the time of the + * transaction processing + * * There may by auto-versioned references pointing to these unchanged targets + * + * If we're not doing any auto-versioned references, we'll just iterate through all resources in the + * transaction and save them one at a time. + * + * However, if we have any auto-versioned references we do this in 2 passes: First the resources from the + * transaction that don't have any auto-versioned references are stored. We do them first since there's + * a chance they may be a NOP and we'll need to account for their version number not actually changing. + * Then we do a second pass for any resources that have auto-versioned references. These happen in a separate + * pass because it's too complex to try and insert the auto-versioned references and still + * account for NOPs, so we block NOPs in that pass. + */ + private void resolveReferencesThenSaveAndIndexResources(RequestDetails theRequest, TransactionDetails theTransactionDetails, Map theIdSubstitutions, Map theIdToPersistedOutcome, StopWatch theTransactionStopWatch, Map entriesToProcess, Set nonUpdatedEntities, Set updatedEntities) { + FhirTerser terser = myContext.newTerser(); + theTransactionStopWatch.startTask("Index " + theIdToPersistedOutcome.size() + " resources"); + IdentityHashMap> deferredIndexesForAutoVersioning = null; + int i = 0; + for (DaoMethodOutcome nextOutcome : theIdToPersistedOutcome.values()) { + + if (i++ % 250 == 0) { + ourLog.debug("Have indexed {} entities out of {} in transaction", i, theIdToPersistedOutcome.values().size()); + } + + if (nextOutcome.isNop()) { + continue; + } + + IBaseResource nextResource = nextOutcome.getResource(); + if (nextResource == null) { + continue; + } + + Set referencesToAutoVersion = BaseStorageDao.extractReferencesToAutoVersion(myContext, myModelConfig, nextResource); + if (referencesToAutoVersion.isEmpty()) { + resolveReferencesThenSaveAndIndexResource(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, entriesToProcess, nonUpdatedEntities, updatedEntities, terser, nextOutcome, nextResource, referencesToAutoVersion); + } else { + if (deferredIndexesForAutoVersioning == null) { + deferredIndexesForAutoVersioning = new IdentityHashMap<>(); + } + deferredIndexesForAutoVersioning.put(nextOutcome, referencesToAutoVersion); + } + + } + + // If we have any resources we'll be auto-versioning, index these next + if (deferredIndexesForAutoVersioning != null) { + for (Map.Entry> nextEntry : deferredIndexesForAutoVersioning.entrySet()) { + DaoMethodOutcome nextOutcome = nextEntry.getKey(); + Set referencesToAutoVersion = nextEntry.getValue(); + IBaseResource nextResource = nextOutcome.getResource(); + resolveReferencesThenSaveAndIndexResource(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, entriesToProcess, nonUpdatedEntities, updatedEntities, terser, nextOutcome, nextResource, referencesToAutoVersion); + } + } + } + + private void resolveReferencesThenSaveAndIndexResource(RequestDetails theRequest, TransactionDetails theTransactionDetails, Map theIdSubstitutions, Map theIdToPersistedOutcome, Map entriesToProcess, Set nonUpdatedEntities, Set updatedEntities, FhirTerser terser, DaoMethodOutcome nextOutcome, IBaseResource nextResource, Set theReferencesToAutoVersion) { + // References + List allRefs = terser.getAllResourceReferences(nextResource); + for (ResourceReferenceInfo nextRef : allRefs) { + IBaseReference resourceReference = nextRef.getResourceReference(); + IIdType nextId = resourceReference.getReferenceElement(); + IIdType newId = null; + if (!nextId.hasIdPart()) { + if (resourceReference.getResource() != null) { + IIdType targetId = resourceReference.getResource().getIdElement(); + if (targetId.getValue() == null) { + // This means it's a contained resource + continue; + } else if (theIdSubstitutions.containsValue(targetId)) { + newId = targetId; + } else { + throw new InternalErrorException("References by resource with no reference ID are not supported in DAO layer"); + } + } else { + continue; + } + } + if (newId != null || theIdSubstitutions.containsKey(nextId)) { + if (newId == null) { + newId = theIdSubstitutions.get(nextId); + } + ourLog.debug(" * Replacing resource ref {} with {}", nextId, newId); + if (theReferencesToAutoVersion.contains(resourceReference)) { + resourceReference.setReference(newId.getValue()); + } else { + resourceReference.setReference(newId.toVersionless().getValue()); + } + } else if (nextId.getValue().startsWith("urn:")) { + throw new InvalidRequestException("Unable to satisfy placeholder ID " + nextId.getValue() + " found in element named '" + nextRef.getName() + "' within resource of type: " + nextResource.getIdElement().getResourceType()); + } else { + if (theReferencesToAutoVersion.contains(resourceReference)) { + DaoMethodOutcome outcome = theIdToPersistedOutcome.get(nextId); + if (!outcome.isNop() && !Boolean.TRUE.equals(outcome.getCreated())) { + resourceReference.setReference(nextId.getValue()); + } + } + } + } + + // URIs + Class> uriType = (Class>) myContext.getElementDefinition("uri").getImplementingClass(); + List> allUris = terser.getAllPopulatedChildElementsOfType(nextResource, uriType); + for (IPrimitiveType nextRef : allUris) { + if (nextRef instanceof IIdType) { + continue; // No substitution on the resource ID itself! + } + IIdType nextUriString = newIdType(nextRef.getValueAsString()); + if (theIdSubstitutions.containsKey(nextUriString)) { + IIdType newId = theIdSubstitutions.get(nextUriString); + ourLog.debug(" * Replacing resource ref {} with {}", nextUriString, newId); + nextRef.setValueAsString(newId.toVersionless().getValue()); + } else { + ourLog.debug(" * Reference [{}] does not exist in bundle", nextUriString); + } + } + + IPrimitiveType deletedInstantOrNull; + if (nextResource instanceof IAnyResource) { + deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get((IAnyResource) nextResource); + } else { + deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get((IResource) nextResource); + } + Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null; + + IFhirResourceDao dao = myDaoRegistry.getResourceDao(nextResource.getClass()); + IJpaDao jpaDao = (IJpaDao) dao; + + IBasePersistedResource updateOutcome = null; + if (updatedEntities.contains(nextOutcome.getEntity())) { + boolean forceUpdateVersion = false; + if (!theReferencesToAutoVersion.isEmpty()) { + forceUpdateVersion = true; + } + + updateOutcome = jpaDao.updateInternal(theRequest, nextResource, true, forceUpdateVersion, nextOutcome.getEntity(), nextResource.getIdElement(), nextOutcome.getPreviousResource(), theTransactionDetails); + } else if (!nonUpdatedEntities.contains(nextOutcome.getId())) { + updateOutcome = jpaDao.updateEntity(theRequest, nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, false, theTransactionDetails, false, true); + } + + // Make sure we reflect the actual final version for the resource. + if (updateOutcome != null) { + IIdType newId = updateOutcome.getIdDt(); + for (IIdType nextEntry : entriesToProcess.values()) { + if (nextEntry.getResourceType().equals(newId.getResourceType())) { + if (nextEntry.getIdPart().equals(newId.getIdPart())) { + if (!nextEntry.hasVersionIdPart() || !nextEntry.getVersionIdPart().equals(newId.getVersionIdPart())) { + nextEntry.setParts(nextEntry.getBaseUrl(), nextEntry.getResourceType(), nextEntry.getIdPart(), newId.getVersionIdPart()); + } + } + } + } + + nextOutcome.setId(newId); + + for (IIdType next : theIdSubstitutions.values()) { + if (next.getResourceType().equals(newId.getResourceType())) { + if (next.getIdPart().equals(newId.getIdPart())) { + if (!next.getValue().equals(newId.getValue())) { + next.setValue(newId.getValue()); + } + } + } + } + + } + } + private void validateNoDuplicates(RequestDetails theRequest, String theActionName, Map> conditionalRequestUrls) { for (Map.Entry> nextEntry : conditionalRequestUrls.entrySet()) { String matchUrl = nextEntry.getKey(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java index 334c04f027c..76ebb3e0b74 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java @@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.model.entity.ModelConfig; +import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.TokenParam; @@ -11,8 +12,10 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Condition; import org.hl7.fhir.r4.model.Encounter; +import org.hl7.fhir.r4.model.ExplanationOfBenefit; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Observation; +import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Task; @@ -23,8 +26,12 @@ import java.util.Arrays; import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; import java.util.stream.Collectors; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.matchesPattern; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -40,6 +47,167 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { myModelConfig.setAutoVersionReferenceAtPaths(new ModelConfig().getAutoVersionReferenceAtPaths()); } + @Test + public void testCreateAndUpdateVersionedReferencesInTransaction_VersionedReferenceToUpsertWithNop() { + myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); + myModelConfig.setAutoVersionReferenceAtPaths("ExplanationOfBenefit.patient"); + + // We'll submit the same bundle twice. It has an UPSERT (with no changes + // the second time) on a Patient, and a CREATE on an ExplanationOfBenefit + // referencing that Patient. + Supplier supplier = () -> { + BundleBuilder bb = new BundleBuilder(myFhirCtx); + + Patient patient = new Patient(); + patient.setId("Patient/A"); + patient.setActive(true); + bb.addTransactionUpdateEntry(patient); + + ExplanationOfBenefit eob = new ExplanationOfBenefit(); + eob.setId(IdType.newRandomUuid()); + eob.setPatient(new Reference("Patient/A")); + bb.addTransactionCreateEntry(eob); + + return (Bundle) bb.getBundle(); + }; + + // Send it the first time + Bundle outcome1 = mySystemDao.transaction(new SystemRequestDetails(), supplier.get()); + assertEquals("Patient/A/_history/1", outcome1.getEntry().get(0).getResponse().getLocation()); + String eobId1 = outcome1.getEntry().get(1).getResponse().getLocation(); + assertThat(eobId1, matchesPattern("ExplanationOfBenefit/[0-9]+/_history/1")); + + ExplanationOfBenefit eob1 = myExplanationOfBenefitDao.read(new IdType(eobId1), new SystemRequestDetails()); + assertEquals("Patient/A/_history/1", eob1.getPatient().getReference()); + + // Send it again + Bundle outcome2 = mySystemDao.transaction(new SystemRequestDetails(), supplier.get()); + assertEquals("Patient/A/_history/1", outcome2.getEntry().get(0).getResponse().getLocation()); + String eobId2 = outcome2.getEntry().get(1).getResponse().getLocation(); + assertThat(eobId2, matchesPattern("ExplanationOfBenefit/[0-9]+/_history/1")); + + ExplanationOfBenefit eob2 = myExplanationOfBenefitDao.read(new IdType(eobId2), new SystemRequestDetails()); + assertEquals("Patient/A/_history/1", eob2.getPatient().getReference()); + } + + + @Test + public void testCreateAndUpdateVersionedReferencesInTransaction_VersionedReferenceToVersionedReferenceToUpsertWithNop() { + myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); + myModelConfig.setAutoVersionReferenceAtPaths( + "Patient.managingOrganization", + "ExplanationOfBenefit.patient" + ); + + // We'll submit the same bundle twice. It has an UPSERT (with no changes + // the second time) on a Patient, and a CREATE on an ExplanationOfBenefit + // referencing that Patient. + Supplier supplier = () -> { + BundleBuilder bb = new BundleBuilder(myFhirCtx); + + Organization organization = new Organization(); + organization.setId("Organization/O"); + organization.setActive(true); + bb.addTransactionUpdateEntry(organization); + + Patient patient = new Patient(); + patient.setId("Patient/A"); + patient.setManagingOrganization(new Reference("Organization/O")); + patient.setActive(true); + bb.addTransactionUpdateEntry(patient); + + ExplanationOfBenefit eob = new ExplanationOfBenefit(); + eob.setId(IdType.newRandomUuid()); + eob.setPatient(new Reference("Patient/A")); + bb.addTransactionCreateEntry(eob); + + return (Bundle) bb.getBundle(); + }; + + // Send it the first time + Bundle outcome1 = mySystemDao.transaction(new SystemRequestDetails(), supplier.get()); + assertEquals("Organization/O/_history/1", outcome1.getEntry().get(0).getResponse().getLocation()); + assertEquals("Patient/A/_history/1", outcome1.getEntry().get(1).getResponse().getLocation()); + String eobId1 = outcome1.getEntry().get(2).getResponse().getLocation(); + assertThat(eobId1, matchesPattern("ExplanationOfBenefit/[0-9]+/_history/1")); + + ExplanationOfBenefit eob1 = myExplanationOfBenefitDao.read(new IdType(eobId1), new SystemRequestDetails()); + assertEquals("Patient/A/_history/1", eob1.getPatient().getReference()); + + // Send it again + Bundle outcome2 = mySystemDao.transaction(new SystemRequestDetails(), supplier.get()); + assertEquals("Organization/O/_history/1", outcome2.getEntry().get(0).getResponse().getLocation()); + // Technically the patient did not change - If this ever got optimized so that the version here + // was 1 that would be even better + String patientId = outcome2.getEntry().get(1).getResponse().getLocation(); + assertEquals("Patient/A/_history/2", patientId); + String eobId2 = outcome2.getEntry().get(2).getResponse().getLocation(); + assertThat(eobId2, matchesPattern("ExplanationOfBenefit/[0-9]+/_history/1")); + + Patient patient = myPatientDao.read(new IdType("Patient/A"), new SystemRequestDetails()); + assertEquals(patientId, patient.getId()); + + ExplanationOfBenefit eob2 = myExplanationOfBenefitDao.read(new IdType(eobId2), new SystemRequestDetails()); + assertEquals(patientId, eob2.getPatient().getReference()); + } + + @Test + public void testCreateAndUpdateVersionedReferencesInTransaction_VersionedReferenceToVersionedReferenceToUpsertWithChange() { + myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); + myModelConfig.setAutoVersionReferenceAtPaths( + "Patient.managingOrganization", + "ExplanationOfBenefit.patient" + ); + + AtomicInteger counter = new AtomicInteger(); + Supplier supplier = () -> { + BundleBuilder bb = new BundleBuilder(myFhirCtx); + + Organization organization = new Organization(); + organization.setId("Organization/O"); + organization.setName("Org " + counter.incrementAndGet()); // change each time + organization.setActive(true); + bb.addTransactionUpdateEntry(organization); + + Patient patient = new Patient(); + patient.setId("Patient/A"); + patient.setManagingOrganization(new Reference("Organization/O")); + patient.setActive(true); + bb.addTransactionUpdateEntry(patient); + + ExplanationOfBenefit eob = new ExplanationOfBenefit(); + eob.setId(IdType.newRandomUuid()); + eob.setPatient(new Reference("Patient/A")); + bb.addTransactionCreateEntry(eob); + + return (Bundle) bb.getBundle(); + }; + + // Send it the first time + Bundle outcome1 = mySystemDao.transaction(new SystemRequestDetails(), supplier.get()); + assertEquals("Organization/O/_history/1", outcome1.getEntry().get(0).getResponse().getLocation()); + assertEquals("Patient/A/_history/1", outcome1.getEntry().get(1).getResponse().getLocation()); + String eobId1 = outcome1.getEntry().get(2).getResponse().getLocation(); + assertThat(eobId1, matchesPattern("ExplanationOfBenefit/[0-9]+/_history/1")); + + ExplanationOfBenefit eob1 = myExplanationOfBenefitDao.read(new IdType(eobId1), new SystemRequestDetails()); + assertEquals("Patient/A/_history/1", eob1.getPatient().getReference()); + + // Send it again + Bundle outcome2 = mySystemDao.transaction(new SystemRequestDetails(), supplier.get()); + assertEquals("Organization/O/_history/2", outcome2.getEntry().get(0).getResponse().getLocation()); + String patientId = outcome2.getEntry().get(1).getResponse().getLocation(); + assertEquals("Patient/A/_history/2", patientId); + String eobId2 = outcome2.getEntry().get(2).getResponse().getLocation(); + assertThat(eobId2, matchesPattern("ExplanationOfBenefit/[0-9]+/_history/1")); + + Patient patient = myPatientDao.read(new IdType("Patient/A"), new SystemRequestDetails()); + assertEquals(patientId, patient.getId()); + + ExplanationOfBenefit eob2 = myExplanationOfBenefitDao.read(new IdType(eobId2), new SystemRequestDetails()); + assertEquals(patientId, eob2.getPatient().getReference()); + } + @Test public void testStoreAndRetrieveVersionedReference() { myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); @@ -185,7 +353,7 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { } // Verify Patient Version - assertEquals("2", myPatientDao.search(SearchParameterMap.newSynchronous("active", new TokenParam("false"))).getResources(0,1).get(0).getIdElement().getVersionIdPart()); + assertEquals("2", myPatientDao.search(SearchParameterMap.newSynchronous("active", new TokenParam("false"))).getResources(0, 1).get(0).getIdElement().getVersionIdPart()); BundleBuilder builder = new BundleBuilder(myFhirCtx); @@ -430,9 +598,9 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { IBundleProvider outcome = myTaskDao.search(SearchParameterMap.newSynchronous().addInclude(Task.INCLUDE_BASED_ON)); assertEquals(2, outcome.size()); List resources = outcome.getResources(0, 2); - assertEquals(2, resources.size(), resources.stream().map(t->t.getIdElement().toUnqualified().getValue()).collect(Collectors.joining(", "))); + assertEquals(2, resources.size(), resources.stream().map(t -> t.getIdElement().toUnqualified().getValue()).collect(Collectors.joining(", "))); assertEquals(taskId.getValue(), resources.get(0).getIdElement().getValue()); - assertEquals(conditionId.getValue(), ((Task)resources.get(0)).getBasedOn().get(0).getReference()); + assertEquals(conditionId.getValue(), ((Task) resources.get(0)).getBasedOn().get(0).getReference()); assertEquals(conditionId.withVersion("1").getValue(), resources.get(1).getIdElement().getValue()); // Now, update the Condition to generate another version of it @@ -445,7 +613,7 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { resources = outcome.getResources(0, 2); assertEquals(2, resources.size()); assertEquals(taskId.getValue(), resources.get(0).getIdElement().getValue()); - assertEquals(conditionId.getValue(), ((Task)resources.get(0)).getBasedOn().get(0).getReference()); + assertEquals(conditionId.getValue(), ((Task) resources.get(0)).getBasedOn().get(0).getReference()); assertEquals(conditionId.withVersion("1").getValue(), resources.get(1).getIdElement().getValue()); } @@ -478,9 +646,9 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { IBundleProvider outcome = myTaskDao.search(SearchParameterMap.newSynchronous().addInclude(Task.INCLUDE_BASED_ON)); assertEquals(2, outcome.size()); List resources = outcome.getResources(0, 2); - assertEquals(2, resources.size(), resources.stream().map(t->t.getIdElement().toUnqualified().getValue()).collect(Collectors.joining(", "))); + assertEquals(2, resources.size(), resources.stream().map(t -> t.getIdElement().toUnqualified().getValue()).collect(Collectors.joining(", "))); assertEquals(taskId.getValue(), resources.get(0).getIdElement().getValue()); - assertEquals(conditionId.getValue(), ((Task)resources.get(0)).getBasedOn().get(0).getReference()); + assertEquals(conditionId.getValue(), ((Task) resources.get(0)).getBasedOn().get(0).getReference()); assertEquals(conditionId.withVersion("4").getValue(), resources.get(1).getIdElement().getValue()); } @@ -522,9 +690,9 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { IBundleProvider outcome = myTaskDao.search(SearchParameterMap.newSynchronous().addInclude(Task.INCLUDE_BASED_ON)); assertEquals(2, outcome.size()); List resources = outcome.getResources(0, 2); - assertEquals(2, resources.size(), resources.stream().map(t->t.getIdElement().toUnqualified().getValue()).collect(Collectors.joining(", "))); + assertEquals(2, resources.size(), resources.stream().map(t -> t.getIdElement().toUnqualified().getValue()).collect(Collectors.joining(", "))); assertEquals(taskId.getValue(), resources.get(0).getIdElement().getValue()); - assertEquals(conditionId.getValue(), ((Task)resources.get(0)).getBasedOn().get(0).getReference()); + assertEquals(conditionId.getValue(), ((Task) resources.get(0)).getBasedOn().get(0).getReference()); assertEquals(conditionId.withVersion("4").getValue(), resources.get(1).getIdElement().getValue()); } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java index d7bedccaaa8..cbbcb824d10 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java @@ -122,11 +122,13 @@ public abstract class BaseTask { return changesCount; } catch (DataAccessException e) { if (myFailureAllowed) { - ourLog.info("Task did not exit successfully, but task is allowed to fail"); + ourLog.info("Task {} did not exit successfully, but task is allowed to fail", getFlywayVersion()); ourLog.debug("Error was: {}", e.getMessage(), e); return 0; } else { - throw e; + throw new DataAccessException("Failed during task " + getFlywayVersion() + ": " + e, e) { + private static final long serialVersionUID = 8211678931579252166L; + }; } } });