Merge branch 'import-fixes' into 'master'

Import fixes

See merge request openlp/openlp!123

appveyor.yml

@@ -29,7 +29,7 @@ test_script:
 after_test:
   # This is where we create a package using PyInstaller
   # Install PyInstaller
-  - "%PYTHON%\\python.exe -m pip install pyinstaller"
+  - "%PYTHON%\\python.exe -m pip install pyinstaller==3.5"
   # Disabled portable installers - can't figure out how to make them silent
   # - curl -L -O http://downloads.sourceforge.net/project/portableapps/PortableApps.com%20Installer/PortableApps.comInstaller_3.4.4.paf.exe
   # - PortableApps.comInstaller_3.4.4.paf.exe /S

openlp/core/common/settings.py

@@ -306,7 +306,7 @@ class Settings(QtCore.QSettings):
         'songs/db hostname': '',
         'songs/db database': '',
         'songs/last used search type': SongSearch.Entire,
-        'songs/last import type': None,
+        'songs/last import type': 0,
         'songs/update service on edit': False,
         'songs/add song from service': True,
         'songs/add songbook slide': False,

openlp/core/ui/style.py

@@ -21,10 +21,9 @@
 """
 The :mod:`~openlp.core.ui.dark` module looks for and loads a dark theme
 """
-from PyQt5 import QtGui
+from PyQt5 import QtGui, QtWidgets
 
 from openlp.core.common import is_win
-from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 
 
@@ -88,7 +87,7 @@ def get_application_stylesheet():
         stylesheet = qdarkstyle.load_stylesheet_pyqt5()
     else:
         if not Settings().value('advanced/alternate rows'):
-            base_color = Registry().get('application').palette().color(QtGui.QPalette.Active, QtGui.QPalette.Base)
+            base_color = QtWidgets.QApplication.palette().color(QtGui.QPalette.Active, QtGui.QPalette.Base)
             alternate_rows_repair_stylesheet = \
                 'QTableWidget, QListWidget, QTreeWidget {alternate-background-color: ' + base_color.name() + ';}\n'
             stylesheet += alternate_rows_repair_stylesheet

openlp/plugins/bibles/forms/bibleimportform.py

@@ -40,6 +40,7 @@ from openlp.core.common.settings import Settings
 from openlp.core.lib.db import delete_database
 from openlp.core.lib.exceptions import ValidationError
 from openlp.core.lib.ui import critical_error_message_box
+from openlp.core.widgets.enums import PathEditType
 from openlp.core.widgets.edits import PathEdit
 from openlp.core.widgets.wizard import OpenLPWizard, WizardStrings
 from openlp.plugins.bibles.lib.db import clean_filename
