Recover from failed merge again. This time with feeling!

hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java

@@ -1166,9 +1166,14 @@ public enum Pointcut {
 	/**
 	 * Invoked before a resource is about to be expunged via the <code>$expunge</code> operation.
 	 * <p>
+	 * Hooks will be passed a reference to a counter containing the current number of records that have been deleted.
+	 * If the hook deletes any records, the hook is expected to increment this counter by the number of records deleted.
+	 * </p>
+	 * <p>
 	 * Hooks may accept the following parameters:
 	 * </p>
 	 * <ul>
+	 * <li>java.util.concurrent.atomic.AtomicInteger - The counter holding the number of records deleted.</li>
 	 * <li>org.hl7.fhir.instance.model.api.IIdType - The ID of the resource that is about to be deleted</li>
 	 * <li>org.hl7.fhir.instance.model.api.IBaseResource - The resource that is about to be deleted</li>
 	 * <li>
@@ -1193,6 +1198,7 @@ public enum Pointcut {
 		// Return type
 		void.class,
 		// Params
+		"java.util.concurrent.atomic.AtomicInteger",
 		"org.hl7.fhir.instance.model.api.IIdType",
 		"org.hl7.fhir.instance.model.api.IBaseResource",
 		"ca.uhn.fhir.rest.api.server.RequestDetails",

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/binstore/BinaryStorageInterceptor.java

@@ -32,6 +32,7 @@ import org.hl7.fhir.instance.model.api.IPrimitiveType;
 import org.springframework.beans.factory.annotation.Autowired;
 
 import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
 @Interceptor
@@ -43,7 +44,7 @@ public class BinaryStorageInterceptor {
 	private FhirContext myCtx;
 
 	@Hook(Pointcut.STORAGE_PRESTORAGE_EXPUNGE_RESOURCE)
-	public void expungeResource(IBaseResource theResource) {
+	public void expungeResource(AtomicInteger theCounter, IBaseResource theResource) {
 
 		Class<? extends IBase> binaryType = myCtx.getElementDefinition("base64Binary").getImplementingClass();
 		List<? extends IBase> binaryElements = myCtx.newTerser().getAllPopulatedChildElementsOfType(theResource, binaryType);
@@ -57,6 +58,7 @@ public class BinaryStorageInterceptor {
 
 		for (String next : attachmentIds) {
 			myBinaryStorageSvc.expungeBlob(theResource.getIdElement(), next);
+			theCounter.incrementAndGet();
 		}
 
 	}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ExpungeOperation.java

@@ -73,8 +73,6 @@ public class ExpungeOperation implements Callable<ExpungeOutcome> {
 	public ExpungeOutcome call() {
 		final IdDt id;
 
-		callHooks();
-
 		if (expungeLimitReached()) {
 			return expungeOutcome();
 		}
@@ -97,30 +95,6 @@ public class ExpungeOperation implements Callable<ExpungeOutcome> {
 		return expungeOutcome();
 	}
 
-	private void callHooks() {
-		final AtomicInteger counter = new AtomicInteger();
-
-		if (myResourceId == null) {
-			return;
-		}
-		IdDt id;
-		if (myVersion == null) {
-			id = new IdDt(myResourceName, myResourceId);
-		} else {
-			id = new IdDt(myResourceName, myResourceId.toString(), myVersion.toString());
-		}
-
-		// Notify Interceptors about pre-action call
-		HookParams hooks = new HookParams()
-			.add(AtomicInteger.class, counter)
-			.add(IdDt.class, id)
-			.add(RequestDetails.class, myRequestDetails)
-			.addIfMatchesType(ServletRequestDetails.class, myRequestDetails);
-		JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, myRequestDetails, Pointcut.STORAGE_PRESTORAGE_EXPUNGE_RESOURCE, hooks);
-
-		myRemainingCount.addAndGet(-1 * counter.get());
-	}
-
 	private void expungeDeletedResources() {
 		Slice<Long> resourceIds = findHistoricalVersionsOfDeletedResources();
 

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ResourceExpungeService.java

@@ -139,27 +139,36 @@ class ResourceExpungeService implements IResourceExpungeService {
 		}
 	}
 
-	private void expungeHistoricalVersion(RequestDetails theRequestDetails, Long theNextVersionId) {
+	private void expungeHistoricalVersion(RequestDetails theRequestDetails, Long theNextVersionId, AtomicInteger theRemainingCount) {
 		ResourceHistoryTable version = myResourceHistoryTableDao.findById(theNextVersionId).orElseThrow(IllegalArgumentException::new);
 		IdDt id = version.getIdDt();
 		ourLog.info("Deleting resource version {}", id.getValue());
 
-		// Interceptor call: STORAGE_PRESTORAGE_EXPUNGE_RESOURCE
+		callHooks(theRequestDetails, theRemainingCount, version, id);
+
+		myResourceHistoryTagDao.deleteAll(version.getTags());
+		myResourceHistoryTableDao.delete(version);
+
+		theRemainingCount.decrementAndGet();
+	}
+
+	private void callHooks(RequestDetails theRequestDetails, AtomicInteger theRemainingCount, ResourceHistoryTable theVersion, IdDt theId) {
+		final AtomicInteger counter = new AtomicInteger();
 		if (JpaInterceptorBroadcaster.hasHooks(Pointcut.STORAGE_PRESTORAGE_EXPUNGE_RESOURCE, myInterceptorBroadcaster, theRequestDetails)) {
-			IFhirResourceDao resourceDao = myDaoRegistry.getResourceDao(id.getResourceType());
+			IFhirResourceDao resourceDao = myDaoRegistry.getResourceDao(theId.getResourceType());
-			IBaseResource resource = resourceDao.toResource(version, false);
+			IBaseResource resource = resourceDao.toResource(theVersion, false);
 			HookParams params = new HookParams()
-				.add(IIdType.class, id)
+				.add(AtomicInteger.class, counter)
+				.add(IIdType.class, theId)
 				.add(IBaseResource.class, resource)
 				.add(RequestDetails.class, theRequestDetails)
 				.addIfMatchesType(ServletRequestDetails.class, theRequestDetails);
 			JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PRESTORAGE_EXPUNGE_RESOURCE, params);
 		}
-
+		theRemainingCount.addAndGet(-1 * counter.get());
-		myResourceHistoryTagDao.deleteAll(version.getTags());
-		myResourceHistoryTableDao.delete(version);
 	}
 
+
 	@Override
 	@Transactional
 	public void expungeHistoricalVersionsOfIds(RequestDetails theRequestDetails, List<Long> theResourceIds, AtomicInteger theRemainingCount) {
@@ -175,8 +184,8 @@ class ResourceExpungeService implements IResourceExpungeService {
 	@Transactional
 	public void expungeHistoricalVersions(RequestDetails theRequestDetails, List<Long> theHistoricalIds, AtomicInteger theRemainingCount) {
 		for (Long next : theHistoricalIds) {
-			expungeHistoricalVersion(theRequestDetails, next);
+			expungeHistoricalVersion(theRequestDetails, next, theRemainingCount);
-			if (theRemainingCount.decrementAndGet() <= 0) {
+			if (theRemainingCount.get() <= 0) {
 				return;
 			}
 		}
@@ -187,7 +196,7 @@ class ResourceExpungeService implements IResourceExpungeService {
 
 		ResourceHistoryTable currentVersion = myResourceHistoryTableDao.findForIdAndVersion(resource.getId(), resource.getVersion());
 		if (currentVersion != null) {
-			expungeHistoricalVersion(theRequestDetails, currentVersion.getId());
+			expungeHistoricalVersion(theRequestDetails, currentVersion.getId(), theRemainingCount);
 		}
 
 		ourLog.info("Expunging current version of resource {}", resource.getIdDt().getValue());
@@ -203,8 +212,6 @@ class ResourceExpungeService implements IResourceExpungeService {
 		}
 
 		myResourceTableDao.delete(resource);
-
-		theRemainingCount.decrementAndGet();
 	}
 
 	@Override
@@ -230,8 +237,8 @@ class ResourceExpungeService implements IResourceExpungeService {
 		Slice<Long> versionIds = myResourceHistoryTableDao.findForResourceId(page, resource.getId(), resource.getVersion());
 		ourLog.debug("Found {} versions of resource {} to expunge", versionIds.getNumberOfElements(), resource.getIdDt().getValue());
 		for (Long nextVersionId : versionIds) {
-			expungeHistoricalVersion(theRequestDetails, nextVersionId);
+			expungeHistoricalVersion(theRequestDetails, nextVersionId, theRemainingCount);
-			if (theRemainingCount.decrementAndGet() <= 0) {
+			if (theRemainingCount.get() <= 0) {
 				return;
 			}
 		}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/expunge/ExpungeHookTest.java

@@ -82,14 +82,16 @@ public class ExpungeHookTest {
 	public void expungeResourceHook() throws InterruptedException {
 		IIdType expungeId = myPatientDao.create(new Patient()).getId();
 		assertNotNull(myPatientDao.read(expungeId));
-		myExpungeResourceLatch.setExpectedCount(1);
 		myPatientDao.delete(expungeId);
 
 		ExpungeOptions options = new ExpungeOptions();
 		options.setExpungeDeletedResources(true);
-		myExpungeService.expunge("Patient", expungeId.getIdPartAsLong(), expungeId.getVersionIdPartAsLong(), options, null);
+
+		myExpungeResourceLatch.setExpectedCount(2);
+		myExpungeService.expunge("Patient", expungeId.getIdPartAsLong(), null, options, null);
 		HookParams hookParams = myExpungeResourceLatch.awaitExpected().get(0);
-		IdDt hookId = hookParams.get(IdDt.class);
+
+		IIdType hookId = hookParams.get(IIdType.class);
 		assertEquals(expungeId.getValue(), hookId.getValue());
 	}
 }