mirror of
https://github.com/rstrouse/ESPSomfy-RTS.git
synced 2026-10-31 13:52:15 +01:00
fix: restore reads the options the web UI ticks
The web UI sends the restore options as the form field "data". The param helpers looked up query parameters only, so /restore never saw them and restored the shades alone, since 3.0.0. The helpers now take a form field too. /restore uses an upload once, and drops the file when the options aren't JSON. e2e: test_restore.py, a probe that the field is read and, with --reboot, a settings-only round trip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
0ee52d6d9f
commit
3d45ba59c1
8 changed files with 97 additions and 18 deletions
|
|
@ -93,7 +93,11 @@ void registerHttpRoutes(AsyncWebServer &server) {
|
||||||
|
|
||||||
server.on(AsyncURIMatcher::exact("/restore"), HTTP_POST,
|
server.on(AsyncURIMatcher::exact("/restore"), HTTP_POST,
|
||||||
[](AsyncWebServerRequest *request) {
|
[](AsyncWebServerRequest *request) {
|
||||||
if(webServer.uploadSuccess) {
|
// An upload is restored once: a later /restore without a file must not
|
||||||
|
// find this one.
|
||||||
|
const bool uploaded = webServer.uploadSuccess;
|
||||||
|
webServer.uploadSuccess = false;
|
||||||
|
if(uploaded) {
|
||||||
restore_options_t opts;
|
restore_options_t opts;
|
||||||
if(asyncHasParam(request, "data")) {
|
if(asyncHasParam(request, "data")) {
|
||||||
String dataStr = asyncParam(request, "data");
|
String dataStr = asyncParam(request, "data");
|
||||||
|
|
@ -101,6 +105,7 @@ void registerHttpRoutes(AsyncWebServer &server) {
|
||||||
JsonDocument doc;
|
JsonDocument doc;
|
||||||
DeserializationError err = deserializeJson(doc, dataStr);
|
DeserializationError err = deserializeJson(doc, dataStr);
|
||||||
if(err) {
|
if(err) {
|
||||||
|
LittleFS.remove("/shades.tmp");
|
||||||
request->send(500, _encoding_json, "{\"status\":\"ERROR\",\"desc\":\"JSON parse error\"}");
|
request->send(500, _encoding_json, "{\"status\":\"ERROR\",\"desc\":\"JSON parse error\"}");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -143,13 +143,18 @@ bool Web::createAPIToken(const IPAddress ipAddress, char *token) {
|
||||||
// =====================================================
|
// =====================================================
|
||||||
// Async API Handlers
|
// Async API Handlers
|
||||||
// =====================================================
|
// =====================================================
|
||||||
// Helper: get a query param as String, or empty if missing
|
// The library keeps query parameters and form fields apart, and looks up only
|
||||||
|
// the first unless asked. /restore's options come as a form field.
|
||||||
|
static const AsyncWebParameter *queryOrFormParam(AsyncWebServerRequest *request, const char *name) {
|
||||||
|
const AsyncWebParameter *p = request->getParam(name);
|
||||||
|
return p ? p : request->getParam(name, true);
|
||||||
|
}
|
||||||
String asyncParam(AsyncWebServerRequest *request, const char *name) {
|
String asyncParam(AsyncWebServerRequest *request, const char *name) {
|
||||||
if(request->hasParam(name)) return request->getParam(name)->value();
|
const AsyncWebParameter *p = queryOrFormParam(request, name);
|
||||||
return String();
|
return p ? p->value() : String();
|
||||||
}
|
}
|
||||||
bool asyncHasParam(AsyncWebServerRequest *request, const char *name) {
|
bool asyncHasParam(AsyncWebServerRequest *request, const char *name) {
|
||||||
return request->hasParam(name);
|
return queryOrFormParam(request, name) != nullptr;
|
||||||
}
|
}
|
||||||
|
|
||||||
// -- Serialization helpers --
|
// -- Serialization helpers --
|
||||||
|
|
|
||||||
|
|
@ -18,7 +18,7 @@ extern const char _encoding_json[];
|
||||||
// Hands an item to loop(), which owns the shades. When it can't be the
|
// Hands an item to loop(), which owns the shades. When it can't be the
|
||||||
// request is answered with the reason and false is returned.
|
// request is answered with the reason and false is returned.
|
||||||
bool queueLoopItem(AsyncWebServerRequest *request, const loop_item_t &item);
|
bool queueLoopItem(AsyncWebServerRequest *request, const loop_item_t &item);
|
||||||
// A query parameter, or an empty string when it is missing.
|
// A query parameter or form field, or an empty string when it is missing.
|
||||||
String asyncParam(AsyncWebServerRequest *request, const char *name);
|
String asyncParam(AsyncWebServerRequest *request, const char *name);
|
||||||
bool asyncHasParam(AsyncWebServerRequest *request, const char *name);
|
bool asyncHasParam(AsyncWebServerRequest *request, const char *name);
|
||||||
#endif
|
#endif
|
||||||
|
|
|
||||||
|
|
@ -12,11 +12,11 @@ Runs against a real device on your network, by hand, never in CI. It checks sock
|
||||||
| `--password`, `--username` / `--pin` | the credentials of a Secured device |
|
| `--password`, `--username` / `--pin` | the credentials of a Secured device |
|
||||||
| `--radio --shade <id>` | also send real commands: that shade moves a little and back |
|
| `--radio --shade <id>` | also send real commands: that shade moves a little and back |
|
||||||
| `--edit` | also add a room and delete it again |
|
| `--edit` | also add a room and delete it again |
|
||||||
| `--reboot` | also reboot the device and wait for it to come back |
|
| `--reboot` | also reboot the device and wait for it to come back, and restore the settings from a backup taken a moment before (it changes the NTP server, which the restore puts back) |
|
||||||
| `--lockout` | on a Secured device, also fail `/login` until this PC is refused, then wait out the 30 s lockout |
|
| `--lockout` | on a Secured device, also fail `/login` until this PC is refused, then wait out the 30 s lockout |
|
||||||
| `--record` | save hello, the sync and the schema to `test/fixtures/proto2/`, for the web UI and Home Assistant tests |
|
| `--record` | save hello, the sync and the schema to `test/fixtures/proto2/`, for the web UI and Home Assistant tests |
|
||||||
|
|
||||||
A plain run sends nothing on air, changes no setting (it saves only values the device already has) and doesn't reboot the device. The command tests send ops that are refused before they reach the radio, and one `setPositions` that sets a shade to the position the sync gave: that drops the fraction of a percent the shade's believed position may have had. The edit tests send edits that are refused, and one shade sort order set to the order it has, which saves the shades again unchanged. The settings tests send saves that are refused, and one that saves the NTP server and time zone the device already has. The frequency scan test pauses the radio's sending for a second or two, as a scan does. It does fill every socket slot for a few seconds, and replays a page load.
|
A plain run sends nothing on air, changes no setting (it saves only values the device already has) and doesn't reboot the device. The command tests send ops that are refused before they reach the radio, and one `setPositions` that sets a shade to the position the sync gave: that drops the fraction of a percent the shade's believed position may have had. The edit tests send edits that are refused, and one shade sort order set to the order it has, which saves the shades again unchanged. The settings tests send saves that are refused, and one that saves the NTP server and time zone the device already has. The restore test sends options that aren't JSON with a file that isn't a backup, so nothing is restored. The frequency scan test pauses the radio's sending for a second or two, as a scan does. It does fill every socket slot for a few seconds, and replays a page load.
|
||||||
|
|
||||||
The web UI holds one of the device's 5 socket slots for each open tab, and Home Assistant holds one. The tests that fill every slot fill the ones left, so close the UI's tabs for the full load.
|
The web UI holds one of the device's 5 socket slots for each open tab, and Home Assistant holds one. The tests that fill every slot fill the ones left, so close the UI's tabs for the full load.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -18,7 +18,7 @@ def pytest_addoption(parser):
|
||||||
parser.addoption("--radio", action="store_true", help="also run the tests that send commands on air")
|
parser.addoption("--radio", action="store_true", help="also run the tests that send commands on air")
|
||||||
parser.addoption("--shade", type=int, default=None, help="with --radio: the id of the shade the commands may move")
|
parser.addoption("--shade", type=int, default=None, help="with --radio: the id of the shade the commands may move")
|
||||||
parser.addoption("--edit", action="store_true", help="also run the edits that change the shade model, then undo them")
|
parser.addoption("--edit", action="store_true", help="also run the edits that change the shade model, then undo them")
|
||||||
parser.addoption("--reboot", action="store_true", help="also run the test that reboots the device")
|
parser.addoption("--reboot", action="store_true", help="also run the tests that reboot the device")
|
||||||
parser.addoption("--lockout", action="store_true", help="also run the test that locks this PC out of /login for 30 s")
|
parser.addoption("--lockout", action="store_true", help="also run the test that locks this PC out of /login for 30 s")
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
59
test/e2e/test_restore.py
Normal file
59
test/e2e/test_restore.py
Normal file
|
|
@ -0,0 +1,59 @@
|
||||||
|
"""/restore and the options the web UI sends with the backup file.
|
||||||
|
|
||||||
|
The round trip restores a backup and reboots the device: pass --reboot.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from proto_client import run_sync
|
||||||
|
from test_commands import is_ok, op, synced_model
|
||||||
|
from test_routes import http, upload
|
||||||
|
from test_settings import desc, settings, uptime, wait_for_restart
|
||||||
|
|
||||||
|
# Saved for the round trip and put back by the restore. .invalid never resolves.
|
||||||
|
CHANGED_NTP_SERVER = "restore.e2e.invalid"
|
||||||
|
|
||||||
|
|
||||||
|
def auth(token):
|
||||||
|
return {"apikey": token} if token else {}
|
||||||
|
|
||||||
|
|
||||||
|
def test_restore_reads_the_options_sent_as_a_form_field(host, token):
|
||||||
|
"""The options come as the form field "data", next to the file.
|
||||||
|
|
||||||
|
They are read before the file is checked, so options that aren't JSON
|
||||||
|
are refused, and this file, which isn't a backup, restores nothing
|
||||||
|
either way. /restore once looked up only query parameters, missed the
|
||||||
|
field and restored the shades alone, whatever was ticked.
|
||||||
|
"""
|
||||||
|
status, _, body = upload(host, "/restore", auth(token), fields={"data": "{"})
|
||||||
|
assert status == 500, body
|
||||||
|
assert json.loads(body)["desc"] == "JSON parse error"
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_restore_brings_back_the_parts_that_were_ticked(request, host, token, connect, schema):
|
||||||
|
if not request.config.getoption("--reboot"):
|
||||||
|
pytest.skip("restores a backup and reboots the device: pass --reboot")
|
||||||
|
c = connect(schema=schema.hash)
|
||||||
|
_, frames, _ = run_sync(c)
|
||||||
|
uptime_before = uptime(schema, frames)
|
||||||
|
general = settings(c, schema, "general")
|
||||||
|
ntp = {name: general.get(name, schema.default(name)) for name in ("ntpServer", "posixZone")}
|
||||||
|
assert ntp["ntpServer"] != CHANGED_NTP_SERVER
|
||||||
|
status, _, backup = http(host, "/backup", headers=auth(token))
|
||||||
|
assert status == 200
|
||||||
|
|
||||||
|
reply = op(c, schema, "saveSettings", dict(ntp, ntpServer=CHANGED_NTP_SERVER, section="general"))
|
||||||
|
assert is_ok(schema, reply), desc(schema, reply)
|
||||||
|
try:
|
||||||
|
options = json.dumps({"shades": False, "settings": True})
|
||||||
|
status, _, body = upload(host, "/restore", auth(token), content=backup, fields={"data": options})
|
||||||
|
assert status == 200, body
|
||||||
|
wait_for_restart(host, token, schema, uptime_before)
|
||||||
|
finally:
|
||||||
|
c, _, _ = synced_model(connect, schema)
|
||||||
|
restored = settings(c, schema, "general").get("ntpServer", schema.default("ntpServer"))
|
||||||
|
if restored != ntp["ntpServer"]:
|
||||||
|
op(c, schema, "saveSettings", dict(ntp, section="general"))
|
||||||
|
assert restored == ntp["ntpServer"]
|
||||||
|
|
@ -38,16 +38,21 @@ def http(host, path, method="GET", body=None, headers=None):
|
||||||
return e.code, e.headers, e.read()
|
return e.code, e.headers, e.read()
|
||||||
|
|
||||||
|
|
||||||
def upload(host, path, headers=None, with_file=True):
|
def upload(host, path, headers=None, content=b"\0" * 256, fields=None):
|
||||||
"""Posts a small file that isn't a backup or an image, or a form without a file."""
|
"""Posts a form as the web UI does: the file, then the fields.
|
||||||
|
|
||||||
|
The file is by default a small one that isn't a backup or an image, and
|
||||||
|
content=None leaves it out.
|
||||||
|
"""
|
||||||
boundary = "e2e-upload"
|
boundary = "e2e-upload"
|
||||||
if with_file:
|
parts = []
|
||||||
part = (
|
if content is not None:
|
||||||
|
parts.append(
|
||||||
f"--{boundary}\r\nContent-Disposition: form-data; name=\"file\"; filename=\"e2e.bin\"\r\n"
|
f"--{boundary}\r\nContent-Disposition: form-data; name=\"file\"; filename=\"e2e.bin\"\r\n"
|
||||||
"Content-Type: application/octet-stream\r\n\r\n").encode() + b"\0" * 256
|
"Content-Type: application/octet-stream\r\n\r\n".encode() + content)
|
||||||
else:
|
for name, value in (fields or {}).items():
|
||||||
part = f"--{boundary}\r\nContent-Disposition: form-data; name=\"e2e\"\r\n\r\n1".encode()
|
parts.append(f"--{boundary}\r\nContent-Disposition: form-data; name=\"{name}\"\r\n\r\n{value}".encode())
|
||||||
body = part + f"\r\n--{boundary}--\r\n".encode()
|
body = b"\r\n".join(parts) + f"\r\n--{boundary}--\r\n".encode()
|
||||||
return http(host, path, "POST", body, {"Content-Type": f"multipart/form-data; boundary={boundary}", **(headers or {})})
|
return http(host, path, "POST", body, {"Content-Type": f"multipart/form-data; boundary={boundary}", **(headers or {})})
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -104,7 +109,7 @@ def test_secured_refuses_an_upload_without_a_token(host, secured, path):
|
||||||
|
|
||||||
|
|
||||||
def restore_without_a_file(host, token):
|
def restore_without_a_file(host, token):
|
||||||
status, _, body = upload(host, "/restore", {"apikey": token}, with_file=False)
|
status, _, body = upload(host, "/restore", {"apikey": token}, content=None, fields={"e2e": "1"})
|
||||||
return status, json.loads(body or b"{}").get("desc", "")
|
return status, json.loads(body or b"{}").get("desc", "")
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -123,6 +123,11 @@ def test_reboot(request, host, token, connect, schema):
|
||||||
uptime_before = uptime(schema, frames)
|
uptime_before = uptime(schema, frames)
|
||||||
assert is_ok(schema, op(c, schema, "reboot", {}))
|
assert is_ok(schema, op(c, schema, "reboot", {}))
|
||||||
assert c.closed.wait(15), "the socket stayed open"
|
assert c.closed.wait(15), "the socket stayed open"
|
||||||
|
wait_for_restart(host, token, schema, uptime_before)
|
||||||
|
|
||||||
|
|
||||||
|
def wait_for_restart(host, token, schema, uptime_before):
|
||||||
|
"""Waits until the device is back, and checks it restarted."""
|
||||||
# The device closes its sockets just before it restarts, and may still
|
# The device closes its sockets just before it restarts, and may still
|
||||||
# take a connection in between: that one never gets its hello.
|
# take a connection in between: that one never gets its hello.
|
||||||
time.sleep(3)
|
time.sleep(3)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue