ClientModel Prototype: Change approach to response.Content

sdk/core/Azure.Core/api/Azure.Core.net461.cs

@@ -239,7 +239,6 @@ public abstract partial class Response : System.ClientModel.Primitives.PipelineR
     {
         protected Response() { }
         public abstract string ClientRequestId { get; set; }
-        public virtual new System.BinaryData Content { get { throw null; } }
         public virtual new Azure.Core.ResponseHeaders Headers { get { throw null; } }
         protected internal abstract bool ContainsHeader(string name);
         protected internal abstract System.Collections.Generic.IEnumerable<Azure.Core.HttpHeader> EnumerateHeaders();

sdk/core/Azure.Core/api/Azure.Core.net472.cs

@@ -239,7 +239,6 @@ public abstract partial class Response : System.ClientModel.Primitives.PipelineR
     {
         protected Response() { }
         public abstract string ClientRequestId { get; set; }
-        public virtual new System.BinaryData Content { get { throw null; } }
         public virtual new Azure.Core.ResponseHeaders Headers { get { throw null; } }
         protected internal abstract bool ContainsHeader(string name);
         protected internal abstract System.Collections.Generic.IEnumerable<Azure.Core.HttpHeader> EnumerateHeaders();

sdk/core/Azure.Core/api/Azure.Core.net6.0.cs

@@ -239,7 +239,6 @@ public abstract partial class Response : System.ClientModel.Primitives.PipelineR
     {
         protected Response() { }
         public abstract string ClientRequestId { get; set; }
-        public virtual new System.BinaryData Content { get { throw null; } }
         public virtual new Azure.Core.ResponseHeaders Headers { get { throw null; } }
         protected internal abstract bool ContainsHeader(string name);
         protected internal abstract System.Collections.Generic.IEnumerable<Azure.Core.HttpHeader> EnumerateHeaders();

sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs

@@ -239,7 +239,6 @@ public abstract partial class Response : System.ClientModel.Primitives.PipelineR
     {
         protected Response() { }
         public abstract string ClientRequestId { get; set; }
-        public virtual new System.BinaryData Content { get { throw null; } }
         public virtual new Azure.Core.ResponseHeaders Headers { get { throw null; } }
         protected internal abstract bool ContainsHeader(string name);
         protected internal abstract System.Collections.Generic.IEnumerable<Azure.Core.HttpHeader> EnumerateHeaders();

sdk/core/Azure.Core/src/Response.cs

@@ -29,14 +29,6 @@ public abstract class Response : PipelineResponse
         // TODO: is is possible to not new-slot this?
         public new virtual ResponseHeaders Headers => new ResponseHeaders(this);
 
-        /// <summary>
-        /// Gets the contents of HTTP response, if it is available.
-        /// </summary>
-        /// <remarks>
-        /// Throws <see cref="InvalidOperationException"/> when <see cref="PipelineResponse.ContentStream"/> is not a <see cref="MemoryStream"/>.
-        /// </remarks>
-        public new virtual BinaryData Content => base.Content;
-
         /// <summary>
         /// TBD.
         /// </summary>

sdk/core/Azure.Core/tests/HttpPipelineFunctionalTests.cs

@@ -95,7 +95,7 @@ public async Task NonBufferedExtractedStreamReadableAfterMessageDisposed()
                     await ExecuteRequest(message, httpPipeline);
 
                     Assert.False(message.Response.ContentStream.CanSeek);
-                    Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
+                    //Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
 
                     extractedStream = message.ExtractResponseContent();
                 }
@@ -149,7 +149,7 @@ public async Task NonBufferedFailedResponsesAreDisposedOf()
                     await ExecuteRequest(message, httpPipeline);
 
                     Assert.AreEqual(message.Response.ContentStream.CanSeek, false);
-                    Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
+                    //Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
 
                     extractedStream = message.ExtractResponseContent();
                 }
@@ -568,7 +568,7 @@ public async Task TimeoutsUnbufferedBodyReads()
 
             Assert.AreEqual(message.Response.Status, 200);
             var responseContentStream = message.Response.ContentStream;
-            Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
+            //Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
             var buffer = new byte[10];
             Assert.AreEqual(1, await responseContentStream.ReadAsync(buffer, 0, 1));
             var exception = Assert.ThrowsAsync<TaskCanceledException>(async () => await responseContentStream.ReadAsync(buffer, 0, 10));

sdk/identity/Azure.Identity/tests/HttpPipelineMessageTest.cs → sdk/core/Azure.Core/tests/HttpPipelineMessageTest.cs

@@ -101,8 +101,8 @@ public void ContentPropertyThrowsResponseIsExtracted()
 
             Stream stream = message.ExtractResponseContent();
 
-            Assert.AreSame(memoryStream, stream);
-            Assert.Throws<InvalidOperationException>(() => { var x = response.Content; });
+            //Assert.AreSame(memoryStream, stream);
+            //Assert.Throws<InvalidOperationException>(() => { var x = response.Content; });
         }
     }
 }

sdk/core/Azure.Core/tests/ResponseTests.cs

@@ -126,14 +126,14 @@ public void ContentPropertyGetsContent()
             Assert.AreEqual("body content", responseWithBody.Content.ToString());
         }
 
