Skip to content

Commit

Permalink
Issue #2070 - Support _total with _include and _revinclude
Browse files Browse the repository at this point in the history
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
  • Loading branch information
tbieste committed Mar 25, 2021
1 parent cdfd7b7 commit c55bd65
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 36 deletions.
4 changes: 2 additions & 2 deletions docs/src/pages/Conformance.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
layout: post
title: Conformance
description: Notes on the Conformance of the IBM FHIR Server
date: 2021-03-22 12:00:00 -0400
date: 2021-03-25 12:00:00 -0400
permalink: /conformance/
---

Expand Down Expand Up @@ -148,7 +148,7 @@ The `_include` and `_revinclude` parameters can be used to return resources rela

The `:iterate` modifier is supported for the `_include` and `_revinclude` parameters. The number of iterations is limited to 1. This means the iteration depth will be limited to one level beyond the depth of the resources being iterated against, whether primary search resources or included resources. One exception to this is the case where an iterative `_include` or `_revinclude` is specified that will return the same resource type as the primary search resource type (for example `.../Patient?_include:iterate=Patient:link:Patient`). In this case, the iteration depth will be limited to a maximum of two levels beyond the primary search resource type.

The `_sort` and `_total` parameters cannot be used in combination with the `_include` or `_revinclude` parameters.
The `_sort` parameter cannot be used in combination with the `_include` or `_revinclude` parameters.

The `_contained` and `_containedType` parameters are not supported at this time.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public boolean isIterate() {
return Modifier.ITERATE.equals(modifier);
}


