fix(devices): sync users.device_serials when PUT /api/devices/{id} changes user_list
DeviceUpdate accepts user_list, so a device PUT could add or remove users without touching their device_serials - leaving the MQTT app ACL stale (a removed user would keep access; an added user would be denied). update_device now diffs the old vs new user_list and, in the same atomic batch as the device write, ArrayUnion/ArrayRemoves the device's serial on each added/removed user, then invalidates their MQTT ACL cache entries. Dangling user references are skipped (updating a missing doc would fail the whole batch). PUTs without user_list take the old single-update path. Covered by tests/test_device_serials_sync.py (fake Firestore). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -230,12 +230,45 @@ def update_device(device_doc_id: str, data: DeviceUpdate) -> DeviceInDB:
|
||||
update_data[key] = _deep_merge(existing[key], update_data[key])
|
||||
|
||||
update_data = _restore_timestamps(update_data)
|
||||
doc_ref.update(update_data)
|
||||
|
||||
if "user_list" not in update_data:
|
||||
doc_ref.update(update_data)
|
||||
else:
|
||||
# A user_list change must also update each affected user's
|
||||
# device_serials (MQTT ACL), in the same atomic batch.
|
||||
from users import service as users_service
|
||||
|
||||
old_ids = {_user_list_entry_id(e) for e in (existing.get("user_list") or [])} - {""}
|
||||
new_ids = {_user_list_entry_id(e) for e in update_data["user_list"]} - {""}
|
||||
serial = users_service.device_serial_of(existing)
|
||||
|
||||
batch = db.batch()
|
||||
batch.update(doc_ref, update_data)
|
||||
touched = []
|
||||
for user_id in (old_ids ^ new_ids):
|
||||
user_ref = db.collection("users").document(user_id)
|
||||
user_doc = user_ref.get()
|
||||
if not user_doc.exists:
|
||||
continue # dangling reference — nothing to sync
|
||||
users_service.stage_device_serial_link(batch, user_ref, serial, linked=user_id in new_ids)
|
||||
touched.append(user_doc.to_dict())
|
||||
batch.commit()
|
||||
for user_data in touched:
|
||||
users_service.invalidate_mqtt_acl_cache(user_data)
|
||||
|
||||
updated_doc = doc_ref.get()
|
||||
return _doc_to_device(updated_doc)
|
||||
|
||||
|
||||
def _user_list_entry_id(entry) -> str:
|
||||
"""user_list entries are DocumentReferences, "users/{id}" paths or raw doc IDs."""
|
||||
if isinstance(entry, DocumentReference):
|
||||
return entry.id
|
||||
if isinstance(entry, str):
|
||||
return entry.strip().split("/")[-1]
|
||||
return ""
|
||||
|
||||
|
||||
def get_device_users(device_doc_id: str) -> list[dict]:
|
||||
"""Get users assigned to a device from the device_users sub-collection.
|
||||
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
"""
|
||||
update_device must keep users.device_serials in sync when a PUT changes the
|
||||
device's user_list (the MQTT ACL reads device_serials). Firestore is faked.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from google.cloud.firestore_v1 import ArrayRemove, ArrayUnion
|
||||
|
||||
from devices import service as devices_service
|
||||
from devices.models import DeviceUpdate
|
||||
from mqtt import app_users
|
||||
|
||||
SERIAL = "BSVSPR-26C13X-STD01R-X7KQA"
|
||||
|
||||
|
||||
class _Snap:
|
||||
def __init__(self, ref, data):
|
||||
self.reference = ref
|
||||
self.id = ref.id
|
||||
self._data = data
|
||||
self.exists = data is not None
|
||||
|
||||
def to_dict(self):
|
||||
return dict(self._data) if self._data is not None else None
|
||||
|
||||
|
||||
class _Ref:
|
||||
def __init__(self, db, collection, doc_id):
|
||||
self._db, self._col, self.id = db, collection, doc_id
|
||||
|
||||
def get(self):
|
||||
return _Snap(self, self._db.docs[self._col].get(self.id))
|
||||
|
||||
def update(self, data):
|
||||
self._db.apply(self._col, self.id, data)
|
||||
|
||||
|
||||
class _Batch:
|
||||
def __init__(self, db):
|
||||
self._db, self.ops = db, []
|
||||
|
||||
def update(self, ref, data):
|
||||
self.ops.append((ref, data))
|
||||
|
||||
def commit(self):
|
||||
for ref, data in self.ops:
|
||||
ref.update(data)
|
||||
self._db.commits += 1
|
||||
|
||||
|
||||
class _Collection:
|
||||
def __init__(self, db, name):
|
||||
self._db, self._name = db, name
|
||||
|
||||
def document(self, doc_id):
|
||||
return _Ref(self._db, self._name, doc_id)
|
||||
|
||||
|
||||
class FakeDB:
|
||||
def __init__(self):
|
||||
self.docs = {"devices": {}, "users": {}}
|
||||
self.commits = 0
|
||||
|
||||
def collection(self, name):
|
||||
return _Collection(self, name)
|
||||
|
||||
def batch(self):
|
||||
return _Batch(self)
|
||||
|
||||
def apply(self, col, doc_id, data):
|
||||
doc = self.docs[col][doc_id]
|
||||
for key, val in data.items():
|
||||
if isinstance(val, ArrayUnion):
|
||||
doc[key] = list(doc.get(key) or []) + [v for v in val.values if v not in (doc.get(key) or [])]
|
||||
elif isinstance(val, ArrayRemove):
|
||||
doc[key] = [v for v in (doc.get(key) or []) if v not in val.values]
|
||||
else:
|
||||
doc[key] = val
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def db(monkeypatch):
|
||||
fake = FakeDB()
|
||||
fake.docs["devices"]["dev1"] = {
|
||||
"serial_number": SERIAL, "mfg_status": "claimed", "user_list": ["users/alice", "users/bob"],
|
||||
}
|
||||
fake.docs["users"]["alice"] = {"uid": "alice", "device_serials": [SERIAL]}
|
||||
fake.docs["users"]["bob"] = {"uid": "bob", "device_serials": [SERIAL, "OTHER"]}
|
||||
fake.docs["users"]["carol"] = {"uid": "carol"}
|
||||
monkeypatch.setattr(devices_service, "get_db", lambda: fake)
|
||||
return fake
|
||||
|
||||
|
||||
def test_put_user_list_syncs_added_and_removed_users(db, monkeypatch):
|
||||
invalidated = []
|
||||
monkeypatch.setattr(app_users, "invalidate", invalidated.append)
|
||||
|
||||
devices_service.update_device("dev1", DeviceUpdate(user_list=["users/alice", "users/carol"]))
|
||||
|
||||
assert db.docs["devices"]["dev1"]["user_list"] == ["users/alice", "users/carol"]
|
||||
assert db.docs["users"]["alice"]["device_serials"] == [SERIAL] # unchanged
|
||||
assert db.docs["users"]["bob"]["device_serials"] == ["OTHER"] # removed
|
||||
assert db.docs["users"]["carol"]["device_serials"] == [SERIAL] # added
|
||||
assert sorted(invalidated) == ["bob", "carol"]
|
||||
assert db.commits == 1 # one atomic batch
|
||||
|
||||
|
||||
def test_put_user_list_ignores_dangling_user_refs(db):
|
||||
devices_service.update_device("dev1", DeviceUpdate(user_list=["users/alice", "users/bob", "users/ghost"]))
|
||||
assert db.docs["devices"]["dev1"]["user_list"] == ["users/alice", "users/bob", "users/ghost"]
|
||||
assert "ghost" not in db.docs["users"]
|
||||
|
||||
|
||||
def test_put_without_user_list_does_not_touch_users(db):
|
||||
devices_service.update_device("dev1", DeviceUpdate(device_name="Church"))
|
||||
assert db.docs["devices"]["dev1"]["device_name"] == "Church"
|
||||
assert db.commits == 0
|
||||
assert db.docs["users"]["bob"]["device_serials"] == [SERIAL, "OTHER"]
|
||||
Reference in New Issue
Block a user