Yelp / bravado-core

Other
109 stars 98 forks source link

Spec.from_dict() fails to return models in spec.definitions #310

Closed tscheburaschka closed 5 years ago

tscheburaschka commented 5 years ago

Problem

I was trying to evaluate bravado_core for use as a validating deserializer in our project. As we do not expose a true REST-frontend via HTTP but receive JSON-requests via other means, I am more interested in the python models returned from bravado_core. I was trying the following:

import yaml
from bravado_core.spec import Spec

schema_path = 'scripts/petstoreExample_20.yaml'
with open(schema_path, 'r') as f:
    raw_spec = yaml.load(f)
spec = Spec.from_dict(raw_spec)

The file petstoreExample.yaml is identical to the original example under: https://github.com/OAI/OpenAPI-Specification/blob/master/examples/v2.0/yaml/petstore.yaml

I expected to find at least the explicitly referenced Python models Pets and Error in spec.definitions but instead I got an empty dictionary. After a long time of debugging and tracing I narrowed the problem down to a bug in the URI to the 'definitions' part of the schema that was probably caused by an empty origin_url.

Therefore I tried next to give a valid URI for the local file but then I got another error (see: #311).

tscheburaschka commented 5 years ago

With issue #311 gone, I am back at the original situation, where spec.definitions is empty, even when explicitly specifying the origin_url argument. Like so:

from pathlib import Path
import yaml
from bravado_core.spec import Spec

schema_path = Path.cwd() / 'scripts' / 'petstoreExample_20.yaml'
with open(schema_path, 'r') as f:
    raw_spec = yaml.load(f)
spec = Spec.from_dict(raw_spec, origin_url=schema_path.as_uri())
tscheburaschka commented 5 years ago

pip freeze:

asn1crypto==0.24.0
atomicwrites==1.2.1
attrs==18.2.0
azure-common==1.1.14
azure-nspkg==2.0.0
azure-servicebus==0.21.1
azure-storage-blob==1.3.1
azure-storage-common==1.3.0
azure-storage-nspkg==3.0.0
azure-storage-queue==1.3.0
backcall==0.1.0
bravado==10.2.1
bravado-core==5.10.0
certifi==2018.10.15
cffi==1.11.5
chardet==3.0.4
colorama==0.3.9
coverage==4.5.1
cryptography==2.3.1
cycler==0.10.0
Cython==0.29
decorator==4.3.0
flake8==3.6.0
idna==2.7
ipython==6.5.0
ipython-genutils==0.2.0
isodate==0.6.0
jedi==0.12.1
JSON-minify==0.3.0
jsonref==0.2
jsonschema==2.6.0
kiwisolver==1.0.1
marshmallow==3.0.0b20
matplotlib==2.2.3
mccabe==0.6.1
mkl-fft==1.0.6
mkl-random==1.0.1
mockito==1.1.1
monotonic==1.5
more-itertools==4.3.0
msgpack-python==0.5.6
networkx==2.2
numpy==1.15.4
odata==0.3
pandas==0.23.4
parso==0.3.1
pickleshare==0.7.4
pluggy==0.7.1
ply==3.11
prompt-toolkit==1.0.15
py==1.6.0
py-cpuinfo==4.0.0
pycodestyle==2.4.0
pycparser==2.18
pyflakes==2.0.0
Pygments==2.2.0
PyJWT==1.6.4
pyparsing==2.3.0
pytest==3.8.0
pytest-benchmark==3.1.1
pytest-cov==2.6.0
pytest-mockito==0.0.4
python-dateutil==2.7.5
pytz==2018.7
PyYAML==3.13
requests==2.19.1
requests-mock==1.5.2
rfc3987==1.3.8
scipy==1.1.0
simplegeneric==0.8.1
simplejson==3.16.0
sip==4.19.13
six==1.11.0
smop==0.41
strict-rfc3339==0.7
swagger-spec-validator==2.4.1
tornado==5.1.1
traitlets==4.3.2
urllib3==1.23
wcwidth==0.1.7
webcolors==1.8.1
wincertstore==0.2
macisamuele commented 5 years ago

@tscheburaschka thanks for the result of pip freeze, it was not helpful as the issue is a different one.

I would suggest you to read more about how bravado discovers models. The documentation says that all the models defined into #/definitions will be discovered and all the models that will have x-model vendor extension defined.

The referred specs have Pet, Pets and Error defined in #/definitions and you're right on the fact that those models should be in spec.definitions. They are not there because bravado-core generates the model only if the type is object and the referred specs does not have it

definitions:
 Pet:
  required: [id, name]
  properties:
   id: {type: integer, format: int64}
   name: {type: string}
   tag: {type: string}
  type: object  # This line is present in the specs, if you add it you'll find the object

As in general you could not force the service providing the specs, bravado-core, exposes via a configuration that allows to default type to object if missing. The configuration key is default_type_to_object (documentation).

How to validate the theory

import requests
import yaml
from bravado_core.spec import Spec
spec_url = 'https://raw.githubusercontent.com/OAI/OpenAPI-Specification/master/examples/v2.0/yaml/petstore.yaml'
raw_spec = yaml.load(requests.get(spec_url).content)
spec_no_config = Spec.from_dict(raw_spec, origin_url=spec_url)
spec_with_config = Spec.from_dict(raw_spec, origin_url=spec_url, config={'default_type_to_object': True})
print('spec_no_config.definitions =', spec_no_config.definitions)
print('spec_with_config.definitions =', spec_with_config.definitions)

The output is

spec_no_config.definitions = {}
spec_with_config.definitions = {'Error': <class 'abc.Error'>, 'Pet': <class 'abc.Pet'>}
tscheburaschka commented 5 years ago

Thank you for the clarification. I was not aware that omitting the type is still valid OpenAPI spec. On that note (but off topic in a way): Does the bravado-core team have plans to support OpenAPI v3 at some point in time?