openstack
/
keystone
Code Issues Proposed changes

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
This commit is contained in:
Morgan Fainberg 2013-07-28 16:16:13 -07:00
parent 97a5b49d96
commit 5519e1ba16
6 changed files with 29 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
keystone/common/wsgi.py
View File

@ -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)

4
keystone/contrib/ec2/core.py
View File

@ -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,

10
keystone/token/controllers.py
View File

@ -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)

4
keystone/token/providers/uuid.py
View File

@ -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,

32
tests/test_backend.py
View File

@ -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',

12
tests/test_v3_identity.py
View File

@ -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)