merge trunk

openlp/core/common/__init__.py

@@ -195,7 +195,7 @@ def verify_ip_address(addr):
     return True if verify_ipv4(addr) else verify_ipv6(addr)
 
 
-def md5_hash(salt, data=None):
+def md5_hash(salt=None, data=None):
     """
     Returns the hashed output of md5sum on salt,data
     using Python3 hashlib
@@ -205,8 +205,11 @@ def md5_hash(salt, data=None):
     :returns: str
     """
     log.debug('md5_hash(salt="{text}")'.format(text=salt))
+    if not salt and not data:
+        return None
     hash_obj = hashlib.new('md5')
-    hash_obj.update(salt)
+    if salt:
+        hash_obj.update(salt)
     if data:
         hash_obj.update(data)
     hash_value = hash_obj.hexdigest()
@@ -214,22 +217,30 @@ def md5_hash(salt, data=None):
     return hash_value
 
 
-def qmd5_hash(salt, data=None):
+def qmd5_hash(salt=None, data=None):
     """
     Returns the hashed output of MD5Sum on salt, data
-    using PyQt5.QCryptographicHash.
+    using PyQt5.QCryptographicHash. Function returns a
+    QByteArray instead of a text string.
+    If you need a string instead, call with
+
+        result = str(qmd5_hash(salt=..., data=...), encoding='ascii')
 
     :param salt: Initial salt
     :param data: OPTIONAL Data to hash
-    :returns: str
+    :returns: QByteArray
     """
     log.debug('qmd5_hash(salt="{text}"'.format(text=salt))
+    if salt is None and data is None:
+        return None
     hash_obj = QHash(QHash.Md5)
-    hash_obj.addData(salt)
-    hash_obj.addData(data)
+    if salt:
+        hash_obj.addData(salt)
+    if data:
+        hash_obj.addData(data)
     hash_value = hash_obj.result().toHex()
-    log.debug('qmd5_hash() returning "{text}"'.format(text=hash_value))
-    return hash_value.data()
+    log.debug('qmd5_hash() returning "{hash}"'.format(hash=hash_value))
+    return hash_value
 
 
 def clean_button_text(button_text):

openlp/core/common/actions.py

@@ -138,7 +138,7 @@ class CategoryList(object):
         for category in self.categories:
             if category.name == key:
                 return category
-        raise KeyError('Category "{keY}" does not exist.'.format(key=key))
+        raise KeyError('Category "{key}" does not exist.'.format(key=key))
 
     def __len__(self):
         """

openlp/core/common/db.py

@@ -22,11 +22,12 @@
 """
 The :mod:`db` module provides helper functions for database related methods.
 """
-import sqlalchemy
 import logging
-
 from copy import deepcopy
 
+import sqlalchemy
+
+
 log = logging.getLogger(__name__)
 
 

openlp/core/common/languagemanager.py

@@ -168,7 +168,7 @@ def format_time(text, local_time):
         """
         return local_time.strftime(match.group())
 
-    return re.sub('\%[a-zA-Z]', match_formatting, text)
+    return re.sub(r'\%[a-zA-Z]', match_formatting, text)
 
 
 def get_locale_key(string):

openlp/core/common/settings.py

@@ -26,7 +26,7 @@ import datetime
 import logging
 import os
 
-from PyQt5 import QtCore, QtGui, QtWidgets
+from PyQt5 import QtCore, QtGui
 
 from openlp.core.common import ThemeLevel, SlideLimits, UiStrings, is_win, is_linux
 

openlp/core/common/uistrings.py

@@ -68,7 +68,7 @@ class UiStrings(object):
         self.Default = translate('OpenLP.Ui', 'Default')
         self.DefaultColor = translate('OpenLP.Ui', 'Default Color:')
         self.DefaultServiceName = translate('OpenLP.Ui', 'Service %Y-%m-%d %H-%M',
-                                            'This may not contain any of the following characters: /\\?*|<>\[\]":+\n'
+                                            'This may not contain any of the following characters: /\\?*|<>[]":+\n'
                                             'See http://docs.python.org/library/datetime'
                                             '.html#strftime-strptime-behavior for more information.')
         self.Delete = translate('OpenLP.Ui', '&Delete')

openlp/core/common/versionchecker.py

@@ -10,10 +10,10 @@ from datetime import datetime
 from distutils.version import LooseVersion
 from subprocess import Popen, PIPE
 
-from openlp.core.common import AppLocation, Settings
-
 from PyQt5 import QtCore
 
+from openlp.core.common import AppLocation, Settings
+
 log = logging.getLogger(__name__)
 
 APPLICATION_VERSION = {}

openlp/core/lib/__init__.py

@@ -97,7 +97,7 @@ def get_text_file_string(text_file):
     content = None
     try:
         file_handle = open(text_file, 'r', encoding='utf-8')
-        if not file_handle.read(3) == '\xEF\xBB\xBF':
+        if file_handle.read(3) != '\xEF\xBB\xBF':
             # no BOM was found
             file_handle.seek(0)
         content = file_handle.read()

openlp/core/lib/db.py

@@ -122,6 +122,21 @@ def get_upgrade_op(session):
     return Operations(context)
 
 
+class BaseModel(object):
+    """
+    BaseModel provides a base object with a set of generic functions
+    """
+    @classmethod
+    def populate(cls, **kwargs):
+        """
+        Creates an instance of a class and populates it, returning the instance
+        """
+        instance = cls()
+        for key, value in kwargs.items():
+            instance.__setattr__(key, value)
+        return instance
+
+
 def upgrade_db(url, upgrade):
     """
     Upgrade a database.
@@ -178,9 +193,9 @@ def upgrade_db(url, upgrade):
         version_meta = Metadata.populate(key='version', value=int(upgrade.__version__))
         session.commit()
     upgrade_version = upgrade.__version__
-    version_meta = int(version_meta.value)
+    version = int(version_meta.value)
     session.close()
-    return version_meta, upgrade_version
+    return version, upgrade_version
 
 
 def delete_database(plugin_name, db_file_name=None):
@@ -197,21 +212,6 @@ def delete_database(plugin_name, db_file_name=None):
     return delete_file(db_file_path)
 
 
-class BaseModel(object):
-    """
-    BaseModel provides a base object with a set of generic functions
-    """
-    @classmethod
-    def populate(cls, **kwargs):
-        """
-        Creates an instance of a class and populates it, returning the instance
-        """
-        instance = cls()
-        for key, value in kwargs.items():
-            instance.__setattr__(key, value)
-        return instance
-
-
 class Manager(object):
     """
     Provide generic object persistence management

openlp/core/lib/htmlbuilder.py

@@ -389,8 +389,8 @@ is the function which has to be called from outside. The generated and returned
 """
 import logging
 
-from PyQt5 import QtWebKit
 from string import Template
+from PyQt5 import QtWebKit
 
 from openlp.core.common import Settings
 from openlp.core.lib.theme import BackgroundType, BackgroundGradientType, VerticalType, HorizontalType
@@ -670,7 +670,7 @@ def webkit_version():
         webkit_ver = float(QtWebKit.qWebKitVersion())
         log.debug('Webkit version = {version}'.format(version=webkit_ver))
     except AttributeError:
-        webkit_ver = 0
+        webkit_ver = 0.0
     return webkit_ver
 
 

openlp/core/lib/imagemanager.py

@@ -272,7 +272,7 @@ class ImageManager(QtCore.QObject):
         Add image to cache if it is not already there.
         """
         log.debug('add_image {path}'.format(path=path))
-        if not (path, source, width, height) in self._cache:
+        if (path, source, width, height) not in self._cache:
             image = Image(path, source, background, width, height)
             self._cache[(path, source, width, height)] = image
             self._conversion_queue.put((image.priority, image.secondary_priority, image))

openlp/core/lib/pluginmanager.py

@@ -23,7 +23,6 @@
 Provide plugin management
 """
 import os
-import sys
 import imp
 
 from openlp.core.lib import Plugin, PluginStatus

openlp/core/lib/projector/db.py

@@ -40,13 +40,12 @@ log.debug('projector.lib.db module loaded')
 
 from sqlalchemy import Column, ForeignKey, Integer, MetaData, String, and_
 from sqlalchemy.ext.declarative import declarative_base, declared_attr
-from sqlalchemy.orm import backref, relationship
+from sqlalchemy.orm import relationship
 
 from openlp.core.lib.db import Manager, init_db, init_url
 from openlp.core.lib.projector.constants import PJLINK_DEFAULT_CODES
 
-metadata = MetaData()
-Base = declarative_base(metadata)
+Base = declarative_base(MetaData())
 
 
 class CommonBase(object):
@@ -54,8 +53,8 @@ class CommonBase(object):
     Base class to automate table name and ID column.
     """
     @declared_attr
-    def __tablename__(cls):
-        return cls.__name__.lower()
+    def __tablename__(self):
+        return self.__name__.lower()
 
     id = Column(Integer, primary_key=True)
 
@@ -257,7 +256,7 @@ class ProjectorDB(Manager):
         projector = self.get_object_filtered(Projector, Projector.id == dbid)
         if projector is None:
             # Not found
-            log.warn('get_projector_by_id() did not find {data}'.format(data=id))
+            log.warning('get_projector_by_id() did not find {data}'.format(data=id))
             return None
         log.debug('get_projectorby_id() returning 1 entry for "{entry}" id="{data}"'.format(entry=dbid,
                                                                                             data=projector.id))
@@ -290,7 +289,7 @@ class ProjectorDB(Manager):
         projector = self.get_object_filtered(Projector, Projector.ip == ip)
         if projector is None:
             # Not found
-            log.warn('get_projector_by_ip() did not find {ip}'.format(ip=ip))
+            log.warning('get_projector_by_ip() did not find {ip}'.format(ip=ip))
             return None
         log.debug('get_projectorby_ip() returning 1 entry for "{ip}" id="{data}"'.format(ip=ip,
                                                                                          data=projector.id))
@@ -307,7 +306,7 @@ class ProjectorDB(Manager):
         projector = self.get_object_filtered(Projector, Projector.name == name)
         if projector is None:
             # Not found
-            log.warn('get_projector_by_name() did not find "{name}"'.format(name=name))
+            log.warning('get_projector_by_name() did not find "{name}"'.format(name=name))
             return None
         log.debug('get_projector_by_name() returning one entry for "{name}" id="{data}"'.format(name=name,
                                                                                                 data=projector.id))
@@ -324,7 +323,7 @@ class ProjectorDB(Manager):
         """
         old_projector = self.get_object_filtered(Projector, Projector.ip == projector.ip)
         if old_projector is not None:
-            log.warn('add_new() skipping entry ip="{ip}" (Already saved)'.format(ip=old_projector.ip))
+            log.warning('add_new() skipping entry ip="{ip}" (Already saved)'.format(ip=old_projector.ip))
             return False
         log.debug('add_new() saving new entry')
         log.debug('ip="{ip}", name="{name}", location="{location}"'.format(ip=projector.ip,
@@ -408,10 +407,10 @@ class ProjectorDB(Manager):
         :param source: ProjectorSource id
         :returns: ProjetorSource instance or None
         """
-        source_entry = self.get_object_filtered(ProjetorSource, ProjectorSource.id == source)
+        source_entry = self.get_object_filtered(ProjectorSource, ProjectorSource.id == source)
         if source_entry is None:
             # Not found
-            log.warn('get_source_by_id() did not find "{source}"'.format(source=source))
+            log.warning('get_source_by_id() did not find "{source}"'.format(source=source))
             return None
         log.debug('get_source_by_id() returning one entry for "{source}""'.format(source=source))
         return source_entry
@@ -430,8 +429,8 @@ class ProjectorDB(Manager):
 
         if source_entry is None:
             # Not found
-            log.warn('get_source_by_id() not found')
-            log.warn('code="{code}" projector_id="{data}"'.format(code=code, data=projector_id))
+            log.warning('get_source_by_id() not found')
+            log.warning('code="{code}" projector_id="{data}"'.format(code=code, data=projector_id))
             return None
         log.debug('get_source_by_id() returning one entry')
         log.debug('code="{code}" projector_id="{data}"'.format(code=code, data=projector_id))

