Swashbuckle.aspnetcore: Strange operationId name

[PaddoSwam]

I ran into a situation where I ended up with the following operationId which seems a little odd.

"operationId":"ApiV{versionHelloWorldGet"

Now I've traced it back to the FriendlyId function but have yet to discover the actual cause of this odd behaviour.

I'm actively investigating this, and it _might_ be caused by something else in my current project.

[PaddoSwam]

Duplicate of #244

[commonsensesoftware]

IMHO this should be reopened and investigated for resolution. I do not believe this behavior is intended; therefore, it's a bug. The issue isn't specifically related to ASP.NET API Versioning, so it's not a duplicate either. There are other cases where this can happen.

The bug exists at: ApiDescriptionExtensions.cs (Line 43). This code assumes that any URL segment that contains a route parameter will have the form of {param name} and extracts the parameter name from within (ex: api/helloworld/{id}); however, that is incorrect. It's perfectly valid to have route parameters that include other literal characters in the URL segment. Given the assumption on the format, the code then presumes it can just trim off the leading { and trailing }, which would theoretically leave only the route parameter name.

Consider the path api/v{api-version}/helloworld/{id}. This is what a route might look like using API versioning via a URL segment (which isn't the only way). The current code processes the second segment such that:

• api -> trim { and } -> api
• v{api-version} -> trim { and } -> v{api-version
• helloworld -> trim { and } -> helloworld
• {id} -> trim { and } -> id
• api + v{api-version + helloworld + By + id + GET -> to titlecase -> ApiV{versionHelloWorldGet

My suggestion would be to either not mangle the URL at all or come up with an alternate method that considers all the variations of route parameters and any associated route constraints. For example, consider: api/employees/{ssn:regex(^\\d{{3}}-\\d{{2}}-\\d{{4}}$)}. This would become something like ApiEmployeesBySsn:regex(^\d{{3}}-\d{{2}}-\d{{4}}$), which is hardly a _friendly_ identifier IMO.

[domaindrivendev]

@commonsensesoftware coming up with a "friendly" identifier that works in all cases is near impossible. But, you've pointed out some cases where improvements can be made.

I would also point out that you can use an IOperationFilter (see readme) to implement your own strategy for generating operationIds. Alternatively, you can decorate actions with SwaggerOperationAttribute to explicitly set the operationId for any given one.

[commonsensesoftware]

@domaindrivendev I agree - coming up with a universal _friendly_ name isn't so easy.

Why not just use ActionDescriptor.DisplayName? It's not without flaws, but it does produce a friendly name. The format can be updated via extensions as well. It also seems that the ActionDescriptor passed around is usually a ControllerActionDescriptor, which allows building a _friendly_ name from the type and method name - similar to the default behavior, but with more control. Some other alternatives could include using various combinations of ActionDescriptor.GroupName, ActionDescriptor.ControllerName, ActionDescriptor.ActionName, and/or ActionDescriptor.HttpMethod.

In the spirit of keeping things simple, perhaps ActionDescriptor.DisplayName or a close variation by default. Then you can use an albeit new XML Comment to override the friendly name. For example:

c# /// <summary> /// Gets a single resource. /// </summary> /// <param name="id">The requested resource identifier.</param> /// <returns>The requested resource.</returns> /// <friendlyName>GetResourceById</friendlyName> /// <response code="200">The resource was successfully retrieved.</response> /// <response code="404">The resource does not exist.</response> [HttpGet( "{id:int}", Name = "GetResourceById" )] [ProducesResponseType( typeof( Resource ), 200 )] [ProducesResponseType( 404 )] public IActionResult Get( int id ) => Ok();

Pardon my ignorance, but I don't know whether the _friendly_ names need to be unique and, if so, whether it's unique by API version or for everything. If the name must span everything, then mixing in the GroupName somehow should provide disambiguation.

There are many ways this problem could be solved or improved. May you find a few of these ideas useful or spark additional thoughts.

[greenmooseSE]

FWIW, when using NSwag.CodeGeneration.CSharp (11.12.13) to generate API client, this issue results in below (invalid) generated code

        /// <summary>Returns "Pong!".</summary>
        /// <param name="version">The requested API version</param>
        /// <returns>Success</returns>
        /// <exception cref="SwaggerException">A server side error occurred.</exception>
        public System.Threading.Tasks.Task<string> ApiV{versionPingGetAsync(string version)
        {
            return ApiV{versionPingGetAsync(version, System.Threading.CancellationToken.None);
        }

[domaindrivendev]

This functionality has gone through many iterations to try get a systematic generation of a "friendly id" correct and looks like there's still many issues. At this point, I'm tempted to go back to the original approach which just used the action name in the method signature. That puts the onus on developers to come up with meaningful and unique names. I understand that won't work for everyone, but folks can always override that strategy with their own implementation. It's just too trick for SB to come up with something that works for all cases.

[commonsensesoftware]

@domaindrivendev - I agree. I think we have surpassed the point of being able to create a _friendly_ identifier and you have already provided SwaggerOperationAttribute as an alternative. To be crisp on language, how you free about having a _safe_ identifier as opposed to _friendly_ one?

The current implementation simply uses Trim, which is inadequate. I would suggest the following enhancements:

1. Remove { and } regardless of where they appear in a segment.
2. Only keep the name of a route parameter (e.g. ditch parameters and default values). For example, {id=42} becomes id and {ssn:regex(^\\d{{3}}-\\d{{2}}-\\d{{4}}$)} becomes ssn.
3. Remove _unsafe_ characters or replace them with something _safe_ like _. For example, v{api-version} becomes vApi_Version or vApiVersion.

Beyond that, I don't know that there is an _auto-magical_ answer. Using SwaggerOperationAttribute or IOperationFilter seems more than adequate for those that want full control over the operationId.

[jboarman]

@domaindrivendev I completely agree. Personally, I think the simplest way to publish the operation name in one's spec is to simply name the operation the way one wants it published.

So, instead of a lazily naming a method Get, we should name it GetThingyById. If we want to maintain an internal name vs an external operation name, then it make sense to use the SwaggerOperation.

If people are not happy with exposing an operationId at all, then there could be a c.DescribeOperationId(false) setting to control that. BUT, since there is an available workaround to set the friendly name to blank, then I don't think a new setting is necessary.

[domaindrivendev]

@commonsensesoftware, @jboarman - see the above PR. It reverts to using the action name OR the route name. So, developers can choose to be more explicit with their method names OR they can provide an explicit OperationId through the HttpMethodAttribute:

[HttpGet]
public IActionResult GetProducts()
...

[HttpGet(Name = "GetProducts"]
public IActionResult Get() 
...

// Both of the above will generate an operationId = "GetProducts"

[domaindrivendev]

Merged - will be generally available with the 4.0.0 release (coming soon). In the meantime you can try it out with the latest preview package on myget.org:

https://www.myget.org/feed/domaindrivendev/package/nuget/Swashbuckle.AspNetCore