Skip to content

Conversation

@ndbroadbent
Copy link
Contributor

@ndbroadbent ndbroadbent commented Sep 6, 2018

I just saw this line in RubyClientCodegenTest.java that said:

// TODO comment out the following until https://github.com/swagger-api/swagger-parser/issues/820 is solved

That issue is now solved, so I've updated the swagger-parser-version to 2.0.4 and uncommented this line.

@jmini
Copy link
Member

jmini commented Sep 6, 2018

Thank you for having a look at this... I am not sure if it will work out of the box, but this is something we need to investigate.

@ndbroadbent
Copy link
Contributor Author

ndbroadbent commented Sep 6, 2018

@jmini I realized that I also needed to update the swagger-parser version to 2.0.4 which includes this fix. Here's the release: https://github.com/swagger-api/swagger-parser/releases/tag/v.2.0.4

EDIT: Actually I'm getting this error:

[ERROR] Failed to execute goal on project openapi-generator: Could not resolve dependencies for project org.openapitools:openapi-generator:jar:3.3.0-SNAPSHOT: Failure to find org.openapitools.swagger.parser:swagger-parser:jar:2.0.4-OpenAPITools.org-1 in https://oss.sonatype.org/content/repositories/snapshots was cached in the local repository, resolution will not be reattempted until the update interval of sonatype-snapshots has elapsed or updates are forced -> [Help 1]

Maybe the package has not been updated in the repository, or I made a mistake.

@jmini
Copy link
Member

jmini commented Sep 6, 2018

@jmini I realized that I also needed to update the swagger-parser version to 2.0.4 which includes this fix. Here's the release: https://github.com/swagger-api/swagger-parser/releases/tag/v.2.0.4

Not exactly. The current 3.3.0-SNAPSHOT (prior to your PR) already contains the fix.

I did a release of org.openapitools.swagger.parser:swagger-parser:2.0.3-OpenAPITools.org-1, and we updated to this version with: #951

The PR to update to io.swagger.parser.v3:swagger-parser:2.0.4 is #787, I hope it will be merged soon (I am waiting for approval).

Sorry if you spend time on figuring out which version you should use. Please revert Swagger-Parser version changes in your PR.


Now to come back to the change you want to make in this PR, I am not sure that the code prepared by @wing328 in the PR #889 to cover the case of a nullable form parameter (OASv2) in the generator code, might not be exactly compatible with the way Swagger-Parser does the conversion.

When form parameters are present in OASv2, a request-body is added in OASv3. Each parameter becomes a property in the Schema of the request body.

Example (OASv2 input):

swagger: "2.0"
info:
  version: "1.0.1"
  title: "OpenAPI-Generator PR 889"
basePath: "/v1"
host: "localhost:8099"
schemes: 
 - http
paths:
  /one:
    get:
      operationId: "op1"
      parameters:
        - name: myparam
          in: formData
          description: first description
          required: false
          type: string
          x-nullable: true
      responses:
        200:
          description: "success"

Output of the Swagger-Parser (OASv3):

openapi: 3.0.1
info:
  title: OpenAPI-Generator PR 889
  version: 1.0.1
servers:
- url: http://localhost:8099/v1
paths:
  /one:
    get:
      operationId: op1
      requestBody:
        content:
          multipart/form-data:
            schema:
              properties:
                myparam:
                  type: string
                  description: first description
                  nullable: true
      responses:
        200:
          description: success

Now if you have 3 parameters (OASv2):

parameters:
  - name: foo
    in: formData
    description: some foo param
    required: false
    type: string
    x-ext: some foo
  - name: bar
    in: formData
    description: some bar param
    required: true
    type: integer
    x-nullable: true
    x-ext: some bar
  - name: baz
    in: formData
    description: some baz param
    required: true
    type: integer
    x-nullable: false
    x-ext: some baz

Output of the Swagger-Parser (OASv3):

requestBody:
content:
    multipart/form-data:
    schema:
        required:
        - bar
        - baz
        properties:
        foo:
            type: string
            description: some foo param
        bar:
            type: integer
            description: some bar param
            nullable: true
        baz:
            type: integer
            description: some baz param
            nullable: false

I do not think that Swagger-Parser can do a better job.

My take is that for the moment, OpenAPI-Generator is only looking for the schema of the multipart/form-data to be nullable or not. But it should look at schema properties level... Only If all properties are nullable, the schema itself is nullable (this is pure theory, I did not investigate it in detail).

@wing328
Copy link
Member

wing328 commented Mar 31, 2019

The test is already uncomment in the master

@wing328 wing328 closed this Mar 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants