[Bugfix] HeaderDictionary - Support concurrent operations (#3729)

* Bundle - Concurrent access

* Introducing new ThreadSafeHeaderDictionary

* Adding tests

* Fix conccurent access to http headers.

* Fix concurrent access to http headers.

* New test to validate IHeaderDictionary clones.

* Using statements. Small refactorings

* remove clone

* Expose FHIR operation outcome in case of errors

* Adding more details to the exception

src/Microsoft.Health.Fhir.Api/Features/ActionResults/ResourceActionResult.cs

@@ -3,6 +3,7 @@
 // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
 // -------------------------------------------------------------------------------------------------
 
+using System;
 using System.Collections.Generic;
 using System.Net;
 using System.Threading.Tasks;
@@ -21,9 +22,11 @@ namespace Microsoft.Health.Fhir.Api.Features.ActionResults
     {
         protected ResourceActionResult()
         {
+            Headers = new HeaderDictionary();
         }
 
         protected ResourceActionResult(TResult result)
+            : this()
         {
             Result = result;
         }
@@ -47,9 +50,9 @@ namespace Microsoft.Health.Fhir.Api.Features.ActionResults
         /// <summary>
         /// Gets or sets the action result Headers.
         /// </summary>
-        internal IHeaderDictionary Headers { get; } = new HeaderDictionary();
+        internal IHeaderDictionary Headers { get; }
 
-        public override Task ExecuteResultAsync(ActionContext context)
+        public override async Task ExecuteResultAsync(ActionContext context)
         {
             EnsureArg.IsNotNull(context, nameof(context));
 
@@ -68,12 +71,15 @@ namespace Microsoft.Health.Fhir.Api.Features.ActionResults
 
             foreach (KeyValuePair<string, StringValues> header in Headers)
             {
-                if (response.Headers.ContainsKey(header.Key))
+                try
                 {
-                    response.Headers.Remove(header.Key);
+                    response.Headers[header.Key] = header.Value;
+                }
+                catch (InvalidOperationException ioe)
+                {
+                    // Catching operations that change non-concurrent collections.
+                    throw new InvalidOperationException($"Failed to set header '{header.Key}'.", ioe);
                 }
-
-                response.Headers.Add(header);
             }
 
             ActionResult result;
@@ -86,7 +92,7 @@ namespace Microsoft.Health.Fhir.Api.Features.ActionResults
                 result = new ObjectResult(GetResultToSerialize());
             }
 
-            return result.ExecuteResultAsync(context);
+            await result.ExecuteResultAsync(context);
         }
 
         protected virtual object GetResultToSerialize()

src/Microsoft.Health.Fhir.Core/Features/Conformance/SystemConformanceProvider.cs

@@ -6,6 +6,8 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Linq;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using EnsureThat;
@@ -88,15 +90,7 @@ namespace Microsoft.Health.Fhir.Core.Features.Conformance
             _contextAccessor = contextAccessor;
             _searchParameterStatusManager = searchParameterStatusManager;
 
-            if (!string.IsNullOrEmpty(_configuration.Value.Versioning.Default))
-            {
-                _logger.LogInformation("Default version is:{VersioningDefault}.", _configuration.Value.Versioning.Default);
-
-                foreach (var resourcetype in _configuration.Value.Versioning.ResourceTypeOverrides)
-                {
-                    _logger.LogInformation("{ResourceTypeKey} version overridden to:{ResourceTypeVersioningOverride}.", resourcetype.Key, resourcetype.Value);
-                }
-            }
+            LogVersioningPolicyConfiguration();
         }
 
         public override async Task<ResourceElement> GetCapabilityStatementOnStartup(CancellationToken cancellationToken = default(CancellationToken))
@@ -361,5 +355,21 @@ namespace Microsoft.Health.Fhir.Core.Features.Conformance
                 _metadataSemaphore?.Release();
             }
         }
+
+        private void LogVersioningPolicyConfiguration()
+        {
+            if (!string.IsNullOrEmpty(_configuration.Value.Versioning.Default) || _configuration.Value.Versioning.ResourceTypeOverrides.Any())
+            {
+                StringBuilder versioning = new StringBuilder();
+                versioning.AppendLine($"Default version is: '{_configuration.Value.Versioning.Default ?? "(default)"}'.");
+
+                foreach (var resourceTypeVersioning in _configuration.Value.Versioning.ResourceTypeOverrides)
+                {
+                    versioning.AppendLine($"'{resourceTypeVersioning.Key}' version overridden to: '{resourceTypeVersioning.Value}'.");
+                }
+
+                _logger.LogInformation(versioning.ToString());
+            }
+        }
     }
 }

src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/ActionResults/FhirResultTests.cs

@@ -71,7 +71,7 @@ namespace Microsoft.Health.Fhir.Api.UnitTests.Features.ActionResults
         }
 
         [Fact]
-        public void GivenAFhirResult_WhenHeadersThatAlreadyExistsInResponseArePassed_ThenDuplicteHeadersAreRemoved()
+        public async Task GivenAFhirResult_WhenHeadersThatAlreadyExistsInResponseArePassed_ThenDuplicteHeadersAreRemoved()
         {
             var result = FhirResult.Gone();
             var context = new ActionContext
@@ -94,7 +94,7 @@ namespace Microsoft.Health.Fhir.Api.UnitTests.Features.ActionResults
             result.Headers["testKey2"] = "2";
             context.HttpContext.Response.Headers["testKey2"] = "1";
 
-            result.ExecuteResultAsync(context);
+            await result.ExecuteResultAsync(context);
 
             Assert.Null(result.Result);
             Assert.Equal(HttpStatusCode.Gone, result.StatusCode.GetValueOrDefault());

src/Microsoft.Health.Fhir.Shared.Api/Features/ActionResults/OperationVersionsResult.cs

@@ -40,11 +40,11 @@ namespace Microsoft.Health.Fhir.Api.Features.ActionResults
             _versionsResult = versionsResult;
         }
 
