Skip to content

Commit

Permalink
Refine client pipeline abstraction (#5044)
Browse files Browse the repository at this point in the history 
- Make `ClientPipelineApi.ProcessMessage(Async)` abstract directly
- Remove `ClientPipelineApi.Send(Async)`, it is covered in above item
- Remove `HttpMessageApi.ExtractResponse`, it is covered in the first
item

Corresponding change in Azure plugin:
Azure/azure-sdk-for-net#47049
  • Loading branch information
@live1206
live1206 authored Nov 12, 2024
1 parent 986c2b1 commit e10bab8
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 72 deletions.
8 changes: 3 additions & 5 deletions ...or/Microsoft.Generator.CSharp.ClientModel/src/Providers/Abstractions/ClientPipelineApi.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using Microsoft.Generator.CSharp.Expressions;
using Microsoft.Generator.CSharp.Primitives;
using Microsoft.Generator.CSharp.Providers;
using Microsoft.Generator.CSharp.Snippets;
using Microsoft.Generator.CSharp.Statements;

Expand All @@ -20,11 +19,10 @@ protected ClientPipelineApi(Type type, ValueExpression original) : base(type, or
{
}

public abstract ValueExpression CreateMessage(HttpRequestOptionsApi requestOptions, ValueExpression responseClassifier);

public abstract MethodBodyStatement Send(HttpMessageApi message, HttpRequestOptionsApi options);
public abstract MethodBodyStatement[] ProcessMessage(HttpMessageApi message, HttpRequestOptionsApi options);
public abstract MethodBodyStatement[] ProcessMessageAsync(HttpMessageApi message, HttpRequestOptionsApi options);

public abstract MethodBodyStatement SendAsync(HttpMessageApi message, HttpRequestOptionsApi options);
public abstract ValueExpression CreateMessage(HttpRequestOptionsApi requestOptions, ValueExpression responseClassifier);

public abstract ValueExpression Create(ValueExpression options, ValueExpression perRetryPolicies);

Expand Down
2 changes: 0 additions & 2 deletions ...rator/Microsoft.Generator.CSharp.ClientModel/src/Providers/Abstractions/HttpMessageApi.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ protected HttpMessageApi(Type type, ValueExpression original) : base(type, origi

public abstract MethodBodyStatement ApplyRequestOptions(HttpRequestOptionsApi options);

public abstract MethodBodyStatement[] ExtractResponse();

public abstract HttpMessageApi FromExpression(ValueExpression original);

public abstract HttpMessageApi ToExpression();
Expand Down
42 changes: 3 additions & 39 deletions ...icrosoft.Generator.CSharp.ClientModel/src/Providers/ClientPipelineExtensionsDefinition.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using Microsoft.Generator.CSharp.Expressions;
using Microsoft.Generator.CSharp.Primitives;
using Microsoft.Generator.CSharp.Providers;
using Microsoft.Generator.CSharp.Snippets;
using Microsoft.Generator.CSharp.Statements;
using static Microsoft.Generator.CSharp.Snippets.Snippet;

Expand Down Expand Up @@ -49,8 +48,7 @@ protected override MethodProvider[] BuildMethods()
BuildProcessMessageAsync(),
BuildProcessMessage(),
ProcessHeadAsBoolMessageAsync(),
ProcessHeadAsBoolMessage(),
BuildExtractResponseContent()
ProcessHeadAsBoolMessage()
];
}

Expand Down Expand Up @@ -120,28 +118,7 @@ private MethodSignature GetProcessHeadAsBoolMessageSignature(bool isAsync)
private MethodProvider BuildProcessMessage()
{
MethodSignature signature = GetProcessMessageSignature(false);

var clientErrorNoThrow = _options.NoThrow();
return new MethodProvider(signature, new MethodBodyStatement[]
{
_pipeline.Send(_message, _options),
MethodBodyStatement.EmptyLine,
new IfStatement(_message.Response().IsError().And(new BinaryOperatorExpression("&", _options.NullConditional().Property("ErrorOptions"), clientErrorNoThrow).NotEqual(clientErrorNoThrow)))
{
Throw(New.Instance(ClientModelPlugin.Instance.TypeFactory.ClientResponseApi.ClientResponseExceptionType, _message.Response()))
},
MethodBodyStatement.EmptyLine,
Declare("response", ClientModelPlugin.Instance.TypeFactory.HttpResponseApi.HttpResponseType, new TernaryConditionalExpression(_message.BufferResponse(), _message.Response(), Static().Invoke(ExtractResponseContentMethodName, [_messageParam])), out var response),
Return(response)
}, this);
}

private const string ExtractResponseContentMethodName = "ExtractResponseContent";
private MethodProvider BuildExtractResponseContent()
{
var signature = new MethodSignature(ExtractResponseContentMethodName, null, MethodSignatureModifiers.Private | MethodSignatureModifiers.Static, ClientModelPlugin.Instance.TypeFactory.HttpResponseApi.HttpResponseType, null, [_messageParam]);
var body = _message.ExtractResponse();
return new MethodProvider(signature, body, this);
return new MethodProvider(signature, _pipeline.ProcessMessage(_message, _options), this);
}

private MethodSignature GetProcessMessageSignature(bool isAsync)
Expand All @@ -163,20 +140,7 @@ private MethodSignature GetProcessMessageSignature(bool isAsync)
private MethodProvider BuildProcessMessageAsync()
{
MethodSignature signature = GetProcessMessageSignature(true);

var clientErrorNoThrow = _options.NoThrow();
return new MethodProvider(signature, new MethodBodyStatement[]
{
_pipeline.SendAsync(_message, _options),
MethodBodyStatement.EmptyLine,
new IfStatement(_message.Response().IsError().And(new BinaryOperatorExpression("&", _options.NullConditional().Property("ErrorOptions"), clientErrorNoThrow).NotEqual(clientErrorNoThrow)))
{
Throw(ClientModelPlugin.Instance.TypeFactory.ClientResponseApi.ToExpression().CreateAsync(_message.Response()))
},
MethodBodyStatement.EmptyLine,
Declare("response", ClientModelPlugin.Instance.TypeFactory.HttpResponseApi.HttpResponseType, new TernaryConditionalExpression(_message.BufferResponse(), _message.Response(), Static().Invoke(ExtractResponseContentMethodName, [_messageParam])), out var response),
Return(response)
}, this);
return new MethodProvider(signature, _pipeline.ProcessMessageAsync(_message, _options), this);
}
}
}
33 changes: 28 additions & 5 deletions .../generator/Microsoft.Generator.CSharp.ClientModel/src/Providers/ClientPipelineProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.Generator.CSharp.Expressions;
using Microsoft.Generator.CSharp.Primitives;
using Microsoft.Generator.CSharp.Statements;
using Microsoft.Generator.CSharp.Snippets;
using static Microsoft.Generator.CSharp.Snippets.Snippet;