openlp/core/lib/projector/pjlink1.py

@@ -297,6 +297,8 @@ class PJLink1(QTcpSocket):
         Processes the initial connection and authentication (if needed).
         Starts poll timer if connection is established.
 
+        NOTE: Qt md5 hash function doesn't work with projector authentication. Use the python md5 hash function.
+
         :param data: Optional data if called from another routine
         """
         log.debug('({ip}) check_login(data="{data}")'.format(ip=self.ip, data=data))
@@ -310,10 +312,10 @@ class PJLink1(QTcpSocket):
             read = self.readLine(self.maxSize)
             dontcare = self.readLine(self.maxSize)  # Clean out the trailing \r\n
             if read is None:
-                log.warn('({ip}) read is None - socket error?'.format(ip=self.ip))
+                log.warning('({ip}) read is None - socket error?'.format(ip=self.ip))
                 return
             elif len(read) < 8:
-                log.warn('({ip}) Not enough data read)'.format(ip=self.ip))
+                log.warning('({ip}) Not enough data read)'.format(ip=self.ip))
                 return
             data = decode(read, 'ascii')
             # Possibility of extraneous data on input when reading.
@@ -344,23 +346,33 @@ class PJLink1(QTcpSocket):
             return
         elif data_check[1] == '0' and self.pin is not None:
             # Pin set and no authentication needed
+            log.warning('({ip}) Regular connection but PIN set'.format(ip=self.name))
             self.disconnect_from_host()
             self.change_status(E_AUTHENTICATION)
-            log.debug('({ip}) emitting projectorNoAuthentication() signal'.format(ip=self.name))
+            log.debug('({ip}) Emitting projectorNoAuthentication() signal'.format(ip=self.name))
             self.projectorNoAuthentication.emit(self.name)
             return
         elif data_check[1] == '1':
             # Authenticated login with salt
-            log.debug('({ip}) Setting hash with salt="{data}"'.format(ip=self.ip, data=data_check[2]))
-            log.debug('({ip}) pin="{data}"'.format(ip=self.ip, data=self.pin))
-            salt = qmd5_hash(salt=data_check[2].encode('ascii'), data=self.pin.encode('ascii'))
+            if self.pin is None:
+                log.warning('({ip}) Authenticated connection but no pin set'.format(ip=self.name))
+                self.disconnect_from_host()
+                self.change_status(E_AUTHENTICATION)
+                log.debug('({ip}) Emitting projectorAuthentication() signal'.format(ip=self.name))
+                self.projectorAuthentication.emit(self.name)
+                return
+            else:
+                log.debug('({ip}) Setting hash with salt="{data}"'.format(ip=self.ip, data=data_check[2]))
+                log.debug('({ip}) pin="{data}"'.format(ip=self.ip, data=self.pin))
+                data_hash = str(qmd5_hash(salt=data_check[2].encode('utf-8'), data=self.pin.encode('utf-8')),
+                                encoding='ascii')
         else:
-            salt = None
-        # We're connected at this point, so go ahead and do regular I/O
+            data_hash = None
+        # We're connected at this point, so go ahead and setup regular I/O
         self.readyRead.connect(self.get_data)
         self.projectorReceivedData.connect(self._send_command)
         # Initial data we should know about
-        self.send_command(cmd='CLSS', salt=salt)
+        self.send_command(cmd='CLSS', salt=data_hash)
         self.waitForReadyRead()
         if (not self.no_poll) and (self.state() == self.ConnectedState):
             log.debug('({ip}) Starting timer'.format(ip=self.ip))
@@ -402,7 +414,7 @@ class PJLink1(QTcpSocket):
             self.projectorReceivedData.emit()
             return
         elif '=' not in data:
-            log.warn('({ip}) get_data(): Invalid packet received'.format(ip=self.ip))
+            log.warning('({ip}) get_data(): Invalid packet received'.format(ip=self.ip))
             self.send_busy = False
             self.projectorReceivedData.emit()
             return
@@ -410,15 +422,15 @@ class PJLink1(QTcpSocket):
         try:
             (prefix, class_, cmd, data) = (data_split[0][0], data_split[0][1], data_split[0][2:], data_split[1])
         except ValueError as e:
-            log.warn('({ip}) get_data(): Invalid packet - expected header + command + data'.format(ip=self.ip))
-            log.warn('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip()))
+            log.warning('({ip}) get_data(): Invalid packet - expected header + command + data'.format(ip=self.ip))
+            log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip()))
             self.change_status(E_INVALID_DATA)
             self.send_busy = False
             self.projectorReceivedData.emit()
             return
 
         if not (self.pjlink_class in PJLINK_VALID_CMD and cmd in PJLINK_VALID_CMD[self.pjlink_class]):
-            log.warn('({ip}) get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.ip, data=cmd))
+            log.warning('({ip}) get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.ip, data=cmd))
             self.send_busy = False
             self.projectorReceivedData.emit()
             return
@@ -461,7 +473,7 @@ class PJLink1(QTcpSocket):
         :param queue: Option to force add to queue rather than sending directly
         """
         if self.state() != self.ConnectedState:
-            log.warn('({ip}) send_command(): Not connected - returning'.format(ip=self.ip))
+            log.warning('({ip}) send_command(): Not connected - returning'.format(ip=self.ip))
             self.send_queue = []
             return
         self.projectorNetwork.emit(S_NETWORK_SENDING)
@@ -577,7 +589,7 @@ class PJLink1(QTcpSocket):
         if cmd in self.PJLINK1_FUNC:
             self.PJLINK1_FUNC[cmd](data)
         else:
-            log.warn('({ip}) Invalid command {data}'.format(ip=self.ip, data=cmd))
+            log.warning('({ip}) Invalid command {data}'.format(ip=self.ip, data=cmd))
         self.send_busy = False
         self.projectorReceivedData.emit()
 
@@ -596,7 +608,7 @@ class PJLink1(QTcpSocket):
                 fill = {'Hours': int(data_dict[0]), 'On': False if data_dict[1] == '0' else True}
             except ValueError:
                 # In case of invalid entry
-                log.warn('({ip}) process_lamp(): Invalid data "{data}"'.format(ip=self.ip, data=data))
+                log.warning('({ip}) process_lamp(): Invalid data "{data}"'.format(ip=self.ip, data=data))
                 return
             lamps.append(fill)
             data_dict.pop(0)  # Remove lamp hours
@@ -623,7 +635,7 @@ class PJLink1(QTcpSocket):
                     self.send_command('INST')
         else:
             # Log unknown status response
-            log.warn('({ip}) Unknown power response: {data}'.format(ip=self.ip, data=data))
+            log.warning('({ip}) Unknown power response: {data}'.format(ip=self.ip, data=data))
         return
 
     def process_avmt(self, data):
@@ -648,7 +660,7 @@ class PJLink1(QTcpSocket):
             shutter = True
             mute = True
         else:
-            log.warn('({ip}) Unknown shutter response: {data}'.format(ip=self.ip, data=data))
+            log.warning('({ip}) Unknown shutter response: {data}'.format(ip=self.ip, data=data))
         update_icons = shutter != self.shutter
         update_icons = update_icons or mute != self.mute
         self.shutter = shutter
@@ -797,7 +809,7 @@ class PJLink1(QTcpSocket):
         Initiate connection to projector.
         """
         if self.state() == self.ConnectedState:
-            log.warn('({ip}) connect_to_host(): Already connected - returning'.format(ip=self.ip))
+            log.warning('({ip}) connect_to_host(): Already connected - returning'.format(ip=self.ip))
             return
         self.change_status(S_CONNECTING)
         self.connectToHost(self.ip, self.port if type(self.port) is int else int(self.port))
@@ -809,9 +821,9 @@ class PJLink1(QTcpSocket):
         """
         if abort or self.state() != self.ConnectedState:
             if abort:
-                log.warn('({ip}) disconnect_from_host(): Aborting connection'.format(ip=self.ip))
+                log.warning('({ip}) disconnect_from_host(): Aborting connection'.format(ip=self.ip))
             else:
-                log.warn('({ip}) disconnect_from_host(): Not connected - returning'.format(ip=self.ip))
+                log.warning('({ip}) disconnect_from_host(): Not connected - returning'.format(ip=self.ip))
             self.reset_information()
         self.disconnectFromHost()
         try:

openlp/core/lib/renderer.py

@@ -531,7 +531,7 @@ def words_split(line):
     :param line: Line to be split
     """
     # this parse we are to be wordy
-    return re.split('\s+', line)
+    return re.split(r'\s+', line)
 
 
 def get_start_tags(raw_text):

openlp/core/lib/theme.py

@@ -23,7 +23,6 @@
 Provide the theme XML and handling functions for OpenLP v2 themes.
 """
 import os
-import re
 import logging
 import json
 
@@ -165,6 +164,7 @@ class ThemeXML(object):
         jsn = get_text_file_string(json_file)
         jsn = json.loads(jsn)
         self.expand_json(jsn)
+        self.background_filename = None
 
     def expand_json(self, var, prev=None):
         """
@@ -474,15 +474,16 @@ class ThemeXML(object):
             if element.startswith('shadow') or element.startswith('outline'):
                 master = 'font_main'
         # fix bold font
+        ret_value = None
         if element == 'weight':
             element = 'bold'
             if value == 'Normal':
-                value = False
+                ret_value = False
             else:
-                value = True
+                ret_value = True
         if element == 'proportion':
             element = 'size'
-        return False, master, element, value
+        return False, master, element, ret_value if ret_value is not None else value
 
     def _create_attr(self, master, element, value):
         """

openlp/core/lib/webpagereader.py

@@ -179,5 +179,4 @@ def get_web_page(url, header=None, update_openlp=False):
     return page
 
 
-__all__ = ['get_application_version', 'check_latest_version',
-           'get_web_page']
+__all__ = ['get_web_page']

openlp/core/ui/exceptionform.py

@@ -32,8 +32,6 @@ import sqlalchemy
 from PyQt5 import Qt, QtCore, QtGui, QtWebKit, QtWidgets
 from lxml import etree
 
-from openlp.core.common import RegistryProperties, is_linux
-
 try:
     import migrate
     MIGRATE_VERSION = getattr(migrate, '__version__', '< 0.7')
@@ -74,6 +72,7 @@ except ImportError:
 
 from openlp.core.common import Settings, UiStrings, translate
 from openlp.core.common.versionchecker import get_application_version
+from openlp.core.common import RegistryProperties, is_linux
 
 from .exceptiondialog import Ui_ExceptionDialog
 

openlp/core/ui/firsttimeform.py

@@ -666,14 +666,14 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         if missed_files:
             file_list = ''
             for entry in missed_files:
-                file_list += '{text}<br \>'.format(text=entry)
+                file_list += '{text}<br \\>'.format(text=entry)
             msg = QtWidgets.QMessageBox()
             msg.setIcon(QtWidgets.QMessageBox.Warning)
             msg.setWindowTitle(translate('OpenLP.FirstTimeWizard', 'Network Error'))
             msg.setText(translate('OpenLP.FirstTimeWizard', 'Unable to download some files'))
             msg.setInformativeText(translate('OpenLP.FirstTimeWizard',
                                              'The following files were not able to be '
-                                             'downloaded:<br \>{text}'.format(text=file_list)))
+                                             'downloaded:<br \\>{text}'.format(text=file_list)))
             msg.setStandardButtons(msg.Ok)
             ans = msg.exec()
         return True

openlp/core/ui/formattingtagcontroller.py

@@ -84,7 +84,7 @@ class FormattingTagController(object):
             'desc': desc,
             'start tag': '{{{tag}}}'.format(tag=tag),
             'start html': start_html,
-            'end tag': '{/{tag}}}'.format(tag=tag),
+            'end tag': '{{{tag}}}'.format(tag=tag),
             'end html': end_html,
             'protected': False,
             'temporary': False

