Fix paging issue #1300 (#1584)

* Fix paging issue

* Add changelog with credit

* Add a test
This commit is contained in:
James Agnew 2019-11-07 18:17:09 -05:00 committed by GitHub
parent ebc52e6b48
commit 056756dbab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 141 additions and 32 deletions

View File

@ -2071,6 +2071,7 @@ public class SearchBuilder implements ISearchBuilder {
outerQuery.multiselect(myBuilder.countDistinct(myResourceTableRoot));
} else {
outerQuery.multiselect(myResourceTableRoot.get("myId").as(Long.class));
outerQuery.distinct(true);
}
}

View File

@ -1,31 +0,0 @@
package ca.uhn.fhir.jpa.provider;
/*-
* #%L
* HAPI FHIR JPA Server
* %%
* Copyright (C) 2014 - 2019 University Health Network
* %%
* 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.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
/**
* @deprecated Use ca.uhn.fhir.jpa.binstore.BinaryAccessProvider instead
*/
@Deprecated
public class BinaryAccessProvider extends ca.uhn.fhir.jpa.binstore.BinaryAccessProvider {
// FIXME: JA delete before 4.0.0
}

View File

@ -233,6 +233,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
ourLog.trace("Going to try to start next search");
Optional<Search> newSearch = mySearchCacheSvc.tryToMarkSearchAsInProgress(search);
if (newSearch.isPresent()) {
ourLog.trace("Launching new search");
search = newSearch.get();
String resourceType = search.getResourceType();
SearchParameterMap params = search.getSearchParameterMap().orElseThrow(() -> new IllegalStateException("No map in PASSCOMPLET search"));
@ -1113,7 +1114,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
throw newResourceGoneException(getSearch().getUuid());
}
ourLog.debug("Have {} previously added IDs in search: {}", previouslyAddedResourcePids.size(), getSearch().getUuid());
ourLog.trace("Have {} previously added IDs in search: {}", previouslyAddedResourcePids.size(), getSearch().getUuid());
setPreviouslyAddedResourcePids(previouslyAddedResourcePids);
return null;
});

View File

@ -1,15 +1,19 @@
package ca.uhn.fhir.jpa.provider.r4;
import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao;
import ca.uhn.fhir.jpa.dao.DaoConfig;
import ca.uhn.fhir.jpa.entity.ResourceReindexJobEntity;
import ca.uhn.fhir.jpa.entity.Search;
import ca.uhn.fhir.jpa.model.entity.ModelConfig;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
import ca.uhn.fhir.jpa.model.search.SearchStatusEnum;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.gclient.ReferenceClientParam;
import ca.uhn.fhir.rest.gclient.TokenClientParam;
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import ca.uhn.fhir.util.BundleUtil;
import ca.uhn.fhir.util.TestUtil;
import org.apache.commons.io.IOUtils;
import org.apache.http.client.methods.CloseableHttpResponse;
@ -49,6 +53,8 @@ public class ResourceProviderCustomSearchParamR4Test extends BaseResourceProvide
super.after();
myModelConfig.setDefaultSearchParamsCanBeOverridden(new ModelConfig().isDefaultSearchParamsCanBeOverridden());
myDaoConfig.setAllowContainsSearches(new DaoConfig().isAllowContainsSearches());
}
@Override
@ -431,6 +437,102 @@ public class ResourceProviderCustomSearchParamR4Test extends BaseResourceProvide
}
/**
* See #1300
*/
@Test
public void testCustomParameterMatchingManyValues() {
myDaoConfig.setAllowContainsSearches(true);
// Add a custom search parameter
SearchParameter fooSp = new SearchParameter();
fooSp.addBase("Questionnaire");
fooSp.setCode("item-text");
fooSp.setName("item-text");
fooSp.setType(Enumerations.SearchParamType.STRING);
fooSp.setTitle("FOO SP");
fooSp.setExpression("Questionnaire.item.text | Questionnaire.item.item.text | Questionnaire.item.item.item.text");
fooSp.setXpathUsage(org.hl7.fhir.r4.model.SearchParameter.XPathUsageType.NORMAL);
fooSp.setStatus(org.hl7.fhir.r4.model.Enumerations.PublicationStatus.ACTIVE);
mySearchParameterDao.create(fooSp, mySrd);
mySearchParamRegistry.forceRefresh();
int textIndex = 0;
for (int i = 0; i < 200; i++) {
//Lots and lots of matches
Questionnaire q = new Questionnaire();
q
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++));
q
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++));
q
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++));
q
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++));
q
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++))
.addItem()
.setText("Section " + (textIndex++));
myQuestionnaireDao.create(q);
}
int foundCount = 0;
Bundle bundle = null;
do {
if (bundle == null) {
bundle = ourClient
.search()
.byUrl(ourServerBase + "/Questionnaire?item-text:contains=Section")
.returnBundle(Bundle.class)
.execute();
} else {
bundle = ourClient
.loadPage()
.next(bundle)
.execute();
}
foundCount += BundleUtil.toListOfResources(myFhirCtx, bundle).size();
} while (bundle.getLink("next") != null);
runInTransaction(()->{
List<Search> searches = mySearchEntityDao.findAll();
assertEquals(1, searches.size());
Search search = searches.get(0);
assertEquals(search.toString(), 200, search.getNumFound());
assertEquals(search.toString(), 200, search.getTotalCount().intValue());
assertEquals(search.toString(), SearchStatusEnum.FINISHED, search.getStatus());
});
assertEquals(200, foundCount);
}
@AfterClass
public static void afterClassClearContext() {
TestUtil.clearAllStaticFieldsForUnitTest();

View File

@ -0,0 +1,29 @@
{
"resourceType": "SearchParameter",
"id": "54805",
"meta": {
"versionId": "1",
"lastUpdated": "2019-11-04T18:33:47.918+00:00",
"source": "#pt4ERkOO6LGm5ZoA"
},
"text": {
"status": "generated",
"div": "<div xmlns=\"http://www.w3.org/1999/xhtml\">Search for a questionnaire by item.text nested field - up to 3 levels.</div>"
},
"url": "https://impact-fhir.mitre.org/r4/SearchParameter/QuestionnaireItemText",
"version": "0.0.1",
"name": "QuestionnaireItemText",
"status": "active",
"date": "2019-10-25T15:38:45-04:00",
"description": "Search for a questionnaire by item.text nested field - up to 3 levels.",
"code": "item-text",
"base": [
"Questionnaire"
],
"type": "string",
"expression": "Questionnaire.item.text | Questionnaire.item.item.text | Questionnaire.item.item.item.text",
"xpathUsage": "normal",
"modifier": [
"contains"
]
}

View File

@ -506,6 +506,13 @@
filter could bring in search results of the wrong type. Thanks to Anthony Sute for the Pull
Request and Jens Villadsen for reporting!
</action>
<action type="fix" issue="1300">
In some cases where where a single search parameter matches the same resource many times with
different distinct values (e.g. a search by Patient:name where there are hundreds of patients having
hundreds of distinct names each) the Search Coordinator would end up in an infinite loop and never
return all of the possible results. Thanks to @imranmoezkhan for reporting, and to
Tim Shaffer for providing a reproducible test case!
</action>
</release>
<release version="4.0.3" date="2019-09-03" description="Igloo (Point Release)">
<action type="fix">