Merge lp:~phill-ridout/openlp/fixes-IV into lp:openlp

[Phill]

Refactors and fixes

Refactor `singleton` classes to use a `singleton` metaclass
Fixes for a number of reported issues
Tidy ups, and improvements as suggested by pycharm

[Raoul Snyman (raoul-snyman)]

Linux tests passed!

[Raoul Snyman (raoul-snyman)]

Linting passed!

[Tim Bentley (trb143)]

review: Approve

=== modified file 'openlp/core/common/__init__.py'
--- openlp/core/common/__init__.py	2019-07-19 18:43:14 +0000
+++ openlp/core/common/__init__.py	2019-08-06 21:48:11 +0000
@@ -172,6 +172,21 @@
     Next = 3
 
 
+class Singleton(type):
+    """
+    Provide a `Singleton` metaclass https://stackoverflow.com/questions/6760685/creating-a-singleton-in-python
+    """
+    _instances = {}
+
+    def __call__(cls, *args, **kwargs):
+        """
+        Create a new instance if one does not already exist.
+        """
+        if cls not in cls._instances:
+            cls._instances[cls] = super().__call__(*args, **kwargs)
+        return cls._instances[cls]
+
+
 def de_hump(name):
     """
     Change any Camel Case string to python string
@@ -385,7 +400,7 @@
     global IMAGES_FILTER
     if not IMAGES_FILTER:
         log.debug('Generating images filter.')
-        formats = list(map(bytes.decode, list(map(bytes, QtGui.QImageReader.supportedImageFormats()))))
+        formats = list(map(bytes.decode, map(bytes, QtGui.QImageReader.supportedImageFormats())))
         visible_formats = '(*.{text})'.format(text='; *.'.join(formats))
         actual_formats = '(*.{text})'.format(text=' *.'.join(formats))
         IMAGES_FILTER = '{text} {visible} {actual}'.format(text=translate('OpenLP', 'Image Files'),

=== modified file 'openlp/core/common/actions.py'
--- openlp/core/common/actions.py	2019-04-13 13:00:22 +0000
+++ openlp/core/common/actions.py	2019-08-06 21:48:11 +0000
@@ -260,7 +260,7 @@
             return
         # We have to do this to ensure that the loaded shortcut list e. g. STRG+O (German) is converted to CTRL+O,
         # which is only done when we convert the strings in this way (QKeySequencet -> uncode).
-        shortcuts = list(map(QtGui.QKeySequence.toString, list(map(QtGui.QKeySequence, shortcuts))))
+        shortcuts = list(map(QtGui.QKeySequence.toString, map(QtGui.QKeySequence, shortcuts)))
         # Check the alternate shortcut first, to avoid problems when the alternate shortcut becomes the primary shortcut
         #  after removing the (initial) primary shortcut due to conflicts.
         if len(shortcuts) == 2:

=== modified file 'openlp/core/common/i18n.py'
--- openlp/core/common/i18n.py	2019-07-03 13:23:23 +0000
+++ openlp/core/common/i18n.py	2019-08-06 21:48:11 +0000
@@ -29,7 +29,7 @@
 
 from PyQt5 import QtCore, QtWidgets
 
-from openlp.core.common import is_macosx, is_win
+from openlp.core.common import Singleton, is_macosx, is_win
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.settings import Settings
 
@@ -327,22 +327,11 @@
         return LanguageManager.__qm_list__
 
 
-class UiStrings(object):
+class UiStrings(metaclass=Singleton):
     """
     Provide standard strings for objects to use.
     """
-    __instance__ = None
-
-    def __new__(cls):
-        """
-        Override the default object creation method to return a single instance.
-        """
-        if not cls.__instance__:
-            cls.__instance__ = super().__new__(cls)
-            cls.__instance__.load()
-        return cls.__instance__
-
-    def load(self):
+    def __init__(self):
         """
         These strings should need a good reason to be retranslated elsewhere.
         Should some/more/less of these have an & attached?
@@ -436,6 +425,7 @@
         self.ResetLiveBG = translate('OpenLP.Ui', 'Reset live background.')
         self.RequiredShowInFooter = translate('OpenLP.Ui', 'Required, this will be displayed in footer.')
         self.Seconds = translate('OpenLP.Ui', 's', 'The abbreviated unit for seconds')
+        self.SaveAndClose = translate('OpenLP.ui', translate('SongsPlugin.EditSongForm', '&Save && Close'))
         self.SaveAndPreview = translate('OpenLP.Ui', 'Save && Preview')
         self.Search = translate('OpenLP.Ui', 'Search')
         self.SearchThemes = translate('OpenLP.Ui', 'Search Themes...', 'Search bar place holder text ')

