Improve Observation Value Merge Type Mismatch Errors (#187)

* Add additional error information on existing type when merge value detect type mismatch.
* Add additional check to fail observation save if the observation found on the initial get doesn't contain the desired identifier.
* Update failing tests to properly mock data to pass the new identifier check.

src/lib/Microsoft.Health.Fhir.Ingest/Template/FhirValueProcessor.cs

@@ -11,6 +11,7 @@ namespace Microsoft.Health.Fhir.Ingest.Template
     public abstract class FhirValueProcessor<TValueType, TInValue, TOutValue> : IFhirValueProcessor<TInValue, TOutValue>
         where TValueType : FhirValueType
     {
+        private const string NullValueText = "null";
         private static readonly Type SupportedType = typeof(TValueType);
 
         public Type SupportedValueType => SupportedType;
@@ -40,5 +41,10 @@ protected TValueType CastTemplate(FhirValueType template)
         protected abstract TOutValue CreateValueImpl(TValueType template, TInValue inValue);
 
         protected abstract TOutValue MergeValueImpl(TValueType template, TInValue inValue, TOutValue existingValue);
+
+        protected static string GetValueTypeName(object value)
+        {
+            return value?.GetType()?.Name ?? NullValueText;
+        }
     }
 }

src/lib/Microsoft.Health.Fhir.R4.Ingest.Templates/Template/CodeableConceptFhirValueProcessor.cs

@@ -35,7 +35,7 @@ protected override DataType MergeValueImpl(CodeableConceptFhirValueType template
         {
             if (!(existingValue is CodeableConcept))
             {
-                throw new NotSupportedException($"Element {nameof(existingValue)} expected to be of type {typeof(CodeableConcept)}.");
+                throw new NotSupportedException($"Element {nameof(existingValue)} expected to be of type {typeof(CodeableConcept)}. Actual type is {GetValueTypeName(existingValue)}.");
             }
 
             // Currently no way to merge codeable concepts. Just replace.

src/lib/Microsoft.Health.Fhir.R4.Ingest.Templates/Template/QuantityFhirValueProcessor.cs

@@ -46,7 +46,7 @@ protected override DataType MergeValueImpl(QuantityFhirValueType template, IObse
         {
             if (!(existingValue is Quantity))
             {
-                throw new NotSupportedException($"Element {nameof(existingValue)} expected to be of type {typeof(Quantity)}.");
+                throw new NotSupportedException($"Element {nameof(existingValue)} expected to be of type {typeof(Quantity)}. Actual type is {GetValueTypeName(existingValue)}.");
             }
 
             // Only a single value, just replace.

src/lib/Microsoft.Health.Fhir.R4.Ingest.Templates/Template/SampledDataFhirValueProcessor.cs

@@ -46,7 +46,7 @@ protected override DataType MergeValueImpl(SampledDataFhirValueType template, IO
 
             if (!(existingValue is SampledData sampledData))
             {
-                throw new NotSupportedException($"Element {nameof(existingValue)} expected to be of type {typeof(SampledData)}.");
+                throw new NotSupportedException($"Element {nameof(existingValue)} expected to be of type {typeof(SampledData)}. Actual type is {GetValueTypeName(existingValue)}.");
             }
 
             if (sampledData.Dimensions > 1)

src/lib/Microsoft.Health.Fhir.R4.Ingest.Templates/Template/StringFhirValueProcessor.cs

@@ -26,7 +26,7 @@ protected override DataType MergeValueImpl(StringFhirValueType template, IObserv
         {
             if (!(existingValue is FhirString))
             {
-                throw new NotSupportedException($"Element {nameof(existingValue)} expected to be of type {typeof(FhirString)}.");
+                throw new NotSupportedException($"Element {nameof(existingValue)} expected to be of type {typeof(FhirString)}. Actual type is {GetValueTypeName(existingValue)}.");
             }
 
             // Only a single value, just replace.

src/lib/Microsoft.Health.Fhir.R4.Ingest/Service/R4FhirImportService.cs

@@ -76,6 +76,12 @@ public virtual async Task<string> SaveObservationAsync(ILookupTemplate<IFhirTemp
             if (!_observationCache.TryGetValue(cacheKey, out Model.Observation existingObservation))
             {
                 existingObservation = await GetObservationFromServerAsync(identifier).ConfigureAwait(false);
+
+                // Discovered an issue where FHIR Service is only matching on first 128 characters of the identifier.  This is a temporary measure to prevent merging of different observations until a fix is available.
+                if (existingObservation != null && !existingObservation.Identifier.Exists(i => i.IsExactly(identifier)))
+                {
+                    throw new NotSupportedException("FHIR Service returned matching observation but expected identifier was not present.");
+                }
             }
 
             var policyResult = await Policy<(Model.Observation observation, ResourceOperation operationType)>

test/Microsoft.Health.Fhir.R4.Ingest.UnitTests/Service/R4FhirImportServiceTests.cs

@@ -113,14 +113,7 @@ public async void GivenFoundObservation_WhenSaveObservationAsync_ThenUpdateInvok
                 Device = new ResourceReference(@"Device/123"),
                 Subject = new ResourceReference(@"Patient/abc"),
                 Status = ObservationStatus.Final,
-                Identifier = new List<Identifier>
-                {
-                    new Identifier
-                    {
-                        System = "Test",
-                        Value = "id",
-                    },
-                },
+                Identifier = BuildIdentifiers(),
                 Effective = new Period(new FhirDateTime(DateTimeOffset.Now.AddHours(-1)), new FhirDateTime(DateTimeOffset.Now)),
             };
 
@@ -170,7 +163,7 @@ public async void GivenFoundObservation_WhenSaveObservationAsync_ThenUpdateInvok
         [Fact]
         public async void GivenFoundObservationAndConflictOnSave_WhenSaveObservationAsync_ThenGetAndUpdateInvoked_Test()
         {
-            var foundObservation1 = new Observation { Id = "1" };
+            var foundObservation1 = new Observation { Id = "1", Identifier = BuildIdentifiers() };
             var foundBundle1 = new Bundle
             {
                 Entry = new List<Bundle.EntryComponent>
@@ -350,7 +343,7 @@ public async Task GivenCachedObservationUnchanged_WhenGenerateObservation_ThenCa
         [Fact]
         public async void GivenFoundObservationUnchanged_WhenSaveObservationAsync_ThenUpdateInvoked_Test()
         {
-            var foundObservation = new Observation { Id = "1" };
+            var foundObservation = new Observation { Id = "1", Identifier = BuildIdentifiers() };
             var foundBundle = new Bundle
             {
                 Entry = new List<Bundle.EntryComponent>
@@ -398,6 +391,18 @@ private static IDictionary<ResourceType, string> BuildIdCollection()
             return lookup;
         }
 
+        private static List<Identifier> BuildIdentifiers()
+        {
+            return new List<Identifier>
+            {
+                new Identifier
+                {
+                    System = "https://azure.microsoft.com/en-us/services/iomt-fhir-connector/",
+                    Value = "patientId.deviceId..",
+                },
+            };
+        }
+
         private static Observation ThrowConflictException()
         {
             throw new FhirException(new FhirResponse<OperationOutcome>(new HttpResponseMessage(HttpStatusCode.Conflict), new OperationOutcome()));