From cef4fbc9971e87cc822900ef3b6e0ec6029e5fa0 Mon Sep 17 00:00:00 2001
From: Morgan Fainberg <morgan.fainberg@gmail.com>
Date: Wed, 18 Jun 2014 18:38:19 -0700
Subject: [PATCH] Properly invalidate cache for get_*_by_name methods

Use the original name for invalidating the cache on the
get_project_by_name and get_domain_by_name when performing update
operations on the name attributes.

Change-Id: If58a3b0a47501096f9fa9eb9cd7a6057f7b29298
Closes-Bug: #1311142
---
 keystone/assignment/core.py         |  8 ++++---
 keystone/tests/test_backend.py      | 33 +++++++++++++++++++++++++++++
 keystone/tests/test_backend_ldap.py | 12 +++++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py
index f1fa75cb0c..a695f32bb6 100644
--- a/keystone/assignment/core.py
+++ b/keystone/assignment/core.py
@@ -91,6 +91,7 @@ class Manager(manager.Manager):
 
     @notifications.updated(_PROJECT)
     def update_project(self, tenant_id, tenant):
+        original_tenant = self.driver.get_project(tenant_id)
         tenant = tenant.copy()
         if 'enabled' in tenant:
             tenant['enabled'] = clean.project_enabled(tenant['enabled'])
@@ -98,8 +99,8 @@ class Manager(manager.Manager):
             self._disable_project(tenant_id)
         ret = self.driver.update_project(tenant_id, tenant)
         self.get_project.invalidate(self, tenant_id)
-        self.get_project_by_name.invalidate(self, ret['name'],
-                                            ret['domain_id'])
+        self.get_project_by_name.invalidate(self, original_tenant['name'],
+                                            original_tenant['domain_id'])
         return ret
 
     @notifications.deleted(_PROJECT)
@@ -320,6 +321,7 @@ class Manager(manager.Manager):
 
     @notifications.updated('domain')
     def update_domain(self, domain_id, domain):
+        original_domain = self.driver.get_domain(domain_id)
         if 'enabled' in domain:
             domain['enabled'] = clean.domain_enabled(domain['enabled'])
         ret = self.driver.update_domain(domain_id, domain)
@@ -328,7 +330,7 @@ class Manager(manager.Manager):
         if not domain.get('enabled', True):
             self._disable_domain(domain_id)
         self.get_domain.invalidate(self, domain_id)
-        self.get_domain_by_name.invalidate(self, ret['name'])
+        self.get_domain_by_name.invalidate(self, original_domain['name'])
         return ret
 
     @notifications.deleted('domain')
diff --git a/keystone/tests/test_backend.py b/keystone/tests/test_backend.py
index 216cf588fa..ac92e486ab 100644
--- a/keystone/tests/test_backend.py
+++ b/keystone/tests/test_backend.py
@@ -2755,6 +2755,20 @@ class IdentityTests(object):
         user_projects = self.assignment_api.list_projects_for_user(user1['id'])
         self.assertEqual(len(user_projects), 3)
 
+    @tests.skip_if_cache_disabled('assignment')
+    def test_domain_rename_invalidates_get_domain_by_name_cache(self):
+        domain = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
+                  'enabled': True}
+        domain_id = domain['id']
+        domain_name = domain['name']
+        self.assignment_api.create_domain(domain_id, domain)
+        domain_ref = self.assignment_api.get_domain(domain_id)
+        domain_ref['name'] = uuid.uuid4().hex
+        self.assignment_api.update_domain(domain_id, domain_ref)
+        self.assertRaises(exception.DomainNotFound,
+                          self.assignment_api.get_domain_by_name,
+                          domain_name)
+
     @tests.skip_if_cache_disabled('assignment')
     def test_cache_layer_domain_crud(self):
         domain = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
@@ -2810,6 +2824,25 @@ class IdentityTests(object):
                           self.assignment_api.get_domain,
                           domain_id)
 
+    @tests.skip_if_cache_disabled('assignment')
+    def test_project_rename_invalidates_get_project_by_name_cache(self):
+        domain = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
+                  'enabled': True}
+        project = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
+                   'domain_id': domain['id']}
+        project_id = project['id']
+        project_name = project['name']
+        self.assignment_api.create_domain(domain['id'], domain)
+        # Create a project
+        self.assignment_api.create_project(project_id, project)
+        self.assignment_api.get_project(project_id)
+        project['name'] = uuid.uuid4().hex
+        self.assignment_api.update_project(project_id, project)
+        self.assertRaises(exception.ProjectNotFound,
+                          self.assignment_api.get_project_by_name,
+                          project_name,
+                          domain['id'])
+
     @tests.skip_if_cache_disabled('assignment')
     def test_cache_layer_project_crud(self):
         domain = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
diff --git a/keystone/tests/test_backend_ldap.py b/keystone/tests/test_backend_ldap.py
index 6d3035998b..2d35a6b031 100644
--- a/keystone/tests/test_backend_ldap.py
+++ b/keystone/tests/test_backend_ldap.py
@@ -1144,6 +1144,18 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase):
         # just skip this time.
         self.skipTest('Domains are read-only against LDAP')
 
+    def test_domain_rename_invalidates_get_domain_by_name_cache(self):
+        parent = super(LDAPIdentity, self)
+        self.assertRaises(
+            exception.Forbidden,
+            parent.test_domain_rename_invalidates_get_domain_by_name_cache)
+
+    def test_project_rename_invalidates_get_project_by_name_cache(self):
+        parent = super(LDAPIdentity, self)
+        self.assertRaises(
+            exception.Forbidden,
+            parent.test_project_rename_invalidates_get_project_by_name_cache)
+
     def test_project_crud(self):
         # NOTE(topol): LDAP implementation does not currently support the
         #              updating of a project name so this method override