Policy and HttpPipelineBuilder changes (#6517)

Azure · Jun 7, 2019 · 68d3b3a · 68d3b3a
1 parent 6d3cabc
commit 68d3b3a
Show file tree

Hide file tree

Showing 19 changed files with 100 additions and 82 deletions.
diff --git a/sdk/appconfiguration/Azure.ApplicationModel.Configuration/src/ConfigurationClient.cs b/sdk/appconfiguration/Azure.ApplicationModel.Configuration/src/ConfigurationClient.cs
@@ -51,13 +51,12 @@ public ConfigurationClient(string connectionString, ConfigurationClientOptions o
 
             ParseConnectionString(connectionString, out _baseUri, out var credential, out var secret);
 
-            _pipeline = HttpPipeline.Build(options,
-                    options.ResponseClassifier,
+            _pipeline = HttpPipelineBuilder.Build(options,
                     options.RetryPolicy,
-                    ClientRequestIdPolicy.Singleton,
+                    ClientRequestIdPolicy.Shared,
                     new AuthenticationPolicy(credential, secret),
                     options.LoggingPolicy,
-                    BufferResponsePolicy.Singleton);
+                    BufferResponsePolicy.Shared);
         }
 
         /// <summary>

diff --git a/sdk/appconfiguration/Azure.ApplicationModel.Configuration/src/ConfigurationClientOptions.cs b/sdk/appconfiguration/Azure.ApplicationModel.Configuration/src/ConfigurationClientOptions.cs
@@ -7,7 +7,7 @@
 
 namespace Azure.ApplicationModel.Configuration
 {
-    public class ConfigurationClientOptions: HttpClientOptions
+    public class ConfigurationClientOptions: ClientOptions
     {
 
         public FixedRetryPolicy RetryPolicy { get; set; }

diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs
@@ -2,13 +2,9 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Collections.Generic;
-using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
-using Azure.Core;
-using Azure.Core.Pipeline.Policies;
 
 namespace Azure.Core.Pipeline
 {
@@ -54,25 +50,6 @@ public Response SendRequest(Request request, CancellationToken cancellationToken
             _pipeline.Span[0].Process(message, _pipeline.Slice(1));
             return message.Response;
         }
-
-        public static HttpPipeline Build(HttpClientOptions options, ResponseClassifier responseClassifier, params HttpPipelinePolicy[] clientPolicies)
-        {
-            var policies = new List<HttpPipelinePolicy>();
-
-            policies.AddRange(options.PerCallPolicies);
-
-            policies.Add(options.TelemetryPolicy);
-
-            policies.AddRange(clientPolicies);
-
-            policies.AddRange(options.PerRetryPolicies);
-
-            policies.Add(options.LoggingPolicy);
-
-            policies.RemoveAll(policy => policy == null);
-
-            return new HttpPipeline(options.Transport, policies.ToArray(), options.ResponseClassifier);
-        }
     }
 }
 
diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs
@@ -0,0 +1,29 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Collections.Generic;
+
+namespace Azure.Core.Pipeline
+{
+    public static class HttpPipelineBuilder
+    {
+        public static HttpPipeline Build(ClientOptions options, params HttpPipelinePolicy[] clientPolicies)
+        {
+            var policies = new List<HttpPipelinePolicy>();
+
+            policies.AddRange(options.PerCallPolicies);
+
+            policies.Add(options.TelemetryPolicy);
+
+            policies.AddRange(clientPolicies);
+
+            policies.AddRange(options.PerRetryPolicies);
+
+            policies.Add(options.LoggingPolicy);
+
+            policies.RemoveAll(policy => policy == null);
+
+            return new HttpPipeline(options.Transport, policies.ToArray(), options.ResponseClassifier);
+        }
+    }
+}
diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineOptions.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineOptions.cs
@@ -4,17 +4,20 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
+using System.Reflection;
 using Azure.Core.Pipeline.Policies;
 
 namespace Azure.Core.Pipeline
 {
-    public class HttpClientOptions
+    public abstract class ClientOptions
     {
         private HttpPipelineTransport _transport = HttpClientTransport.Shared;
 
-        public HttpClientOptions()
+        protected ClientOptions()
         {
-            TelemetryPolicy = new TelemetryPolicy(GetType().Assembly);
+            (string name, string version)= GetComponentNameAndVersion();
+
+            TelemetryPolicy = new TelemetryPolicy(name, version);
             LoggingPolicy = LoggingPolicy.Shared;
         }
 
@@ -33,6 +36,19 @@ public HttpPipelineTransport Transport {
 
         public IList<HttpPipelinePolicy> PerRetryPolicies { get; } = new List<HttpPipelinePolicy>();
 
+        private (string ComponentName, string ComponentVersion) GetComponentNameAndVersion()
+        {
+            Assembly clientAssembly = GetType().Assembly;
+            AzureSdkClientLibraryAttribute componentAttribute = clientAssembly.GetCustomAttribute<AzureSdkClientLibraryAttribute>();
+            if (componentAttribute == null)
+            {
+                throw new InvalidOperationException(
+                    $"{nameof(AzureSdkClientLibraryAttribute)} is required to be set on client SDK assembly '{clientAssembly.FullName}'.");
+            }
+
+            return (componentAttribute.ComponentName, clientAssembly.GetName().Version.ToString());
+        }
+
         #region nobody wants to see these
         [EditorBrowsable(EditorBrowsableState.Never)]
         public override bool Equals(object obj) => base.Equals(obj);

diff --git a/sdk/core/Azure.Core/src/Pipeline/Policies/BufferResponsePolicy.cs b/sdk/core/Azure.Core/src/Pipeline/Policies/BufferResponsePolicy.cs
@@ -13,7 +13,7 @@ protected BufferResponsePolicy()
         {
         }
 
-        public static HttpPipelinePolicy Singleton { get; set; } = new BufferResponsePolicy();
+        public static HttpPipelinePolicy Shared { get; set; } = new BufferResponsePolicy();
 
         public override async Task ProcessAsync(HttpPipelineMessage message, ReadOnlyMemory<HttpPipelinePolicy> pipeline)
         {

diff --git a/sdk/core/Azure.Core/src/Pipeline/Policies/ClientRequestIdPolicy.cs b/sdk/core/Azure.Core/src/Pipeline/Policies/ClientRequestIdPolicy.cs
@@ -1,9 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-using System;
-using System.Threading.Tasks;
-
 namespace Azure.Core.Pipeline.Policies
 {
     public class ClientRequestIdPolicy : SynchronousHttpPipelinePolicy
@@ -15,7 +12,7 @@ protected ClientRequestIdPolicy()
         {
         }
 
-        public static ClientRequestIdPolicy Singleton { get; } = new ClientRequestIdPolicy();
+        public static ClientRequestIdPolicy Shared { get; } = new ClientRequestIdPolicy();
 
         public override void OnSendingRequest(HttpPipelineMessage message)
         {

diff --git a/sdk/core/Azure.Core/src/Pipeline/Policies/TelemetryPolicy.cs b/sdk/core/Azure.Core/src/Pipeline/Policies/TelemetryPolicy.cs
@@ -9,7 +9,9 @@ namespace Azure.Core.Pipeline.Policies
 {
     public class TelemetryPolicy : SynchronousHttpPipelinePolicy
     {
-        private readonly Assembly _clientAssembly;
+        private readonly string _componentName;
+
+        private readonly string _componentVersion;
 
         private string _header;
 
@@ -29,33 +31,24 @@ public string ApplicationId
             }
         }
 
-        public TelemetryPolicy(Assembly clientAssembly)
+        public TelemetryPolicy(string componentName, string componentVersion)
         {
+            _componentName = componentName;
+            _componentVersion = componentVersion;
             _applicationId = DefaultApplicationId;
-            _clientAssembly = clientAssembly;
             InitializeHeader();
         }
 
         private void InitializeHeader()
         {
-            var componentAttribute = _clientAssembly.GetCustomAttribute<AzureSdkClientLibraryAttribute>();
-            if (componentAttribute == null)
-            {
-                throw new InvalidOperationException(
-                    $"{nameof(AzureSdkClientLibraryAttribute)} is required to be set on client SDK assembly '{_clientAssembly.FullName}'.");
-            }
-
-            var componentName = componentAttribute.ComponentName;
-            var componentVersion = _clientAssembly.GetName().Version.ToString();
-
             var platformInformation = $"({RuntimeInformation.FrameworkDescription}; {RuntimeInformation.OSDescription})";
             if (_applicationId != null)
             {
-                _header = $"{_applicationId} azsdk-net-{componentName}/{componentVersion} {platformInformation}";
+                _header = $"{_applicationId} azsdk-net-{_componentName}/{_componentVersion} {platformInformation}";
             }
             else
             {
-                _header = $"azsdk-net-{componentName}/{componentVersion} {platformInformation}";
+                _header = $"azsdk-net-{_componentName}/{_componentVersion} {platformInformation}";
             }
         }
 

diff --git a/sdk/core/Azure.Core/tests/BufferResponsePolicyTests.cs b/sdk/core/Azure.Core/tests/BufferResponsePolicyTests.cs
@@ -22,7 +22,7 @@ public async Task ReadsEntireBodyIntoMemoryStream()
             mockResponse.ContentStream = readTrackingStream;
 
             var mockTransport = CreateMockTransport(mockResponse);
-            var response = await SendGetRequest(mockTransport, BufferResponsePolicy.Singleton);
+            var response = await SendGetRequest(mockTransport, BufferResponsePolicy.Shared);
 
             Assert.IsInstanceOf<MemoryStream>(response.ContentStream);
             var ms = (MemoryStream)response.ContentStream;
@@ -45,7 +45,7 @@ public void SurfacesStreamReadingExceptions()
             };
 
             var mockTransport = CreateMockTransport(mockResponse);
-            Assert.ThrowsAsync<IOException>(async () => await SendGetRequest(mockTransport, BufferResponsePolicy.Singleton));
+            Assert.ThrowsAsync<IOException>(async () => await SendGetRequest(mockTransport, BufferResponsePolicy.Shared));
         }
 
         [Test]
@@ -54,7 +54,7 @@ public async Task SkipsResponsesWithoutContent()
             MockResponse mockResponse = new MockResponse(200);
 
             var mockTransport = CreateMockTransport(mockResponse);
-            var response = await SendGetRequest(mockTransport, BufferResponsePolicy.Singleton);
+            var response = await SendGetRequest(mockTransport, BufferResponsePolicy.Shared);
             Assert.Null(response.ContentStream);
         }
 

diff --git a/sdk/core/Azure.Core/tests/ClientRequestIdPolicyTests.cs b/sdk/core/Azure.Core/tests/ClientRequestIdPolicyTests.cs
@@ -14,7 +14,7 @@ public class ClientRequestIdPolicyTests: PolicyTestBase
         public async Task SetsHeaders()
         {
             var mockTransport = new MockTransport();
-            Task<Response> task = SendGetRequest(mockTransport, ClientRequestIdPolicy.Singleton);
+            Task<Response> task = SendGetRequest(mockTransport, ClientRequestIdPolicy.Shared);
             MockRequest request = await mockTransport.RequestGate.Cycle(new MockResponse(200));
             await task;
 

diff --git a/sdk/core/Azure.Core/tests/TelemetryPolicyTests.cs b/sdk/core/Azure.Core/tests/TelemetryPolicyTests.cs
@@ -5,6 +5,7 @@
 using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Threading.Tasks;
+using Azure.Core.Pipeline;
 using Azure.Core.Pipeline.Policies;
 using Azure.Core.Testing;
 using NUnit.Framework;
@@ -17,7 +18,8 @@ public class TelemetryPolicyTests: PolicyTestBase
         public async Task ComponentNameAndVersionReadFromAssembly()
         {
             var transport = new MockTransport(new MockResponse(200));
-            var telemetryPolicy = new TelemetryPolicy(typeof(TelemetryPolicyTests).Assembly);
+            var testOptions = new TestOptions();
+            var telemetryPolicy = testOptions.TelemetryPolicy;
 
             var assemblyVersion = Assembly.GetExecutingAssembly().GetName().Version.ToString();
             await SendGetRequest(transport, telemetryPolicy);
@@ -30,7 +32,9 @@ public async Task ComponentNameAndVersionReadFromAssembly()
         public async Task ApplicationIdIsIncluded()
         {
             var transport = new MockTransport(new MockResponse(200));
-            var telemetryPolicy = new TelemetryPolicy(typeof(TelemetryPolicyTests).Assembly) { ApplicationId = "application-id" };
+            var testOptions = new TestOptions();
+            var telemetryPolicy = testOptions.TelemetryPolicy;
+            telemetryPolicy.ApplicationId = "application-id";
 
             await SendGetRequest(transport, telemetryPolicy);
 
@@ -50,7 +54,8 @@ public async Task CanDisableTelemetryWithEnvironmentVariable(string value)
                 Environment.SetEnvironmentVariable("AZURE_TELEMETRY_DISABLED", value);
 
                 var transport = new MockTransport(new MockResponse(200));
-                var telemetryPolicy = new TelemetryPolicy(typeof(TelemetryPolicyTests).Assembly);
+                var testOptions = new TestOptions();
+                var telemetryPolicy = testOptions.TelemetryPolicy;
                 await SendGetRequest(transport, telemetryPolicy);
 
                 Assert.False(transport.SingleRequest.TryGetHeader("User-Agent", out var userAgent));
@@ -70,7 +75,8 @@ public async Task UsesDefaultApplicationId()
                 TelemetryPolicy.DefaultApplicationId = "Global-application-id";
 
                 var transport = new MockTransport(new MockResponse(200));
-                var telemetryPolicy = new TelemetryPolicy(typeof(TelemetryPolicyTests).Assembly);
+                var testOptions = new TestOptions();
+                var telemetryPolicy = testOptions.TelemetryPolicy;
 
                 await SendGetRequest(transport, telemetryPolicy);
 
@@ -82,5 +88,10 @@ public async Task UsesDefaultApplicationId()
                 TelemetryPolicy.DefaultApplicationId = null;
             }
         }
+
+        private class TestOptions: ClientOptions
+        {
+
+        }
     }
 }
diff --git a/sdk/core/Azure.Core/tests/TestFramework/TestRecording.cs b/sdk/core/Azure.Core/tests/TestFramework/TestRecording.cs
@@ -120,7 +120,7 @@ public void Dispose()
             }
         }
 
-        public T InstrumentClientOptions<T>(T clientOptions) where T: HttpClientOptions
+        public T InstrumentClientOptions<T>(T clientOptions) where T: ClientOptions
         {
             clientOptions.Transport = CreateTransport(clientOptions.Transport);
             return clientOptions;

diff --git a/sdk/identity/Azure.Identity/src/IdentityClient.cs b/sdk/identity/Azure.Identity/src/IdentityClient.cs
@@ -27,11 +27,10 @@ public IdentityClient(IdentityClientOptions options = null)
         {
             _options = options ?? new IdentityClientOptions();
 
-            _pipeline = HttpPipeline.Build(_options,
-                    _options.ResponseClassifier,
+            _pipeline = HttpPipelineBuilder.Build(_options,
                     _options.RetryPolicy,
-                    ClientRequestIdPolicy.Singleton,
-                    BufferResponsePolicy.Singleton);
+                    ClientRequestIdPolicy.Shared,
+                    BufferResponsePolicy.Shared);
         }
 
         public virtual async Task<AccessToken> AuthenticateAsync(string tenantId, string clientId, string clientSecret, string[] scopes, CancellationToken cancellationToken = default)
@@ -150,7 +149,7 @@ private Request CreateClientSecretAuthRequest(string tenantId, string clientId,
 
             return request;
         }
-        
+
         private async Task<AccessToken> DeserializeAsync(Stream content, CancellationToken cancellationToken)
         {
             using (JsonDocument json = await JsonDocument.ParseAsync(content, default, cancellationToken).ConfigureAwait(false))

diff --git a/sdk/identity/Azure.Identity/src/IdentityClientOptions.cs b/sdk/identity/Azure.Identity/src/IdentityClientOptions.cs
@@ -7,7 +7,7 @@
 
 namespace Azure.Identity
 {
-    public class IdentityClientOptions : HttpClientOptions
+    public class IdentityClientOptions : ClientOptions
     {
         private readonly static Uri DefaultAuthorityHost = new Uri("https://login.microsoftonline.com/");
         private readonly static TimeSpan DefaultRefreshBuffer = TimeSpan.FromMinutes(2);

diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeyClient.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeyClient.cs
@@ -32,13 +32,12 @@ public KeyClient(Uri vaultUri, TokenCredential credential, KeyClientOptions opti
             _vaultUri = vaultUri ?? throw new ArgumentNullException(nameof(credential));
             options = options ?? new KeyClientOptions();
 
-            _pipeline = HttpPipeline.Build(options,
-                    options.ResponseClassifier,
+            _pipeline = HttpPipelineBuilder.Build(options,
                     options.RetryPolicy,
-                    ClientRequestIdPolicy.Singleton,
+                    ClientRequestIdPolicy.Shared,
                     new BearerTokenAuthenticationPolicy(credential, "https://vault.azure.net//.default"),
                     options.LoggingPolicy,
-                    BufferResponsePolicy.Singleton);
+                    BufferResponsePolicy.Shared);
         }
 
         public virtual Response<Key> CreateKey(string name, KeyType keyType, KeyCreateOptions keyOptions = default, CancellationToken cancellationToken = default)
@@ -57,7 +56,7 @@ public virtual async Task<Response<Key>> CreateKeyAsync(string name, KeyType key
             if (keyType == default) throw new ArgumentNullException(nameof(keyType));
 
             var parameters = new KeyRequestParameters(keyType, keyOptions);
-            
+
             return await SendRequestAsync(HttpPipelineMethod.Put, parameters, () => new Key(name), cancellationToken, KeysPath, name, "create");
         }
 

diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeyClientOptions.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeyClientOptions.cs
@@ -8,7 +8,7 @@
 
 namespace Azure.Security.KeyVault.Keys
 {
-    public class KeyClientOptions : HttpClientOptions
+    public class KeyClientOptions : ClientOptions
     {
         public RetryPolicy RetryPolicy { get; set; }
-Original file line number
+Diff line change
@@ Expand Up / @@ -13,7 +13,7 @@ protected BufferResponsePolicy() @@
             {
             }
-            public static HttpPipelinePolicy Singleton { get; set; } = new BufferResponsePolicy();
+            public static HttpPipelinePolicy Shared { get; set; } = new BufferResponsePolicy();
             public override async Task ProcessAsync(HttpPipelineMessage message, ReadOnlyMemory<HttpPipelinePolicy> pipeline)
             {
@@ Expand Down @@