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

Consider minLength, maxLength and pattern in referenced schema #45

Merged
merged 1 commit into from May 16, 2018

Conversation

jmini
Copy link
Member

@jmini jmini commented May 15, 2018

Reported by @FatCash on swagger-api/swagger-codegen#8001

OAS2 example:

definitions:
  SomeObj:
    type: object
    properties:
      nick:
        $ref: "#/definitions/NickName"
  NickName:
    type: string
    minLength: 1
    maxLength: 3
    pattern: "^[A-Z]+$"

The solution proposed here is really temporary. When we will work on #20, I think that a lot of the methods in DefaultCodegen will be refactored. For example the OpenAPI instance will be propagated everywhere instead of Map<String, Schema>. This way we can use ModelUtils to resolve referenced Schema consistently.

@jmini
Copy link
Member Author

jmini commented May 15, 2018

@FatCash: I wanted to give my fix a try on a real use case. Which generator are you using? Where do you expect the annotation to be present?

@jmini
Copy link
Member Author

jmini commented May 16, 2018

I have tested it with OAS3:

openapi: 3.0.1
info:
  title: OpenAPI Petstore
  description: This is a sample server Petstore server. For this sample, you can use
    the api key `special-key` to test the authorization filters.
  license:
    name: Apache-2.0
    url: http://www.apache.org/licenses/LICENSE-2.0.html
  version: 1.0.0
servers:
- url: http://petstore.swagger.io/v2
paths:
  /ping:
    post:
      summary: test
      description: test it
      operationId: pingOp
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/SomeObj'
        required: true
      responses:
        200:
          description: OK
          content: {}
  /pong:
    post:
      summary: test
      description: test it
      operationId: pongOp
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/OtherObj'
        required: true
      responses:
        200:
          description: OK
          content: {}
components:
  schemas:
    RefObj:
      type: string
      maxLength: 3
      minLength: 1
      pattern: ^[A-Z]+$
    OtherObj:
      type: object
      properties:
        someProp:
          maxLength: 3
          minLength: 1
          pattern: ^[A-Z]+$
          type: string
    SomeObj:
      type: object
      properties:
        someProp:
          $ref: '#/components/schemas/RefObj'

With OAS2 there is a parser issue:

swagger: '2.0'
info:
  description: 'This is a sample server Petstore server. For this sample, you can use the api key `special-key` to test the authorization filters.'
  version: 1.0.0
  title: OpenAPI Petstore
  license:
    name: Apache-2.0
    url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
host: petstore.swagger.io
basePath: /v2
tags:
  - name: someTag
    description: This is a tag
schemes:
  - http
paths:
  /ping:
    post:
      tags:
        - someTag
      summary: test
      description: 'test it'
      operationId: pingOp
      consumes:
        - application/json
      produces:
        - application/json
      parameters:
        - in: body
          name: body
          required: true
          schema:
            $ref: '#/definitions/SomeObj'
      responses:
        '200':
          description: OK
  /pong:
    post:
      tags:
        - someTag
      summary: test
      description: 'test it'
      operationId: pongOp
      consumes:
        - application/json
      produces:
        - application/json
      parameters:
        - in: body
          name: body
          required: true
          schema:
            $ref: '#/definitions/OtherObj'
      responses:
        '200':
          description: OK
definitions:
  SomeObj:
    type: object
    properties:
      someProp:
        $ref: "#/definitions/RefObj"
  RefObj:
    type: string
    minLength: 1
    maxLength: 3
    pattern: "^[A-Z]+$"
  OtherObj:
    type: object
    properties:
      someProp:
        type: string
        minLength: 1
        maxLength: 3
        pattern: "^[A-Z]+$"

I have filed swagger-api/swagger-parser#708 for this parser issue.

@jmini jmini merged commit 6b80798 into master May 16, 2018
@jmini jmini deleted the min_max_pattern_fix branch May 16, 2018 07:19
@jmini
Copy link
Member Author

jmini commented May 16, 2018

There is an other issue with the pattern. The parser does not read the value with OAS2 and OAS3. see swagger-api/swagger-parser#709

@FatCash
Copy link

FatCash commented May 16, 2018

Hi, thank you for looking into this issue!
For workflow I use io.swagger:swagger-codegen-cli:2.3.1 in my working project. And -l spring.

