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
142 stars 42 forks source link

Sushi auto-adds extensions in R5 resources implementing interfaces that overlap with user-specified extensions in profiles #1364

Open glichtner opened 11 months ago

glichtner commented 11 months ago

Hi,

First of all thank you for your work on this great software!

I've encountered the following problem in FHIR R5 with resources that implement interfaces such as the MetadataResource (e.g. PlanDefinition) or CanonicalResource (e.g. ActorDefinition): When running sushi on profiles of resources that implement these interfaces, sushi always inserts the following extension in the resulting code:

Empty profile of PlanDefinition

Profile: PlanDefinitionProfile
Parent: PlanDefinition
Id: plan-definition-profile
Title: "Plan Definition Profile"
Description: "Plan Definition Profile Without Content"

JSON after conversion

{
  "resourceType": "StructureDefinition",
  "id": "plan-definition-profile",
  "extension": [
    {
      "url": "http://hl7.org/fhir/StructureDefinition/structuredefinition-implements",
      "valueUri": "http://hl7.org/fhir/StructureDefinition/CanonicalResource"
    }
  ],

This is not a problem per se (but also not really required I think, because it's specified in the base resource already), but when specifying an own extension like so:

Profile: PlanDefinitionProfileWithExtensions
Parent: PlanDefinition
Id: plan-definition-profile-with-extension
Title: "Plan Definition Profile With Extension"
Description: "Plan Definition Profile With Extension"
* ^extension[+].url = "http://hl7.org/fhir/StructureDefinition/structuredefinition-fmm"
* ^extension[=].valueInteger = 0

this gets converted to:

{
  "resourceType": "StructureDefinition",
  "id": "plan-definition-profile-with-extension",
  "extension": [
    {
      "url": "http://hl7.org/fhir/StructureDefinition/structuredefinition-fmm",
      "valueUri": "http://hl7.org/fhir/StructureDefinition/MetadataResource",
      "valueInteger": 0
    }
  ],

i.e. mixing the two extensions together, resulting in incorrect StructureDefinitions - because the ...-fmm extension should only contain valueInteger field and not the valueUri from the ...-implements extension.

cmoesel commented 11 months ago

Hi @glichtner. Thank you for reporting this. When creating profiles, some extensions are meant to be inherited (or copied over) from the parent and others are not. We'll check to see structuredefinition-implements is supposed to be copied over or if we need to update SUSHI to suppress it.

Either way, there are cases in which a profile will retain some of the extensions from its parent. In your case, the [+] in the FSH path is being converted to [0] since there is no other indexed path before it -- and this then overwrites the extension structure that's already there. To avoid this, we suggest that when you add extensions, you use the special extension path syntax to do so. This syntax is preferred because, unlike the numeric-based indices, it does not overwrite existing extensions (unless the extension w/ the specified URL is already present in the structure).

So, in your example, try this instead:

* ^extension[http://hl7.org/fhir/StructureDefinition/structuredefinition-fmm].valueInteger = 0

You can also reference the extension just by its id:

* ^extension[structuredefinition-fmm].valueInteger = 0

Or if you prefer, set up an alias:

Alias: $FMM = http://hl7.org/fhir/StructureDefinition/structuredefinition-fmm

and use that:

* ^extension[$FMM].valueInteger = 0
glichtner commented 11 months ago

Hi @cmoesel,

Thank you for your quick response and the explanation of the extension path syntax - I wasn't aware of it.

When using the [+] syntax, wouldn't it be better/expected that SUSHI uses the next free index, i.e. keeps track of the copied over extensions / elements from a parent profile/resource and sets the index accordingly (in our case: to 1 instead of 0)?

Also, it might be helpful if SUSHI printed a warning message when a profile overwrites an inherited extension (/element) using numeric indexing, because in our reported case, the IG publisher was failing because of the additional valueUri element and it was not trivial to identify the reason (as the IG publisher's error message wasn't also particularly helpful).

Thank you for your consideration, Gregor

cmoesel commented 11 months ago

The behavior of [+] is something we've thought about before. There are some reasons, mainly related to legacy, that it behaves as it does. I agree, however, that it's not always what people intuitively expect. We can revisit it at some point, but many IGs now depend on and/or expect this behavior.

There are some cases where we do warning on overwrites, but they're mostly related to some tricky interplay between items in arrays that are referenced via indices and slice names. We can look into warning on all overwrites -- but it's not always obvious when overwrites are intentional or not, so supporting it in a general way has some challenges. Still -- I agree, this would be nice to do. We can definitely look into it some more.