diff --git a/openlp/core/common/httputils.py b/openlp/core/common/httputils.py index 887bf4a8a..9a73659bd 100644 --- a/openlp/core/common/httputils.py +++ b/openlp/core/common/httputils.py @@ -24,7 +24,6 @@ The :mod:`openlp.core.utils` module provides the utility libraries for OpenLP. """ import hashlib import logging -import os import sys import time from random import randint diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index 6d13c1d9b..9d6330dc6 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -23,12 +23,13 @@ """ The :mod:`db` module provides the core database functionality for OpenLP """ +import json import logging import os from copy import copy from urllib.parse import quote_plus as urlquote -from sqlalchemy import Table, MetaData, Column, types, create_engine +from sqlalchemy import Table, MetaData, Column, types, create_engine, UnicodeText from sqlalchemy.engine.url import make_url from sqlalchemy.exc import SQLAlchemyError, InvalidRequestError, DBAPIError, OperationalError, ProgrammingError from sqlalchemy.orm import scoped_session, sessionmaker, mapper @@ -37,7 +38,8 @@ from sqlalchemy.pool import NullPool from alembic.migration import MigrationContext from alembic.operations import Operations -from openlp.core.common import AppLocation, Settings, translate, delete_file +from openlp.core.common import AppLocation, Settings, delete_file, translate +from openlp.core.common.json import OpenLPJsonDecoder, OpenLPJsonEncoder from openlp.core.lib.ui import critical_error_message_box log = logging.getLogger(__name__) @@ -133,9 +135,10 @@ def get_db_path(plugin_name, db_file_name=None): if db_file_name is None: return 'sqlite:///{path}/{plugin}.sqlite'.format(path=AppLocation.get_section_data_path(plugin_name), plugin=plugin_name) + elif os.path.isabs(db_file_name): + return 'sqlite:///{db_file_name}'.format(db_file_name=db_file_name) else: - return 'sqlite:///{path}/{name}'.format(path=AppLocation.get_section_data_path(plugin_name), - name=db_file_name) + return 'sqlite:///{path}/{name}'.format(path=AppLocation.get_section_data_path(plugin_name), name=db_file_name) def handle_db_error(plugin_name, db_file_name): @@ -200,6 +203,55 @@ class BaseModel(object): return instance +class PathType(types.TypeDecorator): + """ + Create a PathType for storing Path objects with SQLAlchemy. Behind the scenes we convert the Path object to a JSON + representation and store it as a Unicode type + """ + impl = types.UnicodeText + + def coerce_compared_value(self, op, value): + """ + Some times it make sense to compare a PathType with a string. In the case a string is used coerce the the + PathType to a UnicodeText type. + + :param op: The operation being carried out. Not used, as we only care about the type that is being used with the + operation. + :param openlp.core.common.path.Path | str value: The value being used for the comparison. Most likely a Path + Object or str. + :return: The coerced value stored in the db + :rtype: PathType or UnicodeText + """ + if isinstance(value, str): + return UnicodeText() + else: + return self + + def process_bind_param(self, value, dialect): + """ + Convert the Path object to a JSON representation + + :param openlp.core.common.path.Path value: The value to convert + :param dialect: Not used + :return: The Path object as a JSON string + :rtype: str + """ + data_path = AppLocation.get_data_path() + return json.dumps(value, cls=OpenLPJsonEncoder, base_path=data_path) + + def process_result_value(self, value, dialect): + """ + Convert the JSON representation back + + :param types.UnicodeText value: The value to convert + :param dialect: Not used + :return: The JSON object converted Python object (in this case it should be a Path object) + :rtype: openlp.core.common.path.Path + """ + data_path = AppLocation.get_data_path() + return json.loads(value, cls=OpenLPJsonDecoder, base_path=data_path) + + def upgrade_db(url, upgrade): """ Upgrade a database. @@ -208,7 +260,7 @@ def upgrade_db(url, upgrade): :param upgrade: The python module that contains the upgrade instructions. """ if not database_exists(url): - log.warn("Database {db} doesn't exist - skipping upgrade checks".format(db=url)) + log.warning("Database {db} doesn't exist - skipping upgrade checks".format(db=url)) return (0, 0) log.debug('Checking upgrades for DB {db}'.format(db=url)) @@ -273,10 +325,11 @@ def delete_database(plugin_name, db_file_name=None): :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 = AppLocation.get_section_data_path(plugin_name) if db_file_name: - db_file_path = AppLocation.get_section_data_path(plugin_name) / db_file_name + db_file_path = db_file_path / db_file_name else: - db_file_path = AppLocation.get_section_data_path(plugin_name) / plugin_name + db_file_path = db_file_path / plugin_name return delete_file(db_file_path) @@ -284,30 +337,30 @@ class Manager(object): """ Provide generic object persistence management """ - def __init__(self, plugin_name, init_schema, db_file_name=None, upgrade_mod=None, session=None): + def __init__(self, plugin_name, init_schema, db_file_path=None, upgrade_mod=None, session=None): """ Runs the initialisation process that includes creating the connection to the database and the tables if they do not exist. :param plugin_name: The name to setup paths and settings section names :param init_schema: The init_schema function for this database - :param db_file_name: The upgrade_schema function for this database - :param upgrade_mod: The file name to use for this database. Defaults to None resulting in the plugin_name - being used. + :param openlp.core.common.path.Path db_file_path: The file name to use for this database. Defaults to None + resulting in the plugin_name being used. + :param upgrade_mod: The upgrade_schema function for this database """ self.is_dirty = False self.session = None self.db_url = None - if db_file_name: + if db_file_path: log.debug('Manager: Creating new DB url') - self.db_url = init_url(plugin_name, db_file_name) + self.db_url = init_url(plugin_name, str(db_file_path)) else: self.db_url = init_url(plugin_name) if upgrade_mod: try: db_ver, up_ver = upgrade_db(self.db_url, upgrade_mod) except (SQLAlchemyError, DBAPIError): - handle_db_error(plugin_name, db_file_name) + handle_db_error(plugin_name, str(db_file_path)) return if db_ver > up_ver: critical_error_message_box( @@ -322,7 +375,7 @@ class Manager(object): try: self.session = init_schema(self.db_url) except (SQLAlchemyError, DBAPIError): - handle_db_error(plugin_name, db_file_name) + handle_db_error(plugin_name, str(db_file_path)) else: self.session = session diff --git a/openlp/core/ui/advancedtab.py b/openlp/core/ui/advancedtab.py index 4d9b0543f..8ec55999b 100644 --- a/openlp/core/ui/advancedtab.py +++ b/openlp/core/ui/advancedtab.py @@ -22,9 +22,8 @@ """ The :mod:`advancedtab` provides an advanced settings facility. """ -from datetime import datetime, timedelta import logging -import os +from datetime import datetime, timedelta from PyQt5 import QtCore, QtGui, QtWidgets @@ -492,24 +491,27 @@ class AdvancedTab(SettingsTab): self.service_name_edit.setText(UiStrings().DefaultServiceName) self.service_name_edit.setFocus() - def on_data_directory_path_edit_path_changed(self, new_data_path): + def on_data_directory_path_edit_path_changed(self, new_path): """ - Browse for a new data directory location. + Handle the `editPathChanged` signal of the data_directory_path_edit + + :param openlp.core.common.path.Path new_path: The new path + :rtype: None """ # Make sure they want to change the data. answer = QtWidgets.QMessageBox.question(self, translate('OpenLP.AdvancedTab', 'Confirm Data Directory Change'), translate('OpenLP.AdvancedTab', 'Are you sure you want to change the ' 'location of the OpenLP data directory to:\n\n{path}' '\n\nThe data directory will be changed when OpenLP is ' - 'closed.').format(path=new_data_path), + 'closed.').format(path=new_path), defaultButton=QtWidgets.QMessageBox.No) if answer != QtWidgets.QMessageBox.Yes: self.data_directory_path_edit.path = AppLocation.get_data_path() return # Check if data already exists here. - self.check_data_overwrite(path_to_str(new_data_path)) + self.check_data_overwrite(new_path) # Save the new location. - self.main_window.set_new_data_path(path_to_str(new_data_path)) + self.main_window.new_data_path = new_path self.data_directory_cancel_button.show() def on_data_directory_copy_check_box_toggled(self): @@ -526,9 +528,10 @@ class AdvancedTab(SettingsTab): def check_data_overwrite(self, data_path): """ Check if there's already data in the target directory. + + :param openlp.core.common.path.Path data_path: The target directory to check """ - test_path = os.path.join(data_path, 'songs') - if os.path.exists(test_path): + if (data_path / 'songs').exists(): self.data_exists = True # Check is they want to replace existing data. answer = QtWidgets.QMessageBox.warning(self, @@ -537,7 +540,7 @@ class AdvancedTab(SettingsTab): 'WARNING: \n\nThe location you have selected \n\n{path}' '\n\nappears to contain OpenLP data files. Do you wish to ' 'replace these files with the current data ' - 'files?').format(path=os.path.abspath(data_path,)), + 'files?'.format(path=data_path)), QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No), QtWidgets.QMessageBox.No) @@ -559,7 +562,7 @@ class AdvancedTab(SettingsTab): """ self.data_directory_path_edit.path = AppLocation.get_data_path() self.data_directory_copy_check_box.setChecked(False) - self.main_window.set_new_data_path(None) + self.main_window.new_data_path = None self.main_window.set_copy_data(False) self.data_directory_copy_check_box.hide() self.data_directory_cancel_button.hide() diff --git a/openlp/core/ui/exceptionform.py b/openlp/core/ui/exceptionform.py index 9cd88158a..2a5f6b2e7 100644 --- a/openlp/core/ui/exceptionform.py +++ b/openlp/core/ui/exceptionform.py @@ -149,21 +149,11 @@ class ExceptionForm(QtWidgets.QDialog, Ui_ExceptionDialog, RegistryProperties): opts = self._create_report() report_text = self.report_text.format(version=opts['version'], description=opts['description'], traceback=opts['traceback'], libs=opts['libs'], system=opts['system']) - filename = str(file_path) try: - report_file = open(filename, 'w') - try: + with file_path.open('w') as report_file: report_file.write(report_text) - except UnicodeError: - report_file.close() - report_file = open(filename, 'wb') - report_file.write(report_text.encode('utf-8')) - finally: - report_file.close() except IOError: log.exception('Failed to write crash report') - finally: - report_file.close() def on_send_report_button_clicked(self): """ @@ -219,7 +209,7 @@ class ExceptionForm(QtWidgets.QDialog, Ui_ExceptionDialog, RegistryProperties): translate('ImagePlugin.ExceptionDialog', 'Select Attachment'), Settings().value(self.settings_section + '/last directory'), '{text} (*)'.format(text=UiStrings().AllFiles)) - log.info('New file {file}'.format(file=file_path)) + log.info('New files {file_path}'.format(file_path=file_path)) if file_path: self.file_attachment = str(file_path) diff --git a/openlp/core/ui/lib/filedialog.py b/openlp/core/ui/lib/filedialog.py index 0f3ef4058..f1c2bcc24 100755 --- a/openlp/core/ui/lib/filedialog.py +++ b/openlp/core/ui/lib/filedialog.py @@ -31,11 +31,11 @@ class FileDialog(QtWidgets.QFileDialog): """ Wraps `getExistingDirectory` so that it can be called with, and return Path objects - :type parent: QtWidgets.QWidget or None + :type parent: QtWidgets.QWidget | None :type caption: str :type directory: openlp.core.common.path.Path :type options: QtWidgets.QFileDialog.Options - :rtype: tuple[Path, str] + :rtype: tuple[openlp.core.common.path.Path, str] """ args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) @@ -50,13 +50,13 @@ class FileDialog(QtWidgets.QFileDialog): """ Wraps `getOpenFileName` so that it can be called with, and return Path objects - :type parent: QtWidgets.QWidget or None + :type parent: QtWidgets.QWidget | None :type caption: str :type directory: openlp.core.common.path.Path :type filter: str :type initialFilter: str :type options: QtWidgets.QFileDialog.Options - :rtype: tuple[Path, str] + :rtype: tuple[openlp.core.common.path.Path, str] """ args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) @@ -71,13 +71,13 @@ class FileDialog(QtWidgets.QFileDialog): """ Wraps `getOpenFileNames` so that it can be called with, and return Path objects - :type parent: QtWidgets.QWidget or None + :type parent: QtWidgets.QWidget | None :type caption: str :type directory: openlp.core.common.path.Path :type filter: str :type initialFilter: str :type options: QtWidgets.QFileDialog.Options - :rtype: tuple[list[Path], str] + :rtype: tuple[list[openlp.core.common.path.Path], str] """ args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) @@ -93,13 +93,13 @@ class FileDialog(QtWidgets.QFileDialog): """ Wraps `getSaveFileName` so that it can be called with, and return Path objects - :type parent: QtWidgets.QWidget or None + :type parent: QtWidgets.QWidget | None :type caption: str :type directory: openlp.core.common.path.Path :type filter: str :type initialFilter: str :type options: QtWidgets.QFileDialog.Options - :rtype: tuple[Path or None, str] + :rtype: tuple[openlp.core.common.path.Path | None, str] """ args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 508b6a34c..e0232e115 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -1351,12 +1351,6 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): if self.application: self.application.process_events() - def set_new_data_path(self, new_data_path): - """ - Set the new data path - """ - self.new_data_path = new_data_path - def set_copy_data(self, copy_data): """ Set the flag to copy the data @@ -1368,7 +1362,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): Change the data directory. """ log.info('Changing data path to {newpath}'.format(newpath=self.new_data_path)) - old_data_path = str(AppLocation.get_data_path()) + old_data_path = AppLocation.get_data_path() # Copy OpenLP data to new location if requested. self.application.set_busy_cursor() if self.copy_data: @@ -1377,7 +1371,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): self.show_status_message( translate('OpenLP.MainWindow', 'Copying OpenLP data to new data directory location - {path} ' '- Please wait for copy to finish').format(path=self.new_data_path)) - dir_util.copy_tree(old_data_path, self.new_data_path) + dir_util.copy_tree(str(old_data_path), str(self.new_data_path)) log.info('Copy successful') except (IOError, os.error, DistutilsFileError) as why: self.application.set_normal_cursor() @@ -1392,9 +1386,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): log.info('No data copy requested') # Change the location of data directory in config file. settings = QtCore.QSettings() - settings.setValue('advanced/data path', Path(self.new_data_path)) + settings.setValue('advanced/data path', self.new_data_path) # Check if the new data path is our default. - if self.new_data_path == str(AppLocation.get_directory(AppLocation.DataDir)): + if self.new_data_path == AppLocation.get_directory(AppLocation.DataDir): settings.remove('advanced/data path') self.application.set_normal_cursor() diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index b393ad736..b5b77d265 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -376,7 +376,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa self._file_name = path_to_str(file_path) self.main_window.set_service_modified(self.is_modified(), self.short_file_name()) Settings().setValue('servicemanager/last file', file_path) - if file_path and file_path.suffix() == '.oszl': + if file_path and file_path.suffix == '.oszl': self._save_lite = True else: self._save_lite = False @@ -699,13 +699,15 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa default_file_name = format_time(default_pattern, local_time) else: default_file_name = '' + default_file_path = Path(default_file_name) directory_path = Settings().value(self.main_window.service_manager_settings_section + '/last directory') - file_path = directory_path / default_file_name + if directory_path: + default_file_path = directory_path / default_file_path # SaveAs from osz to oszl is not valid as the files will be deleted on exit which is not sensible or usable in # the long term. if self._file_name.endswith('oszl') or self.service_has_all_original_files: file_path, filter_used = FileDialog.getSaveFileName( - self.main_window, UiStrings().SaveService, file_path, + self.main_window, UiStrings().SaveService, default_file_path, translate('OpenLP.ServiceManager', 'OpenLP Service Files (*.osz);; OpenLP Service Files - lite (*.oszl)')) else: diff --git a/openlp/plugins/alerts/forms/alertform.py b/openlp/plugins/alerts/forms/alertform.py index a9cb6cac3..a27d93390 100644 --- a/openlp/plugins/alerts/forms/alertform.py +++ b/openlp/plugins/alerts/forms/alertform.py @@ -70,7 +70,7 @@ class AlertForm(QtWidgets.QDialog, Ui_AlertDialog): item_name = QtWidgets.QListWidgetItem(alert.text) item_name.setData(QtCore.Qt.UserRole, alert.id) self.alert_list_widget.addItem(item_name) - if alert.text == str(self.alert_text_edit.text()): + if alert.text == self.alert_text_edit.text(): self.item_id = alert.id self.alert_list_widget.setCurrentRow(self.alert_list_widget.row(item_name)) diff --git a/openlp/plugins/alerts/lib/alertstab.py b/openlp/plugins/alerts/lib/alertstab.py index 1dfe0a7c3..f5934a30e 100644 --- a/openlp/plugins/alerts/lib/alertstab.py +++ b/openlp/plugins/alerts/lib/alertstab.py @@ -32,9 +32,6 @@ class AlertsTab(SettingsTab): """ AlertsTab is the alerts settings tab in the settings dialog. """ - def __init__(self, parent, name, visible_title, icon_path): - super(AlertsTab, self).__init__(parent, name, visible_title, icon_path) - def setupUi(self): self.setObjectName('AlertsTab') super(AlertsTab, self).setupUi() diff --git a/openlp/plugins/custom/lib/customtab.py b/openlp/plugins/custom/lib/customtab.py index 4ae1dab5b..167aa6d0d 100644 --- a/openlp/plugins/custom/lib/customtab.py +++ b/openlp/plugins/custom/lib/customtab.py @@ -34,9 +34,6 @@ class CustomTab(SettingsTab): """ CustomTab is the Custom settings tab in the settings dialog. """ - def __init__(self, parent, title, visible_title, icon_path): - super(CustomTab, self).__init__(parent, title, visible_title, icon_path) - def setupUi(self): self.setObjectName('CustomTab') super(CustomTab, self).setupUi() diff --git a/openlp/plugins/images/imageplugin.py b/openlp/plugins/images/imageplugin.py index a4310f170..36051a505 100644 --- a/openlp/plugins/images/imageplugin.py +++ b/openlp/plugins/images/imageplugin.py @@ -29,7 +29,7 @@ from openlp.core.common import Settings, translate from openlp.core.lib import Plugin, StringContent, ImageSource, build_icon from openlp.core.lib.db import Manager from openlp.plugins.images.endpoint import api_images_endpoint, images_endpoint -from openlp.plugins.images.lib import ImageMediaItem, ImageTab +from openlp.plugins.images.lib import ImageMediaItem, ImageTab, upgrade from openlp.plugins.images.lib.db import init_schema log = logging.getLogger(__name__) @@ -50,7 +50,7 @@ class ImagePlugin(Plugin): def __init__(self): super(ImagePlugin, self).__init__('images', __default_settings__, ImageMediaItem, ImageTab) - self.manager = Manager('images', init_schema) + self.manager = Manager('images', init_schema, upgrade_mod=upgrade) self.weight = -7 self.icon_path = ':/plugins/plugin_images.png' self.icon = build_icon(self.icon_path) diff --git a/openlp/plugins/images/lib/db.py b/openlp/plugins/images/lib/db.py index feb1174a8..be7c299b3 100644 --- a/openlp/plugins/images/lib/db.py +++ b/openlp/plugins/images/lib/db.py @@ -22,11 +22,10 @@ """ The :mod:`db` module provides the database and schema that is the backend for the Images plugin. """ - from sqlalchemy import Column, ForeignKey, Table, types from sqlalchemy.orm import mapper -from openlp.core.lib.db import BaseModel, init_db +from openlp.core.lib.db import BaseModel, PathType, init_db class ImageGroups(BaseModel): @@ -65,7 +64,7 @@ def init_schema(url): * id * group_id - * filename + * file_path """ session, metadata = init_db(url) @@ -80,7 +79,7 @@ def init_schema(url): image_filenames_table = Table('image_filenames', metadata, Column('id', types.Integer(), primary_key=True), Column('group_id', types.Integer(), ForeignKey('image_groups.id'), default=None), - Column('filename', types.Unicode(255), nullable=False) + Column('file_path', PathType(), nullable=False) ) mapper(ImageGroups, image_groups_table) diff --git a/openlp/plugins/images/lib/imagetab.py b/openlp/plugins/images/lib/imagetab.py index bacf03ce6..565ef6543 100644 --- a/openlp/plugins/images/lib/imagetab.py +++ b/openlp/plugins/images/lib/imagetab.py @@ -31,9 +31,6 @@ class ImageTab(SettingsTab): """ ImageTab is the images settings tab in the settings dialog. """ - def __init__(self, parent, name, visible_title, icon_path): - super(ImageTab, self).__init__(parent, name, visible_title, icon_path) - def setupUi(self): self.setObjectName('ImagesTab') super(ImageTab, self).setupUi() diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index d1ea2003f..b436d2708 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -21,7 +21,6 @@ ############################################################################### import logging -import os from PyQt5 import QtCore, QtGui, QtWidgets @@ -99,11 +98,11 @@ class ImageMediaItem(MediaManagerItem): self.list_view.setIconSize(QtCore.QSize(88, 50)) self.list_view.setIndentation(self.list_view.default_indentation) self.list_view.allow_internal_dnd = True - self.service_path = os.path.join(str(AppLocation.get_section_data_path(self.settings_section)), 'thumbnails') - check_directory_exists(Path(self.service_path)) + self.service_path = AppLocation.get_section_data_path(self.settings_section) / 'thumbnails' + check_directory_exists(self.service_path) # Load images from the database self.load_full_list( - self.manager.get_all_objects(ImageFilenames, order_by_ref=ImageFilenames.filename), initial_load=True) + self.manager.get_all_objects(ImageFilenames, order_by_ref=ImageFilenames.file_path), initial_load=True) def add_list_view_to_toolbar(self): """ @@ -211,8 +210,8 @@ class ImageMediaItem(MediaManagerItem): """ images = self.manager.get_all_objects(ImageFilenames, ImageFilenames.group_id == image_group.id) for image in images: - delete_file(Path(self.service_path, os.path.split(image.filename)[1])) - delete_file(Path(self.generate_thumbnail_path(image))) + delete_file(self.service_path / image.file_path.name) + delete_file(self.generate_thumbnail_path(image)) self.manager.delete_object(ImageFilenames, image.id) image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == image_group.id) for group in image_groups: @@ -234,8 +233,8 @@ class ImageMediaItem(MediaManagerItem): if row_item: item_data = row_item.data(0, QtCore.Qt.UserRole) if isinstance(item_data, ImageFilenames): - delete_file(Path(self.service_path, row_item.text(0))) - delete_file(Path(self.generate_thumbnail_path(item_data))) + delete_file(self.service_path / row_item.text(0)) + delete_file(self.generate_thumbnail_path(item_data)) if item_data.group_id == 0: self.list_view.takeTopLevelItem(self.list_view.indexOfTopLevelItem(row_item)) else: @@ -326,17 +325,19 @@ class ImageMediaItem(MediaManagerItem): """ Generate a path to the thumbnail - :param image: An instance of ImageFileNames - :return: A path to the thumbnail of type str + :param openlp.plugins.images.lib.db.ImageFilenames image: The image to generate the thumbnail path for. + :return: A path to the thumbnail + :rtype: openlp.core.common.path.Path """ - ext = os.path.splitext(image.filename)[1].lower() - return os.path.join(self.service_path, '{}{}'.format(str(image.id), ext)) + ext = image.file_path.suffix.lower() + return self.service_path / '{name:d}{ext}'.format(name=image.id, ext=ext) def load_full_list(self, images, initial_load=False, open_group=None): """ Replace the list of images and groups in the interface. - :param images: A List of Image Filenames objects that will be used to reload the mediamanager list. + :param list[openlp.plugins.images.lib.db.ImageFilenames] images: A List of Image Filenames objects that will be + used to reload the mediamanager list. :param initial_load: When set to False, the busy cursor and progressbar will be shown while loading images. :param open_group: ImageGroups object of the group that must be expanded after reloading the list in the interface. @@ -352,34 +353,34 @@ class ImageMediaItem(MediaManagerItem): self.expand_group(open_group.id) # Sort the images by its filename considering language specific. # characters. - images.sort(key=lambda image_object: get_locale_key(os.path.split(str(image_object.filename))[1])) - for image_file in images: - log.debug('Loading image: {name}'.format(name=image_file.filename)) - filename = os.path.split(image_file.filename)[1] - thumb = self.generate_thumbnail_path(image_file) - if not os.path.exists(image_file.filename): + images.sort(key=lambda image_object: get_locale_key(image_object.file_path.name)) + for image in images: + log.debug('Loading image: {name}'.format(name=image.file_path)) + file_name = image.file_path.name + thumbnail_path = self.generate_thumbnail_path(image) + if not image.file_path.exists(): icon = build_icon(':/general/general_delete.png') else: - if validate_thumb(Path(image_file.filename), Path(thumb)): - icon = build_icon(thumb) + if validate_thumb(image.file_path, thumbnail_path): + icon = build_icon(thumbnail_path) else: - icon = create_thumb(image_file.filename, thumb) - item_name = QtWidgets.QTreeWidgetItem([filename]) - item_name.setText(0, filename) + icon = create_thumb(image.file_path, thumbnail_path) + item_name = QtWidgets.QTreeWidgetItem([file_name]) + item_name.setText(0, file_name) item_name.setIcon(0, icon) - item_name.setToolTip(0, image_file.filename) - item_name.setData(0, QtCore.Qt.UserRole, image_file) - if image_file.group_id == 0: + item_name.setToolTip(0, str(image.file_path)) + item_name.setData(0, QtCore.Qt.UserRole, image) + if image.group_id == 0: self.list_view.addTopLevelItem(item_name) else: - group_items[image_file.group_id].addChild(item_name) + group_items[image.group_id].addChild(item_name) if not initial_load: self.main_window.increment_progress_bar() if not initial_load: self.main_window.finished_progress_bar() self.application.set_normal_cursor() - def validate_and_load(self, files, target_group=None): + def validate_and_load(self, file_paths, target_group=None): """ Process a list for files either from the File Dialog or from Drag and Drop. This method is overloaded from MediaManagerItem. @@ -388,15 +389,15 @@ class ImageMediaItem(MediaManagerItem): :param target_group: The QTreeWidgetItem of the group that will be the parent of the added files """ self.application.set_normal_cursor() - self.load_list(files, target_group) - last_dir = os.path.split(files[0])[0] - Settings().setValue(self.settings_section + '/last directory', Path(last_dir)) + self.load_list(file_paths, target_group) + last_dir = file_paths[0].parent + Settings().setValue(self.settings_section + '/last directory', last_dir) - def load_list(self, images, target_group=None, initial_load=False): + def load_list(self, image_paths, target_group=None, initial_load=False): """ Add new images to the database. This method is called when adding images using the Add button or DnD. - :param images: A List of strings containing the filenames of the files to be loaded + :param list[openlp.core.common.Path] image_paths: A list of file paths to the images to be loaded :param target_group: The QTreeWidgetItem of the group that will be the parent of the added files :param initial_load: When set to False, the busy cursor and progressbar will be shown while loading images """ @@ -429,7 +430,7 @@ class ImageMediaItem(MediaManagerItem): else: self.choose_group_form.existing_radio_button.setDisabled(False) self.choose_group_form.group_combobox.setDisabled(False) - # Ask which group the images should be saved in + # Ask which group the image_paths should be saved in if self.choose_group_form.exec(selected_group=preselect_group): if self.choose_group_form.nogroup_radio_button.isChecked(): # User chose 'No group' @@ -461,33 +462,33 @@ class ImageMediaItem(MediaManagerItem): return # Initialize busy cursor and progress bar self.application.set_busy_cursor() - self.main_window.display_progress_bar(len(images)) - # Save the new images in the database - self.save_new_images_list(images, group_id=parent_group.id, reload_list=False) - self.load_full_list(self.manager.get_all_objects(ImageFilenames, order_by_ref=ImageFilenames.filename), + self.main_window.display_progress_bar(len(image_paths)) + # Save the new image_paths in the database + self.save_new_images_list(image_paths, group_id=parent_group.id, reload_list=False) + self.load_full_list(self.manager.get_all_objects(ImageFilenames, order_by_ref=ImageFilenames.file_path), initial_load=initial_load, open_group=parent_group) self.application.set_normal_cursor() - def save_new_images_list(self, images_list, group_id=0, reload_list=True): + def save_new_images_list(self, image_paths, group_id=0, reload_list=True): """ Convert a list of image filenames to ImageFilenames objects and save them in the database. - :param images_list: A List of strings containing image filenames + :param list[Path] image_paths: A List of file paths to image :param group_id: The ID of the group to save the images in :param reload_list: This boolean is set to True when the list in the interface should be reloaded after saving the new images """ - for filename in images_list: - if not isinstance(filename, str): + for image_path in image_paths: + if not isinstance(image_path, Path): continue - log.debug('Adding new image: {name}'.format(name=filename)) + log.debug('Adding new image: {name}'.format(name=image_path)) image_file = ImageFilenames() image_file.group_id = group_id - image_file.filename = str(filename) + image_file.file_path = image_path self.manager.save_object(image_file) self.main_window.increment_progress_bar() - if reload_list and images_list: - self.load_full_list(self.manager.get_all_objects(ImageFilenames, order_by_ref=ImageFilenames.filename)) + if reload_list and image_paths: + self.load_full_list(self.manager.get_all_objects(ImageFilenames, order_by_ref=ImageFilenames.file_path)) def dnd_move_internal(self, target): """ @@ -581,8 +582,8 @@ class ImageMediaItem(MediaManagerItem): return False # Find missing files for image in images: - if not os.path.exists(image.filename): - missing_items_file_names.append(image.filename) + if not image.file_path.exists(): + missing_items_file_names.append(str(image.file_path)) # We cannot continue, as all images do not exist. if not images: if not remote: @@ -601,9 +602,9 @@ class ImageMediaItem(MediaManagerItem): return False # Continue with the existing images. for image in images: - name = os.path.split(image.filename)[1] - thumbnail = self.generate_thumbnail_path(image) - service_item.add_from_image(image.filename, name, background, thumbnail) + name = image.file_path.name + thumbnail_path = self.generate_thumbnail_path(image) + service_item.add_from_image(str(image.file_path), name, background, str(thumbnail_path)) return True def check_group_exists(self, new_group): @@ -640,7 +641,7 @@ class ImageMediaItem(MediaManagerItem): if not self.check_group_exists(new_group): if self.manager.save_object(new_group): self.load_full_list(self.manager.get_all_objects( - ImageFilenames, order_by_ref=ImageFilenames.filename)) + ImageFilenames, order_by_ref=ImageFilenames.file_path)) self.expand_group(new_group.id) self.fill_groups_combobox(self.choose_group_form.group_combobox) self.fill_groups_combobox(self.add_group_form.parent_group_combobox) @@ -675,9 +676,9 @@ class ImageMediaItem(MediaManagerItem): if not isinstance(bitem.data(0, QtCore.Qt.UserRole), ImageFilenames): # Only continue when an image is selected. return - filename = bitem.data(0, QtCore.Qt.UserRole).filename - if os.path.exists(filename): - if self.live_controller.display.direct_image(filename, background): + file_path = bitem.data(0, QtCore.Qt.UserRole).file_path + if file_path.exists(): + if self.live_controller.display.direct_image(str(file_path), background): self.reset_action.setVisible(True) else: critical_error_message_box( @@ -687,22 +688,22 @@ class ImageMediaItem(MediaManagerItem): critical_error_message_box( UiStrings().LiveBGError, translate('ImagePlugin.MediaItem', 'There was a problem replacing your background, ' - 'the image file "{name}" no longer exists.').format(name=filename)) + 'the image file "{name}" no longer exists.').format(name=file_path)) def search(self, string, show_error=True): """ Perform a search on the image file names. - :param string: The glob to search for - :param show_error: Unused. + :param str string: The glob to search for + :param bool show_error: Unused. """ files = self.manager.get_all_objects( - ImageFilenames, filter_clause=ImageFilenames.filename.contains(string), - order_by_ref=ImageFilenames.filename) + ImageFilenames, filter_clause=ImageFilenames.file_path.contains(string), + order_by_ref=ImageFilenames.file_path) results = [] for file_object in files: - filename = os.path.split(str(file_object.filename))[1] - results.append([file_object.filename, filename]) + file_name = file_object.file_path.name + results.append([str(file_object.file_path), file_name]) return results def create_item_from_id(self, item_id): @@ -711,8 +712,9 @@ class ImageMediaItem(MediaManagerItem): :param item_id: Id to make live """ + item_id = Path(item_id) item = QtWidgets.QTreeWidgetItem() - item_data = self.manager.get_object_filtered(ImageFilenames, ImageFilenames.filename == item_id) - item.setText(0, os.path.basename(item_data.filename)) + item_data = self.manager.get_object_filtered(ImageFilenames, ImageFilenames.file_path == item_id) + item.setText(0, item_data.file_path.name) item.setData(0, QtCore.Qt.UserRole, item_data) return item diff --git a/openlp/plugins/images/lib/upgrade.py b/openlp/plugins/images/lib/upgrade.py new file mode 100644 index 000000000..63690d404 --- /dev/null +++ b/openlp/plugins/images/lib/upgrade.py @@ -0,0 +1,70 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2017 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:`upgrade` module provides the migration path for the OLP Paths database +""" +import json +import logging + +from sqlalchemy import Column, Table + +from openlp.core.common import AppLocation +from openlp.core.common.db import drop_columns +from openlp.core.common.json import OpenLPJsonEncoder +from openlp.core.common.path import Path +from openlp.core.lib.db import PathType, get_upgrade_op + +log = logging.getLogger(__name__) +__version__ = 2 + + +def upgrade_1(session, metadata): + """ + Version 1 upgrade - old db might/might not be versioned. + """ + log.debug('Skipping upgrade_1 of files DB - not used') + + +def upgrade_2(session, metadata): + """ + Version 2 upgrade - Move file path from old db to JSON encoded path to new db. Added during 2.5 dev + """ + # TODO: Update tests + log.debug('Starting upgrade_2 for file_path to JSON') + old_table = Table('image_filenames', metadata, autoload=True) + if 'file_path' not in [col.name for col in old_table.c.values()]: + op = get_upgrade_op(session) + op.add_column('image_filenames', Column('file_path', PathType())) + conn = op.get_bind() + results = conn.execute('SELECT * FROM image_filenames') + data_path = AppLocation.get_data_path() + for row in results.fetchall(): + file_path_json = json.dumps(Path(row.filename), cls=OpenLPJsonEncoder, base_path=data_path) + sql = 'UPDATE image_filenames SET file_path = \'{file_path_json}\' WHERE id = {id}'.format( + file_path_json=file_path_json, id=row.id) + conn.execute(sql) + # Drop old columns + if metadata.bind.url.get_dialect().name == 'sqlite': + drop_columns(op, 'image_filenames', ['filename', ]) + else: + op.drop_constraint('image_filenames', 'foreignkey') + op.drop_column('image_filenames', 'filenames') diff --git a/openlp/plugins/songusage/forms/songusagedetaildialog.py b/openlp/plugins/songusage/forms/songusagedetaildialog.py index 082173bf5..23ae92957 100644 --- a/openlp/plugins/songusage/forms/songusagedetaildialog.py +++ b/openlp/plugins/songusage/forms/songusagedetaildialog.py @@ -19,7 +19,6 @@ # with this program; if not, write to the Free Software Foundation, Inc., 59 # # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### - from PyQt5 import QtCore, QtWidgets from openlp.core.common import translate diff --git a/openlp/plugins/songusage/forms/songusagedetailform.py b/openlp/plugins/songusage/forms/songusagedetailform.py index 7aa636635..930baf1d6 100644 --- a/openlp/plugins/songusage/forms/songusagedetailform.py +++ b/openlp/plugins/songusage/forms/songusagedetailform.py @@ -19,7 +19,6 @@ # with this program; if not, write to the Free Software Foundation, Inc., 59 # # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### - import logging import os @@ -60,7 +59,7 @@ class SongUsageDetailForm(QtWidgets.QDialog, Ui_SongUsageDetailDialog, RegistryP def on_report_path_edit_path_changed(self, file_path): """ - Called when the path in the `PathEdit` has changed + Handle the `pathEditChanged` signal from report_path_edit :param openlp.core.common.path.Path file_path: The new path. :rtype: None @@ -72,7 +71,7 @@ class SongUsageDetailForm(QtWidgets.QDialog, Ui_SongUsageDetailDialog, RegistryP Ok was triggered so lets save the data and run the report """ log.debug('accept') - path = path_to_str(self.report_path_edit.path) + path = self.report_path_edit.path if not path: self.main_window.error_message( translate('SongUsagePlugin.SongUsageDetailForm', 'Output Path Not Selected'), @@ -80,7 +79,7 @@ class SongUsageDetailForm(QtWidgets.QDialog, Ui_SongUsageDetailDialog, RegistryP ' song usage report. \nPlease select an existing path on your computer.') ) return - check_directory_exists(Path(path)) + check_directory_exists(path) file_name = translate('SongUsagePlugin.SongUsageDetailForm', 'usage_detail_{old}_{new}.txt' ).format(old=self.from_date_calendar.selectedDate().toString('ddMMyyyy'), @@ -91,29 +90,25 @@ class SongUsageDetailForm(QtWidgets.QDialog, Ui_SongUsageDetailDialog, RegistryP SongUsageItem, and_(SongUsageItem.usagedate >= self.from_date_calendar.selectedDate().toPyDate(), SongUsageItem.usagedate < self.to_date_calendar.selectedDate().toPyDate()), [SongUsageItem.usagedate, SongUsageItem.usagetime]) - report_file_name = os.path.join(path, file_name) - file_handle = None + report_file_name = path / file_name try: - file_handle = open(report_file_name, 'wb') - for instance in usage: - record = ('\"{date}\",\"{time}\",\"{title}\",\"{copyright}\",\"{ccli}\",\"{authors}\",' - '\"{name}\",\"{source}\"\n').format(date=instance.usagedate, time=instance.usagetime, - title=instance.title, copyright=instance.copyright, - ccli=instance.ccl_number, authors=instance.authors, - name=instance.plugin_name, source=instance.source) - file_handle.write(record.encode('utf-8')) - self.main_window.information_message( - translate('SongUsagePlugin.SongUsageDetailForm', 'Report Creation'), - translate('SongUsagePlugin.SongUsageDetailForm', - 'Report \n{name} \nhas been successfully created. ').format(name=report_file_name) - ) + with report_file_name.open('wb') as file_handle: + for instance in usage: + record = ('\"{date}\",\"{time}\",\"{title}\",\"{copyright}\",\"{ccli}\",\"{authors}\",' + '\"{name}\",\"{source}\"\n').format(date=instance.usagedate, time=instance.usagetime, + title=instance.title, copyright=instance.copyright, + ccli=instance.ccl_number, authors=instance.authors, + name=instance.plugin_name, source=instance.source) + file_handle.write(record.encode('utf-8')) + self.main_window.information_message( + translate('SongUsagePlugin.SongUsageDetailForm', 'Report Creation'), + translate('SongUsagePlugin.SongUsageDetailForm', + 'Report \n{name} \nhas been successfully created. ').format(name=report_file_name) + ) except OSError as ose: log.exception('Failed to write out song usage records') critical_error_message_box(translate('SongUsagePlugin.SongUsageDetailForm', 'Report Creation Failed'), translate('SongUsagePlugin.SongUsageDetailForm', 'An error occurred while creating the report: {error}' ).format(error=ose.strerror)) - finally: - if file_handle: - file_handle.close() self.close() diff --git a/tests/functional/openlp_core_ui/test_exceptionform.py b/tests/functional/openlp_core_ui/test_exceptionform.py index 016f18ae3..28707410c 100644 --- a/tests/functional/openlp_core_ui/test_exceptionform.py +++ b/tests/functional/openlp_core_ui/test_exceptionform.py @@ -22,11 +22,11 @@ """ Package to test the openlp.core.ui.exeptionform package. """ - import os import tempfile + from unittest import TestCase -from unittest.mock import mock_open, patch +from unittest.mock import call, patch from openlp.core.common import Registry from openlp.core.common.path import Path @@ -122,15 +122,15 @@ class TestExceptionForm(TestMixin, TestCase): test_form = exceptionform.ExceptionForm() test_form.file_attachment = None - with patch.object(test_form, '_pyuno_import') as mock_pyuno: - with patch.object(test_form.exception_text_edit, 'toPlainText') as mock_traceback: - with patch.object(test_form.description_text_edit, 'toPlainText') as mock_description: - mock_pyuno.return_value = 'UNO Bridge Test' - mock_traceback.return_value = 'openlp: Traceback Test' - mock_description.return_value = 'Description Test' + with patch.object(test_form, '_pyuno_import') as mock_pyuno, \ + patch.object(test_form.exception_text_edit, 'toPlainText') as mock_traceback, \ + patch.object(test_form.description_text_edit, 'toPlainText') as mock_description: + mock_pyuno.return_value = 'UNO Bridge Test' + mock_traceback.return_value = 'openlp: Traceback Test' + mock_description.return_value = 'Description Test' - # WHEN: on_save_report_button_clicked called - test_form.on_send_report_button_clicked() + # WHEN: on_save_report_button_clicked called + test_form.on_send_report_button_clicked() # THEN: Verify strings were formatted properly mocked_add_query_item.assert_called_with('body', MAIL_ITEM_TEXT) @@ -153,25 +153,24 @@ class TestExceptionForm(TestMixin, TestCase): mocked_qt.PYQT_VERSION_STR = 'PyQt5 Test' mocked_is_linux.return_value = False mocked_get_version.return_value = 'Trunk Test' - mocked_save_filename.return_value = (Path('testfile.txt'), 'filter') - test_form = exceptionform.ExceptionForm() - test_form.file_attachment = None + with patch.object(Path, 'open') as mocked_path_open: + test_path = Path('testfile.txt') + mocked_save_filename.return_value = test_path, 'ext' - with patch.object(test_form, '_pyuno_import') as mock_pyuno: - with patch.object(test_form.exception_text_edit, 'toPlainText') as mock_traceback: - with patch.object(test_form.description_text_edit, 'toPlainText') as mock_description: - with patch("openlp.core.ui.exceptionform.open", mock_open(), create=True) as mocked_open: - mock_pyuno.return_value = 'UNO Bridge Test' - mock_traceback.return_value = 'openlp: Traceback Test' - mock_description.return_value = 'Description Test' + test_form = exceptionform.ExceptionForm() + test_form.file_attachment = None - # WHEN: on_save_report_button_clicked called - test_form.on_save_report_button_clicked() + with patch.object(test_form, '_pyuno_import') as mock_pyuno, \ + patch.object(test_form.exception_text_edit, 'toPlainText') as mock_traceback, \ + patch.object(test_form.description_text_edit, 'toPlainText') as mock_description: + mock_pyuno.return_value = 'UNO Bridge Test' + mock_traceback.return_value = 'openlp: Traceback Test' + mock_description.return_value = 'Description Test' + + # WHEN: on_save_report_button_clicked called + test_form.on_save_report_button_clicked() # THEN: Verify proper calls to save file # self.maxDiff = None - check_text = "call().write({text})".format(text=MAIL_ITEM_TEXT.__repr__()) - write_text = "{text}".format(text=mocked_open.mock_calls[1]) - mocked_open.assert_called_with('testfile.txt', 'w') - self.assertEquals(check_text, write_text, "Saved information should match test text") + mocked_path_open.assert_has_calls([call().__enter__().write(MAIL_ITEM_TEXT)]) diff --git a/tests/functional/openlp_plugins/images/test_lib.py b/tests/functional/openlp_plugins/images/test_lib.py index 821a64bb0..650179e07 100644 --- a/tests/functional/openlp_plugins/images/test_lib.py +++ b/tests/functional/openlp_plugins/images/test_lib.py @@ -58,7 +58,7 @@ class TestImageMediaItem(TestCase): Test that the validate_and_load_test() method when called without a group """ # GIVEN: A list of files - file_list = ['/path1/image1.jpg', '/path2/image2.jpg'] + file_list = [Path('path1', 'image1.jpg'), Path('path2', 'image2.jpg')] # WHEN: Calling validate_and_load with the list of files self.media_item.validate_and_load(file_list) @@ -66,7 +66,7 @@ class TestImageMediaItem(TestCase): # THEN: load_list should have been called with the file list and None, # the directory should have been saved to the settings mocked_load_list.assert_called_once_with(file_list, None) - mocked_settings().setValue.assert_called_once_with(ANY, Path('/', 'path1')) + mocked_settings().setValue.assert_called_once_with(ANY, Path('path1')) @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_list') @patch('openlp.plugins.images.lib.mediaitem.Settings') @@ -75,7 +75,7 @@ class TestImageMediaItem(TestCase): Test that the validate_and_load_test() method when called with a group """ # GIVEN: A list of files - file_list = ['/path1/image1.jpg', '/path2/image2.jpg'] + file_list = [Path('path1', 'image1.jpg'), Path('path2', 'image2.jpg')] # WHEN: Calling validate_and_load with the list of files and a group self.media_item.validate_and_load(file_list, 'group') @@ -83,7 +83,7 @@ class TestImageMediaItem(TestCase): # THEN: load_list should have been called with the file list and the group name, # the directory should have been saved to the settings mocked_load_list.assert_called_once_with(file_list, 'group') - mocked_settings().setValue.assert_called_once_with(ANY, Path('/', 'path1')) + mocked_settings().setValue.assert_called_once_with(ANY, Path('path1')) @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') def test_save_new_images_list_empty_list(self, mocked_load_full_list): @@ -107,8 +107,8 @@ class TestImageMediaItem(TestCase): Test that the save_new_images_list() calls load_full_list() when reload_list is set to True """ # GIVEN: A list with 1 image and a mocked out manager - image_list = ['test_image.jpg'] - ImageFilenames.filename = '' + image_list = [Path('test_image.jpg')] + ImageFilenames.file_path = None self.media_item.manager = MagicMock() # WHEN: We run save_new_images_list with reload_list=True @@ -118,7 +118,7 @@ class TestImageMediaItem(TestCase): self.assertEquals(mocked_load_full_list.call_count, 1, 'load_full_list() should have been called') # CLEANUP: Remove added attribute from ImageFilenames - delattr(ImageFilenames, 'filename') + delattr(ImageFilenames, 'file_path') @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') def test_save_new_images_list_single_image_without_reload(self, mocked_load_full_list): @@ -126,7 +126,7 @@ class TestImageMediaItem(TestCase): Test that the save_new_images_list() doesn't call load_full_list() when reload_list is set to False """ # GIVEN: A list with 1 image and a mocked out manager - image_list = ['test_image.jpg'] + image_list = [Path('test_image.jpg')] self.media_item.manager = MagicMock() # WHEN: We run save_new_images_list with reload_list=False @@ -141,7 +141,7 @@ class TestImageMediaItem(TestCase): Test that the save_new_images_list() saves all images in the list """ # GIVEN: A list with 3 images - image_list = ['test_image_1.jpg', 'test_image_2.jpg', 'test_image_3.jpg'] + image_list = [Path('test_image_1.jpg'), Path('test_image_2.jpg'), Path('test_image_3.jpg')] self.media_item.manager = MagicMock() # WHEN: We run save_new_images_list with the list of 3 images @@ -157,7 +157,7 @@ class TestImageMediaItem(TestCase): Test that the save_new_images_list() ignores everything in the provided list except strings """ # GIVEN: A list with images and objects - image_list = ['test_image_1.jpg', None, True, ImageFilenames(), 'test_image_2.jpg'] + image_list = [Path('test_image_1.jpg'), None, True, ImageFilenames(), Path('test_image_2.jpg')] self.media_item.manager = MagicMock() # WHEN: We run save_new_images_list with the list of images and objects @@ -191,7 +191,7 @@ class TestImageMediaItem(TestCase): ImageGroups.parent_id = 1 self.media_item.manager = MagicMock() self.media_item.manager.get_all_objects.side_effect = self._recursively_delete_group_side_effect - self.media_item.service_path = '' + self.media_item.service_path = Path() test_group = ImageGroups() test_group.id = 1 @@ -215,13 +215,13 @@ class TestImageMediaItem(TestCase): # Create some fake objects that should be removed returned_object1 = ImageFilenames() returned_object1.id = 1 - returned_object1.filename = '/tmp/test_file_1.jpg' + returned_object1.file_path = Path('/', 'tmp', 'test_file_1.jpg') returned_object2 = ImageFilenames() returned_object2.id = 2 - returned_object2.filename = '/tmp/test_file_2.jpg' + returned_object2.file_path = Path('/', 'tmp', 'test_file_2.jpg') returned_object3 = ImageFilenames() returned_object3.id = 3 - returned_object3.filename = '/tmp/test_file_3.jpg' + returned_object3.file_path = Path('/', 'tmp', 'test_file_3.jpg') return [returned_object1, returned_object2, returned_object3] if args[1] == ImageGroups and args[2]: # Change the parent_id that is matched so we don't get into an endless loop @@ -243,9 +243,9 @@ class TestImageMediaItem(TestCase): test_image = ImageFilenames() test_image.id = 1 test_image.group_id = 1 - test_image.filename = 'imagefile.png' + test_image.file_path = Path('imagefile.png') self.media_item.manager = MagicMock() - self.media_item.service_path = '' + self.media_item.service_path = Path() self.media_item.list_view = MagicMock() mocked_row_item = MagicMock() mocked_row_item.data.return_value = test_image @@ -265,13 +265,13 @@ class TestImageMediaItem(TestCase): # GIVEN: An ImageFilenames that already exists in the database image_file = ImageFilenames() image_file.id = 1 - image_file.filename = '/tmp/test_file_1.jpg' + image_file.file_path = Path('/', 'tmp', 'test_file_1.jpg') self.media_item.manager = MagicMock() self.media_item.manager.get_object_filtered.return_value = image_file - ImageFilenames.filename = '' + ImageFilenames.file_path = None # WHEN: create_item_from_id() is called - item = self.media_item.create_item_from_id(1) + item = self.media_item.create_item_from_id('1') # THEN: A QTreeWidgetItem should be created with the above model object as it's data self.assertIsInstance(item, QtWidgets.QTreeWidgetItem) @@ -279,4 +279,4 @@ class TestImageMediaItem(TestCase): item_data = item.data(0, QtCore.Qt.UserRole) self.assertIsInstance(item_data, ImageFilenames) self.assertEqual(1, item_data.id) - self.assertEqual('/tmp/test_file_1.jpg', item_data.filename) + self.assertEqual(Path('/', 'tmp', 'test_file_1.jpg'), item_data.file_path) diff --git a/tests/functional/openlp_plugins/images/test_upgrade.py b/tests/functional/openlp_plugins/images/test_upgrade.py new file mode 100644 index 000000000..c80e9c3c9 --- /dev/null +++ b/tests/functional/openlp_plugins/images/test_upgrade.py @@ -0,0 +1,83 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2017 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 # +############################################################################### +""" +This module contains tests for the lib submodule of the Images plugin. +""" +import os +import shutil +from tempfile import mkdtemp +from unittest import TestCase +from unittest.mock import patch + +from openlp.core.common import AppLocation, Settings +from openlp.core.common.path import Path +from openlp.core.lib.db import Manager +from openlp.plugins.images.lib import upgrade +from openlp.plugins.images.lib.db import ImageFilenames, init_schema + +from tests.helpers.testmixin import TestMixin +from tests.utils.constants import TEST_RESOURCES_PATH + +__default_settings__ = { + 'images/db type': 'sqlite', + 'images/background color': '#000000', +} + + +class TestImageDBUpgrade(TestCase, TestMixin): + """ + Test that the image database is upgraded correctly + """ + def setUp(self): + self.build_settings() + Settings().extend_default_settings(__default_settings__) + self.tmp_folder = mkdtemp() + + def tearDown(self): + """ + Delete all the C++ objects at the end so that we don't have a segfault + """ + self.destroy_settings() + # Ignore errors since windows can have problems with locked files + shutil.rmtree(self.tmp_folder, ignore_errors=True) + + def test_image_filenames_table(self): + """ + Test that the ImageFilenames table is correctly upgraded to the latest version + """ + # GIVEN: An unversioned image database + temp_db_name = os.path.join(self.tmp_folder, 'image-v0.sqlite') + shutil.copyfile(os.path.join(TEST_RESOURCES_PATH, 'images', 'image-v0.sqlite'), temp_db_name) + + with patch.object(AppLocation, 'get_data_path', return_value=Path('/', 'test', 'dir')): + # WHEN: Initalising the database manager + manager = Manager('images', init_schema, db_file_path=temp_db_name, upgrade_mod=upgrade) + + # THEN: The database should have been upgraded and image_filenames.file_path should return Path objects + upgraded_results = manager.get_all_objects(ImageFilenames) + + expected_result_data = {1: Path('/', 'test', 'image1.jpg'), + 2: Path('/', 'test', 'dir', 'image2.jpg'), + 3: Path('/', 'test', 'dir', 'subdir', 'image3.jpg')} + + for result in upgraded_results: + self.assertEqual(expected_result_data[result.id], result.file_path) diff --git a/tests/resources/images/image-v0.sqlite b/tests/resources/images/image-v0.sqlite new file mode 100644 index 000000000..92e199bfd Binary files /dev/null and b/tests/resources/images/image-v0.sqlite differ