Repository: marshmallow-code/apispec
Pull request #336 (2018-11-06 11:37:14)

Allow helpers to return None
Fixes https://github.com/marshmallow-code/apispec/issues/331.


Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 6eaadf1..a64b687 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -30,11 +30,21 @@ Features:
     spec.path('/pets/', operations=...)
     spec.components.schema('Pet', properties=...)
 
+- Plugins can define ``parameter_helper`` and ``response_helper`` to modify
+  parameter and response components definitions.
+- ``MarshmallowPlugin`` resolves schemas in parameters and responses components.
+- Components helpers may return ``None`` as a no-op rather than an empty `dict`
+  (:pr:`336`).
+
+Bug fixes:
+
+- ``MarshmallowPlugin.schema_helper`` does not crash when no schema is passed
+  (:pr:`336`).
 
 Deprecations/Removals:
 
-- The ``response_helper`` feature is removed. The same can be achieved from
-  ``operation_helper``.
+- The legacy ``response_helper`` feature is removed. The same can be achieved
+  from ``operation_helper``.
 
 1.0.0b4 (2018-10-28)
 ++++++++++++++++++++
diff --git a/apispec/core.py b/apispec/core.py
index a8203d2..d7d2a00 100644
--- a/apispec/core.py
+++ b/apispec/core.py
@@ -127,7 +127,7 @@ class Components(object):
         # Execute all helpers from plugins
         for plugin in self._plugins:
             try:
-                ret.update(plugin.schema_helper(name, definition=ret, **kwargs))
+                ret.update(plugin.schema_helper(name, definition=ret, **kwargs) or {})
             except PluginMethodNotImplementedError:
                 continue
         if properties:
@@ -153,7 +153,7 @@ class Components(object):
         # Execute all helpers from plugins
         for plugin in self._plugins:
             try:
-                ret.update(plugin.parameter_helper(**kwargs))
+                ret.update(plugin.parameter_helper(**kwargs) or {})
             except PluginMethodNotImplementedError:
                 continue
         self._parameters[param_id] = ret
@@ -168,7 +168,7 @@ class Components(object):
         # Execute all helpers from plugins
         for plugin in self._plugins:
             try:
-                ret.update(plugin.response_helper(**kwargs))
+                ret.update(plugin.response_helper(**kwargs) or {})
             except PluginMethodNotImplementedError:
                 continue
         self._responses[ref_id] = ret
diff --git a/apispec/ext/marshmallow/__init__.py b/apispec/ext/marshmallow/__init__.py
index d4b7f76..ec796cd 100644
--- a/apispec/ext/marshmallow/__init__.py
+++ b/apispec/ext/marshmallow/__init__.py
@@ -155,13 +155,15 @@ class MarshmallowPlugin(BasePlugin):
         """
         return self.openapi.map_to_openapi_type(*args)
 
-    def schema_helper(self, name, schema, **kwargs):
+    def schema_helper(self, name, schema=None, **kwargs):
         """Definition helper that allows using a marshmallow
         :class:`Schema <marshmallow.Schema>` to provide OpenAPI
         metadata.
 
         :param type|Schema schema: A marshmallow Schema class or instance.
         """
+        if schema is None:
+            return None
 
         schema_cls = resolve_schema_cls(schema)
         schema_instance = resolve_schema_instance(schema)
diff --git a/apispec/plugin.py b/apispec/plugin.py
index 2c49a57..16f25fc 100644
--- a/apispec/plugin.py
+++ b/apispec/plugin.py
@@ -13,15 +13,15 @@ class BasePlugin(object):
         """
 
     def schema_helper(self, name, definition, **kwargs):
-        """Must return definition as a dict."""
+        """May return definition as a dict."""
         raise PluginMethodNotImplementedError
 
     def parameter_helper(self, **kwargs):
-        """Must return parameter component description as a dict."""
+        """May return parameter component description as a dict."""
         raise PluginMethodNotImplementedError
 
     def response_helper(self, **kwargs):
-        """Must return response component description as a dict."""
+        """May return response component description as a dict."""
         raise PluginMethodNotImplementedError
 
     def path_helper(self, path=None, operations=None, **kwargs):
@@ -41,7 +41,7 @@ class BasePlugin(object):
         raise PluginMethodNotImplementedError
 
     def operation_helper(self, path=None, operations=None, **kwargs):
-        """Should mutate operations.
+        """May mutate operations.
 
         :param str path: Path to the resource
         :param dict operations: A `dict` mapping HTTP methods to operation object. See


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 6650166..99ec31d 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -353,81 +353,106 @@ class TestPath:
 
 class TestPlugins:
 
-    class TestPlugin(BasePlugin):
-        def schema_helper(self, name, definition, **kwargs):
-            return {'properties': {'name': {'type': 'string'}}}
+    @staticmethod
+    def test_plugin_factory(return_none=False):
 
-        def parameter_helper(self, **kwargs):
-            return {'name': 'parameter'}
+        class TestPlugin(BasePlugin):
+            def schema_helper(self, name, definition, **kwargs):
+                if not return_none:
+                    return {'properties': {'name': {'type': 'string'}}}
 
-        def response_helper(self, **kwargs):
-            return {'description': '42'}
+            def parameter_helper(self, **kwargs):
+                if not return_none:
+                    return {'description': 'some parameter'}
 
-        def path_helper(self, path, operations, **kwargs):
-            if path == '/path_1':
-                operations.update({'get': {'responses': {'200': {}}}})
-                return '/path_1_modified'
+            def response_helper(self, **kwargs):
+                if not return_none:
+                    return {'description': '42'}
 
