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

type field is now placed inside of schema node for referenced objects #145

Closed
wants to merge 1 commit into from

Conversation

AHBaber
Copy link
Contributor

@AHBaber AHBaber commented Nov 25, 2019

I have repeatedly gotten an error when validating objects passed into @doc.consumes(). The error being:

Failed validating 'oneOf' in schema['properties']['paths']['patternProperties']['^/']['properties']['post']['properties']['parameters']['items']:
{'oneOf': [{'$ref': '#/definitions/parameter'},
           {'$ref': '#/definitions/jsonReference'}]}

Specific error instance:

On instance['paths']['/endpointtest']['post']['parameters'][0]:
{'description': 'data object description',
 'in': 'body',
 'name': 'PassedInDataObject',
 'required': True,
 'schema': {'$ref': '#/definitions/PassedInDataObject', 'x-scope': ['']},
 'type': 'object'}

When we look at the documentation for the OpenAPI Specification we can find several examples that show that the "type" field should be inside the "schema" block. Whereas in the example above, the "type" field is at the same level as the "schema" block.

Here are some links to the examples listed in the OpenAPI Specification showing that "type" should be inside of "schema":

Solution:

In the sanic_openapi/swagger.py file, at line 213, we can see that if a field named "$ref" is dedicated, that a "schema" block is created, and the "$ref" is placed inside the "schema" block.

The new code I am submitting also checks to see if a "type" field exists, and then places it inside of the "schema" block as well.

@chenjr0719
Copy link
Member

Hi @AHBaber

Could you provide an example of the issue you mentioned? I can not reproduce the error. And, please tell me which version of Sanic-OpenAPI you are using.

BTW, the examples you provide are using OpenAPI Specification 3.0. But, this project only supports OpenAPI Specification 2.0 and I'm not sure your issue is caused by the version compatibility or not.

@AHBaber
Copy link
Contributor Author

AHBaber commented Nov 25, 2019

@chenjr0719

versions of involved libraries:

  • sanic_openapi: 0.6.0
  • sanic: 19.9.0
  • swagger-parser: 1.0.1
  • swagger-spec-validator: 2.4.3

I was validating the swagger data using swagger parser.

That is using swagger-spec-validator for the actual validation.

Swagger-spec-validator is using what it lists as v2.0/schema.json

@chenjr0719
Copy link
Member

chenjr0719 commented Nov 26, 2019

After using swagger-spec-validator and reading the code base of this project, I think the problem is about this line:

https://github.com/huge-success/sanic-openapi/blob/3cdf8801836a3b31f19d8f9d19b7d27737a4257f/sanic_openapi/doc.py#L152

So, maybe just remove this line is a better solution. What do you think?

And this is the example I used to verify with swagger-spec-validator:

from sanic import Sanic
from sanic.response import json

from sanic_openapi import doc, swagger_blueprint

app = Sanic()
app.blueprint(swagger_blueprint)

app.config["API_HOST"] = "localhost:8000"
app.config["API_BASEPATH"] = "/"
app.config["API_CONTACT_EMAIL"] = "chenjr0719@gmail.com"
app.config["API_LICENSE_NAME"] = "MIT"
app.config[
    "API_LICENSE_URL"
] = "https://github.com/huge-success/sanic-openapi/blob/master/LICENSE"

app.config["API_SECURITY"] = []
app.config["API_SECURITY_DEFINITIONS"] = {}


class TestSchema:
    id = int


@app.post("/test")
@doc.consumes(
    TestSchema, location="body", required=True, content_type="application/json"
)
@doc.response(200, {"message": str}, description="Works")
async def test(request):
    return json({"message": "TEST"})


app.run(debug=True)

@AHBaber
Copy link
Contributor Author

AHBaber commented Nov 27, 2019

I agree that deleting line 152 eliminates the error.

If you could apply that fix, I would deeply appreciate.

Thank you for your time and assistance.

@chenjr0719
Copy link
Member

Unfortunately, I can't fix it by myself. I can only review and merge PR which created by other guys, not including my PR. So, let's fix this issue through this PR. 😃

@AHBaber
Copy link
Contributor Author

AHBaber commented Dec 2, 2019

I apologize for the delay (holiday)

I pulled the the changes from sanic-openapi and merged them into the master branch of my fork.

I have then made the change we discussed above, and checked that into object_fix_2 so as to avoid cluttering the repos with unnecessary commits.

Do you need me to create a new pull request with the new branch?

@chenjr0719
Copy link
Member

Sorry about no response for a couple of days.
I will close this PR first and review your new PR later if you don't mind.

@chenjr0719 chenjr0719 closed this Dec 16, 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.

None yet

2 participants