@@ -276,6 +277,7 @@ class BibleImportForm(OpenLPWizard):
         self.sword_folder_label.setObjectName('SwordFolderLabel')
         self.sword_folder_path_edit = PathEdit(
             self.sword_folder_tab,
+            path_type=PathEditType.Directories,
             default_path=Settings().value('bibles/last directory import'),
             dialog_caption=WizardStrings.OpenTypeFile.format(file_type=WizardStrings.SWORD),
             show_revert=False,
@@ -502,6 +504,11 @@ class BibleImportForm(OpenLPWizard):
                         self.sword_folder_path_edit.setFocus()
                         return False
                     key = self.sword_bible_combo_box.itemData(self.sword_bible_combo_box.currentIndex())
+                    if not key:
+                        critical_error_message_box(UiStrings().NFSs,
+                                                   WizardStrings.YouSpecifyFolder % WizardStrings.SWORD)
+                        self.sword_folder_path_edit.setFocus()
+                        return False
                     if 'description' in self.pysword_folder_modules_json[key]:
                         self.version_name_edit.setText(self.pysword_folder_modules_json[key]['description'])
                     if 'distributionlicense' in self.pysword_folder_modules_json[key]:

openlp/plugins/bibles/lib/importers/csvbible.py

@@ -49,6 +49,7 @@ There are two acceptable formats of the verses file.  They are:
 All CSV files are expected to use a comma (',') as the delimiter and double quotes ('"') as the quote symbol.
 """
 import csv
+import logging
 from collections import namedtuple
 
 from openlp.core.common import get_file_encoding
@@ -56,6 +57,7 @@ from openlp.core.common.i18n import translate
 from openlp.core.lib.exceptions import ValidationError
 from openlp.plugins.bibles.lib.bibleimport import BibleImport
 
+log = logging.getLogger(__name__)
 
 Book = namedtuple('Book', 'id, testament_id, name, abbreviation')
 Verse = namedtuple('Verse', 'book_id_name, chapter_number, number, text')
@@ -105,7 +107,8 @@ class CSVBible(BibleImport):
             with file_path.open('r', encoding=encoding, newline='') as csv_file:
                 csv_reader = csv.reader(csv_file, delimiter=',', quotechar='"')
                 return [results_tuple(*line) for line in csv_reader]
-        except (OSError, csv.Error):
+        except (OSError, csv.Error, TypeError, UnicodeDecodeError):
+            log.exception('Parsing {file} failed.'.format(file=file_path))
             raise ValidationError(msg='Parsing "{file}" failed'.format(file=file_path))
 
     def process_books(self, books):

openlp/plugins/bibles/lib/importers/wordproject.py

@@ -22,10 +22,12 @@ import logging
 import re
 from pathlib import Path
 from tempfile import TemporaryDirectory
-from zipfile import ZipFile
+from zipfile import ZipFile, BadZipFile
 
 from bs4 import BeautifulSoup, NavigableString, Tag
 
+from openlp.core.common.i18n import translate
+from openlp.core.lib.ui import critical_error_message_box
 from openlp.plugins.bibles.lib.bibleimport import BibleImport
 
 
@@ -50,18 +52,30 @@ class WordProjectBible(BibleImport):
         Unzip the file to a temporary directory
         """
         self.tmp = TemporaryDirectory()
-        with ZipFile(self.file_path) as zip_file:
+        try:
-            zip_file.extractall(self.tmp.name)
+            with ZipFile(self.file_path) as zip_file:
+                zip_file.extractall(self.tmp.name)
+        except BadZipFile:
+            self.log_exception('Extracting {file} failed.'.format(file=self.file_path))
+            critical_error_message_box(message=translate('BiblesPlugin.WordProjectBible',
+                                                         'Incorrect Bible file type, not a Zip file.'))
+            return False
         self.base_path = Path(self.tmp.name, self.file_path.stem)
+        return True
 
     def process_books(self):
         """
         Extract and create the bible books from the parsed html
 
         :param bible_data: parsed xml
-        :return: None
+        :return: True if books was parsed, otherwise False
         """
-        page = (self.base_path / 'index.htm').read_text(encoding='utf-8', errors='ignore')
+        idx_file = (self.base_path / 'index.htm')
+        if not idx_file.exists():
+            critical_error_message_box(message=translate('BiblesPlugin.WordProjectBible',
+                                                         'Incorrect Bible file type, files are missing.'))
+            return False
+        page = idx_file.read_text(encoding='utf-8', errors='ignore')
         soup = BeautifulSoup(page, 'lxml')
         bible_books = soup.find('div', 'textOptions').find_all('li')
         book_count = len(bible_books)
@@ -81,6 +95,7 @@ class WordProjectBible(BibleImport):
             db_book = self.find_and_create_book(book_name, book_count, self.language_id, book_id)
             self.process_chapters(db_book, book_id, book_link)
             self.session.commit()
+        return True
 
     def process_chapters(self, db_book, book_id, book_link):
         """
@@ -154,11 +169,11 @@ class WordProjectBible(BibleImport):
         Loads a Bible from file.
         """
         self.log_debug('Starting WordProject import from "{name}"'.format(name=self.file_path))
-        self._unzip_file()
+        if not self._unzip_file():
+            return False
         self.language_id = self.get_language_id(None, bible_name=str(self.file_path))
         result = False
         if self.language_id:
-            self.process_books()
+            result = self.process_books()
-            result = True
         self._cleanup()
         return result

openlp/plugins/bibles/lib/importers/zefania.py

@@ -54,7 +54,12 @@ class ZefaniaBible(BibleImport):
             if not language_id:
                 return False
             no_of_books = int(xmlbible.xpath('count(//BIBLEBOOK)'))
-            self.wizard.progress_bar.setMaximum(int(xmlbible.xpath('count(//CHAPTER)')))
+            no_of_chap = int(xmlbible.xpath('count(//CHAPTER)'))
+            if not no_of_books or not no_of_chap:
+                critical_error_message_box(message=translate('BiblesPlugin.ZefaniaImport',
+                                                             'Incorrect Bible file type. Expected data is missing.'))
+                return False
+            self.wizard.progress_bar.setMaximum(no_of_chap)
             for BIBLEBOOK in xmlbible:
                 if self.stop_import_flag:
                     break

openlp/plugins/songs/forms/songimportform.py

@@ -396,7 +396,10 @@ class SongImportForm(OpenLPWizard, RegistryProperties):
                 dialog_caption = WizardStrings.OpenTypeFolder.format(folder_name=format_name)
             path_edit = PathEdit(
                 parent=import_widget, path_type=path_type, dialog_caption=dialog_caption, show_revert=False)
-            path_edit.filters = path_edit.filters + filters
+            if path_edit.filters:
+                path_edit.filters = filters + ';;' + path_edit.filters
+            else:
+                path_edit.filters = filters
             path_edit.path = Settings().value(self.plugin.settings_section + '/last directory import')
             file_path_layout.addWidget(path_edit)
             import_layout.addLayout(file_path_layout)

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

@@ -66,11 +66,16 @@ class CCLIFileImport(SongImport):
                     except UnicodeDecodeError:
                         details = chardet.detect(detect_content)
                 in_file = codecs.open(file_path, 'r', details['encoding'])
-                if not in_file.read(1) == '\ufeff':
+                try:
-                    # not UTF or no BOM was found
+                    if not in_file.read(1) == '\ufeff':
-                    in_file.seek(0)
+                        # not UTF or no BOM was found
-                lines = in_file.readlines()
+                        in_file.seek(0)
-                in_file.close()
+                    lines = in_file.readlines()
+                    in_file.close()
+                except UnicodeDecodeError:
+                    self.log_error(file_path, translate('SongsPlugin.CCLIFileImport',
+                                                        'The file contains unreadable characters.'))
+                    continue
                 ext = file_path.suffix.lower()
                 if ext == '.usr' or ext == '.bin':
                     log.info('SongSelect USR format file found: {name}'.format(name=file_path))

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

@@ -25,6 +25,7 @@ ChordPro files into the current database.
 import logging
 import re
 
+from openlp.core.common.i18n import translate
 from openlp.core.common.settings import Settings
 from openlp.plugins.songs.lib.importers.songimport import SongImport
 from openlp.plugins.songs.lib.db import AuthorType
@@ -60,7 +61,12 @@ class ChordProImport(SongImport):
         """
         self.set_defaults()
         # Loop over the lines of the file
-        file_content = song_file.read()
+        try:
+            file_content = song_file.read()
+        except UnicodeDecodeError:
+            self.log_error(song_file.name, translate('SongsPlugin.CCLIFileImport',
+                                                     'The file contains unreadable characters.'))
+            return
         current_verse = ''
         current_verse_type = 'v'
         skip_block = False
@@ -181,7 +187,7 @@ class ChordProImport(SongImport):
                 current_verse = re.sub(r'\[.*?\]', '', current_verse)
             self.add_verse(current_verse.rstrip(), current_verse_type)
         # if no title was in directives, get it from the first line
-        if not self.title:
+        if not self.title and self.verses:
             (verse_def, verse_text, lang) = self.verses[0]
             # strip any chords from the title
             self.title = re.sub(r'\[.*?\]', '', verse_text.split('\n')[0])

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

@@ -87,21 +87,30 @@ class DreamBeamImport(SongImport):
                     return
                 self.set_defaults()
                 author_copyright = ''
-                parser = etree.XMLParser(remove_blank_text=True)
+                parser = etree.XMLParser(remove_blank_text=True, recover=True)
                 try:
                     with file_path.open('r') as xml_file:
                         parsed_file = etree.parse(xml_file, parser)
                 except etree.XMLSyntaxError:
-                    log.exception('XML syntax error in file_path {name}'.format(name=file_path))
+                    log.exception('XML syntax error in file {name}'.format(name=file_path))
                     self.log_error(file_path, SongStrings.XMLSyntaxError)
                     continue
-                xml = etree.tostring(parsed_file).decode()
+                except UnicodeDecodeError:
+                    log.exception('Unreadable characters in {name}'.format(name=file_path))
+                    self.log_error(file_path, SongStrings.XMLSyntaxError)
+                    continue
+                file_str = etree.tostring(parsed_file)
+                if not file_str:
+                    log.exception('Could not find XML in file {name}'.format(name=file_path))
+                    self.log_error(file_path, SongStrings.XMLSyntaxError)
+                    continue
+                xml = file_str.decode()
                 song_xml = objectify.fromstring(xml)
                 if song_xml.tag != 'DreamSong':
                     self.log_error(
                         file_path,
                         translate('SongsPlugin.DreamBeamImport',
-                                  'Invalid DreamBeam song file_path. Missing DreamSong tag.'))
+                                  'Invalid DreamBeam song file. Missing DreamSong tag.'))
                     continue
                 if hasattr(song_xml, 'Version'):
                     self.version = float(song_xml.Version.text)

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

@@ -25,9 +25,10 @@ import re
 from lxml import etree, objectify
 
 from openlp.core.common import normalize_str
+from openlp.core.common.i18n import translate
 from openlp.plugins.songs.lib import VerseType
 from openlp.plugins.songs.lib.importers.songimport import SongImport
-
+from openlp.plugins.songs.lib.ui import SongStrings
 
 log = logging.getLogger(__name__)
 
@@ -48,10 +49,32 @@ class EasySlidesImport(SongImport):
     def do_import(self):
         log.info('Importing EasySlides XML file {source}'.format(source=self.import_source))
         parser = etree.XMLParser(remove_blank_text=True, recover=True)
-        parsed_file = etree.parse(str(self.import_source), parser)
+        try:
-        xml = etree.tostring(parsed_file).decode()
+            with self.import_source.open('r') as xml_file:
+                parsed_file = etree.parse(str(self.import_source), parser)
+        except etree.XMLSyntaxError:
+            log.exception('XML syntax error in file {name}'.format(name=xml_file))
+            self.log_error(self.import_source, SongStrings.XMLSyntaxError)
+            return
+        except UnicodeDecodeError:
+            log.exception('Unreadable characters in {name}'.format(name=xml_file))
+            self.log_error(self.import_source, SongStrings.XMLSyntaxError)
+            return
+        file_str = etree.tostring(parsed_file)
+        if not file_str:
+            log.exception('Could not find XML in file {name}'.format(name=xml_file))
+            self.log_error(self.import_source, SongStrings.XMLSyntaxError)
+            return
+        xml = file_str.decode()
         song_xml = objectify.fromstring(xml)
-        self.import_wizard.progress_bar.setMaximum(len(song_xml.Item))
+        try:
+            item_count = len(song_xml.Item)
+        except AttributeError:
+            log.exception('No root attribute "Item"')
+            self.log_error(self.import_source, translate('SongsPlugin.EasySlidesImport',
+                                                         'Invalid EasySlides song file. Missing Item tag.'))
+            return
+        self.import_wizard.progress_bar.setMaximum(item_count)
         for song in song_xml.Item:
             if self.stop_import_flag:
                 return

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

@@ -78,12 +78,17 @@ class EasyWorshipSongImport(SongImport):
         """
         self.import_source = Path(self.import_source)
         ext = self.import_source.suffix.lower()
-        if ext == '.ews':
+        try:
-            self.import_ews()
+            if ext == '.ews':
-        elif ext == '.db':
+                self.import_ews()
-            self.import_db()
+            elif ext == '.db':
-        else:
+                self.import_db()
-            self.import_sqlite_db()
+            else:
+                self.import_sqlite_db()
+        except Exception:
+            log.exception('Unexpected data in file {name}'.format(name=self.import_source))
+            self.log_error(self.import_source,
+                           '{name} contains unexpected data and can not be imported'.format(name=self.import_source))
 
     def import_ews(self):
         """

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

@@ -127,6 +127,10 @@ class FoilPresenterImport(SongImport):
             except etree.XMLSyntaxError:
                 self.log_error(file_path, SongStrings.XMLSyntaxError)
                 log.exception('XML syntax error in file {path}'.format(path=file_path))
+            except AttributeError:
+                self.log_error(file_path, translate('SongsPlugin.FoilPresenterSongImport',
+                                                    'Invalid Foilpresenter song file. Missing expected tags'))
+                log.exception('Missing content in file {path}'.format(path=file_path))
 
 
 class FoilPresenter(object):

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

@@ -130,13 +130,18 @@ class OpenLPSongImport(SongImport):
         else:
             has_authors_songs = False
         # Load up the tabls and map them out
-        source_authors_table = source_meta.tables['authors']
+        try:
-        source_song_books_table = source_meta.tables['song_books']
+            source_authors_table = source_meta.tables['authors']
-        source_songs_table = source_meta.tables['songs']
+            source_song_books_table = source_meta.tables['song_books']
-        source_topics_table = source_meta.tables['topics']
+            source_songs_table = source_meta.tables['songs']
-        source_authors_songs_table = source_meta.tables['authors_songs']
+            source_topics_table = source_meta.tables['topics']
-        source_songs_topics_table = source_meta.tables['songs_topics']
+            source_authors_songs_table = source_meta.tables['authors_songs']
-        source_media_files_songs_table = None
+            source_songs_topics_table = source_meta.tables['songs_topics']
+            source_media_files_songs_table = None
+        except KeyError:
+            self.log_error(self.import_source, translate('SongsPlugin.OpenLPSongImport',
+                                                         'Not a valid OpenLP 2 song database.'))
+            return
         # Set up media_files relations
         if has_media_files:
             source_media_files_table = source_meta.tables['media_files']

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

@@ -58,7 +58,7 @@ class OPSProImport(SongImport):
         try:
             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:
+        except Exception as e:
             log.warning('Unable to connect the OPS Pro database {source}. {error}'.format(source=self.import_source,
                                                                                           error=str(e)))
             # Unfortunately no specific exception type

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

@@ -22,10 +22,16 @@
 The :mod:`powerpraiseimport` module provides the functionality for importing
 Powerpraise song files into the current database.
 """
-from lxml import objectify
+import logging
 
+from lxml import objectify, etree
+
+from openlp.core.common.i18n import translate
 from openlp.core.widgets.wizard import WizardStrings
 from openlp.plugins.songs.lib.importers.songimport import SongImport
+from openlp.plugins.songs.lib.ui import SongStrings
+
+log = logging.getLogger(__name__)
 
 
 class PowerPraiseImport(SongImport):
@@ -40,43 +46,56 @@ class PowerPraiseImport(SongImport):
                 return
             self.import_wizard.increment_progress_bar(WizardStrings.ImportingType.format(source=file_path.name))
             with file_path.open('rb') as xml_file:
-                root = objectify.parse(xml_file).getroot()
+                try:
-                self.process_song(root)
+                    root = objectify.parse(xml_file).getroot()
+                except etree.XMLSyntaxError:
+                    log.exception('XML syntax error in file {name}'.format(name=file_path))
+                    self.log_error(file_path, SongStrings.XMLSyntaxError)
+                    continue
+                except UnicodeDecodeError:
+                    log.exception('Unreadable characters in {name}'.format(name=file_path))
+                    self.log_error(file_path, SongStrings.XMLSyntaxError)
+                    continue
+                self.process_song(root, file_path)
 
-    def process_song(self, root):
+    def process_song(self, root, file_path):
         self.set_defaults()
-        self.title = str(root.general.title)
+        try:
-        verse_order_list = []
+            self.title = str(root.general.title)
-        verse_count = {}
+            verse_order_list = []
-        for item in root.order.item:
+            verse_count = {}
-            verse_order_list.append(str(item))
+            for item in root.order.item:
-        for part in root.songtext.part:
+                verse_order_list.append(str(item))
-            original_verse_def = part.get('caption')
+            for part in root.songtext.part:
-            # There are some predefined verse defitions in PowerPraise, try to parse these
+                original_verse_def = part.get('caption')
-            if original_verse_def.startswith("Strophe") or original_verse_def.startswith("Teil"):
+                # There are some predefined verse defitions in PowerPraise, try to parse these
-                verse_def = 'v'
+                if original_verse_def.startswith("Strophe") or original_verse_def.startswith("Teil"):
-            elif original_verse_def.startswith("Refrain"):
+                    verse_def = 'v'
-                verse_def = 'c'
+                elif original_verse_def.startswith("Refrain"):
-            elif original_verse_def.startswith("Bridge"):
+                    verse_def = 'c'
-                verse_def = 'b'
+                elif original_verse_def.startswith("Bridge"):
-            elif original_verse_def.startswith("Schluss"):
+                    verse_def = 'b'
-                verse_def = 'e'
+                elif original_verse_def.startswith("Schluss"):
-            else:
+                    verse_def = 'e'
-                verse_def = 'o'
+                else:
-            verse_count[verse_def] = verse_count.get(verse_def, 0) + 1
+                    verse_def = 'o'
-            verse_def = '{verse}{count:d}'.format(verse=verse_def, count=verse_count[verse_def])
+                verse_count[verse_def] = verse_count.get(verse_def, 0) + 1
-            verse_text = []
+                verse_def = '{verse}{count:d}'.format(verse=verse_def, count=verse_count[verse_def])
-            for slide in part.slide:
+                verse_text = []
-                if not hasattr(slide, 'line'):
+                for slide in part.slide:
-                    continue  # No content
+                    if not hasattr(slide, 'line'):
-                for line in slide.line:
+                        continue  # No content
-                    verse_text.append(str(line))
+                    for line in slide.line:
-            self.add_verse('\n'.join(verse_text), verse_def)
+                        verse_text.append(str(line))
-            # Update verse name in verse order list
+                self.add_verse('\n'.join(verse_text), verse_def)
-            for i in range(len(verse_order_list)):
+                # Update verse name in verse order list
-                if verse_order_list[i].lower() == original_verse_def.lower():
+                for i in range(len(verse_order_list)):
-                    verse_order_list[i] = verse_def
+                    if verse_order_list[i].lower() == original_verse_def.lower():
+                        verse_order_list[i] = verse_def
 
-        self.verse_order_list = verse_order_list
+            self.verse_order_list = verse_order_list
-        if not self.finish():
+            if not self.finish():
-            self.log_error(self.import_source)
+                self.log_error(file_path)
+        except AttributeError:
+            self.log_error(file_path, translate('SongsPlugin.PowerPraiseImport',
+                                                'Invalid PowerPraise song file. Missing needed tag.'))

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

@@ -22,6 +22,7 @@
 The :mod:`presentationmanager` module provides the functionality for importing
 Presentationmanager song files into the current database.
 """
+import logging
 import re
 
 from lxml import etree, objectify
@@ -31,6 +32,8 @@ from openlp.core.common.i18n import translate
 from openlp.core.widgets.wizard import WizardStrings
 from openlp.plugins.songs.lib.importers.songimport import SongImport
 
+log = logging.getLogger(__name__)
+
 
 class PresentationManagerImport(SongImport):
     """
@@ -54,12 +57,24 @@ class PresentationManagerImport(SongImport):
                 try:
                     tree = etree.fromstring(text, parser=etree.XMLParser(recover=True))
                 except ValueError:
+                    log.exception('XML syntax error in file {name}'.format(name=file_path))
                     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))
