Skip to content

Commit

Permalink
issue #2843 - added test for config-based sp version filtering
Browse files Browse the repository at this point in the history 
This changeset introduces the fhir-ig-us-core as a test dependency for
fhir-search. This was strictly out of convenience...the alternative
would be to create a new test-only PackagedRegistryResourceProvider that
provides multiple versions of the same search parameter(s).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
  • Loading branch information
@lmsurpre
lmsurpre committed Oct 29, 2021
1 parent 8a9c044 commit a7af948
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 71 deletions.
7 changes: 7 additions & 0 deletions fhir-search/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@
<artifactId>jakarta.ws.rs-api</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<!-- Strictly for testing with search parameters loaded from new providers -->
<groupId>${project.groupId}</groupId>
<artifactId>fhir-ig-us-core</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>fhir-validation</artifactId>
Expand Down
137 changes: 80 additions & 57 deletions fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,15 +230,15 @@ protected static List<SearchParameter> 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)) {
// Retrieve the SPs associated with the "Resource" resource type and filter per the filter rules.
spMap = spBuiltin.get(SearchConstants.RESOURCE_RESOURCE);
if (spMap != null && !spMap.isEmpty()) {
Collection<SearchParameter> superParams =
filterSearchParameters(filterRules, SearchConstants.RESOURCE_RESOURCE, spMap.values());
filterSearchParameters(filterRules, SearchConstants.RESOURCE_RESOURCE, spMap);
Map<String, SearchParameter> resultCodes = result.stream()
.collect(Collectors.toMap(sp -> sp.getCode().getValue(), sp -> sp));

Expand Down Expand Up @@ -266,73 +266,72 @@ protected static List<SearchParameter> getFilteredBuiltinSearchParameters(String
* The filter rules are contained in a Map<String, List<String>> 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<SearchParameter> filterSearchParameters(Map<String, Map<String, String>> filterRules,
String resourceType, Collection<SearchParameter> unfilteredSearchParameters) {
private static Collection<SearchParameter> filterSearchParameters(Map<String, Map<String, String>> allFilterRules,
String resourceType, ParametersMap unfilteredSearchParameters) {
Map<String, SearchParameter> 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<String, String> includedSPs = filterRules.get(resourceType);
Map<String, String> 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<String, String> 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<String, Set<SearchParameter>> spByCode : unfilteredSearchParameters.codeEntries()) {
String code = spByCode.getKey();
Set<SearchParameter> 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());
}
}
}
Expand Down Expand Up @@ -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<SearchParameter> 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<String, Map<String, String>> allFilterRules = getFilterRules();
Map<String, String> 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<String, String> 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;
Expand Down
34 changes: 32 additions & 2 deletions fhir-search/src/test/java/com/ibm/fhir/search/parameters/ParametersSearchUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,6 +30,7 @@
* Tests the ParametersUtil through the SearchUtil.
*/
public class ParametersSearchUtilTest extends BaseSearchTest {
public static final boolean DEBUG = true;

@Override
@BeforeClass
Expand Down Expand Up @@ -63,7 +65,7 @@ public void testGetSearchParameters2Default() throws Exception {
List<SearchParameter> result = SearchUtil.getApplicableSearchParameters("Patient");
assertNotNull(result);
printSearchParameters("testGetSearchParameters2/Patient", result);
assertEquals(35, result.size());
assertEquals(37, result.size());

result = SearchUtil.getApplicableSearchParameters("Observation");
assertNotNull(result);
Expand Down Expand Up @@ -146,11 +148,39 @@ public void testGetSearchParameters6Default() throws Exception {
List<SearchParameter> 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<SearchParameter> 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());
}
}
}
}
9 changes: 1 addition & 8 deletions fhir-search/src/test/java/com/ibm/fhir/search/test/ExtractParameterValuesTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2019
* (C) Copyright IBM Corp. 2019, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -204,8 +201,4 @@ public void testPatientDeceasedTime() throws Exception {

runTest(testFile, Patient.class, false, builder.build(), false);
}




}
4 changes: 2 additions & 2 deletions fhir-search/src/test/resources/config/tenant4/fhir-server-config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
4 changes: 2 additions & 2 deletions fhir-search/src/test/resources/config/tenant5/fhir-server-config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down

0 comments on commit a7af948

Please sign in to comment.