FirelyTeam / firely-net-sdk

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

TypeReference.GetTypeProfiles() does not work correctly with FhirPath types. #2165

Open ewoutkramer opened 2 years ago

ewoutkramer commented 2 years ago

Describe the bug The extension method GetTypeProfiles on TypeReference (in ElementDefinitionExtensions.cs) does not work with the situation in R4+ where the TypeRef.Code can contain a FhirPath primitive type.

There must be zillions of places where we just assume TypeRef.Code is a string with just a FHIR type, but this is no longer the case.

Expected behavior I have written another implementation in the ElementSchema compiler:

  internal const string SYSTEMTYPEURI = "http://hl7.org/fhirpath/System.";
        internal const string SDXMLTYPEEXTENSION = "http://hl7.org/fhir/StructureDefinition/structuredefinition-xml-type";

        // TODO: This would probably be useful for the SDK too
        public static string GetCodeFromTypeRef(this ElementDefinition.TypeRefComponent typeRef)
        {
            // Note, in R3, this can be empty for system primitives (so the .value element of datatypes),
            // and there are some R4 profiles in the wild that still use this old schema too.
            if (string.IsNullOrEmpty(typeRef.Code))
            {
                var r3TypeIndicator = typeRef.CodeElement.GetStringExtension(SDXMLTYPEEXTENSION);
                if (r3TypeIndicator is null)
                    throw new IncorrectElementDefinitionException($"Encountered a typeref without a code.");

                return deriveSystemTypeFromXsdType(r3TypeIndicator);
            }
            else
                return typeRef.Code;

            static string deriveSystemTypeFromXsdType(string xsdTypeName)
            {
                // This R3-specific mapping is derived from the possible xsd types from the primitive datatype table
                // at http://www.hl7.org/fhir/stu3/datatypes.html, and the mapping of these types to
                // FhirPath from http://hl7.org/fhir/fhirpath.html#types
                return makeSystemType(xsdTypeName switch
                {
                    "xsd:boolean" => "Boolean",
                    "xsd:int" => "Integer",
                    "xsd:string" => "String",
                    "xsd:decimal" => "Decimal",
                    "xsd:anyURI" => "String",
                    "xsd:anyUri" => "String",
                    "xsd:base64Binary" => "String",
                    "xsd:dateTime" => "DateTime",
                    "xsd:gYear OR xsd:gYearMonth OR xsd:date" => "DateTime",
                    "xsd:gYear OR xsd:gYearMonth OR xsd:date OR xsd:dateTime" => "DateTime",
                    "xsd:time" => "Time",
                    "xsd:token" => "String",
                    "xsd:nonNegativeInteger" => "Integer",
                    "xsd:positiveInteger" => "Integer",
                    "xhtml:div" => "String", // used in R3 xhtml
                    _ => throw new NotSupportedException($"The xsd type {xsdTypeName} is not supported as a primitive type in R3.")
                });

                static string makeSystemType(string name) => SYSTEMTYPEURI + name;
            }
        }

        /// <summary>
        ///    Returns the profiles on the given Hl7.Fhir.Model.ElementDefinition.TypeRefComponent
        ///     if specified, or otherwise the core profile url for the specified type code.
        /// </summary>
        public static IEnumerable<string>? GetTypeProfilesCorrect(this ElementDefinition.TypeRefComponent elemType)
        {
            if (elemType == null) return null;

            if (elemType.Profile.Any()) return elemType.Profile;

            var type = elemType.GetCodeFromTypeRef();
            return new[] { Canonical.ForCoreType(type).Original };
        }

Note that this also needs us to merge the Canonical type from the serverless validator project with the SDK Canonical type (a good idea anyway).

ewoutkramer commented 1 year ago

Although the chances are small that this influences current behaviour, doing this is a (minor) breaking change. Since it is also not really urgent, I will mark this issue as something to do for the next major release.