openlp/core/ui/lib/spelltextedit.py

@@ -164,7 +164,7 @@ class Highlighter(QtGui.QSyntaxHighlighter):
     """
     Provides a text highlighter for pointing out spelling errors in text.
     """
-    WORDS = '(?iu)[\w\']+'
+    WORDS = r'(?iu)[\w\']+'
 
     def __init__(self, *args):
         """

openlp/core/ui/maindisplay.py

@@ -33,7 +33,7 @@ import html
 import logging
 import os
 
-from PyQt5 import QtCore, QtWidgets, QtWebKit, QtWebKitWidgets, QtOpenGL, QtGui, QtMultimedia
+from PyQt5 import QtCore, QtWidgets, QtWebKit, QtWebKitWidgets, QtGui, QtMultimedia
 
 from openlp.core.common import AppLocation, Registry, RegistryProperties, OpenLPMixin, Settings, translate,\
     is_macosx, is_win
@@ -468,9 +468,9 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties):
                     self.service_item.theme_data.background_filename, ImageSource.Theme)
             if image_path:
                 image_bytes = self.image_manager.get_image_bytes(image_path, ImageSource.ImagePlugin)
-        html = build_html(self.service_item, self.screen, self.is_live, background, image_bytes,
-                          plugins=self.plugin_manager.plugins)
-        self.web_view.setHtml(html)
+        created_html = build_html(self.service_item, self.screen, self.is_live, background, image_bytes,
+                                  plugins=self.plugin_manager.plugins)
+        self.web_view.setHtml(created_html)
         if service_item.foot_text:
             self.footer(service_item.foot_text)
         # if was hidden keep it hidden

openlp/core/ui/mainwindow.py

@@ -46,7 +46,6 @@ from openlp.core.ui.firsttimeform import FirstTimeForm
 from openlp.core.ui.media import MediaController
 from openlp.core.ui.printserviceform import PrintServiceForm
 from openlp.core.ui.projector.manager import ProjectorManager
-from openlp.core.ui.lib.toolbar import OpenLPToolbar
 from openlp.core.ui.lib.dockwidget import OpenLPDockWidget
 from openlp.core.ui.lib.mediadockmanager import MediaDockManager
 

openlp/core/ui/media/__init__.py

@@ -24,10 +24,10 @@ The :mod:`~openlp.core.ui.media` module contains classes and objects for media p
 """
 import logging
 
-from openlp.core.common import Settings
-
 from PyQt5 import QtCore
 
+from openlp.core.common import Settings
+
 log = logging.getLogger(__name__ + '.__init__')
 
 

openlp/core/ui/media/mediacontroller.py

@@ -38,7 +38,6 @@ from openlp.core.ui.media.mediaplayer import MediaPlayer
 from openlp.core.ui.media import MediaState, MediaInfo, MediaType, get_media_players, set_media_players,\
     parse_optical_path
 from openlp.core.ui.lib.toolbar import OpenLPToolbar
-from openlp.core.ui.lib.dockwidget import OpenLPDockWidget
 
 log = logging.getLogger(__name__)
 
@@ -175,7 +174,7 @@ class MediaController(RegistryMixin, OpenLPMixin, RegistryProperties):
         log.debug('_check_available_media_players')
         controller_dir = os.path.join(AppLocation.get_directory(AppLocation.AppDir), 'core', 'ui', 'media')
         for filename in os.listdir(controller_dir):
-            if filename.endswith('player.py') and not filename == 'mediaplayer.py':
+            if filename.endswith('player.py') and filename != 'mediaplayer.py':
                 path = os.path.join(controller_dir, filename)
                 if os.path.isfile(path):
                     module_name = 'openlp.core.ui.media.' + os.path.splitext(filename)[0]
@@ -554,7 +553,7 @@ class MediaController(RegistryMixin, OpenLPMixin, RegistryProperties):
         default_player = [used_players[0]]
         if service_item.processor and service_item.processor != UiStrings().Automatic:
             # check to see if the player is usable else use the default one.
-            if not service_item.processor.lower() in used_players:
+            if service_item.processor.lower() not in used_players:
                 used_players = default_player
             else:
                 used_players = [service_item.processor.lower()]

openlp/core/ui/media/playertab.py

@@ -224,9 +224,11 @@ class PlayerTab(SettingsTab):
             self.settings_form.register_post_process('mediaitem_media_rebuild')
             self.settings_form.register_post_process('config_screen_changed')
 
-    def post_set_up(self):
+    def post_set_up(self, post_update=False):
         """
         Late setup for players as the MediaController has to be initialised first.
+
+        :param post_update: Indicates if called before or after updates.
         """
         for key, player in self.media_players.items():
             player = self.media_players[key]

openlp/core/ui/media/vlcplayer.py

@@ -112,7 +112,6 @@ def get_vlc():
 # This needs to happen on module load and not in get_vlc(), otherwise it can cause crashes on some DE on some setups
 # (reported on Gnome3, Unity, Cinnamon, all GTK+ based) when using native filedialogs...
 if is_linux() and 'nose' not in sys.argv[0] and get_vlc():
-    import ctypes
     try:
         try:
             x11 = ctypes.cdll.LoadLibrary('libX11.so.6')
@@ -233,7 +232,7 @@ class VlcPlayer(MediaPlayer):
         """
         vlc = get_vlc()
         start = datetime.now()
-        while not media_state == display.vlc_media.get_state():
+        while media_state != display.vlc_media.get_state():
             if display.vlc_media.get_state() == vlc.State.Error:
                 return False
             self.application.process_events()

openlp/core/ui/media/webkitplayer.py

@@ -22,10 +22,10 @@
 """
 The :mod:`~openlp.core.ui.media.webkit` module contains our WebKit video player
 """
-from PyQt5 import QtGui, QtWebKitWidgets
-
 import logging
 
+from PyQt5 import QtGui, QtWebKitWidgets
+
 from openlp.core.common import Settings
 from openlp.core.lib import translate
 from openlp.core.ui.media import MediaState

openlp/core/ui/projector/sourceselectform.py

@@ -141,23 +141,23 @@ def Build_Tab(group, source_key, default, projector, projectordb, edit=False):
     return widget, button_count, buttonchecked
 
 
-def set_button_tooltip(bar):
+def set_button_tooltip(button_bar):
     """
     Set the toolip for the standard buttons used
 
-    :param bar: QDialogButtonBar instance to update
+    :param button_bar: QDialogButtonBar instance to update
     """
-    for button in bar.buttons():
-        if bar.standardButton(button) == QDialogButtonBox.Cancel:
+    for button in button_bar.buttons():
+        if button_bar.standardButton(button) == QDialogButtonBox.Cancel:
             button.setToolTip(translate('OpenLP.SourceSelectForm',
                                         'Ignoring current changes and return to OpenLP'))
-        elif bar.standardButton(button) == QDialogButtonBox.Reset:
+        elif button_bar.standardButton(button) == QDialogButtonBox.Reset:
             button.setToolTip(translate('OpenLP.SourceSelectForm',
                                         'Delete all user-defined text and revert to PJLink default text'))
-        elif bar.standardButton(button) == QDialogButtonBox.Discard:
+        elif button_bar.standardButton(button) == QDialogButtonBox.Discard:
             button.setToolTip(translate('OpenLP.SourceSelectForm',
                                         'Discard changes and reset to previous user-defined text'))
-        elif bar.standardButton(button) == QDialogButtonBox.Ok:
+        elif button_bar.standardButton(button) == QDialogButtonBox.Ok:
             button.setToolTip(translate('OpenLP.SourceSelectForm',
                                         'Save changes and return to OpenLP'))
         else:

openlp/core/ui/projector/tab.py

@@ -133,7 +133,7 @@ class ProjectorTab(SettingsTab):
         settings.setValue('socket timeout', self.socket_timeout_spin_box.value())
         settings.setValue('poll time', self.socket_poll_spin_box.value())
         settings.setValue('source dialog type', self.dialog_type_combo_box.currentIndex())
-        settings.endGroup
+        settings.endGroup()
 
     def on_dialog_type_combo_box_changed(self):
         self.dialog_type = self.dialog_type_combo_box.currentIndex()

openlp/core/ui/servicemanager.py

@@ -480,9 +480,10 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa
         :return: service array
         """
         service = []
-        core = {'lite-service': self._save_lite,
-                'service-theme': self.service_theme
-                }
+        core = {
+            'lite-service': self._save_lite,
+            'service-theme': self.service_theme
+        }
         service.append({'openlp_core': core})
         return service
 
@@ -597,7 +598,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa
         if success:
             try:
                 shutil.copy(temp_file_name, path_file_name)
-            except shutil.Error:
+            except (shutil.Error, PermissionError):
                 return self.save_file_as()
             except OSError as ose:
                 QtWidgets.QMessageBox.critical(self, translate('OpenLP.ServiceManager', 'Error Saving File'),
@@ -658,7 +659,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa
         if success:
             try:
                 shutil.copy(temp_file_name, path_file_name)
-            except shutil.Error:
+            except (shutil.Error, PermissionError):
                 return self.save_file_as()
             self.main_window.add_recent_file(path_file_name)
             self.set_modified(False)
@@ -774,7 +775,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa
             else:
                 critical_error_message_box(message=translate('OpenLP.ServiceManager', 'File is not a valid service.'))
                 self.log_error('File contains no service data')
-        except (IOError, NameError, zipfile.BadZipfile):
+        except (IOError, NameError):
             self.log_exception('Problem loading service file {name}'.format(name=file_name))
             critical_error_message_box(message=translate('OpenLP.ServiceManager',
                                        'File could not be opened because it is corrupt.'))
@@ -1327,7 +1328,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa
         """
         The theme may have changed in the settings dialog so make sure the theme combo box is in the correct state.
         """
-        visible = not self.renderer.theme_level == ThemeLevel.Global
+        visible = self.renderer.theme_level != ThemeLevel.Global
         self.toolbar.actions['theme_combo_box'].setVisible(visible)
         self.toolbar.actions['theme_label'].setVisible(visible)
         self.regenerate_service_items()

openlp/core/ui/slidecontroller.py

@@ -37,7 +37,6 @@ from openlp.core.lib import ItemCapabilities, ServiceItem, ImageSource, ServiceI
     build_html
 from openlp.core.lib.ui import create_action
 from openlp.core.ui.lib.toolbar import OpenLPToolbar
-from openlp.core.ui.lib.dockwidget import OpenLPDockWidget
 from openlp.core.ui.lib.listpreviewwidget import ListPreviewWidget
 from openlp.core.ui import HideMode, MainDisplay, Display, DisplayControllerType
 

openlp/core/ui/themeform.py

@@ -249,7 +249,7 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties):
         NOTE the font_main_override is the inverse of the check box value
         """
         if self.update_theme_allowed:
-            self.theme.font_main_override = not (value == QtCore.Qt.Checked)
+            self.theme.font_main_override = (value != QtCore.Qt.Checked)
 
     def on_footer_position_check_box_state_changed(self, value):
         """
@@ -257,7 +257,7 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties):
         NOTE the font_footer_override is the inverse of the check box value
         """
         if self.update_theme_allowed:
-            self.theme.font_footer_override = not (value == QtCore.Qt.Checked)
+            self.theme.font_footer_override = (value != QtCore.Qt.Checked)
 
     def exec(self, edit=False):
         """

openlp/core/ui/thememanager.py

@@ -583,7 +583,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage
                     out_file.write(theme_zip.read(name))
                 out_file.close()
         except (IOError, zipfile.BadZipfile):
-            self.log_exception('Importing theme from zip failed {name|'.format(name=file_name))
+            self.log_exception('Importing theme from zip failed {name}'.format(name=file_name))
             raise ValidationError
         except ValidationError:
             critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'),

openlp/plugins/bibles/lib/manager.py

