Improve server management.

distro-packages/ArchLinux/navipy-git/PKGBUILD

@@ -1,6 +1,6 @@
 # Maintainer: Storm Dragon <storm_dragon@stormux.org>
 pkgname=navipy-git
-pkgver=r16.geb63cfe
+pkgver=r17.g3ab834d
 pkgrel=1
 pkgdesc='Accessible Subsonic desktop client tested with Navidrome'
 arch=('any')

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"""

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."""

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

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

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)