Fix for PR #1990

src/Hl7.Fhir.Specification.Tests/Snapshot/SnapshotGeneratorTest.cs

@@ -89,6 +89,7 @@ private StructureDefinition CreateStructureDefinition(string url, params Element
         }
 
         [DataTestMethod]
+        [DataRow(null, null, null)]
         [DataRow(null, "1", "1")]
         [DataRow("1", null, "1")]
         [DataRow("1", "1", "1")]
@@ -108,14 +109,41 @@ public void TestMergeMax(string snap, string diff, string expected)
             Assert.AreEqual(expected, actual.Value);
         }
 
+        [DataTestMethod]
+        [DataRow(null, null, null)]
+        [DataRow(null, "1", "1")]
+        [DataRow(null, "2", "2")]
+        [DataRow(null, "3", "3")]
+        [DataRow(null, "*", "*")]
+        [DataRow(0, null, "0")]
+        [DataRow(0, "1", "1")]
+        [DataRow(0, "2", "2")]
+        [DataRow(0, "3", "3")]
+        [DataRow(0, "*", "*")]
+        [DataRow(2, null, "2")]
+        [DataRow(2, "1", "2")]
+        [DataRow(2, "2", "2")]
+        [DataRow(2, "3", "3")]
+        [DataRow(2, "*", "*")]
+        [DataRow(4, null, "4")]
+        [DataRow(4, "1", "4")]
+        [DataRow(4, "2", "4")]
+        [DataRow(4, "3", "4")]
+        [DataRow(4, "*", "*")]
+        public void TestConstrainMax(int? snapMin, string snapMax, string expected)
+        {
+            var actual = SnapshotGenerator.ElementDefnMerger.constrainMax(new FhirString(snapMax), new UnsignedInt(snapMin));
+            Assert.AreEqual(expected, actual.Value);
+        }
+
         [DataTestMethod]
         [DataRow(null, null, null)]
         [DataRow(null, 1, 1)]
         [DataRow(1, null, 1)]
         [DataRow(1, 2, 2)]
         [DataRow(2, 1, 2)]
         [DataRow(1, 1, 1)]