+            file_str = etree.tostring(tree)
-            self.process_song(root, file_path)
+            if not file_str:
+                log.exception('Could not find XML in file {name}'.format(name=file_path))
+                self.log_error(file_path, translate('SongsPlugin.PresentationManagerImport',
+                                                    'File is not in XML-format, which is the only format supported.'))
+                continue
+            root = objectify.fromstring(file_str)
+            try:
+                self.process_song(root, file_path)
+            except AttributeError:
+                log.exception('XML syntax error in file {name}'.format(name=file_path))
+                self.log_error(file_path, translate('SongsPlugin.PresentationManagerImport',
+                                                    'File is not a valid PresentationManager XMl file.'))
 
     def _get_attr(self, elem, name):
         """

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

@@ -25,11 +25,13 @@ ProPresenter song files into the current installation database.
 import base64
 import logging
 
-from lxml import objectify
+from lxml import objectify, etree
 
+from openlp.core.common.i18n import translate
 from openlp.core.widgets.wizard import WizardStrings
 from openlp.plugins.songs.lib import strip_rtf
 from openlp.plugins.songs.lib.importers.songimport import SongImport
+from openlp.plugins.songs.lib.ui import SongStrings
 
 
 log = logging.getLogger(__name__)
@@ -48,8 +50,23 @@ class ProPresenterImport(SongImport):
             self.import_wizard.increment_progress_bar(
                 WizardStrings.ImportingType.format(source=file_path.name))
             with file_path.open('rb') as xml_file:
-                root = objectify.parse(xml_file).getroot()
+                try:
-                self.process_song(root, file_path)
+                    root = objectify.parse(xml_file).getroot()
+                except etree.XMLSyntaxError:
+                    log.exception('XML syntax error in file {name}'.format(name=file_path))
+                    self.log_error(file_path, SongStrings.XMLSyntaxError)
+                    continue
+                except UnicodeDecodeError:
+                    log.exception('Unreadable characters in {name}'.format(name=file_path))
+                    self.log_error(file_path, SongStrings.XMLSyntaxError)
+                    continue
+                try:
+                    self.process_song(root, file_path)
+                except AttributeError:
+                    log.exception('XML syntax error in file {name}'.format(name=file_path))
+                    self.log_error(file_path, translate('SongsPlugin.ProPresenterImport',
+                                                        'File is not a valid ProPresenter XMl file.'))
+                    continue
 
     def process_song(self, root, file_path):
         """
