From 3f6bf74a2a3dffed793339f1069ba085d011761d Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 5 Nov 2021 15:39:21 -0700 Subject: [PATCH 01/26] add test for AddPolicy --- .../tests/LowLevelClientTests.cs | 55 +++++++++++++++++++ sdk/core/Azure.Core/src/RequestContext.cs | 27 +++++++++ 2 files changed, 82 insertions(+) diff --git a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs index 95ca5c724d6c..263a13f02f46 100644 --- a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs +++ b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs @@ -211,6 +211,37 @@ public void ThrowOnErrorThrowsOnError() }); } + [Test] + public void CanAddPerCallPolicy() + { + } + + [Test] + public async Task CanAddPerRetryPolicy() + { + var mockResponse = new MockResponse(200); + + Pet pet = new("snoopy", "beagle"); + mockResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); + + var mockTransport = new MockTransport(mockResponse); + PetStoreClient client = CreateClient(mockTransport); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("MyHeader", "MyValue"), HttpPipelinePosition.PerRetry); + + Response response = await client.GetPetAsync("snoopy", new RequestContext()); + + Request request = mockTransport.Requests[0]; + Assert.IsTrue(request.Headers.TryGetValue("MyHeader", out string headerValue)); + Assert.AreEqual("MyValue", headerValue); + } + + [Test] + public void CanAddBeforeTransportPolicy() + { + } + #region Helpers private void SerializePet(ref Utf8JsonWriter writer, Pet pet) { @@ -224,6 +255,30 @@ private void SerializePet(ref Utf8JsonWriter writer, Pet pet) writer.WriteEndObject(); } + + public class AddHeaderPolicy : HttpPipelinePolicy + { + private string _headerName; + private string _headerVaue; + + public AddHeaderPolicy(string headerName, string headerValue) : base() + { + _headerName = headerName; + _headerVaue = headerValue; + } + + public override void Process(HttpMessage message, ReadOnlyMemory pipeline) + { + message.Request.Headers.Add(_headerName, _headerVaue); + ProcessNext(message, pipeline); + } + + public override ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory pipeline) + { + message.Request.Headers.Add(_headerName, _headerVaue); + return ProcessNextAsync(message, pipeline); + } + } #endregion } } diff --git a/sdk/core/Azure.Core/src/RequestContext.cs b/sdk/core/Azure.Core/src/RequestContext.cs index e3322fed7109..1d673973f139 100644 --- a/sdk/core/Azure.Core/src/RequestContext.cs +++ b/sdk/core/Azure.Core/src/RequestContext.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Threading; using Azure.Core; using Azure.Core.Pipeline; @@ -13,6 +14,10 @@ namespace Azure /// public class RequestContext { + private readonly List _perRetryPolicies = new(); + private readonly List _perCallPolicies = new(); + private readonly List _beforeTrasportPolicies = new(); + /// /// Initializes a new instance of the class. /// @@ -35,5 +40,27 @@ public RequestContext() /// Controls under what conditions the operation raises an exception if the underlying response indicates a failure. /// public ErrorOptions ErrorOptions { get; set; } = ErrorOptions.Default; + + /// + /// + /// + /// + public void AddPolicy(HttpPipelinePolicy policy, HttpPipelinePosition position) + { + switch (position) + { + case HttpPipelinePosition.PerCall: + _perCallPolicies.Add(policy); + break; + case HttpPipelinePosition.PerRetry: + _perRetryPolicies.Add(policy); + break; + case HttpPipelinePosition.BeforeTransport: + _beforeTrasportPolicies.Add(policy); + break; + default: + break; + } + } } } From 3343e315a5be98be8d222cb376ded61cbbfcd8bc Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Mon, 8 Nov 2021 11:19:12 -0800 Subject: [PATCH 02/26] share proposed refactoring to HttpPipelineExtensions --- .../src/HttpPipelineExtensions.cs | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs diff --git a/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs b/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs new file mode 100644 index 000000000000..c7206cfb3edf --- /dev/null +++ b/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Threading; +using System.Threading.Tasks; +using Azure.Core.Pipeline; + +namespace Azure.Core.Experimental +{ + internal static class HttpPipelineExtensions + { + public static async ValueTask ProcessMessageAsync(this HttpPipeline pipeline, HttpMessage message, ClientDiagnostics clientDiagnostics, RequestContext? requestContext, CancellationToken cancellationToken = default) + { + ErrorOptions errorOptions = ErrorOptions.Default; + CancellationToken operationCancellationToken = CancellationToken.None; + + if (requestContext != null) + { + errorOptions = requestContext.ErrorOptions; + operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); + } + + await pipeline.SendAsync(message, operationCancellationToken).ConfigureAwait(false); + + if (errorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) + { + return message.Response; + } + + throw await clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); + } + + public static Response ProcessMessage(this HttpPipeline pipeline, HttpMessage message, ClientDiagnostics clientDiagnostics, RequestContext? requestContext, CancellationToken cancellationToken = default) + { + ErrorOptions errorOptions = ErrorOptions.Default; + CancellationToken operationCancellationToken = CancellationToken.None; + + if (requestContext != null) + { + errorOptions = requestContext.ErrorOptions; + operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); + } + + pipeline.Send(message, operationCancellationToken); + + if (errorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) + { + return message.Response; + } + + throw clientDiagnostics.CreateRequestFailedException(message.Response); + } + + private static CancellationToken MergeCancellationTokens(RequestContext context, CancellationToken cancellationToken) + { + if (context.CancellationToken.CanBeCanceled && cancellationToken.CanBeCanceled) + { + using var cts = CancellationTokenSource.CreateLinkedTokenSource(context.CancellationToken, cancellationToken); + return cts.Token; + } + + if (cancellationToken.CanBeCanceled) + { + return cancellationToken; + } + + return context.CancellationToken; + } + } +} From 83388fa14a9ff7fbec211883783982091a2a87b8 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Mon, 8 Nov 2021 17:27:53 -0800 Subject: [PATCH 03/26] ideas in the direction of implementation --- .../src/HttpPipelineExtensions.cs | 15 ++-- sdk/core/Azure.Core/src/HttpMessage.cs | 10 +++ .../Azure.Core/src/Pipeline/HttpPipeline.cs | 69 ++++++++++++++++++- .../src/Pipeline/HttpPipelineBuilder.cs | 11 +++ sdk/core/Azure.Core/src/RequestContext.cs | 24 +++++++ 5 files changed, 118 insertions(+), 11 deletions(-) diff --git a/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs b/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs index c7206cfb3edf..caf105ab5eb0 100644 --- a/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs +++ b/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs @@ -11,18 +11,14 @@ internal static class HttpPipelineExtensions { public static async ValueTask ProcessMessageAsync(this HttpPipeline pipeline, HttpMessage message, ClientDiagnostics clientDiagnostics, RequestContext? requestContext, CancellationToken cancellationToken = default) { - ErrorOptions errorOptions = ErrorOptions.Default; - CancellationToken operationCancellationToken = CancellationToken.None; - - if (requestContext != null) - { - errorOptions = requestContext.ErrorOptions; - operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); - } + // Note: this is thinking about how this could be refactored. + requestContext ??= new RequestContext(); + CancellationToken operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); + message.Apply(requestContext); await pipeline.SendAsync(message, operationCancellationToken).ConfigureAwait(false); - if (errorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) + if (requestContext.ErrorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) { return message.Response; } @@ -55,6 +51,7 @@ private static CancellationToken MergeCancellationTokens(RequestContext context, { if (context.CancellationToken.CanBeCanceled && cancellationToken.CanBeCanceled) { + // TODO: This is disposed when the method returns -- solve this. using var cts = CancellationTokenSource.CreateLinkedTokenSource(context.CancellationToken, cancellationToken); return cts.Token; } diff --git a/sdk/core/Azure.Core/src/HttpMessage.cs b/sdk/core/Azure.Core/src/HttpMessage.cs index 6a16bb58e694..9793f17ba0ad 100644 --- a/sdk/core/Azure.Core/src/HttpMessage.cs +++ b/sdk/core/Azure.Core/src/HttpMessage.cs @@ -123,6 +123,16 @@ public void SetProperty(string name, object value) } } + public void Apply(RequestContext context) + { + if (context.PolicyCount > 0) + { + CustomizedPipeline = // Do we want this on this type? It would be like ResponseClassifier + } + } + + internal ReadOnlyMemory? CustomizedPipeline { get; private set; } + /// /// Disposes the request and response. /// diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index c283475c0d86..bfb379273c76 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -19,6 +19,10 @@ public class HttpPipeline private readonly ReadOnlyMemory _pipeline; + private readonly int _perCallIndex; + private readonly int _perRetryIndex; + private readonly int _transportIndex; + /// /// Creates a new instance of with the provided transport, policies and response classifier. /// @@ -36,9 +40,17 @@ public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? polic all[policies.Length] = new HttpPipelineTransportPolicy(_transport); policies.CopyTo(all, 0); + _transportIndex = policies.Length; + _pipeline = all; } + internal HttpPipeline(HttpPipelineTransport transport, int perCallIndex, int perRetryIndex, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null) : this(transport, policies, responseClassifier) + { + _perCallIndex = perCallIndex; + _perRetryIndex = perRetryIndex; + } + /// /// Creates a new instance. /// @@ -70,7 +82,8 @@ public ValueTask SendAsync(HttpMessage message, CancellationToken cancellationTo { message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - return _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)); + var pipeline = message.CustomizedPipeline ?? _pipeline; + return pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); } /// @@ -82,8 +95,37 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) { message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - _pipeline.Span[0].Process(message, _pipeline.Slice(1)); + var pipeline = message.CustomizedPipeline ?? _pipeline; + pipeline.Span[0].Process(message, pipeline.Slice(1)); } + + ///// + ///// Invokes the pipeline asynchronously. After the task completes response would be set to the property. + ///// + ///// The to send. + ///// + ///// The to use. + ///// The representing the asynchronous operation. + //public static ValueTask SendAsync(HttpMessage message, ReadOnlyMemory pipeline, CancellationToken cancellationToken) + //{ + // message.CancellationToken = cancellationToken; + // AddHttpMessageProperties(message); + // return pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); + //} + + ///// + ///// Invokes the pipeline synchronously. After the task completes response would be set to the property. + ///// + ///// The to send. + ///// + ///// The to use. + //public static void Send(HttpMessage message, ReadOnlyMemory pipeline, CancellationToken cancellationToken) + //{ + // message.CancellationToken = cancellationToken; + // AddHttpMessageProperties(message); + // pipeline.Span[0].Process(message, pipeline.Slice(1)); + //} + /// /// Invokes the pipeline asynchronously with the provided request. /// @@ -144,6 +186,29 @@ public static IDisposable CreateHttpMessagePropertiesScope(IDictionary GetCustomizedPipeline(RequestContext context) + { + int length = _pipeline.Length + context.PolicyCount; + + var policies = new HttpPipelinePolicy[length]; + + _pipeline.Slice(0, _perCallIndex).CopyTo(policies); + + int customCount = context.AppendPolicies(policies, HttpPipelinePosition.PerCall, policies.Length); + int sectionCount = _perRetryIndex - _perCallIndex; + _pipeline.Slice(_perCallIndex, sectionCount).CopyTo(new Memory(policies, policies.Length, sectionCount)); + + customCount += context.AppendPolicies(policies, HttpPipelinePosition.PerRetry, policies.Length); + sectionCount = _transportIndex - _perRetryIndex; + _pipeline.Slice(_perRetryIndex, sectionCount).CopyTo(new Memory(policies, policies.Length, sectionCount)); + + customCount += context.AppendPolicies(policies, HttpPipelinePosition.BeforeTransport, policies.Length); + sectionCount = _pipeline.Length - _transportIndex; + _pipeline.Slice(_transportIndex, sectionCount).CopyTo(new Memory(policies, policies.Length, sectionCount)); + + return new ReadOnlyMemory(policies); + } + private static void AddHttpMessageProperties(HttpMessage message) { if (CurrentHttpMessagePropertiesScope.Value != null) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index 7f1e03718141..13eb877841fb 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -34,6 +34,9 @@ public static HttpPipeline Build(ClientOptions options, params HttpPipelinePolic /// A new instance of public static HttpPipeline Build(ClientOptions options, HttpPipelinePolicy[] perCallPolicies, HttpPipelinePolicy[] perRetryPolicies, ResponseClassifier responseClassifier) { + int perCallIndex; + int perRetryIndex; + if (perCallPolicies == null) { throw new ArgumentNullException(nameof(perCallPolicies)); @@ -75,6 +78,9 @@ void AddCustomerPolicies(HttpPipelinePosition position) AddCustomerPolicies(HttpPipelinePosition.PerCall); + policies.RemoveAll(static policy => policy == null); + perCallIndex = policies.Count; + policies.Add(ClientRequestIdPolicy.Shared); if (diagnostics.IsTelemetryEnabled) @@ -91,6 +97,9 @@ void AddCustomerPolicies(HttpPipelinePosition position) AddCustomerPolicies(HttpPipelinePosition.PerRetry); + policies.RemoveAll(static policy => policy == null); + perRetryIndex = policies.Count; + if (diagnostics.IsLoggingEnabled) { string assemblyName = options.GetType().Assembly!.GetName().Name!; @@ -107,6 +116,8 @@ void AddCustomerPolicies(HttpPipelinePosition position) policies.RemoveAll(static policy => policy == null); return new HttpPipeline(options.Transport, + perCallIndex, + perRetryIndex, policies.ToArray(), responseClassifier); } diff --git a/sdk/core/Azure.Core/src/RequestContext.cs b/sdk/core/Azure.Core/src/RequestContext.cs index 1d673973f139..cb820f553249 100644 --- a/sdk/core/Azure.Core/src/RequestContext.cs +++ b/sdk/core/Azure.Core/src/RequestContext.cs @@ -18,6 +18,8 @@ public class RequestContext private readonly List _perCallPolicies = new(); private readonly List _beforeTrasportPolicies = new(); + internal int PolicyCount => _perRetryPolicies.Count + _perCallPolicies.Count + _beforeTrasportPolicies.Count; + /// /// Initializes a new instance of the class. /// @@ -51,16 +53,38 @@ public void AddPolicy(HttpPipelinePolicy policy, HttpPipelinePosition position) { case HttpPipelinePosition.PerCall: _perCallPolicies.Add(policy); + HasPolicies = true; break; case HttpPipelinePosition.PerRetry: _perRetryPolicies.Add(policy); + HasPolicies = true; break; case HttpPipelinePosition.BeforeTransport: _beforeTrasportPolicies.Add(policy); + HasPolicies = true; break; default: break; } } + + internal int AppendPolicies(HttpPipelinePolicy[] target, HttpPipelinePosition position, int start) + { + var source = position switch + { + HttpPipelinePosition.PerCall => _perCallPolicies, + HttpPipelinePosition.PerRetry => _perRetryPolicies, + HttpPipelinePosition.BeforeTransport => _beforeTrasportPolicies, + _ => throw new NotSupportedException("Unexpected 'position' value.") + }; + + int i = 0; + foreach (var policy in source) + { + target[start + i++] = policy; + } + + return i; + } } } From f15d1e935865b069b795bf2809ab2370934215f7 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 9 Nov 2021 14:50:50 -0800 Subject: [PATCH 04/26] mark retry policy --- sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs | 9 +++------ sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs | 2 -- sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs | 5 +++++ sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs | 1 + 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index bfb379273c76..4a4e79412d00 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using System.Linq; namespace Azure.Core.Pipeline { @@ -40,17 +41,13 @@ public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? polic all[policies.Length] = new HttpPipelineTransportPolicy(_transport); policies.CopyTo(all, 0); + _perCallIndex = 0; + _perRetryIndex = policies.ToList().FindIndex(p => p.IsRetryPolicy) + 1; _transportIndex = policies.Length; _pipeline = all; } - internal HttpPipeline(HttpPipelineTransport transport, int perCallIndex, int perRetryIndex, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null) : this(transport, policies, responseClassifier) - { - _perCallIndex = perCallIndex; - _perRetryIndex = perRetryIndex; - } - /// /// Creates a new instance. /// diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index 13eb877841fb..ac2c7d58cbd7 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -116,8 +116,6 @@ void AddCustomerPolicies(HttpPipelinePosition position) policies.RemoveAll(static policy => policy == null); return new HttpPipeline(options.Transport, - perCallIndex, - perRetryIndex, policies.ToArray(), responseClassifier); } diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs index 1de64f1a73ea..420931248d1b 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs @@ -13,6 +13,11 @@ namespace Azure.Core.Pipeline /// public abstract class HttpPipelinePolicy { + /// + /// Indicates whether this policy implements the pipeline's retry policy. + /// + public bool IsRetryPolicy { get; protected set; } + /// /// Applies the policy to the . Implementers are expected to mutate before calling and observe the changes after. /// diff --git a/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs b/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs index 899e91949470..68d7dc921837 100644 --- a/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs +++ b/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs @@ -26,6 +26,7 @@ public RetryPolicy(RetryMode mode, TimeSpan delay, TimeSpan maxDelay, int maxRet _delay = delay; _maxDelay = maxDelay; _maxRetries = maxRetries; + IsRetryPolicy = true; } private const string RetryAfterHeaderName = "Retry-After"; From c5128a6226e7671e38ace9db033754bde0d3993d Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 10 Nov 2021 11:47:58 -0800 Subject: [PATCH 05/26] revert mark retry policy --- sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs | 9 +++++++-- sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs | 2 ++ sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs | 5 ----- sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs | 1 - 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 4a4e79412d00..86c1d119ed27 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -20,6 +20,7 @@ public class HttpPipeline private readonly ReadOnlyMemory _pipeline; + private bool _builderConstructed; private readonly int _perCallIndex; private readonly int _perRetryIndex; private readonly int _transportIndex; @@ -41,12 +42,16 @@ public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? polic all[policies.Length] = new HttpPipelineTransportPolicy(_transport); policies.CopyTo(all, 0); - _perCallIndex = 0; - _perRetryIndex = policies.ToList().FindIndex(p => p.IsRetryPolicy) + 1; _transportIndex = policies.Length; _pipeline = all; } + internal HttpPipeline(HttpPipelineTransport transport, int perCallIndex, int perRetryIndex, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null) : this(transport, policies, responseClassifier) + { + _perCallIndex = perCallIndex; + _perRetryIndex = perRetryIndex; + _builderConstructed = true; + } /// /// Creates a new instance. diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index ac2c7d58cbd7..13eb877841fb 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -116,6 +116,8 @@ void AddCustomerPolicies(HttpPipelinePosition position) policies.RemoveAll(static policy => policy == null); return new HttpPipeline(options.Transport, + perCallIndex, + perRetryIndex, policies.ToArray(), responseClassifier); } diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs index 420931248d1b..1de64f1a73ea 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelinePolicy.cs @@ -13,11 +13,6 @@ namespace Azure.Core.Pipeline /// public abstract class HttpPipelinePolicy { - /// - /// Indicates whether this policy implements the pipeline's retry policy. - /// - public bool IsRetryPolicy { get; protected set; } - /// /// Applies the policy to the . Implementers are expected to mutate before calling and observe the changes after. /// diff --git a/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs b/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs index 68d7dc921837..899e91949470 100644 --- a/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs +++ b/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs @@ -26,7 +26,6 @@ public RetryPolicy(RetryMode mode, TimeSpan delay, TimeSpan maxDelay, int maxRet _delay = delay; _maxDelay = maxDelay; _maxRetries = maxRetries; - IsRetryPolicy = true; } private const string RetryAfterHeaderName = "Retry-After"; From f378649098d75d4c22f30aa0b17ce1764b5e68d9 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 11 Nov 2021 08:50:26 -0800 Subject: [PATCH 06/26] shifting API around --- sdk/core/Azure.Core/src/HttpMessage.cs | 12 +-- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 84 +++++++++++-------- sdk/core/Azure.Core/src/RequestContext.cs | 45 +--------- 3 files changed, 53 insertions(+), 88 deletions(-) diff --git a/sdk/core/Azure.Core/src/HttpMessage.cs b/sdk/core/Azure.Core/src/HttpMessage.cs index 9793f17ba0ad..c0a63bd23950 100644 --- a/sdk/core/Azure.Core/src/HttpMessage.cs +++ b/sdk/core/Azure.Core/src/HttpMessage.cs @@ -81,6 +81,8 @@ public Response Response /// public TimeSpan? NetworkTimeout { get; set; } + internal List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)>? Policies { get; set; } + /// /// Gets a property that modifies the pipeline behavior. Please refer to individual policies documentation on what properties it supports. /// @@ -123,16 +125,6 @@ public void SetProperty(string name, object value) } } - public void Apply(RequestContext context) - { - if (context.PolicyCount > 0) - { - CustomizedPipeline = // Do we want this on this type? It would be like ResponseClassifier - } - } - - internal ReadOnlyMemory? CustomizedPipeline { get; private set; } - /// /// Disposes the request and response. /// diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 86c1d119ed27..52e68cec0378 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -69,6 +69,19 @@ public HttpMessage CreateMessage() return new HttpMessage(CreateRequest(), ResponseClassifier); } + /// + /// Creates a new instance. + /// + /// Context specifying the message options. + /// The message. + public HttpMessage CreateMessage(RequestContext context) + { + var message = CreateMessage(); + message.CancellationToken = context.CancellationToken; + message.Policies = context.Policies; + return message; + } + /// /// The instance used in this pipeline invocations. /// @@ -84,7 +97,7 @@ public ValueTask SendAsync(HttpMessage message, CancellationToken cancellationTo { message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - var pipeline = message.CustomizedPipeline ?? _pipeline; + var pipeline = message.Policies == null ? _pipeline : CreateRequestPipeline(message.Policies); return pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); } @@ -97,37 +110,10 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) { message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - var pipeline = message.CustomizedPipeline ?? _pipeline; + var pipeline = message.Policies == null ? _pipeline : CreateRequestPipeline(message.Policies); pipeline.Span[0].Process(message, pipeline.Slice(1)); } - ///// - ///// Invokes the pipeline asynchronously. After the task completes response would be set to the property. - ///// - ///// The to send. - ///// - ///// The to use. - ///// The representing the asynchronous operation. - //public static ValueTask SendAsync(HttpMessage message, ReadOnlyMemory pipeline, CancellationToken cancellationToken) - //{ - // message.CancellationToken = cancellationToken; - // AddHttpMessageProperties(message); - // return pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); - //} - - ///// - ///// Invokes the pipeline synchronously. After the task completes response would be set to the property. - ///// - ///// The to send. - ///// - ///// The to use. - //public static void Send(HttpMessage message, ReadOnlyMemory pipeline, CancellationToken cancellationToken) - //{ - // message.CancellationToken = cancellationToken; - // AddHttpMessageProperties(message); - // pipeline.Span[0].Process(message, pipeline.Slice(1)); - //} - /// /// Invokes the pipeline asynchronously with the provided request. /// @@ -188,29 +174,55 @@ public static IDisposable CreateHttpMessagePropertiesScope(IDictionary GetCustomizedPipeline(RequestContext context) + private ReadOnlyMemory CreateRequestPipeline(List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> customPolicies) { - int length = _pipeline.Length + context.PolicyCount; - - var policies = new HttpPipelinePolicy[length]; + var policies = new HttpPipelinePolicy[_pipeline.Length + customPolicies.Count]; + + //void AddRequestPolicies(HttpPipelinePosition position, int start) + //{ + // if (customPolicies != null) + // { + // int i = 0; + // foreach (var policy in customPolicies) + // { + // if (policy.Position == position) + // { + // policies[start + i++] = policy.Policy; + // } + // } + // } + //} _pipeline.Slice(0, _perCallIndex).CopyTo(policies); - int customCount = context.AppendPolicies(policies, HttpPipelinePosition.PerCall, policies.Length); + //AddRequestPolicies(HttpPipelinePosition.PerCall, ) + SetRequestPolicies(customPolicies, policies, HttpPipelinePosition.PerCall, policies.Length); int sectionCount = _perRetryIndex - _perCallIndex; _pipeline.Slice(_perCallIndex, sectionCount).CopyTo(new Memory(policies, policies.Length, sectionCount)); - customCount += context.AppendPolicies(policies, HttpPipelinePosition.PerRetry, policies.Length); + SetRequestPolicies(customPolicies, policies, HttpPipelinePosition.PerRetry, policies.Length); sectionCount = _transportIndex - _perRetryIndex; _pipeline.Slice(_perRetryIndex, sectionCount).CopyTo(new Memory(policies, policies.Length, sectionCount)); - customCount += context.AppendPolicies(policies, HttpPipelinePosition.BeforeTransport, policies.Length); + SetRequestPolicies(customPolicies, policies, HttpPipelinePosition.BeforeTransport, policies.Length); sectionCount = _pipeline.Length - _transportIndex; _pipeline.Slice(_transportIndex, sectionCount).CopyTo(new Memory(policies, policies.Length, sectionCount)); return new ReadOnlyMemory(policies); } + private static void SetRequestPolicies(List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> source, + HttpPipelinePolicy[] target, HttpPipelinePosition position, int start) + { + var policies = source.Where(p => p.Position == position).Select(p => p.Policy); + + int i = 0; + foreach (var policy in policies) + { + target[start + i++] = policy; + } + } + private static void AddHttpMessageProperties(HttpMessage message) { if (CurrentHttpMessagePropertiesScope.Value != null) diff --git a/sdk/core/Azure.Core/src/RequestContext.cs b/sdk/core/Azure.Core/src/RequestContext.cs index cb820f553249..2f26d1f333d4 100644 --- a/sdk/core/Azure.Core/src/RequestContext.cs +++ b/sdk/core/Azure.Core/src/RequestContext.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System; using System.Collections.Generic; using System.Threading; using Azure.Core; @@ -14,11 +13,7 @@ namespace Azure /// public class RequestContext { - private readonly List _perRetryPolicies = new(); - private readonly List _perCallPolicies = new(); - private readonly List _beforeTrasportPolicies = new(); - - internal int PolicyCount => _perRetryPolicies.Count + _perCallPolicies.Count + _beforeTrasportPolicies.Count; + internal List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)>? Policies { get; private set; } /// /// Initializes a new instance of the class. @@ -49,42 +44,8 @@ public RequestContext() /// public void AddPolicy(HttpPipelinePolicy policy, HttpPipelinePosition position) { - switch (position) - { - case HttpPipelinePosition.PerCall: - _perCallPolicies.Add(policy); - HasPolicies = true; - break; - case HttpPipelinePosition.PerRetry: - _perRetryPolicies.Add(policy); - HasPolicies = true; - break; - case HttpPipelinePosition.BeforeTransport: - _beforeTrasportPolicies.Add(policy); - HasPolicies = true; - break; - default: - break; - } - } - - internal int AppendPolicies(HttpPipelinePolicy[] target, HttpPipelinePosition position, int start) - { - var source = position switch - { - HttpPipelinePosition.PerCall => _perCallPolicies, - HttpPipelinePosition.PerRetry => _perRetryPolicies, - HttpPipelinePosition.BeforeTransport => _beforeTrasportPolicies, - _ => throw new NotSupportedException("Unexpected 'position' value.") - }; - - int i = 0; - foreach (var policy in source) - { - target[start + i++] = policy; - } - - return i; + Policies ??= new(); + Policies.Add((position, policy)); } } } From d4f88c8164663f4f268028d7177ea82b5cf1d73d Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 11 Nov 2021 13:19:06 -0800 Subject: [PATCH 07/26] experiment with using MemoryPool --- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 52e68cec0378..16cf7691c667 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -2,10 +2,11 @@ // Licensed under the MIT License. using System; +using System.Buffers; using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; -using System.Linq; namespace Azure.Core.Pipeline { @@ -93,12 +94,22 @@ public HttpMessage CreateMessage(RequestContext context) /// The to send. /// The to use. /// The representing the asynchronous operation. - public ValueTask SendAsync(HttpMessage message, CancellationToken cancellationToken) + public async ValueTask SendAsync(HttpMessage message, CancellationToken cancellationToken) { message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - var pipeline = message.Policies == null ? _pipeline : CreateRequestPipeline(message.Policies); - return pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); + + if (message.Policies == null) + { + await _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)).ConfigureAwait(false); + } + else + { + var length = _pipeline.Length + message.Policies.Count; + var pool = MemoryPool.Shared; + using var pipeline = pool.Rent(length); + await pipeline.Memory.Span[0].ProcessAsync(message, pipeline.Memory.Slice(1)).ConfigureAwait(false); + } } /// From b6cb9d5631e708c924f9c14c67528418d29914ef Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 11 Nov 2021 15:06:09 -0800 Subject: [PATCH 08/26] use ArrayPool --- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 70 ++++++++----------- 1 file changed, 28 insertions(+), 42 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 16cf7691c667..44c746713618 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -106,9 +106,10 @@ public async ValueTask SendAsync(HttpMessage message, CancellationToken cancella else { var length = _pipeline.Length + message.Policies.Count; - var pool = MemoryPool.Shared; - using var pipeline = pool.Rent(length); - await pipeline.Memory.Span[0].ProcessAsync(message, pipeline.Memory.Slice(1)).ConfigureAwait(false); + var pipeline = ArrayPool.Shared.Rent(length); + CreateRequestPipeline(pipeline, message.Policies); + await pipeline[0].ProcessAsync(message, new ReadOnlyMemory(pipeline, 1, length - 1)).ConfigureAwait(false); + ArrayPool.Shared.Return(pipeline); } } @@ -185,55 +186,40 @@ public static IDisposable CreateHttpMessagePropertiesScope(IDictionary CreateRequestPipeline(List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> customPolicies) + private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePolicy[] policies, List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> customPolicies) { - var policies = new HttpPipelinePolicy[_pipeline.Length + customPolicies.Count]; - - //void AddRequestPolicies(HttpPipelinePosition position, int start) - //{ - // if (customPolicies != null) - // { - // int i = 0; - // foreach (var policy in customPolicies) - // { - // if (policy.Position == position) - // { - // policies[start + i++] = policy.Policy; - // } - // } - // } - //} + void AddRequestPolicies(HttpPipelinePosition position, int start) + { + if (customPolicies != null) + { + int i = 0; + foreach (var policy in customPolicies) + { + if (policy.Position == position) + { + policies[start + i++] = policy.Policy; + } + } + } + } _pipeline.Slice(0, _perCallIndex).CopyTo(policies); - //AddRequestPolicies(HttpPipelinePosition.PerCall, ) - SetRequestPolicies(customPolicies, policies, HttpPipelinePosition.PerCall, policies.Length); - int sectionCount = _perRetryIndex - _perCallIndex; - _pipeline.Slice(_perCallIndex, sectionCount).CopyTo(new Memory(policies, policies.Length, sectionCount)); + AddRequestPolicies(HttpPipelinePosition.PerCall, _perCallIndex); + int count = _perRetryIndex - _perCallIndex; + _pipeline.Slice(_perCallIndex, count).CopyTo(new Memory(policies, policies.Length, count)); - SetRequestPolicies(customPolicies, policies, HttpPipelinePosition.PerRetry, policies.Length); - sectionCount = _transportIndex - _perRetryIndex; - _pipeline.Slice(_perRetryIndex, sectionCount).CopyTo(new Memory(policies, policies.Length, sectionCount)); + AddRequestPolicies(HttpPipelinePosition.PerRetry, _perRetryIndex); + count = _transportIndex - _perRetryIndex; + _pipeline.Slice(_perRetryIndex, count).CopyTo(new Memory(policies, policies.Length, count)); - SetRequestPolicies(customPolicies, policies, HttpPipelinePosition.BeforeTransport, policies.Length); - sectionCount = _pipeline.Length - _transportIndex; - _pipeline.Slice(_transportIndex, sectionCount).CopyTo(new Memory(policies, policies.Length, sectionCount)); + AddRequestPolicies(HttpPipelinePosition.BeforeTransport, _transportIndex); + count = _pipeline.Length - _transportIndex; + _pipeline.Slice(_transportIndex, count).CopyTo(new Memory(policies, policies.Length, count)); return new ReadOnlyMemory(policies); } - private static void SetRequestPolicies(List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> source, - HttpPipelinePolicy[] target, HttpPipelinePosition position, int start) - { - var policies = source.Where(p => p.Position == position).Select(p => p.Policy); - - int i = 0; - foreach (var policy in policies) - { - target[start + i++] = policy; - } - } - private static void AddHttpMessageProperties(HttpMessage message) { if (CurrentHttpMessagePropertiesScope.Value != null) From 0daab39ae1b655c0fac05f3e20bf56287322950b Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 11 Nov 2021 15:07:15 -0800 Subject: [PATCH 09/26] use ArrayPool --- sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 44c746713618..6ef42f1eaecf 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -122,8 +122,19 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) { message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - var pipeline = message.Policies == null ? _pipeline : CreateRequestPipeline(message.Policies); - pipeline.Span[0].Process(message, pipeline.Slice(1)); + + if (message.Policies == null) + { + _pipeline.Span[0].Process(message, _pipeline.Slice(1)); + } + else + { + var length = _pipeline.Length + message.Policies.Count; + var pipeline = ArrayPool.Shared.Rent(length); + CreateRequestPipeline(pipeline, message.Policies); + pipeline[0].Process(message, new ReadOnlyMemory(pipeline, 1, length - 1)); + ArrayPool.Shared.Return(pipeline); + } } /// From 04b5d15e7c9908ab2438db0ea855394623cf5bcb Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 11 Nov 2021 17:22:57 -0800 Subject: [PATCH 10/26] add tests --- .../src/HttpPipelineExtensions.cs | 105 +++++++++-------- .../tests/LowLevelClient/PetStoreClient.cs | 2 +- .../tests/LowLevelClientTests.cs | 110 ++++++++++++++++-- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 44 +++---- .../src/Pipeline/HttpPipelineTransport.cs | 2 +- sdk/core/Azure.Core/src/Request.cs | 2 +- 6 files changed, 180 insertions(+), 85 deletions(-) diff --git a/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs b/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs index caf105ab5eb0..dbe6838dc24d 100644 --- a/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs +++ b/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs @@ -1,67 +1,66 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System.Threading; -using System.Threading.Tasks; -using Azure.Core.Pipeline; +//using System.Threading; +//using System.Threading.Tasks; +//using Azure.Core.Pipeline; -namespace Azure.Core.Experimental -{ - internal static class HttpPipelineExtensions - { - public static async ValueTask ProcessMessageAsync(this HttpPipeline pipeline, HttpMessage message, ClientDiagnostics clientDiagnostics, RequestContext? requestContext, CancellationToken cancellationToken = default) - { - // Note: this is thinking about how this could be refactored. - requestContext ??= new RequestContext(); - CancellationToken operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); - message.Apply(requestContext); +//namespace Azure.Core.Experimental +//{ +// internal static class HttpPipelineExtensions +// { +// public static async ValueTask ProcessMessageAsync(this HttpPipeline pipeline, HttpMessage message, ClientDiagnostics clientDiagnostics, RequestContext? requestContext, CancellationToken cancellationToken = default) +// { +// // Note: this is thinking about how this could be refactored. +// requestContext ??= new RequestContext(); +// CancellationToken operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); +// +// await pipeline.SendAsync(message, operationCancellationToken).ConfigureAwait(false); - await pipeline.SendAsync(message, operationCancellationToken).ConfigureAwait(false); +// if (requestContext.ErrorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) +// { +// return message.Response; +// } - if (requestContext.ErrorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) - { - return message.Response; - } +// throw await clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); +// } - throw await clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); - } +// public static Response ProcessMessage(this HttpPipeline pipeline, HttpMessage message, ClientDiagnostics clientDiagnostics, RequestContext? requestContext, CancellationToken cancellationToken = default) +// { +// ErrorOptions errorOptions = ErrorOptions.Default; +// CancellationToken operationCancellationToken = CancellationToken.None; - public static Response ProcessMessage(this HttpPipeline pipeline, HttpMessage message, ClientDiagnostics clientDiagnostics, RequestContext? requestContext, CancellationToken cancellationToken = default) - { - ErrorOptions errorOptions = ErrorOptions.Default; - CancellationToken operationCancellationToken = CancellationToken.None; +// if (requestContext != null) +// { +// errorOptions = requestContext.ErrorOptions; +// operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); +// } - if (requestContext != null) - { - errorOptions = requestContext.ErrorOptions; - operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); - } +// pipeline.Send(message, operationCancellationToken); - pipeline.Send(message, operationCancellationToken); +// if (errorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) +// { +// return message.Response; +// } - if (errorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) - { - return message.Response; - } +// throw clientDiagnostics.CreateRequestFailedException(message.Response); +// } - throw clientDiagnostics.CreateRequestFailedException(message.Response); - } +// private static CancellationToken MergeCancellationTokens(RequestContext context, CancellationToken cancellationToken) +// { +// if (context.CancellationToken.CanBeCanceled && cancellationToken.CanBeCanceled) +// { +// // TODO: This is disposed when the method returns -- solve this. +// using var cts = CancellationTokenSource.CreateLinkedTokenSource(context.CancellationToken, cancellationToken); +// return cts.Token; +// } - private static CancellationToken MergeCancellationTokens(RequestContext context, CancellationToken cancellationToken) - { - if (context.CancellationToken.CanBeCanceled && cancellationToken.CanBeCanceled) - { - // TODO: This is disposed when the method returns -- solve this. - using var cts = CancellationTokenSource.CreateLinkedTokenSource(context.CancellationToken, cancellationToken); - return cts.Token; - } +// if (cancellationToken.CanBeCanceled) +// { +// return cancellationToken; +// } - if (cancellationToken.CanBeCanceled) - { - return cancellationToken; - } - - return context.CancellationToken; - } - } -} +// return context.CancellationToken; +// } +// } +//} diff --git a/sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs b/sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs index 3859098d8b2f..aa8c3d14f49c 100644 --- a/sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs +++ b/sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs @@ -146,7 +146,7 @@ public virtual Response GetPet(string id, RequestContext context = null) /// The request options. private HttpMessage CreateGetPetRequest(string id, RequestContext context = null) { - var message = Pipeline.CreateMessage(); + var message = Pipeline.CreateMessage(context); var request = message.Request; request.Method = RequestMethod.Get; var uri = new RawRequestUriBuilder(); diff --git a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs index 263a13f02f46..4b1c3161ca55 100644 --- a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs +++ b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Linq; using System.Text.Json; using System.Threading.Tasks; using Azure.Core.Experimental.Tests; @@ -212,34 +213,125 @@ public void ThrowOnErrorThrowsOnError() } [Test] - public void CanAddPerCallPolicy() + public async Task CanAddPerCallPolicy() { + var petResponse = new MockResponse(200); + + Pet pet = new("snoopy", "beagle"); + petResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); + + var mockTransport = new MockTransport(petResponse); + PetStoreClient client = CreateClient(mockTransport); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); + + Response response = await client.GetPetAsync("snoopy", context); + + Request request = mockTransport.Requests[0]; + Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader", out var values)); + Assert.AreEqual(1, values.Count()); + Assert.AreEqual("Value", values.ElementAt(0)); } [Test] public async Task CanAddPerRetryPolicy() { - var mockResponse = new MockResponse(200); + var retryResponse = new MockResponse(408); // Request Timeout + var petResponse = new MockResponse(200); Pet pet = new("snoopy", "beagle"); - mockResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); + petResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); - var mockTransport = new MockTransport(mockResponse); + // retry twice -- this will add the header three times. + var mockTransport = new MockTransport(retryResponse, retryResponse, petResponse); PetStoreClient client = CreateClient(mockTransport); var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("MyHeader", "MyValue"), HttpPipelinePosition.PerRetry); + context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "Value"), HttpPipelinePosition.PerRetry); - Response response = await client.GetPetAsync("snoopy", new RequestContext()); + Response response = await client.GetPetAsync("snoopy", context); Request request = mockTransport.Requests[0]; - Assert.IsTrue(request.Headers.TryGetValue("MyHeader", out string headerValue)); - Assert.AreEqual("MyValue", headerValue); + Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var values)); + Assert.AreEqual(3, values.Count()); + Assert.AreEqual("Value", values.ElementAt(0)); + Assert.AreEqual("Value", values.ElementAt(1)); + Assert.AreEqual("Value", values.ElementAt(2)); + } + + [Test] + public async Task CanAddBeforeTransportPolicy() + { + var retryResponse = new MockResponse(408); // Request Timeout + var petResponse = new MockResponse(200); + + Pet pet = new("snoopy", "beagle"); + petResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); + + // retry twice + var mockTransport = new MockTransport(retryResponse, retryResponse, petResponse); + PetStoreClient client = CreateClient(mockTransport); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "Value"), HttpPipelinePosition.BeforeTransport); + + Response response = await client.GetPetAsync("snoopy", context); + + Request request = mockTransport.Requests[0]; + Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var values)); + Assert.AreEqual(3, values.Count()); + Assert.AreEqual("Value", values.ElementAt(0)); + Assert.AreEqual("Value", values.ElementAt(1)); + Assert.AreEqual("Value", values.ElementAt(2)); + } + + [Test] + public async Task CanAddPoliciesAllPositions() + { + var retryResponse = new MockResponse(408); // Request Timeout + var petResponse = new MockResponse(200); + + Pet pet = new("snoopy", "beagle"); + petResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); + + // retry twice -- this will add the header three times. + var mockTransport = new MockTransport(retryResponse, retryResponse, petResponse); + PetStoreClient client = CreateClient(mockTransport); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader1", "PerCall1"), HttpPipelinePosition.PerCall); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader2", "PerCall2"), HttpPipelinePosition.PerCall); + context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "PerRetry"), HttpPipelinePosition.PerRetry); + context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "BeforeTransport"), HttpPipelinePosition.BeforeTransport); + + Response response = await client.GetPetAsync("snoopy", context); + + Request request = mockTransport.Requests[0]; + + Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader1", out var perCall1Values)); + Assert.AreEqual(1, perCall1Values.Count()); + Assert.AreEqual("PerCall1", perCall1Values.ElementAt(0)); + + Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader2", out var perCall2Values)); + Assert.AreEqual(1, perCall2Values.Count()); + Assert.AreEqual("PerCall2", perCall2Values.ElementAt(0)); + + Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var perRetryValues)); + Assert.AreEqual("PerRetry", perRetryValues.ElementAt(0)); + Assert.AreEqual("PerRetry", perRetryValues.ElementAt(1)); + Assert.AreEqual("PerRetry", perRetryValues.ElementAt(2)); + + Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var beforeTransportValues)); + Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(0)); + Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(1)); + Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(2)); } [Test] - public void CanAddBeforeTransportPolicy() + public void ThrowsIfUsePipelineConstructor() { + // TODO: Add this test } #region Helpers diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 6ef42f1eaecf..3baa4d483b3b 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -4,7 +4,6 @@ using System; using System.Buffers; using System.Collections.Generic; -using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -21,7 +20,6 @@ public class HttpPipeline private readonly ReadOnlyMemory _pipeline; - private bool _builderConstructed; private readonly int _perCallIndex; private readonly int _perRetryIndex; private readonly int _transportIndex; @@ -51,7 +49,7 @@ internal HttpPipeline(HttpPipelineTransport transport, int perCallIndex, int per { _perCallIndex = perCallIndex; _perRetryIndex = perRetryIndex; - _builderConstructed = true; + //_builderConstructed = true; } /// @@ -106,10 +104,10 @@ public async ValueTask SendAsync(HttpMessage message, CancellationToken cancella else { var length = _pipeline.Length + message.Policies.Count; - var pipeline = ArrayPool.Shared.Rent(length); - CreateRequestPipeline(pipeline, message.Policies); - await pipeline[0].ProcessAsync(message, new ReadOnlyMemory(pipeline, 1, length - 1)).ConfigureAwait(false); - ArrayPool.Shared.Return(pipeline); + var policies = ArrayPool.Shared.Rent(length); + var pipeline = CreateRequestPipeline(policies, message.Policies); + await pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)).ConfigureAwait(false); + ArrayPool.Shared.Return(policies); } } @@ -130,10 +128,10 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) else { var length = _pipeline.Length + message.Policies.Count; - var pipeline = ArrayPool.Shared.Rent(length); - CreateRequestPipeline(pipeline, message.Policies); - pipeline[0].Process(message, new ReadOnlyMemory(pipeline, 1, length - 1)); - ArrayPool.Shared.Return(pipeline); + var policies = ArrayPool.Shared.Rent(length); + var pipeline = CreateRequestPipeline(policies, message.Policies); + pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); + ArrayPool.Shared.Return(policies); } } @@ -199,11 +197,11 @@ public static IDisposable CreateHttpMessagePropertiesScope(IDictionary CreateRequestPipeline(HttpPipelinePolicy[] policies, List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> customPolicies) { - void AddRequestPolicies(HttpPipelinePosition position, int start) + int AddCustomPolicies(HttpPipelinePosition position, int start) { + int i = 0; if (customPolicies != null) { - int i = 0; foreach (var policy in customPolicies) { if (policy.Position == position) @@ -212,23 +210,29 @@ void AddRequestPolicies(HttpPipelinePosition position, int start) } } } + + return i; } + // Copy over client policies and splice in custom policies at designated indices _pipeline.Slice(0, _perCallIndex).CopyTo(policies); - AddRequestPolicies(HttpPipelinePosition.PerCall, _perCallIndex); + int custom = AddCustomPolicies(HttpPipelinePosition.PerCall, _perCallIndex); + int count = _perRetryIndex - _perCallIndex; - _pipeline.Slice(_perCallIndex, count).CopyTo(new Memory(policies, policies.Length, count)); + _pipeline.Slice(_perCallIndex, count).CopyTo(new Memory(policies, _perCallIndex + custom, count)); + + custom += AddCustomPolicies(HttpPipelinePosition.PerRetry, _perRetryIndex + custom); - AddRequestPolicies(HttpPipelinePosition.PerRetry, _perRetryIndex); count = _transportIndex - _perRetryIndex; - _pipeline.Slice(_perRetryIndex, count).CopyTo(new Memory(policies, policies.Length, count)); + _pipeline.Slice(_perRetryIndex, count).CopyTo(new Memory(policies, _perRetryIndex + custom, count)); + + custom += AddCustomPolicies(HttpPipelinePosition.BeforeTransport, _transportIndex + custom); - AddRequestPolicies(HttpPipelinePosition.BeforeTransport, _transportIndex); count = _pipeline.Length - _transportIndex; - _pipeline.Slice(_transportIndex, count).CopyTo(new Memory(policies, policies.Length, count)); + _pipeline.Slice(_transportIndex, count).CopyTo(new Memory(policies, _transportIndex + custom, count)); - return new ReadOnlyMemory(policies); + return new ReadOnlyMemory(policies, 0, _pipeline.Length + custom); } private static void AddHttpMessageProperties(HttpMessage message) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs index 42c9c8af5bfa..676882a53588 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs @@ -24,7 +24,7 @@ public abstract class HttpPipelineTransport /// /// Creates a new transport specific instance of . This should not be called directly, or - /// should be used instead. + /// should be used instead. /// /// public abstract Request CreateRequest(); diff --git a/sdk/core/Azure.Core/src/Request.cs b/sdk/core/Azure.Core/src/Request.cs index 7c9f13d99420..011af269df7b 100644 --- a/sdk/core/Azure.Core/src/Request.cs +++ b/sdk/core/Azure.Core/src/Request.cs @@ -10,7 +10,7 @@ namespace Azure.Core { /// - /// Represents an HTTP request. Use or to create an instance. + /// Represents an HTTP request. Use or to create an instance. /// #pragma warning disable AZC0012 // Avoid single word type names public abstract class Request : IDisposable From 901d34c764255e7a943cec23a0275ffd6d37b8c4 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 12 Nov 2021 08:43:34 -0800 Subject: [PATCH 11/26] update API --- sdk/core/Azure.Core/api/Azure.Core.net461.cs | 2 ++ sdk/core/Azure.Core/api/Azure.Core.net5.0.cs | 2 ++ sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs | 2 ++ sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs | 2 ++ sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs | 7 +++++-- 5 files changed, 13 insertions(+), 2 deletions(-) diff --git a/sdk/core/Azure.Core/api/Azure.Core.net461.cs b/sdk/core/Azure.Core/api/Azure.Core.net461.cs index fc8113abb445..cc27d497c445 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.net461.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.net461.cs @@ -190,6 +190,7 @@ public partial class RequestContext public RequestContext() { } public System.Threading.CancellationToken CancellationToken { get { throw null; } set { } } public Azure.ErrorOptions ErrorOptions { get { throw null; } set { } } + public void AddPolicy(Azure.Core.Pipeline.HttpPipelinePolicy policy, Azure.Core.HttpPipelinePosition position) { } public static implicit operator Azure.RequestContext (Azure.ErrorOptions options) { throw null; } } public partial class RequestFailedException : System.Exception, System.Runtime.Serialization.ISerializable @@ -755,6 +756,7 @@ public HttpPipeline(Azure.Core.Pipeline.HttpPipelineTransport transport, Azure.C public static System.IDisposable CreateClientRequestIdScope(string? clientRequestId) { throw null; } public static System.IDisposable CreateHttpMessagePropertiesScope(System.Collections.Generic.IDictionary messageProperties) { throw null; } public Azure.Core.HttpMessage CreateMessage() { throw null; } + public Azure.Core.HttpMessage CreateMessage(Azure.RequestContext context) { throw null; } public Azure.Core.Request CreateRequest() { throw null; } public void Send(Azure.Core.HttpMessage message, System.Threading.CancellationToken cancellationToken) { } public System.Threading.Tasks.ValueTask SendAsync(Azure.Core.HttpMessage message, System.Threading.CancellationToken cancellationToken) { throw null; } diff --git a/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs b/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs index 447772be550f..dff43316757e 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs @@ -190,6 +190,7 @@ public partial class RequestContext public RequestContext() { } public System.Threading.CancellationToken CancellationToken { get { throw null; } set { } } public Azure.ErrorOptions ErrorOptions { get { throw null; } set { } } + public void AddPolicy(Azure.Core.Pipeline.HttpPipelinePolicy policy, Azure.Core.HttpPipelinePosition position) { } public static implicit operator Azure.RequestContext (Azure.ErrorOptions options) { throw null; } } public partial class RequestFailedException : System.Exception, System.Runtime.Serialization.ISerializable @@ -755,6 +756,7 @@ public HttpPipeline(Azure.Core.Pipeline.HttpPipelineTransport transport, Azure.C public static System.IDisposable CreateClientRequestIdScope(string? clientRequestId) { throw null; } public static System.IDisposable CreateHttpMessagePropertiesScope(System.Collections.Generic.IDictionary messageProperties) { throw null; } public Azure.Core.HttpMessage CreateMessage() { throw null; } + public Azure.Core.HttpMessage CreateMessage(Azure.RequestContext context) { throw null; } public Azure.Core.Request CreateRequest() { throw null; } public void Send(Azure.Core.HttpMessage message, System.Threading.CancellationToken cancellationToken) { } public System.Threading.Tasks.ValueTask SendAsync(Azure.Core.HttpMessage message, System.Threading.CancellationToken cancellationToken) { throw null; } diff --git a/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs b/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs index fc8113abb445..cc27d497c445 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs @@ -190,6 +190,7 @@ public partial class RequestContext public RequestContext() { } public System.Threading.CancellationToken CancellationToken { get { throw null; } set { } } public Azure.ErrorOptions ErrorOptions { get { throw null; } set { } } + public void AddPolicy(Azure.Core.Pipeline.HttpPipelinePolicy policy, Azure.Core.HttpPipelinePosition position) { } public static implicit operator Azure.RequestContext (Azure.ErrorOptions options) { throw null; } } public partial class RequestFailedException : System.Exception, System.Runtime.Serialization.ISerializable @@ -755,6 +756,7 @@ public HttpPipeline(Azure.Core.Pipeline.HttpPipelineTransport transport, Azure.C public static System.IDisposable CreateClientRequestIdScope(string? clientRequestId) { throw null; } public static System.IDisposable CreateHttpMessagePropertiesScope(System.Collections.Generic.IDictionary messageProperties) { throw null; } public Azure.Core.HttpMessage CreateMessage() { throw null; } + public Azure.Core.HttpMessage CreateMessage(Azure.RequestContext context) { throw null; } public Azure.Core.Request CreateRequest() { throw null; } public void Send(Azure.Core.HttpMessage message, System.Threading.CancellationToken cancellationToken) { } public System.Threading.Tasks.ValueTask SendAsync(Azure.Core.HttpMessage message, System.Threading.CancellationToken cancellationToken) { throw null; } diff --git a/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs b/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs index fc8113abb445..cc27d497c445 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs @@ -190,6 +190,7 @@ public partial class RequestContext public RequestContext() { } public System.Threading.CancellationToken CancellationToken { get { throw null; } set { } } public Azure.ErrorOptions ErrorOptions { get { throw null; } set { } } + public void AddPolicy(Azure.Core.Pipeline.HttpPipelinePolicy policy, Azure.Core.HttpPipelinePosition position) { } public static implicit operator Azure.RequestContext (Azure.ErrorOptions options) { throw null; } } public partial class RequestFailedException : System.Exception, System.Runtime.Serialization.ISerializable @@ -755,6 +756,7 @@ public HttpPipeline(Azure.Core.Pipeline.HttpPipelineTransport transport, Azure.C public static System.IDisposable CreateClientRequestIdScope(string? clientRequestId) { throw null; } public static System.IDisposable CreateHttpMessagePropertiesScope(System.Collections.Generic.IDictionary messageProperties) { throw null; } public Azure.Core.HttpMessage CreateMessage() { throw null; } + public Azure.Core.HttpMessage CreateMessage(Azure.RequestContext context) { throw null; } public Azure.Core.Request CreateRequest() { throw null; } public void Send(Azure.Core.HttpMessage message, System.Threading.CancellationToken cancellationToken) { } public System.Threading.Tasks.ValueTask SendAsync(Azure.Core.HttpMessage message, System.Threading.CancellationToken cancellationToken) { throw null; } diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 3baa4d483b3b..6a53037b3c97 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -76,8 +76,11 @@ public HttpMessage CreateMessage() public HttpMessage CreateMessage(RequestContext context) { var message = CreateMessage(); - message.CancellationToken = context.CancellationToken; - message.Policies = context.Policies; + if (context != null) + { + message.CancellationToken = context.CancellationToken; + message.Policies = context.Policies; + } return message; } From 40046ddff5be85bc31bf9896892ecebd34a7a62f Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 12 Nov 2021 09:02:09 -0800 Subject: [PATCH 12/26] throw if didn't use pipeline builder --- .../src/HttpPipelineExtensions.cs | 66 ------------------- .../tests/LowLevelClientTests.cs | 21 +++++- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 8 ++- 3 files changed, 26 insertions(+), 69 deletions(-) delete mode 100644 sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs diff --git a/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs b/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs deleted file mode 100644 index dbe6838dc24d..000000000000 --- a/sdk/core/Azure.Core.Experimental/src/HttpPipelineExtensions.cs +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -//using System.Threading; -//using System.Threading.Tasks; -//using Azure.Core.Pipeline; - -//namespace Azure.Core.Experimental -//{ -// internal static class HttpPipelineExtensions -// { -// public static async ValueTask ProcessMessageAsync(this HttpPipeline pipeline, HttpMessage message, ClientDiagnostics clientDiagnostics, RequestContext? requestContext, CancellationToken cancellationToken = default) -// { -// // Note: this is thinking about how this could be refactored. -// requestContext ??= new RequestContext(); -// CancellationToken operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); -// -// await pipeline.SendAsync(message, operationCancellationToken).ConfigureAwait(false); - -// if (requestContext.ErrorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) -// { -// return message.Response; -// } - -// throw await clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); -// } - -// public static Response ProcessMessage(this HttpPipeline pipeline, HttpMessage message, ClientDiagnostics clientDiagnostics, RequestContext? requestContext, CancellationToken cancellationToken = default) -// { -// ErrorOptions errorOptions = ErrorOptions.Default; -// CancellationToken operationCancellationToken = CancellationToken.None; - -// if (requestContext != null) -// { -// errorOptions = requestContext.ErrorOptions; -// operationCancellationToken = MergeCancellationTokens(requestContext, cancellationToken); -// } - -// pipeline.Send(message, operationCancellationToken); - -// if (errorOptions == ErrorOptions.NoThrow || !message.ResponseClassifier.IsErrorResponse(message)) -// { -// return message.Response; -// } - -// throw clientDiagnostics.CreateRequestFailedException(message.Response); -// } - -// private static CancellationToken MergeCancellationTokens(RequestContext context, CancellationToken cancellationToken) -// { -// if (context.CancellationToken.CanBeCanceled && cancellationToken.CanBeCanceled) -// { -// // TODO: This is disposed when the method returns -- solve this. -// using var cts = CancellationTokenSource.CreateLinkedTokenSource(context.CancellationToken, cancellationToken); -// return cts.Token; -// } - -// if (cancellationToken.CanBeCanceled) -// { -// return cancellationToken; -// } - -// return context.CancellationToken; -// } -// } -//} diff --git a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs index 4b1c3161ca55..b4b94057fe6c 100644 --- a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs +++ b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs @@ -329,9 +329,26 @@ public async Task CanAddPoliciesAllPositions() } [Test] - public void ThrowsIfUsePipelineConstructor() + public async Task ThrowsIfUsePipelineConstructor() { - // TODO: Add this test + HttpPipeline pipeline = new HttpPipeline(new MockTransport()); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); + + var message = pipeline.CreateMessage(context); + + bool throws = false; + try + { + await pipeline.SendAsync(message, context.CancellationToken); + } + catch (InvalidOperationException) + { + throws = true; + } + + Assert.IsTrue(throws); } #region Helpers diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 6a53037b3c97..464801fcac8a 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -20,6 +20,7 @@ public class HttpPipeline private readonly ReadOnlyMemory _pipeline; + private bool _internallyConstructed; private readonly int _perCallIndex; private readonly int _perRetryIndex; private readonly int _transportIndex; @@ -49,7 +50,7 @@ internal HttpPipeline(HttpPipelineTransport transport, int perCallIndex, int per { _perCallIndex = perCallIndex; _perRetryIndex = perRetryIndex; - //_builderConstructed = true; + _internallyConstructed = true; } /// @@ -200,6 +201,11 @@ public static IDisposable CreateHttpMessagePropertiesScope(IDictionary CreateRequestPipeline(HttpPipelinePolicy[] policies, List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> customPolicies) { + if (!_internallyConstructed) + { + throw new InvalidOperationException("Cannot send messages with per-request policies if the pipeline wasn't constructed with HttpPipelineBuilder."); + } + int AddCustomPolicies(HttpPipelinePosition position, int start) { int i = 0; From 89962b64ea7444370ecca92e46d846e1ae13ec73 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 12 Nov 2021 13:18:56 -0800 Subject: [PATCH 13/26] pr feedback and merge follow-up --- .../tests/LowLevelClientTests.cs | 11 ++--------- .../Azure.Core/src/Pipeline/HttpPipelineBuilder.cs | 1 - 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs index b4b94057fe6c..2e2fbd1aa19b 100644 --- a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs +++ b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs @@ -365,7 +365,7 @@ private void SerializePet(ref Utf8JsonWriter writer, Pet pet) writer.WriteEndObject(); } - public class AddHeaderPolicy : HttpPipelinePolicy + public class AddHeaderPolicy : HttpPipelineSynchronousPolicy { private string _headerName; private string _headerVaue; @@ -376,16 +376,9 @@ public AddHeaderPolicy(string headerName, string headerValue) : base() _headerVaue = headerValue; } - public override void Process(HttpMessage message, ReadOnlyMemory pipeline) + public override void OnSendingRequest(HttpMessage message) { message.Request.Headers.Add(_headerName, _headerVaue); - ProcessNext(message, pipeline); - } - - public override ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory pipeline) - { - message.Request.Headers.Add(_headerName, _headerVaue); - return ProcessNextAsync(message, pipeline); } } #endregion diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index 726c9f7a6148..e7a25eebd81a 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -53,7 +53,6 @@ public static HttpPipeline Build( /// A new instance of public static DisposableHttpPipeline Build(ClientOptions options, HttpPipelinePolicy[] perCallPolicies, HttpPipelinePolicy[] perRetryPolicies, ResponseClassifier responseClassifier, HttpPipelineTransportOptions? defaultTransportOptions) { - int perCallIndex; int perRetryIndex; if (perCallPolicies == null) From faafa33aa34f2a811c034e11d2da8549028791e8 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 12 Nov 2021 16:00:20 -0800 Subject: [PATCH 14/26] pr fb and merge follow-up --- .../src/Pipeline/DisposableHttpPipeline.cs | 9 ++++--- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 27 ++++++++++++++----- .../src/Pipeline/HttpPipelineBuilder.cs | 2 ++ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/DisposableHttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/DisposableHttpPipeline.cs index df7e0d3ac8d7..a5652f19c590 100644 --- a/sdk/core/Azure.Core/src/Pipeline/DisposableHttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/DisposableHttpPipeline.cs @@ -14,11 +14,14 @@ public sealed class DisposableHttpPipeline : HttpPipeline, IDisposable /// Creates a new instance of with the provided transport, policies and response classifier. /// /// The to use for sending the requests. + /// + /// /// Policies to be invoked as part of the pipeline in order. /// The response classifier to be used in invocations. - internal DisposableHttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null) - : base(transport, policies, responseClassifier) - { } + internal DisposableHttpPipeline(HttpPipelineTransport transport, int perCallIndex, int perRetryIndex, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null) + : base(transport, perCallIndex, perRetryIndex, policies, responseClassifier) + { + } /// /// Calls Dispose on the underlying . diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 7efbfdcb26d1..28d0830d1c62 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -20,7 +20,7 @@ public class HttpPipeline private readonly ReadOnlyMemory _pipeline; - private bool _internallyConstructed; + private readonly bool _internallyConstructed; private readonly int _perCallIndex; private readonly int _perRetryIndex; private readonly int _transportIndex; @@ -46,6 +46,7 @@ public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? polic _pipeline = all; } + internal HttpPipeline(HttpPipelineTransport transport, int perCallIndex, int perRetryIndex, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null) : this(transport, policies, responseClassifier) { _perCallIndex = perCallIndex; @@ -109,9 +110,15 @@ public async ValueTask SendAsync(HttpMessage message, CancellationToken cancella { var length = _pipeline.Length + message.Policies.Count; var policies = ArrayPool.Shared.Rent(length); - var pipeline = CreateRequestPipeline(policies, message.Policies); - await pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)).ConfigureAwait(false); - ArrayPool.Shared.Return(policies); + try + { + var pipeline = CreateRequestPipeline(policies, message.Policies); + await pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)).ConfigureAwait(false); + } + finally + { + ArrayPool.Shared.Return(policies); + } } } @@ -133,9 +140,15 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) { var length = _pipeline.Length + message.Policies.Count; var policies = ArrayPool.Shared.Rent(length); - var pipeline = CreateRequestPipeline(policies, message.Policies); - pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); - ArrayPool.Shared.Return(policies); + try + { + var pipeline = CreateRequestPipeline(policies, message.Policies); + pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); + } + finally + { + ArrayPool.Shared.Return(policies); + } } } diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index f002b0426c0b..d132ae1995ee 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -149,6 +149,8 @@ void AddCustomerPolicies(HttpPipelinePosition position) { transport = HttpPipelineTransport.Create(defaultTransportOptions); return new DisposableHttpPipeline(transport, + perCallIndex, + perRetryIndex, policies.ToArray(), responseClassifier); } From a11d08e89773fa7a0fbd87c3d90a0ce71f527178 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Mon, 15 Nov 2021 16:26:49 -0800 Subject: [PATCH 15/26] pr fb --- sdk/core/Azure.Core/src/HttpMessage.cs | 32 ++++++++-- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 63 ++++++++----------- .../src/Pipeline/HttpPipelineBuilder.cs | 8 +-- sdk/core/Azure.Core/src/RequestContext.cs | 46 ++++++++++++-- 4 files changed, 96 insertions(+), 53 deletions(-) diff --git a/sdk/core/Azure.Core/src/HttpMessage.cs b/sdk/core/Azure.Core/src/HttpMessage.cs index c0a63bd23950..f5b9b2b05cab 100644 --- a/sdk/core/Azure.Core/src/HttpMessage.cs +++ b/sdk/core/Azure.Core/src/HttpMessage.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Net.Http; using System.Threading; using Azure.Core.Pipeline; @@ -13,7 +12,7 @@ namespace Azure.Core /// /// Represents a context flowing through the . /// - public sealed class HttpMessage: IDisposable + public sealed class HttpMessage : IDisposable { private Dictionary? _properties; @@ -81,7 +80,30 @@ public Response Response /// public TimeSpan? NetworkTimeout { get; set; } - internal List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)>? Policies { get; set; } + internal void AddPolicies(RequestContext context) + { + if (context == null || context.Policies == null) + { + return; + } + + var policies = new Memory(new HttpPipelinePolicy[context.Policies.Value.Length]); + context.Policies?.CopyTo(policies); + + PerCallPolicies = policies.Slice(RequestContext.PerCallOffset, context.PerCallPolicies); + PerRetryPolicies = policies.Slice(RequestContext.PerRetryOffset, context.PerRetryPolicies); + BeforeTransportPolicies = policies.Slice(RequestContext.BeforeTransportOffset, context.BeforeTransportPolicies); + + PolicyCount = context.PerCallPolicies + context.PerRetryPolicies + context.BeforeTransportPolicies; + + _policies = policies; + } + + private ReadOnlyMemory? _policies; + internal int PolicyCount { get; private set; } + internal ReadOnlyMemory PerCallPolicies { get; private set; } + internal ReadOnlyMemory PerRetryPolicies { get; private set; } + internal ReadOnlyMemory BeforeTransportPolicies { get; private set; } /// /// Gets a property that modifies the pipeline behavior. Please refer to individual policies documentation on what properties it supports. @@ -117,7 +139,7 @@ public void SetProperty(string name, object value) { case ResponseShouldNotBeUsedStream responseContent: return responseContent.Original; - case Stream stream : + case Stream stream: _response.ContentStream = new ResponseShouldNotBeUsedStream(_response.ContentStream); return stream; default: @@ -134,7 +156,7 @@ public void Dispose() _response?.Dispose(); } - private class ResponseShouldNotBeUsedStream: Stream + private class ResponseShouldNotBeUsedStream : Stream { public Stream Original { get; } diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 28d0830d1c62..6458b3418b65 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -78,11 +78,7 @@ public HttpMessage CreateMessage() public HttpMessage CreateMessage(RequestContext context) { var message = CreateMessage(); - if (context != null) - { - message.CancellationToken = context.CancellationToken; - message.Policies = context.Policies; - } + message.AddPolicies(context); return message; } @@ -102,17 +98,17 @@ public async ValueTask SendAsync(HttpMessage message, CancellationToken cancella message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - if (message.Policies == null) + if (message.PolicyCount == 0) { await _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)).ConfigureAwait(false); } else { - var length = _pipeline.Length + message.Policies.Count; + var length = _pipeline.Length + message.PolicyCount; var policies = ArrayPool.Shared.Rent(length); try { - var pipeline = CreateRequestPipeline(policies, message.Policies); + var pipeline = CreateRequestPipeline(policies, message); await pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)).ConfigureAwait(false); } finally @@ -132,17 +128,17 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - if (message.Policies == null) + if (message.PolicyCount == 0) { _pipeline.Span[0].Process(message, _pipeline.Slice(1)); } else { - var length = _pipeline.Length + message.Policies.Count; + var length = _pipeline.Length + message.PolicyCount; var policies = ArrayPool.Shared.Rent(length); try { - var pipeline = CreateRequestPipeline(policies, message.Policies); + var pipeline = CreateRequestPipeline(policies, message); pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); } finally @@ -212,49 +208,40 @@ public static IDisposable CreateHttpMessagePropertiesScope(IDictionary CreateRequestPipeline(HttpPipelinePolicy[] policies, List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> customPolicies) + private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePolicy[] policies, HttpMessage message) { if (!_internallyConstructed) { throw new InvalidOperationException("Cannot send messages with per-request policies if the pipeline wasn't constructed with HttpPipelineBuilder."); } - int AddCustomPolicies(HttpPipelinePosition position, int start) - { - int i = 0; - if (customPolicies != null) - { - foreach (var policy in customPolicies) - { - if (policy.Position == position) - { - policies[start + i++] = policy.Policy; - } - } - } - - return i; - } - // Copy over client policies and splice in custom policies at designated indices _pipeline.Slice(0, _perCallIndex).CopyTo(policies); - int custom = AddCustomPolicies(HttpPipelinePosition.PerCall, _perCallIndex); + int index = _perCallIndex; + int count = message.PerCallPolicies.Length; + message.PerCallPolicies.CopyTo(new Memory(policies, index, count)); - int count = _perRetryIndex - _perCallIndex; - _pipeline.Slice(_perCallIndex, count).CopyTo(new Memory(policies, _perCallIndex + custom, count)); + index += count; + count = _perRetryIndex - _perCallIndex; + _pipeline.Slice(_perCallIndex, count).CopyTo(new Memory(policies, index, count)); - custom += AddCustomPolicies(HttpPipelinePosition.PerRetry, _perRetryIndex + custom); + index += count; + count = message.PerRetryPolicies.Length; + message.PerRetryPolicies.CopyTo(new Memory(policies, index, count)); + index += count; count = _transportIndex - _perRetryIndex; - _pipeline.Slice(_perRetryIndex, count).CopyTo(new Memory(policies, _perRetryIndex + custom, count)); + _pipeline.Slice(_perRetryIndex, count).CopyTo(new Memory(policies, index, count)); - custom += AddCustomPolicies(HttpPipelinePosition.BeforeTransport, _transportIndex + custom); + index += count; + count = message.BeforeTransportPolicies.Length; + message.BeforeTransportPolicies.CopyTo(new Memory(policies, index, count)); - count = _pipeline.Length - _transportIndex; - _pipeline.Slice(_transportIndex, count).CopyTo(new Memory(policies, _transportIndex + custom, count)); + index += count; + policies[index] = _pipeline.Span[_transportIndex]; - return new ReadOnlyMemory(policies, 0, _pipeline.Length + custom); + return new ReadOnlyMemory(policies, 0, index + 1); } private static void AddHttpMessageProperties(HttpMessage message) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index d132ae1995ee..90aec1225e6c 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -82,6 +82,8 @@ void AddCustomerPolicies(HttpPipelinePosition position) } } } + + policies.RemoveAll(static policy => policy == null); } DiagnosticsOptions diagnostics = options.Diagnostics; @@ -95,8 +97,6 @@ void AddCustomerPolicies(HttpPipelinePosition position) policies.AddRange(perCallPolicies); AddCustomerPolicies(HttpPipelinePosition.PerCall); - - policies.RemoveAll(static policy => policy == null); perCallIndex = policies.Count; policies.Add(ClientRequestIdPolicy.Shared); @@ -114,8 +114,6 @@ void AddCustomerPolicies(HttpPipelinePosition position) policies.AddRange(perRetryPolicies); AddCustomerPolicies(HttpPipelinePosition.PerRetry); - - policies.RemoveAll(static policy => policy == null); perRetryIndex = policies.Count; if (diagnostics.IsLoggingEnabled) @@ -131,8 +129,6 @@ void AddCustomerPolicies(HttpPipelinePosition position) AddCustomerPolicies(HttpPipelinePosition.BeforeTransport); - policies.RemoveAll(static policy => policy == null); - // Override the provided Transport with the provided transport options if the transport has not been set after default construction and options are not null. HttpPipelineTransport transport = options.Transport; if (defaultTransportOptions != null) diff --git a/sdk/core/Azure.Core/src/RequestContext.cs b/sdk/core/Azure.Core/src/RequestContext.cs index 2f26d1f333d4..f19229de19ad 100644 --- a/sdk/core/Azure.Core/src/RequestContext.cs +++ b/sdk/core/Azure.Core/src/RequestContext.cs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System.Collections.Generic; +using System; using System.Threading; using Azure.Core; using Azure.Core.Pipeline; @@ -13,7 +13,17 @@ namespace Azure /// public class RequestContext { - internal List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)>? Policies { get; private set; } + internal Memory? Policies { get; private set; } + private const int PolicySections = 3; + private const int SectionSize = 4; + + internal static int PerCallOffset => 0 * SectionSize; + internal static int PerRetryOffset => 1 * SectionSize; + internal static int BeforeTransportOffset => 2 * SectionSize; + + internal int PerCallPolicies { get; private set; } + internal int PerRetryPolicies { get; private set; } + internal int BeforeTransportPolicies { get; private set; } /// /// Initializes a new instance of the class. @@ -44,8 +54,36 @@ public RequestContext() /// public void AddPolicy(HttpPipelinePolicy policy, HttpPipelinePosition position) { - Policies ??= new(); - Policies.Add((position, policy)); + Policies ??= new Memory(new HttpPipelinePolicy[PolicySections * SectionSize]); + + switch (position) + { + case HttpPipelinePosition.PerCall: + CheckPolicyCounter(PerCallPolicies); + Policies.Value.Span[PerCallPolicies++] = policy; + break; + + case HttpPipelinePosition.PerRetry: + CheckPolicyCounter(PerRetryPolicies); + Policies.Value.Span[SectionSize + PerRetryPolicies++] = policy; + break; + + case HttpPipelinePosition.BeforeTransport: + CheckPolicyCounter(BeforeTransportPolicies); + Policies.Value.Span[2 * SectionSize + BeforeTransportPolicies++] = policy; + break; + + default: + break; + } + } + + private static void CheckPolicyCounter(int length) + { + if (length >= SectionSize) + { + throw new InvalidOperationException($"Cannot add more than {SectionSize} policies at a pipeline position."); + } } } } From b442ee1c8296363369b5b6f18361a0f8a29f669a Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 16 Nov 2021 10:58:26 -0800 Subject: [PATCH 16/26] simplify request pipeline creation --- sdk/core/Azure.Core/src/HttpMessage.cs | 19 ++------ .../Azure.Core/src/Pipeline/HttpPipeline.cs | 41 +++++++++++------ sdk/core/Azure.Core/src/RequestContext.cs | 45 ++----------------- 3 files changed, 35 insertions(+), 70 deletions(-) diff --git a/sdk/core/Azure.Core/src/HttpMessage.cs b/sdk/core/Azure.Core/src/HttpMessage.cs index f5b9b2b05cab..a24614b0bf21 100644 --- a/sdk/core/Azure.Core/src/HttpMessage.cs +++ b/sdk/core/Azure.Core/src/HttpMessage.cs @@ -82,28 +82,15 @@ public Response Response internal void AddPolicies(RequestContext context) { - if (context == null || context.Policies == null) + if (context == null || context.Policies == null || context.Policies.Count == 0) { return; } - var policies = new Memory(new HttpPipelinePolicy[context.Policies.Value.Length]); - context.Policies?.CopyTo(policies); - - PerCallPolicies = policies.Slice(RequestContext.PerCallOffset, context.PerCallPolicies); - PerRetryPolicies = policies.Slice(RequestContext.PerRetryOffset, context.PerRetryPolicies); - BeforeTransportPolicies = policies.Slice(RequestContext.BeforeTransportOffset, context.BeforeTransportPolicies); - - PolicyCount = context.PerCallPolicies + context.PerRetryPolicies + context.BeforeTransportPolicies; - - _policies = policies; + Policies ??= new(context.Policies); } - private ReadOnlyMemory? _policies; - internal int PolicyCount { get; private set; } - internal ReadOnlyMemory PerCallPolicies { get; private set; } - internal ReadOnlyMemory PerRetryPolicies { get; private set; } - internal ReadOnlyMemory BeforeTransportPolicies { get; private set; } + internal List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)>? Policies { get; set; } /// /// Gets a property that modifies the pipeline behavior. Please refer to individual policies documentation on what properties it supports. diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 6458b3418b65..d553dd2452ec 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -98,17 +98,17 @@ public async ValueTask SendAsync(HttpMessage message, CancellationToken cancella message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - if (message.PolicyCount == 0) + if (message.Policies == null) { await _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)).ConfigureAwait(false); } else { - var length = _pipeline.Length + message.PolicyCount; + var length = _pipeline.Length + message.Policies.Count; var policies = ArrayPool.Shared.Rent(length); try { - var pipeline = CreateRequestPipeline(policies, message); + var pipeline = CreateRequestPipeline(policies, message.Policies); await pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)).ConfigureAwait(false); } finally @@ -128,17 +128,17 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - if (message.PolicyCount == 0) + if (message.Policies == null) { _pipeline.Span[0].Process(message, _pipeline.Slice(1)); } else { - var length = _pipeline.Length + message.PolicyCount; + var length = _pipeline.Length + message.Policies.Count; var policies = ArrayPool.Shared.Rent(length); try { - var pipeline = CreateRequestPipeline(policies, message); + var pipeline = CreateRequestPipeline(policies, message.Policies); pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); } finally @@ -208,7 +208,7 @@ public static IDisposable CreateHttpMessagePropertiesScope(IDictionary CreateRequestPipeline(HttpPipelinePolicy[] policies, HttpMessage message) + private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePolicy[] policies, List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> customPolicies) { if (!_internallyConstructed) { @@ -219,24 +219,21 @@ private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePol _pipeline.Slice(0, _perCallIndex).CopyTo(policies); int index = _perCallIndex; - int count = message.PerCallPolicies.Length; - message.PerCallPolicies.CopyTo(new Memory(policies, index, count)); + int count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.PerCall, index); index += count; count = _perRetryIndex - _perCallIndex; _pipeline.Slice(_perCallIndex, count).CopyTo(new Memory(policies, index, count)); index += count; - count = message.PerRetryPolicies.Length; - message.PerRetryPolicies.CopyTo(new Memory(policies, index, count)); + count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.PerRetry, index); index += count; count = _transportIndex - _perRetryIndex; _pipeline.Slice(_perRetryIndex, count).CopyTo(new Memory(policies, index, count)); index += count; - count = message.BeforeTransportPolicies.Length; - message.BeforeTransportPolicies.CopyTo(new Memory(policies, index, count)); + count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.BeforeTransport, index); index += count; policies[index] = _pipeline.Span[_transportIndex]; @@ -244,6 +241,24 @@ private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePol return new ReadOnlyMemory(policies, 0, index + 1); } + private static int AddCustomPolicies(List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)> source, HttpPipelinePolicy[] target, HttpPipelinePosition position, int start) + { + int count = 0; + if (source != null) + { + foreach (var policy in source) + { + if (policy.Position == position) + { + target[start + count] = policy.Policy; + count++; + } + } + } + + return count; + } + private static void AddHttpMessageProperties(HttpMessage message) { if (CurrentHttpMessagePropertiesScope.Value != null) diff --git a/sdk/core/Azure.Core/src/RequestContext.cs b/sdk/core/Azure.Core/src/RequestContext.cs index f19229de19ad..9e8dda9ada0e 100644 --- a/sdk/core/Azure.Core/src/RequestContext.cs +++ b/sdk/core/Azure.Core/src/RequestContext.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Threading; using Azure.Core; using Azure.Core.Pipeline; @@ -13,17 +14,7 @@ namespace Azure /// public class RequestContext { - internal Memory? Policies { get; private set; } - private const int PolicySections = 3; - private const int SectionSize = 4; - - internal static int PerCallOffset => 0 * SectionSize; - internal static int PerRetryOffset => 1 * SectionSize; - internal static int BeforeTransportOffset => 2 * SectionSize; - - internal int PerCallPolicies { get; private set; } - internal int PerRetryPolicies { get; private set; } - internal int BeforeTransportPolicies { get; private set; } + internal List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)>? Policies { get; private set; } /// /// Initializes a new instance of the class. @@ -54,36 +45,8 @@ public RequestContext() /// public void AddPolicy(HttpPipelinePolicy policy, HttpPipelinePosition position) { - Policies ??= new Memory(new HttpPipelinePolicy[PolicySections * SectionSize]); - - switch (position) - { - case HttpPipelinePosition.PerCall: - CheckPolicyCounter(PerCallPolicies); - Policies.Value.Span[PerCallPolicies++] = policy; - break; - - case HttpPipelinePosition.PerRetry: - CheckPolicyCounter(PerRetryPolicies); - Policies.Value.Span[SectionSize + PerRetryPolicies++] = policy; - break; - - case HttpPipelinePosition.BeforeTransport: - CheckPolicyCounter(BeforeTransportPolicies); - Policies.Value.Span[2 * SectionSize + BeforeTransportPolicies++] = policy; - break; - - default: - break; - } - } - - private static void CheckPolicyCounter(int length) - { - if (length >= SectionSize) - { - throw new InvalidOperationException($"Cannot add more than {SectionSize} policies at a pipeline position."); - } + Policies ??= new(); + Policies.Add((position, policy)); } } } From 18120d7a1fc4ad17b8e06a356855d02c46be6d29 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 16 Nov 2021 14:21:37 -0800 Subject: [PATCH 17/26] move AddPolicy tests to Core; pr fb --- .../tests/LowLevelClientTests.cs | 139 ------ .../Azure.Core/src/Pipeline/HttpPipeline.cs | 4 +- .../src/Pipeline/HttpPipelineBuilder.cs | 4 +- .../Azure.Core/tests/Azure.Core.Tests.csproj | 4 + .../Azure.Core/tests/RequestContextTests.cs | 256 +++++++++++ .../Azure.Template.LLC.csproj | 35 ++ .../Generated/TemplateServiceClient.cs | 406 ++++++++++++++++++ .../Generated/TemplateServiceClientOptions.cs | 37 ++ .../RequestContextTestClient/autorest.md | 16 + 9 files changed, 757 insertions(+), 144 deletions(-) create mode 100644 sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj create mode 100644 sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClient.cs create mode 100644 sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClientOptions.cs create mode 100644 sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/autorest.md diff --git a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs index 2e2fbd1aa19b..7b8f0400c895 100644 --- a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs +++ b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs @@ -212,145 +212,6 @@ public void ThrowOnErrorThrowsOnError() }); } - [Test] - public async Task CanAddPerCallPolicy() - { - var petResponse = new MockResponse(200); - - Pet pet = new("snoopy", "beagle"); - petResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); - - var mockTransport = new MockTransport(petResponse); - PetStoreClient client = CreateClient(mockTransport); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); - - Response response = await client.GetPetAsync("snoopy", context); - - Request request = mockTransport.Requests[0]; - Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader", out var values)); - Assert.AreEqual(1, values.Count()); - Assert.AreEqual("Value", values.ElementAt(0)); - } - - [Test] - public async Task CanAddPerRetryPolicy() - { - var retryResponse = new MockResponse(408); // Request Timeout - var petResponse = new MockResponse(200); - - Pet pet = new("snoopy", "beagle"); - petResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); - - // retry twice -- this will add the header three times. - var mockTransport = new MockTransport(retryResponse, retryResponse, petResponse); - PetStoreClient client = CreateClient(mockTransport); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "Value"), HttpPipelinePosition.PerRetry); - - Response response = await client.GetPetAsync("snoopy", context); - - Request request = mockTransport.Requests[0]; - Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var values)); - Assert.AreEqual(3, values.Count()); - Assert.AreEqual("Value", values.ElementAt(0)); - Assert.AreEqual("Value", values.ElementAt(1)); - Assert.AreEqual("Value", values.ElementAt(2)); - } - - [Test] - public async Task CanAddBeforeTransportPolicy() - { - var retryResponse = new MockResponse(408); // Request Timeout - var petResponse = new MockResponse(200); - - Pet pet = new("snoopy", "beagle"); - petResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); - - // retry twice - var mockTransport = new MockTransport(retryResponse, retryResponse, petResponse); - PetStoreClient client = CreateClient(mockTransport); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "Value"), HttpPipelinePosition.BeforeTransport); - - Response response = await client.GetPetAsync("snoopy", context); - - Request request = mockTransport.Requests[0]; - Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var values)); - Assert.AreEqual(3, values.Count()); - Assert.AreEqual("Value", values.ElementAt(0)); - Assert.AreEqual("Value", values.ElementAt(1)); - Assert.AreEqual("Value", values.ElementAt(2)); - } - - [Test] - public async Task CanAddPoliciesAllPositions() - { - var retryResponse = new MockResponse(408); // Request Timeout - var petResponse = new MockResponse(200); - - Pet pet = new("snoopy", "beagle"); - petResponse.SetContent(SerializationHelpers.Serialize(pet, SerializePet)); - - // retry twice -- this will add the header three times. - var mockTransport = new MockTransport(retryResponse, retryResponse, petResponse); - PetStoreClient client = CreateClient(mockTransport); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("PerCallHeader1", "PerCall1"), HttpPipelinePosition.PerCall); - context.AddPolicy(new AddHeaderPolicy("PerCallHeader2", "PerCall2"), HttpPipelinePosition.PerCall); - context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "PerRetry"), HttpPipelinePosition.PerRetry); - context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "BeforeTransport"), HttpPipelinePosition.BeforeTransport); - - Response response = await client.GetPetAsync("snoopy", context); - - Request request = mockTransport.Requests[0]; - - Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader1", out var perCall1Values)); - Assert.AreEqual(1, perCall1Values.Count()); - Assert.AreEqual("PerCall1", perCall1Values.ElementAt(0)); - - Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader2", out var perCall2Values)); - Assert.AreEqual(1, perCall2Values.Count()); - Assert.AreEqual("PerCall2", perCall2Values.ElementAt(0)); - - Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var perRetryValues)); - Assert.AreEqual("PerRetry", perRetryValues.ElementAt(0)); - Assert.AreEqual("PerRetry", perRetryValues.ElementAt(1)); - Assert.AreEqual("PerRetry", perRetryValues.ElementAt(2)); - - Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var beforeTransportValues)); - Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(0)); - Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(1)); - Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(2)); - } - - [Test] - public async Task ThrowsIfUsePipelineConstructor() - { - HttpPipeline pipeline = new HttpPipeline(new MockTransport()); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); - - var message = pipeline.CreateMessage(context); - - bool throws = false; - try - { - await pipeline.SendAsync(message, context.CancellationToken); - } - catch (InvalidOperationException) - { - throws = true; - } - - Assert.IsTrue(throws); - } - #region Helpers private void SerializePet(ref Utf8JsonWriter writer, Pet pet) { diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index d553dd2452ec..397bb18d1acc 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -98,7 +98,7 @@ public async ValueTask SendAsync(HttpMessage message, CancellationToken cancella message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - if (message.Policies == null) + if (message.Policies == null || message.Policies.Count == 0) { await _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)).ConfigureAwait(false); } @@ -128,7 +128,7 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - if (message.Policies == null) + if (message.Policies == null || message.Policies.Count == 0) { _pipeline.Span[0].Process(message, _pipeline.Slice(1)); } diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index 90aec1225e6c..c24eaff71300 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -76,14 +76,12 @@ void AddCustomerPolicies(HttpPipelinePosition position) { foreach (var policy in options.Policies) { - if (policy.Position == position) + if (policy.Position == position && policy.Policy != null) { policies.Add(policy.Policy); } } } - - policies.RemoveAll(static policy => policy == null); } DiagnosticsOptions diagnostics = options.Diagnostics; diff --git a/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj b/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj index 14d9256a4d56..d160a3259ea7 100644 --- a/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj +++ b/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj @@ -5,6 +5,7 @@ $(DefineConstants);HAS_INTERNALS_VISIBLE_CORE true true + $(DefineConstants);EXPERIMENTAL @@ -45,6 +46,9 @@ + + + diff --git a/sdk/core/Azure.Core/tests/RequestContextTests.cs b/sdk/core/Azure.Core/tests/RequestContextTests.cs index f73dda61475c..38b1242d6433 100644 --- a/sdk/core/Azure.Core/tests/RequestContextTests.cs +++ b/sdk/core/Azure.Core/tests/RequestContextTests.cs @@ -5,13 +5,30 @@ using System.Collections.Generic; using System.Linq; using System.Text; +using System.Text.Json; using System.Threading.Tasks; +using Azure.Core.Pipeline; +using Azure.Core.TestFramework; +using Azure.Template.LLC; using NUnit.Framework; namespace Azure.Core.Tests { public class RequestContextTests { + private readonly Uri _url = new Uri("https://example.azuretemplateservice.com"); + + public TemplateServiceClient CreateClient(HttpPipelineTransport transport) + { + var options = new TemplateServiceClientOptions() + { + Transport = transport + }; + + // TODO: Fix ordering + return new TemplateServiceClient(new MockCredential(), _url, options); + } + [Test] public void CanCastFromErrorOptions() { @@ -27,5 +44,244 @@ public void CanSetErrorOptions() Assert.IsTrue(context.ErrorOptions == ErrorOptions.NoThrow); } + + [Test] + public async Task CanAddPolicy_PerCall() + { + var resourceResponse = new MockResponse(200); + + var resource = + @"{ + name = ""snoopy"", + id = ""beagle"" + }"; + + resourceResponse.SetContent(resource); + + var mockTransport = new MockTransport(resourceResponse); + TemplateServiceClient client = CreateClient(mockTransport); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); + + Response response = await client.GetAsync("snoopy", context); + + Request request = mockTransport.Requests[0]; + Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader", out var values)); + Assert.AreEqual(1, values.Count()); + Assert.AreEqual("Value", values.ElementAt(0)); + } + + [Test] + public async Task CanAddPolicy_PerRetry() + { + var retryResponse = new MockResponse(408); // Request Timeout + var resourceResponse = new MockResponse(200); + + var resource = + @"{ + name = ""snoopy"", + id = ""beagle"" + }"; + + resourceResponse.SetContent(resource); + + // retry twice + var mockTransport = new MockTransport(retryResponse, retryResponse, resourceResponse); + TemplateServiceClient client = CreateClient(mockTransport); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "Value"), HttpPipelinePosition.PerRetry); + + Response response = await client.GetAsync("snoopy", context); + + Request request = mockTransport.Requests[0]; + Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var values)); + Assert.AreEqual(3, values.Count()); + Assert.AreEqual("Value", values.ElementAt(0)); + Assert.AreEqual("Value", values.ElementAt(1)); + Assert.AreEqual("Value", values.ElementAt(2)); + } + + [Test] + public async Task CanAddPolicy_BeforeTransport() + { + var retryResponse = new MockResponse(408); // Request Timeout + var resourceResponse = new MockResponse(200); + + var resource = + @"{ + name = ""snoopy"", + id = ""beagle"" + }"; + + resourceResponse.SetContent(resource); + + // retry twice + var mockTransport = new MockTransport(retryResponse, retryResponse, resourceResponse); + TemplateServiceClient client = CreateClient(mockTransport); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "Value"), HttpPipelinePosition.BeforeTransport); + + Response response = await client.GetAsync("snoopy", context); + + Request request = mockTransport.Requests[0]; + Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var values)); + Assert.AreEqual(3, values.Count()); + Assert.AreEqual("Value", values.ElementAt(0)); + Assert.AreEqual("Value", values.ElementAt(1)); + Assert.AreEqual("Value", values.ElementAt(2)); + } + + [Test] + public async Task CanAddPolicies_AllPositions() + { + var retryResponse = new MockResponse(408); // Request Timeout + var resourceResponse = new MockResponse(200); + + var resource = + @"{ + name = ""snoopy"", + id = ""beagle"" + }"; + + resourceResponse.SetContent(resource); + + // retry twice -- this will add the header three times. + var mockTransport = new MockTransport(retryResponse, retryResponse, resourceResponse); + TemplateServiceClient client = CreateClient(mockTransport); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader1", "PerCall1"), HttpPipelinePosition.PerCall); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader2", "PerCall2"), HttpPipelinePosition.PerCall); + context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "PerRetry"), HttpPipelinePosition.PerRetry); + context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "BeforeTransport"), HttpPipelinePosition.BeforeTransport); + + // TODO: Should be Get + Response response = await client.GetAsync("snoopy", context); + + Request request = mockTransport.Requests[0]; + + Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader1", out var perCall1Values)); + Assert.AreEqual(1, perCall1Values.Count()); + Assert.AreEqual("PerCall1", perCall1Values.ElementAt(0)); + + Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader2", out var perCall2Values)); + Assert.AreEqual(1, perCall2Values.Count()); + Assert.AreEqual("PerCall2", perCall2Values.ElementAt(0)); + + Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var perRetryValues)); + Assert.AreEqual("PerRetry", perRetryValues.ElementAt(0)); + Assert.AreEqual("PerRetry", perRetryValues.ElementAt(1)); + Assert.AreEqual("PerRetry", perRetryValues.ElementAt(2)); + + Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var beforeTransportValues)); + Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(0)); + Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(1)); + Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(2)); + } + + [Test] + public async Task CanAddPolicies_ThreeWays() + { + var retryResponse = new MockResponse(408); // Request Timeout + var resourceResponse = new MockResponse(200); + var resource = + @"{ + name = ""snoopy"", + id = ""beagle"" + }"; + resourceResponse.SetContent(resource); + + var mockTransport = new MockTransport(retryResponse, resourceResponse); + var options = new TemplateServiceClientOptions() + { + Transport = mockTransport, + }; + + var perCallPolicies = new HttpPipelinePolicy[] { new AddHeaderPolicy("PerCall", "Builder") }; + var perRetryPolicies = new HttpPipelinePolicy[] { new AddHeaderPolicy("PerRetry", "Builder") }; + + options.AddPolicy(new AddHeaderPolicy("BeforeTransport", "ClientOptions"), HttpPipelinePosition.BeforeTransport); + options.AddPolicy(new AddHeaderPolicy("PerRetry", "ClientOptions"), HttpPipelinePosition.PerRetry); + options.AddPolicy(new AddHeaderPolicy("PerCall", "ClientOptions"), HttpPipelinePosition.PerCall); + + var pipeline = HttpPipelineBuilder.Build(options, perCallPolicies, perRetryPolicies, null); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerRetry", "RequestContext"), HttpPipelinePosition.PerRetry); + context.AddPolicy(new AddHeaderPolicy("PerCall", "RequestContext"), HttpPipelinePosition.PerCall); + context.AddPolicy(new AddHeaderPolicy("BeforeTransport", "RequestContext"), HttpPipelinePosition.BeforeTransport); + + var message = pipeline.CreateMessage(context); + await pipeline.SendAsync(message, message.CancellationToken); + + Request request = mockTransport.Requests[0]; + + Assert.IsTrue(request.Headers.TryGetValues("PerCall", out var perCallValues)); + Assert.AreEqual(3, perCallValues.Count()); + Assert.AreEqual("Builder", perCallValues.ElementAt(0)); + Assert.AreEqual("ClientOptions", perCallValues.ElementAt(1)); + Assert.AreEqual("RequestContext", perCallValues.ElementAt(2)); + + Assert.IsTrue(request.Headers.TryGetValues("PerRetry", out var perRetryValues)); + Assert.AreEqual(6, perRetryValues.Count()); + Assert.AreEqual("Builder", perRetryValues.ElementAt(0)); + Assert.AreEqual("ClientOptions", perRetryValues.ElementAt(1)); + Assert.AreEqual("RequestContext", perRetryValues.ElementAt(2)); + Assert.AreEqual("Builder", perRetryValues.ElementAt(3)); + Assert.AreEqual("ClientOptions", perRetryValues.ElementAt(4)); + Assert.AreEqual("RequestContext", perRetryValues.ElementAt(5)); + + Assert.IsTrue(request.Headers.TryGetValues("BeforeTransport", out var beforeTransportValues)); + Assert.AreEqual(4, beforeTransportValues.Count()); + Assert.AreEqual("ClientOptions", beforeTransportValues.ElementAt(0)); + Assert.AreEqual("RequestContext", beforeTransportValues.ElementAt(1)); + Assert.AreEqual("ClientOptions", beforeTransportValues.ElementAt(2)); + Assert.AreEqual("RequestContext", beforeTransportValues.ElementAt(3)); + } + + [Test] + public async Task ThrowsIfUsePipelineConstructor() + { + HttpPipeline pipeline = new HttpPipeline(new MockTransport()); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); + + var message = pipeline.CreateMessage(context); + + bool throws = false; + try + { + await pipeline.SendAsync(message, context.CancellationToken); + } + catch (InvalidOperationException) + { + throws = true; + } + + Assert.IsTrue(throws); + } + + #region Helpers + public class AddHeaderPolicy : HttpPipelineSynchronousPolicy + { + private string _headerName; + private string _headerVaue; + + public AddHeaderPolicy(string headerName, string headerValue) : base() + { + _headerName = headerName; + _headerVaue = headerValue; + } + + public override void OnSendingRequest(HttpMessage message) + { + message.Request.Headers.Add(_headerName, _headerVaue); + } + } + #endregion } } diff --git a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj new file mode 100644 index 000000000000..cab5f1be7398 --- /dev/null +++ b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj @@ -0,0 +1,35 @@ + + + This is a LLC template project to demonstrate how to create a package that uses code generation as well as use for testing our build and release pipelines + Azure SDK LLC Template + 1.0.0-beta.4 + Azure Template + $(RequiredTargetFrameworks) + $(NoWarn);419 + true + $(DefineConstants);EXPERIMENTAL + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClient.cs b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClient.cs new file mode 100644 index 000000000000..0069f7dcbef1 --- /dev/null +++ b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClient.cs @@ -0,0 +1,406 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// + +#nullable disable + +using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Threading; +using System.Threading.Tasks; +using Azure; +using Azure.Core; +using Azure.Core.Pipeline; + +namespace Azure.Template.LLC +{ + /// The TemplateService service client. + public partial class TemplateServiceClient + { + private static readonly string[] AuthorizationScopes = { "https://dev.LLCtemplate.net/.default" }; + private readonly TokenCredential _tokenCredential; + + private readonly HttpPipeline _pipeline; + private readonly ClientDiagnostics _clientDiagnostics; + private readonly Uri _endpoint; + + /// The HTTP pipeline for sending and receiving REST requests and responses. + public virtual HttpPipeline Pipeline { get => _pipeline; } + + /// Initializes a new instance of TemplateServiceClient for mocking. + protected TemplateServiceClient() + { + } + + /// Initializes a new instance of TemplateServiceClient. + /// A credential used to authenticate to an Azure Service. + /// server parameter. + /// The options for configuring the client. + /// is null. + public TemplateServiceClient(TokenCredential credential, Uri endpoint = null, TemplateServiceClientOptions options = null) + { + if (credential == null) + { + throw new ArgumentNullException(nameof(credential)); + } + endpoint ??= new Uri("http://localhost:3000"); + + options ??= new TemplateServiceClientOptions(); + + _clientDiagnostics = new ClientDiagnostics(options); + _tokenCredential = credential; + _pipeline = HttpPipelineBuilder.Build(options, new HttpPipelinePolicy[] { new LowLevelCallbackPolicy() }, new HttpPipelinePolicy[] { new BearerTokenAuthenticationPolicy(_tokenCredential, AuthorizationScopes) }, new ResponseClassifier()); + _endpoint = endpoint; + } + + /// Create or update resource. + /// The content to send as the body of the request. + /// The request context. + /// + /// Schema for Request Body: + /// { + /// name: string, + /// id: string + /// } + /// + /// Schema for Response Body: + /// { + /// name: string, + /// id: string + /// } + /// + /// + /// +#pragma warning disable AZC0002 + public virtual async Task CreateAsync(RequestContent content, RequestContext context = null) +#pragma warning restore AZC0002 + { + using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Create"); + scope.Start(); + try + { + using HttpMessage message = CreateCreateRequest(content, context); + return await _pipeline.ProcessMessageAsync(message, _clientDiagnostics, context).ConfigureAwait(false); + } + catch (Exception e) + { + scope.Failed(e); + throw; + } + } + + /// Create or update resource. + /// The content to send as the body of the request. + /// The request context. + /// + /// Schema for Request Body: + /// { + /// name: string, + /// id: string + /// } + /// + /// Schema for Response Body: + /// { + /// name: string, + /// id: string + /// } + /// + /// + /// +#pragma warning disable AZC0002 + public virtual Response Create(RequestContent content, RequestContext context = null) +#pragma warning restore AZC0002 + { + using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Create"); + scope.Start(); + try + { + using HttpMessage message = CreateCreateRequest(content, context); + return _pipeline.ProcessMessage(message, _clientDiagnostics, context); + } + catch (Exception e) + { + scope.Failed(e); + throw; + } + } + + /// Delete resource. + /// The id of the resource. + /// The request context. + /// is null. +#pragma warning disable AZC0002 + public virtual async Task DeleteAsync(string resourceId, RequestContext context = null) +#pragma warning restore AZC0002 + { + using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Delete"); + scope.Start(); + try + { + using HttpMessage message = CreateDeleteRequest(resourceId, context); + return await _pipeline.ProcessMessageAsync(message, _clientDiagnostics, context).ConfigureAwait(false); + } + catch (Exception e) + { + scope.Failed(e); + throw; + } + } + + /// Delete resource. + /// The id of the resource. + /// The request context. + /// is null. +#pragma warning disable AZC0002 + public virtual Response Delete(string resourceId, RequestContext context = null) +#pragma warning restore AZC0002 + { + using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Delete"); + scope.Start(); + try + { + using HttpMessage message = CreateDeleteRequest(resourceId, context); + return _pipeline.ProcessMessage(message, _clientDiagnostics, context); + } + catch (Exception e) + { + scope.Failed(e); + throw; + } + } + + /// Retrieves information about the resource. + /// The id of the resource. + /// The request context. + /// is null. + /// + /// Schema for Response Body: + /// { + /// name: string, + /// id: string + /// } + /// + /// + /// +#pragma warning disable AZC0002 + public virtual async Task GetAsync(string resourceId, RequestContext context = null) +#pragma warning restore AZC0002 + { + using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Get"); + scope.Start(); + try + { + using HttpMessage message = CreateGetRequest(resourceId, context); + return await _pipeline.ProcessMessageAsync(message, _clientDiagnostics, context).ConfigureAwait(false); + } + catch (Exception e) + { + scope.Failed(e); + throw; + } + } + + /// Retrieves information about the resource. + /// The id of the resource. + /// The request context. + /// is null. + /// + /// Schema for Response Body: + /// { + /// name: string, + /// id: string + /// } + /// + /// + /// +#pragma warning disable AZC0002 + public virtual Response Get(string resourceId, RequestContext context = null) +#pragma warning restore AZC0002 + { + using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Get"); + scope.Start(); + try + { + using HttpMessage message = CreateGetRequest(resourceId, context); + return _pipeline.ProcessMessage(message, _clientDiagnostics, context); + } + catch (Exception e) + { + scope.Failed(e); + throw; + } + } + + /// Retrieves the list of resources. + /// The request context. + /// + /// Schema for Response Body: + /// { + /// value: [ + /// { + /// name: string, + /// id: string + /// } + /// ], + /// nextLink: string + /// } + /// + /// + /// +#pragma warning disable AZC0002 + public virtual AsyncPageable GetResourcesAsync(RequestContext context = null) +#pragma warning restore AZC0002 + { + return PageableHelpers.CreateAsyncPageable(CreateEnumerableAsync, _clientDiagnostics, "TemplateServiceClient.GetResources"); + async IAsyncEnumerable> CreateEnumerableAsync(string nextLink, int? pageSizeHint, [EnumeratorCancellation] CancellationToken cancellationToken = default) + { + do + { + var message = string.IsNullOrEmpty(nextLink) + ? CreateGetResourcesRequest(context) + : CreateGetResourcesNextPageRequest(nextLink, context); + var page = await LowLevelPageableHelpers.ProcessMessageAsync(_pipeline, message, _clientDiagnostics, context, "value", "nextLink", cancellationToken).ConfigureAwait(false); + nextLink = page.ContinuationToken; + yield return page; + } while (!string.IsNullOrEmpty(nextLink)); + } + } + + /// Retrieves the list of resources. + /// The request context. + /// + /// Schema for Response Body: + /// { + /// value: [ + /// { + /// name: string, + /// id: string + /// } + /// ], + /// nextLink: string + /// } + /// + /// + /// +#pragma warning disable AZC0002 + public virtual Pageable GetResources(RequestContext context = null) +#pragma warning restore AZC0002 + { + return PageableHelpers.CreatePageable(CreateEnumerable, _clientDiagnostics, "TemplateServiceClient.GetResources"); + IEnumerable> CreateEnumerable(string nextLink, int? pageSizeHint) + { + do + { + var message = string.IsNullOrEmpty(nextLink) + ? CreateGetResourcesRequest(context) + : CreateGetResourcesNextPageRequest(nextLink, context); + var page = LowLevelPageableHelpers.ProcessMessage(_pipeline, message, _clientDiagnostics, context, "value", "nextLink"); + nextLink = page.ContinuationToken; + yield return page; + } while (!string.IsNullOrEmpty(nextLink)); + } + } + + internal HttpMessage CreateCreateRequest(RequestContent content, RequestContext context) + { + var message = _pipeline.CreateMessage(context); + var request = message.Request; + request.Method = RequestMethod.Put; + var uri = new RawRequestUriBuilder(); + uri.Reset(_endpoint); + uri.AppendPath("/template/resources", false); + request.Uri = uri; + request.Headers.Add("Accept", "application/json"); + request.Headers.Add("Content-Type", "application/json"); + request.Content = content; + message.ResponseClassifier = ResponseClassifier200.Instance; + return message; + } + + internal HttpMessage CreateGetResourcesRequest(RequestContext context) + { + var message = _pipeline.CreateMessage(context); + var request = message.Request; + request.Method = RequestMethod.Get; + var uri = new RawRequestUriBuilder(); + uri.Reset(_endpoint); + uri.AppendPath("/template/resources", false); + request.Uri = uri; + request.Headers.Add("Accept", "application/json"); + message.ResponseClassifier = ResponseClassifier200.Instance; + return message; + } + + internal HttpMessage CreateDeleteRequest(string resourceId, RequestContext context) + { + var message = _pipeline.CreateMessage(context); + var request = message.Request; + request.Method = RequestMethod.Delete; + var uri = new RawRequestUriBuilder(); + uri.Reset(_endpoint); + uri.AppendPath("/template/resources/", false); + uri.AppendPath(resourceId, true); + request.Uri = uri; + message.ResponseClassifier = ResponseClassifier204.Instance; + return message; + } + + internal HttpMessage CreateGetRequest(string resourceId, RequestContext context) + { + var message = _pipeline.CreateMessage(context); + var request = message.Request; + request.Method = RequestMethod.Get; + var uri = new RawRequestUriBuilder(); + uri.Reset(_endpoint); + uri.AppendPath("/template/resources/", false); + uri.AppendPath(resourceId, true); + request.Uri = uri; + request.Headers.Add("Accept", "application/json"); + message.ResponseClassifier = ResponseClassifier200.Instance; + return message; + } + + internal HttpMessage CreateGetResourcesNextPageRequest(string nextLink, RequestContext context) + { + var message = _pipeline.CreateMessage(); + var request = message.Request; + request.Method = RequestMethod.Get; + var uri = new RawRequestUriBuilder(); + uri.Reset(_endpoint); + uri.AppendRawNextLink(nextLink, false); + request.Uri = uri; + request.Headers.Add("Accept", "application/json"); + message.ResponseClassifier = ResponseClassifier200.Instance; + return message; + } + + private sealed class ResponseClassifier200 : ResponseClassifier + { + private static ResponseClassifier _instance; + public static ResponseClassifier Instance => _instance ??= new ResponseClassifier200(); + public override bool IsErrorResponse(HttpMessage message) + { + return message.Response.Status switch + { + 200 => false, + _ => true + }; + } + } + private sealed class ResponseClassifier204 : ResponseClassifier + { + private static ResponseClassifier _instance; + public static ResponseClassifier Instance => _instance ??= new ResponseClassifier204(); + public override bool IsErrorResponse(HttpMessage message) + { + return message.Response.Status switch + { + 204 => false, + _ => true + }; + } + } + } +} diff --git a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClientOptions.cs b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClientOptions.cs new file mode 100644 index 000000000000..8cc3f1a29cef --- /dev/null +++ b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClientOptions.cs @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// + +#nullable disable + +using System; +using Azure.Core; + +namespace Azure.Template.LLC +{ + /// Client options for TemplateServiceClient. + public partial class TemplateServiceClientOptions : ClientOptions + { + private const ServiceVersion LatestVersion = ServiceVersion.V1_0_0; + + /// The version of the service to use. + public enum ServiceVersion + { + /// Service version "1.0.0". + V1_0_0 = 1, + } + + internal string Version { get; } + + /// Initializes new instance of TemplateServiceClientOptions. + public TemplateServiceClientOptions(ServiceVersion version = LatestVersion) + { + Version = version switch + { + ServiceVersion.V1_0_0 => "1.0.0", + _ => throw new NotSupportedException() + }; + } + } +} diff --git a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/autorest.md b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/autorest.md new file mode 100644 index 000000000000..e3fa624ba2a6 --- /dev/null +++ b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/autorest.md @@ -0,0 +1,16 @@ +# Azure.Template.LLC Code Generation + +Run `dotnet build /t:GenerateCode` to generate code. + +### AutoRest Configuration +> see https://aka.ms/autorest + +``` yaml +input-file: +- https://github.com/Azure/azure-sdk-for-net/blob/dd410c32cee19e300245e11149c28c2ea7fb169e/sdk/template-LLC/Azure.Template.LLC/src/swagger/swagger.json +namespace: Azure.Template.LLC +public-clients: true +low-level-client: true +security: AADToken +security-scopes: https://dev.LLCtemplate.net/.default +``` From 88d0b747c89680d9956111a22d12786b4e62ec56 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 16 Nov 2021 14:25:55 -0800 Subject: [PATCH 18/26] nit --- .../tests/LowLevelClientTests.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs index 7b8f0400c895..95ca5c724d6c 100644 --- a/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs +++ b/sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using System; -using System.Linq; using System.Text.Json; using System.Threading.Tasks; using Azure.Core.Experimental.Tests; @@ -225,23 +224,6 @@ private void SerializePet(ref Utf8JsonWriter writer, Pet pet) writer.WriteEndObject(); } - - public class AddHeaderPolicy : HttpPipelineSynchronousPolicy - { - private string _headerName; - private string _headerVaue; - - public AddHeaderPolicy(string headerName, string headerValue) : base() - { - _headerName = headerName; - _headerVaue = headerValue; - } - - public override void OnSendingRequest(HttpMessage message) - { - message.Request.Headers.Add(_headerName, _headerVaue); - } - } #endregion } } From acba4154d5d95350d4cd566ce2f5688fe7626716 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 16 Nov 2021 15:26:24 -0800 Subject: [PATCH 19/26] pr fb --- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 397bb18d1acc..ce0b346c6ec9 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -20,10 +20,24 @@ public class HttpPipeline private readonly ReadOnlyMemory _pipeline; + /// + /// Indicates whether or not the pipeline was created using its internal constructor. + /// If it was, we know the indices where we can add per-request policies at positions + /// and . + /// private readonly bool _internallyConstructed; + + /// + /// The pipeline index where policies will be added, + /// if any are specified using . + /// private readonly int _perCallIndex; + + /// + /// The pipeline index where policies will be added, + /// if any are specified using . + /// private readonly int _perRetryIndex; - private readonly int _transportIndex; /// /// Creates a new instance of with the provided transport, policies and response classifier. @@ -42,8 +56,6 @@ public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? polic all[policies.Length] = new HttpPipelineTransportPolicy(_transport); policies.CopyTo(all, 0); - _transportIndex = policies.Length; - _pipeline = all; } @@ -229,14 +241,14 @@ private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePol count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.PerRetry, index); index += count; - count = _transportIndex - _perRetryIndex; + count = _pipeline.Length - _perRetryIndex; _pipeline.Slice(_perRetryIndex, count).CopyTo(new Memory(policies, index, count)); index += count; count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.BeforeTransport, index); index += count; - policies[index] = _pipeline.Span[_transportIndex]; + policies[index] = _pipeline.Span[_pipeline.Length]; return new ReadOnlyMemory(policies, 0, index + 1); } From 2c325a169e053ca859ce1c85bc74244e9f0bcbda Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 16 Nov 2021 15:46:47 -0800 Subject: [PATCH 20/26] pr fb & fix build --- sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs | 5 +++-- .../RequestContextTestClient/Azure.Template.LLC.csproj | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index ce0b346c6ec9..05fbfa90a783 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -227,6 +227,7 @@ private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePol throw new InvalidOperationException("Cannot send messages with per-request policies if the pipeline wasn't constructed with HttpPipelineBuilder."); } + int transportIndex = _pipeline.Length - 1; // Copy over client policies and splice in custom policies at designated indices _pipeline.Slice(0, _perCallIndex).CopyTo(policies); @@ -241,14 +242,14 @@ private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePol count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.PerRetry, index); index += count; - count = _pipeline.Length - _perRetryIndex; + count = transportIndex - _perRetryIndex; _pipeline.Slice(_perRetryIndex, count).CopyTo(new Memory(policies, index, count)); index += count; count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.BeforeTransport, index); index += count; - policies[index] = _pipeline.Span[_pipeline.Length]; + policies[index] = _pipeline.Span[transportIndex]; return new ReadOnlyMemory(policies, 0, index + 1); } diff --git a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj index cab5f1be7398..a1d9b1c9ba0f 100644 --- a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj +++ b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj @@ -29,7 +29,7 @@ - + From cdaf0d4d023fdaf6b71783065ab0f47a1eb78563 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 16 Nov 2021 16:44:58 -0800 Subject: [PATCH 21/26] pr fb --- sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 05fbfa90a783..461b2c592262 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -227,8 +227,10 @@ private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePol throw new InvalidOperationException("Cannot send messages with per-request policies if the pipeline wasn't constructed with HttpPipelineBuilder."); } - int transportIndex = _pipeline.Length - 1; // Copy over client policies and splice in custom policies at designated indices + var pipeline = _pipeline.Span; + int transportIndex = _pipeline.Length - 1; + _pipeline.Slice(0, _perCallIndex).CopyTo(policies); int index = _perCallIndex; @@ -236,20 +238,20 @@ private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePol index += count; count = _perRetryIndex - _perCallIndex; - _pipeline.Slice(_perCallIndex, count).CopyTo(new Memory(policies, index, count)); + pipeline.Slice(_perCallIndex, count).CopyTo(policies.AsSpan(index, count)); index += count; count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.PerRetry, index); index += count; count = transportIndex - _perRetryIndex; - _pipeline.Slice(_perRetryIndex, count).CopyTo(new Memory(policies, index, count)); + pipeline.Slice(_perRetryIndex, count).CopyTo(policies.AsSpan(index, count)); index += count; count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.BeforeTransport, index); index += count; - policies[index] = _pipeline.Span[transportIndex]; + policies[index] = pipeline[transportIndex]; return new ReadOnlyMemory(policies, 0, index + 1); } From 9409ca433d8c8aa45c87f86fc1899cd57eca2f3c Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 17 Nov 2021 08:47:30 -0800 Subject: [PATCH 22/26] pr fb & build fix --- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 4 +-- .../Azure.Core/tests/Azure.Core.Tests.csproj | 3 -- .../Azure.Template.LLC.csproj | 35 ------------------- .../RequestContextTestClient/autorest.md | 16 --------- 4 files changed, 2 insertions(+), 56 deletions(-) delete mode 100644 sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj delete mode 100644 sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/autorest.md diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 461b2c592262..bd67642b049e 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -228,10 +228,10 @@ private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePol } // Copy over client policies and splice in custom policies at designated indices - var pipeline = _pipeline.Span; int transportIndex = _pipeline.Length - 1; + var pipeline = _pipeline.Span; - _pipeline.Slice(0, _perCallIndex).CopyTo(policies); + pipeline.Slice(0, _perCallIndex).CopyTo(policies); int index = _perCallIndex; int count = AddCustomPolicies(customPolicies, policies, HttpPipelinePosition.PerCall, index); diff --git a/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj b/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj index d160a3259ea7..ab788e2854c8 100644 --- a/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj +++ b/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj @@ -46,9 +46,6 @@ - - - diff --git a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj deleted file mode 100644 index a1d9b1c9ba0f..000000000000 --- a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Azure.Template.LLC.csproj +++ /dev/null @@ -1,35 +0,0 @@ - - - This is a LLC template project to demonstrate how to create a package that uses code generation as well as use for testing our build and release pipelines - Azure SDK LLC Template - 1.0.0-beta.4 - Azure Template - $(RequiredTargetFrameworks) - $(NoWarn);419 - true - $(DefineConstants);EXPERIMENTAL - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/autorest.md b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/autorest.md deleted file mode 100644 index e3fa624ba2a6..000000000000 --- a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/autorest.md +++ /dev/null @@ -1,16 +0,0 @@ -# Azure.Template.LLC Code Generation - -Run `dotnet build /t:GenerateCode` to generate code. - -### AutoRest Configuration -> see https://aka.ms/autorest - -``` yaml -input-file: -- https://github.com/Azure/azure-sdk-for-net/blob/dd410c32cee19e300245e11149c28c2ea7fb169e/sdk/template-LLC/Azure.Template.LLC/src/swagger/swagger.json -namespace: Azure.Template.LLC -public-clients: true -low-level-client: true -security: AADToken -security-scopes: https://dev.LLCtemplate.net/.default -``` From a681bc11dcb55b15537e1f3ae738535dddf0398a Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 17 Nov 2021 10:09:37 -0800 Subject: [PATCH 23/26] pr fb --- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 31 +- sdk/core/Azure.Core/src/RequestContext.cs | 8 +- .../Azure.Core/tests/Azure.Core.Tests.csproj | 1 - .../Azure.Core/tests/HttpPipelineTests.cs | 212 +++++++++ .../Azure.Core/tests/RequestContextTests.cs | 251 ----------- .../Generated/TemplateServiceClient.cs | 406 ------------------ .../Generated/TemplateServiceClientOptions.cs | 37 -- 7 files changed, 235 insertions(+), 711 deletions(-) delete mode 100644 sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClient.cs delete mode 100644 sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClientOptions.cs diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index bd67642b049e..dc9fbd9d636f 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -105,28 +105,31 @@ public HttpMessage CreateMessage(RequestContext context) /// The to send. /// The to use. /// The representing the asynchronous operation. - public async ValueTask SendAsync(HttpMessage message, CancellationToken cancellationToken) + public ValueTask SendAsync(HttpMessage message, CancellationToken cancellationToken) { message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); if (message.Policies == null || message.Policies.Count == 0) { - await _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)).ConfigureAwait(false); + return _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)); } - else + + return SendAsync(message); + } + + private async ValueTask SendAsync(HttpMessage message) + { + var length = _pipeline.Length + message.Policies!.Count; + var policies = ArrayPool.Shared.Rent(length); + try { - var length = _pipeline.Length + message.Policies.Count; - var policies = ArrayPool.Shared.Rent(length); - try - { - var pipeline = CreateRequestPipeline(policies, message.Policies); - await pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)).ConfigureAwait(false); - } - finally - { - ArrayPool.Shared.Return(policies); - } + var pipeline = CreateRequestPipeline(policies, message.Policies); + await pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)).ConfigureAwait(false); + } + finally + { + ArrayPool.Shared.Return(policies); } } diff --git a/sdk/core/Azure.Core/src/RequestContext.cs b/sdk/core/Azure.Core/src/RequestContext.cs index 9e8dda9ada0e..b569d9bac49d 100644 --- a/sdk/core/Azure.Core/src/RequestContext.cs +++ b/sdk/core/Azure.Core/src/RequestContext.cs @@ -40,9 +40,13 @@ public RequestContext() public ErrorOptions ErrorOptions { get; set; } = ErrorOptions.Default; /// + /// Adds an into the pipeline for the duration of this request. + /// The position of policy in the pipeline is controlled by parameter. + /// If you want the policy to execute once per client request use + /// otherwise use to run the policy for every retry. /// - /// - /// + /// The instance to be added to the pipeline. + /// The position of the policy in the pipeline. public void AddPolicy(HttpPipelinePolicy policy, HttpPipelinePosition position) { Policies ??= new(); diff --git a/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj b/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj index ab788e2854c8..14d9256a4d56 100644 --- a/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj +++ b/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj @@ -5,7 +5,6 @@ $(DefineConstants);HAS_INTERNALS_VISIBLE_CORE true true - $(DefineConstants);EXPERIMENTAL diff --git a/sdk/core/Azure.Core/tests/HttpPipelineTests.cs b/sdk/core/Azure.Core/tests/HttpPipelineTests.cs index 7582b41237b2..3f2a682c5395 100644 --- a/sdk/core/Azure.Core/tests/HttpPipelineTests.cs +++ b/sdk/core/Azure.Core/tests/HttpPipelineTests.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; +using System.Linq; using System.Threading.Tasks; using Azure.Core.Pipeline; using Azure.Core.TestFramework; @@ -29,5 +31,215 @@ public async Task DoesntDisposeRequestInSendRequestAsync() private class TestOptions : ClientOptions { } + + [Test] + public async Task CanAddPolicy_PerCall() + { + var mockTransport = new MockTransport(new MockResponse(200)); + var options = new TestOptions() + { + Transport = mockTransport, + }; + var pipeline = HttpPipelineBuilder.Build(options); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); + + var message = pipeline.CreateMessage(context); + await pipeline.SendAsync(message, message.CancellationToken); + + Request request = mockTransport.Requests[0]; + Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader", out var values)); + Assert.AreEqual(1, values.Count()); + Assert.AreEqual("Value", values.ElementAt(0)); + } + + [Test] + public async Task CanAddPolicy_PerRetry() + { + var retryResponse = new MockResponse(408); // Request Timeout + var mockTransport = new MockTransport(retryResponse, retryResponse, new MockResponse(200)); + var options = new TestOptions() + { + Transport = mockTransport, + }; + + var pipeline = HttpPipelineBuilder.Build(options); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "Value"), HttpPipelinePosition.PerRetry); + + var message = pipeline.CreateMessage(context); + await pipeline.SendAsync(message, message.CancellationToken); + + Request request = mockTransport.Requests[0]; + Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var values)); + Assert.AreEqual(3, values.Count()); + Assert.AreEqual("Value", values.ElementAt(0)); + Assert.AreEqual("Value", values.ElementAt(1)); + Assert.AreEqual("Value", values.ElementAt(2)); + } + + [Test] + public async Task CanAddPolicy_BeforeTransport() + { + var retryResponse = new MockResponse(408); // Request Timeout + + // retry twice + var mockTransport = new MockTransport(retryResponse, retryResponse, new MockResponse(200)); + var options = new TestOptions() + { + Transport = mockTransport, + }; + + var pipeline = HttpPipelineBuilder.Build(options); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "Value"), HttpPipelinePosition.BeforeTransport); + + var message = pipeline.CreateMessage(context); + await pipeline.SendAsync(message, message.CancellationToken); + + Request request = mockTransport.Requests[0]; + + Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var values)); + Assert.AreEqual(3, values.Count()); + Assert.AreEqual("Value", values.ElementAt(0)); + Assert.AreEqual("Value", values.ElementAt(1)); + Assert.AreEqual("Value", values.ElementAt(2)); + } + + [Test] + public async Task CanAddRequestPolicies_AllPositions() + { + var retryResponse = new MockResponse(408); // Request Timeout + + // retry twice -- this will add the header three times. + var mockTransport = new MockTransport(retryResponse, retryResponse, new MockResponse(200)); + var options = new TestOptions() + { + Transport = mockTransport, + }; + + var pipeline = HttpPipelineBuilder.Build(options); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader1", "PerCall1"), HttpPipelinePosition.PerCall); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader2", "PerCall2"), HttpPipelinePosition.PerCall); + context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "PerRetry"), HttpPipelinePosition.PerRetry); + context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "BeforeTransport"), HttpPipelinePosition.BeforeTransport); + + var message = pipeline.CreateMessage(context); + await pipeline.SendAsync(message, message.CancellationToken); + + Request request = mockTransport.Requests[0]; + + Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader1", out var perCall1Values)); + Assert.AreEqual(1, perCall1Values.Count()); + Assert.AreEqual("PerCall1", perCall1Values.ElementAt(0)); + + Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader2", out var perCall2Values)); + Assert.AreEqual(1, perCall2Values.Count()); + Assert.AreEqual("PerCall2", perCall2Values.ElementAt(0)); + + Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var perRetryValues)); + Assert.AreEqual("PerRetry", perRetryValues.ElementAt(0)); + Assert.AreEqual("PerRetry", perRetryValues.ElementAt(1)); + Assert.AreEqual("PerRetry", perRetryValues.ElementAt(2)); + + Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var beforeTransportValues)); + Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(0)); + Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(1)); + Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(2)); + } + + [Test] + public async Task CanAddPolicies_ThreeWays() + { + var mockTransport = new MockTransport(new MockResponse(200)); + var options = new TestOptions() + { + Transport = mockTransport, + }; + + var perCallPolicies = new HttpPipelinePolicy[] { new AddHeaderPolicy("PerCall", "Builder") }; + var perRetryPolicies = new HttpPipelinePolicy[] { new AddHeaderPolicy("PerRetry", "Builder") }; + + options.AddPolicy(new AddHeaderPolicy("BeforeTransport", "ClientOptions"), HttpPipelinePosition.BeforeTransport); + options.AddPolicy(new AddHeaderPolicy("PerRetry", "ClientOptions"), HttpPipelinePosition.PerRetry); + options.AddPolicy(new AddHeaderPolicy("PerCall", "ClientOptions"), HttpPipelinePosition.PerCall); + + var pipeline = HttpPipelineBuilder.Build(options, perCallPolicies, perRetryPolicies, null); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerRetry", "RequestContext"), HttpPipelinePosition.PerRetry); + context.AddPolicy(new AddHeaderPolicy("PerCall", "RequestContext"), HttpPipelinePosition.PerCall); + context.AddPolicy(new AddHeaderPolicy("BeforeTransport", "RequestContext"), HttpPipelinePosition.BeforeTransport); + + var message = pipeline.CreateMessage(context); + await pipeline.SendAsync(message, message.CancellationToken); + + Request request = mockTransport.Requests[0]; + + Assert.IsTrue(request.Headers.TryGetValues("PerCall", out var perCallValues)); + Assert.AreEqual(3, perCallValues.Count()); + Assert.AreEqual("Builder", perCallValues.ElementAt(0)); + Assert.AreEqual("ClientOptions", perCallValues.ElementAt(1)); + Assert.AreEqual("RequestContext", perCallValues.ElementAt(2)); + + Assert.IsTrue(request.Headers.TryGetValues("PerRetry", out var perRetryValues)); + Assert.AreEqual(3, perRetryValues.Count()); + Assert.AreEqual("Builder", perRetryValues.ElementAt(0)); + Assert.AreEqual("ClientOptions", perRetryValues.ElementAt(1)); + Assert.AreEqual("RequestContext", perRetryValues.ElementAt(2)); + + Assert.IsTrue(request.Headers.TryGetValues("BeforeTransport", out var beforeTransportValues)); + Assert.AreEqual(2, beforeTransportValues.Count()); + Assert.AreEqual("ClientOptions", beforeTransportValues.ElementAt(0)); + Assert.AreEqual("RequestContext", beforeTransportValues.ElementAt(1)); + } + + [Test] + public async Task ThrowsIfUsePipelineConstructor() + { + HttpPipeline pipeline = new HttpPipeline(new MockTransport()); + + var context = new RequestContext(); + context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); + + var message = pipeline.CreateMessage(context); + + bool throws = false; + try + { + await pipeline.SendAsync(message, context.CancellationToken); + } + catch (InvalidOperationException) + { + throws = true; + } + + Assert.IsTrue(throws); + } + + #region Helpers + public class AddHeaderPolicy : HttpPipelineSynchronousPolicy + { + private string _headerName; + private string _headerVaue; + + public AddHeaderPolicy(string headerName, string headerValue) : base() + { + _headerName = headerName; + _headerVaue = headerValue; + } + + public override void OnSendingRequest(HttpMessage message) + { + message.Request.Headers.Add(_headerName, _headerVaue); + } + } + #endregion + } } diff --git a/sdk/core/Azure.Core/tests/RequestContextTests.cs b/sdk/core/Azure.Core/tests/RequestContextTests.cs index 38b1242d6433..0a0f0e125b41 100644 --- a/sdk/core/Azure.Core/tests/RequestContextTests.cs +++ b/sdk/core/Azure.Core/tests/RequestContextTests.cs @@ -9,7 +9,6 @@ using System.Threading.Tasks; using Azure.Core.Pipeline; using Azure.Core.TestFramework; -using Azure.Template.LLC; using NUnit.Framework; namespace Azure.Core.Tests @@ -18,17 +17,6 @@ public class RequestContextTests { private readonly Uri _url = new Uri("https://example.azuretemplateservice.com"); - public TemplateServiceClient CreateClient(HttpPipelineTransport transport) - { - var options = new TemplateServiceClientOptions() - { - Transport = transport - }; - - // TODO: Fix ordering - return new TemplateServiceClient(new MockCredential(), _url, options); - } - [Test] public void CanCastFromErrorOptions() { @@ -44,244 +32,5 @@ public void CanSetErrorOptions() Assert.IsTrue(context.ErrorOptions == ErrorOptions.NoThrow); } - - [Test] - public async Task CanAddPolicy_PerCall() - { - var resourceResponse = new MockResponse(200); - - var resource = - @"{ - name = ""snoopy"", - id = ""beagle"" - }"; - - resourceResponse.SetContent(resource); - - var mockTransport = new MockTransport(resourceResponse); - TemplateServiceClient client = CreateClient(mockTransport); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); - - Response response = await client.GetAsync("snoopy", context); - - Request request = mockTransport.Requests[0]; - Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader", out var values)); - Assert.AreEqual(1, values.Count()); - Assert.AreEqual("Value", values.ElementAt(0)); - } - - [Test] - public async Task CanAddPolicy_PerRetry() - { - var retryResponse = new MockResponse(408); // Request Timeout - var resourceResponse = new MockResponse(200); - - var resource = - @"{ - name = ""snoopy"", - id = ""beagle"" - }"; - - resourceResponse.SetContent(resource); - - // retry twice - var mockTransport = new MockTransport(retryResponse, retryResponse, resourceResponse); - TemplateServiceClient client = CreateClient(mockTransport); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "Value"), HttpPipelinePosition.PerRetry); - - Response response = await client.GetAsync("snoopy", context); - - Request request = mockTransport.Requests[0]; - Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var values)); - Assert.AreEqual(3, values.Count()); - Assert.AreEqual("Value", values.ElementAt(0)); - Assert.AreEqual("Value", values.ElementAt(1)); - Assert.AreEqual("Value", values.ElementAt(2)); - } - - [Test] - public async Task CanAddPolicy_BeforeTransport() - { - var retryResponse = new MockResponse(408); // Request Timeout - var resourceResponse = new MockResponse(200); - - var resource = - @"{ - name = ""snoopy"", - id = ""beagle"" - }"; - - resourceResponse.SetContent(resource); - - // retry twice - var mockTransport = new MockTransport(retryResponse, retryResponse, resourceResponse); - TemplateServiceClient client = CreateClient(mockTransport); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "Value"), HttpPipelinePosition.BeforeTransport); - - Response response = await client.GetAsync("snoopy", context); - - Request request = mockTransport.Requests[0]; - Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var values)); - Assert.AreEqual(3, values.Count()); - Assert.AreEqual("Value", values.ElementAt(0)); - Assert.AreEqual("Value", values.ElementAt(1)); - Assert.AreEqual("Value", values.ElementAt(2)); - } - - [Test] - public async Task CanAddPolicies_AllPositions() - { - var retryResponse = new MockResponse(408); // Request Timeout - var resourceResponse = new MockResponse(200); - - var resource = - @"{ - name = ""snoopy"", - id = ""beagle"" - }"; - - resourceResponse.SetContent(resource); - - // retry twice -- this will add the header three times. - var mockTransport = new MockTransport(retryResponse, retryResponse, resourceResponse); - TemplateServiceClient client = CreateClient(mockTransport); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("PerCallHeader1", "PerCall1"), HttpPipelinePosition.PerCall); - context.AddPolicy(new AddHeaderPolicy("PerCallHeader2", "PerCall2"), HttpPipelinePosition.PerCall); - context.AddPolicy(new AddHeaderPolicy("PerRetryHeader", "PerRetry"), HttpPipelinePosition.PerRetry); - context.AddPolicy(new AddHeaderPolicy("BeforeTransportHeader", "BeforeTransport"), HttpPipelinePosition.BeforeTransport); - - // TODO: Should be Get - Response response = await client.GetAsync("snoopy", context); - - Request request = mockTransport.Requests[0]; - - Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader1", out var perCall1Values)); - Assert.AreEqual(1, perCall1Values.Count()); - Assert.AreEqual("PerCall1", perCall1Values.ElementAt(0)); - - Assert.IsTrue(request.Headers.TryGetValues("PerCallHeader2", out var perCall2Values)); - Assert.AreEqual(1, perCall2Values.Count()); - Assert.AreEqual("PerCall2", perCall2Values.ElementAt(0)); - - Assert.IsTrue(request.Headers.TryGetValues("PerRetryHeader", out var perRetryValues)); - Assert.AreEqual("PerRetry", perRetryValues.ElementAt(0)); - Assert.AreEqual("PerRetry", perRetryValues.ElementAt(1)); - Assert.AreEqual("PerRetry", perRetryValues.ElementAt(2)); - - Assert.IsTrue(request.Headers.TryGetValues("BeforeTransportHeader", out var beforeTransportValues)); - Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(0)); - Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(1)); - Assert.AreEqual("BeforeTransport", beforeTransportValues.ElementAt(2)); - } - - [Test] - public async Task CanAddPolicies_ThreeWays() - { - var retryResponse = new MockResponse(408); // Request Timeout - var resourceResponse = new MockResponse(200); - var resource = - @"{ - name = ""snoopy"", - id = ""beagle"" - }"; - resourceResponse.SetContent(resource); - - var mockTransport = new MockTransport(retryResponse, resourceResponse); - var options = new TemplateServiceClientOptions() - { - Transport = mockTransport, - }; - - var perCallPolicies = new HttpPipelinePolicy[] { new AddHeaderPolicy("PerCall", "Builder") }; - var perRetryPolicies = new HttpPipelinePolicy[] { new AddHeaderPolicy("PerRetry", "Builder") }; - - options.AddPolicy(new AddHeaderPolicy("BeforeTransport", "ClientOptions"), HttpPipelinePosition.BeforeTransport); - options.AddPolicy(new AddHeaderPolicy("PerRetry", "ClientOptions"), HttpPipelinePosition.PerRetry); - options.AddPolicy(new AddHeaderPolicy("PerCall", "ClientOptions"), HttpPipelinePosition.PerCall); - - var pipeline = HttpPipelineBuilder.Build(options, perCallPolicies, perRetryPolicies, null); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("PerRetry", "RequestContext"), HttpPipelinePosition.PerRetry); - context.AddPolicy(new AddHeaderPolicy("PerCall", "RequestContext"), HttpPipelinePosition.PerCall); - context.AddPolicy(new AddHeaderPolicy("BeforeTransport", "RequestContext"), HttpPipelinePosition.BeforeTransport); - - var message = pipeline.CreateMessage(context); - await pipeline.SendAsync(message, message.CancellationToken); - - Request request = mockTransport.Requests[0]; - - Assert.IsTrue(request.Headers.TryGetValues("PerCall", out var perCallValues)); - Assert.AreEqual(3, perCallValues.Count()); - Assert.AreEqual("Builder", perCallValues.ElementAt(0)); - Assert.AreEqual("ClientOptions", perCallValues.ElementAt(1)); - Assert.AreEqual("RequestContext", perCallValues.ElementAt(2)); - - Assert.IsTrue(request.Headers.TryGetValues("PerRetry", out var perRetryValues)); - Assert.AreEqual(6, perRetryValues.Count()); - Assert.AreEqual("Builder", perRetryValues.ElementAt(0)); - Assert.AreEqual("ClientOptions", perRetryValues.ElementAt(1)); - Assert.AreEqual("RequestContext", perRetryValues.ElementAt(2)); - Assert.AreEqual("Builder", perRetryValues.ElementAt(3)); - Assert.AreEqual("ClientOptions", perRetryValues.ElementAt(4)); - Assert.AreEqual("RequestContext", perRetryValues.ElementAt(5)); - - Assert.IsTrue(request.Headers.TryGetValues("BeforeTransport", out var beforeTransportValues)); - Assert.AreEqual(4, beforeTransportValues.Count()); - Assert.AreEqual("ClientOptions", beforeTransportValues.ElementAt(0)); - Assert.AreEqual("RequestContext", beforeTransportValues.ElementAt(1)); - Assert.AreEqual("ClientOptions", beforeTransportValues.ElementAt(2)); - Assert.AreEqual("RequestContext", beforeTransportValues.ElementAt(3)); - } - - [Test] - public async Task ThrowsIfUsePipelineConstructor() - { - HttpPipeline pipeline = new HttpPipeline(new MockTransport()); - - var context = new RequestContext(); - context.AddPolicy(new AddHeaderPolicy("PerCallHeader", "Value"), HttpPipelinePosition.PerCall); - - var message = pipeline.CreateMessage(context); - - bool throws = false; - try - { - await pipeline.SendAsync(message, context.CancellationToken); - } - catch (InvalidOperationException) - { - throws = true; - } - - Assert.IsTrue(throws); - } - - #region Helpers - public class AddHeaderPolicy : HttpPipelineSynchronousPolicy - { - private string _headerName; - private string _headerVaue; - - public AddHeaderPolicy(string headerName, string headerValue) : base() - { - _headerName = headerName; - _headerVaue = headerValue; - } - - public override void OnSendingRequest(HttpMessage message) - { - message.Request.Headers.Add(_headerName, _headerVaue); - } - } - #endregion } } diff --git a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClient.cs b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClient.cs deleted file mode 100644 index 0069f7dcbef1..000000000000 --- a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClient.cs +++ /dev/null @@ -1,406 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -// - -#nullable disable - -using System; -using System.Collections.Generic; -using System.Runtime.CompilerServices; -using System.Threading; -using System.Threading.Tasks; -using Azure; -using Azure.Core; -using Azure.Core.Pipeline; - -namespace Azure.Template.LLC -{ - /// The TemplateService service client. - public partial class TemplateServiceClient - { - private static readonly string[] AuthorizationScopes = { "https://dev.LLCtemplate.net/.default" }; - private readonly TokenCredential _tokenCredential; - - private readonly HttpPipeline _pipeline; - private readonly ClientDiagnostics _clientDiagnostics; - private readonly Uri _endpoint; - - /// The HTTP pipeline for sending and receiving REST requests and responses. - public virtual HttpPipeline Pipeline { get => _pipeline; } - - /// Initializes a new instance of TemplateServiceClient for mocking. - protected TemplateServiceClient() - { - } - - /// Initializes a new instance of TemplateServiceClient. - /// A credential used to authenticate to an Azure Service. - /// server parameter. - /// The options for configuring the client. - /// is null. - public TemplateServiceClient(TokenCredential credential, Uri endpoint = null, TemplateServiceClientOptions options = null) - { - if (credential == null) - { - throw new ArgumentNullException(nameof(credential)); - } - endpoint ??= new Uri("http://localhost:3000"); - - options ??= new TemplateServiceClientOptions(); - - _clientDiagnostics = new ClientDiagnostics(options); - _tokenCredential = credential; - _pipeline = HttpPipelineBuilder.Build(options, new HttpPipelinePolicy[] { new LowLevelCallbackPolicy() }, new HttpPipelinePolicy[] { new BearerTokenAuthenticationPolicy(_tokenCredential, AuthorizationScopes) }, new ResponseClassifier()); - _endpoint = endpoint; - } - - /// Create or update resource. - /// The content to send as the body of the request. - /// The request context. - /// - /// Schema for Request Body: - /// { - /// name: string, - /// id: string - /// } - /// - /// Schema for Response Body: - /// { - /// name: string, - /// id: string - /// } - /// - /// - /// -#pragma warning disable AZC0002 - public virtual async Task CreateAsync(RequestContent content, RequestContext context = null) -#pragma warning restore AZC0002 - { - using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Create"); - scope.Start(); - try - { - using HttpMessage message = CreateCreateRequest(content, context); - return await _pipeline.ProcessMessageAsync(message, _clientDiagnostics, context).ConfigureAwait(false); - } - catch (Exception e) - { - scope.Failed(e); - throw; - } - } - - /// Create or update resource. - /// The content to send as the body of the request. - /// The request context. - /// - /// Schema for Request Body: - /// { - /// name: string, - /// id: string - /// } - /// - /// Schema for Response Body: - /// { - /// name: string, - /// id: string - /// } - /// - /// - /// -#pragma warning disable AZC0002 - public virtual Response Create(RequestContent content, RequestContext context = null) -#pragma warning restore AZC0002 - { - using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Create"); - scope.Start(); - try - { - using HttpMessage message = CreateCreateRequest(content, context); - return _pipeline.ProcessMessage(message, _clientDiagnostics, context); - } - catch (Exception e) - { - scope.Failed(e); - throw; - } - } - - /// Delete resource. - /// The id of the resource. - /// The request context. - /// is null. -#pragma warning disable AZC0002 - public virtual async Task DeleteAsync(string resourceId, RequestContext context = null) -#pragma warning restore AZC0002 - { - using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Delete"); - scope.Start(); - try - { - using HttpMessage message = CreateDeleteRequest(resourceId, context); - return await _pipeline.ProcessMessageAsync(message, _clientDiagnostics, context).ConfigureAwait(false); - } - catch (Exception e) - { - scope.Failed(e); - throw; - } - } - - /// Delete resource. - /// The id of the resource. - /// The request context. - /// is null. -#pragma warning disable AZC0002 - public virtual Response Delete(string resourceId, RequestContext context = null) -#pragma warning restore AZC0002 - { - using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Delete"); - scope.Start(); - try - { - using HttpMessage message = CreateDeleteRequest(resourceId, context); - return _pipeline.ProcessMessage(message, _clientDiagnostics, context); - } - catch (Exception e) - { - scope.Failed(e); - throw; - } - } - - /// Retrieves information about the resource. - /// The id of the resource. - /// The request context. - /// is null. - /// - /// Schema for Response Body: - /// { - /// name: string, - /// id: string - /// } - /// - /// - /// -#pragma warning disable AZC0002 - public virtual async Task GetAsync(string resourceId, RequestContext context = null) -#pragma warning restore AZC0002 - { - using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Get"); - scope.Start(); - try - { - using HttpMessage message = CreateGetRequest(resourceId, context); - return await _pipeline.ProcessMessageAsync(message, _clientDiagnostics, context).ConfigureAwait(false); - } - catch (Exception e) - { - scope.Failed(e); - throw; - } - } - - /// Retrieves information about the resource. - /// The id of the resource. - /// The request context. - /// is null. - /// - /// Schema for Response Body: - /// { - /// name: string, - /// id: string - /// } - /// - /// - /// -#pragma warning disable AZC0002 - public virtual Response Get(string resourceId, RequestContext context = null) -#pragma warning restore AZC0002 - { - using var scope = _clientDiagnostics.CreateScope("TemplateServiceClient.Get"); - scope.Start(); - try - { - using HttpMessage message = CreateGetRequest(resourceId, context); - return _pipeline.ProcessMessage(message, _clientDiagnostics, context); - } - catch (Exception e) - { - scope.Failed(e); - throw; - } - } - - /// Retrieves the list of resources. - /// The request context. - /// - /// Schema for Response Body: - /// { - /// value: [ - /// { - /// name: string, - /// id: string - /// } - /// ], - /// nextLink: string - /// } - /// - /// - /// -#pragma warning disable AZC0002 - public virtual AsyncPageable GetResourcesAsync(RequestContext context = null) -#pragma warning restore AZC0002 - { - return PageableHelpers.CreateAsyncPageable(CreateEnumerableAsync, _clientDiagnostics, "TemplateServiceClient.GetResources"); - async IAsyncEnumerable> CreateEnumerableAsync(string nextLink, int? pageSizeHint, [EnumeratorCancellation] CancellationToken cancellationToken = default) - { - do - { - var message = string.IsNullOrEmpty(nextLink) - ? CreateGetResourcesRequest(context) - : CreateGetResourcesNextPageRequest(nextLink, context); - var page = await LowLevelPageableHelpers.ProcessMessageAsync(_pipeline, message, _clientDiagnostics, context, "value", "nextLink", cancellationToken).ConfigureAwait(false); - nextLink = page.ContinuationToken; - yield return page; - } while (!string.IsNullOrEmpty(nextLink)); - } - } - - /// Retrieves the list of resources. - /// The request context. - /// - /// Schema for Response Body: - /// { - /// value: [ - /// { - /// name: string, - /// id: string - /// } - /// ], - /// nextLink: string - /// } - /// - /// - /// -#pragma warning disable AZC0002 - public virtual Pageable GetResources(RequestContext context = null) -#pragma warning restore AZC0002 - { - return PageableHelpers.CreatePageable(CreateEnumerable, _clientDiagnostics, "TemplateServiceClient.GetResources"); - IEnumerable> CreateEnumerable(string nextLink, int? pageSizeHint) - { - do - { - var message = string.IsNullOrEmpty(nextLink) - ? CreateGetResourcesRequest(context) - : CreateGetResourcesNextPageRequest(nextLink, context); - var page = LowLevelPageableHelpers.ProcessMessage(_pipeline, message, _clientDiagnostics, context, "value", "nextLink"); - nextLink = page.ContinuationToken; - yield return page; - } while (!string.IsNullOrEmpty(nextLink)); - } - } - - internal HttpMessage CreateCreateRequest(RequestContent content, RequestContext context) - { - var message = _pipeline.CreateMessage(context); - var request = message.Request; - request.Method = RequestMethod.Put; - var uri = new RawRequestUriBuilder(); - uri.Reset(_endpoint); - uri.AppendPath("/template/resources", false); - request.Uri = uri; - request.Headers.Add("Accept", "application/json"); - request.Headers.Add("Content-Type", "application/json"); - request.Content = content; - message.ResponseClassifier = ResponseClassifier200.Instance; - return message; - } - - internal HttpMessage CreateGetResourcesRequest(RequestContext context) - { - var message = _pipeline.CreateMessage(context); - var request = message.Request; - request.Method = RequestMethod.Get; - var uri = new RawRequestUriBuilder(); - uri.Reset(_endpoint); - uri.AppendPath("/template/resources", false); - request.Uri = uri; - request.Headers.Add("Accept", "application/json"); - message.ResponseClassifier = ResponseClassifier200.Instance; - return message; - } - - internal HttpMessage CreateDeleteRequest(string resourceId, RequestContext context) - { - var message = _pipeline.CreateMessage(context); - var request = message.Request; - request.Method = RequestMethod.Delete; - var uri = new RawRequestUriBuilder(); - uri.Reset(_endpoint); - uri.AppendPath("/template/resources/", false); - uri.AppendPath(resourceId, true); - request.Uri = uri; - message.ResponseClassifier = ResponseClassifier204.Instance; - return message; - } - - internal HttpMessage CreateGetRequest(string resourceId, RequestContext context) - { - var message = _pipeline.CreateMessage(context); - var request = message.Request; - request.Method = RequestMethod.Get; - var uri = new RawRequestUriBuilder(); - uri.Reset(_endpoint); - uri.AppendPath("/template/resources/", false); - uri.AppendPath(resourceId, true); - request.Uri = uri; - request.Headers.Add("Accept", "application/json"); - message.ResponseClassifier = ResponseClassifier200.Instance; - return message; - } - - internal HttpMessage CreateGetResourcesNextPageRequest(string nextLink, RequestContext context) - { - var message = _pipeline.CreateMessage(); - var request = message.Request; - request.Method = RequestMethod.Get; - var uri = new RawRequestUriBuilder(); - uri.Reset(_endpoint); - uri.AppendRawNextLink(nextLink, false); - request.Uri = uri; - request.Headers.Add("Accept", "application/json"); - message.ResponseClassifier = ResponseClassifier200.Instance; - return message; - } - - private sealed class ResponseClassifier200 : ResponseClassifier - { - private static ResponseClassifier _instance; - public static ResponseClassifier Instance => _instance ??= new ResponseClassifier200(); - public override bool IsErrorResponse(HttpMessage message) - { - return message.Response.Status switch - { - 200 => false, - _ => true - }; - } - } - private sealed class ResponseClassifier204 : ResponseClassifier - { - private static ResponseClassifier _instance; - public static ResponseClassifier Instance => _instance ??= new ResponseClassifier204(); - public override bool IsErrorResponse(HttpMessage message) - { - return message.Response.Status switch - { - 204 => false, - _ => true - }; - } - } - } -} diff --git a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClientOptions.cs b/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClientOptions.cs deleted file mode 100644 index 8cc3f1a29cef..000000000000 --- a/sdk/core/Azure.Core/tests/TestClients/RequestContextTestClient/Generated/TemplateServiceClientOptions.cs +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -// - -#nullable disable - -using System; -using Azure.Core; - -namespace Azure.Template.LLC -{ - /// Client options for TemplateServiceClient. - public partial class TemplateServiceClientOptions : ClientOptions - { - private const ServiceVersion LatestVersion = ServiceVersion.V1_0_0; - - /// The version of the service to use. - public enum ServiceVersion - { - /// Service version "1.0.0". - V1_0_0 = 1, - } - - internal string Version { get; } - - /// Initializes new instance of TemplateServiceClientOptions. - public TemplateServiceClientOptions(ServiceVersion version = LatestVersion) - { - Version = version switch - { - ServiceVersion.V1_0_0 => "1.0.0", - _ => throw new NotSupportedException() - }; - } - } -} From 38b68801f7330dd5db62b2430ef534a4b08523b0 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 17 Nov 2021 11:24:32 -0800 Subject: [PATCH 24/26] pr fb --- .../tests/LowLevelClient/PetStoreClient.cs | 2 +- sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs | 2 +- sdk/core/Azure.Core/tests/RequestContextTests.cs | 5 ----- .../Azure.MixedReality.ObjectAnchors.Conversion.sln | 10 ++++++++-- .../Azure.MixedReality.ObjectAnchors.Conversion.csproj | 4 +++- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs b/sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs index aa8c3d14f49c..3859098d8b2f 100644 --- a/sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs +++ b/sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs @@ -146,7 +146,7 @@ public virtual Response GetPet(string id, RequestContext context = null) /// The request options. private HttpMessage CreateGetPetRequest(string id, RequestContext context = null) { - var message = Pipeline.CreateMessage(context); + var message = Pipeline.CreateMessage(); var request = message.Request; request.Method = RequestMethod.Get; var uri = new RawRequestUriBuilder(); diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index dc9fbd9d636f..7657bab34768 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -231,8 +231,8 @@ private ReadOnlyMemory CreateRequestPipeline(HttpPipelinePol } // Copy over client policies and splice in custom policies at designated indices - int transportIndex = _pipeline.Length - 1; var pipeline = _pipeline.Span; + int transportIndex = pipeline.Length - 1; pipeline.Slice(0, _perCallIndex).CopyTo(policies); diff --git a/sdk/core/Azure.Core/tests/RequestContextTests.cs b/sdk/core/Azure.Core/tests/RequestContextTests.cs index 0a0f0e125b41..f73dda61475c 100644 --- a/sdk/core/Azure.Core/tests/RequestContextTests.cs +++ b/sdk/core/Azure.Core/tests/RequestContextTests.cs @@ -5,18 +5,13 @@ using System.Collections.Generic; using System.Linq; using System.Text; -using System.Text.Json; using System.Threading.Tasks; -using Azure.Core.Pipeline; -using Azure.Core.TestFramework; using NUnit.Framework; namespace Azure.Core.Tests { public class RequestContextTests { - private readonly Uri _url = new Uri("https://example.azuretemplateservice.com"); - [Test] public void CanCastFromErrorOptions() { diff --git a/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/Azure.MixedReality.ObjectAnchors.Conversion.sln b/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/Azure.MixedReality.ObjectAnchors.Conversion.sln index f3852dc0317b..0d841ff0e254 100644 --- a/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/Azure.MixedReality.ObjectAnchors.Conversion.sln +++ b/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/Azure.MixedReality.ObjectAnchors.Conversion.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio Version 16 -VisualStudioVersion = 16.0.30907.101 +# Visual Studio Version 17 +VisualStudioVersion = 17.1.31903.286 MinimumVisualStudioVersion = 10.0.40219.1 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.MixedReality.ObjectAnchors.Conversion", "src\Azure.MixedReality.ObjectAnchors.Conversion.csproj", "{F2DECE5E-D006-41CC-A4F2-2ACE91D18E16}" EndProject @@ -9,6 +9,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core.TestFramework", EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.MixedReality.ObjectAnchors.Conversion.Tests", "tests\Azure.MixedReality.ObjectAnchors.Conversion.Tests.csproj", "{FDE9619C-6A39-4A72-8E78-9ACB3B8C64FB}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core", "..\..\core\Azure.Core\src\Azure.Core.csproj", "{120306F5-9206-499A-846F-1C425D6DD5C4}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -27,6 +29,10 @@ Global {FDE9619C-6A39-4A72-8E78-9ACB3B8C64FB}.Debug|Any CPU.Build.0 = Debug|Any CPU {FDE9619C-6A39-4A72-8E78-9ACB3B8C64FB}.Release|Any CPU.ActiveCfg = Release|Any CPU {FDE9619C-6A39-4A72-8E78-9ACB3B8C64FB}.Release|Any CPU.Build.0 = Release|Any CPU + {120306F5-9206-499A-846F-1C425D6DD5C4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {120306F5-9206-499A-846F-1C425D6DD5C4}.Debug|Any CPU.Build.0 = Debug|Any CPU + {120306F5-9206-499A-846F-1C425D6DD5C4}.Release|Any CPU.ActiveCfg = Release|Any CPU + {120306F5-9206-499A-846F-1C425D6DD5C4}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/src/Azure.MixedReality.ObjectAnchors.Conversion.csproj b/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/src/Azure.MixedReality.ObjectAnchors.Conversion.csproj index d0a5a4a856c7..032184c22b74 100644 --- a/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/src/Azure.MixedReality.ObjectAnchors.Conversion.csproj +++ b/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/src/Azure.MixedReality.ObjectAnchors.Conversion.csproj @@ -10,7 +10,6 @@ - @@ -30,5 +29,8 @@ + + + From eaf6f28a1efadff701e9d66e3dd8cc8b68d4be1b Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 17 Nov 2021 13:31:37 -0800 Subject: [PATCH 25/26] pr fb & fix core ci --- sdk/core/Azure.Core/src/HttpMessage.cs | 3 ++- sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs | 2 +- .../Azure.Core/src/Pipeline/HttpPipelineBuilder.cs | 7 ++++++- .../Azure.Core/tests/HttpPipelineBuilderTest.cs | 13 +++++++++++++ 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/sdk/core/Azure.Core/src/HttpMessage.cs b/sdk/core/Azure.Core/src/HttpMessage.cs index a24614b0bf21..c387c028089e 100644 --- a/sdk/core/Azure.Core/src/HttpMessage.cs +++ b/sdk/core/Azure.Core/src/HttpMessage.cs @@ -87,7 +87,8 @@ internal void AddPolicies(RequestContext context) return; } - Policies ??= new(context.Policies); + Policies ??= new(context.Policies.Count); + Policies.AddRange(context.Policies); } internal List<(HttpPipelinePosition Position, HttpPipelinePolicy Policy)>? Policies { get; set; } diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 7657bab34768..817454226122 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -154,7 +154,7 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) try { var pipeline = CreateRequestPipeline(policies, message.Policies); - pipeline.Span[0].ProcessAsync(message, pipeline.Slice(1)); + pipeline.Span[0].Process(message, pipeline.Slice(1)); } finally { diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index c24eaff71300..8cb7371ae000 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -76,7 +76,7 @@ void AddCustomerPolicies(HttpPipelinePosition position) { foreach (var policy in options.Policies) { - if (policy.Position == position && policy.Policy != null) + if (policy.Position == position) { policies.Add(policy.Policy); } @@ -95,6 +95,8 @@ void AddCustomerPolicies(HttpPipelinePosition position) policies.AddRange(perCallPolicies); AddCustomerPolicies(HttpPipelinePosition.PerCall); + + policies.RemoveAll(static policy => policy == null); perCallIndex = policies.Count; policies.Add(ClientRequestIdPolicy.Shared); @@ -112,6 +114,8 @@ void AddCustomerPolicies(HttpPipelinePosition position) policies.AddRange(perRetryPolicies); AddCustomerPolicies(HttpPipelinePosition.PerRetry); + + policies.RemoveAll(static policy => policy == null); perRetryIndex = policies.Count; if (diagnostics.IsLoggingEnabled) @@ -126,6 +130,7 @@ void AddCustomerPolicies(HttpPipelinePosition position) policies.Add(new RequestActivityPolicy(isDistributedTracingEnabled, ClientDiagnostics.GetResourceProviderNamespace(options.GetType().Assembly), sanitizer)); AddCustomerPolicies(HttpPipelinePosition.BeforeTransport); + policies.RemoveAll(static policy => policy == null); // Override the provided Transport with the provided transport options if the transport has not been set after default construction and options are not null. HttpPipelineTransport transport = options.Transport; diff --git a/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs b/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs index 272e7c141b71..9a61131b9aea 100644 --- a/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs +++ b/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs @@ -234,6 +234,19 @@ public void SetTransportOptions([Values(true, false)] bool isCustomTransportSet) } } + [Test] + public void CanPassNullPolicies([Values(true, false)] bool isCustomTransportSet) + { + var pipeline = HttpPipelineBuilder.Build( + new TestOptions(), + new HttpPipelinePolicy[] { null }, + new HttpPipelinePolicy[] { null }, + null); + + var message = pipeline.CreateMessage(); + pipeline.SendAsync(message, message.CancellationToken); + } + private class TestOptions : ClientOptions { public TestOptions() From fa7c8e01ba173a0923609cbb01700b18bb1d1976 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 17 Nov 2021 13:45:14 -0800 Subject: [PATCH 26/26] revert files used for testing --- .../Azure.MixedReality.ObjectAnchors.Conversion.sln | 10 ++-------- .../Azure.MixedReality.ObjectAnchors.Conversion.csproj | 4 +--- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/Azure.MixedReality.ObjectAnchors.Conversion.sln b/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/Azure.MixedReality.ObjectAnchors.Conversion.sln index 0d841ff0e254..f3852dc0317b 100644 --- a/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/Azure.MixedReality.ObjectAnchors.Conversion.sln +++ b/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/Azure.MixedReality.ObjectAnchors.Conversion.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio Version 17 -VisualStudioVersion = 17.1.31903.286 +# Visual Studio Version 16 +VisualStudioVersion = 16.0.30907.101 MinimumVisualStudioVersion = 10.0.40219.1 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.MixedReality.ObjectAnchors.Conversion", "src\Azure.MixedReality.ObjectAnchors.Conversion.csproj", "{F2DECE5E-D006-41CC-A4F2-2ACE91D18E16}" EndProject @@ -9,8 +9,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core.TestFramework", EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.MixedReality.ObjectAnchors.Conversion.Tests", "tests\Azure.MixedReality.ObjectAnchors.Conversion.Tests.csproj", "{FDE9619C-6A39-4A72-8E78-9ACB3B8C64FB}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core", "..\..\core\Azure.Core\src\Azure.Core.csproj", "{120306F5-9206-499A-846F-1C425D6DD5C4}" -EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -29,10 +27,6 @@ Global {FDE9619C-6A39-4A72-8E78-9ACB3B8C64FB}.Debug|Any CPU.Build.0 = Debug|Any CPU {FDE9619C-6A39-4A72-8E78-9ACB3B8C64FB}.Release|Any CPU.ActiveCfg = Release|Any CPU {FDE9619C-6A39-4A72-8E78-9ACB3B8C64FB}.Release|Any CPU.Build.0 = Release|Any CPU - {120306F5-9206-499A-846F-1C425D6DD5C4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {120306F5-9206-499A-846F-1C425D6DD5C4}.Debug|Any CPU.Build.0 = Debug|Any CPU - {120306F5-9206-499A-846F-1C425D6DD5C4}.Release|Any CPU.ActiveCfg = Release|Any CPU - {120306F5-9206-499A-846F-1C425D6DD5C4}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/src/Azure.MixedReality.ObjectAnchors.Conversion.csproj b/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/src/Azure.MixedReality.ObjectAnchors.Conversion.csproj index 032184c22b74..d0a5a4a856c7 100644 --- a/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/src/Azure.MixedReality.ObjectAnchors.Conversion.csproj +++ b/sdk/objectanchors/Azure.MixedReality.ObjectAnchors.Conversion/src/Azure.MixedReality.ObjectAnchors.Conversion.csproj @@ -10,6 +10,7 @@ + @@ -29,8 +30,5 @@ - - -