Merge branch 'revert-registry-get' into 'master'

Revert the Registry behaviour

See merge request openlp/openlp!668
This commit is contained in:
Raoul Snyman 2023-09-14 16:04:56 +00:00
commit f000948713
11 changed files with 104 additions and 87 deletions

View File

@ -313,6 +313,8 @@ def set_up_logging(log_path):
logfile = logging.FileHandler(file_path, 'w', encoding='UTF-8')
logfile.setFormatter(logging.Formatter('%(asctime)s %(threadName)s %(name)-55s %(levelname)-8s %(message)s'))
log.addHandler(logfile)
# Send warnings to the log file
logging.captureWarnings(True)
print(f'Logging to: {file_path} and level {log.level}')

View File

@ -23,7 +23,6 @@ Provide Error Handling and login Services
"""
import inspect
import logging
from typing import Any
from openlp.core.common import trace_error_handler
from openlp.core.common.platform import is_win
@ -128,13 +127,6 @@ class RegistryProperties(object):
_projector_manager = None
_settings = None
def _get_object(self, name: str) -> Any:
"""Get an object, or return None"""
try:
return Registry().get(name)
except KeyError:
return None
@property
def application(self):
"""
@ -142,10 +134,10 @@ class RegistryProperties(object):
Windows needs to access the application in a dynamic manner.
"""
if is_win():
return self._get_object('application')
return Registry().get('application')
else:
if not hasattr(self, '_application') or not self._application:
self._application = self._get_object('application')
self._application = Registry().get('application')
return self._application
@property
@ -154,7 +146,7 @@ class RegistryProperties(object):
Adds the plugin manager to the class dynamically
"""
if not hasattr(self, '_plugin_manager') or not self._plugin_manager:
self._plugin_manager = self._get_object('plugin_manager')
self._plugin_manager = Registry().get('plugin_manager')
return self._plugin_manager
@property
@ -163,7 +155,7 @@ class RegistryProperties(object):
Adds the media controller to the class dynamically
"""
if not hasattr(self, '_media_controller') or not self._media_controller:
self._media_controller = self._get_object('media_controller')
self._media_controller = Registry().get('media_controller')
return self._media_controller
@property
@ -172,7 +164,7 @@ class RegistryProperties(object):
Adds the service manager to the class dynamically
"""
if not hasattr(self, '_service_manager') or not self._service_manager:
self._service_manager = self._get_object('service_manager')
self._service_manager = Registry().get('service_manager')
return self._service_manager
@property
@ -181,7 +173,7 @@ class RegistryProperties(object):
Adds the preview controller to the class dynamically
"""
if not hasattr(self, '_preview_controller') or not self._preview_controller:
self._preview_controller = self._get_object('preview_controller')
self._preview_controller = Registry().get('preview_controller')
return self._preview_controller
@property
@ -190,7 +182,7 @@ class RegistryProperties(object):
Adds the live controller to the class dynamically
"""
if not hasattr(self, '_live_controller') or not self._live_controller:
self._live_controller = self._get_object('live_controller')
self._live_controller = Registry().get('live_controller')
return self._live_controller
@property
@ -199,7 +191,7 @@ class RegistryProperties(object):
Adds the main window to the class dynamically
"""
if not hasattr(self, '_main_window') or not self._main_window:
self._main_window = self._get_object('main_window')
self._main_window = Registry().get('main_window')
return self._main_window
@property
@ -208,7 +200,7 @@ class RegistryProperties(object):
Adds the Renderer to the class dynamically
"""
if not hasattr(self, '_renderer') or not self._renderer:
self._renderer = self._get_object('renderer')
self._renderer = Registry().get('renderer')
return self._renderer
@property
@ -217,7 +209,7 @@ class RegistryProperties(object):
Adds the theme manager to the class dynamically
"""
if not hasattr(self, '_theme_manager') or not self._theme_manager:
self._theme_manager = self._get_object('theme_manager')
self._theme_manager = Registry().get('theme_manager')
return self._theme_manager
@property
@ -226,7 +218,7 @@ class RegistryProperties(object):
Adds the settings form to the class dynamically
"""
if not hasattr(self, '_settings_form') or not self._settings_form:
self._settings_form = self._get_object('settings_form')
self._settings_form = Registry().get('settings_form')
return self._settings_form
@property
@ -235,7 +227,7 @@ class RegistryProperties(object):
Adds the alerts manager to the class dynamically
"""
if not hasattr(self, '_alerts_manager') or not self._alerts_manager:
self._alerts_manager = self._get_object('alerts_manager')
self._alerts_manager = Registry().get('alerts_manager')
return self._alerts_manager
@property
@ -244,7 +236,7 @@ class RegistryProperties(object):
Adds the projector manager to the class dynamically
"""
if not hasattr(self, '_projector_manager') or not self._projector_manager:
self._projector_manager = self._get_object('projector_manager')
self._projector_manager = Registry().get('projector_manager')
return self._projector_manager
@property
@ -253,5 +245,5 @@ class RegistryProperties(object):
Adds the settings object to the class dynamically
"""
if not hasattr(self, '_settings') or not self._settings:
self._settings = self._get_object('settings')
self._settings = Registry().get('settings')
return self._settings

View File

@ -102,6 +102,8 @@ def str_to_path(string):
:return: None if :param:`string` is empty, or a Path object representation of :param:`string`
:rtype: Path | None
"""
if isinstance(string, Path):
return string
if not isinstance(string, str):
log.error('parameter \'string\' must be of type str, got {} which is a {} instead'.format(string, type(string)))
return None

