write autosave state to backup file then rename, add configuration op…

…tion for backup_on_load

add additional autosave tests

softioc/autosave.py

@@ -1,5 +1,4 @@
 import atexit
-import shutil
 import sys
 import threading
 import traceback
@@ -9,6 +8,8 @@
 import numpy
 import yaml
 from numpy import ndarray
+from os import rename
+from shutil import copy2
 
 SAV_SUFFIX = "softsav"
 SAVB_SUFFIX = "softsavB"
@@ -20,7 +21,7 @@ def _ndarray_representer(dumper, array):
     )
 
 
-def configure(directory, name, save_period=None, enabled=True):
+def configure(directory, name, save_period=None, backup=True, enabled=True):
     """This should be called before initialising the IOC. Configures the
     autosave thread for periodic backing up of PV values.
 
@@ -31,10 +32,13 @@ def configure(directory, name, save_period=None, enabled=True):
             could be the same as the device prefix.
         save_period: time in seconds between backups. Backups are only performed
             if PV values have changed.
+        backup: creates a backup of the loaded autosave file on load,
+            timestamped with the time of backup.
         enabled: boolean which enables or disables autosave, set to True by
             default, or False if configure not called.
     """
     Autosave.directory = Path(directory)
+    Autosave.backup_on_load = backup
     Autosave.save_period = save_period or Autosave.save_period
     Autosave.enabled = enabled
     Autosave.device_name = name
@@ -79,12 +83,13 @@ def __init__(self, pv, field=None):
 class Autosave:
     _pvs = {}
     _last_saved_state = {}
+    _last_saved_time = datetime.now()
     _stop_event = threading.Event()
     save_period = 30.0
     device_name = None
     directory = None
     enabled = False
-    backup_on_restart = True
+    backup_on_load = True
 
     def __init__(self):
         if not self.enabled:
@@ -106,23 +111,23 @@ def __init__(self):
                 f"{self.directory} is not a valid autosave directory"
             )
         self._last_saved_time = datetime.now()
-        if self.backup_on_restart:
-            self._backup_sav_file()
 
-    def _backup_sav_file(self):
-        sav_path = self._get_current_sav_path()
+    @classmethod
+    def _backup_sav_file(cls):
+        sav_path = cls._get_current_sav_path()
         if sav_path.is_file():
-            shutil.copy2(sav_path, self._get_timestamped_backup_sav_path())
+            copy2(sav_path, cls._get_timestamped_backup_sav_path())
         else:
             sys.stderr.write(
                 f"Could not back up autosave, {sav_path} is not a file\n"
             )
             sys.stderr.flush()
 
-    def _get_timestamped_backup_sav_path(self):
-        sav_path = self._get_current_sav_path()
+    @classmethod
+    def _get_timestamped_backup_sav_path(cls):
+        sav_path = cls._get_current_sav_path()
         return sav_path.parent / (
-            sav_path.name + self._last_saved_time.strftime("_%y%m%d-%H%M%S")
+            sav_path.name + cls._last_saved_time.strftime("_%y%m%d-%H%M%S")
         )
 
     @classmethod
@@ -165,17 +170,20 @@ def _state_changed(self, state):
     def _save(self):
         state = self._get_state()
         if self._state_changed(state):
-            for path in [
-                self._get_current_sav_path(),
-                self._get_backup_sav_path(),
-            ]:
-                with open(path, "w") as f:
-                    yaml.dump(state, f, indent=4)
+            sav_path = self._get_current_sav_path()
+            backup_path = self._get_backup_sav_path()
+            # write to backup file first then use atomic os.rename
+            # to safely update stored state
+            with open(backup_path, "w") as backup:
+                yaml.dump(state, backup, indent=4)
+            rename(backup_path, sav_path)
             self._last_saved_state = state
             self._last_saved_time = datetime.now()
 
     @classmethod
     def _load(cls, path=None):
+        if cls.backup_on_load:
+            cls._backup_sav_file()
         if not cls.enabled or not cls._pvs:
             return
         sav_path = path or cls._get_current_sav_path()

tests/test_autosave.py

@@ -4,6 +4,7 @@
 import threading
 import shutil
 import numpy
+import re
 import yaml
 
 DEVICE_NAME = "MY-DEVICE"
