$delete-expunge over 10k resources will now delete all resources

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5150--delete-expunge-over-10k-resources-deletes-only-10k.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 5150
+title: "When running a $delete-expunge with over 10,000 resources, only the first 10,000 resources were deleted.
+        This is now fixed."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/Batch2SupportConfig.java

@@ -19,16 +19,21 @@
  */
 package ca.uhn.fhir.jpa.config;
 
+import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
+import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
 import ca.uhn.fhir.jpa.api.svc.IBatch2DaoSvc;
 import ca.uhn.fhir.jpa.api.svc.IDeleteExpungeSvc;
 import ca.uhn.fhir.jpa.api.svc.IIdHelperService;
 import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc;
 import ca.uhn.fhir.jpa.dao.data.IResourceLinkDao;
+import ca.uhn.fhir.jpa.dao.data.IResourceTableDao;
 import ca.uhn.fhir.jpa.dao.expunge.ResourceTableFKProvider;
+import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService;
 import ca.uhn.fhir.jpa.delete.batch2.DeleteExpungeSqlBuilder;
 import ca.uhn.fhir.jpa.delete.batch2.DeleteExpungeSvcImpl;
 import ca.uhn.fhir.jpa.reindex.Batch2DaoSvcImpl;
+import ca.uhn.fhir.jpa.searchparam.MatchUrlService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 
@@ -37,8 +42,20 @@
 public class Batch2SupportConfig {
 
 	@Bean
-	public IBatch2DaoSvc batch2DaoSvc() {
-		return new Batch2DaoSvcImpl();
+	public IBatch2DaoSvc batch2DaoSvc(
+			IResourceTableDao theResourceTableDao,
+			MatchUrlService theMatchUrlService,
+			DaoRegistry theDaoRegistry,
+			FhirContext theFhirContext,
+			IHapiTransactionService theTransactionService,
+			JpaStorageSettings theJpaStorageSettings) {
+		return new Batch2DaoSvcImpl(
+				theResourceTableDao,
+				theMatchUrlService,
+				theDaoRegistry,
+				theFhirContext,
+				theTransactionService,
+				theJpaStorageSettings);
 	}
 
 	@Bean

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/reindex/Batch2DaoSvcImpl.java

@@ -21,7 +21,9 @@
 
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.RuntimeResourceDefinition;
+import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.interceptor.model.RequestPartitionId;
+import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
 import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
 import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
 import ca.uhn.fhir.jpa.api.pid.EmptyResourcePidList;
@@ -39,46 +41,59 @@
 import ca.uhn.fhir.rest.api.SortSpec;
 import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
 import ca.uhn.fhir.rest.api.server.storage.IResourcePersistentId;
-import ca.uhn.fhir.rest.param.DateRangeParam;
-import ca.uhn.fhir.util.DateRangeUtil;
-import org.springframework.beans.factory.annotation.Autowired;
+import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Slice;
 
+import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
-import static org.apache.commons.collections4.CollectionUtils.isNotEmpty;
-
 public class Batch2DaoSvcImpl implements IBatch2DaoSvc {
+	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(Batch2DaoSvcImpl.class);
+
+	private final IResourceTableDao myResourceTableDao;
 
-	@Autowired
-	private IResourceTableDao myResourceTableDao;
+	private final MatchUrlService myMatchUrlService;
 
-	@Autowired
-	private MatchUrlService myMatchUrlService;
+	private final DaoRegistry myDaoRegistry;
 
-	@Autowired
-	private DaoRegistry myDaoRegistry;
+	private final FhirContext myFhirContext;
 
-	@Autowired
-	private FhirContext myFhirContext;
+	private final IHapiTransactionService myTransactionService;
 
-	@Autowired
-	private IHapiTransactionService myTransactionService;
+	private final JpaStorageSettings myJpaStorageSettings;
 
 	@Override
 	public boolean isAllResourceTypeSupported() {
 		return true;
 	}
 
+	public Batch2DaoSvcImpl(
+			IResourceTableDao theResourceTableDao,
+			MatchUrlService theMatchUrlService,
+			DaoRegistry theDaoRegistry,
+			FhirContext theFhirContext,
+			IHapiTransactionService theTransactionService,
+			JpaStorageSettings theJpaStorageSettings) {
+		myResourceTableDao = theResourceTableDao;
+		myMatchUrlService = theMatchUrlService;
+		myDaoRegistry = theDaoRegistry;
+		myFhirContext = theFhirContext;
+		myTransactionService = theTransactionService;
+		myJpaStorageSettings = theJpaStorageSettings;
+	}
+
 	@Override
 	public IResourcePidList fetchResourceIdsPage(
 			Date theStart,
 			Date theEnd,
+			// TODO:  LD: We must keep thePageSize parameter here for the time being even if though it's no longer used
+			// in this class because
+			// otherwise we'll break other implementors of IBatch2DaoSvc
 			@Nonnull Integer thePageSize,
 			@Nullable RequestPartitionId theRequestPartitionId,
 			@Nullable String theUrl) {
@@ -87,45 +102,67 @@ public IResourcePidList fetchResourceIdsPage(
 				.withRequestPartitionId(theRequestPartitionId)
 				.execute(() -> {
 					if (theUrl == null) {
-						return fetchResourceIdsPageNoUrl(theStart, theEnd, thePageSize, theRequestPartitionId);
+						return fetchResourceIdsPageNoUrl(theStart, theEnd, theRequestPartitionId);
 					} else {
-						return fetchResourceIdsPageWithUrl(
-								theStart, theEnd, thePageSize, theUrl, theRequestPartitionId);
+						return fetchResourceIdsPageWithUrl(theEnd, theUrl, theRequestPartitionId);
 					}
 				});
 	}
 
-	private IResourcePidList fetchResourceIdsPageWithUrl(
-			Date theStart, Date theEnd, int thePageSize, String theUrl, RequestPartitionId theRequestPartitionId) {
+	@Nonnull
+	private HomogeneousResourcePidList fetchResourceIdsPageWithUrl(
+			Date theEnd, @Nonnull String theUrl, @Nullable RequestPartitionId theRequestPartitionId) {
+		if (!theUrl.contains("?")) {
+			throw new InternalErrorException(Msg.code(2422) + "this should never happen: URL is missing a '?'");
+		}
+
+		final Integer internalSynchronousSearchSize = myJpaStorageSettings.getInternalSynchronousSearchSize();
+
+		if (internalSynchronousSearchSize == null || internalSynchronousSearchSize <= 0) {
+			throw new InternalErrorException(Msg.code(2423)
+					+ "this should never happen: internalSynchronousSearchSize is null or less than or equal to 0");
+		}
+
+		List<IResourcePersistentId> currentIds = fetchResourceIdsPageWithUrl(0, theUrl, theRequestPartitionId);
+		ourLog.debug("FIRST currentIds: {}", currentIds.size());
 
+		final List<IResourcePersistentId> allIds = new ArrayList<>(currentIds);
+
+		while (internalSynchronousSearchSize < currentIds.size()) {
+			// Ensure the offset is set to the last ID in the cumulative List, otherwise, we'll be stuck in an infinite
+			// loop here:
+			currentIds = fetchResourceIdsPageWithUrl(allIds.size(), theUrl, theRequestPartitionId);
+			ourLog.debug("NEXT currentIds: {}", currentIds.size());
+
+			allIds.addAll(currentIds);
+		}
+
+		final String resourceType = theUrl.substring(0, theUrl.indexOf('?'));
+
+		return new HomogeneousResourcePidList(resourceType, allIds, theEnd, theRequestPartitionId);
+	}
+
+	private List<IResourcePersistentId> fetchResourceIdsPageWithUrl(
+			int theOffset, String theUrl, RequestPartitionId theRequestPartitionId) {
 		String resourceType = theUrl.substring(0, theUrl.indexOf('?'));
 		RuntimeResourceDefinition def = myFhirContext.getResourceDefinition(resourceType);
 
 		SearchParameterMap searchParamMap = myMatchUrlService.translateMatchUrl(theUrl, def);
-		searchParamMap.setSort(new SortSpec(Constants.PARAM_LASTUPDATED, SortOrderEnum.ASC));
-		DateRangeParam chunkDateRange =
-				DateRangeUtil.narrowDateRange(searchParamMap.getLastUpdated(), theStart, theEnd);
-		searchParamMap.setLastUpdated(chunkDateRange);
-		searchParamMap.setCount(thePageSize);
+		searchParamMap.setSort(new SortSpec(Constants.PARAM_ID, SortOrderEnum.ASC));
+		searchParamMap.setOffset(theOffset);
+		searchParamMap.setLoadSynchronousUpTo(myJpaStorageSettings.getInternalSynchronousSearchSize() + 1);
 
 		IFhirResourceDao<?> dao = myDaoRegistry.getResourceDao(resourceType);
 		SystemRequestDetails request = new SystemRequestDetails();
 		request.setRequestPartitionId(theRequestPartitionId);
-		List<IResourcePersistentId> ids = dao.searchForIds(searchParamMap, request);
-
-		Date lastDate = null;
-		if (isNotEmpty(ids)) {
-			IResourcePersistentId lastResourcePersistentId = ids.get(ids.size() - 1);
-			lastDate = dao.readByPid(lastResourcePersistentId, true).getMeta().getLastUpdated();
-		}
 
-		return new HomogeneousResourcePidList(resourceType, ids, lastDate, theRequestPartitionId);
+		return dao.searchForIds(searchParamMap, request);
 	}
 
 	@Nonnull
 	private IResourcePidList fetchResourceIdsPageNoUrl(
-			Date theStart, Date theEnd, int thePagesize, RequestPartitionId theRequestPartitionId) {
-		Pageable page = Pageable.ofSize(thePagesize);
+			Date theStart, Date theEnd, RequestPartitionId theRequestPartitionId) {
+		final Pageable page = Pageable.unpaged();
 		Slice<Object[]> slice;
 		if (theRequestPartitionId == null || theRequestPartitionId.isAllPartitions()) {
 			slice = myResourceTableDao.findIdsTypesAndUpdateTimesOfResourcesWithinUpdatedRangeOrderedFromOldest(

hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/GoldenResourceSearchSvcImpl.java

@@ -82,7 +82,6 @@ private IResourcePidList fetchResourceIdsPageWithResourceType(
 		DateRangeParam chunkDateRange =
 				DateRangeUtil.narrowDateRange(searchParamMap.getLastUpdated(), theStart, theEnd);
 		searchParamMap.setLastUpdated(chunkDateRange);
-		searchParamMap.setCount(thePageSize); // request this many pids
 		searchParamMap.add(
 				"_tag", new TokenParam(MdmConstants.SYSTEM_GOLDEN_RECORD_STATUS, MdmConstants.CODE_GOLDEN_RECORD));
 

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantBatchOperationR4Test.java

@@ -32,10 +32,8 @@
 import java.util.stream.Collectors;
 
 import static ca.uhn.fhir.jpa.model.util.JpaConstants.DEFAULT_PARTITION_NAME;
-import static org.awaitility.Awaitility.await;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasSize;
-import static org.hamcrest.Matchers.in;
 import static org.hamcrest.Matchers.isA;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
@@ -108,7 +106,7 @@ public void testDeleteExpungeOperation() {
 		String jobId = BatchHelperR4.jobIdFromBatch2Parameters(response);
 		myBatch2JobHelper.awaitJobCompletion(jobId);
 
-		assertThat(interceptor.requestPartitionIds, hasSize(5));
+		assertThat(interceptor.requestPartitionIds, hasSize(4));
 		RequestPartitionId partitionId = interceptor.requestPartitionIds.get(0);
 		assertEquals(TENANT_B_ID, partitionId.getFirstPartitionIdOrNull());
 		assertEquals(TENANT_B, partitionId.getFirstPartitionNameOrNull());

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/reindex/Batch2DaoSvcImplTest.java

@@ -0,0 +1,144 @@
+package ca.uhn.fhir.jpa.reindex;
+
+import ca.uhn.fhir.interceptor.model.RequestPartitionId;
+import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
+import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
+import ca.uhn.fhir.jpa.api.pid.IResourcePidList;
+import ca.uhn.fhir.jpa.api.svc.IBatch2DaoSvc;
+import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService;
+import ca.uhn.fhir.jpa.searchparam.MatchUrlService;
+import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
+import ca.uhn.fhir.model.primitive.IdDt;
+import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
+import org.hl7.fhir.instance.model.api.IIdType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.springframework.beans.factory.annotation.Autowired;
+
+import javax.annotation.Nonnull;
+import java.time.LocalDate;
+import java.time.Month;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.List;
+import java.util.stream.IntStream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+class Batch2DaoSvcImplTest extends BaseJpaR4Test {
+
+	private static final Date PREVIOUS_MILLENNIUM = toDate(LocalDate.of(1999, Month.DECEMBER, 31));
+	private static final Date TOMORROW = toDate(LocalDate.now().plusDays(1));
+	private static final String URL_PATIENT_EXPUNGE_TRUE = "Patient?_expunge=true";
+	private static final String PATIENT = "Patient";
+	private static final int INTERNAL_SYNCHRONOUS_SEARCH_SIZE = 10;
+
+	@Autowired
+	private JpaStorageSettings myJpaStorageSettings;
+	@Autowired
+	private MatchUrlService myMatchUrlService;
+	@Autowired
+	private IHapiTransactionService myIHapiTransactionService ;
+
+	private DaoRegistry mySpiedDaoRegistry;
+
+	private IBatch2DaoSvc mySubject;
+
+	@BeforeEach
+	void beforeEach() {
+		myJpaStorageSettings.setInternalSynchronousSearchSize(INTERNAL_SYNCHRONOUS_SEARCH_SIZE);
+
+		mySpiedDaoRegistry = spy(myDaoRegistry);
+
+		mySubject = new Batch2DaoSvcImpl(myResourceTableDao, myMatchUrlService, mySpiedDaoRegistry, myFhirContext, myIHapiTransactionService, myJpaStorageSettings);
+	}
+
+	// TODO: LD this test won't work with the nonUrl variant yet:  error:   No existing transaction found for transaction marked with propagation 'mandatory'
+
+	@Test
+	void fetchResourcesByUrlEmptyUrl() {
+		final InternalErrorException exception = assertThrows(InternalErrorException.class, () -> mySubject.fetchResourceIdsPage(PREVIOUS_MILLENNIUM, TOMORROW, 800, RequestPartitionId.defaultPartition(), ""));
+
+		assertEquals("HAPI-2422: this should never happen: URL is missing a '?'", exception.getMessage());
+	}
+
+	@Test
+	void fetchResourcesByUrlSingleQuestionMark() {
+		final InternalErrorException exception = assertThrows(InternalErrorException.class, () -> mySubject.fetchResourceIdsPage(PREVIOUS_MILLENNIUM, TOMORROW, 800, RequestPartitionId.defaultPartition(), "?"));
+
+		assertEquals("HAPI-2223: theResourceName must not be blank", exception.getMessage());
+	}
+
+	@Test
+	void fetchResourcesByUrlNonsensicalResource() {
+		final InternalErrorException exception = assertThrows(InternalErrorException.class, () -> mySubject.fetchResourceIdsPage(PREVIOUS_MILLENNIUM, TOMORROW, 800, RequestPartitionId.defaultPartition(), "Banana?_expunge=true"));
+
+		assertEquals("HAPI-2223: HAPI-1684: Unknown resource name \"Banana\" (this name is not known in FHIR version \"R4\")", exception.getMessage());
+	}
+
+	@ParameterizedTest
+	@ValueSource(ints = {0, 9, 10, 11, 21, 22, 23, 45})
+	void fetchResourcesByUrl(int expectedNumResults) {
+		final List<IIdType> patientIds = IntStream.range(0, expectedNumResults)
+			.mapToObj(num -> createPatient())
+			.toList();
+
+		final IResourcePidList resourcePidList = mySubject.fetchResourceIdsPage(PREVIOUS_MILLENNIUM, TOMORROW, 800, RequestPartitionId.defaultPartition(), URL_PATIENT_EXPUNGE_TRUE);
+
+		final List<? extends IIdType> actualPatientIds =
+			resourcePidList.getTypedResourcePids()
+				.stream()
+				.map(typePid -> new IdDt(typePid.resourceType, (Long) typePid.id.getId()))
+				.toList();
+		assertIdsEqual(patientIds, actualPatientIds);
+
+		verify(mySpiedDaoRegistry, times(getExpectedNumOfInvocations(expectedNumResults))).getResourceDao(PATIENT);
+	}
+
+	@ParameterizedTest
+	@ValueSource(ints = {0, 9, 10, 11, 21, 22, 23, 45})
+	void fetchResourcesNoUrl(int expectedNumResults) {
+		final int pageSizeWellBelowThreshold = 2;
+		final List<IIdType> patientIds = IntStream.range(0, expectedNumResults)
+			.mapToObj(num -> createPatient())
+			.toList();
+
+		final IResourcePidList resourcePidList = mySubject.fetchResourceIdsPage(PREVIOUS_MILLENNIUM, TOMORROW, pageSizeWellBelowThreshold, RequestPartitionId.defaultPartition(), null);
+
+		final List<? extends IIdType> actualPatientIds =
+			resourcePidList.getTypedResourcePids()
+				.stream()
+				.map(typePid -> new IdDt(typePid.resourceType, (Long) typePid.id.getId()))
+				.toList();
+		assertIdsEqual(patientIds, actualPatientIds);
+	}
+
+	private int getExpectedNumOfInvocations(int expectedNumResults) {
+		final int maxResultsPerQuery = INTERNAL_SYNCHRONOUS_SEARCH_SIZE + 1;
+		final int division = expectedNumResults / maxResultsPerQuery;
+		return division + 1;
+	}
+
+	private static void assertIdsEqual(List<IIdType> expectedResourceIds, List<? extends IIdType> actualResourceIds) {
+		assertEquals(expectedResourceIds.size(), actualResourceIds.size());
+
+		for (int index = 0; index < expectedResourceIds.size(); index++) {
+			final IIdType expectedIdType = expectedResourceIds.get(index);
+			final IIdType actualIdType = actualResourceIds.get(index);
+
+			assertEquals(expectedIdType.getResourceType(), actualIdType.getResourceType());
+			assertEquals(expectedIdType.getIdPartAsLong(), actualIdType.getIdPartAsLong());
+		}
+	}
+
+	@Nonnull
+	private static Date toDate(LocalDate theLocalDate) {
+		return Date.from(theLocalDate.atStartOfDay(ZoneId.systemDefault()).toInstant());
+	}
+}