From 0a8900ee78b5aad1418fbfb1795bfda728d5c5f2 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 10 Apr 2017 16:29:24 -0400 Subject: [PATCH] Perf updates possibly working? --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 57 ++++++----- .../FhirResourceDaoDstu2SearchNoFtTest.java | 14 +-- .../fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java | 3 + ...ceDaoDstu3SearchCustomSearchParamTest.java | 18 +++- .../FhirResourceDaoDstu3SearchNoFtTest.java | 97 +++++++++++++++++++ 5 files changed, 152 insertions(+), 37 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index 7a75807286d..2d2ebde8cc3 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -143,7 +143,7 @@ public class SearchBuilder { Join join = myResourceTableRoot.join("myParamsDate", JoinType.LEFT); if (theList.get(0).getMissing() != null) { - addPredicateParamMissing(theParamName, theList.get(0).getMissing(), join); + addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); return; } @@ -269,7 +269,7 @@ public class SearchBuilder { Join join = myResourceTableRoot.join("myParamsNumber", JoinType.LEFT); if (theList.get(0).getMissing() != null) { - addPredicateParamMissing(theParamName, theList.get(0).getMissing(), join); + addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); return; } @@ -302,18 +302,18 @@ public class SearchBuilder { myPredicates.add(myBuilder.or(toArray(codePredicates))); } - private void addPredicateParamMissing(String theParamName, boolean theMissing, Join theJoin) { + private void addPredicateParamMissing(String theResourceName, String theParamName, boolean theMissing, Join theJoin) { - myPredicates.add(myBuilder.equal(theJoin.get("myResourceType"), myResourceName)); + myPredicates.add(myBuilder.equal(theJoin.get("myResourceType"), theResourceName)); myPredicates.add(myBuilder.equal(theJoin.get("myParamName"), theParamName)); myPredicates.add(myBuilder.equal(theJoin.get("myMissing"), theMissing)); } - private void addPredicateParamMissing(String theParamName, boolean theMissing) { + private void addPredicateParamMissing(String theResourceName, String theParamName, boolean theMissing) { Join paramPresentJoin = myResourceTableRoot.join("mySearchParamPresents", JoinType.LEFT); Join paramJoin = paramPresentJoin.join("mySearchParam", JoinType.LEFT); - myPredicates.add(myBuilder.equal(paramJoin.get("myResourceName"), myResourceName)); + myPredicates.add(myBuilder.equal(paramJoin.get("myResourceName"), theResourceName)); myPredicates.add(myBuilder.equal(paramJoin.get("myParamName"), theParamName)); myPredicates.add(myBuilder.equal(paramPresentJoin.get("myPresent"), !theMissing)); } @@ -322,7 +322,7 @@ public class SearchBuilder { Join join = myResourceTableRoot.join("myParamsQuantity", JoinType.LEFT); if (theList.get(0).getMissing() != null) { - addPredicateParamMissing(theParamName, theList.get(0).getMissing(), join); + addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); return; } @@ -336,11 +336,14 @@ public class SearchBuilder { myPredicates.add(myBuilder.or(toArray(codePredicates))); } - private void addPredicateReference(String theParamName, List theList) { + /** + * Add reference predicate to the current search + */ + private void addPredicateReference(String theResourceName, String theParamName, List theList) { assert theParamName.contains(".") == false; if (theList.get(0).getMissing() != null) { - addPredicateParamMissing(theParamName, theList.get(0).getMissing()); + addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing()); return; } @@ -377,8 +380,10 @@ public class SearchBuilder { } for (Long next : targetPid) { ourLog.debug("Searching for resource link with target PID: {}", next); - Predicate eq = myBuilder.equal(join.get("myTargetResourcePid"), next); - codePredicates.add(eq); + + Predicate pathPredicate = createResourceLinkPathPredicate(theResourceName, theParamName, join); + Predicate pidPredicate = myBuilder.equal(join.get("myTargetResourcePid"), next); + codePredicates.add(myBuilder.and(pathPredicate, pidPredicate)); } } else { @@ -469,7 +474,6 @@ public class SearchBuilder { } foundChainMatch = true; - // Set pids = dao.searchForIds(chain, chainValue); Subquery subQ = myResourceTableQuery.subquery(Long.class); Root subQfrom = subQ.from(ResourceTable.class); @@ -501,8 +505,9 @@ public class SearchBuilder { myResourceTableRoot = stackRoot; myPredicates = stackPredicates; - Predicate eq = join.get("myTargetResourcePid").in(subQ); - codePredicates.add(eq); + Predicate pathPredicate = createResourceLinkPathPredicate(theResourceName, theParamName, join); + Predicate pidPredicate = join.get("myTargetResourcePid").in(subQ); + codePredicates.add(myBuilder.and(pathPredicate, pidPredicate)); } @@ -525,7 +530,7 @@ public class SearchBuilder { Join join = myResourceTableRoot.join("myParamsString", JoinType.LEFT); if (theList.get(0).getMissing() != null) { - addPredicateParamMissing(theParamName, theList.get(0).getMissing(), join); + addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); return; } @@ -673,7 +678,7 @@ public class SearchBuilder { Join join = myResourceTableRoot.join("myParamsToken", JoinType.LEFT); if (theList.get(0).getMissing() != null) { - addPredicateParamMissing(theParamName, theList.get(0).getMissing(), join); + addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); return; } @@ -700,12 +705,12 @@ public class SearchBuilder { myPredicates.add(spPredicate); } - private void addPredicateUri(String theParamName, List theList) { + private void addPredicateUri(String theResourceName, String theParamName, List theList) { Join join = myResourceTableRoot.join("myParamsUri", JoinType.LEFT); if (theList.get(0).getMissing() != null) { - addPredicateParamMissing(theParamName, theList.get(0).getMissing(), join); + addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); return; } @@ -1172,8 +1177,8 @@ public class SearchBuilder { return combineParamIndexPredicateWithParamNamePredicate(theResourceName, theParamName, theFrom, singleCode); } - private Predicate createResourceLinkPathPredicate(String theParamName, From from) { - return createResourceLinkPathPredicate(myCallingDao, myContext, theParamName, from, myResourceType); + private Predicate createResourceLinkPathPredicate(String theResourceName, String theParamName, From from) { + return createResourceLinkPathPredicate(myCallingDao, myContext, theParamName, from, theResourceName); } private TypedQuery createSearchAllByTypeQuery(DateRangeParam theLastUpdated) { @@ -1750,7 +1755,7 @@ public class SearchBuilder { break; case REFERENCE: for (List nextAnd : theAndOrParams) { - addPredicateReference(theParamName, nextAnd); + addPredicateReference(theResourceName, theParamName, nextAnd); } break; case STRING: @@ -1775,7 +1780,7 @@ public class SearchBuilder { break; case URI: for (List nextAnd : theAndOrParams) { - addPredicateUri(theParamName, nextAnd); + addPredicateUri(theResourceName, theParamName, nextAnd); } break; case HAS: @@ -1915,12 +1920,12 @@ public class SearchBuilder { return likeExpression.replace("%", "[%]") + "%"; } - private static Predicate createResourceLinkPathPredicate(IDao theCallingDao, FhirContext theContext, String theParamName, From from, - Class resourceType) { - RuntimeResourceDefinition resourceDef = theContext.getResourceDefinition(resourceType); + private static Predicate createResourceLinkPathPredicate(IDao theCallingDao, FhirContext theContext, String theParamName, From theFrom, + String theResourceType) { + RuntimeResourceDefinition resourceDef = theContext.getResourceDefinition(theResourceType); RuntimeSearchParam param = theCallingDao.getSearchParamByName(resourceDef, theParamName); List path = param.getPathsSplit(); - Predicate type = from.get("mySourcePath").in(path); + Predicate type = theFrom.get("mySourcePath").in(path); return type; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2SearchNoFtTest.java index 5466a038e28..0b2d38a93d6 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2SearchNoFtTest.java @@ -24,6 +24,7 @@ import java.util.Set; import javax.servlet.http.HttpServletRequest; import org.apache.commons.lang3.StringUtils; +import org.hl7.fhir.dstu3.model.Task; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.junit.AfterClass; @@ -106,6 +107,9 @@ import ca.uhn.fhir.util.TestUtil; public class FhirResourceDaoDstu2SearchNoFtTest extends BaseJpaDstu2Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoDstu2SearchNoFtTest.class); + @Autowired + private ISearchParamPresentDao mySearchParamPresentDao; + @Test public void testCodeSearch() { Subscription subs = new Subscription(); @@ -1264,15 +1268,12 @@ public class FhirResourceDaoDstu2SearchNoFtTest extends BaseJpaDstu2Test { result = toList(myObservationDao.search(Observation.SP_SUBJECT, new ReferenceParam("testSearchResourceLinkWithTextLogicalId99"))); assertEquals(0, result.size()); - + result = toList(myObservationDao.search(Observation.SP_SUBJECT, new ReferenceParam("999999999999999"))); assertEquals(0, result.size()); } - @Autowired - private ISearchParamPresentDao mySearchParamPresentDao; - @SuppressWarnings("unused") @Test public void testSearchResourceReferenceMissing() { @@ -1311,7 +1312,7 @@ public class FhirResourceDaoDstu2SearchNoFtTest extends BaseJpaDstu2Test { } ourLog.info("" + mySearchParamPresentDao.findAll()); - + SearchParameterMap params; params = new SearchParameterMap(); @@ -1417,7 +1418,8 @@ public class FhirResourceDaoDstu2SearchNoFtTest extends BaseJpaDstu2Test { Patient patient = new Patient(); patient.addIdentifier().setSystem("urn:system").setValue("testSearchTokenParam001"); patient.addName().addFamily("Tester").addGiven("testSearchTokenParam1"); - patient.addCommunication().getLanguage().setText("testSearchTokenParamComText").addCoding().setCode("testSearchTokenParamCode").setSystem("testSearchTokenParamSystem").setDisplay("testSearchTokenParamDisplay"); + patient.addCommunication().getLanguage().setText("testSearchTokenParamComText").addCoding().setCode("testSearchTokenParamCode").setSystem("testSearchTokenParamSystem") + .setDisplay("testSearchTokenParamDisplay"); myPatientDao.create(patient, mySrd); patient = new Patient(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java index 394f95654be..7f2d8524f5a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java @@ -148,6 +148,9 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Qualifier("myOrganizationDaoDstu3") protected IFhirResourceDao myOrganizationDao; @Autowired + @Qualifier("myTaskDaoDstu3") + protected IFhirResourceDao myTaskDao; + @Autowired @Qualifier("myPatientDaoDstu3") protected IFhirResourceDaoPatient myPatientDao; @Autowired diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchCustomSearchParamTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchCustomSearchParamTest.java index ddfd3744dae..0cfb7563984 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchCustomSearchParamTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchCustomSearchParamTest.java @@ -20,6 +20,7 @@ import org.junit.Test; import ca.uhn.fhir.jpa.dao.SearchParameterMap; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.IBundleProvider; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.util.TestUtil; @@ -238,8 +239,12 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu // Try with custom gender SP map = new SearchParameterMap(); map.add("foo", new TokenParam(null, "male")); - IBundleProvider res = myPatientDao.search(map); - assertEquals(0, res.size()); + try { + myPatientDao.search(map); + fail(); + } catch (InvalidRequestException e) { + assertEquals("Unknown search parameter foo for resource type Patient", e.getMessage()); + } } @Test @@ -272,9 +277,12 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu // Try with custom gender SP (should find nothing) map = new SearchParameterMap(); map.add("foo", new TokenParam(null, "male")); - results = myPatientDao.search(map); - foundResources = toUnqualifiedVersionlessIdValues(results); - assertThat(foundResources, empty()); + try { + myPatientDao.search(map); + fail(); + } catch (InvalidRequestException e) { + assertEquals("Unknown search parameter foo for resource type Patient", e.getMessage()); + } // Try with normal gender SP map = new SearchParameterMap(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java index 7143a21b3f1..d89c4f0015a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java @@ -85,6 +85,103 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { myDaoConfig.setExpireSearchResultsAfterMillis(new DaoConfig().getExpireSearchResultsAfterMillis()); } + + @SuppressWarnings("unused") + @Test + public void testSearchResourceReferenceOnlyCorrectPath() { + IIdType oid1; + { + Organization org = new Organization(); + org.setActive(true); + oid1 = myOrganizationDao.create(org, mySrd).getId().toUnqualifiedVersionless(); + } + IIdType tid1; + { + Task task = new Task(); + task.getRequester().setOnBehalfOf(new Reference(oid1)); + tid1 = myTaskDao.create(task, mySrd).getId().toUnqualifiedVersionless(); + } + IIdType tid2; + { + Task task = new Task(); + task.setOwner(new Reference(oid1)); + tid2 = myTaskDao.create(task, mySrd).getId().toUnqualifiedVersionless(); + } + + SearchParameterMap map; + List ids; + + map = new SearchParameterMap(); + map.add(Task.SP_ORGANIZATION, new ReferenceParam(oid1.getValue())); + ids = toUnqualifiedVersionlessIds(myTaskDao.search(map)); + assertThat(ids, contains(tid1)); // NOT tid2 + + } + + @SuppressWarnings("unused") + @Test + public void testSearchResourceReferenceMissingChain() { + IIdType oid1; + { + Organization org = new Organization(); + org.setActive(true); + oid1 = myOrganizationDao.create(org, mySrd).getId().toUnqualifiedVersionless(); + } + IIdType tid1; + { + Task task = new Task(); + task.getRequester().setOnBehalfOf(new Reference(oid1)); + tid1 = myTaskDao.create(task, mySrd).getId().toUnqualifiedVersionless(); + } + IIdType tid2; + { + Task task = new Task(); + task.setOwner(new Reference(oid1)); + tid2 = myTaskDao.create(task, mySrd).getId().toUnqualifiedVersionless(); + } + + IIdType oid2; + { + Organization org = new Organization(); + org.setActive(true); + org.setName("NAME"); + oid2 = myOrganizationDao.create(org, mySrd).getId().toUnqualifiedVersionless(); + } + IIdType tid3; + { + Task task = new Task(); + task.getRequester().setOnBehalfOf(new Reference(oid2)); + tid3 = myTaskDao.create(task, mySrd).getId().toUnqualifiedVersionless(); + } + + SearchParameterMap map; + List ids; + + map = new SearchParameterMap(); + map.add(Organization.SP_NAME, new StringParam().setMissing(true)); + ids = toUnqualifiedVersionlessIds(myOrganizationDao.search(map)); + assertThat(ids, contains(oid1)); + + ourLog.info("Starting Search 2"); + + map = new SearchParameterMap(); + map.add(Task.SP_ORGANIZATION, new ReferenceParam("Organization", "name:missing", "true")); + ids = toUnqualifiedVersionlessIds(myTaskDao.search(map)); + assertThat(ids, contains(tid1)); // NOT tid2 + + map = new SearchParameterMap(); + map.add(Task.SP_ORGANIZATION, new ReferenceParam("Organization", "name:missing", "false")); + ids = toUnqualifiedVersionlessIds(myTaskDao.search(map)); + assertThat(ids, contains(tid3)); + + map = new SearchParameterMap(); + map.add(Task.SP_ORGANIZATION, new ReferenceParam("Organization", "name:missing", "true")); + ids = toUnqualifiedVersionlessIds(myPatientDao.search(map)); + assertThat(ids, empty()); + + } + + /** * See #441 */