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

Typescript generated client uses inconsistent casing for property names #1447

Open simeyla opened 6 years ago

simeyla commented 6 years ago

For this definition in JSON I have Username and Password properties.

"definitions": {
    "TokenRequest": {
      "type": "object",
      "additionalProperties": false,
      "properties": {
        "Username": {
          "type": "string"
        },
        "Password": {
          "type": "string"
        }
      }
    },

The angular client I'm generating contains the following interface for an ITokenRequest and (as expected) makes the properties lowercase to conform to JS conventions.

export interface ITokenRequest {
    username?: string | undefined;
    password?: string | undefined;
}

However the init() function does not change the casing and gives me this.username = data["Username"]; and you end up with undefined for all the properties.

export class TokenRequest implements ITokenRequest {
    username?: string | undefined;
    password?: string | undefined;

    constructor(data?: ITokenRequest) {
        if (data) {
            for (var property in data) {
                if (data.hasOwnProperty(property))
                    (<any>this)[property] = (<any>data)[property];
            }
        }
    }

    init(data?: any) {
        if (data) {
            this.username = data["Username"];
            this.password = data["Password"];
        }
    }

    static fromJS(data: any): TokenRequest {
        data = typeof data === 'object' ? data : {};
        let result = new TokenRequest();
        result.init(data);
        return result;
    }

    toJSON(data?: any) {
        data = typeof data === 'object' ? data : {};
        data["Username"] = this.username;
        data["Password"] = this.password;
        return data; 
    }
}

This is with all the latest bits from today using msbuild targets to generate the code. I haven't used nswag in a couple months, so can't say exactly when this started.

// Generated using the NSwag toolchain v11.17.19.0 (NJsonSchema v9.10.58.0 (Newtonsoft.Json v9.0.0.0)) (http://NSwag.org)

sh2ezo commented 6 years ago

Try check you have DataContractAttribute on TokenRequest in your C# code. Yesterday met with this bug)

jmatter commented 6 years ago

I'm generating a Angular 6 Client with DTO Interfaces. The interface gets generated with uppercase property Name (first letter) instead of all lowercase. So I also end up with undefined because the JSON Parsing won't work. If i manually change the property names in the generated interface to lowercase (as it should be, also according to JS conventions), everything is working fine.

Generated using the NSwag toolchain v11.18.6.0 (NJsonSchema v9.10.67.0 (Newtonsoft.Json v9.0.0.0)) (http://NSwag.org)

sh2ezo commented 6 years ago

Did you see my previous answer? Or you don't have code written in C#?

simeyla commented 6 years ago

@sh2ezo I did see it - but that's not a good solution. I want to minimize unnecessary attributes.

There's definitely a bug in the generator. It modifies the casing in one place and not in another.

RicoSuter commented 6 years ago

If you generate dto classes then the generated interface is for simple initializing the class via ctor and not to cast your JSON to it...

jmatter commented 6 years ago

@RSuter I agree, but I'm generating interfaces only ("typeStyle": "Interface"). The generated code parses the responseText with JSON.parse:

result200 = _responseText === "" ? null : <MyInterface>JSON.parse(_responseText, this.jsonParseReviver);

This won't throw any error, but results in an object with undefined properties (because the response text is lowercase, but the generated interface properties are not).

The interface properties are 1:1 to the defined properties in the json-schema (swagger-file). I think they should be transformed to lower-case.

I generate the swagger json from my AspNetCore MVC API, so for now I added the JsonProperty annotation to the C# Model:

[JsonProperty(PropertyName = "name")]
public string Name { get; set; }
RicoSuter commented 6 years ago

Ah, I see... If you use the "old" generator (UseSwagger() and not UseSwaggerWithApiExplorer()) you have to change the DefaultPropertyNameHandling to CamelCase (setting) because this (the serializer is configured to serialize to lower camel case) is not automatically detected.

RicoSuter commented 6 years ago

… if you look at the generated spec - are the property names correct according to the serialized JSON or are they already wrong in the spec?

jmatter commented 6 years ago

The property names were already wrong (uppercase) in the generated spec. I'm already using the Api Explorer, but changing the DefaultPropertyNameHandling to CamelCase solved it, the spec is now generated with lower case property names, and now the generated Typescript interfaces are also lowercae and everything is working fine.

Thanks for your help!

RicoSuter commented 6 years ago

Hmm strange, api explorer should auto detect this, can you post the c# code of the middleware registration + settings?

RicoSuter commented 6 years ago

Do not set any of these settings here: https://github.com/RSuter/NSwag/blob/master/src/NSwag.AspNetCore/Middlewares/AspNetCoreToSwaggerMiddleware.cs#L83

Then the settings should be auto detected, can you confirm?

jmatter commented 6 years ago

The generated spec in the API is and was perfectly fine!

app.UseSwaggerUi3WithApiExplorer(settings => {
                settings.PostProcess = document => {
                    document.Info.Version = "v1.0.0";
                    document.Info.Title = "My API";
                    document.Info.Description = "My API Description";
                    document.Info.Contact = new NSwag.SwaggerContact {
                        Name = "My Compay",
                        Email = "mail@company.com",
                        Url = "https://www.company.com"
                    };
                };                
            });

So this is auto detected like you said.

But if I generate the spec with NSwagStudio (I want to switch to MS Build later), I have to specify DefaultPropertyNameHandling to CamelCase:

{
  "runtime": "NetCore21",
  "defaultVariables": null,
  "swaggerGenerator": {
    "aspNetCoreToSwagger": {
      "project": "../myapi/myapi.csproj",
      "msBuildProjectExtensionsPath": null,
      "configuration": "Debug",
      "runtime": "",
      "targetFramework": null,
      "noBuild": true,
      "verbose": false,
      "workingDirectory": null,
      "defaultPropertyNameHandling": "CamelCase",
      "defaultReferenceTypeNullHandling": "Null",
      "defaultEnumHandling": "Integer",
      "flattenInheritanceHierarchy": false,
      "generateKnownTypes": true,
      "generateXmlObjects": false,
      "generateAbstractProperties": false,
      "ignoreObsoleteProperties": false,
      "allowReferencesWithProperties": false,
      "excludedTypeNames": [],
      "serviceHost": null,
      "serviceBasePath": null,
      "serviceSchemes": [],
      "infoTitle": "My API",
      "infoDescription": null,
      "infoVersion": "1.0.0",
      "includedVersions": null,
      "documentTemplate": null,
      "documentProcessorTypes": [],
      "operationProcessorTypes": [],
      "typeNameGeneratorType": null,
      "schemaNameGeneratorType": null,
      "contractResolverType": null,
      "serializerSettingsType": null,
      "output": "myApiSpec.json",
      "outputType": "OpenApi3",
      "assemblyPaths": [],
      "assemblyConfig": null,
      "referencePaths": []
    }
  },
  "codeGenerators": {}
}
RicoSuter commented 6 years ago

Ah, I think for that to work we need the same logic here

https://github.com/RSuter/NSwag/blob/master/src/NSwag.Commands/Commands/SwaggerGeneration/AspNetCore/AspNetCoreToSwaggerGeneratorCommandEntryPoint.cs#L35

and here

https://github.com/RSuter/NSwag/blob/master/src/NSwag.Commands/Commands/SwaggerGeneration/AspNetCore/AspNetCoreToSwaggerCommand.cs#L239

RicoSuter commented 6 years ago

I think this PR fixes that + makes using the same settings in the middleware and CLI easier: https://github.com/RSuter/NSwag/pull/1430

Not sure yet if the actual implementation has the best possible design