@@ -62,7 +79,7 @@ class ProPresenterImport(SongImport):
         # Extract ProPresenter versionNumber
         try:
             self.version = int(root.get('versionNumber'))
-        except ValueError:
+        except (ValueError, TypeError):
             log.debug('ProPresenter versionNumber invalid or missing')
             return
 

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

@@ -29,6 +29,7 @@ import re
 from pathlib import Path
 
 from openlp.core.common import get_file_encoding, is_macosx, is_win
+from openlp.core.common.i18n import translate
 from openlp.core.common.settings import Settings
 from openlp.plugins.songs.lib import VerseType
 from openlp.plugins.songs.lib.importers.songimport import SongImport
@@ -130,7 +131,13 @@ class SongBeamerImport(SongImport):
                 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()
+                    try:
+                        song_data = song_file.readlines()
+                    except UnicodeDecodeError:
+                        log.exception('Unreadable characters in {name}'.format(name=file_path))
+                        self.log_error(file_path, translate('SongsPlugin.SongBeamerImport',
+                                                            'File is not a valid SongBeamer file.'))
+                        continue
             else:
                 continue
             self.title = file_path.stem

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

@@ -22,12 +22,16 @@
 The :mod:`songpro` module provides the functionality for importing SongPro
 songs into the OpenLP database.
 """
+import logging
 import re
 from pathlib import Path
 
+from openlp.core.common.i18n import translate
 from openlp.plugins.songs.lib import strip_rtf
 from openlp.plugins.songs.lib.importers.songimport import SongImport
 
+log = logging.getLogger(__name__)
+
 
 class SongProImport(SongImport):
     """
