Repository: marshmallow-code/apispec
Pull request #360 (2019-01-25 00:24:33)

Revise field2properties
Fixes an issue with an invalid reference object being created by extracting properties from field metadata before handling nested schemas.  

Also extracts methods from `field2properties` to make the method smaller and add documentation about each step.


Patch:
diff --git a/apispec/ext/marshmallow/openapi.py b/apispec/ext/marshmallow/openapi.py
index c61310a..b18a21a 100644
--- a/apispec/ext/marshmallow/openapi.py
+++ b/apispec/ext/marshmallow/openapi.py
@@ -148,6 +148,44 @@ class OpenAPIConverter(object):
 
         return inner
 
+    def field2type_and_format(self, field):
+        """Return the dictionary of OpenAPI type and format based on the field
+        type
+
+        :param Field field: A marshmallow field.
+        :rtype: dict
+        """
+        type_, fmt = self.field_mapping.get(type(field), ('string', None))
+
+        ret = {
+            'type': type_,
+        }
+
+        if fmt:
+            ret['format'] = fmt
+
+        return ret
+
+    def field2default(self, field):
+        """Return the dictionary containing the field's default value
+
+        Will first look for a `doc_default` key in the field's metadata and then
+        fall back on the field's `missing` parameter. A callable passed to the
+        field's missing parameter will be ignored.
+
+        :param Field field: A marshmallow field.
+        :rtype: dict
+        """
+        ret = {}
+        if 'doc_default' in field.metadata:
+            ret['default'] = field.metadata['doc_default']
+        else:
+            default = field.missing
+            if default is not marshmallow.missing and not callable(default):
+                    ret['default'] = default
+
+        return ret
+
     def field2choices(self, field, **kwargs):
         """Return the dictionary of OpenAPI field attributes for valid choices definition
 
@@ -292,6 +330,36 @@ class OpenAPIConverter(object):
                 attributes[max_attr] = validator.equal
         return attributes
 
+    def metadata2properties(self, field):
+        """Return a dictionary of properties extracted from field Metadata
+
+        Will include field metadata that are valid properties of `OpenAPI schema
+        objects
+        <https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#schemaObject>`_
+        (e.g. “description”, “enum”, “example”).
+
+        In addition, `specification extensions
+        <https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#patterned-objects-9>`_
+        are supported.  Prefix `x_` to the desired extension when passing the
+        keyword argument to the field constructor. apispec will convert `x_` to
+        `x-` to comply with OpenAPI.
+
+        :param Field field: A marshmallow field.
+        :rtype: dict
+        """
+        # Dasherize metadata that starts with x_
+        metadata = {
+            key.replace('_', '-') if key.startswith('x_') else key: value
+            for key, value in iteritems(field.metadata)
+        }
+
+        # Avoid validation error with "Additional properties not allowed"
+        ret = {
+            key: value for key, value in metadata.items()
+            if key in _VALID_PROPERTIES or key.startswith(_VALID_PREFIX)
+        }
+        return ret
+
     def field2property(self, field, use_refs=True, name=None):
         """Return the JSON Schema property definition given a marshmallow
         :class:`Field <marshmallow.fields.Field>`.
@@ -306,30 +374,18 @@ class OpenAPIConverter(object):
         :param str name: The definition name, if applicable, used to construct the $ref value.
         :rtype: dict, a Property Object
         """
-
-        type_, fmt = self.field_mapping.get(type(field), ('string', None))
-
-        ret = {
-            'type': type_,
-        }
-
-        if fmt:
-            ret['format'] = fmt
-
-        if 'doc_default' in field.metadata:
-            ret['default'] = field.metadata['doc_default']
-        else:
-            default = field.missing
-            if default is not marshmallow.missing and not callable(default):
-                    ret['default'] = default
+        ret = {}
 
         for attr_func in (
+                self.field2type_and_format,
+                self.field2default,
                 self.field2choices,
                 self.field2read_only,
                 self.field2write_only,
                 self.field2nullable,
                 self.field2range,
                 self.field2length,
+                self.metadata2properties,
         ):
             ret.update(attr_func(field))
 
@@ -377,18 +433,6 @@ class OpenAPIConverter(object):
                         field.value_container, use_refs=use_refs,
                     )
 
-        # Dasherize metadata that starts with x_
-        metadata = {
-            key.replace('_', '-') if key.startswith('x_') else key: value
-            for key, value in iteritems(field.metadata)
-        }
-        for key, value in iteritems(metadata):
-            if key in _VALID_PROPERTIES or key.startswith(_VALID_PREFIX):
-                ret[key] = value
-        # Avoid validation error with "Additional properties not allowed"
-        # Property "ref" is not valid in this context
-        ret.pop('ref', None)
-
         return ret
 
     def resolve_nested_schema(self, schema):


Test patch:
diff --git a/tests/test_openapi.py b/tests/test_openapi.py
index 7d94286..df990b3 100644
--- a/tests/test_openapi.py
+++ b/tests/test_openapi.py
@@ -574,11 +574,17 @@ class TestNesting:
 
     def test_field2property_nested_spec_metadatas(self, spec_fixture):
         spec_fixture.spec.components.schema('Category', schema=CategorySchema)
-        category = fields.Nested(CategorySchema, description='A category')
+        category = fields.Nested(
+            CategorySchema,
+            description='A category',
+            invalid_property='not in the result',
+            x_extension='A great extension',
+        )
         result = spec_fixture.openapi.field2property(category)
         assert result == {
-            '$ref': ref_path(spec_fixture.spec) + 'Category',
+            'allOf': [{'$ref': ref_path(spec_fixture.spec) + 'Category'}],
             'description': 'A category',
+            'x-extension': 'A great extension',
         }
 
     def test_field2property_nested_spec(self, spec_fixture):
@@ -599,11 +605,6 @@ class TestNesting:
         cat_with_ref = fields.Nested(CategorySchema, ref='Category')
         assert openapi.field2property(cat_with_ref) == {'$ref': 'Category'}
 
-    def test_field2property_nested_ref_with_meta(self, openapi):
-        cat_with_ref = fields.Nested(CategorySchema, ref='Category', description='A category')
-        result = openapi.field2property(cat_with_ref)
-        assert result == {'$ref': 'Category', 'description': 'A category'}
-
     def test_field2property_nested_many(self, spec_fixture):
         categories = fields.Nested(CategorySchema, many=True)
         res = spec_fixture.openapi.field2property(categories)