Merge lp:~phill-ridout/openlp/pathlib6 into lp:openlp

[Phill]

This is ready to go now!

Upgrade the image plugin to use pathlib.
Other minor changes and fixes.

Add this to your merge proposal:
--------------------------------
lp:~phill-ridout/openlp/pathlib6 (revision 2774)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/2207/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/2110/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1997/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1364/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1196/
[SUCCESS] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/326/
[FAILURE] https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/165/
Stopping after failure

[Tim Bentley (trb143)]

Why have you removed a couple of __init__ methods?

review: Needs Information

[Phill (phill-ridout)]

those __init__ methods just call the super method. They don't do anything I
fail to see any reason for keeping them!

On Sun, 24 Sep 2017, 22:01 Tim Bentley <email address hidden> wrote:

> Review: Needs Information
>
> Why have you removed a couple of __init__ methods?
> --
> https://code.launchpad.net/~phill-ridout/openlp/pathlib6/+merge/331252
> You are the owner of lp:~phill-ridout/openlp/pathlib6.
>

[Phill (phill-ridout)]

Just noticed one minor mistake.

review: Needs Fixing

[Tim Bentley (trb143)]

review: Approve

[Tomas Groth (tomasgroth)]

Just a minor thing, see inline comment.

review: Needs Fixing

[Tomas Groth (tomasgroth)]

review: Approve

=== modified file 'openlp/core/common/httputils.py'
--- openlp/core/common/httputils.py	2017-09-06 20:18:08 +0000
+++ openlp/core/common/httputils.py	2017-09-25 17:10:35 +0000
@@ -24,7 +24,6 @@
 """
 import hashlib
 import logging
-import os
 import platform
 import socket
 import sys

=== modified file 'openlp/core/lib/db.py'
--- openlp/core/lib/db.py	2017-08-12 17:45:56 +0000
+++ openlp/core/lib/db.py	2017-09-25 17:10:35 +0000
@@ -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 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 @@
     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 @@
         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 @@
     :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 @@
     :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 @@
     """
     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 @@
             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
 

=== modified file 'openlp/core/ui/advancedtab.py'
--- openlp/core/ui/advancedtab.py	2017-08-23 20:21:11 +0000
+++ openlp/core/ui/advancedtab.py	2017-09-25 17:10:35 +0000
@@ -22,9 +22,8 @@
 """
 The :mod:`advancedtab` provides an advanced settings facility.
 """
+import logging
 from datetime import datetime, timedelta
-import logging
-import os
 
 from PyQt5 import QtCore, QtGui, QtWidgets
 
@@ -492,24 +491,27 @@
         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 @@
     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 @@
                                                              '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 @@
         """
         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()

=== modified file 'openlp/core/ui/exceptionform.py'
--- openlp/core/ui/exceptionform.py	2017-08-26 15:06:11 +0000
+++ openlp/core/ui/exceptionform.py	2017-09-25 17:10:35 +0000
@@ -149,21 +149,11 @@
             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 @@
                                        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)
 

=== modified file 'openlp/core/ui/lib/filedialog.py'
--- openlp/core/ui/lib/filedialog.py	2017-09-20 20:44:57 +0000
+++ openlp/core/ui/lib/filedialog.py	2017-09-25 17:10:35 +0000
@@ -31,11 +31,11 @@
         """
         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 @@
         """
         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 @@
         """
         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 @@
         """
         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),))
 

=== modified file 'openlp/core/ui/mainwindow.py'
--- openlp/core/ui/mainwindow.py	2017-09-20 20:44:57 +0000
+++ openlp/core/ui/mainwindow.py	2017-09-25 17:10:35 +0000
@@ -1332,12 +1332,6 @@
             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
@@ -1349,7 +1343,7 @@
         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:
@@ -1358,7 +1352,7 @@
                 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()
@@ -1373,9 +1367,9 @@
             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()
 

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2017-09-18 06:20:06 +0000
+++ openlp/core/ui/servicemanager.py	2017-09-25 17:10:35 +0000
@@ -376,7 +376,7 @@
         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 @@
             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:

=== modified file 'openlp/plugins/alerts/forms/alertform.py'
--- openlp/plugins/alerts/forms/alertform.py	2017-06-09 06:06:49 +0000
+++ openlp/plugins/alerts/forms/alertform.py	2017-09-25 17:10:35 +0000
@@ -70,7 +70,7 @@
             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))
 

=== modified file 'openlp/plugins/alerts/lib/alertstab.py'
--- openlp/plugins/alerts/lib/alertstab.py	2017-08-03 17:54:40 +0000
+++ openlp/plugins/alerts/lib/alertstab.py	2017-09-25 17:10:35 +0000
@@ -32,9 +32,6 @@
     """
     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()

