Skip to content

Fix/child extensions#1624

Open
jkiddo wants to merge 2 commits into
FHIR:masterfrom
trifork:fix/child-extensions
Open

Fix/child extensions#1624
jkiddo wants to merge 2 commits into
FHIR:masterfrom
trifork:fix/child-extensions

Conversation

@jkiddo

@jkiddo jkiddo commented May 26, 2026

Copy link
Copy Markdown

This pull request improves the handling of FHIR Extensions in the StructureDefinition serialization logic to ensure compliance with FHIR rules. Specifically, it adds logic to strip any value[x] fields from Extension objects that also contain child extension arrays, preventing invalid combinations that can arise from certain mutations. The changes also include comprehensive tests to verify this behavior.

Key changes:

FHIR Extension Sanitization:

  • Added the sanitizeElementDefinitionExtensions and cleanExtensionArray functions to recursively remove any value[x] fields from Extension objects that also have child extension arrays, ensuring the emitted JSON is always valid according to FHIR rules.
  • Updated the StructureDefinition.toJSON method to invoke this sanitization on both snapshot and differential elements during serialization.

Testing:

  • Added tests to verify that orphaned value[x] fields are stripped from Extensions with child extensions, including nested cases, and that well-formed Extensions are preserved.

@cmoesel cmoesel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the contribution, @jkiddo. I apologize for my delay in reviewing this.

I understand the motivation for doing this, but it seems like it is just masking a problem that ideally would be solved another way. If SUSHI is causing overwrites of existing extensions, it usually means that the FSH is writing extensions using indices (e.g., ^extension[+] or ^extension[0]). This can usually be fixed by changing the FSH to use extension paths (e.g., ^extension[http://hl7.org/fhir/StructureDefinition/obligation] or ^extension[$obligation] if you've defined that alias). When you use extension paths, it won't overwrite existing definitions of a different type. Note that you can also use numeric indices w/ extension paths to write multiple of the same kind of extension (e.g., ^extension[$obligation][+]).

That said, I'm ok with doing something here to improve the situation when someone does write FSH that causes extension overwrites -- but I think it might be good to log a warning so the author is aware of the situation and can choose to fix it the correct way if they'd like. Would you be open to adding a logging statement in these cases?

The other thing that concerns me is that this makes an arbitrary assumption that when both value[x] and extension exist, you should always remove the value[x]. That assumption is only correct when a complex extension overwrites a simple extension; but you'd want it to do the exact opposite when a simple extension overwrites a complex extension. Instead of guessing which thing needs to be removed, the code should use the fisher to look up the extension by its url (available as elem.url in your new code) to see if it is a simple extension (so remove extension) or a complex extension (so remove value[x]). Would you be open to making this change as well? Otherwise we would likely see some unexpected regressions for cases where the intended extension is a simple one.

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