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

removes passing reference paths in nested field metadata #363

Merged
merged 2 commits into from
Feb 3, 2019
Merged

removes passing reference paths in nested field metadata #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2065466
removes passing reference paths in nested field metadata
Bangertm Jan 25, 2019
88eba0f
changes comparision to an instance
Bangertm Jan 28, 2019
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
12 changes: 5 additions & 7 deletions apispec/ext/marshmallow/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ class UserSchema(Schema):
from __future__ import absolute_import
import warnings

import marshmallow

from apispec import BasePlugin
from .common import resolve_schema_cls, resolve_schema_instance, make_schema_key
from .common import resolve_schema_instance, make_schema_key
from .openapi import OpenAPIConverter


Expand Down Expand Up @@ -82,11 +80,11 @@ def resolve_parameters(self, parameters):
resolved = []
for parameter in parameters:
if not isinstance(parameter.get('schema', {}), dict):
schema_cls = resolve_schema_cls(parameter['schema'])
if issubclass(schema_cls, marshmallow.Schema) and 'in' in parameter:
schema_instance = resolve_schema_instance(parameter['schema'])
if 'in' in parameter:
del parameter['schema']
resolved += self.openapi.schema2parameters(
schema_cls,
schema_instance,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is to prevent a schema like SelfReferencingSchema passed as a query string parameter from throwing an error due to unbound nested self referencing fields. It is not valid to have a self referencing schema in that location, but at least it doesn't throw an error.

default_in=parameter.pop('in'), **parameter
)
continue
Expand Down Expand Up @@ -157,7 +155,7 @@ def schema_helper(self, name, schema=None, **kwargs):
self.warn_if_schema_already_in_spec(schema_key)
self.openapi.refs[schema_key] = name

json_schema = self.openapi.schema2jsonschema(schema_instance, name=name)
json_schema = self.openapi.schema2jsonschema(schema_instance)

return json_schema

Expand Down
60 changes: 15 additions & 45 deletions apispec/ext/marshmallow/openapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ def metadata2properties(self, field):
}
return ret

def field2property(self, field, use_refs=True, name=None):
def field2property(self, field):
"""Return the JSON Schema property definition given a marshmallow
:class:`Field <marshmallow.fields.Field>`.

Expand All @@ -369,8 +369,6 @@ def field2property(self, field, use_refs=True, name=None):
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#schemaObject

:param Field field: A marshmallow field.
:param bool use_refs: Use JSONSchema ``refs``.
:param str name: The definition name, if applicable, used to construct the $ref value.
:rtype: dict, a Property Object
"""
ret = {}
Expand All @@ -393,43 +391,18 @@ def field2property(self, field, use_refs=True, name=None):
# marshmallow>=2.7.0 compat
field.metadata.pop('many', None)

is_unbound_self_referencing = not getattr(field, 'parent', None) and field.nested == 'self'
if (use_refs and 'ref' in field.metadata) or is_unbound_self_referencing:
if 'ref' in field.metadata:
ref_name = field.metadata['ref']
else:
if not name:
raise ValueError(
'Must pass `name` argument for self-referencing Nested fields.',
)
# We need to use the `name` argument when the field is self-referencing and
# unbound (doesn't have `parent` set) because we can't access field.schema
ref_path = self.get_ref_path()
ref_name = '#/{ref_path}/{name}'.format(ref_path=ref_path, name=name)
ref_schema = {'$ref': ref_name}
if field.many:
ret['type'] = 'array'
ret['items'] = ref_schema
else:
if ret:
ret.update({'allOf': [ref_schema]})
else:
ret.update(ref_schema)
schema_dict = self.resolve_nested_schema(field.schema)
if ret and '$ref' in schema_dict:
ret.update({'allOf': [schema_dict]})
else:
schema_dict = self.resolve_nested_schema(field.schema)
if ret and '$ref' in schema_dict:
ret.update({'allOf': [schema_dict]})
else:
ret.update(schema_dict)
ret.update(schema_dict)
elif isinstance(field, marshmallow.fields.List):
ret['items'] = self.field2property(
field.container, use_refs=use_refs,
)
ret['items'] = self.field2property(field.container)
elif isinstance(field, marshmallow.fields.Dict):
if MARSHMALLOW_VERSION_INFO[0] >= 3:
if field.value_container:
ret['additionalProperties'] = self.field2property(
field.value_container, use_refs=use_refs,
field.value_container,
)