@@ -370,7 +370,7 @@ class BibleManager(RegistryProperties):
         """
         log.debug('save_meta data {bible}, {version}, {copyright}, {perms}'.format(bible=bible,
                                                                                    version=version,
-                                                                                   cr=copyright,
+                                                                                   copyright=copyright,
                                                                                    perms=permissions))
         self.db_cache[bible].save_meta('name', version)
         self.db_cache[bible].save_meta('copyright', copyright)

openlp/plugins/bibles/lib/opensong.py

@@ -117,7 +117,7 @@ class OpenSongBible(BibleDB):
                                 if len(verse_parts) > 1:
                                     number = int(verse_parts[0])
                             except TypeError:
-                                log.warning('Illegal verse number: {verse:d}'.format(verse.attrib['n']))
+                                log.warning('Illegal verse number: {verse:d}'.format(verse=verse.attrib['n']))
                             verse_number = number
                         else:
                             verse_number += 1

openlp/plugins/bibles/lib/osis.py

@@ -126,8 +126,6 @@ class OSISBible(BibleDB):
                 # Remove div-tags in the book
                 etree.strip_tags(book, ('{http://www.bibletechnologies.net/2003/OSIS/namespace}div'))
                 book_ref_id = self.get_book_ref_id_by_name(book.get('osisID'), num_books, language_id)
-                if not book_ref_id:
-                    book_ref_id = self.get_book_ref_id_by_localised_name(book.get('osisID'))
                 if not book_ref_id:
                     log.error('Importing books from "{name}" failed'.format(name=self.filename))
                     return False

openlp/plugins/bibles/lib/zefania.py

@@ -86,8 +86,6 @@ class ZefaniaBible(BibleDB):
                     continue
                 if bname:
                     book_ref_id = self.get_book_ref_id_by_name(bname, num_books, language_id)
-                    if not book_ref_id:
-                        book_ref_id = self.get_book_ref_id_by_localised_name(bname)
                 else:
                     log.debug('Could not find a name, will use number, basically a guess.')
                     book_ref_id = int(bnumber)

openlp/plugins/presentations/lib/messagelistener.py

@@ -28,7 +28,7 @@ from PyQt5 import QtCore
 
 from openlp.core.common import Registry
 from openlp.core.ui import HideMode
-from openlp.core.lib import ServiceItemContext, ServiceItem
+from openlp.core.lib import ServiceItemContext
 from openlp.plugins.presentations.lib.pdfcontroller import PDF_CONTROLLER_FILETYPES
 
 log = logging.getLogger(__name__)

openlp/plugins/presentations/lib/pdfcontroller.py

@@ -80,7 +80,7 @@ class PdfController(PresentationController):
             found_mutool = re.search('usage: mutool.*', decoded_line, re.IGNORECASE)
             if found_mutool:
                 # Test that mutool contains mudraw
-                if re.search('draw\s+--\s+convert document.*', runlog.decode(), re.IGNORECASE | re.MULTILINE):
+                if re.search(r'draw\s+--\s+convert document.*', runlog.decode(), re.IGNORECASE | re.MULTILINE):
                     program_type = 'mutool'
                     break
             found_gs = re.search('GPL Ghostscript.*', decoded_line, re.IGNORECASE)
@@ -215,8 +215,8 @@ class PdfDocument(PresentationDocument):
         height = 0.0
         for line in runlog.splitlines():
             try:
-                width = float(re.search('.*Size: x: (\d+\.?\d*), y: \d+.*', line.decode()).group(1))
-                height = float(re.search('.*Size: x: \d+\.?\d*, y: (\d+\.?\d*).*', line.decode()).group(1))
+                width = float(re.search(r'.*Size: x: (\d+\.?\d*), y: \d+.*', line.decode()).group(1))
+                height = float(re.search(r'.*Size: x: \d+\.?\d*, y: (\d+\.?\d*).*', line.decode()).group(1))
                 break
             except AttributeError:
                 continue

openlp/plugins/presentations/lib/powerpointcontroller.py

@@ -362,7 +362,7 @@ class PowerpointDocument(PresentationDocument):
         # it is the powerpoint presentation window.
         (left, top, right, bottom) = win32gui.GetWindowRect(hwnd)
         window_title = win32gui.GetWindowText(hwnd)
-        log.debug('window size:  left=left:d}, top={top:d}, '
+        log.debug('window size:  left={left:d}, top={top:d}, '
                   'right={right:d}, bottom={bottom:d}'.format(left=left, top=top, right=right, bottom=bottom))
         log.debug('compare size:  {y:d} and {top:d}, {height:d} and {vertical:d}, '
                   '{x:d} and {left}, {width:d} and {horizontal:d}'.format(y=size.y(),

openlp/plugins/presentations/lib/pptviewcontroller.py

@@ -181,13 +181,13 @@ class PptviewDocument(PresentationDocument):
                     index = -1
                     list_to_add = None
                     # check if it is a slide
-                    match = re.search("slides/slide(.+)\.xml", zip_info.filename)
+                    match = re.search(r'slides/slide(.+)\.xml', zip_info.filename)
                     if match:
                         index = int(match.group(1)) - 1
                         node_type = 'ctrTitle'
                         list_to_add = titles
                     # or a note
-                    match = re.search("notesSlides/notesSlide(.+)\.xml", zip_info.filename)
+                    match = re.search(r'notesSlides/notesSlide(.+)\.xml', zip_info.filename)
                     if match:
                         index = int(match.group(1)) - 1
                         node_type = 'body'

openlp/plugins/presentations/presentationplugin.py

@@ -124,7 +124,7 @@ class PresentationPlugin(Plugin):
         log.debug('check_pre_conditions')
         controller_dir = os.path.join(AppLocation.get_directory(AppLocation.PluginsDir), 'presentations', 'lib')
         for filename in os.listdir(controller_dir):
-            if filename.endswith('controller.py') and not filename == 'presentationcontroller.py':
+            if filename.endswith('controller.py') and filename != 'presentationcontroller.py':
                 path = os.path.join(controller_dir, filename)
                 if os.path.isfile(path):
                     module_name = 'openlp.plugins.presentations.lib.' + os.path.splitext(filename)[0]

openlp/plugins/songs/lib/importers/cclifile.py

@@ -255,6 +255,7 @@ class CCLIFileImport(SongImport):
         song_author = ''
         verse_start = False
         for line in text_list:
+            verse_type = 'v'
             clean_line = line.strip()
             if not clean_line:
                 if line_number == 0:

openlp/plugins/songs/lib/importers/dreambeam.py

@@ -124,8 +124,8 @@ class DreamBeamImport(SongImport):
                     if hasattr(song_xml, 'Sequence'):
                         for lyrics_sequence_item in (song_xml.Sequence.iterchildren()):
                             item = lyrics_sequence_item.get('Type')[:1]
-                            self.verse_order_list.append("{item}{number}".format(item=item),
-                                                         lyrics_sequence_item.get('Number'))
+                            number = lyrics_sequence_item.get('Number')
+                            self.verse_order_list.append("{item}{number}".format(item=item, number=number))
                     if hasattr(song_xml, 'Notes'):
                         self.comments = str(song_xml.Notes.text)
                 else:

openlp/plugins/songs/lib/importers/easyworship.py

@@ -27,6 +27,7 @@ import os
 import struct
 import re
 import zlib
+import logging
 
 from openlp.core.lib import translate
 from openlp.plugins.songs.lib import VerseType
@@ -38,6 +39,8 @@ SLIDE_BREAK_REGEX = re.compile(r'\n *?\n[\n ]*')
 NUMBER_REGEX = re.compile(r'[0-9]+')
 NOTE_REGEX = re.compile(r'\(.*?\)')
 
+log = logging.getLogger(__name__)
+
 
 class FieldDescEntry:
     def __init__(self, name, field_type, size):

openlp/plugins/songs/lib/importers/mediashout.py

@@ -24,15 +24,17 @@ The :mod:`mediashout` module provides the functionality for importing
 a MediaShout database into the OpenLP database.
 """
 
-# WARNING: See https://docs.python.org/2/library/sqlite3.html for value substitution
+# WARNING: See https://docs.python.org/3/library/sqlite3.html for value substitution
 #          in SQL statements
 
 import pyodbc
+import logging
 
 from openlp.core.lib import translate
 from openlp.plugins.songs.lib.importers.songimport import SongImport
 
 VERSE_TAGS = ['V', 'C', 'B', 'O', 'P', 'I', 'E']
+log = logging.getLogger(__name__)
 
 
 class MediaShoutImport(SongImport):
@@ -44,17 +46,18 @@ class MediaShoutImport(SongImport):
         """
         Initialise the MediaShout importer.
         """
-        SongImport.__init__(self, manager, **kwargs)
+        super(MediaShoutImport, self).__init__(manager, **kwargs)
 
     def do_import(self):
         """
         Receive a single file to import.
         """
         try:
-            conn = pyodbc.connect('DRIVER={Microsoft Access Driver (*.mdb)};DBQ={source};'
-                                  'PWD=6NOZ4eHK7k'.format(sorce=self.import_source))
-        except:
+            conn = pyodbc.connect('DRIVER={{Microsoft Access Driver (*.mdb)}};DBQ={source};'
+                                  'PWD=6NOZ4eHK7k'.format(source=self.import_source))
+        except Exception as e:
             # Unfortunately no specific exception type
+            log.exception(e)
             self.log_error(self.import_source, translate('SongsPlugin.MediaShoutImport',
                                                          'Unable to open the MediaShout database.'))
             return
@@ -63,17 +66,21 @@ class MediaShoutImport(SongImport):
         songs = cursor.fetchall()
         self.import_wizard.progress_bar.setMaximum(len(songs))
         for song in songs:
+            topics = []
             if self.stop_import_flag:
                 break
-            cursor.execute('SELECT Type, Number, Text FROM Verses WHERE Record = ? ORDER BY Type, Number', song.Record)
+            cursor.execute('SELECT Type, Number, Text FROM Verses WHERE Record = ? ORDER BY Type, Number',
+                           float(song.Record))
             verses = cursor.fetchall()
-            cursor.execute('SELECT Type, Number, POrder FROM PlayOrder WHERE Record = ? ORDER BY POrder', song.Record)
+            cursor.execute('SELECT Type, Number, POrder FROM PlayOrder WHERE Record = ? ORDER BY POrder',
+                           float(song.Record))
             verse_order = cursor.fetchall()
-            cursor.execute('SELECT Name FROM Themes INNER JOIN SongThemes ON SongThemes.ThemeId = Themes.ThemeId '
-                           'WHERE SongThemes.Record = ?', song.Record)
-            topics = cursor.fetchall()
+            if cursor.tables(table='TableName', tableType='TABLE').fetchone():
+                cursor.execute('SELECT Name FROM Themes INNER JOIN SongThemes ON SongThemes.ThemeId = Themes.ThemeId '
+                               'WHERE SongThemes.Record = ?', float(song.Record))
+                topics = cursor.fetchall()
             cursor.execute('SELECT Name FROM Groups INNER JOIN SongGroups ON SongGroups.GroupId = Groups.GroupId '
-                           'WHERE SongGroups.Record = ?', song.Record)
+                           'WHERE SongGroups.Record = ?', float(song.Record))
             topics += cursor.fetchall()
             self.process_song(song, verses, verse_order, topics)
 

openlp/plugins/songs/lib/importers/openlyrics.py

@@ -67,7 +67,7 @@ class OpenLyricsImport(SongImport):
                 xml = etree.tostring(parsed_file).decode()
                 self.open_lyrics.xml_to_song(xml)
             except etree.XMLSyntaxError:
-                log.exception('XML syntax error in file {path}'.format(file_path))
+                log.exception('XML syntax error in file {path}'.format(path=file_path))
                 self.log_error(file_path, SongStrings.XMLSyntaxError)
             except OpenLyricsError as exception:
                 log.exception('OpenLyricsException {error:d} in file {name}: {text}'.format(error=exception.type,

openlp/plugins/songs/lib/importers/opspro.py

@@ -55,7 +55,7 @@ class OPSProImport(SongImport):
         """
         password = self.extract_mdb_password()
         try:
-            conn = pyodbc.connect('DRIVER={Microsoft Access Driver (*.mdb)};DBQ={source};'
+            conn = pyodbc.connect('DRIVER={{Microsoft Access Driver (*.mdb)}};DBQ={source};'
                                   'PWD={password}'.format(source=self.import_source, password=password))
         except (pyodbc.DatabaseError, pyodbc.IntegrityError, pyodbc.InternalError, pyodbc.OperationalError) as e:
             log.warning('Unable to connect the OPS Pro database {source}. {error}'.format(source=self.import_source,
@@ -74,11 +74,11 @@ class OPSProImport(SongImport):
                 break
             # Type means: 0=Original, 1=Projection, 2=Own
             cursor.execute('SELECT Lyrics, Type, IsDualLanguage FROM Lyrics WHERE SongID = ? AND Type < 2 '
-                           'ORDER BY Type DESC', song.ID)
+                           'ORDER BY Type DESC', float(song.ID))
             lyrics = cursor.fetchone()
             cursor.execute('SELECT CategoryName FROM Category INNER JOIN SongCategory '
                            'ON Category.ID = SongCategory.CategoryID WHERE SongCategory.SongID = ? '
-                           'ORDER BY CategoryName', song.ID)
+                           'ORDER BY CategoryName', float(song.ID))
             topics = cursor.fetchall()
             try:
                 self.process_song(song, lyrics, topics)

openlp/plugins/songs/lib/importers/presentationmanager.py

@@ -30,6 +30,7 @@ import re
 import chardet
 from lxml import objectify, etree
 
+from openlp.core.common import translate
 from openlp.core.ui.lib.wizard import WizardStrings
 from .songimport import SongImport
 
@@ -56,7 +57,13 @@ class PresentationManagerImport(SongImport):
                 # Open file with detected encoding and remove encoding declaration
                 text = open(file_path, mode='r', encoding=encoding).read()
                 text = re.sub('.+\?>\n', '', text)
-                tree = etree.fromstring(text, parser=etree.XMLParser(recover=True))
+                try:
+                    tree = etree.fromstring(text, parser=etree.XMLParser(recover=True))
+                except ValueError:
+                    self.log_error(file_path,
+                                   translate('SongsPlugin.PresentationManagerImport',
+                                             'File is not in XML-format, which is the only format supported.'))
+                    continue
             root = objectify.fromstring(etree.tostring(tree))
             self.process_song(root)
 

openlp/plugins/songs/lib/importers/songpro.py

@@ -72,7 +72,7 @@ class SongProImport(SongImport):
         Receive a single file or a list of files to import.
         """
         self.encoding = None
-        with open(self.import_source, 'rt') as songs_file:
+        with open(self.import_source, 'rt', errors='ignore') as songs_file:
             self.import_wizard.progress_bar.setMaximum(0)
             tag = ''
             text = ''

openlp/plugins/songs/lib/importers/songshowplus.py

@@ -106,6 +106,7 @@ class SongShowPlusImport(SongImport):
             song_data = open(file, 'rb')
             while True:
                 block_key, = struct.unpack("I", song_data.read(4))
+                log.debug('block_key: %d' % block_key)
                 # The file ends with 4 NULL's
                 if block_key == 0:
                     break
@@ -117,7 +118,13 @@ class SongShowPlusImport(SongImport):
                     null, verse_name_length, = struct.unpack("BB", song_data.read(2))
                     verse_name = self.decode(song_data.read(verse_name_length))
                 length_descriptor_size, = struct.unpack("B", song_data.read(1))
-                log.debug(length_descriptor_size)
+                log.debug('length_descriptor_size: %d' % length_descriptor_size)
+                # In the case of song_numbers the number is in the data from the
+                # current position to the next block starts
+                if block_key == SONG_NUMBER:
+                    sn_bytes = song_data.read(length_descriptor_size - 1)
+                    self.song_number = int.from_bytes(sn_bytes, byteorder='little')
+                    continue
                 # Detect if/how long the length descriptor is
                 if length_descriptor_size == 12 or length_descriptor_size == 20:
                     length_descriptor, = struct.unpack("I", song_data.read(4))
@@ -127,8 +134,9 @@ class SongShowPlusImport(SongImport):
                     length_descriptor = 0
                 else:
                     length_descriptor, = struct.unpack("B", song_data.read(1))
-                log.debug(length_descriptor_size)
+                log.debug('length_descriptor: %d' % length_descriptor)
                 data = song_data.read(length_descriptor)
+                log.debug(data)
                 if block_key == TITLE:
                     self.title = self.decode(data)
                 elif block_key == AUTHOR:
@@ -168,8 +176,6 @@ class SongShowPlusImport(SongImport):
                         self.ssp_verse_order_list.append(verse_tag)
                 elif block_key == SONG_BOOK:
                     self.song_book_name = self.decode(data)
-                elif block_key == SONG_NUMBER:
-                    self.song_number = ord(data)
                 elif block_key == CUSTOM_VERSE:
                     verse_tag = self.to_openlp_verse_tag(verse_name)
                     self.add_verse(self.decode(data), verse_tag)

openlp/plugins/songs/lib/importers/videopsalm.py

@@ -122,6 +122,4 @@ class VideoPsalmImport(SongImport):
                 if not self.finish():
                     self.log_error('Could not import {title}'.format(title=self.title))
         except Exception as e:
-            self.log_error(translate('SongsPlugin.VideoPsalmImport', 'File {name}').format(name=file.name),
-                           translate('SongsPlugin.VideoPsalmImport', 'Error: {error}').format(error=e))
-        song_file.close()
+            self.log_error(song_file.name, translate('SongsPlugin.VideoPsalmImport', 'Error: {error}').format(error=e))

openlp/plugins/songs/lib/importers/worshipcenterpro.py

@@ -49,7 +49,7 @@ class WorshipCenterProImport(SongImport):
         Receive a single file to import.
         """
         try:
-            conn = pyodbc.connect('DRIVER={Microsoft Access Driver (*.mdb)};'
+            conn = pyodbc.connect('DRIVER={{Microsoft Access Driver (*.mdb)}};'
                                   'DBQ={source}'.format(source=self.import_source))
         except (pyodbc.DatabaseError, pyodbc.IntegrityError, pyodbc.InternalError, pyodbc.OperationalError) as e:
             log.warning('Unable to connect the WorshipCenter Pro '

pylintrc

@@ -0,0 +1,379 @@
+[MASTER]
+
+# Specify a configuration file.
+#rcfile=
+
+# Python code to execute, usually for sys.path manipulation such as
+# pygtk.require().
+#init-hook=
+
+# Add files or directories to the blacklist. They should be base names, not
+# paths.
+ignore=vlc.py
+
+# Pickle collected data for later comparisons.
+persistent=yes
+
+# List of plugins (as comma separated values of python modules names) to load,
+# usually to register additional checkers.
+load-plugins=
+
+# Use multiple processes to speed up Pylint.
+#jobs=4
+
+# Allow loading of arbitrary C extensions. Extensions are imported into the
+# active Python interpreter and may run arbitrary code.
+unsafe-load-any-extension=no
+
+# A comma-separated list of package or module names from where C extensions may
+# be loaded. Extensions are loading into the active Python interpreter and may
+# run arbitrary code
+extension-pkg-whitelist=
+
+# Allow optimization of some AST trees. This will activate a peephole AST
+# optimizer, which will apply various small optimizations. For instance, it can
+# be used to obtain the result of joining multiple strings with the addition
+# operator. Joining a lot of strings can lead to a maximum recursion error in
+# Pylint and this flag can prevent that. It has one side effect, the resulting
+# AST will be different than the one from reality.
+optimize-ast=no
+
+
+[MESSAGES CONTROL]
+
+# Only show warnings with the listed confidence levels. Leave empty to show
+# all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED
+confidence=
+
+# Enable the message, report, category or checker with the given id(s). You can
+# either give multiple identifier separated by comma (,) or put this option
+# multiple time. See also the "--disable" option for examples.
+#enable=
+
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifiers separated by comma (,) or put this
+# option multiple times (only on the command line, not in the configuration
+# file where it should appear only once).You can also use "--disable=all" to
+# disable everything first and then reenable specific checks. For example, if
+# you want to run only the similarities checker, you can use "--disable=all
+# --enable=similarities". If you want to run only the classes checker, but have
+# no Warning level messages displayed, use"--disable=all --enable=classes
+# --disable=W"
+disable=reload-builtin,too-many-locals,no-name-in-module,hex-method,bad-builtin,old-raise-syntax,bad-continuation,reduce-builtin,unicode-builtin,unused-wildcard-import,apply-builtin,no-member,filter-builtin-not-iterating,execfile-builtin,import-star-module-level,input-builtin,duplicate-code,old-division,print-statement,cmp-method,fixme,no-absolute-import,cmp-builtin,no-self-use,too-many-nested-blocks,standarderror-builtin,long-builtin,raising-string,delslice-method,metaclass-assignment,coerce-builtin,old-octal-literal,basestring-builtin,xrange-builtin,line-too-long,suppressed-message,unused-variable,round-builtin,raw_input-builtin,too-many-instance-attributes,unused-argument,next-method-called,oct-method,attribute-defined-outside-init,too-many-public-methods,too-many-statements,logging-format-interpolation,map-builtin-not-iterating,buffer-builtin,parameter-unpacking,range-builtin-not-iterating,intern-builtin,wrong-import-position,coerce-method,useless-suppression,using-cmp-argument,dict-view-method,backtick,old-ne-operator,missing-docstring,setslice-method,access-member-before-definition,long-suffix,too-few-public-methods,file-builtin,zip-builtin-not-iterating,invalid-name,getslice-method,unpacking-in-except,too-many-arguments,dict-iter-method,unichr-builtin,too-many-lines,too-many-branches,wildcard-import,indexing-exception,nonzero-method
+
+
+[REPORTS]
+
+# Set the output format. Available formats are text, parseable, colorized, msvs
+# (visual studio) and html. You can also give a reporter class, eg
+# mypackage.mymodule.MyReporterClass.
+output-format=text
+
+# Put messages in a separate file for each module / package specified on the
+# command line instead of printing them on stdout. Reports (if any) will be
+# written in a file name "pylint_global.[txt|html]".
+files-output=no
+
+# Tells whether to display a full report or only the messages
+reports=no
+
+# Python expression which should return a note less than 10 (10 is the highest
+# note). You have access to the variables errors warning, statement which
+# respectively contain the number of errors / warnings messages and the total
+# number of statements analyzed. This is used by the global evaluation report
+# (RP0004).
+evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10)
+
+# Template used to display messages. This is a python new-style format string
+# used to format the message information. See doc for all details
+#msg-template=
+
+
+[SPELLING]
+
+# Spelling dictionary name. Available dictionaries: en_ZA (myspell), en_US
+# (myspell), en_GB (myspell), en_AU (myspell), da_DK (myspell), en (aspell),
+# en_CA (aspell).
+spelling-dict=
+
+# List of comma separated words that should not be checked.
+spelling-ignore-words=
+
+# A path to a file that contains private dictionary; one word per line.
+spelling-private-dict-file=
+
+# Tells whether to store unknown words to indicated private dictionary in
+# --spelling-private-dict-file option instead of raising a message.
+spelling-store-unknown-words=no
+
+
+[SIMILARITIES]
+
+# Minimum lines number of a similarity.
+min-similarity-lines=4
+
+# Ignore comments when computing similarities.
+ignore-comments=yes
+
+# Ignore docstrings when computing similarities.
+ignore-docstrings=yes
+
+# Ignore imports when computing similarities.
+ignore-imports=no
+
+
+[LOGGING]
+
+# Logging modules to check that the string format arguments are in logging
+# function parameter format
+logging-modules=logging
+
+
+[BASIC]
+
+# List of builtins function names that should not be used, separated by a comma
+bad-functions=map,filter
+
+# Good variable names which should always be accepted, separated by a comma
+good-names=i,j,k,ex,Run,_
+
+# Bad variable names which should always be refused, separated by a comma
+bad-names=foo,bar,baz,toto,tutu,tata
+
+# Colon-delimited sets of names that determine each other's naming style when
+# the name regexes allow several styles.
+name-group=
+
+# Include a hint for the correct naming format with invalid-name
+include-naming-hint=no
+
+# Regular expression matching correct class attribute names
+class-attribute-rgx=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$
+
+# Naming hint for class attribute names
+class-attribute-name-hint=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$
+
+# Regular expression matching correct attribute names
+attr-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Naming hint for attribute names
+attr-name-hint=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression matching correct variable names
+variable-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Naming hint for variable names
+variable-name-hint=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression matching correct argument names
+argument-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Naming hint for argument names
+argument-name-hint=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression matching correct function names
+function-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Naming hint for function names
+function-name-hint=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression matching correct method names
+method-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Naming hint for method names
+method-name-hint=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression matching correct class names
+class-rgx=[A-Z_][a-zA-Z0-9]+$
+
+# Naming hint for class names
+class-name-hint=[A-Z_][a-zA-Z0-9]+$
+
+# Regular expression matching correct module names
+module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$
+
+# Naming hint for module names
+module-name-hint=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$
+
+# Regular expression matching correct inline iteration names
+inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$
+
+# Naming hint for inline iteration names
+inlinevar-name-hint=[A-Za-z_][A-Za-z0-9_]*$
+
+# Regular expression matching correct constant names
+const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$
+
+# Naming hint for constant names
+const-name-hint=(([A-Z_][A-Z0-9_]*)|(__.*__))$
+
+# Regular expression which should only match function or class names that do
+# not require a docstring.
+no-docstring-rgx=^_
+
+# Minimum line length for functions/classes that require docstrings, shorter
+# ones are exempt.
+docstring-min-length=-1
+
+
+[ELIF]
+
+# Maximum number of nested blocks for function / method body
+max-nested-blocks=5
+
+
+[FORMAT]
+
+# Maximum number of characters on a single line.
+max-line-length=100
+
+# Regexp for a line that is allowed to be longer than the limit.
+ignore-long-lines=^\s*(# )?<?https?://\S+>?$
+
+# Allow the body of an if to be on the same line as the test if there is no
+# else.
+single-line-if-stmt=no
+
+# List of optional constructs for which whitespace checking is disabled. `dict-
+# separator` is used to allow tabulation in dicts, etc.: {1  : 1,\n222: 2}.
+# `trailing-comma` allows a space between comma and closing bracket: (a, ).
+# `empty-line` allows space-only lines.
+no-space-check=trailing-comma,dict-separator
+
+# Maximum number of lines in a module
+max-module-lines=1000
+
+# String used as indentation unit. This is usually "    " (4 spaces) or "\t" (1
+# tab).
+indent-string='    '
+
+# Number of spaces of indent required inside a hanging  or continued line.
+indent-after-paren=4
+
+# Expected format of line ending, e.g. empty (any line ending), LF or CRLF.
+expected-line-ending-format=
+
+
+[MISCELLANEOUS]
+
+# List of note tags to take in consideration, separated by a comma.
+notes=FIXME,XXX,TODO
+
+
+[VARIABLES]
+
+# Tells whether we should check for unused import in __init__ files.
+init-import=no
+
+# A regular expression matching the name of dummy variables (i.e. expectedly
+# not used).
+dummy-variables-rgx=_$|dummy
+
+# List of additional names supposed to be defined in builtins. Remember that
+# you should avoid to define new builtins when possible.
+additional-builtins=
+
+# List of strings which can identify a callback function by name. A callback
+# name must start or end with one of those strings.
+callbacks=cb_,_cb
+
+
+[TYPECHECK]
+
+# Tells whether missing members accessed in mixin class should be ignored. A
+# mixin class is detected if its name ends with "mixin" (case insensitive).
+ignore-mixin-members=yes
+
+# List of module names for which member attributes should not be checked
+# (useful for modules/projects where namespaces are manipulated during runtime
+# and thus existing member attributes cannot be deduced by static analysis. It
+# supports qualified module names, as well as Unix pattern matching.
+ignored-modules=
+
+# List of classes names for which member attributes should not be checked
+# (useful for classes with attributes dynamically set). This supports can work
+# with qualified names.
+ignored-classes=
+
+# List of members which are set dynamically and missed by pylint inference
+# system, and so shouldn't trigger E1101 when accessed. Python regular
+# expressions are accepted.
+generated-members=
+
+
+[DESIGN]
+
+# Maximum number of arguments for function / method
+max-args=5
+
+# Argument names that match this expression will be ignored. Default to name
+# with leading underscore
+ignored-argument-names=_.*
+
+# Maximum number of locals for function / method body
+max-locals=15
+
+# Maximum number of return / yield for function / method body
+max-returns=6
+
+# Maximum number of branch for function / method body
+max-branches=12
+
+# Maximum number of statements in function / method body
+max-statements=50
+
+# Maximum number of parents for a class (see R0901).
+max-parents=7
+
+# Maximum number of attributes for a class (see R0902).
+max-attributes=7
+
+# Minimum number of public methods for a class (see R0903).
+min-public-methods=2
+
+# Maximum number of public methods for a class (see R0904).
+max-public-methods=20
+
+# Maximum number of boolean expressions in a if statement
+max-bool-expr=5
+
+
+[IMPORTS]
+
+# Deprecated modules which should not be used, separated by a comma
+deprecated-modules=optparse
+
+# Create a graph of every (i.e. internal and external) dependencies in the
+# given file (report RP0402 must not be disabled)
+import-graph=
+
+# Create a graph of external dependencies in the given file (report RP0402 must
+# not be disabled)
+ext-import-graph=
+
+# Create a graph of internal dependencies in the given file (report RP0402 must
+# not be disabled)
+int-import-graph=
+
+
+[CLASSES]
+
+# List of method names used to declare (i.e. assign) instance attributes.
+defining-attr-methods=__init__,__new__,setUp
+
+# List of valid names for the first argument in a class method.
+valid-classmethod-first-arg=cls
+
+# List of valid names for the first argument in a metaclass class method.
+valid-metaclass-classmethod-first-arg=mcs
+
+# List of member names, which should be excluded from the protected access
+# warning.
+exclude-protected=_asdict,_fields,_replace,_source,_make
+
+
+[EXCEPTIONS]
+
+# Exceptions that will emit a warning when being caught. Defaults to
+# "Exception"
+overgeneral-exceptions=Exception

tests/functional/openlp_core_common/test_projector_utilities.py

@@ -124,7 +124,7 @@ class testProjectorUtilities(TestCase):
         Test MD5 hash from salt+data pass (python)
         """
         # WHEN: Given a known salt+data
-        hash_ = md5_hash(salt=salt.encode('ascii'), data=pin.encode('ascii'))
+        hash_ = md5_hash(salt=salt.encode('utf-8'), data=pin.encode('utf-8'))
 
         # THEN: Validate return has is same
         self.assertEquals(hash_, test_hash, 'MD5 should have returned a good hash')
@@ -134,7 +134,7 @@ class testProjectorUtilities(TestCase):
         Test MD5 hash from salt+data fail (python)
         """
         # WHEN: Given a different salt+hash
-        hash_ = md5_hash(salt=pin.encode('ascii'), data=salt.encode('ascii'))
+        hash_ = md5_hash(salt=pin.encode('utf-8'), data=salt.encode('utf-8'))
 
         # THEN: return data is different
         self.assertNotEquals(hash_, test_hash, 'MD5 should have returned a bad hash')
@@ -144,20 +144,20 @@ class testProjectorUtilities(TestCase):
         Test MD5 hash from salt+data pass (Qt)
         """
         # WHEN: Given a known salt+data
-        hash_ = qmd5_hash(salt=salt.encode('ascii'), data=pin.encode('ascii'))
+        hash_ = qmd5_hash(salt=salt.encode('utf-8'), data=pin.encode('utf-8'))
 
         # THEN: Validate return has is same
-        self.assertEquals(hash_.decode('ascii'), test_hash, 'Qt-MD5 should have returned a good hash')
+        self.assertEquals(hash_, test_hash, 'Qt-MD5 should have returned a good hash')
 
     def test_qmd5_hash_bad(self):
         """
         Test MD5 hash from salt+hash fail (Qt)
         """
         # WHEN: Given a different salt+hash
-        hash_ = qmd5_hash(salt=pin.encode('ascii'), data=salt.encode('ascii'))
+        hash_ = qmd5_hash(salt=pin.encode('utf-8'), data=salt.encode('utf-8'))
 
         # THEN: return data is different
-        self.assertNotEquals(hash_.decode('ascii'), test_hash, 'Qt-MD5 should have returned a bad hash')
+        self.assertNotEquals(hash_, test_hash, 'Qt-MD5 should have returned a bad hash')
 
     def test_md5_non_ascii_string(self):
         """
@@ -174,7 +174,7 @@ class testProjectorUtilities(TestCase):
         Test MD5 hash with non-ascii string - bug 1417809
         """
         # WHEN: Non-ascii string is hashed
-        hash_ = md5_hash(salt=test_non_ascii_string.encode('utf-8'), data=None)
+        hash_ = md5_hash(data=test_non_ascii_string.encode('utf-8'))
 
         # THEN: Valid MD5 hash should be returned
         self.assertEqual(hash_, test_non_ascii_hash, 'Qt-MD5 should have returned a valid hash')

tests/functional/openlp_core_lib/test_projector_pjlink1.py

@@ -30,7 +30,7 @@ from openlp.core.lib.projector.constants import E_PARAMETER, ERROR_STRING, S_OFF
     S_COOLDOWN, PJLINK_POWR_STATUS
 
 from tests.functional import patch
-from tests.resources.projector.data import TEST_PIN, TEST_SALT, TEST_CONNECT_AUTHENTICATE
+from tests.resources.projector.data import TEST_PIN, TEST_SALT, TEST_CONNECT_AUTHENTICATE, TEST_HASH
 
 pjlink_test = PJLink1(name='test', ip='127.0.0.1', pin=TEST_PIN, no_poll=True)
 
@@ -332,3 +332,53 @@ class TestPJLink(TestCase):
         self.assertFalse(pjlink.send_busy, 'Projector send_busy should be False')
         self.assertTrue(mock_timer.called, 'Projector timer.stop()  should have been called')
         self.assertTrue(mock_socket_timer.called, 'Projector socket_timer.stop() should have been called')
+
+    @patch.object(pjlink_test, 'send_command')
+    @patch.object(pjlink_test, 'waitForReadyRead')
+    @patch.object(pjlink_test, 'projectorAuthentication')
+    @patch.object(pjlink_test, 'timer')
+    @patch.object(pjlink_test, 'socket_timer')
+    def test_bug_1593882_no_pin_authenticated_connection(self, mock_socket_timer,
+                                                         mock_timer,
+                                                         mock_authentication,
+                                                         mock_ready_read,
+                                                         mock_send_command):
+        """
+        Test bug 1593882 no pin and authenticated request exception
+        """
+        # GIVEN: Test object and mocks
+        pjlink = pjlink_test
+        pjlink.pin = None
+        mock_ready_read.return_value = True
+
+        # WHEN: call with authentication request and pin not set
+        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
+
+        # THEN: No Authentication signal should have been sent
+        mock_authentication.emit.assert_called_with(pjlink.name)
+
+    @patch.object(pjlink_test, 'waitForReadyRead')
+    @patch.object(pjlink_test, 'state')
+    @patch.object(pjlink_test, '_send_command')
+    @patch.object(pjlink_test, 'timer')
+    @patch.object(pjlink_test, 'socket_timer')
+    def test_bug_1593883_pjlink_authentication(self, mock_socket_timer,
+                                               mock_timer,
+                                               mock_send_command,
+                                               mock_state,
+                                               mock_waitForReadyRead):
+        """
+        Test bugfix 1593883 pjlink authentication
+        """
+        # GIVEN: Test object and data
+        pjlink = pjlink_test
+        pjlink.pin = TEST_PIN
+        mock_state.return_value = pjlink.ConnectedState
+        mock_waitForReadyRead.return_value = True
+
+        # WHEN: Athenticated connection is called
+        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
+
+        # THEN: send_command should have the proper authentication
+        self.assertEquals("{test}".format(test=mock_send_command.call_args),
+                          "call(data='{hash}%1CLSS ?\\r')".format(hash=TEST_HASH))

tests/functional/openlp_core_lib/test_serviceitem.py

@@ -112,7 +112,6 @@ class TestServiceItem(TestCase):
         # WHEN: adding an image from a saved Service and mocked exists
         line = convert_file_service_item(TEST_PATH, 'serviceitem_image_1.osj')
         with patch('openlp.core.ui.servicemanager.os.path.exists') as mocked_exists,\
-                patch('openlp.core.lib.serviceitem.create_thumb') as mocked_create_thumb,\
                 patch('openlp.core.lib.serviceitem.AppLocation.get_section_data_path') as \
                 mocked_get_section_data_path:
             mocked_exists.return_value = True
@@ -164,7 +163,6 @@ class TestServiceItem(TestCase):
         line2 = convert_file_service_item(TEST_PATH, 'serviceitem_image_2.osj', 1)
 
         with patch('openlp.core.ui.servicemanager.os.path.exists') as mocked_exists, \
-                patch('openlp.core.lib.serviceitem.create_thumb') as mocked_create_thumb, \
                 patch('openlp.core.lib.serviceitem.AppLocation.get_section_data_path') as \
                 mocked_get_section_data_path:
             mocked_exists.return_value = True

tests/functional/openlp_core_lib/test_theme.py

@@ -22,43 +22,82 @@
 """
 Package to test the openlp.core.lib.theme package.
 """
-from tests.functional import MagicMock, patch
 from unittest import TestCase
+import os
 
 from openlp.core.lib.theme import ThemeXML
 
 
-class TestTheme(TestCase):
+class TestThemeXML(TestCase):
     """
-    Test the functions in the Theme module
+    Test the ThemeXML class
     """
-    def setUp(self):
-        """
-        Create the UI
-        """
-        pass
-
-    def tearDown(self):
-        """
-        Delete all the C++ objects at the end so that we don't have a segfault
-        """
-        pass
-
     def test_new_theme(self):
         """
-        Test the theme creation - basic test
+        Test the ThemeXML constructor
         """
-        # GIVEN: A new theme
-
-        # WHEN: A theme is created
+        # GIVEN: The ThemeXML class
+        # WHEN: A theme object is created
         default_theme = ThemeXML()
 
-        # THEN: We should get some default behaviours
-        self.assertTrue(default_theme.background_border_color == '#000000', 'The theme should have a black border')
-        self.assertTrue(default_theme.background_type == 'solid', 'The theme should have a solid backgrounds')
-        self.assertTrue(default_theme.display_vertical_align == 0,
-                        'The theme should have a display_vertical_align of 0')
-        self.assertTrue(default_theme.font_footer_name == "Arial",
-                        'The theme should have a font_footer_name of Arial')
-        self.assertTrue(default_theme.font_main_bold is False, 'The theme should have a font_main_bold of false')
-        self.assertTrue(len(default_theme.__dict__) == 47, 'The theme should have 47 variables')
+        # THEN: The default values should be correct
+        self.assertEqual('#000000', default_theme.background_border_color,
+                         'background_border_color should be "#000000"')
+        self.assertEqual('solid', default_theme.background_type, 'background_type should be "solid"')
+        self.assertEqual(0, default_theme.display_vertical_align, 'display_vertical_align should be 0')
+        self.assertEqual('Arial', default_theme.font_footer_name, 'font_footer_name should be "Arial"')
+        self.assertFalse(default_theme.font_main_bold, 'font_main_bold should be False')
+        self.assertEqual(47, len(default_theme.__dict__), 'The theme should have 47 attributes')
+
+    def test_expand_json(self):
+        """
+        Test the expand_json method
+        """
+        # GIVEN: A ThemeXML object and some JSON to "expand"
+        theme = ThemeXML()
+        theme_json = {
+            'background': {
+                'border_color': '#000000',
+                'type': 'solid'
+            },
+            'display': {
+                'vertical_align': 0
+            },
+            'font': {
+                'footer': {
+                    'bold': False
+                },
+                'main': {
+                    'name': 'Arial'
+                }
+            }
+        }
+
+        # WHEN: ThemeXML.expand_json() is run
+        theme.expand_json(theme_json)
+
+        # THEN: The attributes should be set on the object
+        self.assertEqual('#000000', theme.background_border_color, 'background_border_color should be "#000000"')
+        self.assertEqual('solid', theme.background_type, 'background_type should be "solid"')
+        self.assertEqual(0, theme.display_vertical_align, 'display_vertical_align should be 0')
+        self.assertFalse(theme.font_footer_bold, 'font_footer_bold should be False')
+        self.assertEqual('Arial', theme.font_main_name, 'font_main_name should be "Arial"')
+
+    def test_extend_image_filename(self):
+        """
+        Test the extend_image_filename method
+        """
+        # GIVEN: A theme object
+        theme = ThemeXML()
+        theme.theme_name = 'MyBeautifulTheme   '
+        theme.background_filename = '    video.mp4'
+        theme.background_type = 'video'
+        path = os.path.expanduser('~')
+
+        # WHEN: ThemeXML.extend_image_filename is run
+        theme.extend_image_filename(path)
+
+        # THEN: The filename of the background should be correct
+        expected_filename = os.path.join(path, 'MyBeautifulTheme', 'video.mp4')
+        self.assertEqual(expected_filename, theme.background_filename)
+        self.assertEqual('MyBeautifulTheme', theme.theme_name)

tests/functional/openlp_core_ui/test_servicemanager.py

@@ -22,10 +22,12 @@
 """
 Package to test the openlp.core.ui.slidecontroller package.
 """
-import PyQt5
+import os
 from unittest import TestCase
 
-from openlp.core.common import Registry, ThemeLevel, Settings
+import PyQt5
+
+from openlp.core.common import Registry, ThemeLevel
 from openlp.core.lib import ServiceItem, ServiceItemType, ItemCapabilities
 from openlp.core.ui import ServiceManager
 
@@ -33,6 +35,9 @@ from tests.functional import MagicMock, patch
 
 
 class TestServiceManager(TestCase):
+    """
+    Test the service manager
+    """
 
     def setUp(self):
         """
@@ -108,20 +113,20 @@ class TestServiceManager(TestCase):
         # WHEN I define a context menu
         service_manager.context_menu(1)
         # THEN the following calls should have occurred.
-        self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have been called once')
-        self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have been called once')
-        self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have been called once')
-        self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have been called once')
-        self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have been called once')
-        self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have been called once')
-        self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have been called once')
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
-                          'Should have been called once')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
-                          'Should have been called once')
+        self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have been called once')
+        self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have been called once')
+        self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have been called once')
+        self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have been called once')
+        self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have been called once')
+        self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have been called once')
+        self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have been called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
+                         'Should have been called once')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
+                         'Should have been called once')
 
     def test_build_song_context_menu(self):
         """
@@ -139,12 +144,10 @@ class TestServiceManager(TestCase):
         service_manager.service_manager_list = MagicMock()
         service_manager.service_manager_list.itemAt.return_value = item
         service_item = ServiceItem(None)
-        service_item.add_capability(ItemCapabilities.CanEdit)
-        service_item.add_capability(ItemCapabilities.CanPreview)
-        service_item.add_capability(ItemCapabilities.CanLoop)
-        service_item.add_capability(ItemCapabilities.OnLoadUpdate)
-        service_item.add_capability(ItemCapabilities.AddIfNewItem)
-        service_item.add_capability(ItemCapabilities.CanSoftBreak)
+        for capability in [ItemCapabilities.CanEdit, ItemCapabilities.CanPreview, ItemCapabilities.CanLoop,
+                           ItemCapabilities.OnLoadUpdate, ItemCapabilities.AddIfNewItem,
+                           ItemCapabilities.CanSoftBreak]:
+            service_item.add_capability(capability)
         service_item.service_item_type = ServiceItemType.Text
         service_item.edit_id = 1
         service_item._display_frames.append(MagicMock())
@@ -165,29 +168,29 @@ class TestServiceManager(TestCase):
         # WHEN I define a context menu
         service_manager.context_menu(1)
         # THEN the following calls should have occurred.
-        self.assertEquals(service_manager.edit_action.setVisible.call_count, 2, 'Should have be called twice')
-        self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 2,
-                          'Should have be called twice')
+        self.assertEqual(service_manager.edit_action.setVisible.call_count, 2, 'Should have be called twice')
+        self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 2,
+                         'Should have be called twice')
         # THEN we add a 2nd display frame
         service_item._display_frames.append(MagicMock())
         service_manager.context_menu(1)
         # THEN the following additional calls should have occurred.
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2,
-                          'Should have be called twice')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2,
+                         'Should have be called twice')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once')
 
     def test_build_bible_context_menu(self):
         """
@@ -205,11 +208,10 @@ class TestServiceManager(TestCase):
         service_manager.service_manager_list = MagicMock()
         service_manager.service_manager_list.itemAt.return_value = item
         service_item = ServiceItem(None)
-        service_item.add_capability(ItemCapabilities.NoLineBreaks)
-        service_item.add_capability(ItemCapabilities.CanPreview)
-        service_item.add_capability(ItemCapabilities.CanLoop)
-        service_item.add_capability(ItemCapabilities.CanWordSplit)
-        service_item.add_capability(ItemCapabilities.CanEditTitle)
+        for capability in [ItemCapabilities.NoLineBreaks, ItemCapabilities.CanPreview,
+                           ItemCapabilities.CanLoop, ItemCapabilities.CanWordSplit,
+                           ItemCapabilities.CanEditTitle]:
+            service_item.add_capability(capability)
         service_item.service_item_type = ServiceItemType.Text
         service_item.edit_id = 1
         service_item._display_frames.append(MagicMock())
@@ -230,29 +232,29 @@ class TestServiceManager(TestCase):
         # WHEN I define a context menu
         service_manager.context_menu(1)
         # THEN the following calls should have occurred.
-        self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice')
-        self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 2,
-                          'Should have be called twice')
+        self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice')
+        self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 2,
+                         'Should have be called twice')
         # THEN we add a 2nd display frame
         service_item._display_frames.append(MagicMock())
         service_manager.context_menu(1)
         # THEN the following additional calls should have occurred.
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2,
-                          'Should have be called twice')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2,
+                         'Should have be called twice')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once')
 
     def test_build_custom_context_menu(self):
         """
