Fix websocket crash

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('<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.

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

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() == \
         "<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'

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