-        def operation_helper(self, path, operations, **kwargs):
-            if path == '/path_2':
-                operations['post'] = {'responses': {'201': {}}}
+            def path_helper(self, path, operations, **kwargs):
+                if not return_none:
+                    if path == '/path_1':
+                        operations.update({'get': {'responses': {'200': {}}}})
+                        return '/path_1_modified'
+
+            def operation_helper(self, path, operations, **kwargs):
+                if path == '/path_2':
+                    operations['post'] = {'responses': {'201': {}}}
+
+        return TestPlugin()
 
-    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0', ))
-    def test_plugin_schema_helper_is_used(self, openapi_version):
+    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0'))
+    @pytest.mark.parametrize('return_none', (True, False))
+    def test_plugin_schema_helper_is_used(self, openapi_version, return_none):
         spec = APISpec(
             title='Swagger Petstore',
             version='1.0.0',
             openapi_version=openapi_version,
-            plugins=(self.TestPlugin(), ),
+            plugins=(self.test_plugin_factory(return_none), ),
         )
         spec.components.schema('Pet', {})
         definitions = get_definitions(spec)
-        assert definitions['Pet'] == {'properties': {'name': {'type': 'string'}}}
+        if return_none:
+            assert definitions['Pet'] == {}
+        else:
+            assert definitions['Pet'] == {'properties': {'name': {'type': 'string'}}}
 
-    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0', ))
-    def test_plugin_parameter_helper_is_used(self, openapi_version):
+    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0'))
+    @pytest.mark.parametrize('return_none', (True, False))
+    def test_plugin_parameter_helper_is_used(self, openapi_version, return_none):
         spec = APISpec(
             title='Swagger Petstore',
             version='1.0.0',
             openapi_version=openapi_version,
-            plugins=(self.TestPlugin(), ),
+            plugins=(self.test_plugin_factory(return_none), ),
         )
         spec.components.parameter('Pet', 'body', **{})
         parameters = get_parameters(spec)
-        assert parameters['Pet'] == {'in': 'body', 'name': 'parameter'}
+        if return_none:
+            assert parameters['Pet'] == {'in': 'body', 'name': 'Pet'}
+        else:
+            assert parameters['Pet'] == {'in': 'body', 'name': 'Pet', 'description': 'some parameter'}
 
-    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0', ))
-    def test_plugin_response_helper_is_used(self, openapi_version):
+    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0'))
+    @pytest.mark.parametrize('return_none', (True, False))
+    def test_plugin_response_helper_is_used(self, openapi_version, return_none):
         spec = APISpec(
             title='Swagger Petstore',
             version='1.0.0',
             openapi_version=openapi_version,
-            plugins=(self.TestPlugin(), ),
+            plugins=(self.test_plugin_factory(return_none), ),
         )
         spec.components.response('Pet', **{})
         responses = get_responses(spec)
-        assert responses['Pet'] == {'description': '42'}
+        if return_none:
+            assert responses['Pet'] == {}
+        else:
+            assert responses['Pet'] == {'description': '42'}
 
-    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0', ))
-    def test_plugin_path_helper_is_used(self, openapi_version):
+    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0'))
+    @pytest.mark.parametrize('return_none', (True, False))
+    def test_plugin_path_helper_is_used(self, openapi_version, return_none):
         spec = APISpec(
             title='Swagger Petstore',
             version='1.0.0',
             openapi_version=openapi_version,
-            plugins=(self.TestPlugin(), ),
+            plugins=(self.test_plugin_factory(return_none), ),
         )
         spec.path('/path_1')
         paths = get_paths(spec)
         assert len(paths) == 1
-        assert paths['/path_1_modified'] == {'get': {'responses': {'200': {}}}}
+        if return_none:
+            assert paths['/path_1'] == {}
+        else:
+            assert paths['/path_1_modified'] == {'get': {'responses': {'200': {}}}}
 
-    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0', ))
+    @pytest.mark.parametrize('openapi_version', ('2.0', '3.0.0'))
     def test_plugin_operation_helper_is_used(self, openapi_version):
         spec = APISpec(
             title='Swagger Petstore',
             version='1.0.0',
             openapi_version=openapi_version,
-            plugins=(self.TestPlugin(), ),
+            plugins=(self.test_plugin_factory(), ),
         )
         spec.path('/path_2', operations={'post': {'responses': {'200': {}}}})
         paths = get_paths(spec)
diff --git a/tests/test_ext_marshmallow.py b/tests/test_ext_marshmallow.py
index 48a8a4e..4cdb845 100644
--- a/tests/test_ext_marshmallow.py
+++ b/tests/test_ext_marshmallow.py
@@ -31,12 +31,16 @@ class TestDefinitionHelper:
     def test_can_use_schema_as_definition(self, spec, schema):
         spec.components.schema('Pet', schema=schema)
         definitions = get_definitions(spec)
-        assert 'Pet' in definitions
         props = definitions['Pet']['properties']
 
         assert props['id']['type'] == 'integer'
         assert props['name']['type'] == 'string'
 
+    def test_schema_helper_without_schema(self, spec):
+        spec.components.schema('Pet', properties={'key': {'type': 'integer'}})
+        definitions = get_definitions(spec)
+        assert definitions['Pet']['properties'] == {'key': {'type': 'integer'}}
+
     @pytest.mark.parametrize('schema', [AnalysisSchema, AnalysisSchema()])
     def test_resolve_schema_dict_auto_reference(self, schema):
         def resolver(schema):