Phill has proposed merging lp:~phill-ridout/openlp/fixes-V into lp:openlp. Requested reviews: OpenLP Core (openlp-core) Related bugs: Bug #1736274 in OpenLP: "SWORD importer - Information text not wrapped" https://bugs.launchpad.net/openlp/+bug/1736274 Bug #1738047 in OpenLP: "OpenLP picks up macOS hidden files when loading plugins" https://bugs.launchpad.net/openlp/+bug/1738047
For more details, see: https://code.launchpad.net/~phill-ridout/openlp/fixes-V/+merge/335288 Number of fixes, including: * Fix to creation and saving of services * SongBeamer encoding detection * OSX plugin, media and presentation controller discovery and import fixes * Make the ftw thread work in its own thread, rather than the main thread lp:~phill-ridout/openlp/fixes-V (revision 2801) https://ci.openlp.io/job/Branch-01-Pull/2351/ [WAITING] [RUNNING] [SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/2252/ [WAITING] [RUNNING] [SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/2117/ [WAITING] [RUNNING] [SUCCESS] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1443/ [WAITING] [RUNNING] [SUCCESS] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1260/ [WAITING] [RUNNING] [SUCCESS] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/390/ [WAITING] [RUNNING] [SUCCESS] https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/219/ [WAITING] [FAILURE] Stopping after failure Failed builds: - Branch-05-AppVeyor-Tests #219: https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/219/console -- Your team OpenLP Core is requested to review the proposed merge of lp:~phill-ridout/openlp/fixes-V into lp:openlp.
=== modified file 'openlp/core/api/http/server.py' --- openlp/core/api/http/server.py 2017-12-02 21:47:11 +0000 +++ openlp/core/api/http/server.py 2017-12-17 04:34:16 +0000 @@ -64,6 +64,7 @@ """ Run the thread. """ + log.debug('Starting HttpWorker') address = Settings().value('api/ip address') port = Settings().value('api/port') Registry().execute('get_website_version') @@ -71,6 +72,7 @@ serve(application, host=address, port=port) except OSError: log.exception('An error occurred when serving the application.') + log.debug('HttpWorker Finished') def stop(self): pass === modified file 'openlp/core/api/websockets.py' --- openlp/core/api/websockets.py 2017-12-02 09:11:22 +0000 +++ openlp/core/api/websockets.py 2017-12-17 04:34:16 +0000 @@ -55,7 +55,9 @@ """ Run the thread. """ + log.debug('Starting WebSocketWorker') self.ws_server.start_server() + log.debug('WebSocketWorker Finished') def stop(self): self.ws_server.stop = True @@ -116,7 +118,7 @@ async def handle_websocket(request, path): """ Handle web socket requests and return the poll information. - Check ever 0.2 seconds to get the latest position and send if changed. + Check every 0.2 seconds to get the latest position and send if changed. Only gets triggered when 1st client attaches :param request: request from client === modified file 'openlp/core/common/__init__.py' --- openlp/core/common/__init__.py 2017-11-18 23:14:28 +0000 +++ openlp/core/common/__init__.py 2017-12-17 04:34:16 +0000 @@ -80,6 +80,7 @@ extension_path = extension_path.relative_to(app_dir) if extension_path.name in excluded_files: continue + log.debug('Attempting to import %s', extension_path) module_name = path_to_module(extension_path) try: importlib.import_module(module_name) === modified file 'openlp/core/lib/pluginmanager.py' --- openlp/core/lib/pluginmanager.py 2017-11-18 22:37:24 +0000 +++ openlp/core/lib/pluginmanager.py 2017-12-17 04:34:16 +0000 @@ -71,7 +71,7 @@ """ Scan a directory for objects inheriting from the ``Plugin`` class. """ - glob_pattern = os.path.join('plugins', '*', '*plugin.py') + glob_pattern = os.path.join('plugins', '*', '[!.]*plugin.py') extension_loader(glob_pattern) plugin_classes = Plugin.__subclasses__() plugin_objects = [] === modified file 'openlp/core/threading.py' --- openlp/core/threading.py 2017-09-09 05:57:06 +0000 +++ openlp/core/threading.py 2017-12-17 04:34:16 +0000 @@ -25,31 +25,26 @@ from PyQt5 import QtCore -def run_thread(parent, worker, prefix='', auto_start=True): +def run_thread(worker, auto_start=True, clean_up=True): """ Create a thread and assign a worker to it. This removes a lot of boilerplate code from the codebase. - :param object parent: The parent object so that the thread and worker are not orphaned. :param QObject worker: A QObject-based worker object which does the actual work. - :param str prefix: A prefix to be applied to the attribute names. :param bool auto_start: Automatically start the thread. Defaults to True. + :param bool clean_up: Schedule the the thread to be deleted when finished. Defaults to True + :return: The thread object + :rtype: QtCore.QThread """ - # Set up attribute names - thread_name = 'thread' - worker_name = 'worker' - if prefix: - thread_name = '_'.join([prefix, thread_name]) - worker_name = '_'.join([prefix, worker_name]) # Create the thread and add the thread and the worker to the parent thread = QtCore.QThread() - setattr(parent, thread_name, thread) - setattr(parent, worker_name, worker) # Move the worker into the thread's context worker.moveToThread(thread) # Connect slots and signals thread.started.connect(worker.start) worker.quit.connect(thread.quit) worker.quit.connect(worker.deleteLater) - thread.finished.connect(thread.deleteLater) + if clean_up: + thread.finished.connect(thread.deleteLater) if auto_start: thread.start() + return thread === modified file 'openlp/core/ui/firsttimeform.py' --- openlp/core/ui/firsttimeform.py 2017-10-23 22:09:57 +0000 +++ openlp/core/ui/firsttimeform.py 2017-12-17 04:34:16 +0000 @@ -41,9 +41,10 @@ from openlp.core.common.mixins import RegistryProperties from openlp.core.common.registry import Registry from openlp.core.common.settings import Settings +from openlp.core.common.httputils import get_web_page, get_url_file_size, url_get_file, CONNECTION_TIMEOUT from openlp.core.lib import PluginStatus, build_icon from openlp.core.lib.ui import critical_error_message_box -from openlp.core.common.httputils import get_web_page, get_url_file_size, url_get_file, CONNECTION_TIMEOUT +from openlp.core.threading import run_thread from .firsttimewizard import UiFirstTimeWizard, FirstTimePage log = logging.getLogger(__name__) @@ -54,7 +55,7 @@ This thread downloads a theme's screenshot """ screenshot_downloaded = QtCore.pyqtSignal(str, str, str) - finished = QtCore.pyqtSignal() + quit = QtCore.pyqtSignal() def __init__(self, themes_url, title, filename, sha256, screenshot): """ @@ -69,7 +70,7 @@ socket.setdefaulttimeout(CONNECTION_TIMEOUT) super(ThemeScreenshotWorker, self).__init__() - def run(self): + def start(self): """ Overridden method to run the thread. """ @@ -83,7 +84,7 @@ except: log.exception('Unable to download screenshot') finally: - self.finished.emit() + self.quit.emit() @QtCore.pyqtSlot(bool) def set_download_canceled(self, toggle): @@ -256,14 +257,10 @@ sha256 = self.config.get('theme_{theme}'.format(theme=theme), 'sha256', fallback='') screenshot = self.config.get('theme_{theme}'.format(theme=theme), 'screenshot') worker = ThemeScreenshotWorker(self.themes_url, title, filename, sha256, screenshot) + worker.screenshot_downloaded.connect(self.on_screenshot_downloaded) + thread = run_thread(worker, clean_up=False) self.theme_screenshot_workers.append(worker) - worker.screenshot_downloaded.connect(self.on_screenshot_downloaded) - thread = QtCore.QThread(self) self.theme_screenshot_threads.append(thread) - thread.started.connect(worker.run) - worker.finished.connect(thread.quit) - worker.moveToThread(thread) - thread.start() self.application.process_events() def set_defaults(self): === modified file 'openlp/core/ui/media/mediacontroller.py' --- openlp/core/ui/media/mediacontroller.py 2017-11-22 21:56:56 +0000 +++ openlp/core/ui/media/mediacontroller.py 2017-12-17 04:34:16 +0000 @@ -181,7 +181,8 @@ """ log.debug('_check_available_media_players') controller_dir = os.path.join('core', 'ui', 'media') - glob_pattern = os.path.join(controller_dir, '*player.py') + # Find all files that do not begin with '.' (lp:#1738047) and end with player.py + glob_pattern = os.path.join(controller_dir, '[!.]*player.py') extension_loader(glob_pattern, ['mediaplayer.py']) player_classes = MediaPlayer.__subclasses__() for player_class in player_classes: === modified file 'openlp/core/ui/servicemanager.py' --- openlp/core/ui/servicemanager.py 2017-12-02 21:47:11 +0000 +++ openlp/core/ui/servicemanager.py 2017-12-17 04:34:16 +0000 @@ -370,7 +370,7 @@ :rtype: None """ self._service_path = file_path - self.main_window.set_service_modified(self.is_modified(), file_path.name) + self.set_modified(self.is_modified()) Settings().setValue('servicemanager/last file', file_path) if file_path and file_path.suffix == '.oszl': self._save_lite = True === modified file 'openlp/core/version.py' --- openlp/core/version.py 2017-11-18 11:23:15 +0000 +++ openlp/core/version.py 2017-12-17 04:34:16 +0000 @@ -127,7 +127,9 @@ worker.quit.connect(update_check_date) # TODO: Use this to figure out if there's an Internet connection? # worker.no_internet.connect(parent.on_no_internet) - run_thread(parent, worker, 'version') + + parent.version_thread = run_thread(worker) + parent.version_worker = worker def get_version(): === modified file 'openlp/plugins/bibles/forms/bibleimportform.py' --- openlp/plugins/bibles/forms/bibleimportform.py 2017-11-06 22:41:36 +0000 +++ openlp/plugins/bibles/forms/bibleimportform.py 2017-12-17 04:34:16 +0000 @@ -336,6 +336,7 @@ self.sword_layout.addWidget(self.sword_tab_widget) self.sword_disabled_label = QtWidgets.QLabel(self.sword_widget) self.sword_disabled_label.setObjectName('SwordDisabledLabel') + self.sword_disabled_label.setWordWrap(True) self.sword_layout.addWidget(self.sword_disabled_label) self.select_stack.addWidget(self.sword_widget) self.wordproject_widget = QtWidgets.QWidget(self.select_page) === modified file 'openlp/plugins/presentations/presentationplugin.py' --- openlp/plugins/presentations/presentationplugin.py 2017-11-19 21:57:38 +0000 +++ openlp/plugins/presentations/presentationplugin.py 2017-12-17 04:34:16 +0000 @@ -129,7 +129,8 @@ """ log.debug('check_pre_conditions') controller_dir = os.path.join('plugins', 'presentations', 'lib') - glob_pattern = os.path.join(controller_dir, '*controller.py') + # Find all files that do not begin with '.' (lp:#1738047) and end with controller.py + glob_pattern = os.path.join(controller_dir, '[!.]*controller.py') extension_loader(glob_pattern, ['presentationcontroller.py']) controller_classes = PresentationController.__subclasses__() for controller_class in controller_classes: === modified file 'openlp/plugins/songs/lib/importers/songbeamer.py' --- openlp/plugins/songs/lib/importers/songbeamer.py 2017-10-10 02:29:56 +0000 +++ openlp/plugins/songs/lib/importers/songbeamer.py 2017-12-17 04:34:16 +0000 @@ -22,20 +22,29 @@ """ The :mod:`songbeamer` module provides the functionality for importing SongBeamer songs into the OpenLP database. """ +import base64 +import codecs import logging +import math import os import re -import base64 -import math -from openlp.core.common import is_win, is_macosx, get_file_encoding +from openlp.core.common import is_win, is_macosx +from openlp.core.common.path import Path from openlp.core.common.settings import Settings -from openlp.core.common.path import Path from openlp.plugins.songs.lib import VerseType from openlp.plugins.songs.lib.importers.songimport import SongImport log = logging.getLogger(__name__) +# https://forum.songbeamer.com/viewtopic.php?t=166 lists the encodings that SongBeamer supports, when they specify +# Unicode and Big Endian Unicode, they probably mean UTF16. +bom_encodings = { + codecs.BOM_UTF8: 'utf-8-sig', + codecs.BOM_UTF16_BE: 'utf-16-be', + codecs.BOM_UTF16_LE: 'utf-16-le' +} + class SongBeamerTypes(object): MarkTypes = { @@ -121,18 +130,11 @@ self.current_verse = '' self.current_verse_type = VerseType.tags[VerseType.Verse] self.chord_table = None - if file_path.is_file(): - # Detect the encoding - self.input_file_encoding = get_file_encoding(file_path)['encoding'] - # The encoding should only be ANSI (cp1252), UTF-8, Unicode, Big-Endian-Unicode. - # So if it doesn't start with 'u' we default to cp1252. See: - # https://forum.songbeamer.com/viewtopic.php?p=419&sid=ca4814924e37c11e4438b7272a98b6f2 - if not self.input_file_encoding.lower().startswith('u'): - self.input_file_encoding = 'cp1252' - with file_path.open(encoding=self.input_file_encoding) as song_file: - song_data = song_file.readlines() - else: + if not file_path.is_file(): continue + self.input_file_encoding = detect_file_encoding(file_path) + with file_path.open(encoding=self.input_file_encoding) as song_file: + song_data = song_file.readlines() self.title = file_path.stem read_verses = False # The first verse separator doesn't count, but the others does, so line count starts at -1 @@ -427,3 +429,22 @@ else: log.debug('Could not import mediafile "{audio_file_path}" since it does not exists!' .format(audio_file_path=audio_file_path)) + + +def detect_file_encoding(file_path): + """ + Detect the encoding of the song file using any present BOM as per the rules at + https://forum.songbeamer.com/viewtopic.php?t=166 + + :param openlp.core.common.path.Path file_path: The file to detect the encoding of + :return: The file encoding + :rtype: str + """ + with file_path.open(mode='rb') as song_file: + bom = song_file.read(3) + for key, value in bom_encodings.items(): + if bom.startswith(key): + log.debug('%s BOM detected at the start of %s file.', value, file_path) + return value + log.debug('No BOM detected at the start of %s file. Falling back to cp1252.', file_path) + return 'cp1252' === modified file 'tests/functional/openlp_plugins/presentations/test_impresscontroller.py' --- tests/functional/openlp_plugins/presentations/test_impresscontroller.py 2017-10-07 07:05:07 +0000 +++ tests/functional/openlp_plugins/presentations/test_impresscontroller.py 2017-12-17 04:34:16 +0000 @@ -23,7 +23,7 @@ Functional tests to test the Impress class and related methods. """ from unittest import TestCase -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import shutil from tempfile import mkdtemp @@ -72,6 +72,60 @@ self.assertEqual('Impress', controller.name, 'The name of the presentation controller should be correct') + @patch('openlp.plugins.presentations.lib.impresscontroller.log') + def test_check_available(self, mocked_log): + """ + Test `ImpressController.check_available` on Windows + """ + # GIVEN: An instance of :class:`ImpressController` + controller = ImpressController(plugin=self.mock_plugin) + + # WHEN: `check_available` is called on Windows and `get_com_servicemanager` returns None + with patch('openlp.plugins.presentations.lib.impresscontroller.is_win', return_value=True), \ + patch.object(controller, 'get_com_servicemanager', return_value=None) as mocked_get_com_servicemanager: + result = controller.check_available() + + # THEN: `check_available` should return False + assert mocked_get_com_servicemanager.called is True + assert result is False + + @patch('openlp.plugins.presentations.lib.impresscontroller.log') + def test_check_available1(self, mocked_log): + """ + Test `ImpressController.check_available` on Windows + """ + # GIVEN: An instance of :class:`ImpressController` + controller = ImpressController(plugin=self.mock_plugin) + + # WHEN: `check_available` is called on Windows and `get_com_servicemanager` returns an object + mocked_com_object = MagicMock() + with patch('openlp.plugins.presentations.lib.impresscontroller.is_win', return_value=True), \ + patch.object(controller, 'get_com_servicemanager', return_value=mocked_com_object) \ + as mocked_get_com_servicemanager: + result = controller.check_available() + + # THEN: `check_available` should return True + assert mocked_get_com_servicemanager.called is True + assert result is True + + @patch('openlp.plugins.presentations.lib.impresscontroller.log') + @patch('openlp.plugins.presentations.lib.impresscontroller.is_win', return_value=False) + def test_check_available2(self, mocked_is_win, mocked_log): + """ + Test `ImpressController.check_available` when not on Windows + """ + # GIVEN: An instance of :class:`ImpressController` + controller = ImpressController(plugin=self.mock_plugin) + + # WHEN: `check_available` is called on Windows and `uno_available` is True + with patch('openlp.plugins.presentations.lib.impresscontroller.uno_available', True), \ + patch.object(controller, 'get_com_servicemanager') as mocked_get_com_servicemanager: + result = controller.check_available() + + # THEN: `check_available` should return True + assert mocked_get_com_servicemanager.called is False + assert result is True + class TestImpressDocument(TestCase): """ === modified file 'tests/functional/openlp_plugins/songs/test_songbeamerimport.py' --- tests/functional/openlp_plugins/songs/test_songbeamerimport.py 2017-10-10 02:29:56 +0000 +++ tests/functional/openlp_plugins/songs/test_songbeamerimport.py 2017-12-17 04:34:16 +0000 @@ -26,9 +26,9 @@ from unittest import TestCase from unittest.mock import MagicMock, patch +from openlp.core.common.path import Path from openlp.core.common.registry import Registry -from openlp.core.common.path import Path -from openlp.plugins.songs.lib.importers.songbeamer import SongBeamerImport, SongBeamerTypes +from openlp.plugins.songs.lib.importers.songbeamer import SongBeamerImport, SongBeamerTypes, detect_file_encoding from tests.helpers.songfileimport import SongImportTestHelper @@ -224,3 +224,54 @@ for tag in SongBeamerTypes.MarkTypes.keys(): # THEN: tag should be defined in lowercase self.assertEquals(tag, tag.lower(), 'Tags should be defined in lowercase') + + +class TestSongBeamerModuleFunctions(TestCase): + """ + Test the functions in the :mod:`songbeamerimport` module. + """ + @patch('openlp.plugins.songs.lib.importers.songbeamer.log.debug') + def test_valid_bom(self, mocked_debug): + """ + Test `detect_file_encoding` when the files contain valid BOM's + """ + # GIVEN: A mocked file object and some test data with valid BOM's + mocked_file = MagicMock() + mocked_path = MagicMock(**{'open.return_value.__enter__.return_value': mocked_file}) + test_data = [(b'\xef\xbb\xbf', 'utf-8-sig'), # UTF-8 + (b'\xff\xfe\x00', 'utf-16-le'), # UTF-16, little endian + (b'\xfe\xff\x00', 'utf-16-be')] # UTF-16, big endian + + # WHEN: calling `detect_file_encoding` with each BOM in test_data + for input_data, result_data in test_data: + mocked_file.read.return_value = input_data + result = detect_file_encoding(mocked_path) + + # THEN: Each file should be logged with the correct encoding and the encoding should be returned + mocked_debug.assert_called_once_with('%s BOM detected at the start of %s file.', result_data, mocked_path) + assert result == result_data + + mocked_debug.reset_mock() + + @patch('openlp.plugins.songs.lib.importers.songbeamer.log.debug') + def test_no_bom(self, mocked_debug): + """ + Test `detect_file_encoding` when the files do not contain a valid BOM + """ + # GIVEN: A mocked file object and some test data with invalid BOM's + mocked_file = MagicMock(**{'read.side_effect': [b'#La', b'abc', b'123']}) + mocked_path = MagicMock(**{'open.return_value.__enter__.return_value': mocked_file}) + + while True: # Loop until the values in the mocked_file read side_effect are exhausted + try: + # WHEN: calling `detect_file_encoding` + result = detect_file_encoding(mocked_path) + + # THEN: No BOM should be detected, and 'cp1252' should be returned as the encoding + mocked_debug.assert_called_once_with('No BOM detected at the start of %s file. Falling back to cp1252.', + mocked_path) + assert result == 'cp1252' + + mocked_debug.reset_mock() + except StopIteration: + break
_______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp