Opensong refactors and tests

This commit is contained in:
Philip Ridout 2016-08-18 07:31:36 +01:00
parent 6ab2686b09
commit 46b6d041cd
3 changed files with 220 additions and 61 deletions

View File

@ -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__)
@ -39,6 +41,21 @@ class BibleImport(OpenLPMixin, BibleDB):
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.

View File

@ -21,12 +21,12 @@
###############################################################################
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__)
@ -51,12 +51,69 @@ class OpenSongBible(BibleImport):
verse_text += element.tail
return verse_text
@staticmethod
def process_chapter_no(number, previous_number):
"""
Process 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 process_verse_no(number, previous_number):
"""
Process 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():
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 do_import(self, bible_name=None):
"""
Loads a Bible from file.
"""
self.validate_file(self.filename)
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'
@ -78,46 +135,21 @@ class OpenSongBible(BibleImport):
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
chapter_number = self.process_chapter_no(chapter.attrib['n'], chapter_number)
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
verse_number = self.process_verse_no(verse.attrib['n'], verse_number)
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.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

View File

@ -27,9 +27,13 @@ import os
import json
from unittest import TestCase
from lxml import objectify
from tests.functional import MagicMock, patch
from openlp.core.lib.exceptions import ValidationError
from openlp.plugins.bibles.lib.importers.opensong import OpenSongBible
from openlp.plugins.bibles.lib.db import BibleDB
from openlp.plugins.bibles.lib.bibleimport import BibleImport
TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__),
'..', '..', '..', 'resources', 'bibles'))
@ -41,14 +45,12 @@ class TestOpenSongImport(TestCase):
"""
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.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry')
self.addCleanup(self.registry_patcher.stop)
self.registry_patcher.start()
def test_create_importer(self):
"""
@ -61,7 +63,134 @@ 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 process_chapter_no_test(self):
"""
Test process_chapter_no when supplied with chapter number and an instance of OpenSongBible
"""
# GIVEN: The number 10 represented as a string
# WHEN: Calling process_chapter_no
result = OpenSongBible.process_chapter_no('10', 0)
# THEN: The 10 should be returned as an Int
self.assertEqual(result, 10)
def process_chapter_no_empty_attribute_test(self):
"""
Test process_chapter_no 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 process_chapter_no
result = OpenSongBible.process_chapter_no('', 12)
# THEN: process_chapter_no should increment the previous verse number
self.assertEqual(result, 13)
def process_verse_no_valid_verse_no_test(self):
"""
Test process_verse_no when supplied with a valid verse number
"""
# GIVEN: The number 15 represented as a string and an instance of OpenSongBible
# WHEN: Calling process_verse_no
result = OpenSongBible.process_verse_no('15', 0)
# THEN: process_verse_no should return the verse number
self.assertEqual(result, 15)
def process_verse_no_verse_range_test(self):
"""
Test process_verse_no when supplied with a verse range
"""
# GIVEN: The range 24-26 represented as a string
# WHEN: Calling process_verse_no
result = OpenSongBible.process_verse_no('24-26', 0)
# THEN: process_verse_no should return the first verse number in the range
self.assertEqual(result, 24)
def process_verse_no_invalid_verse_no_test(self):
"""
Test process_verse_no when supplied with a invalid verse number
"""
# GIVEN: An non numeric string represented as a string
# WHEN: Calling process_verse_no
result = OpenSongBible.process_verse_no('invalid', 41)
# THEN: process_verse_no should increment the previous verse number
self.assertEqual(result, 42)
def process_verse_no_empty_attribute_test(self):
"""
Test process_verse_no when the verse number is an empty string. (Bug #1074727)
"""
# GIVEN: An empty string, and the previous verse number set as 14
# WHEN: Calling process_verse_no
result = OpenSongBible.process_verse_no('', 14)
# THEN: process_verse_no should increment the previous verse number
self.assertEqual(result, 15)
@patch('openlp.plugins.bibles.lib.importers.opensong.log')
def process_verse_no_invalid_type_test(self, mocked_log):
"""
Test process_verse_no 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 process_verse_no
result = OpenSongBible.process_verse_no((1,2,3), 12)
# THEN: process_verse_no 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.importers.opensong.BibleImport')
def validate_xml_bible_test(self, mocked_bible_import):
"""
Test that validate_xml returns True with valid XML
"""
# GIVEN: Some test data with an OpenSong Bible "bible" root tag
mocked_bible_import.parse_xml.return_value = objectify.fromstring('<bible></bible>')
# WHEN: Calling validate_xml
result = OpenSongBible.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')
def validate_xml_zefania_root_test(self, mocked_bible_import, mocked_message_box):
"""
Test that validate_xml raises a ValidationError with a Zefinia root tag
"""
# GIVEN: Some test data with a Zefinia "XMLBIBLE" root tag
mocked_bible_import.parse_xml.return_value = objectify.fromstring('<XMLBIBLE></XMLBIBLE>')
# WHEN: Calling validate_xml
# 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')
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')
def validate_xml_invalid_root_test(self, mocked_bible_import, mocked_message_box):
"""
Test that validate_xml raises a ValidationError with an invalid root tag
"""
# GIVEN: Some test data with an invalid root tag and an instance of OpenSongBible
mocked_bible_import.parse_xml.return_value = objectify.fromstring('<song></song>')
# WHEN: Calling validate_xml
# 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')
self.assertEqual(context.exception.msg, 'Invalid xml.')
self.assertFalse(mocked_message_box.called)
def test_file_import(self):
"""
@ -92,22 +221,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.')