Move existing attributes to _id suffixed attributes

There is confusion now between whether parameters refer to the name or
id. Similar to adding _name we should rename the other variables with
_id to make it more obvious.

Change-Id: I203acefae8270bd3373b006fa096bea5ef3106f3
This commit is contained in:
Jamie Lennox 2016-05-12 12:44:05 +10:00
parent b788a184a0
commit f25543fcc7
3 changed files with 133 additions and 42 deletions

View File

@ -32,6 +32,7 @@ import threading
import uuid
import warnings
import debtcollector
from positional import positional
@ -118,6 +119,38 @@ class _DeprecatedPolicyValues(collections.MutableMapping):
return d
def _moved_msg(new_name, old_name):
if old_name:
deprecated_msg = "Property '%(old_name)s' has moved to '%(new_name)s'"
deprecated_msg = deprecated_msg % {'old_name': old_name,
'new_name': new_name}
debtcollector.deprecate(deprecated_msg,
version='2.6',
removal_version='3.0',
stacklevel=5)
def _moved_property(new_name, old_name=None, target=None):
if not target:
target = new_name
def getter(self):
_moved_msg(new_name, old_name)
return getattr(self, target)
def setter(self, value):
_moved_msg(new_name, old_name)
setattr(self, target, value)
def deleter(self):
_moved_msg(new_name, old_name)
delattr(self, target)
return property(getter, setter, deleter)
class RequestContext(object):
"""Helper class to represent useful information about a request context.
@ -145,18 +178,18 @@ class RequestContext(object):
True for backwards compatibility.
:type is_admin_project: bool
"""
# setting to private variables to avoid triggering subclass properties
self._user_id = user
self._project_id = tenant
self._domain_id = domain
self._user_domain_id = user_domain
self._project_domain_id = project_domain
self.auth_token = auth_token
self.user = user
self.user_name = user_name
# NOTE (rbradfor): tenant will become project
# See spec discussion on https://review.openstack.org/#/c/290907/
self.tenant = tenant
self.project_name = project_name
self.domain = domain
self.domain_name = domain_name
self.user_domain = user_domain
self.user_domain_name = user_domain_name
self.project_domain = project_domain
self.project_domain_name = project_domain_name
self.is_admin = is_admin
self.is_admin_project = is_admin_project
@ -170,6 +203,25 @@ class RequestContext(object):
if overwrite or not get_current():
self.update_store()
# NOTE(jamielennox): To prevent circular lookups on subclasses that might
# point user to user_id we make user/user_id tenant/project_id etc point
# to the same private variable rather than each other.
tenant = _moved_property('project_id', 'tenant', target='_project_id')
user = _moved_property('user_id', 'user', target='_user_id')
domain = _moved_property('domain_id', 'domain', target='_domain_id')
user_domain = _moved_property('user_domain_id',
'user_domain',
target='_user_domain_id')
project_domain = _moved_property('project_domain_id',
'project_domain',
target='_project_domain_id')
user_id = _moved_property('_user_id')
project_id = _moved_property('_project_id')
domain_id = _moved_property('_domain_id')
user_domain_id = _moved_property('_user_domain_id')
project_domain_id = _moved_property('_project_domain_id')
def update_store(self):
"""Store the context in the current thread."""
_request_store.context = self
@ -191,27 +243,27 @@ class RequestContext(object):
# of our standard ones. This object acts like a dict but only values
# from oslo.policy don't show a warning.
return _DeprecatedPolicyValues({
'user_id': self.user,
'user_domain_id': self.user_domain,
'project_id': self.tenant,
'project_domain_id': self.project_domain,
'user_id': self.user_id,
'user_domain_id': self.user_domain_id,
'project_id': self.project_id,
'project_domain_id': self.project_domain_id,
'roles': self.roles,
'is_admin_project': self.is_admin_project})
def to_dict(self):
"""Return a dictionary of context attributes."""
user_idt = (
self.user_idt_format.format(user=self.user or '-',
tenant=self.tenant or '-',
domain=self.domain or '-',
user_domain=self.user_domain or '-',
p_domain=self.project_domain or '-'))
user_idt = self.user_idt_format.format(
user=self.user_id or '-',
tenant=self.project_id or '-',
domain=self.domain_id or '-',
user_domain=self.user_domain_id or '-',
p_domain=self.project_domain_id or '-')
return {'user': self.user,
'tenant': self.tenant,
'domain': self.domain,
'user_domain': self.user_domain,
'project_domain': self.project_domain,
return {'user': self.user_id,
'tenant': self.project_id,
'domain': self.domain_id,
'user_domain': self.user_domain_id,
'project_domain': self.project_domain_id,
'is_admin': self.is_admin,
'read_only': self.read_only,
'show_deleted': self.show_deleted,

View File

@ -43,6 +43,12 @@ class WarningsFixture(fixtures.Fixture):
self.addCleanup(self._w.__exit__)
warnings.simplefilter(self.action, self.category)
def __len__(self):
return len(self.log)
def __getitem__(self, item):
return self.log[item]
class Object(object):
pass
@ -93,7 +99,7 @@ class ContextTest(test_base.BaseTestCase):
self.assertIsInstance(ctx, context.RequestContext)
self.assertTrue(ctx.is_admin)
self.assertFalse(ctx.show_deleted)
self.assertIsNone(ctx.tenant)
self.assertIsNone(ctx.project_id)
def test_admin_context_show_deleted_flag_set(self):
ctx = context.get_admin_context(show_deleted=True)
@ -122,11 +128,11 @@ class ContextTest(test_base.BaseTestCase):
}
ctx = context.RequestContext.from_dict(dct)
self.assertEqual(dct['auth_token'], ctx.auth_token)
self.assertEqual(dct['user'], ctx.user)
self.assertEqual(dct['tenant'], ctx.tenant)
self.assertEqual(dct['domain'], ctx.domain)
self.assertEqual(dct['user_domain'], ctx.user_domain)
self.assertEqual(dct['project_domain'], ctx.project_domain)
self.assertEqual(dct['user'], ctx.user_id)
self.assertEqual(dct['tenant'], ctx.project_id)
self.assertEqual(dct['domain'], ctx.domain_id)
self.assertEqual(dct['user_domain'], ctx.user_domain_id)
self.assertEqual(dct['project_domain'], ctx.project_domain_id)
self.assertTrue(ctx.is_admin)
self.assertTrue(ctx.read_only)
self.assertTrue(ctx.show_deleted)
@ -149,8 +155,8 @@ class ContextTest(test_base.BaseTestCase):
}
ctx = context.RequestContext.from_dict(dct)
self.assertEqual("token1", ctx.auth_token)
self.assertEqual("user1", ctx.user)
self.assertIsNone(ctx.tenant)
self.assertEqual("user1", ctx.user_id)
self.assertIsNone(ctx.project_id)
self.assertFalse(ctx.is_admin)
self.assertTrue(ctx.read_only)
self.assertRaises(KeyError, lambda: ctx.__dict__['color'])
@ -210,13 +216,13 @@ class ContextTest(test_base.BaseTestCase):
ctx = context.RequestContext.from_environ(environ)
self.assertEqual(auth_token, ctx.auth_token)
self.assertEqual(user_id, ctx.user)
self.assertEqual(user_id, ctx.user_id)
self.assertEqual(user_name, ctx.user_name)
self.assertEqual(project_id, ctx.tenant)
self.assertEqual(project_id, ctx.project_id)
self.assertEqual(project_name, ctx.project_name)
self.assertEqual(user_domain_id, ctx.user_domain)
self.assertEqual(user_domain_id, ctx.user_domain_id)
self.assertEqual(user_domain_name, ctx.user_domain_name)
self.assertEqual(project_domain_id, ctx.project_domain)
self.assertEqual(project_domain_id, ctx.project_domain_id)
self.assertEqual(project_domain_name, ctx.project_domain_name)
self.assertEqual(roles, ctx.roles)
self.assertEqual(request_id, ctx.request_id)
@ -237,7 +243,7 @@ class ContextTest(test_base.BaseTestCase):
environ = {'HTTP_X_TENANT_ID': value}
ctx = context.RequestContext.from_environ(environ=environ)
self.assertEqual(value, ctx.tenant)
self.assertEqual(value, ctx.project_id)
environ = {'HTTP_X_STORAGE_TOKEN': value}
ctx = context.RequestContext.from_environ(environ=environ)
@ -274,11 +280,11 @@ class ContextTest(test_base.BaseTestCase):
'HTTP_X_PROJECT_ID': new}
ctx = context.RequestContext.from_environ(environ=environ)
self.assertEqual(new, ctx.tenant)
self.assertEqual(new, ctx.project_id)
ctx = context.RequestContext.from_environ(environ=environ,
tenant=override)
self.assertEqual(override, ctx.tenant)
self.assertEqual(override, ctx.project_id)
environ = {'HTTP_X_TENANT_NAME': old,
'HTTP_X_PROJECT_NAME': new}
@ -358,15 +364,15 @@ class ContextTest(test_base.BaseTestCase):
request_id=request_id,
resource_uuid=resource_uuid)
self.assertEqual(auth_token, ctx.auth_token)
self.assertEqual(user_id, ctx.user)
self.assertEqual(user_id, ctx.user_id)
self.assertEqual(user_name, ctx.user_name)
self.assertEqual(project_id, ctx.tenant)
self.assertEqual(project_id, ctx.project_id)
self.assertEqual(project_name, ctx.project_name)
self.assertEqual(domain_id, ctx.domain)
self.assertEqual(domain_id, ctx.domain_id)
self.assertEqual(domain_name, ctx.domain_name)
self.assertEqual(user_domain_id, ctx.user_domain)
self.assertEqual(user_domain_id, ctx.user_domain_id)
self.assertEqual(user_domain_name, ctx.user_domain_name)
self.assertEqual(project_domain_id, ctx.project_domain)
self.assertEqual(project_domain_id, ctx.project_domain_id)
self.assertEqual(project_domain_name, ctx.project_domain_name)
self.assertEqual(is_admin, ctx.is_admin)
self.assertEqual(read_only, ctx.read_only)
@ -521,3 +527,35 @@ class ContextTest(test_base.BaseTestCase):
self.assertIs(val, policy[key])
self.assertEqual(1, len(w))
self.assertIn(key, str(w[0].message))
def test_deprecated_args(self):
user = uuid.uuid4().hex
tenant = uuid.uuid4().hex
domain = uuid.uuid4().hex
user_domain = uuid.uuid4().hex
project_domain = uuid.uuid4().hex
ctx = context.RequestContext(user=user,
tenant=tenant,
domain=domain,
user_domain=user_domain,
project_domain=project_domain)
self.assertEqual(0, len(self.warnings))
self.assertEqual(user, ctx.user_id)
self.assertEqual(tenant, ctx.project_id)
self.assertEqual(domain, ctx.domain_id)
self.assertEqual(user_domain, ctx.user_domain_id)
self.assertEqual(project_domain, ctx.project_domain_id)
self.assertEqual(0, len(self.warnings))
self.assertEqual(user, ctx.user)
self.assertEqual(1, len(self.warnings))
self.assertEqual(tenant, ctx.tenant)
self.assertEqual(2, len(self.warnings))
self.assertEqual(domain, ctx.domain)
self.assertEqual(3, len(self.warnings))
self.assertEqual(user_domain, ctx.user_domain)
self.assertEqual(4, len(self.warnings))
self.assertEqual(project_domain, ctx.project_domain)
self.assertEqual(5, len(self.warnings))

View File

@ -4,4 +4,5 @@
pbr>=1.8 # Apache-2.0
debtcollector>=1.2.0 # Apache-2.0
positional>=1.1.1 # Apache-2.0