openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #22746
[Merge] lp:~raoul-snyman/openlp/bug-1136278 into lp:openlp
Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/bug-1136278 into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
Related bugs:
Bug #1136278 in OpenLP: "DB upgrade tracebacks"
https://bugs.launchpad.net/openlp/+bug/1136278
For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/bug-1136278/+merge/211633
Try to fix bug #1136278 by detecting if the upgrades have already run
--
https://code.launchpad.net/~raoul-snyman/openlp/bug-1136278/+merge/211633
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/bug-1136278 into lp:openlp.
=== modified file 'openlp/core/lib/db.py'
--- openlp/core/lib/db.py 2014-03-11 19:01:09 +0000
+++ openlp/core/lib/db.py 2014-03-18 21:12:38 +0000
@@ -93,10 +93,11 @@
"""
pass
- metadata_table = Table('metadata', metadata,
- Column('key', types.Unicode(64), primary_key=True),
- Column('value', types.UnicodeText(), default=None)
- )
+ metadata_table = Table(
+ 'metadata', metadata,
+ Column('key', types.Unicode(64), primary_key=True),
+ Column('value', types.UnicodeText(), default=None)
+ )
metadata_table.create(checkfirst=True)
mapper(Metadata, metadata_table)
version_meta = session.query(Metadata).get('version')
@@ -137,7 +138,6 @@
:param plugin_name: The name of the plugin to remove the database for
:param db_file_name: The database file name. Defaults to None resulting in the plugin_name being used.
"""
- db_file_path = None
if db_file_name:
db_file_path = os.path.join(AppLocation.get_section_data_path(plugin_name), db_file_name)
else:
=== modified file 'openlp/plugins/bibles/lib/upgrade.py'
--- openlp/plugins/bibles/lib/upgrade.py 2013-12-24 08:56:50 +0000
+++ openlp/plugins/bibles/lib/upgrade.py 2014-03-18 21:12:38 +0000
@@ -31,10 +31,8 @@
"""
import logging
-from sqlalchemy import Table, func, select, insert
-
+log = logging.getLogger(__name__)
__version__ = 1
-log = logging.getLogger(__name__)
def upgrade_1(session, metadata):
@@ -43,136 +41,4 @@
This upgrade renames a number of keys to a single naming convention.
"""
- metadata_table = Table('metadata', metadata, autoload=True)
- # Copy "Version" to "name" ("version" used by upgrade system)
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- 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"
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- 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"
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- 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"
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- 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"
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- 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"
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- 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"
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- 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"
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- 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"
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- 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()
- ))
- # TODO: Clean up in a subsequent release of OpenLP (like 2.0 final)
- #session.execute(delete(metadata_table)\
- # .where(metadata_table.c.key == u'dbversion'))
- session.commit()
+ log.info('No upgrades to perform')
=== modified file 'openlp/plugins/songs/lib/upgrade.py'
--- openlp/plugins/songs/lib/upgrade.py 2013-12-24 08:56:50 +0000
+++ openlp/plugins/songs/lib/upgrade.py 2014-03-18 21:12:38 +0000
@@ -30,12 +30,15 @@
The :mod:`upgrade` module provides a way for the database and schema that is the
backend for the Songs plugin
"""
+import logging
from sqlalchemy import Column, types
+from sqlalchemy.exc import OperationalError
from sqlalchemy.sql.expression import func, false, null, text
from openlp.core.lib.db import get_upgrade_op
+log = logging.getLogger(__name__)
__version__ = 3
@@ -50,14 +53,20 @@
In order to facilitate this one-to-many relationship, a song_id column is
added to the media_files table, and a weight column so that the media
files can be ordered.
+
+ :param session:
+ :param metadata:
"""
- op = get_upgrade_op(session)
- op.drop_table('media_files_songs')
- op.add_column('media_files', Column('song_id', types.Integer(), server_default=null()))
- op.add_column('media_files', Column('weight', types.Integer(), server_default=text('0')))
- if metadata.bind.url.get_dialect().name != 'sqlite':
- # SQLite doesn't support ALTER TABLE ADD CONSTRAINT
- op.create_foreign_key('fk_media_files_song_id', 'media_files', 'songs', ['song_id', 'id'])
+ try:
+ op = get_upgrade_op(session)
+ op.drop_table('media_files_songs')
+ op.add_column('media_files', Column('song_id', types.Integer(), server_default=null()))
+ op.add_column('media_files', Column('weight', types.Integer(), server_default=text('0')))
+ if metadata.bind.url.get_dialect().name != 'sqlite':
+ # SQLite doesn't support ALTER TABLE ADD CONSTRAINT
+ op.create_foreign_key('fk_media_files_song_id', 'media_files', 'songs', ['song_id', 'id'])
+ except OperationalError:
+ log.info('Upgrade 1 has already been run')
def upgrade_2(session, metadata):
@@ -66,9 +75,12 @@
This upgrade adds a create_date and last_modified date to the songs table
"""
- op = get_upgrade_op(session)
- op.add_column('songs', Column('create_date', types.DateTime(), default=func.now()))
- op.add_column('songs', Column('last_modified', types.DateTime(), default=func.now()))
+ try:
+ op = get_upgrade_op(session)
+ op.add_column('songs', Column('create_date', types.DateTime(), default=func.now()))
+ op.add_column('songs', Column('last_modified', types.DateTime(), default=func.now()))
+ except OperationalError:
+ log.info('Upgrade 2 has already been run')
def upgrade_3(session, metadata):
@@ -77,9 +89,11 @@
This upgrade adds a temporary song flag to the songs table
"""
- op = get_upgrade_op(session)
- if metadata.bind.url.get_dialect().name == 'sqlite':
- op.add_column('songs', Column('temporary', types.Boolean(create_constraint=False), server_default=false()))
- else:
- op.add_column('songs', Column('temporary', types.Boolean(), server_default=false()))
-
+ try:
+ op = get_upgrade_op(session)
+ if metadata.bind.url.get_dialect().name == 'sqlite':
+ op.add_column('songs', Column('temporary', types.Boolean(create_constraint=False), server_default=false()))
+ else:
+ op.add_column('songs', Column('temporary', types.Boolean(), server_default=false()))
+ except OperationalError:
+ log.info('Upgrade 3 has already been run')
=== modified file 'openlp/plugins/songusage/lib/upgrade.py'
--- openlp/plugins/songusage/lib/upgrade.py 2013-12-24 08:56:50 +0000
+++ openlp/plugins/songusage/lib/upgrade.py 2014-03-18 21:12:38 +0000
@@ -30,10 +30,14 @@
The :mod:`upgrade` module provides a way for the database and schema that is the
backend for the SongsUsage plugin
"""
+import logging
+
+from sqlalchemy.exc import OperationalError
+from sqlalchemy import Column, types
+
from openlp.core.lib.db import get_upgrade_op
-from sqlalchemy import Column, types
-
+log = logging.getLogger(__name__)
__version__ = 1
@@ -42,7 +46,13 @@
Version 1 upgrade.
This upgrade adds two new fields to the songusage database
+
+ :param session: SQLAlchemy Session object
+ :param metadata: SQLAlchemy MetaData object
"""
- op = get_upgrade_op(session)
- op.add_column('songusage_data', Column('plugin_name', types.Unicode(20), server_default=''))
- op.add_column('songusage_data', Column('source', types.Unicode(10), server_default=''))
+ try:
+ op = get_upgrade_op(session)
+ op.add_column('songusage_data', Column('plugin_name', types.Unicode(20), server_default=''))
+ op.add_column('songusage_data', Column('source', types.Unicode(10), server_default=''))
+ except OperationalError:
+ log.info('Upgrade 1 has already taken place')
=== modified file 'tests/functional/openlp_core_lib/test_db.py'
--- tests/functional/openlp_core_lib/test_db.py 2013-12-24 08:56:50 +0000
+++ tests/functional/openlp_core_lib/test_db.py 2014-03-18 21:12:38 +0000
@@ -29,13 +29,14 @@
"""
Package to test the openlp.core.lib package.
"""
+import os
from unittest import TestCase
from sqlalchemy.pool import NullPool
from sqlalchemy.orm.scoping import ScopedSession
from sqlalchemy import MetaData
-from openlp.core.lib.db import init_db, get_upgrade_op
+from openlp.core.lib.db import init_db, get_upgrade_op, delete_database
from tests.functional import patch, MagicMock
@@ -110,3 +111,44 @@
mocked_session.bind.connect.assert_called_with()
MockedMigrationContext.configure.assert_called_with(mocked_connection)
MockedOperations.assert_called_with(mocked_context)
+
+ def delete_database_without_db_file_name_test(self):
+ """
+ Test that the ``delete_database`` function removes a database file, without the file name parameter
+ """
+ # GIVEN: Mocked out AppLocation class and delete_file method, a test plugin name and a db location
+ with patch('openlp.core.lib.db.AppLocation') as MockedAppLocation, \
+ patch('openlp.core.lib.db.delete_file') as mocked_delete_file:
+ MockedAppLocation.get_section_data_path.return_value = 'test-dir'
+ mocked_delete_file.return_value = True
+ test_plugin = 'test'
+ test_location = os.path.join('test-dir', test_plugin)
+
+ # WHEN: delete_database is run without a database file
+ result = delete_database(test_plugin)
+
+ # THEN: The AppLocation.get_section_data_path and delete_file methods should have been called
+ MockedAppLocation.get_section_data_path.assert_called_with(test_plugin)
+ mocked_delete_file.assert_called_with(test_location)
+ self.assertTrue(result, 'The result of delete_file should be True (was rigged that way)')
+
+ def delete_database_with_db_file_name_test(self):
+ """
+ Test that the ``delete_database`` function removes a database file, with the file name supplied
+ """
+ # GIVEN: Mocked out AppLocation class and delete_file method, a test plugin name and a db location
+ with patch('openlp.core.lib.db.AppLocation') as MockedAppLocation, \
+ patch('openlp.core.lib.db.delete_file') as mocked_delete_file:
+ MockedAppLocation.get_section_data_path.return_value = 'test-dir'
+ mocked_delete_file.return_value = False
+ test_plugin = 'test'
+ test_db_file = 'mydb.sqlite'
+ test_location = os.path.join('test-dir', test_db_file)
+
+ # WHEN: delete_database is run without a database file
+ result = delete_database(test_plugin, test_db_file)
+
+ # THEN: The AppLocation.get_section_data_path and delete_file methods should have been called
+ MockedAppLocation.get_section_data_path.assert_called_with(test_plugin)
+ mocked_delete_file.assert_called_with(test_location)
+ self.assertFalse(result, 'The result of delete_file should be False (was rigged that way)')
Follow ups
