Skip to content

Conversation

@lukedegruchy
Copy link
Contributor

@lukedegruchy lukedegruchy commented Aug 29, 2025

  • Introduce NpmPackageLoader as an interface with default method to enforce the NPM loading contract
  • Make this FHIR version agnostic, as it supports both R4 and R5.
  • Add MeasureOrNpmResourceHolder, which will polymorphically hold either a Repository or NPM Measure and Library
  • Add an NpmNamespaceManager which captures the function to extract namespaces from NPM packages
  • Enhance IgRepository to read from directories that contain NPM tgz files.
  • Add R4 and R5 tests verify the contract for loading NPM packages works with packages built for both R4 and R5

@github-actions
Copy link

github-actions bot commented Aug 29, 2025

Formatting check succeeded!

@codecov
Copy link

codecov bot commented Aug 29, 2025

Codecov Report

❌ Patch coverage is 56.84932% with 189 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.98%. Comparing base (a164822) to head (fbc36c2).

Files with missing lines Patch % Lines
...qf/fhir/utility/npm/NpmPackageLoaderWithCache.java 0.00% 42 Missing ⚠️
...cqf/fhir/utility/npm/NpmPackageLoaderInMemory.java 71.83% 28 Missing and 12 partials ⚠️
...pencds/cqf/fhir/utility/npm/NpmResourceHolder.java 53.01% 31 Missing and 8 partials ⚠️
...f/fhir/utility/npm/MeasureOrNpmResourceHolder.java 43.90% 17 Missing and 6 partials ⚠️
...s/cqf/fhir/utility/repository/ig/IgRepository.java 35.48% 18 Missing and 2 partials ⚠️
...opencds/cqf/fhir/utility/npm/NpmPackageLoader.java 15.78% 16 Missing ⚠️
...ir/utility/npm/MeasureOrNpmResourceHolderList.java 78.37% 6 Missing and 2 partials ⚠️
...ncds/cqf/fhir/utility/npm/NpmNamespaceManager.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #724      +/-   ##
============================================
- Coverage     73.31%   72.98%   -0.33%     
  Complexity      211      211              
============================================
  Files           487      495       +8     
  Lines         22979    23383     +404     
  Branches       2963     3003      +40     
============================================
+ Hits          16846    17067     +221     
- Misses         4675     4831     +156     
- Partials       1458     1485      +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lukedegruchy lukedegruchy marked this pull request as ready for review August 29, 2025 19:39
@lukedegruchy lukedegruchy changed the title Add all utilities classes as well as tests to support NPM. Test tgzs… Add NPM package loader classes as a first step to introducing wider NPM functionality Aug 29, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 2, 2025

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this file seem like formatting changes? Do we really need to make these changes as part of this PR?


/**
* This interface exposes common functionality across all FHIR Questionnaire versions.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment seems wrong?

.map(r -> (IKnowledgeArtifactAdapter) IAdapterFactory.forFhirVersion(r.getStructureFhirVersionEnum())
.createResource(r))
.sorted((a, b) -> versionComparator.compare(a.getVersion(), b.getVersion()))
.sorted((a, b) -> Versions.compareVersions(a.getVersion(), b.getVersion()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar question, is this change needed by this PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have a PR that is just for the Measure adapter?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we talked about removing this? I don't understand why I would need to know where a Measure was loaded from?

* <p/>
* Helps implement a migration from the old world of FHIR/Repository based resources for Libraries,
* Measures and eventually other clinical intelligence resources (such as PlanDefinitions or
* ValueSets), and the new world where they're derived from NPM packages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to get on the same page here, I don't think it's right to characterize this as an "old world" and a "new world", they are different ways of getting at the resources, and one isn't replacing the other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants