Skip to content

Commit 3830b78

Browse files
committed
fix(spoolman): harden slot-assignment CRUD, sync robustness, and full test coverage
- Add SpoolmanSlotAssignment model and DB migration for (printer, ams, tray) → spool_id mapping - Slot-assignment CRUD endpoints: verify spool exists before DB commit (H2), re-raise non-404 Spoolman errors from GET (H3), return success on 404 after DELETE (H4/CR1) - DB rollbacks added to all sync batch-persist except blocks (C1/C2/C3) and unlink_spool (H6) - Slot-map load failures promoted from debug to warning level (H1, 3 sites) - Sync: add SkippedSpool tracking for no-RFID/no-hint trays (CR3), fix asymmetry in sync_all_printers so not-found errors are also reported for non-BL RFID trays - sync_ams_tray non-BL RFID path: wrap find_or_create_filament in try/except (H5) - on_ams_change: add isinstance guards for ams_unit/tray_data (CR4); catch ValueError from init_spoolman_client so SSRF-blocked URLs log a warning and return cleanly - delete_printer: explicitly delete SpoolmanSlotAssignment rows (SQLite FK cascade not enforced without PRAGMA foreign_keys; pattern matches existing maintenance cleanup) - S4: move SpoolmanSlotAssignment import to module top-level in spoolman.py and spoolman_inventory.py; remove 3 duplicate in-function imports - Remove dead variables current_tray_uuids/synced_spool_ids across sync routes (S2/CA1) - Update stale docstrings: clear_location_for_removed_spools (CA3), ON CONFLICT comment for assigned_at (CA4), assign_spoolman_slot 404 note (CA5), get_all raw dict format (CA6) - DB init: add spoolman_slot_assignment to model import list; fix active_print_spoolman and spool_usage_history DDL to use is_sqlite() branching (removes AUTOINCREMENT from PostgreSQL path) - Frontend: slot-assignment API client methods, AssignSpoolModal integration, PrintersPage AMS slot UI, i18n keys for all 8 locales - Tests T1-T9: link/unlink Spoolman error paths, slot GET stale-row cleanup, 503 propagation, sync DB write, hint forwarding, no-RFID skipped entry, non-BL RFID error guard, hint uncached path, hint ignored when RFID present, CASCADE delete
1 parent 989672a commit 3830b78

27 files changed

Lines changed: 10785 additions & 10402 deletions

