Repository: marshmallow-code/apispec (python)

Issue:
Raise exception upon name collision when registering components
When registering e.g. two schemas with the same name, the latter silently overwrites the former.

I think we should raise an Exception to inform the user.

Theoretically, it could break code where for some reason the user relied on this (like register a first version, then conditionally register a second version), but this shouldn't be a blocker.

Pull request description:
Raise DuplicateComponentNameError when registering two components with the same name
Closes #340.

Patch:
diff --git a/apispec/core.py b/apispec/core.py
index 7fdf0d4..bec13ca 100644
--- a/apispec/core.py
+++ b/apispec/core.py
@@ -3,7 +3,7 @@
 from collections import OrderedDict
 
 from apispec.compat import iterkeys, iteritems
-from .exceptions import APISpecError, PluginMethodNotImplementedError
+from .exceptions import APISpecError, PluginMethodNotImplementedError, DuplicateComponentNameError
 from .utils import OpenAPIVersion, deepupdate
 
 VALID_METHODS = [
@@ -122,6 +122,10 @@ class Components(object):
 
         https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#definitionsObject
         """
+        if name in self._schemas:
+            raise DuplicateComponentNameError(
+                'Another schema with name "{}" is already registered.'.format(name),
+            )
         ret = {}
         # Execute all helpers from plugins
         for plugin in self._plugins:
@@ -147,6 +151,10 @@ class Components(object):
         :param str location: location of the parameter.
         :param dict kwargs: parameter fields.
         """
+        if param_id in self._parameters:
+            raise DuplicateComponentNameError(
+                'Another parameter with name "{}" is already registered.'.format(param_id),
+            )
         ret = kwargs.copy()
         ret.setdefault('name', param_id)
         ret['in'] = location
@@ -165,6 +173,10 @@ class Components(object):
         :param str ref_id: ref_id to use as reference
         :param dict kwargs: response fields
         """
+        if ref_id in self._responses:
+            raise DuplicateComponentNameError(
+                'Another response with name "{}" is already registered.'.format(ref_id),
+            )
         ret = kwargs.copy()
         # Execute all helpers from plugins
         for plugin in self._plugins:
diff --git a/apispec/exceptions.py b/apispec/exceptions.py
index b382a27..b562f8f 100644
--- a/apispec/exceptions.py
+++ b/apispec/exceptions.py
@@ -8,6 +8,9 @@ class APISpecError(Exception):
 class PluginMethodNotImplementedError(APISpecError, NotImplementedError):
     """Raised when calling an unimplemented helper method in a plugin"""
 
+class DuplicateComponentNameError(APISpecError):
+    """Raised when registering two components with the same name"""
+
 class OpenAPIError(APISpecError):
     """Raised when a OpenAPI spec validation fails."""
 


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 6405a35..b1b9aec 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -5,7 +5,7 @@ import pytest
 import yaml
 
 from apispec import APISpec, BasePlugin
-from apispec.exceptions import APISpecError
+from apispec.exceptions import APISpecError, DuplicateComponentNameError
 
 from .utils import get_definitions, get_paths, get_parameters, get_responses
 
@@ -171,6 +171,14 @@ class TestDefinitions:
             defs = spec.to_dict()['components']['schemas']
         assert defs['Pet']['discriminator'] == 'name'
 
+    def test_definition_duplicate_name(self, spec):
+        spec.components.schema('Pet', properties=self.properties)
+        with pytest.raises(
+            DuplicateComponentNameError,
+            match='Another schema with name "Pet" is already registered.',
+        ):
+            spec.components.schema('Pet', properties=self.properties)
+
     def test_to_yaml(self, spec):
         enum = ['name', 'photoUrls']
         spec.components.schema(
@@ -342,6 +350,15 @@ class TestPath:
         assert 'param1' in params
         assert 'param2' in params
 
+    def test_parameter_duplicate_name(self, spec):
+        route_spec = self.paths['/pet/{petId}']['get']
+        spec.components.parameter('test_parameter', 'path', **route_spec['parameters'][0])
+        with pytest.raises(
+            DuplicateComponentNameError,
+            match='Another parameter with name "test_parameter" is already registered.',
+        ):
+            spec.components.parameter('test_parameter', 'path', **route_spec['parameters'][0])
+
     def test_response(self, spec):
         route_spec = self.paths['/pet/{petId}']['get']
 
@@ -371,6 +388,15 @@ class TestPath:
         assert 'resp1' in responses
         assert 'resp2' in responses
 
+    def test_response_duplicate_name(self, spec):
+        route_spec = self.paths['/pet/{petId}']['get']
+        spec.components.response('test_response', **route_spec['responses']['200'])
+        with pytest.raises(
+            DuplicateComponentNameError,
+            match='Another response with name "test_response" is already registered.',
+        ):
+            spec.components.response('test_response', **route_spec['responses']['200'])
+
     def test_path_check_invalid_http_method(self, spec):
         spec.path('/pet/{petId}', operations={'get': {}})
         spec.path('/pet/{petId}', operations={'x-dummy': {}})
diff --git a/tests/test_ext_marshmallow.py b/tests/test_ext_marshmallow.py
index 4437a29..d0d07df 100644
--- a/tests/test_ext_marshmallow.py
+++ b/tests/test_ext_marshmallow.py
@@ -128,7 +128,7 @@ class TestDefinitionHelper:
     def test_warning_when_schema_added_twice(self, spec, schema):
         spec.components.schema('Analysis', schema=schema)
         with pytest.warns(UserWarning, match='has already been added to the spec'):
-            spec.components.schema('Analysis', schema=schema)
+            spec.components.schema('DuplicateAnalysis', schema=schema)
 
     def test_schema_instances_with_different_modifiers_added(self, spec):
         class MultiModifierSchema(Schema):