From 4b1965520ccaec65ad84113dc5a6df0e425d9654 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Wed, 27 Dec 2017 16:06:36 +0000 Subject: [PATCH 01/11] Start on refactoring file saving --- openlp/core/ui/servicemanager.py | 136 ++++++++++++++----------------- 1 file changed, 61 insertions(+), 75 deletions(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 875a48fdf..e67637abc 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -27,8 +27,9 @@ import json import os import shutil import zipfile +from contextlib import suppress from datetime import datetime, timedelta -from tempfile import mkstemp +from tempfile import NamedTemporaryFile, mkstemp from PyQt5 import QtCore, QtGui, QtWidgets @@ -503,35 +504,9 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi service.append({'openlp_core': core}) return service - def save_file(self, field=None): - """ - Save the current service file. - - A temporary file is created so that we don't overwrite the existing one and leave a mangled service file should - there be an error when saving. Audio files are also copied into the service manager directory, and then packaged - into the zip file. - """ - if not self.file_name(): - return self.save_file_as() - temp_file, temp_file_name = mkstemp('.osz', 'openlp_') - # We don't need the file handle. - os.close(temp_file) - self.log_debug(temp_file_name) - path_file_name = str(self.file_name()) - path, file_name = os.path.split(path_file_name) - base_name = os.path.splitext(file_name)[0] - service_file_name = '{name}.osj'.format(name=base_name) - self.log_debug('ServiceManager.save_file - {name}'.format(name=path_file_name)) - Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', Path(path)) - service = self.create_basic_service() + def get_write_file_list(self): write_list = [] missing_list = [] - audio_files = [] - total_size = 0 - self.application.set_busy_cursor() - # Number of items + 1 to zip it - self.main_window.display_progress_bar(len(self.service_items) + 1) - # Get list of missing files, and list of files to write for item in self.service_items: if not item['service_item'].uses_file(): continue @@ -543,6 +518,32 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi missing_list.append(path_from) else: write_list.append(path_from) + return write_list, missing_list + + + + def save_file(self): + """ + Save the current service file. + + A temporary file is created so that we don't overwrite the existing one and leave a mangled service file should + there be an error when saving. Audio files are also copied into the service manager directory, and then packaged + into the zip file. + """ + file_path = self.file_name() + service_file_name = '{name}.osj'.format(name=file_path.suffix) + self.log_debug('ServiceManager.save_file - {name}'.format(name=file_path)) + Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', file_path.parent) + service = self.create_basic_service() + self.application.set_busy_cursor() + # Number of items + 1 to zip it + self.main_window.display_progress_bar(len(self.service_items) + 1) + # Get list of missing files, and list of files to write + + audio_files = [] + + write_list, missing_list = self.get_write_file_list() + if missing_list: self.application.set_normal_cursor() title = translate('OpenLP.ServiceManager', 'Service File(s) Missing') @@ -572,62 +573,47 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi # Add the service item to the service. service.append({'serviceitem': service_item}) self.repaint_service_list(-1, -1) + total_size = 0 for file_item in write_list: file_size = os.path.getsize(file_item) total_size += file_size self.log_debug('ServiceManager.save_file - ZIP contents size is %i bytes' % total_size) service_content = json.dumps(service) - # Usual Zip file cannot exceed 2GiB, file with Zip64 cannot be extracted using unzip in UNIX. - allow_zip_64 = (total_size > 2147483648 + len(service_content)) - self.log_debug('ServiceManager.save_file - allowZip64 is {text}'.format(text=allow_zip_64)) - zip_file = None - success = True + + self.main_window.increment_progress_bar() try: - zip_file = zipfile.ZipFile(temp_file_name, 'w', zipfile.ZIP_STORED, allow_zip_64) - # First we add service contents.. - zip_file.writestr(service_file_name, service_content) - # Finally add all the listed media files. - for write_from in write_list: - zip_file.write(write_from, write_from) - for audio_from, audio_to in audio_files: - audio_from = str(audio_from) - audio_to = str(audio_to) - if audio_from.startswith('audio'): - # When items are saved, they get new unique_identifier. Let's copy the file to the new location. - # Unused files can be ignored, OpenLP automatically cleans up the service manager dir on exit. - audio_from = os.path.join(self.service_path, audio_from) - save_file = os.path.join(self.service_path, audio_to) - save_path = os.path.split(save_file)[0] - create_paths(Path(save_path)) - if not os.path.exists(save_file): - shutil.copy(audio_from, save_file) - zip_file.write(audio_from, audio_to) - except OSError: - self.log_exception('Failed to save service to disk: {name}'.format(name=temp_file_name)) - self.main_window.error_message(translate('OpenLP.ServiceManager', 'Error Saving File'), - translate('OpenLP.ServiceManager', 'There was an error saving your file.')) - success = False - finally: - if zip_file: - zip_file.close() + with NamedTemporaryFile() as temp_file, \ + zipfile.ZipFile(temp_file, 'w') as zip_file: + # First we add service contents.. + zip_file.writestr(service_file_name, service_content) + # Finally add all the listed media files. + for write_from in write_list: + zip_file.write(write_from, write_from) + for audio_from, audio_to in audio_files: + audio_from = str(audio_from) + audio_to = str(audio_to) + if audio_from.startswith('audio'): + # When items are saved, they get new unique_identifier. Let's copy the file to the new location. + # Unused files can be ignored, OpenLP automatically cleans up the service manager dir on exit. + audio_from = os.path.join(self.service_path, audio_from) + zip_file.write(audio_from, audio_to) + with suppress(FileNotFoundError): + file_path.unlink() + os.link(temp_file.name, file_path) + except (PermissionError, OSError) as error: + self.log_exception('Failed to save service to disk: {name}'.format(name=temp_file.name)) + self.main_window.error_message( + translate('OpenLP.ServiceManager', 'Error Saving File'), + translate('OpenLP.ServiceManager', 'There was an error saving your file.\n\n{error}').format(error=error)) + return self.save_file_as() + + self.main_window.finished_progress_bar() self.application.set_normal_cursor() - if success: - try: - shutil.copy(temp_file_name, path_file_name) - except (shutil.Error, PermissionError): - return self.save_file_as() - except OSError as ose: - QtWidgets.QMessageBox.critical(self, translate('OpenLP.ServiceManager', 'Error Saving File'), - translate('OpenLP.ServiceManager', 'An error occurred while writing the ' - 'service file: {error}').format(error=ose.strerror), - QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok)) - success = False - self.main_window.add_recent_file(path_file_name) - self.set_modified(False) - delete_file(Path(temp_file_name)) - return success + self.main_window.add_recent_file(file_path) + self.set_modified(False) + return True def save_local_file(self): """ From a1ea35c4e766e0dc38abc20967a8dbba229a3fde Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 30 Dec 2017 08:08:10 +0000 Subject: [PATCH 02/11] continued refactor of saving packaged files --- openlp/core/ui/servicemanager.py | 61 +++++++++++++------------------- 1 file changed, 25 insertions(+), 36 deletions(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index e67637abc..0c2b9b473 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -37,6 +37,7 @@ from openlp.core.common import ThemeLevel, delete_file from openlp.core.common.actions import ActionList, CategoryOrder from openlp.core.common.applocation import AppLocation from openlp.core.common.i18n import UiStrings, format_time, translate +from openlp.core.common.json import OpenLPJsonDecoder, OpenLPJsonEncoder from openlp.core.common.mixins import LogMixin, RegistryProperties from openlp.core.common.path import Path, create_paths, str_to_path from openlp.core.common.registry import Registry, RegistryBase @@ -515,13 +516,11 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi if path_from in write_list or path_from in missing_list: continue if not os.path.exists(path_from): - missing_list.append(path_from) + missing_list.append(Path(path_from)) else: - write_list.append(path_from) + write_list.append(Path(path_from)) return write_list, missing_list - - def save_file(self): """ Save the current service file. @@ -531,16 +530,15 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi into the zip file. """ file_path = self.file_name() - service_file_name = '{name}.osj'.format(name=file_path.suffix) self.log_debug('ServiceManager.save_file - {name}'.format(name=file_path)) - Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', file_path.parent) - service = self.create_basic_service() + service_file_name = '{name}.osj'.format(name=file_path.suffix) self.application.set_busy_cursor() - # Number of items + 1 to zip it - self.main_window.display_progress_bar(len(self.service_items) + 1) + + service = self.create_basic_service() + + # Get list of missing files, and list of files to write - audio_files = [] write_list, missing_list = self.get_write_file_list() @@ -550,12 +548,11 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi message = translate('OpenLP.ServiceManager', 'The following file(s) in the service are missing: {name}\n\n' 'These files will be removed if you continue to save.' - ).format(name="\n\t".join(missing_list)) + ).format(name='\n\t'.join(missing_list)) answer = QtWidgets.QMessageBox.critical(self, title, message, QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok | QtWidgets.QMessageBox.Cancel)) if answer == QtWidgets.QMessageBox.Cancel: - self.main_window.finished_progress_bar() return False # Check if item contains a missing file. for item in list(self.service_items): @@ -563,25 +560,24 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi item['service_item'].remove_invalid_frames(missing_list) if item['service_item'].missing_frames(): self.service_items.remove(item) - else: - service_item = item['service_item'].get_service_repr(self._save_lite) - if service_item['header']['background_audio']: - for i, file_name in enumerate(service_item['header']['background_audio']): - new_file = os.path.join('audio', item['service_item'].unique_identifier, str(file_name)) - audio_files.append((file_name, new_file)) - service_item['header']['background_audio'][i] = new_file - # Add the service item to the service. - service.append({'serviceitem': service_item}) + continue + service_item = item['service_item'].get_service_repr(self._save_lite) + for audio_path in service_item['header']['background_audio']: + write_list.append(audio_path) + # Add the service item to the service. + service.append({'serviceitem': service_item}) self.repaint_service_list(-1, -1) - total_size = 0 + service_content = json.dumps(service, cls=OpenLPJsonEncoder) + total_size = len(bytes(service_content, encoding='utf-8')) for file_item in write_list: file_size = os.path.getsize(file_item) total_size += file_size self.log_debug('ServiceManager.save_file - ZIP contents size is %i bytes' % total_size) - service_content = json.dumps(service) + # Number of items + 1 to zip it + self.main_window.display_progress_bar(total_size) - self.main_window.increment_progress_bar() + #self.main_window.increment_progress_bar() try: with NamedTemporaryFile() as temp_file, \ zipfile.ZipFile(temp_file, 'w') as zip_file: @@ -590,17 +586,10 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi # Finally add all the listed media files. for write_from in write_list: zip_file.write(write_from, write_from) - for audio_from, audio_to in audio_files: - audio_from = str(audio_from) - audio_to = str(audio_to) - if audio_from.startswith('audio'): - # When items are saved, they get new unique_identifier. Let's copy the file to the new location. - # Unused files can be ignored, OpenLP automatically cleans up the service manager dir on exit. - audio_from = os.path.join(self.service_path, audio_from) - zip_file.write(audio_from, audio_to) - with suppress(FileNotFoundError): - file_path.unlink() - os.link(temp_file.name, file_path) + with suppress(FileNotFoundError): + file_path.unlink() + os.link(temp_file.name, 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)) self.main_window.error_message( @@ -767,7 +756,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi if 'p_file' in locals(): file_to = open(p_file, 'r') if p_file.endswith('osj'): - items = json.load(file_to) + items = json.load(file_to, cls=OpenLPJsonDecoder) else: critical_error_message_box(message=translate('OpenLP.ServiceManager', 'The service file you are trying to open is in an old ' From e1400dc227da3667e2cf3099e79ff0be839c53ce Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 6 Jan 2018 09:55:19 +0000 Subject: [PATCH 03/11] Finally merge the save lite method in to the 'standard' save method' --- openlp/core/lib/serviceitem.py | 11 +- openlp/core/ui/mainwindow.py | 2 +- openlp/core/ui/servicemanager.py | 234 ++++++++++--------------------- 3 files changed, 75 insertions(+), 172 deletions(-) diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index 3a824d424..1ae450cad 100644 --- a/openlp/core/lib/serviceitem.py +++ b/openlp/core/lib/serviceitem.py @@ -427,13 +427,8 @@ class ServiceItem(RegistryProperties): self.has_original_files = True if 'background_audio' in header: self.background_audio = [] - for filename in header['background_audio']: - # Give them real file paths. - filepath = str(filename) - if path: - # Windows can handle both forward and backward slashes, so we use ntpath to get the basename - filepath = os.path.join(path, ntpath.basename(str(filename))) - self.background_audio.append(filepath) + for file_path in header['background_audio']: + self.background_audio.append(file_path) self.theme_overwritten = header.get('theme_overwritten', False) if self.service_item_type == ServiceItemType.Text: for slide in service_item['serviceitem']['data']: @@ -538,7 +533,7 @@ class ServiceItem(RegistryProperties): Confirms if the ServiceItem uses a file """ return self.service_item_type == ServiceItemType.Image or \ - (self.service_item_type == ServiceItemType.Command and not self.is_capable(ItemCapabilities.IsOptical)) + (self.service_item_type == ServiceItemType.Command and not self.is_capable(ItemCapabilities.IsOptical)) def is_text(self): """ diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index ee07cbd69..a2aef0e6d 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -1382,4 +1382,4 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): if not isinstance(filename, str): filename = str(filename, sys.getfilesystemencoding()) if filename.endswith(('.osz', '.oszl')): - self.service_manager_contents.load_file(filename) + self.service_manager_contents.load_file(Path(filename)) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 0c2b9b473..0e27923a9 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -43,6 +43,7 @@ from openlp.core.common.path import Path, create_paths, str_to_path from openlp.core.common.registry import Registry, RegistryBase from openlp.core.common.settings import Settings from openlp.core.lib import ServiceItem, ItemCapabilities, PluginStatus, build_icon +from openlp.core.lib.exceptions import ValidationError from openlp.core.lib.ui import critical_error_message_box, create_widget_action, find_and_set_in_combo_box from openlp.core.ui import ServiceNoteForm, ServiceItemEditForm, StartTimeForm from openlp.core.widgets.dialogs import FileDialog @@ -451,7 +452,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi else: file_path = str_to_path(load_file) Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', file_path.parent) - self.load_file(str(file_path)) + self.load_file(file_path) def save_modified_service(self): """ @@ -477,7 +478,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi elif result == QtWidgets.QMessageBox.Save: self.decide_save_method() sender = self.sender() - self.load_file(sender.data()) + self.load_file(Path(sender.data())) def new_file(self): """ @@ -509,16 +510,19 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi write_list = [] missing_list = [] for item in self.service_items: - if not item['service_item'].uses_file(): - continue - for frame in item['service_item'].get_frames(): - path_from = item['service_item'].get_frame_path(frame=frame) - if path_from in write_list or path_from in missing_list: + if item['service_item'].uses_file(): + for frame in item['service_item'].get_frames(): + path_from = item['service_item'].get_frame_path(frame=frame) + if path_from in write_list or path_from in missing_list: + continue + if not os.path.exists(path_from): + missing_list.append(Path(path_from)) + else: + write_list.append(Path(path_from)) + for audio_path in item['service_item'].background_audio: + if audio_path in write_list: continue - if not os.path.exists(path_from): - missing_list.append(Path(path_from)) - else: - write_list.append(Path(path_from)) + write_list.append(audio_path) return write_list, missing_list def save_file(self): @@ -531,39 +535,37 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi """ file_path = self.file_name() self.log_debug('ServiceManager.save_file - {name}'.format(name=file_path)) - service_file_name = '{name}.osj'.format(name=file_path.suffix) self.application.set_busy_cursor() service = self.create_basic_service() + write_list = [] + missing_list = [] - # Get list of missing files, and list of files to write + if not self._save_lite: + write_list, missing_list = self.get_write_file_list() - - write_list, missing_list = self.get_write_file_list() - - if missing_list: - self.application.set_normal_cursor() - title = translate('OpenLP.ServiceManager', 'Service File(s) Missing') - message = translate('OpenLP.ServiceManager', - 'The following file(s) in the service are missing: {name}\n\n' - 'These files will be removed if you continue to save.' - ).format(name='\n\t'.join(missing_list)) - answer = QtWidgets.QMessageBox.critical(self, title, message, - QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok | - QtWidgets.QMessageBox.Cancel)) - if answer == QtWidgets.QMessageBox.Cancel: - return False + if missing_list: + self.application.set_normal_cursor() + title = translate('OpenLP.ServiceManager', 'Service File(s) Missing') + message = translate('OpenLP.ServiceManager', + 'The following file(s) in the service are missing: {name}\n\n' + 'These files will be removed if you continue to save.' + ).format(name='\n\t'.join(missing_list)) + answer = QtWidgets.QMessageBox.critical(self, title, message, + QtWidgets.QMessageBox.StandardButtons( + QtWidgets.QMessageBox.Ok | QtWidgets.QMessageBox.Cancel)) + if answer == QtWidgets.QMessageBox.Cancel: + return False # Check if item contains a missing file. for item in list(self.service_items): - self.main_window.increment_progress_bar() - item['service_item'].remove_invalid_frames(missing_list) - if item['service_item'].missing_frames(): - self.service_items.remove(item) - continue + if not self._save_lite: + item['service_item'].remove_invalid_frames(missing_list) + if item['service_item'].missing_frames(): + self.service_items.remove(item) + continue service_item = item['service_item'].get_service_repr(self._save_lite) - for audio_path in service_item['header']['background_audio']: - write_list.append(audio_path) + # Add the service item to the service. service.append({'serviceitem': service_item}) self.repaint_service_list(-1, -1) @@ -582,7 +584,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi with NamedTemporaryFile() as temp_file, \ zipfile.ZipFile(temp_file, 'w') as zip_file: # First we add service contents.. - zip_file.writestr(service_file_name, service_content) + zip_file.writestr('service_data.osj', service_content) # Finally add all the listed media files. for write_from in write_list: zip_file.write(write_from, write_from) @@ -604,61 +606,6 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi self.set_modified(False) return True - def save_local_file(self): - """ - Save the current service file but leave all the file references alone to point to the current machine. - This format is not transportable as it will not contain any files. - """ - if not self.file_name(): - return self.save_file_as() - temp_file, temp_file_name = mkstemp('.oszl', 'openlp_') - # We don't need the file handle. - os.close(temp_file) - self.log_debug(temp_file_name) - path_file_name = str(self.file_name()) - path, file_name = os.path.split(path_file_name) - base_name = os.path.splitext(file_name)[0] - service_file_name = '{name}.osj'.format(name=base_name) - self.log_debug('ServiceManager.save_file - {name}'.format(name=path_file_name)) - Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', Path(path)) - service = self.create_basic_service() - self.application.set_busy_cursor() - # Number of items + 1 to zip it - self.main_window.display_progress_bar(len(self.service_items) + 1) - for item in self.service_items: - self.main_window.increment_progress_bar() - service_item = item['service_item'].get_service_repr(self._save_lite) - # TODO: check for file item on save. - service.append({'serviceitem': service_item}) - self.main_window.increment_progress_bar() - service_content = json.dumps(service) - zip_file = None - success = True - self.main_window.increment_progress_bar() - try: - zip_file = zipfile.ZipFile(temp_file_name, 'w', zipfile.ZIP_STORED, True) - # First we add service contents. - zip_file.writestr(service_file_name, service_content) - except OSError: - self.log_exception('Failed to save service to disk: {name}'.format(name=temp_file_name)) - self.main_window.error_message(translate('OpenLP.ServiceManager', 'Error Saving File'), - translate('OpenLP.ServiceManager', 'There was an error saving your file.')) - success = False - finally: - if zip_file: - zip_file.close() - self.main_window.finished_progress_bar() - self.application.set_normal_cursor() - if success: - try: - shutil.copy(temp_file_name, path_file_name) - except (shutil.Error, PermissionError): - return self.save_file_as() - self.main_window.add_recent_file(path_file_name) - self.set_modified(False) - delete_file(Path(temp_file_name)) - return success - def save_file_as(self, field=None): """ Get a file name and then call :func:`ServiceManager.save_file` to save the file. @@ -718,87 +665,47 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi """ if not self.file_name(): return self.save_file_as() - if self._save_lite: - return self.save_local_file() - else: - return self.save_file() + return self.save_file() - def load_file(self, file_name): + def load_file(self, file_path): """ Load an existing service file - :param file_name: + :param file_path: """ - if not file_name: + if not file_path.exists(): return False - file_name = str(file_name) - if not os.path.exists(file_name): - return False - zip_file = None - file_to = None + service_data = None self.application.set_busy_cursor() try: - zip_file = zipfile.ZipFile(file_name) - for zip_info in zip_file.infolist(): - try: - ucs_file = zip_info.filename - except UnicodeDecodeError: - self.log_exception('file_name "{name}" is not valid UTF-8'.format(name=zip_info.file_name)) - critical_error_message_box(message=translate('OpenLP.ServiceManager', - 'File is not a valid service.\n The content encoding is not UTF-8.')) - continue - os_file = ucs_file.replace('/', os.path.sep) - os_file = os.path.basename(os_file) - self.log_debug('Extract file: {name}'.format(name=os_file)) - zip_info.filename = os_file - zip_file.extract(zip_info, self.service_path) - if os_file.endswith('osj') or os_file.endswith('osd'): - p_file = os.path.join(self.service_path, os_file) - if 'p_file' in locals(): - file_to = open(p_file, 'r') - if p_file.endswith('osj'): - items = json.load(file_to, cls=OpenLPJsonDecoder) - else: - critical_error_message_box(message=translate('OpenLP.ServiceManager', - 'The service file you are trying to open is in an old ' - 'format.\n Please save it using OpenLP 2.0.2 or ' - 'greater.')) - return - file_to.close() + with zipfile.ZipFile(str(file_path)) as zip_file: + for zip_info in zip_file.infolist(): + self.log_debug('Extract file: {name}'.format(name=zip_info.filename)) + # The json file has been called 'service_data.osj' since OpenLP 3.0 + if zip_info.filename == 'service_data.osj' or zip_info.filename.endswith('osj'): + with zip_file.open(zip_info, 'r') as json_file: + service_data = json_file.read() + else: + zip_info.filename = os.path.basename(zip_info.filename) + zip_file.extract(zip_info, str(self.service_path)) + if service_data: + items = json.loads(service_data, cls=OpenLPJsonDecoder) self.new_file() - self.set_file_name(str_to_path(file_name)) - self.main_window.display_progress_bar(len(items)) self.process_service_items(items) - delete_file(Path(p_file)) - self.main_window.add_recent_file(file_name) + self.set_file_name(file_path) + self.main_window.display_progress_bar(len(items)) + self.main_window.add_recent_file(file_path) self.set_modified(False) - Settings().setValue('servicemanager/last file', Path(file_name)) + Settings().setValue('servicemanager/last file', file_path) else: - critical_error_message_box(message=translate('OpenLP.ServiceManager', 'File is not a valid service.')) - self.log_error('File contains no service data') - except (OSError, NameError): - self.log_exception('Problem loading service file {name}'.format(name=file_name)) - critical_error_message_box(message=translate('OpenLP.ServiceManager', - 'File could not be opened because it is corrupt.')) - except zipfile.BadZipFile: - if os.path.getsize(file_name) == 0: - self.log_exception('Service file is zero sized: {name}'.format(name=file_name)) - QtWidgets.QMessageBox.information(self, translate('OpenLP.ServiceManager', 'Empty File'), - translate('OpenLP.ServiceManager', - 'This service file does not contain ' - 'any data.')) - else: - self.log_exception('Service file is cannot be extracted as zip: {name}'.format(name=file_name)) - QtWidgets.QMessageBox.information(self, translate('OpenLP.ServiceManager', 'Corrupt File'), - translate('OpenLP.ServiceManager', - 'This file is either corrupt or it is not an OpenLP 2 ' - 'service file.')) + raise ValidationError(msg='No service data found') + except (NameError, OSError, ValidationError, zipfile.BadZipFile) as e: + self.log_exception('Problem loading service file {name}'.format(name=file_path)) + critical_error_message_box( + message=translate('OpenLP.ServiceManager', + 'The service file {file_path} could not be loaded because it is either corrupt, or ' + 'not a valid OpenLP 2 or OpenLP 3 service file.'.format(file_path=file_path))) self.application.set_normal_cursor() return - finally: - if file_to: - file_to.close() - if zip_file: - zip_file.close() self.main_window.finished_progress_bar() self.application.set_normal_cursor() self.repaint_service_list(-1, -1) @@ -813,7 +720,8 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi self.main_window.increment_progress_bar() service_item = ServiceItem() if 'openlp_core' in item: - item = item.get('openlp_core') + item = item['openlp_core'] + self._save_lite = item.get('lite-service', False) theme = item.get('service-theme', None) if theme: find_and_set_in_combo_box(self.theme_combo_box, theme, set_missing=False) @@ -836,9 +744,9 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi Load the last service item from the service manager when the service was last closed. Can be blank if there was no service present. """ - file_name = str_to_path(Settings().value('servicemanager/last file')) - if file_name: - self.load_file(file_name) + file_path = Settings().value('servicemanager/last file') + if file_path: + self.load_file(file_path) def context_menu(self, point): """ From 2095a00dc9ab7643753716f0fb69b3f4e86e606f Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 13 Jan 2018 07:24:20 +0000 Subject: [PATCH 04/11] Fix up code so test passes --- openlp/core/lib/serviceitem.py | 10 ++++- openlp/core/ui/servicemanager.py | 2 +- .../openlp_core/lib/test_serviceitem.py | 3 +- .../openlp_core/ui/test_mainwindow.py | 4 +- .../openlp_core/ui/test_servicemanager.py | 38 +++---------------- 5 files changed, 18 insertions(+), 39 deletions(-) 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): """ From 42465605ac5986b575007d0d0179cba2036f4d70 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 13 Jan 2018 09:02:29 +0000 Subject: [PATCH 05/11] fix saving and loading progress bar Fixes: https://launchpad.net/bugs/1734432 --- openlp/core/ui/mainwindow.py | 9 ++++++--- openlp/core/ui/servicemanager.py | 21 ++++++++++++--------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index c01cd47cf..5046dd6ce 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -1315,11 +1315,14 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): self.load_progress_bar.setValue(0) self.application.process_events() - def increment_progress_bar(self): + def increment_progress_bar(self, increment=1): """ - Increase the Progress Bar value by 1 + Increase the Progress Bar by the value in increment. + + :param int increment: The value you to increase the progress bar by. """ - self.load_progress_bar.setValue(self.load_progress_bar.value() + 1) + # TODO: Test increment default value + self.load_progress_bar.setValue(self.load_progress_bar.value() + increment) self.application.process_events() def finished_progress_bar(self): diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 48fb9a9c9..f10ce85ab 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -570,24 +570,25 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi service.append({'serviceitem': service_item}) self.repaint_service_list(-1, -1) service_content = json.dumps(service, cls=OpenLPJsonEncoder) - total_size = len(bytes(service_content, encoding='utf-8')) + service_content_size = len(bytes(service_content, encoding='utf-8')) + total_size = service_content_size for file_item in write_list: - file_size = os.path.getsize(file_item) - total_size += file_size + total_size += file_item.stat().st_size self.log_debug('ServiceManager.save_file - ZIP contents size is %i bytes' % total_size) # Number of items + 1 to zip it self.main_window.display_progress_bar(total_size) - #self.main_window.increment_progress_bar() try: with NamedTemporaryFile() as temp_file, \ zipfile.ZipFile(temp_file, 'w') as zip_file: # First we add service contents.. zip_file.writestr('service_data.osj', service_content) + self.main_window.increment_progress_bar(service_content_size) # Finally add all the listed media files. - for write_from in write_list: - zip_file.write(write_from, write_from) + for write_path in write_list: + zip_file.write(str(write_path), str(write_path)) + self.main_window.increment_progress_bar(write_path.stat().st_size) with suppress(FileNotFoundError): file_path.unlink() os.link(temp_file.name, str(file_path)) @@ -678,6 +679,10 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi self.application.set_busy_cursor() try: with zipfile.ZipFile(str(file_path)) as zip_file: + compressed_size = 0 + for zip_info in zip_file.infolist(): + compressed_size += zip_info.compress_size + self.main_window.display_progress_bar(compressed_size) for zip_info in zip_file.infolist(): self.log_debug('Extract file: {name}'.format(name=zip_info.filename)) # The json file has been called 'service_data.osj' since OpenLP 3.0 @@ -687,12 +692,12 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi else: zip_info.filename = os.path.basename(zip_info.filename) zip_file.extract(zip_info, str(self.service_path)) + self.main_window.increment_progress_bar(zip_info.compress_size) if service_data: items = json.loads(service_data, cls=OpenLPJsonDecoder) self.new_file() self.process_service_items(items) self.set_file_name(file_path) - self.main_window.display_progress_bar(len(items)) self.main_window.add_recent_file(file_path) self.set_modified(False) Settings().setValue('servicemanager/last file', file_path) @@ -704,8 +709,6 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi message=translate('OpenLP.ServiceManager', 'The service file {file_path} could not be loaded because it is either corrupt, or ' 'not a valid OpenLP 2 or OpenLP 3 service file.'.format(file_path=file_path))) - self.application.set_normal_cursor() - return self.main_window.finished_progress_bar() self.application.set_normal_cursor() self.repaint_service_list(-1, -1) From 21b362c9a2101f277ef8a29a9169f48d93a3c071 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sun, 14 Jan 2018 09:01:00 +0000 Subject: [PATCH 06/11] doc string! --- openlp/core/ui/servicemanager.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index f10ce85ab..bfe0b0a6a 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -507,6 +507,11 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi return service def get_write_file_list(self): + """ + + :return: + :rtype: (list[openlp.core.common.path.Path], list[openlp.core.common.path.Path]) + """ write_list = [] missing_list = [] for item in self.service_items: From 7ced39594f76073472901fb0d5dbbbf25712cff4 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Fri, 19 Jan 2018 21:00:56 +0000 Subject: [PATCH 07/11] Add test --- openlp/core/ui/servicemanager.py | 2 -- .../openlp_core/ui/test_mainwindow.py | 27 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index bfe0b0a6a..6c3e1ca9b 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -580,8 +580,6 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi for file_item in write_list: total_size += file_item.stat().st_size self.log_debug('ServiceManager.save_file - ZIP contents size is %i bytes' % total_size) - - # Number of items + 1 to zip it self.main_window.display_progress_bar(total_size) try: diff --git a/tests/functional/openlp_core/ui/test_mainwindow.py b/tests/functional/openlp_core/ui/test_mainwindow.py index 4ae21e1e8..888ac435d 100644 --- a/tests/functional/openlp_core/ui/test_mainwindow.py +++ b/tests/functional/openlp_core/ui/test_mainwindow.py @@ -242,3 +242,30 @@ class TestMainWindow(TestCase, TestMixin): # THEN: projector_manager_dock.setVisible should had been called once mocked_dock.setVisible.assert_called_once_with(False) + + def test_increment_progress_bar_default_increment(self): + """ + Test that increment_progress_bar increments the progress bar by 1 when called without the `increment` arg. + """ + # GIVEN: A mocked progress bar + with patch.object(self.main_window, 'load_progress_bar', **{'value.return_value': 0}) as mocked_progress_bar: + + # WHEN: Calling increment_progress_bar without the `increment` arg + self.main_window.increment_progress_bar() + + # THEN: The progress bar value should have been incremented by 1 + mocked_progress_bar.setValue.assert_called_once_with(1) + + def test_increment_progress_bar_custom_increment(self): + """ + Test that increment_progress_bar increments the progress bar by the `increment` arg when called with the + `increment` arg with a set value. + """ + # GIVEN: A mocked progress bar + with patch.object(self.main_window, 'load_progress_bar', **{'value.return_value': 0}) as mocked_progress_bar: + + # WHEN: Calling increment_progress_bar with `increment` set to 10 + self.main_window.increment_progress_bar(increment=10) + + # THEN: The progress bar value should have been incremented by 10 + mocked_progress_bar.setValue.assert_called_once_with(10) From 368b0ce75c4ee54bbfa3db918829329d2baafa50 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 20 Jan 2018 09:30:30 +0000 Subject: [PATCH 08/11] Tidy ups --- openlp/core/ui/mainwindow.py | 1 - openlp/core/ui/servicemanager.py | 7 ++----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 958077891..a48fb3dfe 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -1320,7 +1320,6 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): :param int increment: The value you to increase the progress bar by. """ - # TODO: Test increment default value self.load_progress_bar.setValue(self.load_progress_bar.value() + increment) self.application.process_events() diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 6c3e1ca9b..e4ca1d6fe 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -508,8 +508,9 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi def get_write_file_list(self): """ + Get a list of files used in the service and files that are missing. - :return: + :return: A list of files used in the service that exist, and a list of files that don't. :rtype: (list[openlp.core.common.path.Path], list[openlp.core.common.path.Path]) """ write_list = [] @@ -570,7 +571,6 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi self.service_items.remove(item) continue service_item = item['service_item'].get_service_repr(self._save_lite) - # Add the service item to the service. service.append({'serviceitem': service_item}) self.repaint_service_list(-1, -1) @@ -581,7 +581,6 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi total_size += file_item.stat().st_size self.log_debug('ServiceManager.save_file - ZIP contents size is %i bytes' % total_size) self.main_window.display_progress_bar(total_size) - try: with NamedTemporaryFile() as temp_file, \ zipfile.ZipFile(temp_file, 'w') as zip_file: @@ -602,8 +601,6 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi translate('OpenLP.ServiceManager', 'Error Saving File'), translate('OpenLP.ServiceManager', 'There was an error saving your file.\n\n{error}').format(error=error)) return self.save_file_as() - - self.main_window.finished_progress_bar() self.application.set_normal_cursor() self.main_window.add_recent_file(file_path) From 7c68e9d022b4958b5c2f4aeac213861302eacb4d Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sun, 21 Jan 2018 07:40:26 +0000 Subject: [PATCH 09/11] pep --- openlp/core/lib/serviceitem.py | 2 +- openlp/core/ui/servicemanager.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index 054697b19..94a0e19f1 100644 --- a/openlp/core/lib/serviceitem.py +++ b/openlp/core/lib/serviceitem.py @@ -539,7 +539,7 @@ class ServiceItem(RegistryProperties): Confirms if the ServiceItem uses a file """ return self.service_item_type == ServiceItemType.Image or \ - (self.service_item_type == ServiceItemType.Command and not self.is_capable(ItemCapabilities.IsOptical)) + (self.service_item_type == ServiceItemType.Command and not self.is_capable(ItemCapabilities.IsOptical)) def is_text(self): """ diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index e4ca1d6fe..0946f2571 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -599,7 +599,8 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi self.log_exception('Failed to save service to disk: {name}'.format(name=temp_file.name)) self.main_window.error_message( translate('OpenLP.ServiceManager', 'Error Saving File'), - translate('OpenLP.ServiceManager', 'There was an error saving your file.\n\n{error}').format(error=error)) + translate('OpenLP.ServiceManager', + 'There was an error saving your file.\n\n{error}').format(error=error)) return self.save_file_as() self.main_window.finished_progress_bar() self.application.set_normal_cursor() From ee144ad68b025ff206496f78fe6ec4d1bb387219 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Mon, 22 Jan 2018 20:41:30 +0000 Subject: [PATCH 10/11] Save the temp file in a different dir --- openlp/core/ui/servicemanager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 0946f2571..7143159a4 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -29,7 +29,7 @@ import shutil import zipfile from contextlib import suppress from datetime import datetime, timedelta -from tempfile import NamedTemporaryFile, mkstemp +from tempfile import NamedTemporaryFile from PyQt5 import QtCore, QtGui, QtWidgets @@ -39,7 +39,7 @@ from openlp.core.common.applocation import AppLocation from openlp.core.common.i18n import UiStrings, format_time, translate from openlp.core.common.json import OpenLPJsonDecoder, OpenLPJsonEncoder from openlp.core.common.mixins import LogMixin, RegistryProperties -from openlp.core.common.path import Path, create_paths, str_to_path +from openlp.core.common.path import Path, str_to_path from openlp.core.common.registry import Registry, RegistryBase from openlp.core.common.settings import Settings from openlp.core.lib import ServiceItem, ItemCapabilities, PluginStatus, build_icon @@ -582,7 +582,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi self.log_debug('ServiceManager.save_file - ZIP contents size is %i bytes' % total_size) self.main_window.display_progress_bar(total_size) try: - with NamedTemporaryFile() as temp_file, \ + with NamedTemporaryFile(dir=str(file_path.parent), prefix='.') as temp_file, \ zipfile.ZipFile(temp_file, 'w') as zip_file: # First we add service contents.. zip_file.writestr('service_data.osj', service_content) From 4423b61fc99882d3992cfa886ae91af814028b53 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Mon, 22 Jan 2018 21:37:00 +0000 Subject: [PATCH 11/11] Small fix --- openlp/core/ui/servicemanager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 7143159a4..d2453480b 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -596,7 +596,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi 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)) + self.log_exception('Failed to save service to disk: {name}'.format(name=file_path)) self.main_window.error_message( translate('OpenLP.ServiceManager', 'Error Saving File'), translate('OpenLP.ServiceManager',