Merge branch 'fix-1627' into 'master'

Silence error when shutting down threads

Closes #1627

See merge request openlp/openlp!661

openlp/core/common/registry.py

@@ -22,6 +22,8 @@
 Provide Registry Services
 """
 import logging
+from contextlib import contextmanager
+from typing import Any, Callable
 
 from openlp.core.common import Singleton, de_hump, trace_error_handler
 
@@ -37,7 +39,7 @@ class Registry(metaclass=Singleton):
     log.info('Registry loaded')
 
     @classmethod
-    def create(cls):
+    def create(cls) -> 'Registry':
         """
         The constructor for the component registry providing a single registry of objects.
         """
@@ -46,14 +48,17 @@ class Registry(metaclass=Singleton):
         registry.service_list = {}
         registry.functions_list = {}
         registry.working_flags = {}
-        registry._is_initialising = False
+        registry._is_suppressing = False
         return registry
 
-    def set_initialising(self, is_initialising: bool) -> None:
-        """A method to set if the Registry is initialising or not"""
-        self._is_initialising = is_initialising
+    @contextmanager
+    def suppress_error(self):
+        """Suppress errors temporarily"""
+        self._is_suppressing = True
+        yield
+        self._is_suppressing = False
 
-    def get(self, key):
+    def get(self, key: str) -> Any | None:
         """
         Extracts the registry value from the list based on the key passed in
 
@@ -62,12 +67,14 @@ class Registry(metaclass=Singleton):
         if key in self.service_list:
             return self.service_list[key]
         else:
-            if not self._is_initialising:
+            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))
 
-    def register(self, key, reference):
+    def register(self, key: str, reference: Any):
         """
         Registers a component against a key.
 
@@ -82,7 +89,7 @@ class Registry(metaclass=Singleton):
         else:
             self.service_list[key] = reference
 
-    def remove(self, key):
+    def remove(self, key: str):
         """
         Removes the registry value from the list based on the key passed in.
 
@@ -91,7 +98,7 @@ class Registry(metaclass=Singleton):
         if key in self.service_list:
             del self.service_list[key]
 
-    def register_function(self, event, function):
+    def register_function(self, event: str, function: Callable):
         """
         Register an event and associated function to be called
 
@@ -106,7 +113,7 @@ class Registry(metaclass=Singleton):
         else:
             self.functions_list[event] = [function]
 
-    def remove_function(self, event, function):
+    def remove_function(self, event: str, function: Callable):
         """
         Remove an event and associated handler
 
@@ -116,7 +123,7 @@ class Registry(metaclass=Singleton):
         if event in self.functions_list and function in self.functions_list[event]:
             self.functions_list[event].remove(function)
 
-    def has_function(self, event):
+    def has_function(self, event: str) -> bool:
         """
         Returns whether there's any handler associated with the event.
 
@@ -132,7 +139,7 @@ class Registry(metaclass=Singleton):
         """
         return service_name in self.service_list
 
-    def execute(self, event, *args, **kwargs):
+    def execute(self, event: str, *args, **kwargs) -> Any | None:
         """
         Execute all the handlers associated with the event and return an array of results.
 
@@ -155,10 +162,10 @@ class Registry(metaclass=Singleton):
         else:
             if log.getEffectiveLevel() == logging.DEBUG:
                 trace_error_handler(log)
-                log.exception('Event {event} called but not registered'.format(event=event))
+                log.error('Event {event} called but not registered'.format(event=event))
         return results
 
-    def get_flag(self, key):
+    def get_flag(self, key: str) -> Any | None:
         """
         Extracts the working_flag value from the list based on the key passed in
 
@@ -166,12 +173,14 @@ class Registry(metaclass=Singleton):
         """
         if key in self.working_flags:
             return self.working_flags[key]
+        elif self._is_suppressing:
+            return None
         else:
             trace_error_handler(log)
             log.error('Working Flag {key} not found in list'.format(key=key))
             raise KeyError('Working Flag {key} not found in list'.format(key=key))
 
-    def set_flag(self, key, reference):
+    def set_flag(self, key: str, reference: Any):
         """
         Sets a working_flag based on the key passed in.
 
@@ -180,7 +189,7 @@ class Registry(metaclass=Singleton):
         """
         self.working_flags[key] = reference
 
-    def remove_flag(self, key):
+    def remove_flag(self, key: str):
         """
         Removes the working flags value from the list based on the key passed.
 

openlp/core/loader.py

@@ -37,15 +37,13 @@ def loader():
 
     :return: None
     """
-    # Stop errors when calling logging due initialisation due to incomplete plugin initialization.
-    Registry().set_initialising(True)
-    State().load_settings()
-    MediaController()
-    PluginManager()
-    # Set up the path with plugins
-    Renderer(window_title='Renderer')
-    # Create slide controllers
-    PreviewController()
-    LiveController()
-    # Turn logging back on again.
-    Registry().set_initialising(False)
+    # 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()

openlp/core/threading.py

@@ -100,8 +100,11 @@ def is_thread_finished(thread_name):
     :param str thread_name: The name of the thread
     :returns: True if the thread is finished, False if it is still running
     """
