. node ids are now generated from a lockable store - no more race conditions

We're using the portalocker code by Jonathan Feinberg that was contributed
to the ASPN Python cookbook. This gives us locking across Unix and Windows.

git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/trunk@703 57a73879-2fb5-44c3-a270-3262357dd7e2

diff --git a/.cvsignore b/.cvsignore
index 23e56ac7c9b52d1d3f06652fea2b7ce183d19717..e1993865ed4598fadc9ba53dda233766552c3b6c 100644 (file)
--- a/.cvsignore
+++ b/.cvsignore
@@ -1,3 +1,4 @@
 *.pyc
 localconfig.py
 build
+MANIFEST

diff --git a/CHANGES.txt b/CHANGES.txt
index de2c49d05804cf26a3ed9bc68a0017e037611e95..10a2368a2f3a2adab8127ab09b5e18035b8c16bb 100644 (file)
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -23,6 +23,7 @@ Feature:
 Fixed:
  . stop sending blank (whitespace-only) notes
  . cleanup of serialisation for database storage
+ . node ids are now generated from a lockable store - no more race conditions
 
 
 2002-03-25 - 0.4.1

diff --git a/doc/.cvsignore b/doc/.cvsignore
index 5f8047098c35fb3d3804449fa52381e59988809a..33086638be192b68635e5d2b18b2527c73d1ab96 100644 (file)
--- a/doc/.cvsignore
+++ b/doc/.cvsignore
@@ -6,3 +6,4 @@ implementation.html
 index.html
 installation.html
 user_guide.html
+FAQ.html

diff --git a/roundup/backends/back_anydbm.py b/roundup/backends/back_anydbm.py
index 65607c8e1c619db9bca4a093abf6d8994b665a1c..4735d04364299f6a80495b855e0e2431eaaf84a2 100644 (file)
--- a/roundup/backends/back_anydbm.py
+++ b/roundup/backends/back_anydbm.py
@@ -15,7 +15,7 @@
 # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 # 
-#$Id: back_anydbm.py,v 1.31 2002-04-03 05:54:31 richard Exp $
+#$Id: back_anydbm.py,v 1.32 2002-04-15 23:25:15 richard Exp $
 '''
 This module defines a backend that saves the hyperdatabase in a database
 chosen by anydbm. It is guaranteed to always be available in python
@@ -26,6 +26,7 @@ serious bugs, and is not available)
 import whichdb, anydbm, os, marshal
 from roundup import hyperdb, date
 from blobfiles import FileStorage
+from locking import acquire_lock, release_lock
 
 #
 # Now the database
@@ -130,6 +131,7 @@ class Database(FileStorage, hyperdb.Database):
         '''
         if hyperdb.DEBUG:
             print '_opendb', (self, name, mode)
+
         # determine which DB wrote the class file
         db_type = ''
         path = os.path.join(os.getcwd(), self.dir, name)
@@ -159,6 +161,31 @@ class Database(FileStorage, hyperdb.Database):
             print "_opendb %r.open(%r, %r)"%(db_type, path, mode)
         return dbm.open(path, mode)
 
+    def _lockdb(self, name):
+        ''' Lock a database file
+        '''
+        path = os.path.join(os.getcwd(), self.dir, '%s.lock'%name)
+        return acquire_lock(path)
+
+    #
+    # Node IDs
+    #
+    def newid(self, classname):
+        ''' Generate a new id for the given class
+        '''
+        # open the ids DB - create if if doesn't exist
+        lock = self._lockdb('_ids')
+        db = self._opendb('_ids', 'c')
+        if db.has_key(classname):
+            newid = db[classname] = str(int(db[classname]) + 1)
+        else:
+            # the count() bit is transitional - older dbs won't start at 1
+            newid = str(self.getclass(classname).count()+1)
+            db[classname] = newid
+        db.close()
+        release_lock(lock)
+        return newid
+
     #
     # Nodes
     #
@@ -442,6 +469,14 @@ class Database(FileStorage, hyperdb.Database):
 
 #
 #$Log: not supported by cvs2svn $
+#Revision 1.31  2002/04/03 05:54:31  richard
+#Fixed serialisation problem by moving the serialisation step out of the
+#hyperdb.Class (get, set) into the hyperdb.Database.
+#
+#Also fixed htmltemplate after the showid changes I made yesterday.
+#
+#Unit tests for all of the above written.
+#
 #Revision 1.30  2002/02/27 03:40:59  richard
 #Ran it through pychecker, made fixes
 #

