utils: Added LockFile including test

GitConfigFile is now derived from LockFile using its capabilities
Implemented ConcurrentWriteOperation, test is yet to be done

3 files changed, 205 insertions, 69 deletions

diff --git a/lib/git/config.py b/lib/git/config.py
index 6f979c73..ccfbae48 100644
--- a/lib/git/config.py
+++ b/lib/git/config.py
@@ -11,10 +11,12 @@ configuration files
 import re
 import os
 import ConfigParser as cp
-from git.odict import OrderedDict
 import inspect
 import cStringIO
 
+from git.odict import OrderedDict
+from git.utils import LockFile
+
 class _MetaParserBuilder(type):
 	"""
 	Utlity class wrapping base-class methods into decorators that assure read-only properties
@@ -70,7 +72,7 @@ def _set_dirty_and_flush_changes(non_const_func):
 	
 	
 
-class GitConfigParser(cp.RawConfigParser, object):
+class GitConfigParser(cp.RawConfigParser, LockFile):
 	"""
 	Implements specifics required to read git style configuration files.
 	
@@ -100,6 +102,7 @@ class GitConfigParser(cp.RawConfigParser, object):
 	
 	# list of RawConfigParser methods able to change the instance
 	_mutating_methods_ = ("add_section", "remove_section", "remove_option", "set")
+	__slots__ = ("_sections", "_defaults", "_file_or_files", "_read_only","_is_initialized")
 	
 	def __init__(self, file_or_files, read_only=True):
 		"""
@@ -120,7 +123,6 @@ class GitConfigParser(cp.RawConfigParser, object):
 		
 		self._file_or_files = file_or_files
 		self._read_only = read_only
-		self._owns_lock = False
 		self._is_initialized = False
 		
 		
@@ -129,10 +131,11 @@ class GitConfigParser(cp.RawConfigParser, object):
 				raise ValueError("Write-ConfigParsers can operate on a single file only, multiple files have been passed")
 			# END single file check
 			
-			self._file_name = file_or_files
-			if not isinstance(self._file_name, basestring):
-				self._file_name = file_or_files.name
-			# END get filename
+			if not isinstance(file_or_files, basestring):
+				file_or_files = file_or_files.name
+			# END get filename from handle/stream
+			# initialize lock base - we want to write
+			LockFile.__init__(self, file_or_files)
 			
 			self._obtain_lock_or_raise()	
 		# END read-only check
@@ -155,67 +158,6 @@ class GitConfigParser(cp.RawConfigParser, object):
 		finally:
 			self._release_lock()
 	
-	def _lock_file_path(self):
-		"""
-		Return
-			Path to lockfile
-		"""
-		return "%s.lock" % (self._file_name)
-	
-	def _has_lock(self):
-		"""
-		Return
-			True if we have a lock and if the lockfile still exists
-			
-		Raise
-			AssertionError if our lock-file does not exist
-		"""
-		if not self._owns_lock:
-			return False
-		
-		lock_file = self._lock_file_path()
-		try:
-			fp = open(lock_file, "r")
-			pid = int(fp.read())
-			fp.close()
-		except IOError:
-			raise AssertionError("GitConfigParser has a lock but the lock file at %s could not be read" % lock_file)
-		
-		if pid != os.getpid():
-			raise AssertionError("We claim to own the lock at %s, but it was not owned by our process: %i" % (lock_file, os.getpid()))
-		
-		return True
-		
-	def _obtain_lock_or_raise(self):
-		"""
-		Create a lock file as flag for other instances, mark our instance as lock-holder
-		
-		Raise
-			IOError if a lock was already present or a lock file could not be written
-		"""
-		if self._has_lock():
-			return 
-			
-		lock_file = self._lock_file_path()
-		if os.path.exists(lock_file):
-			raise IOError("Lock for file %r did already exist, delete %r in case the lock is illegal" % (self._file_name, lock_file))
-		
-		fp = open(lock_file, "w")
-		fp.write(str(os.getpid()))
-		fp.close()
-		
-		self._owns_lock = True
-		
-	def _release_lock(self):
-		"""
-		Release our lock if we have one
-		"""
-		if not self._has_lock():
-			return 
-			
-		os.remove(self._lock_file_path())
-		self._owns_lock = False
-		
 	def optionxform(self, optionstr):
 		"""
 		Do not transform options in any way when writing
diff --git a/lib/git/utils.py b/lib/git/utils.py
index cdc7c55b..dd1117b6 100644
--- a/lib/git/utils.py
+++ b/lib/git/utils.py
@@ -5,6 +5,8 @@
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 
 import os
+import sys
+import tempfile
 
 try:
     import hashlib
@@ -58,6 +60,163 @@ class SHA1Writer(object):
         return self.f.tell()
 
 
