diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java index 672fa91f0d7..ae054d93fb5 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java @@ -1244,7 +1244,7 @@ public enum Pointcut { /** * Storage Hook: - * Invoked before a resource will be created + * Invoked before a resource will be deleted *
* Hooks will have access to the contents of the resource being deleted * but should not make any changes as storage has already occurred @@ -1282,7 +1282,7 @@ public enum Pointcut { /** * Storage Hook: - * Invoked when a resource delete operation is about to fail due to referential integrity hcts. + * Invoked when a resource delete operation is about to fail due to referential integrity checks. Intended for use with ca.uhn.fhir.jpa.interceptor.CascadingDeleteInterceptor. *
* Hooks will have access to the list of resources that have references to the resource being deleted. *
diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java index d2807f4cc35..e1f46cdfd4c 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java @@ -83,6 +83,9 @@ public class DaoConfig { private static final Logger ourLog = LoggerFactory.getLogger(DaoConfig.class); private static final int DEFAULT_EXPUNGE_BATCH_SIZE = 800; private IndexEnabledEnum myIndexMissingFieldsEnabled = IndexEnabledEnum.DISABLED; + private static final int DEFAULT_MAXIMUM_DELETE_CONFLICT_COUNT = 60; + private static final int DEFAULT_MAXIMUM_DELETE_CONFLICT_RETRY_ATTEMPTS = 10; + /** * Child Configurations @@ -153,6 +156,10 @@ public class DaoConfig { private ClientIdStrategyEnum myResourceClientIdStrategy = ClientIdStrategyEnum.ALPHANUMERIC; private boolean myFilterParameterEnabled = false; private StoreMetaSourceInformationEnum myStoreMetaSourceInformation = StoreMetaSourceInformationEnum.SOURCE_URI_AND_REQUEST_ID; + /** + * update setter javadoc if default changes + */ + private Integer myMaximumDeleteConflictQueryCount = DEFAULT_MAXIMUM_DELETE_CONFLICT_COUNT; /** * Do not change default of {@code true}! * @@ -2021,4 +2028,33 @@ public class DaoConfig { */ ANY } + + /** + *+ * This determines the maximum number of conflicts that should be fetched and handled while retrying a delete of a resource. + *
+ *+ * The default value for this setting is {@code 60}. + *
+ * + * @since 4.1.0 + */ + public Integer getMaximumDeleteConflictQueryCount() { + return myMaximumDeleteConflictQueryCount; + } + + /** + *+ * This determines the maximum number of conflicts that should be fetched and handled while retrying a delete of a resource. + *
+ *+ * The default value for this setting is {@code 60}. + *
+ * + * @since 4.1.0 + */ + public void setMaximumDeleteConflictQueryCount(Integer theMaximumDeleteConflictQueryCount) { + myMaximumDeleteConflictQueryCount = theMaximumDeleteConflictQueryCount; + } + } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictService.java index 6ec78c4a2dd..7a51e487a28 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictService.java @@ -38,6 +38,7 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.OperationOutcomeUtil; +import com.google.common.annotations.VisibleForTesting; import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,8 +52,8 @@ import java.util.List; public class DeleteConflictService { private static final Logger ourLog = LoggerFactory.getLogger(DeleteConflictService.class); public static final int FIRST_QUERY_RESULT_COUNT = 1; - public static final int RETRY_QUERY_RESULT_COUNT = 60; - public static final int MAX_RETRY_ATTEMPTS = 10; + public static int MAX_RETRY_ATTEMPTS = 10; + public static String MAX_RETRY_ATTEMPTS_EXCEEDED_MSG = "Requested delete operation stopped before all conflicts were handled. May need to increase the configured Maximum Delete Conflict Query Count."; @Autowired DeleteConflictFinderService myDeleteConflictFinderService; @@ -81,11 +82,16 @@ public class DeleteConflictService { while (outcome != null) { int shouldRetryCount = Math.min(outcome.getShouldRetryCount(), MAX_RETRY_ATTEMPTS); if (!(retryCount < shouldRetryCount)) break; - newConflicts = new DeleteConflictList(); - outcome = findAndHandleConflicts(theRequest, newConflicts, theEntity, theForValidate, RETRY_QUERY_RESULT_COUNT, theTransactionDetails); + newConflicts = new DeleteConflictList(newConflicts); + outcome = findAndHandleConflicts(theRequest, newConflicts, theEntity, theForValidate, myDaoConfig.getMaximumDeleteConflictQueryCount(), theTransactionDetails); ++retryCount; } theDeleteConflicts.addAll(newConflicts); + if(retryCount >= MAX_RETRY_ATTEMPTS && !newConflicts.isEmpty()) { + IBaseOperationOutcome oo = OperationOutcomeUtil.newInstance(myFhirContext); + OperationOutcomeUtil.addIssue(myFhirContext, oo, BaseHapiFhirDao.OO_SEVERITY_ERROR, MAX_RETRY_ATTEMPTS_EXCEEDED_MSG,null, "processing"); + throw new ResourceVersionConflictException(MAX_RETRY_ATTEMPTS_EXCEEDED_MSG, oo); + } return retryCount; } @@ -162,4 +168,9 @@ public class DeleteConflictService { throw new ResourceVersionConflictException(firstMsg, oo); } + + @VisibleForTesting + static void setMaxRetryAttempts(Integer theMaxRetryAttempts) { + MAX_RETRY_ATTEMPTS = theMaxRetryAttempts; + } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/delete/DeleteConflictServiceR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/delete/DeleteConflictServiceR4Test.java index 9175d898e40..212c0863d1e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/delete/DeleteConflictServiceR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/delete/DeleteConflictServiceR4Test.java @@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.delete; import ca.uhn.fhir.interceptor.api.Hook; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.model.DeleteConflictList; import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4Test; import ca.uhn.fhir.jpa.api.model.DeleteConflict; @@ -17,6 +18,7 @@ import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import java.util.ArrayList; import java.util.Iterator; @@ -28,6 +30,9 @@ import static org.junit.Assert.*; public class DeleteConflictServiceR4Test extends BaseJpaR4Test { private static final Logger ourLog = LoggerFactory.getLogger(DeleteConflictServiceR4Test.class); + @Autowired + DaoConfig myDaoConfig; + private DeleteConflictInterceptor myDeleteInterceptor = new DeleteConflictInterceptor(); private int myInterceptorDeleteCount; @@ -133,6 +138,48 @@ public class DeleteConflictServiceR4Test extends BaseJpaR4Test { assertEquals(3, myInterceptorDeleteCount); } + @Test + public void testDeleteHookDeletesLargeNumberOfConflicts() { + + Organization organization = new Organization(); + organization.setName("FOO"); + IIdType organizationId = myOrganizationDao.create(organization).getId().toUnqualifiedVersionless(); + + // Create 12 conflicts. + for (int j=0; j < 12 ; j++) { + Patient patient = new Patient(); + patient.setManagingOrganization(new Reference(organizationId)); + myPatientDao.create(patient).getId().toUnqualifiedVersionless(); + } + + DeleteConflictService.setMaxRetryAttempts(3); + myDaoConfig.setMaximumDeleteConflictQueryCount(5); + myDeleteInterceptor.deleteConflictFunction = this::deleteConflictsFixedRetryCount; + try { + myOrganizationDao.delete(organizationId); + // Needs a fourth pass to ensure that all conflicts are now gone. + fail(); + } catch (ResourceVersionConflictException e) { + assertEquals(DeleteConflictService.MAX_RETRY_ATTEMPTS_EXCEEDED_MSG, e.getMessage()); + } + + // Try again with Maximum conflict count set to 6. + myDeleteInterceptor.myCallCount=0; + myInterceptorDeleteCount = 0; + myDaoConfig.setMaximumDeleteConflictQueryCount(6); + + try { + myOrganizationDao.delete(organizationId); + } catch (ResourceVersionConflictException e) { + fail(); + } + + assertNotNull(myDeleteInterceptor.myDeleteConflictList); + assertEquals(3, myDeleteInterceptor.myCallCount); + assertEquals(12, myInterceptorDeleteCount); + + } + @Test public void testBadInterceptorNoInfiniteLoop() { Organization organization = new Organization(); @@ -150,7 +197,7 @@ public class DeleteConflictServiceR4Test extends BaseJpaR4Test { myOrganizationDao.delete(organizationId); fail(); } catch (ResourceVersionConflictException e) { - assertEquals("Unable to delete Organization/" + organizationId.getIdPart() + " because at least one resource has a reference to this resource. First reference found was resource Patient/" + patientId.getIdPart() + " in path Patient.managingOrganization", e.getMessage()); + assertEquals(DeleteConflictService.MAX_RETRY_ATTEMPTS_EXCEEDED_MSG, e.getMessage()); } assertEquals(1 + DeleteConflictService.MAX_RETRY_ATTEMPTS, myDeleteInterceptor.myCallCount); } @@ -198,6 +245,21 @@ public class DeleteConflictServiceR4Test extends BaseJpaR4Test { return new DeleteConflictOutcome().setShouldRetryCount(myInterceptorDeleteCount); } + private DeleteConflictOutcome deleteConflictsFixedRetryCount(DeleteConflictList theList) { + Iterator