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.

Sign up for GitHub

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

Snapshot generator: corrected cardinality of element with a derived type #1826

Merged
merged 2 commits into from
Aug 5, 2021
Merged

Snapshot generator: corrected cardinality of element with a derived type #1826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d70400f
Use cardinality from base (so do not use the cardinality of the type).
marcovisserFurore Jul 27, 2021
e4e063d
Merge branch 'develop-stu3' into feature/1824-wrong-cardinality
marcovisserFurore Aug 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions src/Hl7.Fhir.Specification.Tests/Snapshot/SnapshotGeneratorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7389,7 +7389,7 @@ public async T.Task SnapshotSucceedsWithExtendedVariantElementDef()

structureDef.Differential = new StructureDefinition.DifferentialComponent
{
Element = new System.Collections.Generic.List<ElementDefinition>{
Element = new System.Collections.Generic.List<ElementDefinition>{
new ElementDefinition
{
ElementId = "Observation.value[x].extension",
Expand Down Expand Up @@ -7417,7 +7417,7 @@ public async T.Task SnapshotSucceedsWithExtendedVariantElementDef()
new ElementDefinition.TypeRefComponent
{
Code = "Extension",
Profile = "http://example.org/fhir/StructureDefinition/MyExtension"
Profile = "http://example.org/fhir/StructureDefinition/MyExtension"
}
}
}
Expand All @@ -7431,6 +7431,29 @@ public async T.Task SnapshotSucceedsWithExtendedVariantElementDef()

structureDef.Snapshot.Element.Where(element => element.Path == "Observation.value[x].extension").Should().HaveCount(2, "Elements are in the snapshot");
structureDef.Snapshot.Element.Where(element => element.Path == "Observation.extension").Should().HaveCount(1, "Only the root extension should be there");
}
}

[TestMethod]
public async T.Task CheckCardinalityOfProfiledType()
{
var resolver = new CachedResolver(new MultiResolver(ZipSource.CreateValidationSource(), new TestProfileArtifactSource()));
var snapshotGenerator = new SnapshotGenerator(resolver, SnapshotGeneratorSettings.CreateDefault());
var sd = await resolver.ResolveByCanonicalUriAsync("http://hl7.org/fhir/StructureDefinition/Observation") as StructureDefinition;
var sut = await resolver.ResolveByCanonicalUriAsync("http://validationtest.org/fhir/StructureDefinition/ObservationWithTranslatableCode") as StructureDefinition;

// Act
var elements = await snapshotGenerator.GenerateAsync(sut);

// Assert
snapshotGenerator.Outcome.Should().BeNull();

const string codeId = "Observation.code";

var sdCode = sd.Snapshot.Element.Single(x => x.ElementId == codeId);
var sutCode = elements.Single(x => x.ElementId == codeId);

sutCode.Max.Should().Be(sdCode.Max);
sutCode.Min.Should().Be(sdCode.Min);
}
}
}
34 changes: 33 additions & 1 deletion src/Hl7.Fhir.Specification.Tests/Validation/TestProfileArtifactSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,41 @@ internal class TestProfileArtifactSource : IResourceResolver
buildMiPatient(),
slicingWithCodeableConcept(),
slicingWithQuantity(),
buildPatientWithExistsSlicing()
buildPatientWithExistsSlicing(),
buildTranslatableCodeableConcept(),
buildObservationWithTranslatableCode(),
};

private static StructureDefinition buildTranslatableCodeableConcept()
{
var result = createTestSD("http://validationtest.org/fhir/StructureDefinition/CodeableConceptTranslatable", "CodeableConceptTranslatable",
"Test CodeableConcept with an extension on CodeableConcept.text", FHIRAllTypes.CodeableConcept);

var cons = result.Differential.Element;
var ed = new ElementDefinition("CodeableConcept.text")
{
ElementId = "CodeableConcept.text",
};
ed.AddExtension("http://hl7.org/fhir/StructureDefinition/elementdefinition-translatable", new FhirBoolean(true));
cons.Add(ed);

return result;
}

private static StructureDefinition buildObservationWithTranslatableCode()
{
var result = createTestSD("http://validationtest.org/fhir/StructureDefinition/ObservationWithTranslatableCode", "ObservationWithTranslatableCode",
"Test Observation with a profiled CodeableConcept for Observation.code", FHIRAllTypes.Observation);

var cons = result.Differential.Element;
cons.Add(new ElementDefinition("Observation.code")
{
ElementId = "Observation.code"
}.OfType(FHIRAllTypes.CodeableConcept, "http://validationtest.org/fhir/StructureDefinition/CodeableConceptTranslatable"));

return result;
}

