From d636461cbba5b6e68e9ea0b53dfede0dd74f09f5 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Fri, 29 Apr 2016 18:30:17 +0200 Subject: [PATCH 01/13] Fix performance in songbook search Speedup by ~500% Fixes: https://launchpad.net/bugs/1552563 --- openlp/plugins/songs/lib/mediaitem.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 8edac2877..d08be8422 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -207,9 +207,11 @@ class SongMediaItem(MediaManagerItem): search_keywords = search_keywords.rpartition(' ') search_book = search_keywords[0] + '%' search_entry = search_keywords[2] + '%' - search_results = (self.plugin.manager.session.query(SongBookEntry) + search_results = (self.plugin.manager.session.query(SongBookEntry.entry, Book.name, Song.title, Song.id) + .join(Song) .join(Book) - .filter(Book.name.like(search_book), SongBookEntry.entry.like(search_entry)).all()) + .filter(Book.name.like(search_book), SongBookEntry.entry.like(search_entry), + Song.temporary.is_(False)).all()) self.display_results_book(search_results) elif search_type == SongSearch.Themes: log.debug('Theme Search') @@ -316,20 +318,17 @@ class SongMediaItem(MediaManagerItem): :param search_results: A list of db SongBookEntry objects :return: None """ - def get_songbook_key(songbook_entry): + def get_songbook_key(result): """Get the key to sort by""" - return (get_natural_key(songbook_entry.songbook.name), get_natural_key(songbook_entry.entry)) + return (get_natural_key(result[1]), get_natural_key(result[0]), get_natural_key(result[2])) log.debug('display results Book') self.list_view.clear() search_results.sort(key=get_songbook_key) - for songbook_entry in search_results: - # Do not display temporary songs - if songbook_entry.song.temporary: - continue - song_detail = '%s #%s: %s' % (songbook_entry.songbook.name, songbook_entry.entry, songbook_entry.song.title) + for result in search_results: + song_detail = '%s #%s: %s' % (result[1], result[0], result[2]) song_name = QtWidgets.QListWidgetItem(song_detail) - song_name.setData(QtCore.Qt.UserRole, songbook_entry.song.id) + song_name.setData(QtCore.Qt.UserRole, result[3]) self.list_view.addItem(song_name) def display_results_topic(self, search_results): From 48c5737bd1eee1dc26ee00d2ab289d0fe8c56baa Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Fri, 29 Apr 2016 18:43:14 +0200 Subject: [PATCH 02/13] Fix comment --- openlp/plugins/songs/lib/mediaitem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index d08be8422..9cc1bc558 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -315,7 +315,7 @@ class SongMediaItem(MediaManagerItem): """ Display the song search results in the media manager list, grouped by book and entry - :param search_results: A list of db SongBookEntry objects + :param search_results: A tuple containing (songbook entry, book name, song title, song id) :return: None """ def get_songbook_key(result): From 30dc1bd2ebf64a2ca73e3aa62587bd17617099b7 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Fri, 29 Apr 2016 18:44:24 +0200 Subject: [PATCH 03/13] Unused import --- openlp/plugins/songs/lib/mediaitem.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 9cc1bc558..11deeb31d 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -21,7 +21,6 @@ ############################################################################### import logging -import re import os import shutil From 868b538dd2625caeb77ffa485c0aaf477654a329 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Fri, 29 Apr 2016 19:10:50 +0200 Subject: [PATCH 04/13] Fix test and add new one --- .../openlp_plugins/songs/test_mediaitem.py | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 4b9fd50ee..85911e640 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -23,6 +23,7 @@ This module contains tests for the lib submodule of the Songs plugin. """ from unittest import TestCase +from unittest.mock import call from PyQt5 import QtCore @@ -151,29 +152,7 @@ class TestMediaItem(TestCase, TestMixin): # GIVEN: Search results grouped by book and entry, plus a mocked QtListWidgetItem with patch('openlp.core.lib.QtWidgets.QListWidgetItem') as MockedQListWidgetItem, \ patch('openlp.core.lib.QtCore.Qt.UserRole') as MockedUserRole: - mock_search_results = [] - mock_songbook_entry = MagicMock() - mock_songbook_entry_temp = MagicMock() - mock_songbook = MagicMock() - mock_song = MagicMock() - mock_song_temp = MagicMock() - mock_songbook_entry.entry = '1' - mock_songbook_entry_temp.entry = '2' - mock_songbook.name = 'My Book' - mock_song.id = 1 - mock_song.title = 'My Song' - mock_song.sort_key = 'My Song' - mock_song.temporary = False - mock_song_temp.id = 2 - mock_song_temp.title = 'My Temporary' - mock_song_temp.sort_key = 'My Temporary' - mock_song_temp.temporary = True - mock_songbook_entry.song = mock_song - mock_songbook_entry.songbook = mock_songbook - mock_songbook_entry_temp.song = mock_song_temp - mock_songbook_entry_temp.songbook = mock_songbook - mock_search_results.append(mock_songbook_entry) - mock_search_results.append(mock_songbook_entry_temp) + mock_search_results = [('1', 'My Book', 'My Song', 1)] mock_qlist_widget = MagicMock() MockedQListWidgetItem.return_value = mock_qlist_widget @@ -183,9 +162,35 @@ class TestMediaItem(TestCase, TestMixin): # THEN: The current list view is cleared, the widget is created, and the relevant attributes set self.media_item.list_view.clear.assert_called_with() MockedQListWidgetItem.assert_called_once_with('My Book #1: My Song') - mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, mock_songbook_entry.song.id) + mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, 1) self.media_item.list_view.addItem.assert_called_once_with(mock_qlist_widget) + def songbook_natural_sorting_test(self): + """ + Test that songbooks are sorted naturally + """ + # GIVEN: Search results grouped by book and entry, plus a mocked QtListWidgetItem + with patch('openlp.core.lib.QtWidgets.QListWidgetItem') as MockedQListWidgetItem: + mock_search_results = [('2', 'Thy Book', 'Thy Song', 50), + ('2', 'My Book', 'Your Song', 7), + ('10', 'My Book', 'Our Song', 12), + ('1', 'My Book', 'My Song', 1), + ('2', 'Thy Book', 'A Song', 8)] + mock_qlist_widget = MagicMock() + MockedQListWidgetItem.return_value = mock_qlist_widget + + # WHEN: I display song search results grouped by book + self.media_item.display_results_book(mock_search_results) + + # THEN: The songbooks are inserted in the right (natural) order, + # grouped first by book, then by number, then by song title + calls = [call('My Book #1: My Song'), call().setData(QtCore.Qt.UserRole, 1), + call('My Book #2: Your Song'), call().setData(QtCore.Qt.UserRole, 7), + call('My Book #10: Our Song'), call().setData(QtCore.Qt.UserRole, 12), + call('Thy Book #2: A Song'), call().setData(QtCore.Qt.UserRole, 8), + call('Thy Book #2: Thy Song'), call().setData(QtCore.Qt.UserRole, 50)] + MockedQListWidgetItem.assert_has_calls(calls) + def display_results_topic_test(self): """ Test displaying song search results grouped by topic with basic song From 9cd49f194ff16ef4a0a1b38c53efbdb7499c2425 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Fri, 29 Apr 2016 19:32:09 +0200 Subject: [PATCH 05/13] PEP8 --- tests/functional/openlp_plugins/songs/test_mediaitem.py | 4 ++-- tests/functional/openlp_plugins/songs/test_openlpimporter.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 85911e640..12447368b 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -187,8 +187,8 @@ class TestMediaItem(TestCase, TestMixin): calls = [call('My Book #1: My Song'), call().setData(QtCore.Qt.UserRole, 1), call('My Book #2: Your Song'), call().setData(QtCore.Qt.UserRole, 7), call('My Book #10: Our Song'), call().setData(QtCore.Qt.UserRole, 12), - call('Thy Book #2: A Song'), call().setData(QtCore.Qt.UserRole, 8), - call('Thy Book #2: Thy Song'), call().setData(QtCore.Qt.UserRole, 50)] + call('Thy Book #2: A Song'), call().setData(QtCore.Qt.UserRole, 8), + call('Thy Book #2: Thy Song'), call().setData(QtCore.Qt.UserRole, 50)] MockedQListWidgetItem.assert_has_calls(calls) def display_results_topic_test(self): diff --git a/tests/functional/openlp_plugins/songs/test_openlpimporter.py b/tests/functional/openlp_plugins/songs/test_openlpimporter.py index 113db16e0..b78d5c43b 100644 --- a/tests/functional/openlp_plugins/songs/test_openlpimporter.py +++ b/tests/functional/openlp_plugins/songs/test_openlpimporter.py @@ -73,4 +73,3 @@ class TestOpenLPImport(TestCase): self.assertIsNone(importer.do_import(), 'do_import should return None when import_source is not a list') self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False, 'setMaximum on import_wizard.progress_bar should not have been called') - From 5601e61c0fbeae169773c0fcdfdc3fd50f833f41 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Fri, 29 Apr 2016 13:25:12 -0700 Subject: [PATCH 06/13] Convert strings to python3 in mainwindow --- openlp/core/ui/maindisplay.py | 14 +++-- openlp/core/ui/mainwindow.py | 56 ++++++++++--------- .../openlp_core_lib/test_projectordb.py | 24 ++++++-- 3 files changed, 56 insertions(+), 38 deletions(-) diff --git a/openlp/core/ui/maindisplay.py b/openlp/core/ui/maindisplay.py index 079235c2d..b803df205 100644 --- a/openlp/core/ui/maindisplay.py +++ b/openlp/core/ui/maindisplay.py @@ -247,7 +247,7 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties): """ Set up and build the output screen """ - self.log_debug('Start MainDisplay setup (live = %s)' % self.is_live) + self.log_debug('Start MainDisplay setup (live = {islive})'.format(islive=self.is_live)) self.screen = self.screens.current self.setVisible(False) Display.setup(self) @@ -288,7 +288,9 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties): self.application.process_events() self.setGeometry(self.screen['size']) if animate: - self.frame.evaluateJavaScript('show_text("%s")' % slide.replace('\\', '\\\\').replace('\"', '\\\"')) + # NOTE: Verify this works with ''.format() + _text = slide.replace('\\', '\\\\').replace('\"', '\\\"') + self.frame.evaluateJavaScript('show_text("{text}")'.format(text=_text)) else: # This exists for https://bugs.launchpad.net/openlp/+bug/1016843 # For unknown reasons if evaluateJavaScript is called @@ -309,10 +311,10 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties): text_prepared = expand_tags(html.escape(text)).replace('\\', '\\\\').replace('\"', '\\\"') if self.height() != self.screen['size'].height() or not self.isVisible(): shrink = True - js = 'show_alert("%s", "%s")' % (text_prepared, 'top') + js = 'show_alert("{text}", "{top}")'.format(text=text_prepared, top='top') else: shrink = False - js = 'show_alert("%s", "")' % text_prepared + js = 'show_alert("{text}", "")'.format(text=text_prepared) height = self.frame.evaluateJavaScript(js) if shrink: if text: @@ -368,7 +370,7 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties): """ self.setGeometry(self.screen['size']) if image: - js = 'show_image("data:image/png;base64,%s");' % image + js = 'show_image("data:image/png;base64,{image}");'.format(image=image) else: js = 'show_image("");' self.frame.evaluateJavaScript(js) @@ -492,7 +494,7 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties): :param mode: How the screen is to be hidden """ - self.log_debug('hide_display mode = %d' % mode) + self.log_debug('hide_display mode = {mode:d}'.format(mode=mode)) if self.screens.display_count == 1: # Only make visible if setting enabled. if not Settings().value('core/display on monitor'): diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 39e0ac518..a0235bb9b 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -622,11 +622,10 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): :param version: The Version to be displayed. """ log.debug('version_notice') - version_text = translate('OpenLP.MainWindow', 'Version %s of OpenLP is now available for download (you are ' - 'currently running version %s). \n\nYou can download the latest version from ' - 'http://openlp.org/.') - QtWidgets.QMessageBox.question(self, translate('OpenLP.MainWindow', 'OpenLP Version Updated'), - version_text % (version, get_application_version()[u'full'])) + version_text = translate('OpenLP.MainWindow', 'Version {new} of OpenLP is now available for download (you are ' + 'currently running version {current}). \n\nYou can download the latest version from ' + 'http://openlp.org/.').format(new=version, current=get_application_version()[u'full']) + QtWidgets.QMessageBox.question(self, translate('OpenLP.MainWindow', 'OpenLP Version Updated'), version_text) def show(self): """ @@ -642,7 +641,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): self.service_manager_contents.load_last_file() # This will store currently used layout preset so it remains enabled on next startup. # If any panel is enabled/disabled after preset is set, this setting is not saved. - view_mode = Settings().value('%s/view mode' % self.general_settings_section) + view_mode = Settings().value('{section}/view mode'.format(section=self.general_settings_section)) if view_mode == 'default' and Settings().value('user interface/is preset layout'): self.mode_default_item.setChecked(True) elif view_mode == 'setup' and Settings().value('user interface/is preset layout'): @@ -731,8 +730,8 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): """ settings = Settings() self.live_controller.main_display_set_background() - if settings.value('%s/screen blank' % self.general_settings_section): - if settings.value('%s/blank warning' % self.general_settings_section): + if settings.value('{section}/screen blank'.format(section=self.general_settings_section)): + if settings.value('{section}/blank warning'.format(section=self.general_settings_section)): QtWidgets.QMessageBox.question(self, translate('OpenLP.MainWindow', 'OpenLP Main Display Blanked'), translate('OpenLP.MainWindow', 'The Main Display has been blanked out')) @@ -924,9 +923,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): try: value = import_settings.value(section_key) except KeyError: - log.warning('The key "%s" does not exist (anymore), so it will be skipped.' % section_key) + log.warning('The key "{key}" does not exist (anymore), so it will be skipped.'.format(key=section_key)) if value is not None: - settings.setValue('%s' % (section_key), value) + settings.setValue('{key}'.format(key=section_key), value) now = datetime.now() settings.beginGroup(self.header_section) settings.setValue('file_imported', import_file_name) @@ -1003,9 +1002,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): key_value = settings.value(section_key) except KeyError: QtWidgets.QMessageBox.critical(self, translate('OpenLP.MainWindow', 'Export setting error'), - translate('OpenLP.MainWindow', 'The key "%s" does not have a default ' + translate('OpenLP.MainWindow', 'The key "{key}" does not have a default ' 'value so it will be skipped in this ' - 'export.') % section_key, + 'export.').format(key=section_key), QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok)) key_value = None if key_value is not None: @@ -1027,8 +1026,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): os.remove(temp_file) except OSError as ose: QtWidgets.QMessageBox.critical(self, translate('OpenLP.MainWindow', 'Export setting error'), - translate('OpenLP.MainWindow', 'An error occurred while exporting the ' - 'settings: %s') % ose.strerror, + translate('OpenLP.MainWindow', + 'An error occurred while exporting the ' + 'settings: {err}').format(err=ose.strerror), QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok)) def on_mode_default_item_clicked(self): @@ -1061,7 +1061,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): """ if mode: settings = Settings() - settings.setValue('%s/view mode' % self.general_settings_section, mode) + settings.setValue('{section}/view mode'.format(section=self.general_settings_section), mode) self.media_manager_dock.setVisible(media) self.service_manager_dock.setVisible(service) self.theme_manager_dock.setVisible(theme) @@ -1168,9 +1168,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): :param file_name: The file name of the service file. """ if modified: - title = '%s - %s*' % (UiStrings().OLPV2x, file_name) + title = '{title} - {name}*'.format(title=UiStrings().OLPV2x, name=file_name) else: - title = '%s - %s' % (UiStrings().OLPV2x, file_name) + title = '{title} - {name}'.format(title=UiStrings().OLPV2x, name=file_name) self.setWindowTitle(title) def show_status_message(self, message): @@ -1183,8 +1183,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): """ Update the default theme indicator in the status bar """ - self.default_theme_label.setText(translate('OpenLP.MainWindow', 'Default Theme: %s') % - Settings().value('themes/global theme')) + theme_name = Settings().value('themes/global theme') + self.default_theme_label.setText(translate('OpenLP.MainWindow', + 'Default Theme: {theme}').format(theme=theme_name)) def toggle_media_manager(self): """ @@ -1331,7 +1332,8 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): recent_files_to_display = existing_recent_files[0:recent_file_count] self.recent_files_menu.clear() for file_id, filename in enumerate(recent_files_to_display): - log.debug('Recent file name: %s', filename) + log.debug('Recent file name: {name}'.format(name=filename)) + # TODO: Verify ''.format() before committing action = create_action(self, '', text='&%d %s' % (file_id + 1, os.path.splitext(os.path.basename(str(filename)))[0]), data=filename, triggers=self.service_manager_contents.on_recent_service_clicked) @@ -1424,7 +1426,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): """ Change the data directory. """ - log.info('Changing data path to %s' % self.new_data_path) + log.info('Changing data path to {newpath}'.format(newpath=self.new_data_path)) old_data_path = str(AppLocation.get_data_path()) # Copy OpenLP data to new location if requested. self.application.set_busy_cursor() @@ -1432,17 +1434,17 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): log.info('Copying data to new path') try: self.show_status_message( - translate('OpenLP.MainWindow', 'Copying OpenLP data to new data directory location - %s ' - '- Please wait for copy to finish').replace('%s', self.new_data_path)) + translate('OpenLP.MainWindow', 'Copying OpenLP data to new data directory location - {path} ' + '- Please wait for copy to finish').format(path=self.new_data_path)) dir_util.copy_tree(old_data_path, self.new_data_path) log.info('Copy successful') except (IOError, os.error, DistutilsFileError) as why: self.application.set_normal_cursor() - log.exception('Data copy failed %s' % str(why)) + log.exception('Data copy failed {err}'.format(err=str(why))) + err_text = translate('OpenLP.MainWindow', + 'OpenLP Data directory copy failed\n\n{err}').format(err=str(why)), QtWidgets.QMessageBox.critical(self, translate('OpenLP.MainWindow', 'New Data Directory Error'), - translate('OpenLP.MainWindow', - 'OpenLP Data directory copy failed\n\n%s'). - replace('%s', str(why)), + err_text, QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok)) return False else: diff --git a/tests/functional/openlp_core_lib/test_projectordb.py b/tests/functional/openlp_core_lib/test_projectordb.py index 7a8a57d19..f6d52e476 100644 --- a/tests/functional/openlp_core_lib/test_projectordb.py +++ b/tests/functional/openlp_core_lib/test_projectordb.py @@ -28,7 +28,7 @@ PREREQUISITE: add_record() and get_all() functions validated. import os from unittest import TestCase -from openlp.core.lib.projector.db import Projector, ProjectorDB, ProjectorSource +from openlp.core.lib.projector.db import Manufacturer, Model, Projector, ProjectorDB, ProjectorSource from tests.functional import MagicMock, patch from tests.resources.projector.data import TEST_DB, TEST1_DATA, TEST2_DATA, TEST3_DATA @@ -82,13 +82,13 @@ class TestProjectorDB(TestCase): """ Test case for ProjectorDB """ - def setUp(self): + @patch('openlp.core.lib.projector.db.init_url') + def setUp(self, mocked_init_url): """ Set up anything necessary for all tests """ - with patch('openlp.core.lib.projector.db.init_url') as mocked_init_url: - mocked_init_url.return_value = 'sqlite:///%s' % TEST_DB - self.projector = ProjectorDB() + mocked_init_url.return_value = 'sqlite:///{db}'.format(db=TEST_DB) + self.projector = ProjectorDB() def tearDown(self): """ @@ -192,3 +192,17 @@ class TestProjectorDB(TestCase): # THEN: Projector should have the same source entry item = self.projector.get_projector_by_id(item_id) self.assertTrue(compare_source(item.source_list[0], source)) + + def manufacturer_repr_test(self): + """ + Test manufacturer class __repr__ text + """ + # GIVEN: Test object + manufacturer = Manufacturer() + + # WHEN: Name is set + manufacturer.name = 'OpenLP Test' + + # THEN: __repr__ should return a proper string + self.assertEqual(str(manufacturer), '', + 'Manufacturer.__repr__() should have returned a proper representation string') From 956c9d1653d845811aac31ad5613a0a6b51abe46 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Fri, 29 Apr 2016 13:35:54 -0700 Subject: [PATCH 07/13] pep8 in test_openlpimporter.py --- tests/functional/openlp_plugins/songs/test_openlpimporter.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_openlpimporter.py b/tests/functional/openlp_plugins/songs/test_openlpimporter.py index 113db16e0..b78d5c43b 100644 --- a/tests/functional/openlp_plugins/songs/test_openlpimporter.py +++ b/tests/functional/openlp_plugins/songs/test_openlpimporter.py @@ -73,4 +73,3 @@ class TestOpenLPImport(TestCase): self.assertIsNone(importer.do_import(), 'do_import should return None when import_source is not a list') self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False, 'setMaximum on import_wizard.progress_bar should not have been called') - From b7a32ebbb2875064473827fe7aa1d0600cd8819f Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sat, 30 Apr 2016 11:05:10 +0200 Subject: [PATCH 08/13] Added support for using the new mutool in mudraw mode --- openlp/plugins/media/mediaplugin.py | 1 - .../presentations/lib/pdfcontroller.py | 42 +++++++++++++++---- .../presentations/lib/presentationtab.py | 2 +- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/openlp/plugins/media/mediaplugin.py b/openlp/plugins/media/mediaplugin.py index 1d5529084..e258b5809 100644 --- a/openlp/plugins/media/mediaplugin.py +++ b/openlp/plugins/media/mediaplugin.py @@ -160,7 +160,6 @@ def process_check_binary(program_path): """ program_type = None runlog = check_binary_exists(program_path) - print(runlog, type(runlog)) # Analyse the output to see it the program is mediainfo for line in runlog.splitlines(): decoded_line = line.decode() diff --git a/openlp/plugins/presentations/lib/pdfcontroller.py b/openlp/plugins/presentations/lib/pdfcontroller.py index 48150a9f2..9bfc25eb1 100644 --- a/openlp/plugins/presentations/lib/pdfcontroller.py +++ b/openlp/plugins/presentations/lib/pdfcontroller.py @@ -77,6 +77,12 @@ class PdfController(PresentationController): if found_mudraw: program_type = 'mudraw' break + found_mutool = re.search('usage: mutool.*', decoded_line, re.IGNORECASE) + if found_mutool: + # Test that mutool contains mudraw + if re.search('draw\s+--\s+convert document.*', runlog.decode(), re.IGNORECASE | re.MULTILINE): + program_type = 'mutool' + break found_gs = re.search('GPL Ghostscript.*', decoded_line, re.IGNORECASE) if found_gs: program_type = 'gs' @@ -101,6 +107,7 @@ class PdfController(PresentationController): """ log.debug('check_installed Pdf') self.mudrawbin = '' + self.mutoolbin = '' self.gsbin = '' self.also_supports = [] # Use the user defined program if given @@ -111,27 +118,36 @@ class PdfController(PresentationController): self.gsbin = pdf_program elif program_type == 'mudraw': self.mudrawbin = pdf_program + elif program_type == 'mutool': + self.mutoolbin = pdf_program else: # Fallback to autodetection application_path = AppLocation.get_directory(AppLocation.AppDir) if is_win(): - # for windows we only accept mudraw.exe in the base folder + # for windows we only accept mudraw.exe or mutool.exe in the base folder application_path = AppLocation.get_directory(AppLocation.AppDir) if os.path.isfile(os.path.join(application_path, 'mudraw.exe')): self.mudrawbin = os.path.join(application_path, 'mudraw.exe') + elif os.path.isfile(os.path.join(application_path, 'mutool.exe')): + self.mutoolbin = os.path.join(application_path, 'mutool.exe') else: DEVNULL = open(os.devnull, 'wb') - # First try to find mupdf + # First try to find mudraw self.mudrawbin = which('mudraw') - # if mupdf isn't installed, fallback to ghostscript + # if mudraw isn't installed, try mutool if not self.mudrawbin: - self.gsbin = which('gs') - # Last option: check if mudraw is placed in OpenLP base folder - if not self.mudrawbin and not self.gsbin: + self.mutoolbin = which('mutool') + # Check we got a working mutool + if not self.mutoolbin or self.process_check_binary(self.mutoolbin) != 'mutool': + self.gsbin = which('gs') + # Last option: check if mudraw or mutool is placed in OpenLP base folder + if not self.mudrawbin and not self.mutoolbin and not self.gsbin: application_path = AppLocation.get_directory(AppLocation.AppDir) - if os.path.isfile(os.path.join(application_path, 'mudraw')): - self.mudrawbin = os.path.join(application_path, 'mudraw') - if self.mudrawbin: + if os.path.isfile(os.path.join(application_path, 'mudraw.exe')): + self.mudrawbin = os.path.join(application_path, 'mudraw.exe') + elif os.path.isfile(os.path.join(application_path, 'mutool.exe')): + self.mutoolbin = os.path.join(application_path, 'mutool.exe') + if self.mudrawbin or self.mutoolbin: self.also_supports = ['xps', 'oxps'] return True elif self.gsbin: @@ -238,10 +254,18 @@ class PdfDocument(PresentationDocument): if not os.path.isdir(self.get_temp_folder()): os.makedirs(self.get_temp_folder()) if self.controller.mudrawbin: + log.debug('loading presentation using mudraw') runlog = check_output([self.controller.mudrawbin, '-w', str(size.width()), '-h', str(size.height()), '-o', os.path.join(self.get_temp_folder(), 'mainslide%03d.png'), self.file_path], startupinfo=self.startupinfo) + elif self.controller.mutoolbin: + log.debug('loading presentation using mutool') + runlog = check_output([self.controller.mutoolbin, 'draw', '-w', str(size.width()), '-h', + str(size.height()), + '-o', os.path.join(self.get_temp_folder(), 'mainslide%03d.png'), self.file_path], + startupinfo=self.startupinfo) elif self.controller.gsbin: + log.debug('loading presentation using gs') resolution = self.gs_get_resolution(size) runlog = check_output([self.controller.gsbin, '-dSAFER', '-dNOPAUSE', '-dBATCH', '-sDEVICE=png16m', '-r' + str(resolution), '-dTextAlphaBits=4', '-dGraphicsAlphaBits=4', diff --git a/openlp/plugins/presentations/lib/presentationtab.py b/openlp/plugins/presentations/lib/presentationtab.py index cbe881853..8076b33fe 100644 --- a/openlp/plugins/presentations/lib/presentationtab.py +++ b/openlp/plugins/presentations/lib/presentationtab.py @@ -235,7 +235,7 @@ class PresentationTab(SettingsTab): self, translate('PresentationPlugin.PresentationTab', 'Select mudraw or ghostscript binary.'), self.pdf_program_path.text()) if filename: - program_type = PdfController.check_binary(filename) + program_type = PdfController.process_check_binary(filename) if not program_type: critical_error_message_box(UiStrings().Error, translate('PresentationPlugin.PresentationTab', From 17fa45b1dd584c06541a79169d7b5cbe05017939 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 4 May 2016 22:58:44 +0200 Subject: [PATCH 09/13] Added tests. --- .../presentations/test_pdfcontroller.py | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/presentations/test_pdfcontroller.py b/tests/functional/openlp_plugins/presentations/test_pdfcontroller.py index 79b907230..f73655dc7 100644 --- a/tests/functional/openlp_plugins/presentations/test_pdfcontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_pdfcontroller.py @@ -29,7 +29,7 @@ from tempfile import mkdtemp from PyQt5 import QtCore, QtGui from openlp.plugins.presentations.lib.pdfcontroller import PdfController, PdfDocument -from tests.functional import MagicMock +from tests.functional import MagicMock, patch from openlp.core.common import Settings from openlp.core.lib import ScreenList from tests.utils.constants import TEST_RESOURCES_PATH @@ -137,3 +137,74 @@ class TestPdfController(TestCase, TestMixin): else: self.assertEqual(768, image.height(), 'The height should be 768') self.assertEqual(543, image.width(), 'The width should be 543') + + @patch('openlp.plugins.presentations.lib.pdfcontroller.check_binary_exists') + def process_check_binary_mudraw_test(self, mocked_check_binary_exists): + """ + Test that the correct output from mudraw is detected + """ + # GIVEN: A mocked check_binary_exists that returns mudraw output + mudraw_output = (b'usage: mudraw [options] input [pages]\n\t-o -\toutput filename (%d for page number)n\t\tsupp' + b'orted formats: pgm, ppm, pam, png, pbmn\t-p -\tpasswordn\t-r -\tresolution in dpi (default: ' + b'72)n\t-w -\twidth (in pixels) (maximum width if -r is specified)n\t-h -\theight (in pixels) ' + b'(maximum height if -r is specified)') + mocked_check_binary_exists.return_value = mudraw_output + + # WHEN: Calling process_check_binary + ret = PdfController.process_check_binary('test') + + # THEN: mudraw should be detected + self.assertEqual('mudraw', ret, 'mudraw should have been detected') + + @patch('openlp.plugins.presentations.lib.pdfcontroller.check_binary_exists') + def process_check_binary_new_motool_test(self, mocked_check_binary_exists): + """ + Test that the correct output from the new mutool is detected + """ + # GIVEN: A mocked check_binary_exists that returns new mutool output + new_mutool_output = (b'usage: mutool [options]\n\tdraw\t-- convert document\n\trun\t-- run javascript' + b'\n\tclean\t-- rewrite pdf file\n\textract\t-- extract font and image resources\n\tinfo\t' + b'-- show information about pdf resources\n\tpages\t-- show information about pdf pages\n' + b'\tposter\t-- split large page into many tiles\n\tshow\t-- show internal pdf objects\n\t' + b'create\t-- create pdf document\n\tmerge\t-- merge pages from multiple pdf sources into a' + b'new pdf\n') + mocked_check_binary_exists.return_value = new_mutool_output + + # WHEN: Calling process_check_binary + ret = PdfController.process_check_binary('test') + + # THEN: mutool should be detected + self.assertEqual('mutool', ret, 'mutool should have been detected') + + @patch('openlp.plugins.presentations.lib.pdfcontroller.check_binary_exists') + def process_check_binary_old_motool_test(self, mocked_check_binary_exists): + """ + Test that the output from the old mutool is not accepted + """ + # GIVEN: A mocked check_binary_exists that returns old mutool output + old_mutool_output = (b'usage: mutool [options]\n\tclean\t-- rewrite pdf file\n\textract\t-- extract ' + b'font and image resources\n\tinfo\t-- show information about pdf resources\n\tposter\t-- ' + b'split large page into many tiles\n\tshow\t-- show internal pdf objects') + mocked_check_binary_exists.return_value = old_mutool_output + + # WHEN: Calling process_check_binary + ret = PdfController.process_check_binary('test') + + # THEN: mutool should be detected + self.assertIsNone(ret, 'old mutool should not be accepted!') + + @patch('openlp.plugins.presentations.lib.pdfcontroller.check_binary_exists') + def process_check_binary_gs_test(self, mocked_check_binary_exists): + """ + Test that the correct output from gs is detected + """ + # GIVEN: A mocked check_binary_exists that returns gs output + gs_output = (b'GPL Ghostscript 9.19 (2016-03-23)\nCopyright (C) 2016 Artifex Software, Inc. All rights reserv' + b'ed.\nUsage: gs [switches] [file1.ps file2.ps ...]') + mocked_check_binary_exists.return_value = gs_output + + # WHEN: Calling process_check_binary + ret = PdfController.process_check_binary('test') + + # THEN: mutool should be detected + self.assertEqual('gs', ret, 'mutool should have been detected') From 907282d3cd5207df0e7ea0f7c6b66417d13f7194 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 5 May 2016 20:14:33 +0200 Subject: [PATCH 10/13] make mudraw/mutool detection work on mac --- openlp/plugins/presentations/lib/pdfcontroller.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openlp/plugins/presentations/lib/pdfcontroller.py b/openlp/plugins/presentations/lib/pdfcontroller.py index 9bfc25eb1..cf89d8d32 100644 --- a/openlp/plugins/presentations/lib/pdfcontroller.py +++ b/openlp/plugins/presentations/lib/pdfcontroller.py @@ -143,10 +143,10 @@ class PdfController(PresentationController): # Last option: check if mudraw or mutool is placed in OpenLP base folder if not self.mudrawbin and not self.mutoolbin and not self.gsbin: application_path = AppLocation.get_directory(AppLocation.AppDir) - if os.path.isfile(os.path.join(application_path, 'mudraw.exe')): - self.mudrawbin = os.path.join(application_path, 'mudraw.exe') - elif os.path.isfile(os.path.join(application_path, 'mutool.exe')): - self.mutoolbin = os.path.join(application_path, 'mutool.exe') + if os.path.isfile(os.path.join(application_path, 'mudraw')): + self.mudrawbin = os.path.join(application_path, 'mudraw') + elif os.path.isfile(os.path.join(application_path, 'mutool')): + self.mutoolbin = os.path.join(application_path, 'mutool') if self.mudrawbin or self.mutoolbin: self.also_supports = ['xps', 'oxps'] return True From 26e72a1fecf1ea0e7ffc35d11bd279eef44109b9 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 5 May 2016 20:17:45 +0200 Subject: [PATCH 11/13] make mediainfo detection work on win and mac. --- openlp/plugins/media/mediaplugin.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/media/mediaplugin.py b/openlp/plugins/media/mediaplugin.py index e258b5809..f9874c57b 100644 --- a/openlp/plugins/media/mediaplugin.py +++ b/openlp/plugins/media/mediaplugin.py @@ -71,8 +71,12 @@ class MediaPlugin(Plugin): :return: true or false """ log.debug('check_installed Mediainfo') - # Use the user defined program if given - return process_check_binary('mediainfo') + # Try to find mediainfo in the path + exists = process_check_binary('mediainfo') + # If mediainfo is not in the path, try to find it in the application folder + if not exists: + exists = process_check_binary(os.path.join(AppLocation.get_directory(AppLocation.AppDir), 'mediainfo')) + return exists def app_startup(self): """ From 7ee0af01b263126fc941d31588a363b58bce48b8 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 5 May 2016 22:30:00 +0200 Subject: [PATCH 12/13] fix traceback if bible list download fails. --- openlp/plugins/bibles/forms/bibleimportform.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/bibles/forms/bibleimportform.py b/openlp/plugins/bibles/forms/bibleimportform.py index 66ba252fc..3289e66e9 100644 --- a/openlp/plugins/bibles/forms/bibleimportform.py +++ b/openlp/plugins/bibles/forms/bibleimportform.py @@ -577,7 +577,7 @@ class BibleImportForm(OpenLPWizard): :param index: The index of the combo box. """ self.web_translation_combo_box.clear() - if self.web_bible_list: + if self.web_bible_list and index in self.web_bible_list: bibles = list(self.web_bible_list[index].keys()) bibles.sort(key=get_locale_key) self.web_translation_combo_box.addItems(bibles) From fa9a29d30301040bb555e07c79fdfe6dc04aeba7 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Fri, 6 May 2016 07:33:43 +0200 Subject: [PATCH 13/13] Fix crosswalk bible list download --- openlp/plugins/bibles/lib/http.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/bibles/lib/http.py b/openlp/plugins/bibles/lib/http.py index 4398688ee..2076bec87 100644 --- a/openlp/plugins/bibles/lib/http.py +++ b/openlp/plugins/bibles/lib/http.py @@ -520,7 +520,7 @@ class CWExtract(RegistryProperties): returns a list in the form [(biblename, biblekey, language_code)] """ log.debug('CWExtract.get_bibles_from_http') - bible_url = 'http://www.biblestudytools.com/search/bible-search.part/' + bible_url = 'http://www.biblestudytools.com/' soup = get_soup_for_bible_ref(bible_url) if not soup: return None @@ -528,7 +528,7 @@ class CWExtract(RegistryProperties): if not bible_select: log.debug('No select tags found - did site change?') return None - option_tags = bible_select.find_all('option') + option_tags = bible_select.find_all('option', {'class': 'log-translation'}) if not option_tags: log.debug('No option tags found - did site change?') return None