From ab0254750a2ea991053b8865f019c1483d9c5fa3 Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Tue, 11 Oct 2016 11:44:44 +1100 Subject: [PATCH] Refactor get_manager_for_store in an OO manner get_manager_for_store is a weird function that switches based on the class type of the first parameter. This is an odd throw back to pre object orientated days where the object defines what it wants. Refactor it to put the class on the object. Change-Id: I0bca2607267aef3bda720cdfbbbe0e5a8093a20d --- glance_store/_drivers/swift/store.py | 73 +++++++++++---------- glance_store/tests/unit/test_swift_store.py | 16 ++--- 2 files changed, 43 insertions(+), 46 deletions(-) diff --git a/glance_store/_drivers/swift/store.py b/glance_store/_drivers/swift/store.py index 8d4065d2..d1cd53af 100644 --- a/glance_store/_drivers/swift/store.py +++ b/glance_store/_drivers/swift/store.py @@ -775,8 +775,8 @@ class BaseStore(driver.Store): # initialize manager to receive valid connections allow_retry = \ self.conf.glance_store.swift_store_retry_get_count > 0 - with get_manager_for_store(self, location, context, - allow_reauth=allow_retry) as manager: + with self.get_manager(location, context, + allow_reauth=allow_retry) as manager: (resp_headers, resp_body) = self._get_object(location, manager=manager) @@ -832,8 +832,8 @@ class BaseStore(driver.Store): # initialize a manager with re-auth if image need to be splitted need_chunks = (image_size == 0) or ( image_size >= self.large_object_size) - with get_manager_for_store(self, location, context, - allow_reauth=need_chunks) as manager: + with self.get_manager(location, context, + allow_reauth=need_chunks) as manager: self._create_container_if_missing(location.container, manager.get_connection()) @@ -1090,6 +1090,23 @@ class BaseStore(driver.Store): ssl_compression=self.ssl_compression, cacert=self.cacert) + def get_manager(self, store_location, context=None, allow_reauth=False): + """Return appropriate connection manager for store + + The method detects store type (singletenant or multitenant) and returns + appropriate connection manager (singletenant or multitenant) that + allows to request swiftclient connections. + + :param store_location: StoreLocation object that define image location + :param context: user context + :param allow_reauth: defines if we allow re-authentication when user + token is expired and refresh swift connection + + :return: connection manager for store + """ + msg = _("There is no Connection Manager implemented for %s class.") + raise NotImplementedError(msg % self.__class__.__name__) + class SingleTenantStore(BaseStore): EXAMPLE_URL = "swift://:@//" @@ -1260,6 +1277,12 @@ class SingleTenantStore(BaseStore): return ks_client.Client(session=sess) + def get_manager(self, store_location, context=None, allow_reauth=False): + return connection_manager.SingleTenantConnectionManager(self, + store_location, + context, + allow_reauth) + class MultiTenantStore(BaseStore): EXAMPLE_URL = "swift:////" @@ -1423,6 +1446,17 @@ class MultiTenantStore(BaseStore): client_sess = ks_session.Session(auth=client_password) return ks_client.Client(session=client_sess) + def get_manager(self, store_location, context=None, allow_reauth=False): + # if global toggle is turned off then do not allow re-authentication + # with trusts + if not self.conf.glance_store.swift_store_use_trusts: + allow_reauth = False + + return connection_manager.MultiTenantConnectionManager(self, + store_location, + context, + allow_reauth) + class ChunkReader(object): def __init__(self, fd, checksum, total, verifier=None): @@ -1453,34 +1487,3 @@ class ChunkReader(object): if self.verifier: self.verifier.update(result) return result - - -def get_manager_for_store(store, store_location, - context=None, - allow_reauth=False): - """Return appropriate connection manager for store - - The method detects store type (singletenant or multitenant) and returns - appropriate connection manager (singletenant or multitenant) that allows - to request swiftclient connections. - :param store: store that needs swift connections - :param store_location: StoreLocation object that define image location - :param context: user context - :param allow_reauth: defines if we allow re-authentication when user token - is expired and refresh swift connection - :return: connection manager for store - """ - if store.__class__ == SingleTenantStore: - return connection_manager.SingleTenantConnectionManager( - store, store_location, context, allow_reauth) - elif store.__class__ == MultiTenantStore: - # if global toggle is turned off then do not allow re-authentication - # with trusts - if not store.conf.glance_store.swift_store_use_trusts: - allow_reauth = False - return connection_manager.MultiTenantConnectionManager( - store, store_location, context, allow_reauth) - else: - raise NotImplementedError(_("There is no Connection Manager " - "implemented for %s class.") % - store.__class__.__name__) diff --git a/glance_store/tests/unit/test_swift_store.py b/glance_store/tests/unit/test_swift_store.py index e30febe4..427f06a3 100644 --- a/glance_store/tests/unit/test_swift_store.py +++ b/glance_store/tests/unit/test_swift_store.py @@ -292,8 +292,7 @@ class SwiftTests(object): resp_full = b''.join([chunk for chunk in image_swift.wrapped]) resp_half = resp_full[:len(resp_full) // 2] resp_half = six.BytesIO(resp_half) - manager = swift.get_manager_for_store(self.store, loc.store_location, - ctxt) + manager = self.store.get_manager(loc.store_location, ctxt) image_swift.wrapped = swift.swift_retry_iter(resp_half, image_size, self.store, @@ -1108,9 +1107,7 @@ class SwiftTests(object): store = Store(self.conf) store.configure() loc = mock.MagicMock() - swift.get_manager_for_store(store, loc) - self.assertEqual(swift.get_manager_for_store(store, loc), - manager) + self.assertEqual(store.get_manager(loc), manager) @mock.patch("glance_store._drivers.swift." "connection_manager.SingleTenantConnectionManager") @@ -1120,15 +1117,12 @@ class SwiftTests(object): store = Store(self.conf) store.configure() loc = mock.MagicMock() - swift.get_manager_for_store(store, loc) - self.assertEqual(swift.get_manager_for_store(store, loc), - manager) + self.assertEqual(store.get_manager(loc), manager) def test_get_connection_manager_failed(self): - store = mock.MagicMock() + store = swift.BaseStore(mock.MagicMock()) loc = mock.MagicMock() - self.assertRaises(NotImplementedError, swift.get_manager_for_store, - store, loc) + self.assertRaises(NotImplementedError, store.get_manager, loc) @mock.patch("glance_store._drivers.swift.store.ks_v3") @mock.patch("glance_store._drivers.swift.store.ks_session")