=== modified file 'openlp/plugins/custom/lib/customtab.py'
--- openlp/plugins/custom/lib/customtab.py	2016-12-31 11:01:36 +0000
+++ openlp/plugins/custom/lib/customtab.py	2017-09-25 17:10:35 +0000
@@ -34,9 +34,6 @@
     """
     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()

=== modified file 'openlp/plugins/images/imageplugin.py'
--- openlp/plugins/images/imageplugin.py	2017-09-09 20:00:48 +0000
+++ openlp/plugins/images/imageplugin.py	2017-09-25 17:10:35 +0000
@@ -29,7 +29,7 @@
 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 @@
 
     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)

=== modified file 'openlp/plugins/images/lib/db.py'
--- openlp/plugins/images/lib/db.py	2016-12-31 11:01:36 +0000
+++ openlp/plugins/images/lib/db.py	2017-09-25 17:10:35 +0000
@@ -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 @@
 
             * id
             * group_id
-            * filename
+            * file_path
     """
     session, metadata = init_db(url)
 
@@ -80,7 +79,7 @@
     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)

=== modified file 'openlp/plugins/images/lib/imagetab.py'
--- openlp/plugins/images/lib/imagetab.py	2016-12-31 11:01:36 +0000
+++ openlp/plugins/images/lib/imagetab.py	2017-09-25 17:10:35 +0000
@@ -31,9 +31,6 @@
     """
     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()

=== modified file 'openlp/plugins/images/lib/mediaitem.py'
--- openlp/plugins/images/lib/mediaitem.py	2017-09-17 19:43:15 +0000
+++ openlp/plugins/images/lib/mediaitem.py	2017-09-25 17:10:35 +0000
@@ -21,7 +21,6 @@
 ###############################################################################
 
 import logging
-import os
 
 from PyQt5 import QtCore, QtGui, QtWidgets
 
@@ -99,11 +98,11 @@
         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 @@
         """
         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 @@
                 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 @@
         """
         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 @@
             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 @@
         :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 @@
             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 @@
             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 @@
             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 @@
             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 @@
             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 @@
             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 @@
                 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 @@
 
         :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

=== added file 'openlp/plugins/images/lib/upgrade.py'
--- openlp/plugins/images/lib/upgrade.py	1970-01-01 00:00:00 +0000
+++ openlp/plugins/images/lib/upgrade.py	2017-09-25 17:10:35 +0000
@@ -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')

=== modified file 'openlp/plugins/songusage/forms/songusagedetaildialog.py'
--- openlp/plugins/songusage/forms/songusagedetaildialog.py	2017-06-09 06:06:49 +0000
+++ openlp/plugins/songusage/forms/songusagedetaildialog.py	2017-09-25 17:10:35 +0000
@@ -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

=== modified file 'openlp/plugins/songusage/forms/songusagedetailform.py'
--- openlp/plugins/songusage/forms/songusagedetailform.py	2017-08-26 15:06:11 +0000
+++ openlp/plugins/songusage/forms/songusagedetailform.py	2017-09-25 17:10:35 +0000
@@ -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 @@
 
     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 @@
         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 @@
                           ' 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 @@
             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()

=== modified file 'tests/functional/openlp_core_ui/test_exceptionform.py'
--- tests/functional/openlp_core_ui/test_exceptionform.py	2017-09-07 21:52:39 +0000
+++ tests/functional/openlp_core_ui/test_exceptionform.py	2017-09-25 17:10:35 +0000
@@ -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
@@ -142,15 +142,15 @@
         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)
@@ -182,25 +182,24 @@
         mocked_qt.PYQT_VERSION_STR = 'PyQt5 Test'
         mocked_is_linux.return_value = False
         mocked_application_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(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'
-
-                        # WHEN: on_save_report_button_clicked called
-                        test_form.on_save_report_button_clicked()
+
+        with patch.object(Path, 'open') as mocked_path_open:
+            test_path = Path('testfile.txt')
+            mocked_save_filename.return_value = test_path, 'ext'
+
+            test_form = exceptionform.ExceptionForm()
+            test_form.file_attachment = None
+
+            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)])

=== modified file 'tests/functional/openlp_plugins/images/test_lib.py'
--- tests/functional/openlp_plugins/images/test_lib.py	2017-08-26 15:06:11 +0000
+++ tests/functional/openlp_plugins/images/test_lib.py	2017-09-25 17:10:35 +0000
@@ -58,7 +58,7 @@
         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 @@
         # 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 @@
         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 @@
         # 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 @@
         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 @@
         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 @@
         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 @@
         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 @@
         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 @@
         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 @@
             # 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 @@
         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 @@
         # 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 @@
         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)

=== added file 'tests/functional/openlp_plugins/images/test_upgrade.py'
--- tests/functional/openlp_plugins/images/test_upgrade.py	1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_plugins/images/test_upgrade.py	2017-09-25 17:10:35 +0000
@@ -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)

=== added directory 'tests/resources/images'
=== added file 'tests/resources/images/image-v0.sqlite'
Binary files tests/resources/images/image-v0.sqlite	1970-01-01 00:00:00 +0000 and tests/resources/images/image-v0.sqlite	2017-09-25 17:10:35 +0000 differ