Browse Source

fix(spoolbuddy): route weight sync by inventory mode exclusively (#1530)

  POST /spoolbuddy/scale/update-spool-weight tried the local DB first
  and only fell back to Spoolman on a local miss. Combined with
  nfc/tag-scanned's post-#1119 always-Spoolman routing, a stale local
  Spool row sharing a numeric id with a Spoolman spool would absorb
  the sync silently while the Spoolman row stayed unchanged.

  Mirror the routing already used by nfc/tag-scanned: pick the branch
  via _get_spoolman_client_or_none() and never cross. Local mode now
  returns 404 on a local miss instead of falling through.

  New TestUpdateSpoolWeightSpoolman.test_stale_local_row_does_not_shadow_spoolman
  asserts both directions: Spoolman gets the update, the colliding
  local row's weight_used and last_scale_weight are untouched.
maziggy 2 days ago
parent
commit
4387a09162
3 changed files with 52 additions and 10 deletions
  1. 0 0
      CHANGELOG.md
  2. 19 10
      backend/app/api/routes/spoolbuddy.py
  3. 33 0
      backend/tests/integration/test_spoolbuddy.py

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 19 - 10
backend/app/api/routes/spoolbuddy.py

@@ -867,15 +867,28 @@ async def update_spool_weight(
     db: AsyncSession = Depends(get_db),
     db: AsyncSession = Depends(get_db),
     _: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE),
     _: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE),
 ):
 ):
-    """Update spool's used weight from scale reading."""
+    """Update spool's used weight from scale reading.
+
+    Routes the update to whichever inventory backend Bambuddy is configured
+    for: Spoolman exclusively when ``spoolman_enabled`` is true, local DB
+    exclusively otherwise. The previous implementation tried local first and
+    only consulted Spoolman on a local-DB miss, which meant a stale local row
+    sharing a numeric id with a Spoolman spool would silently absorb the
+    update while the Spoolman row the user is actually looking at stayed
+    unchanged (#1530). Mirrors the routing already used by ``nfc/tag-scanned``.
+    """
     from backend.app.api.routes._spoolman_helpers import _safe_float
     from backend.app.api.routes._spoolman_helpers import _safe_float
     from backend.app.models.spool import Spool
     from backend.app.models.spool import Spool
 
 
-    # Try local DB first — local spool IDs must not be forwarded to Spoolman.
-    db_result = await db.execute(select(Spool).where(Spool.id == req.spool_id))
-    spool = db_result.scalar_one_or_none()
+    sm_client = await _get_spoolman_client_or_none(db)
+
+    if sm_client is None:
+        # Local mode — exclusive update, no Spoolman fallback.
+        db_result = await db.execute(select(Spool).where(Spool.id == req.spool_id))
+        spool = db_result.scalar_one_or_none()
+        if not spool:
+            raise HTTPException(status_code=404, detail="Spool not found")
 
 
-    if spool:
         net_filament = max(0, req.weight_grams - spool.core_weight)
         net_filament = max(0, req.weight_grams - spool.core_weight)
         spool.weight_used = max(0, spool.label_weight - net_filament)
         spool.weight_used = max(0, spool.label_weight - net_filament)
         spool.last_scale_weight = req.weight_grams
         spool.last_scale_weight = req.weight_grams
@@ -889,11 +902,7 @@ async def update_spool_weight(
         )
         )
         return {"status": "ok", "weight_used": spool.weight_used}
         return {"status": "ok", "weight_used": spool.weight_used}
 
 
-    # Local miss — fall back to Spoolman when enabled.
-    sm_client = await _get_spoolman_client_or_none(db)
-    if sm_client is None:
-        raise HTTPException(status_code=404, detail="Spool not found")
-
+    # Spoolman mode — exclusive update, never touch local DB.
     async with _translate_spoolbuddy_errors():
     async with _translate_spoolbuddy_errors():
         sm_spool = await sm_client.get_spool(req.spool_id)
         sm_spool = await sm_client.get_spool(req.spool_id)
 
 

+ 33 - 0
backend/tests/integration/test_spoolbuddy.py

@@ -1608,6 +1608,39 @@ class TestUpdateSpoolWeightSpoolman:
         assert resp.status_code == 200
         assert resp.status_code == 200
         assert resp.json()["weight_used"] == 500
         assert resp.json()["weight_used"] == 500
 
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_stale_local_row_does_not_shadow_spoolman(
+        self, async_client: AsyncClient, db_session, spool_factory, spoolman_settings
+    ):
+        """Regression for #1530: when Spoolman mode is on, a stale local Spool
+        sharing the same numeric id must NOT absorb the update — Spoolman is
+        the authoritative target."""
+        local_spool = await spool_factory(label_weight=1000, core_weight=250, weight_used=0)
+        # Spoolman spool with the SAME numeric id as the local stale row.
+        sm_spool = _spoolman_spool_fixture(local_spool.id, spool_weight=250.0, filament_weight=1000.0)
+        mock_client = _mock_spoolman_client()
+        mock_client.get_spool = AsyncMock(return_value=sm_spool)
+        mock_client.update_spool = AsyncMock(return_value=sm_spool)
+
+        with (
+            patch("backend.app.services.spoolman.get_spoolman_client", AsyncMock(return_value=mock_client)),
+            patch("backend.app.services.spoolman.init_spoolman_client", AsyncMock(return_value=mock_client)),
+        ):
+            resp = await async_client.post(
+                f"{API}/scale/update-spool-weight",
+                json={"spool_id": local_spool.id, "weight_grams": 750},
+            )
+
+        assert resp.status_code == 200
+        # Spoolman got the update.
+        mock_client.update_spool.assert_called_once_with(spool_id=local_spool.id, remaining_weight=pytest.approx(500.0))
+        # Local row is untouched — the bug was that the local update silently
+        # absorbed the request while Spoolman stayed stale.
+        await db_session.refresh(local_spool)
+        assert local_spool.weight_used == 0
+        assert local_spool.last_scale_weight is None
+
     @pytest.mark.asyncio
     @pytest.mark.asyncio
     @pytest.mark.integration
     @pytest.mark.integration
     async def test_spool_level_spool_weight_takes_priority(self, async_client: AsyncClient, spoolman_settings):
     async def test_spool_level_spool_weight_takes_priority(self, async_client: AsyncClient, spoolman_settings):

Some files were not shown because too many files changed in this diff