namespace Microsoft.Generator.CSharp.ClientModel.Providers
Expand Down Expand Up @@ -36,12 +37,34 @@ public override ClientPipelineApi FromExpression(ValueExpression expression)
public override ValueExpression PerRetryPolicy(params ValueExpression[] arguments)
=> Static<ApiKeyAuthenticationPolicy>().Invoke(nameof(ApiKeyAuthenticationPolicy.CreateHeaderApiKeyPolicy), arguments).As<ApiKeyAuthenticationPolicy>();

public override MethodBodyStatement Send(HttpMessageApi message, HttpRequestOptionsApi options)
=> Original.Invoke(nameof(ClientPipeline.Send), [message]).Terminate();
public override ClientPipelineApi ToExpression() => this;

public override MethodBodyStatement SendAsync(HttpMessageApi message, HttpRequestOptionsApi options)
=> Original.Invoke(nameof(ClientPipeline.SendAsync), [message], true).Terminate();
public override MethodBodyStatement[] ProcessMessage(HttpMessageApi message, HttpRequestOptionsApi options)
=>
[
Original.Invoke(nameof(ClientPipeline.Send), [message]).Terminate(),
MethodBodyStatement.EmptyLine,
new IfStatement(message.Response().IsError().And(new BinaryOperatorExpression("&", options.NullConditional().Property("ErrorOptions"), options.NoThrow()).NotEqual(options.NoThrow())))
{
Throw(New.Instance(ClientModelPlugin.Instance.TypeFactory.ClientResponseApi.ClientResponseExceptionType, message.Response()))
},
MethodBodyStatement.EmptyLine,
Declare("response", ClientModelPlugin.Instance.TypeFactory.HttpResponseApi.HttpResponseType, new TernaryConditionalExpression(message.BufferResponse(), message.Response(), message.Invoke(nameof(PipelineMessage.ExtractResponse))), out var response),
Return(response)
];