@@ -82,7 +86,13 @@ class SongProImport(SongImport):
                     break
                 file_text = file_line.rstrip()
                 if file_text and file_text[0] == '#':
-                    self.process_section(tag, text.rstrip())
+                    try:
+                        self.process_section(tag, text.rstrip())
+                    except ValueError:
+                        log.exception('Missing data in {name}'.format(name=self.import_source))
+                        self.log_error(self.import_source, translate('SongsPlugin.SongProImport',
+                                                                     'File is not a valid SongPro file.'))
+                        return
                     tag = file_text[1:]
                     text = ''
                 else:

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

@@ -26,6 +26,7 @@ import logging
 import re
 import struct
 
+from openlp.core.common.i18n import translate
 from openlp.core.widgets.wizard import WizardStrings
 from openlp.plugins.songs.lib import VerseType, retrieve_windows_encoding
 from openlp.plugins.songs.lib.importers.songimport import SongImport
@@ -100,84 +101,91 @@ class SongShowPlusImport(SongImport):
             self.other_list = {}
             self.import_wizard.increment_progress_bar(WizardStrings.ImportingType.format(source=file_path.name), 0)
             with file_path.open('rb') as song_file:
-                while True:
+                try:
-                    block_key, = struct.unpack("I", song_file.read(4))
+                    while True:
-                    log.debug('block_key: %d' % block_key)
+                        block_key, = struct.unpack("I", song_file.read(4))
-                    # The file ends with 4 NULL's
+                        log.debug('block_key: %d' % block_key)
-                    if block_key == 0:
+                        # The file ends with 4 NULL's
-                        break
+                        if block_key == 0:
-                    next_block_starts, = struct.unpack("I", song_file.read(4))
+                            break
-                    next_block_starts += song_file.tell()
+                        next_block_starts, = struct.unpack("I", song_file.read(4))
-                    if block_key in (VERSE, CHORUS, BRIDGE):
+                        next_block_starts += song_file.tell()
-                        null, verse_no, = struct.unpack("BB", song_file.read(2))
+                        if block_key in (VERSE, CHORUS, BRIDGE):
-                    elif block_key == CUSTOM_VERSE:
+                            null, verse_no, = struct.unpack("BB", song_file.read(2))
-                        null, verse_name_length, = struct.unpack("BB", song_file.read(2))
+                        elif block_key == CUSTOM_VERSE:
-                        verse_name = self.decode(song_file.read(verse_name_length))
+                            null, verse_name_length, = struct.unpack("BB", song_file.read(2))
-                    length_descriptor_size, = struct.unpack("B", song_file.read(1))
+                            verse_name = self.decode(song_file.read(verse_name_length))
-                    log.debug('length_descriptor_size: %d' % length_descriptor_size)
+                        length_descriptor_size, = struct.unpack("B", song_file.read(1))
-                    # In the case of song_numbers the number is in the data from the
+                        log.debug('length_descriptor_size: %d' % length_descriptor_size)
-                    # current position to the next block starts
+                        # In the case of song_numbers the number is in the data from the
-                    if block_key == SONG_NUMBER:
+                        # current position to the next block starts
-                        sn_bytes = song_file.read(length_descriptor_size - 1)
+                        if block_key == SONG_NUMBER:
-                        self.song_number = int.from_bytes(sn_bytes, byteorder='little')
+                            sn_bytes = song_file.read(length_descriptor_size - 1)
-                        continue
+                            self.song_number = int.from_bytes(sn_bytes, byteorder='little')
-                    # Detect if/how long the length descriptor is
+                            continue
-                    if length_descriptor_size == 12 or length_descriptor_size == 20:
+                        # Detect if/how long the length descriptor is
-                        length_descriptor, = struct.unpack("I", song_file.read(4))
+                        if length_descriptor_size == 12 or length_descriptor_size == 20:
-                    elif length_descriptor_size == 2:
+                            length_descriptor, = struct.unpack("I", song_file.read(4))
-                        length_descriptor = 1
+                        elif length_descriptor_size == 2:
-                    elif length_descriptor_size == 9:
+                            length_descriptor = 1
-                        length_descriptor = 0
+                        elif length_descriptor_size == 9:
-                    else:
+                            length_descriptor = 0
-                        length_descriptor, = struct.unpack("B", song_file.read(1))
-                    log.debug('length_descriptor: %d' % length_descriptor)
-                    data = song_file.read(length_descriptor)
-                    log.debug(data)
-                    if block_key == TITLE:
-                        self.title = self.decode(data)
-                    elif block_key == AUTHOR:
-                        authors = self.decode(data).split(" / ")
-                        for author in authors:
-                            if author.find(",") != -1:
-                                author_parts = author.split(", ")
-                                author = author_parts[1] + " " + author_parts[0]
-                            self.parse_author(author)
-                    elif block_key == COPYRIGHT:
-                        self.add_copyright(self.decode(data))
-                    elif block_key == CCLI_NO:
-                        # Try to get the CCLI number even if the field contains additional text
-                        match = re.search(r'\d+', self.decode(data))
-                        if match:
-                            self.ccli_number = int(match.group())
                         else:
