From 14c3d27fedb884e73f029665ae10afa3cb4d0a4b Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Wed, 18 Feb 2015 20:43:51 +0000 Subject: [PATCH 1/6] revert db upgrade code --- openlp/plugins/bibles/lib/upgrade.py | 126 ++++++++++++++++++++++++++- 1 file changed, 125 insertions(+), 1 deletion(-) diff --git a/openlp/plugins/bibles/lib/upgrade.py b/openlp/plugins/bibles/lib/upgrade.py index bd7de8368..53013edbc 100644 --- a/openlp/plugins/bibles/lib/upgrade.py +++ b/openlp/plugins/bibles/lib/upgrade.py @@ -24,6 +24,8 @@ The :mod:`upgrade` module provides a way for the database and schema that is the """ import logging +from sqlalchemy import delete, func, insert, select + log = logging.getLogger(__name__) __version__ = 1 @@ -34,4 +36,126 @@ def upgrade_1(session, metadata): This upgrade renames a number of keys to a single naming convention. """ - log.info('No upgrades to perform') + metadata_table = metadata.tables['metadata'] + # Copy "Version" to "name" ("version" used by upgrade system) + session.execute(insert(metadata_table).values( + key='name', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'Version' + ).as_scalar() + )) + # Copy "Copyright" to "copyright" + session.execute(insert(metadata_table).values( + key='copyright', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'Copyright' + ).as_scalar() + )) + # Copy "Permissions" to "permissions" + session.execute(insert(metadata_table).values( + key='permissions', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'Permissions' + ).as_scalar() + )) + # Copy "Bookname language" to "book_name_language" + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], + metadata_table.c.key == 'Bookname language' + ) + ).scalar() + if value_count > 0: + session.execute(insert(metadata_table).values( + key='book_name_language', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'Bookname language' + ).as_scalar() + )) + # Copy "download source" to "download_source" + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], + metadata_table.c.key == 'download source' + ) + ).scalar() + log.debug('download source: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='download_source', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'download source' + ).as_scalar() + )) + # Copy "download name" to "download_name" + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], + metadata_table.c.key == 'download name' + ) + ).scalar() + log.debug('download name: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='download_name', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'download name' + ).as_scalar() + )) + # Copy "proxy server" to "proxy_server" + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], + metadata_table.c.key == 'proxy server' + ) + ).scalar() + log.debug('proxy server: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='proxy_server', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'proxy server' + ).as_scalar() + )) + # Copy "proxy username" to "proxy_username" + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], + metadata_table.c.key == 'proxy username' + ) + ).scalar() + log.debug('proxy username: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='proxy_username', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'proxy username' + ).as_scalar() + )) + # Copy "proxy password" to "proxy_password" + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], + metadata_table.c.key == 'proxy password' + ) + ).scalar() + log.debug('proxy password: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='proxy_password', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'proxy password' + ).as_scalar() + )) + session.execute(delete(metadata_table)\ + .where(metadata_table.c.key == 'dbversion')) + session.commit() From 5e8ad1710b38874d3279a8f685385e8a80a7862f Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Wed, 18 Feb 2015 20:55:30 +0000 Subject: [PATCH 2/6] remove old redundant keys --- openlp/plugins/bibles/lib/upgrade.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/bibles/lib/upgrade.py b/openlp/plugins/bibles/lib/upgrade.py index 53013edbc..378247c7b 100644 --- a/openlp/plugins/bibles/lib/upgrade.py +++ b/openlp/plugins/bibles/lib/upgrade.py @@ -45,6 +45,7 @@ def upgrade_1(session, metadata): metadata_table.c.key == 'Version' ).as_scalar() )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'Version')) # Copy "Copyright" to "copyright" session.execute(insert(metadata_table).values( key='copyright', @@ -53,6 +54,7 @@ def upgrade_1(session, metadata): metadata_table.c.key == 'Copyright' ).as_scalar() )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'Copyright')) # Copy "Permissions" to "permissions" session.execute(insert(metadata_table).values( key='permissions', @@ -61,6 +63,7 @@ def upgrade_1(session, metadata): metadata_table.c.key == 'Permissions' ).as_scalar() )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'Permissions')) # Copy "Bookname language" to "book_name_language" value_count = session.execute( select( @@ -76,6 +79,7 @@ def upgrade_1(session, metadata): metadata_table.c.key == 'Bookname language' ).as_scalar() )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'Bookname language')) # Copy "download source" to "download_source" value_count = session.execute( select( @@ -92,6 +96,7 @@ def upgrade_1(session, metadata): metadata_table.c.key == 'download source' ).as_scalar() )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'download source')) # Copy "download name" to "download_name" value_count = session.execute( select( @@ -108,6 +113,7 @@ def upgrade_1(session, metadata): metadata_table.c.key == 'download name' ).as_scalar() )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'download name')) # Copy "proxy server" to "proxy_server" value_count = session.execute( select( @@ -124,6 +130,7 @@ def upgrade_1(session, metadata): metadata_table.c.key == 'proxy server' ).as_scalar() )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy server')) # Copy "proxy username" to "proxy_username" value_count = session.execute( select( @@ -140,6 +147,7 @@ def upgrade_1(session, metadata): metadata_table.c.key == 'proxy username' ).as_scalar() )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy username')) # Copy "proxy password" to "proxy_password" value_count = session.execute( select( @@ -156,6 +164,6 @@ def upgrade_1(session, metadata): metadata_table.c.key == 'proxy password' ).as_scalar() )) - session.execute(delete(metadata_table)\ - .where(metadata_table.c.key == 'dbversion')) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy password')) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'dbversion')) session.commit() From a6dfbdcf7182703a3960e5e12ea8dc52aa5c5106 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Wed, 18 Feb 2015 21:13:12 +0000 Subject: [PATCH 3/6] fix upgrade_db --- openlp/core/lib/db.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index e1c9c9117..7b8b11296 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -145,9 +145,13 @@ def upgrade_db(url, upgrade): version_meta = session.query(Metadata).get('version') if version_meta is None: # Tables have just been created - fill the version field with the most recent version - version = upgrade.__version__ + if session.query(Metadata).get('dbversion'): + version = 0 + else: + version = upgrade.__version__ version_meta = Metadata.populate(key='version', value=version) session.add(version_meta) + session.commit() else: version = int(version_meta.value) if version > upgrade.__version__: From 4f94b2acd20d936024265db89a2d12e1323a7572 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Wed, 18 Feb 2015 21:20:35 +0000 Subject: [PATCH 4/6] remove fallback --- openlp/plugins/bibles/lib/db.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py index 5c8443f23..8f1b0d752 100644 --- a/openlp/plugins/bibles/lib/db.py +++ b/openlp/plugins/bibles/lib/db.py @@ -164,9 +164,6 @@ class BibleDB(QtCore.QObject, Manager, RegistryProperties): Returns the version name of the Bible. """ version_name = self.get_object(BibleMeta, 'name') - # Fallback to old way of naming - if not version_name: - version_name = self.get_object(BibleMeta, 'Version') self.name = version_name.value if version_name else None return self.name From 75c9391d8e46f27ea69148f08fd515acfb7b2f3d Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Thu, 19 Feb 2015 18:57:05 +0000 Subject: [PATCH 5/6] Tests --- .../openlp_core_ui/test_thememanager.py | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_core_ui/test_thememanager.py b/tests/functional/openlp_core_ui/test_thememanager.py index 9985eb3b3..85d470e4f 100644 --- a/tests/functional/openlp_core_ui/test_thememanager.py +++ b/tests/functional/openlp_core_ui/test_thememanager.py @@ -22,13 +22,14 @@ """ Package to test the openlp.core.ui.thememanager package. """ -import zipfile import os import shutil from unittest import TestCase from tempfile import mkdtemp +from PyQt4 import QtGui + from openlp.core.ui import ThemeManager from openlp.core.common import Registry @@ -130,6 +131,46 @@ class TestThemeManager(TestCase): # THEN: The mocked_copyfile should not have been called self.assertTrue(mocked_copyfile.called, 'shutil.copyfile should be called') + def over_write_message_box_yes_test(self): + """ + Test that theme_manager.over_write_message_box returns True when the user clicks yes. + """ + # GIVEN: A patched QMessageBox.question and an instance of ThemeManager + with patch('openlp.core.ui.thememanager.QtGui.QMessageBox.question', return_value=QtGui.QMessageBox.Yes)\ + as mocked_qmessagebox_question,\ + patch('openlp.core.ui.thememanager.translate') as mocked_translate: + mocked_translate.side_effect = lambda context, text: text + theme_manager = ThemeManager(None) + + # WHEN: Calling over_write_message_box with 'Theme Name' + result = theme_manager.over_write_message_box('Theme Name') + + # THEN: over_write_message_box should return True and the message box should contain the theme name + self.assertTrue(result) + mocked_qmessagebox_question.assert_called_once_with( + theme_manager, 'Theme Already Exists', 'Theme Theme Name already exists. Do you want to replace it?', + ANY, ANY) + + def over_write_message_box_no_test(self): + """ + Test that theme_manager.over_write_message_box returns False when the user clicks no. + """ + # GIVEN: A patched QMessageBox.question and an instance of ThemeManager + with patch('openlp.core.ui.thememanager.QtGui.QMessageBox.question', return_value=QtGui.QMessageBox.No)\ + as mocked_qmessagebox_question,\ + patch('openlp.core.ui.thememanager.translate') as mocked_translate: + mocked_translate.side_effect = lambda context, text: text + theme_manager = ThemeManager(None) + + # WHEN: Calling over_write_message_box with 'Theme Name' + result = theme_manager.over_write_message_box('Theme Name') + + # THEN: over_write_message_box should return False and the message box should contain the theme name + self.assertFalse(result) + mocked_qmessagebox_question.assert_called_once_with( + theme_manager, 'Theme Already Exists', 'Theme Theme Name already exists. Do you want to replace it?', + ANY, ANY) + def unzip_theme_test(self): """ Test that unzipping of themes works From fb20f9243f3353a8460d502ad7a5c8bb077b71a6 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Thu, 19 Feb 2015 21:24:12 +0000 Subject: [PATCH 6/6] Further fixes --- openlp/core/common/historycombobox.py | 6 +- openlp/plugins/bibles/lib/upgrade.py | 259 ++++++++++++++------------ openlp/plugins/songs/lib/upgrade.py | 1 + 3 files changed, 149 insertions(+), 117 deletions(-) diff --git a/openlp/core/common/historycombobox.py b/openlp/core/common/historycombobox.py index e55d08924..cc5e40649 100644 --- a/openlp/core/common/historycombobox.py +++ b/openlp/core/common/historycombobox.py @@ -28,9 +28,9 @@ from PyQt4 import QtCore, QtGui class HistoryComboBox(QtGui.QComboBox): """ - 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. + 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. """ returnPressed = QtCore.pyqtSignal() diff --git a/openlp/plugins/bibles/lib/upgrade.py b/openlp/plugins/bibles/lib/upgrade.py index 378247c7b..63bb1c27f 100644 --- a/openlp/plugins/bibles/lib/upgrade.py +++ b/openlp/plugins/bibles/lib/upgrade.py @@ -30,6 +30,7 @@ log = logging.getLogger(__name__) __version__ = 1 +# TODO: When removing an upgrade path the ftw-data needs updating to the minimum supported version def upgrade_1(session, metadata): """ Version 1 upgrade. @@ -38,132 +39,162 @@ def upgrade_1(session, metadata): """ metadata_table = metadata.tables['metadata'] # Copy "Version" to "name" ("version" used by upgrade system) - session.execute(insert(metadata_table).values( - key='name', - value=select( - [metadata_table.c.value], - metadata_table.c.key == 'Version' - ).as_scalar() - )) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'Version')) + try: + session.execute(insert(metadata_table).values( + key='name', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'Version' + ).as_scalar() + )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'Version')) + except: + log.exception('Exception when upgrading Version') # Copy "Copyright" to "copyright" - session.execute(insert(metadata_table).values( - key='copyright', - value=select( - [metadata_table.c.value], - metadata_table.c.key == 'Copyright' - ).as_scalar() - )) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'Copyright')) + try: + session.execute(insert(metadata_table).values( + key='copyright', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'Copyright' + ).as_scalar() + )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'Copyright')) + except: + log.exception('Exception when upgrading Copyright') # Copy "Permissions" to "permissions" - session.execute(insert(metadata_table).values( - key='permissions', - value=select( - [metadata_table.c.value], - metadata_table.c.key == 'Permissions' - ).as_scalar() - )) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'Permissions')) + try: + session.execute(insert(metadata_table).values( + key='permissions', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'Permissions' + ).as_scalar() + )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'Permissions')) + except: + log.exception('Exception when upgrading Permissions') # Copy "Bookname language" to "book_name_language" - value_count = session.execute( - select( - [func.count(metadata_table.c.value)], - metadata_table.c.key == 'Bookname language' - ) - ).scalar() - if value_count > 0: - session.execute(insert(metadata_table).values( - key='book_name_language', - value=select( - [metadata_table.c.value], + try: + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], metadata_table.c.key == 'Bookname language' - ).as_scalar() - )) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'Bookname language')) + ) + ).scalar() + if value_count > 0: + session.execute(insert(metadata_table).values( + key='book_name_language', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'Bookname language' + ).as_scalar() + )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'Bookname language')) + except: + log.exception('Exception when upgrading Bookname language') # Copy "download source" to "download_source" - value_count = session.execute( - select( - [func.count(metadata_table.c.value)], - metadata_table.c.key == 'download source' - ) - ).scalar() - log.debug('download source: %s', value_count) - if value_count > 0: - session.execute(insert(metadata_table).values( - key='download_source', - value=select( - [metadata_table.c.value], + try: + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], metadata_table.c.key == 'download source' - ).as_scalar() - )) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'download source')) + ) + ).scalar() + log.debug('download source: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='download_source', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'download source' + ).as_scalar() + )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'download source')) + except: + log.exception('Exception when upgrading download source') # Copy "download name" to "download_name" - value_count = session.execute( - select( - [func.count(metadata_table.c.value)], - metadata_table.c.key == 'download name' - ) - ).scalar() - log.debug('download name: %s', value_count) - if value_count > 0: - session.execute(insert(metadata_table).values( - key='download_name', - value=select( - [metadata_table.c.value], + try: + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], metadata_table.c.key == 'download name' - ).as_scalar() - )) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'download name')) + ) + ).scalar() + log.debug('download name: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='download_name', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'download name' + ).as_scalar() + )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'download name')) + except: + log.exception('Exception when upgrading download name') # Copy "proxy server" to "proxy_server" - value_count = session.execute( - select( - [func.count(metadata_table.c.value)], - metadata_table.c.key == 'proxy server' - ) - ).scalar() - log.debug('proxy server: %s', value_count) - if value_count > 0: - session.execute(insert(metadata_table).values( - key='proxy_server', - value=select( - [metadata_table.c.value], + try: + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], metadata_table.c.key == 'proxy server' - ).as_scalar() - )) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy server')) + ) + ).scalar() + log.debug('proxy server: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='proxy_server', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'proxy server' + ).as_scalar() + )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy server')) + except: + log.exception('Exception when upgrading proxy server') # Copy "proxy username" to "proxy_username" - value_count = session.execute( - select( - [func.count(metadata_table.c.value)], - metadata_table.c.key == 'proxy username' - ) - ).scalar() - log.debug('proxy username: %s', value_count) - if value_count > 0: - session.execute(insert(metadata_table).values( - key='proxy_username', - value=select( - [metadata_table.c.value], + try: + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], metadata_table.c.key == 'proxy username' - ).as_scalar() - )) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy username')) + ) + ).scalar() + log.debug('proxy username: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='proxy_username', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'proxy username' + ).as_scalar() + )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy username')) + except: + log.exception('Exception when upgrading proxy username') # Copy "proxy password" to "proxy_password" - value_count = session.execute( - select( - [func.count(metadata_table.c.value)], - metadata_table.c.key == 'proxy password' - ) - ).scalar() - log.debug('proxy password: %s', value_count) - if value_count > 0: - session.execute(insert(metadata_table).values( - key='proxy_password', - value=select( - [metadata_table.c.value], + try: + value_count = session.execute( + select( + [func.count(metadata_table.c.value)], metadata_table.c.key == 'proxy password' - ).as_scalar() - )) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy password')) - session.execute(delete(metadata_table).where(metadata_table.c.key == 'dbversion')) + ) + ).scalar() + log.debug('proxy password: %s', value_count) + if value_count > 0: + session.execute(insert(metadata_table).values( + key='proxy_password', + value=select( + [metadata_table.c.value], + metadata_table.c.key == 'proxy password' + ).as_scalar() + )) + session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy password')) + except: + log.exception('Exception when upgrading proxy password') + try: + session.execute(delete(metadata_table).where(metadata_table.c.key == 'dbversion')) + except: + log.exception('Exception when deleting dbversion') session.commit() diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py index 21d488b88..9b2cf70a2 100644 --- a/openlp/plugins/songs/lib/upgrade.py +++ b/openlp/plugins/songs/lib/upgrade.py @@ -35,6 +35,7 @@ log = logging.getLogger(__name__) __version__ = 4 +# TODO: When removing an upgrade path the ftw-data needs updating to the minimum supported version def upgrade_1(session, metadata): """ Version 1 upgrade.