Skip to content

Commit

Permalink
VS Expansion Caching (#614)
Browse files Browse the repository at this point in the history
* [APHL-1180] updated implementation

* add cache to constructor

* [APHL-1180] finalize constructors

* [APHL-1180] update tests

* [APHL-1180] code quality

* Update cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/visitor/PackageVisitor.java

Co-authored-by: Brenin Rhodes <[email protected]>

* [APHL-1180] cleanup as per MR feedback

* [APHL-1180] spotless

* Update cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/visitor/BaseKnowledgeArtifactVisitor.java

Co-authored-by: Brenin Rhodes <[email protected]>

---------

Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Brenin Rhodes <[email protected]>
  • Loading branch information
3 people authored Dec 17, 2024
1 parent c44521a commit 3e493f5
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,16 @@
public abstract class BaseKnowledgeArtifactVisitor implements IKnowledgeArtifactVisitor {
String isOwnedUrl = "http://hl7.org/fhir/StructureDefinition/artifact-isOwned";
protected final Repository repository;
protected final Optional<IValueSetExpansionCache> valueSetExpansionCache;

protected BaseKnowledgeArtifactVisitor(Repository repository) {
this.repository = repository;
this.valueSetExpansionCache = Optional.empty();
}

protected BaseKnowledgeArtifactVisitor(Repository repository, IValueSetExpansionCache valueSetExpansionCache) {
this.repository = repository;
this.valueSetExpansionCache = Optional.ofNullable(valueSetExpansionCache);
}

protected FhirContext fhirContext() {
Expand All @@ -47,6 +54,10 @@ protected FhirVersionEnum fhirVersion() {
return fhirContext().getVersion().getVersion();
}

protected Optional<IValueSetExpansionCache> getExpansionCache() {
return valueSetExpansionCache;
}

protected List<IBaseBackboneElement> findArtifactCommentsToUpdate(
IBaseResource artifact, String releaseVersion, Repository repository) {
if (artifact instanceof org.hl7.fhir.dstu3.model.MetadataResource) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.opencds.cqf.fhir.cr.visitor;

import java.util.Optional;
import org.opencds.cqf.fhir.utility.adapter.IKnowledgeArtifactAdapter;
import org.opencds.cqf.fhir.utility.adapter.IValueSetAdapter;

public interface IValueSetExpansionCache {
IValueSetAdapter getExpansionForCanonical(String canonical, String expansionParametersHash);

Optional<String> getExpansionParametersHash(IKnowledgeArtifactAdapter artifactContainingExpansionParameters);

boolean addToCache(IValueSetAdapter expandedValueSet, String expansionParametersHash);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,17 @@ public class PackageVisitor extends BaseKnowledgeArtifactVisitor {
protected Map<String, List<?>> resourceTypes = new HashMap<>();

public PackageVisitor(Repository repository) {
super(repository);
this(repository, null);
}

public PackageVisitor(Repository repository, IValueSetExpansionCache cache) {
super(repository, cache);
terminologyServerClient = new TerminologyServerClient(fhirContext());
expandHelper = new ExpandHelper(this.repository, terminologyServerClient);
setupResourceTypes();
}

public void setupResourceTypes() {
switch (fhirVersion()) {
case DSTU3:
resourceTypes.put(
Expand Down Expand Up @@ -184,15 +192,34 @@ protected void handleValueSets(IBaseBundle packagedBundle, Optional<IBaseResourc
.filter(r -> r.fhirType().equals("ValueSet"))
.map(v -> (IValueSetAdapter) createAdapterForResource(v))
.collect(Collectors.toList());

valueSets.stream()
.forEach(valueSet -> expandHelper.expandValueSet(
var expansionCache = getExpansionCache();
var expansionParamsHash = expansionCache.map(
e -> e.getExpansionParametersHash(rootSpecificationLibrary).orElse(null));
if (expansionCache.isPresent()) {
valueSets.forEach(v -> {
var cachedExpansion = expansionCache
.get()
.getExpansionForCanonical(v.getCanonical(), expansionParamsHash.orElse(null));
if (cachedExpansion != null) {
v.setExpansion(cachedExpansion.getExpansion());
expandedList.add(v.getUrl());
}
});
}
valueSets.forEach(valueSet -> {
if (!expandedList.contains(valueSet.getUrl())) {
expandHelper.expandValueSet(
valueSet,
params,
terminologyEndpoint.map(e -> (IEndpointAdapter) createAdapterForResource(e)),
valueSets,
expandedList,
new Date()));
new Date());
if (expansionCache.isPresent()) {
expansionCache.get().addToCache(valueSet, expansionParamsHash.orElse(null));
}
}
});
}

protected void setCorrectBundleType(Optional<Integer> count, Optional<Integer> offset, IBaseBundle bundle) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.opencds.cqf.fhir.utility.dstu3.Parameters.parameters;
import static org.opencds.cqf.fhir.utility.dstu3.Parameters.part;

Expand Down Expand Up @@ -34,14 +39,19 @@
import org.hl7.fhir.dstu3.model.StringType;
import org.hl7.fhir.dstu3.model.UriType;
import org.hl7.fhir.dstu3.model.ValueSet;
import org.hl7.fhir.dstu3.model.ValueSet.ValueSetExpansionComponent;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.opencds.cqf.fhir.api.Repository;
import org.opencds.cqf.fhir.cr.visitor.IValueSetExpansionCache;
import org.opencds.cqf.fhir.cr.visitor.PackageVisitor;
import org.opencds.cqf.fhir.utility.Constants;
import org.opencds.cqf.fhir.utility.adapter.ILibraryAdapter;
import org.opencds.cqf.fhir.utility.adapter.dstu3.AdapterFactory;
import org.opencds.cqf.fhir.utility.adapter.dstu3.LibraryAdapter;
import org.opencds.cqf.fhir.utility.adapter.dstu3.ValueSetAdapter;
import org.opencds.cqf.fhir.utility.repository.InMemoryFhirRepository;

class PackageVisitorTests {
Expand Down Expand Up @@ -413,4 +423,42 @@ void packageOperation_should_respect_include() {
}
}
}

@Test
public void packageVisitorShouldUseExpansionCacheIfProvided() {
// Arrange
var bundle = (Bundle) jsonParser.parseResource(
PackageVisitorTests.class.getResourceAsStream("Bundle-ersd-small-active.json"));
repo.transaction(bundle);
var library = repo.read(Library.class, new IdType("Library/SpecificationLibrary"))
.copy();
var libraryAdapter = new AdapterFactory().createLibrary(library);
var mockCache = Mockito.mock(IValueSetExpansionCache.class);
var packageVisitor = new PackageVisitor(repo, mockCache);

var canonical1 = "http://cts.nlm.nih.gov/fhir/ValueSet/123-this-will-be-routine|20210526";
var mockValueSetAdapter1 = Mockito.mock(ValueSetAdapter.class);
when(mockValueSetAdapter1.getExpansion()).thenReturn(new ValueSetExpansionComponent());
when(mockValueSetAdapter1.getCanonical()).thenReturn(canonical1);

when(mockCache.getExpansionForCanonical(canonical1, null)).thenReturn(mockValueSetAdapter1);

// Act
var params = parameters();
libraryAdapter.accept(packageVisitor, params);

// Assert
// there are three ValueSets being expanded and only the first one is in the cache
verify(mockCache, times(1)).getExpansionForCanonical(canonical1, null);

verify(mockCache, times(1))
.getExpansionForCanonical(
"http://cts.nlm.nih.gov/fhir/ValueSet/2.16.840.1.113762.1.4.1146.6|20210526", null);
verify(mockCache, times(1))
.getExpansionForCanonical("http://ersd.aimsplatform.org/fhir/ValueSet/dxtc|2022-11-19", null);

// the other two will be added to the cache after expansion if possible
verify(mockCache, times(2)).addToCache(any(), isNull());
verify(mockCache, times(1)).getExpansionParametersHash(any(LibraryAdapter.class));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.opencds.cqf.fhir.utility.r4.Parameters.parameters;
import static org.opencds.cqf.fhir.utility.r4.Parameters.part;

Expand Down Expand Up @@ -34,14 +39,19 @@
import org.hl7.fhir.r4.model.ResourceType;
import org.hl7.fhir.r4.model.StringType;
import org.hl7.fhir.r4.model.ValueSet;
import org.hl7.fhir.r4.model.ValueSet.ValueSetExpansionComponent;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.opencds.cqf.fhir.api.Repository;
import org.opencds.cqf.fhir.cr.visitor.IValueSetExpansionCache;
import org.opencds.cqf.fhir.cr.visitor.PackageVisitor;
import org.opencds.cqf.fhir.utility.Constants;
import org.opencds.cqf.fhir.utility.adapter.ILibraryAdapter;
import org.opencds.cqf.fhir.utility.adapter.r4.AdapterFactory;
import org.opencds.cqf.fhir.utility.adapter.r4.LibraryAdapter;
import org.opencds.cqf.fhir.utility.adapter.r4.ValueSetAdapter;
import org.opencds.cqf.fhir.utility.repository.InMemoryFhirRepository;

class PackageVisitorTests {
Expand Down Expand Up @@ -407,4 +417,42 @@ void packageOperation_should_respect_include() {
}
}
}

@Test
public void packageVisitorShouldUseExpansionCacheIfProvided() {
// Arrange
var bundle = (Bundle) jsonParser.parseResource(
PackageVisitorTests.class.getResourceAsStream("Bundle-ersd-small-active.json"));
repo.transaction(bundle);
var library = repo.read(Library.class, new IdType("Library/SpecificationLibrary"))
.copy();
var libraryAdapter = new AdapterFactory().createLibrary(library);
var mockCache = Mockito.mock(IValueSetExpansionCache.class);
var packageVisitor = new PackageVisitor(repo, mockCache);

var canonical1 = "http://cts.nlm.nih.gov/fhir/ValueSet/123-this-will-be-routine|20210526";
var mockValueSetAdapter1 = Mockito.mock(ValueSetAdapter.class);
when(mockValueSetAdapter1.getExpansion()).thenReturn(new ValueSetExpansionComponent());
when(mockValueSetAdapter1.getCanonical()).thenReturn(canonical1);

when(mockCache.getExpansionForCanonical(canonical1, null)).thenReturn(mockValueSetAdapter1);

// Act
var params = parameters();
libraryAdapter.accept(packageVisitor, params);

// Assert
// there are three ValueSets being expanded and only the first one is in the cache
verify(mockCache, times(1)).getExpansionForCanonical(canonical1, null);

verify(mockCache, times(1))
.getExpansionForCanonical(
"http://cts.nlm.nih.gov/fhir/ValueSet/2.16.840.1.113762.1.4.1146.6|20210526", null);
verify(mockCache, times(1))
.getExpansionForCanonical("http://ersd.aimsplatform.org/fhir/ValueSet/dxtc|2022-11-19", null);

// the other two will be added to the cache after expansion if possible
verify(mockCache, times(2)).addToCache(any(), isNull());
verify(mockCache, times(1)).getExpansionParametersHash(any(LibraryAdapter.class));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.opencds.cqf.fhir.utility.r5.Parameters.parameters;
import static org.opencds.cqf.fhir.utility.r5.Parameters.part;

Expand Down Expand Up @@ -34,14 +39,19 @@
import org.hl7.fhir.r5.model.ResourceType;
import org.hl7.fhir.r5.model.StringType;
import org.hl7.fhir.r5.model.ValueSet;
import org.hl7.fhir.r5.model.ValueSet.ValueSetExpansionComponent;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.opencds.cqf.fhir.api.Repository;
import org.opencds.cqf.fhir.cr.visitor.IValueSetExpansionCache;
import org.opencds.cqf.fhir.cr.visitor.PackageVisitor;
import org.opencds.cqf.fhir.utility.Constants;
import org.opencds.cqf.fhir.utility.adapter.ILibraryAdapter;
import org.opencds.cqf.fhir.utility.adapter.r5.AdapterFactory;
import org.opencds.cqf.fhir.utility.adapter.r5.LibraryAdapter;
import org.opencds.cqf.fhir.utility.adapter.r5.ValueSetAdapter;
import org.opencds.cqf.fhir.utility.repository.InMemoryFhirRepository;

class PackageVisitorTests {
Expand Down Expand Up @@ -414,4 +424,42 @@ void packageOperation_should_respect_include() {
}
}
}

@Test
public void packageVisitorShouldUseExpansionCacheIfProvided() {
// Arrange
var bundle = (Bundle) jsonParser.parseResource(
PackageVisitorTests.class.getResourceAsStream("Bundle-ersd-small-active.json"));
repo.transaction(bundle);
var library = repo.read(Library.class, new IdType("Library/SpecificationLibrary"))
.copy();
var libraryAdapter = new AdapterFactory().createLibrary(library);
var mockCache = Mockito.mock(IValueSetExpansionCache.class);
var packageVisitor = new PackageVisitor(repo, mockCache);

var canonical1 = "http://cts.nlm.nih.gov/fhir/ValueSet/123-this-will-be-routine|20210526";
var mockValueSetAdapter1 = Mockito.mock(ValueSetAdapter.class);
when(mockValueSetAdapter1.getExpansion()).thenReturn(new ValueSetExpansionComponent());
when(mockValueSetAdapter1.getCanonical()).thenReturn(canonical1);

when(mockCache.getExpansionForCanonical(canonical1, null)).thenReturn(mockValueSetAdapter1);

// Act
var params = parameters();
libraryAdapter.accept(packageVisitor, params);

// Assert
// there are three ValueSets being expanded and only the first one is in the cache
verify(mockCache, times(1)).getExpansionForCanonical(canonical1, null);

verify(mockCache, times(1))
.getExpansionForCanonical(
"http://cts.nlm.nih.gov/fhir/ValueSet/2.16.840.1.113762.1.4.1146.6|20210526", null);
verify(mockCache, times(1))
.getExpansionForCanonical("http://ersd.aimsplatform.org/fhir/ValueSet/dxtc|2022-11-19", null);

// the other two will be added to the cache after expansion if possible
verify(mockCache, times(2)).addToCache(any(), isNull());
verify(mockCache, times(1)).getExpansionParametersHash(any(LibraryAdapter.class));
}
}

0 comments on commit 3e493f5

Please sign in to comment.