Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding cleaning up of the resources owned by the terminology service. #1391

Merged
merged 8 commits into from
Jun 26, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package au.csiro.pathling.fhir;

import au.csiro.pathling.utilities.ResourceCloser;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.rest.gclient.IOperationUntypedWithInput;
import javax.annotation.Nonnull;
Expand All @@ -29,16 +30,19 @@
import org.hl7.fhir.r4.model.StringType;
import org.hl7.fhir.r4.model.UriType;
import org.hl7.fhir.r4.model.ValueSet;
import java.io.Closeable;

/**
* An implementation of {@link TerminologyClient} that uses cacheable GET requests.
*/
class DefaultTerminologyClient implements TerminologyClient {
class DefaultTerminologyClient extends ResourceCloser implements TerminologyClient {

@Nonnull
final IGenericClient fhirClient;

DefaultTerminologyClient(@Nonnull final IGenericClient fhirClient) {
DefaultTerminologyClient(@Nonnull final IGenericClient fhirClient, @Nonnull final Closeable...
resourcesToAdopt) {
super(resourcesToAdopt);
this.fhirClient = fhirClient;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,15 @@
import org.hl7.fhir.r4.model.ValueSet;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.Closeable;

/**
* The client interface to FHIR terminology operations.
*
* @author John Grimes
* @author Piotr Szul
*/
public interface TerminologyClient {
public interface TerminologyClient extends Closeable {

Logger log = LoggerFactory.getLogger(TerminologyClient.class);

Expand All @@ -56,8 +57,7 @@ public interface TerminologyClient {
* @param version the version of the code system to validate against
* @param code the code to validate
* @return a {@link Parameters} resource
* @see <a
* href="https://www.hl7.org/fhir/R4/valueset-operation-validate-code.html">ValueSet/$validate-code</a>
* @see <a href="https://www.hl7.org/fhir/R4/valueset-operation-validate-code.html">ValueSet/$validate-code</a>
*/
@Operation(name = "$validate-code", type = ValueSet.class, idempotent = true)
@Nonnull
Expand All @@ -76,8 +76,7 @@ Parameters validateCode(
* @param version the version of the code system to validate against
* @param code the code to validate
* @return an {@link IOperationUntypedWithInput} that can be customized and executed later
* @see <a
* href="https://www.hl7.org/fhir/R4/valueset-operation-validate-code.html">ValueSet/$validate-code</a>
* @see <a href="https://www.hl7.org/fhir/R4/valueset-operation-validate-code.html">ValueSet/$validate-code</a>
*/
IOperationUntypedWithInput<Parameters> buildValidateCode(@Nonnull UriType url,
@Nonnull UriType system,
Expand All @@ -91,8 +90,7 @@ IOperationUntypedWithInput<Parameters> buildValidateCode(@Nonnull UriType url,
* @param reverse if true, the translation will be reversed
* @param target the URL of the value set within which the translation is sought
* @return a {@link Parameters} resource
* @see <a
* href="https://www.hl7.org/fhir/R4/operation-conceptmap-translate.html">ConceptMap/$translate</a>
* @see <a href="https://www.hl7.org/fhir/R4/operation-conceptmap-translate.html">ConceptMap/$translate</a>
*/
@Operation(name = "$translate", type = CodeSystem.class, idempotent = true)
@Nonnull
Expand All @@ -115,8 +113,7 @@ Parameters translate(
* @param reverse if true, the translation will be reversed
* @param target the URL of the value set within which the translation is sought
* @return an {@link IOperationUntypedWithInput} that can be customized and executed later
* @see <a
* href="https://www.hl7.org/fhir/R4/operation-conceptmap-translate.html">ConceptMap/$translate</a>
* @see <a href="https://www.hl7.org/fhir/R4/operation-conceptmap-translate.html">ConceptMap/$translate</a>
*/
@Nonnull
IOperationUntypedWithInput<Parameters> buildTranslate(@Nonnull UriType url,
Expand All @@ -130,8 +127,7 @@ IOperationUntypedWithInput<Parameters> buildTranslate(@Nonnull UriType url,
* @param system the system of the codes being tested
* @param version the version of the code system that the codes are from
* @return a {@link Parameters} resource
* @see <a
* href="https://www.hl7.org/fhir/R4/codesystem-operation-subsumes.html">CodeSystem/$subsumes</a>
* @see <a href="https://www.hl7.org/fhir/R4/codesystem-operation-subsumes.html">CodeSystem/$subsumes</a>
*/
@Operation(name = "$subsumes", type = CodeSystem.class, idempotent = true)
@Nonnull
Expand All @@ -150,8 +146,7 @@ Parameters subsumes(
* @param system the system of the codes being tested
* @param version the version of the code system that the codes are from
* @return an {@link IOperationUntypedWithInput} that can be customized and executed later
* @see <a
* href="https://www.hl7.org/fhir/R4/codesystem-operation-subsumes.html">CodeSystem/$subsumes</a>
* @see <a href="https://www.hl7.org/fhir/R4/codesystem-operation-subsumes.html">CodeSystem/$subsumes</a>
*/
@Nonnull
IOperationUntypedWithInput<Parameters> buildSubsumes(@Nonnull CodeType codeA,
Expand All @@ -163,8 +158,7 @@ IOperationUntypedWithInput<Parameters> buildSubsumes(@Nonnull CodeType codeA,
* @param code the code to lookup
* @param property the property or properties to be returned in the response
* @return a {@link Parameters} resource
* @see <a
* href="https://www.hl7.org/fhir/R4/codesystem-operation-lookup.html">CodeSystem/$lookup</a>
* @see <a href="https://www.hl7.org/fhir/R4/codesystem-operation-lookup.html">CodeSystem/$lookup</a>
*/
@Operation(name = "$lookup", type = CodeSystem.class, idempotent = true)
@Nonnull
Expand All @@ -183,8 +177,7 @@ Parameters lookup(
* @param code the code to lookup
* @param property the property or properties to be returned in the response
* @return an {@link IOperationUntypedWithInput} that can be customized and executed later
* @see <a
* href="https://www.hl7.org/fhir/R4/codesystem-operation-lookup.html">CodeSystem/$lookup</a>
* @see <a href="https://www.hl7.org/fhir/R4/codesystem-operation-lookup.html">CodeSystem/$lookup</a>
*/
@Nonnull
IOperationUntypedWithInput<Parameters> buildLookup(@Nonnull UriType system,
Expand Down Expand Up @@ -223,9 +216,13 @@ static TerminologyClient build(@Nonnull final FhirContext fhirContext,
// sending them.
final TerminologyAuthConfiguration authConfig = terminologyConfiguration.getAuthentication();
if (authConfig.isEnabled()) {
genericClient.registerInterceptor(new ClientAuthInterceptor(authConfig));
final ClientAuthInterceptor clientAuthInterceptor = new ClientAuthInterceptor(authConfig);
genericClient.registerInterceptor(clientAuthInterceptor);
// pass the client auth interceptor as a resource to close when the client is closed
return new DefaultTerminologyClient(genericClient, clientAuthInterceptor);
} else {
return new DefaultTerminologyClient(genericClient);
}
return new DefaultTerminologyClient(genericClient);
}

@Nonnull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@

package au.csiro.pathling.terminology;

import static java.util.Objects.nonNull;

import au.csiro.pathling.fhir.TerminologyClient;
import au.csiro.pathling.utilities.ResourceCloser;
import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
import java.io.Closeable;
import java.io.IOException;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

Expand All @@ -31,18 +29,15 @@
*
* @author John Grimes
*/
public abstract class BaseTerminologyService implements TerminologyService, Closeable {
public abstract class BaseTerminologyService extends ResourceCloser implements TerminologyService {

@Nonnull
protected final TerminologyClient terminologyClient;

@Nullable
protected final Closeable toClose;

public BaseTerminologyService(@Nonnull final TerminologyClient terminologyClient,
@Nullable final Closeable toClose) {
@Nonnull final Closeable... resourcesToClose) {
super(resourcesToClose);
this.terminologyClient = terminologyClient;
this.toClose = toClose;
}

/**
Expand All @@ -64,12 +59,4 @@ public static <ResultType> ResultType handleError(@Nonnull final BaseServerRespo
throw e;
}
}

@Override
public void close() throws IOException {
if (nonNull(toClose)) {
toClose.close();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@
public class DefaultTerminologyService extends BaseTerminologyService {

public DefaultTerminologyService(@Nonnull final TerminologyClient terminologyClient,
@Nullable final Closeable toClose) {
super(terminologyClient, toClose);
@Nonnull final Closeable... resourcesToClose) {
super(terminologyClient, resourcesToClose);
}

@Override
public boolean validateCode(@Nonnull final String valueSetUrl, @Nonnull final Coding coding) {
final ValidateCodeParameters parameters = new ValidateCodeParameters(valueSetUrl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,21 @@ private TerminologyService createService() {
// If caching is enabled and storage type is disk, use a persistent caching terminology
// service implementation.
log.debug("Creating PersistentCachingTerminologyService with cache config: {}", cacheConfig);
return new PersistentCachingTerminologyService(terminologyClient, httpClient, cacheConfig);
return new PersistentCachingTerminologyService(terminologyClient, cacheConfig, httpClient,
terminologyClient);

} else if (cacheConfig.isEnabled() && cacheConfig.getStorageType().equals(
HttpClientCachingStorageType.MEMORY)) {
// If caching is enabled and storage type is memory, use an in-memory caching terminology
// service implementation.
log.debug("Creating InMemoryCachingTerminologyService with cache config: {}", cacheConfig);
return new InMemoryCachingTerminologyService(terminologyClient, httpClient, cacheConfig);
return new InMemoryCachingTerminologyService(terminologyClient, cacheConfig, httpClient,
terminologyClient);

} else {
// If caching is disabled, use a terminology service implementation that does not cache.
log.debug("Creating DefaultTerminologyService with no caching");
return new DefaultTerminologyService(terminologyClient, httpClient);
return new DefaultTerminologyService(terminologyClient, httpClient, terminologyClient);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,12 @@ public abstract class CachingTerminologyService extends BaseTerminologyService {

@SuppressWarnings("unchecked")
public CachingTerminologyService(@Nonnull final TerminologyClient terminologyClient,
@Nullable final Closeable toClose,
@Nonnull final HttpClientCachingConfiguration configuration) {
super(terminologyClient, toClose);
@Nonnull final HttpClientCachingConfiguration configuration,
@Nonnull final Closeable... resourcesToClose) {
super(terminologyClient, resourcesToClose);
this.configuration = configuration;
cacheManager = buildCacheManager();
// register manager as a closeable resource
cacheManager = registerResource(buildCacheManager());
validateCodeCache = (Cache<Integer, TerminologyResult<Boolean>>) buildCache(cacheManager,
VALIDATE_CODE_CACHE_NAME);
subsumesCache = (Cache<Integer, TerminologyResult<ConceptSubsumptionOutcome>>) buildCache(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import au.csiro.pathling.fhir.TerminologyClient;
import java.io.Closeable;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.infinispan.Cache;
import org.infinispan.configuration.cache.Configuration;
import org.infinispan.configuration.cache.ConfigurationBuilder;
Expand All @@ -37,9 +36,9 @@
public class InMemoryCachingTerminologyService extends CachingTerminologyService {

public InMemoryCachingTerminologyService(@Nonnull final TerminologyClient terminologyClient,
@Nullable final Closeable toClose,
@Nonnull final HttpClientCachingConfiguration configuration) {
super(terminologyClient, toClose, configuration);
@Nonnull final HttpClientCachingConfiguration configuration,
@Nonnull final Closeable... resourcesToClose) {
super(terminologyClient, configuration, resourcesToClose);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import java.io.Closeable;
import java.nio.file.Path;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import lombok.extern.slf4j.Slf4j;
import org.infinispan.Cache;
import org.infinispan.commons.marshall.JavaSerializationMarshaller;
import org.infinispan.configuration.cache.Configuration;
Expand All @@ -42,15 +42,16 @@
*
* @author John Grimes
*/
@Slf4j
public class PersistentCachingTerminologyService extends CachingTerminologyService {

private static final String DATA_DIRECTORY = "data";
private static final String INDEX_DIRECTORY = "index";

public PersistentCachingTerminologyService(@Nonnull final TerminologyClient terminologyClient,
@Nullable final Closeable toClose,
@Nonnull final HttpClientCachingConfiguration configuration) {
super(terminologyClient, toClose, configuration);
@Nonnull final HttpClientCachingConfiguration configuration,
@Nonnull final Closeable... resourcesToClose) {
super(terminologyClient, configuration, resourcesToClose);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private static Parameters translation(@Nonnull final Translation... entries) {
void setUp() {
terminologyClient = mock(TerminologyClient.class);
terminologyService = new DefaultTerminologyService(
terminologyClient, null);
terminologyClient);
}

@Test
Expand Down
12 changes: 8 additions & 4 deletions utilities/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@
<artifactId>scala-library</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
Expand All @@ -84,13 +88,13 @@
<scope>test</scope>
</dependency>
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
Expand Down
Loading
Loading