From 0e5c7d2d137ce489173530068a040c5862d794a3 Mon Sep 17 00:00:00 2001
From: DavHau <d.hauer.it@gmail.com>
Date: Wed, 25 Oct 2023 17:48:30 +0100
Subject: [PATCH] api/machines: split off config validation into separate
 endpoint

- This speeds up PUT /machines{name}/config as it doesn't do the expensive check anymore
- instead use PUT /machines/{name}/verify which allows a dry-run evaluation of a config which is passed without writing it to disk
---
 pkgs/clan-cli/clan_cli/config/machine.py      |  6 +----
 .../clan_cli/webui/routers/machines.py        | 18 +++++++++----
 pkgs/clan-cli/tests/test_machines_api.py      | 26 +++++++++++++------
 3 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/pkgs/clan-cli/clan_cli/config/machine.py b/pkgs/clan-cli/clan_cli/config/machine.py
index e15a5f29a..f2dc9d7ca 100644
--- a/pkgs/clan-cli/clan_cli/config/machine.py
+++ b/pkgs/clan-cli/clan_cli/config/machine.py
@@ -64,16 +64,13 @@ def config_for_machine(machine_name: str) -> dict:
         return json.load(f)
 
 
-def set_config_for_machine(machine_name: str, config: dict) -> Optional[str]:
+def set_config_for_machine(machine_name: str, config: dict) -> None:
     # write the config to a json file located at {flake}/machines/{machine_name}/settings.json
     if not machine_folder(machine_name).exists():
         raise HTTPException(
             status_code=404,
             detail=f"Machine {machine_name} not found. Create the machine first`",
         )
-    error = verify_machine_config(machine_name, config)
-    if error is not None:
-        return error
     settings_path = machine_settings_file(machine_name)
     settings_path.parent.mkdir(parents=True, exist_ok=True)
     with open(settings_path, "w") as f:
@@ -82,7 +79,6 @@ def set_config_for_machine(machine_name: str, config: dict) -> Optional[str]:
 
     if repo_dir is not None:
         commit_file(settings_path, repo_dir)
-    return None
 
 
 def schema_for_machine(
diff --git a/pkgs/clan-cli/clan_cli/webui/routers/machines.py b/pkgs/clan-cli/clan_cli/webui/routers/machines.py
index 2cfca86ee..c510a49fa 100644
--- a/pkgs/clan-cli/clan_cli/webui/routers/machines.py
+++ b/pkgs/clan-cli/clan_cli/webui/routers/machines.py
@@ -2,7 +2,7 @@
 import logging
 from typing import Annotated
 
-from fastapi import APIRouter, Body, HTTPException
+from fastapi import APIRouter, Body
 
 from ...config.machine import (
     config_for_machine,
@@ -57,9 +57,7 @@ async def get_machine_config(name: str) -> ConfigResponse:
 async def set_machine_config(
     name: str, config: Annotated[dict, Body()]
 ) -> ConfigResponse:
-    error = set_config_for_machine(name, config)
-    if error is not None:
-        raise HTTPException(status_code=400, detail=error)
+    set_config_for_machine(name, config)
     return ConfigResponse(config=config)
 
 
@@ -78,7 +76,17 @@ async def set_machine_schema(
 
 
 @router.get("/api/machines/{name}/verify")
-async def put_verify_machine_config(name: str) -> VerifyMachineResponse:
+async def get_verify_machine_config(name: str) -> VerifyMachineResponse:
     error = verify_machine_config(name)
     success = error is None
     return VerifyMachineResponse(success=success, error=error)
+
+
+@router.put("/api/machines/{name}/verify")
+async def put_verify_machine_config(
+    name: str,
+    config: Annotated[dict, Body()],
+) -> VerifyMachineResponse:
+    error = verify_machine_config(name, config)
+    success = error is None
+    return VerifyMachineResponse(success=success, error=error)
diff --git a/pkgs/clan-cli/tests/test_machines_api.py b/pkgs/clan-cli/tests/test_machines_api.py
index a9fcb6cc3..dd03970ed 100644
--- a/pkgs/clan-cli/tests/test_machines_api.py
+++ b/pkgs/clan-cli/tests/test_machines_api.py
@@ -45,29 +45,39 @@ def test_configure_machine(api: TestClient, test_flake: Path) -> None:
     json_response = response.json()
     assert "schema" in json_response and "properties" in json_response["schema"]
 
-    # set come invalid config (fileSystems missing)
-    config = dict(
+    # an invalid config missing the fileSystems
+    invalid_config = dict(
         clan=dict(
             jitsi=dict(
                 enable=True,
             ),
         ),
     )
+
+    # verify an invalid config (fileSystems missing) fails
     response = api.put(
-        "/api/machines/machine1/config",
-        json=config,
+        "/api/machines/machine1/verify",
+        json=invalid_config,
     )
-    assert response.status_code == 400
+    assert response.status_code == 200
     assert (
         "The ‘fileSystems’ option does not specify your root"
-        in response.json()["detail"]
+        in response.json()["error"]
     )
 
-    # ensure config is still empty after the invalid attempt
+    # set come invalid config (fileSystems missing)
+    response = api.put(
+        "/api/machines/machine1/config",
+        json=invalid_config,
+    )
+    assert response.status_code == 200
+
+    # ensure the config has actually been updated
     response = api.get("/api/machines/machine1/config")
     assert response.status_code == 200
-    assert response.json() == {"config": {}}
+    assert response.json() == {"config": invalid_config}
 
+    # the part of the config that makes the evaluation pass
     fs_config = dict(
         fileSystems={
             "/": dict(