fix(#918): RFID auto-match handles Quick-Add and rejects non-Bambu brands

`find_matching_untagged_spool` is supposed to attach an incoming Bambu
  RFID UUID to a pre-existing manually-logged spool of the same
  material/color so users who log inventory before scanning don't end up
  with duplicate rows. Two bugs meant it almost never worked for the
  actual reporting workflow:

  1. Subtype filter was strict. AMS reports `tray_sub_brands="PLA Basic"`
     → matcher required `Spool.subtype = 'Basic'` exactly. The form's
     Quick-Add mode only requires `material`, so bulk-logged rows have
     `subtype=NULL` and were always excluded → duplicate on first AMS
     read.
  2. Brand wasn't filtered. The docstring claimed brand was matched but
     the WHERE clause didn't include it, so a same-color Polymaker (or
     any non-Bambu) untagged row could acquire a Bambu UUID — silent
     data corruption.

  Fix in the same query: subtype prefers exact match but accepts NULL as
  fallback (CASE in ORDER BY ensures exact wins when both exist); brand
  restricted to NULL or LOWER(brand) LIKE '%bambu%' (covers 'Bambu',
  'Bambu Lab', 'BambuLab', 'bambu lab' — the spellings users actually
  type).

  6 regression tests added in test_spool_tag_matcher.py.
