Fix v1-Prerelease Version for Operations Controller (#1322)

The OperationsController was missing version attributes, so it only accepted the latest one. - Add v1-prerelease and v1 support for the OperationsController - Refactor "reference" models in the client project such that the client can easily retrieve the URL - Refactor Json serializer options to be static - Return IReadOnlyList<T> for XQT endpoints - Validate URIs in e2e tests - Fix build configuration for docker compose project
microsoft · Feb 1, 2022 · 38c5ca0 · 38c5ca0
1 parent e00291f
commit 38c5ca0
Show file tree

Hide file tree

Showing 14 changed files with 133 additions and 51 deletions.
diff --git a/Microsoft.Health.Dicom.sln b/Microsoft.Health.Dicom.sln
@@ -57,10 +57,10 @@ Global
 		Release|x64 = Release|x64
 	EndGlobalSection
 	GlobalSection(ProjectConfigurationPlatforms) = postSolution
-		{336B1FB4-EEF8-4E11-BDD5-818983D4E1CD}.Debug|x64.ActiveCfg = Debug|x64
-		{336B1FB4-EEF8-4E11-BDD5-818983D4E1CD}.Debug|x64.Build.0 = Debug|x64
-		{336B1FB4-EEF8-4E11-BDD5-818983D4E1CD}.Release|x64.ActiveCfg = Release|x64
-		{336B1FB4-EEF8-4E11-BDD5-818983D4E1CD}.Release|x64.Build.0 = Release|x64
+		{336B1FB4-EEF8-4E11-BDD5-818983D4E1CD}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{336B1FB4-EEF8-4E11-BDD5-818983D4E1CD}.Debug|x64.Build.0 = Debug|Any CPU
+		{336B1FB4-EEF8-4E11-BDD5-818983D4E1CD}.Release|x64.ActiveCfg = Release|Any CPU
+		{336B1FB4-EEF8-4E11-BDD5-818983D4E1CD}.Release|x64.Build.0 = Release|Any CPU
 		{B0570D75-E376-44AC-870B-87ECB54F0AE3}.Debug|x64.ActiveCfg = Debug|x64
 		{B0570D75-E376-44AC-870B-87ECB54F0AE3}.Debug|x64.Build.0 = Debug|x64
 		{B0570D75-E376-44AC-870B-87ECB54F0AE3}.Release|x64.ActiveCfg = Release|x64
@@ -153,7 +153,7 @@ Global
 		{1D0ECFDA-2AF2-4796-995D-A7C6E18C9CD1} = {8C9A0050-5D22-4398-9F93-DDCD80B3BA51}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
-		RESX_SortFileContentOnSave = True
 		SolutionGuid = {E370FB31-CF95-47D1-B1E1-863A77973FF8}
+		RESX_SortFileContentOnSave = True
 	EndGlobalSection
 EndGlobal
diff --git a/src/Microsoft.Health.Dicom.Api/Controllers/OperationsController.cs b/src/Microsoft.Health.Dicom.Api/Controllers/OperationsController.cs
@@ -25,6 +25,8 @@ namespace Microsoft.Health.Dicom.Api.Controllers
     /// <summary>
     /// Represents the REST API controller for interacting with long-running DICOM operations.
     /// </summary>
+    [ApiVersion("1.0-prerelease")]
+    [ApiVersion("1")]
     [ServiceFilter(typeof(DicomApiAuditLoggingFilterAttribute))]
     public class OperationsController : ControllerBase
     {

diff --git a/src/Microsoft.Health.Dicom.Client/DicomWebClient.ExtendedQueryTag.cs b/src/Microsoft.Health.Dicom.Client/DicomWebClient.ExtendedQueryTag.cs
@@ -49,7 +49,7 @@ public async Task<DicomWebResponse> DeleteExtendedQueryTagAsync(string tagPath,
             return new DicomWebResponse(response);
         }
 
-        public async Task<DicomWebResponse<IEnumerable<GetExtendedQueryTagEntry>>> GetExtendedQueryTagsAsync(int limit, int offset, CancellationToken cancellationToken)
+        public async Task<DicomWebResponse<IReadOnlyList<GetExtendedQueryTagEntry>>> GetExtendedQueryTagsAsync(int limit, int offset, CancellationToken cancellationToken)
         {
             EnsureArg.IsGte(limit, 1, nameof(limit));
             EnsureArg.IsGte(offset, 0, nameof(offset));
@@ -58,7 +58,7 @@ public async Task<DicomWebResponse<IEnumerable<GetExtendedQueryTagEntry>>> GetEx
             using var request = new HttpRequestMessage(HttpMethod.Get, uri);
             HttpResponseMessage response = await HttpClient.SendAsync(request, cancellationToken).ConfigureAwait(false);
             await EnsureSuccessStatusCodeAsync(response).ConfigureAwait(false);
-            return new DicomWebResponse<IEnumerable<GetExtendedQueryTagEntry>>(response, ValueFactory<IEnumerable<GetExtendedQueryTagEntry>>);
+            return new DicomWebResponse<IReadOnlyList<GetExtendedQueryTagEntry>>(response, ValueFactory<IReadOnlyList<GetExtendedQueryTagEntry>>);
         }
 
         public async Task<DicomWebResponse<GetExtendedQueryTagEntry>> GetExtendedQueryTagAsync(string tagPath, CancellationToken cancellationToken)
@@ -73,7 +73,7 @@ public async Task<DicomWebResponse<GetExtendedQueryTagEntry>> GetExtendedQueryTa
             return new DicomWebResponse<GetExtendedQueryTagEntry>(response, ValueFactory<GetExtendedQueryTagEntry>);
         }
 
-        public async Task<DicomWebResponse<IEnumerable<ExtendedQueryTagError>>> GetExtendedQueryTagErrorsAsync(string tagPath, int limit, int offset, CancellationToken cancellationToken)
+        public async Task<DicomWebResponse<IReadOnlyList<ExtendedQueryTagError>>> GetExtendedQueryTagErrorsAsync(string tagPath, int limit, int offset, CancellationToken cancellationToken)
         {
             EnsureArg.IsNotNullOrWhiteSpace(tagPath, nameof(tagPath));
             EnsureArg.IsGte(limit, 1, nameof(limit));
@@ -89,15 +89,15 @@ public async Task<DicomWebResponse<IEnumerable<ExtendedQueryTagError>>> GetExten
             using var request = new HttpRequestMessage(HttpMethod.Get, uri);
             HttpResponseMessage response = await HttpClient.SendAsync(request, cancellationToken).ConfigureAwait(false);
             await EnsureSuccessStatusCodeAsync(response).ConfigureAwait(false);
-            return new DicomWebResponse<IEnumerable<ExtendedQueryTagError>>(response, ValueFactory<IEnumerable<ExtendedQueryTagError>>);
+            return new DicomWebResponse<IReadOnlyList<ExtendedQueryTagError>>(response, ValueFactory<IReadOnlyList<ExtendedQueryTagError>>);
         }
 
         public async Task<DicomWebResponse<GetExtendedQueryTagEntry>> UpdateExtendedQueryTagAsync(string tagPath, UpdateExtendedQueryTagEntry newValue, CancellationToken cancellationToken)
         {
             EnsureArg.IsNotNullOrWhiteSpace(tagPath, nameof(tagPath));
             EnsureArg.IsNotNull(newValue, nameof(newValue));
             EnsureArg.EnumIsDefined(newValue.QueryStatus, nameof(newValue));
-            string jsonString = JsonSerializer.Serialize(newValue, _jsonSerializerOptions);
+            string jsonString = JsonSerializer.Serialize(newValue, JsonSerializerOptions);
             var uri = new Uri($"/{_apiVersion}{DicomWebConstants.BaseExtendedQueryTagUri}/{tagPath}", UriKind.Relative);
 
             using var request = new HttpRequestMessage(HttpMethod.Patch, uri);

diff --git a/src/Microsoft.Health.Dicom.Client/DicomWebClient.Reference.cs b/src/Microsoft.Health.Dicom.Client/DicomWebClient.Reference.cs
@@ -0,0 +1,26 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+using System.Net.Http;
+using System.Threading;
+using System.Threading.Tasks;
+using EnsureThat;
+using Microsoft.Health.Dicom.Client.Models;
+
+namespace Microsoft.Health.Dicom.Client
+{
+    public partial class DicomWebClient : IDicomWebClient
+    {
+        public async Task<DicomWebResponse<T>> ResolveReferenceAsync<T>(IResourceReference<T> resourceReference, CancellationToken cancellationToken = default)
+        {
+            EnsureArg.IsNotNull(resourceReference, nameof(resourceReference));
+
+            using var request = new HttpRequestMessage(HttpMethod.Get, resourceReference.Href);
+            HttpResponseMessage response = await HttpClient.SendAsync(request, cancellationToken).ConfigureAwait(false);
+            await EnsureSuccessStatusCodeAsync(response).ConfigureAwait(false);
+            return new DicomWebResponse<T>(response, ValueFactory<T>);
+        }
+    }
+}
diff --git a/src/Microsoft.Health.Dicom.Client/DicomWebClient.Store.cs b/src/Microsoft.Health.Dicom.Client/DicomWebClient.Store.cs
@@ -143,7 +143,7 @@ await EnsureSuccessStatusCodeAsync(
                             statusCode,
                             responseHeaders,
                             contentHeaders,
-                            JsonSerializer.Deserialize<DicomDataset>(responseBody, _jsonSerializerOptions));
+                            JsonSerializer.Deserialize<DicomDataset>(responseBody, JsonSerializerOptions));
                     }
 
                     return false;

