From a4d6120bd4855f4f43e52b9ef19a84d2f0b7cb20 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 7 Aug 2016 11:15:43 +0100 Subject: [PATCH] Refactor xml parsing and language detection out in to a bible importer class --- openlp/plugins/bibles/bibleplugin.py | 42 ---- openlp/plugins/bibles/forms/__init__.py | 24 +- openlp/plugins/bibles/lib/csvbible.py | 12 +- openlp/plugins/bibles/lib/db.py | 137 +---------- openlp/plugins/bibles/lib/http.py | 13 +- openlp/plugins/bibles/lib/manager.py | 1 - openlp/plugins/bibles/lib/opensong.py | 20 +- openlp/plugins/bibles/lib/osis.py | 121 +++++----- openlp/plugins/bibles/lib/sword.py | 19 +- openlp/plugins/bibles/lib/zefania.py | 43 ++-- .../openlp_plugins/bibles/test_bibleimport.py | 215 ++++++++++++++++++ 11 files changed, 312 insertions(+), 335 deletions(-) create mode 100644 tests/functional/openlp_plugins/bibles/test_bibleimport.py diff --git a/openlp/plugins/bibles/bibleplugin.py b/openlp/plugins/bibles/bibleplugin.py index ccc61ba56..070a9cd66 100644 --- a/openlp/plugins/bibles/bibleplugin.py +++ b/openlp/plugins/bibles/bibleplugin.py @@ -22,12 +22,9 @@ import logging -from PyQt5 import QtWidgets - from openlp.core.common.actions import ActionList from openlp.core.lib import Plugin, StringContent, build_icon, translate from openlp.core.lib.ui import UiStrings, create_action -from openlp.plugins.bibles.forms import BibleUpgradeForm from openlp.plugins.bibles.lib import BibleManager, BiblesTab, BibleMediaItem, LayoutStyle, DisplayStyle, \ LanguageSelection from openlp.plugins.bibles.lib.mediaitem import BibleSearch @@ -87,7 +84,6 @@ class BiblePlugin(Plugin): action_list.add_action(self.import_bible_item, UiStrings().Import) # Set to invisible until we can export bibles self.export_bible_item.setVisible(False) - self.tools_upgrade_item.setVisible(bool(self.manager.old_bible_databases)) def finalise(self): """ @@ -101,20 +97,6 @@ class BiblePlugin(Plugin): self.import_bible_item.setVisible(False) self.export_bible_item.setVisible(False) - def app_startup(self): - """ - Perform tasks on application startup - """ - super(BiblePlugin, self).app_startup() - if self.manager.old_bible_databases: - if QtWidgets.QMessageBox.information( - self.main_window, translate('OpenLP', 'Information'), - translate('OpenLP', 'Bible format has changed.\nYou have to upgrade your ' - 'existing Bibles.\nShould OpenLP upgrade now?'), - QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No)) == \ - QtWidgets.QMessageBox.Yes: - self.on_tools_upgrade_item_triggered() - def add_import_menu_item(self, import_menu): """ Add an import menu item @@ -136,30 +118,6 @@ class BiblePlugin(Plugin): text=translate('BiblesPlugin', '&Bible'), visible=False) export_menu.addAction(self.export_bible_item) - def add_tools_menu_item(self, tools_menu): - """ - Give the bible plugin the opportunity to add items to the **Tools** menu. - - :param tools_menu: The actual **Tools** menu item, so that your actions can use it as their parent. - """ - log.debug('add tools menu') - self.tools_upgrade_item = create_action( - tools_menu, 'toolsUpgradeItem', - text=translate('BiblesPlugin', '&Upgrade older Bibles'), - statustip=translate('BiblesPlugin', 'Upgrade the Bible databases to the latest format.'), - visible=False, triggers=self.on_tools_upgrade_item_triggered) - tools_menu.addAction(self.tools_upgrade_item) - - def on_tools_upgrade_item_triggered(self): - """ - Upgrade older bible databases. - """ - if not hasattr(self, 'upgrade_wizard'): - self.upgrade_wizard = BibleUpgradeForm(self.main_window, self.manager, self) - # If the import was not cancelled then reload. - if self.upgrade_wizard.exec(): - self.media_item.reload_bibles() - def on_bible_import_click(self): """ Show the Bible Import wizard diff --git a/openlp/plugins/bibles/forms/__init__.py b/openlp/plugins/bibles/forms/__init__.py index 145a8a2fa..c116766ae 100644 --- a/openlp/plugins/bibles/forms/__init__.py +++ b/openlp/plugins/bibles/forms/__init__.py @@ -21,30 +21,12 @@ ############################################################################### """ -Forms in OpenLP are made up of two classes. One class holds all the graphical elements, like buttons and lists, and the -other class holds all the functional code, like slots and loading and saving. - -The first class, commonly known as the **Dialog** class, is typically named ``Ui_Dialog``. It is a slightly -modified version of the class that the ``pyuic5`` command produces from Qt5's .ui file. Typical modifications will be -converting most strings from "" to '' and using OpenLP's ``translate()`` function for translating strings. - -The second class, commonly known as the **Form** class, is typically named ``Form``. This class is the one which -is instantiated and used. It uses dual inheritance to inherit from (usually) QtWidgets.QDialog and the Ui class -mentioned above, like so:: - - class BibleImportForm(QtWidgets.QWizard, Ui_BibleImportWizard): - - def __init__(self, parent, manager, bible_plugin): - super(BibleImportForm, self).__init__(parent) - self.setupUi(self) - -This allows OpenLP to use ``self.object`` for all the GUI elements while keeping them separate from the functionality, -so that it is easier to recreate the GUI from the .ui files later if necessary. +The :mod:`forms` module contains all the ui functionality for the bibles +plugin. """ from .booknameform import BookNameForm from .languageform import LanguageForm from .bibleimportform import BibleImportForm -from .bibleupgradeform import BibleUpgradeForm from .editbibleform import EditBibleForm -__all__ = ['BookNameForm', 'LanguageForm', 'BibleImportForm', 'BibleUpgradeForm', 'EditBibleForm'] +__all__ = ['BookNameForm', 'LanguageForm', 'BibleImportForm', 'EditBibleForm'] diff --git a/openlp/plugins/bibles/lib/csvbible.py b/openlp/plugins/bibles/lib/csvbible.py index ef8616051..7e04d7535 100644 --- a/openlp/plugins/bibles/lib/csvbible.py +++ b/openlp/plugins/bibles/lib/csvbible.py @@ -54,25 +54,26 @@ import chardet import csv from openlp.core.common import translate -from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB +from openlp.plugins.bibles.lib.bibleimport import BibleImport +from openlp.plugins.bibles.lib.db import BiblesResourcesDB log = logging.getLogger(__name__) -class CSVBible(BibleDB): +class CSVBible(BibleImport): """ This class provides a specialisation for importing of CSV Bibles. """ log.info('CSVBible loaded') - def __init__(self, parent, **kwargs): + def __init__(self, *args, **kwargs): """ Loads a Bible from a set of CSV files. This class assumes the files contain all the information and a clean bible is being loaded. """ log.info(self.__class__.__name__) - BibleDB.__init__(self, parent, **kwargs) + super().__init__(*args, **kwargs) self.books_file = kwargs['booksfile'] self.verses_file = kwargs['versefile'] @@ -84,9 +85,8 @@ class CSVBible(BibleDB): self.wizard.progress_bar.setMinimum(0) self.wizard.progress_bar.setMaximum(66) success = True - language_id = self.get_language(bible_name) + language_id = self.get_language_id(bible_name=self.books_file) if not language_id: - log.error('Importing books from "{name}" failed'.format(name=self.filename)) return False books_file = None book_list = {} diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py index ff305dec6..e5142ae98 100644 --- a/openlp/plugins/bibles/lib/db.py +++ b/openlp/plugins/bibles/lib/db.py @@ -477,7 +477,7 @@ class BibleDB(Manager, RegistryProperties): combo_box = language_form.language_combo_box language_id = combo_box.itemData(combo_box.currentIndex()) if not language_id: - return False + return None self.save_meta('language_id', language_id) return language_id @@ -864,138 +864,3 @@ class AlternativeBookNamesDB(QtCore.QObject, Manager): return AlternativeBookNamesDB.run_sql( 'INSERT INTO alternative_book_names(book_reference_id, language_id, name) ' 'VALUES (?, ?, ?)', (book_reference_id, language_id, name), True) - - -class OldBibleDB(QtCore.QObject, Manager): - """ - This class connects to the old bible databases to reimport them to the new - database scheme. - """ - cursor = None - - def __init__(self, parent, **kwargs): - """ - The constructor loads up the database and creates and initialises the tables if the database doesn't exist. - - **Required keyword arguments:** - - ``path`` - The path to the bible database file. - - ``name`` - The name of the database. This is also used as the file name for SQLite databases. - """ - log.info('OldBibleDB loaded') - QtCore.QObject.__init__(self) - if 'path' not in kwargs: - raise KeyError('Missing keyword argument "path".') - if 'file' not in kwargs: - raise KeyError('Missing keyword argument "file".') - if 'path' in kwargs: - self.path = kwargs['path'] - if 'file' in kwargs: - self.file = kwargs['file'] - - def get_cursor(self): - """ - Return the cursor object. Instantiate one if it doesn't exist yet. - """ - if self.cursor is None: - file_path = os.path.join(self.path, self.file) - self.connection = sqlite3.connect(file_path) - self.cursor = self.connection.cursor() - return self.cursor - - def run_sql(self, query, parameters=()): - """ - Run an SQL query on the database, returning the results. - - :param query: The actual SQL query to run. - :param parameters: Any variable parameters to add to the query. - """ - cursor = self.get_cursor() - cursor.execute(query, parameters) - return cursor.fetchall() - - def get_name(self): - """ - Returns the version name of the Bible. - """ - self.name = None - version_name = self.run_sql('SELECT value FROM metadata WHERE key = "name"') - if version_name: - self.name = version_name[0][0] - else: - # Fallback to old way of naming - version_name = self.run_sql('SELECT value FROM metadata WHERE key = "Version"') - if version_name: - self.name = version_name[0][0] - return self.name - - def get_metadata(self): - """ - Returns the metadata of the Bible. - """ - metadata = self.run_sql('SELECT key, value FROM metadata ORDER BY rowid') - if metadata: - return [{ - 'key': str(meta[0]), - 'value': str(meta[1]) - } for meta in metadata] - else: - return None - - def get_book(self, name): - """ - Return a book by name or abbreviation. - - ``name`` - The name or abbreviation of the book. - """ - if not isinstance(name, str): - name = str(name) - books = self.run_sql( - 'SELECT id, testament_id, name, abbreviation FROM book WHERE LOWER(name) = ? OR ' - 'LOWER(abbreviation) = ?', (name.lower(), name.lower())) - if books: - return { - 'id': books[0][0], - 'testament_id': books[0][1], - 'name': str(books[0][2]), - 'abbreviation': str(books[0][3]) - } - else: - return None - - def get_books(self): - """ - Returns the books of the Bible. - """ - books = self.run_sql('SELECT name, id FROM book ORDER BY id') - if books: - return [{ - 'name': str(book[0]), - 'id':int(book[1]) - } for book in books] - else: - return None - - def get_verses(self, book_id): - """ - Returns the verses of the Bible. - """ - verses = self.run_sql( - 'SELECT book_id, chapter, verse, text FROM verse WHERE book_id = ? ORDER BY id', (book_id, )) - if verses: - return [{ - 'book_id': int(verse[0]), - 'chapter': int(verse[1]), - 'verse': int(verse[2]), - 'text': str(verse[3]) - } for verse in verses] - else: - return None - - def close_connection(self): - self.cursor.close() - self.connection.close() diff --git a/openlp/plugins/bibles/lib/http.py b/openlp/plugins/bibles/lib/http.py index 392cce05a..6921c9005 100644 --- a/openlp/plugins/bibles/lib/http.py +++ b/openlp/plugins/bibles/lib/http.py @@ -34,6 +34,7 @@ from openlp.core.common import Registry, RegistryProperties, translate from openlp.core.lib.ui import critical_error_message_box from openlp.core.lib.webpagereader import get_web_page from openlp.plugins.bibles.lib import SearchResults +from openlp.plugins.bibles.lib.bibleimport import BibleImport from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB, Book CLEANER_REGEX = re.compile(r' |
|\'\+\'') @@ -576,10 +577,10 @@ class CWExtract(RegistryProperties): return bibles -class HTTPBible(BibleDB, RegistryProperties): +class HTTPBible(BibleImport, RegistryProperties): log.info('{name} HTTPBible loaded'.format(name=__name__)) - def __init__(self, parent, **kwargs): + def __init__(self, *args, **kwargs): """ Finds all the bibles defined for the system. Creates an Interface Object for each bible containing connection information. @@ -588,7 +589,7 @@ class HTTPBible(BibleDB, RegistryProperties): Init confirms the bible exists and stores the database path. """ - BibleDB.__init__(self, parent, **kwargs) + super().__init__(*args, **kwargs) self.download_source = kwargs['download_source'] self.download_name = kwargs['download_name'] # TODO: Clean up proxy stuff. We probably want one global proxy per connection type (HTTP and HTTPS) at most. @@ -638,12 +639,8 @@ class HTTPBible(BibleDB, RegistryProperties): return False self.wizard.progress_bar.setMaximum(len(books) + 2) self.wizard.increment_progress_bar(translate('BiblesPlugin.HTTPBible', 'Registering Language...')) - if self.language_id: - self.save_meta('language_id', self.language_id) - else: - self.language_id = self.get_language(bible_name) + self.language_id = self.get_language_id(bible_name=bible_name) if not self.language_id: - log.error('Importing books from {name} failed'.format(name=self.filename)) return False for book in books: if self.stop_import_flag: diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py index 1c55222f2..3e69d309f 100644 --- a/openlp/plugins/bibles/lib/manager.py +++ b/openlp/plugins/bibles/lib/manager.py @@ -124,7 +124,6 @@ class BibleManager(RegistryProperties): files.remove('alternative_book_names.sqlite') log.debug('Bible Files {text}'.format(text=files)) self.db_cache = {} - self.old_bible_databases = [] for filename in files: bible = BibleDB(self.parent, path=self.path, file=filename) if not bible.session: diff --git a/openlp/plugins/bibles/lib/opensong.py b/openlp/plugins/bibles/lib/opensong.py index 9c01a1fe0..b358d481a 100644 --- a/openlp/plugins/bibles/lib/opensong.py +++ b/openlp/plugins/bibles/lib/opensong.py @@ -25,23 +25,24 @@ from lxml import etree, objectify from openlp.core.common import translate, trace_error_handler 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__) -class OpenSongBible(BibleDB): +class OpenSongBible(BibleImport): """ OpenSong Bible format importer class. """ - def __init__(self, parent, **kwargs): + def __init__(self, *args, **kwargs): """ Constructor to create and set up an instance of the OpenSongBible class. This class is used to import Bibles from OpenSong's XML format. """ log.debug(self.__class__.__name__) - BibleDB.__init__(self, parent, **kwargs) + super().__init__(*args, **kwargs) self.filename = kwargs['filename'] def get_text(self, element): @@ -66,14 +67,9 @@ class OpenSongBible(BibleDB): log.debug('Starting OpenSong import from "{name}"'.format(name=self.filename)) if not isinstance(self.filename, str): self.filename = str(self.filename, 'utf8') - import_file = None success = True try: - # 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. - import_file = open(self.filename, 'rb') - opensong = objectify.parse(import_file) - bible = opensong.getroot() + 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': critical_error_message_box( @@ -82,9 +78,8 @@ class OpenSongBible(BibleDB): 'please use the Zefania import option.')) return False # No language info in the opensong format, so ask the user - language_id = self.get_language(bible_name) + language_id = self.get_language_id(bible_name=self.filename) if not language_id: - log.error('Importing books from "{name}" failed'.format(name=self.filename)) return False for book in bible.b: if self.stop_import_flag: @@ -138,9 +133,6 @@ class OpenSongBible(BibleDB): except (IOError, AttributeError): log.exception('Loading Bible from OpenSong file failed') success = False - finally: - if import_file: - import_file.close() if self.stop_import_flag: return False else: diff --git a/openlp/plugins/bibles/lib/osis.py b/openlp/plugins/bibles/lib/osis.py index 4b217022e..219abf3b2 100644 --- a/openlp/plugins/bibles/lib/osis.py +++ b/openlp/plugins/bibles/lib/osis.py @@ -24,25 +24,64 @@ import logging from lxml import etree from openlp.core.common import languages, translate, trace_error_handler -from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB 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__) +# Tags we don't use and can remove the content +REMOVABLE_ELEMENTS = ('{http://www.bibletechnologies.net/2003/OSIS/namespace}note', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}milestone', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}title', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}abbr', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}catchWord', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}index', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}rdg', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}rdgGroup', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}figure') +# Tags we don't use but need to keep the content +REMOVABLE_TAGS = ('{http://www.bibletechnologies.net/2003/OSIS/namespace}p', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}l', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}lg', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}q', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}a', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}w', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}divineName', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}foreign', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}hi', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}inscription', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}mentioned', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}name', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}reference', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}seg', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}transChange', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}salute', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}signed', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}closer', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}speech', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}speaker', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}list', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}item', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}table', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}head', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}row', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}cell', + '{http://www.bibletechnologies.net/2003/OSIS/namespace}caption') def replacement(match): return match.group(2).upper() -class OSISBible(BibleDB): +class OSISBible(BibleImport): """ `OSIS `_ Bible format importer class. """ log.info('BibleOSISImpl loaded') - def __init__(self, parent, **kwargs): + def __init__(self, *args, **kwargs): log.debug(self.__class__.__name__) - BibleDB.__init__(self, parent, **kwargs) + super().__init__(*args, **kwargs) self.filename = kwargs['filename'] def do_import(self, bible_name=None): @@ -52,71 +91,20 @@ class OSISBible(BibleDB): log.debug('Starting OSIS import from "{name}"'.format(name=self.filename)) if not isinstance(self.filename, str): self.filename = str(self.filename, 'utf8') - import_file = None success = True try: - # 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. - import_file = open(self.filename, 'rb') - osis_bible_tree = etree.parse(import_file, parser=etree.XMLParser(recover=True)) - namespace = {'ns': 'http://www.bibletechnologies.net/2003/OSIS/namespace'} - # Find bible language - language_id = None - lang = osis_bible_tree.xpath("//ns:osisText/@xml:lang", namespaces=namespace) - if lang: - language = languages.get_language(lang[0]) - if hasattr(language, 'id'): - language_id = language.id - # The language couldn't be detected, ask the user - if not language_id: - language_id = self.get_language(bible_name) - if not language_id: - log.error('Importing books from "{name}" failed'.format(name=self.filename)) - return False - self.save_meta('language_id', language_id) - num_books = int(osis_bible_tree.xpath("count(//ns:div[@type='book'])", namespaces=namespace)) self.wizard.increment_progress_bar(translate('BiblesPlugin.OsisImport', 'Removing unused tags (this may take a few minutes)...')) - # We strip unused tags from the XML, this should leave us with only chapter, verse and div tags. - # Strip tags we don't use - remove content - etree.strip_elements(osis_bible_tree, ('{http://www.bibletechnologies.net/2003/OSIS/namespace}note', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}milestone', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}title', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}abbr', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}catchWord', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}index', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}rdg', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}rdgGroup', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}figure'), - with_tail=False) - # Strip tags we don't use - keep content - etree.strip_tags(osis_bible_tree, ('{http://www.bibletechnologies.net/2003/OSIS/namespace}p', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}l', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}lg', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}q', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}a', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}w', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}divineName', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}foreign', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}hi', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}inscription', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}mentioned', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}name', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}reference', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}seg', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}transChange', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}salute', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}signed', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}closer', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}speech', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}speaker', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}list', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}item', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}table', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}head', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}row', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}cell', - '{http://www.bibletechnologies.net/2003/OSIS/namespace}caption')) + osis_bible_tree = self.parse_xml(self.filename, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS) + namespace = {'ns': 'http://www.bibletechnologies.net/2003/OSIS/namespace'} + # Find bible language] + language = osis_bible_tree.xpath("//ns:osisText/@xml:lang", namespaces=namespace) + 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=namespace)) + + # Precompile a few xpath-querys verse_in_chapter = etree.XPath('count(//ns:chapter[1]/ns:verse)', namespaces=namespace) text_in_verse = etree.XPath('count(//ns:verse[1]/text())', namespaces=namespace) @@ -189,9 +177,6 @@ class OSISBible(BibleDB): critical_error_message_box(message=translate('BiblesPlugin.OsisImport', 'The file is not a valid OSIS-XML file:' '\n{text}').format(text=e.msg)) - finally: - if import_file: - import_file.close() if self.stop_import_flag: return False else: diff --git a/openlp/plugins/bibles/lib/sword.py b/openlp/plugins/bibles/lib/sword.py index b37e9b583..00313f344 100644 --- a/openlp/plugins/bibles/lib/sword.py +++ b/openlp/plugins/bibles/lib/sword.py @@ -23,25 +23,26 @@ import logging from pysword import modules -from openlp.core.common import languages, translate +from openlp.core.common import translate from openlp.core.lib.ui import critical_error_message_box -from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB +from openlp.plugins.bibles.lib.bibleimport import BibleImport +from openlp.plugins.bibles.lib.db import BiblesResourcesDB log = logging.getLogger(__name__) -class SwordBible(BibleDB): +class SwordBible(BibleImport): """ SWORD Bible format importer class. """ - def __init__(self, parent, **kwargs): + def __init__(self, *args, **kwargs): """ Constructor to create and set up an instance of the SwordBible class. This class is used to import Bibles from SWORD bible modules. """ log.debug(self.__class__.__name__) - BibleDB.__init__(self, parent, **kwargs) + super().__init__(*args, **kwargs) self.sword_key = kwargs['sword_key'] self.sword_path = kwargs['sword_path'] if self.sword_path == '': @@ -57,13 +58,11 @@ class SwordBible(BibleDB): pysword_modules = modules.SwordModules(self.sword_path) pysword_module_json = pysword_modules.parse_modules()[self.sword_key] bible = pysword_modules.get_bible_from_module(self.sword_key) - language_id = None language = pysword_module_json['lang'] language = language[language.find('.') + 1:] - language = languages.get_language(language) - if hasattr(language, 'id'): - language_id = language.id - self.save_meta('language_id', language_id) + language_id = self.get_language_id(language, bible_name=self.filename) + if not language_id: + return False books = bible.get_structure().get_books() # Count number of books num_books = 0 diff --git a/openlp/plugins/bibles/lib/zefania.py b/openlp/plugins/bibles/lib/zefania.py index 96ab69072..e71075e17 100644 --- a/openlp/plugins/bibles/lib/zefania.py +++ b/openlp/plugins/bibles/lib/zefania.py @@ -25,23 +25,29 @@ from lxml import etree from openlp.core.common import languages, translate 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__) +# Tags we don't use and can remove the content +REMOVABLE_ELEMENTS = ('PROLOG', 'REMARK', 'CAPTION', 'MEDIA') +# Tags we don't use but need to keep the content +REMOVABLE_TAGS = ('STYLE', 'GRAM', 'NOTE', 'SUP', 'XREF') -class ZefaniaBible(BibleDB): + +class ZefaniaBible(BibleImport): """ Zefania Bible format importer class. """ - def __init__(self, parent, **kwargs): + def __init__(self, *args, **kwargs): """ Constructor to create and set up an instance of the ZefaniaBible class. This class is used to import Bibles from ZefaniaBible's XML format. """ log.debug(self.__class__.__name__) - BibleDB.__init__(self, parent, **kwargs) + super().__init__(*args, **kwargs) self.filename = kwargs['filename'] def do_import(self, bible_name=None): @@ -51,34 +57,16 @@ class ZefaniaBible(BibleDB): log.debug('Starting Zefania import from "{name}"'.format(name=self.filename)) if not isinstance(self.filename, str): self.filename = str(self.filename, 'utf8') - import_file = None success = True try: - # 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. - import_file = open(self.filename, 'rb') - zefania_bible_tree = etree.parse(import_file, parser=etree.XMLParser(recover=True)) + xmlbible = self.parse_xml(self.filename, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS) # Find bible language - language_id = None - language = zefania_bible_tree.xpath("/XMLBIBLE/INFORMATION/language/text()") - if language: - language = languages.get_language(language[0]) - if hasattr(language, 'id'): - language_id = language.id - # The language couldn't be detected, ask the user + language = xmlbible.xpath("/XMLBIBLE/INFORMATION/language/text()") + language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename) if not language_id: - language_id = self.get_language(bible_name) - if not language_id: - log.error('Importing books from "{name}" failed'.format(name=self.filename)) return False - self.save_meta('language_id', language_id) - num_books = int(zefania_bible_tree.xpath('count(//BIBLEBOOK)')) - self.wizard.progress_bar.setMaximum(int(zefania_bible_tree.xpath('count(//CHAPTER)'))) - # Strip tags we don't use - keep content - etree.strip_tags(zefania_bible_tree, ('STYLE', 'GRAM', 'NOTE', 'SUP', 'XREF')) - # Strip tags we don't use - remove content - etree.strip_elements(zefania_bible_tree, ('PROLOG', 'REMARK', 'CAPTION', 'MEDIA'), with_tail=False) - xmlbible = zefania_bible_tree.getroot() + num_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: break @@ -116,9 +104,6 @@ class ZefaniaBible(BibleDB): 'compressed. You must decompress them before import.')) log.exception(str(e)) success = False - finally: - if import_file: - import_file.close() if self.stop_import_flag: return False else: diff --git a/tests/functional/openlp_plugins/bibles/test_bibleimport.py b/tests/functional/openlp_plugins/bibles/test_bibleimport.py new file mode 100644 index 000000000..1a8a2ad17 --- /dev/null +++ b/tests/functional/openlp_plugins/bibles/test_bibleimport.py @@ -0,0 +1,215 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2015 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +This module contains tests for the bibleimport module. +""" + +from io import BytesIO +from lxml import etree, objectify + +from unittest import TestCase + +from openlp.core.common.languages import Language +from openlp.plugins.bibles.lib.bibleimport import BibleImport +from tests.functional import MagicMock, patch + + +class TestBibleImport(TestCase): + """ + Test the functions in the :mod:`bibleimport` module. + """ + + def setUp(self): + 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.mock_log = self.log_patcher.start() + self.setup_patcher.start() + + + def get_language_id_language_found_test(self): + """ + Test get_language_id() when called with a name 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=Language(30, 'English', 'en')) \ + as mocked_languages_get_language, \ + patch('openlp.plugins.bibles.lib.db.BibleDB.get_language') as mocked_db_get_language: + instance = BibleImport(MagicMock()) + instance.save_meta = MagicMock() + + # WHEN: Calling get_language_id() with a language name and bible name + result = instance.get_language_id('English', 'KJV') + + # THEN: The id of the language returned from languages.get_language should be returned + mocked_languages_get_language.assert_called_once_with('English') + self.assertFalse(mocked_db_get_language.called) + instance.save_meta.assert_called_once_with('language_id', 30) + self.assertEqual(result, 30) + + def get_language_id_language_not_found_test(self): + """ + 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, \ + patch('openlp.plugins.bibles.lib.db.BibleDB.get_language', return_value=20) as mocked_db_get_language: + instance = BibleImport(MagicMock()) + instance.save_meta = MagicMock() + + # WHEN: Calling get_language_id() with a language name and bible name + result = instance.get_language_id('RUS', 'KJV') + + # THEN: The id of the language returned from languages.get_language should be returned + mocked_languages_get_language.assert_called_once_with('RUS') + mocked_db_get_language.assert_called_once_with('KJV') + instance.save_meta.assert_called_once_with('language_id', 20) + self.assertEqual(result, 20) + + def get_language_id_user_choice_test(self): + """ + Test get_language_id() when the language is not found and the user is asked for the language + """ + # GIVEN: A mocked languages.get_language which returns None a mocked BibleDB.get_language which returns a + # language id. + 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=40) as mocked_db_get_language: + self.mock_log.error.reset_mock() + instance = BibleImport(MagicMock()) + instance.save_meta = MagicMock() + + # WHEN: Calling get_language_id() with a language name and bible name + result = instance.get_language_id('English', 'KJV') + + # THEN: The id of the language returned from BibleDB.get_language should be returned + mocked_languages_get_language.assert_called_once_with('English') + mocked_db_get_language.assert_called_once_with('KJV') + self.assertFalse(self.mock_log.error.called) + instance.save_meta.assert_called_once_with('language_id', 40) + self.assertEqual(result, 40) + + def get_language_id_user_choice_rejected_test(self): + """ + Test get_language_id() when the language is not found and the user rejects the dilaog box + """ + # GIVEN: A mocked languages.get_language which returns None a mocked BibleDB.get_language which returns a + # language id. + 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=None) as mocked_db_get_language: + self.mock_log.error.reset_mock() + instance = BibleImport(MagicMock()) + instance.save_meta = MagicMock() + + # WHEN: Calling get_language_id() with a language name and bible name + result = instance.get_language_id('Qwerty', 'KJV') + + # THEN: None should be returned and an error should be logged + mocked_languages_get_language.assert_called_once_with('Qwerty') + mocked_db_get_language.assert_called_once_with('KJV') + self.mock_log.error.assert_called_once_with('Language detection failed when importing from "KJV". ' + 'User aborted language selection.') + self.assertFalse(instance.save_meta.called) + self.assertIsNone(result) + + def parse_xml_etree_test(self): + """ + Test BibleImport.parse_xml() when called with the use_objectify default value + """ + # GIVEN: A sample "file" to parse + # WHEN: Calling parse_xml + result = BibleImport.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), + b'\n
Test

