Add optional semaphore to protect integrity of persistent storage backend

CHANGELOG

@@ -1,3 +1,5 @@
+- 0.2.4
+	* Add optional concurrency lock for persistence store, implemented for file store
 - 0.2.3
 	* Add noop-store, for convenience for code using persist constructor but will only use memory state
 - 0.2.2

setup.cfg

@@ -1,6 +1,6 @@
 [metadata]
 name = shep
-version = 0.2.3
+version = 0.2.4
 description = Multi-state key stores using bit masks
 author = Louis Holbrook
 author_email = dev@holbrook.no
@@ -22,7 +22,7 @@ licence_files =
 
 [options]
 include_package_data = True
-python_requires = >= 3.6
+python_requires = >= 3.7
 packages = 
 	shep
 	shep.store

shep/error.py

@@ -32,3 +32,9 @@ class StateTransitionInvalid(Exception):
     """Raised if state transition verification fails
     """
     pass
+
+
+class StateLockedKey(Exception):
+    """Attempt to write to a state key that is being written to by another client
+    """
+    pass

shep/persist.py

@@ -3,7 +3,10 @@ import datetime
 
 # local imports
 from .state import State
-from .error import StateItemExists
+from .error import (
+        StateItemExists,
+        StateLockedKey,
+        )
 
 
 class PersistedState(State):
@@ -34,13 +37,14 @@ class PersistedState(State):
 
         See shep.state.State.put
         """
-        to_state = super(PersistedState, self).put(key, state=state, contents=contents)
+        k = self.to_name(state)
-
-        k = self.name(to_state)
 
         self.__ensure_store(k)
+
         self.__stores[k].put(key, contents)
 
+        super(PersistedState, self).put(key, state=state, contents=contents)
+
         self.register_modify(key)
 
 
@@ -56,9 +60,14 @@ class PersistedState(State):
         k_to = self.name(to_state)
         self.__ensure_store(k_to)
 
+        contents = None
+        try:
             contents = self.__stores[k_from].get(key)
             self.__stores[k_to].put(key, contents)
             self.__stores[k_from].remove(key)
+        except StateLockedKey as e:
+            super(PersistedState, self).unset(key, or_state, allow_base=True)
+            raise e
        
         self.sync(to_state)
 
@@ -143,6 +152,7 @@ class PersistedState(State):
         :raises StateItemExists: A content key is already recorded with a different state in memory than in persisted store.
         # :todo: if sync state is none, sync all
         """
+
         states = []
         if state == None:
             states = list(self.all())
@@ -208,10 +218,11 @@ class PersistedState(State):
 
         See shep.state.State.replace
         """
-        super(PersistedState, self).replace(key, contents)
         state = self.state(key)
         k = self.name(state)
-        return self.__stores[k].replace(key, contents)
+        r = self.__stores[k].replace(key, contents)
+        super(PersistedState, self).replace(key, contents)
+        return r
 
 
     def modified(self, key):

shep/state.py

@@ -185,6 +185,12 @@ class State:
         self.__set(k, v)
 
 
+    def to_name(self, k):
+        if k == None:
+            k = 0
+        return self.name(k)
+
+
     def __alias(self, k, *args):
         v = 0
         for a in args:
@@ -436,7 +442,7 @@ class State:
         return self.__move(key, current_state, to_state)
 
 
-    def unset(self, key, not_state):
+    def unset(self, key, not_state, allow_base=False):
         """Unset a single bit, moving to a pure or alias state.
         
         The resulting state cannot be State.base_state_name (0).
@@ -462,7 +468,7 @@ class State:
         if to_state == current_state:
             raise ValueError('invalid change for state {}: {}'.format(key, not_state))
 
-        if to_state == getattr(self, self.base_state_name):
+        if to_state == getattr(self, self.base_state_name) and not allow_base:
             raise ValueError('State {} for {} cannot be reverted to {}'.format(current_state, key, self.base_state_name))
 
         new_state = self.__reverse.get(to_state)

shep/store/file.py

@@ -7,6 +7,7 @@ from .base import (
         re_processedname,
         StoreFactory,
         )
+from shep.error import StateLockedKey
 
 
 class SimpleFileStore:
@@ -15,13 +16,45 @@ class SimpleFileStore:
     :param path: Filesystem base path for all state directory
     :type path: str
     """
-    def __init__(self, path, binary=False):
+    def __init__(self, path, binary=False, lock_path=None):
         self.__path = path
         os.makedirs(self.__path, exist_ok=True)
         if binary:
             self.__m = ['rb', 'wb']
         else:
             self.__m = ['r', 'w']
+        self.__lock_path = lock_path
+        if self.__lock_path != None:
+            os.makedirs(lock_path, exist_ok=True)
+
+
+    def __is_locked(self, k):
+        if self.__lock_path == None:
+            return False
+        for v in os.listdir(self.__lock_path):
+            if k == v:
+                return True
+        return False
+       
+
+    def __lock(self, k):
+        if self.__lock_path == None:
+            return
+        if self.__is_locked(k):
+            raise StateLockedKey(k)
+        fp = os.path.join(self.__lock_path, k)
+        f = open(fp, 'w')
+        f.close()
+
+
+    def __unlock(self, k):
+        if self.__lock_path == None:
+            return
+        fp = os.path.join(self.__lock_path, k)
+        try:
+            os.unlink(fp)
+        except FileNotFoundError:
+            pass
         
 
     def put(self, k, contents=None):
@@ -32,6 +65,7 @@ class SimpleFileStore:
         :param contents: Optional contents to assign for content key
         :type contents: any
         """