public override ClientPipelineApi ToExpression() => this;
public override MethodBodyStatement[] ProcessMessageAsync(HttpMessageApi message, HttpRequestOptionsApi options)
=>
[
Original.Invoke(nameof(ClientPipeline.SendAsync), [message], true).Terminate(),
MethodBodyStatement.EmptyLine,
new IfStatement(message.Response().IsError().And(new BinaryOperatorExpression("&", options.NullConditional().Property("ErrorOptions"), options.NoThrow()).NotEqual(options.NoThrow())))
{
Throw(ClientModelPlugin.Instance.TypeFactory.ClientResponseApi.ToExpression().CreateAsync(message.Response()))
},
MethodBodyStatement.EmptyLine,
Declare("response", ClientModelPlugin.Instance.TypeFactory.HttpResponseApi.HttpResponseType, new TernaryConditionalExpression(message.BufferResponse(), message.Response(), message.Invoke(nameof(PipelineMessage.ExtractResponse))), out var response),
Return(response)
];
}
}
3 changes: 0 additions & 3 deletions ...generator/Microsoft.Generator.CSharp.ClientModel/src/Providers/PipelineMessageProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ public override MethodBodyStatement ApplyResponseClassifier(StatusCodeClassifier
public override MethodBodyStatement ApplyRequestOptions(HttpRequestOptionsApi options)
=> Original.Invoke(nameof(PipelineMessage.Apply), options).Terminate();

public override MethodBodyStatement[] ExtractResponse()
=> [Return(Original.Invoke(nameof(PipelineMessage.ExtractResponse)))];

public override HttpMessageApi FromExpression(ValueExpression original)
=> new PipelineMessageProvider(original);

Expand Down
11 changes: 5 additions & 6 deletions ...rosoft.Generator.CSharp.ClientModel/test/Providers/Abstractions/ClientPipelineApiTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,13 @@ public override ClientPipelineApi FromExpression(ValueExpression expression)
public override ValueExpression PerRetryPolicy(params ValueExpression[] arguments)
=> Original.Invoke("GetFakePerRetryPolicy", arguments);

public override MethodBodyStatement Send(HttpMessageApi message, HttpRequestOptionsApi options)
=> Original.Invoke("GetFakeSend", [message, options]).Terminate();
public override ClientPipelineApi ToExpression() => this;

public override MethodBodyStatement SendAsync(HttpMessageApi message, HttpRequestOptionsApi options)
=> Original.Invoke("GetFakeSendAsync", [message, options]).Terminate();
public override MethodBodyStatement[] ProcessMessage(HttpMessageApi message, HttpRequestOptionsApi options)
=> [Original.Invoke("GetFakeProcessMessage", [message, options]).Terminate()];

public override ClientPipelineApi ToExpression() => this;
public override MethodBodyStatement[] ProcessMessageAsync(HttpMessageApi message, HttpRequestOptionsApi options)
=> [Original.Invoke("GetFakeProcessMessageAsync", [message, options]).Terminate()];
}
}

}
4 changes: 0 additions & 4 deletions ...Microsoft.Generator.CSharp.ClientModel/test/Providers/Abstractions/HttpMessageApiTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using Microsoft.Generator.CSharp.Statements;
using Microsoft.Generator.CSharp.Tests.Common;
using NUnit.Framework;
using static Microsoft.Generator.CSharp.Snippets.Snippet;

namespace Microsoft.Generator.CSharp.ClientModel.Tests.Providers.Abstractions
{
Expand Down Expand Up @@ -60,9 +59,6 @@ public override MethodBodyStatement ApplyRequestOptions(HttpRequestOptionsApi op
public override ValueExpression BufferResponse()
=> Original.Invoke("GetFakeBufferResponse");

public override MethodBodyStatement[] ExtractResponse()
=> [Return(Original.Invoke("GetFakeExtractResponse"))];

public override HttpMessageApi FromExpression(ValueExpression original)
=> new TestHttpMessageApi(original);

Expand Down
2 changes: 1 addition & 1 deletion ...p.ClientModel/test/Providers/Definitions/ClientPipelineExtensionsDefCustomizationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public async Task CanReplaceMethod()

// The definitions should not have the custom method
var definitionMethods = definition!.Methods;
Assert.AreEqual(4, definitionMethods.Count);
Assert.AreEqual(3, definitionMethods.Count);
Assert.IsFalse(definitionMethods.Any(m => m.Signature.Name == "ProcessMessageAsync"));

// The custom code view should contain the method
Expand Down
9 changes: 2 additions & 7 deletions .../TestProjects/Local/Unbranded-TypeSpec/src/Generated/Internal/ClientPipelineExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public static async ValueTask<PipelineResponse> ProcessMessageAsync(this ClientP
throw await ClientResultException.CreateAsync(message.Response).ConfigureAwait(false);
}

PipelineResponse response = message.BufferResponse ? message.Response : ExtractResponseContent(message);
PipelineResponse response = message.BufferResponse ? message.Response : message.ExtractResponse();
return response;
}

Expand All @@ -32,7 +32,7 @@ public static PipelineResponse ProcessMessage(this ClientPipeline pipeline, Pipe
throw new ClientResultException(message.Response);
}

PipelineResponse response = message.BufferResponse ? message.Response : ExtractResponseContent(message);
PipelineResponse response = message.BufferResponse ? message.Response : message.ExtractResponse();
return response;
}

Expand Down Expand Up @@ -63,10 +63,5 @@ public static ClientResult<bool> ProcessHeadAsBoolMessage(this ClientPipeline pi
return new ErrorResult<bool>(response, new ClientResultException(response));
}
}

private static PipelineResponse ExtractResponseContent(PipelineMessage message)
{
return message.ExtractResponse();
}
}
}

0 comments on commit e10bab8

Please sign in to comment.