Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Policy and HttpPipelineBuilder changes #6517

Merged
merged 2 commits into from
Jun 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Azure.ApplicationModel.Configuration
{
public class ConfigurationClientOptions: HttpClientOptions
public class ConfigurationClientOptions: ClientOptions
{

public FixedRetryPolicy RetryPolicy { get; set; }
Expand Down
23 changes: 0 additions & 23 deletions sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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);
}
}
}

29 changes: 29 additions & 0 deletions sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
22 changes: 19 additions & 3 deletions sdk/core/Azure.Core/src/Pipeline/HttpPipelineOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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);
Expand Down
Original file line number Diff 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
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
{
Expand Down
23 changes: 8 additions & 15 deletions sdk/core/Azure.Core/src/Pipeline/Policies/TelemetryPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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}";
}
}

Expand Down
6 changes: 3 additions & 3 deletions sdk/core/Azure.Core/tests/BufferResponsePolicyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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]
Expand All @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion sdk/core/Azure.Core/tests/ClientRequestIdPolicyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
19 changes: 15 additions & 4 deletions sdk/core/Azure.Core/tests/TelemetryPolicyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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));
Expand All @@ -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);

Expand All @@ -82,5 +88,10 @@ public async Task UsesDefaultApplicationId()
TelemetryPolicy.DefaultApplicationId = null;
}
}

private class TestOptions: ClientOptions
{

}
}
}
2 changes: 1 addition & 1 deletion sdk/core/Azure.Core/tests/TestFramework/TestRecording.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 4 additions & 5 deletions sdk/identity/Azure.Identity/src/IdentityClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/Azure.Identity/src/IdentityClientOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 4 additions & 5 deletions sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeyClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

namespace Azure.Security.KeyVault.Keys
{
public class KeyClientOptions : HttpClientOptions
public class KeyClientOptions : ClientOptions
{
public RetryPolicy RetryPolicy { get; set; }

Expand Down
Loading