ClientModel: API Updates from architect feedback

sdk/core/System.ClientModel/api/System.ClientModel.net6.0.cs

@@ -79,23 +79,18 @@ public partial class ClientRetryPolicy : System.ClientModel.Primitives.PipelineP
     {
         public static readonly System.ClientModel.Primitives.ClientRetryPolicy Default;
         public ClientRetryPolicy(int maxRetries = 3) { }
-        public System.TimeSpan GetNextDelay(System.ClientModel.Primitives.PipelineMessage message, int tryCount) { throw null; }
-        protected virtual System.TimeSpan GetNextDelayCore(System.ClientModel.Primitives.PipelineMessage message, int tryCount) { throw null; }
+        protected virtual System.TimeSpan GetNextDelay(System.ClientModel.Primitives.PipelineMessage message, int tryCount) { throw null; }
         protected virtual void OnRequestSent(System.ClientModel.Primitives.PipelineMessage message) { }
         protected virtual System.Threading.Tasks.ValueTask OnRequestSentAsync(System.ClientModel.Primitives.PipelineMessage message) { throw null; }
         protected virtual void OnSendingRequest(System.ClientModel.Primitives.PipelineMessage message) { }
         protected virtual System.Threading.Tasks.ValueTask OnSendingRequestAsync(System.ClientModel.Primitives.PipelineMessage message) { throw null; }
         protected virtual void OnTryComplete(System.ClientModel.Primitives.PipelineMessage message) { }
         public sealed override void Process(System.ClientModel.Primitives.PipelineMessage message, System.Collections.Generic.IReadOnlyList<System.ClientModel.Primitives.PipelinePolicy> pipeline, int currentIndex) { }
         public sealed override System.Threading.Tasks.ValueTask ProcessAsync(System.ClientModel.Primitives.PipelineMessage message, System.Collections.Generic.IReadOnlyList<System.ClientModel.Primitives.PipelinePolicy> pipeline, int currentIndex) { throw null; }
-        public bool ShouldRetry(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
-        public System.Threading.Tasks.ValueTask<bool> ShouldRetryAsync(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
-        protected virtual bool ShouldRetryCore(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
-        protected virtual System.Threading.Tasks.ValueTask<bool> ShouldRetryCoreAsync(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
-        public void Wait(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
-        public System.Threading.Tasks.Task WaitAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
-        protected virtual void WaitCore(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
-        protected virtual System.Threading.Tasks.Task WaitCoreAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
+        protected virtual bool ShouldRetry(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
+        protected virtual System.Threading.Tasks.ValueTask<bool> ShouldRetryAsync(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
+        protected virtual void Wait(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
+        protected virtual System.Threading.Tasks.Task WaitAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
     }
     public partial class HttpClientPipelineTransport : System.ClientModel.Primitives.PipelineTransport, System.IDisposable
     {
@@ -147,10 +142,10 @@ public partial class PipelineMessage : System.IDisposable
         protected internal PipelineMessage(System.ClientModel.Primitives.PipelineRequest request) { }
         public bool BufferResponse { get { throw null; } set { } }
         public System.Threading.CancellationToken CancellationToken { get { throw null; } protected internal set { } }
-        public System.ClientModel.Primitives.PipelineMessageClassifier MessageClassifier { get { throw null; } set { } }
         public System.TimeSpan? NetworkTimeout { get { throw null; } set { } }
         public System.ClientModel.Primitives.PipelineRequest Request { get { throw null; } }
         public System.ClientModel.Primitives.PipelineResponse? Response { get { throw null; } protected internal set { } }
+        public System.ClientModel.Primitives.PipelineMessageClassifier ResponseClassifier { get { throw null; } set { } }
         public void Apply(System.ClientModel.Primitives.RequestOptions options) { }
         public void Dispose() { }
         protected virtual void Dispose(bool disposing) { }

sdk/core/System.ClientModel/api/System.ClientModel.netstandard2.0.cs

@@ -79,23 +79,18 @@ public partial class ClientRetryPolicy : System.ClientModel.Primitives.PipelineP
     {
         public static readonly System.ClientModel.Primitives.ClientRetryPolicy Default;
         public ClientRetryPolicy(int maxRetries = 3) { }
-        public System.TimeSpan GetNextDelay(System.ClientModel.Primitives.PipelineMessage message, int tryCount) { throw null; }
-        protected virtual System.TimeSpan GetNextDelayCore(System.ClientModel.Primitives.PipelineMessage message, int tryCount) { throw null; }
+        protected virtual System.TimeSpan GetNextDelay(System.ClientModel.Primitives.PipelineMessage message, int tryCount) { throw null; }
         protected virtual void OnRequestSent(System.ClientModel.Primitives.PipelineMessage message) { }
         protected virtual System.Threading.Tasks.ValueTask OnRequestSentAsync(System.ClientModel.Primitives.PipelineMessage message) { throw null; }
         protected virtual void OnSendingRequest(System.ClientModel.Primitives.PipelineMessage message) { }
         protected virtual System.Threading.Tasks.ValueTask OnSendingRequestAsync(System.ClientModel.Primitives.PipelineMessage message) { throw null; }
         protected virtual void OnTryComplete(System.ClientModel.Primitives.PipelineMessage message) { }
         public sealed override void Process(System.ClientModel.Primitives.PipelineMessage message, System.Collections.Generic.IReadOnlyList<System.ClientModel.Primitives.PipelinePolicy> pipeline, int currentIndex) { }
         public sealed override System.Threading.Tasks.ValueTask ProcessAsync(System.ClientModel.Primitives.PipelineMessage message, System.Collections.Generic.IReadOnlyList<System.ClientModel.Primitives.PipelinePolicy> pipeline, int currentIndex) { throw null; }
-        public bool ShouldRetry(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
-        public System.Threading.Tasks.ValueTask<bool> ShouldRetryAsync(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
-        protected virtual bool ShouldRetryCore(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
-        protected virtual System.Threading.Tasks.ValueTask<bool> ShouldRetryCoreAsync(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
-        public void Wait(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
-        public System.Threading.Tasks.Task WaitAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
-        protected virtual void WaitCore(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
-        protected virtual System.Threading.Tasks.Task WaitCoreAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
+        protected virtual bool ShouldRetry(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
+        protected virtual System.Threading.Tasks.ValueTask<bool> ShouldRetryAsync(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
+        protected virtual void Wait(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
+        protected virtual System.Threading.Tasks.Task WaitAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
     }
     public partial class HttpClientPipelineTransport : System.ClientModel.Primitives.PipelineTransport, System.IDisposable
     {
@@ -146,10 +141,10 @@ public partial class PipelineMessage : System.IDisposable
         protected internal PipelineMessage(System.ClientModel.Primitives.PipelineRequest request) { }
         public bool BufferResponse { get { throw null; } set { } }
         public System.Threading.CancellationToken CancellationToken { get { throw null; } protected internal set { } }
-        public System.ClientModel.Primitives.PipelineMessageClassifier MessageClassifier { get { throw null; } set { } }
         public System.TimeSpan? NetworkTimeout { get { throw null; } set { } }
         public System.ClientModel.Primitives.PipelineRequest Request { get { throw null; } }
         public System.ClientModel.Primitives.PipelineResponse? Response { get { throw null; } protected internal set { } }
+        public System.ClientModel.Primitives.PipelineMessageClassifier ResponseClassifier { get { throw null; } set { } }
         public void Apply(System.ClientModel.Primitives.RequestOptions options) { }
         public void Dispose() { }
         protected virtual void Dispose(bool disposing) { }

sdk/core/System.ClientModel/src/Message/PipelineMessage.cs

@@ -19,7 +19,7 @@ protected internal PipelineMessage(PipelineRequest request)
         _propertyBag = new ArrayBackedPropertyBag<ulong, object>();
 
         BufferResponse = true;
-        MessageClassifier = PipelineMessageClassifier.Default;
+        ResponseClassifier = PipelineMessageClassifier.Default;
     }
 
     public PipelineRequest Request { get; }
@@ -65,7 +65,7 @@ public CancellationToken CancellationToken
     // the client-provided classifier or compose a chain of classification
     // handlers that preserve the functionality of the client-provided classifier
     // at the end of the chain.
-    public PipelineMessageClassifier MessageClassifier { get; set; }
+    public PipelineMessageClassifier ResponseClassifier { get; set; }
 
     public void Apply(RequestOptions options)
     {

sdk/core/System.ClientModel/src/Pipeline/ClientRetryPolicy.cs

@@ -78,8 +78,8 @@ private async ValueTask ProcessSyncOrAsync(PipelineMessage message, IReadOnlyLis
             }
 
             bool shouldRetry = async ?
-                await ShouldRetryAsync(message, thisTryException).ConfigureAwait(false) :
-                ShouldRetry(message, thisTryException);
+                await ShouldRetryInternalAsync(message, thisTryException).ConfigureAwait(false) :
+                ShouldRetryInternal(message, thisTryException);
 
             if (shouldRetry)
             {
@@ -133,13 +133,13 @@ protected virtual void OnRequestSent(PipelineMessage message) { }
 
     protected virtual void OnTryComplete(PipelineMessage message) { }
 
-    public bool ShouldRetry(PipelineMessage message, Exception? exception)
-        => ShouldRetrySyncOrAsync(message, exception, async: false).EnsureCompleted();
+    internal bool ShouldRetryInternal(PipelineMessage message, Exception? exception)
+        => ShouldRetryInternalSyncOrAsync(message, exception, async: false).EnsureCompleted();
 
-    public async ValueTask<bool> ShouldRetryAsync(PipelineMessage message, Exception? exception)
-        => await ShouldRetrySyncOrAsync(message, exception, async: true).ConfigureAwait(false);
+    internal async ValueTask<bool> ShouldRetryInternalAsync(PipelineMessage message, Exception? exception)
+        => await ShouldRetryInternalSyncOrAsync(message, exception, async: true).ConfigureAwait(false);
 
-    private async ValueTask<bool> ShouldRetrySyncOrAsync(PipelineMessage message, Exception? exception, bool async)
+    private async ValueTask<bool> ShouldRetryInternalSyncOrAsync(PipelineMessage message, Exception? exception, bool async)
     {
         // If there was no exception and we got a success response, don't retry.
         if (exception is null && message.Response is not null && !message.Response.IsError)
@@ -149,23 +149,23 @@ private async ValueTask<bool> ShouldRetrySyncOrAsync(PipelineMessage message, Ex
 
         if (async)
         {
-            return await ShouldRetryCoreAsync(message, exception).ConfigureAwait(false);
+            return await ShouldRetryAsync(message, exception).ConfigureAwait(false);
         }
         else
         {
-            return ShouldRetryCore(message, exception);
+            return ShouldRetry(message, exception);
         }
     }
 
-    protected virtual bool ShouldRetryCore(PipelineMessage message, Exception? exception)
+    protected virtual bool ShouldRetry(PipelineMessage message, Exception? exception)
     {
         if (message.RetryCount >= _maxRetries)
         {
             // We've exceeded the maximum number of retries, so don't retry.
             return false;
         }
 
-        if (!message.MessageClassifier.TryClassify(message, exception, out bool isRetriable))
+        if (!message.ResponseClassifier.TryClassify(message, exception, out bool isRetriable))
         {
             bool classified = PipelineMessageClassifier.Default.TryClassify(message, exception, out isRetriable);
 
@@ -175,30 +175,21 @@ protected virtual bool ShouldRetryCore(PipelineMessage message, Exception? excep
         return isRetriable;
     }
 
-    protected virtual ValueTask<bool> ShouldRetryCoreAsync(PipelineMessage message, Exception? exception)
-        => new(ShouldRetryCore(message, exception));
+    protected virtual ValueTask<bool> ShouldRetryAsync(PipelineMessage message, Exception? exception)
+        => new(ShouldRetry(message, exception));
 
-    public TimeSpan GetNextDelay(PipelineMessage message, int tryCount)
-        => GetNextDelayCore(message, tryCount);
-
-    protected virtual TimeSpan GetNextDelayCore(PipelineMessage message, int tryCount)
+    protected virtual TimeSpan GetNextDelay(PipelineMessage message, int tryCount)
     {
         // Default implementation is exponential backoff
         return TimeSpan.FromMilliseconds((1 << (tryCount - 1)) * _initialDelay.TotalMilliseconds);
     }
 
-    public async Task WaitAsync(TimeSpan time, CancellationToken cancellationToken)
-        => await WaitCoreAsync(time, cancellationToken).ConfigureAwait(false);
-
-    protected virtual async Task WaitCoreAsync(TimeSpan time, CancellationToken cancellationToken)
+    protected virtual async Task WaitAsync(TimeSpan time, CancellationToken cancellationToken)
     {
         await Task.Delay(time, cancellationToken).ConfigureAwait(false);
     }
 
-    public void Wait(TimeSpan time, CancellationToken cancellationToken)
-        => WaitCore(time, cancellationToken);
-
-    protected virtual void WaitCore(TimeSpan time, CancellationToken cancellationToken)
+    protected virtual void Wait(TimeSpan time, CancellationToken cancellationToken)
     {
         if (cancellationToken.WaitHandle.WaitOne(time))
         {

sdk/core/System.ClientModel/src/Pipeline/PipelineTransport.cs

@@ -43,7 +43,7 @@ public async ValueTask ProcessAsync(PipelineMessage message)
 
     private static bool ClassifyResponse(PipelineMessage message)
     {
-        if (!message.MessageClassifier.TryClassify(message, out bool isError))
+        if (!message.ResponseClassifier.TryClassify(message, out bool isError))
         {
             bool classified = PipelineMessageClassifier.Default.TryClassify(message, out isError);
 

sdk/core/System.ClientModel/tests/Pipeline/ClientRetryPolicyTests.cs

@@ -345,7 +345,7 @@ int responseFactory(int i)
     [Test]
     public void WaitThrowsOnCancellation()
     {
-        ClientRetryPolicy retryPolicy = new();
+        MockRetryPolicy retryPolicy = new();
 
         CancellationTokenSource cts = new CancellationTokenSource();
 
@@ -356,4 +356,4 @@ public void WaitThrowsOnCancellation()
         Assert.ThrowsAsync<TaskCanceledException>(async () =>
             await retryPolicy.WaitSyncOrAsync(delay, cts.Token, IsAsync));
     }
-}
+}

sdk/core/System.ClientModel/tests/TestFramework/Mocks/MockRetryPolicy.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.ClientModel.Primitives;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace ClientModel.Tests.Mocks;
@@ -24,11 +25,11 @@ public MockRetryPolicy(int maxRetries, Func<int, TimeSpan>? delayFactory) : base
         _delayFactory = delayFactory;
     }
 
-    public Exception? LastException {  get; private set; }
+    public Exception? LastException { get; private set; }
 
     public bool ShouldRetryCalled { get; private set; }
 
-    public bool OnRequestSentCalled {  get; private set; }
+    public bool OnRequestSentCalled { get; private set; }
 
     public bool OnSendingRequestCalled { get; private set; }
 
@@ -41,20 +42,20 @@ public void Reset()
         OnRequestSentCalled = false;
     }
 
-    protected override bool ShouldRetryCore(PipelineMessage message, Exception? exception)
+    protected override bool ShouldRetry(PipelineMessage message, Exception? exception)
     {
         ShouldRetryCalled = true;
         LastException = exception;
 
-        return base.ShouldRetryCore(message, exception);
+        return base.ShouldRetry(message, exception);
     }
 
-    protected override ValueTask<bool> ShouldRetryCoreAsync(PipelineMessage message, Exception? exception)
+    protected override ValueTask<bool> ShouldRetryAsync(PipelineMessage message, Exception? exception)
     {
         ShouldRetryCalled = true;
         LastException = exception;
 
-        return base.ShouldRetryCoreAsync(message, exception);
+        return base.ShouldRetryAsync(message, exception);
     }
 
     protected override void OnRequestSent(PipelineMessage message)
@@ -85,13 +86,19 @@ protected override ValueTask OnSendingRequestAsync(PipelineMessage message)
         return base.OnSendingRequestAsync(message);
     }
 
-    protected override TimeSpan GetNextDelayCore(PipelineMessage message, int tryCount)
+    protected override TimeSpan GetNextDelay(PipelineMessage message, int tryCount)
     {
         if (_delayFactory is not null)
         {
             return _delayFactory(tryCount);
         }
 
-        return base.GetNextDelayCore(message, tryCount);
+        return base.GetNextDelay(message, tryCount);
     }
+
+    public void DoWait(TimeSpan time, CancellationToken cancellationToken)
+        => Wait(time, cancellationToken);
+
+    public async Task DoWaitAsync(TimeSpan time, CancellationToken cancellationToken)
+        => await WaitAsync(time, cancellationToken).ConfigureAwait(false);
 }

sdk/core/System.ClientModel/tests/TestFramework/Mocks/MockSyncAsyncExtensions.cs

@@ -73,15 +73,15 @@ public static async Task ProcessSyncOrAsync(this HttpClientPipelineTransport tra
         }
     }
 
-    public static async Task WaitSyncOrAsync(this ClientRetryPolicy policy, TimeSpan delay, CancellationToken cancellationToken, bool isAsync)
+    public static async Task WaitSyncOrAsync(this MockRetryPolicy policy, TimeSpan delay, CancellationToken cancellationToken, bool isAsync)
     {
         if (isAsync)
         {
-            await policy.WaitAsync(delay, cancellationToken).ConfigureAwait(false);
+            await policy.DoWaitAsync(delay, cancellationToken).ConfigureAwait(false);
         }
         else
         {
-            policy.Wait(delay, cancellationToken);
+            policy.DoWait(delay, cancellationToken);
         }
     }
 }