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.

Sign up for GitHub

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

Jump to bottom

Get models from definitions of referred files #273

Merged
merged 3 commits into from
May 30, 2018
Merged

Get models from definitions of referred files #273

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
877850a
Add support to detect models from definitions on referred files
macisamuele May 26, 2018
f784077
Update flattened specs file and tests
macisamuele May 26, 2018
bf3f31f
Update logic to extract un-processed URIs
macisamuele May 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 57 additions & 22 deletions bravado_core/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,28 +770,63 @@ def descend(fragment, json_reference=None):

def _run_post_processing(spec):
visited_models = {}
# Discover all the models
_post_process_spec(
spec_dict=spec.spec_dict,
spec_resolver=spec.resolver,
on_container_callbacks=[
functools.partial(
_tag_models,
visited_models=visited_models,
swagger_spec=spec,
),
functools.partial(
_bless_models,
visited_models=visited_models,
swagger_spec=spec,
),
functools.partial(
_collect_models,
models=spec.definitions,
swagger_spec=spec,
),
],
)

def _call_post_process_spec(spec_dict):
# Discover all the models in spec_dict
_post_process_spec(
spec_dict=spec_dict,
spec_resolver=spec.resolver,
on_container_callbacks=[
functools.partial(
_tag_models,
visited_models=visited_models,
swagger_spec=spec,
),
functools.partial(
_bless_models,
visited_models=visited_models,
swagger_spec=spec,
),
functools.partial(
_collect_models,
models=spec.definitions,
swagger_spec=spec,
),
],
)

# Post process specs to identify models
_call_post_process_spec(spec.spec_dict)

for additional_file in _get_referred_files(spec):
# Post process each referenced specs to identify models in definitions of linked files
with spec.resolver.in_scope(additional_file):
_call_post_process_spec(
spec.resolver.store[additional_file],
)


def _get_referred_files(swagger_spec):
"""
:type swagger_spec: bravado_core.spec.Spec
"""
processed_files = {
uri
for uri in swagger_spec.resolver.store
if uri == swagger_spec.origin_url or re.match(r'http://json-schema.org/draft-\d+/schema', uri)
}

while True:
referred_files = [
uri
for uri in swagger_spec.resolver.store
if uri not in processed_files
]
if referred_files and referred_files[0]:
processed_files.add(referred_files[0])
yield referred_files[0]
else:
return
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be doing a lot of duplicate work, recalculating referred_files after yeach yield... maybe I'm missing something, but wouldn't it be easier to write the function like this:

def _get_referred_files(swagger_spec):
    for uri in swagger_spec.resolver.store:
        if uri != swagger_spec.origin_url and not re.match(r'http://json-schema.org/draft-\d+/schema', uri):
            yield uri

If, on the other hand, you function needs to deal with the fact that swagger_spec.resolver.store will be different after each yield then I think you need to refactor your code; using yield doesn't seem to be right approach. I'd expect the caller to own processed_files and pass that in on every call (since that contains the state). You'd then return from this function once you find the first unprocessed_file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As you pointed out swagger_spec.resolver.store could change during the iteration.
I've already created a specific example of this on the tests (check test-data/2.0/multi-file-recursive/aux.json#/not_used_remote_reference/properties/random_number which points to test-data/2.0/multi-file-recursive/aux_2.json#/definitions/random_integer)
In such case the new file aux_2.json get's added only after aux.json is fully processed.

I'll change the method implementation to make it a bit more efficient by scanning all the list of files in the store and redoing it if there are new files at the end of the scan



def model_discovery(swagger_spec):
Expand Down
5 changes: 5 additions & 0 deletions test-data/2.0/multi-file-recursive/aux.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
},
"not_used_remote_reference": {
"type": "object",
"properties": {
"random_number": {
"$ref": "aux_2.json#/definitions/random_integer"
}
},
"x-model": "not_used_remote_reference"
}
},
Expand Down
21 changes: 21 additions & 0 deletions test-data/2.0/multi-file-recursive/aux_2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"definitions": {
"not_referenced_models_in_not_direcly_linked_file": {
"type": "object"
},
"random_integer": {
"type": "object",
"properties": {
"value": {
"type": "integer"
},
"min-value": {
"type": "integer"
},
"max-value": {
"type": "integer"
}
}
}
}
}
29 changes: 24 additions & 5 deletions test-data/2.0/multi-file-recursive/flattened-multi-file-recursive-spec.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
{
"definitions": {
"lfile:aux.json|..definitions..not_used_remote_reference": {
"properties": {
"random_number": {
"$ref": "#/definitions/lfile:aux_2.json|..definitions..random_integer"
}
},
"type": "object",
"x-model": "not_used_remote_reference"
},
Expand All @@ -19,6 +24,25 @@
"type": "object",
"x-model": "ping"
},
"lfile:aux_2.json|..definitions..not_referenced_models_in_not_direcly_linked_file": {
"type": "object",
"x-model": "not_referenced_models_in_not_direcly_linked_file"
},
"lfile:aux_2.json|..definitions..random_integer": {
"properties": {
"max-value": {
"type": "integer"
},
"min-value": {
"type": "integer"
},
"value": {
"type": "integer"
}
},
"type": "object",
"x-model": "random_integer"
},
"lfile:swagger.json|..definitions..model_with_allOf_recursive": {
"allOf": [
{
Expand All @@ -39,11 +63,6 @@
"x-model": "model_with_allOf_recursive"
},
"lfile:swagger.json|..definitions..not_used": {
"properties": {
"not_used_remote_reference": {
"$ref": "#/definitions/lfile:aux.json|..definitions..not_used_remote_reference"
}
},
"type": "object",
"x-model": "not_used"
},
Expand Down
5 changes: 0 additions & 5 deletions test-data/2.0/multi-file-recursive/swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@
},
"not_used": {
"type": "object",
"properties": {
"not_used_remote_reference": {
"$ref": "aux.json#/definitions/not_used_remote_reference"
}
},
"x-model": "not_used"
},
"pong": {
Expand Down
2 changes: 1 addition & 1 deletion test-data/2.0/multi-file-specs-with-no-x-model/flattened-multi-file-with-no-xmodel.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"definitions": {
"lfile:aux.json|..definitions..referenced_object": {
"type": "object",
"x-model": "lfile:aux.json|..definitions..referenced_object"
"x-model": "referenced_object"
},
"lfile:swagger.json|..definitions..object": {
"properties": {
Expand Down
12 changes: 8 additions & 4 deletions tests/model/collect_models_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,22 @@ def pet_model_spec():
}


def test_simple(minimal_swagger_dict, pet_model_spec):
@pytest.mark.parametrize(
'origin_url', [None, 'origin_url'],
)
def test_simple(minimal_swagger_dict, pet_model_spec, origin_url):
minimal_swagger_dict['definitions']['Pet'] = pet_model_spec
swagger_spec = Spec(minimal_swagger_dict)
swagger_spec = Spec(minimal_swagger_dict, origin_url=origin_url)
models = {}
json_reference = '{}#/definitions/Pet'.format(origin_url or '')
_collect_models(
minimal_swagger_dict['definitions']['Pet'],
models=models,
swagger_spec=swagger_spec,
json_reference='#/definitions/Pet/x-model',
json_reference=json_reference + '/x-model',
)
assert 'Pet' in models
assert models['Pet']._json_reference == '#/definitions/Pet'
assert models['Pet']._json_reference == json_reference


def test_no_model_type_generation_for_not_object_type(minimal_swagger_dict):
Expand Down