data

tokeep
\n' + b' Testdatatodiscard\n') + self.assertIsInstance(result, etree._Element) + + def parse_xml_etree_use_objectify_test(self): + """ + Test BibleImport.parse_xml() when called with use_objectify set to True + """ + # GIVEN: A sample "file" to parse + # WHEN: Calling parse_xml + result = BibleImport.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), + b'
Test

data

tokeep
' + b'Testdatatodiscard') + self.assertIsInstance(result, objectify.ObjectifiedElement) + + def parse_xml_elements_test(self): + """ + Test BibleImport.parse_xml() when given a tuple of elements to remove + """ + # GIVEN: A tuple of elements to remove + elements = ('unsupported', 'x', 'y') + + # WHEN: Calling parse_xml, with a test file + result = BibleImport.parse_xml('file.tst', elements=elements) + + # THEN: The result returned should contain the correct data + self.assertEqual(etree.tostring(result), + b'\n
Test

data

tokeep
\n \n') + + def parse_xml_tags_test(self): + """ + Test BibleImport.parse_xml() when given a tuple of tags to remove + """ + # GIVEN: A tuple of tags to remove + tags = ('div', 'p', 'a') + + # WHEN: Calling parse_xml, with a test file + result = BibleImport.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' + b'datatodiscard\n') + + def parse_xml_elements_tags_test(self): + """ + 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 + elements = ('unsupported', 'x', 'y') + tags = ('div', 'p', 'a') + + # WHEN: Calling parse_xml, with a test file + result = BibleImport.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')