Open JamesNK opened 1 year ago
Given the intention of this wouldn't it be better to just encourage people to use the minimal version of the API?
+1 On adding ShortCircuitAttribute
. As long as the metadata is internal, frameworks like YARP can not add have short circuited routes.
Given the intention of this wouldn't it be better to just encourage people to use the minimal version of the API?
Controllers (and proxies 😆) are a lot more expensive than minimal, it's unclear how much relative cost an MVC endpoint would save with this attribute, and if making it available would be misleading. That said, the inconsistency is also confusing, they may legitimately have an expensive middleware to skip.
I'm interested in doing this. Can I take a look and submit a PR on that issue?
Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:
API Review Notes:
API Approved!
namespace Microsoft.AspNetCore.Routing.ShortCircuit;
+ public sealed class ShortCircuitAttribute : Attribute, IShortCircuitMetadata
+ {
+ public ShortCircuitAttribute();
+ public ShortCircuitAttribute(int statusCode);
+
+ public int? StatusCode { get; }
+ }
+ public interface IShortCircuitMetadata
+ {
+ int? StatusCode { get; }
+ }
If anyone else is not working on it, im interested to work it out, may i send a PR?
@danirzv, PR already exists https://github.com/dotnet/aspnetcore/pull/52571
API Approved!
namespace Microsoft.AspNetCore.Routing.ShortCircuit; + public sealed class ShortCircuitAttribute : Attribute, IShortCircuitMetadata + { + public ShortCircuitAttribute(); + public ShortCircuitAttribute(int statusCode); + + public int? StatusCode { get; } + } + public interface IShortCircuitMetadata + { + int? StatusCode { get; } + }
@halter73, sorry for long pause.
It seems to me that there are some issues with the ambiguity of attribute behaviour when there is a public constructor with parameter
public ShortCircuitAttribute(int statusCode);
For example, how should an application behave, with a controller like this:
public class TestController : ControllerBase
{
[ShortCircuit(500)]
public ActionResult Index()
{
if (foo)
return new UnauthorizedResult();
if (bar)
return new BadRequestResult();
return new OkResult();
}
}
The statusCode
specified in the ShortCircuit
attribute does not match any of those returned from the method.
Would it still be better to make the constructor with the statusCode
parameter as internal
?
public sealed class ShortCircuitAttribute : Attribute, IShortCircuitMetadata
{
public ShortCircuitAttribute();
- public ShortCircuitAttribute(int statusCode);
+ internal ShortCircuitAttribute(int statusCode);
public int? StatusCode { get; }
}
P.S. I'm work on it #52571
Is there an existing issue for this?
Is your feature request related to a problem? Please describe the problem.
The short circuit feature only has extension methods for adding the required metadata to endpoints. Should there be an attribute? That would allow MVC actions and gRPC methods to support short circuiting.
For example,
Brought up in YouTube video: https://youtu.be/rXdwX2X4-gw?t=422
Note that a short-circuited MVC action would still execute filters, and a gRPC method would still execute interceptors.
Describe the solution you'd like
ShortCircuitMetadata
to an interface. (optional)Additional context
No response