backend/app/api/routes/printers.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ async def delete_printer(
299299

300300
from backend.app.models.archive import PrintArchive
301301
from backend.app.models.maintenance import MaintenanceHistory, PrinterMaintenance
302+
from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment
302303

303304
result = await db.execute(select(Printer).where(Printer.id == printer_id))
304305
printer = result.scalar_one_or_none()
@@ -316,6 +317,9 @@ async def delete_printer(
316317

317318
await db.execute(update(PrintArchive).where(PrintArchive.printer_id == printer_id).values(printer_id=None))
318319

320+
# Delete slot assignments for this printer (SQLite doesn't enforce FK cascades)
321+
await db.execute(sql_delete(SpoolmanSlotAssignment).where(SpoolmanSlotAssignment.printer_id == printer_id))
322+
319323
# Delete maintenance history and items for this printer
320324
# (SQLite doesn't enforce FK cascades, so do it explicitly)
321325
maintenance_ids = (

backend/app/api/routes/spoolman.py

Lines changed: 184 additions & 104 deletions
Large diffs are not rendered by default.

backend/app/api/routes/spoolman_inventory.py

Lines changed: 154 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from fastapi import APIRouter, Depends, HTTPException, Path, Query, Response
1616
from fastapi.responses import JSONResponse
1717
from pydantic import BaseModel, Field, field_validator, model_validator
18-
from sqlalchemy import select
18+
from sqlalchemy import delete, select, text
1919
from sqlalchemy.ext.asyncio import AsyncSession
2020

2121
from backend.app.api.routes._spoolman_helpers import (
@@ -27,7 +27,9 @@
2727
from backend.app.core.auth import RequirePermissionIfAuthEnabled
2828
from backend.app.core.database import get_db
2929
from backend.app.core.permissions import Permission
30+
from backend.app.models.printer import Printer
3031
from backend.app.models.settings import Settings
32+
from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment
3133
from backend.app.models.user import User
3234
from backend.app.services.spoolman import (
3335
SpoolmanClient,
@@ -234,6 +236,13 @@ class SpoolWeightUpdate(BaseModel):
234236
weight_grams: float = Field(..., ge=0.0, le=100_000.0)
235237

236238

239+
class SpoolSlotAssignmentRequest(BaseModel):
240+
spoolman_spool_id: int = Field(..., gt=0)
241+
printer_id: int = Field(..., gt=0)
242+
ams_id: int = Field(..., ge=0)
243+
tray_id: int = Field(..., ge=0)
244+
245+
237246
# ---------------------------------------------------------------------------
238247
# Endpoints
239248
# ---------------------------------------------------------------------------
@@ -529,3 +538,147 @@ async def sync_spool_weight(
529538
label_weight = _safe_int(upd_filament.get("weight"), 1000)
530539
weight_used = max(0.0, label_weight - remaining)
531540
return {"status": "ok", "weight_used": weight_used}
541+
542+
543+
@router.get("/slot-assignments/all")
544+
async def get_all_spoolman_slot_assignments(
545+
printer_id: int | None = Query(None, gt=0),
546+
db: AsyncSession = Depends(get_db),
547+
_: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_READ),
548+
) -> list[dict]:
549+
"""Return all Spoolman slot assignments, optionally filtered by printer.
550+
551+
Each item is a raw assignment dict with keys ``printer_id``, ``ams_id``,
552+
``tray_id``, and ``spoolman_spool_id`` — not an ``InventorySpool`` object.
553+
"""
554+
query = select(SpoolmanSlotAssignment)
555+
if printer_id is not None:
556+
query = query.where(SpoolmanSlotAssignment.printer_id == printer_id)
557+
result = await db.execute(query)
558+
slots = result.scalars().all()
559+
return [
560+
{
561+
"printer_id": s.printer_id,
562+
"ams_id": s.ams_id,
563+
"tray_id": s.tray_id,
564+
"spoolman_spool_id": s.spoolman_spool_id,
565+
}
566+
for s in slots
567+
]
568+
569+
570+
@router.post("/slot-assignments")
571+
async def assign_spoolman_slot(
572+
body: SpoolSlotAssignmentRequest,
573+
db: AsyncSession = Depends(get_db),
574+
_: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE),
575+
) -> dict:
576+
"""Assign a Spoolman spool to a printer AMS slot (stored in local DB only).
577+
578+
Raises 404 if the printer does not exist or the spool is not found in Spoolman.
579+
Spoolman's own ``spool.location`` field is NOT touched — it is user-managed.
580+
"""
581+
582+
client = await _get_client(db)
583+
result = await db.execute(select(Printer).where(Printer.id == body.printer_id))
584+
printer = result.scalar_one_or_none()
585+
if not printer:
586+
raise HTTPException(status_code=404, detail="Printer not found")
587+
588+
# Verify the Spoolman spool exists before committing to local DB.
589+
# This prevents ghost rows pointing at non-existent spool IDs.
590+
async with _translate_spoolman_errors():
591+
spool = await client.get_spool(body.spoolman_spool_id)
592+
593+
# Spool confirmed in Spoolman — upsert into local slot-assignment table
594+
# assigned_at is intentionally not refreshed on re-assign (original timestamp preserved)
595+
await db.execute(
596+
text(
597+
"INSERT INTO spoolman_slot_assignments"
598+
" (printer_id, ams_id, tray_id, spoolman_spool_id)"
599+
" VALUES (:printer_id, :ams_id, :tray_id, :spool_id)"
600+
" ON CONFLICT(printer_id, ams_id, tray_id)"
601+
" DO UPDATE SET spoolman_spool_id = excluded.spoolman_spool_id"
602+
),
603+
{
604+
"printer_id": body.printer_id,
605+
"ams_id": body.ams_id,
606+
"tray_id": body.tray_id,
607+
"spool_id": body.spoolman_spool_id,
608+
},
609+
)
610+
await db.commit()
611+
612+
return _map_spoolman_spool(spool)
613+
614+
615+
@router.delete("/slot-assignments/{spoolman_spool_id}")
616+
async def unassign_spoolman_slot(
617+
spoolman_spool_id: int = Path(..., gt=0),
618+
db: AsyncSession = Depends(get_db),
619+
_: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE),
620+
) -> dict:
621+
"""Remove the local slot assignment for a Spoolman spool.
622+
623+
Spoolman's own ``spool.location`` field is NOT touched — it is user-managed.
624+
"""
625+
client = await _get_client(db)
626+
627+
# Delete from local slot-assignment table (all slots for this spool ID)
628+
await db.execute(
629+
delete(SpoolmanSlotAssignment).where(SpoolmanSlotAssignment.spoolman_spool_id == spoolman_spool_id)
630+
)
631+
await db.commit()
632+
633+
# Fetch the spool from Spoolman to return in InventorySpool format.
634+
# If the spool no longer exists in Spoolman, the local unassignment still succeeded.
635+
try:
636+
async with _translate_spoolman_errors():
637+
spool = await client.get_spool(spoolman_spool_id)
638+
return _map_spoolman_spool(spool)
639+
except HTTPException as exc:
640+
if exc.status_code != 404:
641+
raise
642+
# Spool no longer exists in Spoolman; unassignment still succeeded.
643+
return {"id": spoolman_spool_id}
644+
645+
646+
@router.get("/slot-assignments")
647+
async def get_spoolman_slot_assignment(
648+
printer_id: int = Query(..., gt=0),
649+
ams_id: int = Query(..., ge=0),
650+
tray_id: int = Query(..., ge=0),
651+
db: AsyncSession = Depends(get_db),
652+
_: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_READ),
653+
) -> dict | None:
654+
"""Return the Spoolman spool assigned to a specific printer slot, or null if unassigned."""
655+
client = await _get_client(db)
656+
result = await db.execute(select(Printer).where(Printer.id == printer_id))
657+
printer = result.scalar_one_or_none()
658+
if not printer:
659+
raise HTTPException(status_code=404, detail="Printer not found")
660+
661+
slot_result = await db.execute(
662+
select(SpoolmanSlotAssignment).where(
663+
SpoolmanSlotAssignment.printer_id == printer_id,
664+
SpoolmanSlotAssignment.ams_id == ams_id,
665+
SpoolmanSlotAssignment.tray_id == tray_id,
666+
)
667+
)
668+
slot = slot_result.scalar_one_or_none()
669+
if not slot:
670+
return None
671+
672+
try:
673+
async with _translate_spoolman_errors():
674+
spool = await client.get_spool(slot.spoolman_spool_id)
675+
return _map_spoolman_spool(spool)
676+
except HTTPException as exc:
677+
if exc.status_code != 404:
678+
raise
679+
# Spool deleted in Spoolman — clean up stale assignment
680+
await db.execute(
681+
delete(SpoolmanSlotAssignment).where(SpoolmanSlotAssignment.id == slot.id)
682+
)
683+
await db.commit()
684+
return None

backend/app/core/database.py

Lines changed: 92 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ async def init_db():
189189
spool_k_profile,
190190
spool_usage_history,
191191
spoolbuddy_device,
192+
spoolman_slot_assignment,
192193
user,
193194
user_email_pref,
194195
user_otp_code,
@@ -1062,25 +1063,36 @@ async def run_migrations(conn):
10621063
pass # Already applied
10631064

10641065
# Create active_print_spoolman table for Spoolman per-filament tracking
1065-
try:
1066-
async with conn.begin_nested():
1067-
await conn.execute(
1068-
text("""
1069-
CREATE TABLE IF NOT EXISTS active_print_spoolman (
1070-
id INTEGER PRIMARY KEY AUTOINCREMENT,
1071-
printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE,
1072-
archive_id INTEGER NOT NULL REFERENCES print_archives(id) ON DELETE CASCADE,
1073-
filament_usage TEXT NOT NULL,
1074-
ams_trays TEXT NOT NULL,
1075-
slot_to_tray TEXT,
1076-
layer_usage TEXT,
1077-
filament_properties TEXT,
1078-
UNIQUE(printer_id, archive_id)
1079-
)
1080-
""")
1081-
)
1082-
except (OperationalError, ProgrammingError):
1083-
pass # Already applied
1066+
await _safe_execute(
1067+
conn,
1068+
"""
1069+
CREATE TABLE IF NOT EXISTS active_print_spoolman (
1070+
id INTEGER PRIMARY KEY AUTOINCREMENT,
1071+
printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE,
1072+
archive_id INTEGER NOT NULL REFERENCES print_archives(id) ON DELETE CASCADE,
1073+
filament_usage TEXT NOT NULL,
1074+
ams_trays TEXT NOT NULL,
1075+
slot_to_tray TEXT,
1076+
layer_usage TEXT,
1077+
filament_properties TEXT,
1078+
UNIQUE(printer_id, archive_id)
1079+
)
1080+
"""
1081+
if is_sqlite()
1082+
else """
1083+
CREATE TABLE IF NOT EXISTS active_print_spoolman (
1084+
id SERIAL PRIMARY KEY,
1085+
printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE,
1086+
archive_id INTEGER NOT NULL REFERENCES print_archives(id) ON DELETE CASCADE,
1087+
filament_usage TEXT NOT NULL,
1088+
ams_trays TEXT NOT NULL,
1089+
slot_to_tray TEXT,
1090+
layer_usage TEXT,
1091+
filament_properties TEXT,
1092+
UNIQUE(printer_id, archive_id)
1093+
)
1094+
""",
1095+
)
10841096

10851097
# Migration: Add preset_source column to slot_preset_mappings for local preset support
10861098
try:
@@ -1109,24 +1121,34 @@ async def run_migrations(conn):
11091121
await _safe_execute(conn, "ALTER TABLE spool ADD COLUMN core_weight_catalog_id INTEGER")
11101122

11111123
# Migration: Create spool_usage_history table for filament consumption tracking
1112-
try:
1113-
async with conn.begin_nested():
1114-
await conn.execute(
1115-
text("""
1116-
CREATE TABLE IF NOT EXISTS spool_usage_history (
1117-
id INTEGER PRIMARY KEY AUTOINCREMENT,
1118-
spool_id INTEGER NOT NULL REFERENCES spool(id) ON DELETE CASCADE,
1119-
printer_id INTEGER REFERENCES printers(id) ON DELETE SET NULL,
1120-
print_name VARCHAR(500),
1121-
weight_used REAL NOT NULL DEFAULT 0,
1122-
percent_used INTEGER NOT NULL DEFAULT 0,
1123-
status VARCHAR(20) NOT NULL DEFAULT 'completed',
1124-
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
1125-
)
1126-
""")
1127-
)
1128-
except (OperationalError, ProgrammingError):
1129-
pass # Already applied
1124+
await _safe_execute(
1125+
conn,
1126+
"""
1127+
CREATE TABLE IF NOT EXISTS spool_usage_history (
1128+
id INTEGER PRIMARY KEY AUTOINCREMENT,
1129+
spool_id INTEGER NOT NULL REFERENCES spool(id) ON DELETE CASCADE,
1130+
printer_id INTEGER REFERENCES printers(id) ON DELETE SET NULL,
1131+
print_name VARCHAR(500),
1132+
weight_used REAL NOT NULL DEFAULT 0,
1133+
percent_used INTEGER NOT NULL DEFAULT 0,
1134+
status VARCHAR(20) NOT NULL DEFAULT 'completed',
1135+
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
1136+
)
1137+
"""
1138+
if is_sqlite()
1139+
else """
1140+
CREATE TABLE IF NOT EXISTS spool_usage_history (
1141+
id SERIAL PRIMARY KEY,
1142+
spool_id INTEGER NOT NULL REFERENCES spool(id) ON DELETE CASCADE,
1143+
printer_id INTEGER REFERENCES printers(id) ON DELETE SET NULL,
1144+
print_name VARCHAR(500),
1145+
weight_used REAL NOT NULL DEFAULT 0,
1146+
percent_used INTEGER NOT NULL DEFAULT 0,
1147+
status VARCHAR(20) NOT NULL DEFAULT 'completed',
1148+
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
1149+
)
1150+
""",
1151+
)
11301152

11311153
# Migration: Add open_in_new_tab column to external_links
11321154
await _safe_execute(conn, "ALTER TABLE external_links ADD COLUMN open_in_new_tab BOOLEAN DEFAULT 0")
@@ -1162,8 +1184,9 @@ async def run_migrations(conn):
11621184
await _safe_execute(conn, "ALTER TABLE spool ADD COLUMN storage_location VARCHAR(255)")
11631185
# Migration: Widen tag_uid column from VARCHAR(16) to VARCHAR(32) to accommodate 7-byte NFC
11641186
# UIDs (14 hex chars) in addition to 8-byte Bambu Lab UIDs (16 hex chars).
1165-
# SQLite ignores VARCHAR sizes; this is a no-op there but needed for PostgreSQL.
1166-
await _safe_execute(conn, "ALTER TABLE spool ALTER COLUMN tag_uid TYPE VARCHAR(32)")
1187+
# ALTER COLUMN ... TYPE is PostgreSQL-only syntax; SQLite ignores VARCHAR sizes so no-op there.
1188+
if not is_sqlite():
1189+
await _safe_execute(conn, "ALTER TABLE spool ALTER COLUMN tag_uid TYPE VARCHAR(32)")
11671190
# Migration: Add cost field to spool_usage_history table
11681191
await _safe_execute(conn, "ALTER TABLE spool_usage_history ADD COLUMN cost REAL")
11691192
# Migration: Add archive_id field to spool_usage_history table
@@ -1677,6 +1700,36 @@ async def run_migrations(conn):
16771700
)
16781701
)
16791702

