From 20654669cb46e9bcda3ac5cbb816cb2aaee507c5 Mon Sep 17 00:00:00 2001 From: bangertm Date: Fri, 25 Jan 2019 15:36:07 -0500 Subject: [PATCH 1/2] removes passing reference paths in nested field metadata field2property now ignores a ref key in field metadata and relies only on the field autoreferencing capability Self referencing instances of `marshmallow.fields.Nested` that are unbound cannot be passed to field2property, which means that the `schema` argument of `OpenAPI.schema2jsonschema` and `OpenAPI.schema2parameters` must be a schema instance and not a schema class --- apispec/ext/marshmallow/__init__.py | 12 ++--- apispec/ext/marshmallow/openapi.py | 60 ++++++---------------- tests/schemas.py | 4 -- tests/test_ext_marshmallow.py | 13 ----- tests/test_openapi.py | 80 +++++------------------------ 5 files changed, 34 insertions(+), 135 deletions(-) diff --git a/apispec/ext/marshmallow/__init__.py b/apispec/ext/marshmallow/__init__.py index 9c27f223..fdf92d63 100644 --- a/apispec/ext/marshmallow/__init__.py +++ b/apispec/ext/marshmallow/__init__.py @@ -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 @@ -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, default_in=parameter.pop('in'), **parameter ) continue @@ -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 diff --git a/apispec/ext/marshmallow/openapi.py b/apispec/ext/marshmallow/openapi.py index 334cb9f0..ebf3d2c8 100644 --- a/apispec/ext/marshmallow/openapi.py +++ b/apispec/ext/marshmallow/openapi.py @@ -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 `. @@ -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 = {} @@ -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 @@ -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 ` 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 @@ -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: @@ -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 `. 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, @@ -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 `. Schema may optionally provide the ``title`` and ``description`` class Meta options. @@ -641,7 +613,7 @@ 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) @@ -649,7 +621,7 @@ class Meta: 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 @@ -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 ` objects. @@ -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: diff --git a/tests/schemas.py b/tests/schemas.py index 105daedc..21566f19 100644 --- a/tests/schemas.py +++ b/tests/schemas.py @@ -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): diff --git a/tests/test_ext_marshmallow.py b/tests/test_ext_marshmallow.py index d0d07dfa..6959074f 100644 --- a/tests/test_ext_marshmallow.py +++ b/tests/test_ext_marshmallow.py @@ -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: diff --git a/tests/test_openapi.py b/tests/test_openapi.py index e0a688a3..11e4eba6 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -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: @@ -593,9 +589,10 @@ 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) @@ -603,39 +600,8 @@ def test_field2property_nested_many(self, spec_fixture): 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'} @@ -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', } @@ -748,7 +698,6 @@ def test_openapi_tools_validate_v2(): location='querystring', ), name='body', - use_refs=False, ), ] + openapi.schema2parameters(PageSchema, default_in='query'), 'responses': { @@ -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, @@ -814,7 +763,6 @@ def test_openapi_tools_validate_v3(): location='querystring', ), name='body', - use_refs=False, ), ] + openapi.schema2parameters(PageSchema, default_in='query'), 'responses': { @@ -822,7 +770,7 @@ def test_openapi_tools_validate_v3(): 'description': 'success', 'content': { 'application/json': { - 'schema': PetSchemaV3, + 'schema': PetSchema, }, }, }, @@ -851,7 +799,7 @@ def test_openapi_tools_validate_v3(): 'description': 'created', 'content': { 'application/json': { - 'schema': PetSchemaV3, + 'schema': PetSchema, }, }, }, From 88eba0f3c8bae701a20f43cef7ed30f36dc18961 Mon Sep 17 00:00:00 2001 From: bangertm Date: Mon, 28 Jan 2019 11:58:30 -0500 Subject: [PATCH 2/2] changes comparision to an instance --- tests/test_ext_marshmallow.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_ext_marshmallow.py b/tests/test_ext_marshmallow.py index 6959074f..c457d921 100644 --- a/tests/test_ext_marshmallow.py +++ b/tests/test_ext_marshmallow.py @@ -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( @@ -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)