raoul
/
openlp
forked from openlp/openlp
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge branch 'fix-cached-theme-background-management' into 'master' 

Fix cached theme background management

Closes #145 and #301

See merge request openlp/openlp!49
This commit is contained in:
Raoul Snyman 2019-10-23 22:54:30 +00:00
parent 1a2aabf8a4 c0aa199ac3
commit 66f19e05a1
5 changed files with 126 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
openlp/core/lib/theme.py
View File

@ -169,6 +169,7 @@ class Theme(object):
jsn = get_text_file_string(json_path)
self.load_theme(jsn)
self.background_filename = None
self.background_source = None
self.version = 2
def expand_json(self, var, prev=None):

14
openlp/core/ui/themeform.py
View File

@ -280,7 +280,7 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties):
Run the wizard.
"""
log.debug('Editing theme {name}'.format(name=self.theme.theme_name))
self.temp_background_filename = None
self.temp_background_filename = self.theme.background_source
self.update_theme_allowed = False
self.set_defaults()
self.update_theme_allowed = True
@ -325,11 +325,11 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties):
self.setField('background_type', 1)
elif self.theme.background_type == BackgroundType.to_string(BackgroundType.Image):
self.image_color_button.color = self.theme.background_border_color
self.image_path_edit.path = self.theme.background_filename
self.image_path_edit.path = self.theme.background_source
self.setField('background_type', 2)
elif self.theme.background_type == BackgroundType.to_string(BackgroundType.Video):
self.video_color_button.color = self.theme.background_border_color
self.video_path_edit.path = self.theme.background_filename
self.video_path_edit.path = self.theme.background_source
self.setField('background_type', 4)
elif self.theme.background_type == BackgroundType.to_string(BackgroundType.Stream):
self.setField('background_type', 5)
@ -467,6 +467,7 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties):
:param pathlib.Path new_path: Path to the new image
:rtype: None
"""
self.theme.background_source = new_path
self.theme.background_filename = new_path
self.set_background_page_values()
@ -477,6 +478,7 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties):
:param pathlib.Path new_path: Path to the new video
:rtype: None
"""
self.theme.background_source = new_path
self.theme.background_filename = new_path
self.set_background_page_values()
@ -553,14 +555,14 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties):
translate('OpenLP.ThemeWizard', 'Theme Name Invalid'),
translate('OpenLP.ThemeWizard', 'Invalid theme name. Please enter one.'))
return
source_path = None
destination_path = None
if self.theme.background_type == BackgroundType.to_string(BackgroundType.Image) or \
self.theme.background_type == BackgroundType.to_string(BackgroundType.Video):
file_name = self.theme.background_filename.name
destination_path = self.path / self.theme.theme_name / file_name
source_path = self.theme.background_filename
if not self.edit_mode and not self.theme_manager.check_if_theme_exists(self.theme.theme_name):
return
self.theme_manager.save_theme(self.theme, source_path, destination_path, self.preview_box.save_screenshot())
# Set the theme background to the cache location
self.theme.background_filename = destination_path
self.theme_manager.save_theme(self.theme, self.preview_box.save_screenshot())
return QtWidgets.QDialog.accept(self)

25
openlp/core/ui/thememanager.py
View File

@ -325,14 +325,11 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
:param str new_theme_name: The new theme name of the theme
:rtype: None
"""
destination_path = None
source_path = None
if theme_data.background_type == 'image' or theme_data.background_type == 'video':
destination_path = self.theme_path / new_theme_name / theme_data.background_filename.name
source_path = theme_data.background_filename
theme_data.background_filename = self.theme_path / new_theme_name / theme_data.background_filename.name
theme_data.theme_name = new_theme_name
theme_data.extend_image_filename(self.theme_path)
self.save_theme(theme_data, source_path, destination_path)
self.save_theme(theme_data)
self.load_themes()
def on_edit_theme(self, field=None):
@ -648,14 +645,12 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
return False
return True
def save_theme(self, theme, image_source_path=None, image_destination_path=None, image=None):
def save_theme(self, theme, image=None):
"""
Writes the theme to the disk and handles the background image if necessary
Writes the theme to the disk and including the background image and thumbnail if necessary
:param Theme theme: The theme data object.
:param Path image_source_path: Where the theme image is currently located.
:param Path image_destination_path: Where the Theme Image is to be saved to
:param image: The example image of the theme. Optionally.
:param image: The theme thumbnail. Optionally.
:rtype: None
"""
name = theme.theme_name
@ -667,12 +662,14 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
theme_path.write_text(theme_pretty)
except OSError:
self.log_exception('Saving theme to file failed')
if image_source_path and image_destination_path:
if self.old_background_image_path and image_destination_path != self.old_background_image_path:
if theme.background_source and theme.background_filename:
if self.old_background_image_path and theme.background_filename != self.old_background_image_path:
delete_file(self.old_background_image_path)
if image_source_path != image_destination_path:
if not theme.background_source.exists():
self.log_warning('Background does not exist, retaining cached background')
elif theme.background_source != theme.background_filename:
try:
shutil.copyfile(image_source_path, image_destination_path)
shutil.copyfile(theme.background_source, theme.background_filename)
except OSError:
self.log_exception('Failed to save theme image')
if image:

