From 871a3e40f306f1bff056a86edcd644a82ab29c67 Mon Sep 17 00:00:00 2001
From: Alistair Coles <alistair.coles@hp.com>
Date: Wed, 15 Oct 2014 18:56:58 +0100
Subject: [PATCH] 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
---
swiftclient/client.py | 2 +-
tests/unit/test_swiftclient.py | 98 ++++++++++++++++++++++++++++++++++
tests/unit/utils.py | 3 ++
3 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/swiftclient/client.py b/swiftclient/client.py
index def40bf4..8372e63b 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 1a5c7721..e366f7de 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 09b31c13..c149abf2 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