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

2 files changed, 133 insertions, 43 deletions

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