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

Duplicated schemas in parsed result #1081

Closed
ackintosh opened this issue Apr 20, 2019 · 11 comments
Closed

Duplicated schemas in parsed result #1081

ackintosh opened this issue Apr 20, 2019 · 11 comments

Comments

@ackintosh
Copy link
Contributor

Related issue: OpenAPITools/openapi-generator#2701


Specs that reproduces the issue

  • openapi.yaml
openapi: 3.0.0
info:
  title: Common Data Types
  version: "1.0"
paths:
  /subscriptions:
    get:
      summary: read all of the active subscriptions for the app.
      operationId: getSubscriptionsById
      parameters:
        - name: appId
          in: path
          description: App ID
          required: true
          schema:
            type: integer
            format: int64
      responses:
        '200':
          description: OK (Successful)
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/Subscription'
        '500':
          $ref: './common.yaml#/components/responses/E500'  
components:
  schemas:
      ProblemDetails:
        type: object
        properties:
          title:
            type: string
            description: A short, human-readable summary of the problem
          status:
            type: integer
            description: The HTTP status code for this occurrence of the problem.
      Subscription:
        type: string
  • common.yaml
openapi: 3.0.0
info:
  title: Common Data Types
  version: "1.0"
paths: {}
components:
  responses:
    E500:
      description: Server Error
      content:
        application/json:
          schema:
            $ref: './openapi.yaml#/components/schemas/ProblemDetails'

Test codes which shows the issue

  • OpenAPIV3ParserTest.java
    @Test
    public void fooBar() {
        String location = "src/test/resources/xxx/openapi.yaml";
        OpenAPIV3Parser parser = new OpenAPIV3Parser();

        OpenAPI result = parser.read(location);
    }

The result has a duplicated schema ProblemDetails_2.

image

@r-sreesaran
Copy link
Contributor

@ackintosh can you add a failing test case.

ackintosh added a commit to ackintosh/swagger-parser that referenced this issue May 18, 2019
@ackintosh
Copy link
Contributor Author

@r-sreesaran Thanks for your reply. I've filed the PR to add a failing test case.
#1102

@r-sreesaran
Copy link
Contributor

@ackintosh This may be valid as "Problem Details" is referenced from external open api document. It needs to be duplicated and added in the object.

Consider a case where there are two schema having same name but of different type. Then in that case if we remove duplicate, then one schema will be lost.

Here the Problems Details are present in both documents and have different definitions.
If we remove the duplicate entries then only one schema will be present. But we need both.

info:
  title: Common Data Types
  version: "1.0"
paths:
  /subscriptions:
    get:
      summary: read all of the active subscriptions for the app.
      operationId: getSubscriptionsById
      parameters:
        - name: appId
          in: path
          description: App ID
          required: true
          schema:
            type: integer
            format: int64
      responses:
        '200':
          description: OK (Successful)
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/Subscription'
        '500':
          $ref: './issue-1108-1.yaml#/components/responses/E500'
        '501':
          $ref: '#/components/schemas/ProblemDetails' 
components:
  schemas:
    ProblemDetails:
      type: object
      properties:
        title:
          type: string
          description: A short, human-readable summary of the problem
        status:
          type: integer
          description: The HTTP status code for this occurrence of the problem.
    Subscription:
      type: string

issue-1108-.yaml

openapi: 3.0.0
info:
  title: Common Data Types
  version: "1.0"
paths: {}
components:
  responses:
    E500:
      description: Server Error
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/ProblemDetails'
  schemas:
   ProblemDetails:
      type: object         

@gracekarina @ymohdriz please do share your comments on this issue.

@gracekarina
Copy link
Contributor

gracekarina commented Jun 6, 2019

Hi @r-sreesaran thanks for the reply, the case @ackintosh presents it's the same schema, so it should not be duplicating it. The case you present above it's ok for "duplicating" because there are different schemas. Sorry it's taking long for me to check this, I hope to have a fix for this week.

@ymohdriz
Copy link
Contributor

ymohdriz commented Jun 7, 2019

@r-sreesaran Issue here is a new schema (ProblemDetails_2) is introduced in the components which is incorrect. We need to fix it

@r-sreesaran
Copy link
Contributor

@gracekarina Few clarifications about the duplication part from the example i have mentioned. As per Open API specification no two schema definitions can have the same name. Since these references are added from the same file this does qualify them to be treated as schemas from same document and in such case the open api document becomes invalid as there two schemas with name name.

As of now we are not handling the references like wise.
Should this be done this way or in existing way where a different name is created and then the schema is added using the created name.

@jphorx
Copy link

jphorx commented Jun 25, 2019

We've seen this issue as well using version 2.0.12. These two swagger files can be used to reproduce it if needed and will produce two models for flowDirection.

spec1.yaml

openapi: "3.0.0"
info:
  version: 1.0.0
  title: spec1
paths:
  /flow:
    get:
      responses:
        '200':
          description: Expected response to a valid request
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/FlowInformation'
components:            
  schemas:
    FlowDirection:
      enum:
        - DOWNLINK
        - UPLINK
        - BIDIRECTIONAL
        - UNSPECIFIED
    FlowInformation:
      type: object
      properties:
        flowDescription:
          $ref: 'spec2.yaml#/components/schemas/FlowDescription'
        ethFlowDescription:
          $ref: 'spec2.yaml#/components/schemas/EthFlowDescription'
        flowDirection:
          $ref: '#/components/schemas/FlowDirection'

spec2.yaml

openapi: "3.0.0"
info:
  version: 1.0.0
  title: spec2
components:            
  schemas:
    FlowDescription:
      type: string
    EthFlowDescription:
      type: object
      properties:
        fDesc:
          $ref: '#/components/schemas/FlowDescription'
        fDir:
          $ref: 'spec1.yaml#/components/schemas/FlowDirection'

@binaryunary
Copy link

I'm cross-posting my comment from an issue in openapi-generator as the issue is related to swagger-parser. Original link: OpenAPITools/openapi-generator#2701 (comment).

After playing around with external refs for a bit I discovered that swagger-parser will do some normalization on them, but it seems to happen lazily.

If you have a ref like

somefile.yml#/components/schemas/SomeSchema

then it will become

./somefile.yml#/components/schemas/SomeSchema

inside the parser after the first time this ref is resolved.
Internally SomeSchema is cached when it is first read, when accessed the second time its source is different (somefile.yml != ./somefile.yml) so a number is appended to its name to avoid conflicts.

Prefixing relative paths with ./ solves this problem, but it seems the parser still has problems normalizing the paths relative to the root spec file as having complex relative references (e.g. different levels, from different subdirectories) still break the parser.

@schlagi123
Copy link

The Issue #1292 resolves this issue. The current master branch works.

@gracekarina
Copy link
Contributor

gracekarina commented Apr 1, 2020

yes that is true!, we solve the duplication issue, thanks for pointing this here!
@ackintosh please test and in case the issue is no longer happening please close.
fixed by #1343

@schlagi123
Copy link

I tested the two examples from @ackintosh and @jphorx and they work. Please close the issue.

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

No branches or pull requests

7 participants