-                            log.warning("Can't parse CCLI Number from string: {text}".format(text=self.decode(data)))
+                            length_descriptor, = struct.unpack("B", song_file.read(1))
-                    elif block_key == VERSE:
+                        log.debug('length_descriptor: %d' % length_descriptor)
-                        self.add_verse(self.decode(data), "{tag}{number}".format(tag=VerseType.tags[VerseType.Verse],
+                        data = song_file.read(length_descriptor)
-                                                                                 number=verse_no))
+                        log.debug(data)
-                    elif block_key == CHORUS:
+                        if block_key == TITLE:
-                        self.add_verse(self.decode(data), "{tag}{number}".format(tag=VerseType.tags[VerseType.Chorus],
+                            self.title = self.decode(data)
-                                                                                 number=verse_no))
+                        elif block_key == AUTHOR:
-                    elif block_key == BRIDGE:
+                            authors = self.decode(data).split(" / ")
-                        self.add_verse(self.decode(data), "{tag}{number}".format(tag=VerseType.tags[VerseType.Bridge],
+                            for author in authors:
-                                                                                 number=verse_no))
+                                if author.find(",") != -1:
-                    elif block_key == TOPIC:
+                                    author_parts = author.split(", ")
-                        self.topics.append(self.decode(data))
+                                    author = author_parts[1] + " " + author_parts[0]
-                    elif block_key == COMMENTS:
+                                self.parse_author(author)
-                        self.comments = self.decode(data)
+                        elif block_key == COPYRIGHT:
-                    elif block_key == VERSE_ORDER:
+                            self.add_copyright(self.decode(data))
-                        verse_tag = self.to_openlp_verse_tag(self.decode(data), True)
+                        elif block_key == CCLI_NO:
-                        if verse_tag:
+                            # Try to get the CCLI number even if the field contains additional text
-                            if not isinstance(verse_tag, str):
+                            match = re.search(r'\d+', self.decode(data))
-                                verse_tag = self.decode(verse_tag)
+                            if match:
-                            self.ssp_verse_order_list.append(verse_tag)
+                                self.ccli_number = int(match.group())
-                    elif block_key == SONG_BOOK:
+                            else:
-                        self.song_book_name = self.decode(data)
+                                log.warning("Can't parse CCLI Number from string: {text}".format(
-                    elif block_key == CUSTOM_VERSE:
+                                    text=self.decode(data)))
-                        verse_tag = self.to_openlp_verse_tag(verse_name)
+                        elif block_key == VERSE:
-                        self.add_verse(self.decode(data), verse_tag)
+                            self.add_verse(self.decode(data), "{tag}{number}".format(
-                    else:
+                                tag=VerseType.tags[VerseType.Verse], number=verse_no))
-                        log.debug("Unrecognised blockKey: {key}, data: {data}".format(key=block_key, data=data))
+                        elif block_key == CHORUS:
-                        song_file.seek(next_block_starts)
+                            self.add_verse(self.decode(data), "{tag}{number}".format(
+                                tag=VerseType.tags[VerseType.Chorus], number=verse_no))
+                        elif block_key == BRIDGE:
+                            self.add_verse(self.decode(data), "{tag}{number}".format(
+                                tag=VerseType.tags[VerseType.Bridge], number=verse_no))
+                        elif block_key == TOPIC:
+                            self.topics.append(self.decode(data))
+                        elif block_key == COMMENTS:
+                            self.comments = self.decode(data)
+                        elif block_key == VERSE_ORDER:
+                            verse_tag = self.to_openlp_verse_tag(self.decode(data), True)
+                            if verse_tag:
+                                if not isinstance(verse_tag, str):
+                                    verse_tag = self.decode(verse_tag)
+                                self.ssp_verse_order_list.append(verse_tag)
+                        elif block_key == SONG_BOOK:
+                            self.song_book_name = self.decode(data)
+                        elif block_key == CUSTOM_VERSE:
+                            verse_tag = self.to_openlp_verse_tag(verse_name)
+                            self.add_verse(self.decode(data), verse_tag)
+                        else:
+                            log.debug("Unrecognised blockKey: {key}, data: {data}".format(key=block_key, data=data))
+                            song_file.seek(next_block_starts)
+                except struct.error:
+                    log.exception('Unexpected data in {name}'.format(name=file_path))
+                    self.log_error(file_path, translate('SongsPlugin.SongShowPlusImport',
+                                                        'File is not a valid SongShowPlus file.'))
+                    continue
                 self.verse_order_list = self.ssp_verse_order_list
                 if not self.finish():
                     self.log_error(file_path)

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

