Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,14 @@

gatherDependenciesWithClassification(adapter, new ArrayList<>(), relatedArtifacts, ctx);

// Surface key bindings the dependency walk missed. Some bindings (e.g., on
// sliced elements whose binding lives on a complex element type) aren't
// reachable through differential + baseDefinition walking, but the snapshot-based
// KeyElementAnalyzer correctly identifies them. Promote each transitiveKeyCanonical
// not already in relatedArtifacts to a synthetic depends-on entry with role=key,
// matching what the IG-publisher's Key Elements Table would show.
synthesizeMissingKeyDependencies(relatedArtifacts, transitiveKeyCanonicals, conformanceResolver);

// Add root IG as a composed-of entry
if (adapter.get() instanceof org.hl7.fhir.r4.model.ImplementationGuide
|| adapter.get() instanceof org.hl7.fhir.r5.model.ImplementationGuide) {
Expand Down Expand Up @@ -281,6 +289,17 @@
IKnowledgeArtifactAdapter dependencyAdapter,
ConformanceResourceResolver conformanceResolver) {
if (dependencyAdapter != null) {
var url = dependencyAdapter.getUrl();
// External CodeSystems shipped as "stub" entries (CodeSystem.content = not-present)
// carry a version field that reflects the publishing package, not the actual
// terminology version. NpmRepository excludes these from the version index, so a
// present URL with a missing indexed version identifies a stub. Skip the pin.
if (conformanceResolver != null
&& url != null
&& conformanceResolver.getResourceType(url) != null
&& conformanceResolver.getVersion(url) == null) {
return url;
}
return dependencyAdapter.getCanonical();
}
var reference = dependency.getReference();
Expand Down Expand Up @@ -424,6 +443,53 @@
}
}

@SuppressWarnings("unchecked")
private <T extends ICompositeType & IBaseHasExtensions> void synthesizeMissingKeyDependencies(

Check failure on line 447 in cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/visitor/DataRequirementsVisitor.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 18 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=cqframework_clinical-reasoning&issues=AZ4dFIIKQ-ridzDd7jva&open=AZ4dFIIKQ-ridzDd7jva&pullRequest=1032
List<T> relatedArtifacts,
Set<String> transitiveKeyCanonicals,
ConformanceResourceResolver conformanceResolver) {
if (transitiveKeyCanonicals == null || transitiveKeyCanonicals.isEmpty()) {
return;
}

var existingUrls = relatedArtifacts.stream()
.map(IKnowledgeArtifactAdapter::getRelatedArtifactReference)
.filter(StringUtils::isNotBlank)
.map(Canonicals::getUrl)
.collect(Collectors.toSet());

for (var keyCanonical : transitiveKeyCanonicals) {
var url = Canonicals.getUrl(keyCanonical);
if (url == null || existingUrls.contains(url)) {
continue;
}

var indexedVersion = conformanceResolver != null ? conformanceResolver.getVersion(url) : null;
var enrichedRef = indexedVersion != null ? url + "|" + indexedVersion : url;

var newDep = (T) IKnowledgeArtifactAdapter.newRelatedArtifact(
fhirVersion(), Constants.RELATEDARTIFACT_TYPE_DEPENDSON, enrichedRef, null);

var extensionList = (List<IBaseExtension<?, ?>>) newDep.getExtension();
extensionList.add(ExtensionBuilders.buildDependencyRoleExt(fhirVersion(), "key"));

if (conformanceResolver != null) {
var resourceType = conformanceResolver.getResourceType(url);
if (resourceType != null) {
addResourceTypeExtension(resourceType, newDep);
}
var pkgInfo = conformanceResolver.getPackageInfo(url);
if (pkgInfo != null) {
extensionList.add(ExtensionBuilders.buildComplexPackageSourceExt(
fhirVersion(), pkgInfo.packageId(), pkgInfo.version(), pkgInfo.canonical()));
}
}

relatedArtifacts.add(newDep);
existingUrls.add(url);
}
}

private void addResourceTypeExtension(String resourceType, ICompositeType relatedArtifact) {
if (fhirVersion().equals(FhirVersionEnum.R4)) {
((org.hl7.fhir.r4.model.RelatedArtifact) relatedArtifact)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,32 @@ public static List<String> classifyDependencyRoles(
/**
* Checks if a dependency is in the set of transitive key canonicals discovered
* via ValueSet compose chain walking.
* <p>
* Normalizes both sides on version: dependency references and key-set entries are
* compared on URL only. The set may contain version-pinned URLs (FHIR core writes
* bindings inconsistently — e.g. {@code publication-status|4.0.1} pinned but
* {@code library-type} not), and dep references are version-stripped at lookup, so
* symmetric stripping is required to avoid spurious misses.
*/
private static boolean isTransitiveKeyDependency(IDependencyInfo dependency, Set<String> transitiveKeyCanonicals) {
if (transitiveKeyCanonicals == null || transitiveKeyCanonicals.isEmpty()) {
return false;
}
var canonical = getDependencyCanonical(dependency.getReference());
return canonical != null && transitiveKeyCanonicals.contains(canonical);
if (canonical == null) {
return false;
}
// Fast path: exact URL hit (covers entries that are already version-stripped).
if (transitiveKeyCanonicals.contains(canonical)) {
return true;
}
// Fallback: strip versions on the key-set side and compare URLs.
for (var keyEntry : transitiveKeyCanonicals) {
if (canonical.equals(getDependencyCanonical(keyEntry))) {
return true;
}
}
return false;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,4 +524,46 @@ void testTransitiveKeyCanonicals_emptySet_noEffect() {
assertFalse(roles.contains("key"));
assertTrue(roles.contains("default"));
}

@Test
void testTransitiveKeyMatchesWhenSetHasVersionPinAndDepDoesNot() {
// FHIR core writes some bindings with version pins (e.g.
// "http://hl7.org/fhir/ValueSet/publication-status|4.0.1") and others without.
// When KeyElementAnalyzer collects these into the transitive key set, dep
// references discovered through other paths may be version-stripped. The
// classifier must normalize so set membership is URL-only.
var resolver = createResolver();

var library = new Library();
library.setUrl("http://example.org/Library/main");
var libraryAdapter = (IKnowledgeArtifactAdapter) adapterFactory.createKnowledgeArtifactAdapter(library);

var transitiveKeySet = Set.of("http://hl7.org/fhir/ValueSet/publication-status|4.0.1");
var dependency = createDependency("http://hl7.org/fhir/ValueSet/publication-status");

var roles = DependencyRoleClassifier.classifyDependencyRoles(
dependency, libraryAdapter, null, resolver, transitiveKeySet);

assertTrue(
roles.contains("key"), "Should match version-pinned set entry against version-stripped dep reference");
}

@Test
void testTransitiveKeyMatchesWhenDepHasVersionPinAndSetDoesNot() {
// Symmetric case: set entry has no pin, dep reference does. Classifier
// strips dep on lookup, so this branch already worked, but lock it down.
var resolver = createResolver();

var library = new Library();
library.setUrl("http://example.org/Library/main");
var libraryAdapter = (IKnowledgeArtifactAdapter) adapterFactory.createKnowledgeArtifactAdapter(library);

var transitiveKeySet = Set.of("http://hl7.org/fhir/ValueSet/library-type");
var dependency = createDependency("http://hl7.org/fhir/ValueSet/library-type|4.0.1");

var roles = DependencyRoleClassifier.classifyDependencyRoles(
dependency, libraryAdapter, null, resolver, transitiveKeySet);

assertTrue(roles.contains("key"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ public static String getResourceType(String canonical) {
return null;
}

canonical = canonical.replace(canonical.substring(canonical.lastIndexOf("/")), "");
// Drop only the trailing /<id> segment. Using substring rather than replace,
// since replace is global and would mangle self-referential URLs like
// http://hl7.org/fhir/StructureDefinition/StructureDefinition.
canonical = canonical.substring(0, canonical.lastIndexOf("/"));
return canonical.contains("/") ? canonical.substring(canonical.lastIndexOf("/") + 1) : canonical;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package org.opencds.cqf.fhir.utility.repository;

import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.model.api.IQueryParameterType;
import ca.uhn.fhir.rest.param.UriParam;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import com.google.common.collect.Multimap;
import java.io.File;
import java.io.InputStream;
import java.util.ArrayList;
Expand All @@ -10,13 +13,17 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Queue;
import java.util.Set;
import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.utilities.npm.FilesystemPackageCacheManager;
import org.hl7.fhir.utilities.npm.NpmPackage;
import org.opencds.cqf.fhir.utility.BundleHelper;
import org.opencds.cqf.fhir.utility.Canonicals;
import org.opencds.cqf.fhir.utility.Ids;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -41,6 +48,7 @@ public class NpmRepository extends InMemoryFhirRepository {
private Map<String, String> resourceTypeIndex; // canonical URL → FHIR resource type
private Map<String, String> versionIndex; // canonical URL → version
private Map<String, PackageInfo> packageIndex; // canonical URL → owning package info
private Map<String, String> idIndex; // canonical URL → resource id (for canonical-by-URL search)

/** Package provenance information for a resource. */
public record PackageInfo(String packageId, String version, String canonical) {}
Expand Down Expand Up @@ -90,6 +98,70 @@ public <T extends IBaseResource, I extends IIdType> T read(
}
}

/**
* Canonical-by-URL search support: when an in-memory search comes up empty and the
* search includes a {@code url} parameter, look up the URL in the package index and
* trigger {@link #read} to lazy-load the resource. Then re-run the in-memory search.
*
* <p>Without this override, callers that resolve canonicals via search (e.g.
* {@code SearchHelper.searchRepositoryByCanonicalWithPaging}) silently miss
* NPM-only resources because lazy loading is only wired into {@code read()}.
*/
@Override
public <B extends IBaseBundle, T extends IBaseResource> B search(
Class<B> bundleType,
Class<T> resourceType,
Multimap<String, List<IQueryParameterType>> searchParameters,
Map<String, String> headers) {

var result = super.search(bundleType, resourceType, searchParameters, headers);

if (!BundleHelper.getEntry(result).isEmpty() || dependsOnPackages.isEmpty()) {
return result;
}

var url = extractUrlSearchParam(searchParameters);
if (url == null) {
return result;
}

ensurePackagesLoaded();

var indexedType = resourceTypeIndex.get(url);
if (indexedType == null || !indexedType.equals(resourceType.getSimpleName())) {
return result;
}

var indexedId = idIndex.get(url);
if (indexedId == null) {
return result;
}

try {
IIdType id = Ids.newId(fhirContext(), indexedType, indexedId);
read(resourceType, id, headers);
} catch (Exception e) {
logger.debug("NPM lazy-load by URL '{}' failed: {}", url, e.getMessage());
return result;
}

return super.search(bundleType, resourceType, searchParameters, headers);
}

private static String extractUrlSearchParam(Multimap<String, List<IQueryParameterType>> searchParameters) {
if (searchParameters == null || !searchParameters.containsKey("url")) {
return null;
}
return searchParameters.get("url").stream()
.flatMap(List::stream)
.filter(UriParam.class::isInstance)
.map(UriParam.class::cast)
.map(UriParam::getValue)
.filter(Objects::nonNull)
.findFirst()
.orElse(null);
}

/**
* Returns the loaded NPM packages, triggering lazy loading if needed.
*/
Expand Down Expand Up @@ -142,17 +214,19 @@ private synchronized void ensurePackagesLoaded() {
var typeIndex = new HashMap<String, String>();
var verIndex = new HashMap<String, String>();
var pkgIndex = new HashMap<String, PackageInfo>();
var idMap = new HashMap<String, String>();
logger.info("Loading NPM packages from {} seed(s)", dependsOnPackages.size());
try {
var mgr = createPackageCacheManager();
loadPackagesTransitively(mgr, result, typeIndex, verIndex, pkgIndex);
loadPackagesTransitively(mgr, result, typeIndex, verIndex, pkgIndex, idMap);
logger.info("NPM package loading complete: {} package(s) loaded", result.size());
} catch (Exception e) {
logger.warn("Could not initialize NPM package cache manager: {}", e.getMessage(), e);
}
resourceTypeIndex = typeIndex;
versionIndex = verIndex;
packageIndex = pkgIndex;
idIndex = idMap;
loadedPackages = result;
}

Expand All @@ -170,7 +244,8 @@ private void loadPackagesTransitively(
List<NpmPackage> result,
Map<String, String> typeIndex,
Map<String, String> verIndex,
Map<String, PackageInfo> pkgIndex) {
Map<String, PackageInfo> pkgIndex,
Map<String, String> idMap) {
Set<String> loaded = new HashSet<>();
Queue<String[]> toLoad = new LinkedList<>(dependsOnPackages);
while (!toLoad.isEmpty()) {
Expand All @@ -182,7 +257,7 @@ private void loadPackagesTransitively(
if (!loaded.add(key)) {
continue;
}
loadSinglePackage(mgr, pkgInfo, key, result, typeIndex, verIndex, pkgIndex, toLoad);
loadSinglePackage(mgr, pkgInfo, key, result, typeIndex, verIndex, pkgIndex, idMap, toLoad);
}
}

Expand All @@ -194,6 +269,7 @@ private void loadSinglePackage(
Map<String, String> typeIndex,
Map<String, String> verIndex,
Map<String, PackageInfo> pkgIndex,
Map<String, String> idMap,
Queue<String[]> toLoad) {
try {
var npmPkg = mgr.loadPackage(pkgInfo[0], pkgInfo[1]);
Expand All @@ -204,7 +280,7 @@ private void loadSinglePackage(
result.add(npmPkg);
logger.info("Loaded NPM package: {}", key);

indexPackageResources(npmPkg, key, typeIndex, verIndex, pkgIndex);
indexPackageResources(npmPkg, key, typeIndex, verIndex, pkgIndex, idMap);

for (var dep : npmPkg.dependencies()) {
var parts = dep.split("#", 2);
Expand All @@ -222,7 +298,8 @@ private void indexPackageResources(
String key,
Map<String, String> typeIndex,
Map<String, String> verIndex,
Map<String, PackageInfo> pkgIndex) {
Map<String, PackageInfo> pkgIndex,
Map<String, String> idMap) {
try {
var owningPackage = new PackageInfo(npmPkg.id(), npmPkg.version(), npmPkg.canonical());
for (var info : npmPkg.listIndexedResources()) {
Expand All @@ -232,6 +309,9 @@ private void indexPackageResources(
if (info.getResourceType() != null) {
typeIndex.putIfAbsent(info.getUrl(), info.getResourceType());
}
if (info.getId() != null) {
idMap.putIfAbsent(info.getUrl(), info.getId());
}
// Stub CodeSystems (content: not-present) are placeholders —
// their version and package provenance are not meaningful.
var isStub = "CodeSystem".equals(info.getResourceType()) && "not-present".equals(info.getContent());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ void partialCanonicalType() {
assertNull(Canonicals.getFragment(testUrl));
}

@Test
void selfReferentialUrlResolvesResourceType() {
// Regression: previously String.replace() stripped both occurrences of
// "/StructureDefinition" and returned "fhir" for this self-referential URL.
String selfRef = "http://hl7.org/fhir/StructureDefinition/StructureDefinition";

assertEquals("StructureDefinition", Canonicals.getResourceType(selfRef));
assertEquals("StructureDefinition", Canonicals.getIdPart(selfRef));
assertEquals(selfRef, Canonicals.getUrl(selfRef));
}

@Test
void canonicalParts() {
CanonicalType testUrl = new CanonicalType("http://fhir.acme.com/Questionnaire/example|1.0#vs1");
Expand Down
Loading
Loading