-        [Test]
-        public void ContentPropertyThrowsForNonMemoryStream()
-        {
-            var response = new MockResponse(200);
-            response.ContentStream = new ThrowingStream();
-
-            Assert.Throws<InvalidOperationException>(() => { BinaryData d = response.Content; });
-        }
+        //[Test]
+        //public void ContentPropertyThrowsForNonMemoryStream()
+        //{
+        //    var response = new MockResponse(200);
+        //    response.ContentStream = new ThrowingStream();
+
+        //    Assert.Throws<InvalidOperationException>(() => { BinaryData d = response.Content; });
+        //}
 
         [Test]
         public void ContentPropertyWorksForMemoryStreamsWithPrivateBuffers()

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

@@ -3,6 +3,7 @@
 
 using System.Buffers;
 using System.ClientModel.Internal;
+using System.Diagnostics;
 using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
@@ -35,28 +36,24 @@ public abstract class PipelineResponse : IDisposable
     /// </summary>
     public abstract Stream? ContentStream { get; set; }
 
+    private byte[]? _contentBytes;
     public virtual BinaryData Content
     {
         get
         {
-            if (ContentStream == null)
+            if (ContentStream is null)
             {
                 return s_emptyBinaryData;
             }
 
-            if (!TryGetBufferedContent(out MemoryStream bufferedContent))
+            if (_contentBytes is not null)
             {
-                throw new InvalidOperationException($"The response is not buffered.");
+                return BinaryData.FromBytes(_contentBytes);
             }
 
-            if (bufferedContent.TryGetBuffer(out ArraySegment<byte> segment))
-            {
-                return new BinaryData(segment.AsMemory());
-            }
-            else
-            {
-                return new BinaryData(bufferedContent.ToArray());
-            }
+            BufferContent();
+            Debug.Assert(_contentBytes is not null);
+            return BinaryData.FromBytes(_contentBytes!);
         }
     }
 
@@ -103,7 +100,7 @@ internal async Task BufferContentAsync(TimeSpan? timeout = default, Cancellation
     private async Task BufferContentSyncOrAsync(TimeSpan? timeout, CancellationTokenSource? cts, bool async)
     {
         Stream? responseContentStream = ContentStream;
-        if (responseContentStream == null || TryGetBufferedContent(out _))
+        if (responseContentStream == null || _contentBytes is not null)
         {
             // No need to buffer content.
             return;
@@ -123,6 +120,9 @@ private async Task BufferContentSyncOrAsync(TimeSpan? timeout, CancellationToken
         responseContentStream.Dispose();
         bufferStream.Position = 0;
         ContentStream = bufferStream;
+
+        // TODO: Come back and optimize - this is only for POC at this stage.
+        _contentBytes= bufferStream.ToArray();
     }
 
     private static async Task CopyToAsync(Stream source, Stream destination, TimeSpan timeout, CancellationTokenSource cancellationTokenSource)

sdk/core/System.ClientModel/src/Pipeline/HttpClientPipelineTransport.Response.cs

@@ -87,16 +87,16 @@ protected virtual void Dispose(bool disposing)
                 // not disposed, because the entity that replaced the response content
                 // intentionally left the network stream undisposed.
 
-                var contentStream = _contentStream;
-                if (contentStream is not null && !TryGetBufferedContent(out _))
-                {
-                    contentStream?.Dispose();
-                    _contentStream = null;
-                }
+                //var contentStream = _contentStream;
+                //if (contentStream is not null && !TryGetBufferedContent(out _))
+                //{
+                //    contentStream?.Dispose();
+                //    _contentStream = null;
+                //}
 
                 _disposed = true;
             }
         }
         #endregion
     }
-}
+}

sdk/core/System.ClientModel/tests/Message/PipelineResponseTests.cs

@@ -33,14 +33,14 @@ public void ContentPropertyGetsContent()
         Assert.AreEqual("body content", responseWithBody.Content.ToString());
     }
 
-    [Test]
-    public void ContentPropertyThrowsForNonMemoryStream()
-    {
-        var response = new MockPipelineResponse(200);
-        response.ContentStream = new ThrowingStream();
-
-        Assert.Throws<InvalidOperationException>(() => { BinaryData d = response.Content; });
-    }
+    //[Test]
+    //public void ContentPropertyThrowsForNonMemoryStream()
+    //{
+    //    var response = new MockPipelineResponse(200);
+    //    response.ContentStream = new ThrowingStream();
+
+    //    Assert.Throws<InvalidOperationException>(() => { BinaryData d = response.Content; });
+    //}
 
     #region Helpers
 

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

@@ -174,7 +174,7 @@ public async Task NonBufferedFailedResponseStreamDisposed()
             await pipeline.SendSyncOrAsync(message, IsAsync);
 
             Assert.AreEqual(message.Response!.ContentStream!.CanSeek, false);
-            Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
+            //Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
         }
 
         Assert.Greater(reqNum, requestCount);
@@ -269,7 +269,7 @@ public async Task TimesOutNonBufferedBodyReads()
 
         Assert.AreEqual(message.Response!.Status, 200);
         var responseContentStream = message.Response.ContentStream;
-        Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
+        //Assert.Throws<InvalidOperationException>(() => { var content = message.Response.Content; });
         var buffer = new byte[10];
         Assert.AreEqual(1, await responseContentStream!.ReadAsync(buffer, 0, 1));
         var exception = Assert.ThrowsAsync<TaskCanceledException>(async () => await responseContentStream.ReadAsync(buffer, 0, 10));