private static StructureDefinition slicingWithCodeableConcept()
{
var result = createTestSD("http://validationtest.org/fhir/StructureDefinition/ObservationSlicingCodeableConcept", "ObservationSlicingCodeableConcept",
Expand Down
49 changes: 26 additions & 23 deletions src/Hl7.Fhir.Specification/Specification/Snapshot/SnapshotGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@
// [WMR 20180409] Resolve contentReference from core resource/datatype (StructureDefinition.type)
#define FIX_CONTENTREFERENCE

using System;
using System.Collections.Generic;
using System.Linq;
using Hl7.Fhir.Model;
using Hl7.Fhir.Specification.Navigation;
using Hl7.Fhir.Specification.Source;
using System.Diagnostics;
using Hl7.Fhir.Utility;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using T = System.Threading.Tasks;

namespace Hl7.Fhir.Specification.Snapshot
Expand Down Expand Up @@ -441,7 +441,7 @@ private async T.Task<List<ElementDefinition>> generate(StructureDefinition struc
// e.g. Extension : BaseDefinition => Element : Element.extension => Extension
// Then snapshot root is already generated and annotated to differential.Element[0]
//Debug.WriteLineIf(diffRoot.HasSnapshotElementAnnotation(), $"[{nameof(SnapshotGenerator)}.{nameof(generate)} (before merge)] diff root is annotated with cached snapshot root...");

await merge(nav, diff).ConfigureAwait(false);

result = nav.ToListOfElements();
Expand Down Expand Up @@ -490,7 +490,7 @@ void fixInvalidSliceNameOnRootElement(ElementDefinition elem, StructureDefinitio
if (sd.Url != ModelInfo.CanonicalUriForFhirCoreType(FHIRAllTypes.SimpleQuantity))
{
addIssueInvalidSliceNameOnRootElement(elem, sd);
}
}
elem.SliceName = null;
}
}
Expand Down Expand Up @@ -571,16 +571,16 @@ private async T.Task<bool> expandElement(ElementDefinitionNavigator nav)
// WMR: Only expand common elements, i.e. .id | .extension | .modifierExtension
// Also verify that diff only specifies child constraints on common elements (.extension | .modifierExtension) ... ?
// Actually, we should determine the intersection of the specified type profiles... ouch

// [MS 20210614] When we can't find a CommonTypeCode assume "Element" for .id and .extension
var distinctTypeCode = defn.CommonTypeCode() ?? FHIRAllTypes.Element.GetLiteral();
var distinctTypeCode = defn.CommonTypeCode() ?? FHIRAllTypes.Element.GetLiteral();

// Different profiles for common base type => expand the common base type (w/o custom profile)
// var typeRef = new ElementDefinition.TypeRefComponent() { Code = distinctTypeCodes[0] };
var typeRef = new ElementDefinition.TypeRefComponent() { Code = distinctTypeCode };
StructureDefinition typeStructure = await getStructureForTypeRef(defn, typeRef, true).ConfigureAwait(false);
return await expandElementType(nav, typeStructure).ConfigureAwait(false);

// Alternatively, we could try to expand the most specific common base profile, e.g. (Backbone)Element
// TODO: Determine the intersection, i.e. the most specific common type that all types are derived from

Expand Down Expand Up @@ -712,16 +712,16 @@ private async T.Task merge(ElementDefinitionNavigator snap, ElementDefinitionNav
// Create a new resource element without a base element definition (for core type & resource profiles)
private async T.Task createNewElement(ElementDefinitionNavigator snap, ElementDefinitionNavigator diff)
{
var (baseElement,typeStructure) = await getBaseElementForElementType(diff.Current).ConfigureAwait(false);
var (baseElement, typeStructure) = await getBaseElementForElementType(diff.Current).ConfigureAwait(false);

if (baseElement != null)
{
var newElement = (ElementDefinition)baseElement.DeepCopy();
newElement.Path = ElementDefinitionNavigator.ReplacePathRoot(newElement.Path, diff.Path);
newElement.Base = null;

//Remove type specific constraints on polymorph type elements.
if(diff.Current.Type.Count > 1)
if (diff.Current.Type.Count > 1)
{
removeNewTypeConstraint(newElement, typeStructure);
}
Expand Down Expand Up @@ -768,12 +768,12 @@ private void removeNewTypeConstraint(ElementDefinition element, StructureDefinit
if (typeStructure?.Differential?.Element != null && !element.Constraint.IsNullOrEmpty())
{
List<ElementDefinition.ConstraintComponent> newConstraints = null;

//See if there are new constraints introduced by the type
var nav = new ElementDefinitionNavigator(typeStructure.Differential.Element);
if (nav.MoveToFirstChild())
{
if(nav.Current.IsRootElement())
if (nav.Current.IsRootElement())
newConstraints = nav.Current.Constraint;
}
//If there are any newly introduced constraints, remove them from the new element.
Expand Down Expand Up @@ -1033,8 +1033,8 @@ private async T.Task<bool> mergeTypeProfiles(ElementDefinitionNavigator snap, El
}

typeStructure = await AsyncResolver.FindStructureDefinitionAsync(primaryDiffTypeProfile).ConfigureAwait(false);
if(_settings.GenerateSnapshotForExternalProfiles)

if (_settings.GenerateSnapshotForExternalProfiles)
await ensureSnapshot(typeStructure, primaryDiffTypeProfile).ConfigureAwait(false);

// [WMR 20170224] Verify that the resolved StructureDefinition is compatible with the element type
Expand Down Expand Up @@ -1167,6 +1167,9 @@ private async T.Task<bool> mergeTypeProfiles(ElementDefinitionNavigator snap, El
// Rebase before merging
var rebasedRootElem = (ElementDefinition)typeRootElem.DeepCopy();
rebasedRootElem.Path = diff.Path;
// MV 20210727: copy cardinality from base (so do not use the cardinality of the type). See issue #1824
rebasedRootElem.Min = diff.Current.Min;
rebasedRootElem.Max = diff.Current.Max;

// Merge the type profile root element; no need to expand children
mergeElementDefinition(snap.Current, rebasedRootElem, false);
Expand Down Expand Up @@ -1242,7 +1245,7 @@ bool copyChildren(ElementDefinitionNavigator nav, ElementDefinitionNavigator typ
var elems = nav.Elements;

for (int pos = nav.OrdinalPosition.Value + 1, i = typeRootPos + 1;
i < typeElems.Count && pos < elems.Count;
i < typeElems.Count && pos < elems.Count;
i++, pos++)
{
var typeElem = typeElems[i];
Expand All @@ -1251,7 +1254,7 @@ bool copyChildren(ElementDefinitionNavigator nav, ElementDefinitionNavigator typ
// Proceed while current target element is a (grand)child of the start element

if (typeRootPos > 0 // If typeNav represents target of a contentReference...
// and if this element is NOT a child of the target contentReference...
// and if this element is NOT a child of the target contentReference...
&& !ElementDefinitionNavigator.IsChildPath(typeRootPath, typeElem.Path))
{
// Then we're done processing the subtree
Expand Down Expand Up @@ -1784,7 +1787,7 @@ private async T.Task<StructureDefinition> getStructureForTypeRef(ElementDefiniti
// [WMR 20161004] Remove configuration setting; always merge type profiles
// [WMR 20180723] Also expand custom profile on Reference
if (!string.IsNullOrEmpty(typeProfile)) // && !typeRef.IsReference()) // && _settings.MergeTypeProfiles
{
{
// Try to resolve the custom element type profile reference
baseStructure = await AsyncResolver.FindStructureDefinitionAsync(typeProfile).ConfigureAwait(false);
isValidProfile = ensureSnapshot
Expand Down Expand Up @@ -1950,9 +1953,9 @@ private async T.Task<bool> ensureSnapshot(StructureDefinition sd, string profile
private async T.Task<(ElementDefinition, StructureDefinition typeProfile)> getBaseElementForTypeRef(ElementDefinition elementDef, ElementDefinition.TypeRefComponent typeRef)
{
var typeProfile = await getStructureForTypeRef(elementDef, typeRef, false).ConfigureAwait(false);
return typeProfile != null ?
(await getSnapshotRootElement(typeProfile, typeProfile.Url, elementDef.Path).ConfigureAwait(false), typeProfile)

return typeProfile != null ?
(await getSnapshotRootElement(typeProfile, typeProfile.Url, elementDef.Path).ConfigureAwait(false), typeProfile)
: default;
}

Expand Down