Fix bug in $everything processing

This commit is contained in:
James Agnew 2018-10-15 05:44:53 -04:00
parent 8b46257423
commit 796b12e33e
8 changed files with 95 additions and 32 deletions

View File

@ -514,18 +514,18 @@ public class DaoConfig {
/**
* This may be used to optionally register server interceptors directly against the DAOs.
*/
public void setInterceptors(IServerInterceptor... theInterceptor) {
setInterceptors(new ArrayList<IServerInterceptor>());
if (theInterceptor != null && theInterceptor.length != 0) {
getInterceptors().addAll(Arrays.asList(theInterceptor));
}
public void setInterceptors(List<IServerInterceptor> theInterceptors) {
myInterceptors = theInterceptors;
}
/**
* This may be used to optionally register server interceptors directly against the DAOs.
*/
public void setInterceptors(List<IServerInterceptor> theInterceptors) {
myInterceptors = theInterceptors;
public void setInterceptors(IServerInterceptor... theInterceptor) {
setInterceptors(new ArrayList<IServerInterceptor>());
if (theInterceptor != null && theInterceptor.length != 0) {
getInterceptors().addAll(Arrays.asList(theInterceptor));
}
}
/**

View File

@ -23,4 +23,7 @@ package ca.uhn.fhir.jpa.dao;
import java.util.Iterator;
public interface IResultIterator extends Iterator<Long> {
int getSkippedCount();
}

View File

@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.dao;
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*
* http://www.apache.org/licenses/LICENSE-2.0
*
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@ -1779,7 +1779,8 @@ public class SearchBuilder implements ISearchBuilder {
}
/**
* THIS SHOULD RETURN HASHSET and not just Set because we add to it later (so it can't be Collections.emptySet())
* THIS SHOULD RETURN HASHSET and not just Set because we add to it later
* so it can't be Collections.emptySet() or some such thing
*/
@Override
public HashSet<Long> loadIncludes(IDao theCallingDao, FhirContext theContext, EntityManager theEntityManager, Collection<Long> theMatches, Set<Include> theRevIncludes,
@ -2173,6 +2174,7 @@ public class SearchBuilder implements ISearchBuilder {
private SortSpec mySort;
private boolean myStillNeedToFetchIncludes;
private StopWatch myStopwatch = null;
private int mySkipCount = 0;
private QueryIterator() {
mySort = myParams.getSort();
@ -2212,20 +2214,26 @@ public class SearchBuilder implements ISearchBuilder {
if (myPreResultsIterator != null && myPreResultsIterator.hasNext()) {
while (myPreResultsIterator.hasNext()) {
Long next = myPreResultsIterator.next();
if (next != null && myPidSet.add(next)) {
myNext = next;
break;
}
if (next != null)
if (myPidSet.add(next)) {
myNext = next;
break;
} else {
mySkipCount++;
}
}
}
if (myNext == null) {
while (myResultsIterator.hasNext()) {
Long next = myResultsIterator.next();
if (next != null && myPidSet.add(next)) {
myNext = next;
break;
}
if (next != null)
if (myPidSet.add(next)) {
myNext = next;
break;
} else {
mySkipCount++;
}
}
}
@ -2237,10 +2245,13 @@ public class SearchBuilder implements ISearchBuilder {
if (myIncludesIterator != null) {
while (myIncludesIterator.hasNext()) {
Long next = myIncludesIterator.next();
if (next != null && myPidSet.add(next)) {
myNext = next;
break;
}
if (next != null)
if (myPidSet.add(next)) {
myNext = next;
break;
} else {
mySkipCount++;
}
}
if (myNext == null) {
myNext = NO_MORE;
@ -2279,6 +2290,11 @@ public class SearchBuilder implements ISearchBuilder {
Validate.isTrue(retVal != NO_MORE, "No more elements");
return retVal;
}
@Override
public int getSkippedCount() {
return mySkipCount;
}
}
private class UniqueIndexIterator implements IResultIterator {
@ -2315,6 +2331,11 @@ public class SearchBuilder implements ISearchBuilder {
public void remove() {
throw new UnsupportedOperationException();
}
@Override
public int getSkippedCount() {
return 0;
}
}
private static class CountQueryIterator implements Iterator<Long> {

View File

@ -519,7 +519,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
});
}
private void saveUnsynced(final Iterator<Long> theResultIter) {
private void saveUnsynced(final IResultIterator theResultIter) {
TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager);
txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED);
txTemplate.execute(new TransactionCallbackWithoutResult() {
@ -547,7 +547,8 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
if (theResultIter.hasNext() == false) {
mySearch.setNumFound(myCountSaved);
if (myMaxResultsToFetch != null && myCountSaved < myMaxResultsToFetch) {
int loadedCountThisPass = theResultIter.getSkippedCount() + myCountSaved;
if (myMaxResultsToFetch != null && loadedCountThisPass < myMaxResultsToFetch) {
mySearch.setStatus(SearchStatusEnum.FINISHED);
mySearch.setTotalCount(myCountSaved);
} else if (myAdditionalPrefetchThresholdsRemaining) {
@ -785,7 +786,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
/*
* Construct the SQL query we'll be sending to the database
*/
Iterator<Long> theResultIterator = sb.createQuery(myParams, mySearch.getUuid());
IResultIterator theResultIterator = sb.createQuery(myParams, mySearch.getUuid());
/*
* The following loop actually loads the PIDs of the resources

View File

@ -4,6 +4,7 @@ import ca.uhn.fhir.jpa.dao.DaoConfig;
import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor;
import ca.uhn.fhir.validation.ResultSeverityEnum;
import net.ttddyy.dsproxy.listener.ThreadQueryCountHolder;
import net.ttddyy.dsproxy.listener.logging.SLF4JLogLevel;
import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder;
import org.apache.commons.dbcp2.BasicDataSource;
import org.springframework.context.annotation.Bean;

View File

@ -158,6 +158,7 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
myDaoConfig.setAllowMultipleDelete(true);
ourClient.registerInterceptor(myCapturingInterceptor);
myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getPreFetchThresholds());
}
@Test
@ -1633,6 +1634,8 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
*/
@Test
public void testEverythingWithLargeSet2() {
myDaoConfig.setSearchPreFetchThresholds(Arrays.asList(15, 30, -1));
Patient p = new Patient();
p.setActive(true);
IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless();
@ -1644,19 +1647,25 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
ourClient.update().resource(obs).execute();
}
Bundle responseBundle = ourClient.operation().onInstance(id).named("everything").withParameter(Parameters.class, "_count", new IntegerType(50)).useHttpGet().returnResourceType(Bundle.class)
Bundle responseBundle = ourClient
.operation()
.onInstance(id)
.named("everything")
.withParameter(Parameters.class, "_count", new IntegerType(50))
.useHttpGet()
.returnResourceType(Bundle.class)
.execute();
TreeSet<String> ids = new TreeSet<String>();
TreeSet<String> ids = new TreeSet<>();
for (int i = 0; i < responseBundle.getEntry().size(); i++) {
for (BundleEntryComponent nextEntry : responseBundle.getEntry()) {
ids.add(nextEntry.getResource().getIdElement().getIdPart());
}
}
ourLog.info("Have {} IDs: {}", ids.size(), ids);
BundleLinkComponent nextLink = responseBundle.getLink("next");
ourLog.info("Have {} IDs with next link: ", ids.size(), nextLink);
while (nextLink != null) {
String nextUrl = nextLink.getUrl();
responseBundle = ourClient.fetchResourceFromUrl(Bundle.class, nextUrl);
@ -1666,8 +1675,8 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
}
}
ourLog.info("Have {} IDs: {}", ids.size(), ids);
nextLink = responseBundle.getLink("next");
ourLog.info("Have {} IDs with next link: ", ids.size(), nextLink);
}
assertThat(ids, hasItem(id.getIdPart()));

View File

@ -422,6 +422,10 @@ public class SearchCoordinatorSvcImplTest {
return myWrap.next();
}
@Override
public int getSkippedCount() {
return myWrap.getSkippedCount();
}
}
public static class ResultIterator extends BaseIterator<Long> implements IResultIterator {
@ -441,16 +445,29 @@ public class SearchCoordinatorSvcImplTest {
public Long next() {
return myWrap.next();
}
@Override
public int getSkippedCount() {
return 0;
}
}
public static class SlowIterator extends BaseIterator<Long> implements IResultIterator {
private final IResultIterator myResultIteratorWrap;
private int myDelay;
private Iterator<Long> myWrap;
public SlowIterator(Iterator<Long> theWrap, int theDelay) {
myWrap = theWrap;
myDelay = theDelay;
myResultIteratorWrap = null;
}
public SlowIterator(IResultIterator theWrap, int theDelay) {
myWrap = theWrap;
myResultIteratorWrap = theWrap;
myDelay = theDelay;
}
@Override
@ -468,6 +485,15 @@ public class SearchCoordinatorSvcImplTest {
return myWrap.next();
}
@Override
public int getSkippedCount() {
if (myResultIteratorWrap == null) {
return 0;
} else {
return myResultIteratorWrap.getSkippedCount();
}
}
}
@AfterClass

View File

@ -215,9 +215,11 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi
linkPrev = RestfulServerUtils.createPagingLink(theIncludes, serverBase, searchId, theResult.getPreviousPageId(), theRequest.getParameters(), prettyPrint, theBundleType);
}
} else if (searchId != null) {
int offset = theOffset + resourceList.size();
// We're doing offset pages
if (numTotalResults == null || theOffset + numToReturn < numTotalResults) {
linkNext = (RestfulServerUtils.createPagingLink(theIncludes, serverBase, searchId, theOffset + numToReturn, numToReturn, theRequest.getParameters(), prettyPrint, theBundleType));
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);