openlp
/
openlp
mirror of https://gitlab.com/openlp/openlp.git
1
0
Fork 1
Code Issues Projects Releases Wiki Activity

Merge branch 'move-not-link-servicefile' into 'master' 

Use a simpler approach when creating a tmp file when saving service files

See merge request openlp/openlp!550
This commit is contained in:
Raoul Snyman 2023-01-15 22:40:53 +00:00
parent c6f077af33 3f68c156d0
commit a7d46a3a01
2 changed files with 7 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
openlp/core/ui/servicemanager.py
View File

@ -29,7 +29,6 @@ import zipfile
from contextlib import suppress
from datetime import datetime, timedelta
from pathlib import Path
from tempfile import NamedTemporaryFile
from PyQt5 import QtCore, QtGui, QtWidgets
@ -690,8 +689,8 @@ 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(1000)
try:
with NamedTemporaryFile(dir=str(file_path.parent), prefix='.') as temp_file, \
zipfile.ZipFile(temp_file, 'w', zipfile.ZIP_DEFLATED) as zip_file:
tmp_file_path = str(file_path) + ".saving"
with zipfile.ZipFile(tmp_file_path, 'w', zipfile.ZIP_DEFLATED) 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 / total_size * 1000)
@ -701,11 +700,8 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
self.main_window.increment_progress_bar(local_file_item.stat().st_size / total_size * 1000)
with suppress(FileNotFoundError):
file_path.unlink()
# Try to link rather than copy to prevent writing another file
try:
os.link(temp_file.name, file_path)
except OSError:
shutil.copyfile(temp_file.name, file_path)
# Move rather than copy to prevent writing another file if possible
shutil.move(tmp_file_path, file_path)
self.settings.setValue('servicemanager/last directory', file_path.parent)
except (PermissionError, OSError) as error:
self.log_exception('Failed to save service to disk: {name}'.format(name=file_path))

45
tests/openlp_core/ui/test_servicemanager.py
View File

@ -690,11 +690,8 @@ def test_single_click_timeout_double(mocked_make_live, mocked_make_preview, sett
@patch('openlp.core.ui.servicemanager.zipfile')
@patch('openlp.core.ui.servicemanager.ServiceManager.save_file_as')
@patch('openlp.core.ui.servicemanager.os')
@patch('openlp.core.ui.servicemanager.shutil')
@patch('openlp.core.ui.servicemanager.NamedTemporaryFile')
def test_save_file_raises_permission_error(mocked_temp_file, mocked_shutil, mocked_os, mocked_save_file_as,
mocked_zipfile, settings):
def test_save_file_raises_permission_error(mocked_shutil, mocked_save_file_as, mocked_zipfile, settings):
"""
Test that when a PermissionError is raised when trying to save a file, it is handled correctly
"""
@ -709,8 +706,7 @@ def test_save_file_raises_permission_error(mocked_temp_file, mocked_shutil, mock
service_manager.service_manager_list = MagicMock()
mocked_save_file_as.return_value = False
mocked_zipfile.ZipFile.return_value = MagicMock()
mocked_os.link.side_effect = PermissionError
mocked_shutil.copyfile.side_effect = PermissionError
mocked_shutil.move.side_effect = PermissionError
# WHEN: The service is saved and a PermissionError is raised
result = service_manager.save_file()
@ -725,9 +721,7 @@ def test_save_file_raises_permission_error(mocked_temp_file, mocked_shutil, mock
@patch('openlp.core.ui.servicemanager.os')
@patch('openlp.core.ui.servicemanager.shutil')
@patch('openlp.core.ui.servicemanager.len')
@patch('openlp.core.ui.servicemanager.NamedTemporaryFile')
def test_save_file_large_file(mocked_temp_file, mocked_len, mocked_shutil, mocked_os, mocked_save_file_as,
mocked_zipfile, registry):
def test_save_file_large_file(mocked_len, mocked_shutil, mocked_os, mocked_save_file_as, mocked_zipfile, registry):
"""
Test that when a file size size larger than a 32bit signed int is attempted to save, the progress bar
should be provided a value that fits in a 32bit int (because it's passed to C++ as a 32bit unsigned int)
@ -759,39 +753,6 @@ def test_save_file_large_file(mocked_temp_file, mocked_len, mocked_shutil, mocke
mocked_save_file_as.assert_not_called()
@patch('openlp.core.ui.servicemanager.zipfile')
@patch('openlp.core.ui.servicemanager.ServiceManager.save_file_as')
@patch('openlp.core.ui.servicemanager.os')
@patch('openlp.core.ui.servicemanager.shutil')
@patch('openlp.core.ui.servicemanager.NamedTemporaryFile')
def test_save_file_falls_back_to_shutil(mocked_temp_file, mocked_shutil, mocked_os, mocked_save_file_as, mocked_zipfile,
registry):
"""
Test that when a PermissionError is raised when trying to save a file, it is handled correctly
"""
# GIVEN: A service manager, a service to save
mocked_main_window = MagicMock()
Registry().register('main_window', mocked_main_window)
Registry().register('application', MagicMock())
Registry().register('settings', MagicMock())
service_manager = ServiceManager(None)
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 = False
mocked_zipfile.ZipFile.return_value = MagicMock()
mocked_os.link.side_effect = OSError
# WHEN: The service is saved and a PermissionError is raised
result = service_manager.save_file()
# THEN: The result is true
assert result is True
mocked_shutil.copyfile.assert_called_once()
@patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
def test_theme_change_global(mocked_regenerate_service_items, settings):
"""