@@ -295,29 +297,29 @@ class TestServiceManager(TestCase):
         # WHEN I define a context menu
         service_manager.context_menu(1)
         # THEN the following calls should have occurred.
-        self.assertEquals(service_manager.edit_action.setVisible.call_count, 2, 'Should have be called twice')
-        self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 2,
-                          'Should have be called twice')
+        self.assertEqual(service_manager.edit_action.setVisible.call_count, 2, 'Should have be called twice')
+        self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 2,
+                         'Should have be called twice')
         # THEN we add a 2nd display frame
         service_item._display_frames.append(MagicMock())
         service_manager.context_menu(1)
         # THEN the following additional calls should have occurred.
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2,
-                          'Should have be called twice')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2,
+                         'Should have be called twice')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once')
 
     def test_build_image_context_menu(self):
         """
@@ -358,29 +360,29 @@ class TestServiceManager(TestCase):
         # WHEN I define a context menu
         service_manager.context_menu(1)
         # THEN the following calls should have occurred.
-        self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice')
-        self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.maintain_action.setVisible.call_count, 2, 'Should have be called twice')
-        self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
+        self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice')
+        self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.maintain_action.setVisible.call_count, 2, 'Should have be called twice')
+        self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
         # THEN we add a 2nd display frame and regenerate the menu.
         service_item._raw_frames.append(MagicMock())
         service_manager.context_menu(1)
         # THEN the following additional calls should have occurred.
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2,
-                          'Should have be called twice')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2,
+                         'Should have be called twice')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once')
 
     def test_build_media_context_menu(self):
         """
