Fixes bug1419691 by checking the theme version number (OpenLP1 themes didn't have a version no.) and by removing the *.theme filter from the file dialog.

Also added a ValidationError exception class, to tidy up the unzip code slightly

bzr-revno: 2501
This commit is contained in:
Philip Ridout 2015-02-17 05:51:24 +00:00 committed by Tim Bentley
commit f7cb420a31
5 changed files with 68 additions and 16 deletions

View File

@ -20,7 +20,7 @@
# Temple Place, Suite 330, Boston, MA 02111-1307 USA # # 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 from PyQt4 import QtCore, QtGui
@ -28,7 +28,7 @@ from PyQt4 import QtCore, QtGui
class HistoryComboBox(QtGui.QComboBox): 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 for when the :kbd:`Enter` or :kbd:`Return` keys are pressed, and saves anything that is typed into the edit box into
its list. its list.
""" """

View File

@ -312,6 +312,7 @@ def create_separated_list(string_list):
from .colorbutton import ColorButton from .colorbutton import ColorButton
from .exceptions import ValidationError
from .filedialog import FileDialog from .filedialog import FileDialog
from .screen import ScreenList from .screen import ScreenList
from .listwidgetwithdnd import ListWidgetWithDnD from .listwidgetwithdnd import ListWidgetWithDnD

View File

@ -0,0 +1,32 @@
# -*- 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

View File

@ -31,7 +31,7 @@ from PyQt4 import QtCore, QtGui
from openlp.core.common import Registry, RegistryProperties, AppLocation, Settings, OpenLPMixin, RegistryMixin, \ from openlp.core.common import Registry, RegistryProperties, AppLocation, Settings, OpenLPMixin, RegistryMixin, \
check_directory_exists, UiStrings, translate, is_win 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 check_item_selected, create_thumb, validate_thumb
from openlp.core.lib.theme import ThemeXML, BackgroundType from openlp.core.lib.theme import ThemeXML, BackgroundType
from openlp.core.lib.ui import critical_error_message_box, create_widget_action from openlp.core.lib.ui import critical_error_message_box, create_widget_action
@ -413,13 +413,13 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R
def on_import_theme(self, field=None): 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 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: :param field:
""" """
files = FileDialog.getOpenFileNames(self, files = FileDialog.getOpenFileNames(self,
translate('OpenLP.ThemeManager', 'Select Theme Import File'), translate('OpenLP.ThemeManager', 'Select Theme Import File'),
Settings().value(self.settings_section + '/last directory import'), 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)) self.log_info('New Themes %s' % str(files))
if not files: if not files:
return return
@ -534,7 +534,6 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R
:param directory: :param directory:
""" """
self.log_debug('Unzipping theme %s' % file_name) self.log_debug('Unzipping theme %s' % file_name)
file_name = str(file_name)
theme_zip = None theme_zip = None
out_file = None out_file = None
file_xml = None file_xml = None
@ -544,8 +543,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'] xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml']
if len(xml_file) != 1: if len(xml_file) != 1:
self.log_error('Theme contains "%s" XML files' % len(xml_file)) 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() 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_name = xml_tree.find('name').text.strip()
theme_folder = os.path.join(directory, theme_name) theme_folder = os.path.join(directory, theme_name)
theme_exists = os.path.exists(theme_folder) theme_exists = os.path.exists(theme_folder)
@ -572,13 +575,10 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R
out_file.close() out_file.close()
except (IOError, zipfile.BadZipfile): except (IOError, zipfile.BadZipfile):
self.log_exception('Importing theme from zip failed %s' % file_name) self.log_exception('Importing theme from zip failed %s' % file_name)
raise Exception('validation') raise ValidationError
except Exception as info: except ValidationError:
if str(info) == 'validation':
critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'), critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'),
translate('OpenLP.ThemeManager', 'File is not a valid theme.')) translate('OpenLP.ThemeManager', 'File is not a valid theme.'))
else:
raise
finally: finally:
# Close the files, to be able to continue creating the theme. # Close the files, to be able to continue creating the theme.
if theme_zip: if theme_zip:

View File

@ -27,14 +27,13 @@ import os
import shutil import shutil
from unittest import TestCase from unittest import TestCase
from tests.functional import MagicMock
from tempfile import mkdtemp from tempfile import mkdtemp
from openlp.core.ui import ThemeManager from openlp.core.ui import ThemeManager
from openlp.core.common import Registry from openlp.core.common import Registry
from tests.utils.constants import TEST_RESOURCES_PATH from tests.utils.constants import TEST_RESOURCES_PATH
from tests.functional import MagicMock, patch from tests.functional import ANY, MagicMock, patch
class TestThemeManager(TestCase): class TestThemeManager(TestCase):
@ -152,3 +151,23 @@ class TestThemeManager(TestCase):
self.assertTrue(os.path.exists(os.path.join(folder, 'Moss on tree', 'Moss on tree.xml'))) 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') self.assertEqual(mocked_critical_error_message_box.call_count, 0, 'No errors should have happened')
shutil.rmtree(folder) 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)