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

Versioned instances #1495

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

sebg-mio42
Copy link
Contributor

Description:
Allows multiple copies of the same instance, as long as the meta.versionId is unique.
Includes a new test for the instance generation.
It does not seem to break any of the current tests.

pattern for filenames changed to accomodate the changes:

  • no versionId -> Patient-id.json
  • versionId eg "2" -> -> Patient-id_v2.json

Related Issue:
#1494

@cmoesel
Copy link
Member

cmoesel commented Jul 17, 2024

Hi @sebg-mio42. Thanks for submitting another contribution to SUSHI. After reviewing and testing this, I have a few concerns that I'd like to discuss.

1. Duplicate type/id pairings

My first concern is that the capability you are enabling is technically invalid against the FHIR Shorthand specification. 3.3.12 Item Identifiers says:

Item identifiers (ids) MUST be unique within the scope of its item type in the FSH project. For example, two Profiles with the same id cannot coexist, but it is possible to have a Profile and a ValueSet with the same id in the same FSH Project.

So... FSH does not allow a single project to define two instances of the same type (e.g., Patient in the #1494 example) with same id (e.g., dfe48ee5-88dc-4784-aed6-ac5a6d95c253 in that same example). Although your PR uses meta.versionId to distinguish the instances, the type and id are still the same

2. Compatibility with IG Publisher

My second concern is that the IG publisher also does not allow this. I used your PR along with the example in #1494 to build a simple FSH project. When I ran it through the IG Publisher, I got the following error:

Duplicate Resource in IG: Patient/dfe48ee5-88dc-4784-aed6-ac5a6d95c253. first found in /Users/cmoesel/data/fsh/VersionedInstances/fsh-generated/resources/Patient-dfe48ee5-88dc-4784-aed6-ac5a6d95c253_v1.json, now in /Users/cmoesel/data/fsh/VersionedInstances/fsh-generated/resources/Patient-dfe48ee5-88dc-4784-aed6-ac5a6d95c253_v2.json
Publishing Content Failed: Duplicate source reference 'Patient/dfe48ee5-88dc-4784-aed6-ac5a6d95c253' on a resource in the IG with the name 'dfe48ee5-88dc-4784-aed6-ac5a6d95c253' (index = 1) (00:00.070 / 00:24.259, 510Mb)

This means that if authors attempt to use this capability in an HL7 implementation guide (which must be published using the IG Publisher), it will fail.

3. Sensitivity to File Placement

My third concern is that the functionality requires that each FSH instance definition be in a different file. I discovered that if you put multiple versions of the same instance into a single FSH file, it does not export all of the versions. This, unfortunately, is also a violation of the FSH spec. 3.2.2 Items says (emphasis mine):

Text files containing FSH items MUST use the .fsh extension. Items from all files in one project SHALL be considered globally pooled for the purposes of FSH. Changing the order of items within a .fsh file or moving FSH items between files SHALL NOT affect the interpretation of the content.

4. Referring to Instances w/ Multiple Versions

My last concern is how this affects authors' ability to deterministically refer to a specific instance when that instance has multiple versions. If an author refers to an instance via a FSH Reference (e.g., Reference(dfe48ee5-88dc-4784-aed6-ac5a6d95c253), which instance is it referring to? Or if an author attempts to assign the instance into a bundle (e.g., entry[+].resource = dfe48ee5-88dc-4784-aed6-ac5a6d95c253), which instance should SUSHI copy over? By having the versioned instances use the same entity name/handle, there is no unique way to refer to one of them in FSH.

This could possibly be addressed by requiring that the entity name (what comes after Instance:) must be unique across the project, but allowing an assignment rule to set the id to a common non-unique id. E.g.:

Instance: dfe48ee5-88dc-4784-aed6-ac5a6d95c253-v1
InstanceOf: Patient
* id = "dfe48ee5-88dc-4784-aed6-ac5a6d95c253"
// more rules...

Instance: dfe48ee5-88dc-4784-aed6-ac5a6d95c253-v2
InstanceOf: Patient
* id = "dfe48ee5-88dc-4784-aed6-ac5a6d95c253"
// more rules...

By the way, your PR actually already handles this -- and it has the added bonus of getting around the previous concern, because with this approach, you can put both definitions into the same file and it still exports both of them as well.

Next Steps

So... It seems there is a way around my last two concerns, but the first two concerns remain. Perhaps it would be helpful if you could describe your use case. What are you trying to achieve with this?

Thanks again for your contribution. It's always exciting to see this type of involvement from the community.

@sebg-mio42
Copy link
Contributor Author

Hi @cmoesel, thank you very much for your in-depth review.

Our specific use case is creating resources for different points in time that reflect the state of the FHIR server at that point in time.

  • 01.01. : patient visits their general practitioner and gets prescribed new medication.
  • 15.01. : patient returns and dosages for medications are adjusted.
  • 31.01. : patient returns and dosages for medications are adjusted again.
    Another example would be to get the history for a specific MedicationStatement to see how the dosage changed over time.

Regarding your concerns:

1. Duplicate type/id pairings

That is a fair concern that I overlooked. I was working off the FHIR specification https://www.hl7.org/fhir/resource.html:
"The logical id is unique within the space of all resources of the same type on the same server."
“All resources are conceptually versioned, and each resource sits at the head of a linear list of past versions.”
This definition makes more sense to me as it covers the case that multiple versions of the same instance can exist.
One example where I feel the current FSH definition contradicts FHIR outside of a server is concerning bundles: “bdl-7 :FullUrl must be unique in a bundle, or else entries with the same fullUrl must have different meta.versionId (except in history bundles)”.

2. Compatibility with IG Publisher

I should have thought about the effect on the publisher. I tried running the publisher with versioned references but ran into the following problem:

Bad Source Reference 'Patient/dfe48ee5-88dc-4784-aed6-ac5a6d95c253/_history/1' - should have the format [Type]/[id] where id is a valid FHIR id type

To me, this appears to be a bug in the publisher as I cannot find any reason for this reference format according to FHIR. According to the FHIR specification, references may be version specific: https://hl7.org/fhir/references.html#Reference and there are no contradictory definitions for the ImplementationGuide reaource that I was able to find.
Testing this I did notice, that for this to work the IG creation in SUSHI would need to be fixed so that references are created in this format where applicable as.
What is your opinion on this?

3. Sensitivity to File Placement / Referring to Instances w/ Multiple Versions

These are both good points that I did not consider initially. I think your solution using a different entity name and specifying the id separately is elegant and avoids potential issues with referencing from FSH.

@cmoesel
Copy link
Member

cmoesel commented Jul 19, 2024

Hi @sebg-mio42. Thanks for providing the use case. That's helpful.

Regarding bundle constraint bdl-7, I agree that it seems to suggest that a bundle can have matching type/id pairs as long as the meta.versionId is different. SUSHI already supports this if you make the instances #inline and assign ids using rules (see example) -- but I understand your point. Perhaps we should consider relaxing the language around unique type/id in the FSH spec.

Regarding the IG Publisher, it does seem that it's more strict about the reference format than it should be. That said, SUSHI may have the same flaw -- we'd have to look at that. This, however, is separate from the concern that the IG Publisher won't allow two standalone instances with the same type/id -- that's still a problem, unless your use case is to always use them only inline in bundles.

I think that perhaps this is a discussion worth surfacing on Zulip so the community can participate. It would be helpful to see if others would also like to have this capability and if the community has any other thoughts about it. Does that sound good to you?

@sebg-mio42
Copy link
Contributor Author

Hi @cmoesel, I think a discussion on Zulip is a great idea. Seeing as the potential issues / consequences and the effect on the rest of the tooling are greater than I first assumed, I will prepare a bit more for the discussion before bringing it up.

@cmoesel
Copy link
Member

cmoesel commented Aug 2, 2024

Hi @sebg-mio42. I'm just checking in on this. Are you still planning to introduce this topic to the Zulip community? Or would you rather that I introduce it?

@sebg-mio42
Copy link
Contributor Author

@cmoesel , sorry for the late reply. Feel free to introduce the topic if you like. I have sadly not had time to go though in detail and prepare the topic fully.

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.

2 participants