4190 offset count and total are wrong for some queries offset produces short pages and repeats results on different pages (#4193)
* created failing test * implemented the fix, added more tests * added change log Co-authored-by: Steven Li <steven@smilecdr.com>
This commit is contained in:
parent
51627fb873
commit
e1e33df38c
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
type: fix
|
||||
issue: 4190
|
||||
title: "Previously, when using _offset, the queries will result in short pages, and repeats results on different pages.
|
||||
This has now been fixed."
|
|
@ -951,6 +951,11 @@ public class QueryStack {
|
|||
}
|
||||
}
|
||||
|
||||
public void addGrouping() {
|
||||
BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
|
||||
mySqlBuilder.getSelect().addGroupings(firstPredicateBuilder.getResourceIdColumn());
|
||||
}
|
||||
|
||||
private class ChainElement {
|
||||
private final String myResourceType;
|
||||
private final String mySearchParameterName;
|
||||
|
|
|
@ -648,6 +648,10 @@ public class SearchBuilder implements ISearchBuilder {
|
|||
createSort(queryStack3, sort);
|
||||
}
|
||||
|
||||
if (theOffset != null) {
|
||||
queryStack3.addGrouping();
|
||||
}
|
||||
|
||||
/*
|
||||
* Now perform the search
|
||||
*/
|
||||
|
|
|
@ -2,20 +2,32 @@ package ca.uhn.fhir.jpa.dao.r4;
|
|||
|
||||
import ca.uhn.fhir.i18n.Msg;
|
||||
import ca.uhn.fhir.jpa.api.config.DaoConfig;
|
||||
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
|
||||
import ca.uhn.fhir.jpa.dao.data.IResourceHistoryProvenanceDao;
|
||||
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
|
||||
import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
|
||||
import ca.uhn.fhir.rest.api.Constants;
|
||||
import ca.uhn.fhir.rest.api.server.IBundleProvider;
|
||||
import ca.uhn.fhir.rest.param.StringParam;
|
||||
import ca.uhn.fhir.rest.param.TokenParam;
|
||||
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
|
||||
import org.hamcrest.Matchers;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
import org.hl7.fhir.instance.model.api.IIdType;
|
||||
import org.hl7.fhir.r4.model.CarePlan;
|
||||
import org.hl7.fhir.r4.model.DateType;
|
||||
import org.hl7.fhir.r4.model.DecimalType;
|
||||
import org.hl7.fhir.r4.model.Enumerations;
|
||||
import org.hl7.fhir.r4.model.HealthcareService;
|
||||
import org.hl7.fhir.r4.model.Identifier;
|
||||
import org.hl7.fhir.r4.model.Location;
|
||||
import org.hl7.fhir.r4.model.Meta;
|
||||
import org.hl7.fhir.r4.model.Patient;
|
||||
import org.hl7.fhir.r4.model.Practitioner;
|
||||
import org.hl7.fhir.r4.model.PractitionerRole;
|
||||
import org.hl7.fhir.r4.model.Reference;
|
||||
import org.hl7.fhir.r4.model.RiskAssessment;
|
||||
import org.hl7.fhir.r4.model.SearchParameter;
|
||||
import org.hl7.fhir.r4.model.ValueSet;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
|
@ -23,6 +35,7 @@ import org.junit.jupiter.api.Test;
|
|||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.beans.factory.annotation.Qualifier;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.stream.Collectors;
|
||||
|
@ -39,6 +52,9 @@ public class FhirResourceDaoR4FilterTest extends BaseJpaR4Test {
|
|||
private static final Logger ourLog = LoggerFactory.getLogger(FhirResourceDaoR4FilterTest.class);
|
||||
@Autowired
|
||||
private IResourceHistoryProvenanceDao myResourceProvenanceDao;
|
||||
@Autowired
|
||||
@Qualifier("myHealthcareServiceDaoR4")
|
||||
protected IFhirResourceDao<HealthcareService> myHealthcareServiceDao;
|
||||
|
||||
@AfterEach
|
||||
public void after() {
|
||||
|
@ -349,7 +365,6 @@ public class FhirResourceDaoR4FilterTest extends BaseJpaR4Test {
|
|||
}
|
||||
|
||||
|
||||
|
||||
@Test
|
||||
public void testStringComparatorCo() {
|
||||
|
||||
|
@ -1238,5 +1253,72 @@ public class FhirResourceDaoR4FilterTest extends BaseJpaR4Test {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSearchWithChainingForNonExistingReference() {
|
||||
SearchParameter searchParameter = new SearchParameter();
|
||||
searchParameter.setId("isplaceholder");
|
||||
searchParameter.setUrl("http://hl7.org/fhir/SearchParameter/isplaceholder");
|
||||
searchParameter.setName("isplaceholder");
|
||||
searchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE);
|
||||
searchParameter.setVersion("1.0.1");
|
||||
searchParameter.setDescription("Custom SearchParameter for HCSC");
|
||||
searchParameter.setCode("isplaceholder");
|
||||
searchParameter.addBase("PractitionerRole");
|
||||
searchParameter.addBase("Organization");
|
||||
searchParameter.addBase("OrganizationAffiliation");
|
||||
searchParameter.addBase("Location");
|
||||
searchParameter.addBase("Practitioner");
|
||||
searchParameter.addBase("HealthcareService");
|
||||
searchParameter.setType(Enumerations.SearchParamType.TOKEN);
|
||||
searchParameter.setExpression("extension.where(url='http://hapifhir.io/fhir/StructureDefinition/resource-placeholder').exists()");
|
||||
searchParameter.setXpathUsage(SearchParameter.XPathUsageType.NORMAL);
|
||||
mySearchParameterDao.update(searchParameter);
|
||||
mySearchParamRegistry.forceRefresh();
|
||||
|
||||
Location l = new Location();
|
||||
l.setId("loc-real");
|
||||
l.setMeta(new Meta().addTag("http://www.smilecdr.com/custom_tag/uuid",
|
||||
"51f922e6-1ae6-48c4-a41e-0f978326000dSP1975060918", ""));
|
||||
myLocationDao.update(l);
|
||||
|
||||
HealthcareService hs = new HealthcareService();
|
||||
hs.setId("hs-real");
|
||||
hs.setMeta(new Meta().addTag("http://www.smilecdr.com/custom_tag/uuid",
|
||||
"51f922e6-1ae6-48c4-a41e-0f978326000dSP1975060918", ""));
|
||||
hs.setActive(true);
|
||||
myHealthcareServiceDao.update(hs);
|
||||
|
||||
Practitioner p = new Practitioner();
|
||||
p.setId("prac-real");
|
||||
Identifier identifier = new Identifier();
|
||||
identifier.setSystem("http://hl7.org/fhir/sid/us-npi");
|
||||
identifier.setValue("1234567890");
|
||||
p.setActive(true);
|
||||
p.setGender(Enumerations.AdministrativeGender.MALE);
|
||||
p.addIdentifier(identifier);
|
||||
myPractitionerDao.update(p);
|
||||
{
|
||||
for (int i = 0; i < 5; i++) {
|
||||
PractitionerRole pr = new PractitionerRole();
|
||||
pr.setId("ref" + i + 1 + "-prac-loc-hcs");
|
||||
pr.setMeta(new Meta().addTag("http://www.smilecdr.com/custom_tag/uuid",
|
||||
"51f922e6-1ae6-48c4-a41e-0f978326000dSP1975060918", ""));
|
||||
pr.setPractitioner(new Reference("Practitioner/prac-real"));
|
||||
pr.addLocation(new Reference("Location/loc-real"));
|
||||
pr.addHealthcareService(new Reference("HealthcareService/hs-real"));
|
||||
myPractitionerRoleDao.create(pr, mySrd).getId().toUnqualifiedVersionless().getValue();
|
||||
}
|
||||
}
|
||||
SearchParameterMap spMap;
|
||||
List<IBaseResource> actual;
|
||||
|
||||
spMap = SearchParameterMap.newSynchronous();
|
||||
spMap.add(Constants.PARAM_TAG, new TokenParam("51f922e6-1ae6-48c4-a41e-0f978326000dSP1975060918"));
|
||||
spMap.add(Constants.PARAM_FILTER, new StringParam("service.isplaceholder eq false or organization.isplaceholder eq false or practitioner.isplaceholder eq false or location.isplaceholder eq false"));
|
||||
spMap.setOffset(0);
|
||||
IBundleProvider search = myPractitionerRoleDao.search(spMap);
|
||||
actual = search.getResources(0, 100);
|
||||
|
||||
assertEquals(5, actual.size());
|
||||
}
|
||||
}
|
||||
|
|
|
@ -34,6 +34,7 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider;
|
|||
import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId;
|
||||
import ca.uhn.fhir.rest.param.DateParam;
|
||||
import ca.uhn.fhir.rest.param.DateRangeParam;
|
||||
import ca.uhn.fhir.rest.param.HasParam;
|
||||
import ca.uhn.fhir.rest.param.ParamPrefixEnum;
|
||||
import ca.uhn.fhir.rest.param.QuantityParam;
|
||||
import ca.uhn.fhir.rest.param.ReferenceParam;
|
||||
|
@ -83,6 +84,7 @@ import org.hl7.fhir.r4.model.DiagnosticReport;
|
|||
import org.hl7.fhir.r4.model.Encounter;
|
||||
import org.hl7.fhir.r4.model.Enumerations.AdministrativeGender;
|
||||
import org.hl7.fhir.r4.model.Enumerations.PublicationStatus;
|
||||
import org.hl7.fhir.r4.model.HumanName;
|
||||
import org.hl7.fhir.r4.model.IdType;
|
||||
import org.hl7.fhir.r4.model.Meta;
|
||||
import org.hl7.fhir.r4.model.MolecularSequence;
|
||||
|
@ -133,6 +135,7 @@ import java.util.concurrent.Executors;
|
|||
import java.util.concurrent.Future;
|
||||
|
||||
import static ca.uhn.fhir.batch2.jobs.termcodesystem.TermCodeSystemJobConfig.TERM_CODE_SYSTEM_VERSION_DELETE_JOB_NAME;
|
||||
import static ca.uhn.fhir.rest.api.Constants.PARAM_HAS;
|
||||
import static org.apache.commons.lang3.StringUtils.countMatches;
|
||||
import static org.apache.commons.lang3.StringUtils.defaultString;
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
|
@ -336,7 +339,7 @@ public class FhirResourceDaoR4Test extends BaseJpaR4Test {
|
|||
}
|
||||
|
||||
|
||||
@Tag("intermittent")
|
||||
@Tag("intermittent")
|
||||
// @Test
|
||||
public void testTermConceptReindexingDoesntDuplicateData() {
|
||||
myDaoConfig.setSchedulingDisabled(true);
|
||||
|
@ -4260,6 +4263,72 @@ public class FhirResourceDaoR4Test extends BaseJpaR4Test {
|
|||
assertNotEquals(uuid, results.getUuid());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSearchWithReverseChainingAndOffset() {
|
||||
int amountOfPatients = 5;
|
||||
{
|
||||
for (int i = 0; i < amountOfPatients; i++) {
|
||||
Patient p = new Patient();
|
||||
p.setActive(true);
|
||||
String pid = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless().getValue();
|
||||
for (int j = 0; j < 20; j++) {
|
||||
Observation o = new Observation();
|
||||
o.setSubject(new Reference().setReference(pid));
|
||||
o.setCode(new CodeableConcept().addCoding(new Coding().setCode("sample")));
|
||||
myObservationDao.create(o, mySrd);
|
||||
}
|
||||
}
|
||||
}
|
||||
SearchParameterMap spMap;
|
||||
List<IBaseResource> actual;
|
||||
|
||||
spMap = SearchParameterMap.newSynchronous();
|
||||
spMap.add(PARAM_HAS, new HasParam("Observation", "patient", "code", "sample"));
|
||||
spMap.setOffset(0);
|
||||
spMap.setCount(5);
|
||||
|
||||
IBundleProvider search = myPatientDao.search(spMap);
|
||||
actual = search.getResources(0, 100);
|
||||
assertEquals(amountOfPatients, actual.size());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSearchWithReverseChainingOffsetAndSort() {
|
||||
int amountOfPatients = 5;
|
||||
String[] namesNotInAlpha = {"Charlie", "Tim", "Adam", "Dan", "Bob"};
|
||||
String[] namesInAlpha = {"Adam", "Bob", "Charlie", "Dan", "Tim"};
|
||||
{
|
||||
for (int i = 0; i < amountOfPatients; i++) {
|
||||
Patient p = new Patient();
|
||||
p.setActive(true);
|
||||
p.addName(new HumanName().addGiven(namesNotInAlpha[i]));
|
||||
String pid = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless().getValue();
|
||||
for (int j = 0; j < 20; j++) {
|
||||
Observation o = new Observation();
|
||||
o.setSubject(new Reference().setReference(pid));
|
||||
o.setCode(new CodeableConcept().addCoding(new Coding().setCode("sample")));
|
||||
myObservationDao.create(o, mySrd);
|
||||
}
|
||||
}
|
||||
}
|
||||
SearchParameterMap spMap;
|
||||
List<IBaseResource> actual;
|
||||
|
||||
spMap = SearchParameterMap.newSynchronous();
|
||||
spMap.add(PARAM_HAS, new HasParam("Observation", "patient", "code", "sample"));
|
||||
spMap.setOffset(0);
|
||||
spMap.setCount(5);
|
||||
spMap.setSort(new SortSpec(Patient.SP_GIVEN));
|
||||
|
||||
IBundleProvider search = myPatientDao.search(spMap);
|
||||
actual = search.getResources(0, 100);
|
||||
List<String> actualNameList = actual.stream().map((resource) -> ((Patient) resource).getName().get(0).getGiven().get(0).toString()).toList();
|
||||
ourLog.info("Results: {}", actualNameList);
|
||||
|
||||
assertEquals(amountOfPatients, actual.size());
|
||||
assertThat(actualNameList, contains(namesInAlpha));
|
||||
}
|
||||
|
||||
public static void assertConflictException(String theResourceType, ResourceVersionConflictException e) {
|
||||
assertThat(e.getMessage(), matchesPattern(
|
||||
Msg.code(550) + Msg.code(515) + "Unable to delete [a-zA-Z]+/[0-9]+ because at least one resource has a reference to this resource. First reference found was resource " + theResourceType + "/[0-9]+ in path [a-zA-Z]+.[a-zA-Z]+"));
|
||||
|
|
Loading…
Reference in New Issue