diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index 9e1ad413b..054697b19 100644 --- a/openlp/core/lib/serviceitem.py +++ b/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) - self.add_from_image(filename, text_image, background) + file_path = os.path.join(path, text_image) + 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) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 69fc118a6..48fb9a9c9 100644 --- a/openlp/core/ui/servicemanager.py +++ b/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)) diff --git a/tests/functional/openlp_core/lib/test_serviceitem.py b/tests/functional/openlp_core/lib/test_serviceitem.py index 63544458a..85d3b1050 100644 --- a/tests/functional/openlp_core/lib/test_serviceitem.py +++ b/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' diff --git a/tests/functional/openlp_core/ui/test_mainwindow.py b/tests/functional/openlp_core/ui/test_mainwindow.py index 81b29a939..25078f813 100644 --- a/tests/functional/openlp_core/ui/test_mainwindow.py +++ b/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): """ diff --git a/tests/functional/openlp_core/ui/test_servicemanager.py b/tests/functional/openlp_core/ui/test_servicemanager.py index 191bd236e..53c612698 100644 --- a/tests/functional/openlp_core/ui/test_servicemanager.py +++ b/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): """