diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java index f1018576981..d79e1edbf1c 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java @@ -31,6 +31,7 @@ public class InclusionQuerySegmentAggregator extends QuerySegmentAggregator { private static final String SELECT_ROOT = "SELECT RESOURCE_ID, LOGICAL_RESOURCE_ID, VERSION_ID, LAST_UPDATED, IS_DELETED, DATA, LOGICAL_ID FROM "; private static final String UNION_ALL = " UNION ALL "; private static final String REVINCLUDE_JOIN = "JOIN {0}_STR_VALUES P1 ON P1.LOGICAL_RESOURCE_ID = R.LOGICAL_RESOURCE_ID "; + private static final String ORDERING = " ORDER BY R.LOGICAL_RESOURCE_ID ASC "; private List includeParameters; private List revIncludeParameters; @@ -132,9 +133,6 @@ protected SqlQueryData buildCountQuery() throws Exception { queryString.append(super.buildFromClause()); queryString.append(super.buildWhereClause()); - this.processIncludeParameters(queryString, allBindVariables); - this.processRevIncludeParameters(queryString, allBindVariables); - queryString.append(")"); queryString.append(COMBINED_RESULTS); @@ -164,20 +162,22 @@ protected SqlQueryData buildQuery() throws Exception { this.addBindVariables(allBindVariables); + queryString.append(InclusionQuerySegmentAggregator.SELECT_ROOT); + queryString.append("("); queryString.append(InclusionQuerySegmentAggregator.SELECT_ROOT); queryString.append("("); queryString.append(QuerySegmentAggregator.SELECT_ROOT); queryString.append(super.buildFromClause()); queryString.append(super.buildWhereClause()); - + // Add ordering + queryString.append(ORDERING); + this.addPaginationClauses(queryString); + queryString.append(") RESULT "); this.processIncludeParameters(queryString, allBindVariables); this.processRevIncludeParameters(queryString, allBindVariables); queryString.append(")"); - queryString.append(COMBINED_RESULTS); - - this.addPaginationClauses(queryString); - + queryString.append(COMBINED_RESULTS); queryData = new SqlQueryData(queryString.toString(), allBindVariables); log.exiting(CLASSNAME, METHODNAME); return queryData; @@ -201,6 +201,10 @@ private void executeIncludeSubQuery(StringBuilder queryString, InclusionParamete subQueryString.append(super.buildFromClause()); // Add WHERE clause for "root" resource type subQueryString.append(super.buildWhereClause()); + // ORDER BY R.LOGICAL_RESOURCE_ID ASC + subQueryString.append(ORDERING); + // Only include resources related to the required page of the main resources. + this.addPaginationClauses(subQueryString); subQueryString.append(")"); queryString.append("("); @@ -276,6 +280,10 @@ private void processRevIncludeParameters(StringBuilder queryString, List queryString.append(super.buildFromClause()); // Add WHERE clause for "root" resource type queryString.append(super.buildWhereClause()); + // ORDER BY R.LOGICAL_RESOURCE_ID ASC + queryString.append(ORDERING); + // Only include resources related to the required page of the main resources. + this.addPaginationClauses(queryString); queryString.append(")"); diff --git a/fhir-persistence/src/test/java/com/ibm/fhir/persistence/test/common/AbstractIncludeRevincludeTest.java b/fhir-persistence/src/test/java/com/ibm/fhir/persistence/test/common/AbstractIncludeRevincludeTest.java index 27f5ace7c03..17fdc9321ea 100644 --- a/fhir-persistence/src/test/java/com/ibm/fhir/persistence/test/common/AbstractIncludeRevincludeTest.java +++ b/fhir-persistence/src/test/java/com/ibm/fhir/persistence/test/common/AbstractIncludeRevincludeTest.java @@ -15,6 +15,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; @@ -482,7 +483,65 @@ else if (resource instanceof Patient) { } } - + + private void checkIncludeAndRevIncludeResources(List resources, int numOfPatients) { + HashSet foundPatientOrgIds = new HashSet(); + HashSet foundPatientIds = new HashSet(); + int numOfMatchedOrgs = 0; + for (Resource resource : resources) { + if (resource instanceof Patient) { + if (((Patient)resource).getManagingOrganization() != null) { + foundPatientOrgIds.add(((Patient)resource).getManagingOrganization() + .getReference().getValue().substring(13)); + } + foundPatientIds.add(resource.getId().getValue()); + } + } + // verify the number of patients in the result page. + assertTrue(numOfPatients == foundPatientIds.size()); + for (Resource resource : resources) { + if (resource instanceof Observation) { + // verify the returned observations are related to the patients of the same page + assertTrue(foundPatientIds.contains(((Observation)resource).getSubject() + .getReference().getValue().substring(8))); + } else if (resource instanceof Organization) { + // verify the returned managing organizations are related to the patients of the same page + assertTrue(foundPatientOrgIds.contains(resource.getId().getValue())); + numOfMatchedOrgs++; + } else if (resource instanceof Patient) { + assertTrue(foundPatientIds.contains(resource.getId().getValue())); + } else { + fail("Unexpected resource type returned."); + } + } + // verify all related managing organizations are in the same page. + assertTrue(numOfMatchedOrgs == foundPatientOrgIds.size()); + } + /** + * This test queries page of Patient with inclusion of the referenced managing organizations. + * and also requests the reverse inclusion of Observations. + * It verifies: + * (1) the number of patients in the result page; + * (2) the returned managing organizations and observations are related to the patients of the same page; + * (3) all related managing organizations are in the same page. + * @throws Exception + */ + @Test + public void testIncludeAndRevIncludeMultiplePatients() throws Exception { + Map> queryParms = new HashMap>(); + queryParms.put("_include", Collections.singletonList("Patient:organization")); + queryParms.put("_revinclude", Collections.singletonList("Observation:patient")); + queryParms.put("_count", Collections.singletonList("2")); + queryParms.put("_page", Collections.singletonList("1")); + List resources = runQueryTest(Patient.class, queryParms); + assertNotNull(resources); + // check the first page + checkIncludeAndRevIncludeResources(resources, 2); + queryParms.put("_page", Collections.singletonList("2")); + // check the second page + checkIncludeAndRevIncludeResources(resources, 2); + } + private Reference reference(String reference) { return Reference.builder().reference(string(reference)).build(); }