Repository: marshmallow-code/apispec
Pull request #581 (2020-07-10 22:38:27)

Refactor openapi converter
Based on #526.

Implements refactor discussed in #500 and #526.

I did not address handling webargs field dicts (https://github.com/marshmallow-code/apispec/issues/500#issuecomment-570598012). It can be done later anytime (perhaps easier once MA2 is dropped).

Once #526 is merged, I can rebase this for a clearer diff.

lafrech: Thanks @Bangertm for taking the time to review this.

I think I'm gonna merge this and open an issue about the location mapping.

Patch:
diff --git a/src/apispec/ext/marshmallow/common.py b/src/apispec/ext/marshmallow/common.py
index 14d9f2e..de96750 100644
--- a/src/apispec/ext/marshmallow/common.py
+++ b/src/apispec/ext/marshmallow/common.py
@@ -91,14 +91,16 @@ def filter_excluded_fields(fields, Meta, *, exclude_dump_only):
 
     :param dict fields: A dictionary of fields name field object pairs
     :param Meta: the schema's Meta class
-    :param bool exclude_dump_only: whether to filter fields in Meta.dump_only
+    :param bool exclude_dump_only: whether to filter dump_only fields
     """
     exclude = list(getattr(Meta, "exclude", []))
     if exclude_dump_only:
         exclude.extend(getattr(Meta, "dump_only", []))
 
     filtered_fields = OrderedDict(
-        (key, value) for key, value in fields.items() if key not in exclude
+        (key, value)
+        for key, value in fields.items()
+        if key not in exclude and not (exclude_dump_only and value.dump_only)
     )
 
     return filtered_fields
diff --git a/src/apispec/ext/marshmallow/openapi.py b/src/apispec/ext/marshmallow/openapi.py
index cb13646..b379899 100644
--- a/src/apispec/ext/marshmallow/openapi.py
+++ b/src/apispec/ext/marshmallow/openapi.py
@@ -117,22 +117,22 @@ class OpenAPIConverter(FieldConverterMixin):
         of a single parameter; else return an array of a parameter for each included field in
         the :class:`Schema <marshmallow.Schema>`.
 
+        In OpenAPI 3, only "query", "header", "path" or "cookie" are allowed for the location
+        of parameters. "requestBody" is used when fields are in the body.
+
         https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#parameterObject
         """
-        openapi_location = __location_map__.get(location, location)
-        if self.openapi_version.major < 3 and openapi_location == "body":
-            prop = self.resolve_nested_schema(schema)
-
+        location = __location_map__.get(location, location)
+        # OAS 2 body parameter
+        if location == "body":
             param = {
-                "in": openapi_location,
+                "in": location,
                 "required": required,
                 "name": name,
-                "schema": prop,
+                "schema": self.resolve_nested_schema(schema),
             }
-
             if description:
                 param["description"] = description
-
             return [param]
 
         assert not getattr(
@@ -141,99 +141,37 @@ class OpenAPIConverter(FieldConverterMixin):
 
         fields = get_fields(schema, exclude_dump_only=True)
 
-        return self.fields2parameters(fields, location=location)
-
-    def fields2parameters(self, fields, *, location):
-        """Return an array of OpenAPI parameters given a mapping between field names and
-        :class:`Field <marshmallow.Field>` objects. If `location` is "body", then return an array
-        of a single parameter; else return an array of a parameter for each included field in
-        the :class:`Schema <marshmallow.Schema>`.
-
-        In OpenAPI3, only "query", "header", "path" or "cookie" are allowed for the location
-        of parameters. In OpenAPI 3, "requestBody" is used when fields are in the body.
-
-        This function always returns a list, with a parameter
-        for each included field in the :class:`Schema <marshmallow.Schema>`.
-
-        https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#parameterObject
-        """
-        parameters = []
-        body_param = None
-        for field_name, field_obj in fields.items():
-            if field_obj.dump_only:
-                continue
-            param = self.field2parameter(
+        return [
+            self._field2parameter(
                 field_obj,
                 name=self._observed_name(field_obj, field_name),
                 location=location,
             )
-            if (
-                self.openapi_version.major < 3
-                and param["in"] == "body"
-                and body_param is not None
-            ):
-                body_param["schema"]["properties"].update(param["schema"]["properties"])
-                required_fields = param["schema"].get("required", [])
-                if required_fields:
-                    body_param["schema"].setdefault("required", []).extend(
-                        required_fields
-                    )
-            else:
-                if self.openapi_version.major < 3 and param["in"] == "body":
-                    body_param = param
-                parameters.append(param)
-        return parameters
+            for field_name, field_obj in fields.items()
+        ]
 
-    def field2parameter(self, field, *, name, location):
+    def _field2parameter(self, field, *, name, location):
         """Return an OpenAPI parameter as a `dict`, given a marshmallow
         :class:`Field <marshmallow.Field>`.
 
         https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#parameterObject
         """
-        prop = self.field2property(field)
-        return self.property2parameter(
-            prop,
-            name=name,
-            required=field.required,
-            multiple=isinstance(field, marshmallow.fields.List),
-            location=location,
-        )
+        ret = {"in": location, "name": name, "required": field.required}
 
-    def property2parameter(self, prop, *, name, required, multiple, location):
-        """Return the Parameter Object definition for a JSON Schema property.
-
-        https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#parameterObject
+        prop = self.field2property(field)
+        multiple = isinstance(field, marshmallow.fields.List)
 
-        :param dict prop: JSON Schema property
-        :param str name: Field name
-        :param bool required: Parameter is required
-        :param bool multiple: Parameter is repeated
-        :param str location: Location to look for ``name``
-        :raise: TranslationError if arg object cannot be translated to a Parameter Object schema.
-        :rtype: dict, a Parameter Object
-        """
-        openapi_location = __location_map__.get(location, location)
-        ret = {"in": openapi_location, "name": name}
-
-        if openapi_location == "body":
-            ret["required"] = False
-            ret["name"] = "body"
-            ret["schema"] = {"type": "object", "properties": {name: prop}}
-            if required:
-                ret["schema"]["required"] = [name]
+        if self.openapi_version.major < 3:
+            if multiple:
+                ret["collectionFormat"] = "multi"
+            ret.update(prop)
         else:
-            ret["required"] = required
-            if self.openapi_version.major < 3:
-                if multiple:
-                    ret["collectionFormat"] = "multi"
-                ret.update(prop)
-            else:
-                if multiple:
-                    ret["explode"] = True
-                    ret["style"] = "form"
-                if prop.get("description", None):
-                    ret["description"] = prop.pop("description")
-                ret["schema"] = prop
+            if multiple:
+                ret["explode"] = True
+                ret["style"] = "form"
+            if prop.get("description", None):
+                ret["description"] = prop.pop("description")
+            ret["schema"] = prop
         return ret
 
     def schema2jsonschema(self, schema):


Test patch:
diff --git a/tests/test_ext_marshmallow_openapi.py b/tests/test_ext_marshmallow_openapi.py
index b2e8730..488e4c2 100644
--- a/tests/test_ext_marshmallow_openapi.py
+++ b/tests/test_ext_marshmallow_openapi.py
@@ -12,23 +12,15 @@ from .utils import get_schemas, build_ref
 
 class TestMarshmallowFieldToOpenAPI:
     def test_fields_with_missing_load(self, openapi):
-        field_dict = {"field": fields.Str(default="foo", missing="bar")}
-        res = openapi.fields2parameters(field_dict, location="query")
+        class MySchema(Schema):
+            field = fields.Str(default="foo", missing="bar")
+
+        res = openapi.schema2parameters(MySchema, location="query")
         if openapi.openapi_version.major < 3:
             assert res[0]["default"] == "bar"
         else:
             assert res[0]["schema"]["default"] == "bar"
 
-    def test_fields_location_mapping(self, openapi):
-        field_dict = {"field": fields.Str()}
-        res = openapi.fields2parameters(field_dict, location="cookies")
-        assert res[0]["in"] == "cookie"
-
-    def test_fields_default_location_mapping(self, openapi):
-        field_dict = {"field": fields.Str()}
-        res = openapi.fields2parameters(field_dict, location="headers")
-        assert res[0]["in"] == "header"
-
     # json/body is invalid for OpenAPI 3
     @pytest.mark.parametrize("openapi", ("2.0",), indirect=True)
     def test_fields_default_location_mapping_if_schema_many(self, openapi):
@@ -43,9 +35,7 @@ class TestMarshmallowFieldToOpenAPI:
         class UserSchema(Schema):
             name = fields.Str(dump_only=True)
 
-        res = openapi.fields2parameters(UserSchema._declared_fields, location="query")
-        assert len(res) == 0
-        res = openapi.fields2parameters(UserSchema().fields, location="query")
+        res = openapi.schema2parameters(schema=UserSchema(), location="query")
         assert len(res) == 0
 
         class UserSchema(Schema):
@@ -54,7 +44,7 @@ class TestMarshmallowFieldToOpenAPI:
             class Meta:
                 dump_only = ("name",)
 
-        res = openapi.schema2parameters(schema=UserSchema, location="query")
+        res = openapi.schema2parameters(schema=UserSchema(), location="query")
         assert len(res) == 0
 
 
@@ -229,7 +219,7 @@ class TestMarshmallowSchemaToParameters:
     @pytest.mark.parametrize("ListClass", [fields.List, CustomList])
     def test_field_multiple(self, ListClass, openapi):
         field = ListClass(fields.Str)
-        res = openapi.field2parameter(field, name="field", location="querystring")
+        res = openapi._field2parameter(field, name="field", location="query")
         assert res["in"] == "query"
         if openapi.openapi_version.major < 3:
             assert res["type"] == "array"
@@ -243,7 +233,7 @@ class TestMarshmallowSchemaToParameters:
 
     def test_field_required(self, openapi):
         field = fields.Str(required=True)
-        res = openapi.field2parameter(field, name="field", location="query")
+        res = openapi._field2parameter(field, name="field", location="query")
         assert res["required"] is True
 
     def test_invalid_schema(self, openapi):
@@ -325,8 +315,11 @@ class TestMarshmallowSchemaToParameters:
             openapi.schema2parameters(UserSchema(many=True), location="query")
 
     def test_fields_query(self, openapi):
-        field_dict = {"name": fields.Str(), "email": fields.Email()}
-        res = openapi.fields2parameters(field_dict, location="query")
+        class MySchema(Schema):
+            name = fields.Str()
+            email = fields.Email()
+
+        res = openapi.schema2parameters(MySchema, location="query")
         assert len(res) == 2
         res.sort(key=lambda param: param["name"])
         assert res[0]["name"] == "email"
@@ -444,11 +437,11 @@ def test_openapi_tools_validate_v2():
                         "required": True,
                         "type": "string",
                     },
-                    openapi.field2parameter(
+                    openapi._field2parameter(
                         field=fields.List(
                             fields.Str(), validate=validate.OneOf(["freddie", "roger"]),
                         ),
-                        location="querystring",
+                        location="query",
                         name="body",
                     ),
                 ]
@@ -500,11 +493,11 @@ def test_openapi_tools_validate_v3():
                         "required": True,
                         "schema": {"type": "string"},
                     },
-                    openapi.field2parameter(
+                    openapi._field2parameter(
                         field=fields.List(
                             fields.Str(), validate=validate.OneOf(["freddie", "roger"]),
                         ),
-                        location="querystring",
+                        location="query",
                         name="body",
                     ),
                 ]