1703+
# Migration: Create spoolman_slot_assignments table for local AMS-slot→Spoolman-spool mapping.
1704+
# Replaces the pattern of writing spool.location in Spoolman (which polluted the
1705+
# user-editable storage_location field in the UI).
1706+
await _safe_execute(
1707+
conn,
1708+
"""
1709+
CREATE TABLE IF NOT EXISTS spoolman_slot_assignments (
1710+
id INTEGER PRIMARY KEY AUTOINCREMENT,
1711+
printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE,
1712+
ams_id INTEGER NOT NULL,
1713+
tray_id INTEGER NOT NULL,
1714+
spoolman_spool_id INTEGER NOT NULL,
1715+
assigned_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
1716+
UNIQUE(printer_id, ams_id, tray_id)
1717+
)
1718+
"""
1719+
if is_sqlite()
1720+
else """
1721+
CREATE TABLE IF NOT EXISTS spoolman_slot_assignments (
1722+
id SERIAL PRIMARY KEY,
1723+
printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE,
1724+
ams_id INTEGER NOT NULL,
1725+
tray_id INTEGER NOT NULL,
1726+
spoolman_spool_id INTEGER NOT NULL,
1727+
assigned_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
1728+
UNIQUE(printer_id, ams_id, tray_id)
1729+
)
1730+
""",
1731+
)
1732+
16801733
# Seed default settings keys that must exist on fresh install
16811734
default_settings = [
16821735
("advanced_auth_enabled", "false"),

0 commit comments

Comments
 (0)