openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #32513
[Merge] lp:~phill-ridout/openlp/fixes-V into lp:openlp
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
Follow ups
