diff --git a/openlp/plugins/bibles/bibleplugin.py b/openlp/plugins/bibles/bibleplugin.py index f63b85a92..e9168d695 100644 --- a/openlp/plugins/bibles/bibleplugin.py +++ b/openlp/plugins/bibles/bibleplugin.py @@ -140,10 +140,10 @@ class BiblePlugin(Plugin): def uses_theme(self, theme): """ - Called to find out if the bible plugin is currently using a theme. Returns ``1`` if the theme is being used, - otherwise returns ``0``. + Called to find out if the bible plugin is currently using a theme. :param theme: The theme + :return: 1 if the theme is being used, otherwise returns 0 """ if str(self.settings_tab.bible_theme) == theme: return 1 @@ -151,11 +151,11 @@ class BiblePlugin(Plugin): def rename_theme(self, old_theme, new_theme): """ - Rename the theme the bible plugin is using making the plugin use the - new name. + Rename the theme the bible plugin is using, making the plugin use the new name. :param old_theme: The name of the theme the plugin should stop using. Unused for this particular plugin. :param new_theme: The new name the plugin should now use. + :return: None """ self.settings_tab.bible_theme = new_theme self.settings_tab.save() diff --git a/openlp/plugins/bibles/lib/__init__.py b/openlp/plugins/bibles/lib/__init__.py index 804755d18..e730009e7 100644 --- a/openlp/plugins/bibles/lib/__init__.py +++ b/openlp/plugins/bibles/lib/__init__.py @@ -173,7 +173,7 @@ class BibleStrings(object): def update_reference_separators(): """ - Updates separators and matches for parsing and formating scripture references. + Updates separators and matches for parsing and formatting scripture references. """ default_separators = [ '|'.join([ @@ -215,7 +215,7 @@ def update_reference_separators(): # escape reserved characters for character in '\\.^$*+?{}[]()': source_string = source_string.replace(character, '\\' + character) - # add various unicode alternatives + # add various Unicode alternatives source_string = source_string.replace('-', '(?:[-\u00AD\u2010\u2011\u2012\u2014\u2014\u2212\uFE63\uFF0D])') source_string = source_string.replace(',', '(?:[,\u201A])') REFERENCE_SEPARATORS['sep_{role}'.format(role=role)] = '\s*(?:{source})\s*'.format(source=source_string) diff --git a/openlp/plugins/bibles/lib/bibleimport.py b/openlp/plugins/bibles/lib/bibleimport.py index 4d015223b..d6cfb83fa 100644 --- a/openlp/plugins/bibles/lib/bibleimport.py +++ b/openlp/plugins/bibles/lib/bibleimport.py @@ -23,9 +23,11 @@ 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 +from openlp.core.lib import ValidationError, translate +from openlp.core.lib.ui import critical_error_message_box from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB log = logging.getLogger(__name__) @@ -35,11 +37,25 @@ class BibleImport(OpenLPMixin, BibleDB): """ Helper class to import bibles from a third party source into OpenLP """ - # TODO: Test def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.filename = kwargs['filename'] if 'filename' in kwargs else None + @staticmethod + def is_compressed(file): + """ + Check if the supplied file is compressed + + :param file: A path to the file to check + """ + if is_zipfile(file): + critical_error_message_box( + message=translate('BiblesPlugin.BibleImport', + 'The file "{file}" you supplied is compressed. You must decompress it before import.' + ).format(file=file)) + return True + return False + def get_language_id(self, file_language=None, bible_name=None): """ Get the language_id for the language of the bible. Fallback to user input if we cannot do this pragmatically. diff --git a/openlp/plugins/bibles/lib/importers/csvbible.py b/openlp/plugins/bibles/lib/importers/csvbible.py index 549cec581..3733145b6 100644 --- a/openlp/plugins/bibles/lib/importers/csvbible.py +++ b/openlp/plugins/bibles/lib/importers/csvbible.py @@ -142,7 +142,7 @@ class CSVBible(BibleImport): book_ptr = None for verse in verses: if self.stop_import_flag: - return None + break verse_book = self.get_book_name(verse.book_id_name, books) if book_ptr != verse_book: book = self.get_book(verse_book) diff --git a/openlp/plugins/bibles/lib/importers/opensong.py b/openlp/plugins/bibles/lib/importers/opensong.py index 43c1cf8ca..01be8407e 100644 --- a/openlp/plugins/bibles/lib/importers/opensong.py +++ b/openlp/plugins/bibles/lib/importers/opensong.py @@ -21,108 +21,164 @@ ############################################################################### import logging -from lxml import etree, objectify +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 BibleDB, BiblesResourcesDB log = logging.getLogger(__name__) +def get_text(element): + """ + Recursively get all text in an objectify element and its child elements. + + :param element: An objectify element to get the text from + :return: The text content of the element (str) + """ + verse_text = '' + if element.text: + verse_text = element.text + for sub_element in element.iterchildren(): + verse_text += get_text(sub_element) + if element.tail: + verse_text += element.tail + return verse_text + + +def parse_chapter_number(number, previous_number): + """ + Parse the chapter number + + :param number: The raw data from the xml + :param previous_number: The previous chapter number + :return: Number of current chapter. (Int) + """ + if number: + return int(number.split()[-1]) + return previous_number + 1 + + +def parse_verse_number(number, previous_number): + """ + Parse the verse number retrieved from the xml + + :param number: The raw data from the xml + :param previous_number: The previous verse number + :return: Number of current verse. (Int) + """ + if not number: + return previous_number + 1 + try: + return int(number) + except ValueError: + verse_parts = number.split('-') + if len(verse_parts) > 1: + number = int(verse_parts[0]) + return number + except TypeError: + log.warning('Illegal verse number: {verse_no}'.format(verse_no=str(number))) + return previous_number + 1 + + class OpenSongBible(BibleImport): """ OpenSong Bible format importer class. This class is used to import Bibles from OpenSong's XML format. """ - def get_text(self, element): + def process_books(self, books): """ - Recursively get all text in an objectify element and its child elements. + Extract and create the books from the objectified xml - :param element: An objectify element to get the text from + :param books: Objectified xml + :return: None """ - verse_text = '' - if element.text: - verse_text = element.text - for sub_element in element.iterchildren(): - verse_text += self.get_text(sub_element) - if element.tail: - verse_text += element.tail - return verse_text + for book in books: + if self.stop_import_flag: + break + db_book = self.find_and_create_book(str(book.attrib['n']), len(books), self.language_id) + self.process_chapters(db_book, book.c) + self.session.commit() - def do_import(self, bible_name=None): + def process_chapters(self, book, chapters): """ - Loads a Bible from file. + Extract and create the chapters from the objectified xml for the book `book` + + :param book: A database Book object to add the chapters to + :param chapters: Objectified xml containing chapters + :return: None """ - log.debug('Starting OpenSong import from "{name}"'.format(name=self.filename)) - success = True - try: - 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' - if bible.tag.upper() == 'XMLBIBLE': + chapter_number = 0 + for chapter in chapters: + if self.stop_import_flag: + break + chapter_number = parse_chapter_number(chapter.attrib['n'], chapter_number) + self.process_verses(book, chapter_number, chapter.v) + self.wizard.increment_progress_bar(translate('BiblesPlugin.Opensong', + 'Importing {name} {chapter}...' + ).format(name=book.name, chapter=chapter_number)) + + def process_verses(self, book, chapter_number, verses): + """ + Extract and create the verses from the objectified xml + + :param book: A database Book object + :param chapter_number: The chapter number to add the verses to (int) + :param verses: Objectified xml containing verses + :return: None + """ + verse_number = 0 + for verse in verses: + if self.stop_import_flag: + break + verse_number = parse_verse_number(verse.attrib['n'], verse_number) + self.create_verse(book.id, chapter_number, verse_number, get_text(verse)) + + 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) + root_tag = bible.tag.lower() + if root_tag != 'bible': + if root_tag == 'xmlbible': + # Zefania bibles have a root tag of XMLBIBLE". Sometimes these bibles are referred to as 'OpenSong' 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.')) - return False + raise ValidationError(msg='Invalid xml.') + return True + + def do_import(self, bible_name=None): + """ + Loads an Open Song Bible from a file. + + :param bible_name: The name of the bible being imported + :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 - language_id = self.get_language_id(bible_name=self.filename) - if not language_id: + self.language_id = self.get_language_id(bible_name=self.filename) + if not self.language_id: return False - for book in bible.b: - if self.stop_import_flag: - break - book_ref_id = self.get_book_ref_id_by_name(str(book.attrib['n']), len(bible.b), language_id) - if not book_ref_id: - log.error('Importing books from "{name}" failed'.format(name=self.filename)) - return False - book_details = BiblesResourcesDB.get_book_by_id(book_ref_id) - db_book = self.create_book(book.attrib['n'], book_ref_id, book_details['testament_id']) - chapter_number = 0 - for chapter in book.c: - if self.stop_import_flag: - break - number = chapter.attrib['n'] - if number: - chapter_number = int(number.split()[-1]) - else: - chapter_number += 1 - verse_number = 0 - for verse in chapter.v: - if self.stop_import_flag: - break - number = verse.attrib['n'] - if number: - try: - number = int(number) - except ValueError: - verse_parts = number.split('-') - if len(verse_parts) > 1: - number = int(verse_parts[0]) - except TypeError: - log.warning('Illegal verse number: {verse:d}'.format(verse=verse.attrib['n'])) - verse_number = number - else: - verse_number += 1 - self.create_verse(db_book.id, chapter_number, verse_number, self.get_text(verse)) - self.wizard.increment_progress_bar(translate('BiblesPlugin.Opensong', - 'Importing {name} {chapter}...' - ).format(name=db_book.name, chapter=chapter_number)) - self.session.commit() + self.process_books(bible.b) self.application.process_events() - except etree.XMLSyntaxError as inst: - trace_error_handler(log) - critical_error_message_box( - message=translate('BiblesPlugin.OpenSongImport', - 'Incorrect Bible file type supplied. OpenSong Bibles may be ' - 'compressed. You must decompress them before import.')) - log.exception(inst) - success = False - except (IOError, AttributeError): + except (AttributeError, ValidationError, etree.XMLSyntaxError): log.exception('Loading Bible from OpenSong file failed') - success = False + trace_error_handler(log) + return False if self.stop_import_flag: return False - else: - return success + return True diff --git a/openlp/plugins/bibles/lib/importers/osis.py b/openlp/plugins/bibles/lib/importers/osis.py index 99a138acd..db12bb7e9 100644 --- a/openlp/plugins/bibles/lib/importers/osis.py +++ b/openlp/plugins/bibles/lib/importers/osis.py @@ -98,7 +98,7 @@ class OSISBible(BibleImport): language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename) if not language_id: return False - num_books = int(osis_bible_tree.xpath("count(//ns:div[@type='book'])", namespaces=NS)) + 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) @@ -108,13 +108,8 @@ class OSISBible(BibleImport): if self.stop_import_flag: break # Remove div-tags in the book - etree.strip_tags(book, ('{http://www.bibletechnologies.net/2003/OSIS/namespace}div')) - book_ref_id = self.get_book_ref_id_by_name(book.get('osisID'), num_books, language_id) - if not book_ref_id: - log.error('Importing books from "{name}" failed'.format(name=self.filename)) - return False - book_details = BiblesResourcesDB.get_book_by_id(book_ref_id) - db_book = self.create_book(book_details['name'], book_ref_id, book_details['testament_id']) + 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 diff --git a/openlp/plugins/bibles/lib/importers/zefania.py b/openlp/plugins/bibles/lib/importers/zefania.py index 61ee41166..bc31a1664 100644 --- a/openlp/plugins/bibles/lib/importers/zefania.py +++ b/openlp/plugins/bibles/lib/importers/zefania.py @@ -54,7 +54,7 @@ class ZefaniaBible(BibleImport): language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename) if not language_id: return False - num_books = int(xmlbible.xpath('count(//BIBLEBOOK)')) + no_of_books = int(xmlbible.xpath('count(//BIBLEBOOK)')) self.wizard.progress_bar.setMaximum(int(xmlbible.xpath('count(//CHAPTER)'))) for BIBLEBOOK in xmlbible: if self.stop_import_flag: @@ -64,7 +64,7 @@ class ZefaniaBible(BibleImport): if not bname and not bnumber: continue if bname: - book_ref_id = self.get_book_ref_id_by_name(bname, num_books, language_id) + book_ref_id = self.get_book_ref_id_by_name(bname, no_of_books, language_id) else: log.debug('Could not find a name, will use number, basically a guess.') book_ref_id = int(bnumber) diff --git a/tests/functional/openlp_core_ui/test_exceptionform.py b/tests/functional/openlp_core_ui/test_exceptionform.py index 452a8dee9..493b2baeb 100644 --- a/tests/functional/openlp_core_ui/test_exceptionform.py +++ b/tests/functional/openlp_core_ui/test_exceptionform.py @@ -24,18 +24,13 @@ Package to test the openlp.core.ui.exeptionform package. """ import os -import socket import tempfile -import urllib from unittest import TestCase from unittest.mock import mock_open -from PyQt5.QtCore import QUrlQuery - from openlp.core.common import Registry -from openlp.core.ui.firsttimeform import FirstTimeForm -from tests.functional import MagicMock, patch +from tests.functional import patch from tests.helpers.testmixin import TestMixin from openlp.core.ui import exceptionform diff --git a/tests/functional/openlp_plugins/bibles/test_bibleimport.py b/tests/functional/openlp_plugins/bibles/test_bibleimport.py index e2076df55..37c6c3fda 100644 --- a/tests/functional/openlp_plugins/bibles/test_bibleimport.py +++ b/tests/functional/openlp_plugins/bibles/test_bibleimport.py @@ -29,8 +29,10 @@ from lxml import etree, objectify from unittest import TestCase from openlp.core.common.languages import Language +from openlp.core.lib.exceptions import ValidationError from openlp.plugins.bibles.lib.bibleimport import BibleImport -from tests.functional import MagicMock, patch +from openlp.plugins.bibles.lib.db import BibleDB +from tests.functional import ANY, MagicMock, patch class TestBibleImport(TestCase): @@ -39,23 +41,48 @@ class TestBibleImport(TestCase): """ def setUp(self): - test_file = BytesIO(b'\n' - b'\n' - b'
Test

data

tokeep
\n' - b' Testdatatodiscard\n' - b'
') + test_file = BytesIO( + b'\n' + b'\n' + b'
Test

data

tokeep
\n' + b' Testdatatodiscard\n' + b'
' + ) self.file_patcher = patch('builtins.open', return_value=test_file) - self.log_patcher = patch('openlp.plugins.bibles.lib.bibleimport.log') - self.setup_patcher = patch('openlp.plugins.bibles.lib.db.BibleDB._setup') - self.addCleanup(self.file_patcher.stop) - self.addCleanup(self.log_patcher.stop) - self.addCleanup(self.setup_patcher.stop) - self.file_patcher.start() + self.log_patcher = patch('openlp.plugins.bibles.lib.bibleimport.log') + self.addCleanup(self.log_patcher.stop) self.mock_log = self.log_patcher.start() + self.setup_patcher = patch('openlp.plugins.bibles.lib.db.BibleDB._setup') + self.addCleanup(self.setup_patcher.stop) self.setup_patcher.start() + def init_kwargs_none_test(self): + """ + Test the initialisation of the BibleImport Class when no key word arguments are supplied + """ + # GIVEN: A patched BibleDB._setup, BibleImport class and mocked parent + # WHEN: Creating an instance of BibleImport with no key word arguments + instance = BibleImport(MagicMock()) + + # THEN: The filename attribute should be None + self.assertIsNone(instance.filename) + self.assertIsInstance(instance, BibleDB) + + def init_kwargs_set_test(self): + """ + Test the initialisation of the BibleImport Class when supplied with select keyword arguments + """ + # GIVEN: A patched BibleDB._setup, BibleImport class and mocked parent + # WHEN: Creating an instance of BibleImport with selected key word arguments + kwargs = {'filename': 'bible.xml'} + instance = BibleImport(MagicMock(), **kwargs) + + # THEN: The filename keyword should be set to bible.xml + self.assertEqual(instance.filename, 'bible.xml') + self.assertIsInstance(instance, BibleDB) + def get_language_id_language_found_test(self): """ Test get_language_id() when called with a name found in the languages list @@ -81,8 +108,7 @@ class TestBibleImport(TestCase): Test get_language_id() when called with a name not found in the languages list """ # GIVEN: A mocked languages.get_language which returns language and an instance of BibleImport - with patch('openlp.core.common.languages.get_language', return_value=None) \ - as mocked_languages_get_language, \ + with patch('openlp.core.common.languages.get_language', return_value=None) as mocked_languages_get_language, \ patch('openlp.plugins.bibles.lib.db.BibleDB.get_language', return_value=20) as mocked_db_get_language: instance = BibleImport(MagicMock()) instance.save_meta = MagicMock() diff --git a/tests/functional/openlp_plugins/bibles/test_http.py b/tests/functional/openlp_plugins/bibles/test_bibleserver.py similarity index 100% rename from tests/functional/openlp_plugins/bibles/test_http.py rename to tests/functional/openlp_plugins/bibles/test_bibleserver.py diff --git a/tests/functional/openlp_plugins/bibles/test_csvimport.py b/tests/functional/openlp_plugins/bibles/test_csvimport.py index ada03a07d..f6d3697af 100644 --- a/tests/functional/openlp_plugins/bibles/test_csvimport.py +++ b/tests/functional/openlp_plugins/bibles/test_csvimport.py @@ -46,10 +46,10 @@ class TestCSVImport(TestCase): def setUp(self): self.manager_patcher = patch('openlp.plugins.bibles.lib.db.Manager') - self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry') self.addCleanup(self.manager_patcher.stop) - self.addCleanup(self.registry_patcher.stop) self.manager_patcher.start() + self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry') + self.addCleanup(self.registry_patcher.stop) self.registry_patcher.start() def test_create_importer(self): @@ -240,7 +240,7 @@ class TestCSVImport(TestCase): importer.wizard = MagicMock() # WHEN: Calling process_verses - result = importer.process_verses([], []) + result = importer.process_verses(['Dummy Verse'], []) # THEN: get_book_name should not be called and the return value should be None self.assertFalse(importer.get_book_name.called) diff --git a/tests/functional/openlp_plugins/bibles/test_opensongimport.py b/tests/functional/openlp_plugins/bibles/test_opensongimport.py index d6997135b..0f5c404ac 100644 --- a/tests/functional/openlp_plugins/bibles/test_opensongimport.py +++ b/tests/functional/openlp_plugins/bibles/test_opensongimport.py @@ -23,32 +23,37 @@ This module contains tests for the OpenSong Bible importer. """ -import os import json +import os from unittest import TestCase -from tests.functional import MagicMock, patch -from openlp.plugins.bibles.lib.importers.opensong import OpenSongBible -from openlp.plugins.bibles.lib.db import BibleDB +from lxml import etree, objectify + +from tests.functional import MagicMock, patch, call +from tests.helpers.testmixin import TestMixin +from openlp.core.common import Registry +from openlp.core.lib.exceptions import ValidationError +from openlp.plugins.bibles.lib.importers.opensong import OpenSongBible, get_text, parse_chapter_number,\ + parse_verse_number +from openlp.plugins.bibles.lib.bibleimport import BibleImport TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', 'resources', 'bibles')) -class TestOpenSongImport(TestCase): +class TestOpenSongImport(TestCase, TestMixin): """ Test the functions in the :mod:`opensongimport` module. """ def setUp(self): - self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry') - 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() + self.setup_application() + self.app.process_events = MagicMock() + Registry.create() + Registry().register('application', self.app) def test_create_importer(self): """ @@ -61,7 +66,446 @@ class TestOpenSongImport(TestCase): importer = OpenSongBible(mocked_manager, path='.', name='.', filename='') # THEN: The importer should be an instance of BibleDB - self.assertIsInstance(importer, BibleDB) + self.assertIsInstance(importer, BibleImport) + + def get_text_no_text_test(self): + """ + Test that get_text handles elements containing text in a combination of text and tail attributes + """ + # GIVEN: Some test data which contains an empty element and an instance of OpenSongBible + test_data = objectify.fromstring('') + + # WHEN: Calling get_text + result = get_text(test_data) + + # THEN: A blank string should be returned + self.assertEqual(result, '') + + def get_text_text_test(self): + """ + Test that get_text handles elements containing text in a combination of text and tail attributes + """ + # GIVEN: Some test data which contains all possible permutation of text and tail text possible and an instance + # of OpenSongBible + test_data = objectify.fromstring('Element text ' + 'sub_text_tail text sub_text_tail tail ' + 'sub_text text ' + 'sub_tail tail') + + # WHEN: Calling get_text + result = get_text(test_data) + + # THEN: The text returned should be as expected + self.assertEqual(result, 'Element text sub_text_tail text sub_text_tail tail sub_text text sub_tail tail') + + def parse_chapter_number_test(self): + """ + Test parse_chapter_number when supplied with chapter number and an instance of OpenSongBible + """ + # GIVEN: The number 10 represented as a string + # WHEN: Calling parse_chapter_nnumber + result = parse_chapter_number('10', 0) + + # THEN: The 10 should be returned as an Int + self.assertEqual(result, 10) + + def parse_chapter_number_empty_attribute_test(self): + """ + Testparse_chapter_number when the chapter number is an empty string. (Bug #1074727) + """ + # GIVEN: An empty string, and the previous chapter number set as 12 and an instance of OpenSongBible + # WHEN: Calling parse_chapter_number + result = parse_chapter_number('', 12) + + # THEN: parse_chapter_number should increment the previous verse number + self.assertEqual(result, 13) + + def parse_verse_number_valid_verse_no_test(self): + """ + Test parse_verse_number when supplied with a valid verse number + """ + # GIVEN: The number 15 represented as a string and an instance of OpenSongBible + # WHEN: Calling parse_verse_number + result = parse_verse_number('15', 0) + + # THEN: parse_verse_number should return the verse number + self.assertEqual(result, 15) + + def parse_verse_number_verse_range_test(self): + """ + Test parse_verse_number when supplied with a verse range + """ + # GIVEN: The range 24-26 represented as a string + # WHEN: Calling parse_verse_number + result = parse_verse_number('24-26', 0) + + # THEN: parse_verse_number should return the first verse number in the range + self.assertEqual(result, 24) + + def parse_verse_number_invalid_verse_no_test(self): + """ + Test parse_verse_number when supplied with a invalid verse number + """ + # GIVEN: An non numeric string represented as a string + # WHEN: Calling parse_verse_number + result = parse_verse_number('invalid', 41) + + # THEN: parse_verse_number should increment the previous verse number + self.assertEqual(result, 42) + + def parse_verse_number_empty_attribute_test(self): + """ + Test parse_verse_number when the verse number is an empty string. (Bug #1074727) + """ + # GIVEN: An empty string, and the previous verse number set as 14 + # WHEN: Calling parse_verse_number + result = parse_verse_number('', 14) + + # THEN: parse_verse_number should increment the previous verse number + self.assertEqual(result, 15) + + @patch('openlp.plugins.bibles.lib.importers.opensong.log') + def parse_verse_number_invalid_type_test(self, mocked_log): + """ + Test parse_verse_number when the verse number is an invalid type) + """ + # GIVEN: A mocked out log, a Tuple, and the previous verse number set as 12 + # WHEN: Calling parse_verse_number + result = parse_verse_number((1, 2, 3), 12) + + # THEN: parse_verse_number should log the verse number it was called with increment the previous verse number + mocked_log.warning.assert_called_once_with('Illegal verse number: (1, 2, 3)') + self.assertEqual(result, 13) + + @patch('openlp.plugins.bibles.lib.bibleimport.BibleImport.find_and_create_book') + def process_books_stop_import_test(self, mocked_find_and_create_book): + """ + Test process_books when stop_import is set to True + """ + # GIVEN: An isntance of OpenSongBible + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + + # WHEN: stop_import_flag is set to True + importer.stop_import_flag = True + importer.process_books(['Book']) + + # THEN: find_and_create_book should not have been called + self.assertFalse(mocked_find_and_create_book.called) + + @patch('openlp.plugins.bibles.lib.bibleimport.BibleImport.find_and_create_book', + **{'side_effect': ['db_book1', 'db_book2']}) + def process_books_completes_test(self, mocked_find_and_create_book): + """ + Test process_books when it processes all books + """ + # GIVEN: An instance of OpenSongBible Importer and two mocked books + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + + book1 = MagicMock() + book1.attrib = {'n': 'Name1'} + book1.c = 'Chapter1' + book2 = MagicMock() + book2.attrib = {'n': 'Name2'} + book2.c = 'Chapter2' + importer.language_id = 10 + importer.process_chapters = MagicMock() + importer.session = MagicMock() + importer.stop_import_flag = False + + # WHEN: Calling process_books with the two books + importer.process_books([book1, book2]) + + # THEN: find_and_create_book and process_books should be called with the details from the mocked books + self.assertEqual(mocked_find_and_create_book.call_args_list, [call('Name1', 2, 10), call('Name2', 2, 10)]) + self.assertEqual(importer.process_chapters.call_args_list, + [call('db_book1', 'Chapter1'), call('db_book2', 'Chapter2')]) + self.assertEqual(importer.session.commit.call_count, 2) + + def process_chapters_stop_import_test(self): + """ + Test process_chapters when stop_import is set to True + """ + # GIVEN: An isntance of OpenSongBible + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + importer.parse_chapter_number = MagicMock() + + # WHEN: stop_import_flag is set to True + importer.stop_import_flag = True + importer.process_chapters('Book', ['Chapter1']) + + # THEN: importer.parse_chapter_number not have been called + self.assertFalse(importer.parse_chapter_number.called) + + @patch('openlp.plugins.bibles.lib.importers.opensong.translate', **{'side_effect': lambda x, y: y}) + @patch('openlp.plugins.bibles.lib.importers.opensong.parse_chapter_number', **{'side_effect': [1, 2]}) + def process_chapters_completes_test(self, mocked_parse_chapter_number, mocked_translate): + """ + Test process_chapters when it completes + """ + # GIVEN: An instance of OpenSongBible + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + importer.wizard = MagicMock() + + # WHEN: called with some valid data + book = MagicMock() + book.name = "Book" + chapter1 = MagicMock() + chapter1.attrib = {'n': '1'} + chapter1.c = 'Chapter1' + chapter1.v = ['Chapter1 Verses'] + chapter2 = MagicMock() + chapter2.attrib = {'n': '2'} + chapter2.c = 'Chapter2' + chapter2.v = ['Chapter2 Verses'] + + importer.process_verses = MagicMock() + importer.stop_import_flag = False + importer.process_chapters(book, [chapter1, chapter2]) + + # THEN: parse_chapter_number, process_verses and increment_process_bar should have been called + self.assertEqual(mocked_parse_chapter_number.call_args_list, [call('1', 0), call('2', 1)]) + self.assertEqual( + importer.process_verses.call_args_list, + [call(book, 1, ['Chapter1 Verses']), call(book, 2, ['Chapter2 Verses'])]) + self.assertEqual(importer.wizard.increment_progress_bar.call_args_list, + [call('Importing Book 1...'), call('Importing Book 2...')]) + + def process_verses_stop_import_test(self): + """ + Test process_verses when stop_import is set to True + """ + # GIVEN: An isntance of OpenSongBible + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + importer.parse_verse_number = MagicMock() + + # WHEN: stop_import_flag is set to True + importer.stop_import_flag = True + importer.process_verses('Book', 1, 'Verses') + + # THEN: importer.parse_verse_number not have been called + self.assertFalse(importer.parse_verse_number.called) + + @patch('openlp.plugins.bibles.lib.importers.opensong.parse_verse_number', **{'side_effect': [1, 2]}) + @patch('openlp.plugins.bibles.lib.importers.opensong.get_text', **{'side_effect': ['Verse1 Text', 'Verse2 Text']}) + def process_verses_completes_test(self, mocked_get_text, mocked_parse_verse_number): + """ + Test process_verses when it completes + """ + # GIVEN: An instance of OpenSongBible + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + importer.wizard = MagicMock() + + # WHEN: called with some valid data + book = MagicMock() + book.id = 1 + verse1 = MagicMock() + verse1.attrib = {'n': '1'} + verse1.c = 'Chapter1' + verse1.v = ['Chapter1 Verses'] + verse2 = MagicMock() + verse2.attrib = {'n': '2'} + verse2.c = 'Chapter2' + verse2.v = ['Chapter2 Verses'] + + importer.create_verse = MagicMock() + importer.stop_import_flag = False + importer.process_verses(book, 1, [verse1, verse2]) + + # THEN: parse_chapter_number, process_verses and increment_process_bar should have been called + self.assertEqual(mocked_parse_verse_number.call_args_list, [call('1', 0), call('2', 1)]) + self.assertEqual(mocked_get_text.call_args_list, [call(verse1), call(verse2)]) + self.assertEqual( + importer.create_verse.call_args_list, + [call(1, 1, 1, 'Verse1 Text'), call(1, 1, 2, 'Verse2 Text')]) + + @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.is_compressed') + def validate_file_compressed_test(self, mocked_is_compressed): + """ + Test that validate_file raises a ValidationError when supplied with a compressed file + """ + # GIVEN: A mocked is_compressed method which returns True + mocked_is_compressed.return_value = True + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + + # WHEN: Calling validate_file + # THEN: ValidationError should be raised + with self.assertRaises(ValidationError) as context: + importer.validate_file('file.name') + self.assertEqual(context.exception.msg, 'Compressed file') + + @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.parse_xml') + @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.is_compressed', **{'return_value': False}) + def validate_file_bible_test(self, mocked_is_compressed, mocked_parse_xml): + """ + Test that validate_file returns True with valid XML + """ + # GIVEN: Some test data with an OpenSong Bible "bible" root tag + mocked_parse_xml.return_value = objectify.fromstring('') + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + + # WHEN: Calling validate_file + result = importer.validate_file('file.name') + + # THEN: A True should be returned + self.assertTrue(result) + + @patch('openlp.plugins.bibles.lib.importers.opensong.critical_error_message_box') + @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.parse_xml') + @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.is_compressed', **{'return_value': False}) + def validate_file_zefania_root_test(self, mocked_is_compressed, mocked_parse_xml, mocked_message_box): + """ + Test that validate_file raises a ValidationError with a Zefinia root tag + """ + # GIVEN: Some test data with a Zefinia "XMLBIBLE" root tag + mocked_parse_xml.return_value = objectify.fromstring('') + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + + # WHEN: Calling validate_file + # THEN: critical_error_message_box should be called and an ValidationError should be raised + with self.assertRaises(ValidationError) as context: + importer.validate_file('file.name') + self.assertEqual(context.exception.msg, 'Invalid xml.') + mocked_message_box.assert_called_once_with( + message='Incorrect Bible file type supplied. This looks like a Zefania XML bible, please use the ' + 'Zefania import option.') + + @patch('openlp.plugins.bibles.lib.importers.opensong.critical_error_message_box') + @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.parse_xml') + @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.is_compressed', **{'return_value': False}) + def validate_file_invalid_root_test(self, mocked_is_compressed, mocked_parse_xml, mocked_message_box): + """ + Test that validate_file raises a ValidationError with an invalid root tag + """ + # GIVEN: Some test data with an invalid root tag and an instance of OpenSongBible + mocked_parse_xml.return_value = objectify.fromstring('') + importer = OpenSongBible(MagicMock(), path='.', name='.', filename='') + + # WHEN: Calling validate_file + # THEN: ValidationError should be raised, and the critical error message box should not have been called + with self.assertRaises(ValidationError) as context: + importer.validate_file('file.name') + 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): + """ + Test do_import when an AttributeError exception is raised + """ + # 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() + + # 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) + + @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): + """ + 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() + + # 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) + + @patch('openlp.plugins.bibles.lib.importers.opensong.log') + def do_import_stop_import_test(self, mocked_log): + """ + 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 + + # 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.assertTrue(importer.application.process_events.called) + + self.assertTrue(importer.application.process_events.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 + 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 + + # WHEN: Calling do_import + result = importer.do_import() + + # THEN: do_import should return True + self.assertTrue(result) def test_file_import(self): """ @@ -92,22 +536,3 @@ class TestOpenSongImport(TestCase): 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, int(verse_tag), verse_text) - - def test_zefania_import_error(self): - """ - Test that we give an error message if trying to import a zefania bible - """ - # GIVEN: A mocked out "manager" and mocked out critical_error_message_box and an import - with patch('openlp.plugins.bibles.lib.importers.opensong.critical_error_message_box') as \ - mocked_critical_error_message_box: - mocked_manager = MagicMock() - importer = OpenSongBible(mocked_manager, path='.', name='.', filename='') - - # WHEN: An trying to import a zefania bible - importer.filename = os.path.join(TEST_PATH, 'zefania-dk1933.xml') - importer.do_import() - - # THEN: The importer should have "shown" an error message - mocked_critical_error_message_box.assert_called_with(message='Incorrect Bible file type supplied. ' - 'This looks like a Zefania XML bible, ' - 'please use the Zefania import option.') diff --git a/tests/functional/openlp_plugins/bibles/test_zefaniaimport.py b/tests/functional/openlp_plugins/bibles/test_zefaniaimport.py index 200a36f45..5294b7f5c 100644 --- a/tests/functional/openlp_plugins/bibles/test_zefaniaimport.py +++ b/tests/functional/openlp_plugins/bibles/test_zefaniaimport.py @@ -42,14 +42,12 @@ class TestZefaniaImport(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 Zefania file importer