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

fixes for issue #799 returning ref as internal #801

Merged
merged 3 commits into from Aug 14, 2018
Merged

Conversation

gracekarina
Copy link
Member

@gracekarina gracekarina commented Aug 13, 2018

fixes for issue #799 returning ref as internal

@gracekarina gracekarina changed the title fixes for issue#799 returning ref as internal fixes for issue #799 returning ref as internal Aug 13, 2018
@jmini
Copy link
Contributor

jmini commented Aug 14, 2018

This does not solve my problem.

My use case is the conversation to an OpenAPI v3 model. Here is my test procedure:

git fetch
git checkout origin/issue_799_master -B issue_799_master
//replace 1.0.38-SNAPSHOT to 1.0.38.pr801-SNAPSHOT to be sure to use the version of this PR.
mvn clean install

Then in the Main class:

OpenAPIParser openApiParser = new OpenAPIParser();
ParseOptions options = new ParseOptions();
options.setResolve(true);
options.setFlatten(true);

OpenAPI openAPI = openApiParser.readLocation(inputSpec, null, options).getOpenAPI();
String string = Yaml.mapper().writerWithDefaultPrettyPrinter().writeValueAsString(openAPI);

System.out.println(string);

Using v1beta3.json as input spec for inputSpec

With this dependencies:

    <dependency>
      <groupId>io.swagger.parser.v3</groupId>
      <artifactId>swagger-parser</artifactId>
      <version>2.0.2</version>
    </dependency>
    <dependency>
      <groupId>io.swagger</groupId>
      <artifactId>swagger-parser</artifactId>
      <version>1.0.38.pr801-SNAPSHOT</version>
    </dependency>

Still the same output as the one described in #799. I get:

    v1beta3.Binding:
      required:
      - target
      type: object
      properties:
        apiVersion:
          type: string
          description: version of the schema the object should have
        kind:
          type: string
          description: kind of object, in CamelCase; cannot be updated
        metadata:
          $ref: v1beta3.ObjectMeta
        target:
          $ref: v1beta3.ObjectReference

Instead of:

    v1beta3.Binding:
      required:
      - target
      type: object
      properties:
        apiVersion:
          type: string
          description: version of the schema the object should have
        kind:
          type: string
          description: kind of object, in CamelCase; cannot be updated
        metadata:
          $ref: '#/components/schemas/v1beta3.ObjectMeta'
        target:
          $ref: '#/components/schemas/v1beta3.ObjectReference'

which was the case with 1.0.36

@jmini
Copy link
Contributor

jmini commented Aug 14, 2018

This PR tries to fix conversation of from "swaggerVersion": "1.2" to "swagger" : "2.0", so I did an other test just on this part (maybe something else is requested for my usecase described in #799).

I have executed your test:
io.swagger.converter.LegacyConverterTest.testIssue799()

In this test you have Json.prettyPrint(swagger); so I looked at the sysout output and the content to http://editor.swagger.io/

I see 2 semantic errors:

Semantic error at definitions.v1beta3.ObjectMeta.properties.annotations.$ref
$refs must reference a valid location in the document
Jump to line 149
Semantic error at definitions.v1beta3.ObjectMeta.properties.labels.$ref
$refs must reference a valid location in the document
Jump to line 165

So I guess that the v1.2 to v2.0 feature is also not working in all cases.

PS: Thank you a lot for looking into this issue!


Swagger swagger = converter.read("src/test/resources/specs/v1_2/issue799.json");
Assert.assertEquals( swagger.getPaths().get("/api/v1beta3/namespaces/{namespaces}/bindings").getPost().getResponses().get("200").getResponseSchema().getReference(), "#/definitions/v1beta3.Binding");
Json.prettyPrint(swagger);
Copy link
Contributor

@jmini jmini Aug 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add following assertions before the Json.prettyPrint(swagger) line:

        Parameter bodyParameter = swagger.getPaths().get("/api/v1beta3/namespaces/{namespaces}/bindings").getPost().getParameters().get(1);
        Assert.assertEquals( bodyParameter.getName(), "body");
        Assert.assertTrue( bodyParameter instanceof BodyParameter);
        Assert.assertEquals( ((BodyParameter)bodyParameter).getSchema().getReference(), "#/definitions/v1beta3.Binding");
        Assert.assertEquals( swagger.getPaths().get("/api/v1beta3/namespaces/{namespaces}/componentstatuses/{name}").getGet().getResponses().get("200").getResponseSchema().getReference(), "#/definitions/v1beta3.ComponentStatus");
        Assert.assertEquals( swagger.getPaths().get("/api/v1beta3/namespaces/{namespaces}/componentstatuses").getGet().getResponses().get("200").getResponseSchema().getReference(), "#/definitions/v1beta3.ComponentStatusList");
        Property conditionsProperty = swagger.getDefinitions().get("v1beta3.ComponentStatus").getProperties().get("conditions");
        Assert.assertTrue( conditionsProperty instanceof ArrayProperty);
        Property items = ((ArrayProperty)conditionsProperty).getItems();
        Assert.assertTrue( items instanceof RefProperty);
        Assert.assertEquals( ((RefProperty)items).get$ref(), "#/definitions/v1beta3.ObjectReference");
        Property metadataProperty = swagger.getDefinitions().get("v1beta3.ComponentStatus").getProperties().get("metadata");
        Assert.assertTrue( metadataProperty instanceof RefProperty);
        Assert.assertEquals( ((RefProperty)metadataProperty).get$ref(), "#/definitions/v1beta3.ObjectMeta");

@jmini
Copy link
Contributor

jmini commented Aug 14, 2018

This looks good to me, you could extend the unit test as I have suggested.

With this PR the regression introduced with 1.0.37 is fixed, the result is similar to what I get with 1.0.36

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.

None yet

2 participants