=== modified file 'openlp/core/common/registry.py'
--- openlp/core/common/registry.py	2019-05-05 18:41:17 +0000
+++ openlp/core/common/registry.py	2019-08-06 21:48:11 +0000
@@ -23,29 +23,19 @@
 Provide Registry Services
 """
 import logging
-import sys
 
-from openlp.core.common import de_hump, trace_error_handler
+from openlp.core.common import Singleton, de_hump, trace_error_handler
 
 
 log = logging.getLogger(__name__)
 
 
-class Registry(object):
+class Registry(metaclass=Singleton):
     """
     This is the Component Registry.  It is a singleton object and is used to provide a look up service for common
     objects.
     """
     log.info('Registry loaded')
-    __instance__ = None
-
-    def __new__(cls):
-        """
-        Re-implement the __new__ method to make sure we create a true singleton.
-        """
-        if not cls.__instance__:
-            cls.__instance__ = object.__new__(cls)
-        return cls.__instance__
 
     @classmethod
     def create(cls):
@@ -57,20 +47,9 @@
         registry.service_list = {}
         registry.functions_list = {}
         registry.working_flags = {}
-        # Allow the tests to remove Registry entries but not the live system
-        registry.running_under_test = 'nose' in sys.argv[0] or 'pytest' in sys.argv[0]
         registry.initialising = True
         return registry
 
-    @classmethod
-    def destroy(cls):
-        """
-        Destroy the Registry.
-        """
-        if cls.__instance__.running_under_test:
-            del cls.__instance__
-            cls.__instance__ = None
-
     def get(self, key):
         """
         Extracts the registry value from the list based on the key passed in

