mirror of
https://github.com/rommapp/romm.git
synced 2026-10-12 02:21:58 +00:00
fix(devices): count a device that polls claim as online (#5190)
A client that drains installs by polling claim (Argosy) never holds the /devices socket, so the install dialog showed it offline while awake. Each claim now sets a presence key that lapses after the same 90 seconds as a socket's. Also add TZ to env.template, since save version names and log timestamps follow the container's time zone. Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
e722e96d91
commit
1060afc1af
@@ -21,6 +21,7 @@ from endpoints.sockets.devices import (
|
||||
emit_install_cancelled,
|
||||
emit_install_queued,
|
||||
emit_install_updated,
|
||||
mark_claimed,
|
||||
online_device_ids,
|
||||
)
|
||||
from exceptions.endpoint_exceptions import (
|
||||
@@ -149,7 +150,7 @@ async def _notify_outcome(install: InstallRequestSchema, device: Device) -> None
|
||||
responses={status.HTTP_404_NOT_FOUND: {}},
|
||||
)
|
||||
async def get_online_devices(request: Request) -> list[str]:
|
||||
"""Ids of the caller's devices holding an open device socket."""
|
||||
"""Ids of the caller's devices holding an open device socket or that claimed lately."""
|
||||
if not DEVICE_INSTALL_ENABLED:
|
||||
raise DeviceInstallDisabledException()
|
||||
device_ids = [
|
||||
@@ -273,6 +274,7 @@ async def claim_install_requests(
|
||||
with _install_errors():
|
||||
claim = await device_install_handler.claim(device_id)
|
||||
|
||||
await mark_claimed(device_id)
|
||||
await asyncio.gather(*(emit_install_updated(i) for i in claim.newly_taken))
|
||||
return claim.taken
|
||||
|
||||
|
||||
@@ -25,7 +25,8 @@ CLIENT_TOKEN_ID_KEY: Final = "client_token_id"
|
||||
DEVICE_ID_KEY: Final = "device_id"
|
||||
# Clears entries a crashed worker left behind.
|
||||
SOCKET_TRACKING_TTL_SECONDS: Final = SESSION_MAX_AGE_SECONDS
|
||||
# A device is online while one of its sockets refreshed its presence within the TTL.
|
||||
# A device is online while one of its sockets, or its last claim, refreshed its
|
||||
# presence within the TTL.
|
||||
PRESENCE_TTL_SECONDS: Final = 90
|
||||
PRESENCE_REFRESH_SECONDS: Final = 30
|
||||
|
||||
@@ -38,6 +39,10 @@ def _presence_key(device_id: str) -> str:
|
||||
return f"device_presence:{device_id}"
|
||||
|
||||
|
||||
def _claim_presence_key(device_id: str) -> str:
|
||||
return f"device_claim_presence:{device_id}"
|
||||
|
||||
|
||||
def _raw_client_token(environ: dict[str, Any], auth: Any) -> str | None:
|
||||
"""The token from the handshake's auth payload, else its bearer header."""
|
||||
token = auth.get("token") if isinstance(auth, dict) else None
|
||||
@@ -145,13 +150,23 @@ async def disconnect(sid: str) -> None:
|
||||
await socket_handler.untrack(_presence_key(device_id), sid)
|
||||
|
||||
|
||||
async def mark_claimed(device_id: str) -> None:
|
||||
"""Count a device that polls claim instead of holding a socket as online for a while."""
|
||||
try:
|
||||
await async_cache.set(
|
||||
_claim_presence_key(device_id), "1", ex=PRESENCE_TTL_SECONDS
|
||||
)
|
||||
except Exception: # noqa: BLE001
|
||||
log.warning(f"Failed to record the claim of device {device_id}", exc_info=True)
|
||||
|
||||
|
||||
async def online_device_ids(device_ids: Sequence[str]) -> list[str]:
|
||||
"""The devices among ``device_ids`` with a socket that refreshed its presence lately."""
|
||||
"""The devices among ``device_ids`` with a socket or a claim that refreshed its presence lately."""
|
||||
if not device_ids:
|
||||
return []
|
||||
async with async_cache.pipeline(transaction=False) as pipe:
|
||||
for device_id in device_ids:
|
||||
pipe.exists(_presence_key(device_id))
|
||||
pipe.exists(_presence_key(device_id), _claim_presence_key(device_id))
|
||||
counts = await pipe.execute()
|
||||
return [
|
||||
device_id for device_id, count in zip(device_ids, counts, strict=True) if count
|
||||
|
||||
@@ -295,6 +295,15 @@ class TestOnline:
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.json() == ["dev-online"]
|
||||
|
||||
def test_a_device_that_just_claimed_is_online_without_a_socket(
|
||||
self, client, headers, device_headers, device
|
||||
):
|
||||
_claim(client, device_headers)
|
||||
|
||||
response = client.get("/api/devices/online", headers=headers)
|
||||
|
||||
assert response.json() == ["dev-1"]
|
||||
|
||||
def test_is_not_found_while_disabled(self, mocker, client, headers):
|
||||
mocker.patch.object(device_install, "DEVICE_INSTALL_ENABLED", False)
|
||||
|
||||
|
||||
@@ -366,6 +366,20 @@ class TestPresence:
|
||||
"dev-1",
|
||||
]
|
||||
|
||||
async def test_a_recent_claim_is_presence(self):
|
||||
await devices.mark_claimed("dev-1")
|
||||
|
||||
assert await devices.online_device_ids(["dev-1", "dev-2"]) == ["dev-1"]
|
||||
ttl = await async_cache.ttl("device_claim_presence:dev-1")
|
||||
assert 0 < ttl <= devices.PRESENCE_TTL_SECONDS
|
||||
|
||||
async def test_a_failed_claim_record_is_only_logged(self, mocker):
|
||||
mocker.patch.object(async_cache, "set", AsyncMock(side_effect=OSError))
|
||||
|
||||
await devices.mark_claimed("dev-1")
|
||||
|
||||
assert await devices.online_device_ids(["dev-1"]) == []
|
||||
|
||||
async def test_a_revocation_entry_alone_is_not_presence(self):
|
||||
await async_cache.sadd("device_token_sockets:42", "sid-1")
|
||||
|
||||
|
||||
@@ -1060,7 +1060,7 @@ World`), so a name shared across platforms resolves to the lowest visible ROM id
|
||||
| ------ | ------------------------- | ------------------------ | ------------------------------------------------------------- |
|
||||
| POST | `/` | DEVICES_WRITE | Register device (fingerprint dedup) |
|
||||
| GET | `/` | DEVICES_READ | List devices |
|
||||
| GET | `/online` | DEVICES_READ | Ids of devices with a live device socket |
|
||||
| GET | `/online` | DEVICES_READ | Ids of devices with a live device socket or a recent claim |
|
||||
| GET | `/{id}` | DEVICES_READ | Get device |
|
||||
| PUT | `/{id}` | DEVICES_WRITE | Update device |
|
||||
| DELETE | `/{id}` | DEVICES_WRITE | Delete device |
|
||||
@@ -1617,8 +1617,10 @@ closed when its token is deleted, regenerated or re-paired, its device or
|
||||
owner is deleted, its owner is disabled, or its owner's permissions change,
|
||||
and within 30 seconds of the token's `expires_at`. Each open socket refreshes `device_presence:{device_id}` and its token's
|
||||
`device_token_sockets:{id}` every 30 seconds. The presence key lapses 90
|
||||
seconds after the last refresh and loses the socket on disconnect;
|
||||
`GET /api/devices/online` lists the caller's devices whose key is live (404
|
||||
seconds after the last refresh and loses the socket on disconnect. A device
|
||||
that polls claim instead sets `device_claim_presence:{device_id}` on each
|
||||
claim, which lapses after the same 90 seconds.
|
||||
`GET /api/devices/online` lists the caller's devices with either key live (404
|
||||
while `DEVICE_INSTALL_ENABLED` is off). The
|
||||
per-token socket sets exist only to close sockets on revocation and never
|
||||
answer presence.
|
||||
|
||||
@@ -4,6 +4,7 @@ ROMM_TMP_PATH= # Custom temporary directory path
|
||||
ROMM_BASE_URL=http://0.0.0.0 # Public URL of this instance
|
||||
ROMM_PORT=8080 # Port on which the application listens
|
||||
KIOSK_MODE=false # Let visitors browse without logging in (read-only)
|
||||
TZ=Etc/UTC # Time zone of log timestamps and save version names (e.g. America/New_York)
|
||||
|
||||
# Database
|
||||
ROMM_DB_DRIVER=mariadb # Database driver to use (mariadb, mysql, postgresql)
|
||||
|
||||
Reference in New Issue
Block a user