-        public void TestMinMax(int? snap, int? diff, int? expected)
+        public void TestMergeMin(int? snap, int? diff, int? expected)
         {
             var sg = new SnapshotGenerator.ElementDefnMerger();
 
@@ -7560,21 +7588,97 @@ public async T.Task ContinueMergingChildConstraintMultipleTypes(string url, stri
             extensionElement.Should().NotBeNull();
         }
 
+        /// <summary>
+        /// Test cases that have non corrected values:
+        /// [N1] Max lt Min
+        /// Test cases that have corrected values:
+        /// [C1] Max diff: * -> take Max snap: 1
+        /// [C2] Min diff lte snap -> take Min snap: 1
+        /// </summary>
         [TestMethod]
-        public async T.Task CardinalityOfExtension()
+        [DataRow("TestExtension01", null, null, 0, "1", 0, "1")]
+        [DataRow("TestExtension01", null, "0", 0, "0", 0, "1")]
+        [DataRow("TestExtension01", null, "1", 0, "1", 0, "1")]
+        [DataRow("TestExtension01", null, "*", 0, "1", 0, "1")] // [C1]
+        [DataRow("TestExtension01", 0, null, 0, "1", 0, "1")]
+        [DataRow("TestExtension01", 0, "0", 0, "0", 0, "1")]
+        [DataRow("TestExtension01", 0, "1", 0, "1", 0, "1")]
+        [DataRow("TestExtension01", 0, "*", 0, "1", 0, "1")] // [C1]
+        [DataRow("TestExtension01", 1, null, 1, "1", 0, "1")]
+        [DataRow("TestExtension01", 1, "0", 1, "0", 0, "1")] // [N1]
+        [DataRow("TestExtension01", 1, "1", 1, "1", 0, "1")]
+        [DataRow("TestExtension01", 1, "*", 1, "1", 0, "1")] // [C1]
+
+        [DataRow("TestExtension11", null, null, 1, "1", 1, "1")]
+        [DataRow("TestExtension11", null, "0", 1, "0", 1, "1")] // [N1]
+        [DataRow("TestExtension11", null, "1", 1, "1", 1, "1")]
+        [DataRow("TestExtension11", null, "*", 1, "1", 1, "1")] // [C1]
+        [DataRow("TestExtension11", 0, null, 1, "1", 1, "1")] // [C2]
+        [DataRow("TestExtension11", 0, "0", 1, "0", 1, "1")] // [C2][N1]
+        [DataRow("TestExtension11", 0, "1", 1, "1", 1, "1")] // [C2]
+        [DataRow("TestExtension11", 0, "*", 1, "1", 1, "1")] // [C2][C1]
+        [DataRow("TestExtension11", 1, null, 1, "1", 1, "1")]
+        [DataRow("TestExtension11", 1, "0", 1, "0", 1, "1")] // [N1]
+        [DataRow("TestExtension11", 1, "1", 1, "1", 1, "1")]
+        [DataRow("TestExtension11", 1, "*", 1, "1", 1, "1")] // [C1]
+
+        [DataRow("TestExtension0star", null, null, 0, "*", 0, "*")]
+        [DataRow("TestExtension0star", null, "0", 0, "0", 0, "*")]
+        [DataRow("TestExtension0star", null, "1", 0, "1", 0, "*")]
+        [DataRow("TestExtension0star", null, "2", 0, "2", 0, "*")]
+        [DataRow("TestExtension0star", null, "*", 0, "*", 0, "*")]
+        [DataRow("TestExtension0star", 0, null, 0, "*", 0, "*")]
+        [DataRow("TestExtension0star", 0, "0", 0, "0", 0, "*")]
+        [DataRow("TestExtension0star", 0, "1", 0, "1", 0, "*")]
+        [DataRow("TestExtension0star", 0, "2", 0, "2", 0, "*")]
+        [DataRow("TestExtension0star", 0, "*", 0, "*", 0, "*")]
+        [DataRow("TestExtension0star", 1, null, 1, "*", 0, "*")]
+        [DataRow("TestExtension0star", 1, "0", 1, "0", 0, "*")] // [N1]
+        [DataRow("TestExtension0star", 1, "1", 1, "1", 0, "*")]
+        [DataRow("TestExtension0star", 1, "2", 1, "2", 0, "*")]
+        [DataRow("TestExtension0star", 1, "*", 1, "*", 0, "*")]
+        [DataRow("TestExtension0star", 2, null, 2, "*", 0, "*")]
+        [DataRow("TestExtension0star", 2, "0", 2, "0", 0, "*")] // [N1]
+        [DataRow("TestExtension0star", 2, "1", 2, "1", 0, "*")] // [N1]
+        [DataRow("TestExtension0star", 2, "2", 2, "2", 0, "*")]
+        [DataRow("TestExtension0star", 2, "*", 2, "*", 0, "*")]
+
+        [DataRow("TestExtension1star", null, null, 1, "*", 1, "*")]
+        [DataRow("TestExtension1star", null, "0", 1, "0", 1, "*")] // [N1]
+        [DataRow("TestExtension1star", null, "1", 1, "1", 1, "*")]
+        [DataRow("TestExtension1star", null, "2", 1, "2", 1, "*")]
+        [DataRow("TestExtension1star", null, "*", 1, "*", 1, "*")]
+        [DataRow("TestExtension1star", 0, null, 1, "*", 1, "*")] // [C2]
+        [DataRow("TestExtension1star", 0, "0", 1, "0", 1, "*")] // [C2][N1]
+        [DataRow("TestExtension1star", 0, "1", 1, "1", 1, "*")] // [C2]
+        [DataRow("TestExtension1star", 0, "2", 1, "2", 1, "*")] // [C2]
+        [DataRow("TestExtension1star", 0, "*", 1, "*", 1, "*")] // [C2]
+        [DataRow("TestExtension1star", 1, null, 1, "*", 1, "*")]
+        [DataRow("TestExtension1star", 1, "0", 1, "0", 1, "*")] // [N1]
+        [DataRow("TestExtension1star", 1, "1", 1, "1", 1, "*")]
+        [DataRow("TestExtension1star", 1, "2", 1, "2", 1, "*")]
+        [DataRow("TestExtension1star", 1, "*", 1, "*", 1, "*")]
+        [DataRow("TestExtension1star", 2, null, 2, "*", 1, "*")]
+        [DataRow("TestExtension1star", 2, "0", 2, "0", 1, "*")] // [N1]
+        [DataRow("TestExtension1star", 2, "1", 2, "1", 1, "*")] // [N1]
+        [DataRow("TestExtension1star", 2, "2", 2, "2", 1, "*")]
+        [DataRow("TestExtension1star", 2, "*", 2, "*", 1, "*")]
+        public async T.Task CardinalityOfExtension(string extension, int? diffMin, string diffMax, int extMin, string extMax, int baseMin, string baseMax)
         {
             // Arrange
+            string url = $"https://example.org/fhir/StructureDefinition/issue-1981-patient";
             string parentId = "Patient.extension";
-            string elementId = "Patient.extension:birthPlace";
+            string elementId = "Patient.extension:test";
 
-            var sd = await _testResolver.FindStructureDefinitionAsync("https://example.org/fhir/StructureDefinition/issue-1981-patient");
+            var sd = await _testResolver.FindStructureDefinitionAsync(url);
 
             sd.Differential.Element.Should().HaveCount(2);
 
             var extensionElement = sd.Differential.Element.Single(x => x.ElementId == elementId);
 
-            extensionElement.Min.Should().Be(0);
-            extensionElement.Max.Should().BeNull();
+            extensionElement.Min = diffMin;
+            extensionElement.Max = diffMax;
+            extensionElement.Type[0].ProfileElement = new FhirUri($"https://example.org/fhir/StructureDefinition/{extension}");
 
             var snapshotGenerator = new SnapshotGenerator(_testResolver, _settings);
 
@@ -7600,13 +7704,13 @@ public async T.Task CardinalityOfExtension()
             // Assert
             extensionElement = elementsExpanded.Single(x => x.ElementId == elementId);
 
-            extensionElement.Min.Should().Be(0);
-            extensionElement.Max.Should().Be("1");
+            extensionElement.Min.Should().Be(extMin);
+            extensionElement.Max.Should().Be(extMax);
 
             var baseElement = extensionElement.Annotation<TestAnnotation>().BaseElementDefinition;
 
-            baseElement.Min.Should().Be(0);
-            baseElement.Max.Should().Be("1");
+            baseElement.Min.Should().Be(baseMin);
+            baseElement.Max.Should().Be(baseMax);
         }
 
         private sealed class TestAnnotation

src/Hl7.Fhir.Specification.Tests/TestData/snapshot-test/Issue-1981/Issue-1981-Patient.StructureDefinition.xml

@@ -3,7 +3,7 @@
   <url value="https://example.org/fhir/StructureDefinition/issue-1981-patient" />
   <name value="MyPatient" />
   <status value="draft" />
-  <fhirVersion value="4.0.1" />
+  <fhirVersion value="3.0.2" />
   <kind value="resource" />
   <abstract value="false" />
   <type value="Patient" />
@@ -21,13 +21,14 @@
       </slicing>
       <min value="0" />
     </element>
-    <element id="Patient.extension:birthPlace">
+    <element id="Patient.extension:test">
       <path value="Patient.extension" />
-      <sliceName value="birthPlace" />
+      <sliceName value="test" />
       <min value="0" />
+      <max value="1" />
       <type>
         <code value="Extension" />
-        <profile value="http://hl7.org/fhir/StructureDefinition/birthPlace" />
+        <profile value="https://example.org/fhir/StructureDefinition/TestExtension0star" />
       </type>
       <isModifier value="false" />
     </element>

src/Hl7.Fhir.Specification.Tests/TestData/snapshot-test/Issue-1981/TestExtension01.StructureDefinition.xml

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="utf-8"?>
+<StructureDefinition xmlns="http://hl7.org/fhir">
+  <url value="https://example.org/fhir/StructureDefinition/TestExtension01" />
+  <name value="TestExtension01" />
+  <status value="draft" />
+  <fhirVersion value="3.0.2" />
+  <kind value="complex-type" />
+  <abstract value="false" />
+  <contextType value="resource"/>
+  <context value="Patient"/>
+  <type value="Extension" />
+  <baseDefinition value="http://hl7.org/fhir/StructureDefinition/Extension" />
+  <derivation value="constraint" />
+  <differential>
+    <element id="Extension">
+      <path value="Extension" />
+      <max value="1" />
+    </element>
+    <element id="Extension.url">
+      <path value="Extension.url" />
+      <fixedUri value="https://example.org/fhir/StructureDefinition/TestExtension01" />
+    </element>
+  </differential>
+</StructureDefinition>

src/Hl7.Fhir.Specification.Tests/TestData/snapshot-test/Issue-1981/TestExtension0star.StructureDefinition.xml

@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="utf-8"?>
+<StructureDefinition xmlns="http://hl7.org/fhir">
+  <url value="https://example.org/fhir/StructureDefinition/TestExtension0star" />
+  <name value="TestExtension0star" />
+  <status value="draft" />
+  <fhirVersion value="3.0.2" />
+  <kind value="complex-type" />
+  <abstract value="false" />
+  <contextType value="resource"/>
+  <context value="Patient"/>
+  <type value="Extension" />
+  <baseDefinition value="http://hl7.org/fhir/StructureDefinition/Extension" />
+  <derivation value="constraint" />
+  <differential>
+    <element id="Extension">
+      <path value="Extension" />
+    </element>
+    <element id="Extension.url">
+      <path value="Extension.url" />
+      <fixedUri value="https://example.org/fhir/StructureDefinition/TestExtension0star" />
+    </element>
+  </differential>
+</StructureDefinition>

src/Hl7.Fhir.Specification.Tests/TestData/snapshot-test/Issue-1981/TestExtension11.StructureDefinition.xml

@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="utf-8"?>
+<StructureDefinition xmlns="http://hl7.org/fhir">
+  <url value="https://example.org/fhir/StructureDefinition/TestExtension11" />
+  <name value="TestExtension11" />
+  <status value="draft" />
+  <fhirVersion value="3.0.2" />
+  <kind value="complex-type" />
+  <abstract value="false" />
+  <contextType value="resource"/>
+  <context value="Patient"/>
+  <type value="Extension" />
+  <baseDefinition value="http://hl7.org/fhir/StructureDefinition/Extension" />
+  <derivation value="constraint" />
+  <differential>
+    <element id="Extension">
+      <path value="Extension" />
+      <min value="1" />
+      <max value="1" />
+    </element>
+    <element id="Extension.url">
+      <path value="Extension.url" />
+      <fixedUri value="https://example.org/fhir/StructureDefinition/TestExtension11" />
+    </element>
+  </differential>
+</StructureDefinition>

src/Hl7.Fhir.Specification.Tests/TestData/snapshot-test/Issue-1981/TestExtension1star.StructureDefinition.xml

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="utf-8"?>
+<StructureDefinition xmlns="http://hl7.org/fhir">
+  <url value="https://example.org/fhir/StructureDefinition/TestExtension1star" />
+  <name value="TestExtension1star" />
+  <status value="draft" />
+  <fhirVersion value="3.0.2" />
+  <kind value="complex-type" />
+  <abstract value="false" />
+  <contextType value="resource"/>
+  <context value="Patient"/>
+  <type value="Extension" />
+  <baseDefinition value="http://hl7.org/fhir/StructureDefinition/Extension" />
+  <derivation value="constraint" />
+  <differential>
+    <element id="Extension">
+      <path value="Extension" />
+      <min value="1" />
+    </element>
+    <element id="Extension.url">
+      <path value="Extension.url" />
+      <fixedUri value="https://example.org/fhir/StructureDefinition/TestExtension1star" />
+    </element>
+  </differential>
+</StructureDefinition>

src/Hl7.Fhir.Specification/Specification/Snapshot/ElementDefnMerger.cs

@@ -104,9 +104,8 @@ void merge(ElementDefinition snap, ElementDefinition diff, bool mergeElementId)
                 // Mappings are cumulative, but keep unique on full contents
                 snap.Mapping = mergeCollection(snap.Mapping, diff.Mapping, (a, b) => a.IsExactly(b));
 
-                //snap.MinElement = mergePrimitiveAttribute(snap.MinElement, diff.MinElement);
+                // Note that max is not corrected when max < min! constrainMax could be used if that is desired.
                 snap.MinElement = mergeMin(snap.MinElement, diff.MinElement);
-                //snap.MaxElement = mergePrimitiveAttribute(snap.MaxElement, diff.MaxElement);
                 snap.MaxElement = mergeMax(snap.MaxElement, diff.MaxElement);
 
                 // snap.base should already be there, and is not changed by the diff
@@ -240,7 +239,7 @@ T mergePrimitiveAttribute<T>(T snap, T diff, bool allowAppend = false) where T :
             }
 
             /// <summary>