View File

@ -22,8 +22,8 @@
Provide Registry Services
"""
import logging
from contextlib import contextmanager
from typing import Any, Callable
from warnings import warn
from openlp.core.common import Singleton, de_hump, trace_error_handler
@ -51,13 +51,6 @@ class Registry(metaclass=Singleton):
registry._is_suppressing = False
return registry
@contextmanager
def suppress_error(self):
"""Suppress errors temporarily"""
self._is_suppressing = True
yield
self._is_suppressing = False
def get(self, key: str) -> Any | None:
"""
Extracts the registry value from the list based on the key passed in
@ -67,12 +60,8 @@ class Registry(metaclass=Singleton):
if key in self.service_list:
return self.service_list[key]
else:
if self._is_suppressing:
return None
else:
trace_error_handler(log)
log.error('Service {key} not found in list'.format(key=key))
raise KeyError('Service {key} not found in list'.format(key=key))
warn(f'Service "{key}" not found in list', stacklevel=2)
return None
def register(self, key: str, reference: Any):
"""

View File

@ -22,7 +22,6 @@
The :mod:`~openlp.core.loader` module provides a bootstrap for the singleton classes
"""
from openlp.core.common.registry import Registry
from openlp.core.display.render import Renderer
from openlp.core.lib.pluginmanager import PluginManager
from openlp.core.state import State
@ -37,13 +36,11 @@ def loader():
:return: None
"""
# Suppress errors when calling logging due initialisation due to incomplete plugin initialization.
with Registry().suppress_error():
State().load_settings()
MediaController()
PluginManager()
# Set up the path with plugins
Renderer(window_title='Renderer')
# Create slide controllers
PreviewController()
LiveController()
State().load_settings()
MediaController()
PluginManager()
# Set up the path with plugins
Renderer(window_title='Renderer')
# Create slide controllers
PreviewController()
LiveController()

View File

@ -52,11 +52,7 @@ def run_thread(worker, thread_name, can_start=True):
if not thread_name:
raise ValueError('A thread_name is required when calling the "run_thread" function')
application = Registry().get('application')
try:
# In some cases (see First Time Form), the main window does not exist yet
main_window = Registry().get('main_window')
except KeyError:
main_window = None
main_window = Registry().get('main_window')
if thread_name in application.worker_threads:
raise KeyError('A thread with the name "{}" has already been created, please use another'.format(thread_name))
# Create the thread and add the thread and the worker to the parent
@ -121,8 +117,7 @@ def make_remove_thread(thread_name):
:param str thread_name: The name of the thread to stop and remove
"""
with Registry().suppress_error():
application = Registry().get('application')
application = Registry().get('application')
if application and thread_name in application.worker_threads:
del application.worker_threads[thread_name]
return remove_thread

View File

@ -344,6 +344,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
To be called as part of initialisation
"""
self.setup_ui(self)
self.set_theme_visibility()
# Need to use event as called across threads and UI is updated
self.servicemanager_set_item.connect(self.on_set_item)
self.servicemanager_set_item_by_uuid.connect(self.set_item_by_uuid)
@ -455,6 +456,12 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
else:
self.suffixes.update(suffix_list)
def set_theme_visibility(self):
"""Set the visibility of the theme items"""
visible = self.settings.value('themes/theme level') != ThemeLevel.Global
self.toolbar.actions['theme_combo_box'].setVisible(visible)
self.toolbar.actions['theme_label'].setVisible(visible)
def on_new_service_clicked(self):
"""
Create a new service.
@ -1481,9 +1488,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
"""
The theme may have changed in the settings dialog so make sure the theme combo box is in the correct state.
"""
visible = self.settings.value('themes/theme level') != ThemeLevel.Global
self.toolbar.actions['theme_combo_box'].setVisible(visible)
self.toolbar.actions['theme_label'].setVisible(visible)
self.set_theme_visibility()
self.regenerate_service_items()
def on_service_theme_change(self):

