From 5519e1ba16f59f302cda6e8db8f5423eed376ddf Mon Sep 17 00:00:00 2001 From: Morgan Fainberg Date: Sun, 28 Jul 2013 16:16:13 -0700 Subject: Remove kwargs from manager calls where not needed. This patch removes the use of kwargs from manager calls where not required. Dogpile.cache (the targeted caching library) does not support kwargs out of the box with its cache-key-generator. This change allows us to support the default cache-key-generator; while it is possible to create a new cache-key-generator function, there are many possible edge-cases to deal with when making cache invalidation calls (ensuring the arguments are the same) as well as possible performance implications (depending on the depth of method introspection needed to determine how to invalidate the cache). As an added bonus, this change brings the code touched more in-line with the rest of keystone where most manager/driver calls do not use kwargs unless absolutley required. blueprint: caching-layer-for-driver-calls Change-Id: I035c976314fb48f657661f681f7c1760d3c547a6 --- keystone/common/wsgi.py | 3 +-- keystone/contrib/ec2/core.py | 4 +--- keystone/token/controllers.py | 10 ++++------ keystone/token/providers/uuid.py | 4 ++-- tests/test_backend.py | 32 +++++++++++++++----------------- tests/test_v3_identity.py | 12 ++++++------ 6 files changed, 29 insertions(+), 36 deletions(-) diff --git a/keystone/common/wsgi.py b/keystone/common/wsgi.py index c7e30576..fbc63385 100644 --- a/keystone/common/wsgi.py +++ b/keystone/common/wsgi.py @@ -283,8 +283,7 @@ class Application(BaseApplication): def assert_admin(self, context): if not context['is_admin']: try: - user_token_ref = self.token_api.get_token( - token_id=context['token_id']) + user_token_ref = self.token_api.get_token(context['token_id']) except exception.TokenNotFound as e: raise exception.Unauthorized(e) diff --git a/keystone/contrib/ec2/core.py b/keystone/contrib/ec2/core.py index adba7e74..8f72e431 100644 --- a/keystone/contrib/ec2/core.py +++ b/keystone/contrib/ec2/core.py @@ -188,9 +188,7 @@ class Ec2Controller(controller.V2Controller): for role_id in roles] catalog_ref = self.catalog_api.get_catalog( - user_id=user_ref['id'], - tenant_id=tenant_ref['id'], - metadata=metadata_ref) + user_ref['id'], tenant_ref['id'], metadata_ref) auth_token_data = dict(user=user_ref, tenant=tenant_ref, diff --git a/keystone/token/controllers.py b/keystone/token/controllers.py index d2b7d9fd..9ebc29fe 100644 --- a/keystone/token/controllers.py +++ b/keystone/token/controllers.py @@ -91,9 +91,7 @@ class Auth(controller.V2Controller): if tenant_ref: catalog_ref = self.catalog_api.get_catalog( - user_id=user_ref['id'], - tenant_id=tenant_ref['id'], - metadata=metadata_ref) + user_ref['id'], tenant_ref['id'], metadata_ref) else: catalog_ref = {} @@ -457,9 +455,9 @@ class Auth(controller.V2Controller): catalog_ref = None if token_ref.get('tenant'): catalog_ref = self.catalog_api.get_catalog( - user_id=token_ref['user']['id'], - tenant_id=token_ref['tenant']['id'], - metadata=token_ref['metadata']) + token_ref['user']['id'], + token_ref['tenant']['id'], + token_ref['metadata']) return Auth.format_endpoint_list(catalog_ref) diff --git a/keystone/token/providers/uuid.py b/keystone/token/providers/uuid.py index 94896920..acfa9372 100644 --- a/keystone/token/providers/uuid.py +++ b/keystone/token/providers/uuid.py @@ -438,7 +438,7 @@ class Provider(token.provider.Provider): def _verify_token(self, token_id, belongs_to=None): """Verify the given token and return the token_ref.""" - token_ref = self.token_api.get_token(token_id=token_id) + token_ref = self.token_api.get_token(token_id) assert token_ref if belongs_to: assert (token_ref['tenant'] and @@ -518,7 +518,7 @@ class Provider(token.provider.Provider): catalog_ref = self.catalog_api.get_catalog( token_ref['user']['id'], token_ref['tenant']['id'], - metadata=metadata_ref) + metadata_ref) token_data = self.v2_token_data_helper.get_token_data( token_ref=token_ref, roles_ref=role_refs, diff --git a/tests/test_backend.py b/tests/test_backend.py index a43e92ae..7e4d820e 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -118,26 +118,25 @@ class IdentityTests(object): self.assertEqual(unicode_name, ref['name']) def test_get_project(self): - tenant_ref = self.identity_api.get_project( - tenant_id=self.tenant_bar['id']) + tenant_ref = self.identity_api.get_project(self.tenant_bar['id']) self.assertDictEqual(tenant_ref, self.tenant_bar) def test_get_project_404(self): self.assertRaises(exception.ProjectNotFound, self.identity_api.get_project, - tenant_id=uuid.uuid4().hex) + uuid.uuid4().hex) def test_get_project_by_name(self): tenant_ref = self.identity_api.get_project_by_name( - tenant_name=self.tenant_bar['name'], - domain_id=DEFAULT_DOMAIN_ID) + self.tenant_bar['name'], + DEFAULT_DOMAIN_ID) self.assertDictEqual(tenant_ref, self.tenant_bar) def test_get_project_by_name_404(self): self.assertRaises(exception.ProjectNotFound, self.identity_api.get_project_by_name, - tenant_name=uuid.uuid4().hex, - domain_id=DEFAULT_DOMAIN_ID) + uuid.uuid4().hex, + DEFAULT_DOMAIN_ID) def test_get_project_users(self): tenant_ref = self.identity_api.get_project_users(self.tenant_baz['id']) @@ -152,10 +151,10 @@ class IdentityTests(object): def test_get_project_users_404(self): self.assertRaises(exception.ProjectNotFound, self.identity_api.get_project_users, - tenant_id=uuid.uuid4().hex) + uuid.uuid4().hex) def test_get_user(self): - user_ref = self.identity_api.get_user(user_id=self.user_foo['id']) + user_ref = self.identity_api.get_user(self.user_foo['id']) # NOTE(termie): the password field is left in user_foo to make # it easier to authenticate in tests, but should # not be returned by the api @@ -165,12 +164,12 @@ class IdentityTests(object): def test_get_user_404(self): self.assertRaises(exception.UserNotFound, self.identity_api.get_user, - user_id=uuid.uuid4().hex) + uuid.uuid4().hex) def test_get_user_by_name(self): user_ref = self.identity_api.get_user_by_name( - user_name=self.user_foo['name'], - domain_id=DEFAULT_DOMAIN_ID) + self.user_foo['name'], DEFAULT_DOMAIN_ID) + # NOTE(termie): the password field is left in user_foo to make # it easier to authenticate in tests, but should # not be returned by the api @@ -180,19 +179,18 @@ class IdentityTests(object): def test_get_user_by_name_404(self): self.assertRaises(exception.UserNotFound, self.identity_api.get_user_by_name, - user_name=uuid.uuid4().hex, - domain_id=DEFAULT_DOMAIN_ID) + uuid.uuid4().hex, + DEFAULT_DOMAIN_ID) def test_get_role(self): - role_ref = self.identity_api.get_role( - role_id=self.role_admin['id']) + role_ref = self.identity_api.get_role(self.role_admin['id']) role_ref_dict = dict((x, role_ref[x]) for x in role_ref) self.assertDictEqual(role_ref_dict, self.role_admin) def test_get_role_404(self): self.assertRaises(exception.RoleNotFound, self.identity_api.get_role, - role_id=uuid.uuid4().hex) + uuid.uuid4().hex) def test_create_duplicate_role_name_fails(self): role = {'id': 'fake1', diff --git a/tests/test_v3_identity.py b/tests/test_v3_identity.py index 5eaf9085..2f191580 100644 --- a/tests/test_v3_identity.py +++ b/tests/test_v3_identity.py @@ -266,19 +266,19 @@ class IdentityTestCase(test_v3.RestfulTestCase): # Check all the domain2 relevant entities are gone self.assertRaises(exception.DomainNotFound, self.identity_api.get_domain, - domain_id=self.domain2['id']) + self.domain2['id']) self.assertRaises(exception.ProjectNotFound, self.identity_api.get_project, - tenant_id=self.project2['id']) + self.project2['id']) self.assertRaises(exception.GroupNotFound, self.identity_api.get_group, - group_id=self.group2['id']) + self.group2['id']) self.assertRaises(exception.UserNotFound, self.identity_api.get_user, - user_id=self.user2['id']) + self.user2['id']) self.assertRaises(exception.CredentialNotFound, self.credential_api.get_credential, - credential_id=self.credential2['id']) + self.credential2['id']) # ...and that all self.domain entities are still here r = self.identity_api.get_domain(self.domain['id']) @@ -511,7 +511,7 @@ class IdentityTestCase(test_v3.RestfulTestCase): # that reference this project self.assertRaises(exception.CredentialNotFound, self.credential_api.get_credential, - credential_id=self.credential['id']) + self.credential['id']) # And the no tokens we remain valid tokens = self.token_api.list_tokens(self.user['id']) self.assertEquals(len(tokens), 0) -- cgit