-            /// Merge the Min element of the differential into the snapshot. The most contrained will win: so the maximum of both values.
+            /// Merge the Min element of the differential into the snapshot. The most constrained will win: so the maximum of both values.
             /// </summary>
             /// <param name="snap"></param>
             /// <param name="diff"></param>
@@ -300,12 +299,36 @@ internal FhirString mergeMax(FhirString snap, FhirString diff)
                     // do anything to not break it any further.
                     return snap;
                 }
-                else if (!diff.IsNullOrEmpty() && (snap.IsNullOrEmpty() || !diff.IsExactly(snap)))
+
+                if (!diff.IsNullOrEmpty() && (snap.IsNullOrEmpty() || !diff.IsExactly(snap)))
                 {
                     return deepCopyAndRaiseOnConstraint(diff);
                 }
-                else
-                    return snap;
+
+                return snap;
+            }
+
+            /// <summary>
+            /// Make sure max >= min. To be used in conjunction with mergeMax:
+            /// 
+            ///     snap.MaxElement = constrainMax(mergeMax(snap.MaxElement, diff.MaxElement), snap.MinElement);
+            /// 
+            /// However this method is not used at the moment.
+            /// It may be used in the future when the need arises.
+            /// </summary>
+            internal static FhirString constrainMax(FhirString max, UnsignedInt minValue)
+            {
+                if (minValue.IsNullOrEmpty())
+                    return max;
+
+                // Max does not have a value but min does so take min
+                if (max.IsNullOrEmpty())
+                    return new FhirString(minValue.Value.ToString());
+
+                // If max is an int and is smaller than min then take min otherwise take max
+                return int.TryParse(max.Value, out var maxValue) && maxValue < minValue.Value
+                    ? new FhirString(minValue.Value.ToString())
+                    : max;
             }
 
             private T deepCopyAndRaiseOnConstraint<T>(T elt) where T : PrimitiveType