diff --git a/roundup/backends/locking.py b/roundup/backends/locking.py
new file mode 100644 (file)
index 0000000..982c985
--- /dev/null
+++ b/roundup/backends/locking.py
@@ -0,0 +1,51 @@
+#! /usr/bin/env python
+# Copyright (c) 2002 ekit.com Inc (http://www.ekit-inc.com/)
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+#   The above copyright notice and this permission notice shall be included in
+#   all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+# $Id: locking.py,v 1.1 2002-04-15 23:25:15 richard Exp $
+
+'''This module provides a generic interface to acquire and release
+exclusive access to a file.
+
+It should work on Unix and Windows.
+'''
+
+import portalocker
+
+def acquire_lock(path, block=1):
+    '''Acquire a lock for the given path
+    '''
+    import portalocker
+    file = open(path, 'w')
+    if block:
+        portalocker.lock(file, portalocker.LOCK_EX)
+    else:
+        portalocker.lock(file, portalocker.LOCK_EX|portalocker.LOCK_NB)
+    return file
+
+def release_lock(file):
+    '''Release our lock on the given path
+    '''
+    portalocker.unlock(file)
+
+#
+# $Log: not supported by cvs2svn $
+#
+#

diff --git a/roundup/backends/portalocker.py b/roundup/backends/portalocker.py
new file mode 100644 (file)
index 0000000..f9116f0
--- /dev/null
+++ b/roundup/backends/portalocker.py
@@ -0,0 +1,93 @@
+# portalocker.py - Cross-platform (posix/nt) API for flock-style file locking.
+#                  Requires python 1.5.2 or better.
+
+# ID line added by richard for Roundup file tracking
+# $Id: portalocker.py,v 1.1 2002-04-15 23:25:15 richard Exp $
+
+"""Cross-platform (posix/nt) API for flock-style file locking.
+
+Synopsis:
+
+   import portalocker
+   file = open("somefile", "r+")
+   portalocker.lock(file, portalocker.LOCK_EX)
+   file.seek(12)
+   file.write("foo")
+   file.close()
+
+If you know what you're doing, you may choose to
+
+   portalocker.unlock(file)
+
+before closing the file, but why?
+
+Methods:
+
+   lock( file, flags )
+   unlock( file )
+
+Constants:
+
+   LOCK_EX
+   LOCK_SH
+   LOCK_NB
+
+I learned the win32 technique for locking files from sample code
+provided by John Nielsen <nielsenjf@my-deja.com> in the documentation
+that accompanies the win32 modules.
+
+Author: Jonathan Feinberg <jdf@pobox.com>
+Version: Id: portalocker.py,v 1.3 2001/05/29 18:47:55 Administrator Exp 
+         **un-cvsified by richard so the version doesn't change**
+"""
+import os
+
+if os.name == 'nt':
+       import win32con
+       import win32file
+       import pywintypes
+       LOCK_EX = win32con.LOCKFILE_EXCLUSIVE_LOCK
+       LOCK_SH = 0 # the default
+       LOCK_NB = win32con.LOCKFILE_FAIL_IMMEDIATELY
+       # is there any reason not to reuse the following structure?
+       __overlapped = pywintypes.OVERLAPPED()
+elif os.name == 'posix':
+       import fcntl
+       LOCK_EX = fcntl.LOCK_EX
+       LOCK_SH = fcntl.LOCK_SH
+       LOCK_NB = fcntl.LOCK_NB
+else:
+       raise RuntimeError("PortaLocker only defined for nt and posix platforms")
+
+if os.name == 'nt':
+       def lock(file, flags):
+               hfile = win32file._get_osfhandle(file.fileno())
+               win32file.LockFileEx(hfile, flags, 0, 0xffff0000, __overlapped)
+
+       def unlock(file):
+               hfile = win32file._get_osfhandle(file.fileno())
+               win32file.UnlockFileEx(hfile, 0, 0xffff0000, __overlapped)
+
+elif os.name =='posix':
+       def lock(file, flags):
+               fcntl.flock(file.fileno(), flags)
+
+       def unlock(file):
+               fcntl.flock(file.fileno(), fcntl.LOCK_UN)
+
+if __name__ == '__main__':
+       from time import time, strftime, localtime
+       import sys
+       import portalocker
+
+       log = open('log.txt', "a+")
+       portalocker.lock(log, portalocker.LOCK_EX)
+
+       timestamp = strftime("%m/%d/%Y %H:%M:%S\n", localtime(time()))
+       log.write( timestamp )
+
+       print "Wrote lines. Hit enter to release lock."
+       dummy = sys.stdin.readline()
+
+       log.close()
+