public boolean isUserSpecifiedTargetType() {
return userSpecifiedTargetType;
}
Expand Down Expand Up @@ -116,7 +116,7 @@ public boolean equals(Object obj) {
@Override
public String toString() {
return "InclusionParameter [joinResourceType=" + joinResourceType + ", searchParameter=" + searchParameter + ", searchParameterTargetType="
+ searchParameterTargetType + ", modifier=" + modifier.toString() + ", userSpecifiedTargetType=" + userSpecifiedTargetType + "]";
+ searchParameterTargetType + ", modifier=" + modifier + ", userSpecifiedTargetType=" + userSpecifiedTargetType + "]";
}

}
24 changes: 9 additions & 15 deletions fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -757,12 +757,6 @@ public static FHIRSearchContext parseQueryParameters(Class<?> resourceType,
throw SearchExceptionUtil.buildNewInvalidSearchException(
"_sort search result parameter not supported with _include or _revinclude.");
}
// Make sure _total is not present with _include and/or _revinclude.
// TODO: do we really need to forbid this?
if (queryParameters.containsKey(SearchConstants.TOTAL)) {
throw SearchExceptionUtil.buildNewInvalidSearchException(
"_total search result parameter not supported with _include or _revinclude.");
}
// Because _include and _revinclude searches all require certain resource type modifier in
// search parameter, so we just don't support it.
if (Resource.class.equals(resourceType)) {
Expand Down Expand Up @@ -926,7 +920,7 @@ public static FHIRSearchContext parseQueryParameters(Class<?> resourceType,
try {
// Check for valid search parameter combinations
checkSearchParameterCombinations(resourceType, parameters);

// Check include resource type mismatches for :iterate parameters
if (!context.getIncludeParameters().isEmpty() || !context.getRevIncludeParameters().isEmpty()) {
checkInclusionIterateParameters(resourceType.getSimpleName(), context, lenient);
Expand Down Expand Up @@ -1583,7 +1577,7 @@ private static SearchConstants.Prefix getPrefix(String s) throws FHIRSearchExcep

/**
* Determine if the parameter is a search result parameter.
*
*
* @param name - the parameter name
* @return true if the parameter is a search result parameter, false otherwise
*/
Expand Down Expand Up @@ -2244,7 +2238,7 @@ private static void parseInclusionParameter(Class<?> resourceType, FHIRSearchCon
}
searchParametersMap = Collections.singletonMap(searchParameterName, searchParm);
}

List<InclusionParameter> inclusionParameters = new ArrayList<>();
for (SearchParameter searchParameter : searchParametersMap.values()) {
inclusionParameters.addAll(buildInclusionParameters(resourceType, joinResourceType,
Expand Down Expand Up @@ -2323,7 +2317,7 @@ else if (SEARCH_PROPERTY_TYPE_REVINCLUDE.equals(propertyType)) {
* @param inclusionKeyword the inclusion keyword (_include or _revinclude)
* @param lenient the validation level
* @return a list of InclusionParameter objects
* @throws FHIRSearchException
* @throws FHIRSearchException
*/
private static List<InclusionParameter> buildInclusionParameters(Class<?> resourceType, String joinResourceType,
SearchParameter searchParm, String searchParameterTargetType, Modifier modifier, String inclusionKeyword,
Expand Down Expand Up @@ -2581,10 +2575,10 @@ public static String makeCompositeSubCode(String compositeCode, String subParame
result.append(subParameterCode);
return result.toString();
}

/**
* Check if the list of search parameters contains either _include or _revinclude.
*
*
* @param searchParameterCodes the set of search parameters to check
* @return true if either _include or _revinclude found, false otherwise
*/
Expand All @@ -2604,7 +2598,7 @@ public static boolean containsInclusionParameter(Set<String> searchParameterCode
* Check if _include or _revinclude parameters with the :iterate modifier reference invalid resource types.
* If in strict mode, throw a FHIRSearchException. If in lenient mode, log the invalid parameters and
* remove those parameters from the search context.
*
*
* @param resourceType the search resource type
* @param context the search context
* @param lenient flag indicating lenient or strict mode
Expand Down Expand Up @@ -2640,7 +2634,7 @@ public static void checkInclusionIterateParameters(String resourceType, FHIRSear
validTargetResourceTypesForRevInclude.add(p.getJoinResourceType());
});
}

// Get the :iterate inclusion parameters that reference invalid resource types.
// If in lenient mode, or if multiple added because no target type was specified,
// log and remove. If in strict mode, or if only one valid target type, throw an exception.
Expand Down Expand Up @@ -2678,7 +2672,7 @@ public static void checkInclusionIterateParameters(String resourceType, FHIRSear
} catch (RuntimeException e) {
throw SearchExceptionUtil.buildNewInvalidSearchException(e.getMessage());
}

// Remove invalid inclusion parameters
context.getIncludeParameters().removeAll(invalidIncludeParameters);
context.getRevIncludeParameters().removeAll(invalidRevIncludeParameters);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.ibm.fhir.model.resource.Practitioner;
import com.ibm.fhir.model.resource.Resource;
import com.ibm.fhir.search.SearchConstants.Modifier;
import com.ibm.fhir.search.TotalValueSet;
import com.ibm.fhir.search.context.FHIRSearchContext;
import com.ibm.fhir.search.exception.FHIRSearchException;
import com.ibm.fhir.search.parameters.InclusionParameter;
Expand Down Expand Up @@ -85,15 +86,26 @@ public void testIncludeInvalidWithSort() throws Exception {
SearchUtil.parseQueryParameters(resourceType, queryParameters);
}

@Test(expectedExceptions = FHIRSearchException.class)
public void testIncludeInvalidWithTotal() throws Exception {
@Test
public void testIncludeWithTotal() throws Exception {
Map<String, List<String>> queryParameters = new HashMap<>();
Class<Patient> resourceType = Patient.class;
String queryString = "&_include=Patient:general-practitioner:Practitioner&_total=none";
InclusionParameter incParm = new InclusionParameter("Patient", "general-practitioner", "Practitioner", null, true);

// In strict mode, the query should throw a FHIRSearchException
queryParameters.put("_total", Collections.singletonList("none"));
queryParameters.put("_include", Collections.singletonList("Patient:general-practitioner"));
SearchUtil.parseQueryParameters(resourceType, queryParameters);
queryParameters.put("_include", Collections.singletonList("Patient:general-practitioner:Practitioner"));
FHIRSearchContext searchContext = SearchUtil.parseQueryParameters(resourceType, queryParameters);

assertNotNull(searchContext);
assertEquals(searchContext.getTotalParameter(), TotalValueSet.NONE);
assertTrue(searchContext.hasIncludeParameters());
assertFalse(searchContext.hasRevIncludeParameters());
assertEquals(searchContext.getIncludeParameters().size(), 1);
assertEquals(searchContext.getIncludeParameters().get(0), incParm);

String selfUri = SearchUtil.buildSearchSelfUri("http://example.com/Patient", searchContext);
assertTrue(selfUri.contains(queryString));
}

@Test(expectedExceptions = FHIRSearchException.class)
Expand Down Expand Up @@ -316,15 +328,26 @@ public void testIncludeValidTargetType() throws Exception {
assertTrue(selfUri.contains(queryString));
}

@Test(expectedExceptions = FHIRSearchException.class)
public void testRevincludeInvalidWithTotal() throws Exception {
@Test
public void testRevIncludeWithTotal() throws Exception {
Map<String, List<String>> queryParameters = new HashMap<>();
Class<Organization> resourceType = Organization.class;

// In strict mode, the query should throw a FHIRSearchException
queryParameters.put("_total", Collections.singletonList("none"));
queryParameters.put("_revinclude", Collections.singletonList("Patient:organization"));
SearchUtil.parseQueryParameters(resourceType, queryParameters);
String queryString = "&_revinclude=Patient:general-practitioner:Organization&_total=estimate";
InclusionParameter revIncParm = new InclusionParameter("Patient", "general-practitioner", "Organization", null, true);

queryParameters.put("_total", Collections.singletonList("estimate"));
queryParameters.put("_revinclude", Collections.singletonList("Patient:general-practitioner:Organization"));
FHIRSearchContext searchContext = SearchUtil.parseQueryParameters(resourceType, queryParameters);
assertNotNull(searchContext);
assertEquals(searchContext.getTotalParameter(), TotalValueSet.ESTIMATE);
assertFalse(searchContext.hasIncludeParameters());
assertTrue(searchContext.hasRevIncludeParameters());
assertEquals(searchContext.getRevIncludeParameters().size(), 1);
assertEquals(searchContext.getRevIncludeParameters().get(0), revIncParm);

String selfUri = SearchUtil.buildSearchSelfUri("http://example.com/Patient", searchContext);
assertTrue(selfUri.contains(queryString));
}

@Test
Expand Down Expand Up @@ -487,7 +510,7 @@ public void testRevIncludeValidTargetType() throws Exception {
}

@Test
public void testRevIncludeUnpsecifiedTargetType() throws Exception {
public void testRevIncludeUnspecifiedTargetType() throws Exception {
Map<String, List<String>> queryParameters = new HashMap<>();
FHIRSearchContext searchContext;
Class<Organization> resourceType = Organization.class;
Expand Down Expand Up @@ -1201,7 +1224,7 @@ public void testMultiIterateIncludeRevinclude() throws Exception {
String include3 = "&_include=Medication:ingredient:Medication";
String include4 = "&_include:iterate=Organization:endpoint:Endpoint";
String include5 = "&_include:iterate=MedicationDispense:destination:Location";

String revinclude1 = "&_revinclude=MedicationDispense:medication:Medication";
String revinclude2 = "&_revinclude=MedicationAdministration:medication:Medication";
String revinclude3 = "&_revinclude:iterate=MedicationStatement:medication:Medication";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -613,25 +613,28 @@ public void testSearchIncludeMultipleMatchAndIncludedResults() {
WebTarget target = getWebTarget();
Response response =
target.path("Procedure")
.queryParam("_total", "none")
.queryParam("date", now.toString())
.queryParam("_include", "Procedure:patient")
.request(FHIRMediaType.APPLICATION_FHIR_JSON)
.get();
assertResponse(response, Response.Status.OK.getStatusCode());
Bundle bundle = response.readEntity(Bundle.class);
final int expectedMatchCount = 3;

assertNotNull(bundle);
assertNull(bundle.getTotal());
assertEquals(4, bundle.getEntry().size());
List<String> matchResourceIds = new ArrayList<>();
for (int i=0; i<bundle.getTotal().getValue(); ++i) {
for (int i=0; i<expectedMatchCount; ++i) {
matchResourceIds.add(bundle.getEntry().get(i).getResource().getId());
assertEquals(SearchEntryMode.MATCH, bundle.getEntry().get(i).getSearch().getMode());
}
assertTrue(matchResourceIds.contains(procedure1Id));
assertTrue(matchResourceIds.contains(procedure2Id));
assertTrue(matchResourceIds.contains(procedure3Id));
List<String> includeResourceIds = new ArrayList<>();
for (int i=bundle.getTotal().getValue(); i<bundle.getEntry().size(); ++i) {
for (int i=expectedMatchCount; i<bundle.getEntry().size(); ++i) {
includeResourceIds.add(bundle.getEntry().get(i).getResource().getId());
assertEquals(SearchEntryMode.INCLUDE, bundle.getEntry().get(i).getSearch().getMode());
}
Expand Down Expand Up @@ -723,6 +726,7 @@ public void testSearchIncludeIteratePrimaryType() {
WebTarget target = getWebTarget();
Response response =
target.path("Patient")
.queryParam("_total", "none")
.queryParam("_tag", tag)
.queryParam("name", "3" + tag)
.queryParam("_include:iterate", "Patient:link")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -785,25 +785,28 @@ public void testSearchRevIncludeMultipleMatchAndIncludedResults() {
WebTarget target = getWebTarget();
Response response =
target.path("Patient")
.queryParam("_total", "none")
.queryParam("_tag", tag)
.queryParam("_revinclude", "Procedure:patient")
.request(FHIRMediaType.APPLICATION_FHIR_JSON)
.get();
assertResponse(response, Response.Status.OK.getStatusCode());
Bundle bundle = response.readEntity(Bundle.class);
final int expectedMatchCount = 3;

assertNotNull(bundle);
assertNull(bundle.getTotal());
assertEquals(7, bundle.getEntry().size());
List<String> matchResourceIds = new ArrayList<>();
for (int i=0; i<bundle.getTotal().getValue(); ++i) {
for (int i=0; i<expectedMatchCount; ++i) {
matchResourceIds.add(bundle.getEntry().get(i).getResource().getId());
assertEquals(SearchEntryMode.MATCH, bundle.getEntry().get(i).getSearch().getMode());
}
assertTrue(matchResourceIds.contains(patient1Id));
assertTrue(matchResourceIds.contains(patient2Id));
assertTrue(matchResourceIds.contains(patient3Id));
List<String> includeResourceIds = new ArrayList<>();
for (int i=bundle.getTotal().getValue(); i<bundle.getEntry().size(); ++i) {
for (int i=expectedMatchCount; i<bundle.getEntry().size(); ++i) {
includeResourceIds.add(bundle.getEntry().get(i).getResource().getId());
assertEquals(SearchEntryMode.INCLUDE, bundle.getEntry().get(i).getSearch().getMode());
}
Expand Down Expand Up @@ -891,6 +894,7 @@ public void testSearchRevIncludeIteratePrimaryType() {
WebTarget target = getWebTarget();
Response response =
target.path("Patient")
.queryParam("_total", "none")
.queryParam("_tag", tag)
.queryParam("_id", patient1Id)
.queryParam("_revinclude:iterate", "Patient:link")
Expand Down

0 comments on commit c55bd65

Please sign in to comment.