diff --git a/openlp/plugins/bibles/forms/bibleimportform.py b/openlp/plugins/bibles/forms/bibleimportform.py index 3d02228ca..85daa786e 100644 --- a/openlp/plugins/bibles/forms/bibleimportform.py +++ b/openlp/plugins/bibles/forms/bibleimportform.py @@ -25,6 +25,7 @@ The bible import functions for OpenLP import logging import os import urllib.error +from lxml import etree from PyQt5 import QtWidgets try: @@ -33,14 +34,15 @@ try: except: PYSWORD_AVAILABLE = False -from openlp.core.common import AppLocation, Settings, UiStrings, translate, clean_filename +from openlp.core.common import AppLocation, Settings, UiStrings, trace_error_handler, translate +from openlp.core.common.languagemanager import get_locale_key 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.ui.lib.wizard import OpenLPWizard, WizardStrings -from openlp.core.common.languagemanager import get_locale_key -from openlp.plugins.bibles.lib.manager import BibleFormat from openlp.plugins.bibles.lib.db import clean_filename from openlp.plugins.bibles.lib.importers.http import CWExtract, BGExtract, BSExtract +from openlp.plugins.bibles.lib.manager import BibleFormat log = logging.getLogger(__name__) @@ -809,16 +811,22 @@ class BibleImportForm(OpenLPWizard): sword_path=self.field('sword_zip_path'), sword_key=self.sword_zipbible_combo_box.itemData( self.sword_zipbible_combo_box.currentIndex())) - if importer.do_import(license_version): - self.manager.save_meta_data(license_version, license_version, license_copyright, license_permissions) - self.manager.reload_bibles() - if bible_type == BibleFormat.WebDownload: - self.progress_label.setText( - translate('BiblesPlugin.ImportWizardForm', 'Registered Bible. Please note, that verses will be ' - 'downloaded on demand and thus an internet connection is required.')) - else: - self.progress_label.setText(WizardStrings.FinishedImport) - else: - self.progress_label.setText(translate('BiblesPlugin.ImportWizardForm', 'Your Bible import failed.')) - del self.manager.db_cache[importer.name] - delete_database(self.plugin.settings_section, importer.file) + + try: + if importer.do_import(license_version): + self.manager.save_meta_data(license_version, license_version, license_copyright, license_permissions) + self.manager.reload_bibles() + if bible_type == BibleFormat.WebDownload: + self.progress_label.setText( + translate('BiblesPlugin.ImportWizardForm', 'Registered Bible. Please note, that verses will be ' + 'downloaded on demand and thus an internet connection is required.')) + else: + self.progress_label.setText(WizardStrings.FinishedImport) + return + except (AttributeError, ValidationError, etree.XMLSyntaxError): + log.exception('Importing bible failed') + trace_error_handler(log) + + self.progress_label.setText(translate('BiblesPlugin.ImportWizardForm', 'Your Bible import failed.')) + del self.manager.db_cache[importer.name] + delete_database(self.plugin.settings_section, importer.file) diff --git a/openlp/plugins/bibles/lib/bibleimport.py b/openlp/plugins/bibles/lib/bibleimport.py index d6cfb83fa..ff95ac287 100644 --- a/openlp/plugins/bibles/lib/bibleimport.py +++ b/openlp/plugins/bibles/lib/bibleimport.py @@ -25,8 +25,8 @@ import logging from lxml import etree, objectify from zipfile import is_zipfile -from openlp.core.common import OpenLPMixin, languages -from openlp.core.lib import ValidationError, translate +from openlp.core.common import OpenLPMixin, languages, trace_error_handler, translate +from openlp.core.lib import ValidationError from openlp.core.lib.ui import critical_error_message_box from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB @@ -103,8 +103,7 @@ class BibleImport(OpenLPMixin, BibleDB): 'importing {file}'.format(book_ref=book_ref_id, file=self.filename)) return self.create_book(name, book_ref_id, book_details['testament_id']) - @staticmethod - def parse_xml(filename, use_objectify=False, elements=None, tags=None): + def parse_xml(self, filename, use_objectify=False, elements=None, tags=None): """ Parse and clean the supplied file by removing any elements or tags we don't use. :param filename: The filename of the xml file to parse. Str @@ -113,17 +112,28 @@ class BibleImport(OpenLPMixin, BibleDB): :param tags: A tuple of element names (Str) to remove, preserving their content. :return: The root element of the xml document """ - with open(filename, 'rb') as import_file: - # NOTE: We don't need to do any of the normal encoding detection here, because lxml does it's own encoding - # detection, and the two mechanisms together interfere with each other. - if not use_objectify: - tree = etree.parse(import_file, parser=etree.XMLParser(recover=True)) - else: - tree = objectify.parse(import_file, parser=objectify.makeparser(recover=True)) - if elements: - # Strip tags we don't use - remove content - etree.strip_elements(tree, elements, with_tail=False) - if tags: - # Strip tags we don't use - keep content - etree.strip_tags(tree, tags) - return tree.getroot() + try: + with open(filename, 'rb') as import_file: + # NOTE: We don't need to do any of the normal encoding detection here, because lxml does it's own encoding + # detection, and the two mechanisms together interfere with each other. + if not use_objectify: + tree = etree.parse(import_file, parser=etree.XMLParser(recover=True)) + else: + tree = objectify.parse(import_file, parser=objectify.makeparser(recover=True)) + if elements or tags: + self.wizard.increment_progress_bar(translate('BiblesPlugin.OsisImport', + 'Removing unused tags (this may take a few minutes)...')) + if elements: + # Strip tags we don't use - remove content + etree.strip_elements(tree, elements, with_tail=False) + if tags: + # Strip tags we don't use - keep content + etree.strip_tags(tree, tags) + return tree.getroot() + except OSError as e: + log.exception('Opening {file_name} failed.'.format(file_name=e.filename)) + trace_error_handler(log) + critical_error_message_box( title='An Error Occured When Opening A File', + message='The following error occurred when trying to open\n{file_name}:\n\n{error}' + .format(file_name=e.filename, error=e.strerror)) + return None diff --git a/openlp/plugins/bibles/lib/importers/csvbible.py b/openlp/plugins/bibles/lib/importers/csvbible.py index 3733145b6..4ca546232 100644 --- a/openlp/plugins/bibles/lib/importers/csvbible.py +++ b/openlp/plugins/bibles/lib/importers/csvbible.py @@ -128,7 +128,6 @@ class CSVBible(BibleImport): translate('BiblesPlugin.CSVBible', 'Importing books... {book}').format(book=book.name)) self.find_and_create_book(book.name, number_of_books, self.language_id) book_list.update({int(book.id): book.name}) - self.application.process_events() return book_list def process_verses(self, verses, books): @@ -153,7 +152,6 @@ class CSVBible(BibleImport): self.session.commit() self.create_verse(book.id, verse.chapter_number, verse.number, verse.text) self.wizard.increment_progress_bar(translate('BiblesPlugin.CSVBible', 'Importing verses... done.')) - self.application.process_events() self.session.commit() def do_import(self, bible_name=None): @@ -163,24 +161,18 @@ class CSVBible(BibleImport): :param bible_name: Optional name of the bible being imported. Str or None :return: True if the import was successful, False if it failed or was cancelled """ - try: - self.language_id = self.get_language(bible_name) - if not self.language_id: - raise ValidationError(msg='Invalid language selected') - books = self.parse_csv_file(self.books_file, Book) - self.wizard.progress_bar.setValue(0) - self.wizard.progress_bar.setMinimum(0) - self.wizard.progress_bar.setMaximum(len(books)) - book_list = self.process_books(books) - if self.stop_import_flag: - return False - verses = self.parse_csv_file(self.verses_file, Verse) - self.wizard.progress_bar.setValue(0) - self.wizard.progress_bar.setMaximum(len(books) + 1) - self.process_verses(verses, book_list) - if self.stop_import_flag: - return False - except ValidationError: - log.exception('Could not import CSV bible') + self.language_id = self.get_language(bible_name) + if not self.language_id: return False - return True + books = self.parse_csv_file(self.books_file, Book) + self.wizard.progress_bar.setValue(0) + self.wizard.progress_bar.setMinimum(0) + self.wizard.progress_bar.setMaximum(len(books)) + book_list = self.process_books(books) + if self.stop_import_flag: + return False + verses = self.parse_csv_file(self.verses_file, Verse) + self.wizard.progress_bar.setValue(0) + self.wizard.progress_bar.setMaximum(len(books) + 1) + self.process_verses(verses, book_list) + return not self.stop_import_flag diff --git a/openlp/plugins/bibles/lib/importers/opensong.py b/openlp/plugins/bibles/lib/importers/opensong.py index 01be8407e..42b058e48 100644 --- a/openlp/plugins/bibles/lib/importers/opensong.py +++ b/openlp/plugins/bibles/lib/importers/opensong.py @@ -23,7 +23,7 @@ import logging from lxml import etree -from openlp.core.common import translate, trace_error_handler +from openlp.core.common import trace_error_handler, translate from openlp.core.lib.exceptions import ValidationError from openlp.core.lib.ui import critical_error_message_box from openlp.plugins.bibles.lib.bibleimport import BibleImport @@ -146,6 +146,8 @@ class OpenSongBible(BibleImport): if BibleImport.is_compressed(filename): raise ValidationError(msg='Compressed file') bible = self.parse_xml(filename, use_objectify=True) + if bible is None: + raise ValidationError(msg='Error when opening file') root_tag = bible.tag.lower() if root_tag != 'bible': if root_tag == 'xmlbible': @@ -165,20 +167,13 @@ class OpenSongBible(BibleImport): :return: True if import completed, False if import was unsuccessful """ log.debug('Starting OpenSong import from "{name}"'.format(name=self.filename)) - try: - self.validate_file(self.filename) - bible = self.parse_xml(self.filename, use_objectify=True) - # Check that we're not trying to import a Zefania XML bible, it is sometimes refered to as 'OpenSong' - # No language info in the opensong format, so ask the user - self.language_id = self.get_language_id(bible_name=self.filename) - if not self.language_id: - return False - self.process_books(bible.b) - self.application.process_events() - except (AttributeError, ValidationError, etree.XMLSyntaxError): - log.exception('Loading Bible from OpenSong file failed') - trace_error_handler(log) + self.validate_file(self.filename) + bible = self.parse_xml(self.filename, use_objectify=True) + if bible is None: return False - if self.stop_import_flag: + # No language info in the opensong format, so ask the user + self.language_id = self.get_language_id(bible_name=self.filename) + if not self.language_id: return False - return True + self.process_books(bible.b) + return not self.stop_import_flag diff --git a/openlp/plugins/bibles/lib/importers/osis.py b/openlp/plugins/bibles/lib/importers/osis.py index db12bb7e9..2a874076d 100644 --- a/openlp/plugins/bibles/lib/importers/osis.py +++ b/openlp/plugins/bibles/lib/importers/osis.py @@ -24,9 +24,9 @@ import logging from lxml import etree from openlp.core.common import translate, trace_error_handler +from openlp.core.lib.exceptions import ValidationError from openlp.core.lib.ui import critical_error_message_box from openlp.plugins.bibles.lib.bibleimport import BibleImport -from openlp.plugins.bibles.lib.db import BiblesResourcesDB log = logging.getLogger(__name__) @@ -79,94 +79,119 @@ def replacement(match): return match.group(2).upper() +# Precompile a few xpath-querys +verse_in_chapter = etree.XPath('count(//ns:chapter[1]/ns:verse)', namespaces=NS) +text_in_verse = etree.XPath('count(//ns:verse[1]/text())', namespaces=NS) + + class OSISBible(BibleImport): """ `OSIS `_ Bible format importer class. """ + def process_books(self, bible_data): + """ + + :param bible_data: + :return: + """ + no_of_books = int(bible_data.xpath("count(//ns:div[@type='book'])", namespaces=NS)) + # Find books in the bible + bible_books = bible_data.xpath("//ns:div[@type='book']", namespaces=NS) + for book in bible_books: + if self.stop_import_flag: + break + # Remove div-tags in the book + etree.strip_tags(book, '{http://www.bibletechnologies.net/2003/OSIS/namespace}div') + db_book = self.find_and_create_book(book.get('osisID'), no_of_books, self.language_id) + self.process_chapters_and_verses(db_book, book) + self.session.commit() + + def process_chapters_and_verses(self, book, chapters): + """ + + :param book: + :param chapters: + :return: + """ + # Find out if chapter-tags contains the verses, or if it is used as milestone/anchor + if int(verse_in_chapter(chapters)) > 0: + # The chapter tags contains the verses + for chapter in chapters: + chapter_number = chapter.get("osisID").split('.')[1] + # Find out if verse-tags contains the text, or if it is used as milestone/anchor + if int(text_in_verse(chapter)) == 0: + # verse-tags are used as milestone + for verse in chapter: + # If this tag marks the start of a verse, the verse text is between this tag and + # the next tag, which the "tail" attribute gives us. + if verse.get('sID'): + verse_number = verse.get("osisID").split('.')[2] + verse_text = verse.tail + if verse_text: + self.create_verse(book.id, chapter_number, verse_number, verse_text.strip()) + else: + # Verse-tags contains the text + for verse in chapter: + verse_number = verse.get("osisID").split('.')[2] + if verse.text: + self.create_verse(book.id, chapter_number, verse_number, verse.text.strip()) + self.wizard.increment_progress_bar( + translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...') % + {'bookname': book.name, 'chapter': chapter_number}) + else: + # The chapter tags is used as milestones. For now we assume verses is also milestones + chapter_number = 0 + for element in chapters: + if element.tag == '{http://www.bibletechnologies.net/2003/OSIS/namespace}chapter' \ + and element.get('sID'): + chapter_number = element.get("osisID").split('.')[1] + self.wizard.increment_progress_bar( + translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...') % + {'bookname': book.name, 'chapter': chapter_number}) + elif element.tag == '{http://www.bibletechnologies.net/2003/OSIS/namespace}verse' \ + and element.get('sID'): + # If this tag marks the start of a verse, the verse text is between this tag and + # the next tag, which the "tail" attribute gives us. + verse_number = element.get("osisID").split('.')[2] + verse_text = element.tail + if verse_text: + self.create_verse(book.id, chapter_number, verse_number, verse_text.strip()) + + def validate_file(self, filename): + """ + Validate the supplied file + + :param filename: The supplied file + :return: True if valid. ValidationError is raised otherwise. + """ + if BibleImport.is_compressed(filename): + raise ValidationError(msg='Compressed file') + bible = self.parse_xml(filename, use_objectify=True) + if bible is None: + raise ValidationError(msg='Error when opening file') + root_tag = bible.tag + tag_str = '{{{name_space}}}osis'.format(name_space=NS['ns']) + if root_tag != tag_str: + critical_error_message_box( + message=translate('BiblesPlugin.OpenSongImport', + 'Incorrect Bible file type supplied. This looks like a Zefania XML bible, ' + 'please use the Zefania import option.')) + raise ValidationError(msg='Invalid xml.') + return True + def do_import(self, bible_name=None): """ Loads a Bible from file. """ log.debug('Starting OSIS import from "{name}"'.format(name=self.filename)) - success = True - try: - self.wizard.increment_progress_bar(translate('BiblesPlugin.OsisImport', - 'Removing unused tags (this may take a few minutes)...')) - osis_bible_tree = self.parse_xml(self.filename, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS) - # Find bible language] - language = osis_bible_tree.xpath("//ns:osisText/@xml:lang", namespaces=NS) - language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename) - if not language_id: - return False - no_of_books = int(osis_bible_tree.xpath("count(//ns:div[@type='book'])", namespaces=NS)) - # Precompile a few xpath-querys - verse_in_chapter = etree.XPath('count(//ns:chapter[1]/ns:verse)', namespaces=NS) - text_in_verse = etree.XPath('count(//ns:verse[1]/text())', namespaces=NS) - # Find books in the bible - bible_books = osis_bible_tree.xpath("//ns:div[@type='book']", namespaces=NS) - for book in bible_books: - if self.stop_import_flag: - break - # Remove div-tags in the book - etree.strip_tags(book, '{http://www.bibletechnologies.net/2003/OSIS/namespace}div') - db_book = self.find_and_create_book(book.get('osisID'), no_of_books, language_id) - # Find out if chapter-tags contains the verses, or if it is used as milestone/anchor - if int(verse_in_chapter(book)) > 0: - # The chapter tags contains the verses - for chapter in book: - chapter_number = chapter.get("osisID").split('.')[1] - # Find out if verse-tags contains the text, or if it is used as milestone/anchor - if int(text_in_verse(chapter)) == 0: - # verse-tags are used as milestone - for verse in chapter: - # If this tag marks the start of a verse, the verse text is between this tag and - # the next tag, which the "tail" attribute gives us. - if verse.get('sID'): - verse_number = verse.get("osisID").split('.')[2] - verse_text = verse.tail - if verse_text: - self.create_verse(db_book.id, chapter_number, verse_number, verse_text.strip()) - else: - # Verse-tags contains the text - for verse in chapter: - verse_number = verse.get("osisID").split('.')[2] - if verse.text: - self.create_verse(db_book.id, chapter_number, verse_number, verse.text.strip()) - self.wizard.increment_progress_bar( - translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...') % - {'bookname': db_book.name, 'chapter': chapter_number}) - else: - # The chapter tags is used as milestones. For now we assume verses is also milestones - chapter_number = 0 - for element in book: - if element.tag == '{http://www.bibletechnologies.net/2003/OSIS/namespace}chapter' \ - and element.get('sID'): - chapter_number = element.get("osisID").split('.')[1] - self.wizard.increment_progress_bar( - translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...') % - {'bookname': db_book.name, 'chapter': chapter_number}) - elif element.tag == '{http://www.bibletechnologies.net/2003/OSIS/namespace}verse' \ - and element.get('sID'): - # If this tag marks the start of a verse, the verse text is between this tag and - # the next tag, which the "tail" attribute gives us. - verse_number = element.get("osisID").split('.')[2] - verse_text = element.tail - if verse_text: - self.create_verse(db_book.id, chapter_number, verse_number, verse_text.strip()) - self.session.commit() - self.application.process_events() - except (ValueError, IOError): - log.exception('Loading bible from OSIS file failed') - trace_error_handler(log) - success = False - except etree.XMLSyntaxError as e: - log.exception('Loading bible from OSIS file failed') - trace_error_handler(log) - success = False - critical_error_message_box(message=translate('BiblesPlugin.OsisImport', - 'The file is not a valid OSIS-XML file:' - '\n{text}').format(text=e.msg)) - if self.stop_import_flag: + self.validate_file(self.filename) + bible = self.parse_xml(self.filename, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS) + if bible is None: return False - else: - return success + # Find bible language + language = bible.xpath("//ns:osisText/@xml:lang", namespaces=NS) + self.language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename) + if not self.language_id: + return False + self.process_books(bible) + return not self.stop_import_flag diff --git a/tests/functional/openlp_plugins/bibles/test_bibleimport.py b/tests/functional/openlp_plugins/bibles/test_bibleimport.py index 37c6c3fda..97cfd9efa 100644 --- a/tests/functional/openlp_plugins/bibles/test_bibleimport.py +++ b/tests/functional/openlp_plugins/bibles/test_bibleimport.py @@ -171,9 +171,12 @@ class TestBibleImport(TestCase): """ Test BibleImport.parse_xml() when called with the use_objectify default value """ - # GIVEN: A sample "file" to parse + # GIVEN: A sample "file" to parse and an instance of BibleImport + instance = BibleImport(MagicMock()) + instance.wizard = MagicMock() + # WHEN: Calling parse_xml - result = BibleImport.parse_xml('file.tst') + result = instance.parse_xml('file.tst') # THEN: The result returned should contain the correct data, and should be an instance of eetree_Element self.assertEqual(etree.tostring(result), @@ -185,9 +188,12 @@ class TestBibleImport(TestCase): """ Test BibleImport.parse_xml() when called with use_objectify set to True """ - # GIVEN: A sample "file" to parse + # GIVEN: A sample "file" to parse and an instance of BibleImport + instance = BibleImport(MagicMock()) + instance.wizard = MagicMock() + # WHEN: Calling parse_xml - result = BibleImport.parse_xml('file.tst', use_objectify=True) + result = instance.parse_xml('file.tst', use_objectify=True) # THEN: The result returned should contain the correct data, and should be an instance of ObjectifiedElement self.assertEqual(etree.tostring(result), @@ -199,11 +205,13 @@ class TestBibleImport(TestCase): """ Test BibleImport.parse_xml() when given a tuple of elements to remove """ - # GIVEN: A tuple of elements to remove + # GIVEN: A tuple of elements to remove and an instance of BibleImport elements = ('unsupported', 'x', 'y') + instance = BibleImport(MagicMock()) + instance.wizard = MagicMock() # WHEN: Calling parse_xml, with a test file - result = BibleImport.parse_xml('file.tst', elements=elements) + result = instance.parse_xml('file.tst', elements=elements) # THEN: The result returned should contain the correct data self.assertEqual(etree.tostring(result), @@ -213,11 +221,14 @@ class TestBibleImport(TestCase): """ Test BibleImport.parse_xml() when given a tuple of tags to remove """ - # GIVEN: A tuple of tags to remove + # GIVEN: A tuple of tags to remove and an instance of BibleImport tags = ('div', 'p', 'a') + instance = BibleImport(MagicMock()) + instance.wizard = MagicMock() + # WHEN: Calling parse_xml, with a test file - result = BibleImport.parse_xml('file.tst', tags=tags) + result = instance.parse_xml('file.tst', tags=tags) # THEN: The result returned should contain the correct data self.assertEqual(etree.tostring(result), b'\n Testdatatokeep\n Test' @@ -227,12 +238,14 @@ class TestBibleImport(TestCase): """ Test BibleImport.parse_xml() when given a tuple of elements and of tags to remove """ - # GIVEN: A tuple of elements and of tags to remove + # GIVEN: A tuple of elements and of tags to remove and an instacne of BibleImport elements = ('unsupported', 'x', 'y') tags = ('div', 'p', 'a') + instance = BibleImport(MagicMock()) + instance.wizard = MagicMock() # WHEN: Calling parse_xml, with a test file - result = BibleImport.parse_xml('file.tst', elements=elements, tags=tags) + result = instance.parse_xml('file.tst', elements=elements, tags=tags) # THEN: The result returned should contain the correct data self.assertEqual(etree.tostring(result), b'\n Testdatatokeep\n \n') diff --git a/tests/functional/openlp_plugins/bibles/test_csvimport.py b/tests/functional/openlp_plugins/bibles/test_csvimport.py index f6d3697af..50fe17884 100644 --- a/tests/functional/openlp_plugins/bibles/test_csvimport.py +++ b/tests/functional/openlp_plugins/bibles/test_csvimport.py @@ -207,7 +207,6 @@ class TestCSVImport(TestCase): with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'),\ patch('openlp.plugins.bibles.lib.importers.csvbible.translate'): importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verse.csv') - type(importer).application = PropertyMock() importer.find_and_create_book = MagicMock() importer.language_id = 10 importer.stop_import_flag = False @@ -222,7 +221,6 @@ class TestCSVImport(TestCase): # The returned data should be a dictionary with both song's id and names. self.assertEqual(importer.find_and_create_book.mock_calls, [call('1. Mosebog', 2, 10), call('2. Mosebog', 2, 10)]) - importer.application.process_events.assert_called_once_with() self.assertDictEqual(result, {1: '1. Mosebog', 2: '2. Mosebog'}) def process_verses_stopped_import_test(self): @@ -233,7 +231,6 @@ class TestCSVImport(TestCase): mocked_manager = MagicMock() with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'): importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verse.csv') - type(importer).application = PropertyMock() importer.get_book_name = MagicMock() importer.session = MagicMock() importer.stop_import_flag = True @@ -245,7 +242,6 @@ class TestCSVImport(TestCase): # THEN: get_book_name should not be called and the return value should be None self.assertFalse(importer.get_book_name.called) importer.wizard.increment_progress_bar.assert_called_once_with('Importing verses... done.') - importer.application.process_events.assert_called_once_with() self.assertIsNone(result) def process_verses_successful_test(self): @@ -257,7 +253,6 @@ class TestCSVImport(TestCase): with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'),\ patch('openlp.plugins.bibles.lib.importers.csvbible.translate'): importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verse.csv') - type(importer).application = PropertyMock() importer.create_verse = MagicMock() importer.get_book = MagicMock(return_value=Book('1', '1', '1. Mosebog', '1Mos')) importer.get_book_name = MagicMock(return_value='1. Mosebog') @@ -280,7 +275,6 @@ class TestCSVImport(TestCase): [call('1', 1, 1, 'I Begyndelsen skabte Gud Himmelen og Jorden.'), call('1', 1, 2, 'Og Jorden var øde og tom, og der var Mørke over Verdensdybet. ' 'Men Guds Ånd svævede over Vandene.')]) - importer.application.process_events.assert_called_once_with() def do_import_invalid_language_id_test(self): """ @@ -299,7 +293,6 @@ class TestCSVImport(TestCase): # THEN: The log.exception method should have been called to show that it reached the except clause. # False should be returned. importer.get_language.assert_called_once_with('Bible Name') - mocked_log.exception.assert_called_once_with('Could not import CSV bible') self.assertFalse(result) def do_import_stop_import_test(self): diff --git a/tests/functional/openlp_plugins/bibles/test_opensongimport.py b/tests/functional/openlp_plugins/bibles/test_opensongimport.py index 0f5c404ac..ead118fd9 100644 --- a/tests/functional/openlp_plugins/bibles/test_opensongimport.py +++ b/tests/functional/openlp_plugins/bibles/test_opensongimport.py @@ -387,125 +387,82 @@ class TestOpenSongImport(TestCase, TestMixin): self.assertEqual(context.exception.msg, 'Invalid xml.') self.assertFalse(mocked_message_box.called) - @patch('openlp.plugins.bibles.lib.importers.opensong.log') - @patch('openlp.plugins.bibles.lib.importers.opensong.trace_error_handler') - def do_import_attribute_error_test(self, mocked_trace_error_handler, mocked_log): + def do_import_parse_xml_fails_test(self): """ - Test do_import when an AttributeError exception is raised + Test do_import when parse_xml fails (returns None) """ - # GIVEN: An instance of OpenSongBible and a mocked validate_file which raises an AttributeError - importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') - importer.validate_file = MagicMock(**{'side_effect': AttributeError()}) - importer.parse_xml = MagicMock() + # GIVEN: An instance of OpenSongBible and a mocked parse_xml which returns False + with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \ + patch.object(OpenSongBible, 'validate_file'), \ + patch.object(OpenSongBible, 'parse_xml', return_value=None), \ + patch.object(OpenSongBible, 'get_language_id') as mocked_language_id: + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') - # WHEN: Calling do_import - result = importer.do_import() + # WHEN: Calling do_import + result = importer.do_import() - # THEN: do_import should return False after logging the exception - mocked_log.exception.assert_called_once_with('Loading Bible from OpenSong file failed') - mocked_trace_error_handler.assert_called_once_with(mocked_log) - self.assertFalse(result) - self.assertFalse(importer.parse_xml.called) + # THEN: do_import should return False and get_language_id should have not been called + self.assertFalse(result) + self.assertFalse(mocked_language_id.called) - @patch('openlp.plugins.bibles.lib.importers.opensong.log') - @patch('openlp.plugins.bibles.lib.importers.opensong.trace_error_handler') - def do_import_validation_error_test(self, mocked_trace_error_handler, mocked_log): - """ - Test do_import when an ValidationError exception is raised - """ - # GIVEN: An instance of OpenSongBible and a mocked validate_file which raises an ValidationError - importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') - importer.validate_file = MagicMock(**{'side_effect': ValidationError()}) - importer.parse_xml = MagicMock() - - # WHEN: Calling do_import - result = importer.do_import() - - # THEN: do_import should return False after logging the exception. parse_xml should not be called. - mocked_log.exception.assert_called_once_with('Loading Bible from OpenSong file failed') - mocked_trace_error_handler.assert_called_once_with(mocked_log) - self.assertFalse(result) - self.assertFalse(importer.parse_xml.called) - - @patch('openlp.plugins.bibles.lib.importers.opensong.log') - @patch('openlp.plugins.bibles.lib.importers.opensong.trace_error_handler') - def do_import_xml_syntax_error_test(self, mocked_trace_error_handler, mocked_log): - """ - Test do_import when an etree.XMLSyntaxError exception is raised - """ - # GIVEN: An instance of OpenSongBible and a mocked validate_file which raises an etree.XMLSyntaxError - importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') - importer.validate_file = MagicMock(**{'side_effect': etree.XMLSyntaxError(None, None, None, None)}) - importer.parse_xml = MagicMock() - - # WHEN: Calling do_import - result = importer.do_import() - - # THEN: do_import should return False after logging the exception. parse_xml should not be called. - mocked_log.exception.assert_called_once_with('Loading Bible from OpenSong file failed') - mocked_trace_error_handler.assert_called_once_with(mocked_log) - self.assertFalse(result) - self.assertFalse(importer.parse_xml.called) - - @patch('openlp.plugins.bibles.lib.importers.opensong.log') - def do_import_no_language_test(self, mocked_log): + def do_import_no_language_test(self): """ Test do_import when the user cancels the language selection dialog """ # GIVEN: An instance of OpenSongBible and a mocked get_language which returns False - importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') - importer.validate_file = MagicMock() - importer.parse_xml = MagicMock() - importer.get_language_id = MagicMock(**{'return_value': False}) - importer.process_books = MagicMock() + with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \ + patch.object(OpenSongBible, 'validate_file'), \ + patch.object(OpenSongBible, 'parse_xml'), \ + patch.object(OpenSongBible, 'get_language_id', return_value=False), \ + patch.object(OpenSongBible, 'process_books') as mocked_process_books: + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') - # WHEN: Calling do_import - result = importer.do_import() + # WHEN: Calling do_import + result = importer.do_import() - # THEN: do_import should return False and process_books should have not been called - self.assertFalse(result) - self.assertFalse(importer.process_books.called) + # THEN: do_import should return False and process_books should have not been called + self.assertFalse(result) + self.assertFalse(mocked_process_books.called) - @patch('openlp.plugins.bibles.lib.importers.opensong.log') - def do_import_stop_import_test(self, mocked_log): + def do_import_stop_import_test(self): """ Test do_import when the stop_import_flag is set to True """ # GIVEN: An instance of OpenSongBible and stop_import_flag set to True - importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') - importer.validate_file = MagicMock() - importer.parse_xml = MagicMock() - importer.get_language_id = MagicMock(**{'return_value': 10}) - importer.process_books = MagicMock() - importer.stop_import_flag = True + with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \ + patch.object(OpenSongBible, 'validate_file'), \ + patch.object(OpenSongBible, 'parse_xml'), \ + patch.object(OpenSongBible, 'get_language_id', return_value=10), \ + patch.object(OpenSongBible, 'process_books') as mocked_process_books: - # WHEN: Calling do_import - result = importer.do_import() + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + importer.stop_import_flag = True - # THEN: do_import should return False and process_books should have not been called - self.assertFalse(result) - self.assertTrue(importer.application.process_events.called) + # WHEN: Calling do_import + result = importer.do_import() - self.assertTrue(importer.application.process_events.called) + # THEN: do_import should return False and process_books should have not been called + self.assertFalse(result) + self.assertTrue(mocked_process_books.called) - @patch('openlp.plugins.bibles.lib.importers.opensong.log') - def do_import_completes_test(self, mocked_log): + def do_import_completes_test(self): """ Test do_import when it completes successfully """ - # GIVEN: An instance of OpenSongBible and stop_import_flag set to True - importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') - importer.validate_file = MagicMock() - importer.parse_xml = MagicMock() - importer.get_language_id = MagicMock(**{'return_value': 10}) - importer.process_books = MagicMock() - importer.stop_import_flag = False + # GIVEN: An instance of OpenSongBible and stop_import_flag set to False + with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \ + patch.object(OpenSongBible, 'validate_file'), \ + patch.object(OpenSongBible, 'parse_xml'), \ + patch.object(OpenSongBible, 'get_language_id', return_value=10), \ + patch.object(OpenSongBible, 'process_books'): + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + importer.stop_import_flag = False - # WHEN: Calling do_import - result = importer.do_import() + # WHEN: Calling do_import + result = importer.do_import() - # THEN: do_import should return True - self.assertTrue(result) + # THEN: do_import should return True + self.assertTrue(result) def test_file_import(self): """ diff --git a/tests/functional/openlp_plugins/bibles/test_osisimport.py b/tests/functional/openlp_plugins/bibles/test_osisimport.py index e1700fbb7..612a63b52 100644 --- a/tests/functional/openlp_plugins/bibles/test_osisimport.py +++ b/tests/functional/openlp_plugins/bibles/test_osisimport.py @@ -42,143 +42,218 @@ class TestOsisImport(TestCase): def setUp(self): self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry') + self.addCleanup(self.registry_patcher.stop) self.registry_patcher.start() self.manager_patcher = patch('openlp.plugins.bibles.lib.db.Manager') + self.addCleanup(self.manager_patcher.stop) self.manager_patcher.start() - def tearDown(self): - self.registry_patcher.stop() - self.manager_patcher.stop() - - def test_create_importer(self): - """ - Test creating an instance of the OSIS file importer - """ - # GIVEN: A mocked out "manager" - mocked_manager = MagicMock() - - # WHEN: An importer object is created - importer = OSISBible(mocked_manager, path='.', name='.', filename='') - - # THEN: The importer should be an instance of BibleDB - self.assertIsInstance(importer, BibleDB) - - def test_file_import_nested_tags(self): - """ - Test the actual import of OSIS Bible file, with nested chapter and verse tags - """ - # GIVEN: Test files with a mocked out "manager", "import_wizard", and mocked functions - # get_book_ref_id_by_name, create_verse, create_book, session and get_language. - result_file = open(os.path.join(TEST_PATH, 'dk1933.json'), 'rb') - test_data = json.loads(result_file.read().decode()) - bible_file = 'osis-dk1933.xml' - with patch('openlp.plugins.bibles.lib.importers.osis.OSISBible.application'): + def test_create_importer(self): + """ + Test creating an instance of the OSIS file importer + """ + # GIVEN: A mocked out "manager" mocked_manager = MagicMock() - mocked_import_wizard = MagicMock() + + # WHEN: An importer object is created importer = OSISBible(mocked_manager, path='.', name='.', filename='') - importer.wizard = mocked_import_wizard - importer.get_book_ref_id_by_name = MagicMock() - importer.create_verse = MagicMock() - importer.create_book = MagicMock() - importer.session = MagicMock() - importer.get_language = MagicMock() - importer.get_language.return_value = 'Danish' - # WHEN: Importing bible file - importer.filename = os.path.join(TEST_PATH, bible_file) - importer.do_import() + # THEN: The importer should be an instance of BibleDB + self.assertIsInstance(importer, BibleDB) - # THEN: The create_verse() method should have been called with each verse in the file. - self.assertTrue(importer.create_verse.called) - for verse_tag, verse_text in test_data['verses']: - importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text) - - def test_file_import_mixed_tags(self): + def do_import_parse_xml_fails_test(self): """ - Test the actual import of OSIS Bible file, with chapter tags containing milestone verse tags. + Test do_import when parse_xml fails (returns None) """ - # GIVEN: Test files with a mocked out "manager", "import_wizard", and mocked functions - # get_book_ref_id_by_name, create_verse, create_book, session and get_language. - result_file = open(os.path.join(TEST_PATH, 'kjv.json'), 'rb') - test_data = json.loads(result_file.read().decode()) - bible_file = 'osis-kjv.xml' - with patch('openlp.plugins.bibles.lib.importers.osis.OSISBible.application'): - mocked_manager = MagicMock() - mocked_import_wizard = MagicMock() - importer = OSISBible(mocked_manager, path='.', name='.', filename='') - importer.wizard = mocked_import_wizard - importer.get_book_ref_id_by_name = MagicMock() - importer.create_verse = MagicMock() - importer.create_book = MagicMock() - importer.session = MagicMock() - importer.get_language = MagicMock() - importer.get_language.return_value = 'English' + # GIVEN: An instance of OpenSongBible and a mocked parse_xml which returns False + with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \ + patch.object(OSISBible, 'validate_file'), \ + patch.object(OSISBible, 'parse_xml', return_value=None), \ + patch.object(OSISBible, 'get_language_id') as mocked_language_id: + importer = OSISBible(MagicMock(), path='.', name='.', filename='') - # WHEN: Importing bible file - importer.filename = os.path.join(TEST_PATH, bible_file) - importer.do_import() + # WHEN: Calling do_import + result = importer.do_import() - # THEN: The create_verse() method should have been called with each verse in the file. - self.assertTrue(importer.create_verse.called) - for verse_tag, verse_text in test_data['verses']: - importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text) + # THEN: do_import should return False and get_language_id should have not been called + self.assertFalse(result) + self.assertFalse(mocked_language_id.called) - def test_file_import_milestone_tags(self): + def do_import_no_language_test(self): """ - Test the actual import of OSIS Bible file, with milestone chapter and verse tags. + Test do_import when the user cancels the language selection dialog """ - # GIVEN: Test files with a mocked out "manager", "import_wizard", and mocked functions - # get_book_ref_id_by_name, create_verse, create_book, session and get_language. - result_file = open(os.path.join(TEST_PATH, 'web.json'), 'rb') - test_data = json.loads(result_file.read().decode()) - bible_file = 'osis-web.xml' - with patch('openlp.plugins.bibles.lib.importers.osis.OSISBible.application'): - mocked_manager = MagicMock() - mocked_import_wizard = MagicMock() - importer = OSISBible(mocked_manager, path='.', name='.', filename='') - importer.wizard = mocked_import_wizard - importer.get_book_ref_id_by_name = MagicMock() - importer.create_verse = MagicMock() - importer.create_book = MagicMock() - importer.session = MagicMock() - importer.get_language = MagicMock() - importer.get_language.return_value = 'English' + # GIVEN: An instance of OpenSongBible and a mocked get_language which returns False + with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \ + patch.object(OSISBible, 'validate_file'), \ + patch.object(OSISBible, 'parse_xml'), \ + patch.object(OSISBible, 'get_language_id', **{'return_value': False}), \ + patch.object(OSISBible, 'process_books') as mocked_process_books: + importer = OSISBible(MagicMock(), path='.', name='.', filename='') - # WHEN: Importing bible file - importer.filename = os.path.join(TEST_PATH, bible_file) - importer.do_import() + # WHEN: Calling do_import + result = importer.do_import() - # THEN: The create_verse() method should have been called with each verse in the file. - self.assertTrue(importer.create_verse.called) - for verse_tag, verse_text in test_data['verses']: - importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text) + # THEN: do_import should return False and process_books should have not been called + self.assertFalse(result) + self.assertFalse(mocked_process_books.called) - def test_file_import_empty_verse_tags(self): + def do_import_stop_import_test(self): """ - Test the actual import of OSIS Bible file, with an empty verse tags. + Test do_import when the stop_import_flag is set to True """ - # GIVEN: Test files with a mocked out "manager", "import_wizard", and mocked functions - # get_book_ref_id_by_name, create_verse, create_book, session and get_language. - result_file = open(os.path.join(TEST_PATH, 'dk1933.json'), 'rb') - test_data = json.loads(result_file.read().decode()) - bible_file = 'osis-dk1933-empty-verse.xml' - with patch('openlp.plugins.bibles.lib.importers.osis.OSISBible.application'): - mocked_manager = MagicMock() - mocked_import_wizard = MagicMock() - importer = OSISBible(mocked_manager, path='.', name='.', filename='') - importer.wizard = mocked_import_wizard - importer.get_book_ref_id_by_name = MagicMock() - importer.create_verse = MagicMock() - importer.create_book = MagicMock() - importer.session = MagicMock() - importer.get_language = MagicMock() - importer.get_language.return_value = 'Danish' + # GIVEN: An instance of OpenSongBible and stop_import_flag set to True + with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \ + patch.object(OSISBible, 'validate_file'), \ + patch.object(OSISBible, 'parse_xml'), \ + patch.object(OSISBible, 'get_language_id', **{'return_value': 10}), \ + patch.object(OSISBible, 'process_books'): + importer = OSISBible(MagicMock(), path='.', name='.', filename='') + importer.stop_import_flag = True - # WHEN: Importing bible file - importer.filename = os.path.join(TEST_PATH, bible_file) - importer.do_import() + # WHEN: Calling do_import + result = importer.do_import() - # THEN: The create_verse() method should have been called with each verse in the file. - self.assertTrue(importer.create_verse.called) - for verse_tag, verse_text in test_data['verses']: - importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text) + # THEN: do_import should return False and process_books should have not been called + self.assertFalse(result) + self.assertTrue(importer.process_books.called) + + @patch('openlp.plugins.bibles.lib.importers.opensong.log') + def do_import_completes_test(self, mocked_log): + """ + Test do_import when it completes successfully + """ + # GIVEN: An instance of OpenSongBible and stop_import_flag set to True + with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \ + patch.object(OSISBible, 'validate_file'), \ + patch.object(OSISBible, 'parse_xml'), \ + patch.object(OSISBible, 'get_language_id', **{'return_value': 10}), \ + patch.object(OSISBible, 'process_books'): + importer = OSISBible(MagicMock(), path='.', name='.', filename='') + importer.stop_import_flag = False + + # WHEN: Calling do_import + result = importer.do_import() + + # THEN: do_import should return True + self.assertTrue(result) + + def test_file_import_nested_tags(self): + """ + Test the actual import of OSIS Bible file, with nested chapter and verse tags + """ + # GIVEN: Test files with a mocked out "manager", "import_wizard", and mocked functions + # get_book_ref_id_by_name, create_verse, create_book, session and get_language. + result_file = open(os.path.join(TEST_PATH, 'dk1933.json'), 'rb') + test_data = json.loads(result_file.read().decode()) + bible_file = 'osis-dk1933.xml' + with patch('openlp.plugins.bibles.lib.importers.osis.OSISBible.application'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = OSISBible(mocked_manager, path='.', name='.', filename='') + importer.wizard = mocked_import_wizard + importer.get_book_ref_id_by_name = MagicMock() + importer.create_verse = MagicMock() + importer.create_book = MagicMock() + importer.session = MagicMock() + importer.get_language = MagicMock() + importer.get_language.return_value = 'Danish' + + # WHEN: Importing bible file + importer.filename = os.path.join(TEST_PATH, bible_file) + importer.do_import() + + # THEN: The create_verse() method should have been called with each verse in the file. + self.assertTrue(importer.create_verse.called) + for verse_tag, verse_text in test_data['verses']: + importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text) + + def test_file_import_mixed_tags(self): + """ + Test the actual import of OSIS Bible file, with chapter tags containing milestone verse tags. + """ + # GIVEN: Test files with a mocked out "manager", "import_wizard", and mocked functions + # get_book_ref_id_by_name, create_verse, create_book, session and get_language. + result_file = open(os.path.join(TEST_PATH, 'kjv.json'), 'rb') + test_data = json.loads(result_file.read().decode()) + bible_file = 'osis-kjv.xml' + with patch('openlp.plugins.bibles.lib.importers.osis.OSISBible.application'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = OSISBible(mocked_manager, path='.', name='.', filename='') + importer.wizard = mocked_import_wizard + importer.get_book_ref_id_by_name = MagicMock() + importer.create_verse = MagicMock() + importer.create_book = MagicMock() + importer.session = MagicMock() + importer.get_language = MagicMock() + importer.get_language.return_value = 'English' + + # WHEN: Importing bible file + importer.filename = os.path.join(TEST_PATH, bible_file) + importer.do_import() + + # THEN: The create_verse() method should have been called with each verse in the file. + self.assertTrue(importer.create_verse.called) + for verse_tag, verse_text in test_data['verses']: + importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text) + + def test_file_import_milestone_tags(self): + """ + Test the actual import of OSIS Bible file, with milestone chapter and verse tags. + """ + # GIVEN: Test files with a mocked out "manager", "import_wizard", and mocked functions + # get_book_ref_id_by_name, create_verse, create_book, session and get_language. + result_file = open(os.path.join(TEST_PATH, 'web.json'), 'rb') + test_data = json.loads(result_file.read().decode()) + bible_file = 'osis-web.xml' + with patch('openlp.plugins.bibles.lib.importers.osis.OSISBible.application'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = OSISBible(mocked_manager, path='.', name='.', filename='') + importer.wizard = mocked_import_wizard + importer.get_book_ref_id_by_name = MagicMock() + importer.create_verse = MagicMock() + importer.create_book = MagicMock() + importer.session = MagicMock() + importer.get_language = MagicMock() + importer.get_language.return_value = 'English' + + # WHEN: Importing bible file + importer.filename = os.path.join(TEST_PATH, bible_file) + importer.do_import() + + # THEN: The create_verse() method should have been called with each verse in the file. + self.assertTrue(importer.create_verse.called) + for verse_tag, verse_text in test_data['verses']: + importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text) + + def test_file_import_empty_verse_tags(self): + """ + Test the actual import of OSIS Bible file, with an empty verse tags. + """ + # GIVEN: Test files with a mocked out "manager", "import_wizard", and mocked functions + # get_book_ref_id_by_name, create_verse, create_book, session and get_language. + result_file = open(os.path.join(TEST_PATH, 'dk1933.json'), 'rb') + test_data = json.loads(result_file.read().decode()) + bible_file = 'osis-dk1933-empty-verse.xml' + with patch('openlp.plugins.bibles.lib.importers.osis.OSISBible.application'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = OSISBible(mocked_manager, path='.', name='.', filename='') + importer.wizard = mocked_import_wizard + importer.get_book_ref_id_by_name = MagicMock() + importer.create_verse = MagicMock() + importer.create_book = MagicMock() + importer.session = MagicMock() + importer.get_language = MagicMock() + importer.get_language.return_value = 'Danish' + + # WHEN: Importing bible file + importer.filename = os.path.join(TEST_PATH, bible_file) + importer.do_import() + + # THEN: The create_verse() method should have been called with each verse in the file. + self.assertTrue(importer.create_verse.called) + for verse_tag, verse_text in test_data['verses']: + importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text)