Fix up code so test passes

openlp/core/lib/serviceitem.py

@@ -36,6 +36,7 @@ from openlp.core.common import md5_hash
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import RegistryProperties
+from openlp.core.common.path import Path
 from openlp.core.common.settings import Settings
 from openlp.core.lib import ImageSource, build_icon, clean_tags, expand_tags, expand_chords
 
@@ -428,6 +429,11 @@ class ServiceItem(RegistryProperties):
         if 'background_audio' in header:
             self.background_audio = []
             for file_path in header['background_audio']:
+                # In OpenLP 3.0 we switched to storing Path objects in JSON files
+                if isinstance(file_path, str):
+                    # Handle service files prior to OpenLP 3.0
+                    # Windows can handle both forward and backward slashes, so we use ntpath to get the basename
+                    file_path = Path(path, ntpath.basename(file_path))
                 self.background_audio.append(file_path)
         self.theme_overwritten = header.get('theme_overwritten', False)
         if self.service_item_type == ServiceItemType.Text:
@@ -439,8 +445,8 @@ class ServiceItem(RegistryProperties):
             if path:
                 self.has_original_files = False
                 for text_image in service_item['serviceitem']['data']:
-                    filename = os.path.join(path, text_image)
+                    file_path = os.path.join(path, text_image)
-                    self.add_from_image(filename, text_image, background)
+                    self.add_from_image(file_path, text_image, background)
             else:
                 for text_image in service_item['serviceitem']['data']:
                     self.add_from_image(text_image['path'], text_image['title'], background)

openlp/core/ui/servicemanager.py

@@ -590,7 +590,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
                     zip_file.write(write_from, write_from)
                 with suppress(FileNotFoundError):
                     file_path.unlink()
-                os.link(temp_file.name, file_path)
+                os.link(temp_file.name, str(file_path))
             Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', file_path.parent)
         except (PermissionError, OSError) as error:
             self.log_exception('Failed to save service to disk: {name}'.format(name=temp_file.name))

tests/functional/openlp_core/lib/test_serviceitem.py

@@ -27,6 +27,7 @@ from unittest import TestCase
 from unittest.mock import MagicMock, patch
 
 from openlp.core.common import md5_hash
+from openlp.core.common.path import Path
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 from openlp.core.lib import ItemCapabilities, ServiceItem, ServiceItemType, FormattingTags
@@ -351,5 +352,5 @@ class TestServiceItem(TestCase, TestMixin):
             '"Amazing Grace! how sweet the s" has been returned as the title'
         assert '’Twas grace that taught my hea' == service_item.get_frame_title(1), \
             '"’Twas grace that taught my hea" has been returned as the title'
-        assert '/test/amazing_grace.mp3' == service_item.background_audio[0], \
+        assert Path('/test/amazing_grace.mp3') == service_item.background_audio[0], \
             '"/test/amazing_grace.mp3" should be in the background_audio list'

tests/functional/openlp_core/ui/test_mainwindow.py

@@ -23,6 +23,7 @@
 Package to test openlp.core.ui.mainwindow package.
 """
 import os
+from pathlib import Path
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
 
@@ -83,14 +84,13 @@ class TestMainWindow(TestCase, TestMixin):
         """
         # GIVEN a service as an argument to openlp
         service = os.path.join(TEST_RESOURCES_PATH, 'service', 'test.osz')
-        self.main_window.arguments = [service]
 
         # WHEN the argument is processed
         with patch.object(self.main_window.service_manager, 'load_file') as mocked_load_file:
             self.main_window.open_cmd_line_files(service)
 
         # THEN the service from the arguments is loaded
-        mocked_load_file.assert_called_with(service)
+        mocked_load_file.assert_called_with(Path(service))
 
     def test_cmd_line_arg(self):
         """

tests/functional/openlp_core/ui/test_servicemanager.py

@@ -623,10 +623,10 @@ class TestServiceManager(TestCase):
         # THEN: make_preview() should not have been called
         assert mocked_make_preview.call_count == 0, 'ServiceManager.make_preview() should not be called'
 
-    @patch('openlp.core.ui.servicemanager.shutil.copy')
     @patch('openlp.core.ui.servicemanager.zipfile')
     @patch('openlp.core.ui.servicemanager.ServiceManager.save_file_as')
-    def test_save_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy):
+    @patch('openlp.core.ui.servicemanager.os')
+    def test_save_file_raises_permission_error(self, mocked_os, mocked_save_file_as, mocked_zipfile):
         """
         Test that when a PermissionError is raised when trying to save a file, it is handled correctly
         """
@@ -636,50 +636,22 @@ class TestServiceManager(TestCase):
         Registry().register('main_window', mocked_main_window)
         Registry().register('application', MagicMock())
         service_manager = ServiceManager(None)
-        service_manager._service_path = os.path.join('temp', 'filename.osz')
+        service_manager._service_path = MagicMock()
         service_manager._save_lite = False
         service_manager.service_items = []
         service_manager.service_theme = 'Default'
         service_manager.service_manager_list = MagicMock()
         mocked_save_file_as.return_value = True
         mocked_zipfile.ZipFile.return_value = MagicMock()
-        mocked_shutil_copy.side_effect = PermissionError
+        mocked_os.link.side_effect = PermissionError
 
-        # WHEN: The service is saved and a PermissionError is thrown
+        # WHEN: The service is saved and a PermissionError is raised
         result = service_manager.save_file()
 
         # THEN: The "save_as" method is called to save the service
         assert result is True
         mocked_save_file_as.assert_called_with()
 
-    @patch('openlp.core.ui.servicemanager.shutil.copy')
-    @patch('openlp.core.ui.servicemanager.zipfile')
-    @patch('openlp.core.ui.servicemanager.ServiceManager.save_file_as')
-    def test_save_local_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy):
-        """
-        Test that when a PermissionError is raised when trying to save a local file, it is handled correctly
-        """
-        # GIVEN: A service manager, a service to save
-        mocked_main_window = MagicMock()
-        mocked_main_window.service_manager_settings_section = 'servicemanager'
-        Registry().register('main_window', mocked_main_window)
-        Registry().register('application', MagicMock())
-        service_manager = ServiceManager(None)
-        service_manager._service_path = os.path.join('temp', 'filename.osz')
-        service_manager._save_lite = False
-        service_manager.service_items = []
-        service_manager.service_theme = 'Default'
-        mocked_save_file_as.return_value = True
-        mocked_zipfile.ZipFile.return_value = MagicMock()
-        mocked_shutil_copy.side_effect = PermissionError
-
-        # WHEN: The service is saved and a PermissionError is thrown
-        result = service_manager.save_local_file()
-
-        # THEN: The "save_as" method is called to save the service
-        assert result is True
-        mocked_save_file_as.assert_called_with()
-
     @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
     def test_theme_change_global(self, mocked_regenerate_service_items):
         """