@@ -91,6 +91,10 @@ class WorshipAssistantImport(SongImport):
                                translate('SongsPlugin.WorshipAssistantImport',
                                          'Line {number:d}: {error}').format(number=songs_reader.line_num, error=e))
                 return
+            except UnicodeDecodeError as e:
+                self.log_error(translate('SongsPlugin.WorshipAssistantImport',
+                                         'Decoding error: {error}').format(error=e))
+                return
         num_records = len(records)
         log.info('{count} records found in CSV file'.format(count=num_records))
         self.import_wizard.progress_bar.setMaximum(num_records)
@@ -103,11 +107,11 @@ class WorshipAssistantImport(SongImport):
             record = dict((field.upper(), value) for field, value in record.items())
             # The CSV file has a line in the middle of the file where the headers are repeated.
             #  We need to skip this line.
-            if record['TITLE'] == "TITLE" and record['AUTHOR'] == 'AUTHOR' and record['LYRICS2'] == 'LYRICS2':
-                continue
-            self.set_defaults()
-            verse_order_list = []
             try:
+                if record['TITLE'] == "TITLE" and record['AUTHOR'] == 'AUTHOR' and record['LYRICS2'] == 'LYRICS2':
+                    continue
+                self.set_defaults()
+                verse_order_list = []
                 self.title = record['TITLE']
                 if record['AUTHOR'] != EMPTY_STR:
                     self.parse_author(record['AUTHOR'])
@@ -128,6 +132,11 @@ class WorshipAssistantImport(SongImport):
                                          'File not valid WorshipAssistant CSV format.'),
                                'TypeError: {error}'.format(error=e))
                 return
+            except KeyError as e:
+                self.log_error(translate('SongsPlugin.WorshipAssistantImport',
+                                         'File not valid WorshipAssistant CSV format.'),
+                               'KeyError: {error}'.format(error=e))
+                return
             verse = ''
             used_verses = []
             verse_id = VerseType.tags[VerseType.Verse] + '1'

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

@@ -52,7 +52,7 @@ class WorshipCenterProImport(SongImport):
         try:
             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:
+        except Exception as e:
             log.warning('Unable to connect the WorshipCenter Pro '
                         'database {source}. {error}'.format(source=self.import_source, error=str(e)))
             # Unfortunately no specific exception type

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

@@ -87,35 +87,39 @@ class ZionWorxImport(SongImport):
             num_records = len(records)
             log.info('{count} records found in CSV file'.format(count=num_records))
             self.import_wizard.progress_bar.setMaximum(num_records)