@@ -419,25 +421,25 @@ class TestServiceManager(TestCase):
         # WHEN I define a context menu
         service_manager.context_menu(1)
         # THEN the following calls should have occurred.
-        self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice')
-        self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 2, 'Should have be called twice')
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
+        self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice')
+        self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 2, 'Should have be called twice')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
         # THEN I change the length of the media and regenerate the menu.
         service_item.set_media_length(5)
         service_manager.context_menu(1)
         # THEN the following additional calls should have occurred.
-        self.assertEquals(service_manager.time_action.setVisible.call_count, 3, 'Should have be called three times')
+        self.assertEqual(service_manager.time_action.setVisible.call_count, 3, 'Should have be called three times')
 
     def test_build_presentation_pdf_context_menu(self):
         """
@@ -477,20 +479,20 @@ class TestServiceManager(TestCase):
         # WHEN I define a context menu
         service_manager.context_menu(1)
         # THEN the following calls should have occurred.
-        self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.maintain_action.setVisible.call_count, 2, 'Should have be called twice')
-        self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
+        self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.maintain_action.setVisible.call_count, 2, 'Should have be called twice')
+        self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
 
     def test_build_presentation_non_pdf_context_menu(self):
         """
@@ -527,20 +529,20 @@ class TestServiceManager(TestCase):
         # WHEN I define a context menu
         service_manager.context_menu(1)
         # THEN the following calls should have occurred.