diff --git a/src/Microsoft.Health.Dicom.Client/DicomWebClient.cs b/src/Microsoft.Health.Dicom.Client/DicomWebClient.cs
@@ -27,8 +27,8 @@ namespace Microsoft.Health.Dicom.Client
 {
     public partial class DicomWebClient : IDicomWebClient
     {
-        private readonly JsonSerializerOptions _jsonSerializerOptions;
         private readonly string _apiVersion;
+        internal static readonly JsonSerializerOptions JsonSerializerOptions = CreateJsonSerializerOptions();
 
         /// <summary>
         /// New instance of DicomWebClient to talk to the server
@@ -41,26 +41,6 @@ public DicomWebClient(HttpClient httpClient, string apiVersion = DicomApiVersion
 
             HttpClient = httpClient;
             _apiVersion = apiVersion;
-            _jsonSerializerOptions = new JsonSerializerOptions
-            {
-                AllowTrailingCommas = true,
-                DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
-                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase,
-                Encoder = null,
-                IgnoreReadOnlyFields = false,
-                IgnoreReadOnlyProperties = false,
-                IncludeFields = false,
-                MaxDepth = 0, // 0 indicates the max depth of 64
-                NumberHandling = JsonNumberHandling.Strict,
-                PropertyNameCaseInsensitive = true,
-                PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
-                ReadCommentHandling = JsonCommentHandling.Skip,
-                WriteIndented = false,
-            };
-
-            _jsonSerializerOptions.Converters.Add(new JsonStringEnumConverter());
-            _jsonSerializerOptions.Converters.Add(new DicomJsonConverter(writeTagsAsKeywords: true, autoValidate: false));
-
             GetMemoryStream = () => new MemoryStream();
         }
 
@@ -74,10 +54,10 @@ public DicomWebClient(HttpClient httpClient, string apiVersion = DicomApiVersion
         /// </remarks>
         public Func<MemoryStream> GetMemoryStream { get; set; }
 
-        private async Task<T> ValueFactory<T>(HttpContent content)
+        private static async Task<T> ValueFactory<T>(HttpContent content)
         {
             string contentText = await content.ReadAsStringAsync().ConfigureAwait(false);
-            return JsonSerializer.Deserialize<T>(contentText, _jsonSerializerOptions);
+            return JsonSerializer.Deserialize<T>(contentText, JsonSerializerOptions);
         }
 
         [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Callers will dispose of the StreamContent")]
@@ -223,7 +203,7 @@ private async IAsyncEnumerable<DicomFile> ReadMultipartResponseAsDicomFileAsync(
             }
         }
 
-        private async IAsyncEnumerable<T> DeserializeAsAsyncEnumerable<T>(HttpContent content)
+        private static async IAsyncEnumerable<T> DeserializeAsAsyncEnumerable<T>(HttpContent content)
         {
             string contentText = await content.ReadAsStringAsync().ConfigureAwait(false);
 
@@ -232,10 +212,35 @@ private async IAsyncEnumerable<T> DeserializeAsAsyncEnumerable<T>(HttpContent co
                 yield break;
             }
 
-            foreach (T item in JsonSerializer.Deserialize<IReadOnlyList<T>>(contentText, _jsonSerializerOptions))
+            foreach (T item in JsonSerializer.Deserialize<IReadOnlyList<T>>(contentText, JsonSerializerOptions))
             {
                 yield return item;
             }
         }
+
+        private static JsonSerializerOptions CreateJsonSerializerOptions()
+        {
+            var options = new JsonSerializerOptions
+            {
+                AllowTrailingCommas = true,
+                DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase,
+                Encoder = null,
+                IgnoreReadOnlyFields = false,
+                IgnoreReadOnlyProperties = false,
+                IncludeFields = false,
+                MaxDepth = 0, // 0 indicates the max depth of 64
+                NumberHandling = JsonNumberHandling.Strict,
+                PropertyNameCaseInsensitive = true,
+                PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
+                ReadCommentHandling = JsonCommentHandling.Skip,
+                WriteIndented = false,
+            };
+
+            options.Converters.Add(new JsonStringEnumConverter());
+            options.Converters.Add(new DicomJsonConverter(writeTagsAsKeywords: true, autoValidate: false));
+
+            return options;
+        }
     }
 }
diff --git a/src/Microsoft.Health.Dicom.Client/IDicomWebClient.ExtendedQueryTag.cs b/src/Microsoft.Health.Dicom.Client/IDicomWebClient.ExtendedQueryTag.cs
@@ -20,8 +20,8 @@ public partial interface IDicomWebClient
 
         Task<DicomWebResponse<GetExtendedQueryTagEntry>> UpdateExtendedQueryTagAsync(string tagPath, UpdateExtendedQueryTagEntry newValue, CancellationToken cancellationToken = default);
 
-        Task<DicomWebResponse<IEnumerable<GetExtendedQueryTagEntry>>> GetExtendedQueryTagsAsync(int limit = 100, int offset = 0, CancellationToken cancellationToken = default);
+        Task<DicomWebResponse<IReadOnlyList<GetExtendedQueryTagEntry>>> GetExtendedQueryTagsAsync(int limit = 100, int offset = 0, CancellationToken cancellationToken = default);
 
-        Task<DicomWebResponse<IEnumerable<ExtendedQueryTagError>>> GetExtendedQueryTagErrorsAsync(string tagPath, int limit = 100, int offset = 0, CancellationToken cancellationToken = default);
+        Task<DicomWebResponse<IReadOnlyList<ExtendedQueryTagError>>> GetExtendedQueryTagErrorsAsync(string tagPath, int limit = 100, int offset = 0, CancellationToken cancellationToken = default);
     }
 }