-            for index, record in enumerate(records, 1):
+            try:
-                if self.stop_import_flag:
+                for index, record in enumerate(records, 1):
-                    return
+                    if self.stop_import_flag:
-                self.set_defaults()
+                        return
-                try:
+                    self.set_defaults()
-                    self.title = record['Title1']
+                    try:
-                    if record['Title2']:
+                        self.title = record['Title1']
-                        self.alternate_title = record['Title2']
+                        if record['Title2']:
-                    self.parse_author(record['Writer'])
+                            self.alternate_title = record['Title2']
-                    self.add_copyright(record['Copyright'])
+                        self.parse_author(record['Writer'])
-                    lyrics = record['Lyrics']
+                        self.add_copyright(record['Copyright'])
-                except UnicodeDecodeError as e:
+                        lyrics = record['Lyrics']
-                    self.log_error(translate('SongsPlugin.ZionWorxImport', 'Record {index}').format(index=index),
+                    except UnicodeDecodeError as e:
-                                   translate('SongsPlugin.ZionWorxImport', 'Decoding error: {error}').format(error=e))
+                        self.log_error(translate('SongsPlugin.ZionWorxImport', 'Record {index}').format(index=index),
-                    continue
+                                       translate('SongsPlugin.ZionWorxImport',
-                except TypeError as e:
+                                                 'Decoding error: {error}').format(error=e))
-                    self.log_error(translate('SongsPlugin.ZionWorxImport', 'File not valid ZionWorx CSV format.'),
+                        continue
-                                   'TypeError: {error}'.format(error=e))
+                    except TypeError as e:
-                    return
+                        self.log_error(translate('SongsPlugin.ZionWorxImport', 'File not valid ZionWorx CSV format.'),
-                verse = ''
+                                       'TypeError: {error}'.format(error=e))
-                for line in lyrics.splitlines():
+                        return
-                    if line and not line.isspace():
+                    verse = ''
-                        verse += line + '\n'
+                    for line in lyrics.splitlines():
-                    elif verse:
+                        if line and not line.isspace():
+                            verse += line + '\n'
+                        elif verse:
+                            self.add_verse(verse, 'v')
+                            verse = ''
+                    if verse:
                         self.add_verse(verse, 'v')
-                        verse = ''
+                    title = self.title
-                if verse:
+                    if not self.finish():
-                    self.add_verse(verse, 'v')
+                        self.log_error(translate('SongsPlugin.ZionWorxImport', 'Record %d') % index +
-                title = self.title
+                                       (': "' + title + '"' if title else ''))
-                if not self.finish():
+            except AttributeError:
-                    self.log_error(translate('SongsPlugin.ZionWorxImport', 'Record %d') % index +
+                self.log_error(translate('SongsPlugin.ZionWorxImport', 'Error reading CSV file.'))
-                                   (': "' + title + '"' if title else ''))

tests/functional/openlp_core/ui/test_style.py

@@ -51,13 +51,13 @@ def test_get_application_stylesheet_dark(mocked_qdarkstyle, MockSettings):
 @patch('openlp.core.ui.style.HAS_DARK_STYLE', False)
 @patch('openlp.core.ui.style.is_win')
 @patch('openlp.core.ui.style.Settings')
-@patch('openlp.core.ui.style.Registry')
+@patch('openlp.core.app.QtWidgets.QApplication.palette')
-def test_get_application_stylesheet_not_alternate_rows(MockRegistry, MockSettings, mocked_is_win):
+def test_get_application_stylesheet_not_alternate_rows(mocked_palette, MockSettings, mocked_is_win):
     """Test that the alternate rows stylesheet is returned when enabled in settings"""
     # GIVEN: We're on Windows and no dark style is set
     mocked_is_win.return_value = False
     MockSettings.return_value.value.return_value = False
-    MockRegistry.return_value.get.return_value.palette.return_value.color.return_value.name.return_value = 'color'
+    mocked_palette.return_value.color.return_value.name.return_value = 'color'
 
     # WHEN: can_show_icon() is called
     result = get_application_stylesheet()

tests/functional/openlp_plugins/bibles/test_wordprojectimport.py

@@ -48,7 +48,8 @@ class TestWordProjectImport(TestCase):
         self.manager_patcher.start()
 
     @patch.object(Path, 'read_text')
-    def test_process_books(self, mocked_read_text):
+    @patch.object(Path, 'exists')
+    def test_process_books(self, mocked_exists, mocked_read_text):
         """
         Test the process_books() method
         """
@@ -58,11 +59,14 @@ class TestWordProjectImport(TestCase):
         importer.stop_import_flag = False
         importer.language_id = 'en'
         mocked_read_text.return_value = INDEX_PAGE
+        mocked_exists.return_value = True
 
         # WHEN: process_books() is called
-        with patch.object(importer, 'find_and_create_book') as mocked_find_and_create_book, \
+        with patch.object(importer, '_unzip_file') as mocked_unzip_file, \
+                patch.object(importer, 'find_and_create_book') as mocked_find_and_create_book, \
                 patch.object(importer, 'process_chapters') as mocked_process_chapters, \
                 patch.object(importer, 'session') as mocked_session:
+            mocked_unzip_file.return_value = True
             importer.process_books()
 
         # THEN: The right methods should have been called
@@ -173,6 +177,8 @@ class TestWordProjectImport(TestCase):
                 patch.object(importer, 'get_language_id') as mocked_get_language_id, \
                 patch.object(importer, 'process_books') as mocked_process_books, \
                 patch.object(importer, '_cleanup') as mocked_cleanup:
+            mocked_unzip_file.return_value = True
+            mocked_process_books.return_value = True
             mocked_get_language_id.return_value = 1
             result = importer.do_import()