This commit is contained in:
maziggy
2026-04-25 12:49:00 +02:00
parent 35edc036bd
commit 568835c586
3 changed files with 202 additions and 9 deletions
+1
View File
@@ -24,6 +24,7 @@ All notable changes to Bambuddy will be documented in this file.
- **Settings page: permission-gated instead of admin-only** — the Settings sidebar entry has always been visible to any user holding `settings:read`, but the route guard required admin role, so a non-admin with `settings:read` would see the entry, click it, and get silently redirected back to the dashboard. The route guard now matches the sidebar: any user with `settings:read` can open the page, and the individual tabs / cards continue to enforce their own per-feature permissions (`users:read`, `groups:update`, `oidc:*`, etc. — many of them admin-only, some not). Group editor routes moved to permission-based guards too (`groups:create` for `/groups/new`, `groups:update` for `/groups/:id/edit`), so permission delegation works end-to-end. Admins retain full access since admins implicitly hold every permission.
### Fixed
- **Bambu RFID auto-match created duplicate inventory rows for Quick-Add and non-Bambu-branded spools** ([#918](https://github.com/maziggy/bambuddy/issues/918)) — `find_matching_untagged_spool` is supposed to attach a Bambu RFID UID to a pre-existing manually-logged spool of the same material/color so users who log inventory before scanning don't end up with a duplicate row on first AMS read. Two bugs in the matcher meant it almost never worked for the actual reporting workflow: **(1)** the subtype filter was strict — when the AMS tray reports `tray_sub_brands="PLA Basic"` the matcher required `Spool.subtype = 'Basic'` exactly, so any Quick-Add row (Quick-Add only requires `material`, leaving `subtype=NULL`) was excluded and duplicated on first AMS read. **(2)** the docstring claimed it filtered on brand but the WHERE clause didn't, so a same-color *Polymaker* untagged spool would silently acquire a Bambu Lab tray UUID, leaving the user with `brand="Polymaker"` but a Bambu UUID — silent data corruption. Both bugs are addressed in the same query: subtype now prefers an exact match but accepts a NULL-subtype row as fallback (with a `CASE` in `ORDER BY` so an exact match still wins when both exist), and brand is now restricted to "contains 'bambu' (case-insensitive)" or NULL — matching `'Bambu'` (the form's `DEFAULT_BRANDS` value), `'Bambu Lab'` (the catalog value), `'BambuLab'`, `'bambu lab'`, etc., while rejecting any explicitly-named third-party brand. 6 new regression tests in `test_spool_tag_matcher.py` cover the NULL-subtype fallback, exact-subtype-wins-over-NULL ordering, non-Bambu brand rejection, NULL brand acceptance, all four Bambu brand spelling variants, and the full Quick-Add scenario (`brand=NULL` + `subtype=NULL`). The broader UI proposals in #918 (manual override / merge / disambiguation prompt) are intentionally out of scope — once the matcher works, the duplicate-on-RFID complaint that motivated those proposals goes away. Thanks to @ViridityCorn for the report and pointing at the right function, and to @Arn0uDz for confirming with a 20-spool repro.
- **Swagger UI link in Settings → API Keys rendered a blank page** — the global CSP applied by `security_headers_middleware` set `script-src 'self'` and `style-src 'self' 'unsafe-inline' https://fonts.googleapis.com`, which blocked both the inline `<script>` that boots Swagger and the `cdn.jsdelivr.net` URL that ships `swagger-ui-bundle.js` / `swagger-ui.css`. FastAPI's `/docs` page therefore loaded a 1 KB shell with no JS executed, leaving an empty white page. The middleware now emits a docs-scoped CSP for `/docs`, `/redoc`, and `/docs/oauth2-redirect` that allows `https://cdn.jsdelivr.net` for scripts + styles, the FastAPI/Redoc favicon hosts for images, and `'unsafe-inline'` for the Swagger boot script — every other route keeps the unchanged stricter SPA policy.
- **Camera stream second viewer fails / kicks the first off** ([#1089](https://github.com/maziggy/bambuddy/issues/1089)) — Most Bambu Lab printers only allow one concurrent camera connection (RTSP socket on X1/H2/P2, port-6000 chamber-image socket on A1/P1), but `GET /printers/{id}/camera/stream` opened a fresh upstream per viewer keyed on a per-request `stream_id`. Two browser tabs / two dashboard cards → the second viewer either failed silently or kicked the first one off. New `services/camera_fanout.py::MjpegBroadcaster` owns a single upstream per printer and fans pre-formatted MJPEG chunks out to N subscriber queues; new viewers tap the existing connection. When the last subscriber leaves, the upstream stays alive for a 5 s grace window so a tab refresh or "open in new tab" doesn't pay an ffmpeg/RTSP reconnect, then tears down cleanly. Per-subscriber queues are bounded (depth 4) so a slow viewer drops frames for itself rather than blocking the broadcaster — live video, old frames have no value. Stop endpoint and app-shutdown both call into the broadcaster's force-shutdown path so subscribers wake up via an upstream-gone sentinel instead of hanging on `queue.get()`. External-camera path is unchanged (user-supplied MJPEG/RTSP servers handle multi-viewer themselves). The upstream uses a deterministic `{printer_id}-fanout` stream id so every existing prefix-match in `cleanup_orphaned_streams`, `camera_status`, the snapshot fall-through in `main.py`, and the `stop` endpoint continues to find it without changes. Two follow-up correctness fixes from the audit pass: (1) `_stream_start_times[printer_id]` is now set with `setdefault()` so `/camera/status` reports the SHARED upstream's age — previously each new viewer overwrote it, making `stream_uptime` jump backward whenever a second viewer attached; (2) the route now retries `subscribe()` once on `RuntimeError` to close a tiny race where the grace teardown can flip the broadcaster to `stopped` between the registry lookup and the subscribe call (the retry forces the registry to mint a fresh broadcaster). Detach log line shows the post-unsubscribe count returned atomically by `unsubscribe()` — no more two viewers leaving simultaneously both reporting `subscribers=0`. Permission gates unchanged: `/camera/stream` still requires the existing token (minted by `POST /camera/stream-token` with `CAMERA_VIEW`); `/camera/stop` still requires `CAMERA_VIEW`; the broadcaster is internal infra with no FastAPI surface. 13 unit tests for the broadcaster (single subscriber, multi-subscriber-shares-one-pump, slow-subscriber-doesn't-block-fast, grace-window teardown, grace-cancelled-on-rejoin, force-shutdown sentinel, `iter_subscriber` exits on upstream-gone and on client-disconnect, registry replaces stopped broadcasters, `subscribe()` raises on stopped broadcaster, `unsubscribe()` returns post-removal count atomically across concurrent leavers, double-unsubscribe is idempotent, and the route's force-shutdown-then-fresh-subscribe retry path) plus 2 new integration tests on the stop endpoint covering the deterministic fan-out stream id and the `shutdown_broadcaster` wiring. Thanks to @swheettaos for the diagnosis and broadcaster sketch.
- **Uploads to writable external folders silently landed in internal storage** ([#1112](https://github.com/maziggy/bambuddy/issues/1112)) — `LibraryFolder` has an `external_readonly` flag, so the model already distinguishes writable from read-only external mounts, but `POST /library/files` rejected only the read-only branch and then unconditionally wrote to `get_library_files_dir()` with a UUID-scoped filename. The resulting `LibraryFile` row linked back to the external folder via `folder_id`, so the file showed up in the Bambuddy UI and could be printed, but the bytes physically lived in `archive/library/files/` and never touched the mount — invisible from any other machine accessing the same NAS/SMB share. New `_resolve_upload_destination()` helper detects writable external targets and writes through to `<external_path>/<filename>` (keeping the original filename so the file is recognisable on the mount), with guards for missing/inaccessible path (400), non-writable mount (400), pre-existing filename on the mount (409 — no silent overwrite; the user is expected to rename and retry, matching how scan treats external files as externally-owned bytes), and a `resolve + relative_to` path-traversal guard on the joined destination. DB row now matches what scan produces: `is_external=True`, `file_path=<absolute external path>`, so the existing download / delete / dedupe paths work unchanged (`to_absolute_path` already fast-paths `is_absolute()` inputs, and external-file deletion already bypasses trash and only drops the DB row + internal thumbnail). `POST /library/files/extract-zip` is now rejected against *any* external folder (not just read-only) with a clear "extract the ZIP on the external mount and run Scan" message — the nested-subfolder creation path would need to `mkdir` on the mount and create matching `is_external=True` `LibraryFolder` rows, which is a separate design round, and the Scan flow already handles that shape. 7 new integration tests cover: bytes land on the mount; DB row has `is_external=True` + absolute `file_path`; filename collision → 409 with prior bytes preserved; vanished external path → 400; path-traversal filename never escapes the external dir; extract-zip into writable external rejected with the Scan hint; root uploads unchanged.
+33 -9
View File
@@ -2,7 +2,7 @@
import logging
from sqlalchemy import func, or_, select
from sqlalchemy import case, func, or_, select
from sqlalchemy.ext.asyncio import AsyncSession
from sqlalchemy.orm import selectinload
@@ -191,11 +191,22 @@ async def create_spool_from_tray(db: AsyncSession, tray_data: dict) -> Spool:
async def find_matching_untagged_spool(db: AsyncSession, tray_data: dict) -> Spool | None:
"""Find an existing untagged inventory spool matching brand/material/color.
"""Find an existing untagged Bambu inventory spool matching material/color.
When a Bambu Lab spool is detected in the AMS but no tag match exists,
check if the user has a manually-added spool with the same properties
that hasn't been linked to a tag yet. Returns the oldest match (FIFO).
that hasn't been linked to a tag yet. Returns the best match (#918):
- **Brand**: only consider spools whose brand is unspecified or contains
"bambu" (case-insensitive — covers both "Bambu" and "Bambu Lab" as
stored by the form's brand dropdown). This prevents a same-color
Polymaker / generic spool from accidentally attracting a Bambu UUID.
- **Subtype**: prefer an exact match (e.g. AMS "Basic" → spool subtype
"Basic"), but fall back to a NULL-subtype spool — the form's Quick Add
mode leaves subtype empty, so bulk-logged spools rely on this fallback
to attract their RFID tag instead of duplicating on first AMS read.
- **FIFO** within each preference group (user likely logged in purchase
order).
"""
tray_type = tray_data.get("tray_type", "")
tray_sub_brands = tray_data.get("tray_sub_brands", "")
@@ -229,7 +240,7 @@ async def find_matching_untagged_spool(db: AsyncSession, tray_data: dict) -> Spo
elif color_code and color_code[0] == "T":
subtype = "Tri Color"
# Build query: active spools with no tag, matching brand + material + color
# Active, untagged spools matching material + color + Bambu-or-unset brand.
query = (
select(Spool)
.options(selectinload(Spool.k_profiles), selectinload(Spool.assignments))
@@ -239,17 +250,30 @@ async def find_matching_untagged_spool(db: AsyncSession, tray_data: dict) -> Spo
Spool.tray_uuid.is_(None),
func.upper(Spool.material) == material.upper(),
func.upper(Spool.rgba) == tray_color.upper(),
or_(
Spool.brand.is_(None),
func.lower(Spool.brand).like("%bambu%"),
),
)
)
# Match subtype if parsed (e.g. "Basic", "Matte")
if subtype:
query = query.where(func.upper(Spool.subtype) == subtype.upper())
# Exact subtype OR NULL fallback. The CASE in ORDER BY ensures an
# exact-subtype row beats a NULL-subtype row when both exist; FIFO
# within each group.
query = query.where(
or_(
func.upper(Spool.subtype) == subtype.upper(),
Spool.subtype.is_(None),
)
).order_by(
case((func.upper(Spool.subtype) == subtype.upper(), 0), else_=1),
Spool.created_at.asc(),
)
else:
query = query.where(Spool.subtype.is_(None))
query = query.where(Spool.subtype.is_(None)).order_by(Spool.created_at.asc())
# FIFO: oldest spool first (user likely added in purchase order)
query = query.order_by(Spool.created_at.asc()).limit(1)
query = query.limit(1)
result = await db.execute(query)
spool = result.scalar_one_or_none()
@@ -690,6 +690,174 @@ async def test_find_matching_untagged_spool_relationships_loaded(db_session):
assert _relationship_is_loaded(found, "assignments")
# -- find_matching_untagged_spool: #918 regressions ------------------------
@pytest.mark.asyncio
async def test_find_matching_untagged_spool_null_subtype_fallback(db_session):
"""#918: Quick-Add spool (subtype=NULL) matches when AMS reports a subtype.
The form's Quick-Add mode only requires `material`, so bulk-logged spools
have subtype=NULL. Before the fix, the strict `subtype = 'Basic'` filter
excluded these rows and the system created duplicates on first AMS read.
"""
spool = Spool(
material="PLA",
subtype=None, # Quick-Add bulk entry
rgba="FFFFFFFF",
brand="Bambu Lab",
label_weight=1000,
core_weight=250,
)
db_session.add(spool)
await db_session.commit()
# Tray reports "PLA Basic" → subtype parsed as "Basic"
found = await find_matching_untagged_spool(db_session, SAMPLE_TRAY)
assert found is not None
assert found.id == spool.id
@pytest.mark.asyncio
async def test_find_matching_untagged_spool_prefers_exact_subtype_over_null(db_session):
"""#918: When both an exact-subtype and a NULL-subtype row match, exact wins.
The NULL fallback exists only as a backstop for Quick-Add bulk-logged
spools — if the user did the work to record subtype="Basic", it must
take precedence over a vague "PLA" record, even if the latter is older.
"""
import asyncio
null_spool = Spool(
material="PLA",
subtype=None, # Older but vague — should NOT win
rgba="FFFFFFFF",
brand="Bambu Lab",
label_weight=1000,
core_weight=250,
)
db_session.add(null_spool)
await db_session.flush()
await asyncio.sleep(0.05)
exact_spool = Spool(
material="PLA",
subtype="Basic", # Newer but specific — should win
rgba="FFFFFFFF",
brand="Bambu Lab",
label_weight=1000,
core_weight=250,
)
db_session.add(exact_spool)
await db_session.commit()
found = await find_matching_untagged_spool(db_session, SAMPLE_TRAY)
assert found is not None
assert found.id == exact_spool.id
@pytest.mark.asyncio
async def test_find_matching_untagged_spool_rejects_non_bambu_brand(db_session):
"""#918: A same-color non-Bambu spool must NOT attract a Bambu UUID.
Without the brand filter, a Polymaker untagged spool of matching
material/color would silently acquire a Bambu RFID UUID, leaving the
user with brand="Polymaker" but a Bambu Lab tray UUID — corrupt data.
"""
spool = Spool(
material="PLA",
subtype="Basic",
rgba="FFFFFFFF",
brand="Polymaker", # NOT Bambu — must be rejected
label_weight=1000,
core_weight=250,
)
db_session.add(spool)
await db_session.commit()
found = await find_matching_untagged_spool(db_session, SAMPLE_TRAY)
assert found is None
@pytest.mark.asyncio
async def test_find_matching_untagged_spool_accepts_null_brand(db_session):
"""#918: Quick-Add spools with brand=NULL still match a Bambu RFID read.
Quick-Add doesn't require brand, so a user bulk-logging Bambu spools may
leave it empty. The matcher allows NULL brand because the alternative
(forcing every Quick-Add spool to be tagged "Bambu") is the exact
friction the auto-matcher exists to remove.
"""
spool = Spool(
material="PLA",
subtype="Basic",
rgba="FFFFFFFF",
brand=None, # Quick-Add left brand blank
label_weight=1000,
core_weight=250,
)
db_session.add(spool)
await db_session.commit()
found = await find_matching_untagged_spool(db_session, SAMPLE_TRAY)
assert found is not None
assert found.id == spool.id
@pytest.mark.asyncio
async def test_find_matching_untagged_spool_accepts_bambu_brand_variants(db_session):
"""#918: Both 'Bambu' (form dropdown) and 'Bambu Lab' (catalog) match.
DEFAULT_BRANDS in the form lists 'Bambu'; the catalog uses 'Bambu Lab'.
Users can pick either. The fuzzy %bambu% LIKE handles both, plus
'BambuLab', 'bambu lab', etc.
"""
for brand_value in ("Bambu", "Bambu Lab", "BambuLab", "bambu lab"):
spool = Spool(
material="PLA",
subtype="Basic",
rgba="FFFFFFFF",
brand=brand_value,
label_weight=1000,
core_weight=250,
)
db_session.add(spool)
await db_session.commit()
found = await find_matching_untagged_spool(db_session, SAMPLE_TRAY)
assert found is not None, f"brand={brand_value!r} should match"
assert found.id == spool.id
# Clean up so the next iteration starts fresh.
await db_session.delete(spool)
await db_session.commit()
@pytest.mark.asyncio
async def test_find_matching_untagged_spool_null_subtype_with_null_brand(db_session):
"""#918: Pure Quick-Add row (brand=NULL, subtype=NULL) matches.
This is the exact scenario from Arn0uDz's report: 20 spools logged via
Quick Add, then placed in the AMS one at a time. Before the fix every
insertion duplicated; after the fix the first matching row is reused.
"""
spool = Spool(
material="PLA",
subtype=None,
rgba="FFFFFFFF",
brand=None,
label_weight=1000,
core_weight=250,
)
db_session.add(spool)
await db_session.commit()
found = await find_matching_untagged_spool(db_session, SAMPLE_TRAY)
assert found is not None
assert found.id == spool.id
# -- link_tag_to_inventory_spool -------------------------------------------