From 3bfdc61866e5c33786cbe5b05d126675a1fadf59 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 1 Nov 2018 09:15:03 -0400 Subject: [PATCH] Fix a couple of test failures --- ...istedJpaSearchFirstPageBundleProvider.java | 9 +++ .../jpa/search/SearchCoordinatorSvcImpl.java | 19 +++--- .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 64 +++++++++++++++++++ .../search/SearchCoordinatorSvcImplTest.java | 9 ++- .../BaseResourceReturningMethodBinding.java | 10 --- .../ca/uhn/fhir/util/FhirTerserDstu3Test.java | 29 +++++++++ 6 files changed, 117 insertions(+), 23 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaSearchFirstPageBundleProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaSearchFirstPageBundleProvider.java index bc8b61a00c4..09338aed558 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaSearchFirstPageBundleProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaSearchFirstPageBundleProvider.java @@ -65,6 +65,15 @@ public class PersistedJpaSearchFirstPageBundleProvider extends PersistedJpaBundl txTemplate.setPropagationBehavior(TransactionTemplate.PROPAGATION_REQUIRED); List retVal = txTemplate.execute(theStatus -> toResourceList(mySearchBuilder, pids)); + int totalCountWanted = theToIndex - theFromIndex; + if (retVal.size() < totalCountWanted) { + if (mySearch.getStatus() == SearchStatusEnum.PASSCMPLET) { + int remainingWanted = totalCountWanted - retVal.size(); + int fromIndex = theToIndex - remainingWanted; + List remaining = super.getResources(fromIndex, theToIndex); + retVal.addAll(remaining); + } + } ourLog.trace("Loaded resources to return"); return retVal; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index 44ac751ae71..d546ecc9677 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -229,17 +229,19 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); txTemplate.afterPropertiesSet(); return txTemplate.execute(t -> { - myEntityManager.refresh(theSearch); - if (theSearch.getStatus() != SearchStatusEnum.PASSCMPLET) { + Search search = mySearchDao.findById(theSearch.getId()).orElse(theSearch); + + if (search.getStatus() != SearchStatusEnum.PASSCMPLET) { throw new IllegalStateException("Can't change to LOADING because state is " + theSearch.getStatus()); } - theSearch.setStatus(SearchStatusEnum.LOADING); - Search newSearch = mySearchDao.save(theSearch); + search.setStatus(SearchStatusEnum.LOADING); + Search newSearch = mySearchDao.save(search); return Optional.of(newSearch); }); } catch (Exception e) { ourLog.warn("Failed to activate search: {}", e.toString()); - ourLog.trace("Failed to activate search", e); + // FIXME: aaaaa + ourLog.info("Failed to activate search", e); return Optional.empty(); } } @@ -511,9 +513,10 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { * user has requested resources 0-60, then they would get 0-50 back but the search * coordinator would then stop searching.SearchCoordinatorSvcImplTest */ - List remainingResources = SearchCoordinatorSvcImpl.this.getResources(mySearch.getUuid(), mySyncedPids.size(), theToIndex); - ourLog.debug("Adding {} resources to the existing {} synced resource IDs", remainingResources.size(), mySyncedPids.size()); - mySyncedPids.addAll(remainingResources); + // FIXME: aaaaaaaa +// List remainingResources = SearchCoordinatorSvcImpl.this.getResources(mySearch.getUuid(), mySyncedPids.size(), theToIndex); +// ourLog.debug("Adding {} resources to the existing {} synced resource IDs", remainingResources.size(), mySyncedPids.size()); +// mySyncedPids.addAll(remainingResources); keepWaiting = false; break; case FAILED: 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 6e106dcfea2..5f9770f98b1 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 @@ -175,6 +175,70 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { fail(); return null; } + + @Test + public void testTransactionReSavesPreviouslyDeletedResources() { + + { + Bundle input = new Bundle(); + input.setType(BundleType.TRANSACTION); + + Patient pt = new Patient(); + pt.setId("pt"); + pt.setActive(true); + input + .addEntry() + .setResource(pt) + .getRequest() + .setUrl("Patient/pt") + .setMethod(HTTPVerb.PUT); + + Observation obs = new Observation(); + obs.setId("obs"); + obs.getSubject().setReference("Patient/pt"); + input + .addEntry() + .setResource(obs) + .getRequest() + .setUrl("Observation/obs") + .setMethod(HTTPVerb.PUT); + + mySystemDao.transaction(null, input); + } + + myObservationDao.delete(new IdType("Observation/obs")); + myPatientDao.delete(new IdType("Patient/pt")); + + { + Bundle input = new Bundle(); + input.setType(BundleType.TRANSACTION); + + Patient pt = new Patient(); + pt.setId("pt"); + pt.setActive(true); + input + .addEntry() + .setResource(pt) + .getRequest() + .setUrl("Patient/pt") + .setMethod(HTTPVerb.PUT); + + Observation obs = new Observation(); + obs.setId("obs"); + obs.getSubject().setReference("Patient/pt"); + input + .addEntry() + .setResource(obs) + .getRequest() + .setUrl("Observation/obs") + .setMethod(HTTPVerb.PUT); + + mySystemDao.transaction(null, input); + } + + myPatientDao.read(new IdType("Patient/pt")); + } + @Test public void testResourceCounts() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java index bdfa14b4ee2..91d277b53e1 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java @@ -34,7 +34,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; -import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.transaction.PlatformTransactionManager; @@ -52,6 +51,7 @@ import static org.mockito.Mockito.*; @RunWith(MockitoJUnitRunner.class) public class SearchCoordinatorSvcImplTest { + private static final Logger ourLog = LoggerFactory.getLogger(SearchCoordinatorSvcImplTest.class); private static FhirContext ourCtx = FhirContext.forDstu3(); @Captor ArgumentCaptor> mySearchResultIterCaptor; @@ -69,7 +69,6 @@ public class SearchCoordinatorSvcImplTest { @Mock private ISearchResultDao mySearchResultDao; private SearchCoordinatorSvcImpl mySvc; - @Mock private PlatformTransactionManager myTxManager; private DaoConfig myDaoConfig; @@ -159,7 +158,7 @@ public class SearchCoordinatorSvcImplTest { } } -private static final Logger ourLog = LoggerFactory.getLogger(SearchCoordinatorSvcImplTest.class); + @Test public void testAsyncSearchLargeResultSetBigCountSameCoordinator() { SearchParameterMap params = new SearchParameterMap(); @@ -174,7 +173,7 @@ private static final Logger ourLog = LoggerFactory.getLogger(SearchCoordinatorSv List returnedValues = iter.getReturnedValues(); Pageable page = (Pageable) t.getArguments()[1]; int offset = (int) page.getOffset(); - int end = (int)(page.getOffset() + page.getPageSize()); + int end = (int) (page.getOffset() + page.getPageSize()); end = Math.min(end, returnedValues.size()); offset = Math.min(offset, returnedValues.size()); ourLog.info("findWithSearchUuid {} - {} out of {} values", offset, end, returnedValues.size()); @@ -214,7 +213,7 @@ private static final Logger ourLog = LoggerFactory.getLogger(SearchCoordinatorSv assertEquals(10, allResults.get(0).getResourcePid().longValue()); assertEquals(799, allResults.get(789).getResourcePid().longValue()); - myExpectedNumberOfSearchBuildersCreated = 3; + myExpectedNumberOfSearchBuildersCreated = 4; } @Test diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java index bd5c7f3a90c..19b8a282812 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java @@ -223,16 +223,6 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi int start = Math.max(0, theOffset - theLimit); linkPrev = RestfulServerUtils.createPagingLink(theIncludes, serverBase, searchId, start, theLimit, theRequest.getParameters(), prettyPrint, theBundleType); } -// int offset = theOffset + resourceList.size(); -// -// // We're doing offset pages -// if (numTotalResults == null || offset < numTotalResults) { -// linkNext = (RestfulServerUtils.createPagingLink(theIncludes, serverBase, searchId, offset, numToReturn, theRequest.getParameters(), prettyPrint, theBundleType)); -// } -// if (theOffset > 0) { -// int start = Math.max(0, theOffset - theLimit); -// linkPrev = RestfulServerUtils.createPagingLink(theIncludes, serverBase, searchId, start, theLimit, theRequest.getParameters(), prettyPrint, theBundleType); -// } } bundleFactory.addRootPropertiesToBundle(theResult.getUuid(), serverBase, theLinkSelf, linkPrev, linkNext, theResult.size(), theBundleType, theResult.getPublished()); diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/util/FhirTerserDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/util/FhirTerserDstu3Test.java index dadc87e9b62..aa85a74784c 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/util/FhirTerserDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/util/FhirTerserDstu3Test.java @@ -41,6 +41,35 @@ public class FhirTerserDstu3Test { private static FhirContext ourCtx = FhirContext.forDstu3(); + @Test + public void testCloneIntoBundle() { + Bundle input = new Bundle(); + input.setType(Bundle.BundleType.TRANSACTION); + + Patient pt = new Patient(); + pt.setId("pt"); + pt.setActive(true); + input + .addEntry() + .setResource(pt) + .getRequest() + .setUrl("Patient/pt") + .setMethod(Bundle.HTTPVerb.PUT); + + Observation obs = new Observation(); + obs.setId("obs"); + obs.getSubject().setReference("Patient/pt"); + input + .addEntry() + .setResource(obs) + .getRequest() + .setUrl("Observation/obs") + .setMethod(Bundle.HTTPVerb.PUT); + + Bundle ionputClone = new Bundle(); + ourCtx.newTerser().cloneInto(input, ionputClone, false); + } + @Test public void testCloneIntoComposite() { Quantity source = new Quantity();