-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Core.Experimental] Add DynamicContent/DynamicRequest/DynamicResponse for LLC prototype #18323
Changes from 7 commits
f0a195d
43af060
607bdfb
628bf03
52a3856
3e6a43b
720d564
076b06d
9873807
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
using System; | ||
using System.Diagnostics; | ||
using System.IO; | ||
using System.Text; | ||
using System.Text.Json; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
namespace Azure.Core | ||
{ | ||
/// <summary> | ||
/// Represents the <see cref="DynamicJson"/> sent as part of the Azure.Core.Request. | ||
/// </summary> | ||
[DebuggerDisplay("Content: {_body}")] | ||
public class DynamicContent : RequestContent | ||
{ | ||
private readonly DynamicJson _body; | ||
|
||
internal DynamicContent(DynamicJson body) | ||
{ | ||
_body = body; | ||
} | ||
|
||
internal static RequestContent Create(DynamicJson body) => new DynamicContent(body); | ||
|
||
/// <inheritdoc /> | ||
public override async Task WriteToAsync(Stream stream, CancellationToken cancellation) | ||
{ | ||
using Utf8JsonWriter writer = new Utf8JsonWriter(stream); | ||
_body.WriteTo(writer); | ||
await writer.FlushAsync(cancellation).ConfigureAwait(false); | ||
} | ||
|
||
/// <inheritdoc /> | ||
public override void WriteTo(Stream stream, CancellationToken cancellation) | ||
{ | ||
using Utf8JsonWriter writer = new Utf8JsonWriter(stream); | ||
_body.WriteTo(writer); | ||
writer.Flush(); | ||
} | ||
|
||
/// <inheritdoc /> | ||
public override bool TryComputeLength(out long length) | ||
{ | ||
using MemoryStream stream = new MemoryStream(); | ||
WriteTo(stream, CancellationToken.None); | ||
length = Encoding.UTF8.GetString(stream.ToArray()).Length; | ||
return true; | ||
} | ||
|
||
/// <inheritdoc /> | ||
public override void Dispose() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should properly implement the Dispose pattern here, i.e. Dispose(bool), or seal the type. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even sealed I get:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or did you mean seal it and then just GC.SuppressFinalize is fine? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I meant is that if you seal, you don't need to implement full dispose pattern (Dispose(bool). If you don't seal, you need to. |
||
{ | ||
GC.SuppressFinalize(this); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,167 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
using Azure.Core; | ||
using Azure.Core.Pipeline; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Diagnostics; | ||
using System.Diagnostics.CodeAnalysis; | ||
using System.IO; | ||
using System.Text; | ||
using System.Text.Json; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
namespace Azure.Core | ||
{ | ||
/// <summary> | ||
/// Represents an HTTP request with <see cref="DynamicJson"/> content. | ||
/// </summary> | ||
[DebuggerDisplay("Body: {Body}")] | ||
public class DynamicRequest : Request | ||
{ | ||
private Request Request { get; } | ||
private HttpPipeline HttpPipeline { get; } | ||
private bool _disposed; | ||
|
||
private static readonly Encoding Utf8NoBom = new UTF8Encoding(false, true); | ||
|
||
/// <inheritdoc /> | ||
public override RequestContent? Content | ||
{ | ||
get => DynamicContent.Create(Body); | ||
|
||
set { | ||
MemoryStream ms = new MemoryStream(); | ||
if (value != null) | ||
{ | ||
value.WriteTo(ms, default); | ||
ms.Seek(0, SeekOrigin.Begin); | ||
} | ||
using (StreamReader sr = new StreamReader(ms, Utf8NoBom)) | ||
{ | ||
Body = new DynamicJson(sr.ReadToEnd()); | ||
} | ||
Request.Content = value; | ||
} | ||
} | ||
|
||
// TODO(matell): How does the initialization here play into the ability to send a request with an "empty" body? | ||
/// <summary> | ||
/// The JSON body of request. | ||
/// </summary> | ||
public DynamicJson Body { get; set; } = DynamicJson.Object(); | ||
|
||
// TODO(matell): In Krzysztof's prototype we also took DiagnosticScope as a parameter, do we still need that? | ||
/// <summary> | ||
/// Creates an instance of <see cref="RequestContent"/> that wraps <see cref="DynamicJson"/> content. | ||
/// </summary> | ||
/// <param name="request">The <see cref="Request"/> to send.</param> | ||
/// <param name="pipeline">The HTTP pipeline for sending and receiving REST requests and responses.</param> | ||
public DynamicRequest(Request request, HttpPipeline pipeline) | ||
{ | ||
Request = request; | ||
HttpPipeline = pipeline; | ||
} | ||
|
||
/// <summary> | ||
/// Send the request asynchronously. | ||
/// </summary> | ||
/// <param name="cancellationToken">The cancellation token.</param> | ||
/// <returns>The response dynamically typed in a <see cref="DynamicResponse"/>.</returns> | ||
public async Task<DynamicResponse> SendAsync(CancellationToken cancellationToken = default) | ||
{ | ||
// Since we are sending the underlying request, we need to copy the Content on to it, or we'll lose the body. | ||
Request.Content = Content; | ||
|
||
Response res = await HttpPipeline.SendRequestAsync(Request, cancellationToken).ConfigureAwait(false); | ||
DynamicJson dynamicContent; | ||
|
||
if (res.ContentStream != null) | ||
{ | ||
JsonDocument doc = await JsonDocument.ParseAsync(res.ContentStream, new JsonDocumentOptions(), cancellationToken).ConfigureAwait(false); | ||
dynamicContent = new DynamicJson(doc.RootElement); | ||
} | ||
else | ||
{ | ||
dynamicContent = new DynamicJson(JsonDocument.Parse("null").RootElement); | ||
} | ||
|
||
return new DynamicResponse(res, dynamicContent); | ||
} | ||
|
||
/// <summary> | ||
/// Send the request synchronously. | ||
/// </summary> | ||
/// <param name="cancellationToken">The cancellation token.</param> | ||
/// <returns>The response dynamically typed in a <see cref="DynamicResponse"/>.</returns> | ||
public DynamicResponse Send(CancellationToken cancellationToken = default) | ||
{ | ||
// Since we are sending the underlying request, we need to copy the Content on to it, or we'll lose the body. | ||
Request.Content = Content; | ||
|
||
Response res = HttpPipeline.SendRequest(Request, cancellationToken); | ||
DynamicJson dynamicContent; | ||
|
||
if (res.ContentStream != null) | ||
{ | ||
JsonDocument doc = JsonDocument.Parse(res.ContentStream); | ||
dynamicContent = new DynamicJson(doc.RootElement); | ||
} | ||
else | ||
{ | ||
dynamicContent = new DynamicJson(JsonDocument.Parse("null").RootElement); | ||
} | ||
|
||
return new DynamicResponse(res, dynamicContent); | ||
} | ||
|
||
/// <inheritdoc /> | ||
public override string ClientRequestId { get => Request.ClientRequestId; set => Request.ClientRequestId = value; } | ||
|
||
/// <summary> | ||
/// Frees resources held by the <see cref="DynamicRequest"/> object. | ||
/// </summary> | ||
public override void Dispose() | ||
{ | ||
Dispose(true); | ||
GC.SuppressFinalize(this); | ||
} | ||
|
||
/// <summary> | ||
/// Frees resources held by the <see cref="DynamicRequest"/> object. | ||
/// </summary> | ||
/// <param name="disposing">true if we should dispose, otherwise false</param> | ||
protected virtual void Dispose(bool disposing) | ||
{ | ||
if (_disposed) | ||
{ | ||
return; | ||
} | ||
if (disposing) | ||
{ | ||
Request.Dispose(); | ||
} | ||
_disposed = true; | ||
} | ||
|
||
/// <inheritdoc /> | ||
protected override void AddHeader(string name, string value) => Request.Headers.Add(name, value); | ||
|
||
/// <inheritdoc /> | ||
protected override bool ContainsHeader(string name) => Request.Headers.Contains(name); | ||
|
||
/// <inheritdoc /> | ||
protected override IEnumerable<HttpHeader> EnumerateHeaders() => Request.Headers; | ||
|
||
/// <inheritdoc /> | ||
protected override bool RemoveHeader(string name) => Request.Headers.Remove(name); | ||
|
||
/// <inheritdoc /> | ||
protected override bool TryGetHeader(string name, [NotNullWhen(true)] out string? value) => Request.Headers.TryGetValue(name, out value); | ||
|
||
/// <inheritdoc /> | ||
protected override bool TryGetHeaderValues(string name, [NotNullWhen(true)] out IEnumerable<string>? values) => Request.Headers.TryGetValues(name, out values); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Diagnostics; | ||
using System.Text; | ||
|
||
namespace Azure.Core | ||
{ | ||
/// <summary> | ||
/// Represents a result of Azure operation with a <see cref="DynamicJson"/> response. | ||
/// </summary> | ||
[DebuggerDisplay("Status: {Response.Status}, Value: {Value}")] | ||
public class DynamicResponse : Response<DynamicJson> | ||
{ | ||
private Response Response { get; } | ||
|
||
/// <inheritdoc />> | ||
public override DynamicJson Value { get; } | ||
|
||
/// <inheritdoc /> | ||
public override Response GetRawResponse() => Response; | ||
|
||
/// <summary> | ||
/// Represents a result of Azure operation with a <see cref="DynamicJson"/> response. | ||
/// </summary> | ||
/// <param name="response">The response returned by the service.</param> | ||
/// <param name="value">The value returned by the service.</param> | ||
public DynamicResponse(Response response, DynamicJson value) | ||
{ | ||
Response = response; | ||
Value = value; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very expensive way to calculate the length. We can at least try to cache to resulting content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be hard because the referenced
DynamicJson
can change.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should do the following:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is my unfortunate doing. While working on a prototype I found that without implementing this we would get chunked encoding for the request and that was causing a problem with a service I was testing. It is hard with DynamicJson being mutable.
How often is
TryComputeLength
called on average per request? I would have expected just once if it returnstrue
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once