Merge branch 'fix_websocket_crash' into 'master'

Fix websocket crash

Closes #810

See merge request openlp/openlp!315
This commit is contained in:
Tim Bentley 2021-04-25 06:18:16 +00:00
commit 8381adfc36
4 changed files with 162 additions and 4 deletions

View File

@ -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('<a href="{url}">{url}</a>'.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.

View File

@ -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()

View File

@ -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() == \
"<a href=\"http://192.168.1.1:4316/main\">http://192.168.1.1:4316/main</a>", \
'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'

View File

@ -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()