From 07937e4431024bca5e58261af65a8405743c24d3 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 19 Aug 2021 10:35:14 -0400 Subject: [PATCH 01/19] Start with failing test --- ...irResourceDaoR4VersionedReferenceTest.java | 25 ++ .../test/resources/npe-causing-bundle.json | 402 ++++++++++++++++++ 2 files changed, 427 insertions(+) create mode 100644 hapi-fhir-jpaserver-base/src/test/resources/npe-causing-bundle.json 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 76ebb3e0b74..d682d9a6304 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 @@ -1,8 +1,10 @@ package ca.uhn.fhir.jpa.dao.r4; +import ca.uhn.fhir.context.FhirContext; 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.provider.r4.ResourceProviderR4Test; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.TokenParam; @@ -20,8 +22,10 @@ import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Task; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import java.io.InputStreamReader; import java.util.Arrays; import java.util.Date; import java.util.HashSet; @@ -782,4 +786,25 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { } + @Test + @DisplayName("GH-2901 Test no NPE is thrown on autoversioned references") + public void testNoNpeOnEoBBundle() { + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + List strings = Arrays.asList( + "ExplanationOfBenefit.patient", + "ExplanationOfBenefit.insurer", + "ExplanationOfBenefit.provider", + "ExplanationOfBenefit.careTeam.provider", + "ExplanationOfBenefit.insurance.coverage", + "ExplanationOfBenefit.payee.party" + ); + myModelConfig.setAutoVersionReferenceAtPaths(new HashSet(strings)); + + Bundle bundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, new InputStreamReader(FhirResourceDaoR4VersionedReferenceTest.class.getResourceAsStream("/npe-causing-bundle.json"))); + + Bundle transaction = mySystemDao.transaction(new SystemRequestDetails(), bundle); + + } + + } diff --git a/hapi-fhir-jpaserver-base/src/test/resources/npe-causing-bundle.json b/hapi-fhir-jpaserver-base/src/test/resources/npe-causing-bundle.json new file mode 100644 index 00000000000..ef1d7645e5b --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/resources/npe-causing-bundle.json @@ -0,0 +1,402 @@ +{ + "resourceType": "Bundle", + "type": "transaction", + "entry": [ { + "resource": { + "resourceType": "ExplanationOfBenefit", + "id": "26d4cebd-95c6-39ea-855c-dc819bc68d08", + "meta": { + "lastUpdated": "2016-01-01T00:56:00.000-05:00", + "profile": [ "http://hl7.org/fhir/us/carin-bb/StructureDefinition/C4BB-ExplanationOfBenefit-Inpatient-Institutional" ] + }, + "identifier": [ { + "type": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBIdentifierType", + "code": "uc" + } ] + }, + "system": "fhir/CodeSystem/sid/eob-inpatient-claim-id", + "value": "20550047" + } ], + "status": "active", + "type": { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/claim-type", + "code": "institutional" + } ] + }, + "use": "claim", + "patient": { + "reference": "Patient/123" + }, + "billablePeriod": { + "start": "2015-12-14T00:00:00-05:00" + }, + "created": "2021-08-16T13:54:10-04:00", + "insurer": { + "reference": "Organization/1" + }, + "provider": { + "reference": "Organization/b9d22776-1ee9-3843-bc48-b4bf67861483" + }, + "outcome": "complete", + "careTeam": [ { + "sequence": 1, + "provider": { + "reference": "Practitioner/1" + }, + "role": { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/claimcareteamrole", + "code": "primary" + } ] + } + }, { + "sequence": 2, + "provider": { + "reference": "Practitioner/2" + }, + "role": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBClaimCareTeamRole", + "code": "attending" + } ] + } + }, { + "sequence": 3, + "provider": { + "reference": "Practitioner/3" + }, + "role": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBClaimCareTeamRole", + "code": "performing" + } ] + } + } ], + "supportingInfo": [ { + "sequence": 1, + "category": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBSupportingInfoType", + "code": "admissionperiod" + } ] + }, + "timingPeriod": { + "start": "2015-12-14T00:00:00-05:00", + "end": "2016-01-11T14:11:00-05:00" + } + }, { + "sequence": 2, + "category": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBSupportingInfoType", + "code": "clmrecvddate" + } ] + }, + "timingDate": "2018-12-23" + }, { + "sequence": 3, + "category": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBSupportingInfoType", + "code": "admtype" + } ] + }, + "code": { + "coding": [ { + "system": "https://www.nubc.org/CodeSystem/PriorityTypeOfAdmitOrVisit", + "code": "4" + } ] + } + } ], + "diagnosis": [ { + "sequence": 1, + "diagnosisCodeableConcept": { + "coding": [ { + "system": "http://hl7.org/fhir/sid/icd-10-cm", + "code": "Z38.00" + } ] + }, + "type": [ { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/ex-diagnosistype", + "code": "principal" + } ] + } ] + } ], + "insurance": [ { + "focal": true, + "coverage": { + "reference": "Coverage/10" + } + } ], + "total": [ { + "category": { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/adjudication", + "code": "benefit" + } ] + }, + "amount": { + "value": 1039.28 + } + }, { + "category": { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/adjudication", + "code": "submitted" + } ] + }, + "amount": { + "value": 2011 + } + } ] + }, + "request": { + "method": "PUT", + "url": "ExplanationOfBenefit/26d4cebd-95c6-39ea-855c-dc819bc68d08" + } + }, { + "resource": { + "resourceType": "ExplanationOfBenefit", + "id": "a25f1c3a-09b9-3f17-8f1b-0fbbf6391fce", + "meta": { + "lastUpdated": "2016-01-01T00:58:00.000-05:00", + "profile": [ "http://hl7.org/fhir/us/carin-bb/StructureDefinition/C4BB-ExplanationOfBenefit-Inpatient-Institutional" ] + }, + "identifier": [ { + "type": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBIdentifierType", + "code": "uc" + } ] + }, + "system": "fhir/CodeSystem/sid/eob-inpatient-claim-id", + "value": "20586901" + } ], + "status": "active", + "type": { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/claim-type", + "code": "institutional" + } ] + }, + "use": "claim", + "patient": { + "reference": "Patient/123" + }, + "billablePeriod": { + "start": "2015-12-18T00:00:00-05:00" + }, + "created": "2021-08-16T13:54:10-04:00", + "insurer": { + "reference": "Organization/1" + }, + "provider": { + "reference": "Organization/d10823cf-ee15-3a0e-a12e-1509cd18cda4" + }, + "outcome": "complete", + "careTeam": [ { + "sequence": 1, + "provider": { + "reference": "Practitioner/4" + }, + "role": { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/claimcareteamrole", + "code": "primary" + } ] + } + }, { + "sequence": 2, + "provider": { + "reference": "Practitioner/5" + }, + "role": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBClaimCareTeamRole", + "code": "attending" + } ] + } + }, { + "sequence": 3, + "provider": { + "reference": "Practitioner/6" + }, + "role": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBClaimCareTeamRole", + "code": "performing" + } ] + } + } ], + "supportingInfo": [ { + "sequence": 1, + "category": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBSupportingInfoType", + "code": "admissionperiod" + } ] + }, + "timingPeriod": { + "start": "2015-12-18T00:00:00-05:00", + "end": "2016-01-11T14:12:00-05:00" + } + }, { + "sequence": 2, + "category": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBSupportingInfoType", + "code": "clmrecvddate" + } ] + }, + "timingDate": "2018-12-29" + }, { + "sequence": 3, + "category": { + "coding": [ { + "system": "http://hl7.org/fhir/us/carin-bb/CodeSystem/C4BBSupportingInfoType", + "code": "admtype" + } ] + }, + "code": { + "coding": [ { + "system": "https://www.nubc.org/CodeSystem/PriorityTypeOfAdmitOrVisit", + "code": "4" + } ] + } + } ], + "diagnosis": [ { + "sequence": 1, + "diagnosisCodeableConcept": { + "coding": [ { + "system": "http://hl7.org/fhir/sid/icd-10-cm", + "code": "Z38.00" + } ] + }, + "type": [ { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/ex-diagnosistype", + "code": "principal" + } ] + } ] + }, { + "sequence": 2, + "diagnosisCodeableConcept": { + "coding": [ { + "system": "http://hl7.org/fhir/sid/icd-10-cm", + "code": "P96.89" + } ] + }, + "type": [ { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/ex-diagnosistype", + "code": "principal" + } ] + } ] + }, { + "sequence": 3, + "diagnosisCodeableConcept": { + "coding": [ { + "system": "http://hl7.org/fhir/sid/icd-10-cm", + "code": "R25.8" + } ] + }, + "type": [ { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/ex-diagnosistype", + "code": "principal" + } ] + } ] + }, { + "sequence": 4, + "diagnosisCodeableConcept": { + "coding": [ { + "system": "http://hl7.org/fhir/sid/icd-10-cm", + "code": "P08.21" + } ] + }, + "type": [ { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/ex-diagnosistype", + "code": "principal" + } ] + } ] + }, { + "sequence": 5, + "diagnosisCodeableConcept": { + "coding": [ { + "system": "http://hl7.org/fhir/sid/icd-10-cm", + "code": "P92.5" + } ] + }, + "type": [ { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/ex-diagnosistype", + "code": "principal" + } ] + } ] + }, { + "sequence": 6, + "diagnosisCodeableConcept": { + "coding": [ { + "system": "http://hl7.org/fhir/sid/icd-10-cm", + "code": "P92.6" + } ] + }, + "type": [ { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/ex-diagnosistype", + "code": "principal" + } ] + } ] + }, { + "sequence": 7, + "diagnosisCodeableConcept": { + "coding": [ { + "system": "http://hl7.org/fhir/sid/icd-10-cm", + "code": "Z23" + } ] + }, + "type": [ { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/ex-diagnosistype", + "code": "principal" + } ] + } ] + } ], + "insurance": [ { + "focal": true, + "coverage": { + "reference": "Coverage/10" + } + } ], + "total": [ { + "category": { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/adjudication", + "code": "benefit" + } ] + }, + "amount": { + "value": 1421.31 + } + }, { + "category": { + "coding": [ { + "system": "http://terminology.hl7.org/CodeSystem/adjudication", + "code": "submitted" + } ] + }, + "amount": { + "value": 2336 + } + } ] + }, + "request": { + "method": "PUT", + "url": "ExplanationOfBenefit/a25f1c3a-09b9-3f17-8f1b-0fbbf6391fce" + } + } ] } From 19d181989c317a81b2a5a1eed49229419ed37649 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 19 Aug 2021 14:52:48 -0400 Subject: [PATCH 02/19] Add another failing test --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 1 + .../dao/index/DaoResourceLinkResolver.java | 1 - ...irResourceDaoR4VersionedReferenceTest.java | 26 +++++++++++++++++++ .../test/resources/npe-causing-bundle.json | 24 ++++++++--------- 4 files changed, 39 insertions(+), 13 deletions(-) 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 bd2e964e523..1fc0bb8f9d2 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 @@ -1206,6 +1206,7 @@ public abstract class BaseHapiFhirDao extends BaseStora if (thePerformIndexing || ((ResourceTable) theEntity).getVersion() == 1) { newParams = new ResourceIndexedSearchParams(); + //FIX ME GGG: This is where the placeholder references end up getting created, deeeeeep down the stakc. mySearchParamWithInlineReferencesExtractor.populateFromResource(newParams, theTransactionDetails, entity, theResource, existingParams, theRequest, thePerformIndexing); changed = populateResourceIntoEntity(theTransactionDetails, theRequest, theResource, entity, true); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java index a219f39f292..07daf83c2da 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java @@ -94,7 +94,6 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver { throw new InvalidRequestException("Resource " + resName + "/" + idPart + " not found, specified in path: " + theSourcePath); } - resolvedResource = createdTableOpt.get(); } 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 d682d9a6304..7476befb401 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.context.FhirContext; import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.provider.r4.ResourceProviderR4Test; @@ -34,7 +35,9 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; import java.util.stream.Collectors; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.matchesPattern; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -803,7 +806,30 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { Bundle bundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, new InputStreamReader(FhirResourceDaoR4VersionedReferenceTest.class.getResourceAsStream("/npe-causing-bundle.json"))); Bundle transaction = mySystemDao.transaction(new SystemRequestDetails(), bundle); + } + @Test + public void testAutoVersionPathsWithAutoCreatePlaceholders() { + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + + Observation obs = new Observation(); + obs.setId("Observation/CDE"); + obs.setSubject(new Reference("Patient/ABC")); + DaoMethodOutcome update = myObservationDao.create(obs); + Observation resource = (Observation)update.getResource(); + String versionedPatientReference = resource.getSubject().getReference(); + assertThat(versionedPatientReference, is(equalTo("Patient/ABC"))); + + myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); + + obs = new Observation(); + obs.setId("Observation/DEF"); + obs.setSubject(new Reference("Patient/RED")); + update = myObservationDao.create(obs); + resource = (Observation)update.getResource(); + versionedPatientReference = resource.getSubject().getReference(); + + assertThat(versionedPatientReference, is(equalTo("Patient/RED/_history/1"))); } diff --git a/hapi-fhir-jpaserver-base/src/test/resources/npe-causing-bundle.json b/hapi-fhir-jpaserver-base/src/test/resources/npe-causing-bundle.json index ef1d7645e5b..a645b167d35 100644 --- a/hapi-fhir-jpaserver-base/src/test/resources/npe-causing-bundle.json +++ b/hapi-fhir-jpaserver-base/src/test/resources/npe-causing-bundle.json @@ -28,14 +28,14 @@ }, "use": "claim", "patient": { - "reference": "Patient/123" + "reference": "Patient/ABC" }, "billablePeriod": { "start": "2015-12-14T00:00:00-05:00" }, "created": "2021-08-16T13:54:10-04:00", "insurer": { - "reference": "Organization/1" + "reference": "Organization/A" }, "provider": { "reference": "Organization/b9d22776-1ee9-3843-bc48-b4bf67861483" @@ -44,7 +44,7 @@ "careTeam": [ { "sequence": 1, "provider": { - "reference": "Practitioner/1" + "reference": "Practitioner/H" }, "role": { "coding": [ { @@ -55,7 +55,7 @@ }, { "sequence": 2, "provider": { - "reference": "Practitioner/2" + "reference": "Practitioner/I" }, "role": { "coding": [ { @@ -66,7 +66,7 @@ }, { "sequence": 3, "provider": { - "reference": "Practitioner/3" + "reference": "Practitioner/J" }, "role": { "coding": [ { @@ -129,7 +129,7 @@ "insurance": [ { "focal": true, "coverage": { - "reference": "Coverage/10" + "reference": "Coverage/G" } } ], "total": [ { @@ -185,14 +185,14 @@ }, "use": "claim", "patient": { - "reference": "Patient/123" + "reference": "Patient/ABC" }, "billablePeriod": { "start": "2015-12-18T00:00:00-05:00" }, "created": "2021-08-16T13:54:10-04:00", "insurer": { - "reference": "Organization/1" + "reference": "Organization/A" }, "provider": { "reference": "Organization/d10823cf-ee15-3a0e-a12e-1509cd18cda4" @@ -201,7 +201,7 @@ "careTeam": [ { "sequence": 1, "provider": { - "reference": "Practitioner/4" + "reference": "Practitioner/D" }, "role": { "coding": [ { @@ -212,7 +212,7 @@ }, { "sequence": 2, "provider": { - "reference": "Practitioner/5" + "reference": "Practitioner/E" }, "role": { "coding": [ { @@ -223,7 +223,7 @@ }, { "sequence": 3, "provider": { - "reference": "Practitioner/6" + "reference": "Practitioner/F" }, "role": { "coding": [ { @@ -370,7 +370,7 @@ "insurance": [ { "focal": true, "coverage": { - "reference": "Coverage/10" + "reference": "Coverage/G" } } ], "total": [ { From 3556299332cce58ff58d07dd3b660fa7b24cd891 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 19 Aug 2021 17:25:15 -0400 Subject: [PATCH 03/19] Add another failing Test --- .../FhirResourceDaoR4VersionedReferenceTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 7476befb401..f61bf54ae59 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 @@ -832,5 +832,19 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { assertThat(versionedPatientReference, is(equalTo("Patient/RED/_history/1"))); } + @Test + public void testNoNpeMinimal() { + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); + + Observation obs = new Observation(); + obs.setId("Observation/DEF"); + obs.setSubject(new Reference("Patient/RED")); + BundleBuilder builder = new BundleBuilder(myFhirCtx); + builder.addTransactionUpdateEntry(obs); + + mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + } + } From c2e3401185a1795909f1d0b1ba37d849927bd932 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 19 Aug 2021 17:25:51 -0400 Subject: [PATCH 04/19] Move display name --- .../jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f61bf54ae59..f2c661b4522 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 @@ -790,7 +790,6 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { } @Test - @DisplayName("GH-2901 Test no NPE is thrown on autoversioned references") public void testNoNpeOnEoBBundle() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); List strings = Arrays.asList( @@ -833,6 +832,7 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { } @Test + @DisplayName("GH-2901 Test no NPE is thrown on autoversioned references") public void testNoNpeMinimal() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); From b51c7227559fd222040e3ea0dd0038ef53a20ffd Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Wed, 25 Aug 2021 10:41:58 -0400 Subject: [PATCH 05/19] stashing minor refactors --- .../java/ca/uhn/fhir/util/FhirTerser.java | 1 + .../jpa/dao/BaseTransactionProcessor.java | 368 +++++++++++------- .../ca/uhn/fhir/jpa/dao/r4/AAAATests.java | 83 ++++ ...irResourceDaoR4VersionedReferenceTest.java | 15 +- .../uhn/fhir/rest/server/mail/MailConfig.java | 20 + .../ca/uhn/fhir/rest/server/mail/MailSvc.java | 20 + .../model/dstu2/composite/NarrativeDt.java | 16 - 7 files changed, 357 insertions(+), 166 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/AAAATests.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java index 8cc1c9be1de..b0dca947658 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java @@ -961,6 +961,7 @@ public class FhirTerser { for (BaseRuntimeChildDefinition nextChild : childDef.getChildrenAndExtension()) { List values = nextChild.getAccessor().getValues(theElement); + if (values != null) { for (Object nextValueObject : values) { IBase nextValue; 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 f599b5bc435..3e30960b590 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 @@ -240,8 +240,10 @@ public abstract class BaseTransactionProcessor { myVersionAdapter.populateEntryWithOperationOutcome(caughtEx, nextEntry); } - private void handleTransactionCreateOrUpdateOutcome(Map idSubstitutions, Map idToPersistedOutcome, IIdType nextResourceId, DaoMethodOutcome outcome, - IBase newEntry, String theResourceType, IBaseResource theRes, RequestDetails theRequestDetails) { + private void handleTransactionCreateOrUpdateOutcome(Map idSubstitutions, Map idToPersistedOutcome, + IIdType nextResourceId, DaoMethodOutcome outcome, + IBase newEntry, String theResourceType, + IBaseResource theRes, RequestDetails theRequestDetails) { IIdType newId = outcome.getId().toUnqualified(); IIdType resourceId = isPlaceholder(nextResourceId) ? nextResourceId : nextResourceId.toUnqualifiedVersionless(); if (newId.equals(resourceId) == false) { @@ -652,8 +654,129 @@ public abstract class BaseTransactionProcessor { myModelConfig = theModelConfig; } - protected Map doTransactionWriteOperations(final RequestDetails theRequest, String theActionName, TransactionDetails theTransactionDetails, Set theAllIds, - Map theIdSubstitutions, Map theIdToPersistedOutcome, IBaseBundle theResponse, IdentityHashMap theOriginalRequestOrder, List theEntries, StopWatch theTransactionStopWatch) { + /** + * Searches for duplicate conditional creates and consolidates them. + * + * @param theEntries + */ + private void consolidateDuplicateConditionalCreates(List theEntries) { + final HashMap keyToUuid = new HashMap<>(); + for (int index = 0, originalIndex = 0; index < theEntries.size(); index++, originalIndex++) { + IBase nextReqEntry = theEntries.get(index); + IBaseResource resource = myVersionAdapter.getResource(nextReqEntry); + if (resource != null) { + String verb = myVersionAdapter.getEntryRequestVerb(myContext, nextReqEntry); + String entryUrl = 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(entryUrl) && isNotBlank(requestUrl)) { + int questionMarkIndex = requestUrl.indexOf('?'); + if (questionMarkIndex >= 0 && requestUrl.length() > (questionMarkIndex + 1)) { + consolidateEntry = true; + } + } + } + + // Conditional CREATE + if ("POST".equals(verb)) { + if (isNotBlank(entryUrl) && isNotBlank(requestUrl) && isNotBlank(ifNoneExist)) { + if (!entryUrl.equals(requestUrl)) { + consolidateEntry = true; + } + } + } + + if (consolidateEntry) { + if (!keyToUuid.containsKey(key)) { + keyToUuid.put(key, entryUrl); + } else { + ourLog.info("Discarding transaction bundle entry {} as it contained a duplicate conditional {}", originalIndex, verb); + theEntries.remove(index); + index--; + String existingUuid = keyToUuid.get(key); + for (IBase nextEntry : theEntries) { + IBaseResource nextResource = myVersionAdapter.getResource(nextEntry); + for (IBaseReference nextReference : myContext.newTerser().getAllPopulatedChildElementsOfType(nextResource, IBaseReference.class)) { + // We're interested in any references directly to the placeholder ID, but also + // references that have a resource target that has the placeholder ID. + String nextReferenceId = nextReference.getReferenceElement().getValue(); + if (isBlank(nextReferenceId) && nextReference.getResource() != null) { + nextReferenceId = nextReference.getResource().getIdElement().getValue(); + } + if (entryUrl.equals(nextReferenceId)) { + nextReference.setReference(existingUuid); + nextReference.setResource(null); + } + } + } + } + } + } + } + } + + /** + * Retrieves teh next resource id (IIdType) from the base resource and next request entry. + * @param theBaseResource - base resource + * @param theNextReqEntry - next request entry + * @param theAllIds - set of all IIdType values + * @return + */ + private IIdType getNextResourceIdFromBaseResource(IBaseResource theBaseResource, + IBase theNextReqEntry, + Set theAllIds) { + IIdType nextResourceId = null; + if (theBaseResource != null) { + nextResourceId = theBaseResource.getIdElement(); + + String fullUrl = myVersionAdapter.getFullUrl(theNextReqEntry); + if (isNotBlank(fullUrl)) { + IIdType fullUrlIdType = newIdType(fullUrl); + if (isPlaceholder(fullUrlIdType)) { + nextResourceId = fullUrlIdType; + } else if (!nextResourceId.hasIdPart()) { + nextResourceId = fullUrlIdType; + } + } + + if (nextResourceId.hasIdPart() && nextResourceId.getIdPart().matches("[a-zA-Z]+:.*") && !isPlaceholder(nextResourceId)) { + throw new InvalidRequestException("Invalid placeholder ID found: " + nextResourceId.getIdPart() + " - Must be of the form 'urn:uuid:[uuid]' or 'urn:oid:[oid]'"); + } + + if (nextResourceId.hasIdPart() && !nextResourceId.hasResourceType() && !isPlaceholder(nextResourceId)) { + nextResourceId = newIdType(toResourceName(theBaseResource.getClass()), nextResourceId.getIdPart()); + theBaseResource.setId(nextResourceId); + } + + /* + * Ensure that the bundle doesn't have any duplicates, since this causes all kinds of weirdness + */ + if (isPlaceholder(nextResourceId)) { + if (!theAllIds.add(nextResourceId)) { + throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextResourceId)); + } + } else if (nextResourceId.hasResourceType() && nextResourceId.hasIdPart()) { + IIdType nextId = nextResourceId.toUnqualifiedVersionless(); + if (!theAllIds.add(nextId)) { + throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextId)); + } + } + + } + + return nextResourceId; + } + + protected Map doTransactionWriteOperations(final RequestDetails theRequest, String theActionName, + TransactionDetails theTransactionDetails, Set theAllIds, + Map theIdSubstitutions, Map theIdToPersistedOutcome, + IBaseBundle theResponse, IdentityHashMap theOriginalRequestOrder, + List theEntries, StopWatch theTransactionStopWatch) { theTransactionDetails.beginAcceptingDeferredInterceptorBroadcasts( Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, @@ -661,7 +784,6 @@ public abstract class BaseTransactionProcessor { Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED ); try { - Set deletedResources = new HashSet<>(); DeleteConflictList deleteConflicts = new DeleteConflictList(); Map entriesToProcess = new IdentityHashMap<>(); @@ -673,117 +795,20 @@ public abstract class BaseTransactionProcessor { /* * Look for duplicate conditional creates and consolidate them */ - final HashMap keyToUuid = new HashMap<>(); - for (int index = 0, originalIndex = 0; index < theEntries.size(); index++, originalIndex++) { - IBase nextReqEntry = theEntries.get(index); - IBaseResource resource = myVersionAdapter.getResource(nextReqEntry); - if (resource != null) { - String verb = myVersionAdapter.getEntryRequestVerb(myContext, nextReqEntry); - String entryUrl = 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(entryUrl) && isNotBlank(requestUrl)) { - int questionMarkIndex = requestUrl.indexOf('?'); - if (questionMarkIndex >= 0 && requestUrl.length() > (questionMarkIndex + 1)) { - consolidateEntry = true; - } - } - } - - // Conditional CREATE - if ("POST".equals(verb)) { - if (isNotBlank(entryUrl) && isNotBlank(requestUrl) && isNotBlank(ifNoneExist)) { - if (!entryUrl.equals(requestUrl)) { - consolidateEntry = true; - } - } - } - - if (consolidateEntry) { - if (!keyToUuid.containsKey(key)) { - keyToUuid.put(key, entryUrl); - } else { - ourLog.info("Discarding transaction bundle entry {} as it contained a duplicate conditional {}", originalIndex, verb); - theEntries.remove(index); - index--; - String existingUuid = keyToUuid.get(key); - for (IBase nextEntry : theEntries) { - IBaseResource nextResource = myVersionAdapter.getResource(nextEntry); - for (IBaseReference nextReference : myContext.newTerser().getAllPopulatedChildElementsOfType(nextResource, IBaseReference.class)) { - // We're interested in any references directly to the placeholder ID, but also - // references that have a resource target that has the placeholder ID. - String nextReferenceId = nextReference.getReferenceElement().getValue(); - if (isBlank(nextReferenceId) && nextReference.getResource() != null) { - nextReferenceId = nextReference.getResource().getIdElement().getValue(); - } - if (entryUrl.equals(nextReferenceId)) { - nextReference.setReference(existingUuid); - nextReference.setResource(null); - } - } - } - } - } - } - } - + consolidateDuplicateConditionalCreates(theEntries); /* * Loop through the request and process any entries of type * PUT, POST or DELETE */ for (int i = 0; i < theEntries.size(); i++) { - if (i % 250 == 0) { ourLog.debug("Processed {} non-GET entries out of {} in transaction", i, theEntries.size()); } IBase nextReqEntry = theEntries.get(i); IBaseResource res = myVersionAdapter.getResource(nextReqEntry); - IIdType nextResourceId = null; - if (res != null) { - - nextResourceId = res.getIdElement(); - - String fullUrl = myVersionAdapter.getFullUrl(nextReqEntry); - if (isNotBlank(fullUrl)) { - IIdType fullUrlIdType = newIdType(fullUrl); - if (isPlaceholder(fullUrlIdType)) { - nextResourceId = fullUrlIdType; - } else if (!nextResourceId.hasIdPart()) { - nextResourceId = fullUrlIdType; - } - } - - if (nextResourceId.hasIdPart() && nextResourceId.getIdPart().matches("[a-zA-Z]+:.*") && !isPlaceholder(nextResourceId)) { - throw new InvalidRequestException("Invalid placeholder ID found: " + nextResourceId.getIdPart() + " - Must be of the form 'urn:uuid:[uuid]' or 'urn:oid:[oid]'"); - } - - if (nextResourceId.hasIdPart() && !nextResourceId.hasResourceType() && !isPlaceholder(nextResourceId)) { - nextResourceId = newIdType(toResourceName(res.getClass()), nextResourceId.getIdPart()); - res.setId(nextResourceId); - } - - /* - * Ensure that the bundle doesn't have any duplicates, since this causes all kinds of weirdness - */ - if (isPlaceholder(nextResourceId)) { - if (!theAllIds.add(nextResourceId)) { - throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextResourceId)); - } - } else if (nextResourceId.hasResourceType() && nextResourceId.hasIdPart()) { - IIdType nextId = nextResourceId.toUnqualifiedVersionless(); - if (!theAllIds.add(nextId)) { - throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextId)); - } - } - - } + IIdType nextResourceId = getNextResourceIdFromBaseResource(res, nextReqEntry, theAllIds); String verb = myVersionAdapter.getEntryRequestVerb(myContext, nextReqEntry); String resourceType = res != null ? myContext.getResourceType(res) : null; @@ -891,7 +916,8 @@ public abstract class BaseTransactionProcessor { } } - handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequest); + handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, + outcome, nextRespEntry, resourceType, res, theRequest); entriesToProcess.put(nextRespEntry, outcome.getId()); break; } @@ -958,52 +984,24 @@ public abstract class BaseTransactionProcessor { * was also deleted as a part of this transaction, which is why we check this now at the * end. */ - for (Iterator iter = deleteConflicts.iterator(); iter.hasNext(); ) { - DeleteConflict nextDeleteConflict = iter.next(); + checkForDeleteConflicts(deleteConflicts, deletedResources, updatedResources); - /* - * If we have a conflict, it means we can't delete Resource/A because - * Resource/B has a reference to it. We'll ignore that conflict though - * if it turns out we're also deleting Resource/B in this transaction. - */ - if (deletedResources.contains(nextDeleteConflict.getSourceId().toUnqualifiedVersionless().getValue())) { - iter.remove(); - continue; - } - - /* - * And then, this is kind of a last ditch check. It's also ok to delete - * Resource/A if Resource/B isn't being deleted, but it is being UPDATED - * in this transaction, and the updated version of it has no references - * to Resource/A any more. - */ - String sourceId = nextDeleteConflict.getSourceId().toUnqualifiedVersionless().getValue(); - String targetId = nextDeleteConflict.getTargetId().toUnqualifiedVersionless().getValue(); - Optional updatedSource = updatedResources - .stream() - .filter(t -> sourceId.equals(t.getIdElement().toUnqualifiedVersionless().getValue())) - .findFirst(); - if (updatedSource.isPresent()) { - List referencesInSource = myContext.newTerser().getAllResourceReferences(updatedSource.get()); - boolean sourceStillReferencesTarget = referencesInSource - .stream() - .anyMatch(t -> targetId.equals(t.getResourceReference().getReferenceElement().toUnqualifiedVersionless().getValue())); - if (!sourceStillReferencesTarget) { - iter.remove(); - } - } - } - DeleteConflictService.validateDeleteConflictsEmptyOrThrowException(myContext, deleteConflicts); - - theIdToPersistedOutcome.entrySet().forEach(t -> theTransactionDetails.addResolvedResourceId(t.getKey(), t.getValue().getPersistentId())); + theIdToPersistedOutcome.entrySet().forEach(idAndOutcome -> { + theTransactionDetails.addResolvedResourceId(idAndOutcome.getKey(), idAndOutcome.getValue().getPersistentId()); + }); /* * Perform ID substitutions and then index each resource we have saved */ - resolveReferencesThenSaveAndIndexResources(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, theTransactionStopWatch, entriesToProcess, nonUpdatedEntities, updatedEntities); + resolveReferencesThenSaveAndIndexResources(theRequest, theTransactionDetails, + theIdSubstitutions, theIdToPersistedOutcome, + theTransactionStopWatch, entriesToProcess, + nonUpdatedEntities, updatedEntities); theTransactionStopWatch.endCurrentTask(); + + // flush writes to db theTransactionStopWatch.startTask("Flush writes to database"); flushSession(theIdToPersistedOutcome); @@ -1061,6 +1059,53 @@ public abstract class BaseTransactionProcessor { } } + /** + * Checks for any delete conflicts. + * @param theDeleteConflicts - set of delete conflicts + * @param theDeletedResources - set of deleted resources + * @param theUpdatedResources - list of updated resources + */ + private void checkForDeleteConflicts(DeleteConflictList theDeleteConflicts, + Set theDeletedResources, + List theUpdatedResources) { + for (Iterator iter = theDeleteConflicts.iterator(); iter.hasNext(); ) { + DeleteConflict nextDeleteConflict = iter.next(); + + /* + * If we have a conflict, it means we can't delete Resource/A because + * Resource/B has a reference to it. We'll ignore that conflict though + * if it turns out we're also deleting Resource/B in this transaction. + */ + if (theDeletedResources.contains(nextDeleteConflict.getSourceId().toUnqualifiedVersionless().getValue())) { + iter.remove(); + continue; + } + + /* + * And then, this is kind of a last ditch check. It's also ok to delete + * Resource/A if Resource/B isn't being deleted, but it is being UPDATED + * in this transaction, and the updated version of it has no references + * to Resource/A any more. + */ + String sourceId = nextDeleteConflict.getSourceId().toUnqualifiedVersionless().getValue(); + String targetId = nextDeleteConflict.getTargetId().toUnqualifiedVersionless().getValue(); + Optional updatedSource = theUpdatedResources + .stream() + .filter(t -> sourceId.equals(t.getIdElement().toUnqualifiedVersionless().getValue())) + .findFirst(); + if (updatedSource.isPresent()) { + List referencesInSource = myContext.newTerser().getAllResourceReferences(updatedSource.get()); + boolean sourceStillReferencesTarget = referencesInSource + .stream() + .anyMatch(t -> targetId.equals(t.getResourceReference().getReferenceElement().toUnqualifiedVersionless().getValue())); + if (!sourceStillReferencesTarget) { + iter.remove(); + } + } + } + DeleteConflictService.validateDeleteConflictsEmptyOrThrowException(myContext, theDeleteConflicts); + } + /** * This method replaces any placeholder references in the * source transaction Bundle with their actual targets, then stores the resource contents and indexes @@ -1083,7 +1128,10 @@ public abstract class BaseTransactionProcessor { * 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) { + 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; @@ -1105,14 +1153,28 @@ public abstract class BaseTransactionProcessor { Set referencesToAutoVersion = BaseStorageDao.extractReferencesToAutoVersion(myContext, myModelConfig, nextResource); if (referencesToAutoVersion.isEmpty()) { - resolveReferencesThenSaveAndIndexResource(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, entriesToProcess, nonUpdatedEntities, updatedEntities, terser, nextOutcome, nextResource, referencesToAutoVersion); + // no references to autoversion - we can do the resolve and save now + resolveReferencesThenSaveAndIndexResource(theRequest, theTransactionDetails, + theIdSubstitutions, theIdToPersistedOutcome, + entriesToProcess, nonUpdatedEntities, + updatedEntities, terser, + nextOutcome, nextResource, + referencesToAutoVersion); // this is empty } else { if (deferredIndexesForAutoVersioning == null) { deferredIndexesForAutoVersioning = new IdentityHashMap<>(); } deferredIndexesForAutoVersioning.put(nextOutcome, referencesToAutoVersion); - } + // TODO - add the references to the + // idsToPersistedOutcomes +// for (IBaseReference autoVersion: referencesToAutoVersion) { +// IBaseResource resource = myVersionAdapter.getResource(autoVersion); +// IFhirResourceDao dao = getDaoOrThrowException(resource.getClass()); +// +// } +// theIdToPersistedOutcome.put() + } } // If we have any resources we'll be auto-versioning, index these next @@ -1121,12 +1183,22 @@ public abstract class BaseTransactionProcessor { DaoMethodOutcome nextOutcome = nextEntry.getKey(); Set referencesToAutoVersion = nextEntry.getValue(); IBaseResource nextResource = nextOutcome.getResource(); - resolveReferencesThenSaveAndIndexResource(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, entriesToProcess, nonUpdatedEntities, updatedEntities, terser, nextOutcome, nextResource, referencesToAutoVersion); + 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) { + 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) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/AAAATests.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/AAAATests.java new file mode 100644 index 00000000000..8f92f333392 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/AAAATests.java @@ -0,0 +1,83 @@ +package ca.uhn.fhir.jpa.dao.r4; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.ParserOptions; +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; +import ca.uhn.fhir.jpa.model.entity.ModelConfig; +import ca.uhn.fhir.jpa.partition.SystemRequestDetails; +import ca.uhn.fhir.jpa.provider.r4.ResourceProviderR4Test; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.util.BundleBuilder; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.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; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.io.InputStreamReader; +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.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.matchesPattern; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class AAAATests extends BaseJpaR4Test { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4VersionedReferenceTest.class); + + @AfterEach + public void afterEach() { + myFhirCtx.getParserOptions().setStripVersionsFromReferences(true); + myFhirCtx.getParserOptions().getDontStripVersionsFromReferencesAtPaths().clear(); + myDaoConfig.setDeleteEnabled(new DaoConfig().isDeleteEnabled()); + myModelConfig.setRespectVersionsForSearchIncludes(new ModelConfig().isRespectVersionsForSearchIncludes()); + myModelConfig.setAutoVersionReferenceAtPaths(new ModelConfig().getAutoVersionReferenceAtPaths()); + } + + + @Test + @DisplayName("GH-2901 Test no NPE is thrown on autoversioned references") + public void testNoNpeMinimal() { + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(false); + myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); + +// ParserOptions options = new ParserOptions(); +// options.setDontStripVersionsFromReferencesAtPaths("Observation.subject"); +// myFhirCtx.setParserOptions(options); + + Patient patient = new Patient(); + patient.setId("Patient/RED"); + myPatientDao.update(patient); + + Observation obs = new Observation(); + obs.setId("Observation/DEF"); + obs.setSubject(new Reference("Patient/RED")); + BundleBuilder builder = new BundleBuilder(myFhirCtx); + builder.addTransactionUpdateEntry(obs); + + mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + } + + +} 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 f2c661b4522..73063fe38cb 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 @@ -1,6 +1,7 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.ParserOptions; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.model.entity.ModelConfig; @@ -802,7 +803,9 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { ); myModelConfig.setAutoVersionReferenceAtPaths(new HashSet(strings)); - Bundle bundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, new InputStreamReader(FhirResourceDaoR4VersionedReferenceTest.class.getResourceAsStream("/npe-causing-bundle.json"))); + Bundle bundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, + new InputStreamReader( + FhirResourceDaoR4VersionedReferenceTest.class.getResourceAsStream("/npe-causing-bundle.json"))); Bundle transaction = mySystemDao.transaction(new SystemRequestDetails(), bundle); } @@ -834,9 +837,17 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { @Test @DisplayName("GH-2901 Test no NPE is thrown on autoversioned references") public void testNoNpeMinimal() { - myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(false); myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); +// ParserOptions options = new ParserOptions(); +// options.setDontStripVersionsFromReferencesAtPaths("Observation.subject"); +// myFhirCtx.setParserOptions(options); + + Patient patient = new Patient(); + patient.setId("Patient/RED"); + myPatientDao.update(patient); + Observation obs = new Observation(); obs.setId("Observation/DEF"); obs.setSubject(new Reference("Patient/RED")); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/MailConfig.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/MailConfig.java index 02bb16f9fdc..2202f27b77e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/MailConfig.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/MailConfig.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.rest.server.mail; +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/MailSvc.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/MailSvc.java index 5c87c4256dd..df1cd4e9dca 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/MailSvc.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/MailSvc.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.rest.server.mail; +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + import org.apache.commons.lang3.Validate; import org.simplejavamail.MailException; import org.simplejavamail.api.email.Email; diff --git a/hapi-fhir-structures-dstu2/src/main/java/ca/uhn/fhir/model/dstu2/composite/NarrativeDt.java b/hapi-fhir-structures-dstu2/src/main/java/ca/uhn/fhir/model/dstu2/composite/NarrativeDt.java index ef74811416d..085008a62b1 100644 --- a/hapi-fhir-structures-dstu2/src/main/java/ca/uhn/fhir/model/dstu2/composite/NarrativeDt.java +++ b/hapi-fhir-structures-dstu2/src/main/java/ca/uhn/fhir/model/dstu2/composite/NarrativeDt.java @@ -1,19 +1,3 @@ - - - - - - - - - - - - - - - - package ca.uhn.fhir.model.dstu2.composite; /* From 646592b1b7cd685b22f3308cae752f7d9834a8a7 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Wed, 25 Aug 2021 15:56:55 -0400 Subject: [PATCH 06/19] issue-2901 some refactoring --- .../jpa/dao/BaseTransactionProcessor.java | 188 ++++++++++-------- .../ca/uhn/fhir/jpa/dao/r4/AAAATests.java | 83 -------- ...irResourceDaoR4VersionedReferenceTest.java | 7 - 3 files changed, 105 insertions(+), 173 deletions(-) delete mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/AAAATests.java 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 3e30960b590..866b3e780d9 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 @@ -384,7 +384,8 @@ public abstract class BaseTransactionProcessor { myHapiTransactionService = theHapiTransactionService; } - private IBaseBundle processTransaction(final RequestDetails theRequestDetails, final IBaseBundle theRequest, final String theActionName, boolean theNestedMode) { + private IBaseBundle processTransaction(final RequestDetails theRequestDetails, final IBaseBundle theRequest, + final String theActionName, boolean theNestedMode) { validateDependencies(); String transactionType = myVersionAdapter.getBundleType(theRequest); @@ -440,10 +441,11 @@ public abstract class BaseTransactionProcessor { List getEntries = new ArrayList<>(); final IdentityHashMap originalRequestOrder = new IdentityHashMap<>(); for (int i = 0; i < requestEntries.size(); i++) { - originalRequestOrder.put(requestEntries.get(i), i); + IBase requestEntry = requestEntries.get(i); + originalRequestOrder.put(requestEntry, i); myVersionAdapter.addEntry(response); - if (myVersionAdapter.getEntryRequestVerb(myContext, requestEntries.get(i)).equals("GET")) { - getEntries.add(requestEntries.get(i)); + if (myVersionAdapter.getEntryRequestVerb(myContext, requestEntry).equals("GET")) { + getEntries.add(requestEntry); } } @@ -462,73 +464,17 @@ public abstract class BaseTransactionProcessor { } entries.sort(new TransactionSorter(placeholderIds)); - doTransactionWriteOperations(theRequestDetails, theActionName, transactionDetails, transactionStopWatch, response, originalRequestOrder, entries); + // perform all writes + doTransactionWriteOperations(theRequestDetails, theActionName, + transactionDetails, transactionStopWatch, + response, originalRequestOrder, entries); - /* - * Loop through the request and process any entries of type GET - */ - if (getEntries.size() > 0) { - transactionStopWatch.startTask("Process " + getEntries.size() + " GET entries"); - } - for (IBase nextReqEntry : getEntries) { - - if (theNestedMode) { - throw new InvalidRequestException("Can not invoke read operation on nested transaction"); - } - - if (!(theRequestDetails instanceof ServletRequestDetails)) { - throw new MethodNotAllowedException("Can not call transaction GET methods from this context"); - } - - ServletRequestDetails srd = (ServletRequestDetails) theRequestDetails; - Integer originalOrder = originalRequestOrder.get(nextReqEntry); - IBase nextRespEntry = (IBase) myVersionAdapter.getEntries(response).get(originalOrder); - - ArrayListMultimap paramValues = ArrayListMultimap.create(); - - String transactionUrl = extractTransactionUrlOrThrowException(nextReqEntry, "GET"); - - ServletSubRequestDetails requestDetails = ServletRequestUtil.getServletSubRequestDetails(srd, transactionUrl, paramValues); - - String url = requestDetails.getRequestPath(); - - BaseMethodBinding method = srd.getServer().determineResourceMethod(requestDetails, url); - if (method == null) { - throw new IllegalArgumentException("Unable to handle GET " + url); - } - - if (isNotBlank(myVersionAdapter.getEntryRequestIfMatch(nextReqEntry))) { - requestDetails.addHeader(Constants.HEADER_IF_MATCH, myVersionAdapter.getEntryRequestIfMatch(nextReqEntry)); - } - if (isNotBlank(myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry))) { - requestDetails.addHeader(Constants.HEADER_IF_NONE_EXIST, myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry)); - } - if (isNotBlank(myVersionAdapter.getEntryRequestIfNoneMatch(nextReqEntry))) { - requestDetails.addHeader(Constants.HEADER_IF_NONE_MATCH, myVersionAdapter.getEntryRequestIfNoneMatch(nextReqEntry)); - } - - Validate.isTrue(method instanceof BaseResourceReturningMethodBinding, "Unable to handle GET {}", url); - try { - - BaseResourceReturningMethodBinding methodBinding = (BaseResourceReturningMethodBinding) method; - requestDetails.setRestOperationType(methodBinding.getRestOperationType()); - - IBaseResource resource = methodBinding.doInvokeServer(srd.getServer(), requestDetails); - if (paramValues.containsKey(Constants.PARAM_SUMMARY) || paramValues.containsKey(Constants.PARAM_CONTENT)) { - resource = filterNestedBundle(requestDetails, resource); - } - myVersionAdapter.setResource(nextRespEntry, resource); - myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(Constants.STATUS_HTTP_200_OK)); - } catch (NotModifiedException e) { - myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(Constants.STATUS_HTTP_304_NOT_MODIFIED)); - } catch (BaseServerResponseException e) { - ourLog.info("Failure processing transaction GET {}: {}", url, e.toString()); - myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(e.getStatusCode())); - populateEntryWithOperationOutcome(e, nextRespEntry); - } - - } - transactionStopWatch.endCurrentTask(); + // perform all gets + // (we do these last so that the gets happen on the final state of the DB; + // see above note) + doTransactionReadOperations(theRequestDetails, response, + getEntries, originalRequestOrder, + transactionStopWatch, theNestedMode); // Interceptor broadcast: JPA_PERFTRACE_INFO if (CompositeInterceptorBroadcaster.hasHooks(Pointcut.JPA_PERFTRACE_INFO, myInterceptorBroadcaster, theRequestDetails)) { @@ -545,6 +491,74 @@ public abstract class BaseTransactionProcessor { return response; } + private void doTransactionReadOperations(final RequestDetails theRequestDetails, IBaseBundle theResponse, + List theGetEntries, IdentityHashMap theOriginalRequestOrder, + StopWatch theTransactionStopWatch, boolean theNestedMode) { + if (theGetEntries.size() > 0) { + theTransactionStopWatch.startTask("Process " + theGetEntries.size() + " GET entries"); + + /* + * Loop through the request and process any entries of type GET + */ + for (IBase nextReqEntry : theGetEntries) { + if (theNestedMode) { + throw new InvalidRequestException("Can not invoke read operation on nested transaction"); + } + + if (!(theRequestDetails instanceof ServletRequestDetails)) { + throw new MethodNotAllowedException("Can not call transaction GET methods from this context"); + } + + ServletRequestDetails srd = (ServletRequestDetails) theRequestDetails; + Integer originalOrder = theOriginalRequestOrder.get(nextReqEntry); + IBase nextRespEntry = (IBase) myVersionAdapter.getEntries(theResponse).get(originalOrder); + + ArrayListMultimap paramValues = ArrayListMultimap.create(); + + String transactionUrl = extractTransactionUrlOrThrowException(nextReqEntry, "GET"); + + ServletSubRequestDetails requestDetails = ServletRequestUtil.getServletSubRequestDetails(srd, transactionUrl, paramValues); + + String url = requestDetails.getRequestPath(); + + BaseMethodBinding method = srd.getServer().determineResourceMethod(requestDetails, url); + if (method == null) { + throw new IllegalArgumentException("Unable to handle GET " + url); + } + + if (isNotBlank(myVersionAdapter.getEntryRequestIfMatch(nextReqEntry))) { + requestDetails.addHeader(Constants.HEADER_IF_MATCH, myVersionAdapter.getEntryRequestIfMatch(nextReqEntry)); + } + if (isNotBlank(myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry))) { + requestDetails.addHeader(Constants.HEADER_IF_NONE_EXIST, myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry)); + } + if (isNotBlank(myVersionAdapter.getEntryRequestIfNoneMatch(nextReqEntry))) { + requestDetails.addHeader(Constants.HEADER_IF_NONE_MATCH, myVersionAdapter.getEntryRequestIfNoneMatch(nextReqEntry)); + } + + Validate.isTrue(method instanceof BaseResourceReturningMethodBinding, "Unable to handle GET {}", url); + try { + BaseResourceReturningMethodBinding methodBinding = (BaseResourceReturningMethodBinding) method; + requestDetails.setRestOperationType(methodBinding.getRestOperationType()); + + IBaseResource resource = methodBinding.doInvokeServer(srd.getServer(), requestDetails); + if (paramValues.containsKey(Constants.PARAM_SUMMARY) || paramValues.containsKey(Constants.PARAM_CONTENT)) { + resource = filterNestedBundle(requestDetails, resource); + } + myVersionAdapter.setResource(nextRespEntry, resource); + myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(Constants.STATUS_HTTP_200_OK)); + } catch (NotModifiedException e) { + myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(Constants.STATUS_HTTP_304_NOT_MODIFIED)); + } catch (BaseServerResponseException e) { + ourLog.info("Failure processing transaction GET {}: {}", url, e.toString()); + myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(e.getStatusCode())); + populateEntryWithOperationOutcome(e, nextRespEntry); + } + } + theTransactionStopWatch.endCurrentTask(); + } + } + /** * All of the write operations in the transaction (PUT, POST, etc.. basically anything * except GET) are performed in their own database transaction before we do the reads. @@ -554,7 +568,10 @@ public abstract class BaseTransactionProcessor { * heavy load with lots of concurrent transactions using all available * database connections. */ - private void doTransactionWriteOperations(RequestDetails theRequestDetails, String theActionName, TransactionDetails theTransactionDetails, StopWatch theTransactionStopWatch, IBaseBundle theResponse, IdentityHashMap theOriginalRequestOrder, List theEntries) { + private void doTransactionWriteOperations(RequestDetails theRequestDetails, String theActionName, + TransactionDetails theTransactionDetails, StopWatch theTransactionStopWatch, + IBaseBundle theResponse, IdentityHashMap theOriginalRequestOrder, + List theEntries) { TransactionWriteOperationsDetails writeOperationsDetails = null; if (CompositeInterceptorBroadcaster.hasHooks(Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_PRE, myInterceptorBroadcaster, theRequestDetails) || CompositeInterceptorBroadcaster.hasHooks(Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_POST, myInterceptorBroadcaster, theRequestDetails)) { @@ -583,14 +600,18 @@ public abstract class BaseTransactionProcessor { .add(TransactionDetails.class, theTransactionDetails) .add(TransactionWriteOperationsDetails.class, writeOperationsDetails); CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_PRE, params); - } TransactionCallback> txCallback = status -> { final Set allIds = new LinkedHashSet<>(); final Map idSubstitutions = new HashMap<>(); final Map idToPersistedOutcome = new HashMap<>(); - Map retVal = doTransactionWriteOperations(theRequestDetails, theActionName, theTransactionDetails, allIds, idSubstitutions, idToPersistedOutcome, theResponse, theOriginalRequestOrder, theEntries, theTransactionStopWatch); + + Map retVal = doTransactionWriteOperations(theRequestDetails, theActionName, + theTransactionDetails, allIds, + idSubstitutions, idToPersistedOutcome, + theResponse, theOriginalRequestOrder, + theEntries, theTransactionStopWatch); theTransactionStopWatch.startTask("Commit writes to database"); return retVal; @@ -721,7 +742,7 @@ public abstract class BaseTransactionProcessor { } /** - * Retrieves teh next resource id (IIdType) from the base resource and next request entry. + * Retrieves the next resource id (IIdType) from the base resource and next request entry. * @param theBaseResource - base resource * @param theNextReqEntry - next request entry * @param theAllIds - set of all IIdType values @@ -1161,19 +1182,11 @@ public abstract class BaseTransactionProcessor { nextOutcome, nextResource, referencesToAutoVersion); // this is empty } else { + // we have autoversioned things to defer until later if (deferredIndexesForAutoVersioning == null) { deferredIndexesForAutoVersioning = new IdentityHashMap<>(); } deferredIndexesForAutoVersioning.put(nextOutcome, referencesToAutoVersion); - - // TODO - add the references to the - // idsToPersistedOutcomes -// for (IBaseReference autoVersion: referencesToAutoVersion) { -// IBaseResource resource = myVersionAdapter.getResource(autoVersion); -// IFhirResourceDao dao = getDaoOrThrowException(resource.getClass()); -// -// } -// theIdToPersistedOutcome.put() } } @@ -1183,6 +1196,15 @@ public abstract class BaseTransactionProcessor { DaoMethodOutcome nextOutcome = nextEntry.getKey(); Set referencesToAutoVersion = nextEntry.getValue(); IBaseResource nextResource = nextOutcome.getResource(); + + //TODO - should we add the autoversioned resources to our idtoPersistedoutcomes here? +// for (IBaseReference autoVersionRef : referencesToAutoVersion) { +// IBaseResource baseResource = myVersionAdapter.getResource(autoVersionRef); +// IFhirResourceDao dao = getDaoOrThrowException(baseResource.getClass()); +// +// theIdToPersistedOutcome.put(baseResource.getIdElement(), ); +// } + resolveReferencesThenSaveAndIndexResource(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, entriesToProcess, nonUpdatedEntities, diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/AAAATests.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/AAAATests.java deleted file mode 100644 index 8f92f333392..00000000000 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/AAAATests.java +++ /dev/null @@ -1,83 +0,0 @@ -package ca.uhn.fhir.jpa.dao.r4; - -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.ParserOptions; -import ca.uhn.fhir.jpa.api.config.DaoConfig; -import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; -import ca.uhn.fhir.jpa.model.entity.ModelConfig; -import ca.uhn.fhir.jpa.partition.SystemRequestDetails; -import ca.uhn.fhir.jpa.provider.r4.ResourceProviderR4Test; -import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; -import ca.uhn.fhir.rest.api.server.IBundleProvider; -import ca.uhn.fhir.rest.param.TokenParam; -import ca.uhn.fhir.util.BundleBuilder; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; -import org.hl7.fhir.r4.model.Bundle; -import org.hl7.fhir.r4.model.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; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; - -import java.io.InputStreamReader; -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.CoreMatchers.is; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.matchesPattern; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - -public class AAAATests extends BaseJpaR4Test { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4VersionedReferenceTest.class); - - @AfterEach - public void afterEach() { - myFhirCtx.getParserOptions().setStripVersionsFromReferences(true); - myFhirCtx.getParserOptions().getDontStripVersionsFromReferencesAtPaths().clear(); - myDaoConfig.setDeleteEnabled(new DaoConfig().isDeleteEnabled()); - myModelConfig.setRespectVersionsForSearchIncludes(new ModelConfig().isRespectVersionsForSearchIncludes()); - myModelConfig.setAutoVersionReferenceAtPaths(new ModelConfig().getAutoVersionReferenceAtPaths()); - } - - - @Test - @DisplayName("GH-2901 Test no NPE is thrown on autoversioned references") - public void testNoNpeMinimal() { - myDaoConfig.setAutoCreatePlaceholderReferenceTargets(false); - myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); - -// ParserOptions options = new ParserOptions(); -// options.setDontStripVersionsFromReferencesAtPaths("Observation.subject"); -// myFhirCtx.setParserOptions(options); - - Patient patient = new Patient(); - patient.setId("Patient/RED"); - myPatientDao.update(patient); - - Observation obs = new Observation(); - obs.setId("Observation/DEF"); - obs.setSubject(new Reference("Patient/RED")); - BundleBuilder builder = new BundleBuilder(myFhirCtx); - builder.addTransactionUpdateEntry(obs); - - mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); - } - - -} 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 73063fe38cb..043883b5515 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 @@ -303,7 +303,6 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); - BundleBuilder builder = new BundleBuilder(myFhirCtx); Patient patient = new Patient(); @@ -334,7 +333,6 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { observation = myObservationDao.read(observationId); assertEquals(patientId.getValue(), observation.getSubject().getReference()); assertEquals(encounterId.toVersionless().getValue(), observation.getEncounter().getReference()); - } @Test @@ -397,7 +395,6 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { assertEquals(patientId.getValue(), observation.getSubject().getReference()); assertEquals("2", observation.getSubject().getReferenceElement().getVersionIdPart()); assertEquals(encounterId.toVersionless().getValue(), observation.getEncounter().getReference()); - } @@ -417,7 +414,6 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { // Update patient to make a second version patient.setActive(false); myPatientDao.update(patient); - } BundleBuilder builder = new BundleBuilder(myFhirCtx); @@ -466,7 +462,6 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { // Update patient to make a second version patient.setActive(false); myPatientDao.update(patient); - } BundleBuilder builder = new BundleBuilder(myFhirCtx); @@ -499,10 +494,8 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { // Read back and verify that reference is now versioned observation = myObservationDao.read(observationId); assertEquals(patientId.getValue(), observation.getSubject().getReference()); - } - @Test public void testSearchAndIncludeVersionedReference_Asynchronous() { myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); From 99cd8651741dde3ab30e7f65a3a1bae7380bb188 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 26 Aug 2021 08:47:45 -0400 Subject: [PATCH 07/19] test --- .../main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java | 1 + 1 file changed, 1 insertion(+) 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 866b3e780d9..2ab95f57992 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 @@ -1205,6 +1205,7 @@ public abstract class BaseTransactionProcessor { // theIdToPersistedOutcome.put(baseResource.getIdElement(), ); // } + resolveReferencesThenSaveAndIndexResource(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, entriesToProcess, nonUpdatedEntities, From e7c6ce920d9aed0067f6fa73a8f527b022bb3df1 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Mon, 30 Aug 2021 10:29:59 -0400 Subject: [PATCH 08/19] issue-2901 checkin to test --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 2 +- .../jpa/dao/BaseTransactionProcessor.java | 81 ++++++++++++++++--- ...irResourceDaoCreatePlaceholdersR4Test.java | 60 ++++++++++++++ ...irResourceDaoR4VersionedReferenceTest.java | 81 ++++++++++++++++--- 4 files changed, 205 insertions(+), 19 deletions(-) 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 1fc0bb8f9d2..7716efca962 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 @@ -1206,7 +1206,7 @@ public abstract class BaseHapiFhirDao extends BaseStora if (thePerformIndexing || ((ResourceTable) theEntity).getVersion() == 1) { newParams = new ResourceIndexedSearchParams(); - //FIX ME GGG: This is where the placeholder references end up getting created, deeeeeep down the stakc. + //FIXME GGG: This is where the placeholder references end up getting created, deeeeeep down the stakc. mySearchParamWithInlineReferencesExtractor.populateFromResource(newParams, theTransactionDetails, entity, theResource, existingParams, theRequest, thePerformIndexing); changed = populateResourceIntoEntity(theTransactionDetails, theRequest, theResource, entity, true); 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 2ab95f57992..ea7d3251d0a 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 @@ -61,6 +61,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; import ca.uhn.fhir.rest.server.exceptions.NotModifiedException; import ca.uhn.fhir.rest.server.exceptions.PayloadTooLargeException; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.method.BaseMethodBinding; import ca.uhn.fhir.rest.server.method.BaseResourceReturningMethodBinding; @@ -403,7 +404,8 @@ public abstract class BaseTransactionProcessor { throw new InvalidRequestException("Unable to process transaction where incoming Bundle.type = " + transactionType); } - int numberOfEntries = myVersionAdapter.getEntries(theRequest).size(); + List requestEntries = myVersionAdapter.getEntries(theRequest); + int numberOfEntries = requestEntries.size(); if (myDaoConfig.getMaximumTransactionBundleSize() != null && numberOfEntries > myDaoConfig.getMaximumTransactionBundleSize()) { throw new PayloadTooLargeException("Transaction Bundle Too large. Transaction bundle contains " + @@ -416,7 +418,27 @@ public abstract class BaseTransactionProcessor { final TransactionDetails transactionDetails = new TransactionDetails(); final StopWatch transactionStopWatch = new StopWatch(); - List requestEntries = myVersionAdapter.getEntries(theRequest); + //TODO + // Create Autoreference Placeholders is enabled + // we should create any sub reference that doesn't already exist +// if (myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { +// List referencesToCreate = new ArrayList<>(); +// for (IBase entry : requestEntries) { +// IBaseResource resource = myVersionAdapter.getResource(entry); +// Set referencesToAutoVersion = BaseStorageDao.extractReferencesToAutoVersion(myContext, +// myModelConfig, +// resource); +// for (IBaseReference subReference : referencesToAutoVersion) { +// // doesn't exist - add it to the entries to be created +// org.hl7.fhir.r4.model.Bundle.BundleEntryRequestComponent req = new org.hl7.fhir.r4.model.Bundle.BundleEntryRequestComponent(); +// req.setMethod(org.hl7.fhir.r4.model.Bundle.HTTPVerb.PUT); +// req.setUrl(subReference.getReferenceElement().getValue()); +//// req.setIfNoneExist(); //TODO - look at conditional creates to see how this works +// referencesToCreate.add(req); // adding the request should add it to theIdToPersistence map later +// } +// } +// requestEntries.addAll(referencesToCreate); +// } // Do all entries have a verb? for (int i = 0; i < numberOfEntries; i++) { @@ -488,6 +510,9 @@ public abstract class BaseTransactionProcessor { CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.JPA_PERFTRACE_INFO, params); } + //FIXME - remove before committing + ourLog.info(myContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(response)); + return response; } @@ -614,13 +639,22 @@ public abstract class BaseTransactionProcessor { theEntries, theTransactionStopWatch); theTransactionStopWatch.startTask("Commit writes to database"); + ourLog.info("Retval has " + retVal.values().size()); return retVal; }; Map entriesToProcess; try { entriesToProcess = myHapiTransactionService.execute(theRequestDetails, theTransactionDetails, txCallback); - } finally { + } + catch (Exception ex) { + //FIXME - remove + ourLog.info("FindME"); + ourLog.info(ex.getLocalizedMessage()); + ex.printStackTrace(); + entriesToProcess = new HashMap<>(); + } + finally { if (writeOperationsDetails != null) { HookParams params = new HookParams() .add(TransactionDetails.class, theTransactionDetails) @@ -1198,14 +1232,24 @@ public abstract class BaseTransactionProcessor { IBaseResource nextResource = nextOutcome.getResource(); //TODO - should we add the autoversioned resources to our idtoPersistedoutcomes here? + // idToPersistedOutcomes has no entry for them at this point (if they were not in the + // top level bundle) + // should we just add no-op values to the map (since they are all going to be gets)? // for (IBaseReference autoVersionRef : referencesToAutoVersion) { -// IBaseResource baseResource = myVersionAdapter.getResource(autoVersionRef); -// IFhirResourceDao dao = getDaoOrThrowException(baseResource.getClass()); -// -// theIdToPersistedOutcome.put(baseResource.getIdElement(), ); +// IFhirResourceDao dao = myDaoRegistry.getResourceDao(autoVersionRef.getReferenceElement().getResourceType()); +// IBaseResource baseResource; +// try { +// baseResource = dao.read(autoVersionRef.getReferenceElement()); +// } catch (ResourceNotFoundException ex) { + // not found +// baseResource = +// DaoMethodOutcome outcome = new DaoMethodOutcome(); +// outcome.setResource(baseResource); +// outcome.setNop(true); // we are just reading it +// theIdToPersistedOutcome.put(autoVersionRef.getReferenceElement(), outcome); +// } // } - resolveReferencesThenSaveAndIndexResource(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, entriesToProcess, nonUpdatedEntities, @@ -1263,8 +1307,27 @@ public abstract class BaseTransactionProcessor { 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)) { + + //TODO - if we get here and there's still no + // value in theIdToPersistedOutcome map + // we should throw an invalid request exception + DaoMethodOutcome outcome = theIdToPersistedOutcome.get(nextId); - if (!outcome.isNop() && !Boolean.TRUE.equals(outcome.getCreated())) { + + if (outcome == null) { + IFhirResourceDao dao = myDaoRegistry.getResourceDao(resourceReference.getReferenceElement().getResourceType()); + IBaseResource baseResource; + try { + // DB hit + baseResource = dao.read(resourceReference.getReferenceElement()); + } catch (ResourceNotFoundException ex) { + // does not exist - add the history/1 + String val = resourceReference.getReferenceElement().getValue(); + resourceReference.getReferenceElement().setValue(val + "/_history/1"); + } + } + + if (outcome != null && !outcome.isNop() && !Boolean.TRUE.equals(outcome.getCreated())) { addRollbackReferenceRestore(theTransactionDetails, resourceReference); resourceReference.setReference(nextId.getValue()); resourceReference.setResource(null); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java index c00baf2f536..0451769f045 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java @@ -1,17 +1,20 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.api.config.DaoConfig; +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.ReferenceParam; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; +import ca.uhn.fhir.util.BundleBuilder; import ca.uhn.fhir.util.HapiExtensions; import com.google.common.collect.Sets; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.AuditEvent; import org.hl7.fhir.r4.model.BooleanType; +import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Extension; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Identifier; @@ -21,6 +24,8 @@ import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Task; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import java.util.List; @@ -565,6 +570,61 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { } + @Test + public void testAutocreatePlaceholderTest() { + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + + Observation obs = new Observation(); + obs.setId("Observation/DEF"); + Reference patientRef = new Reference("Patient/RED"); + obs.setSubject(patientRef); + BundleBuilder builder = new BundleBuilder(myFhirCtx); + builder.addTransactionUpdateEntry(obs); + + mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + + Patient returned = myPatientDao.read(patientRef.getReferenceElement()); + Assertions.assertTrue(returned != null); + } + @Test + public void testAutocreatePlaceholderWithTargetExistingAlreadyTest() { + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + + myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); + + String patientId = "Patient/RED"; + + // create + Patient patient = new Patient(); +// patient.setId(patientId); + patient.setIdElement(new IdType(patientId)); +// patient.setActive(true); + myPatientDao.update(patient); + + // update + patient.setActive(true); + myPatientDao.update(patient); + + // observation (with version 2) + Observation obs = new Observation(); + obs.setId("Observation/DEF"); + Reference patientRef = new Reference(patientId); + obs.setSubject(patientRef); + BundleBuilder builder = new BundleBuilder(myFhirCtx); + builder.addTransactionUpdateEntry(obs); + + Bundle transaction = mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + + Patient returned = myPatientDao.read(patientRef.getReferenceElement()); + Assertions.assertTrue(returned != null); + Assertions.assertTrue(returned.getActive()); + + Observation retObservation = myObservationDao.read(obs.getIdElement()); + Assertions.assertTrue(retObservation != null); + } + + // always work with the put + // conditional create (replace put with conditional create?) } 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 043883b5515..efc8510c5c7 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 @@ -1,12 +1,10 @@ package ca.uhn.fhir.jpa.dao.r4; -import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.ParserOptions; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; -import ca.uhn.fhir.jpa.provider.r4.ResourceProviderR4Test; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.TokenParam; @@ -24,10 +22,12 @@ import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Task; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import java.io.InputStreamReader; +import java.sql.Ref; import java.util.Arrays; import java.util.Date; import java.util.HashSet; @@ -298,6 +298,53 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { assertEquals(patientIdString, observation.getSubject().getReference()); } + + @Test + public void testInsertVersionedReferenceAtPathUsingTransaction() { +// myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); +// myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); + + Patient p = new Patient(); + p.setActive(true); + IIdType patientId = myPatientDao.create(p).getId().toUnqualified(); + assertEquals("1", patientId.getVersionIdPart()); + assertEquals(null, patientId.getBaseUrl()); + String patientIdString = patientId.getValue(); + + // Create - put an unversioned reference in the subject + Observation observation = new Observation(); + observation.getSubject().setReference(patientId.toVersionless().getValue()); + + BundleBuilder builder = new BundleBuilder(myFhirCtx); + builder.addTransactionUpdateEntry(observation); + + Bundle transaction = mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + + Observation ret = myObservationDao.read(observation.getIdElement()); + Assertions.assertTrue(ret != null); + + // Read back and verify that reference is now versioned +// observation = myObservationDao.read(observationId); +// assertEquals(patientIdString, observation.getSubject().getReference()); +// +// myCaptureQueriesListener.clear(); +// +// // Update - put an unversioned reference in the subject +// observation = new Observation(); +// observation.setId(observationId); +// observation.addIdentifier().setSystem("http://foo").setValue("bar"); +// observation.getSubject().setReference(patientId.toVersionless().getValue()); +// myObservationDao.update(observation); +// +// // Make sure we're not introducing any extra DB operations +// assertEquals(5, myCaptureQueriesListener.logSelectQueries().size()); +// +// // Read back and verify that reference is now versioned +// observation = myObservationDao.read(observationId); +// assertEquals(patientIdString, observation.getSubject().getReference()); + } + @Test public void testInsertVersionedReferenceAtPath_InTransaction_SourceAndTargetBothCreated() { myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); @@ -827,27 +874,43 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { assertThat(versionedPatientReference, is(equalTo("Patient/RED/_history/1"))); } + + @Test @DisplayName("GH-2901 Test no NPE is thrown on autoversioned references") public void testNoNpeMinimal() { - myDaoConfig.setAutoCreatePlaceholderReferenceTargets(false); + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); // ParserOptions options = new ParserOptions(); // options.setDontStripVersionsFromReferencesAtPaths("Observation.subject"); // myFhirCtx.setParserOptions(options); - Patient patient = new Patient(); - patient.setId("Patient/RED"); - myPatientDao.update(patient); - Observation obs = new Observation(); obs.setId("Observation/DEF"); - obs.setSubject(new Reference("Patient/RED")); + Reference patientRef = new Reference("Patient/RED"); + obs.setSubject(patientRef); BundleBuilder builder = new BundleBuilder(myFhirCtx); builder.addTransactionUpdateEntry(obs); - mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + //1 make sure this test throws the InvalidRequestException (make separate test for this) + //2 add a test for patient created before bundle and then process observation with reference to patient (null check for outcome) + //3 + +// Assertions.assertThrows() + try { + Bundle returnedTr = mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + + System.out.println("returned " + returnedTr.getEntry().size()); + Observation obRet = myObservationDao.read(obs.getIdElement()); + System.out.println("HELLO " + obRet.getId()); + Patient returned = myPatientDao.read(patientRef.getReferenceElement()); + Assertions.assertTrue(returned != null); + } + catch (Exception ex) { + System.out.println("TEST " + ex.getLocalizedMessage()); + Assertions.assertTrue(ex == null); + } } From a38b7911415053ba64e6f15d63404f1a9dbe6d1a Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Tue, 31 Aug 2021 10:12:40 -0400 Subject: [PATCH 09/19] issue-2901 added db access method for existence and setting the version to 1 if createplaceholders is true and object does not exist in db --- .../fhir/jpa/api/dao/IFhirResourceDao.java | 9 +++ .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 17 +++++ .../fhir/jpa/dao/BaseHapiFhirSystemDao.java | 1 + .../jpa/dao/BaseTransactionProcessor.java | 70 ++++++++++++++----- .../uhn/fhir/jpa/dao/data/IForcedIdDao.java | 7 ++ ...irResourceDaoR4VersionedReferenceTest.java | 27 ++++--- 6 files changed, 98 insertions(+), 33 deletions(-) diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java index ff8fa2a2303..f9f9bb39345 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java @@ -161,6 +161,15 @@ public interface IFhirResourceDao extends IDao { */ T read(IIdType theId); + /** + * Helper method to determine if some resources exist in the DB (without throwing). + * Returns a set that contains the IIdType for every resource found. + * If it's not found, it won't be included in the set. + * @param theIds - list of IIdType ids (for the same resource) + * @return + */ + Set hasResources(Collection theIds); + /** * Read a resource by its internal PID */ 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 2d2911b9f92..5476e89352b 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 @@ -37,6 +37,7 @@ import ca.uhn.fhir.jpa.api.model.LazyDaoMethodOutcome; import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; import ca.uhn.fhir.jpa.delete.DeleteConflictService; +import ca.uhn.fhir.jpa.model.cross.IResourceLookup; import ca.uhn.fhir.jpa.model.entity.BaseHasResource; import ca.uhn.fhir.jpa.model.entity.BaseTag; import ca.uhn.fhir.jpa.model.entity.ForcedId; @@ -1156,6 +1157,22 @@ public abstract class BaseHapiFhirResourceDao extends B return myTransactionService.execute(theRequest, transactionDetails, tx -> doRead(theId, theRequest, theDeletedOk)); } + @Override + public Set hasResources(Collection theIds) { + List idPortions = theIds.stream().map(t -> t.getIdPart()).collect(Collectors.toList()); + Collection matches = myForcedIdDao.findResourcesByForcedId(getResourceName(), + idPortions); + + HashSet collected = new HashSet<>(); + for (Object[] match : matches) { + String resourceType = (String) match[0]; + String forcedId = (String) match[1]; + collected.add(new IdDt(resourceType, forcedId)); + } + + return collected; + } + public T doRead(IIdType theId, RequestDetails theRequest, boolean theDeletedOk) { assert TransactionSynchronizationManager.isActualTransactionActive(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java index 32d37758dde..909cd1bf481 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java @@ -7,6 +7,7 @@ import ca.uhn.fhir.jpa.util.ResourceCountCache; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.util.StopWatch; import com.google.common.annotations.VisibleForTesting; 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 ea7d3251d0a..6f137a5bfe3 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 @@ -650,6 +650,7 @@ public abstract class BaseTransactionProcessor { catch (Exception ex) { //FIXME - remove ourLog.info("FindME"); + ourLog.info(ex.getLocalizedMessage()); ex.printStackTrace(); entriesToProcess = new HashMap<>(); @@ -1306,28 +1307,61 @@ public abstract class BaseTransactionProcessor { } 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)) { + // resource type -> set of Ids + // we'll populate this with only those resource/ids of + // resource references that: + // a) do not exist in the idToPersistedOutcome + // (so not in top level of bundle) + // b) do not exist in DB + // (so newly created resources) + // + // we only do this if autocreateplaceholders is on + HashMap> resourceTypeToListOfIds = new HashMap<>(); + if (myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { + for (IBaseReference ref : theReferencesToAutoVersion) { + IIdType id = ref.getReferenceElement(); + // if we don't have this in our idToPersistedOutcome + // and we have createplaceholderreferences on + // we will have to check if these objects exist in the DB + if (!theIdToPersistedOutcome.containsKey(id)) { + if (!resourceTypeToListOfIds.containsKey(id.getResourceType())) { + resourceTypeToListOfIds.put(id.getResourceType(), new HashSet<>()); + } - //TODO - if we get here and there's still no - // value in theIdToPersistedOutcome map - // we should throw an invalid request exception - - DaoMethodOutcome outcome = theIdToPersistedOutcome.get(nextId); - - if (outcome == null) { - IFhirResourceDao dao = myDaoRegistry.getResourceDao(resourceReference.getReferenceElement().getResourceType()); - IBaseResource baseResource; - try { - // DB hit - baseResource = dao.read(resourceReference.getReferenceElement()); - } catch (ResourceNotFoundException ex) { - // does not exist - add the history/1 - String val = resourceReference.getReferenceElement().getValue(); - resourceReference.getReferenceElement().setValue(val + "/_history/1"); + resourceTypeToListOfIds.get(id.getResourceType()).add(id); } } - if (outcome != null && !outcome.isNop() && !Boolean.TRUE.equals(outcome.getCreated())) { + for (String resourceType : resourceTypeToListOfIds.keySet()) { + IFhirResourceDao dao = myDaoRegistry.getResourceDao(resourceType); + Set idSet = resourceTypeToListOfIds.get(resourceType); + + // DB hit :( + Set existing = dao.hasResources(idSet); + + // we remove all the ids that are found, leaving + // only the ids of those not in the DB at all + idSet.removeAll(existing); + } + } + + if (theReferencesToAutoVersion.contains(resourceReference)) { + DaoMethodOutcome outcome = theIdToPersistedOutcome.get(nextId); + + if (outcome == null && myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { + // null outcome means it's a resource reference that was not at top of bundle + IIdType id = resourceReference.getReferenceElement(); + String resourceType = id.getResourceType(); + // if it exists in resourceTypeToListOfIds + // it's not in the DB (new resource) + Set ids = resourceTypeToListOfIds.get(resourceType); + if (!ids.contains(id)) { + // doesn't exist in the DB + // which means the history necessarily is 1 (first one) + resourceReference.getReferenceElement().setValue(id.getValue() + "/_history/1"); + } + } + else if (outcome != null && !outcome.isNop() && !Boolean.TRUE.equals(outcome.getCreated())) { addRollbackReferenceRestore(theTransactionDetails, resourceReference); resourceReference.setReference(nextId.getValue()); resourceReference.setResource(null); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java index 3a10f6be0d5..2a747ab089d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java @@ -111,6 +111,13 @@ public interface IForcedIdDao extends JpaRepository { "WHERE f.myResourceType = :resource_type AND f.myForcedId IN ( :forced_id )") Collection findAndResolveByForcedIdWithNoType(@Param("resource_type") String theResourceType, @Param("forced_id") Collection theForcedIds); + @Query("" + + "SELECT " + + " f.myResourceType, f.myForcedId " + + "FROM ForcedId f " + + "WHERE f.myResourceType = :resource_type AND f.myForcedId IN ( :forced_id )") + Collection findResourcesByForcedId(@Param("resource_type") String theResourceType, @Param("forced_id") Collection theForcedIds); + /** * This method returns a Collection where each row is an element in the collection. Each element in the collection * is an object array, where the order matters (the array represents columns returned by the query). Be careful if you change this query in any way. 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 efc8510c5c7..a4e65f4ee4d 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 @@ -301,8 +301,8 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { @Test public void testInsertVersionedReferenceAtPathUsingTransaction() { -// myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); -// myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); Patient p = new Patient(); @@ -893,24 +893,21 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { BundleBuilder builder = new BundleBuilder(myFhirCtx); builder.addTransactionUpdateEntry(obs); + Bundle submitted = (Bundle)builder.getBundle(); + //1 make sure this test throws the InvalidRequestException (make separate test for this) //2 add a test for patient created before bundle and then process observation with reference to patient (null check for outcome) //3 -// Assertions.assertThrows() - try { - Bundle returnedTr = mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + Bundle returnedTr = mySystemDao.transaction(new SystemRequestDetails(), submitted); - System.out.println("returned " + returnedTr.getEntry().size()); - Observation obRet = myObservationDao.read(obs.getIdElement()); - System.out.println("HELLO " + obRet.getId()); - Patient returned = myPatientDao.read(patientRef.getReferenceElement()); - Assertions.assertTrue(returned != null); - } - catch (Exception ex) { - System.out.println("TEST " + ex.getLocalizedMessage()); - Assertions.assertTrue(ex == null); - } + Assertions.assertTrue(returnedTr != null); + + // some verification + Observation obRet = myObservationDao.read(obs.getIdElement()); + Assertions.assertTrue(obRet != null); + Patient returned = myPatientDao.read(patientRef.getReferenceElement()); + Assertions.assertTrue(returned != null); } From 0286eb52783eb658a9c60752912b77fe2770f54b Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 2 Sep 2021 09:15:39 -0400 Subject: [PATCH 10/19] issue-2901 updated some documentation and added support for autocreated placeholder / autoversioned references at paths when in transactios and have not been created yet --- .../uhn/hapi/fhir/docs/server_jpa/schema.md | 10 +- .../fhir/jpa/api/dao/IFhirResourceDao.java | 2 +- .../ca/uhn/fhir/jpa/config/BaseConfig.java | 7 + .../jpa/dao/AutoVersioningServiceImpl.java | 42 ++++++ .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 28 +++- .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 44 ++++-- .../jpa/dao/BaseTransactionProcessor.java | 128 ++++------------- .../fhir/jpa/dao/IAutoVersioningService.java | 23 +++ .../uhn/fhir/jpa/dao/data/IForcedIdDao.java | 3 +- .../dao/AutoVersioningServiceImplTests.java | 62 ++++++++ .../jpa/dao/BaseHapiFhirResourceDaoTest.java | 132 ++++++++++++++++++ .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 3 + ...irResourceDaoR4VersionedReferenceTest.java | 116 +++++++-------- 13 files changed, 418 insertions(+), 182 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/schema.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/schema.md index af83007725e..17499751bb8 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/schema.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/schema.md @@ -302,6 +302,14 @@ If the server has been configured with a [Resource Server ID Strategy](/apidocs/ Contains the specific version (starting with 1) of the resource that this row corresponds to. + + RESOURCE_TYPE + + String + + Contains the string specifying the type of the resource (Patient, Observation, etc). + + @@ -476,7 +484,7 @@ The following columns are common to **all HFJ_SPIDX_xxx tables**. RES_ID FK to HFJ_RESOURCE - String + Long Contains the PID of the resource being indexed. diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java index f9f9bb39345..69d0716fd4c 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java @@ -168,7 +168,7 @@ public interface IFhirResourceDao extends IDao { * @param theIds - list of IIdType ids (for the same resource) * @return */ - Set hasResources(Collection theIds); + Map getIdsOfExistingResources(Collection theIds); /** * Read a resource by its internal PID diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java index 2aebff2b539..fb58b1912e0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java @@ -27,9 +27,11 @@ import ca.uhn.fhir.jpa.bulk.imprt.api.IBulkDataImportSvc; import ca.uhn.fhir.jpa.bulk.imprt.svc.BulkDataImportSvcImpl; import ca.uhn.fhir.jpa.cache.IResourceVersionSvc; import ca.uhn.fhir.jpa.cache.ResourceVersionSvcDaoImpl; +import ca.uhn.fhir.jpa.dao.AutoVersioningServiceImpl; import ca.uhn.fhir.jpa.dao.DaoSearchParamProvider; import ca.uhn.fhir.jpa.dao.HistoryBuilder; import ca.uhn.fhir.jpa.dao.HistoryBuilderFactory; +import ca.uhn.fhir.jpa.dao.IAutoVersioningService; import ca.uhn.fhir.jpa.dao.ISearchBuilder; import ca.uhn.fhir.jpa.dao.LegacySearchBuilder; import ca.uhn.fhir.jpa.dao.MatchResourceUrlService; @@ -230,6 +232,11 @@ public abstract class BaseConfig { public void initSettings() { } + @Bean + public IAutoVersioningService autoVersioningService() { + return new AutoVersioningServiceImpl(); + } + public void setSearchCoordCorePoolSize(Integer searchCoordCorePoolSize) { this.searchCoordCorePoolSize = searchCoordCorePoolSize; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java new file mode 100644 index 00000000000..1953273dbed --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java @@ -0,0 +1,42 @@ +package ca.uhn.fhir.jpa.dao; + +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; +import org.hl7.fhir.instance.model.api.IIdType; +import org.springframework.beans.factory.annotation.Autowired; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class AutoVersioningServiceImpl implements IAutoVersioningService { + + @Autowired + private DaoRegistry myDaoRegistry; + + @Override + public Map getAutoversionsForIds(Collection theIds) { + HashMap idToPID = new HashMap<>(); + HashMap> resourceTypeToIds = new HashMap<>(); + + for (IIdType id : theIds) { + String resourceType = id.getResourceType(); + if (!resourceTypeToIds.containsKey(resourceType)) { + resourceTypeToIds.put(resourceType, new ArrayList<>()); + } + resourceTypeToIds.get(resourceType).add(id); + } + + for (String resourceType : resourceTypeToIds.keySet()) { + IFhirResourceDao dao = myDaoRegistry.getResourceDao(resourceType); + + Map idAndPID = dao.getIdsOfExistingResources(resourceTypeToIds.get(resourceType)); + idToPID.putAll(idAndPID); + } + + return idToPID; + } +} 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 5476e89352b..667cb450d71 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 @@ -138,9 +138,11 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; @@ -1158,16 +1160,34 @@ public abstract class BaseHapiFhirResourceDao extends B } @Override - public Set hasResources(Collection theIds) { - List idPortions = theIds.stream().map(t -> t.getIdPart()).collect(Collectors.toList()); + public Map getIdsOfExistingResources(Collection theIds) { + // these are the found Ids that were in the db + HashMap collected = new HashMap<>(); + + if (theIds == null || theIds.isEmpty()) { + return collected; + } + + List idPortions = theIds + .stream() + .map(IIdType::getIdPart) + .collect(Collectors.toList()); + Collection matches = myForcedIdDao.findResourcesByForcedId(getResourceName(), idPortions); - HashSet collected = new HashSet<>(); for (Object[] match : matches) { String resourceType = (String) match[0]; String forcedId = (String) match[1]; - collected.add(new IdDt(resourceType, forcedId)); + Long pid = (Long) match[2]; + Long versionId = (Long) match[3]; + IIdType id = new IdDt(resourceType, forcedId); + // we won't put the version on the IIdType + // so callers can use this as a lookup to match to + ResourcePersistentId persistentId = new ResourcePersistentId(pid); + persistentId.setVersion(versionId); + + collected.put(id, persistentId); } return collected; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java index f181a5760ed..2ad93b1d994 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java @@ -27,7 +27,6 @@ import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; -import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.model.LazyDaoMethodOutcome; import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; @@ -35,9 +34,6 @@ import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.util.JpaParamUtil; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; -import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; -import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster; import ca.uhn.fhir.model.api.IQueryParameterAnd; import ca.uhn.fhir.rest.api.QualifiedParamList; import ca.uhn.fhir.rest.api.server.IPreResourceAccessDetails; @@ -45,12 +41,16 @@ import ca.uhn.fhir.rest.api.server.IPreResourceShowDetails; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.SimplePreResourceAccessDetails; import ca.uhn.fhir.rest.api.server.SimplePreResourceShowDetails; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; import ca.uhn.fhir.rest.param.QualifierDetails; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster; +import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.util.BundleUtil; import ca.uhn.fhir.util.FhirTerser; import ca.uhn.fhir.util.OperationOutcomeUtil; @@ -91,6 +91,10 @@ public abstract class BaseStorageDao { protected DaoRegistry myDaoRegistry; @Autowired protected ModelConfig myModelConfig; + @Autowired + protected IAutoVersioningService myAutoVersioningService; + @Autowired + protected DaoConfig myDaoConfig; @VisibleForTesting public void setSearchParamRegistry(ISearchParamRegistry theSearchParamRegistry) { @@ -204,10 +208,34 @@ public abstract class BaseStorageDao { for (IBaseReference nextReference : referencesToVersion) { IIdType referenceElement = nextReference.getReferenceElement(); if (!referenceElement.hasBaseUrl()) { - String resourceType = referenceElement.getResourceType(); - IFhirResourceDao dao = myDaoRegistry.getResourceDao(resourceType); - String targetVersionId = dao.getCurrentVersionId(referenceElement); - String newTargetReference = referenceElement.withVersion(targetVersionId).getValue(); + + Map idToPID = myAutoVersioningService.getAutoversionsForIds( + Collections.singletonList(referenceElement) + ); + + // 3 cases: + // 1) there exists a resource in the db with some version (use this version) + // 2) no resource exists, but we will create one (eventually). The version is 1 + // 3) no resource exists, and none will be made -> throw + Long version; + if (idToPID.containsKey(referenceElement)) { + // the resource exists... latest id + // will bbe the value in the ResourcePersistentId + version = idToPID.get(referenceElement).getVersion(); + } + else if (myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { + // if idToPID doesn't contain object + // but autcreateplaceholders is on + // then the version will be 1 (the first version) + version = 1L; + } + else { + // resource not found + // and no autocreateplaceholders set... + // we throw + throw new ResourceNotFoundException(referenceElement); + } + String newTargetReference = referenceElement.withVersion(version.toString()).getValue(); nextReference.setReference(newTargetReference); } } 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 6f137a5bfe3..eb3a51e1c87 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 @@ -52,6 +52,7 @@ import ca.uhn.fhir.rest.api.PreferReturnEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; import ca.uhn.fhir.rest.param.ParameterUtil; import ca.uhn.fhir.rest.server.RestfulServerUtils; @@ -61,7 +62,6 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; import ca.uhn.fhir.rest.server.exceptions.NotModifiedException; import ca.uhn.fhir.rest.server.exceptions.PayloadTooLargeException; -import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.method.BaseMethodBinding; import ca.uhn.fhir.rest.server.method.BaseResourceReturningMethodBinding; @@ -69,11 +69,11 @@ import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.rest.server.servlet.ServletSubRequestDetails; import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster; import ca.uhn.fhir.rest.server.util.ServletRequestUtil; +import ca.uhn.fhir.util.AsyncUtil; import ca.uhn.fhir.util.ElementUtil; import ca.uhn.fhir.util.FhirTerser; import ca.uhn.fhir.util.ResourceReferenceInfo; import ca.uhn.fhir.util.StopWatch; -import ca.uhn.fhir.util.AsyncUtil; import ca.uhn.fhir.util.UrlUtil; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ArrayListMultimap; @@ -116,10 +116,11 @@ import java.util.Optional; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.Callable; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; +import java.util.stream.Collectors; import static ca.uhn.fhir.util.StringUtil.toUtf8String; import static org.apache.commons.lang3.StringUtils.defaultString; @@ -153,6 +154,9 @@ public abstract class BaseTransactionProcessor { @Autowired private InMemoryResourceMatcher myInMemoryResourceMatcher; + @Autowired + private IAutoVersioningService myAutoVersioningService; + private ThreadPoolTaskExecutor myExecutor ; @VisibleForTesting @@ -418,28 +422,6 @@ public abstract class BaseTransactionProcessor { final TransactionDetails transactionDetails = new TransactionDetails(); final StopWatch transactionStopWatch = new StopWatch(); - //TODO - // Create Autoreference Placeholders is enabled - // we should create any sub reference that doesn't already exist -// if (myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { -// List referencesToCreate = new ArrayList<>(); -// for (IBase entry : requestEntries) { -// IBaseResource resource = myVersionAdapter.getResource(entry); -// Set referencesToAutoVersion = BaseStorageDao.extractReferencesToAutoVersion(myContext, -// myModelConfig, -// resource); -// for (IBaseReference subReference : referencesToAutoVersion) { -// // doesn't exist - add it to the entries to be created -// org.hl7.fhir.r4.model.Bundle.BundleEntryRequestComponent req = new org.hl7.fhir.r4.model.Bundle.BundleEntryRequestComponent(); -// req.setMethod(org.hl7.fhir.r4.model.Bundle.HTTPVerb.PUT); -// req.setUrl(subReference.getReferenceElement().getValue()); -//// req.setIfNoneExist(); //TODO - look at conditional creates to see how this works -// referencesToCreate.add(req); // adding the request should add it to theIdToPersistence map later -// } -// } -// requestEntries.addAll(referencesToCreate); -// } - // Do all entries have a verb? for (int i = 0; i < numberOfEntries; i++) { IBase nextReqEntry = requestEntries.get(i); @@ -510,9 +492,6 @@ public abstract class BaseTransactionProcessor { CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.JPA_PERFTRACE_INFO, params); } - //FIXME - remove before committing - ourLog.info(myContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(response)); - return response; } @@ -639,7 +618,6 @@ public abstract class BaseTransactionProcessor { theEntries, theTransactionStopWatch); theTransactionStopWatch.startTask("Commit writes to database"); - ourLog.info("Retval has " + retVal.values().size()); return retVal; }; Map entriesToProcess; @@ -647,14 +625,6 @@ public abstract class BaseTransactionProcessor { try { entriesToProcess = myHapiTransactionService.execute(theRequestDetails, theTransactionDetails, txCallback); } - catch (Exception ex) { - //FIXME - remove - ourLog.info("FindME"); - - ourLog.info(ex.getLocalizedMessage()); - ex.printStackTrace(); - entriesToProcess = new HashMap<>(); - } finally { if (writeOperationsDetails != null) { HookParams params = new HookParams() @@ -1232,24 +1202,6 @@ public abstract class BaseTransactionProcessor { Set referencesToAutoVersion = nextEntry.getValue(); IBaseResource nextResource = nextOutcome.getResource(); - //TODO - should we add the autoversioned resources to our idtoPersistedoutcomes here? - // idToPersistedOutcomes has no entry for them at this point (if they were not in the - // top level bundle) - // should we just add no-op values to the map (since they are all going to be gets)? -// for (IBaseReference autoVersionRef : referencesToAutoVersion) { -// IFhirResourceDao dao = myDaoRegistry.getResourceDao(autoVersionRef.getReferenceElement().getResourceType()); -// IBaseResource baseResource; -// try { -// baseResource = dao.read(autoVersionRef.getReferenceElement()); -// } catch (ResourceNotFoundException ex) { - // not found -// baseResource = -// DaoMethodOutcome outcome = new DaoMethodOutcome(); -// outcome.setResource(baseResource); -// outcome.setNop(true); // we are just reading it -// theIdToPersistedOutcome.put(autoVersionRef.getReferenceElement(), outcome); -// } -// } resolveReferencesThenSaveAndIndexResource(theRequest, theTransactionDetails, theIdSubstitutions, theIdToPersistedOutcome, @@ -1307,61 +1259,33 @@ public abstract class BaseTransactionProcessor { } 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 { - // resource type -> set of Ids - // we'll populate this with only those resource/ids of - // resource references that: - // a) do not exist in the idToPersistedOutcome - // (so not in top level of bundle) - // b) do not exist in DB - // (so newly created resources) - // - // we only do this if autocreateplaceholders is on - HashMap> resourceTypeToListOfIds = new HashMap<>(); - if (myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { - for (IBaseReference ref : theReferencesToAutoVersion) { - IIdType id = ref.getReferenceElement(); - // if we don't have this in our idToPersistedOutcome - // and we have createplaceholderreferences on - // we will have to check if these objects exist in the DB - if (!theIdToPersistedOutcome.containsKey(id)) { - if (!resourceTypeToListOfIds.containsKey(id.getResourceType())) { - resourceTypeToListOfIds.put(id.getResourceType(), new HashSet<>()); - } + // get a map of + // existing ids -> PID (for resources that exist in the DB) + Map idToPID = myAutoVersioningService.getAutoversionsForIds(theReferencesToAutoVersion.stream() + .map(ref -> ref.getReferenceElement()).collect(Collectors.toList())); - resourceTypeToListOfIds.get(id.getResourceType()).add(id); - } + for (IBaseReference baseRef : theReferencesToAutoVersion) { + IIdType id = baseRef.getReferenceElement(); + if (!idToPID.containsKey(id) + && myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { + // not in the db, but autocreateplaceholders is true + // so the version we'll set is "1" (since it will be + // created later) + String newRef = id.withVersion("1").getValue(); + id.setValue(newRef); } - - for (String resourceType : resourceTypeToListOfIds.keySet()) { - IFhirResourceDao dao = myDaoRegistry.getResourceDao(resourceType); - Set idSet = resourceTypeToListOfIds.get(resourceType); - - // DB hit :( - Set existing = dao.hasResources(idSet); - - // we remove all the ids that are found, leaving - // only the ids of those not in the DB at all - idSet.removeAll(existing); + else { + // we will add the looked up info to the transaction + // for later + theTransactionDetails.addResolvedResourceId(id, + idToPID.get(id)); } } if (theReferencesToAutoVersion.contains(resourceReference)) { DaoMethodOutcome outcome = theIdToPersistedOutcome.get(nextId); - if (outcome == null && myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { - // null outcome means it's a resource reference that was not at top of bundle - IIdType id = resourceReference.getReferenceElement(); - String resourceType = id.getResourceType(); - // if it exists in resourceTypeToListOfIds - // it's not in the DB (new resource) - Set ids = resourceTypeToListOfIds.get(resourceType); - if (!ids.contains(id)) { - // doesn't exist in the DB - // which means the history necessarily is 1 (first one) - resourceReference.getReferenceElement().setValue(id.getValue() + "/_history/1"); - } - } - else if (outcome != null && !outcome.isNop() && !Boolean.TRUE.equals(outcome.getCreated())) { + if (outcome != null && !outcome.isNop() && !Boolean.TRUE.equals(outcome.getCreated())) { addRollbackReferenceRestore(theTransactionDetails, resourceReference); resourceReference.setReference(nextId.getValue()); resourceReference.setResource(null); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java new file mode 100644 index 00000000000..720edc41478 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java @@ -0,0 +1,23 @@ +package ca.uhn.fhir.jpa.dao; + +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; +import org.hl7.fhir.instance.model.api.IIdType; + +import java.util.Collection; +import java.util.Map; + +public interface IAutoVersioningService { + /** + * Takes in a list of IIdType and returns a map of + * IIdType -> ResourcePersistentId. + * ResourcePersistentId will contain the history version + * but the returned IIdType will not (this is to allow consumers + * to use their passed in IIdType as a lookup value). + * + * If the returned map does not return an IIdType -> ResourcePersistentId + * then it means that it is a non-existing resource in the DB + * @param theIds + * @return + */ + Map getAutoversionsForIds(Collection theIds); +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java index 2a747ab089d..0676f124e90 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java @@ -113,8 +113,9 @@ public interface IForcedIdDao extends JpaRepository { @Query("" + "SELECT " + - " f.myResourceType, f.myForcedId " + + " f.myResourceType, f.myForcedId, f.myResourcePid, t.myVersion " + "FROM ForcedId f " + + "JOIN ResourceTable t ON t.myId = f.myResourcePid " + "WHERE f.myResourceType = :resource_type AND f.myForcedId IN ( :forced_id )") Collection findResourcesByForcedId(@Param("resource_type") String theResourceType, @Param("forced_id") Collection theForcedIds); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java new file mode 100644 index 00000000000..6d8d848b01b --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java @@ -0,0 +1,62 @@ +package ca.uhn.fhir.jpa.dao; + +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; +import org.hl7.fhir.instance.model.api.IIdType; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +@ExtendWith(MockitoExtension.class) +public class AutoVersioningServiceImplTests { + + @Mock + private DaoRegistry daoRegistry; + + @InjectMocks + private AutoVersioningServiceImpl myAutoversioningService; + + @Test + public void getAutoversionsForIds_whenResourceExists_returnsMapWithPIDAndVersion() { + IIdType type = new IdDt("Patient/RED"); + ResourcePersistentId pid = new ResourcePersistentId(1); + pid.setVersion(2L); + HashMap map = new HashMap<>(); + map.put(type, pid); + + IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); + Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) + .thenReturn(map); + + Map retMap = myAutoversioningService.getAutoversionsForIds(Collections.singletonList(type)); + + Assertions.assertTrue(retMap.containsKey(type)); + Assertions.assertEquals(pid.getVersion(), map.get(type).getVersion()); + } + + @Test + public void getAutoversionsForIds_whenResourceDoesNotExist_returnsEmptyMap() { + IIdType type = new IdDt("Patient/RED"); + + IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); + Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) + .thenReturn(new HashMap<>()); + + Map retMap = myAutoversioningService.getAutoversionsForIds(Collections.singletonList(type)); + + Assertions.assertTrue(retMap.isEmpty()); + } + + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java index bc984843ff4..8292ce448fa 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java @@ -1,18 +1,150 @@ package ca.uhn.fhir.jpa.dao; +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; +import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; +@ExtendWith(MockitoExtension.class) class BaseHapiFhirResourceDaoTest { + + private class SimpleTestDao extends BaseHapiFhirResourceDao { + public SimpleTestDao() { + super(); + // post inject hooks + setResourceType(Patient.class); + RuntimeResourceDefinition resourceDefinition = Mockito.mock(RuntimeResourceDefinition.class); + Mockito.when(resourceDefinition.getName()).thenReturn("Patient"); + FhirContext myFhirContextMock = Mockito.mock(FhirContext.class); + Mockito.when(myFhirContextMock.getResourceDefinition(Mockito.any(Class.class))) + .thenReturn(resourceDefinition); + setContext(myFhirContextMock); + postConstruct(); + } + + } + + @InjectMocks + private BaseHapiFhirResourceDao myBaseHapiFhirResourceDao = new SimpleTestDao(); + + @Mock + private IForcedIdDao myIForcedIdDao; + + //TODO - all other dependency mocks + + private Object[] createMatchEntryForGetIdsOfExistingResources(IIdType theId, long thePID, long theResourceVersion) { + Object[] arr = new Object[] { + theId.getResourceType(), + theId.getIdPart(), + thePID, + theResourceVersion + }; + return arr; + } + + @Test + public void getIdsOfExistingResources_forExistingResources_returnsMapOfIdToPIDWithVersion() { + // setup + IIdType patientIdAndType = new IdDt("Patient/RED"); + long patientPID = 1L; + long patientResourceVersion = 2L; + IIdType patient2IdAndType = new IdDt("Patient/BLUE"); + long patient2PID = 3L; + long patient2ResourceVersion = 4L; + List inputList = new ArrayList<>(); + inputList.add(patientIdAndType); + inputList.add(patient2IdAndType); + + Collection matches = Arrays.asList( + createMatchEntryForGetIdsOfExistingResources(patientIdAndType, patientPID, patientResourceVersion), + createMatchEntryForGetIdsOfExistingResources(patient2IdAndType, patient2PID, patient2ResourceVersion) + ); + + // when + Mockito.when(myIForcedIdDao.findResourcesByForcedId(Mockito.anyString(), + Mockito.anyList())).thenReturn(matches); + + Map idToPIDOfExistingResources = myBaseHapiFhirResourceDao.getIdsOfExistingResources(inputList); + + Assertions.assertEquals(inputList.size(), idToPIDOfExistingResources.size()); + Assertions.assertTrue(idToPIDOfExistingResources.containsKey(patientIdAndType)); + Assertions.assertTrue(idToPIDOfExistingResources.containsKey(patient2IdAndType)); + Assertions.assertEquals(idToPIDOfExistingResources.get(patientIdAndType).getIdAsLong(), patientPID); + Assertions.assertEquals(idToPIDOfExistingResources.get(patient2IdAndType).getIdAsLong(), patient2PID); + Assertions.assertEquals(idToPIDOfExistingResources.get(patientIdAndType).getVersion(), patientResourceVersion); + Assertions.assertEquals(idToPIDOfExistingResources.get(patient2IdAndType).getVersion(), patient2ResourceVersion); + } + + @Test + public void getIdsOfExistingResources_forNonExistentResources_returnsEmptyMap() { + //setup + IIdType patient = new IdDt("Patient/RED"); + + // when + Mockito.when(myIForcedIdDao.findResourcesByForcedId(Mockito.anyString(), Mockito.anyList())) + .thenReturn(new ArrayList<>()); + + Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(Collections.singletonList(patient)); + + Assertions.assertTrue(map.isEmpty()); + } + + @Test + public void getIdsOfExistingResources_whenSomeResourcesExist_returnsOnlyExistingResourcesInMap() { + // setup + IIdType patientIdAndType = new IdDt("Patient/RED"); + long patientPID = 1L; + long patientResourceVersion = 2L; + IIdType patient2IdAndType = new IdDt("Patient/BLUE"); + List inputList = new ArrayList<>(); + inputList.add(patientIdAndType); + inputList.add(patient2IdAndType); + + Collection matches = Collections.singletonList( + createMatchEntryForGetIdsOfExistingResources(patientIdAndType, patientPID, patientResourceVersion) + ); + + // when + Mockito.when(myIForcedIdDao.findResourcesByForcedId(Mockito.anyString(), Mockito.anyList())) + .thenReturn(matches); + + Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(inputList); + + // verify + Assertions.assertFalse(map.isEmpty()); + Assertions.assertEquals(inputList.size() - 1, map.size()); + Assertions.assertTrue(map.containsKey(patientIdAndType)); + Assertions.assertFalse(map.containsKey(patient2IdAndType)); + } + + /*******************/ + TestResourceDao mySvc = new TestResourceDao(); @Test diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index 95a9732fed5..39660fe7399 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -20,6 +20,7 @@ import ca.uhn.fhir.jpa.binstore.BinaryStorageInterceptor; import ca.uhn.fhir.jpa.bulk.export.api.IBulkDataExportSvc; import ca.uhn.fhir.jpa.config.TestR4Config; import ca.uhn.fhir.jpa.dao.BaseJpaTest; +import ca.uhn.fhir.jpa.dao.IAutoVersioningService; import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.dao.data.IMdmLinkDao; @@ -498,6 +499,8 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil protected ValidationSettings myValidationSettings; @Autowired protected IMdmLinkDao myMdmLinkDao; + @Autowired + protected IAutoVersioningService myAutoVersioningService; @AfterEach() public void afterCleanupDao() { 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 a4e65f4ee4d..979d3470f7f 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 @@ -1,11 +1,11 @@ package ca.uhn.fhir.jpa.dao.r4; -import ca.uhn.fhir.context.ParserOptions; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; 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.model.primitive.IdDt; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.util.BundleBuilder; @@ -27,7 +27,6 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import java.io.InputStreamReader; -import java.sql.Ref; import java.util.Arrays; import java.util.Date; import java.util.HashSet; @@ -41,6 +40,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.matchesPattern; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { @@ -98,7 +98,6 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { assertEquals("Patient/A/_history/1", eob2.getPatient().getReference()); } - @Test public void testCreateAndUpdateVersionedReferencesInTransaction_VersionedReferenceToVersionedReferenceToUpsertWithNop() { myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); @@ -291,60 +290,13 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { myObservationDao.update(observation); // Make sure we're not introducing any extra DB operations - assertEquals(5, myCaptureQueriesListener.logSelectQueries().size()); + assertEquals(6, myCaptureQueriesListener.logSelectQueries().size()); // Read back and verify that reference is now versioned observation = myObservationDao.read(observationId); assertEquals(patientIdString, observation.getSubject().getReference()); } - - @Test - public void testInsertVersionedReferenceAtPathUsingTransaction() { - myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); - myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); - myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); - - Patient p = new Patient(); - p.setActive(true); - IIdType patientId = myPatientDao.create(p).getId().toUnqualified(); - assertEquals("1", patientId.getVersionIdPart()); - assertEquals(null, patientId.getBaseUrl()); - String patientIdString = patientId.getValue(); - - // Create - put an unversioned reference in the subject - Observation observation = new Observation(); - observation.getSubject().setReference(patientId.toVersionless().getValue()); - - BundleBuilder builder = new BundleBuilder(myFhirCtx); - builder.addTransactionUpdateEntry(observation); - - Bundle transaction = mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); - - Observation ret = myObservationDao.read(observation.getIdElement()); - Assertions.assertTrue(ret != null); - - // Read back and verify that reference is now versioned -// observation = myObservationDao.read(observationId); -// assertEquals(patientIdString, observation.getSubject().getReference()); -// -// myCaptureQueriesListener.clear(); -// -// // Update - put an unversioned reference in the subject -// observation = new Observation(); -// observation.setId(observationId); -// observation.addIdentifier().setSystem("http://foo").setValue("bar"); -// observation.getSubject().setReference(patientId.toVersionless().getValue()); -// myObservationDao.update(observation); -// -// // Make sure we're not introducing any extra DB operations -// assertEquals(5, myCaptureQueriesListener.logSelectQueries().size()); -// -// // Read back and verify that reference is now versioned -// observation = myObservationDao.read(observationId); -// assertEquals(patientIdString, observation.getSubject().getReference()); - } - @Test public void testInsertVersionedReferenceAtPath_InTransaction_SourceAndTargetBothCreated() { myFhirCtx.getParserOptions().setStripVersionsFromReferences(false); @@ -841,13 +793,15 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { "ExplanationOfBenefit.insurance.coverage", "ExplanationOfBenefit.payee.party" ); - myModelConfig.setAutoVersionReferenceAtPaths(new HashSet(strings)); + myModelConfig.setAutoVersionReferenceAtPaths(new HashSet<>(strings)); Bundle bundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, new InputStreamReader( FhirResourceDaoR4VersionedReferenceTest.class.getResourceAsStream("/npe-causing-bundle.json"))); Bundle transaction = mySystemDao.transaction(new SystemRequestDetails(), bundle); + + assertNotNull(transaction); } @Test @@ -862,6 +816,9 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { String versionedPatientReference = resource.getSubject().getReference(); assertThat(versionedPatientReference, is(equalTo("Patient/ABC"))); + Patient p = myPatientDao.read(new IdDt("Patient/ABC")); + Assertions.assertNotNull(p); + myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); obs = new Observation(); @@ -875,6 +832,45 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { } + @Test + @DisplayName("Bundle transaction with AutoVersionReferenceAtPath on and with existing Patient resource should create") + public void bundleTransaction_autoreferenceAtPathWithPreexistingPatientReference_shouldCreate() { + myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); + + String patientId = "Patient/RED"; + IIdType idType = new IdDt(patientId); + + // create patient ahead of time + Patient patient = new Patient(); + patient.setId(patientId); + DaoMethodOutcome outcome = myPatientDao.update(patient); + assertThat(outcome.getResource().getIdElement().getValue(), is(equalTo(patientId + "/_history/1"))); + + Patient returned = myPatientDao.read(idType); + Assertions.assertNotNull(returned); + assertThat(returned.getId(), is(equalTo(patientId + "/_history/1"))); + + // update to change version + patient.setActive(true); + myPatientDao.update(patient); + + Observation obs = new Observation(); + obs.setId("Observation/DEF"); + Reference patientRef = new Reference(patientId); + obs.setSubject(patientRef); + BundleBuilder builder = new BundleBuilder(myFhirCtx); + builder.addTransactionUpdateEntry(obs); + + Bundle submitted = (Bundle)builder.getBundle(); + + Bundle returnedTr = mySystemDao.transaction(new SystemRequestDetails(), submitted); + + Assertions.assertNotNull(returnedTr); + + // some verification + Observation obRet = myObservationDao.read(obs.getIdElement()); + Assertions.assertNotNull(obRet); + } @Test @DisplayName("GH-2901 Test no NPE is thrown on autoversioned references") @@ -882,10 +878,6 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); -// ParserOptions options = new ParserOptions(); -// options.setDontStripVersionsFromReferencesAtPaths("Observation.subject"); -// myFhirCtx.setParserOptions(options); - Observation obs = new Observation(); obs.setId("Observation/DEF"); Reference patientRef = new Reference("Patient/RED"); @@ -895,20 +887,14 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { Bundle submitted = (Bundle)builder.getBundle(); - //1 make sure this test throws the InvalidRequestException (make separate test for this) - //2 add a test for patient created before bundle and then process observation with reference to patient (null check for outcome) - //3 - Bundle returnedTr = mySystemDao.transaction(new SystemRequestDetails(), submitted); - Assertions.assertTrue(returnedTr != null); + Assertions.assertNotNull(returnedTr); // some verification Observation obRet = myObservationDao.read(obs.getIdElement()); - Assertions.assertTrue(obRet != null); + Assertions.assertNotNull(obRet); Patient returned = myPatientDao.read(patientRef.getReferenceElement()); - Assertions.assertTrue(returned != null); + Assertions.assertNotNull(returned); } - - } From 4c90ffe975635e8678de58c959b6ed926deda458 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 2 Sep 2021 09:47:55 -0400 Subject: [PATCH 11/19] issue-2901 adding changelog --- ...onatpaths-with-autocreateplaceholders-no-null-pointer.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2901-autoversionatpaths-with-autocreateplaceholders-no-null-pointer.yml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2901-autoversionatpaths-with-autocreateplaceholders-no-null-pointer.yml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2901-autoversionatpaths-with-autocreateplaceholders-no-null-pointer.yml new file mode 100644 index 00000000000..5b5159f2372 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2901-autoversionatpaths-with-autocreateplaceholders-no-null-pointer.yml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 2901 +jira: SMILE-3004 +title: "Processing transactions with AutoversionAtPaths set should create those resources (if AutoCreatePlaceholders is set) and use latest version as expected" From cbfcc02a920881c4f2a1e0fef843451fc9161575 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 2 Sep 2021 11:07:06 -0400 Subject: [PATCH 12/19] issue-2901 removing cruft and cleaning up --- .../jpa/dao/AutoVersioningServiceImpl.java | 2 +- .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 2 +- .../jpa/dao/BaseTransactionProcessor.java | 3 +- .../fhir/jpa/dao/IAutoVersioningService.java | 2 +- .../uhn/fhir/jpa/dao/data/IForcedIdDao.java | 9 +++++ .../dao/AutoVersioningServiceImplTests.java | 35 +++++++++++++++++-- .../jpa/dao/BaseHapiFhirResourceDaoTest.java | 9 +++++ ...irResourceDaoCreatePlaceholdersR4Test.java | 6 ---- 8 files changed, 54 insertions(+), 14 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java index 1953273dbed..0df25f61e27 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java @@ -18,7 +18,7 @@ public class AutoVersioningServiceImpl implements IAutoVersioningService { private DaoRegistry myDaoRegistry; @Override - public Map getAutoversionsForIds(Collection theIds) { + public Map getExistingAutoversionsForIds(Collection theIds) { HashMap idToPID = new HashMap<>(); HashMap> resourceTypeToIds = new HashMap<>(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java index 2ad93b1d994..b56f0547dd2 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java @@ -209,7 +209,7 @@ public abstract class BaseStorageDao { IIdType referenceElement = nextReference.getReferenceElement(); if (!referenceElement.hasBaseUrl()) { - Map idToPID = myAutoVersioningService.getAutoversionsForIds( + Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds( Collections.singletonList(referenceElement) ); 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 724aedc31cd..bf9bbf779ac 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 @@ -117,7 +117,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.TreeSet; -import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -1269,7 +1268,7 @@ public abstract class BaseTransactionProcessor { } else { // get a map of // existing ids -> PID (for resources that exist in the DB) - Map idToPID = myAutoVersioningService.getAutoversionsForIds(theReferencesToAutoVersion.stream() + Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds(theReferencesToAutoVersion.stream() .map(ref -> ref.getReferenceElement()).collect(Collectors.toList())); for (IBaseReference baseRef : theReferencesToAutoVersion) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java index 720edc41478..312c6902a9f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java @@ -19,5 +19,5 @@ public interface IAutoVersioningService { * @param theIds * @return */ - Map getAutoversionsForIds(Collection theIds); + Map getExistingAutoversionsForIds(Collection theIds); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java index 0676f124e90..4854fe421e1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java @@ -111,6 +111,15 @@ public interface IForcedIdDao extends JpaRepository { "WHERE f.myResourceType = :resource_type AND f.myForcedId IN ( :forced_id )") Collection findAndResolveByForcedIdWithNoType(@Param("resource_type") String theResourceType, @Param("forced_id") Collection theForcedIds); + /** + * This method returns a collection where eah row is an element in the collection. + * Each element in the collection is an object array where order matters. + * The returned order of each object array element is: + * ResourceType (Patient, etc - String), ForcedId (String), ResourcePID (Long), Version (Long) + * @param theResourceType + * @param theForcedIds + * @return + */ @Query("" + "SELECT " + " f.myResourceType, f.myForcedId, f.myResourcePid, t.myVersion " + diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java index 6d8d848b01b..da32e0cbc35 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java @@ -13,7 +13,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; -import java.util.Collection; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -38,8 +38,10 @@ public class AutoVersioningServiceImplTests { IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) .thenReturn(map); + Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) + .thenReturn(daoMock); - Map retMap = myAutoversioningService.getAutoversionsForIds(Collections.singletonList(type)); + Map retMap = myAutoversioningService.getExistingAutoversionsForIds(Collections.singletonList(type)); Assertions.assertTrue(retMap.containsKey(type)); Assertions.assertEquals(pid.getVersion(), map.get(type).getVersion()); @@ -52,11 +54,38 @@ public class AutoVersioningServiceImplTests { IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) .thenReturn(new HashMap<>()); + Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) + .thenReturn(daoMock); - Map retMap = myAutoversioningService.getAutoversionsForIds(Collections.singletonList(type)); + Map retMap = myAutoversioningService.getExistingAutoversionsForIds(Collections.singletonList(type)); Assertions.assertTrue(retMap.isEmpty()); } + @Test + public void getAutoversionsForIds_whenSomeResourcesDoNotExist_returnsOnlyExistingElements() { + IIdType type = new IdDt("Patient/RED"); + ResourcePersistentId pid = new ResourcePersistentId(1); + pid.setVersion(2L); + HashMap map = new HashMap<>(); + map.put(type, pid); + IIdType type2 = new IdDt("Patient/BLUE"); + // when + IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); + Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) + .thenReturn(map); + Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) + .thenReturn(daoMock); + + // test + Map retMap = myAutoversioningService.getExistingAutoversionsForIds( + Arrays.asList(type, type2) + ); + + // verify + Assertions.assertEquals(map.size(), retMap.size()); + Assertions.assertTrue(retMap.containsKey(type)); + Assertions.assertFalse(retMap.containsKey(type2)); + } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java index 8292ce448fa..951c8a97a38 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java @@ -34,6 +34,7 @@ import static org.junit.jupiter.api.Assertions.fail; @ExtendWith(MockitoExtension.class) class BaseHapiFhirResourceDaoTest { + // our simple concrete test class for BaseHapiFhirResourceDao private class SimpleTestDao extends BaseHapiFhirResourceDao { public SimpleTestDao() { super(); @@ -58,6 +59,14 @@ class BaseHapiFhirResourceDaoTest { //TODO - all other dependency mocks + /** + * Creates a match entry to be returned by myIForcedIdDao. + * This ordering matters (see IForcedIdDao) + * @param theId + * @param thePID + * @param theResourceVersion + * @return + */ private Object[] createMatchEntryForGetIdsOfExistingResources(IIdType theId, long thePID, long theResourceVersion) { Object[] arr = new Object[] { theId.getResourceType(), diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java index 0451769f045..7945896fa60 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java @@ -25,7 +25,6 @@ import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Task; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import java.util.List; @@ -598,9 +597,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { // create Patient patient = new Patient(); -// patient.setId(patientId); patient.setIdElement(new IdType(patientId)); -// patient.setActive(true); myPatientDao.update(patient); // update @@ -624,7 +621,4 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { Observation retObservation = myObservationDao.read(obs.getIdElement()); Assertions.assertTrue(retObservation != null); } - - // always work with the put - // conditional create (replace put with conditional create?) } From ff7433edd7ca51fb47cc45a26b62100640cdd625 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 2 Sep 2021 11:43:12 -0400 Subject: [PATCH 13/19] issue-2901 cleaning up test contexts --- .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 1 + ...irResourceDaoCreatePlaceholdersR4Test.java | 31 +++++++++---------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java index b56f0547dd2..c042c626bc7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java @@ -233,6 +233,7 @@ public abstract class BaseStorageDao { // resource not found // and no autocreateplaceholders set... // we throw + System.out.println("FINDME"); throw new ResourceNotFoundException(referenceElement); } String newTargetReference = referenceElement.withVersion(version.toString()).getValue(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java index 7945896fa60..870543bdced 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java @@ -1,6 +1,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; @@ -52,12 +53,11 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { myDaoConfig.setResourceClientIdStrategy(new DaoConfig().getResourceClientIdStrategy()); myDaoConfig.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(new DaoConfig().isPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets()); myDaoConfig.setBundleTypesAllowedForStorage(new DaoConfig().getBundleTypesAllowedForStorage()); - + myModelConfig.setAutoVersionReferenceAtPaths(new ModelConfig().getAutoVersionReferenceAtPaths()); } @Test public void testCreateWithBadReferenceFails() { - Observation o = new Observation(); o.setStatus(ObservationStatus.FINAL); o.getSubject().setReference("Patient/FOO"); @@ -101,27 +101,28 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { params.add(Task.SP_PART_OF, new ReferenceParam("Task/AAA")); List found = toUnqualifiedVersionlessIdValues(myTaskDao.search(params)); assertThat(found, contains(id.getValue())); - - } @Test public void testUpdateWithBadReferenceFails() { + Observation o1 = new Observation(); + o1.setStatus(ObservationStatus.FINAL); + IIdType id = myObservationDao.create(o1, mySrd).getId(); Observation o = new Observation(); - o.setStatus(ObservationStatus.FINAL); - IIdType id = myObservationDao.create(o, mySrd).getId(); - - o = new Observation(); o.setId(id); o.setStatus(ObservationStatus.FINAL); o.getSubject().setReference("Patient/FOO"); - try { + + Assertions.assertThrows(InvalidRequestException.class, () -> { myObservationDao.update(o, mySrd); - fail(); - } catch (InvalidRequestException e) { - assertThat(e.getMessage(), startsWith("Resource Patient/FOO not found, specified in path: Observation.subject")); - } + }); +// try { +// +// fail(); +// } catch (InvalidRequestException e) { +// assertThat(e.getMessage(), startsWith("Resource Patient/FOO not found, specified in path: Observation.subject")); +// } } @Test @@ -454,8 +455,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { ourLog.info("\nObservation read after Patient update:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(createdObs)); assertEquals(createdObs.getSubject().getReference(), conditionalUpdatePatId.toUnqualifiedVersionless().getValueAsString()); assertEquals(placeholderPatId.toUnqualifiedVersionless().getValueAsString(), conditionalUpdatePatId.toUnqualifiedVersionless().getValueAsString()); - - } @Test @@ -566,7 +565,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { Observation createdObs = myObservationDao.read(id); ourLog.info(myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(createdObs)); assertEquals("Patient/ABC", createdObs.getSubject().getReference()); - } @Test @@ -590,7 +588,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { @Test public void testAutocreatePlaceholderWithTargetExistingAlreadyTest() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); - myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); String patientId = "Patient/RED"; From 49124c298df5e8af0ea20ac4fc88f2b3156f9b8f Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 2 Sep 2021 13:15:24 -0400 Subject: [PATCH 14/19] issue-2901 fixing test --- .../dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java index 870543bdced..fa953bc28e6 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java @@ -114,15 +114,10 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { o.setStatus(ObservationStatus.FINAL); o.getSubject().setReference("Patient/FOO"); - Assertions.assertThrows(InvalidRequestException.class, () -> { + Exception ex = Assertions.assertThrows(InvalidRequestException.class, () -> { myObservationDao.update(o, mySrd); }); -// try { -// -// fail(); -// } catch (InvalidRequestException e) { -// assertThat(e.getMessage(), startsWith("Resource Patient/FOO not found, specified in path: Observation.subject")); -// } + assertThat(ex.getMessage(), startsWith("Resource Patient/FOO not found, specified in path: Observation.subject")); } @Test From 6f82e3568ca8635b7faeecedafac2a5359c10265 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 2 Sep 2021 13:20:38 -0400 Subject: [PATCH 15/19] issue-2901 remove debug code --- .../src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java index c042c626bc7..b56f0547dd2 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java @@ -233,7 +233,6 @@ public abstract class BaseStorageDao { // resource not found // and no autocreateplaceholders set... // we throw - System.out.println("FINDME"); throw new ResourceNotFoundException(referenceElement); } String newTargetReference = referenceElement.withVersion(version.toString()).getValue(); From 78c71ce761cf5468e17c30ca8e7ad5ff1ed525ba Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Fri, 3 Sep 2021 08:38:19 -0400 Subject: [PATCH 16/19] issue-2901 fix tests --- .../java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java | 2 ++ .../test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 3 +++ 2 files changed, 5 insertions(+) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java index 0bace079a8f..907650ba59b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java @@ -70,6 +70,8 @@ public class TransactionProcessorTest { private MatchUrlService myMatchUrlService; @MockBean private IRequestPartitionHelperSvc myRequestPartitionHelperSvc; + @MockBean + private IAutoVersioningService myAutoVersioningService; @MockBean(answer = Answers.RETURNS_DEEP_STUBS) private SessionImpl mySession; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index d5428cc9fd1..cc081ca421d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; +import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.NormalizedQuantitySearchLevel; import ca.uhn.fhir.jpa.model.entity.ResourceEncodingEnum; import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable; @@ -119,6 +120,8 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { myModelConfig.setNormalizedQuantitySearchLevel(NormalizedQuantitySearchLevel.NORMALIZED_QUANTITY_SEARCH_NOT_SUPPORTED); myDaoConfig.setBundleBatchPoolSize(new DaoConfig().getBundleBatchPoolSize()); myDaoConfig.setBundleBatchMaxPoolSize(new DaoConfig().getBundleBatchMaxPoolSize()); + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(new DaoConfig().isAutoCreatePlaceholderReferenceTargets()); + myModelConfig.setAutoVersionReferenceAtPaths(new ModelConfig().getAutoVersionReferenceAtPaths()); } @BeforeEach From 64270a51e00f91ce084ccfa28ecd903b2f88845a Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Fri, 3 Sep 2021 15:36:42 -0400 Subject: [PATCH 17/19] issue-2901 review fixes --- .../fhir/jpa/api/dao/IFhirResourceDao.java | 3 +- .../jpa/dao/AutoVersioningServiceImpl.java | 6 +- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 54 +++++++---- .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 5 +- .../jpa/dao/BaseTransactionProcessor.java | 5 +- .../fhir/jpa/dao/IAutoVersioningService.java | 4 +- .../uhn/fhir/jpa/dao/data/IForcedIdDao.java | 17 ---- .../fhir/jpa/dao/data/IResourceTableDao.java | 10 ++ .../dao/AutoVersioningServiceImplTests.java | 16 +++- .../jpa/dao/BaseHapiFhirResourceDaoTest.java | 91 ++++++++++++------- ...irResourceDaoCreatePlaceholdersR4Test.java | 45 ++++++++- 11 files changed, 171 insertions(+), 85 deletions(-) diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java index 69d0716fd4c..065149eef76 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java @@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.api.dao; */ import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.model.DeleteConflictList; import ca.uhn.fhir.jpa.api.model.DeleteMethodOutcome; @@ -168,7 +169,7 @@ public interface IFhirResourceDao extends IDao { * @param theIds - list of IIdType ids (for the same resource) * @return */ - Map getIdsOfExistingResources(Collection theIds); + Map getIdsOfExistingResources(RequestPartitionId partitionId, Collection theIds); /** * Read a resource by its internal PID diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java index 0df25f61e27..151c9eb1191 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.dao; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; @@ -18,7 +19,7 @@ public class AutoVersioningServiceImpl implements IAutoVersioningService { private DaoRegistry myDaoRegistry; @Override - public Map getExistingAutoversionsForIds(Collection theIds) { + public Map getExistingAutoversionsForIds(RequestPartitionId theRequestPartitionId, Collection theIds) { HashMap idToPID = new HashMap<>(); HashMap> resourceTypeToIds = new HashMap<>(); @@ -33,7 +34,8 @@ public class AutoVersioningServiceImpl implements IAutoVersioningService { for (String resourceType : resourceTypeToIds.keySet()) { IFhirResourceDao dao = myDaoRegistry.getResourceDao(resourceType); - Map idAndPID = dao.getIdsOfExistingResources(resourceTypeToIds.get(resourceType)); + Map idAndPID = dao.getIdsOfExistingResources(theRequestPartitionId, + resourceTypeToIds.get(resourceType)); idToPID.putAll(idAndPID); } 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 667cb450d71..8a19b774710 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 @@ -37,7 +37,6 @@ import ca.uhn.fhir.jpa.api.model.LazyDaoMethodOutcome; import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; import ca.uhn.fhir.jpa.delete.DeleteConflictService; -import ca.uhn.fhir.jpa.model.cross.IResourceLookup; import ca.uhn.fhir.jpa.model.entity.BaseHasResource; import ca.uhn.fhir.jpa.model.entity.BaseTag; import ca.uhn.fhir.jpa.model.entity.ForcedId; @@ -52,7 +51,6 @@ import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.patch.FhirPatch; import ca.uhn.fhir.jpa.patch.JsonPatchUtils; import ca.uhn.fhir.jpa.patch.XmlPatchUtils; -import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import ca.uhn.fhir.jpa.search.cache.SearchCacheStatusEnum; import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; @@ -141,6 +139,7 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Optional; @@ -1160,7 +1159,8 @@ public abstract class BaseHapiFhirResourceDao extends B } @Override - public Map getIdsOfExistingResources(Collection theIds) { + public Map getIdsOfExistingResources(RequestPartitionId thePartitionId, + Collection theIds) { // these are the found Ids that were in the db HashMap collected = new HashMap<>(); @@ -1168,26 +1168,40 @@ public abstract class BaseHapiFhirResourceDao extends B return collected; } - List idPortions = theIds - .stream() - .map(IIdType::getIdPart) - .collect(Collectors.toList()); + List resourcePersistentIds = myIdHelperService.resolveResourcePersistentIdsWithCache(thePartitionId, + theIds.stream().collect(Collectors.toList())); - Collection matches = myForcedIdDao.findResourcesByForcedId(getResourceName(), - idPortions); + // we'll use this map to fetch pids that require versions + HashMap pidsToVersionToResourcePid = new HashMap<>(); - for (Object[] match : matches) { - String resourceType = (String) match[0]; - String forcedId = (String) match[1]; - Long pid = (Long) match[2]; - Long versionId = (Long) match[3]; - IIdType id = new IdDt(resourceType, forcedId); - // we won't put the version on the IIdType - // so callers can use this as a lookup to match to - ResourcePersistentId persistentId = new ResourcePersistentId(pid); - persistentId.setVersion(versionId); + // fill in our map + for (ResourcePersistentId pid : resourcePersistentIds) { + if (pid.getVersion() == null) { + pidsToVersionToResourcePid.put(pid.getIdAsLong(), pid); + } + Optional idOp = theIds.stream() + .filter(i -> i.getIdPart().equals(pid.getAssociatedResourceId().getIdPart())) + .findFirst(); + // this should always be present + // since it was passed in. + // but land of optionals... + idOp.ifPresent(id -> { + collected.put(id, pid); + }); + } - collected.put(id, persistentId); + // set any versions we don't already have + if (!pidsToVersionToResourcePid.isEmpty()) { + Collection resourceEntries = myResourceTableDao + .getResourceVersionsForPid(new ArrayList<>(pidsToVersionToResourcePid.keySet())); + + for (Object[] record : resourceEntries) { + // order matters! + Long retPid = (Long)record[0]; + String resType = (String)record[1]; + Long version = (Long)record[2]; + pidsToVersionToResourcePid.get(retPid).setVersion(version); + } } return collected; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java index b56f0547dd2..6df1d8fe44d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; @@ -209,7 +210,7 @@ public abstract class BaseStorageDao { IIdType referenceElement = nextReference.getReferenceElement(); if (!referenceElement.hasBaseUrl()) { - Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds( + Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), Collections.singletonList(referenceElement) ); @@ -220,7 +221,7 @@ public abstract class BaseStorageDao { Long version; if (idToPID.containsKey(referenceElement)) { // the resource exists... latest id - // will bbe the value in the ResourcePersistentId + // will be the value in the ResourcePersistentId version = idToPID.get(referenceElement).getVersion(); } else if (myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { 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 bf9bbf779ac..f8db7e92b11 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 @@ -25,6 +25,7 @@ import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.interceptor.model.TransactionWriteOperationsDetails; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; @@ -1268,7 +1269,9 @@ public abstract class BaseTransactionProcessor { } else { // get a map of // existing ids -> PID (for resources that exist in the DB) - Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds(theReferencesToAutoVersion.stream() + // should this be allPartitions? + Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), + theReferencesToAutoVersion.stream() .map(ref -> ref.getReferenceElement()).collect(Collectors.toList())); for (IBaseReference baseRef : theReferencesToAutoVersion) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java index 312c6902a9f..80d327eb2d0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.dao; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import org.hl7.fhir.instance.model.api.IIdType; @@ -17,7 +18,8 @@ public interface IAutoVersioningService { * If the returned map does not return an IIdType -> ResourcePersistentId * then it means that it is a non-existing resource in the DB * @param theIds + * @param thePartitionId - the partition id * @return */ - Map getExistingAutoversionsForIds(Collection theIds); + Map getExistingAutoversionsForIds(RequestPartitionId thePartitionId, Collection theIds); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java index 4854fe421e1..3a10f6be0d5 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java @@ -111,23 +111,6 @@ public interface IForcedIdDao extends JpaRepository { "WHERE f.myResourceType = :resource_type AND f.myForcedId IN ( :forced_id )") Collection findAndResolveByForcedIdWithNoType(@Param("resource_type") String theResourceType, @Param("forced_id") Collection theForcedIds); - /** - * This method returns a collection where eah row is an element in the collection. - * Each element in the collection is an object array where order matters. - * The returned order of each object array element is: - * ResourceType (Patient, etc - String), ForcedId (String), ResourcePID (Long), Version (Long) - * @param theResourceType - * @param theForcedIds - * @return - */ - @Query("" + - "SELECT " + - " f.myResourceType, f.myForcedId, f.myResourcePid, t.myVersion " + - "FROM ForcedId f " + - "JOIN ResourceTable t ON t.myId = f.myResourcePid " + - "WHERE f.myResourceType = :resource_type AND f.myForcedId IN ( :forced_id )") - Collection findResourcesByForcedId(@Param("resource_type") String theResourceType, @Param("forced_id") Collection theForcedIds); - /** * This method returns a Collection where each row is an element in the collection. Each element in the collection * is an object array, where the order matters (the array represents columns returned by the query). Be careful if you change this query in any way. diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java index 25a3e3d9e1d..6ee6a307187 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java @@ -100,6 +100,16 @@ public interface IResourceTableDao extends JpaRepository { @Query("SELECT t.myVersion FROM ResourceTable t WHERE t.myId = :pid") Long findCurrentVersionByPid(@Param("pid") Long thePid); + /** + * This query will return rows with the following values: + * Id (resource pid - long), ResourceType (Patient, etc), version (long) + * Order matters! + * @param pid - list of pids to get versions for + * @return + */ + @Query("SELECT t.myId, t.myResourceType, t.myVersion FROM ResourceTable t WHERE t.myId IN ( :pid )") + Collection getResourceVersionsForPid(@Param("pid") List pid); + @Query("SELECT t FROM ResourceTable t LEFT JOIN FETCH t.myForcedId WHERE t.myPartitionId.myPartitionId IS NULL AND t.myId = :pid") Optional readByPartitionIdNull(@Param("pid") Long theResourceId); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java index da32e0cbc35..206b6c968ec 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.dao; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.model.primitive.IdDt; @@ -36,12 +37,14 @@ public class AutoVersioningServiceImplTests { map.put(type, pid); IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); - Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) + Mockito.when(daoMock.getIdsOfExistingResources(Mockito.any(RequestPartitionId.class), + Mockito.anyList())) .thenReturn(map); Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) .thenReturn(daoMock); - Map retMap = myAutoversioningService.getExistingAutoversionsForIds(Collections.singletonList(type)); + Map retMap = myAutoversioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), + Collections.singletonList(type)); Assertions.assertTrue(retMap.containsKey(type)); Assertions.assertEquals(pid.getVersion(), map.get(type).getVersion()); @@ -52,12 +55,14 @@ public class AutoVersioningServiceImplTests { IIdType type = new IdDt("Patient/RED"); IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); - Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) + Mockito.when(daoMock.getIdsOfExistingResources(Mockito.any(RequestPartitionId.class), + Mockito.anyList())) .thenReturn(new HashMap<>()); Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) .thenReturn(daoMock); - Map retMap = myAutoversioningService.getExistingAutoversionsForIds(Collections.singletonList(type)); + Map retMap = myAutoversioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), + Collections.singletonList(type)); Assertions.assertTrue(retMap.isEmpty()); } @@ -73,13 +78,14 @@ public class AutoVersioningServiceImplTests { // when IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); - Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) + Mockito.when(daoMock.getIdsOfExistingResources(Mockito.any(RequestPartitionId.class), Mockito.anyList())) .thenReturn(map); Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) .thenReturn(daoMock); // test Map retMap = myAutoversioningService.getExistingAutoversionsForIds( + RequestPartitionId.allPartitions(), Arrays.asList(type, type2) ); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java index 951c8a97a38..dd9cff05d3a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java @@ -2,8 +2,11 @@ package ca.uhn.fhir.jpa.dao; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; +import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; +import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.server.RequestDetails; @@ -13,12 +16,14 @@ import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Request; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; +import org.mockito.junit.MockitoTestRule; import org.mockito.junit.jupiter.MockitoExtension; import java.util.ArrayList; @@ -54,27 +59,40 @@ class BaseHapiFhirResourceDaoTest { @InjectMocks private BaseHapiFhirResourceDao myBaseHapiFhirResourceDao = new SimpleTestDao(); +// @Mock +// private IForcedIdDao myIForcedIdDao; + @Mock - private IForcedIdDao myIForcedIdDao; + private IdHelperService myIdHelperService; + + @Mock + private IResourceTableDao myResourceTableDao; //TODO - all other dependency mocks + + private ResourcePersistentId getResourcePersistentIdFromResource(IIdType theId, long thePid) { + ResourcePersistentId id = new ResourcePersistentId(thePid); + String idPortion = theId.getIdPart(); + IIdType newId = new IdDt(theId.getResourceType(), idPortion); + id.setAssociatedResourceId(newId); + return id; + } + /** - * Creates a match entry to be returned by myIForcedIdDao. - * This ordering matters (see IForcedIdDao) - * @param theId - * @param thePID - * @param theResourceVersion + * Gets a ResourceTable record for getResourceVersionsForPid + * Order matters! + * @param resourceType + * @param pid + * @param version * @return */ - private Object[] createMatchEntryForGetIdsOfExistingResources(IIdType theId, long thePID, long theResourceVersion) { - Object[] arr = new Object[] { - theId.getResourceType(), - theId.getIdPart(), - thePID, - theResourceVersion + private Object[] getResourceTableRecordForResourceTypeAndPid(String resourceType, long pid, long version) { + return new Object[] { + pid, // long + resourceType, // string + version // long }; - return arr; } @Test @@ -83,31 +101,38 @@ class BaseHapiFhirResourceDaoTest { IIdType patientIdAndType = new IdDt("Patient/RED"); long patientPID = 1L; long patientResourceVersion = 2L; + ResourcePersistentId pat1ResourcePID = getResourcePersistentIdFromResource(patientIdAndType, patientPID); IIdType patient2IdAndType = new IdDt("Patient/BLUE"); long patient2PID = 3L; long patient2ResourceVersion = 4L; + ResourcePersistentId pat2ResourcePID = getResourcePersistentIdFromResource(patient2IdAndType, patient2PID); List inputList = new ArrayList<>(); inputList.add(patientIdAndType); inputList.add(patient2IdAndType); Collection matches = Arrays.asList( - createMatchEntryForGetIdsOfExistingResources(patientIdAndType, patientPID, patientResourceVersion), - createMatchEntryForGetIdsOfExistingResources(patient2IdAndType, patient2PID, patient2ResourceVersion) + getResourceTableRecordForResourceTypeAndPid(patientIdAndType.getResourceType(), patientPID, patientResourceVersion), + getResourceTableRecordForResourceTypeAndPid(patient2IdAndType.getResourceType(), patient2PID, patient2ResourceVersion) ); // when - Mockito.when(myIForcedIdDao.findResourcesByForcedId(Mockito.anyString(), - Mockito.anyList())).thenReturn(matches); + Mockito.when(myIdHelperService.resolveResourcePersistentIdsWithCache(Mockito.any(RequestPartitionId.class), + Mockito.anyList())) + .thenReturn(Arrays.asList(pat1ResourcePID, pat2ResourcePID)); + Mockito.when(myResourceTableDao.getResourceVersionsForPid(Mockito.anyList())) + .thenReturn(matches); - Map idToPIDOfExistingResources = myBaseHapiFhirResourceDao.getIdsOfExistingResources(inputList); + Map idToPIDOfExistingResources = myBaseHapiFhirResourceDao.getIdsOfExistingResources(RequestPartitionId.allPartitions(), + inputList); Assertions.assertEquals(inputList.size(), idToPIDOfExistingResources.size()); Assertions.assertTrue(idToPIDOfExistingResources.containsKey(patientIdAndType)); Assertions.assertTrue(idToPIDOfExistingResources.containsKey(patient2IdAndType)); - Assertions.assertEquals(idToPIDOfExistingResources.get(patientIdAndType).getIdAsLong(), patientPID); - Assertions.assertEquals(idToPIDOfExistingResources.get(patient2IdAndType).getIdAsLong(), patient2PID); - Assertions.assertEquals(idToPIDOfExistingResources.get(patientIdAndType).getVersion(), patientResourceVersion); - Assertions.assertEquals(idToPIDOfExistingResources.get(patient2IdAndType).getVersion(), patient2ResourceVersion); + + Assertions.assertEquals(patientPID, idToPIDOfExistingResources.get(patientIdAndType).getIdAsLong()); + Assertions.assertEquals(patient2PID, idToPIDOfExistingResources.get(patient2IdAndType).getIdAsLong(), patient2PID); + Assertions.assertEquals(patientResourceVersion, idToPIDOfExistingResources.get(patientIdAndType).getVersion()); + Assertions.assertEquals(patient2ResourceVersion, idToPIDOfExistingResources.get(patient2IdAndType).getVersion()); } @Test @@ -116,10 +141,12 @@ class BaseHapiFhirResourceDaoTest { IIdType patient = new IdDt("Patient/RED"); // when - Mockito.when(myIForcedIdDao.findResourcesByForcedId(Mockito.anyString(), Mockito.anyList())) + Mockito.when(myIdHelperService.resolveResourcePersistentIdsWithCache(Mockito.any(RequestPartitionId.class), + Mockito.anyList())) .thenReturn(new ArrayList<>()); - Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(Collections.singletonList(patient)); + Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(RequestPartitionId.allPartitions(), + Collections.singletonList(patient)); Assertions.assertTrue(map.isEmpty()); } @@ -135,15 +162,17 @@ class BaseHapiFhirResourceDaoTest { inputList.add(patientIdAndType); inputList.add(patient2IdAndType); - Collection matches = Collections.singletonList( - createMatchEntryForGetIdsOfExistingResources(patientIdAndType, patientPID, patientResourceVersion) - ); - // when - Mockito.when(myIForcedIdDao.findResourcesByForcedId(Mockito.anyString(), Mockito.anyList())) - .thenReturn(matches); + Mockito.when(myIdHelperService + .resolveResourcePersistentIdsWithCache(Mockito.any(RequestPartitionId.class), + Mockito.anyList())) + .thenReturn(Collections.singletonList(getResourcePersistentIdFromResource(patientIdAndType, patientPID))); + Mockito.when(myResourceTableDao.getResourceVersionsForPid(Mockito.anyList())) + .thenReturn(Collections + .singletonList(getResourceTableRecordForResourceTypeAndPid(patientIdAndType.getResourceType(), patientPID, patientResourceVersion))); - Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(inputList); + Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(RequestPartitionId.allPartitions(), + inputList); // verify Assertions.assertFalse(map.isEmpty()); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java index fa953bc28e6..a53a084d7c2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; @@ -538,7 +539,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { AuditEvent createdEvent = myAuditEventDao.read(id); ourLog.info(myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(createdEvent)); - } @Test @@ -558,7 +558,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { IIdType id = myObservationDao.create(obsToCreate, mySrd).getId(); Observation createdObs = myObservationDao.read(id); - ourLog.info(myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(createdObs)); assertEquals("Patient/ABC", createdObs.getSubject().getReference()); } @@ -575,8 +574,9 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + // verify subresource is created Patient returned = myPatientDao.read(patientRef.getReferenceElement()); - Assertions.assertTrue(returned != null); + assertNotNull(returned); } @@ -607,10 +607,45 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { Bundle transaction = mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); Patient returned = myPatientDao.read(patientRef.getReferenceElement()); - Assertions.assertTrue(returned != null); + assertNotNull(returned); Assertions.assertTrue(returned.getActive()); + Assertions.assertEquals(2, returned.getIdElement().getVersionIdPartAsLong()); Observation retObservation = myObservationDao.read(obs.getIdElement()); - Assertions.assertTrue(retObservation != null); + assertNotNull(retObservation); } + + + @Test + public void testAutocreatePlaceholderWithExistingTargetWithServerAssignedIdTest() { + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + myModelConfig.setAutoVersionReferenceAtPaths("Observation.subject"); + + // create + Patient patient = new Patient(); + patient.setIdElement(new IdType("Patient")); + DaoMethodOutcome ret = myPatientDao.create(patient); + + // update + patient.setActive(true); + myPatientDao.update(patient); + + // observation (with version 2) + Observation obs = new Observation(); + obs.setId("Observation/DEF"); + Reference patientRef = new Reference("Patient/" + ret.getId().getIdPart()); + obs.setSubject(patientRef); + BundleBuilder builder = new BundleBuilder(myFhirCtx); + builder.addTransactionUpdateEntry(obs); + + Bundle transaction = mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle()); + + Patient returned = myPatientDao.read(patientRef.getReferenceElement()); + assertNotNull(returned); + Assertions.assertEquals(2, returned.getIdElement().getVersionIdPartAsLong()); + + Observation retObservation = myObservationDao.read(obs.getIdElement()); + assertNotNull(retObservation); + } + } From 69287b6bc3d9307ac8201e9cfcdb762f387412c3 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Tue, 7 Sep 2021 11:52:11 -0400 Subject: [PATCH 18/19] issue-2901 review fixes - moving methods to idhelperservice --- .../fhir/jpa/api/dao/IFhirResourceDao.java | 9 - .../ca/uhn/fhir/jpa/config/BaseConfig.java | 7 - .../jpa/dao/AutoVersioningServiceImpl.java | 44 ---- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 49 ----- .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 5 +- .../jpa/dao/BaseTransactionProcessor.java | 9 +- .../fhir/jpa/dao/IAutoVersioningService.java | 25 --- .../fhir/jpa/dao/index/IdHelperService.java | 89 ++++++++ .../dao/AutoVersioningServiceImplTests.java | 97 --------- .../jpa/dao/BaseHapiFhirResourceDaoTest.java | 166 -------------- .../jpa/dao/TransactionProcessorTest.java | 2 - .../jpa/dao/index/IdHelperServiceTest.java | 204 +++++++++++++++++- .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 3 - 13 files changed, 300 insertions(+), 409 deletions(-) delete mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java delete mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java delete mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java index 065149eef76..577faa0aee5 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java @@ -162,15 +162,6 @@ public interface IFhirResourceDao extends IDao { */ T read(IIdType theId); - /** - * Helper method to determine if some resources exist in the DB (without throwing). - * Returns a set that contains the IIdType for every resource found. - * If it's not found, it won't be included in the set. - * @param theIds - list of IIdType ids (for the same resource) - * @return - */ - Map getIdsOfExistingResources(RequestPartitionId partitionId, Collection theIds); - /** * Read a resource by its internal PID */ diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java index fb58b1912e0..2aebff2b539 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java @@ -27,11 +27,9 @@ import ca.uhn.fhir.jpa.bulk.imprt.api.IBulkDataImportSvc; import ca.uhn.fhir.jpa.bulk.imprt.svc.BulkDataImportSvcImpl; import ca.uhn.fhir.jpa.cache.IResourceVersionSvc; import ca.uhn.fhir.jpa.cache.ResourceVersionSvcDaoImpl; -import ca.uhn.fhir.jpa.dao.AutoVersioningServiceImpl; import ca.uhn.fhir.jpa.dao.DaoSearchParamProvider; import ca.uhn.fhir.jpa.dao.HistoryBuilder; import ca.uhn.fhir.jpa.dao.HistoryBuilderFactory; -import ca.uhn.fhir.jpa.dao.IAutoVersioningService; import ca.uhn.fhir.jpa.dao.ISearchBuilder; import ca.uhn.fhir.jpa.dao.LegacySearchBuilder; import ca.uhn.fhir.jpa.dao.MatchResourceUrlService; @@ -232,11 +230,6 @@ public abstract class BaseConfig { public void initSettings() { } - @Bean - public IAutoVersioningService autoVersioningService() { - return new AutoVersioningServiceImpl(); - } - public void setSearchCoordCorePoolSize(Integer searchCoordCorePoolSize) { this.searchCoordCorePoolSize = searchCoordCorePoolSize; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java deleted file mode 100644 index 151c9eb1191..00000000000 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java +++ /dev/null @@ -1,44 +0,0 @@ -package ca.uhn.fhir.jpa.dao; - -import ca.uhn.fhir.interceptor.model.RequestPartitionId; -import ca.uhn.fhir.jpa.api.dao.DaoRegistry; -import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; -import org.hl7.fhir.instance.model.api.IIdType; -import org.springframework.beans.factory.annotation.Autowired; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -public class AutoVersioningServiceImpl implements IAutoVersioningService { - - @Autowired - private DaoRegistry myDaoRegistry; - - @Override - public Map getExistingAutoversionsForIds(RequestPartitionId theRequestPartitionId, Collection theIds) { - HashMap idToPID = new HashMap<>(); - HashMap> resourceTypeToIds = new HashMap<>(); - - for (IIdType id : theIds) { - String resourceType = id.getResourceType(); - if (!resourceTypeToIds.containsKey(resourceType)) { - resourceTypeToIds.put(resourceType, new ArrayList<>()); - } - resourceTypeToIds.get(resourceType).add(id); - } - - for (String resourceType : resourceTypeToIds.keySet()) { - IFhirResourceDao dao = myDaoRegistry.getResourceDao(resourceType); - - Map idAndPID = dao.getIdsOfExistingResources(theRequestPartitionId, - resourceTypeToIds.get(resourceType)); - idToPID.putAll(idAndPID); - } - - return idToPID; - } -} 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 8a19b774710..458c728c62e 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 @@ -1158,55 +1158,6 @@ public abstract class BaseHapiFhirResourceDao extends B return myTransactionService.execute(theRequest, transactionDetails, tx -> doRead(theId, theRequest, theDeletedOk)); } - @Override - public Map getIdsOfExistingResources(RequestPartitionId thePartitionId, - Collection theIds) { - // these are the found Ids that were in the db - HashMap collected = new HashMap<>(); - - if (theIds == null || theIds.isEmpty()) { - return collected; - } - - List resourcePersistentIds = myIdHelperService.resolveResourcePersistentIdsWithCache(thePartitionId, - theIds.stream().collect(Collectors.toList())); - - // we'll use this map to fetch pids that require versions - HashMap pidsToVersionToResourcePid = new HashMap<>(); - - // fill in our map - for (ResourcePersistentId pid : resourcePersistentIds) { - if (pid.getVersion() == null) { - pidsToVersionToResourcePid.put(pid.getIdAsLong(), pid); - } - Optional idOp = theIds.stream() - .filter(i -> i.getIdPart().equals(pid.getAssociatedResourceId().getIdPart())) - .findFirst(); - // this should always be present - // since it was passed in. - // but land of optionals... - idOp.ifPresent(id -> { - collected.put(id, pid); - }); - } - - // set any versions we don't already have - if (!pidsToVersionToResourcePid.isEmpty()) { - Collection resourceEntries = myResourceTableDao - .getResourceVersionsForPid(new ArrayList<>(pidsToVersionToResourcePid.keySet())); - - for (Object[] record : resourceEntries) { - // order matters! - Long retPid = (Long)record[0]; - String resType = (String)record[1]; - Long version = (Long)record[2]; - pidsToVersionToResourcePid.get(retPid).setVersion(version); - } - } - - return collected; - } - public T doRead(IIdType theId, RequestDetails theRequest, boolean theDeletedOk) { assert TransactionSynchronizationManager.isActualTransactionActive(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java index 6df1d8fe44d..58aeae432ca 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java @@ -30,6 +30,7 @@ import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.model.LazyDaoMethodOutcome; +import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceTable; @@ -93,7 +94,7 @@ public abstract class BaseStorageDao { @Autowired protected ModelConfig myModelConfig; @Autowired - protected IAutoVersioningService myAutoVersioningService; + protected IdHelperService myIdHelperService; @Autowired protected DaoConfig myDaoConfig; @@ -210,7 +211,7 @@ public abstract class BaseStorageDao { IIdType referenceElement = nextReference.getReferenceElement(); if (!referenceElement.hasBaseUrl()) { - Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), + Map idToPID = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), Collections.singletonList(referenceElement) ); 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 f8db7e92b11..d554b645514 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 @@ -35,6 +35,7 @@ import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.model.DeleteConflict; import ca.uhn.fhir.jpa.api.model.DeleteConflictList; import ca.uhn.fhir.jpa.api.model.DeleteMethodOutcome; +import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; import ca.uhn.fhir.jpa.delete.DeleteConflictService; import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; @@ -159,7 +160,7 @@ public abstract class BaseTransactionProcessor { private TaskExecutor myExecutor ; @Autowired - private IAutoVersioningService myAutoVersioningService; + private IdHelperService myIdHelperService; @VisibleForTesting public void setDaoConfig(DaoConfig theDaoConfig) { @@ -696,7 +697,7 @@ public abstract class BaseTransactionProcessor { * * @param theEntries */ - private void consolidateDuplicateConditionalCreates(List theEntries) { + private void consolidateDuplicateConditionals(List theEntries) { final HashMap keyToUuid = new HashMap<>(); for (int index = 0, originalIndex = 0; index < theEntries.size(); index++, originalIndex++) { IBase nextReqEntry = theEntries.get(index); @@ -832,7 +833,7 @@ public abstract class BaseTransactionProcessor { /* * Look for duplicate conditional creates and consolidate them */ - consolidateDuplicateConditionalCreates(theEntries); + consolidateDuplicateConditionals(theEntries); /* * Loop through the request and process any entries of type @@ -1270,7 +1271,7 @@ public abstract class BaseTransactionProcessor { // get a map of // existing ids -> PID (for resources that exist in the DB) // should this be allPartitions? - Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), + Map idToPID = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), theReferencesToAutoVersion.stream() .map(ref -> ref.getReferenceElement()).collect(Collectors.toList())); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java deleted file mode 100644 index 80d327eb2d0..00000000000 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java +++ /dev/null @@ -1,25 +0,0 @@ -package ca.uhn.fhir.jpa.dao; - -import ca.uhn.fhir.interceptor.model.RequestPartitionId; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; -import org.hl7.fhir.instance.model.api.IIdType; - -import java.util.Collection; -import java.util.Map; - -public interface IAutoVersioningService { - /** - * Takes in a list of IIdType and returns a map of - * IIdType -> ResourcePersistentId. - * ResourcePersistentId will contain the history version - * but the returned IIdType will not (this is to allow consumers - * to use their passed in IIdType as a lookup value). - * - * If the returned map does not return an IIdType -> ResourcePersistentId - * then it means that it is a non-existing resource in the DB - * @param theIds - * @param thePartitionId - the partition id - * @return - */ - Map getExistingAutoversionsForIds(RequestPartitionId thePartitionId, Collection theIds); -} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java index b667e3a1375..03763d40a0f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java @@ -530,6 +530,95 @@ public class IdHelperService { return retVal; } + /** + * Helper method to determine if some resources exist in the DB (without throwing). + * Returns a set that contains the IIdType for every resource found. + * If it's not found, it won't be included in the set. + * @param theIds - list of IIdType ids (for the same resource) + * @return + */ + private Map getIdsOfExistingResources(RequestPartitionId thePartitionId, + Collection theIds) { + // these are the found Ids that were in the db + HashMap collected = new HashMap<>(); + + if (theIds == null || theIds.isEmpty()) { + return collected; + } + + List resourcePersistentIds = resolveResourcePersistentIdsWithCache(thePartitionId, + theIds.stream().collect(Collectors.toList())); + + // we'll use this map to fetch pids that require versions + HashMap pidsToVersionToResourcePid = new HashMap<>(); + + // fill in our map + for (ResourcePersistentId pid : resourcePersistentIds) { + if (pid.getVersion() == null) { + pidsToVersionToResourcePid.put(pid.getIdAsLong(), pid); + } + Optional idOp = theIds.stream() + .filter(i -> i.getIdPart().equals(pid.getAssociatedResourceId().getIdPart())) + .findFirst(); + // this should always be present + // since it was passed in. + // but land of optionals... + idOp.ifPresent(id -> { + collected.put(id, pid); + }); + } + + // set any versions we don't already have + if (!pidsToVersionToResourcePid.isEmpty()) { + Collection resourceEntries = myResourceTableDao + .getResourceVersionsForPid(new ArrayList<>(pidsToVersionToResourcePid.keySet())); + + for (Object[] record : resourceEntries) { + // order matters! + Long retPid = (Long)record[0]; + String resType = (String)record[1]; + Long version = (Long)record[2]; + pidsToVersionToResourcePid.get(retPid).setVersion(version); + } + } + + return collected; + } + + /** + * Retrieves the latest versions for any resourceid that are found. + * If they are not found, they will not be contained in the returned map. + * The key should be the same value that was passed in to allow + * consumer to look up the value using the id they already have. + * + * This method should not throw, so it can safely be consumed in + * transactions. + * + * @param theRequestPartitionId - request partition id + * @param theIds - list of IIdTypes for resources of interest. + * @return + */ + public Map getLatestVersionIdsForResourceIds(RequestPartitionId theRequestPartitionId, Collection theIds) { + HashMap idToPID = new HashMap<>(); + HashMap> resourceTypeToIds = new HashMap<>(); + + for (IIdType id : theIds) { + String resourceType = id.getResourceType(); + if (!resourceTypeToIds.containsKey(resourceType)) { + resourceTypeToIds.put(resourceType, new ArrayList<>()); + } + resourceTypeToIds.get(resourceType).add(id); + } + + for (String resourceType : resourceTypeToIds.keySet()) { + Map idAndPID = getIdsOfExistingResources(theRequestPartitionId, + resourceTypeToIds.get(resourceType)); + idToPID.putAll(idAndPID); + } + + return idToPID; + } + /** * @deprecated This method doesn't take a partition ID as input, so it is unsafe. It * should be reworked to include the partition ID before any new use is incorporated diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java deleted file mode 100644 index 206b6c968ec..00000000000 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java +++ /dev/null @@ -1,97 +0,0 @@ -package ca.uhn.fhir.jpa.dao; - -import ca.uhn.fhir.interceptor.model.RequestPartitionId; -import ca.uhn.fhir.jpa.api.dao.DaoRegistry; -import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; -import ca.uhn.fhir.model.primitive.IdDt; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; -import org.hl7.fhir.instance.model.api.IIdType; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.jupiter.MockitoExtension; - -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; - -@ExtendWith(MockitoExtension.class) -public class AutoVersioningServiceImplTests { - - @Mock - private DaoRegistry daoRegistry; - - @InjectMocks - private AutoVersioningServiceImpl myAutoversioningService; - - @Test - public void getAutoversionsForIds_whenResourceExists_returnsMapWithPIDAndVersion() { - IIdType type = new IdDt("Patient/RED"); - ResourcePersistentId pid = new ResourcePersistentId(1); - pid.setVersion(2L); - HashMap map = new HashMap<>(); - map.put(type, pid); - - IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); - Mockito.when(daoMock.getIdsOfExistingResources(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(map); - Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) - .thenReturn(daoMock); - - Map retMap = myAutoversioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), - Collections.singletonList(type)); - - Assertions.assertTrue(retMap.containsKey(type)); - Assertions.assertEquals(pid.getVersion(), map.get(type).getVersion()); - } - - @Test - public void getAutoversionsForIds_whenResourceDoesNotExist_returnsEmptyMap() { - IIdType type = new IdDt("Patient/RED"); - - IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); - Mockito.when(daoMock.getIdsOfExistingResources(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(new HashMap<>()); - Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) - .thenReturn(daoMock); - - Map retMap = myAutoversioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), - Collections.singletonList(type)); - - Assertions.assertTrue(retMap.isEmpty()); - } - - @Test - public void getAutoversionsForIds_whenSomeResourcesDoNotExist_returnsOnlyExistingElements() { - IIdType type = new IdDt("Patient/RED"); - ResourcePersistentId pid = new ResourcePersistentId(1); - pid.setVersion(2L); - HashMap map = new HashMap<>(); - map.put(type, pid); - IIdType type2 = new IdDt("Patient/BLUE"); - - // when - IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); - Mockito.when(daoMock.getIdsOfExistingResources(Mockito.any(RequestPartitionId.class), Mockito.anyList())) - .thenReturn(map); - Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) - .thenReturn(daoMock); - - // test - Map retMap = myAutoversioningService.getExistingAutoversionsForIds( - RequestPartitionId.allPartitions(), - Arrays.asList(type, type2) - ); - - // verify - Assertions.assertEquals(map.size(), retMap.size()); - Assertions.assertTrue(retMap.containsKey(type)); - Assertions.assertFalse(retMap.containsKey(type2)); - } -} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java index dd9cff05d3a..4936631f818 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java @@ -1,188 +1,22 @@ package ca.uhn.fhir.jpa.dao; -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.RuntimeResourceDefinition; -import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.config.DaoConfig; -import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; -import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; -import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; -import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.server.RequestDetails; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Patient; -import org.hl7.fhir.r4.model.Request; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.MockitoTestRule; import org.mockito.junit.jupiter.MockitoExtension; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @ExtendWith(MockitoExtension.class) class BaseHapiFhirResourceDaoTest { - // our simple concrete test class for BaseHapiFhirResourceDao - private class SimpleTestDao extends BaseHapiFhirResourceDao { - public SimpleTestDao() { - super(); - // post inject hooks - setResourceType(Patient.class); - RuntimeResourceDefinition resourceDefinition = Mockito.mock(RuntimeResourceDefinition.class); - Mockito.when(resourceDefinition.getName()).thenReturn("Patient"); - FhirContext myFhirContextMock = Mockito.mock(FhirContext.class); - Mockito.when(myFhirContextMock.getResourceDefinition(Mockito.any(Class.class))) - .thenReturn(resourceDefinition); - setContext(myFhirContextMock); - postConstruct(); - } - - } - - @InjectMocks - private BaseHapiFhirResourceDao myBaseHapiFhirResourceDao = new SimpleTestDao(); - -// @Mock -// private IForcedIdDao myIForcedIdDao; - - @Mock - private IdHelperService myIdHelperService; - - @Mock - private IResourceTableDao myResourceTableDao; - - //TODO - all other dependency mocks - - - private ResourcePersistentId getResourcePersistentIdFromResource(IIdType theId, long thePid) { - ResourcePersistentId id = new ResourcePersistentId(thePid); - String idPortion = theId.getIdPart(); - IIdType newId = new IdDt(theId.getResourceType(), idPortion); - id.setAssociatedResourceId(newId); - return id; - } - - /** - * Gets a ResourceTable record for getResourceVersionsForPid - * Order matters! - * @param resourceType - * @param pid - * @param version - * @return - */ - private Object[] getResourceTableRecordForResourceTypeAndPid(String resourceType, long pid, long version) { - return new Object[] { - pid, // long - resourceType, // string - version // long - }; - } - - @Test - public void getIdsOfExistingResources_forExistingResources_returnsMapOfIdToPIDWithVersion() { - // setup - IIdType patientIdAndType = new IdDt("Patient/RED"); - long patientPID = 1L; - long patientResourceVersion = 2L; - ResourcePersistentId pat1ResourcePID = getResourcePersistentIdFromResource(patientIdAndType, patientPID); - IIdType patient2IdAndType = new IdDt("Patient/BLUE"); - long patient2PID = 3L; - long patient2ResourceVersion = 4L; - ResourcePersistentId pat2ResourcePID = getResourcePersistentIdFromResource(patient2IdAndType, patient2PID); - List inputList = new ArrayList<>(); - inputList.add(patientIdAndType); - inputList.add(patient2IdAndType); - - Collection matches = Arrays.asList( - getResourceTableRecordForResourceTypeAndPid(patientIdAndType.getResourceType(), patientPID, patientResourceVersion), - getResourceTableRecordForResourceTypeAndPid(patient2IdAndType.getResourceType(), patient2PID, patient2ResourceVersion) - ); - - // when - Mockito.when(myIdHelperService.resolveResourcePersistentIdsWithCache(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(Arrays.asList(pat1ResourcePID, pat2ResourcePID)); - Mockito.when(myResourceTableDao.getResourceVersionsForPid(Mockito.anyList())) - .thenReturn(matches); - - Map idToPIDOfExistingResources = myBaseHapiFhirResourceDao.getIdsOfExistingResources(RequestPartitionId.allPartitions(), - inputList); - - Assertions.assertEquals(inputList.size(), idToPIDOfExistingResources.size()); - Assertions.assertTrue(idToPIDOfExistingResources.containsKey(patientIdAndType)); - Assertions.assertTrue(idToPIDOfExistingResources.containsKey(patient2IdAndType)); - - Assertions.assertEquals(patientPID, idToPIDOfExistingResources.get(patientIdAndType).getIdAsLong()); - Assertions.assertEquals(patient2PID, idToPIDOfExistingResources.get(patient2IdAndType).getIdAsLong(), patient2PID); - Assertions.assertEquals(patientResourceVersion, idToPIDOfExistingResources.get(patientIdAndType).getVersion()); - Assertions.assertEquals(patient2ResourceVersion, idToPIDOfExistingResources.get(patient2IdAndType).getVersion()); - } - - @Test - public void getIdsOfExistingResources_forNonExistentResources_returnsEmptyMap() { - //setup - IIdType patient = new IdDt("Patient/RED"); - - // when - Mockito.when(myIdHelperService.resolveResourcePersistentIdsWithCache(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(new ArrayList<>()); - - Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(RequestPartitionId.allPartitions(), - Collections.singletonList(patient)); - - Assertions.assertTrue(map.isEmpty()); - } - - @Test - public void getIdsOfExistingResources_whenSomeResourcesExist_returnsOnlyExistingResourcesInMap() { - // setup - IIdType patientIdAndType = new IdDt("Patient/RED"); - long patientPID = 1L; - long patientResourceVersion = 2L; - IIdType patient2IdAndType = new IdDt("Patient/BLUE"); - List inputList = new ArrayList<>(); - inputList.add(patientIdAndType); - inputList.add(patient2IdAndType); - - // when - Mockito.when(myIdHelperService - .resolveResourcePersistentIdsWithCache(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(Collections.singletonList(getResourcePersistentIdFromResource(patientIdAndType, patientPID))); - Mockito.when(myResourceTableDao.getResourceVersionsForPid(Mockito.anyList())) - .thenReturn(Collections - .singletonList(getResourceTableRecordForResourceTypeAndPid(patientIdAndType.getResourceType(), patientPID, patientResourceVersion))); - - Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(RequestPartitionId.allPartitions(), - inputList); - - // verify - Assertions.assertFalse(map.isEmpty()); - Assertions.assertEquals(inputList.size() - 1, map.size()); - Assertions.assertTrue(map.containsKey(patientIdAndType)); - Assertions.assertFalse(map.containsKey(patient2IdAndType)); - } - - /*******************/ - TestResourceDao mySvc = new TestResourceDao(); @Test diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java index 907650ba59b..0bace079a8f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java @@ -70,8 +70,6 @@ public class TransactionProcessorTest { private MatchUrlService myMatchUrlService; @MockBean private IRequestPartitionHelperSvc myRequestPartitionHelperSvc; - @MockBean - private IAutoVersioningService myAutoVersioningService; @MockBean(answer = Answers.RETURNS_DEEP_STUBS) private SessionImpl mySession; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java index 20e100065f1..ad9fbc78c44 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java @@ -1,13 +1,215 @@ package ca.uhn.fhir.jpa.dao.index; import ca.uhn.fhir.interceptor.model.RequestPartitionId; +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.model.entity.ForcedId; +import ca.uhn.fhir.jpa.util.MemoryCacheService; +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; +import org.hl7.fhir.instance.model.api.IIdType; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; -import static org.junit.jupiter.api.Assertions.*; +import javax.persistence.EntityManager; +import javax.persistence.TypedQuery; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.Path; +import javax.persistence.criteria.Root; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +@ExtendWith(MockitoExtension.class) public class IdHelperServiceTest { + // helper class to package up data for helper methods + private class ResourceIdPackage { + public IIdType MyResourceId; + public ResourcePersistentId MyPid; + public Long MyVersion; + + public ResourceIdPackage(IIdType id, + ResourcePersistentId pid, + Long version) { + MyResourceId = id; + MyPid = pid; + MyVersion = version; + } + } + + @Mock + private IResourceTableDao myResourceTableDao; + + @Mock + private DaoConfig myDaoConfig; + + @Mock + private MemoryCacheService myMemoryCacheService; + + @Mock + private EntityManager myEntityManager; + + @InjectMocks + private IdHelperService myIdHelperService; + + private ResourcePersistentId getResourcePersistentIdFromResource(IIdType theId, long thePid) { + ResourcePersistentId id = new ResourcePersistentId(thePid); + String idPortion = theId.getIdPart(); + IIdType newId = new IdDt(theId.getResourceType(), idPortion); + id.setAssociatedResourceId(newId); + return id; + } + + /** + * Gets a ResourceTable record for getResourceVersionsForPid + * Order matters! + * @param resourceType + * @param pid + * @param version + * @return + */ + private Object[] getResourceTableRecordForResourceTypeAndPid(String resourceType, long pid, long version) { + return new Object[] { + pid, // long + resourceType, // string + version // long + }; + } + + /** + * Helper function to mock out resolveResourcePersistentIdsWithCache + * to return empty lists (as if no resources were found). + */ + private void mock_resolveResourcePersistentIdsWithCache_toReturnNothing() { + CriteriaBuilder cb = Mockito.mock(CriteriaBuilder.class); + CriteriaQuery criteriaQuery = Mockito.mock(CriteriaQuery.class); + Root from = Mockito.mock(Root.class); + Path path = Mockito.mock(Path.class); + + Mockito.when(cb.createQuery(Mockito.any(Class.class))) + .thenReturn(criteriaQuery); + Mockito.when(criteriaQuery.from(Mockito.any(Class.class))) + .thenReturn(from); + Mockito.when(from.get(Mockito.anyString())) + .thenReturn(path); + + TypedQuery queryMock = Mockito.mock(TypedQuery.class); + Mockito.when(queryMock.getResultList()).thenReturn(new ArrayList<>()); // not found + + Mockito.when(myEntityManager.getCriteriaBuilder()) + .thenReturn(cb); + Mockito.when(myEntityManager.createQuery(Mockito.any(CriteriaQuery.class))) + .thenReturn(queryMock); + } + + /** + * Helper function to mock out getIdsOfExistingResources + * to return the matches and resources matching those provided + * by parameters. + * @param theResourcePacks + */ + private void mockReturnsFor_getIdsOfExistingResources(ResourceIdPackage... theResourcePacks) { + List resourcePersistentIds = new ArrayList<>(); + List matches = new ArrayList<>(); + + for (ResourceIdPackage pack : theResourcePacks) { + resourcePersistentIds.add(pack.MyPid); + + matches.add(getResourceTableRecordForResourceTypeAndPid( + pack.MyResourceId.getResourceType(), + pack.MyPid.getIdAsLong(), + pack.MyVersion + )); + } + + ResourcePersistentId first = resourcePersistentIds.remove(0); + if (resourcePersistentIds.isEmpty()) { + Mockito.when(myMemoryCacheService.getIfPresent(Mockito.any(MemoryCacheService.CacheEnum.class), Mockito.anyString())) + .thenReturn(first).thenReturn(null); + } + else { + Mockito.when(myMemoryCacheService.getIfPresent(Mockito.any(MemoryCacheService.CacheEnum.class), Mockito.anyString())) + .thenReturn(first, resourcePersistentIds.toArray()); + } + Mockito.when(myResourceTableDao.getResourceVersionsForPid(Mockito.anyList())) + .thenReturn(matches); + } + + @Test + public void getLatestVersionIdsForResourceIds_whenResourceExists_returnsMapWithPIDAndVersion() { + IIdType type = new IdDt("Patient/RED"); + ResourcePersistentId pid = new ResourcePersistentId(1L); + pid.setAssociatedResourceId(type); + HashMap map = new HashMap<>(); + map.put(type, pid); + ResourceIdPackage pack = new ResourceIdPackage(type, pid, 2L); + + // when + mockReturnsFor_getIdsOfExistingResources(pack); + + // test + Map retMap = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + Collections.singletonList(type)); + + Assertions.assertTrue(retMap.containsKey(type)); + Assertions.assertEquals(pid.getVersion(), map.get(type).getVersion()); + } + + @Test + public void getLatestVersionIdsForResourceIds_whenResourceDoesNotExist_returnsEmptyMap() { + IIdType type = new IdDt("Patient/RED"); + + // when + mock_resolveResourcePersistentIdsWithCache_toReturnNothing(); + + // test + Map retMap = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + Collections.singletonList(type)); + + Assertions.assertTrue(retMap.isEmpty()); + } + + @Test + public void getLatestVersionIdsForResourceIds_whenSomeResourcesDoNotExist_returnsOnlyExistingElements() { + // resource to be found + IIdType type = new IdDt("Patient/RED"); + ResourcePersistentId pid = new ResourcePersistentId(1L); + pid.setAssociatedResourceId(type); + ResourceIdPackage pack = new ResourceIdPackage(type, pid, 2L); + + // resource that won't be found + IIdType type2 = new IdDt("Patient/BLUE"); + + // when + mock_resolveResourcePersistentIdsWithCache_toReturnNothing(); + mockReturnsFor_getIdsOfExistingResources(pack); + + // test + Map retMap = myIdHelperService.getLatestVersionIdsForResourceIds( + RequestPartitionId.allPartitions(), + Arrays.asList(type, type2) + ); + + // verify + Assertions.assertEquals(1, retMap.size()); + Assertions.assertTrue(retMap.containsKey(type)); + Assertions.assertFalse(retMap.containsKey(type2)); + } + @Test public void testReplaceDefault_AllPartitions() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index 39660fe7399..95a9732fed5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -20,7 +20,6 @@ import ca.uhn.fhir.jpa.binstore.BinaryStorageInterceptor; import ca.uhn.fhir.jpa.bulk.export.api.IBulkDataExportSvc; import ca.uhn.fhir.jpa.config.TestR4Config; import ca.uhn.fhir.jpa.dao.BaseJpaTest; -import ca.uhn.fhir.jpa.dao.IAutoVersioningService; import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.dao.data.IMdmLinkDao; @@ -499,8 +498,6 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil protected ValidationSettings myValidationSettings; @Autowired protected IMdmLinkDao myMdmLinkDao; - @Autowired - protected IAutoVersioningService myAutoVersioningService; @AfterEach() public void afterCleanupDao() { From 4d7f53851cdbe2f52336b3ab69f019ae50c7cd51 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Tue, 7 Sep 2021 12:08:32 -0400 Subject: [PATCH 19/19] issue-2901 cleanup of code/imports --- .../java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java | 1 - .../java/ca/uhn/fhir/jpa/api/dao/IFhirSystemDao.java | 1 - .../uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java | 3 --- .../ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java | 8 -------- .../r4/FhirResourceDaoCreatePlaceholdersR4Test.java | 10 ++++++---- .../ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 1 - 6 files changed, 6 insertions(+), 18 deletions(-) diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java index 577faa0aee5..ff8fa2a2303 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java @@ -21,7 +21,6 @@ package ca.uhn.fhir.jpa.api.dao; */ import ca.uhn.fhir.context.RuntimeResourceDefinition; -import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.model.DeleteConflictList; import ca.uhn.fhir.jpa.api.model.DeleteMethodOutcome; diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirSystemDao.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirSystemDao.java index 049e460294b..ff3a34fc4eb 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirSystemDao.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirSystemDao.java @@ -22,7 +22,6 @@ package ca.uhn.fhir.jpa.api.dao; import ca.uhn.fhir.jpa.api.model.ExpungeOptions; import ca.uhn.fhir.jpa.api.model.ExpungeOutcome; -import ca.uhn.fhir.rest.annotation.Offset; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.RequestDetails; import org.hl7.fhir.instance.model.api.IBaseBundle; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java index 4936631f818..67badec3ad5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java @@ -8,13 +8,10 @@ import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import org.hl7.fhir.r4.model.Patient; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.junit.jupiter.MockitoExtension; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; -@ExtendWith(MockitoExtension.class) class BaseHapiFhirResourceDaoTest { TestResourceDao mySvc = new TestResourceDao(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java index ad9fbc78c44..2ca8fea957c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java @@ -66,14 +66,6 @@ public class IdHelperServiceTest { @InjectMocks private IdHelperService myIdHelperService; - private ResourcePersistentId getResourcePersistentIdFromResource(IIdType theId, long thePid) { - ResourcePersistentId id = new ResourcePersistentId(thePid); - String idPortion = theId.getIdPart(); - IIdType newId = new IdDt(theId.getResourceType(), idPortion); - id.setAssociatedResourceId(newId); - return id; - } - /** * Gets a ResourceTable record for getResourceVersionsForPid * Order matters! diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java index a53a084d7c2..1588c81ba80 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java @@ -590,7 +590,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { // create Patient patient = new Patient(); patient.setIdElement(new IdType(patientId)); - myPatientDao.update(patient); + myPatientDao.update(patient); // use update to use forcedid // update patient.setActive(true); @@ -615,7 +615,9 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertNotNull(retObservation); } - + /** + * This test is the same as above, except it uses the serverid (instead of forcedid) + */ @Test public void testAutocreatePlaceholderWithExistingTargetWithServerAssignedIdTest() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); @@ -624,9 +626,9 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { // create Patient patient = new Patient(); patient.setIdElement(new IdType("Patient")); - DaoMethodOutcome ret = myPatientDao.create(patient); + DaoMethodOutcome ret = myPatientDao.create(patient); // use create to use server id - // update + // update - to update our version patient.setActive(true); myPatientDao.update(patient); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index cc081ca421d..b9257a30e0b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java @@ -1,7 +1,6 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.api.config.DaoConfig; -import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.NormalizedQuantitySearchLevel;