-        public override Task ExecuteResultAsync(ActionContext context)
+        public override async Task ExecuteResultAsync(ActionContext context)
         {
             _httpContext = context.HttpContext;
             _acceptHeaders = _httpContext.Request.GetTypedHeaders().Accept;
-            return base.ExecuteResultAsync(context);
+            await base.ExecuteResultAsync(context);
         }
 
         protected override object GetResultToSerialize()

src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandler.cs

@@ -4,6 +4,7 @@
 // -------------------------------------------------------------------------------------------------
 
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
@@ -80,7 +81,7 @@ namespace Microsoft.Health.Fhir.Api.Features.Resources.Bundle
         private readonly ILogger<BundleHandler> _logger;
         private readonly HTTPVerb[] _verbExecutionOrder;
         private readonly List<int> _emptyRequestsOrder;
-        private readonly Dictionary<string, (string resourceId, string resourceType)> _referenceIdDictionary;
+        private readonly ConcurrentDictionary<string, (string resourceId, string resourceType)> _referenceIdDictionary;
         private readonly TransactionBundleValidator _transactionBundleValidator;
         private readonly ResourceReferenceResolver _referenceResolver;
         private readonly IAuditEventTypeMapping _auditEventTypeMapping;
@@ -151,7 +152,7 @@ namespace Microsoft.Health.Fhir.Api.Features.Resources.Bundle
             _requestServices = outerHttpContext.RequestServices;
             _originalRequestBase = outerHttpContext.Request.PathBase;
             _emptyRequestsOrder = new List<int>();
-            _referenceIdDictionary = new Dictionary<string, (string resourceId, string resourceType)>();
+            _referenceIdDictionary = new ConcurrentDictionary<string, (string resourceId, string resourceType)>();
 
             // Retrieve bundle processing logic.
             _bundleProcessingLogic = GetBundleProcessingLogic(outerHttpContext, _logger);
@@ -780,8 +781,8 @@ namespace Microsoft.Health.Fhir.Api.Features.Resources.Bundle
                 httpContext.Request.GetDisplayUrl(),
                 requestContext.BaseUri.OriginalString,
                 requestContext.CorrelationId,
-                httpContext.Request.Headers,
-                httpContext.Response.Headers)
+                requestHeaders: httpContext.Request.Headers,
+                responseHeaders: httpContext.Response.Headers)
             {
                 Principal = requestContext.Principal,
                 ResourceType = resourceType?.ToString(),
@@ -839,7 +840,7 @@ namespace Microsoft.Health.Fhir.Api.Features.Resources.Bundle
             }
         }
 
-        private void PopulateReferenceIdDictionary(IEnumerable<EntryComponent> bundleEntries, Dictionary<string, (string resourceId, string resourceType)> idDictionary)
+        private void PopulateReferenceIdDictionary(IEnumerable<EntryComponent> bundleEntries, IDictionary<string, (string resourceId, string resourceType)> idDictionary)
         {
             foreach (EntryComponent entry in bundleEntries)
             {

src/Microsoft.Health.Fhir.Shared.Client/FhirClientException.cs

@@ -10,6 +10,7 @@ using System.Net.Http;
 using System.Net.Http.Headers;
 using System.Text;
 using Hl7.Fhir.Model;
+using Hl7.Fhir.Serialization;
 
 namespace Microsoft.Health.Fhir.Client
 {
@@ -102,6 +103,11 @@ namespace Microsoft.Health.Fhir.Client
             message.Append("Timestamp: ").AppendLine(DateTime.UtcNow.ToString("o"));
             message.Append("Health Check Result: ").Append(HealthCheckResult.ToString()).Append('(').Append((int)HealthCheckResult).AppendLine(")");
 
+            if (Response.Resource != null)
+            {
+                message.Append("OperationOutcome: ").AppendLine(Response.Resource.ToJson());
+            }
+
             if (appendResponseInfo)
             {
                 message.Append("Response Info: ").AppendLine(responseInfo);

src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/ResourceReferenceResolver.cs

@@ -36,7 +36,7 @@ namespace Microsoft.Health.Fhir.Core.Features.Resources
             _queryStringParser = queryStringParser;
         }
 
-        public async Task ResolveReferencesAsync(Resource resource, Dictionary<string, (string resourceId, string resourceType)> referenceIdDictionary, string requestUrl, CancellationToken cancellationToken)
+        public async Task ResolveReferencesAsync(Resource resource, IDictionary<string, (string resourceId, string resourceType)> referenceIdDictionary, string requestUrl, CancellationToken cancellationToken)
         {
             IEnumerable<ResourceReference> references = resource.GetAllChildren<ResourceReference>();
 

test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleBatchTests.cs

@@ -71,7 +71,7 @@ namespace Microsoft.Health.Fhir.Tests.E2E.Rest
 
             if (processingLogic == FhirBundleProcessingLogic.Parallel)
             {
-                // Duplicated records. Only one should successed. As the requests are processed in parallel,
+                // Duplicated records. Only one should succeed. As the requests are processed in parallel,
                 // it's not possible to pick the one that will be processed.
                 if (resource.Entry[2].Response.Status == "200")
                 {