diff --git a/openlp/plugins/bibles/lib/importers/opensong.py b/openlp/plugins/bibles/lib/importers/opensong.py index 201bea1da..01be8407e 100644 --- a/openlp/plugins/bibles/lib/importers/opensong.py +++ b/openlp/plugins/bibles/lib/importers/opensong.py @@ -32,84 +32,62 @@ from openlp.plugins.bibles.lib.bibleimport import BibleImport 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. """ - @staticmethod - 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 += OpenSongBible.get_text(sub_element) - if element.tail: - verse_text += element.tail - return verse_text - - @staticmethod - 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 - - @staticmethod - 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 - - @staticmethod - def validate_file(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 = BibleImport.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.')) - raise ValidationError(msg='Invalid xml.') - return True - def process_books(self, books): """ Extract and create the books from the objectified xml @@ -136,7 +114,7 @@ class OpenSongBible(BibleImport): for chapter in chapters: if self.stop_import_flag: break - chapter_number = self.parse_chapter_number(chapter.attrib['n'], chapter_number) + 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}...' @@ -155,8 +133,29 @@ class OpenSongBible(BibleImport): for verse in verses: if self.stop_import_flag: break - verse_number = self.parse_verse_number(verse.attrib['n'], verse_number) - self.create_verse(book.id, chapter_number, verse_number, self.get_text(verse)) + 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.')) + raise ValidationError(msg='Invalid xml.') + return True def do_import(self, bible_name=None): """ diff --git a/tests/functional/openlp_plugins/bibles/test_opensongimport.py b/tests/functional/openlp_plugins/bibles/test_opensongimport.py index ee4e794c0..0f5c404ac 100644 --- a/tests/functional/openlp_plugins/bibles/test_opensongimport.py +++ b/tests/functional/openlp_plugins/bibles/test_opensongimport.py @@ -33,7 +33,8 @@ 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 +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__), @@ -75,7 +76,7 @@ class TestOpenSongImport(TestCase, TestMixin): test_data = objectify.fromstring('') # WHEN: Calling get_text - result = OpenSongBible.get_text(test_data) + result = get_text(test_data) # THEN: A blank string should be returned self.assertEqual(result, '') @@ -92,7 +93,7 @@ class TestOpenSongImport(TestCase, TestMixin): 'sub_tail tail') # WHEN: Calling get_text - result = OpenSongBible.get_text(test_data) + 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') @@ -103,7 +104,7 @@ class TestOpenSongImport(TestCase, TestMixin): """ # GIVEN: The number 10 represented as a string # WHEN: Calling parse_chapter_nnumber - result = OpenSongBible.parse_chapter_number('10', 0) + result = parse_chapter_number('10', 0) # THEN: The 10 should be returned as an Int self.assertEqual(result, 10) @@ -114,7 +115,7 @@ class TestOpenSongImport(TestCase, TestMixin): """ # GIVEN: An empty string, and the previous chapter number set as 12 and an instance of OpenSongBible # WHEN: Calling parse_chapter_number - result = OpenSongBible.parse_chapter_number('', 12) + result = parse_chapter_number('', 12) # THEN: parse_chapter_number should increment the previous verse number self.assertEqual(result, 13) @@ -125,7 +126,7 @@ class TestOpenSongImport(TestCase, TestMixin): """ # GIVEN: The number 15 represented as a string and an instance of OpenSongBible # WHEN: Calling parse_verse_number - result = OpenSongBible.parse_verse_number('15', 0) + result = parse_verse_number('15', 0) # THEN: parse_verse_number should return the verse number self.assertEqual(result, 15) @@ -136,7 +137,7 @@ class TestOpenSongImport(TestCase, TestMixin): """ # GIVEN: The range 24-26 represented as a string # WHEN: Calling parse_verse_number - result = OpenSongBible.parse_verse_number('24-26', 0) + result = parse_verse_number('24-26', 0) # THEN: parse_verse_number should return the first verse number in the range self.assertEqual(result, 24) @@ -147,7 +148,7 @@ class TestOpenSongImport(TestCase, TestMixin): """ # GIVEN: An non numeric string represented as a string # WHEN: Calling parse_verse_number - result = OpenSongBible.parse_verse_number('invalid', 41) + result = parse_verse_number('invalid', 41) # THEN: parse_verse_number should increment the previous verse number self.assertEqual(result, 42) @@ -158,7 +159,7 @@ class TestOpenSongImport(TestCase, TestMixin): """ # GIVEN: An empty string, and the previous verse number set as 14 # WHEN: Calling parse_verse_number - result = OpenSongBible.parse_verse_number('', 14) + result = parse_verse_number('', 14) # THEN: parse_verse_number should increment the previous verse number self.assertEqual(result, 15) @@ -170,7 +171,7 @@ class TestOpenSongImport(TestCase, TestMixin): """ # GIVEN: A mocked out log, a Tuple, and the previous verse number set as 12 # WHEN: Calling parse_verse_number - result = OpenSongBible.parse_verse_number((1, 2, 3), 12) + 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)') @@ -236,14 +237,13 @@ class TestOpenSongImport(TestCase, TestMixin): self.assertFalse(importer.parse_chapter_number.called) @patch('openlp.plugins.bibles.lib.importers.opensong.translate', **{'side_effect': lambda x, y: y}) - def process_chapters_completes_test(self, mocked_translate): + @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.parse_chapter_number = MagicMock() - importer.parse_chapter_number.side_effect = [1, 2] importer.wizard = MagicMock() # WHEN: called with some valid data @@ -263,7 +263,7 @@ class TestOpenSongImport(TestCase, TestMixin): importer.process_chapters(book, [chapter1, chapter2]) # THEN: parse_chapter_number, process_verses and increment_process_bar should have been called - self.assertEqual(importer.parse_chapter_number.call_args_list, [call('1', 0), call('2', 1)]) + 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'])]) @@ -285,16 +285,14 @@ class TestOpenSongImport(TestCase, TestMixin): # THEN: importer.parse_verse_number not have been called self.assertFalse(importer.parse_verse_number.called) - def process_verses_completes_test(self): + @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.get_text = MagicMock() - importer.get_text.side_effect = ['Verse1 Text', 'Verse2 Text'] - importer.parse_verse_number = MagicMock() - importer.parse_verse_number.side_effect = [1, 2] importer.wizard = MagicMock() # WHEN: called with some valid data @@ -314,8 +312,8 @@ class TestOpenSongImport(TestCase, TestMixin): importer.process_verses(book, 1, [verse1, verse2]) # THEN: parse_chapter_number, process_verses and increment_process_bar should have been called - self.assertEqual(importer.parse_verse_number.call_args_list, [call('1', 0), call('2', 1)]) - self.assertEqual(importer.get_text.call_args_list, [call(verse1), call(verse2)]) + 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')]) @@ -327,11 +325,12 @@ class TestOpenSongImport(TestCase, TestMixin): """ # 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: - OpenSongBible.validate_file('file.name') + importer.validate_file('file.name') self.assertEqual(context.exception.msg, 'Compressed file') @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.parse_xml') @@ -342,9 +341,10 @@ class TestOpenSongImport(TestCase, TestMixin): """ # 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 = OpenSongBible.validate_file('file.name') + result = importer.validate_file('file.name') # THEN: A True should be returned self.assertTrue(result) @@ -358,11 +358,12 @@ class TestOpenSongImport(TestCase, TestMixin): """ # 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: - OpenSongBible.validate_file('file.name') + 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 ' @@ -377,11 +378,12 @@ class TestOpenSongImport(TestCase, TestMixin): """ # 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: - OpenSongBible.validate_file('file.name') + importer.validate_file('file.name') self.assertEqual(context.exception.msg, 'Invalid xml.') self.assertFalse(mocked_message_box.called)