From 606b642b1eb1ad7d58a9c571507843fc72bf9895 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 27 Jul 2020 17:29:27 -0700 Subject: [PATCH] Turns out expunge atomic integer didn't do what I expected for historical versions... --- .../uhn/fhir/jpa/empi/dao/EmpiLinkDaoSvc.java | 2 +- .../fhir/jpa/empi/svc/EmpiResetSvcImpl.java | 69 ++++++++++--------- .../provider/EmpiProviderBatchR4Test.java | 4 +- .../provider/EmpiProviderClearLinkR4Test.java | 8 +-- .../jpa/empi/svc/EmpiBatchSvcImplTest.java | 3 +- 5 files changed, 47 insertions(+), 39 deletions(-) diff --git a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/dao/EmpiLinkDaoSvc.java b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/dao/EmpiLinkDaoSvc.java index bb71ce505fb..cc6178ea81a 100644 --- a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/dao/EmpiLinkDaoSvc.java +++ b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/dao/EmpiLinkDaoSvc.java @@ -238,7 +238,7 @@ public class EmpiLinkDaoSvc { } private List deleteEmpiLinksAndReturnPersonPids(List theLinks) { - List collect = theLinks.stream().map(EmpiLink::getPersonPid).collect(Collectors.toList()); + List collect = theLinks.stream().map(EmpiLink::getPersonPid).distinct().collect(Collectors.toList()); theLinks.forEach(empiLink -> myEmpiLinkDao.delete(empiLink)); return collect; } diff --git a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiResetSvcImpl.java b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiResetSvcImpl.java index de94f2e4de8..890f6c48dbd 100644 --- a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiResetSvcImpl.java +++ b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiResetSvcImpl.java @@ -26,7 +26,6 @@ import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.api.model.DeleteConflict; import ca.uhn.fhir.jpa.api.model.DeleteConflictList; -import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; import ca.uhn.fhir.jpa.dao.expunge.IResourceExpungeService; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; import ca.uhn.fhir.jpa.empi.dao.EmpiLinkDaoSvc; @@ -41,7 +40,6 @@ import org.springframework.beans.factory.annotation.Autowired; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.Collectors; /** * This class is in charge of Clearing out existing EMPI links, as well as deleting all persons related to those EMPI Links. @@ -52,13 +50,8 @@ public class EmpiResetSvcImpl implements IEmpiResetSvc { @Autowired EmpiLinkDaoSvc myEmpiLinkDaoSvc; - @Autowired private IResourceExpungeService myResourceExpungeService; - - @Autowired - private IResourceTableDao myResourceTable; - @Autowired private DaoRegistry myDaoRegistry; @Autowired @@ -67,12 +60,35 @@ public class EmpiResetSvcImpl implements IEmpiResetSvc { @Override public long expungeAllEmpiLinksOfTargetType(String theResourceType) { throwExceptionIfInvalidTargetType(theResourceType); - List longs = myEmpiLinkDaoSvc.deleteAllEmpiLinksOfTypeAndReturnPersonPids(theResourceType); - myResourceExpungeService.expungeCurrentVersionOfResources(null, longs, new AtomicInteger(longs.size())); + deleteResourcesAndHandleConflicts(longs); + expungeHistoricalAndCurrentVersiondsOfIds(longs); return longs.size(); } + /** + * Function which will delete all resources by their PIDs, and also delete any resources that were undeletable due to + * VersionConflictException + * @param theLongs + */ + private void deleteResourcesAndHandleConflicts(List theLongs) { + DeleteConflictList + deleteConflictList = new DeleteConflictList(); + myTransactionService.execute(null, tx -> { + theLongs.stream().forEach(pid -> deleteCascade(pid, deleteConflictList)); + return null; + }); + + IFhirResourceDao personDao = myDaoRegistry.getResourceDao("Person"); + while (!deleteConflictList.isEmpty()) { + myTransactionService.execute(null, tx -> { + deleteConflictBatch(deleteConflictList, personDao); + return null; + }); + + } + } + private void throwExceptionIfInvalidTargetType(String theResourceType) { if (!EmpiUtil.supportedTargetType(theResourceType)) { throw new InvalidRequestException(ProviderConstants.EMPI_CLEAR + " does not support resource type: " + theResourceType); @@ -85,30 +101,19 @@ public class EmpiResetSvcImpl implements IEmpiResetSvc { @Override public long expungeAllEmpiLinks() { List longs = myEmpiLinkDaoSvc.deleteAllEmpiLinksAndReturnPersonPids(); - longs = longs.stream() - .distinct().collect(Collectors.toList()); - - DeleteConflictList dlc = new DeleteConflictList(); - List finalLongs = longs; - myTransactionService.execute(null, tx -> { - finalLongs.stream().forEach(pid -> { - deleteCascade(pid, dlc); - }); - return null; - }); - - IFhirResourceDao personDao = myDaoRegistry.getResourceDao("Person"); - while (!dlc.isEmpty()) { - myTransactionService.execute(null, tx -> { - deleteConflictBatch(dlc, personDao); - return null; - }); - } - myResourceExpungeService.expungeHistoricalVersionsOfIds(null, longs, new AtomicInteger(longs.size())); - //myResourceExpungeService.expungeCurrentVersionOfResources(null, longs, new AtomicInteger(longs.size())); + deleteResourcesAndHandleConflicts(longs); + expungeHistoricalAndCurrentVersiondsOfIds(longs); return longs.size(); } + /** + * Use the expunge service to expunge all historical and current versions of the resources associated to the PIDs. + */ + private void expungeHistoricalAndCurrentVersiondsOfIds(List theLongs) { + myResourceExpungeService.expungeHistoricalVersionsOfIds(null, theLongs, new AtomicInteger(Integer.MAX_VALUE - 1)); + myResourceExpungeService.expungeCurrentVersionOfResources(null, theLongs, new AtomicInteger(Integer.MAX_VALUE - 1)); + } + private void deleteConflictBatch(DeleteConflictList theDcl, IFhirResourceDao theDao) { DeleteConflictList newBatch = new DeleteConflictList(); for (DeleteConflict next: theDcl) { @@ -120,10 +125,10 @@ public class EmpiResetSvcImpl implements IEmpiResetSvc { theDcl.addAll(newBatch); } - private void deleteCascade(Long pid, DeleteConflictList theDcl) { + private void deleteCascade(Long pid, DeleteConflictList theDeleteConflictList) { ourLog.debug("About to cascade delete: " + pid); IFhirResourceDao resourceDao = myDaoRegistry.getResourceDao("Person"); - resourceDao.delete(new IdType("Person/" + pid), theDcl, null, null); + resourceDao.delete(new IdType("Person/" + pid), theDeleteConflictList, null, null); } } diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderBatchR4Test.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderBatchR4Test.java index c8453400c16..a067eaeb4b2 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderBatchR4Test.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderBatchR4Test.java @@ -14,6 +14,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import java.io.IOException; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -42,7 +44,7 @@ public class EmpiProviderBatchR4Test extends BaseLinkR4Test { } @AfterEach - public void after() { + public void after() throws IOException { myInterceptorService.unregisterInterceptor(afterEmpiLatch); super.after(); } diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderClearLinkR4Test.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderClearLinkR4Test.java index c19b8314679..7eba048441b 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderClearLinkR4Test.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderClearLinkR4Test.java @@ -66,7 +66,7 @@ public class EmpiProviderClearLinkR4Test extends BaseLinkR4Test { assertLinkCount(2); Person read = myPersonDao.read(new IdDt(myPersonId.getValueAsString()).toVersionless()); assertThat(read, is(notNullValue())); - myEmpiProviderR4.clearEmpiLinks(new StringType("patient")); + myEmpiProviderR4.clearEmpiLinks(new StringType("Patient")); assertNoPatientLinksExist(); try { myPersonDao.read(new IdDt(myPersonId.getValueAsString()).toVersionless()); @@ -141,7 +141,7 @@ public class EmpiProviderClearLinkR4Test extends BaseLinkR4Test { assertLinkCount(2); Person read = myPersonDao.read(new IdDt(myPractitionerPersonId.getValueAsString()).toVersionless()); assertThat(read, is(notNullValue())); - myEmpiProviderR4.clearEmpiLinks(new StringType("practitioner")); + myEmpiProviderR4.clearEmpiLinks(new StringType("Practitioner")); assertNoPractitionerLinksExist(); try { myPersonDao.read(new IdDt(myPractitionerPersonId.getValueAsString()).toVersionless()); @@ -152,10 +152,10 @@ public class EmpiProviderClearLinkR4Test extends BaseLinkR4Test { @Test public void testClearInvalidTargetType() { try { - myEmpiProviderR4.clearEmpiLinks(new StringType("observation")); + myEmpiProviderR4.clearEmpiLinks(new StringType("Observation")); fail(); } catch (InvalidRequestException e) { - assertThat(e.getMessage(), is(equalTo("$empi-clear does not support resource type: observation"))); + assertThat(e.getMessage(), is(equalTo("$empi-clear does not support resource type: Observation"))); } } diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiBatchSvcImplTest.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiBatchSvcImplTest.java index cfd04c34f9c..afb0736cdb5 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiBatchSvcImplTest.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiBatchSvcImplTest.java @@ -11,6 +11,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import java.io.IOException; import java.util.Date; class EmpiBatchSvcImplTest extends BaseEmpiR4Test { @@ -28,7 +29,7 @@ class EmpiBatchSvcImplTest extends BaseEmpiR4Test { myInterceptorService.registerAnonymousInterceptor(Pointcut.EMPI_AFTER_PERSISTED_RESOURCE_CHECKED, afterEmpiLatch); } @AfterEach - public void after() { + public void after() throws IOException { myInterceptorService.unregisterInterceptor(afterEmpiLatch); afterEmpiLatch.clear(); super.after();