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
17k stars 6.03k forks source link

[swagger-codegen Java] codegen generates invalid Java classes for descriptions of simple type integer or string #2314

Open h2m opened 8 years ago

h2m commented 8 years ago

The following description section inside an OAI specification in yaml led to invalid java classes HelloWorldResponse.java and UsageLimit.java after code generation in version 2.1.5

definitions:
  HelloWorldResponse:
    type: string
  UsageLimit:
    type: integer
    description: Defines the usage limit
  ErrorResponse:
    required:
      - message
    properties:
      message:
        type: string

see GIST [https://gist.github.com/h2m/322effb99d5261671ca1]

Is this a bug, an invalid specification or a problem with Java not being able to subclass a String or Integer type?

wing328 commented 8 years ago

@h2m do you expect HelloWordResponse to have one attribute (string) ? What about UsageLimit ?

h2m commented 8 years ago

@wing328 the idea behind was rather than directly using string or integer, we wanted to have a named alias as a reusable type and refer to it with $ref: '#/definitions/mySpecialStringType or $ref: '#/definitions/mySpecialIntegerType at many places in the specification. Isn't that the idea behind the descriptions: section? Or should it only be applied for truely new type definitions?

wing328 commented 8 years ago

I don't think that's supported by codegen at the moment and would suggest you to use just string or integer for the time being.

ggood commented 8 years ago

Here's another example. I've specialized a string property (MACAddress) with a pattern validator, but the generated java class is just created as a bare class:

public class MACAddress

and no constructor that allows me to pass in a string. I can work around that, as you mention, by "inlining" the pattern, but we have quite a few types with MAC addresses in them.

---
swagger: '2.0'
info:
  version: 0.0.0
  title: Exercise java codegen bug with specialization of primitive types
paths:
  /address-bindings:
    post:
      consumes:
        - application/json
      parameters:
        - in: body
          name: Address Binding
          required: true
          schema:
            $ref: '#/definitions/StaticAddressBinding'
      responses:
        200:
          description: OK
definitions:
  MACAddress:
    type: string
    pattern: "^(([0-9A-Fa-f]{2}[:]){5}([0-9A-Fa-f]{2}))|(([0-9A-Fa-f]{2}[-]){5}([0-9A-Fa-f]{2}))$"
  StaticAddressBinding:
    type: object
    properties:
      mac_address:
        $ref: '#/definitions/MACAddress'
      ip_address:
        type: string
        format: ip
wing328 commented 8 years ago

@ggood we've started adding validation to API client and you can track the progress here: https://github.com/swagger-api/swagger-codegen/issues/2663

ggood commented 8 years ago

@wing328 I think there's a misunderstanding about what issue I'm reporting. Lack of support for pattern validation isn't the problem, it's just part of the use case.

The problem is that I was expecting the properties definition above:

    properties:
      mac_address:
        $ref: '#/definitions/MACAddress'

...to behave as if I'd said:

    properties:
      mac_address:
        type: string
        pattern: "^(([0-9A-Fa-f]{2}[:]){5}([0-9A-Fa-f]{2}))|(([0-9A-Fa-f]{2}[-]){5}([0-9A-Fa-f]{2}))$"

But it looks as if the MACAddress definition was treated as if I'd declared it as:

  MACAddress:
    type: object

instead of as type string.

What I'm trying to accomplish is to create a reusable specialization of a primitive type, in this case a string that matches a particular pattern, so that we implement this consistently across our API.

Does that help clarify?

I can see two solutions, from my newbie viewpoint:

fehguy commented 8 years ago

Hi @ggood, this was a limitation in the underlying parsing logic from the swagger-parser library. What you described should be supported because JSON references are technically allowed to point to non-objects. I'll check on the status on that issue and report back.

fehguy commented 8 years ago

OK, I've just checked and it's not supported. It is in the develop branch which will take a bit to add to codegen. Until it's in, will leave this ticket open.

ggood commented 8 years ago

@fehguy, thanks for confirming that this should work, according to the spec. I have a workaround, so this is not all that urgent.

stevecookform3 commented 7 years ago

Looks like this was fixed in the parser some time ago, however the spring codegen is still generating invalid code in this case. e.g. I have a model that I'd like to use all over the place

  IsoCurrencyCode:
    type: string
    pattern: '^[A-Z]{3}$'
    minLength: 3
    maxLength: 3

Currently the generated spring just generates effectively an empty class (note this doesnt inherit from string or implement any private string member):

public class IsoCurrencyCode   {

  @Override
  public boolean equals(java.lang.Object o) {
    if (this == o) {
      return true;
    }
    if (o == null || getClass() != o.getClass()) {
      return false;
    }
    return true;
  }

  @Override
  public int hashCode() {
    return Objects.hash();
  }

  @Override
  public String toString() {
    StringBuilder sb = new StringBuilder();
    sb.append("class IsoCurrencyCode {\n");
    sb.append("}");
    return sb.toString();
  }

  private String toIndentedString(java.lang.Object o) {
    if (o == null) {
      return "null";
    }
    return o.toString().replace("\n", "\n    ");
  }
}
stevecookform3 commented 7 years ago

Looks like this is a duplicate:

3483

Boscop commented 7 years ago

It seems to be still not fixed in the java codegen on http://editor2.swagger.io/

wing328 commented 7 years ago

Swagger Editor (old or new) is not using the latest master.

Please try to build the JAR based on the latest master and see if the issue still persists.

dentych commented 7 years ago

I just tried the latest v2.3.0-SNAPSHOT from master...

Swagger file:

swagger: "2.0"
info:
  description: "Stupid API"
  version: "1.0.0"
  title: "Swagger API"
host: "localhost"
basePath: "/"
schemes:
  - "http"
paths:
  '/what':
    get:
      summary: ok
      responses:
        200:
          description: "Success"
          schema:
            $ref: '#/definitions/SimpleRef'
definitions:
  SimpleRef:
    type: object
    properties:
      someDate:
        $ref: '#/definitions/SimpleDate'
  SimpleDate:
    type: string
    format: date-time

This generates a single class "SimpleRef" which contains:

public class SimpleRef   {

  private @Valid SimpleDate someDate = null;
  ...
}
jbx1 commented 6 years ago

Although the related ticket 243 is now closed the issue described is still present for swagger-codegen 2.3.1 in spring-boot.

I have the same issue with this example:

definitions:
  RegisterCustomerDTO:
    type: object
    properties:
      mobile:
        $ref: '#/definitions/Mobile'
      email:
        type: string
        pattern: '^[a-zA-Z0-9.!#$%&’*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$'
        description: Email address of the customer.

  Mobile:
    type: string
    minLength: 8
    maxLength: 16
    pattern: '^\\+[1-9][0-9]{3,14}$'
    description: The mobile number in E.164 format.

While it correctly creates the mobile property as a String, none of the validations or descriptions are copied. The inline ones for email are done correctly.

 /**
   * Get mobile
   * @return mobile
  **/
  @ApiModelProperty(required = true, value = "")
  @NotNull
  public String getMobile() {
    return mobile;
  }

  /**
   * Email address of the customer.
   * @return email
  **/
  @ApiModelProperty(value = "Email address of the customer.")
  @Pattern(regexp="^[a-zA-Z0-9.!#$%&’*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\\.[a-zA-Z0-9-]+)*$") 
  public String getEmail() {
    return email;
  }
abhijith-prabhakar commented 6 years ago

+1.
I found the issue is how the mapping is done internally. These simple types are mapped as ModelProperty which does have pattern, minLength or maxLength property. These should be mapped to respective type, so that these validations can be added. In above case, this should be StringProperty

abhijith-prabhakar commented 6 years ago

Digging this further it looks like it is only gathering as alias in this method getAllAliases(Map<String, Model> allDefinitions). Once RefProperty is resolved to be a primitive type, it should handled as that type of property