View File

@ -348,8 +348,6 @@ class SongsPlugin(Plugin):
If the first time wizard has run, this function is run to import all the new songs into the database.
"""
self.application.process_events()
self.on_tools_reindex_item_triggered()
self.application.process_events()
db_dir_path = Path(gettempdir(), 'openlp')
if not db_dir_path.exists():
return
@ -376,6 +374,9 @@ class SongsPlugin(Plugin):
importer.do_import(progress)
self.application.process_events()
progress.setValue(song_count)
self.application.process_events()
self.on_tools_reindex_item_triggered()
self.application.process_events()
self.media_item.on_search_text_button_clicked()
def finalise(self):

View File

@ -144,6 +144,17 @@ def test_path_to_str_path_object():
assert result == os.path.join('test', 'path')
def test_str_to_path_path_object():
"""
Test that `str_to_path` returns the Path object it is supplied
"""
# GIVEN: The `str_to_path` function and a Path object
path = Path(__file__)
# WHEN: Calling `str_to_path` with a Path object
# THEN: returns that exact Path object
assert str_to_path(path) is path
def test_str_to_path_type_error():
"""
Test that `str_to_path` returns None if called with invalid information
@ -151,7 +162,7 @@ def test_str_to_path_type_error():
# GIVEN: The `str_to_path` function
# WHEN: Calling `str_to_path` with an invalid Type
# THEN: None is returned
assert str_to_path(Path()) is None
assert str_to_path(42) is None
def test_str_to_path_empty_str():

View File

@ -57,26 +57,13 @@ def test_registry_service(registry: Registry):
'KeyError exception should have been thrown for duplicate service'
# WHEN I try to get back a non existent component
# THEN I will get an exception
with pytest.raises(KeyError):
Registry().get('test2')
# THEN I will get None
assert Registry().get('test2') is None
# WHEN I try to replace a component I should be allowed
Registry().remove('test1')
# THEN I will get an exception
with pytest.raises(KeyError):
Registry().get('test1')
def test_registry_service_suppressed(registry: Registry):
"""Test that None is returned when a service doesn't exist in the registry and errors are suppressed"""
# GIVEN: A registry
# WHEN: Trying to "get" an object
with Registry().suppress_error():
result = Registry().get('main_window')
# THEN: None is returned
assert result is None
assert Registry().get('test1') is None
def test_registry_function(registry: Registry):
@ -179,17 +166,6 @@ def test_registry_working_flags(registry: Registry):
'KeyError exception should have been thrown for duplicate working flag'
def test_registry_working_flags_suppressed(registry: Registry):
"""Test that no exception is raised when a flag doesn't exist and errors are suppressed"""
# GIVEN: A registry
# WHEN: Trying to "get" an object
with Registry().suppress_error():
result = Registry().get_flag('test1')
# THEN: None is returned
assert result is None
def test_remove_function(registry: Registry):
"""
Test the remove_function() method

View File

@ -0,0 +1,47 @@
# -*- coding: utf-8 -*-
##########################################################################
# OpenLP - Open Source Lyrics Projection #
# ---------------------------------------------------------------------- #
# Copyright (c) 2008-2023 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, either version 3 of the License, or #
# (at your option) any later version. #
# #
# 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, see <https://www.gnu.org/licenses/>. #
##########################################################################
"""
Package to test the openlp.core.loader package.
"""
from unittest.mock import MagicMock, patch
from openlp.core.loader import loader
@patch('openlp.core.loader.Renderer', autospec=True)
@patch('openlp.core.loader.PluginManager', autospec=True)
@patch('openlp.core.loader.State', autospec=True)
@patch('openlp.core.loader.MediaController', autospec=True)
@patch('openlp.core.loader.LiveController', autospec=True)
@patch('openlp.core.loader.PreviewController', autospec=True)
def test_loader(MockPreviewController: MagicMock, MockLiveController: MagicMock,
MockMediaController: MagicMock, MockState: MagicMock, MockPluginManager: MagicMock,
MockRenderer: MagicMock):
# GIVEN: A bunch of mocks
# WHEN: loader() is called
loader()
# THEN: The mocks should have been called
MockState.return_value.load_settings.assert_called_once_with()
MockMediaController.assert_called_once_with()
MockPluginManager.assert_called_once_with()
MockRenderer.assert_called_once_with(window_title='Renderer')
MockPreviewController.assert_called_once_with()
MockLiveController.assert_called_once_with()