swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
16.73k stars 6.02k forks source link

Uppercase property names lead to java code that produces wrong json. #4066

Open schnabel opened 7 years ago

schnabel commented 7 years ago
Description

If the swagger declaration contains properties with names containing only uppercase letters and underscores, the generated java code is not java bean conform. As a result jackson maps the properties twice. This actually is part of #4051 but the code currently handles this case earlier.

Swagger-codegen version

Tested with 2.2.1 and 2.2.2-SNAPSHOT

Swagger declaration file content or url
swagger: '2.0'
schemes:
  - http
basePath: '/v0.1/foo'
consumes:
  - 'application/json'
produces:
  - 'application/json'
paths:            
  '/foo':
    get:
      responses:
        200:
          description: "successful operation"
          schema:
            type: "array"
            items:
              $ref: "#/definitions/Pet"

definitions:

  Pet:
    description: >
      Pet
    type: object
    properties:
      ATT_NAME:
        description: >
          Name of the pet
        type: string
Command line used for generation

swagger-codegen-maven-plugin

Steps to reproduce

Build a jaxrs or spring server. Implement controller to return Pet array. Or use jackson mapper and serialize with the generated Pet model code.

Related issues

https://github.com/swagger-api/swagger-codegen/issues/4051

Suggest a Fix

AbstractJavaCodegen

    @Override
    public String toVarName(String name) {
        // sanitize name
        name = sanitizeName(name); // FIXME: a parameter should not be assigned. Also declare the methods parameters as 'final'.

        if ("class".equals(name.toLowerCase())) {
            return "propertyClass";
        }

        if("_".equals(name)) {
          name = "_u";
        }

        // if it's all uppper case, do nothing
//        if (name.matches("^[A-Z_]*$")) {
//            return name;
//        }

        // camelize (lower first character) the variable name
        // pet_id => petId
        name = camelize(name, true);

        // for reserved word or word starting with number, append _
        if (isReservedWord(name) || name.matches("^\\d.*")) {
            name = escapeReservedWord(name);
        }

        return name;
    }
schnabel commented 7 years ago

The fix depends on the fix suggested in #4051 so it is actually:

    public String toVarName(String name) {
        // sanitize name
        name = sanitizeName(name); // FIXME: a parameter should not be assigned. Also declare the methods parameters as 'final'.

        if ("class".equals(name.toLowerCase())) {
            return "propertyClass";
        }

        if("_".equals(name)) {
          name = "_u";
        }

        if(startsWithTwoUppercaseLetters(name)){
            name = name.substring(0, 2).toLowerCase() + name.substring(2);
        }

        // camelize (lower first character) the variable name
        // pet_id => petId
        name = camelize(name, true);

        // for reserved word or word starting with number, append _
        if (isReservedWord(name) || name.matches("^\\d.*")) {
            name = escapeReservedWord(name);
        }

        return name;
    }
wing328 commented 7 years ago

@schnabel #4458 by @y-higuchi has been merged into master.

Would you please pull the latest master to give it a try?

schnabel commented 7 years ago

@wing328

The Problem is not solved by this PR. The java mapping is still the same :-(. Is there a problem with my PR?

lehcim commented 5 years ago

As a result jackson maps the properties twice.

Jackson maps the property twice only if annotation@JsonProperty is set on the property attribute. If annotation is moved to the setter, there is only one mapping.

bugada commented 4 years ago

Actually the startsWithTwoUppercaseLetters(name) is never executed with uppercase properties because this check is still present, that's wrong IMHO.

if (name.matches("^[A-Z_]*$")) { return name; }

Maybe a good approach is:

if (name.matches("^[A-Z_]*$")) { return name.toLowerCase(); }

Thank you Andrea