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

Support fishing all resource types from dependency packages and input/* #1548

Merged
merged 6 commits into from
Dec 30, 2024

Conversation

cmoesel
Copy link
Member

@cmoesel cmoesel commented Dec 23, 2024

Description: In the past, SUSHI only loaded specific resource types from dependency packages and predefined resources in input/* (i.e., StructureDefinitions, ValueSets, and CodeSystems). The recent integration of FPL 2.0.0 now loads all resource types, but SUSHI still did not return them in its results since it only fished (i.e. searched) for the previously supported types. This PR adds support for fishing those types by interpreting a fishing call for the Type.Instance type as a wildcard type (since a FSH Instance can be literally any type).

This also adds a new fishForMetadatas function that will return all the matching Metadatas instead of just the first one. This allows for smarter resolution of Canonical(...) keywords to ensure that a matched type is resolved when one is available. This change, however, required that I add support for fishForMetadatas in all the Fishable implementations (not just FHIRDefinitions, but also Tank, Package, some exporters, etc.).

Backstory for fishForMetadatas change: Regression failed when I added support for fishing on all resource types because it introduced new types that could now come up in search results before the types it returned in previous versions. In one specific case, an IG was assigning CarePlan.instantiatesCanonical to Canonical(SomeGuideline). In previous versions of SUSHI, Canonical(SomeGuideline) resolved to a PlanDefinition defined in input/fsh -- but with support for finding additional resource types in packages and predefined inputs, it was now resolving to a Library in input/resources. This was correct behavior because SUSHI prioritizes predefined resource above FSH definitions (to allow for "patched" resources to override FSH resources) -- but it caused errors because CarePlan.instantiatesCanonical allowed for canonical<PlanDefinition> but not canonical<Library>, so it threw a mismatched type exception (Canonical(Library) is not one of the allowed types...). This seemed like an unfortunate error since there was a valid type in scope (the PlanDefinition defined in FSH), so I updated SUSHI to look through all the matches if the first match does not satisfy the canonical requirements. Now the regression passes with no changes.

Testing Instructions: I've created a simple project that tests both aspects of this: (1) That additional resource types are now loaded (in this case, from predefined resource), and (2) that SUSHI intelligently chooses the right resource based on required Canonical type. You can try running the project using SUSHI 3.13.1, and then this PR branch. The project: AllTypesTest.zip

Related Issues:

If the FHIRDefinitions fish functions are called specifying Type.Instance as a requested type, treat it like no types were passed in at all (i.e. wildcard type search) in order to search all types.
…cal assignments

Now that SUSHI loads additional resource types from dependencies, there is a greater chance of naming collisions when resolving names. In one case, this caused a canoonical assignment that used to work to stop working (i.e. it used to resolve to a PlanDefinition from the tank, which was good; but then it started resolving to a Library from predefined resources, which didn't match the needed Canonical type).

Now we can search for all metadata results from a given search term and, if necessary, choose the one that is a best match during canonical assignment. This may be relevant to other parts of the code as well, but for now we only do it for Canonical assignment in instances.
mint-thompson
mint-thompson previously approved these changes Dec 26, 2024
Copy link
Collaborator

@mint-thompson mint-thompson left a comment

Choose a reason for hiding this comment

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

These changes look good, and the sample project you provided produces the expected output. I left one comment that contains a question, but that's something that would be a future task. Thank you!

src/import/FSHTank.ts Show resolved Hide resolved
test/fhirtypes/ElementDefinition.assignString.test.ts Outdated Show resolved Hide resolved
jafeltra
jafeltra previously approved these changes Dec 26, 2024
Copy link
Collaborator

@jafeltra jafeltra left a comment

Choose a reason for hiding this comment

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

This all makes sense to me!

@cmoesel cmoesel dismissed stale reviews from jafeltra and mint-thompson via 9818474 December 30, 2024 16:44
@cmoesel cmoesel merged commit 09b2585 into master Dec 30, 2024
14 checks passed
@cmoesel cmoesel deleted the other-dep-resource-types-2 branch December 30, 2024 16:52
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