Make RequestOptions.BufferResponse non-nullable (#43990)

Azure · May 13, 2024 · 209d5cf · 209d5cf
1 parent fffa7d7
commit 209d5cf
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 33 deletions.
diff --git a/sdk/core/System.ClientModel/CHANGELOG.md b/sdk/core/System.ClientModel/CHANGELOG.md
@@ -4,7 +4,7 @@
 
 ### Features Added
 
-- Added `BufferResponse` property to `RequestOptions` so protocol method callers can override the client value for response content buffering.
+- Added `BufferResponse` property to `RequestOptions` so protocol method callers can turn off response buffering if desired.
 
 ### Breaking Changes
 

diff --git a/sdk/core/System.ClientModel/api/System.ClientModel.net6.0.cs b/sdk/core/System.ClientModel/api/System.ClientModel.net6.0.cs
@@ -239,7 +239,7 @@ public sealed override void Process(System.ClientModel.Primitives.PipelineMessag
     public partial class RequestOptions
     {
         public RequestOptions() { }
-        public bool? BufferResponse { get { throw null; } set { } }
+        public bool BufferResponse { get { throw null; } set { } }
         public System.Threading.CancellationToken CancellationToken { get { throw null; } set { } }
         public System.ClientModel.Primitives.ClientErrorBehaviors ErrorOptions { get { throw null; } set { } }
         public void AddHeader(string name, string value) { }

diff --git a/sdk/core/System.ClientModel/api/System.ClientModel.netstandard2.0.cs b/sdk/core/System.ClientModel/api/System.ClientModel.netstandard2.0.cs
@@ -238,7 +238,7 @@ public sealed override void Process(System.ClientModel.Primitives.PipelineMessag
     public partial class RequestOptions
     {
         public RequestOptions() { }
-        public bool? BufferResponse { get { throw null; } set { } }
+        public bool BufferResponse { get { throw null; } set { } }
         public System.Threading.CancellationToken CancellationToken { get { throw null; } set { } }
         public System.ClientModel.Primitives.ClientErrorBehaviors ErrorOptions { get { throw null; } set { } }
         public void AddHeader(string name, string value) { }

diff --git a/sdk/core/System.ClientModel/src/Options/RequestOptions.cs b/sdk/core/System.ClientModel/src/Options/RequestOptions.cs
@@ -16,15 +16,14 @@ public class RequestOptions
 
     private CancellationToken _cancellationToken = CancellationToken.None;
     private ClientErrorBehaviors _errorOptions = ClientErrorBehaviors.Default;
+    private bool _bufferResponse = true;
 
     private PipelinePolicy[]? _perCallPolicies;
     private PipelinePolicy[]? _perTryPolicies;
     private PipelinePolicy[]? _beforeTransportPolicies;
 
     private List<HeadersUpdate>? _headersUpdates;
 
-    private bool? _bufferResponse;
-
     /// <summary>
     /// Initializes a new instance of the <see cref="RequestOptions"/> class
     /// </summary>
@@ -64,15 +63,14 @@ public ClientErrorBehaviors ErrorOptions
 
     /// <summary>
     /// Gets or sets a value indicating whether the response content should be
-    /// buffered in-memory by the pipeline. If specified, this value will
-    /// override any client default.
+    /// buffered in-memory by the pipeline. This value defaults to <c>true</c>.
     /// </summary>
     /// <remarks>Please note that setting this value to <c>false</c> will result
     /// in the <see cref="PipelineResponse.ContentStream"/> obtained from
     /// <see cref="ClientResult.GetRawResponse"/> holding a live network stream.
     /// It is the responsibility of the caller to ensure the stream is disposed.
     /// </remarks>
-    public bool? BufferResponse
+    public bool BufferResponse
     {
         get => _bufferResponse;
         set
@@ -174,11 +172,8 @@ protected internal virtual void Apply(PipelineMessage message)
         message.PerTryPolicies = _perTryPolicies;
         message.BeforeTransportPolicies = _beforeTransportPolicies;
 
-        // Override any BufferResponse value set on the message.
-        if (BufferResponse.HasValue)
-        {
-            message.BufferResponse = BufferResponse.Value;
-        }
+        // Tell transport whether or not to buffer response content.
+        message.BufferResponse = BufferResponse;
 
         // Apply adds and sets to request headers if applicable.
         if (_headersUpdates is not null)

diff --git a/sdk/core/System.ClientModel/tests/Options/RequestOptionsTests.cs b/sdk/core/System.ClientModel/tests/Options/RequestOptionsTests.cs
@@ -164,7 +164,7 @@ public void CannotModifyOptionsAfterExplicitlyFrozen()
     }
 
     [Test]
-    public void OverridesBufferResponse()
+    public void SetsBufferResponse()
     {
         ClientPipeline pipeline = ClientPipeline.Create();
         PipelineMessage message = pipeline.CreateMessage();
@@ -181,23 +181,4 @@ public void OverridesBufferResponse()
 
         Assert.IsFalse(message.BufferResponse);
     }
-
-    [Test]
-    public void DoesntChangeBufferResponse()
-    {
-        RequestOptions options = new() { BufferResponse = null };
-
-        ClientPipeline pipeline = ClientPipeline.Create();
-        PipelineMessage message = pipeline.CreateMessage();
-
-        message.BufferResponse = false;
-        message.Apply(options);
-
-        Assert.IsFalse(message.BufferResponse);
-
-        message.BufferResponse = true;
-        message.Apply(options);
-
-        Assert.IsTrue(message.BufferResponse);
-    }
 }