+class LockFile(object):
+	"""
+	Provides methods to obtain, check for, and release a file based lock which 
+	should be used to handle concurrent access to the same file.
+	
+	As we are a utility class to be derived from, we only use protected methods.
+	
+	Locks will automatically be released on destruction
+	"""
+	__slots__ = ("_file_path", "_owns_lock")
+	
+	def __init__(self, file_path):
+		self._file_path = file_path
+		self._owns_lock = False
+	
+	def __del__(self):
+		self._release_lock()
+	
+	def _lock_file_path(self):
+		"""
+		Return
+			Path to lockfile
+		"""
+		return "%s.lock" % (self._file_path)
+	
+	def _has_lock(self):
+		"""
+		Return
+			True if we have a lock and if the lockfile still exists
+			
+		Raise
+			AssertionError if our lock-file does not exist
+		"""
+		if not self._owns_lock:
+			return False
+		
+		lock_file = self._lock_file_path()
+		try:
+			fp = open(lock_file, "r")
+			pid = int(fp.read())
+			fp.close()
+		except IOError:
+			raise AssertionError("GitConfigParser has a lock but the lock file at %s could not be read" % lock_file)
+		
+		if pid != os.getpid():
+			raise AssertionError("We claim to own the lock at %s, but it was not owned by our process: %i" % (lock_file, os.getpid()))
+		
+		return True
+		
+	def _obtain_lock_or_raise(self):
+		"""
+		Create a lock file as flag for other instances, mark our instance as lock-holder
+		
+		Raise
+			IOError if a lock was already present or a lock file could not be written
+		"""
+		if self._has_lock():
+			return 
+			
+		lock_file = self._lock_file_path()
+		if os.path.exists(lock_file):
+			raise IOError("Lock for file %r did already exist, delete %r in case the lock is illegal" % (self._file_path, lock_file))
+		
+		fp = open(lock_file, "w")
+		fp.write(str(os.getpid()))
+		fp.close()
+		
+		self._owns_lock = True
+		
+	def _release_lock(self):
+		"""
+		Release our lock if we have one
+		"""
+		if not self._has_lock():
+			return 
+			
+		os.remove(self._lock_file_path())
+		self._owns_lock = False
+
+
+class ConcurrentWriteOperation(LockFile):
+	"""
+	This class facilitates a safe write operation to a file on disk such that we: 
+	
+		- lock the original file
+		- write to a temporary file
+		- rename temporary file back to the original one on close
+		- unlock the original file
+		
+	This type handles error correctly in that it will assure a consistent state 
+	on destruction
+	"""
+	__slots__ = "_temp_write_fp"
+	
+	def __init__(self, file_path):
+		"""
+		Initialize an instance able to write the given file_path
+		"""
+		super(ConcurrentWriteOperation, self).__init__(file_path)
+		self._temp_write_fp = None
+	
+	def __del__(self):
+		self._end_writing(successful=False)
+		
+	def _begin_writing(self):
+		"""
+		Begin writing our file, hence we get a lock and start writing 
+		a temporary file in the same directory.
+		
+		Returns
+			File Object to write to. It is still maintained by this instance
+			and you do not need to manually close 
+		"""
+		# already writing ? 
+		if self._temp_write_fp is not None:
+			return self._temp_write_fp
+			
+		self._obtain_lock_or_raise()
+		dirname, basename = os.path.split(self._file_path)
+		self._temp_write_fp = open(tempfile.mktemp(basename, '', dirname), "w")
+		return self._temp_write_fp
+		
+	def _is_writing(self):
+		"""
+		Returns 
+			True if we are currently writing a file
+		"""
+		return self._temp_write_fp is not None
+	
+	def _end_writing(self, successful=True):
+		"""
+		Indicate you successfully finished writing the file to:
+		
+			- close the underlying stream
+			- rename the remporary file to the original one
+			- release our lock
+		"""
+		# did we start a write operation ?
+		if self._temp_write_fp is None:
+			return 
+			
+		self._temp_write_fp.close()
+		if successful:
+			# on windows, rename does not silently overwrite the existing one
+			if sys.platform == "win32":
+				os.remove(self._file_path)
+			os.rename(self._temp_write_fp.name, self._file_path)
+		else:
+			# just delete the file so far, we failed
+			os.remove(self._temp_write_fp.name)
+		# END successful handling
+	
+		# finally reset our handle
+		self._release_lock()
+		self._temp_write_fp = None
+
+
 class LazyMixin(object):
 	"""
 	Base class providing an interface to lazily retrieve attribute values upon 
diff --git a/test/git/test_utils.py b/test/git/test_utils.py
index 6852d0ad..61527758 100644
--- a/test/git/test_utils.py
+++ b/test/git/test_utils.py
@@ -5,11 +5,15 @@
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 
 import os
+import tempfile
+
 from test.testlib import *
+from git.utils import *
 from git import *
 from git.cmd import dashify
 
-class TestUtils(object):
+
+class TestUtils(TestCase):
 	def setup(self):
 		self.testdict = {
 			"string":	"42",
@@ -20,3 +24,34 @@ class TestUtils(object):
 	def test_it_should_dashify(self):
 		assert_equal('this-is-my-argument', dashify('this_is_my_argument'))
 		assert_equal('foo', dashify('foo'))
+		
+		
+	def test_lock_file(self):
+		my_file = tempfile.mktemp()
+		lock_file = LockFile(my_file)
+		assert not lock_file._has_lock()
+		# release lock we don't have  - fine
+		lock_file._release_lock()
+		
+		# get lock
+		lock_file._obtain_lock_or_raise()
+		assert lock_file._has_lock()
+		
+		# concurrent access
+		other_lock_file = LockFile(my_file)
+		assert not other_lock_file._has_lock()
+		self.failUnlessRaises(IOError, other_lock_file._obtain_lock_or_raise)
+		
+		lock_file._release_lock()
+		assert not lock_file._has_lock()
+		
+		other_lock_file._obtain_lock_or_raise()
+		self.failUnlessRaises(IOError, lock_file._obtain_lock_or_raise)
+		
+		# auto-release on destruction
+		del(other_lock_file)
+		lock_file._obtain_lock_or_raise()
+		lock_file._release_lock()
+		
+	def test_safe_operation(self):
+		self.fail("todo")