Rev 39: Make take_lock a multi take_locks() in lp:~jameinel/+junk/file_locking

[John Arbash Meinel]

At lp:~jameinel/+junk/file_locking

------------------------------------------------------------
revno: 39
revision-id: john at arbash-meinel.com-20090923195456-7wnp3dmlluft9m99
parent: john at arbash-meinel.com-20090923173135-qyvkjer4i63yatfm
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: file_locking
timestamp: Wed 2009-09-23 14:54:56 -0500
message:
  Make take_lock a multi take_locks()
-------------- next part --------------
=== modified file 'file_lock.py'
--- a/file_lock.py	2009-09-23 17:31:35 +0000
+++ b/file_lock.py	2009-09-23 19:54:56 +0000
@@ -554,22 +554,35 @@
     def have_lock(self, path):
         """Do we have the lock for a given path."""
 
-    def take_lock(self, path):
+    def take_locks(self, paths):
         """Take the lock for the given path."""
-        file_id = self._wt.path2id(path)
-        if file_id is None:
-            raise ValueError('Cannot take a lock on an unversioned path.')
+        # TODO: We also need to make sure that we take out the on-disk lock
+        #       when we update the remote file.
+        # This is the point we should write-lock
+        # Probably it should be a flag on _get_lock_store() to return a
+        # write-locked version. This way we can ensure nobody else is mutating
+        # it while we are reading it.
         ls = self._get_lock_store()
         if ls is None:
             raise ValueError('Cannot take a lock when no lock store has been'
                              ' configured')
-        if file_id not in ls._locking_info._tracked_ids:
-            # TODO: Consider giving a hint exception in the case that the user
-            #       gives 'foo/bar' and 'foo' is tracked but 'foo/bar' is not
-            #       We could go through the tracked_at functionality to check
-            #       this
-            raise ValueError('path %s is not one of the tracked locations.')
-        ls._locking_info.create_lock(file_id)
+        # Check the paths exist and are tracked before taking any locks
+        file_ids = []
+        for path in paths:
+            file_id = self._wt.path2id(path)
+            if file_id is None:
+                raise ValueError('Cannot take a lock on an unversioned path.')
+            if file_id not in ls._locking_info._tracked_ids:
+                # TODO: Consider giving a hint exception in the case that the
+                # user gives 'foo/bar' and 'foo' is tracked but 'foo/bar' is
+                # not We could go through the tracked_at functionality to check
+                # this
+                raise ValueError('path %s is not one of the tracked locations.')
+            file_ids.append(file_id)
+        # TODO: Note that this creates a LockInfo, but doesn't save it to the
+        #       remote
+        for file_id in file_ids:
+            ls._locking_info.create_lock(file_id)
 
     def steal_lock(self, path):
         """There is a lock from someone else on the given path, take it."""

=== modified file 'tests/test_file_lock.py'
--- a/tests/test_file_lock.py	2009-09-23 17:31:35 +0000
+++ b/tests/test_file_lock.py	2009-09-23 19:54:56 +0000
@@ -354,7 +354,7 @@
         self.assertEqual({'a': None, 'b': 'b', 'b/c': 'b'},
                          manager.tracked_at(['a', 'b', 'b/c']))
         self.assertFalse(manager.is_locked('b/c'))
-        manager.take_lock('b')
+        manager.take_locks(['b'])
         self.expectFailure('locking not supported yet',
             self.assertTrue, manager.is_locked('b'))
         self.assertTrue(manager.is_locked('b'))
@@ -434,24 +434,30 @@
                             ['afile', 'adir', 'adir/subfile',
                             'adir/not-versioned', 'not-tracked']))
 
-    def test_take_lock_no_store(self):
+    def test_take_locks_no_store(self):
         tree, manager = self.make_simple_tree_and_manager('tree')
-        self.assertRaises(ValueError, manager.take_lock, 'adir')
-
-    def test_take_lock_not_versioned(self):
-        tree, manager = self.make_dir_tracked('tree')
-        self.assertRaises(ValueError, manager.take_lock, 'unknown')
-
-    def test_take_lock_not_tracked(self):
-        tree, manager = self.make_dir_tracked('tree')
-        self.assertRaises(ValueError, manager.take_lock, 'afile')
+        self.assertRaises(ValueError, manager.take_locks, ['adir'])
+
+    def test_take_locks_not_versioned(self):
+        tree, manager = self.make_dir_tracked('tree')
+        self.assertRaises(ValueError, manager.take_locks, ['unknown'])
+
+    def test_take_locks_not_tracked(self):
+        tree, manager = self.make_dir_tracked('tree')
+        self.assertRaises(ValueError, manager.take_locks, ['afile'])
+
+    def test_take_locks_partial_failure(self):
+        tree, manager = self.make_dir_tracked('tree')
+        self.assertRaises(ValueError, manager.take_locks, ['adir', 'afile'])
+        self.assertEqual('tracked', manager.get_path_status('adir'))
+        self.assertEqual('untracked', manager.get_path_status('afile'))
 
 
 class TestGetPathStatus(TestCaseWithManager):
 
     def make_dir_locked(self):
         tree, manager = self.make_dir_tracked('tree')
-        manager.take_lock('adir')
+        manager.take_locks(['adir'])
         return tree, manager
 
     def test_untracked_no_lock_store(self):