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 Nov 1, 2021
1 parent 8a9c044 commit 28a54e9
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 90 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
4 changes: 2 additions & 2 deletions fhir-search/src/main/java/com/ibm/fhir/search/parameters/ParametersMap.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2019, 2020
* (C) Copyright IBM Corp. 2019, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -71,7 +71,7 @@ public void insert(String code, SearchParameter parameter) {
canonicalMap.put(url, parameter);

if (version != null) {
String canonical = url + (version == null ? "" : "|" + version);
String canonical = url + "|" + version;
if (canonicalMap.containsKey(canonical)) {
SearchParameter previous = canonicalMap.get(canonical);
if (previous.getExpression() == null || previous.getExpression().equals(parameter.getExpression())) {
Expand Down
152 changes: 90 additions & 62 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("|").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 @@ -402,11 +401,12 @@ private static Map<String, Map<String, String>> getFilterRules() throws Exceptio
}

/**
* Returns the SearchParameter map (keyed by resource type) for the specified
* tenant-id, or null if there are no SearchParameters for the tenant.
* Returns the extended search parameters for the specified tenant-id, the default tenant,
* or null if there are no extended search parameters.
*
* @param tenantId
* the tenant-id whose SearchParameters should be returned.
* @return a map of ParametersMap objects keyed by resource type
* @throws FileNotFoundException
*/
private static Map<String, ParametersMap> getTenantOrDefaultSPMap(String tenantId) throws Exception {
Expand Down Expand Up @@ -579,7 +579,7 @@ public static SearchParameter getSearchParameter(Class<?> resourceType, Canonica
/**
* @param resourceType
* @param uri
* @return the SearchParameter for type {@code resourceType} with url {@code uri} or null if it doesn't exist
* @return the SearchParameter for type {@code resourceType} with canonical url {@code uri} or null if it doesn't exist
* @throws Exception
*/
public static SearchParameter getSearchParameter(String resourceType, Canonical uri) throws Exception {
Expand All @@ -596,15 +596,43 @@ 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();

if (filterRule.getValue().equals(uri.getValue()) || filterRule.getValue().equals(url)) {
return result;
} else {
// Configuration has mapped this code to a different search parameter
return null;
}
}
}

// If we got here, then we havn't found an explicit rule for this search parameter
// so filter out the result if there's no applicable wildcard rule
if (!includeAll) {
result = null;
}
}
}
}
return result;
Expand All @@ -622,13 +650,13 @@ private static SearchParameter getSearchParameterByUrlIfPresent(Map<String, Para
if (spMaps != null && !spMaps.isEmpty()) {
ParametersMap parametersMap = spMaps.get(resourceType);
if (parametersMap != null && !parametersMap.isEmpty()) {
result = parametersMap.lookupByUrl(uri.getValue());
result = parametersMap.lookupByCanonical(uri.getValue());
}

if (result == null) {
parametersMap = spMaps.get(SearchConstants.RESOURCE_RESOURCE);
if (parametersMap != null && !parametersMap.isEmpty()) {
result = parametersMap.lookupByUrl(uri.getValue());
result = parametersMap.lookupByCanonical(uri.getValue());
}
}
}
Expand Down
19 changes: 9 additions & 10 deletions fhir-search/src/test/java/com/ibm/fhir/search/parameters/MultiTenantSearchParameterTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
/*
* (C) Copyright IBM Corp. 2019, 2020
* (C) Copyright IBM Corp. 2019, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/

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.assertNull;
Expand Down Expand Up @@ -74,7 +73,7 @@ public void testGetApplicableSearchParameters3() throws Exception {
List<SearchParameter> result = SearchUtil.getApplicableSearchParameters("Patient");
assertNotNull(result);
printSearchParameters("testGetApplicableSearchParameters3/Patient", result);
assertEquals(35, result.size());
assertEquals(37, result.size());

result = SearchUtil.getApplicableSearchParameters("Observation");
assertNotNull(result);
Expand Down Expand Up @@ -144,7 +143,7 @@ public void testGetApplicableSearchParameters7() throws Exception {
List<SearchParameter> result = SearchUtil.getApplicableSearchParameters("Patient");
assertNotNull(result);
printSearchParameters("testGetApplicableSearchParameters7/Patient", result);
assertEquals(35, result.size());
assertEquals(37, result.size());

result = SearchUtil.getApplicableSearchParameters("Device");
assertNotNull(result);
Expand All @@ -162,7 +161,7 @@ public void testGetSearchParameters2() throws Exception {
List<SearchParameter> result = SearchUtil.getApplicableSearchParameters("Patient");
assertNotNull(result);
printSearchParameters("testGetSearchParameters2", result);
assertEquals(29, result.size());
assertEquals(31, result.size());
}

@Test
Expand Down Expand Up @@ -356,10 +355,10 @@ void testDynamicSearchParameters2() throws Exception {
List<SearchParameter> result = SearchUtil.getApplicableSearchParameters("Patient");
assertNotNull(result);
printSearchParameters("testDynamicSearchParameters2/Patient", result);
assertEquals(35, result.size());
assertEquals(37, result.size());

// Sleep a bit to allow file mod times to register.
Thread.sleep(1000);
Thread.sleep(100);

// Copy our first hidden file into place.
// This should add two tenant-specific search parameters.
Expand All @@ -374,7 +373,7 @@ void testDynamicSearchParameters2() throws Exception {
result = SearchUtil.getApplicableSearchParameters("Patient");
assertNotNull(result);
printSearchParameters("testDynamicSearchParameters2/Patient", result);
assertNotEquals(33, result.size());
assertEquals(33, result.size());

// Sleep a bit to allow file mod times to register.
Thread.sleep(1000);
Expand All @@ -389,7 +388,7 @@ void testDynamicSearchParameters2() throws Exception {
result = SearchUtil.getApplicableSearchParameters("Patient");
assertNotNull(result);
printSearchParameters("testDynamicSearchParameters2/Patient", result);
assertEquals(35, result.size());
assertEquals(37, result.size());
}

@Test
Expand All @@ -400,7 +399,7 @@ public void testGetSearchParametersWithAllResource() throws Exception {
List<SearchParameter> result = SearchUtil.getApplicableSearchParameters("Patient");
assertNotNull(result);
printSearchParameters("testGetSearchParametersWithAllResource", result);
assertEquals(31, result.size());
assertEquals(33, result.size());

// confirm that favorite-number exists as well as the RESOURCE level favorite-color
List<String> codes = result.stream().map(r -> r.getCode().getValue()).collect(Collectors.toList());
Expand Down
Loading

0 comments on commit 28a54e9

Please sign in to comment.