[Zope-Checkins] SVN: Zope/branches/2.12/ - LP #142451: If non-recursive ownership changes are made using

[Jens Vagelpohl]

Log message for revision 112514:
  - LP #142451: If non-recursive ownership changes are made using
    ``changeOwnership``, do not touch any children.
  

Changed:
  U   Zope/branches/2.12/doc/CHANGES.rst
  U   Zope/branches/2.12/src/AccessControl/Owned.py
  U   Zope/branches/2.12/src/AccessControl/tests/testOwned.py

-=-
Modified: Zope/branches/2.12/doc/CHANGES.rst
===================================================================
--- Zope/branches/2.12/doc/CHANGES.rst	2010-05-19 10:48:09 UTC (rev 112513)
+++ Zope/branches/2.12/doc/CHANGES.rst	2010-05-19 10:48:18 UTC (rev 112514)
@@ -11,6 +11,9 @@
 Bugs Fixed
 ++++++++++
 
+- LP #142451: If non-recursive ownership changes are made using 
+  ``changeOwnership``, do not touch any children.
+
 Features Added
 ++++++++++++++
 

Modified: Zope/branches/2.12/src/AccessControl/Owned.py
===================================================================
--- Zope/branches/2.12/src/AccessControl/Owned.py	2010-05-19 10:48:09 UTC (rev 112513)
+++ Zope/branches/2.12/src/AccessControl/Owned.py	2010-05-19 10:48:18 UTC (rev 112514)
@@ -157,23 +157,21 @@
         If 'recursive' is true then also take ownership of all sub-objects,
         otherwise sub-objects retain their ownership information.
         """
-
-        new=ownerInfo(user)
-        if new is None: return # Special user!
+        new = ownerInfo(user)
+        if new is None: 
+            return # Special user!
         old = self.getOwnerTuple()
+
         if not recursive:
-            if old==new: return
-            if old is UnownableOwner: return
+            if old == new or old is UnownableOwner:
+                return
 
-        for child in self.objectValues():
-            if recursive:
+        if recursive:
+            for child in self.objectValues():
                 child.changeOwnership(user, 1)
-            else:
-                # make ownership explicit
-                child._owner=new
 
         if old is not UnownableOwner:
-            self._owner=new
+            self._owner = new
 
     def userCanTakeOwnership(self):
         security=getSecurityManager()

Modified: Zope/branches/2.12/src/AccessControl/tests/testOwned.py
===================================================================
--- Zope/branches/2.12/src/AccessControl/tests/testOwned.py	2010-05-19 10:48:09 UTC (rev 112513)
+++ Zope/branches/2.12/src/AccessControl/tests/testOwned.py	2010-05-19 10:48:18 UTC (rev 112514)
@@ -177,8 +177,93 @@
         self.assertEqual(wrapped_owner.getId(), 'user')
 
 
+class OwnershipChangeTests(unittest.TestCase):
+
+    def setUp(self):
+        from AccessControl.Owned import UnownableOwner
+        from AccessControl.User import UserFolder
+        from OFS.Folder import Folder
+        super(OwnershipChangeTests, self).setUp()
+
+        self.root = FauxRoot()
+        self.root.acl_users = UserFolder()
+
+        self.uf = self.root.acl_users
+        self.uf._doAddUser('user1', 'xxx', ['role1'], [])
+        self.uf._doAddUser('user2', 'xxx', ['role1'], [])
+
+        self.root.unownable = Folder('unownable')
+        self.root.unownable._owner = UnownableOwner
+
+        self.root.parent = Folder('parent')
+        parent = self.root.parent
+        parent._owner = (['acl_users'], 'user1')
+        parent._setObject('child', Folder('child'))
+        parent.child._owner = (['acl_users'], 'user1')
+        parent.child._setObject('grandchild', Folder('grandchild'))
+        parent.child.grandchild._owner = (['acl_users'], 'user1')
+
+    def test_changeOwnership_bad_owner(self):
+        from AccessControl.User import nobody
+        previous = self.root.parent._owner
+
+        self.root.parent.changeOwnership(nobody)
+        self.assertEquals(self.root.parent._owner, previous)
+
+    def test_changeOwnership_same_owner(self):
+        previous = self.root.parent._owner
+        sameuser = self.uf.getUser('user1').__of__(self.uf)
+
+        self.root.parent.changeOwnership(sameuser)
+        self.assertEquals(self.root.parent._owner, previous)
+
+    def test_changeOwnership_unownable_owner(self):
+        previous = self.root.unownable._owner
+        newuser = self.uf.getUser('user1').__of__(self.uf)
+
+        self.root.unownable.changeOwnership(newuser)
+        self.assertEquals(self.root.unownable._owner, previous)
+
+    def test_changeOwnership_nonrecursive(self):
+        previous_parent_owner = self.root.parent._owner
+        previous_child_owner = self.root.parent.child._owner
+        previous_grandchild_owner = self.root.parent.child.grandchild._owner
+        newuser = self.uf.getUser('user2').__of__(self.uf)
+
+        self.root.parent.changeOwnership(newuser)
+        self.assertNotEquals(self.root.parent._owner, previous_parent_owner)
+        self.assertEquals(self.root.parent._owner, (['acl_users'], 'user2'))
+        self.assertEquals(self.root.parent.child._owner, previous_child_owner)
+        self.assertEquals( self.root.parent.child.grandchild._owner
+                         , previous_grandchild_owner
+                         )
+
+    def test_changeOwnership_recursive(self):
+        previous_parent_owner = self.root.parent._owner
+        previous_child_owner = self.root.parent.child._owner
+        previous_grandchild_owner = self.root.parent.child.grandchild._owner
+        newuser = self.uf.getUser('user2').__of__(self.uf)
+
+        self.root.parent.changeOwnership(newuser, recursive=True)
+        self.assertNotEquals(self.root.parent._owner, previous_parent_owner)
+        self.assertEquals(self.root.parent._owner, (['acl_users'], 'user2'))
+        self.assertNotEquals(self.root.parent.child._owner, previous_child_owner)
+        self.assertEquals( self.root.parent.child._owner
+                         , (['acl_users'], 'user2')
+                         )
+        self.assertNotEquals( self.root.parent.child.grandchild._owner
+                            , previous_grandchild_owner
+                            )
+        self.assertEquals( self.root.parent.child.grandchild._owner
+                         , (['acl_users'], 'user2')
+                         )
+
+
 def test_suite():
-    return unittest.makeSuite(OwnedTests)
+    return unittest.TestSuite((
+        unittest.makeSuite(OwnedTests),
+        unittest.makeSuite(OwnershipChangeTests),
+        ))
 
 if __name__ == '__main__':
     unittest.main(defaultTest='test_suite')