Skip to content

Commit

Permalink
issues #1625 and #1596 - apply search parameter
Browse files Browse the repository at this point in the history
This is a big, end-user-visible change to the way we load search
parameters and apply filtering from fhir-server-config.

Previously, we loaded the built-in search parameters from the registry,
but then applied tenant-specific extension-search-parameters on top of
that.

Now, we have a new ExtensionSearchParametersResourceProvider that reads
the same tenant-specific extension-search-parameters (for backwards
compatibility) and provides those resources to the registry in a
tenant-aware manner.  This is #1596.

Additionally, we used to apply search filtering on each and every
request. Now, the ParametersUtil will loop through all configured
tenants and load their search parameters from the registry during
initialization. This tenant-aware search parameter map will then be
consulted anytime we need a search parameter or set of search parameters
in the context of a particular request (for a particular tenant).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
  • Loading branch information
lmsurpre committed Nov 5, 2021
1 parent b19bda2 commit 9fed60d
Show file tree
Hide file tree
Showing 23 changed files with 483 additions and 744 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -246,38 +246,26 @@ public void clearConfiguration() {
}

/**
* This method returns the list of tenant id's for which a configuration exists.
* This method returns the list of tenant id's for which a configuration directory exists.
* @return
*/
public List<String> getConfiguredTenants() {
log.entering(this.getClass().getName(), "getConfiguredTenants");

try {
List<String> result = new ArrayList<>();

// 'configDir' represents the directory that contains the tenant ids
// Example: "/opt/ibm/fhir-server/wlp/usr/servers/fhir-server/config".
File configDir = new File(getConfigHome() + CONFIG_LOCATION);
log.fine("Listing tenant id's rooted at directory: " + configDir.getName());

// List the directories within 'configDir' that contain a fhir-server-config.json file.
for (File f : configDir.listFiles()) {
// For a directory, let's verify that a config exists within it.
// If yes, then add the name of the directory to the result list, as that
// represents a tenant id.
if (f.isDirectory()) {
File configFile = new File(f, CONFIG_FILE_BASENAME);
if (configFile.exists() && configFile.isFile()) {
result.add(f.getName());
}
}
}
List<String> result = new ArrayList<>();

log.fine("Returning list of tenant ids: " + result.toString());
// 'configDir' represents the directory that contains the tenant ids
// Example: "/opt/ibm/fhir-server/wlp/usr/servers/fhir-server/config".
File configDir = new File(getConfigHome() + CONFIG_LOCATION);
log.fine("Listing tenant id's rooted at directory: " + configDir.getName());

return result;
} finally {
log.exiting(this.getClass().getName(), "getConfiguredTenants");
// List the directories within 'configDir' that contain a fhir-server-config.json file.
for (File f : configDir.listFiles()) {
if (f.isDirectory()) {
result.add(f.getName());
}
}

log.fine("Returning list of tenant ids: " + result.toString());

return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -336,11 +336,12 @@ public void testTenant5() throws Exception {
public void testGetConfiguredTenants() {
List<String> tenants = FHIRConfiguration.getInstance().getConfiguredTenants();
assertNotNull(tenants);
assertEquals(5, tenants.size());
assertEquals(6, tenants.size());
assertTrue(tenants.contains("default"));
assertTrue(tenants.contains("tenant1"));
assertTrue(tenants.contains("tenant2"));
assertTrue(tenants.contains("tenant3"));
assertTrue(tenants.contains("tenant4"));
assertTrue(tenants.contains("tenant5"));
}

Expand Down
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public Collection<Canonical> getProfiles(String type) {
* Get the resource for the given canonical url and resource type
*
* @param url
* the canonical url
* the canonical url (with optional version postfix)
* @param resourceType
* the resource type
* @return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,9 @@ public int hashCode() {
}
}

/**
* @implNote returns the exact string in the version element of the corresponding resource
*/
@Override
public String toString() {
return version;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ public final FHIRRegistryResource getRegistryResource(Class<? extends Resource>
return null;
}

/**
* Return a sorted list of FHIRRegistryResource with the passed canonical url
*
* @param resourceType
* @param url the canonical url for this resource (without version suffix)
* @return a list of FHIRRegistryResources with this url, sorted from low to high by version
*/
protected abstract List<FHIRRegistryResource> getRegistryResources(Class<? extends Resource> resourceType, String url);

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ public interface FHIRRegistryResourceProvider {
* @param resourceType
* the resource type of the registry resource
* @return
* the registry resources from this provider for the given resource type
* the registry resources from this provider for the given resource type; never null
*/
Collection<FHIRRegistryResource> getRegistryResources(Class<? extends Resource> resourceType);

/**
* Get all the registry resources from this provider
*
* @return
* all of the registry resources from this provider
* all of the registry resources from this provider; never null
*/
Collection<FHIRRegistryResource> getRegistryResources();

Expand All @@ -56,7 +56,7 @@ public interface FHIRRegistryResourceProvider {
* @param type
* the constrained resource type
* @return
* the profile resources from this provider that constrain the given resource type
* the profile resources from this provider that constrain the given resource type; never null
*/
Collection<FHIRRegistryResource> getProfileResources(String type);

Expand All @@ -67,7 +67,7 @@ public interface FHIRRegistryResourceProvider {
* @param type
* the search parameter type
* @return
* the search parameter resources from this provider with the given search parameter type
* the search parameter resources from this provider with the given search parameter type; never null
*/
Collection<FHIRRegistryResource> getSearchParameterResources(String type);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public PackageRegistryResourceProvider() {

@Override
protected List<FHIRRegistryResource> getRegistryResources(Class<? extends Resource> resourceType, String url) {
return registryResourceMap.getOrDefault(resourceType, Collections.emptyMap()).getOrDefault(url, Collections.emptyList());
return registryResourceMap.getOrDefault(resourceType, Collections.emptyMap())
.getOrDefault(url, Collections.emptyList());
}

@Override
Expand Down Expand Up @@ -93,6 +94,9 @@ public Collection<FHIRRegistryResource> getSearchParameterResources(String type)
.collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList));
}

/**
* @implNote the list of FHIRRegistryResource is sorted from low to high on version
*/
private Map<Class<? extends Resource>, Map<String, List<FHIRRegistryResource>>> buildRegistryResourceMap() {
Map<Class<? extends Resource>, Map<String, List<FHIRRegistryResource>>> registryResourceMap = new HashMap<>();
for (FHIRRegistryResource registryResource : registryResources) {
Expand Down
24 changes: 6 additions & 18 deletions fhir-search/src/main/java/com/ibm/fhir/search/SearchConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,13 @@ private SearchConstants() {

public static final String LOG_BOUNDARY = "---------------------------------------------------------";

// XML Processing.
public static final String DTM_MANAGER = "com.sun.org.apache.xml.internal.dtm.DTMManager";

public static final String DTM_MANAGER_DEFAULT = "com.sun.org.apache.xml.internal.dtm.ref.DTMManagerDefault";

// Line Separator
public static final String NL = System.getProperty("line.separator");

// Used to find delimiters escaped by '\' so we don't split on them
// @see https://www.hl7.org/fhir/r4/search.html#escaping
public static final String BACKSLASH_NEGATIVE_LOOKBEHIND = "(?<!\\\\)";

public static final String COMPARTMENTS_JSON = "compartments.json";

// Value Types Regex.
public static final String PARAMETER_DELIMITER_REGEX = "\\|";
public static final String COMPONENT_PATH_REGEX = "\\.";
public static final char START_WHERE = '(';

// This constant represents the maximum number of iterations to perform
// for iterative _include and _revinclude parameters.
// In the future, we might want to make this value configurable.
Expand Down Expand Up @@ -103,16 +91,16 @@ private SearchConstants() {

// _security
public static final String SECURITY = "_security";

// _source
public static final String SOURCE = "_source";

// url
public static final String URL = "url";

// version
public static final String VERSION = "version";

public static final String IMPLICIT_SYSTEM_EXT_URL = FHIRConstants.EXT_BASE + "implicit-system";

// Extracted search parameter suffix for :identifier modifier
Expand All @@ -125,7 +113,7 @@ private SearchConstants() {

// Extracted search parameter suffix for :text modifier
public static final String TEXT_MODIFIER_SUFFIX = ":text";

// Extracted search parameter suffixes for canonical values
public static final String CANONICAL_SUFFIX = "_canonical";
public static final String CANONICAL_COMPONENT_URI = "uri";
Expand All @@ -150,7 +138,7 @@ private SearchConstants() {
// Set of whole-system search parameters indexed in global parameter tables
public static final Set<String> SYSTEM_LEVEL_GLOBAL_PARAMETER_NAMES =
Collections.unmodifiableSet(new HashSet<>(Arrays.asList(ID, LAST_UPDATED, PROFILE, SECURITY, SOURCE, TAG)));

// Empty Query String
public static final String EMPTY_QUERY_STRING = "";

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/*
* (C) Copyright IBM Corp. 2021
*
* SPDX-License-Identifier: Apache-2.0
*/
package com.ibm.fhir.search.parameters;

import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;

import com.ibm.fhir.config.FHIRConfiguration;
import com.ibm.fhir.config.FHIRRequestContext;
import com.ibm.fhir.model.resource.Resource;
import com.ibm.fhir.model.resource.SearchParameter;
import com.ibm.fhir.registry.resource.FHIRRegistryResource;
import com.ibm.fhir.registry.spi.AbstractRegistryResourceProvider;
import com.ibm.fhir.search.parameters.cache.TenantSpecificSearchParameterCache;

/**
* A FHIRRegistryResourceProvider that provides SearchParameter resources from tenant configuration files
* (extension-search-parameters.json by default).
*/
public class ExtensionSearchParametersResourceProvider extends AbstractRegistryResourceProvider {
private static final Logger log = Logger.getLogger(ExtensionSearchParametersResourceProvider.class.getName());

private static final String NO_TENANT_SP_MAP_LOGGING =
"No tenant-specific search parameters found for tenant '%s'; trying %s ";

/*
* This is our in-memory cache of SearchParameter objects. The cache is organized at the top level by tenant-id.
* SearchParameters contained in the default tenant's extension-search-parameters.json file are stored under the
* "default" tenant-id, and other tenants' SearchParameters (defined in their tenant-specific
* extension-search-parameters.json files) will be stored under their respective tenant-ids as well. The objects
* stored in our cache are of type CachedObjectHolder, with each one containing a List<SearchParameter>.
*/
private static TenantSpecificSearchParameterCache searchParameterCache = new TenantSpecificSearchParameterCache();

@Override
protected List<FHIRRegistryResource> getRegistryResources(Class<? extends Resource> resourceType, String url) {
if (resourceType != SearchParameter.class || url == null) {
return Collections.emptyList();
}

String tenantId = FHIRRequestContext.get().getTenantId();
return getTenantOrDefaultExtensionSPs(tenantId).stream()
.filter(sp -> url.equals(sp.getUrl().getValue()))
.map(sp -> FHIRRegistryResource.from(sp))
.sorted()
.collect(Collectors.toList());
}

@Override
public Collection<FHIRRegistryResource> getRegistryResources() {
String tenantId = FHIRRequestContext.get().getTenantId();
return getTenantOrDefaultExtensionSPs(tenantId).stream()
.map(sp -> FHIRRegistryResource.from(sp))
.collect(Collectors.toList());
}

@Override
public Collection<FHIRRegistryResource> getRegistryResources(Class<? extends Resource> resourceType) {
if (resourceType != SearchParameter.class) {
return Collections.emptyList();
}

return getRegistryResources();
}

@Override
public Collection<FHIRRegistryResource> getProfileResources(String type) {
return Collections.emptySet();
}

@Override
public Collection<FHIRRegistryResource> getSearchParameterResources(String type) {
Objects.requireNonNull(type);

return getRegistryResources().stream()
.filter(rr -> type.equals(rr.getType()))
.collect(Collectors.toList());
}

/**
* Returns the extended search parameters for the specified tenant,
* the default tenant (if no tenant-specific extension search parameter file exists),
* or an empty list (if there are no extended search parameters or we hit an error while trying to get them).
*
* @param tenantId
* the tenant whose SearchParameters should be returned.
* @return a list of SearchParameter objects
*/
private static List<SearchParameter> getTenantOrDefaultExtensionSPs(String tenantId) {
List<SearchParameter> cachedObjectForTenant = null;

try {
cachedObjectForTenant = searchParameterCache.getCachedObjectForTenant(tenantId);

if (cachedObjectForTenant == null) {

if (log.isLoggable(Level.FINE)) {
log.fine(String.format(NO_TENANT_SP_MAP_LOGGING, tenantId, FHIRConfiguration.DEFAULT_TENANT_ID));
}

cachedObjectForTenant = searchParameterCache.getCachedObjectForTenant(FHIRConfiguration.DEFAULT_TENANT_ID);
}
} catch (Exception e) {
log.log(Level.WARNING, "Error while loading extension search parameters for tenant " + tenantId, e);
}

return cachedObjectForTenant == null ? Collections.emptyList() : cachedObjectForTenant;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,13 @@ public void insert(String code, SearchParameter parameter) {
Objects.requireNonNull(parameter, "cannot insert a null parameter");

String url = parameter.getUrl().getValue();
String version = parameter.getVersion() == null ? null : parameter.getVersion().getValue();
String version = (parameter.getVersion() == null) ? null : parameter.getVersion().getValue();

Set<SearchParameter> previousParams = codeMap.get(code);
if (previousParams != null && previousParams.size() > 0) {
if (log.isLoggable(Level.FINE)) {
log.fine("SearchParameter with code '" + code + "' already exists; adding additional parameter '" + url + "'");
String canonical = (version == null) ? url : url + "|" + version;
log.fine("SearchParameter with code '" + code + "' already exists; adding additional parameter '" + canonical + "'");
}
}
codeMap.computeIfAbsent(code, k -> new HashSet<>()).add(parameter);
Expand All @@ -63,9 +64,10 @@ public void insert(String code, SearchParameter parameter) {
+ "adding additional code '" + code + "'");
}
} else {
String thatVersion = (previous.getVersion() == null) ? null : previous.getVersion().getValue();
log.info("SearchParameter '" + url + "' already exists with a different expression;\n"
+ "replacing [id=" + previous.getId() + ", version=" + previous.getVersion().getValue() + ", expression=" + previous.getExpression().getValue()
+ "] with [id=" + parameter.getId() + ", version=" + parameter.getVersion().getValue() + ", expression=" + parameter.getExpression().getValue() + "]");
+ "replacing [id=" + previous.getId() + ", version=" + thatVersion + ", expression=" + previous.getExpression().getValue()
+ "] with [id=" + parameter.getId() + ", version=" + version + ", expression=" + parameter.getExpression().getValue() + "]");
}
}
canonicalMap.put(url, parameter);
Expand Down
Loading

0 comments on commit 9fed60d

Please sign in to comment.