Issue #1158 - address review comments

Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>

docs/src/pages/Conformance.md

@@ -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` parameter.
+The `_sort` and `_total` parameters cannot be used in combination with the `_include` or `_revinclude` parameters.
 
 The `_contained` and `_containedType` parameters are not supported at this time.
 

fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dto/Resource.java

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2017,2019
+ * (C) Copyright IBM Corp. 2017,2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -21,7 +21,7 @@ public class Resource {
     private long id;
 
     /**
-     *  This is the <resourceType>_RESOURCES.LOGICAL_RESOURCE_ID column. It is only
+     *  This is the <resourceType>_LOGICAL_RESOURCES.LOGICAL_RESOURCE_ID column. It is only
      *  set when this DTO is used to read table data. It is not set when the DTO is
      *  used to insert/update.
      */

fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/impl/FHIRPersistenceJDBCImpl.java

@@ -821,7 +821,7 @@ private List<com.ibm.fhir.persistence.jdbc.dto.Resource> searchForIncludeResourc
         // - Iteration 1 processes against resources returned by primary search or by non-iterative
         //   _include and _revinclude search.
         // - Iteration 2 and above processes only against resources returned by the previous iteration. Note
-        //   that we currently have a max of only one iteration.
+        //   that we currently have a max of only one iteration (not including special iteration 0).
         //
         for (int i=0; i<=SearchConstants.MAX_INCLUSION_ITERATIONS; ++i) {
             // Get the map of resourceTypes for current iteration level
@@ -921,7 +921,12 @@ private List<com.ibm.fhir.persistence.jdbc.dto.Resource> runIncludeQuery(Class<?
         List<com.ibm.fhir.persistence.jdbc.dto.Resource> includeDTOs =
                 resourceDao.search(includeQuery).stream().filter(r -> !allResourceIds.contains(r.getId())).collect(Collectors.toList());
 
-        // Add query result to map
+        // Add query result to map.
+        // The logical resource IDs are pulled from the returned DTOs and saved in a
+        // map of resource type to logical resource IDs. This map is then saved in a
+        // map of iteration # to resource type map.
+        // On subsequent iterations, _include and _revinclude parameters which target
+        // this resource type will use the associated logical resource IDs in their queries.
         if (!includeDTOs.isEmpty()) {
             Set<String> logicalResourceIds = includeDTOs.stream()
                     .map(r -> Long.toString(r.getLogicalResourceId())).collect(Collectors.toSet());

fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregatorFactory.java

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2018,2020
+ * (C) Copyright IBM Corp. 2018,2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */

fhir-search/src/main/java/com/ibm/fhir/search/parameters/InclusionParameter.java

@@ -21,7 +21,7 @@ public class InclusionParameter {
     private boolean userSpecifiedTargetType;
 
     public InclusionParameter(String joinRt, String searchParm, String searchParmTargetType, Modifier modifier,
-        boolean userSpecifiedTargetType) {
+            boolean userSpecifiedTargetType) {
         super();
         this.joinResourceType = joinRt;
         this.searchParameter = searchParm;
@@ -63,7 +63,7 @@ public int hashCode() {
         result = prime * result + ((searchParameter == null) ? 0 : searchParameter.hashCode());
         result = prime * result + ((searchParameterTargetType == null) ? 0 : searchParameterTargetType.hashCode());
         result = prime * result + ((modifier == null) ? 0 : modifier.hashCode());
-        result = prime * result + (userSpecifiedTargetType ? 1 : 0);
+        result = prime * result + Boolean.hashCode(userSpecifiedTargetType);
         return result;
     }
 

fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java

@@ -1581,6 +1581,12 @@ private static SearchConstants.Prefix getPrefix(String s) throws FHIRSearchExcep
         return returnPrefix;
     }
 
+    /**
+     * 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
+     */
     public static boolean isSearchResultParameter(String name) {
         return (SearchConstants.SEARCH_RESULT_PARAMETER_NAMES.contains(name) ||
                 name.startsWith(SearchConstants.INCLUDE + SearchConstants.COLON_DELIMITER_STR) ||
@@ -1901,7 +1907,7 @@ private static QueryParameter parseChainedParameter(Class<?> resourceType, Strin
      * </pre>
      * @formatter:on
      * See the FHIR specification for details:
-     * <a href="https://www.hl7.org/fhir/search.html#has</a>
+     * <a href="https://www.hl7.org/fhir/search.html#has"</a>
      *
      * @param resourceType
      *     Search type.
@@ -2578,7 +2584,7 @@ public static String makeCompositeSubCode(String compositeCode, String subParame
 
     /**
      * 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
      */