src/Hl7.Fhir.Specification/Specification/Snapshot/SnapshotGenerator.cs

@@ -1159,16 +1159,6 @@ private async T.Task<bool> mergeTypeProfiles(ElementDefinitionNavigator snap, El
                             var rebasedRootElem = (ElementDefinition)typeRootElem.DeepCopy();
                             rebasedRootElem.Path = diff.Path;
 
-                            // MV 20220224: we have here actually a 3-way merge of the cardinality: merge of the snapshot in progress (1), differential (2) and 
-                            // the external profile type (3). We do first a merge with the profile type and the differential and then the differential 
-                            // will be merged into the snapshot.
-
-                            var sg = new ElementDefnMerger();
-                            // merge the min of the external profile type with the diff. Most constrained wins (the maximum of both values)
-                            rebasedRootElem.MinElement = sg.mergeMin(rebasedRootElem.MinElement, diff.Current.MinElement);
-                            // merge the min of the external profile type with the diff. Most constrained wins (the minumum of both values) 
-                            rebasedRootElem.MaxElement = sg.mergeMax(rebasedRootElem.MaxElement, diff.Current.MaxElement);
-
                             // Merge the type profile root element; no need to expand children
                             mergeElementDefinition(snap.Current, rebasedRootElem, false);
                         }