fix: for both config editors, catch and alert on file permission erro…

…rs (#105)

This fixes a user reported bug whereby config saves with invalid (or
impermissible) file paths would fail silently.

Tested by: Both unit tests, and a local sim test to make sure the error
dialog pops up.

software/control/core/core.py

@@ -3598,6 +3598,7 @@ def display_image(self, image, illumination_source):
 class ConfigurationManager(QObject):
     def __init__(self, filename="channel_configurations.xml"):
         QObject.__init__(self)
+        self._log = squid.logging.get_logger(self.__class__.__name__)
         self.config_filename = filename
         self.configurations = []
         self.read_configurations()
@@ -3606,7 +3607,12 @@ def save_configurations(self):
         self.write_configuration(self.config_filename)
 
     def write_configuration(self, filename):
-        self.config_xml_tree.write(filename, encoding="utf-8", xml_declaration=True, pretty_print=True)
+        try:
+            self.config_xml_tree.write(filename, encoding="utf-8", xml_declaration=True, pretty_print=True)
+            return True
+        except IOError:
+            self._log.exception("Couldn't write configuration.")
+            return False
 
     def read_configurations(self):
         if os.path.isfile(self.config_filename) == False:

software/control/widgets.py

@@ -183,7 +183,10 @@ def save_to_file(self):
             self, "Save Acquisition Config File", "", "XML Files (*.xml);;All Files (*)"
         )
         if file_path:
-            self.config.write_configuration(file_path)
+            if not self.config.write_configuration(file_path):
+                QMessageBox.warning(
+                    self, "Warning", f"Failed to write config to file '{file_path}'.  Check permissions!"
+                )
 
     def load_config_from_file(self, only_z_offset=None):
         file_path, _ = QFileDialog.getOpenFileName(
@@ -205,7 +208,7 @@ def load_config_from_file(self, only_z_offset=None):
 class ConfigEditor(QDialog):
     def __init__(self, config):
         super().__init__()
-
+        self._log = squid.logging.get_logger(self.__class__.__name__)
         self.config = config
 
         self.scroll_area = QScrollArea()
@@ -287,12 +290,23 @@ def save_config(self):
                 if old_val != self.config.get(section, option):
                     print(self.config.get(section, option))
 
+    def save_to_filename(self, filename: str):
+        try:
+            with open(filename, "w") as configfile:
+                self.config.write(configfile)
+                return True
+        except IOError:
+            self._log.exception(f"Failed to write config file to '{filename}'")
+            return False
+
     def save_to_file(self):
         self.save_config()
         file_path, _ = QFileDialog.getSaveFileName(self, "Save Config File", "", "INI Files (*.ini);;All Files (*)")
         if file_path:
-            with open(file_path, "w") as configfile:
-                self.config.write(configfile)
+            if not self.save_to_filename(file_path):
+                QMessageBox.warning(
+                    self, "Warning", f"Failed to write config file to '{file_path}'.  Check permissions!"
+                )
 
     def load_config_from_file(self):
         file_path, _ = QFileDialog.getOpenFileName(self, "Load Config File", "", "INI Files (*.ini);;All Files (*)")

software/tests/control/test_gui_config_editors.py

@@ -0,0 +1,42 @@
+import os
+import tempfile
+from configparser import ConfigParser
+
+import pytest
+
+import tests.control.gui_test_stubs
+import control.widgets
+
+
+def test_config_editor_for_acquisitions_save_to_file(qtbot):
+    config_editor = control.widgets.ConfigEditorForAcquisitions(
+        tests.control.gui_test_stubs.get_test_configuration_manager()
+    )
+
+    (good_fd, good_filename) = tempfile.mkstemp()
+    os.close(good_fd)
+    assert config_editor.config.write_configuration(good_filename)
+    os.remove(good_filename)
+
+    (bad_fd, bad_filename) = tempfile.mkstemp()
+    os.close(bad_fd)
+    read_only_permissions = 0o444
+    os.chmod(bad_filename, read_only_permissions)
+
+    assert not config_editor.config.write_configuration(bad_filename)
+
+
+def test_config_editor_save_to_file(qtbot):
+    config_editor = control.widgets.ConfigEditor(ConfigParser())
+
+    (good_fd, good_filename) = tempfile.mkstemp()
+    os.close(good_fd)
+    assert config_editor.save_to_filename(good_filename)
+    os.remove(good_filename)
+
+    (bad_fd, bad_filename) = tempfile.mkstemp()
+    os.close(bad_fd)
+    read_only_permissions = 0o444
+    os.chmod(bad_filename, read_only_permissions)
+
+    assert not config_editor.save_to_filename(bad_filename)