Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
h2m opened this issue Mar 4, 2016 · 17 comments

Comments

@h2m
Copy link

h2m commented Mar 4, 2016

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
Copy link
Contributor

wing328 commented Mar 6, 2016

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

@h2m
Copy link
Author

h2m commented Mar 7, 2016

@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
Copy link
Contributor

wing328 commented Mar 7, 2016

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
Copy link
Contributor

ggood commented May 6, 2016

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
Copy link
Contributor

wing328 commented May 7, 2016

@ggood we've started adding validation to API client and you can track the progress here: #2663

@ggood
Copy link
Contributor

ggood commented May 11, 2016

@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:

  • When a $ref points to something that's not type: object, just inline the referenced thing, or
  • Create a class that inherits from the language-specific type that corresponds to the JSON-Schema primitive type (String, in this case). Any pattern validation could then be encapsulated in that class.

@fehguy
Copy link
Contributor

fehguy commented May 11, 2016

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
Copy link
Contributor

fehguy commented May 12, 2016

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
Copy link
Contributor

ggood commented May 15, 2016

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

@stevecookform3
Copy link
Contributor

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
Copy link
Contributor

Looks like this is a duplicate:

#3483

@Boscop
Copy link

Boscop commented May 28, 2017

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

@wing328
Copy link
Contributor

wing328 commented May 29, 2017

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
Copy link

dentych commented Sep 4, 2017

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;
  ...
}

@wing328 wing328 added this to the Future milestone Sep 5, 2017
@jbx1
Copy link

jbx1 commented Mar 18, 2018

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
Copy link

abhijith-prabhakar commented Mar 30, 2018

+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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants