From d34e39524a05ef8cb1291e3b9cc868d7afc1d51a Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Sun, 25 Apr 2021 06:18:16 +0000 Subject: [PATCH] Fix websocket crash --- openlp/core/api/tab.py | 76 ++++++++++++++++++- openlp/core/api/websockets.py | 4 +- .../openlp_core/api/test_tab.py | 34 +++++++++ .../openlp_core/api/test_websockets.py | 52 ++++++++++++- 4 files changed, 162 insertions(+), 4 deletions(-) rename tests/{functional => }/openlp_core/api/test_tab.py (73%) rename tests/{functional => }/openlp_core/api/test_websockets.py (75%) diff --git a/openlp/core/api/tab.py b/openlp/core/api/tab.py index be9327099..c8e5b2c67 100644 --- a/openlp/core/api/tab.py +++ b/openlp/core/api/tab.py @@ -30,6 +30,7 @@ from openlp.core.common import get_network_interfaces from openlp.core.common.i18n import translate from openlp.core.common.registry import Registry from openlp.core.lib.settingstab import SettingsTab +from openlp.core.threading import is_thread_finished from openlp.core.ui.icons import UiIcons from openlp.core.widgets.dialogs import DownloadProgressDialog @@ -56,12 +57,20 @@ class ApiTab(SettingsTab): self.server_settings_layout.setObjectName('server_settings_layout') self.address_label = QtWidgets.QLabel(self.server_settings_group_box) self.address_label.setObjectName('address_label') + self.server_settings_layout.addRow(self.address_label) self.address_edit = QtWidgets.QLineEdit(self.server_settings_group_box) self.address_edit.setSizePolicy(QtWidgets.QSizePolicy.Preferred, QtWidgets.QSizePolicy.Fixed) self.address_edit.setValidator(QtGui.QRegExpValidator(QtCore.QRegExp(r'\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}'), self)) self.address_edit.setObjectName('address_edit') - self.server_settings_layout.addRow(self.address_label, self.address_edit) + self.address_revert_button = QtWidgets.QToolButton(self.server_settings_group_box) + self.address_revert_button.setObjectName('address_revert_button') + self.address_revert_button.setIcon(UiIcons().undo) + self.address_button_layout = QtWidgets.QHBoxLayout() + self.address_button_layout.setObjectName('address_button_layout') + self.address_button_layout.addWidget(self.address_edit) + self.address_button_layout.addWidget(self.address_revert_button) + self.server_settings_layout.addRow(self.address_button_layout) self.twelve_hour_check_box = QtWidgets.QCheckBox(self.server_settings_group_box) self.twelve_hour_check_box.setObjectName('twelve_hour_check_box') self.server_settings_layout.addRow(self.twelve_hour_check_box) @@ -159,8 +168,29 @@ class ApiTab(SettingsTab): self.app_qr_description_label.setOpenExternalLinks(True) self.app_qr_description_label.setWordWrap(True) self.app_qr_layout.addWidget(self.app_qr_description_label) + self.server_state_group_box = QtWidgets.QGroupBox(self.right_column) + self.server_state_group_box.setObjectName('server_state_group_box') + self.right_layout.addWidget(self.server_state_group_box) + self.server_state_layout = QtWidgets.QFormLayout(self.server_state_group_box) + self.server_http_state_title = QtWidgets.QLabel(self.server_state_group_box) + self.server_http_state_title.setObjectName('server_http_state_title') + self.server_http_state = QtWidgets.QLabel(self.server_state_group_box) + self.server_http_state.setObjectName('server_http_state') + self.server_state_layout.addRow(self.server_http_state_title, self.server_http_state) + self.server_websocket_state_title = QtWidgets.QLabel(self.server_state_group_box) + self.server_websocket_state_title.setObjectName('server_websocket_state_title') + self.server_websocket_state = QtWidgets.QLabel(self.server_state_group_box) + self.server_websocket_state.setObjectName('server_websocket_state') + self.server_state_layout.addRow(self.server_websocket_state_title, self.server_websocket_state) + self.server_zeroconf_state_title = QtWidgets.QLabel(self.server_state_group_box) + self.server_zeroconf_state_title.setObjectName('server_zeroconf_state_title') + self.server_zeroconf_state = QtWidgets.QLabel(self.server_state_group_box) + self.server_zeroconf_state.setObjectName('server_zeroconf_state') + self.server_state_layout.addRow(self.server_zeroconf_state_title, self.server_zeroconf_state) self.left_layout.addStretch() self.right_layout.addStretch() + + self.address_revert_button.clicked.connect(self.address_revert_button_clicked) self.twelve_hour_check_box.stateChanged.connect(self.on_twelve_hour_check_box_changed) self.thumbnails_check_box.stateChanged.connect(self.on_thumbnails_check_box_changed) self.address_edit.textChanged.connect(self.set_urls) @@ -170,7 +200,9 @@ class ApiTab(SettingsTab): def retranslate_ui(self): self.tab_title_visible = translate('RemotePlugin.RemoteTab', 'Remote Interface') self.server_settings_group_box.setTitle(translate('RemotePlugin.RemoteTab', 'Server Settings')) - self.address_label.setText(translate('RemotePlugin.RemoteTab', 'IP address:')) + self.address_label.setText(translate('RemotePlugin.RemoteTab', + 'Listen IP address (0.0.0.0 matches all addresses):')) + self.address_revert_button.setToolTip(translate('OpenLP.ServiceTab', 'Revert to default IP address.')) self.port_label.setText(translate('RemotePlugin.RemoteTab', 'Port number:')) self.remote_url_label.setText(translate('RemotePlugin.RemoteTab', 'Remote URL:')) self.stage_url_label.setText(translate('RemotePlugin.RemoteTab', 'Stage view URL:')) @@ -193,6 +225,13 @@ class ApiTab(SettingsTab): self.current_version_label.setText(translate('RemotePlugin.RemoteTab', 'Current version:')) self.master_version_label.setText(translate('RemotePlugin.RemoteTab', 'Latest version:')) self._unknown_version = translate('RemotePlugin.RemoteTab', '(unknown)') + self.server_state_group_box.setTitle(translate('RemotePlugin.RemoteTab', 'Server Status')) + self.server_http_state_title.setText(translate('RemotePlugin.RemoteTab', 'HTTP Server:')) + self.server_websocket_state_title.setText(translate('RemotePlugin.RemoteTab', 'Websocket Server:')) + self.server_zeroconf_state_title.setText(translate('RemotePlugin.RemoteTab', 'Zeroconf Server:')) + self._server_up = translate('RemotePlugin.RemoteTab', 'Active', 'Server is active') + self._server_down = translate('RemotePlugin.RemoteTab', 'Failed', 'Server failed') + self._server_disabled = translate('RemotePlugin.RemoteTab', 'Disabled', 'Server is disabled') @property def master_version(self): @@ -238,6 +277,31 @@ class ApiTab(SettingsTab): http_url_temp = http_url + 'main' self.live_url.setText('{url}'.format(url=http_url_temp)) + def get_server_states(self): + """ + Update the display with the current state of the servers + """ + if not is_thread_finished('http_server'): + self.server_http_state.setText(self._server_up) + elif Registry().get_flag('no_web_server'): + self.server_http_state.setText(self._server_disabled) + else: + self.server_http_state.setText(self._server_down) + + if not is_thread_finished('websocket_server'): + self.server_websocket_state.setText(self._server_up) + elif Registry().get_flag('no_web_server'): + self.server_websocket_state.setText(self._server_disabled) + else: + self.server_websocket_state.setText(self._server_down) + + if not is_thread_finished('api_zeroconf'): + self.server_zeroconf_state.setText(self._server_up) + elif Registry().get_flag('no_web_server'): + self.server_zeroconf_state.setText(self._server_disabled) + else: + self.server_zeroconf_state.setText(self._server_down) + def get_ip_address(self, ip_address): """ returns the IP address in dependency of the passed address @@ -268,12 +332,17 @@ class ApiTab(SettingsTab): self.current_version_value.setText(self.settings.value('api/download version')) self.set_master_version() self.set_urls() + self.get_server_states() def save(self): """ Save the configuration and update the server configuration if necessary """ if self.settings.value('api/ip address') != self.address_edit.text(): + QtWidgets.QMessageBox.information(self, translate('OpenLP.RemoteTab', 'Restart Required'), + translate('OpenLP.RemoteTab', + 'This change will only take effect once OpenLP ' + 'has been restarted.')) self.settings_form.register_post_process('remotes_config_updated') self.settings.setValue('api/ip address', self.address_edit.text()) self.settings.setValue('api/twelve hour', self.twelve_hour) @@ -282,6 +351,9 @@ class ApiTab(SettingsTab): self.settings.setValue('api/user id', self.user_id.text()) self.settings.setValue('api/password', self.password.text()) + def address_revert_button_clicked(self): + self.address_edit.setText(self.settings.get_default_value('api/ip address')) + def on_twelve_hour_check_box_changed(self, check_state): """ Toggle the 12 hour check box. diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py index da23b2029..8b2892351 100644 --- a/openlp/core/api/websockets.py +++ b/openlp/core/api/websockets.py @@ -130,9 +130,11 @@ class WebSocketWorker(ThreadWorker, RegistryProperties, LogMixin): log.error('Unable to start WebSocket server {addr}:{port}, giving up'.format(addr=address, port=port)) if self.server: # If the websocket server exists, start listening - self.event_loop.run_until_complete(self.server) try: + self.event_loop.run_until_complete(self.server) self.event_loop.run_forever() + except Exception: + log.exception('Failed to start WebSocket server') finally: self.event_loop.close() self.quit.emit() diff --git a/tests/functional/openlp_core/api/test_tab.py b/tests/openlp_core/api/test_tab.py similarity index 73% rename from tests/functional/openlp_core/api/test_tab.py rename to tests/openlp_core/api/test_tab.py index ee410ed8a..812c48548 100644 --- a/tests/functional/openlp_core/api/test_tab.py +++ b/tests/openlp_core/api/test_tab.py @@ -36,6 +36,7 @@ ZERO_URL = '0.0.0.0' @pytest.fixture def api_tab(settings): Registry().set_flag('website_version', '00-00-0000') + Registry().set_flag('no_web_server', False) parent = QtWidgets.QMainWindow() form = ApiTab(parent) yield form @@ -93,3 +94,36 @@ def test_set_urls(api_tab): assert api_tab.live_url.text() == \ "http://192.168.1.1:4316/main", \ 'The return value should be a fully formed main link' + + +def test_address_revert_button_clicked(api_tab, settings): + """ + Test the IP revert function works + """ + # GIVEN: The ip address text set to a non default value + api_tab.address_edit.setText('not the default') + # WHEN: address_revert_button_clicked is called + api_tab.address_revert_button_clicked() + # THEN: The text should have been changed to the default value + assert api_tab.address_edit.text() == settings.get_default_value('api/ip address') + + +def test_save(api_tab, settings): + """ + Test the IP revert function works + """ + # GIVEN: The ip address text set to a non default value + api_tab.address_edit.setText(settings.value('api/ip address')) + api_tab.twelve_hour = True + api_tab.thumbnails = True + api_tab.user_login_group_box.setChecked(True) + api_tab.user_id.setText('user id thing') + api_tab.password.setText('user password thing') + # WHEN: save is called + api_tab.save() + # THEN: The text should have been changed to the default value + assert settings.value('api/twelve hour') is True + assert settings.value('api/thumbnails') is True + assert settings.value('api/authentication enabled') is True + assert settings.value('api/user id') == 'user id thing' + assert settings.value('api/password') == 'user password thing' diff --git a/tests/functional/openlp_core/api/test_websockets.py b/tests/openlp_core/api/test_websockets.py similarity index 75% rename from tests/functional/openlp_core/api/test_websockets.py rename to tests/openlp_core/api/test_websockets.py index 86f43c2a3..012fd758f 100644 --- a/tests/functional/openlp_core/api/test_websockets.py +++ b/tests/openlp_core/api/test_websockets.py @@ -25,7 +25,7 @@ import pytest from unittest.mock import MagicMock, patch from openlp.core.api.websocketspoll import WebSocketPoller -from openlp.core.api.websockets import WebSocketServer +from openlp.core.api.websockets import WebSocketWorker, WebSocketServer from openlp.core.common.registry import Registry @@ -35,6 +35,12 @@ def poller(settings): yield poll +@pytest.fixture +def worker(settings): + worker = WebSocketWorker() + yield worker + + @patch('openlp.core.api.websockets.WebSocketWorker') @patch('openlp.core.api.websockets.run_thread') def test_serverstart(mocked_run_thread, MockWebSocketWorker, registry): @@ -120,3 +126,47 @@ def test_poller_get_state_if_changed(poller, settings): # THEN: The get_state_if_changed function should return None on the second call because the state has not changed assert poll_json is not None, 'The first get_state_if_changed function call should have not returned None' assert poll_json2 is None, 'The second get_state_if_changed function should return None' + + +@patch('openlp.core.api.websockets.serve') +@patch('openlp.core.api.websockets.asyncio') +@patch('openlp.core.api.websockets.log') +def test_worker_start(mocked_log, mocked_asyncio, mocked_serve, worker, settings): + """ + Test the start function of the worker + """ + # GIVEN: A mocked serve function and event loop + mocked_serve.return_value = 'server_thing' + event_loop = MagicMock() + mocked_asyncio.new_event_loop.return_value = event_loop + # WHEN: The start function is called + worker.start() + # THEN: No error occurs + mocked_serve.assert_called_once() + event_loop.run_until_complete.assert_called_once_with('server_thing') + event_loop.run_forever.assert_called_once_with() + mocked_log.exception.assert_not_called() + # Because run_forever is mocked, it doesn't stall the thread so close will be called immediately + event_loop.close.assert_called_once_with() + + +@patch('openlp.core.api.websockets.serve') +@patch('openlp.core.api.websockets.asyncio') +@patch('openlp.core.api.websockets.log') +def test_worker_start_fail(mocked_log, mocked_asyncio, mocked_serve, worker, settings): + """ + Test the start function of the worker handles a error nicely + """ + # GIVEN: A mocked serve function and event loop. run_until_complete returns a error + mocked_serve.return_value = 'server_thing' + event_loop = MagicMock() + mocked_asyncio.new_event_loop.return_value = event_loop + event_loop.run_until_complete.side_effect = Exception() + # WHEN: The start function is called + worker.start() + # THEN: An exception is logged but is handled and the event_loop is closed + mocked_serve.assert_called_once() + event_loop.run_until_complete.assert_called_once_with('server_thing') + event_loop.run_forever.assert_not_called() + mocked_log.exception.assert_called_once() + event_loop.close.assert_called_once_with()