FHIR / sushi

SUSHI (aka "SUSHI Unshortens Short Hand Inputs") is a reference implementation command-line interpreter/compiler for FHIR Shorthand (FSH).
Apache License 2.0
145 stars 44 forks source link

Versioned instances #1495

Open sebg-mio42 opened 3 months ago

sebg-mio42 commented 3 months ago

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:

Related Issue: https://github.com/FHIR/sushi/issues/1494

cmoesel commented 3 months ago

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 commented 3 months ago

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.

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 commented 3 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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.