Summary
The ASP.NET route system fully supports literals in path variables, but the gRPC Swagger library does not. This results in conflicting paths when using JSON Transcoding. I propose a design that 1) Enables extracting literals automatically, and 2) a general purpose hook that lets a user rewrite these paths.
Disclaimer
I'm doing this backwards, and have already implemented much of this when I was noodling around for a solution.
That being said, I acknowledge I've done it in the wrong order, and am completely open to taking the design in a direction unlike what I've implemented. I'll include examples of what I've done, but I'm extremely open to trashing it and starting over if that's what the design consensus is! I know I'm dropping a giant design proposal on you all, and I'd like to be a good citizen about it 😄
Motivation and goals
This would be a design that solves #64879, but I'll also recap the issue here.
I've been working to design a gRPC API, and we've decided to follow the Google AIPs as a framework. A very relevant document is AIP-131 which dictates how the Get method should look. In short, this style relies on fixed literals at the beginning of resource "names", and these literals are then used to disambiguate the individual endpoints.
For example, a standard .NET service might have a Widgets controller and a Things controller:
GET /v1/widgets/{id}
GET /v1/widgets/{widget_id}/gizmos/{gizmo_id}
GET /v1/things/{id}
...
However, an AIP-style service would include the literals in the variables:
GET /v1/{name=widgets/*}
GET /v1/{name=widgets/*/gizmos/*}
GET /v1/{name=things/*}
The router in ASP.NET supports this just fine, and we end up calling the correct endpoints. But when using JSON Transcoding along with Swagger, it tries to generate these paths:
GET /v1/{name}
GET /v1/{name}
GET /v1/{name}
The goal of this design would be that a user would be able to write an AIP-compliant proto, and be able to serve an OpenAPI document with paths that resemble the way a standard .NET service may implement this (my first example). The user should be able to get the basic functionality with minimal intervention. Secondly, they should be able to customize the parameters used if desired. All behavior changes should be opt-in. The existing behavior should not change unless opted-into, meaning an exception will still be thrown.
In scope
- Basic resolution of AIP-style paths to OpenAPI compatible paths, with minimal user intervention.
- Customization of the generated paths, without modification of the protos
Out of scope
I see this as largely self contained. There are four features, and I think they're all important to make this work. We could theoretically cut the scope, but I really do feel like the features complement each other in a way that works well as a single unit.
Risks / unknowns
This would add a substantial amount of public API surface to the (relatively minimal) Microsoft.AspNetCore.Grpc.Swagger package. The goal is to leave existing behavior unchanged, so risks are minimal. I've explored multi-segment paths, but at the moment I'm unclear if there are more gotchas about paths that I would need to worry about.
We're also exposing more public Google.* through our API. I think this is fine, as this is a library explicitly about using these public Google APIs, but still worth calling out.
Examples
Given a proto:
// message definitions elided, they're not important for the example.
service WidgetService {
rpc GetWidget (WidgetRequest) returns (WidgetResponse) {
option (google.api.http) = {
get: "/v1/{name=widgets/*}"
};
};
rpc GetWidgetGizmo (WidgetGizmoRequest) returns (WidgetGizmoResponse) {
option (google.api.http) = {
get: "/v1/{name=widgets/*/gizmos/*}"
};
};
}
service ThingService {
rpc GetThing (ThingRequest) returns (ThingResponse) {
option (google.api.http) = {
get: "/v1/{name=things/*}"
};
};
}
A user should be able to opt-in to our literal expansion:
var builder = WebApplication.CreateBuilder(args);
builder.Services.AddGrpc();
// ==== Change here ====
builder.Services.AddGrpcSwagger(x => {
x.ExpandLiteralPathSegments = true;
});
// Continue setup, including AddSwaggerGen and MapGrpcService
At this point, the app should serve an OpenAPI document, instead of throwing an exception.
Detailed Design
I think it's important to note here that this doesn't touch the ASP.NET routing system at all; this is only manipulating the OpenAPI gRPC generation (still done via SwaggerGen).
This design comes with four parts, in descending order of importance:
The Expansion Mechanism
This is the ExpandLiteralPathSegments property seen above. This would require adding an optional GrpcSwaggerOptions class, as well as an overload to AddGrpcSwagger to facilitate the builder pattern.
When this property is set to true, we take an alternative path in GrpcJsonTranscodingDescriptionProvider.ResolvePath which splits the segments, and adds in new route variables if needed.
This would resolve the basic case very well, but we do not know what the user would want to expand a multi-segment path to, but it is a requirement for the OpenAPI document to not conflict. I propose the default behavior would be to add path parameters between the literals, and simply apply a numeric suffix:
/v1/{name=widgets/*/gizmos/*} => /v1/widgets/{name}/gizmos/{name1}
This is the most basic way this feature could be implemented, so this step could be considered the MVP that would resolve the issue I linked above.
Better Exceptions
The default exception for this situation looks like this:
SwaggerGeneratorException: Conflicting method/path combination "GET v1/{name}" for actions - ,. Actions require a unique method/path combination for Swagger/OpenAPI 3.0. Use ConflictingActionsResolver as a workaround
The exception thrown by default suggests using the ConflictingActionsResolver as a workaround, but this only lets me choose a winner, I don't get to rewrite. I would propose improving the exception to list the conflicting paths, and then offering the new APIs as a way to disambiguate:
InvalidOperationException: gRPC JSON transcoding produced conflicting OpenAPI paths:
- GET /v1/{name}
• WidgetService.GetWidget (template: /v1/{name=widgets/*})
• WidgetService.GetWidgetGizmo (template: /v1/{name=widgets/*/gizmos/*})
• ThingService.GetThing (template: /v1/{name=things/*})
Set GrpcSwaggerOptions.ExpandLiteralPathSegments = true and/or use GrpcSwaggerOptions.ResolveOpenApiPath to disambiguate.
This tells the user what went wrong, why it's wrong, and provides easy avenues to start fixing it.
Customization Hook
I think users would demand the ability to customize these generated paths in the multi-segment use case. I also propose we add a callback that can be used to modify the generated OpenAPI Paths, before they advance further in the document, while we still have information about the protos on hand.
builder.Services.AddGrpcSwagger(x => {
x.ExpandLiteralPathSegments = true;
x.ResolveOpenApiPath = (context) => {
// Some custom logic
return null; // fall back to sequential naming: name, name1, name2
// If ExpandLiteralPathSegments is false, falls back to existing non-expanded behavior
};
});
// Signature:
Func<GrpcSwaggerPathContext, GrpcSwaggerPathResolution?>? ResolveOpenApiPath { get; set; }
A user would receive this callback for each attempted path (notably, regardless of ExpandLiteralPathSegments. One does not imply the state of the other) and could then modify the path segments in any way they see fit.
The GrpcSwaggerPathContext class would contain information extracted from the Proto, including the MethodDescriptor, HttpRule and a list of parameters that we've parsed out already.
The GrpcSwaggerPathResolution class returned from the above Func contains the final path to be used, as well as a list of parameters with user provided names applied.
Tools for the most common use case
This is the part where I can admit, this may end up being out of scope. I think this idea worked well enough that I wanted it to remain included, but I would be open to removing it, as it could easily live in its own library. That being said, I think it covers the use case for 99% of those who may fall into this issue.
The final design element we'd include is a static function that will generate a Func suitable for assignment to ResolveOpenApiPath. This builder would facilitate easy renames of parameters, based off of the gRPC service definitions:
builder.Services.AddGrpcSwagger(x => {
x.ExpandLiteralPathSegments = true;
x.ResolveOpenApiPath = GrpcSwaggerPathResolver.Build(p =>
{
// Rename a parameter, and go with the automatic path adjustment
p.Add("WidgetService.GetWidget").RenameParameter("name", "widget_id");
// Rename multiple parameters, and specify exactly what you want the final path to be
p.Add("WidgetService.GetWidgetGizmo")
.RenameParameter("name", "widget_id")
.RenameParameter("name1", "gizmo_id");
// Alternate - Path only. This one is pretty fragile, but we just match in order
p.Add("WidgetService.GetWidgetGizmo", "/v1/widgets/{widget_id}/gizmos/{gizmo_id}");
// Alternate - very explicit. If you wanted to, you could touch every element explicitly.
p.Add("WidgetService.GetWidgetGizmo", "/v1/widgets/{widget_id}/gizmos/{gizmo_id}")
.RenameParameter("name", "widget_id")
.RenameParameter("name1", "gizmo_id");
});
});
Proposed public API surface
All additions live under the Microsoft.AspNetCore.Grpc.Swagger namespace and assembly. The existing parameterless AddGrpcSwagger(IServiceCollection) overload is preserved unchanged for binary compatibility; a new configure-action overload is added next to it.
namespace Microsoft.AspNetCore.Grpc.Swagger;
public static class GrpcSwaggerServiceExtensions
{
// Existing overload — unchanged.
public static IServiceCollection AddGrpcSwagger(this IServiceCollection services);
// New overload.
public static IServiceCollection AddGrpcSwagger(
this IServiceCollection services,
Action<GrpcSwaggerOptions>? configureOptions);
}
public sealed class GrpcSwaggerOptions
{
// Feature 1: literal expansion. Defaults to false; no behavior change for existing users.
public bool ExpandLiteralPathSegments { get; set; }
// Feature 2: per-RPC override hook. Fires regardless of ExpandLiteralPathSegments;
// Return null to fall back to the default behavior
public Func<GrpcSwaggerPathContext, GrpcSwaggerPathResolution?>? ResolveOpenApiPath { get; set; }
}
public sealed class GrpcSwaggerPathContext
{
// The Protobuf descriptor for the gRPC method whose path is being resolved.
public required Google.Protobuf.Reflection.MethodDescriptor Method { get; init; }
// The Google API HttpRule attached to the method, including the raw binding template.
public required Google.Api.HttpRule HttpRule { get; init; }
// The path the provider would emit absent any override. Reflects ExpandLiteralPathSegments.
public required string DefaultPath { get; init; }
// The parameter list the provider would emit absent any override.
public required IReadOnlyList<GrpcSwaggerPathParameter> DefaultParameters { get; init; }
}
public sealed class GrpcSwaggerPathResolution
{
// Override the path only; we derive parameters by matching {tokens} against known route variables
public GrpcSwaggerPathResolution(string path);
// Override both the path and the parameter list.
public GrpcSwaggerPathResolution(string path, IReadOnlyList<GrpcSwaggerPathParameter> parameters);
public string Path { get; }
public IReadOnlyList<GrpcSwaggerPathParameter>? Parameters { get; }
}
public sealed record GrpcSwaggerPathParameter
{
// The token name in the OpenAPI path (without braces), e.g. "thingId" for `{thingId}`.
public required string Name { get; init; }
// The Protobuf field backing this parameter. Multiple parameters may reference the same field (e.g., multi-wildcard expansions).
public required Google.Protobuf.Reflection.FieldDescriptor Field { get; init; }
}
// Feature 3: declarative builder over ResolveOpenApiPath.
public static class GrpcSwaggerPathResolver
{
public static Func<GrpcSwaggerPathContext, GrpcSwaggerPathResolution?> Build(
Action<GrpcSwaggerPathOverrideBuilder> configure);
}
public sealed class GrpcSwaggerPathOverrideBuilder
{
// String FQN keyed against MethodDescriptor.FullName.
public GrpcSwaggerPathOverride Add(string methodFullName);
public GrpcSwaggerPathOverride Add(string methodFullName, string path);
}
public sealed class GrpcSwaggerPathOverride
{
public GrpcSwaggerPathOverride RenameParameter(string from, string to);
}
Drawbacks
I consider the primary drawback of this design just to be its size, we add a significant amount of new public API surface to an already very minimal library.
Considered Alternatives
Technically, I believe we could do this with just the ResolveOpenApiPath callback, but that is an incredibly un-ergonomic surface, for what most people would want, which is a "make my exception go away" type fix. Plus, we can do a lot of the heavy lifting for them, in a way that would be generally acceptable.
I also considered doing this with ExpandLiteralPathSegments and OperationFilters instead of the ResolveOpenApiPath but we lose so much good context when going down that route. With the callback, I can provide the user with rich information about which RPC we're dealing with, as well as all the default values that I've cooked up, and it composes well with other functions in an extensible way.
I also considered adding more options to the GrpcSwaggerPathOverrideBuilder that would allow strongly typed overrides, that way we could catch issues at compile-time, and refactors would catch these as well. They're a little too verbose for my liking, but I could totally see them as part of this change:
p.Add<ThingService.ThingServiceBase>(nameof(ThingService.ThingServiceBase.GetThing))
.RenameParameter("name", "thing_id");
Summary
The ASP.NET route system fully supports literals in path variables, but the gRPC Swagger library does not. This results in conflicting paths when using JSON Transcoding. I propose a design that 1) Enables extracting literals automatically, and 2) a general purpose hook that lets a user rewrite these paths.
Disclaimer
I'm doing this backwards, and have already implemented much of this when I was noodling around for a solution.
That being said, I acknowledge I've done it in the wrong order, and am completely open to taking the design in a direction unlike what I've implemented. I'll include examples of what I've done, but I'm extremely open to trashing it and starting over if that's what the design consensus is! I know I'm dropping a giant design proposal on you all, and I'd like to be a good citizen about it 😄
Motivation and goals
This would be a design that solves #64879, but I'll also recap the issue here.
I've been working to design a gRPC API, and we've decided to follow the Google AIPs as a framework. A very relevant document is AIP-131 which dictates how the
Getmethod should look. In short, this style relies on fixed literals at the beginning of resource "names", and these literals are then used to disambiguate the individual endpoints.For example, a standard .NET service might have a
Widgetscontroller and aThingscontroller:However, an AIP-style service would include the literals in the variables:
The router in ASP.NET supports this just fine, and we end up calling the correct endpoints. But when using JSON Transcoding along with Swagger, it tries to generate these paths:
The goal of this design would be that a user would be able to write an AIP-compliant proto, and be able to serve an OpenAPI document with paths that resemble the way a standard .NET service may implement this (my first example). The user should be able to get the basic functionality with minimal intervention. Secondly, they should be able to customize the parameters used if desired. All behavior changes should be opt-in. The existing behavior should not change unless opted-into, meaning an exception will still be thrown.
In scope
Out of scope
I see this as largely self contained. There are four features, and I think they're all important to make this work. We could theoretically cut the scope, but I really do feel like the features complement each other in a way that works well as a single unit.
Risks / unknowns
This would add a substantial amount of public API surface to the (relatively minimal) Microsoft.AspNetCore.Grpc.Swagger package. The goal is to leave existing behavior unchanged, so risks are minimal. I've explored multi-segment paths, but at the moment I'm unclear if there are more gotchas about paths that I would need to worry about.
We're also exposing more public
Google.*through our API. I think this is fine, as this is a library explicitly about using these public Google APIs, but still worth calling out.Examples
Given a proto:
A user should be able to opt-in to our literal expansion:
At this point, the app should serve an OpenAPI document, instead of throwing an exception.
Detailed Design
I think it's important to note here that this doesn't touch the ASP.NET routing system at all; this is only manipulating the OpenAPI gRPC generation (still done via SwaggerGen).
This design comes with four parts, in descending order of importance:
The Expansion Mechanism
This is the
ExpandLiteralPathSegmentsproperty seen above. This would require adding an optionalGrpcSwaggerOptionsclass, as well as an overload toAddGrpcSwaggerto facilitate the builder pattern.When this property is set to true, we take an alternative path in
GrpcJsonTranscodingDescriptionProvider.ResolvePathwhich splits the segments, and adds in new route variables if needed.This would resolve the basic case very well, but we do not know what the user would want to expand a multi-segment path to, but it is a requirement for the OpenAPI document to not conflict. I propose the default behavior would be to add path parameters between the literals, and simply apply a numeric suffix:
This is the most basic way this feature could be implemented, so this step could be considered the MVP that would resolve the issue I linked above.
Better Exceptions
The default exception for this situation looks like this:
The exception thrown by default suggests using the
ConflictingActionsResolveras a workaround, but this only lets me choose a winner, I don't get to rewrite. I would propose improving the exception to list the conflicting paths, and then offering the new APIs as a way to disambiguate:This tells the user what went wrong, why it's wrong, and provides easy avenues to start fixing it.
Customization Hook
I think users would demand the ability to customize these generated paths in the multi-segment use case. I also propose we add a callback that can be used to modify the generated OpenAPI Paths, before they advance further in the document, while we still have information about the protos on hand.
A user would receive this callback for each attempted path (notably, regardless of
ExpandLiteralPathSegments. One does not imply the state of the other) and could then modify the path segments in any way they see fit.The
GrpcSwaggerPathContextclass would contain information extracted from the Proto, including theMethodDescriptor,HttpRuleand a list of parameters that we've parsed out already.The
GrpcSwaggerPathResolutionclass returned from the aboveFunccontains the final path to be used, as well as a list of parameters with user provided names applied.Tools for the most common use case
This is the part where I can admit, this may end up being out of scope. I think this idea worked well enough that I wanted it to remain included, but I would be open to removing it, as it could easily live in its own library. That being said, I think it covers the use case for 99% of those who may fall into this issue.
The final design element we'd include is a static function that will generate a
Funcsuitable for assignment toResolveOpenApiPath. This builder would facilitate easy renames of parameters, based off of the gRPC service definitions:Proposed public API surface
All additions live under the
Microsoft.AspNetCore.Grpc.Swaggernamespace and assembly. The existing parameterlessAddGrpcSwagger(IServiceCollection)overload is preserved unchanged for binary compatibility; a new configure-action overload is added next to it.Drawbacks
I consider the primary drawback of this design just to be its size, we add a significant amount of new public API surface to an already very minimal library.
Considered Alternatives
Technically, I believe we could do this with just the
ResolveOpenApiPathcallback, but that is an incredibly un-ergonomic surface, for what most people would want, which is a "make my exception go away" type fix. Plus, we can do a lot of the heavy lifting for them, in a way that would be generally acceptable.I also considered doing this with
ExpandLiteralPathSegmentsandOperationFiltersinstead of theResolveOpenApiPathbut we lose so much good context when going down that route. With the callback, I can provide the user with rich information about which RPC we're dealing with, as well as all the default values that I've cooked up, and it composes well with other functions in an extensible way.I also considered adding more options to the
GrpcSwaggerPathOverrideBuilderthat would allow strongly typed overrides, that way we could catch issues at compile-time, and refactors would catch these as well. They're a little too verbose for my liking, but I could totally see them as part of this change: