From f098b07f5e09768d899cbfad759e5b527b055aee Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Fri, 11 Aug 2023 12:14:14 -0700 Subject: [PATCH] Fix issue #1582 by running the search in the original thread --- openlp/core/api/versions/v2/plugins.py | 12 ++++- openlp/core/ui/library.py | 3 +- openlp/plugins/bibles/lib/mediaitem.py | 4 +- openlp/plugins/custom/lib/mediaitem.py | 5 +- openlp/plugins/songs/lib/mediaitem.py | 4 +- tests/openlp_core/api/v2/test_plugins.py | 61 +++++++++++++++++++++++- 6 files changed, 81 insertions(+), 8 deletions(-) diff --git a/openlp/core/api/versions/v2/plugins.py b/openlp/core/api/versions/v2/plugins.py index 36ea5d89a..616530372 100644 --- a/openlp/core/api/versions/v2/plugins.py +++ b/openlp/core/api/versions/v2/plugins.py @@ -20,10 +20,11 @@ # along with this program. If not, see . # ########################################################################## import logging - import json import re + from flask import abort, request, Blueprint, jsonify, Response +from PyQt5 import QtCore from openlp.core.api.lib import login_required, extract_request, old_success_response, old_auth from openlp.core.lib.plugin import PluginStatus @@ -42,7 +43,14 @@ alert_2_views = Blueprint('v2-alert-plugin', __name__) def search(plugin_name, text): plugin = Registry().get('plugin_manager').get_plugin_by_name(plugin_name) if plugin.status == PluginStatus.Active and plugin.media_item and plugin.media_item.has_search: - results = plugin.media_item.search(text, False) + if hasattr(plugin.media_item.search, '__pyqtSignature__'): + # If this method has a signature, it means that it should be called from the parent thread + results = plugin.media_item.staticMetaObject.invokeMethod( + plugin.media_item, 'search', QtCore.Qt.BlockingQueuedConnection, + QtCore.Q_RETURN_ARG(list), QtCore.Q_ARG(str, text), QtCore.Q_ARG(bool, False)) + else: + # Fall back to original behaviour + results = plugin.media_item.search(text, False) return results return None diff --git a/openlp/core/ui/library.py b/openlp/core/ui/library.py index 47831e6e5..f8a3921b8 100644 --- a/openlp/core/ui/library.py +++ b/openlp/core/ui/library.py @@ -363,7 +363,8 @@ class FolderLibraryItem(MediaManagerItem): """ return [item.file_path, item.file_path] - def search(self, string, show_error): + @QtCore.pyqtSlot(str, bool, result=list) + def search(self, string: str, show_error: bool = True) -> list[list[Any]]: """ Performs a search for items containing ``string`` diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index c5f95044d..7f1db1348 100755 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -22,6 +22,7 @@ import logging import re from enum import IntEnum, unique +from typing import Any from PyQt5 import QtCore, QtWidgets @@ -1072,7 +1073,8 @@ class BibleMediaItem(MediaManagerItem): else: return False - def search(self, string, show_error=True): + @QtCore.pyqtSlot(str, bool, result=list) + def search(self, string: str, show_error: bool = True) -> list[list[Any]]: """ Search for some Bible verses (by reference). :param string: search string diff --git a/openlp/plugins/custom/lib/mediaitem.py b/openlp/plugins/custom/lib/mediaitem.py index 00dc72348..425bac5c4 100644 --- a/openlp/plugins/custom/lib/mediaitem.py +++ b/openlp/plugins/custom/lib/mediaitem.py @@ -18,8 +18,8 @@ # You should have received a copy of the GNU General Public License # # along with this program. If not, see . # ########################################################################## - import logging +from typing import Any from PyQt5 import QtCore, QtWidgets from sqlalchemy.sql import and_, func, or_ @@ -348,7 +348,8 @@ class CustomMediaItem(MediaManagerItem): self.search_text_edit.clear() self.on_search_text_button_clicked() - def search(self, string, show_error): + @QtCore.pyqtSlot(str, bool, result=list) + def search(self, string: str, show_error: bool = True) -> list[list[Any]]: """ Search the database for a given item. diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 3cde9cbdd..a64feb31b 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -23,6 +23,7 @@ import logging import mako import os from shutil import copyfile +from typing import Any from PyQt5 import QtCore, QtWidgets from sqlalchemy.sql import and_, or_ @@ -841,7 +842,8 @@ class SongMediaItem(MediaManagerItem): # List must be empty at the end return not author_list - def search(self, string, show_error=True): + @QtCore.pyqtSlot(str, bool, result=list) + def search(self, string: str, show_error: bool = True) -> list[list[Any]]: """ Search for some songs :param string: The string to show diff --git a/tests/openlp_core/api/v2/test_plugins.py b/tests/openlp_core/api/v2/test_plugins.py index 9bec50247..80f50dedf 100644 --- a/tests/openlp_core/api/v2/test_plugins.py +++ b/tests/openlp_core/api/v2/test_plugins.py @@ -22,16 +22,75 @@ from collections import namedtuple from pathlib import Path from unittest.mock import MagicMock +from PyQt5 import QtCore + from openlp.core.common.registry import Registry from openlp.core.common.enum import PluginStatus from openlp.core.display.render import Renderer from openlp.core.lib.serviceitem import ServiceItem from openlp.core.state import State +from openlp.core.api.versions.v2.plugins import search from tests.openlp_core.lib.test_serviceitem import TEST_PATH as SERVICEITEM_TEST_PATH from tests.utils import convert_file_service_item -# Search options tests +def test_search_threaded(registry, settings): + """Test that the search function calls the search method correctly when threaded""" + # GIVEN: A mocked plugin, and plugin manager + mocked_songs_plugin = MagicMock() + mocked_songs_plugin.status = PluginStatus.Active + mocked_songs_plugin.media_item.has_search = True + mocked_songs_plugin.media_item.search.__pyqtSignature__ = 'slot' + mocked_songs_plugin.media_item.staticMetaObject.invokeMethod.return_value = [[1, 'Test', 'This is a test']] + mocked_plugin_manager = MagicMock(**{'get_plugin_by_name.return_value': mocked_songs_plugin}) + Registry().register('plugin_manager', mocked_plugin_manager) + + # WHEN: the search function is called + result = search('songs', 'test') + + # THEN: A song should be returned via the invokeMethod method + assert result == [[1, 'Test', 'This is a test']] + mocked_songs_plugin.media_item.staticMetaObject.invokeMethod.assert_called_once() + invoke_call = mocked_songs_plugin.media_item.staticMetaObject.invokeMethod.call_args_list[0] + assert invoke_call.args[0] is mocked_songs_plugin.media_item + assert invoke_call.args[1] == 'search' + assert invoke_call.args[2] == QtCore.Qt.BlockingQueuedConnection + + +def test_search_unthreaded(registry, settings): + """Test that the search function calls the search method correctly when we don't care about threads""" + # GIVEN: A mocked plugin, and plugin manager + mocked_songs_plugin = MagicMock() + mocked_songs_plugin.status = PluginStatus.Active + mocked_songs_plugin.media_item.has_search = True + mocked_songs_plugin.media_item.search.return_value = [[1, 'Test', 'This is a test']] + mocked_plugin_manager = MagicMock(**{'get_plugin_by_name.return_value': mocked_songs_plugin}) + Registry().register('plugin_manager', mocked_plugin_manager) + + # WHEN: the search function is called + result = search('songs', 'test') + + # THEN: A song should be returned via the regular search method + assert result == [[1, 'Test', 'This is a test']] + mocked_songs_plugin.media_item.staticMetaObject.invokeMethod.assert_not_called() + mocked_songs_plugin.media_item.search.assert_called_once_with('test', False) + + +def test_search_nothing(registry, settings): + """Test that the search function returns None when there are no plugins by that name enabled""" + # GIVEN: A mocked plugin, and plugin manager + mocked_songs_plugin = MagicMock() + mocked_songs_plugin.status = PluginStatus.Disabled + mocked_plugin_manager = MagicMock(**{'get_plugin_by_name.return_value': mocked_songs_plugin}) + Registry().register('plugin_manager', mocked_plugin_manager) + + # WHEN: the search function is called + result = search('songs', 'test') + + # THEN: None should be returned + assert result is None + + def test_bibles_search_options_returns_list(flask_client, settings): """ Test a list is returned when a plugin's search options are requested.