openstack
/
keystone
Code Issues Proposed changes

Handle unicode at the caching layer more elegantly 

This patchset resolves an issue where in some cases unicode would
cause the cache key generator to raise a UnicodeEncodeError due to
the name/value being outside of the standard ascii character set.
Included is a fix to the cache backend debug code to utilize repr
for passing the keys/values to the logger.

Tests in test_backend provided by chenxiao <chenxiao@cn.ibm.com>

Closes-bug: 1237892
Change-Id: I4df1c9eb6b4a1367defdbb6bcbcd1f1f992348e5
This commit is contained in:
Morgan Fainberg 2013-10-13 18:34:24 -07:00 committed by Dolph Mathews
parent 8ba9898f42
commit a4ee3ae967
3 changed files with 98 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
keystone/common/cache/core.py vendored
View File

@ -28,10 +28,8 @@ from keystone.openstack.common import log
CONF = config.CONF
LOG = log.getLogger(__name__)
REGION = dogpile.cache.make_region()
make_region = dogpile.cache.make_region
on_arguments = REGION.cache_on_arguments
dogpile.cache.register_backend(
'keystone.common.cache.noop',
@ -41,33 +39,39 @@ dogpile.cache.register_backend(
class DebugProxy(proxy.ProxyBackend):
"""Extra Logging ProxyBackend."""
# NOTE(morganfainberg): Pass all key/values through repr to ensure we have
# a clean description of the information. Without use of repr, it might
# be possible to run into encode/decode error(s). For logging/debugging
# purposes encode/decode is irrelevant and we should be looking at the
# data exactly as it stands.
def get(self, key):
value = self.proxied.get(key)
msg = _('CACHE_GET: Key: "%(key)s" Value: "%(value)s"')
LOG.debug(msg % {'key': key, 'value': value})
LOG.debug(msg % {'key': repr(key), 'value': repr(value)})
return value
def get_multi(self, keys):
values = self.proxied.get_multi(keys)
msg = _('CACHE_GET_MULTI: "%(keys)s" Values: "%(values)s"')
LOG.debug(msg % {'keys': keys, 'values': values})
LOG.debug(msg % {'keys': repr(keys), 'values': repr(values)})
return values
def set(self, key, value):
msg = _('CACHE_SET: Key: "%(key)s" Value: "%(value)s"')
LOG.debug(msg % {'key': key, 'value': value})
LOG.debug(msg % {'key': repr(key), 'value': repr(value)})
return self.proxied.set(key, value)
def set_multi(self, keys):
LOG.debug(_('CACHE_SET_MULTI: "%s"') % keys)
LOG.debug(_('CACHE_SET_MULTI: "%s"') % repr(keys))
self.proxied.set_multi(keys)
def delete(self, key):
self.proxied.delete(key)
LOG.debug(_('CACHE_DELETE: "%s"') % key)
LOG.debug(_('CACHE_DELETE: "%s"') % repr(key))
def delete_multi(self, keys):
LOG.debug(_('CACHE_DELETE_MULTI: "%s"') % keys)
LOG.debug(_('CACHE_DELETE_MULTI: "%s"') % repr(keys))
self.proxied.delete_multi(keys)
@ -176,3 +180,23 @@ def should_cache_fn(section):
conf_group = getattr(CONF, section)
return getattr(conf_group, 'caching', True)
return should_cache
def key_generate_to_str(s):
# NOTE(morganfainberg): Since we need to stringify all arguments, attempt
# to stringify and handle the Unicode error explicitly as needed.
try:
return str(s)
except UnicodeEncodeError:
return s.encode('utf-8')
def function_key_generator(namespace, fn, to_str=key_generate_to_str):
# NOTE(morganfainberg): This wraps dogpile.cache's default
# function_key_generator to change the default to_str mechanism.
return util.function_key_generator(namespace, fn, to_str=to_str)
REGION = dogpile.cache.make_region(
function_key_generator=function_key_generator)
on_arguments = REGION.cache_on_arguments

10
keystone/tests/test_backend.py
View File

@ -1607,6 +1607,16 @@ class IdentityTests(object):
self.identity_api.delete_role,
uuid.uuid4().hex)
def test_create_update_delete_unicode_project(self):
unicode_project_name = u'name \u540d\u5b57'
project = {'id': uuid.uuid4().hex,
'name': unicode_project_name,
'description': uuid.uuid4().hex,
'domain_id': CONF.identity.default_domain_id}
self.assignment_api.create_project(project['id'], project)
self.assignment_api.update_project(project['id'], project)
self.assignment_api.delete_project(project['id'])
def test_create_project_case_sensitivity(self):
# create a ref with a lowercase name
ref = {

56
keystone/tests/test_keystoneclient.py
View File

@ -297,6 +297,62 @@ class KeystoneClientTests(object):
self.assertFalse([t for t in client.tenants.list()
if t.id == tenant.id])
def test_tenant_create_update_and_delete_unicode(self):
from keystoneclient import exceptions as client_exceptions
tenant_name = u'original \u540d\u5b57'
tenant_description = 'My original tenant!'
tenant_enabled = True
client = self.get_client(admin=True)
# create, get, and list a tenant
tenant = client.tenants.create(tenant_name,
description=tenant_description,
enabled=tenant_enabled)
self.assertEqual(tenant.name, tenant_name)
self.assertEqual(tenant.description, tenant_description)
self.assertIs(tenant.enabled, tenant_enabled)
tenant = client.tenants.get(tenant.id)
self.assertEqual(tenant.name, tenant_name)
self.assertEqual(tenant.description, tenant_description)
self.assertIs(tenant.enabled, tenant_enabled)
# multiple tenants exist due to fixtures, so find the one we're testing
tenant = [t for t in client.tenants.list() if t.id == tenant.id].pop()
self.assertEqual(tenant.name, tenant_name)
self.assertEqual(tenant.description, tenant_description)
self.assertIs(tenant.enabled, tenant_enabled)
# update, get, and list a tenant
tenant_name = u'updated \u540d\u5b57'
tenant_description = 'Updated tenant!'
tenant_enabled = False
tenant = client.tenants.update(tenant.id,
tenant_name=tenant_name,
enabled=tenant_enabled,
description=tenant_description)
self.assertEqual(tenant.name, tenant_name)
self.assertEqual(tenant.description, tenant_description)
self.assertIs(tenant.enabled, tenant_enabled)
tenant = client.tenants.get(tenant.id)
self.assertEqual(tenant.name, tenant_name)
self.assertEqual(tenant.description, tenant_description)
self.assertIs(tenant.enabled, tenant_enabled)
tenant = [t for t in client.tenants.list() if t.id == tenant.id].pop()
self.assertEqual(tenant.name, tenant_name)
self.assertEqual(tenant.description, tenant_description)
self.assertIs(tenant.enabled, tenant_enabled)
# delete, get, and list a tenant
client.tenants.delete(tenant.id)
self.assertRaises(client_exceptions.NotFound, client.tenants.get,
tenant.id)
self.assertFalse([t for t in client.tenants.list()
if t.id == tenant.id])
def test_tenant_create_no_name(self):
from keystoneclient import exceptions as client_exceptions
client = self.get_client(admin=True)