diff --git a/roundup/hyperdb.py b/roundup/hyperdb.py
index a58e7ef29e6476b8c4ce7f5ac46300f3471a16f4..51cee620725c6d2acd1842a58acf7ba484b56ab3 100644 (file)
--- a/roundup/hyperdb.py
+++ b/roundup/hyperdb.py
@@ -15,7 +15,7 @@
 # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 # 
-# $Id: hyperdb.py,v 1.62 2002-04-03 07:05:50 richard Exp $
+# $Id: hyperdb.py,v 1.63 2002-04-15 23:25:15 richard Exp $
 
 __doc__ = """
 Hyperdatabase implementation, especially field types.
@@ -353,7 +353,7 @@ class Class:
             raise DatabaseError, 'Database open read-only'
 
         # new node's id
-        newid = str(self.count() + 1)
+        newid = self.db.newid(self.classname)
 
         # validate propvalues
         num_re = re.compile('^\d+$')
@@ -1127,6 +1127,10 @@ def Choice(name, db, *options):
 
 #
 # $Log: not supported by cvs2svn $
+# Revision 1.62  2002/04/03 07:05:50  richard
+# d'oh! killed retirement of nodes :(
+# all better now...
+#
 # Revision 1.61  2002/04/03 06:11:51  richard
 # Fix for old databases that contain properties that don't exist any more.
 #

diff --git a/test/test_db.py b/test/test_db.py
index 19dcd1682393729e1ed6482803638ad226018043..b09a44c1e51bfa74edce35c8582cb11280b04c87 100644 (file)
--- a/test/test_db.py
+++ b/test/test_db.py
@@ -15,7 +15,7 @@
 # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 # 
-# $Id: test_db.py,v 1.20 2002-04-03 05:54:31 richard Exp $ 
+# $Id: test_db.py,v 1.21 2002-04-15 23:25:15 richard Exp $ 
 
 import unittest, os, shutil
 
@@ -66,6 +66,8 @@ class anydbmDBTestCase(MyTestCase):
         os.makedirs(config.DATABASE + '/files')
         self.db = anydbm.Database(config, 'test')
         setupSchema(self.db, 1)
+        self.db2 = anydbm.Database(config, 'test')
+        setupSchema(self.db2, 0)
 
     def testChanges(self):
         self.db.issue.create(title="spam", status='1')
@@ -140,8 +142,6 @@ class anydbmDBTestCase(MyTestCase):
         self.db.rollback()
         self.assertNotEqual(num_files, self.db.numfiles())
         self.assertEqual(num_files2, self.db.numfiles())
-            
-
 
     def testExceptions(self):
         # this tests the exceptions that should be raised
@@ -192,17 +192,17 @@ class anydbmDBTestCase(MyTestCase):
         # set up a valid issue for me to work on
         self.db.issue.create(title="spam", status='1')
         # invalid link index
-        ar(IndexError, self.db.issue.set, '1', title='foo', status='bar')
+        ar(IndexError, self.db.issue.set, '6', title='foo', status='bar')
         # invalid link value
-        ar(ValueError, self.db.issue.set, '1', title='foo', status=1)
+        ar(ValueError, self.db.issue.set, '6', title='foo', status=1)
         # invalid multilink type
-        ar(TypeError, self.db.issue.set, '1', title='foo', status='1',
+        ar(TypeError, self.db.issue.set, '6', title='foo', status='1',
             nosy='hello')
         # invalid multilink index type
-        ar(ValueError, self.db.issue.set, '1', title='foo', status='1',
+        ar(ValueError, self.db.issue.set, '6', title='foo', status='1',
             nosy=[1])
         # invalid multilink index
-        ar(IndexError, self.db.issue.set, '1', title='foo', status='1',
+        ar(IndexError, self.db.issue.set, '6', title='foo', status='1',
             nosy=['10'])
 
     def testJournals(self):
@@ -269,6 +269,11 @@ class anydbmDBTestCase(MyTestCase):
     def testRetire(self):
         pass
 
+    def testIDGeneration(self):
+        id1 = self.db.issue.create(title="spam", status='1')
+        id2 = self.db2.issue.create(title="eggs", status='2')
+        self.assertNotEqual(id1, id2)
+
 
 class anydbmReadOnlyDBTestCase(MyTestCase):
     def setUp(self):
@@ -281,6 +286,8 @@ class anydbmReadOnlyDBTestCase(MyTestCase):
         setupSchema(db, 1)
         self.db = anydbm.Database(config)
         setupSchema(self.db, 0)
+        self.db2 = anydbm.Database(config, 'test')
+        setupSchema(self.db2, 0)
 
     def testExceptions(self):
         # this tests the exceptions that should be raised
@@ -301,6 +308,8 @@ class bsddbDBTestCase(anydbmDBTestCase):
         os.makedirs(config.DATABASE + '/files')
         self.db = bsddb.Database(config, 'test')
         setupSchema(self.db, 1)
+        self.db2 = bsddb.Database(config, 'test')
+        setupSchema(self.db2, 0)
 
 class bsddbReadOnlyDBTestCase(anydbmReadOnlyDBTestCase):
     def setUp(self):
@@ -313,6 +322,8 @@ class bsddbReadOnlyDBTestCase(anydbmReadOnlyDBTestCase):
         setupSchema(db, 1)
         self.db = bsddb.Database(config)
         setupSchema(self.db, 0)
+        self.db2 = bsddb.Database(config, 'test')
+        setupSchema(self.db2, 0)
 
 
 class bsddb3DBTestCase(anydbmDBTestCase):
@@ -324,6 +335,8 @@ class bsddb3DBTestCase(anydbmDBTestCase):
         os.makedirs(config.DATABASE + '/files')
         self.db = bsddb3.Database(config, 'test')
         setupSchema(self.db, 1)
+        self.db2 = bsddb3.Database(config, 'test')
+        setupSchema(self.db2, 0)
 
 class bsddb3ReadOnlyDBTestCase(anydbmReadOnlyDBTestCase):
     def setUp(self):
@@ -336,6 +349,8 @@ class bsddb3ReadOnlyDBTestCase(anydbmReadOnlyDBTestCase):
         setupSchema(db, 1)
         self.db = bsddb3.Database(config)
         setupSchema(self.db, 0)
+        self.db2 = bsddb3.Database(config, 'test')
+        setupSchema(self.db2, 0)
 
 
 def suite():
@@ -362,6 +377,14 @@ def suite():
 
 #
 # $Log: not supported by cvs2svn $
+# Revision 1.20  2002/04/03 05:54:31  richard
+# Fixed serialisation problem by moving the serialisation step out of the
+# hyperdb.Class (get, set) into the hyperdb.Database.
+#
+# Also fixed htmltemplate after the showid changes I made yesterday.
+#
+# Unit tests for all of the above written.
+#
 # Revision 1.19  2002/02/25 14:34:31  grubert
 #  . use blobfiles in back_anydbm which is used in back_bsddb.
 #    change test_db as dirlist does not work for subdirectories.

diff --git a/test/test_locking.py b/test/test_locking.py
new file mode 100644 (file)
index 0000000..6f22d9a
--- /dev/null
+++ b/test/test_locking.py
@@ -0,0 +1,55 @@
+# Copyright (c) 2002 ekit.com Inc (http://www.ekit-inc.com/)
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+#   The above copyright notice and this permission notice shall be included in
+#   all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+# $Id: test_locking.py,v 1.1 2002-04-15 23:25:15 richard Exp $
+
+import os, unittest, tempfile
+
+from roundup.backends.locking import acquire_lock, release_lock
+
+class LockingTest(unittest.TestCase):
+    def setUp(self):
+        self.path = tempfile.mktemp()
+        open(self.path, 'w').write('hi\n')
+
+    def test_basics(self):
+        f = acquire_lock(self.path)
+        try:
+            acquire_lock(self.path, block=0)
+        except:
+            pass
+        else:
+            raise AssertionError, 'no exception'
+        release_lock(f)
+        f = acquire_lock(self.path)
+        release_lock(f)
+
+    def tearDown(self):
+        os.remove(self.path)
+
+def suite():
+    return unittest.makeSuite(LockingTest)
+
+
+#
+# $Log: not supported by cvs2svn $
+#
+#
+# vim: set filetype=python ts=4 sw=4 et si