diff --git a/src/Microsoft.Health.Dicom.Client/IDicomWebClient.Reference.cs b/src/Microsoft.Health.Dicom.Client/IDicomWebClient.Reference.cs
@@ -0,0 +1,16 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.Health.Dicom.Client.Models;
+
+namespace Microsoft.Health.Dicom.Client
+{
+    public partial interface IDicomWebClient
+    {
+        Task<DicomWebResponse<T>> ResolveReferenceAsync<T>(IResourceReference<T> resourceReference, CancellationToken cancellationToken = default);
+    }
+}
diff --git a/src/Microsoft.Health.Dicom.Client/Microsoft.Health.Dicom.Client.csproj b/src/Microsoft.Health.Dicom.Client/Microsoft.Health.Dicom.Client.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <Description>Defines a RESTful client for interacting with DICOMweb APIs.</Description>
@@ -14,16 +14,13 @@
     <PackageReference Include="fo-dicom" Version="$(FoDicomVersion)" />
   </ItemGroup>
 
-  <ItemGroup>
-    <None Include="IDicomWebClient.*.cs">
-      <DependentUpon>IDicomWebClient.cs</DependentUpon>
-    </None>
-  </ItemGroup>
-
   <ItemGroup>
     <None Include="DicomWebClient.*.cs">
       <DependentUpon>DicomWebClient.cs</DependentUpon>
     </None>