-        self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
-        self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
-        self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
-                          'Should have be called once')
+        self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
+        self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called')
+        self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
+                         'Should have be called once')
 
     @patch(u'openlp.core.ui.servicemanager.Settings')
     @patch(u'PyQt5.QtCore.QTimer.singleShot')
@@ -573,7 +575,7 @@ class TestServiceManager(TestCase):
         # WHEN: on_single_click_preview() is called
         service_manager.on_single_click_preview()
         # THEN: timer should not be started
-        self.assertEquals(mocked_singleShot.call_count, 0, 'Should not be called')
+        self.assertEqual(mocked_singleShot.call_count, 0, 'Should not be called')
 
     @patch(u'openlp.core.ui.servicemanager.Settings')
     @patch(u'PyQt5.QtCore.QTimer.singleShot')
@@ -591,7 +593,8 @@ class TestServiceManager(TestCase):
         service_manager.on_double_click_live()
         service_manager.on_single_click_preview()
         # THEN: timer should not be started
-        self.assertEquals(mocked_singleShot.call_count, 0, 'Should not be called')
+        mocked_make_live.assert_called_with()
+        self.assertEqual(mocked_singleShot.call_count, 0, 'Should not be called')
 
     @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_preview')
     def test_single_click_timeout_single(self, mocked_make_preview):
@@ -603,7 +606,8 @@ class TestServiceManager(TestCase):
         # WHEN: on_single_click_preview() is called
         service_manager.on_single_click_preview_timeout()
         # THEN: make_preview() should have been called
-        self.assertEquals(mocked_make_preview.call_count, 1, 'Should have been called once')
+        self.assertEqual(mocked_make_preview.call_count, 1,
+                         'ServiceManager.make_preview() should have been called once')
 
     @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_preview')
     @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_live')
@@ -616,5 +620,62 @@ class TestServiceManager(TestCase):
         # WHEN: on_single_click_preview() is called after a double click
         service_manager.on_double_click_live()
         service_manager.on_single_click_preview_timeout()
-        # THEN: make_preview() should have been called
-        self.assertEquals(mocked_make_preview.call_count, 0, 'Should not be called')
+        # THEN: make_preview() should not have been called
+        self.assertEqual(mocked_make_preview.call_count, 0, 'ServiceManager.make_preview() should not be called')
+
+    @patch(u'openlp.core.ui.servicemanager.shutil.copy')
+    @patch(u'openlp.core.ui.servicemanager.zipfile')
+    @patch(u'openlp.core.ui.servicemanager.ServiceManager.save_file_as')
+    def test_save_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy):
+        """
+        Test that when a PermissionError is raised when trying to save a file, it is handled correctly
+        """
+        # GIVEN: A service manager, a service to save
+        mocked_main_window = MagicMock()
+        mocked_main_window.service_manager_settings_section = 'servicemanager'
+        Registry().register('main_window', mocked_main_window)
+        Registry().register('application', MagicMock())
+        service_manager = ServiceManager(None)
+        service_manager._file_name = os.path.join('temp', 'filename.osz')
+        service_manager._save_lite = False
+        service_manager.service_items = []
+        service_manager.service_theme = 'Default'
+        service_manager.service_manager_list = MagicMock()
+        mocked_save_file_as.return_value = True
+        mocked_zipfile.ZipFile.return_value = MagicMock()
+        mocked_shutil_copy.side_effect = PermissionError
+
+        # WHEN: The service is saved and a PermissionError is thrown
+        result = service_manager.save_file()
+
+        # THEN: The "save_as" method is called to save the service
+        self.assertTrue(result)
+        mocked_save_file_as.assert_called_with()
+
+    @patch(u'openlp.core.ui.servicemanager.shutil.copy')
+    @patch(u'openlp.core.ui.servicemanager.zipfile')
+    @patch(u'openlp.core.ui.servicemanager.ServiceManager.save_file_as')
+    def test_save_local_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy):
+        """
+        Test that when a PermissionError is raised when trying to save a local file, it is handled correctly
+        """
+        # GIVEN: A service manager, a service to save
+        mocked_main_window = MagicMock()
+        mocked_main_window.service_manager_settings_section = 'servicemanager'
+        Registry().register('main_window', mocked_main_window)
+        Registry().register('application', MagicMock())
+        service_manager = ServiceManager(None)
+        service_manager._file_name = os.path.join('temp', 'filename.osz')
+        service_manager._save_lite = False
+        service_manager.service_items = []
+        service_manager.service_theme = 'Default'
+        mocked_save_file_as.return_value = True
+        mocked_zipfile.ZipFile.return_value = MagicMock()
+        mocked_shutil_copy.side_effect = PermissionError
+
+        # WHEN: The service is saved and a PermissionError is thrown
+        result = service_manager.save_local_file()
+
+        # THEN: The "save_as" method is called to save the service
+        self.assertTrue(result)
+        mocked_save_file_as.assert_called_with()

tests/functional/openlp_plugins/songs/test_songshowplusimport.py

@@ -52,6 +52,8 @@ class TestSongShowPlusFileImport(SongImportTestHelper):
                          self.load_external_result_data(os.path.join(TEST_PATH, 'Beautiful Garden Of Prayer.json')))
         self.file_import([os.path.join(TEST_PATH, 'a mighty fortress is our god.sbsong')],
                          self.load_external_result_data(os.path.join(TEST_PATH, 'a mighty fortress is our god.json')))
+        self.file_import([os.path.join(TEST_PATH, 'cleanse-me.sbsong')],
+                         self.load_external_result_data(os.path.join(TEST_PATH, 'cleanse-me.json')))
 
 
 class TestSongShowPlusImport(TestCase):

tests/resources/songshowplussongs/cleanse-me.json

@@ -0,0 +1,38 @@
+{
+    "authors": [
+        "J. Edwin Orr"
+    ],
+    "ccli_number": 56307,
+    "comments": "",
+    "copyright": "Public Domain ",
+    "song_book_name": "",
+    "song_number": 438,
+    "title": "Cleanse Me [438]",
+    "topics": [
+        "Cleansing",
+        "Communion",
+        "Consecration",
+        "Holiness",
+        "Holy Spirit",
+        "Revival"
+    ],
+    "verse_order_list": [],
+    "verses": [
+        [
+            "Search me, O God,\r\nAnd know my heart today;\r\nTry me, O Savior,\r\nKnow my thoughts, I pray.\r\nSee if there be\r\nSome wicked way in me;\r\nCleanse me from every sin\r\nAnd set me free.",
+            "v1"
+        ],
+        [
+            "I praise Thee, Lord,\r\nFor cleansing me from sin;\r\nFulfill Thy Word,\r\nAnd make me pure within.\r\nFill me with fire\r\nWhere once I burned with shame;\r\nGrant my desire\r\nTo magnify Thy name.",
+            "v2"
+        ],
+        [
+            "Lord, take my life,\r\nAnd make it wholly Thine;\r\nFill my poor heart\r\nWith Thy great love divine.\r\nTake all my will,\r\nMy passion, self and pride;\r\nI now surrender, Lord\r\nIn me abide.",
+            "v3"
+        ],
+        [
+            "O Holy Ghost,\r\nRevival comes from Thee;\r\nSend a revival,\r\nStart the work in me.\r\nThy Word declares\r\nThou wilt supply our need;\r\nFor blessings now,\r\nO Lord, I humbly plead.",
+            "v4"
+        ]
+    ]
+}

tests/utils/test_pylint.py

@@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2016 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                          #
+###############################################################################
+"""
+Package to test for proper bzr tags.
+"""
+import os
+import logging
+import platform
+from unittest import TestCase, SkipTest
+
+try:
+    from pylint import epylint as lint
+    from pylint.__pkginfo__ import version
+except ImportError:
+    raise SkipTest('pylint not installed - skipping tests using pylint.')
+
+from openlp.core.common import is_win
+
+TOLERATED_ERRORS = {'registryproperties.py': ['access-member-before-definition'],
+                    'opensong.py': ['no-name-in-module'],
+                    'maindisplay.py': ['no-name-in-module']}
+
+
+class TestPylint(TestCase):
+
+    def test_pylint(self):
+        """
+        Test for pylint errors
+        """
+        # GIVEN: Some checks to disable and enable, and the pylint script
+        disabled_checks = 'import-error,no-member'
+        enabled_checks = 'missing-format-argument-key,unused-format-string-argument'
+        if is_win() or 'arch' in platform.dist()[0].lower():
+            pylint_script = 'pylint'
+        else:
+            pylint_script = 'pylint3'
+
+        # WHEN: Running pylint
+        (pylint_stdout, pylint_stderr) = \
+            lint.py_run('openlp --errors-only --disable={disabled} --enable={enabled} '
+                        '--reports=no --output-format=parseable'.format(disabled=disabled_checks,
+                                                                        enabled=enabled_checks),
+                        return_std=True, script=pylint_script)
+        stdout = pylint_stdout.read()
+        stderr = pylint_stderr.read()
+        filtered_stdout = self._filter_tolerated_errors(stdout)
+        print(filtered_stdout)
+        print(stderr)
+
+        # THEN: The output should be empty
+        self.assertTrue(filtered_stdout == '', 'PyLint should find no errors')
+
+    def _filter_tolerated_errors(self, pylint_output):
+        """
+        Filter out errors we tolerate.
+        """
+        filtered_output = ''
+        for line in pylint_output.splitlines():
+            # Filter out module info lines
+            if line.startswith('**'):
+                continue
+            # Filter out undefined-variable error releated to WindowsError
+            elif 'undefined-variable' in line and 'WindowsError' in line:
+                continue
+            # Filter out PyQt related errors
+            elif ('no-name-in-module' in line or 'no-member' in line) and 'PyQt5' in line:
+                continue
+            elif self._is_line_tolerated(line):
+                continue
+            else:
+                filtered_output += line + '\n'
+        return filtered_output.strip()
+
+    def _is_line_tolerated(self, line):
+        """
+        Check if line constains a tolerated error
+        """
+        for filename in TOLERATED_ERRORS:
+            for tolerated_error in TOLERATED_ERRORS[filename]:
+                if filename in line and tolerated_error in line:
+                    return True
+        return False