+        self.__lock(k)
         fp = os.path.join(self.__path, k)
         if contents == None:
             if self.__m[1] == 'wb':
@@ -42,6 +76,7 @@ class SimpleFileStore:
         f = open(fp, self.__m[1])
         f.write(contents)
         f.close()
+        self.__unlock(k)
 
 
     def remove(self, k):
@@ -51,8 +86,10 @@ class SimpleFileStore:
         :type k: str
         :raises FileNotFoundError: Content key does not exist in the state
         """
+        self.__lock(k)
         fp = os.path.join(self.__path, k)
         os.unlink(fp)
+        self.__unlock(k)
 
     
     def get(self, k):
@@ -64,10 +101,12 @@ class SimpleFileStore:
         :rtype: any
         :return: Contents
         """
+        self.__lock(k)
         fp = os.path.join(self.__path, k)
         f = open(fp, self.__m[0])
         r = f.read()
         f.close()
+        self.__unlock(k)
         return r
 
 
@@ -77,6 +116,7 @@ class SimpleFileStore:
         :rtype: list of str
         :return: Content keys in state
         """
+        self.__lock('.list')
         files = []
         for p in os.listdir(self.__path):
             fp = os.path.join(self.__path, p)
@@ -86,6 +126,7 @@ class SimpleFileStore:
             if len(r) == 0:
                 r = None
             files.append((p, r,))
+        self.__unlock('.list')
         return files
 
 
@@ -110,16 +151,20 @@ class SimpleFileStore:
         :param contents: Contents
         :type contents: any
         """
+        self.__lock(k)
         fp = os.path.join(self.__path, k)
         os.stat(fp)
         f = open(fp, self.__m[1])
         r = f.write(contents)
         f.close()
+        self.__unlock(k)
 
 
     def modified(self, k):
+        self.__lock(k)
         path = self.path(k)
         st = os.stat(path)
+        self.__unlock(k)
         return st.st_ctime
 
 
@@ -133,9 +178,10 @@ class SimpleFileStoreFactory(StoreFactory):
     :param path: Filesystem path as base path for states
     :type path: str
     """
-    def __init__(self, path, binary=False):
+    def __init__(self, path, binary=False, use_lock=False):
         self.__path = path
         self.__binary = binary
+        self.__use_lock = use_lock
 
 
     def add(self, k):
@@ -146,9 +192,13 @@ class SimpleFileStoreFactory(StoreFactory):
         :rtype: SimpleFileStore
         :return: A filesystem persistence instance with the given identifier as subdirectory
         """
+        lock_path = None
+        if self.__use_lock:
+            lock_path = os.path.join(self.__path, '.lock')
+
         k = str(k)
         store_path = os.path.join(self.__path, k)
-        return SimpleFileStore(store_path, binary=self.__binary)
+        return SimpleFileStore(store_path, binary=self.__binary, lock_path=lock_path)
 
 
     def ls(self):

tests/test_file.py

@@ -11,6 +11,7 @@ from shep.error import (
         StateExists,
         StateInvalid,
         StateItemExists,
+        StateLockedKey,
         )
 
 
@@ -257,5 +258,52 @@ class TestFileStore(unittest.TestCase):
         self.assertEqual(len(r), 3)
 
 
+    def test_lock(self):
+        factory = SimpleFileStoreFactory(self.d, use_lock=True)
+        states = PersistedState(factory.add, 3)
+        states.add('foo') 
+        states.add('bar') 
+        states.add('baz') 
+        states.alias('xyzzy', states.FOO | states.BAR)
+        states.alias('plugh', states.FOO | states.BAR | states.BAZ)
+        states.put('abcd')
+
+        lock_path = os.path.join(self.d, '.lock')
+        os.stat(lock_path)
+
+        fp = os.path.join(self.d, '.lock', 'xxxx')
+        f = open(fp, 'w')
+        f.close()
+        
+        with self.assertRaises(StateLockedKey):
+            states.put('xxxx')
+
+        os.unlink(fp)
+        states.put('xxxx')
+
+        states.set('xxxx', states.FOO)
+        states.set('xxxx', states.BAR)
+        states.replace('xxxx', contents='zzzz')
+
+        fp = os.path.join(self.d, '.lock', 'xxxx')
+        f = open(fp, 'w')
+        f.close()
+
+        with self.assertRaises(StateLockedKey):
+            states.set('xxxx', states.BAZ)
+        
+        v = states.state('xxxx')
+        self.assertEqual(v, states.XYZZY)
+
+        with self.assertRaises(StateLockedKey):
+            states.unset('xxxx', states.FOO)
+
+        with self.assertRaises(StateLockedKey):
+            states.replace('xxxx', contents='yyyy')
+
+        v = states.get('xxxx')
+        self.assertEqual(v, 'zzzz')
+
+
 if __name__ == '__main__':
     unittest.main()