From: Kevin Benton <blak111@gmail.com>
Date: Fri, 17 Jul 2015 01:45:30 +0000 (-0700)
Subject: Alter unit test to match bug and cleanup ext logic
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=c5cb76315f0af0ab03185575bec91210f61c13e5;p=openstack-build%2Fneutron-build.git

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 8c2e4e2b0..8eb0f9070 100644
--- 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 cc3f71c05..19b9858da 100644
--- 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):