=== modified file 'openlp/core/display/html/display.js'
--- openlp/core/display/html/display.js	2019-06-21 20:53:42 +0000
+++ openlp/core/display/html/display.js	2019-08-06 21:48:11 +0000
@@ -281,8 +281,9 @@
    * Checks if the present slide content fits within the slide
   */
   doesContentFit: function () {
-    console.debug("scrollHeight: " + $(".slides")[0].scrollHeight + ", clientHeight: " + $(".slides")[0].clientHeight);
-    return $(".slides")[0].clientHeight >= $(".slides")[0].scrollHeight;
+    var currSlide = $(".slides")[0];
+    console.debug("scrollHeight: " + currSlide.scrollHeight + ", clientHeight: " + currSlide.clientHeight);
+    return currSlide.clientHeight >= currSlide.scrollHeight;
   },
   /**
    * Generate the OpenLP startup splashscreen
@@ -333,7 +334,7 @@
   /**
    * Set fullscreen image from base64 data
    * @param {string} bg_color - The background color
-   * @param {string} image - Path to the image
+   * @param {string} image_data - base64 encoded image data
    */
   setFullscreenImageFromData: function(bg_color, image_data) {
     Display.clearSlides();
@@ -372,7 +373,6 @@
    * @param {string} verse - The verse number, e.g. "v1"
    * @param {string} text - The HTML for the verse, e.g. "line1<br>line2"
    * @param {string} footer_text - The HTML for the footer"
-   * @param {bool} [reinit=true] - Re-initialize Reveal. Defaults to true.
    */
   addTextSlide: function (verse, text, footer_text) {
     var html = _prepareText(text);
@@ -476,25 +476,28 @@
    * Play a video
    */
   playVideo: function () {
-    if ($("#video").length == 1) {
-      $("#video")[0].play();
+    var videoElem = $("#video");
+    if (videoElem.length == 1) {
+      videoElem[0].play();
     }
   },
   /**
    * Pause a video
    */
   pauseVideo: function () {
-    if ($("#video").length == 1) {
-      $("#video")[0].pause();
+    var videoElem = $("#video");
+    if (videoElem.length == 1) {
+      videoElem[0].pause();
     }
   },
   /**
    * Stop a video
    */
   stopVideo: function () {
-    if ($("#video").length == 1) {
-      $("#video")[0].pause();
-      $("#video")[0].currentTime = 0.0;
+    var videoElem = $("#video");
+    if (videoElem.length == 1) {
+      videoElem[0].pause();
+      videoElem[0].currentTime = 0.0;
     }
   },
   /**
@@ -502,8 +505,9 @@
    * @param seconds The position in seconds to seek to
    */
   seekVideo: function (seconds) {
-    if ($("#video").length == 1) {
-      $("#video")[0].currentTime = seconds;
+    var videoElem = $("#video");
+    if (videoElem.length == 1) {
+      videoElem[0].currentTime = seconds;
     }
   },
   /**
@@ -511,8 +515,9 @@
    * @param rate A Double of the rate. 1.0 => 100% speed, 0.75 => 75% speed, 1.25 => 125% speed, etc.
    */
   setPlaybackRate: function (rate) {
-    if ($("#video").length == 1) {
-      $("#video")[0].playbackRate = rate;
+    var videoElem = $("#video");
+    if (videoElem.length == 1) {
+      videoElem[0].playbackRate = rate;
     }
   },
   /**
@@ -520,24 +525,27 @@
    * @param level The volume level from 0 to 100.
    */
   setVideoVolume: function (level) {
-    if ($("#video").length == 1) {
-      $("#video")[0].volume = level / 100.0;
+    var videoElem = $("#video");
+    if (videoElem.length == 1) {
+      videoElem[0].volume = level / 100.0;
     }
   },
   /**
    * Mute the volume
    */
   toggleVideoMute: function () {
-    if ($("#video").length == 1) {
-      $("#video")[0].muted = !$("#video")[0].muted;
+    var videoElem = $("#video");
+    if (videoElem.length == 1) {
+      videoElem[0].muted = !videoElem[0].muted;
     }
   },
   /**
    * Clear the background audio playlist
    */
   clearPlaylist: function () {
-    if ($("#background-audio").length == 1) {
-      var audio = $("#background-audio")[0];
+    var backgroundAudoElem = $("#background-audio");
+    if (backgroundAudoElem.length == 1) {
+      var audio = backgroundAudoElem[0];
       /* audio.playList */
     }
   },
@@ -619,7 +627,6 @@
   },
   setTheme: function (theme) {
     this._theme = theme;
-    var slidesDiv = $(".slides")
     // Set the background
     var globalBackground = $("#global-background")[0];
     var backgroundStyle = {};

=== modified file 'openlp/core/display/render.py'
--- openlp/core/display/render.py	2019-07-28 15:56:28 +0000
+++ openlp/core/display/render.py	2019-08-06 21:48:11 +0000
@@ -47,7 +47,7 @@
 
 SLIM_CHARS = 'fiíIÍjlĺľrtť.,;/ ()|"\'!:\\'
 CHORD_LINE_MATCH = re.compile(r'\[(.*?)\]([\u0080-\uFFFF,\w]*)'
-                              r'([\u0080-\uFFFF,\w,\s,\.,\,,\!,\?,\;,\:,\|,\",\',\-,\_]*)(\Z)?')
+                              r'([\u0080-\uFFFF\w\s\.\,\!\?\;\:\|\"\'\-\_]*)(\Z)?')
 CHORD_TEMPLATE = '<span class="chordline">{chord}</span>'
 FIRST_CHORD_TEMPLATE = '<span class="chordline firstchordline">{chord}</span>'
 CHORD_LINE_TEMPLATE = '<span class="chord"><span><strong>{chord}</strong></span></span>{tail}{whitespace}{remainder}'

=== modified file 'openlp/core/display/screens.py'
--- openlp/core/display/screens.py	2019-08-04 14:06:00 +0000
+++ openlp/core/display/screens.py	2019-08-06 21:48:11 +0000
@@ -28,6 +28,7 @@
 
 from PyQt5 import QtCore, QtWidgets
 
+from openlp.core.common import Singleton
 from openlp.core.common.i18n import translate
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
@@ -147,24 +148,15 @@
                                                 screen_dict['custom_geometry']['height'])
 
 
-class ScreenList(object):
+class ScreenList(metaclass=Singleton):
     """
     Wrapper to handle the parameters of the display screen.
 
     To get access to the screen list call ``ScreenList()``.
     """
     log.info('Screen loaded')
-    __instance__ = None
     screens = []
 
-    def __new__(cls):
-        """
-        Re-implement __new__ to create a true singleton.
-        """
-        if not cls.__instance__:
-            cls.__instance__ = object.__new__(cls)
-        return cls.__instance__
-
     def __iter__(self):
         """
         Convert this object into an iterable, so that we can iterate over it instead of the inner list

=== modified file 'openlp/core/lib/theme.py'
--- openlp/core/lib/theme.py	2019-06-21 22:09:36 +0000
+++ openlp/core/lib/theme.py	2019-08-06 21:48:11 +0000
@@ -170,6 +170,7 @@
         jsn = get_text_file_string(json_path)
         self.load_theme(jsn)
         self.background_filename = None
+        self.version = 2
 
     def expand_json(self, var, prev=None):
         """

=== modified file 'openlp/core/state.py'
--- openlp/core/state.py	2019-04-13 13:00:22 +0000
+++ openlp/core/state.py	2019-08-06 21:48:11 +0000
@@ -28,6 +28,7 @@
 """
 import logging
 
+from openlp.core.common import Singleton
 from openlp.core.common.registry import Registry
 from openlp.core.common.mixins import LogMixin
 from openlp.core.lib.plugin import PluginStatus
@@ -52,17 +53,7 @@
         self.text = None
 
 
-class State(LogMixin):
-
-    __instance__ = None
-
-    def __new__(cls):
-        """
-        Re-implement the __new__ method to make sure we create a true singleton.
-        """
-        if not cls.__instance__:
-            cls.__instance__ = object.__new__(cls)
-        return cls.__instance__
+class State(LogMixin, metaclass=Singleton):
 
     def load_settings(self):
         self.modules = {}

=== modified file 'openlp/core/ui/advancedtab.py'
--- openlp/core/ui/advancedtab.py	2019-05-22 06:47:00 +0000
+++ openlp/core/ui/advancedtab.py	2019-08-06 21:48:11 +0000
@@ -81,7 +81,7 @@
         self.ui_layout.addRow(self.media_plugin_check_box)
         self.hide_mouse_check_box = QtWidgets.QCheckBox(self.ui_group_box)
         self.hide_mouse_check_box.setObjectName('hide_mouse_check_box')
-        self.ui_layout.addWidget(self.hide_mouse_check_box)
+        self.ui_layout.addRow(self.hide_mouse_check_box)
         self.double_click_live_check_box = QtWidgets.QCheckBox(self.ui_group_box)
         self.double_click_live_check_box.setObjectName('double_click_live_check_box')
         self.ui_layout.addRow(self.double_click_live_check_box)

=== modified file 'openlp/core/ui/icons.py'
--- openlp/core/ui/icons.py	2019-07-03 13:23:23 +0000
+++ openlp/core/ui/icons.py	2019-08-06 21:48:11 +0000
@@ -27,6 +27,7 @@
 import qtawesome as qta
 from PyQt5 import QtGui, QtWidgets
 
+from openlp.core.common import Singleton
 from openlp.core.common.applocation import AppLocation
 from openlp.core.lib import build_icon
 
@@ -34,22 +35,11 @@
 log = logging.getLogger(__name__)
 
 
-class UiIcons(object):
+class UiIcons(metaclass=Singleton):
     """
     Provide standard icons for objects to use.
     """
-    __instance__ = None
-
-    def __new__(cls):
-        """
-        Override the default object creation method to return a single instance.
-        """
-        if not cls.__instance__:
-            cls.__instance__ = super().__new__(cls)
-            cls.__instance__.load()
-        return cls.__instance__
-
-    def load(self):
+    def __init__(self):
         """
         These are the font icons used in the code.
         """
@@ -165,6 +155,7 @@
             'volunteer': {'icon': 'fa.group'}
         }
         self.load_icons(icon_list)
+        self.main_icon = build_icon(':/icon/openlp-logo.svg')
 
     def load_icons(self, icon_list):
         """
@@ -184,7 +175,6 @@
                     setattr(self, key, qta.icon('fa.plus-circle', color='red'))
             except Exception:
                 setattr(self, key, qta.icon('fa.plus-circle', color='red'))
-        self.main_icon = build_icon(':/icon/openlp-logo.svg')
 
     @staticmethod
     def _print_icons():

=== modified file 'openlp/plugins/bibles/lib/__init__.py'
--- openlp/plugins/bibles/lib/__init__.py	2019-04-13 13:00:22 +0000
+++ openlp/plugins/bibles/lib/__init__.py	2019-08-06 21:48:11 +0000
@@ -26,6 +26,7 @@
 import logging
 import re
 
+from openlp.core.common import Singleton
 from openlp.core.common.i18n import translate
 from openlp.core.common.settings import Settings
 
@@ -64,20 +65,10 @@
     English = 2
 
 
-class BibleStrings(object):
+class BibleStrings(metaclass=Singleton):
     """
     Provide standard strings for objects to use.
     """
-    __instance__ = None
-
-    def __new__(cls):
-        """
-        Override the default object creation method to return a single instance.
-        """
-        if not cls.__instance__:
-            cls.__instance__ = object.__new__(cls)
-        return cls.__instance__
-
     def __init__(self):
         """
         These strings should need a good reason to be retranslated elsewhere.
@@ -336,11 +327,13 @@
         log.debug('Matched reference {text}'.format(text=reference))
         book = match.group('book')
         if not book_ref_id:
-            book_ref_id = bible.get_book_ref_id_by_localised_name(book, language_selection)
+            book_ref_ids = bible.get_book_ref_id_by_localised_name(book, language_selection)
         elif not bible.get_book_by_book_ref_id(book_ref_id):
             return []
+        else:
+            book_ref_ids = [book_ref_id]
         # We have not found the book so do not continue
-        if not book_ref_id:
+        if not book_ref_ids:
             return []
         ranges = match.group('ranges')
         range_list = get_reference_match('range_separator').split(ranges)
@@ -381,22 +374,23 @@
                     to_chapter = to_verse
                     to_verse = None
             # Append references to the list
-            if has_range:
-                if not from_verse:
-                    from_verse = 1
-                if not to_verse:
-                    to_verse = -1
-                if to_chapter and to_chapter > from_chapter:
-                    ref_list.append((book_ref_id, from_chapter, from_verse, -1))
-                    for i in range(from_chapter + 1, to_chapter):
-                        ref_list.append((book_ref_id, i, 1, -1))
-                    ref_list.append((book_ref_id, to_chapter, 1, to_verse))
-                elif to_verse >= from_verse or to_verse == -1:
-                    ref_list.append((book_ref_id, from_chapter, from_verse, to_verse))
-            elif from_verse:
-                ref_list.append((book_ref_id, from_chapter, from_verse, from_verse))
-            else:
-                ref_list.append((book_ref_id, from_chapter, 1, -1))
+            for book_ref_id in book_ref_ids:
+                if has_range:
+                    if not from_verse:
+                        from_verse = 1
+                    if not to_verse:
+                        to_verse = -1
+                    if to_chapter and to_chapter > from_chapter:
+                        ref_list.append((book_ref_id, from_chapter, from_verse, -1))
+                        for i in range(from_chapter + 1, to_chapter):
+                            ref_list.append((book_ref_id, i, 1, -1))
+                        ref_list.append((book_ref_id, to_chapter, 1, to_verse))
+                    elif to_verse >= from_verse or to_verse == -1:
+                        ref_list.append((book_ref_id, from_chapter, from_verse, to_verse))
+                elif from_verse:
+                    ref_list.append((book_ref_id, from_chapter, from_verse, from_verse))
+                else:
+                    ref_list.append((book_ref_id, from_chapter, 1, -1))
         return ref_list
     else:
         log.debug('Invalid reference: {text}'.format(text=reference))

=== modified file 'openlp/plugins/bibles/lib/db.py'
--- openlp/plugins/bibles/lib/db.py	2019-05-22 06:47:00 +0000
+++ openlp/plugins/bibles/lib/db.py	2019-08-06 21:48:11 +0000
@@ -281,13 +281,14 @@
         log.debug('BibleDB.get_book("{book}")'.format(book=book))
         return self.get_object_filtered(Book, Book.name.like(book + '%'))
 
-    def get_books(self):
+    def get_books(self, book=None):
         """
         A wrapper so both local and web bibles have a get_books() method that
         manager can call. Used in the media manager advanced search tab.
         """
-        log.debug('BibleDB.get_books()')
-        return self.get_all_objects(Book, order_by_ref=Book.id)
+        log.debug('BibleDB.get_books("{book}")'.format(book=book))
+        filter = Book.name.like(book + '%') if book else None
+        return self.get_all_objects(Book, filter_clause=filter, order_by_ref=Book.id)
 
     def get_book_by_book_ref_id(self, ref_id):
         """
@@ -300,39 +301,35 @@
 
     def get_book_ref_id_by_localised_name(self, book, language_selection):
         """
-        Return the id of a named book.
+        Return the ids of a matching named book.
 
         :param book: The name of the book, according to the selected language.
         :param language_selection:  The language selection the user has chosen in the settings section of the Bible.
+        :rtype: list[int]
         """
         log.debug('get_book_ref_id_by_localised_name("{book}", "{lang}")'.format(book=book, lang=language_selection))
         from openlp.plugins.bibles.lib import LanguageSelection, BibleStrings
         book_names = BibleStrings().BookNames
         # escape reserved characters
-        book_escaped = book
         for character in RESERVED_CHARACTERS:
-            book_escaped = book_escaped.replace(character, '\\' + character)
+            book_escaped = book.replace(character, '\\' + character)
         regex_book = re.compile('\\s*{book}\\s*'.format(book='\\s*'.join(book_escaped.split())), re.IGNORECASE)
         if language_selection == LanguageSelection.Bible:
-            db_book = self.get_book(book)
-            if db_book:
-                return db_book.book_reference_id
-        elif language_selection == LanguageSelection.Application:
-            books = [key for key in list(book_names.keys()) if regex_book.match(str(book_names[key]))]
-            books = [_f for _f in map(BiblesResourcesDB.get_book, books) if _f]
-            for value in books:
-                if self.get_book_by_book_ref_id(value['id']):
-                    return value['id']
-        elif language_selection == LanguageSelection.English:
-            books = BiblesResourcesDB.get_books_like(book)
-            if books:
-                book_list = [value for value in books if regex_book.match(value['name'])]
-                if not book_list:
-                    book_list = books
-                for value in book_list:
-                    if self.get_book_by_book_ref_id(value['id']):
-                        return value['id']
-        return False
+            db_books = self.get_books(book)
+            return [db_book.book_reference_id for db_book in db_books]
+        else:
+            book_list = []
+            if language_selection == LanguageSelection.Application:
+                books = [key for key in list(book_names.keys()) if regex_book.match(book_names[key])]
+                book_list = [_f for _f in map(BiblesResourcesDB.get_book, books) if _f]
+            elif language_selection == LanguageSelection.English:
+                books = BiblesResourcesDB.get_books_like(book)
+                if books:
+                    book_list = [value for value in books if regex_book.match(value['name'])]
+                    if not book_list:
+                        book_list = books
+            return [value['id'] for value in book_list if self.get_book_by_book_ref_id(value['id'])]
+        return []
 
     def get_verses(self, reference_list, show_error=True):
         """

=== modified file 'openlp/plugins/bibles/lib/manager.py'
--- openlp/plugins/bibles/lib/manager.py	2019-07-03 13:23:23 +0000
+++ openlp/plugins/bibles/lib/manager.py	2019-08-06 21:48:11 +0000
@@ -240,8 +240,10 @@
                                                                                         book=book,
                                                                                         chapter=chapter))
         language_selection = self.get_language_selection(bible)
-        book_ref_id = self.db_cache[bible].get_book_ref_id_by_localised_name(book, language_selection)
-        return self.db_cache[bible].get_verse_count(book_ref_id, chapter)
+        book_ref_ids = self.db_cache[bible].get_book_ref_id_by_localised_name(book, language_selection)
+        if book_ref_ids:
+            return self.db_cache[bible].get_verse_count(book_ref_ids[0], chapter)
+        return 0
 
     def get_verse_count_by_book_ref_id(self, bible, book_ref_id, chapter):
         """

=== modified file 'openlp/plugins/custom/forms/editcustomdialog.py'
--- openlp/plugins/custom/forms/editcustomdialog.py	2019-04-13 13:00:22 +0000
+++ openlp/plugins/custom/forms/editcustomdialog.py	2019-08-06 21:48:11 +0000
@@ -97,6 +97,7 @@
         self.preview_button = QtWidgets.QPushButton()
         self.button_box = create_button_box(custom_edit_dialog, 'button_box', ['cancel', 'save'],
                                             [self.preview_button])
+        self.save_button = self.button_box.button(QtWidgets.QDialogButtonBox.Save)
         self.dialog_layout.addWidget(self.button_box)
         self.retranslate_ui(custom_edit_dialog)
 
@@ -112,3 +113,4 @@
         self.theme_label.setText(translate('CustomPlugin.EditCustomForm', 'The&me:'))
         self.credit_label.setText(translate('CustomPlugin.EditCustomForm', '&Credits:'))
         self.preview_button.setText(UiStrings().SaveAndPreview)
+        self.save_button.setText(UiStrings().SaveAndClose)

=== modified file 'openlp/plugins/songs/forms/editsongdialog.py'
--- openlp/plugins/songs/forms/editsongdialog.py	2019-04-13 13:00:22 +0000
+++ openlp/plugins/songs/forms/editsongdialog.py	2019-08-06 21:48:11 +0000
@@ -291,6 +291,7 @@
         self.warning_label.setObjectName('warning_label')
         self.bottom_layout.addWidget(self.warning_label)
         self.button_box = create_button_box(edit_song_dialog, 'button_box', ['cancel', 'save'])
+        self.save_button = self.button_box.button(QtWidgets.QDialogButtonBox.Save)
         self.bottom_layout.addWidget(self.button_box)
         self.dialog_layout.addLayout(self.bottom_layout)
         self.retranslate_ui(edit_song_dialog)
@@ -341,6 +342,7 @@
             translate('SongsPlugin.EditSongForm', '<strong>Warning:</strong> Not all of the verses are in use.')
         self.no_verse_order_entered_warning =  \
             translate('SongsPlugin.EditSongForm', '<strong>Warning:</strong> You have not entered a verse order.')
+        self.save_button.setText(UiStrings().SaveAndPreview)
 
 
 def create_combo_box(parent, name, editable=True):

=== modified file 'openlp/plugins/songs/lib/db.py'
--- openlp/plugins/songs/lib/db.py	2019-04-13 13:00:22 +0000
+++ openlp/plugins/songs/lib/db.py	2019-08-06 21:48:11 +0000
@@ -374,7 +374,9 @@
     mapper(SongBookEntry, songs_songbooks_table, properties={
         'songbook': relation(Book)
     })
-    mapper(Book, song_books_table)
+    mapper(Book, song_books_table, properties={
+        'songs': relation(Song, secondary=songs_songbooks_table)
+    })
     mapper(MediaFile, media_files_table)
     mapper(Song, songs_table, properties={
         # Use the authors_songs relation when you need access to the 'author_type' attribute

=== modified file 'openlp/plugins/songs/lib/importers/cclifile.py'
--- openlp/plugins/songs/lib/importers/cclifile.py	2019-04-13 13:00:22 +0000
+++ openlp/plugins/songs/lib/importers/cclifile.py	2019-08-06 21:48:11 +0000
@@ -146,7 +146,9 @@
         """
         log.debug('USR file text: {text}'.format(text=text_list))
         song_author = ''
+        song_fields = ''
         song_topics = ''
+        song_words = ''
         for line in text_list:
             if line.startswith('[S '):
                 ccli, line = line.split(']', 1)

=== modified file 'openlp/plugins/songs/lib/importers/dreambeam.py'
--- openlp/plugins/songs/lib/importers/dreambeam.py	2019-04-13 13:00:22 +0000
+++ openlp/plugins/songs/lib/importers/dreambeam.py	2019-08-06 21:48:11 +0000
@@ -87,6 +87,7 @@
                 if self.stop_import_flag:
                     return
                 self.set_defaults()
+                author_copyright = ''
                 parser = etree.XMLParser(remove_blank_text=True)
                 try:
                     with file_path.open('r') as xml_file:
@@ -142,7 +143,7 @@
                         author_copyright = song_xml.Text2.Text.text
                 if author_copyright:
                     author_copyright = str(author_copyright)
-                    if author_copyright.find(str(SongStrings.CopyrightSymbol)) >= 0:
+                    if author_copyright.find(SongStrings.CopyrightSymbol) >= 0:
                         self.add_copyright(author_copyright)
                     else:
                         self.parse_author(author_copyright)

=== modified file 'openlp/plugins/songs/lib/importers/easyslides.py'
--- openlp/plugins/songs/lib/importers/easyslides.py	2019-04-13 13:00:22 +0000
+++ openlp/plugins/songs/lib/importers/easyslides.py	2019-08-06 21:48:11 +0000
@@ -137,9 +137,11 @@
         except UnicodeDecodeError:
             log.exception('Unicode decode error while decoding Contents')
             self._success = False
+            return
         except AttributeError:
             log.exception('no Contents')
             self._success = False
+            return
         lines = lyrics.split('\n')
         # we go over all lines first, to determine information,
         # which tells us how to parse verses later

=== modified file 'openlp/plugins/songs/lib/importers/easyworship.py'
--- openlp/plugins/songs/lib/importers/easyworship.py	2019-07-03 13:23:23 +0000
+++ openlp/plugins/songs/lib/importers/easyworship.py	2019-08-06 21:48:11 +0000
@@ -268,13 +268,13 @@
         self.db_set_record_struct(field_descriptions)
         # Pick out the field description indexes we will need
         try:
-            success = True
             fi_title = self.db_find_field(b'Title')
             fi_author = self.db_find_field(b'Author')
             fi_copy = self.db_find_field(b'Copyright')
             fi_admin = self.db_find_field(b'Administrator')
             fi_words = self.db_find_field(b'Words')
             fi_ccli = self.db_find_field(b'Song Number')
+            success = True
         except IndexError:
             # This is the wrong table
             success = False

=== modified file 'openlp/plugins/songs/lib/importers/songbeamer.py'
--- openlp/plugins/songs/lib/importers/songbeamer.py	2019-05-22 06:47:00 +0000
+++ openlp/plugins/songs/lib/importers/songbeamer.py	2019-08-06 21:48:11 +0000
@@ -128,7 +128,7 @@
                 # The encoding should only be ANSI (cp1252), UTF-8, Unicode, Big-Endian-Unicode.
                 # So if it doesn't start with 'u' we default to cp1252. See:
                 # https://forum.songbeamer.com/viewtopic.php?p=419&sid=ca4814924e37c11e4438b7272a98b6f2
-                if not self.input_file_encoding.lower().startswith('u'):
+                if self.input_file_encoding and not self.input_file_encoding.lower().startswith('u'):
                     self.input_file_encoding = 'cp1252'
                 with file_path.open(encoding=self.input_file_encoding) as song_file:
                     song_data = song_file.readlines()

=== modified file 'tests/functional/openlp_core/common/test_common.py'
--- tests/functional/openlp_core/common/test_common.py	2019-05-22 06:47:00 +0000
+++ tests/functional/openlp_core/common/test_common.py	2019-08-06 21:48:11 +0000
@@ -26,7 +26,7 @@
 from unittest import TestCase
 from unittest.mock import MagicMock, call, patch
 
-from openlp.core.common import clean_button_text, de_hump, extension_loader, is_linux, is_macosx, is_win, \
+from openlp.core.common import Singleton, clean_button_text, de_hump, extension_loader, is_linux, is_macosx, is_win, \
     normalize_str, path_to_module, trace_error_handler
 
 
@@ -163,6 +163,48 @@
             mocked_logger.error.assert_called_with(
                 'OpenLP Error trace\n   File openlp.fake at line 56 \n\t called trace_error_handler_test')
 
+    def test_singleton_metaclass_multiple_init(self):
+        """
+        Test that a class using the Singleton Metaclass is only initialised once despite being called several times and
+        that the same instance is returned each time..
+        """
+        # GIVEN: The Singleton Metaclass and a test class using it
+        class SingletonClass(metaclass=Singleton):
+            def __init__(self):
+                pass
+
+        with patch.object(SingletonClass, '__init__', return_value=None) as patched_init:
+
+            # WHEN: Initialising the class multiple times
+            inst_1 = SingletonClass()
+            inst_2 = SingletonClass()
+
+        # THEN: The __init__ method of the SingletonClass should have only been called once, and both returned values
+        #       should be the same instance.
+        assert inst_1 is inst_2
+        assert patched_init.call_count == 1
+
+    def test_singleton_metaclass_multiple_classes(self):
+        """
+        Test that multiple classes using the Singleton Metaclass return the different an appropriate instances.
+        """
+        # GIVEN: Two different classes using the Singleton Metaclass
+        class SingletonClass1(metaclass=Singleton):
+            def __init__(self):
+                pass
+
+        class SingletonClass2(metaclass=Singleton):
+            def __init__(self):
+                pass
+
+        # WHEN: Initialising both classes
+        s_c1 = SingletonClass1()
+        s_c2 = SingletonClass2()
+
+        # THEN: The instances  should be an instance of the appropriate class
+        assert isinstance(s_c1, SingletonClass1)
+        assert isinstance(s_c2, SingletonClass2)
+
     def test_is_win(self):
         """
         Test the is_win() function

=== modified file 'tests/functional/openlp_core/lib/test_theme.py'
--- tests/functional/openlp_core/lib/test_theme.py	2019-07-18 19:14:58 +0000
+++ tests/functional/openlp_core/lib/test_theme.py	2019-08-06 21:48:11 +0000
@@ -182,4 +182,4 @@
         assert 0 == theme.display_vertical_align, 'display_vertical_align should be 0'
         assert theme.font_footer_bold is False, 'font_footer_bold should be False'
         assert 'Arial' == theme.font_main_name, 'font_main_name should be "Arial"'
-        assert 47 == len(theme.__dict__), 'The theme should have 47 attributes'
+        assert 48 == len(theme.__dict__), 'The theme should have 48 attributes'

=== modified file 'tests/functional/openlp_core/ui/test_icons.py'
--- tests/functional/openlp_core/ui/test_icons.py	2019-04-13 13:00:22 +0000
+++ tests/functional/openlp_core/ui/test_icons.py	2019-08-06 21:48:11 +0000
@@ -33,7 +33,7 @@
 
 class TestIcons(TestCase, TestMixin):
 
-    @patch('openlp.core.ui.icons.UiIcons.load')
+    @patch('openlp.core.ui.icons.UiIcons.__init__', return_value=None)
     def test_simple_icon(self, _):
         # GIVEN: an basic set of icons
         icons = UiIcons()