FirelyTeam / firely-net-sdk

The official Firely .NET SDK for HL7 FHIR
Other
829 stars 345 forks source link

Change ClassMappings so it can support custom resources #2850

Open ewoutkramer opened 1 month ago

ewoutkramer commented 1 month ago

As part of our effort to support "custom resources" through Pocos (and more specifically through the new DynamicResource/DynamicDataType), we will need a way to provide metadata about these resources so they can be parsed and serialized. Custom resources are useful in general for supporting data that does not have an equivalent in FHIR (yet), but also to add new elements for newer, in-preview versions of FHIR (or even IGs), without having to generate a full new version of the SDK for it.

Currently, the ClassMappings are constructed by reflecting on the POCO's, but by definition DynamicResources will not have any static reflectable type information, so the information should be supplied by the user. In fact, we have an existing mechanism for that, IStructureDefinitionSummary which was designed to support the ITypedElement subsystem. The current ClassMappings implement IStructureDefinitionSummary, so there is already a strong link between the two abstractions. Whatever we do, the new capabilities shoul;d still support IStructureDefinitionSummary, so the ITypedElement stack can be retained.

I have done a quick survey over the metadata that the serializers & parsers need, and it looks to me that none of it is inherently tied to reflection, even the generated getters/setters could be rewritten to use IDictionary (if we so choose), and thus be re-useable across resources and dynamic resources.

Note that the POCO's should be prepared for this change to use something akin IDictionary (not only IReadonlyDictionary) so the (de)serializers can treat poco's and dynamic resources uniformly and so they can deal with overflow elements. This is a separate task (TODO: add a link to that separate task once created).

ewoutkramer commented 1 month ago

It is definitely worth looking at JsonTypeInfo for inspiration, including its inheritance hierarchy of metadata inspectors and modifiers.

ewoutkramer commented 1 month ago

I have done a spike and the main problems that we have to find a design for are:

Other than this there are some minor issues with dependence on native types in the parser and IStructureDefinitionSummary that can easily be fixed.

ewoutkramer commented 1 month ago

Also, I have tried to remove IsAbstract. It is unclear what we want to achieve here, and therefore it is not clear whether Backbone's are supposed to be abstract (as they are in IStructureDefinitionSummary) or not (as they are in ClassMapping, since they are backed with real POCO's). Even in StructureDefinitions, there is an underlying "type" for backbones, it is just not named, so I think these are not abstract at all. To avoid this confusion, I have removed IsAbstract to see what kind of feedback we will get. The API can live without it (I think).