♻️ Route settings commands through dedicated settings handler

jukebox/admin/app.py

@@ -38,7 +38,6 @@
     SonosSelectCommand,
     SonosShowCommand,
     UiCommand,
-    is_settings_command,
     is_sonos_command,
 )
 from .di_container import (
@@ -72,6 +71,33 @@ def _get_state(ctx: typer.Context) -> AdminCliState:
     return state
 
 
+def _run_settings_command(ctx: typer.Context, command: object) -> None:
+    state = _get_state(ctx)
+
+    try:
+        settings_service = build_settings_service(
+            library=state.library,
+            command=command,
+        )
+        execute_settings_command(
+            command=command,
+            settings_service=settings_service,
+        )
+    except typer.Exit:
+        raise
+    except SettingsError as err:
+        typer.echo(render_cli_error(err, verbose=state.verbose), err=True)
+        raise typer.Exit(code=1)
+    except OSError as err:
+        typer.echo(str(err), err=True)
+        raise typer.Exit(code=1)
+    except Exception as err:
+        typer.echo(render_cli_error(err, verbose=state.verbose), err=True)
+        if state.verbose:
+            traceback.print_exception(type(err), err, err.__traceback__)
+        raise typer.Exit(code=1)
+
+
 def _run_command(ctx: typer.Context, command: object) -> None:
     state = _get_state(ctx)
 
@@ -81,12 +107,7 @@ def _run_command(ctx: typer.Context, command: object) -> None:
             command=command,
         )
         try:
-            if is_settings_command(command):
-                execute_settings_command(
-                    command=command,
-                    settings_service=services.settings,
-                )
-            elif is_sonos_command(command):
+            if is_sonos_command(command):
                 execute_sonos_command(
                     command=command,
                     sonos_service=services.sonos,
@@ -318,7 +339,7 @@ def settings_show(
         typer.Option("--json", help="print the raw machine-readable payload"),
     ] = False,
 ) -> None:
-    _run_command(ctx, SettingsShowCommand(type="settings_show", effective=effective, json_output=json_output))
+    _run_settings_command(ctx, SettingsShowCommand(type="settings_show", effective=effective, json_output=json_output))
 
 
 @settings_app.command("set")
@@ -331,7 +352,7 @@ def settings_set(
         typer.Option("--json", help="print the raw machine-readable payload"),
     ] = False,
 ) -> None:
-    _run_command(
+    _run_settings_command(
         ctx,
         SettingsSetCommand(
             type="settings_set",
@@ -351,7 +372,7 @@ def settings_reset(
         typer.Option("--json", help="print the raw machine-readable payload"),
     ] = False,
 ) -> None:
-    _run_command(
+    _run_settings_command(
         ctx,
         SettingsResetCommand(
             type="settings_reset",

jukebox/admin/commands.py

@@ -81,16 +81,5 @@ def is_admin_command(command: object) -> bool:
     )
 
 
-def is_settings_command(command: object) -> bool:
-    return isinstance(
-        command,
-        (
-            SettingsResetCommand,
-            SettingsSetCommand,
-            SettingsShowCommand,
-        ),
-    )
-
-
 def is_sonos_command(command: object) -> bool:
     return isinstance(command, (SonosListCommand, SonosSelectCommand, SonosShowCommand))

tests/jukebox/admin/test_app.py

@@ -55,27 +55,6 @@ class Mocks:
 @pytest.mark.parametrize(
     ("args", "expected_command", "executor_name"),
     [
-        (["settings", "show"], SettingsShowCommand(type="settings_show", effective=False), "execute_settings_command"),
-        (
-            ["settings", "show", "--effective"],
-            SettingsShowCommand(type="settings_show", effective=True),
-            "execute_settings_command",
-        ),
-        (
-            ["settings", "show", "--json"],
-            SettingsShowCommand(type="settings_show", effective=False, json_output=True),
-            "execute_settings_command",
-        ),
-        (
-            ["settings", "set", "admin.api.port", "9000"],
-            SettingsSetCommand(type="settings_set", dotted_path="admin.api.port", value="9000"),
-            "execute_settings_command",
-        ),
-        (
-            ["settings", "reset", "admin.ui.port", "--json"],
-            SettingsResetCommand(type="settings_reset", dotted_path="admin.ui.port", json_output=True),
-            "execute_settings_command",
-        ),
         (["sonos", "list"], SonosListCommand(type="sonos_list"), "execute_sonos_command"),
         (
             ["sonos", "select", "--uids", "speaker-1,speaker-2", "--coordinator", "speaker-2"],
@@ -119,14 +98,7 @@ def test_jukebox_admin_routes_admin_commands_by_category(app_mocks, args, expect
     executor = getattr(app_mocks, executor_name)
     assert executor.call_count == 1
 
-    if executor_name == "execute_settings_command":
-        executor.assert_called_once_with(
-            command=expected_command,
-            settings_service=services.settings,
-        )
-        app_mocks.execute_sonos_command.assert_not_called()
-        app_mocks.execute_server_command.assert_not_called()
-    elif executor_name == "execute_sonos_command":
+    if executor_name == "execute_sonos_command":
         executor.assert_called_once_with(
             command=expected_command,
             sonos_service=services.sonos,
@@ -163,6 +135,49 @@ def test_jukebox_admin_routes_admin_commands_by_category(app_mocks, args, expect
         app_mocks.execute_sonos_command.assert_not_called()
 
 
+@pytest.mark.parametrize(
+    ("args", "expected_command"),
+    [
+        (["settings", "show"], SettingsShowCommand(type="settings_show", effective=False)),
+        (
+            ["settings", "show", "--effective"],
+            SettingsShowCommand(type="settings_show", effective=True),
+        ),
+        (
+            ["settings", "show", "--json"],
+            SettingsShowCommand(type="settings_show", effective=False, json_output=True),
+        ),
+        (
+            ["settings", "set", "admin.api.port", "9000"],
+            SettingsSetCommand(type="settings_set", dotted_path="admin.api.port", value="9000"),
+        ),
+        (
+            ["settings", "reset", "admin.ui.port", "--json"],
+            SettingsResetCommand(type="settings_reset", dotted_path="admin.ui.port", json_output=True),
+        ),
+    ],
+)
+def test_jukebox_admin_routes_settings_commands_to_settings_handler(app_mocks, args, expected_command):
+    settings_service = MagicMock()
+    app_mocks.build_settings_service.return_value = settings_service
+
+    result = runner.invoke(app, ["--library", "/custom/library.json", "--verbose", *args])
+
+    assert result.exit_code == 0
+    app_mocks.set_logger.assert_called_once_with("jukebox-admin", True)
+    app_mocks.build_settings_service.assert_called_once_with(
+        library="/custom/library.json",
+        command=expected_command,
+    )
+    app_mocks.execute_settings_command.assert_called_once_with(
+        command=expected_command,
+        settings_service=settings_service,
+    )
+    app_mocks.build_admin_services.assert_not_called()
+    app_mocks.execute_sonos_command.assert_not_called()
+    app_mocks.execute_server_command.assert_not_called()
+
+
 def test_jukebox_admin_version_flag(app_mocks, mocker):
     mocker.patch("jukebox.admin.app.get_package_version", return_value="1.2.3")
 
@@ -176,7 +191,7 @@ def test_jukebox_admin_version_flag(app_mocks, mocker):
 
 
 def test_jukebox_admin_renders_friendly_settings_errors(app_mocks):
-    app_mocks.build_admin_services.side_effect = ValueError("boom")
+    app_mocks.build_settings_service.side_effect = ValueError("boom")
 
     result = runner.invoke(app, ["settings", "show"])