RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.78k stars 1.29k forks source link

C# Controller Code Does Not Compile When Operation Id Has Underscore Followed By Digit #2192

Open alex-leroux opened 5 years ago

alex-leroux commented 5 years ago

The C# controller code generated for the definition below, fails to compile, with the following error message: Error CS1519: Invalid token '1' in class, struct, or interface member declaration

{ "openapi": "3.0.1", "info": { "title": "OperationId with an underscore followed by a digit", "version": "v1" }, "paths": { "/gettext": { "get": { "operationId": "gettext_1", "responses": { "200": { "description": "", "content": { "application/json": { "schema": { "type": "string" } } }, "x-annotation-nullable": true } } } } } }

RicoSuter commented 5 years ago

This needs to be fixed in the operation name generators: https://github.com/RicoSuter/NSwag/tree/master/src/NSwag.CodeGeneration/OperationNameGenerators

slomangino123 commented 4 years ago

I have run into a similar issue to this where my api endpoint has an underscore in the name ie: "users/get_v2". Since the OperationNameGenerator uses the underscores to split the operation name, our naming convention broke this. I fixed it by overriding the methods in "MultipleClientsFromOperationIdOperationNameGenerator".

public class CustomMultipleClientsFromOperationIdOperationNameGenerator :
MultipleClientsFromOperationIdOperationNameGenerator
{

    public override string GetClientName(SwaggerDocument document, string path, string httpMethod, SwaggerOperation operation)        
    {
        return GetClientName(operation);
    }

    public override string GetOperationName(SwaggerDocument document, string path, string httpMethod, SwaggerOperation operation)
    {
        var clientName = GetClientName(operation);
        var operationName = GetOperationName(operation);

        var hasOperationWithSameName = document.Operations
            .Where(o => o.Operation != operation)
            .Any(o => GetClientName(o.Operation) == clientName && GetOperationName(o.Operation) == operationName);

        if (hasOperationWithSameName)
        {
            if (operationName.ToLowerInvariant().StartsWith("get"))
            {
                var isArrayResponse = operation.ActualResponses.ContainsKey("200") &&
                                      operation.ActualResponses["200"].Schema?.ActualSchema
                                          .Type.HasFlag(JsonObjectType.Array) == true;

                if (isArrayResponse)
                {
                    return "GetAll" + operationName.Substring(3);
                }
            }
        }

        return operationName;
    }

    private string GetClientName(SwaggerOperation operation)
    {
        var segments = operation.OperationId.Split('_').FirstOrDefault();
        return segments ?? string.Empty;
    }

    private string GetOperationName(SwaggerOperation operation)
    {
        var segments = operation.OperationId.Split('_');
        string operationName = string.Empty;

        for (int i = 1; i < segments.Count(); i++)
        {
            operationName += segments[i];
        }

        return operationName;
    }
}

Then to complete this I needed to set the OperationNameGenerator in the SwaggerToCSharpClientGeneratorSettings:

  var clientSettings = new SwaggerToCSharpClientGeneratorSettings
  {
      ...
      OperationNameGenerator = new CustomMultipleClientsFromOperationIdOperationNameGenerator(),
      ...
      CSharpGeneratorSettings =
      {
          ...
      },
  };

Worked like a charm, although we are on an older version of NSwag(currently 12.0.12) it seems like the code in the generators has not been modified to account for this as they still use the split by underscore convention.

nycdotnet commented 1 year ago

I have a fix for this in development. For the reported example JSON, the generated controller would look like this:

   [Microsoft.AspNetCore.Mvc.HttpGet, Microsoft.AspNetCore.Mvc.Route("gettext")]
   public System.Threading.Tasks.Task<string> Gettext_1()
   {
            return _implementation.Gettext_1Async();
   }

Does that work for you?

Update April 2024: I didn't complete the fix as the api spec that I was consuming was updated to avoid this.