return ret
Expand Down Expand Up @@ -502,7 +475,7 @@ def schema2parameters(

return self.fields2parameters(fields, default_in=default_in)

def fields2parameters(self, fields, use_refs=True, default_in='body'):
def fields2parameters(self, fields, default_in='body'):
"""Return an array of OpenAPI parameters given a mapping between field names and
:class:`Field <marshmallow.Field>` objects. If `default_in` is "body", then return an array
of a single parameter; else return an array of a parameter for each included field in
Expand All @@ -526,7 +499,6 @@ def fields2parameters(self, fields, use_refs=True, default_in='body'):
param = self.field2parameter(
field_obj,
name=self._observed_name(field_obj, field_name),
use_refs=use_refs,
default_in=default_in,
)
if self.openapi_version.major < 3 and param['in'] == 'body' and body_param is not None:
Expand All @@ -540,14 +512,14 @@ def fields2parameters(self, fields, use_refs=True, default_in='body'):
parameters.append(param)
return parameters

def field2parameter(self, field, name='body', use_refs=True, default_in='body'):
def field2parameter(self, field, name='body', default_in='body'):
"""Return an OpenAPI parameter as a `dict`, given a marshmallow
:class:`Field <marshmallow.Field>`.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#parameterObject
"""
location = field.metadata.get('location', None)
prop = self.field2property(field, use_refs=use_refs)
prop = self.field2property(field)
return self.property2parameter(
prop,
name=name,
Expand Down Expand Up @@ -605,7 +577,7 @@ def property2parameter(
ret['schema'] = prop
return ret

def schema2jsonschema(self, schema, **kwargs):
def schema2jsonschema(self, schema):
"""Return the JSON Schema Object for a given marshmallow
:class:`Schema <marshmallow.Schema>`. Schema may optionally provide the ``title`` and
``description`` class Meta options.
Expand Down Expand Up @@ -641,15 +613,15 @@ class Meta:
# }
# }

:param Schema schema: A marshmallow Schema instance or a class object
:param Schema schema: A marshmallow Schema instance
:rtype: dict, a JSON Schema Object
"""
fields = get_fields(schema)
Meta = getattr(schema, 'Meta', None)
partial = getattr(schema, 'partial', None)
ordered = getattr(schema, 'ordered', False)

jsonschema = self.fields2jsonschema(fields, partial=partial, ordered=ordered, **kwargs)
jsonschema = self.fields2jsonschema(fields, partial=partial, ordered=ordered)

if hasattr(Meta, 'title'):
jsonschema['title'] = Meta.title
Expand All @@ -664,7 +636,7 @@ class Meta:

return jsonschema

def fields2jsonschema(self, fields, ordered=False, partial=None, use_refs=True, name=None):
def fields2jsonschema(self, fields, ordered=False, partial=None):
"""Return the JSON Schema Object given a mapping between field names and
:class:`Field <marshmallow.Field>` objects.

Expand All @@ -682,9 +654,7 @@ def fields2jsonschema(self, fields, ordered=False, partial=None, use_refs=True,

for field_name, field_obj in iteritems(fields):
observed_field_name = self._observed_name(field_obj, field_name)
property = self.field2property(
field_obj, use_refs=use_refs, name=name,
)
property = self.field2property(field_obj)
jsonschema['properties'][observed_field_name] = property

if field_obj.required:
Expand Down
4 changes: 0 additions & 4 deletions tests/schemas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@ class PatternedObjectSchema(Schema):
class SelfReferencingSchema(Schema):
id = fields.Int()
single = fields.Nested('self')
single_with_ref_v2 = fields.Nested('self', ref='#/definitions/Self')
single_with_ref_v3 = fields.Nested('self', ref='#/components/schemas/Self')
many = fields.Nested('self', many=True)
many_with_ref_v2 = fields.Nested('self', many=True, ref='#/definitions/Selves')
many_with_ref_v3 = fields.Nested('self', many=True, ref='#/components/schemas/Selves')


class OrderedSchema(Schema):
Expand Down
17 changes: 2 additions & 15 deletions tests/test_ext_marshmallow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def test_schema_expand_parameters_v2(self, spec_fixture):
p = get_paths(spec_fixture.spec)['/pet']
get = p['get']
assert get['parameters'] == spec_fixture.openapi.schema2parameters(
PetSchema, default_in='query',
PetSchema(), default_in='query',
)
post = p['post']
assert post['parameters'] == spec_fixture.openapi.schema2parameters(
Expand Down Expand Up @@ -370,7 +370,7 @@ def test_schema_expand_parameters_v3(self, spec_fixture):
p = get_paths(spec_fixture.spec)['/pet']
get = p['get']
assert get['parameters'] == spec_fixture.openapi.schema2parameters(
PetSchema, default_in='query',
PetSchema(), default_in='query',
)
for parameter in get['parameters']:
description = parameter.get('description', False)
Expand Down Expand Up @@ -721,19 +721,6 @@ def test_self_referencing_field_many(self, spec):
'items': {'$ref': ref_path(spec) + 'SelfReference'},
}

def test_self_referencing_with_ref(self, spec):
version = 'v2' if spec.openapi_version.version[0] < 3 else 'v3'
spec.components.schema('SelfReference', schema=SelfReferencingSchema)
definitions = get_definitions(spec)
result = definitions['SelfReference']['properties'][
'single_with_ref_{}'.format(version)
]
assert result == {'$ref': ref_path(spec) + 'Self'}
result = definitions['SelfReference']['properties'][
'many_with_ref_{}'.format(version)
]
assert result == {'type': 'array', 'items': {'$ref': ref_path(spec) + 'Selves'}}


class TestOrderedSchema:

Expand Down
80 changes: 14 additions & 66 deletions tests/test_openapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,11 +555,7 @@ class PageSchema(Schema):
limit = fields.Int()

class PetSchema(Schema):
category = fields.Nested(CategorySchema, many=True, ref='#/definitions/Category')
name = fields.Str()

class PetSchemaV3(Schema):
category = fields.Nested(CategorySchema, many=True, ref='#/components/schemas/Category')
category = fields.Nested(CategorySchema, many=True)
name = fields.Str()

class TestNesting:
Expand Down Expand Up @@ -593,49 +589,19 @@ def test_field2property_nested_many_spec(self, spec_fixture):
assert ret['type'] == 'array'
assert ret['items'] == {'$ref': ref_path(spec_fixture.spec) + 'Category'}

def test_field2property_nested_ref(self, openapi):
cat_with_ref = fields.Nested(CategorySchema, ref='Category')
assert openapi.field2property(cat_with_ref) == {'$ref': 'Category'}
def test_field2property_nested_ref(self, spec_fixture):
category = fields.Nested(CategorySchema)
ref = spec_fixture.openapi.field2property(category)
assert ref == {'$ref': ref_path(spec_fixture.spec) + 'Category'}

def test_field2property_nested_many(self, spec_fixture):
categories = fields.Nested(CategorySchema, many=True)
res = spec_fixture.openapi.field2property(categories)
assert res['type'] == 'array'
assert res['items'] == {'$ref': ref_path(spec_fixture.spec) + 'Category'}

def test_field2property_nested_self_without_name_raises_error(self, openapi):
self_nesting = fields.Nested('self')
with pytest.raises(ValueError):
openapi.field2property(self_nesting)

def test_field2property_nested_self(self, openapi):
self_nesting = fields.Nested('self')
res = openapi.field2property(self_nesting, name='Foo')
if openapi.openapi_version.major < 3:
assert res == {'$ref': '#/definitions/Foo'}
else:
assert res == {'$ref': '#/components/schemas/Foo'}

def test_field2property_nested_self_many(self, openapi):
self_nesting = fields.Nested('self', many=True)
res = openapi.field2property(self_nesting, name='Foo')
if openapi.openapi_version.major < 3:
assert res == {'type': 'array', 'items': {'$ref': '#/definitions/Foo'}}
else:
assert res == {'type': 'array', 'items': {'$ref': '#/components/schemas/Foo'}}

def test_field2property_nested_self_ref_with_meta(self, openapi):
self_nesting = fields.Nested('self', ref='#/definitions/Bar')
res = openapi.field2property(self_nesting)
assert res == {'$ref': '#/definitions/Bar'}

self_nesting2 = fields.Nested('self', ref='#/definitions/Bar')
# name is passed
res = openapi.field2property(self_nesting2, name='Foo')
assert res == {'$ref': '#/definitions/Bar'}

def test_schema2jsonschema_with_nested_fields(self, spec_fixture):
res = spec_fixture.openapi.schema2jsonschema(PetSchema, use_refs=False)
res = spec_fixture.openapi.schema2jsonschema(PetSchema)
props = res['properties']

assert props['category']['items'] == {'$ref': ref_path(spec_fixture.spec) + 'Category'}
Expand Down Expand Up @@ -686,37 +652,21 @@ def test_nested_field_with_property(self, spec_fixture):
r_path = ref_path(spec_fixture.spec)

category_1 = fields.Nested(CategorySchema)
category_2 = fields.Nested(CategorySchema, ref=r_path + 'Category')
category_3 = fields.Nested(CategorySchema, dump_only=True)
category_4 = fields.Nested(CategorySchema, dump_only=True, ref=r_path + 'Category')
category_5 = fields.Nested(CategorySchema, many=True)
category_6 = fields.Nested(CategorySchema, many=True, ref=r_path + 'Category')
category_7 = fields.Nested(CategorySchema, many=True, dump_only=True)
category_8 = fields.Nested(CategorySchema, many=True, dump_only=True, ref=r_path + 'Category')
category_2 = fields.Nested(CategorySchema, dump_only=True)
category_3 = fields.Nested(CategorySchema, many=True)
category_4 = fields.Nested(CategorySchema, many=True, dump_only=True)
spec_fixture.spec.components.schema('Category', schema=CategorySchema)

assert spec_fixture.openapi.field2property(category_1) == {
'$ref': r_path + 'Category',
}
assert spec_fixture.openapi.field2property(category_2) == {
'$ref': r_path + 'Category',
}
assert spec_fixture.openapi.field2property(category_3) == {
'allOf': [{'$ref': r_path + 'Category'}], 'readOnly': True,
}
assert spec_fixture.openapi.field2property(category_4) == {
'allOf': [{'$ref': r_path + 'Category'}], 'readOnly': True,
}
assert spec_fixture.openapi.field2property(category_5) == {
'items': {'$ref': r_path + 'Category'}, 'type': 'array',
}
assert spec_fixture.openapi.field2property(category_6) == {
assert spec_fixture.openapi.field2property(category_3) == {
'items': {'$ref': r_path + 'Category'}, 'type': 'array',
}
assert spec_fixture.openapi.field2property(category_7) == {
'items': {'$ref': r_path + 'Category'}, 'readOnly': True, 'type': 'array',
}
assert spec_fixture.openapi.field2property(category_8) == {
assert spec_fixture.openapi.field2property(category_4) == {
'items': {'$ref': r_path + 'Category'}, 'readOnly': True, 'type': 'array',
}

Expand Down Expand Up @@ -748,7 +698,6 @@ def test_openapi_tools_validate_v2():
location='querystring',
),
name='body',
use_refs=False,
),
] + openapi.schema2parameters(PageSchema, default_in='query'),
'responses': {
Expand Down Expand Up @@ -788,7 +737,7 @@ def test_openapi_tools_validate_v3():
openapi = ma_plugin.openapi

spec.components.schema('Category', schema=CategorySchema)
spec.components.schema('Pet', schema=PetSchemaV3)
spec.components.schema('Pet', schema=PetSchema)

spec.path(
view=None,
Expand All @@ -814,15 +763,14 @@ def test_openapi_tools_validate_v3():
location='querystring',
),
name='body',
use_refs=False,
),
] + openapi.schema2parameters(PageSchema, default_in='query'),
'responses': {
200: {
'description': 'success',
'content': {
'application/json': {
'schema': PetSchemaV3,
'schema': PetSchema,
},
},
},
Expand Down Expand Up @@ -851,7 +799,7 @@ def test_openapi_tools_validate_v3():
'description': 'created',
'content': {
'application/json': {
'schema': PetSchemaV3,
'schema': PetSchema,
},
},
},
Expand Down