Merge branch 'issue-1877' into 'master'

Attempt to bubble up permissions errors to the user so that we don't run into None files or hashes

See merge request openlp/openlp!746

openlp/core/common/__init__.py

@@ -275,6 +275,8 @@ def sha256_file_hash(filename):
     """
     Returns the hashed output of sha256 on the file content using Python3 hashlib
 
+    This method allows PermissionError to bubble up, while supressing other exceptions
+
     :param filename: Name of the file to hash
     :returns: str
     """
@@ -288,6 +290,8 @@ def sha256_file_hash(filename):
                 hash_obj.update(chunk)
         return hash_obj.hexdigest()
     except PermissionError:
+        raise
+    except Exception:
         return None
 
 

openlp/core/common/i18n.py

@@ -427,6 +427,7 @@ class UiStrings(metaclass=Singleton):
         self.OptionalShowInFooter = translate('OpenLP.Ui', 'Optional, this will be displayed in footer.')
         self.OptionalHideInFooter = translate('OpenLP.Ui', 'Optional, this won\'t be displayed in footer.')
         self.Other = translate('SongsPlugin.VerseType', 'Other')
+        self.PermissionError = translate('OpenLP.Ui', 'Permission Error')
         self.PlaySlidesInLoop = translate('OpenLP.Ui', 'Play Slides in Loop')
         self.PlaySlidesToEnd = translate('OpenLP.Ui', 'Play Slides to End')
         self.PreChorus = translate('SongsPlugin.VerseType', 'Pre-Chorus')
@@ -469,6 +470,9 @@ class UiStrings(metaclass=Singleton):
         self.Tools = translate('OpenLP.Ui', 'Tools')
         self.Top = translate('OpenLP.Ui', 'Top')
         self.Transpose = translate('SongsPlugin.EditVerseForm', 'Transpose:')
+        self.UnableToRead = translate('OpenLP.Ui', 'Unable to read the file(s) listed below, please check that '
+                                                   'your user has permission to read the file(s) or that the '
+                                                   'file(s) are not using cloud storage (e.g. Dropbox, OneDrive).')
         self.UnsupportedFile = translate('OpenLP.Ui', 'Unsupported File')
         self.Up = translate('SongsPlugin.EditVerseForm', 'Up')
         self.Verse = translate('SongsPlugin.VerseType', 'Verse')

openlp/core/ui/servicemanager.py

@@ -668,7 +668,12 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         missing_list = []
 
         if not self._save_lite:
-            write_list, missing_list = self.get_write_file_list()
+            try:
+                write_list, missing_list = self.get_write_file_list()
+            except PermissionError as pe:
+                self.main_window.error_message(UiStrings.PermissionError,
+                                               UiStrings.UnableToRead + '\n\n' + pe.filename)
+                return False
             if missing_list:
                 self.application.set_normal_cursor()
                 title = translate('OpenLP.ServiceManager', 'Service File(s) Missing')

tests/openlp_core/common/test_init.py

@@ -428,12 +428,26 @@ def test_sha256_file_hash_no_exist():
 
 def test_sha256_file_hash_permission_error():
     """
-    Test SHA256 file hash when there is a permission error
+    Test that SHA256 file hash re-raises a permission error
     """
     # GIVEN: A mocked Path object
     mocked_path = MagicMock()
     mocked_path.open.side_effect = PermissionError
 
+    # WHEN: Generating a hash for the file
+    # THEN: The PermissionError should be bubbled up
+    with pytest.raises(PermissionError):
+        sha256_file_hash(mocked_path)
+
+
+def test_sha256_file_hash_other_error():
+    """
+    Test SHA256 file hash when there is an error other than permission error
+    """
+    # GIVEN: A mocked Path object
+    mocked_path = MagicMock()
+    mocked_path.open.side_effect = NotADirectoryError
+
     # WHEN: Generating a hash for the file
     result = sha256_file_hash(mocked_path)