+    <None Include="IDicomWebClient.*.cs">
+      <DependentUpon>IDicomWebClient.cs</DependentUpon>
+    </None>
   </ItemGroup>
 
 </Project>
diff --git a/src/Microsoft.Health.Dicom.Client/Models/ExtendedQueryTagErrorReference.cs b/src/Microsoft.Health.Dicom.Client/Models/ExtendedQueryTagErrorReference.cs
@@ -4,13 +4,14 @@
 // -------------------------------------------------------------------------------------------------
 
 using System;
+using System.Collections.Generic;
 
 namespace Microsoft.Health.Dicom.Client.Models
 {
     /// <summary>
     /// Represents a reference to a one or more extended query tag errors.
     /// </summary>
-    public class ExtendedQueryTagErrorReference
+    public class ExtendedQueryTagErrorReference : IResourceReference<IReadOnlyList<ExtendedQueryTagError>>
     {
         /// <summary>
         /// Gets or sets the number of errors.

diff --git a/src/Microsoft.Health.Dicom.Client/Models/IResourceReference.cs b/src/Microsoft.Health.Dicom.Client/Models/IResourceReference.cs
@@ -0,0 +1,22 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+using System;
+
+namespace Microsoft.Health.Dicom.Client.Models
+{
+    /// <summary>
+    /// Represents a reference to a RESTful resource.
+    /// </summary>
+    /// <typeparam name="T">The type of resource.</typeparam>
+    public interface IResourceReference<T>
+    {
+        /// <summary>
+        /// Gets the endpoint that returns resources of type <typeparamref name="T"/>.
+        /// </summary>
+        /// <value>The resource URL.</value>
+        Uri Href { get; }
+    }
+}
diff --git a/src/Microsoft.Health.Dicom.Client/Models/OperationReference.cs b/src/Microsoft.Health.Dicom.Client/Models/OperationReference.cs
@@ -10,7 +10,7 @@ namespace Microsoft.Health.Dicom.Client.Models
     /// <summary>
     /// Represents a reference to an existing long-running oepration.
     /// </summary>
-    public class OperationReference
+    public class OperationReference : IResourceReference<OperationStatus>
     {
         /// <summary>
         /// Gets or sets the operation ID.

diff --git a/test/Microsoft.Health.Dicom.Web.Tests.E2E/Common/DicomTagsManager.cs b/test/Microsoft.Health.Dicom.Web.Tests.E2E/Common/DicomTagsManager.cs
@@ -5,7 +5,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using EnsureThat;
@@ -14,6 +13,7 @@
 using Microsoft.Health.Dicom.Web.Tests.E2E.Extensions;
 using Polly;
 using Polly.Retry;
+using Xunit;
 
 namespace Microsoft.Health.Dicom.Web.Tests.E2E.Common
 {
@@ -57,11 +57,20 @@ public async Task<OperationStatus> AddTagsAsync(IEnumerable<AddExtendedQueryTagE
 
             DicomWebResponse<OperationReference> response = await _dicomWebClient.AddExtendedQueryTagAsync(entries, cancellationToken);
             OperationReference operation = await response.GetValueAsync();
-            return await GetOperationStatusRetryPolicy.ExecuteAsync(async () =>
+
+            OperationStatus result = await GetOperationStatusRetryPolicy.ExecuteAsync(async () =>
             {
                 var operationStatus = await _dicomWebClient.GetOperationStatusAsync(operation.Id);
                 return await operationStatus.GetValueAsync();
             });
+
+            // Check reference
+            DicomWebResponse<OperationStatus> actualResponse = await _dicomWebClient.ResolveReferenceAsync(operation, cancellationToken);
+            OperationStatus actual = await actualResponse.GetValueAsync();
+            Assert.Equal(result.OperationId, actual.OperationId);
+            Assert.Equal(result.Status, actual.Status);
+
+            return result;
         }
 
         public async Task<GetExtendedQueryTagEntry> GetTagAsync(string tagPath, CancellationToken cancellationToken = default)
@@ -78,7 +87,7 @@ public async Task<IReadOnlyList<GetExtendedQueryTagEntry>> GetTagsAsync(int limi
             EnsureArg.IsGte(offset, 0, nameof(offset));
 
             var response = await _dicomWebClient.GetExtendedQueryTagsAsync(limit, offset, cancellationToken);
-            return (await response.GetValueAsync()).ToList();
+            return await response.GetValueAsync();
         }
 
         public async Task<IReadOnlyList<ExtendedQueryTagError>> GetTagErrorsAsync(string tagPath, int limit, int offset, CancellationToken cancellationToken = default)
@@ -88,7 +97,7 @@ public async Task<IReadOnlyList<ExtendedQueryTagError>> GetTagErrorsAsync(string
             EnsureArg.IsGte(offset, 0, nameof(offset));
 
             var response = await _dicomWebClient.GetExtendedQueryTagErrorsAsync(tagPath, limit, offset, cancellationToken);
-            return (await response.GetValueAsync()).ToList();
+            return await response.GetValueAsync();
         }
 
         public async Task<GetExtendedQueryTagEntry> UpdateExtendedQueryTagAsync(string tagPath, UpdateExtendedQueryTagEntry newValue, CancellationToken cancellationToken = default)

diff --git a/test/Microsoft.Health.Dicom.Web.Tests.E2E/Rest/ExtendedQueryTagTests.cs b/test/Microsoft.Health.Dicom.Web.Tests.E2E/Rest/ExtendedQueryTagTests.cs
@@ -148,6 +148,10 @@ public async Task GivenExtendedQueryTagWithErrors_WhenReindexing_ThenShouldSucce
             Assert.Equal(errors[0].ErrorMessage, (await _tagManager.GetTagErrorsAsync(tag.GetPath(), 1, 0)).Single().ErrorMessage);
             Assert.Equal(errors[1].ErrorMessage, (await _tagManager.GetTagErrorsAsync(tag.GetPath(), 1, 1)).Single().ErrorMessage);
 
+            // Check that the reference API returns the same values
+            var sameErrors = await _client.ResolveReferenceAsync(actual.Errors);
+            Assert.Equal(errors.Select(x => x.ErrorMessage), (await sameErrors.GetValueAsync()).Select(x => x.ErrorMessage));
+
             var exception = await Assert.ThrowsAsync<DicomWebException>(() => _client.QueryInstancesAsync($"{tag.GetPath()}={tagValue}"));
             Assert.Equal(HttpStatusCode.BadRequest, exception.StatusCode);