Tried with the latest openapi-generator with petstore OAS3 and OAS2. Got mixed results:
OAS2, @Pattern and @Size works on non ref objects. On refs, nothing generated :(
OAS3, @Size actually works on both reffed and non reffed objects. @Pattern does not work on either.

jmini added a commit to jmini/openapi-generator that referenced this pull request May 17, 2018
Reverts:
Consider minLength, maxLength and pattern in referenced schema (OpenAPITools#45)
This reverts commit 6b80798.
@jmini jmini mentioned this pull request May 17, 2018
jmini added a commit that referenced this pull request May 17, 2018
Reverts:
Consider minLength, maxLength and pattern in referenced schema (#45)
This reverts commit 6b80798.
@jmini
Copy link
Member Author

jmini commented May 17, 2018

@FatCash: Thank you for your feedback.

My change produce too many side effects, I needed to revert it with #82. Sorry about it.

@FatCash
Copy link

FatCash commented May 23, 2018

Hi, that's unfortunate. Could you try again perhaps now that both #708 and #709 are fixed?

@jmini
Copy link
Member Author

jmini commented May 23, 2018

Well...

Now that the swagger-parser issues are fixed (swagger-api/swagger-parser#708 and swagger-api/swagger-parser#709) we need to integrate them. See discussion in #123.

But since I have reverted the changes, there is in parallel also work to do in the "OpenAPI Generator" project. In order to solve the case correctly we need #83 as prerequisite. Then we can solve your case and uncomment the corresponding tests:

// Assert.assertTrue(cp.isString); //TODO: issue swagger-api/swagger-codegen#8001
Assert.assertEquals(cp.getter, "getSomePropertyWithMinMaxAndPattern");
// Assert.assertEquals(cp.minLength, Integer.valueOf(3)); //TODO: issue swagger-api/swagger-codegen#8001
// Assert.assertEquals(cp.maxLength, Integer.valueOf(10)); //TODO: issue swagger-api/swagger-codegen#8001
// Assert.assertEquals(cp.pattern, "^[A-Z]+$"); //TODO: issue swagger-api/swagger-codegen#8001

I did not forget you ;-)

jmini added a commit to jmini/openapi-generator that referenced this pull request Jun 26, 2018
When they are defined in a in referenced schema
Originally introduced with OpenAPITools#45, reverted with OpenAPITools#82
@jmini
Copy link
Member Author

jmini commented Jun 26, 2018

Thank to the changes made by @wing328 on branch 3.1.x, the issue that this PR tries to solves is now already working with 3.1.0-SNAPSHOT.
=> Due date for 3.1.0 release is next week! (beginning of July 2018).

The Unit-Test that were developed in this PR are restored with PR #401.

@FatCash: maybe you can give 3.1.0-SNAPSHOT a try and report feedback?

jmini added a commit that referenced this pull request Jun 26, 2018
when they are defined in a in referenced schema

Originally introduced with 6b80798 (#45), 
commented with 85090f5 (#82).
@FatCash
Copy link

FatCash commented Jul 13, 2018

Sorry been on vacation. Thank you very much for fixing this issue!
A bit late but I'm positive that it works on:
openapi-generator-cli-3.1.0 works both on 2.0 and 3.0.0 spec
swagger-codegen-cli-2.4.0-SNAPSHOT works on 2.0 spec

@jmini
Copy link
Member Author

jmini commented Jul 13, 2018

@FatCash thank you a lot for the feedback.

@FatCash
Copy link

FatCash commented Jul 13, 2018

Could you close this issue since your fix addresses it:
swagger-api/swagger-codegen#8044

@jmini
Copy link
Member Author

jmini commented Jul 18, 2018

@FatCash: this project is a fork of swagger-api/swagger-codegen due to some disagreements with:

  • some fundamental choices made for Swagger-Codegen-v3.
  • the governance of the project in general.

Swagger-Codegen-v3 and OpenAPI-Generator are both based on Swagger-Codegen-v2, but they now live separately. I never got permission to triage issues in Swagger-Codegen so there is nothing I can do there.

@FatCash
Copy link

FatCash commented Jul 18, 2018

Okay, I see.
Didn't know that Swagger-Codegen-v3 existed. My understanding was that one should use openAPI-Generator instead of swagger-codegen if in need of v3. Thats what I'm doing anyway!
Thank you!

aserkes pushed a commit to aserkes/openapi-generator that referenced this pull request Aug 29, 2022
Some cleanup of properties in docs, pom template and generator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants