Alter unit test to match bug and cleanup ext logic

The unit test for bug #1443342 was only testing that a side effect
leading to the bug didn't occur (comparing object identities). This
patch updates the unit test to fully assert that the bug itself
doesn't occur without checking implementation details.

This also eliminates the branching that led to the original issue by
using setdefault to always return a dict to update.

Related-Bug: #1443342
Change-Id: I6c48681aa49c17bbadffa12bad1eea272c483437

diff --git a/neutron/api/extensions.py b/neutron/api/extensions.py
index 8c2e4e2b0f60c0851a36582bd80f7f6edb9696ab..8eb0f9070c9937fba25a6f173950dfc313bd6f39 100644 (file)
--- a/neutron/api/extensions.py
+++ b/neutron/api/extensions.py
@@ -452,10 +452,7 @@ class ExtensionManager(object):
                 try:
                     extended_attrs = ext.get_extended_resources(version)
                     for res, resource_attrs in six.iteritems(extended_attrs):
-                        if attr_map.get(res, None):
-                            attr_map[res].update(resource_attrs)
-                        else:
-                            attr_map[res] = resource_attrs.copy()
+                        attr_map.setdefault(res, {}).update(resource_attrs)
                 except AttributeError:
                     LOG.exception(_LE("Error fetching extended attributes for "
                                       "extension '%s'"), ext.get_name())

diff --git a/neutron/tests/unit/api/test_extensions.py b/neutron/tests/unit/api/test_extensions.py
index cc3f71c05790dfd63e20f1e58a296efc5ed94a7c..19b9858da5b29e9bf1e8ecfa5f6469dde22e4e6e 100644 (file)
--- a/neutron/tests/unit/api/test_extensions.py
+++ b/neutron/tests/unit/api/test_extensions.py
@@ -487,26 +487,21 @@ class ExtensionManagerTest(base.BaseTestCase):
         self.assertNotIn('invalid_extension', ext_mgr.extensions)
 
     def test_assignment_of_attr_map(self):
+        """Unit test for bug 1443342
 
-        class ExtendResourceExtension(object):
+        In this bug, an extension that extended multiple resources with the
+        same dict would cause future extensions to inadvertently modify the
+        resources of all of the resources since they were referencing the same
+        dictionary.
+        """
+
+        class MultiResourceExtension(ext_stubs.StubExtension):
             """Generated Extended Resources.
 
             This extension's extended resource will assign
             to more than one resource.
             """
 
-            def get_name(self):
-                return "extension"
-
-            def get_alias(self):
-                return "extension"
-
-            def get_description(self):
-                return "Extension for test"
-
-            def get_updated(self):
-                return "2013-07-23T10:00:00-00:00"
-
             def get_extended_resources(self, version):
                 EXTENDED_TIMESTAMP = {
                     'created_at': {'allow_post': False, 'allow_put': False,
@@ -518,13 +513,27 @@ class ExtensionManagerTest(base.BaseTestCase):
 
                 return attrs
 
+        class AttrExtension(ext_stubs.StubExtension):
+            def get_extended_resources(self, version):
+                attrs = {
+                    self.alias: {
+                        '%s-attr' % self.alias: {'allow_post': False,
+                                                 'allow_put': False,
+                                                 'is_visible': True}}}
+                return attrs
+
         ext_mgr = extensions.ExtensionManager('')
-        ext_mgr.add_extension(ExtendResourceExtension())
-        ext_mgr.add_extension(ext_stubs.StubExtension("ext1"))
-        ext_mgr.add_extension(ext_stubs.StubExtension("ext2"))
         attr_map = {}
+        ext_mgr.add_extension(MultiResourceExtension('timestamp'))
+        ext_mgr.extend_resources("2.0", attr_map)
+        ext_mgr.add_extension(AttrExtension("ext1"))
+        ext_mgr.add_extension(AttrExtension("ext2"))
         ext_mgr.extend_resources("2.0", attr_map)
-        self.assertNotEqual(id(attr_map['ext1']), id(attr_map['ext2']))
+        self.assertIn('created_at', attr_map['ext2'])
+        self.assertIn('created_at', attr_map['ext1'])
+        # now we need to make sure the attrextensions didn't leak across
+        self.assertNotIn('ext1-attr', attr_map['ext2'])
+        self.assertNotIn('ext2-attr', attr_map['ext1'])
 
 
 class PluginAwareExtensionManagerTest(base.BaseTestCase):