-    app = Registry().get('application')
-    return thread_name not in app.worker_threads or app.worker_threads[thread_name]['thread'].isFinished()
+    try:
+        app = Registry().get('application')
+        return thread_name not in app.worker_threads or app.worker_threads[thread_name]['thread'].isFinished()
+    except KeyError:
+        return True
 
 
 def make_remove_thread(thread_name):
@@ -118,7 +121,8 @@ def make_remove_thread(thread_name):
 
         :param str thread_name: The name of the thread to stop and remove
         """
-        application = Registry().get('application')
+        with Registry().suppress_error():
+            application = Registry().get('application')
         if application and thread_name in application.worker_threads:
             del application.worker_threads[thread_name]
     return remove_thread

tests/openlp_core/common/test_registry.py

@@ -21,8 +21,10 @@
 """
 Package to test the openlp.core.lib package.
 """
+import logging
+from unittest.mock import MagicMock, patch
+
 import pytest
-from unittest.mock import MagicMock
 
 from openlp.core.common.registry import Registry, RegistryBase
 
@@ -35,7 +37,7 @@ def dummy_function_2():
     return "function_2"
 
 
-def test_registry_service(registry):
+def test_registry_service(registry: Registry):
     """
     Test the registry creation and its usage
     """
@@ -66,7 +68,18 @@ def test_registry_service(registry):
         Registry().get('test1')
 
 
-def test_registry_function(registry):
+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
+
+
+def test_registry_function(registry: Registry):
     """
     Test the registry function creation and their usages
     """
@@ -94,7 +107,43 @@ def test_registry_function(registry):
     assert return_value[0] == 'function_2', 'A return value is provided and matches'
 
 
-def test_registry_working_flags(registry):
+@patch('openlp.core.common.registry.log')
+@patch('openlp.core.common.registry.trace_error_handler')
+def test_registry_function_type_error(mocked_trace_error: MagicMock, mocked_log: MagicMock, registry: Registry):
+    """Test that a TypeError is logged"""
+    # GIVEN: A registry, some mocks, and a rigged function
+    mock_function = MagicMock()
+    mock_function.side_effect = TypeError
+    registry.register_function('mock', mock_function)
+
+    # WHEN: execute is called
+    result = registry.execute('mock', 'arg', kwarg='kwarg')
+
+    # THEN: result should be an empty list, and the correct functions should have been called
+    assert result == []
+    mock_function.assert_called_with('arg', kwarg='kwarg')
+    mocked_trace_error.assert_called_once_with(mocked_log)
+    mocked_log.exception.assert_called_once_with(f'Exception for function {mock_function}')
+
+
+@patch('openlp.core.common.registry.log')
+@patch('openlp.core.common.registry.trace_error_handler')
+def test_registry_function_does_not_exist(mocked_trace_error: MagicMock, mocked_log: MagicMock,
+                                          registry: Registry):
+    """Test that a TypeError is logged"""
+    # GIVEN: A registry, some mocks, and a rigged function
+    mocked_log.getEffectiveLevel.return_value = logging.DEBUG
+
+    # WHEN: execute is called
+    result = registry.execute('mock')
+
+    # THEN: result should be an empty list, and the correct functions should have been called
+    assert result == []
+    mocked_trace_error.assert_called_once_with(mocked_log)
+    mocked_log.error.assert_called_once_with('Event mock called but not registered')
+
+
+def test_registry_working_flags(registry: Registry):
     """
     Test the registry working flags creation and its usage
     """
@@ -130,7 +179,18 @@ def test_registry_working_flags(registry):
         'KeyError exception should have been thrown for duplicate working flag'
 
 
-def test_remove_function(registry):
+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
     """
@@ -154,7 +214,7 @@ class RegistryStub(RegistryBase):
         super().__init__()
 
 
-def test_registry_mixin_missing(registry):
+def test_registry_mixin_missing(registry: Registry):
     """
     Test the registry creation and its usage
     """
@@ -166,7 +226,7 @@ def test_registry_mixin_missing(registry):
     assert len(Registry().functions_list) == 0, 'The function should not be in the dict anymore.'
 
 
-def test_registry_mixin_present(registry):
+def test_registry_mixin_present(registry: Registry):
     """
     Test the registry creation and its usage
     """
@@ -176,3 +236,11 @@ def test_registry_mixin_present(registry):
 
     # THEN: The bootstrap methods should be registered
     assert len(Registry().functions_list) == 3, 'The bootstrap functions should be in the dict.'
+
+
+def test_registry_base_empty_methods(registry: Registry):
+    """Test the RegistryBase class, just to increase coverage"""
+    base = RegistryStub()
+    base.bootstrap_initialise()
+    base.bootstrap_post_set_up()
+    base.bootstrap_completion()