Remove use of contextlib.nested

The contextlib.nested call has been deprecated
in Python 2.7. This causes DeprecationWarning
messages in the unit tests.

There are also known issues with contextlib.nested
that were addressed by the native support for
multiple "with" variables. For instance, if the
first object is created but the second one throws
an exception, the first object's __exit__ is never
called. For more information see
https://docs.python.org/2/library/contextlib.html#contextlib.nested
contextlib.nested is also not compatible in Python 3.

Since Glance no longer supports 2.6 we can remove
the use of these contextlib.nested calls.

Added hacking check to catch if any new instances
are added to the codebase.

Closes-Bug: #1428424
Change-Id: Ic8edfa41d6c468cf6db8d11d3533e4f8cf2053c2

3 files changed, 32 insertions, 19 deletions

diff --git a/glance/hacking/checks.py b/glance/hacking/checks.py
index a79f3f59a..bb41e7c25 100644
--- a/glance/hacking/checks.py
+++ b/glance/hacking/checks.py
@@ -139,6 +139,15 @@ def validate_log_translations(logical_line, physical_line, filename):
         yield (0, msg)
 
 
+def check_no_contextlib_nested(logical_line):
+    msg = ("G327: contextlib.nested is deprecated since Python 2.7. See "
+           "https://docs.python.org/2/library/contextlib.html#contextlib."
+           "nested for more information.")
+    if ("with contextlib.nested(" in logical_line or
+            "with nested(" in logical_line):
+        yield(0, msg)
+
+
 def factory(register):
     register(assert_true_instance)
     register(assert_equal_type)
@@ -146,3 +155,4 @@ def factory(register):
     register(no_translate_debug_logs)
     register(no_direct_use_of_unicode_function)
     register(validate_log_translations)
+    register(check_no_contextlib_nested)
diff --git a/glance/tests/test_hacking.py b/glance/tests/test_hacking.py
index 9ad47d800..685185d71 100644
--- a/glance/tests/test_hacking.py
+++ b/glance/tests/test_hacking.py
@@ -62,3 +62,13 @@ class HackingTestCase(utils.BaseTestCase):
             "six.text_type('party over')"))))
         self.assertEqual(0, len(list(checks.no_direct_use_of_unicode_function(
             "not_actually_unicode('something completely different')"))))
+
+    def test_no_contextlib_nested(self):
+        self.assertEqual(1, len(list(checks.check_no_contextlib_nested(
+            "with contextlib.nested("))))
+
+        self.assertEqual(1, len(list(checks.check_no_contextlib_nested(
+            "with nested("))))
+
+        self.assertEqual(0, len(list(checks.check_no_contextlib_nested(
+            "with foo as bar"))))
diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py
index f64411b94..3996762e9 100644
--- a/glance/tests/unit/v1/test_api.py
+++ b/glance/tests/unit/v1/test_api.py
@@ -15,7 +15,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import contextlib
 import copy
 import datetime
 import hashlib
@@ -394,7 +393,10 @@ class TestGlanceAPI(base.IsolatedUnitTest):
         self.assertEqual(400, res.status_int)
         self.assertIn('Required store bad is invalid', res.body)
 
-    def test_create_with_location_get_store_or_400_raises_exception(self):
+    @mock.patch.object(glance.api.v1.images.Controller, '_external_source')
+    @mock.patch.object(store, 'get_store_from_location')
+    def test_create_with_location_get_store_or_400_raises_exception(
+            self, mock_get_store_from_location, mock_external_source):
         location = 'bad+scheme://localhost:0/image.qcow2'
         scheme = 'bad+scheme'
         fixture_headers = {
@@ -409,23 +411,14 @@ class TestGlanceAPI(base.IsolatedUnitTest):
         for k, v in six.iteritems(fixture_headers):
             req.headers[k] = v
 
-        ctlr = glance.api.v1.images.Controller
-
-        with contextlib.nested(
-                mock.patch.object(ctlr, '_external_source',
-                                  return_value=location),
-                mock.patch.object(store,
-                                  'get_store_from_location',
-                                  return_value=scheme)
-        ) as (
-            mock_external_source,
-            mock_get_store_from_location
-        ):
-            res = req.get_response(self.api)
-            self.assertEqual(400, res.status_int)
-            self.assertEqual(1, mock_external_source.call_count)
-            self.assertEqual(1, mock_get_store_from_location.call_count)
-            self.assertIn('Store for scheme %s not found' % scheme, res.body)
+        mock_external_source.return_value = location
+        mock_get_store_from_location.return_value = scheme
+
+        res = req.get_response(self.api)
+        self.assertEqual(400, res.status_int)
+        self.assertEqual(1, mock_external_source.call_count)
+        self.assertEqual(1, mock_get_store_from_location.call_count)
+        self.assertIn('Store for scheme %s not found' % scheme, res.body)
 
     def test_create_with_location_unknown_scheme(self):
         fixture_headers = {