2
tests/functional/openlp_core/lib/test_theme.py
View File

@ -181,4 +181,4 @@ class TestTheme(TestCase):
assert 0 == theme.display_vertical_align, 'display_vertical_align should be 0'
assert theme.font_footer_bold is False, 'font_footer_bold should be False'
assert 'Arial' == theme.font_main_name, 'font_main_name should be "Arial"'
assert 48 == len(theme.__dict__), 'The theme should have 48 attributes'
assert 49 == len(theme.__dict__), 'The theme should have 49 attributes'

119
tests/functional/openlp_core/ui/test_thememanager.py
View File

@ -87,19 +87,22 @@ class TestThemeManager(TestCase):
Test that we don't try to overwrite a theme background image with itself
"""
# GIVEN: A new theme manager instance, with mocked builtins.open, copyfile,
# theme, create_paths and thememanager-attributes.
# theme, create_paths, thememanager-attributes and background
# .filename path is the same as the source path.
theme_manager = ThemeManager(None)
theme_manager.old_background_image = None
theme_manager.old_background_image_path = None
theme_manager.update_preview_images = MagicMock()
theme_manager.theme_path = MagicMock()
mocked_theme = MagicMock()
mocked_theme.theme_name = 'themename'
mocked_theme.extract_formatted_xml = MagicMock()
mocked_theme.extract_formatted_xml.return_value = 'fake_theme_xml'.encode()
# WHEN: Calling save_theme with path to the same image, but the path written slightly different
file_path_1 = RESOURCE_PATH / 'church.jpg'
theme_manager.save_theme(mocked_theme, file_path_1, file_path_1)
mocked_theme.background_filename = file_path_1
mocked_theme.background_source = file_path_1
# WHEN: Calling save_theme with both background paths to the same image
theme_manager.save_theme(mocked_theme)
# THEN: The mocked_copyfile should not have been called
assert mocked_shutil.copyfile.called is False, 'copyfile should not be called'
@ -111,30 +114,118 @@ class TestThemeManager(TestCase):
Test that we do overwrite a theme background image when a new is submitted
"""
# GIVEN: A new theme manager instance, with mocked builtins.open, copyfile,
# theme, create_paths and thememanager-attributes.
# theme, create_paths, thememanager-attributes and background
# .filename path is the same as the source path.
theme_manager = ThemeManager(None)
theme_manager.old_background_image = None
theme_manager.old_background_image_path = None
theme_manager.update_preview_images = MagicMock()
theme_manager.theme_path = MagicMock()
mocked_theme = MagicMock()
mocked_theme.theme_name = 'themename'
mocked_theme.filename = "filename"
mocked_theme.background_filename = RESOURCE_PATH / 'church.jpg'
mocked_theme.background_source = RESOURCE_PATH / 'church2.jpg'
# WHEN: Calling save_theme with path to different images
file_path_1 = RESOURCE_PATH / 'church.jpg'
file_path_2 = RESOURCE_PATH / 'church2.jpg'
theme_manager.save_theme(mocked_theme, file_path_1, file_path_2)
# WHEN: Calling save_theme with both background paths to different images
theme_manager.save_theme(mocked_theme)
# THEN: The mocked_copyfile should not have been called
# THEN: The mocked_copyfile should have been called
assert mocked_shutil.copyfile.called is True, 'copyfile should be called'
@patch('openlp.core.ui.thememanager.shutil')
@patch('openlp.core.ui.thememanager.delete_file')
@patch('openlp.core.ui.thememanager.create_paths')
def test_save_theme_delete_old_image(self, mocked_create_paths, mocked_delete_file, mocked_shutil):
"""
Test that we do delete a old theme background image when a new is submitted
"""
# GIVEN: A new theme manager instance, with mocked builtins.open,
# theme, create_paths, thememanager-attributes and background
# .filename path is the same as the source path.
theme_manager = ThemeManager(None)
theme_manager.old_background_image_path = RESOURCE_PATH / 'old_church.png'
theme_manager.update_preview_images = MagicMock()
theme_manager.theme_path = MagicMock()
mocked_theme = MagicMock()
mocked_theme.theme_name = 'themename'
mocked_theme.background_filename = RESOURCE_PATH / 'church.jpg'
mocked_theme.background_source = RESOURCE_PATH / 'church2.jpg'
# WHEN: Calling save_theme with both background paths to different images
theme_manager.save_theme(mocked_theme)
# THEN: The mocked_delete_file should have been called to delete the old cached background
assert mocked_delete_file.called is True, 'delete_file should be called'
@patch.object(ThemeManager, 'log_exception')
@patch('openlp.core.ui.thememanager.delete_file')
@patch('openlp.core.ui.thememanager.create_paths')
def test_save_theme_missing_original(self, mocked_paths, mocked_delete, mocked_log_exception):
"""
Test that we revert to the old theme background image if the source is missing
when changing the theme. (User doesn't change background but the original is
missing)
"""
# GIVEN: A new theme manager instance, with invalid files. Setup as if the user
# has left the background the same, or reselected the same path.
# Not using resource dir because I could potentially copy a file
folder_path = Path(mkdtemp())
theme_manager = ThemeManager(None)
theme_manager.old_background_image_path = folder_path / 'old.png'
theme_manager.update_preview_images = MagicMock()
theme_manager.theme_path = MagicMock()
mocked_theme = MagicMock()
mocked_theme.theme_name = 'themename'
mocked_theme.background_filename = folder_path / 'old.png'
mocked_theme.background_source = folder_path / 'non_existent_original.png'
# WHEN: Calling save_theme with a invalid background_filename
# Although all filenames are considered invalid in this test,
# it is important it reverts to the old background path as this in reality is always
# valid unless someone has messed with the cache.
theme_manager.save_theme(mocked_theme)
# THEN: The old background should not have bee deleted
# AND the filename should have been replaced with the old cached background
# AND there is no exception
assert mocked_delete.called is False, 'delete_file should not be called'
assert mocked_theme.background_filename == theme_manager.old_background_image_path, \
'Background path should be reverted'
assert mocked_log_exception.called is False, \
'Should not have been an exception as the file wasn\'t changed'
@patch.object(ThemeManager, 'log_warning')
@patch('openlp.core.ui.thememanager.delete_file')
@patch('openlp.core.ui.thememanager.create_paths')
def test_save_theme_missing_new(self, mocked_paths, mocked_delete, mocked_log_warning):
"""
Test that we log a warning if the new background is missing
"""
# GIVEN: A new theme manager instance, with invalid files. Setup as if the user
# has changed the background to a invalid path.
# Not using resource dir because I could potentially copy a file
folder_path = Path(mkdtemp())
theme_manager = ThemeManager(None)
theme_manager.old_background_image_path = folder_path / 'old.png'
theme_manager.update_preview_images = MagicMock()
theme_manager.theme_path = MagicMock()
mocked_theme = MagicMock()
mocked_theme.theme_name = 'themename'
mocked_theme.background_filename = folder_path / 'new_cached.png'
mocked_theme.background_source = folder_path / 'new_original.png'
# WHEN: Calling save_theme with a invalid background_filename
theme_manager.save_theme(mocked_theme)
# THEN: A warning should have happened due to attempting to copy a missing file
mocked_log_warning.assert_called_once_with('Background does not exist, retaining cached background')
def test_save_theme_special_char_name(self):
"""
Test that we can save themes with special characters in the name
"""
# GIVEN: A new theme manager instance, with mocked theme and thememanager-attributes.
theme_manager = ThemeManager(None)
theme_manager.old_background_image = None
theme_manager.old_background_image_path = None
theme_manager.update_preview_images = MagicMock()
theme_manager.theme_path = Path(self.temp_folder)
mocked_theme = MagicMock()