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 loading custom profiles for validation #29

Open
seanmcilvenna opened this issue Jun 26, 2020 · 4 comments
Open

Support loading custom profiles for validation #29

seanmcilvenna opened this issue Jun 26, 2020 · 4 comments

Comments

@seanmcilvenna
Copy link
Collaborator

See conversation from #27
Need to fill out details of enhancement.

@justinusmenzel
Copy link

Thank you for providing a FHIR validator implementation in JavaScript.
I'm wondering what the gap still is towards full support of profiles. I did some experiments with validating a C4BB Patient resource and it seems like the validator isn't able to pick the loaded C4BB Patient profile. It seems to be looking up the structure definition by resourceType which is Patient but the C4BB structure definition is stored using the key C4BB-Patient.
Would it make sense to use the canonical structure definition URL as key?
Does the validator take meta/profiles attribute into account?
What about structure definitions extending another structure definition, how is inheritance via baseDefinition supported?

@seanmcilvenna
Copy link
Collaborator Author

The inheritance of profiles via baseDefinition is pretty much the biggest road-blocker for me. I did some "bare minimum" validation functionality in FHIR.js, but stopped when I started contemplating the complexities of supporting baseDefinition with multiple inheritance.

At this time, I don't have a lot of personal need to implement additional enhancements to the validation functionality. Of course, I'd always welcome pull-requests that improve that functionality (ideally with unit tests added).

That's not to say that I won't ever have a use-case for enhanced validation capabilities... But, I don't have any right now and it isn't on my road-map for implementation at this time.

With that said, my company (Lantana Consulting Group) is a consulting company, and would of course be open to engaging with you as a potential client if that is something that you're interested in.

@lschmierer
Copy link
Contributor

@seanmcilvenna can you elaborate some of the complexities introduced by baseDefinition?

Currently, the whole library relies on StructureDefinition.snapshot to be populated.
Working only with StructureDefinition.differential is not supported.

It is my understanding, that inheritance (baseDefinition) should already be incorporated into populated StructureDefinition.snapshot for any given profile (e.g. generated by the official ig publisher Java tool).

If I am not missing something, a change along the lines of @justinusmenzel #29 (comment) should be straighforward.

@seanmcilvenna
Copy link
Collaborator Author

@lschmierer I'd love to see a PR for this :)
In order to support "differential", we would need to add in logic to generate the "snapshot". To generate the snapshot, we would need to:

  1. Have all profiles defined in the hierarchy of StructureDefinition.baseProfile available to us. What happens when you get to a base profile that references a baseProfile that we don't have? Throw an error, I guess... Or silently ignore it and validate with what we have?
  2. Even if it has all of the profiles in the base profile's hierarchy, there is some really complex logic in establishing a snapshot for a profile based on the base profile. You have to overlay the changes from each level of profile onto the next, and establish the association between elements in profile A and profile B is a complex process; dealing with slicing and re-slicing, for example. I started the logic in the SnapshotGenerator class, but it's lacking support for quite a few scenarios.

Ultimately, it's doable, I just don't have enough time to do it right. And I'm hesitant to add "incomplete" functionality too much. I would hate to say "Supports validation", when it only validates 1/3rd of the scenarios, or validates incorrectly. That could lead to some bad interoperability. I'd love to see some other contributors fill out more of the SnapshotGenerator logic. Once enough of the SnapshotGenerator has been built out, we could pretty easily support validating against differential.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants