diff --git a/distro-packages/ArchLinux/navipy-git/PKGBUILD b/distro-packages/ArchLinux/navipy-git/PKGBUILD index 77b2139..b9a3e71 100644 --- a/distro-packages/ArchLinux/navipy-git/PKGBUILD +++ b/distro-packages/ArchLinux/navipy-git/PKGBUILD @@ -1,6 +1,6 @@ # Maintainer: Storm Dragon pkgname=navipy-git -pkgver=r16.geb63cfe +pkgver=r17.g3ab834d pkgrel=1 pkgdesc='Accessible Subsonic desktop client tested with Navidrome' arch=('any') diff --git a/src/config/settings.py b/src/config/settings.py index 258fdc3..46957ba 100644 --- a/src/config/settings.py +++ b/src/config/settings.py @@ -133,11 +133,12 @@ class Settings: def getServers(self) -> Dict[str, Dict[str, str]]: """Get all server configurations""" - return self._servers + return {name: dict(server) for name, server in self._servers.items()} def getServer(self, name: str) -> Optional[Dict[str, str]]: """Get a specific server configuration""" - return self._servers.get(name) + server = self._servers.get(name) + return dict(server) if server else None def addServer(self, name: str, url: str, username: str, password: str) -> None: """Add or update a server configuration""" diff --git a/src/main_window.py b/src/main_window.py index f7bde46..521cbd1 100644 --- a/src/main_window.py +++ b/src/main_window.py @@ -479,16 +479,21 @@ class MainWindow(QMainWindow): if removedServerName == self.activeServerName: self._handle_active_server_removed() - if dialogAccepted: - serverName = dialog.selectedServerName - if serverName: - self.logger.info("Server selection completed, connecting to '%s'", serverName) - self.connectToServer(serverName) - elif self.activeServerName in dialog.editedServerNames: - self.logger.info("Reconnecting after editing active server '%s'", self.activeServerName) - self.connectToServer(self.activeServerName) + serverName = dialog.selectedServerName + if serverName and serverName not in dialog.removedServerNames: + self.logger.info("Server selection completed, connecting to '%s'", serverName) + self.connectToServer(serverName) + elif ( + self.activeServerName in dialog.editedServerNames + and self.activeServerName not in dialog.removedServerNames + ): + self.logger.info("Reconnecting after editing active server '%s'", self.activeServerName) + self.connectToServer(self.activeServerName) else: - self.logger.info("Server management dialog canceled") + if dialogAccepted: + self.logger.info("Server management dialog closed without selecting a server") + else: + self.logger.info("Server management dialog canceled") def _handle_active_server_removed(self): """Disconnect safely after the active server is deleted.""" diff --git a/src/widgets/server_dialog.py b/src/widgets/server_dialog.py index 9e6956c..85f9f35 100644 --- a/src/widgets/server_dialog.py +++ b/src/widgets/server_dialog.py @@ -2,19 +2,56 @@ Server connection dialog for adding/editing Navidrome servers """ +from urllib.parse import urlparse +from typing import Callable + +from PySide6.QtCore import QRunnable, QThreadPool, Signal from PySide6.QtWidgets import ( QDialog, QVBoxLayout, QHBoxLayout, QFormLayout, QLineEdit, QPushButton, QLabel, QMessageBox, QCheckBox ) -from PySide6.QtCore import Qt from src.api.client import SubsonicClient, SubsonicError from src.config.settings import Settings +class _ConnectionTestTask(QRunnable): + """Run a connection test without blocking the UI thread.""" + + def __init__(self, url: str, username: str, password: str, finished: Callable): + super().__init__() + self.url = url + self.username = username + self.password = password + self.finished = finished + + def run(self): + client = None + try: + client = SubsonicClient(self.url, self.username, self.password) + succeeded = bool(client.ping()) + if succeeded: + message = "Connection successful!" + else: + message = "Invalid response" + except SubsonicError as error: + succeeded = False + message = error.message + except Exception as error: + succeeded = False + message = str(error) + finally: + if client is not None: + client.session.close() + + self.finished.emit(succeeded, message) + + class ServerDialog(QDialog): """Dialog for configuring a Navidrome server connection""" + _testFinished = Signal(bool, str) + def __init__(self, settings: Settings, serverName: str = None, parent=None): """ Initialize the server dialog @@ -28,6 +65,8 @@ class ServerDialog(QDialog): self.settings = settings self.editingServer = serverName self.isNew = serverName is None + self._connectionTestRunning = False + self._testFinished.connect(self._handleConnectionTest) self.setWindowTitle("Add Server" if self.isNew else "Edit Server") self.setModal(True) @@ -135,25 +174,72 @@ class ServerDialog(QDialog): username = self.usernameEdit.text().strip() password = self.passwordEdit.text() - if not url or not username or not password: + if not url: self.statusLabel.setText("Please fill in all fields") + self.urlEdit.setFocus() + return + if not self._isValidServerUrl(url): + self.statusLabel.setText( + "Enter a valid server URL beginning with http:// or https://" + ) + self.urlEdit.setFocus() + return + if not username: + self.statusLabel.setText("Please enter a username") + self.usernameEdit.setFocus() + return + if not password: + self.statusLabel.setText("Please enter a password") + self.passwordEdit.setFocus() + return + if self._connectionTestRunning: return self.statusLabel.setText("Testing connection...") self.testButton.setEnabled(False) + self._connectionTestRunning = True + QThreadPool.globalInstance().start( + _ConnectionTestTask(url, username, password, self._testFinished) + ) + + def _handleConnectionTest(self, succeeded: bool, message: str): + """Show the result after a background connection test finishes.""" + self._connectionTestRunning = False + self.testButton.setEnabled(True) + if succeeded: + self.statusLabel.setText(message) + else: + self.statusLabel.setText(f"Connection failed: {message}") + + @staticmethod + def _isValidServerUrl(url: str) -> bool: + """Require an HTTP or HTTPS URL with a host name.""" try: - client = SubsonicClient(url, username, password) - if client.ping(): - self.statusLabel.setText("Connection successful!") - else: - self.statusLabel.setText("Connection failed: Invalid response") - except SubsonicError as e: - self.statusLabel.setText(f"Connection failed: {e.message}") - except Exception as e: - self.statusLabel.setText(f"Connection failed: {str(e)}") - finally: - self.testButton.setEnabled(True) + parsedUrl = urlparse(url) + except ValueError: + return False + return ( + parsedUrl.scheme in ("http", "https") + and bool(parsedUrl.hostname) + ) + + def accept(self): + """Disconnect test updates before the dialog closes.""" + self._disconnectConnectionTest() + super().accept() + + def reject(self): + """Disconnect test updates before the dialog closes.""" + self._disconnectConnectionTest() + super().reject() + + def _disconnectConnectionTest(self): + """Stop queued test updates from reaching a closing dialog.""" + try: + self._testFinished.disconnect(self._handleConnectionTest) + except RuntimeError: + pass def saveServer(self): """Save the server configuration""" @@ -172,6 +258,14 @@ class ServerDialog(QDialog): QMessageBox.warning(self, "Validation Error", "Please enter the server URL") self.urlEdit.setFocus() return + if not self._isValidServerUrl(url): + QMessageBox.warning( + self, + "Validation Error", + "Please enter a valid server URL beginning with http:// or https:// and including a host name" + ) + self.urlEdit.setFocus() + return if not username: QMessageBox.warning(self, "Validation Error", "Please enter a username") diff --git a/src/widgets/server_management_dialog.py b/src/widgets/server_management_dialog.py index 30c3561..46697a8 100644 --- a/src/widgets/server_management_dialog.py +++ b/src/widgets/server_management_dialog.py @@ -166,6 +166,7 @@ class ServerManagementDialog(QDialog): self.settings.removeServer(serverName) self.removedServerNames.add(serverName) + self.editedServerNames.discard(serverName) self.selectedServerName = None self.refreshServers() diff --git a/tests/test_server_management.py b/tests/test_server_management.py index 176dbf5..e859c0a 100644 --- a/tests/test_server_management.py +++ b/tests/test_server_management.py @@ -15,6 +15,7 @@ from src.config.settings import Settings from src.widgets.server_dialog import ServerDialog from src.widgets.server_management_dialog import ServerManagementDialog import src.widgets.server_management_dialog as server_management_dialog_module +import src.widgets.server_dialog as server_dialog_module @pytest.fixture(autouse=True) @@ -70,6 +71,41 @@ def test_edit_server_can_clear_default(qt_app): assert settings.getDefaultServer() is None +def test_server_dialog_rejects_invalid_url(qt_app, monkeypatch): + settings = Settings() + dialog = ServerDialog(settings) + monkeypatch.setattr(QMessageBox, "warning", lambda *args, **kwargs: None) + + dialog.nameEdit.setText("Primary") + dialog.urlEdit.setText("music.example.com") + dialog.usernameEdit.setText("Username") + dialog.passwordEdit.setText("password") + dialog.saveServer() + + assert dialog.result() != QDialog.Accepted + assert settings.getServers() == {} + assert dialog.focusWidget() is dialog.urlEdit + + +def test_server_dialog_test_rejects_invalid_url_without_network(qt_app, monkeypatch): + settings = Settings() + dialog = ServerDialog(settings) + + def unexpected_client(*args, **kwargs): + raise AssertionError("Connection test should validate before creating a client") + + monkeypatch.setattr(server_dialog_module, "SubsonicClient", unexpected_client) + dialog.urlEdit.setText("ftp://music.example.com") + dialog.usernameEdit.setText("Username") + dialog.passwordEdit.setText("password") + + dialog.testConnection() + + assert dialog.testButton.isEnabled() + assert dialog.statusLabel.text() == "Enter a valid server URL beginning with http:// or https://" + assert dialog.focusWidget() is dialog.urlEdit + + def test_management_dialog_lists_default_and_selection_actions(qt_app): settings = make_settings() dialog = ServerManagementDialog(settings) @@ -106,6 +142,18 @@ def test_management_dialog_remove_confirmed_server(qt_app, monkeypatch): assert dialog.serverList.currentItem().data(Qt.UserRole) == "Backup" +def test_management_dialog_remove_discards_edit_state(qt_app, monkeypatch): + settings = make_settings() + dialog = ServerManagementDialog(settings) + dialog.editedServerNames.add("Primary") + monkeypatch.setattr(QMessageBox, "question", lambda *args, **kwargs: QMessageBox.Yes) + + dialog.removeSelected() + + assert dialog.removedServerNames == {"Primary"} + assert dialog.editedServerNames == set() + + def test_management_dialog_remove_canceled_server(qt_app, monkeypatch): settings = make_settings() dialog = ServerManagementDialog(settings)