diff --git a/fhir-search/pom.xml b/fhir-search/pom.xml index 4f765a3a3f3..158f09bd935 100644 --- a/fhir-search/pom.xml +++ b/fhir-search/pom.xml @@ -32,6 +32,13 @@ jakarta.ws.rs-api provided + + + ${project.groupId} + fhir-ig-us-core + ${project.version} + test + ${project.groupId} fhir-validation diff --git a/fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java b/fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java index 05260f766ff..fba14da6e42 100644 --- a/fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java +++ b/fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java @@ -230,7 +230,7 @@ protected static List getFilteredBuiltinSearchParameters(String // Retrieve the SPs associated with the specified resource type and filter per the filter rules. ParametersMap spMap = spBuiltin.get(resourceType); if (spMap != null && !spMap.isEmpty()) { - result.addAll(filterSearchParameters(filterRules, resourceType, spMap.values())); + result.addAll(filterSearchParameters(filterRules, resourceType, spMap)); } if (!SearchConstants.RESOURCE_RESOURCE.equals(resourceType)) { @@ -238,7 +238,7 @@ protected static List getFilteredBuiltinSearchParameters(String spMap = spBuiltin.get(SearchConstants.RESOURCE_RESOURCE); if (spMap != null && !spMap.isEmpty()) { Collection superParams = - filterSearchParameters(filterRules, SearchConstants.RESOURCE_RESOURCE, spMap.values()); + filterSearchParameters(filterRules, SearchConstants.RESOURCE_RESOURCE, spMap); Map resultCodes = result.stream() .collect(Collectors.toMap(sp -> sp.getCode().getValue(), sp -> sp)); @@ -266,73 +266,72 @@ protected static List getFilteredBuiltinSearchParameters(String * The filter rules are contained in a Map> that is keyed by resource type. * The value of each Map entry is a list of search parameter names that should be included in our filtered result. * - * @param filterRules - * a Map containing filter rules + * @param allFilterRules + * a Map containing filter rules for all resource types * @param resourceType * the resource type associated with each of the unfiltered SearchParameters * @param unfilteredSearchParameters - * the unfiltered Collection of SearchParameter objects + * the ParametersMap of unfiltered SearchParameter objects for this resource type * @return a filtered Collection of SearchParameters * @implSpec This method guarantees that each search parameter returned in the Collection will have a unique code. */ - private static Collection filterSearchParameters(Map> filterRules, - String resourceType, Collection unfilteredSearchParameters) { + private static Collection filterSearchParameters(Map> allFilterRules, + String resourceType, ParametersMap unfilteredSearchParameters) { Map results = new HashMap<>(); // First, retrieve the filter rule (list of SP urls to be included) for the specified resource type. // We know that the SearchParameters in the unfiltered list are all associated with this resource type, // so we can use this same "url list" for each Search Parameter in the unfiltered list. - Map includedSPs = filterRules.get(resourceType); + Map filterRules = allFilterRules.get(resourceType); - if (includedSPs == null) { + if (filterRules == null) { // If the specified resource type wasn't found in the Map then retrieve the wildcard entry if present. - includedSPs = filterRules.get(SearchConstants.WILDCARD_FILTER); + filterRules = allFilterRules.get(SearchConstants.WILDCARD_FILTER); } // If we found a non-empty list of search parameters to filter on, then do the filtering. - if (includedSPs != null && !includedSPs.isEmpty()) { - boolean includeAll = includedSPs.containsKey(SearchConstants.WILDCARD_FILTER); - - // Walk through the unfiltered list and select the ones to be included in our result. - for (SearchParameter sp : unfilteredSearchParameters) { - String code = sp.getCode().getValue(); - String url = sp.getUrl().getValue(); - String version = sp.getVersion() == null ? null : sp.getVersion().getValue(); - String canonical = url + (version == null ? "" : "|" + version); - - if (includedSPs.containsKey(code)) { - String configuredCanonical = includedSPs.get(code); - if (configuredCanonical != null && configuredCanonical.contains("|")) { - if (configuredCanonical.equals(canonical)) { - results.put(code, sp); - } else if (log.isLoggable(Level.FINE)) { - log.fine("Skipping search parameter with id='" + sp.getId() + "'. " - + "Tenant configuration for resource='" + resourceType + "' code='" + code + "' url='" + configuredCanonical - + "' does not match url '" + url + "'"); - } - } else if (configuredCanonical != null) { - if (configuredCanonical.equals(url)) { - results.put(code, sp); - } else if (log.isLoggable(Level.FINE)) { - log.fine("Skipping search parameter with id='" + sp.getId() + "'. " - + "Tenant configuration for resource='" + resourceType + "' code='" + code + "' url='" + configuredCanonical - + "' does not match url '" + url + "'"); - } - } - } else if (includeAll) { - // If "*" is contained in the included SP urls, then include the search parameter - // if it doesn't conflict with any of the previously added parameters - if (!results.containsKey(code)) { - results.put(code, sp); + if (filterRules != null && !filterRules.isEmpty()) { + boolean includeAll = false; + + for (Entry filterRule : filterRules.entrySet()) { + if (SearchConstants.WILDCARD_FILTER.equals(filterRule.getValue())) { + includeAll = true; + } else { + SearchParameter sp = unfilteredSearchParameters.lookupByCanonical(filterRule.getValue()); + if (sp != null) { + results.put(filterRule.getKey(), sp); + } + } + } + + if (includeAll) { + // Add all search parameters that don't conflict with the configured list + for (Entry> spByCode : unfilteredSearchParameters.codeEntries()) { + String code = spByCode.getKey(); + Set spSet = spByCode.getValue(); + + if (results.containsKey(code)) { + log.fine("Skipping search parameters for code '" + code + "' on resource type '" + resourceType + + "' because the configured set contains " + results.get(code)); } else { - SearchParameter configuredSP = results.get(code); - String configuredUrl = configuredSP.getUrl().getValue(); - String configuredVersion = configuredSP.getVersion() == null ? null : configuredSP.getVersion().getValue(); - String configuredCanonical = configuredUrl + (configuredVersion == null ? "" : "|" + configuredVersion); - - log.warning("Skipping search parameter with id='" + sp.getId() + "'. " - + "Found multiple search parameters, '" + configuredCanonical + "' and '" + canonical + "', for code '" + code - + "' on resource type '" + resourceType + "'; use search parameter filtering to disambiguate."); + if (spSet.size() > 1) { + StringBuilder canonicalUrlSetStringBuilder = new StringBuilder("["); + String delim = ""; + for (SearchParameter sp : spSet) { + canonicalUrlSetStringBuilder.append(delim); + canonicalUrlSetStringBuilder.append(sp.getUrl().getValue()); + if (sp.getVersion() != null) { + canonicalUrlSetStringBuilder.append(sp.getVersion().getValue()); + } + delim = ", "; + } + String canonicalUrls = canonicalUrlSetStringBuilder.append("]").toString(); + + log.warning("Found multiple search parameters for code '" + code + + "' on resource type '" + resourceType + "': " + canonicalUrls + + "; use search parameter filtering to disambiguate."); + } + results.put(code, spSet.iterator().next()); } } } @@ -596,15 +595,39 @@ public static SearchParameter getSearchParameter(String resourceType, Canonical if (result != null) { // Check if this search parameter applies to the base Resource type - ResourceType rt = result.getBase().get(0).as(ResourceType.class); + ResourceType rt = result.getBase().get(0); if (SearchConstants.RESOURCE_RESOURCE.equals(rt.getValue())) { resourceType = rt.getValue(); } - Collection filteredResult = - filterSearchParameters(getFilterRules(), resourceType, Collections.singleton(result)); - // If our filtered result is non-empty, then just return the first (and only) item. - result = (filteredResult.isEmpty() ? null : filteredResult.iterator().next()); + Map> allFilterRules = getFilterRules(); + Map filterRules = getFilterRules().get(resourceType); + if (filterRules == null) { + // If the specified resource type wasn't found in the Map then retrieve the wildcard entry if present. + filterRules = allFilterRules.get(SearchConstants.WILDCARD_FILTER); + } + + // If we found a non-empty list of search parameters to filter on, then do the filtering. + if (filterRules != null && !filterRules.isEmpty()) { + boolean includeAll = false; + + for (Entry filterRule : filterRules.entrySet()) { + if (SearchConstants.WILDCARD_FILTER.equals(filterRule.getValue())) { + includeAll = true; + } else if (result.getCode().getValue().equals(filterRule.getKey())) { + String url = result.getUrl().getValue(); + String version = result.getVersion() == null ? null : result.getVersion().getValue(); + String canonical = url + (version == null ? "" : "|" + version); + + // TODO fix this + if (canonical.equals(filterRule.getValue()) || url.equals(filterRule.getValue())) { + break; + } else { + return null; + } + } + } + } } } return result; diff --git a/fhir-search/src/test/java/com/ibm/fhir/search/parameters/ParametersSearchUtilTest.java b/fhir-search/src/test/java/com/ibm/fhir/search/parameters/ParametersSearchUtilTest.java index 2e213796e07..e95a6575142 100644 --- a/fhir-search/src/test/java/com/ibm/fhir/search/parameters/ParametersSearchUtilTest.java +++ b/fhir-search/src/test/java/com/ibm/fhir/search/parameters/ParametersSearchUtilTest.java @@ -7,6 +7,7 @@ package com.ibm.fhir.search.parameters; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotEquals; import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertNotNull; import static org.testng.AssertJUnit.assertTrue; @@ -29,6 +30,7 @@ * Tests the ParametersUtil through the SearchUtil. */ public class ParametersSearchUtilTest extends BaseSearchTest { + public static final boolean DEBUG = true; @Override @BeforeClass @@ -63,7 +65,7 @@ public void testGetSearchParameters2Default() throws Exception { List result = SearchUtil.getApplicableSearchParameters("Patient"); assertNotNull(result); printSearchParameters("testGetSearchParameters2/Patient", result); - assertEquals(35, result.size()); + assertEquals(37, result.size()); result = SearchUtil.getApplicableSearchParameters("Observation"); assertNotNull(result); @@ -146,11 +148,39 @@ public void testGetSearchParameters6Default() throws Exception { List result = SearchUtil.getApplicableSearchParameters("Patient"); assertNotNull(result); printSearchParameters("testGetSearchParameters6/Patient", result); - assertEquals(35, result.size()); + assertEquals(37, result.size()); result = SearchUtil.getApplicableSearchParameters("Device"); assertNotNull(result); printSearchParameters("testGetSearchParameters6/Device", result); assertEquals(18, result.size()); } + + @Test + public void testVersionedSearchParameterFilter() throws Exception { + // Test filtering of search parameters for Patient (default tenant). + FHIRRequestContext.set(new FHIRRequestContext("tenant4")); + + List result = SearchUtil.getApplicableSearchParameters("Device"); + assertNotNull(result); + printSearchParameters("testVersionedSearchParameterFilter/Device", result); + for (SearchParameter sp : result) { + System.out.println(sp.getUrl().getValue() + "|" + sp.getVersion().getValue()); + if ("http://hl7.org/fhir/us/core/SearchParameter/us-core-device-type".equals(sp.getUrl().getValue())) { + assertNotEquals("4.0.0", sp.getVersion()); + } + } + + FHIRRequestContext.set(new FHIRRequestContext("tenant5")); + + result = SearchUtil.getApplicableSearchParameters("Device"); + assertNotNull(result); + printSearchParameters("testVersionedSearchParameterFilter/Device", result); + for (SearchParameter sp : result) { + System.out.println(sp.getUrl().getValue() + "|" + sp.getVersion().getValue()); + if ("http://hl7.org/fhir/us/core/SearchParameter/us-core-device-type".equals(sp.getUrl().getValue())) { + assertNotEquals("3.1.1", sp.getVersion()); + } + } + } } diff --git a/fhir-search/src/test/java/com/ibm/fhir/search/test/ExtractParameterValuesTest.java b/fhir-search/src/test/java/com/ibm/fhir/search/test/ExtractParameterValuesTest.java index ec843a8a865..598dba7cf76 100644 --- a/fhir-search/src/test/java/com/ibm/fhir/search/test/ExtractParameterValuesTest.java +++ b/fhir-search/src/test/java/com/ibm/fhir/search/test/ExtractParameterValuesTest.java @@ -1,5 +1,5 @@ /* - * (C) Copyright IBM Corp. 2019 + * (C) Copyright IBM Corp. 2019, 2021 * * SPDX-License-Identifier: Apache-2.0 */ @@ -131,9 +131,6 @@ public void testFullObservationWithAllSearchParameters() throws Exception { @Test public void testWithEmptyExpressionAndTenant() throws Exception { - // Looking only for built-in search parameters for "Patient". - - // Use tenant1 since it doesn't have any tenant-specific search parameters for resourceType Medication. FHIRRequestContext.set(new FHIRRequestContext("tenant5")); String testFile = "extract/observation-some.json"; @@ -204,8 +201,4 @@ public void testPatientDeceasedTime() throws Exception { runTest(testFile, Patient.class, false, builder.build(), false); } - - - - } diff --git a/fhir-search/src/test/resources/config/tenant4/fhir-server-config.json b/fhir-search/src/test/resources/config/tenant4/fhir-server-config.json index 259628afbbe..c39ebefc551 100644 --- a/fhir-search/src/test/resources/config/tenant4/fhir-server-config.json +++ b/fhir-search/src/test/resources/config/tenant4/fhir-server-config.json @@ -5,8 +5,8 @@ "open": true, "Device": { "searchParameters": { - "patient": "http://hl7.org/fhir/SearchParameter/Device-patient", - "organization": "http://hl7.org/fhir/SearchParameter/Device-organization" + "patient": "http://hl7.org/fhir/us/core/SearchParameter/us-core-device-patient|3.1.1", + "type": "http://hl7.org/fhir/us/core/SearchParameter/us-core-device-type|3.1.1" } }, "Observation": { diff --git a/fhir-search/src/test/resources/config/tenant5/fhir-server-config.json b/fhir-search/src/test/resources/config/tenant5/fhir-server-config.json index 75c7d8186a5..f2a1fe38526 100644 --- a/fhir-search/src/test/resources/config/tenant5/fhir-server-config.json +++ b/fhir-search/src/test/resources/config/tenant5/fhir-server-config.json @@ -5,8 +5,8 @@ "open": true, "Device": { "searchParameters": { - "patient": "http://hl7.org/fhir/SearchParameter/Device-patient", - "organization": "http://hl7.org/fhir/SearchParameter/Device-organization" + "patient": "http://hl7.org/fhir/us/core/SearchParameter/us-core-device-patient", + "type": "http://hl7.org/fhir/us/core/SearchParameter/us-core-device-type|4.0.0" } }, "Observation": {