From f25543fcc792ebf155728a91fde06e8dc4e96cea Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Thu, 12 May 2016 12:44:05 +1000 Subject: 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 --- oslo_context/context.py | 98 +++++++++++++++++++++++++++++--------- oslo_context/tests/test_context.py | 78 ++++++++++++++++++++++-------- 2 files changed, 133 insertions(+), 43 deletions(-) (limited to 'oslo_context') diff --git a/oslo_context/context.py b/oslo_context/context.py index 481b18c..e4aeba9 100644 --- a/oslo_context/context.py +++ b/oslo_context/context.py @@ -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 '-')) - - return {'user': self.user, - 'tenant': self.tenant, - 'domain': self.domain, - 'user_domain': self.user_domain, - 'project_domain': self.project_domain, + 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_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, diff --git a/oslo_context/tests/test_context.py b/oslo_context/tests/test_context.py index 9a32fac..23c875f 100644 --- a/oslo_context/tests/test_context.py +++ b/oslo_context/tests/test_context.py @@ -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)) -- cgit v1.2.1