Repository: marshmallow-code/apispec
Pull request #431 (2019-04-11 22:18:52)

Fix Marshmallow Schema's Meta exclude field error
Mixing tuple and list when collecting fields in the Marshmallow extension may result in error.

lafrech: Good.

Do you think you could test all combinations ([dump_only; exclude] x [tuple; list])?

Also, there are CI failures.

You should use a set, not a tuple, in the test, as the order doesn't matter.

(Note: no need to close/recreate a PR, you can amend/rebase and then `push -f`.)
blagasz: Ok, I check that.

(Yes, I know, but I'm not a big git magician and accidentally added a
second commit and it would have been very complicated for me to remove.
From now on I will do the rebase and push -f combo.)

On Fri, 12 Apr 2019 at 09:22, Jérôme Lafréchoux <notifications@github.com>
wrote:

> Good.
>
> Do you think you could test all combinations ([dump_only; exclude] x
> [tuple; list])?
>
> Also, there are CI failures.
>
> You should use a set, not a tuple, in the test, as the order doesn't
> matter.
>
> (Note: no need to close/recreate a PR, you can amend/rebase and then push
> -f.)
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/marshmallow-code/apispec/pull/431#issuecomment-482467560>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AFQNeLxeLtuxN-a3tZEM8Md3IYREFRFLks5vgDRGgaJpZM4cq9sU>
> .
>

blagasz: `pyupgrade` and `black` fails but I don't really know from the log what their problem is.
lafrech: Yes, the log is not verbose enough.

If you're using Python 3.6+ you can install pre-commit to do that locally before pushing.
https://github.com/marshmallow-code/apispec/blob/dev/CONTRIBUTING.rst

Patch:
diff --git a/AUTHORS.rst b/AUTHORS.rst
index fc2dc21..3bb7ca8 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -54,3 +54,4 @@ Contributors (chronological)
 - Marcin Lulek `@ergo <https://github.com/ergo>`_
 - Jonathan Beezley `@jbeezley <https://github.com/jbeezley>`_
 - David Stapleton `@dstape <https://github.com/DStape>`_
+- Szabolcs Blága `@blagasz <https://github.com/blagasz>`_
diff --git a/src/apispec/ext/marshmallow/common.py b/src/apispec/ext/marshmallow/common.py
index da177f4..8ae05b5 100644
--- a/src/apispec/ext/marshmallow/common.py
+++ b/src/apispec/ext/marshmallow/common.py
@@ -73,7 +73,7 @@ def warn_if_fields_defined_in_meta(fields, Meta):
     """Warns user that fields defined in Meta.fields or Meta.additional will
     be ignored
 
-    :param dict fields: A dictionary of of fields name field object pairs
+    :param dict fields: A dictionary of fields name field object pairs
     :param Meta: the schema's Meta class
     """
     if getattr(Meta, "fields", None) or getattr(Meta, "additional", None):
@@ -91,13 +91,13 @@ def warn_if_fields_defined_in_meta(fields, Meta):
 def filter_excluded_fields(fields, Meta, exclude_dump_only):
     """Filter fields that should be ignored in the OpenAPI spec
 
-    :param dict fields: A dictionary of of fields name field object pairs
+    :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
     """
-    exclude = getattr(Meta, "exclude", [])
+    exclude = list(getattr(Meta, "exclude", []))
     if exclude_dump_only:
-        exclude += getattr(Meta, "dump_only", [])
+        exclude.extend(getattr(Meta, "dump_only", []))
 
     filtered_fields = OrderedDict(
         (key, value) for key, value in fields.items() if key not in exclude


Test patch:
diff --git a/tests/schemas.py b/tests/schemas.py
index 1201438..35a1221 100644
--- a/tests/schemas.py
+++ b/tests/schemas.py
@@ -53,6 +53,30 @@ class OrderedSchema(Schema):
         ordered = True
 
 
+class ExcludeSchema(Schema):
+    field1 = fields.Int()
+    field2 = fields.Int()
+    field3 = fields.Int()
+    field4 = fields.Int()
+    field5 = fields.Int()
+
+    class Meta:
+        exclude = ("field2", "field4")
+        dump_only = ["field3"]
+
+
+class ExcludeSchema2(Schema):
+    field1 = fields.Int()
+    field2 = fields.Int()
+    field3 = fields.Int()
+    field4 = fields.Int()
+    field5 = fields.Int()
+
+    class Meta:
+        exclude = ["field3", "field5"]
+        dump_only = ("field1",)
+
+
 class DefaultValuesSchema(Schema):
     number_auto_default = fields.Int(missing=12)
     number_manual_default = fields.Int(missing=12, doc_default=42)
diff --git a/tests/test_ext_marshmallow_common.py b/tests/test_ext_marshmallow_common.py
index 11cf929..f53f448 100644
--- a/tests/test_ext_marshmallow_common.py
+++ b/tests/test_ext_marshmallow_common.py
@@ -1,8 +1,12 @@
 # -*- coding: utf-8 -*-
 import pytest
 
-from apispec.ext.marshmallow.common import make_schema_key, get_unique_schema_name
-from .schemas import PetSchema, SampleSchema
+from apispec.ext.marshmallow.common import (
+    make_schema_key,
+    get_unique_schema_name,
+    get_fields,
+)
+from .schemas import PetSchema, SampleSchema, ExcludeSchema, ExcludeSchema2
 
 
 class TestMakeSchemaKey:
@@ -43,3 +47,21 @@ class TestUniqueName:
         ):
             name_2 = get_unique_schema_name(spec.components, "Pet")
         assert name_2 == "Pet2"
+
+
+class TestMetaExclude:
+    def test_meta_tuple_in_exclude(self):
+        keys = set(get_fields(ExcludeSchema).keys())
+        assert keys == {"field1", "field3", "field5"}
+
+    def test_meta_list_in_exclude(self):
+        keys = set(get_fields(ExcludeSchema2).keys())
+        assert keys == {"field1", "field2", "field4"}
+
+    def test_meta_tuple_in_dump_only(self):
+        keys = set(get_fields(ExcludeSchema, exclude_dump_only=True).keys())
+        assert keys == {"field1", "field5"}
+
+    def test_meta_list_in_dump_only(self):
+        keys = set(get_fields(ExcludeSchema2, exclude_dump_only=True).keys())
+        assert keys == {"field2", "field4"}