Fix KeyError raised from client Connection

Some client.Connection methods will raise a KeyError
if a response_dict argument is passed and an error
occurs during authentication or making the request.

The fix is straightforward: add a test for existence
of a response_dict before attempting to get it from
kwargs.

The bulk of this patch is adding unit tests for the
response_dict feature.

Closes-Bug: 1381304

Change-Id: Ic7e1b3dfae33909533931c52ac97355867a08a07

3 files changed, 102 insertions, 1 deletions

diff --git a/swiftclient/client.py b/swiftclient/client.py
index def40bf..8372e63 100644
--- a/swiftclient/client.py
+++ b/swiftclient/client.py
@@ -1215,7 +1215,7 @@ class Connection(object):
                                ssl_compression=self.ssl_compression)
 
     def _add_response_dict(self, target_dict, kwargs):
-        if target_dict is not None:
+        if target_dict is not None and 'response_dict' in kwargs:
             response_dict = kwargs['response_dict']
             if 'response_dicts' in target_dict:
                 target_dict['response_dicts'].append(response_dict)
diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py
index 1a5c772..e366f7d 100644
--- a/tests/unit/test_swiftclient.py
+++ b/tests/unit/test_swiftclient.py
@@ -1118,6 +1118,104 @@ class TestConnection(MockHttpTest):
             c.http_connection = orig_conn
 
 
+class TestResponseDict(MockHttpTest):
+    """
+    Verify handling of optional response_dict argument.
+    """
+    calls = [('post_container', 'c', {}),
+             ('put_container', 'c'),
+             ('delete_container', 'c'),
+             ('post_object', 'c', 'o', {}),
+             ('put_object', 'c', 'o', 'body'),
+             ('delete_object', 'c', 'o')]
+
+    def fake_get_auth(*args, **kwargs):
+        return 'http://url', 'token'
+
+    def test_response_dict_with_auth_error(self):
+        def bad_get_auth(*args, **kwargs):
+            raise c.ClientException('test')
+
+        for call in self.calls:
+            resp_dict = {'test': 'should be untouched'}
+            with mock.patch('swiftclient.client.get_auth',
+                            bad_get_auth):
+                conn = c.Connection('http://127.0.0.1:8080', 'user', 'key')
+                self.assertRaises(c.ClientException, getattr(conn, call[0]),
+                                  *call[1:], response_dict=resp_dict)
+
+            self.assertEqual({'test': 'should be untouched'}, resp_dict)
+
+    def test_response_dict_with_request_error(self):
+        for call in self.calls:
+            resp_dict = {'test': 'should be untouched'}
+            with mock.patch('swiftclient.client.get_auth',
+                            self.fake_get_auth):
+                exc = c.ClientException('test')
+                with mock.patch('swiftclient.client.http_connection',
+                                self.fake_http_connection(200, exc=exc)):
+                    conn = c.Connection('http://127.0.0.1:8080', 'user', 'key')
+                    self.assertRaises(c.ClientException,
+                                      getattr(conn, call[0]),
+                                      *call[1:],
+                                      response_dict=resp_dict)
+
+            self.assertTrue('test' in resp_dict)
+            self.assertEqual('should be untouched', resp_dict['test'])
+            self.assertTrue('response_dicts' in resp_dict)
+            self.assertEqual([{}], resp_dict['response_dicts'])
+
+    def test_response_dict(self):
+        # test response_dict is populated and
+        # new list of response_dicts is created
+        for call in self.calls:
+            resp_dict = {'test': 'should be untouched'}
+            with mock.patch('swiftclient.client.get_auth',
+                            self.fake_get_auth):
+                with mock.patch('swiftclient.client.http_connection',
+                                self.fake_http_connection(200)):
+                    conn = c.Connection('http://127.0.0.1:8080', 'user', 'key')
+                    getattr(conn, call[0])(*call[1:], response_dict=resp_dict)
+
+            for key in ('test', 'status', 'headers', 'reason',
+                        'response_dicts'):
+                self.assertTrue(key in resp_dict)
+            self.assertEqual('should be untouched', resp_dict.pop('test'))
+            self.assertEqual('Fake', resp_dict['reason'])
+            self.assertEqual(200, resp_dict['status'])
+            self.assertTrue('x-works' in resp_dict['headers'])
+            self.assertEqual('yes', resp_dict['headers']['x-works'])
+            children = resp_dict.pop('response_dicts')
+            self.assertEqual(1, len(children))
+            self.assertEqual(resp_dict, children[0])
+
+    def test_response_dict_with_existing(self):
+        # check response_dict is populated and new dict is appended
+        # to existing response_dicts list
+        for call in self.calls:
+            resp_dict = {'test': 'should be untouched',
+                         'response_dicts': [{'existing': 'response dict'}]}
+            with mock.patch('swiftclient.client.get_auth',
+                            self.fake_get_auth):
+                with mock.patch('swiftclient.client.http_connection',
+                                self.fake_http_connection(200)):
+                    conn = c.Connection('http://127.0.0.1:8080', 'user', 'key')
+                    getattr(conn, call[0])(*call[1:], response_dict=resp_dict)
+
+            for key in ('test', 'status', 'headers', 'reason',
+                        'response_dicts'):
+                self.assertTrue(key in resp_dict)
+            self.assertEqual('should be untouched', resp_dict.pop('test'))
+            self.assertEqual('Fake', resp_dict['reason'])
+            self.assertEqual(200, resp_dict['status'])
+            self.assertTrue('x-works' in resp_dict['headers'])
+            self.assertEqual('yes', resp_dict['headers']['x-works'])
+            children = resp_dict.pop('response_dicts')
+            self.assertEqual(2, len(children))
+            self.assertEqual({'existing': 'response dict'}, children[0])
+            self.assertEqual(resp_dict, children[1])
+
+
 class TestLogging(MockHttpTest):
     """
     Make sure all the lines in http_log are covered.
diff --git a/tests/unit/utils.py b/tests/unit/utils.py
index 09b31c1..c149abf 100644
--- a/tests/unit/utils.py
+++ b/tests/unit/utils.py
@@ -172,6 +172,7 @@ class MockHttpTest(testtools.TestCase):
             query_string = kwargs.get('query_string')
             storage_url = kwargs.get('storage_url')
             auth_token = kwargs.get('auth_token')
+            exc = kwargs.get('exc')
 
             def wrapper(url, proxy=None, cacert=None, insecure=False,
                         ssl_compression=True):
@@ -192,6 +193,8 @@ class MockHttpTest(testtools.TestCase):
                     if url.endswith('invalid_cert') and not insecure:
                         from swiftclient import client as c
                         raise c.ClientException("invalid_certificate")
+                    elif exc:
+                        raise exc
                     return
                 conn.request = request