@@ -17,7 +18,7 @@ def reset_autosave_setup_teardown():
     default_device_name = autosave.Autosave.device_name
     default_directory = autosave.Autosave.directory
     default_enabled = autosave.Autosave.enabled
-    default_bor = autosave.Autosave.backup_on_restart
+    default_bol = autosave.Autosave.backup_on_load
     yield
     autosave.Autosave._pvs = default_pvs
     autosave.Autosave._last_saved_state = default_state
@@ -26,7 +27,7 @@ def reset_autosave_setup_teardown():
     autosave.Autosave.device_name = default_device_name
     autosave.Autosave.directory = default_directory
     autosave.Autosave.enabled = default_enabled
-    autosave.Autosave.backup_on_restart = default_bor
+    autosave.Autosave.backup_on_load = default_bol
     if builder.GetRecordNames().prefix:  # reset device name to empty if set
         builder.SetDeviceName("")
 
@@ -67,7 +68,7 @@ def test_autosave_defaults():
     assert autosave.Autosave.device_name is None
     assert autosave.Autosave.directory is None
     assert autosave.Autosave.enabled is False
-    assert autosave.Autosave.backup_on_restart is True
+    assert autosave.Autosave.backup_on_load is True
 
 
 def test_configure_dir_doesnt_exist():
@@ -76,7 +77,7 @@ def test_configure_dir_doesnt_exist():
     DEVICE_NAME = "MY_DEVICE"
     autosave.configure(autosave_dir, DEVICE_NAME)
     with pytest.raises(FileNotFoundError):
-        autosaver = autosave.Autosave()
+        autosave.Autosave()
 
 
 def test_returns_if_init_called_before_configure():
@@ -89,7 +90,7 @@ def test_all_record_types_saveable(autosave_dir):
     autosave.configure(autosave_dir, DEVICE_NAME)
 
     number_types = ["aIn", "aOut", "boolIn", "boolOut", "longIn", "longOut",
-                "int64In", "int64Out", "mbbIn", "mbbOut", "Action"]
+                    "int64In", "int64Out", "mbbIn", "mbbOut", "Action"]
     string_types = ["stringIn", "stringOut", "longStringIn", "longStringOut"]
     waveform_types = ["WaveformIn", "WaveformOut"]
     for pv_type in number_types:
@@ -150,8 +151,32 @@ def test_stop_event(autosave_dir):
 
 def test_load_autosave(existing_autosave_dir):
     builder.SetDeviceName(DEVICE_NAME)
-    autosave.configure(existing_autosave_dir, DEVICE_NAME)
+    autosave.configure(existing_autosave_dir, DEVICE_NAME, backup=False)
     pv = builder.aOut("ALREADYSAVED", autosave=True)
     assert pv.get() == 0.0
     autosave.load()
     assert pv.get() == 20.0
+
+def test_backup_on_load(existing_autosave_dir):
+    autosave.configure(existing_autosave_dir, DEVICE_NAME, backup=True)
+    autosave.load()
+    backup_files = list(existing_autosave_dir.glob("*.softsav_*"))
+    assert len(backup_files) == 1
+    # assert backup file is named <name>.softsave_yymmdd-HHMMSS
+    for file in backup_files:
+        assert re.match(r"^" + DEVICE_NAME + r"\.softsav_[0-9]{6}-[0-9]{6}$",
+                        file.name)
+
+def test_autosave_key_names(autosave_dir):
+    builder.aOut("DEFAULTNAME", autosave=True)
+    builder.SetDeviceName(DEVICE_NAME)
+    builder.aOut("DEFAULTNAMEAFTERPREFIXSET", autosave=True)
+    builder.aOut("RENAMEME", autosave=True, autosave_name="CUSTOMNAME")
+    autosave.configure(autosave_dir, DEVICE_NAME)
+    autosaver = autosave.Autosave()
+    autosaver._save()
+    with open(autosave_dir / f"{DEVICE_NAME}.softsav", "r") as f:
+        saved = yaml.full_load(f)
+    assert "DEFAULTNAME" in saved
+    assert "DEFAULTNAMEAFTERPREFIXSET" in saved
+    assert "CUSTOMNAME" in saved