From af2467ed93beecfac70baf701d3053f588557def Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Fri, 13 Feb 2015 21:47:06 +0000 Subject: [PATCH 1/4] Check for theme version. Implement a custom exception Fixes: https://launchpad.net/bugs/1419691 --- openlp/core/common/historycombobox.py | 4 ++-- openlp/core/lib/__init__.py | 1 + openlp/core/lib/exceptions.py | 31 +++++++++++++++++++++++++++ openlp/core/ui/thememanager.py | 23 ++++++++++---------- 4 files changed, 46 insertions(+), 13 deletions(-) create mode 100644 openlp/core/lib/exceptions.py diff --git a/openlp/core/common/historycombobox.py b/openlp/core/common/historycombobox.py index 862a67a25..e55d08924 100644 --- a/openlp/core/common/historycombobox.py +++ b/openlp/core/common/historycombobox.py @@ -20,7 +20,7 @@ # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### """ -The :mod:`~openlp.core.lib.historycombobox` module contains the HistoryComboBox widget +The :mod:`~openlp.core.common.historycombobox` module contains the HistoryComboBox widget """ from PyQt4 import QtCore, QtGui @@ -28,7 +28,7 @@ from PyQt4 import QtCore, QtGui class HistoryComboBox(QtGui.QComboBox): """ - The :class:`~openlp.core.lib.historycombobox.HistoryComboBox` widget emulates the QLineEdit ``returnPressed`` signal + The :class:`~openlp.core.common.historycombobox.HistoryComboBox` widget emulates the QLineEdit ``returnPressed`` signal for when the :kbd:`Enter` or :kbd:`Return` keys are pressed, and saves anything that is typed into the edit box into its list. """ diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index 14b6095af..0439fc156 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -312,6 +312,7 @@ def create_separated_list(string_list): from .colorbutton import ColorButton +from .exceptions import ValidationError from .filedialog import FileDialog from .screen import ScreenList from .listwidgetwithdnd import ListWidgetWithDnD diff --git a/openlp/core/lib/exceptions.py b/openlp/core/lib/exceptions.py new file mode 100644 index 000000000..82b1fd020 --- /dev/null +++ b/openlp/core/lib/exceptions.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2015 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +The :mod:`~openlp.core.lib.exceptions` module contains custom exceptions +""" + +class ValidationError(Exception): + """ + The :class:`~openlp.core.lib.exceptions.ValidationError` exception provides a custom exception for validating + import files. + """ + pass \ No newline at end of file diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 5ced57ea7..6cd3febca 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -31,7 +31,7 @@ from PyQt4 import QtCore, QtGui from openlp.core.common import Registry, RegistryProperties, AppLocation, Settings, OpenLPMixin, RegistryMixin, \ check_directory_exists, UiStrings, translate, is_win -from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, get_text_file_string, build_icon, \ +from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, ValidationError, get_text_file_string, build_icon, \ check_item_selected, create_thumb, validate_thumb from openlp.core.lib.theme import ThemeXML, BackgroundType from openlp.core.lib.ui import critical_error_message_box, create_widget_action @@ -414,13 +414,13 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R def on_import_theme(self, field=None): """ Opens a file dialog to select the theme file(s) to import before attempting to extract OpenLP themes from - those files. This process will load both OpenLP version 1 and version 2 themes. + those files. This process will only load version 2 themes. :param field: """ files = FileDialog.getOpenFileNames(self, translate('OpenLP.ThemeManager', 'Select Theme Import File'), Settings().value(self.settings_section + '/last directory import'), - translate('OpenLP.ThemeManager', 'OpenLP Themes (*.theme *.otz)')) + translate('OpenLP.ThemeManager', 'OpenLP Themes (*.otz)')) self.log_info('New Themes %s' % str(files)) if not files: return @@ -545,8 +545,12 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml'] if len(xml_file) != 1: self.log_error('Theme contains "%s" XML files' % len(xml_file)) - raise Exception('validation') + raise ValidationError xml_tree = ElementTree(element=XML(theme_zip.read(xml_file[0]))).getroot() + theme_version = xml_tree.get('version', default=None) + if not theme_version or float(theme_version) < 2.0: + self.log_error('Theme version is less than 2.0') + raise ValidationError theme_name = xml_tree.find('name').text.strip() theme_folder = os.path.join(directory, theme_name) theme_exists = os.path.exists(theme_folder) @@ -573,13 +577,10 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R out_file.close() except (IOError, zipfile.BadZipfile): self.log_exception('Importing theme from zip failed %s' % file_name) - raise Exception('validation') - except Exception as info: - if str(info) == 'validation': - critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'), - translate('OpenLP.ThemeManager', 'File is not a valid theme.')) - else: - raise + raise ValidationError + except ValidationError: + critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'), + translate('OpenLP.ThemeManager', 'File is not a valid theme.')) finally: # Close the files, to be able to continue creating the theme. if theme_zip: From 498d17b00008ed24d24174835352828b17bba6e9 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Fri, 13 Feb 2015 23:10:16 +0000 Subject: [PATCH 2/4] Added tests --- openlp/core/ui/thememanager.py | 1 - .../openlp_core_ui/test_thememanager.py | 24 +++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 6cd3febca..1f6c6bede 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -535,7 +535,6 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R :param directory: """ self.log_debug('Unzipping theme %s' % file_name) - file_name = str(file_name) theme_zip = None out_file = None file_xml = None diff --git a/tests/functional/openlp_core_ui/test_thememanager.py b/tests/functional/openlp_core_ui/test_thememanager.py index 31efa1f5e..329f8a202 100644 --- a/tests/functional/openlp_core_ui/test_thememanager.py +++ b/tests/functional/openlp_core_ui/test_thememanager.py @@ -27,14 +27,13 @@ import os import shutil from unittest import TestCase -from tests.functional import MagicMock from tempfile import mkdtemp from openlp.core.ui import ThemeManager from openlp.core.common import Registry from tests.utils.constants import TEST_RESOURCES_PATH -from tests.functional import MagicMock, patch +from tests.functional import ANY, MagicMock, patch class TestThemeManager(TestCase): @@ -152,3 +151,24 @@ class TestThemeManager(TestCase): self.assertTrue(os.path.exists(os.path.join(folder, 'Moss on tree', 'Moss on tree.xml'))) self.assertEqual(mocked_critical_error_message_box.call_count, 0, 'No errors should have happened') shutil.rmtree(folder) + + def unzip_theme_invalid_version_test(self): + """ + Test that themes with invalid (< 2.0) or with no version attributes are rejected + """ + # GIVEN: An instance of ThemeManager whilst mocking a theme that returns a theme with no version attribute + with patch('openlp.core.ui.thememanager.zipfile.ZipFile') as mocked_zip_file,\ + patch('openlp.core.ui.thememanager.ElementTree.getroot') as mocked_getroot,\ + patch('openlp.core.ui.thememanager.XML'),\ + patch('openlp.core.ui.thememanager.critical_error_message_box') as mocked_critical_error_message_box: + + mocked_zip_file.return_value = MagicMock(**{'namelist.return_value': [os.path.join('theme', 'theme.xml')]}) + mocked_getroot.return_value = MagicMock(**{'get.return_value': None}) + theme_manager = ThemeManager(None) + + + # WHEN: unzip_theme is called + theme_manager.unzip_theme('theme.file', 'folder') + + # THEN: The critical_error_message_box should have been called + mocked_critical_error_message_box.assert_called_once(ANY, ANY) From 2eb58a56bddcb077221de9b22c528602f9fadca6 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Fri, 13 Feb 2015 23:21:57 +0000 Subject: [PATCH 3/4] PEP fixes --- openlp/core/lib/exceptions.py | 1 + tests/functional/openlp_core_ui/test_thememanager.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/lib/exceptions.py b/openlp/core/lib/exceptions.py index 82b1fd020..42cc1964c 100644 --- a/openlp/core/lib/exceptions.py +++ b/openlp/core/lib/exceptions.py @@ -23,6 +23,7 @@ The :mod:`~openlp.core.lib.exceptions` module contains custom exceptions """ + class ValidationError(Exception): """ The :class:`~openlp.core.lib.exceptions.ValidationError` exception provides a custom exception for validating diff --git a/tests/functional/openlp_core_ui/test_thememanager.py b/tests/functional/openlp_core_ui/test_thememanager.py index 329f8a202..9985eb3b3 100644 --- a/tests/functional/openlp_core_ui/test_thememanager.py +++ b/tests/functional/openlp_core_ui/test_thememanager.py @@ -166,7 +166,6 @@ class TestThemeManager(TestCase): mocked_getroot.return_value = MagicMock(**{'get.return_value': None}) theme_manager = ThemeManager(None) - # WHEN: unzip_theme is called theme_manager.unzip_theme('theme.file', 'folder') From 6ca70bbc5cf8b999bfe0b3f904f6fb973e3755a3 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Fri, 13 Feb 2015 23:27:25 +0000 Subject: [PATCH 4/4] New line at end of file --- openlp/core/lib/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/lib/exceptions.py b/openlp/core/lib/exceptions.py index 42cc1964c..46c836965 100644 --- a/openlp/core/lib/exceptions.py +++ b/openlp/core/lib/exceptions.py @@ -29,4 +29,4 @@ class ValidationError(Exception): The :class:`~openlp.core.lib.exceptions.ValidationError` exception provides a custom exception for validating import files. """ - pass \ No newline at end of file + pass