ソースを参照

Fix virtual printer config changes ignored on running instances

  sync_from_db() skipped VPs already in self._instances without checking
  if their config had changed. Mode, model, access code, bind IP, remote
  interface IP, and target printer changes were silently ignored until
  manual toggle off/on or full restart. Now detects config drift and
  restarts affected instances.
maziggy 6 ヶ月 前
コミット
f39464d073

+ 1 - 0
CHANGELOG.md

@@ -14,6 +14,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **SpoolBuddy Kiosk Auth Bypass via API Key** — When Bambuddy auth is enabled, the SpoolBuddy kiosk (Chromium on RPi) was redirected to the login page because the `ProtectedRoute` requires a user object from `GET /auth/me`, which only accepted JWT tokens. The `/auth/me` endpoint now also accepts API keys (via `Authorization: Bearer bb_xxx` or `X-API-Key` header) and returns a synthetic admin user with all permissions. The frontend's `AuthContext` reads an optional `?token=` URL parameter on first load, stores it in localStorage, and strips it from the URL to prevent leakage via browser history or referrer. The install script now includes the API key in the kiosk URL (`/spoolbuddy?token=${API_KEY}`), so the device authenticates automatically on boot without manual login.
 - **SpoolBuddy Kiosk Auth Bypass via API Key** — When Bambuddy auth is enabled, the SpoolBuddy kiosk (Chromium on RPi) was redirected to the login page because the `ProtectedRoute` requires a user object from `GET /auth/me`, which only accepted JWT tokens. The `/auth/me` endpoint now also accepts API keys (via `Authorization: Bearer bb_xxx` or `X-API-Key` header) and returns a synthetic admin user with all permissions. The frontend's `AuthContext` reads an optional `?token=` URL parameter on first load, stores it in localStorage, and strips it from the URL to prevent leakage via browser history or referrer. The install script now includes the API key in the kiosk URL (`/spoolbuddy?token=${API_KEY}`), so the device authenticates automatically on boot without manual login.
 
 
 ### Fixed
 ### Fixed
+- **Virtual Printer Config Changes Ignored Until Toggle Off/On** — Changing a virtual printer's mode (e.g. proxy → archive), model, access code, bind IP, remote interface IP, or target printer via the UI updated the database but the running VP instance was never restarted. `sync_from_db()` skipped any VP whose ID was already in the running instances dict without checking if config had changed. Now compares critical fields between the running instance and DB record and restarts the VP when a difference is detected.
 - **Sidebar Navigation Ignores User Permissions** — All sidebar navigation items (Archives, Queue, Stats, Profiles, Maintenance, Projects, Inventory, Files) were visible to every user regardless of their role's permissions. Only the Settings item was permission-gated. Now each nav item is hidden when the user lacks the corresponding read permission (e.g., `archives:read`, `queue:read`, `library:read`). The Printers item remains always visible as the home page. Also added the missing `inventory:read|create|update|delete` permissions to the frontend Permission type (they existed in the backend but were absent from the frontend type definition).
 - **Sidebar Navigation Ignores User Permissions** — All sidebar navigation items (Archives, Queue, Stats, Profiles, Maintenance, Projects, Inventory, Files) were visible to every user regardless of their role's permissions. Only the Settings item was permission-gated. Now each nav item is hidden when the user lacks the corresponding read permission (e.g., `archives:read`, `queue:read`, `library:read`). The Printers item remains always visible as the home page. Also added the missing `inventory:read|create|update|delete` permissions to the frontend Permission type (they existed in the backend but were absent from the frontend type definition).
 - **Camera Button Clickable Without Permission & ffmpeg Process Leak** ([#550](https://github.com/maziggy/bambuddy/issues/550)) — Two camera issues in multi-user environments (e.g., classrooms with multiple printers). First, the camera button on the printer card was clickable even when the user's role lacked `camera:view` permission. Now disabled with a permission tooltip, matching the existing pattern for `printers:control` on the chamber light button. Second, ffmpeg processes (~240MB each) were never cleaned up after closing a camera stream. The `stop_camera_stream` endpoint called `terminate()` but never `wait()`ed or `kill()`ed, and HTTP disconnect detection in the streaming response only checked between frames — if the generator was blocked reading from ffmpeg stdout, disconnect was never detected (due to TCP send buffer masking the closed connection). Three fixes: (1) the stop endpoint now uses `terminate()` → `wait(2s)` → `kill()` → `wait()`; (2) each stream gets a background disconnect monitor task that polls `request.is_disconnected()` every 2 seconds independently of the frame loop, directly killing the ffmpeg process on disconnect; (3) a periodic cleanup (every 60s) scans `/proc` for any ffmpeg process with a Bambu RTSP URL (`rtsps://bblp:`) that isn't in an active stream and `SIGKILL`s it — catching orphans that survive app restarts or generator abandonment.
 - **Camera Button Clickable Without Permission & ffmpeg Process Leak** ([#550](https://github.com/maziggy/bambuddy/issues/550)) — Two camera issues in multi-user environments (e.g., classrooms with multiple printers). First, the camera button on the printer card was clickable even when the user's role lacked `camera:view` permission. Now disabled with a permission tooltip, matching the existing pattern for `printers:control` on the chamber light button. Second, ffmpeg processes (~240MB each) were never cleaned up after closing a camera stream. The `stop_camera_stream` endpoint called `terminate()` but never `wait()`ed or `kill()`ed, and HTTP disconnect detection in the streaming response only checked between frames — if the generator was blocked reading from ffmpeg stdout, disconnect was never detected (due to TCP send buffer masking the closed connection). Three fixes: (1) the stop endpoint now uses `terminate()` → `wait(2s)` → `kill()` → `wait()`; (2) each stream gets a background disconnect monitor task that polls `request.is_disconnected()` every 2 seconds independently of the frame loop, directly killing the ffmpeg process on disconnect; (3) a periodic cleanup (every 60s) scans `/proc` for any ffmpeg process with a Bambu RTSP URL (`rtsps://bblp:`) that isn't in an active stream and `SIGKILL`s it — catching orphans that survive app restarts or generator abandonment.
 - **Windows Install Fails With "Syntax of the Command Is Incorrect"** ([#544](https://github.com/maziggy/bambuddy/issues/544)) — The `start_bambuddy.bat` Python hash verification used a multi-line `for /f "usebackq"` with a backtick-delimited command split across lines. Windows CMD cannot parse line breaks inside backtick-delimited `for /f` commands, causing "The syntax of the command is incorrect" immediately after downloading Python. The entire block was also redundant — it downloaded a separate checksum file from python.org and re-verified the hash, but `verify_sha256` had already checked the archive against the pinned hash on the previous line. Removed the duplicate verification block. Also had a secondary bug: always downloaded the `amd64` checksum even on `arm64` systems.
 - **Windows Install Fails With "Syntax of the Command Is Incorrect"** ([#544](https://github.com/maziggy/bambuddy/issues/544)) — The `start_bambuddy.bat` Python hash verification used a multi-line `for /f "usebackq"` with a backtick-delimited command split across lines. Windows CMD cannot parse line breaks inside backtick-delimited `for /f` commands, causing "The syntax of the command is incorrect" immediately after downloading Python. The entire block was also redundant — it downloaded a separate checksum file from python.org and re-verified the hash, but `verify_sha256` had already checked the archive against the pinned hash on the previous line. Removed the duplicate verification block. Also had a secondary bug: always downloaded the `amd64` checksum even on `arm64` systems.

+ 24 - 0
backend/app/services/virtual_printer/manager.py

@@ -626,6 +626,30 @@ class VirtualPrinterManager:
                         if printer:
                         if printer:
                             proxy_ips[pvp.id] = (printer.ip_address, printer.serial_number)
                             proxy_ips[pvp.id] = (printer.ip_address, printer.serial_number)
 
 
+        # Detect config changes on running instances and restart if needed
+        for vp in enabled_vps:
+            instance = self._instances.get(vp.id)
+            if not instance:
+                continue
+
+            changed = (
+                instance.mode != vp.mode
+                or instance.model != (vp.model or DEFAULT_VIRTUAL_PRINTER_MODEL)
+                or instance.access_code != (vp.access_code or "")
+                or instance.bind_ip != (vp.bind_ip or "")
+                or instance.remote_interface_ip != (vp.remote_interface_ip or "")
+                or instance.target_printer_id != vp.target_printer_id
+            )
+
+            if changed:
+                logger.info(
+                    "VP %s config changed (mode: %s→%s), restarting",
+                    instance.name,
+                    instance.mode,
+                    vp.mode,
+                )
+                await self.remove_instance(vp.id)
+
         # Start instances for all enabled VPs (skip already running)
         # Start instances for all enabled VPs (skip already running)
         for vp in enabled_vps:
         for vp in enabled_vps:
             if vp.id in self._instances:
             if vp.id in self._instances:

+ 186 - 0
backend/tests/unit/services/test_virtual_printer.py

@@ -310,6 +310,192 @@ class TestVirtualPrinterManager:
         await manager.stop_all()
         await manager.stop_all()
         assert len(manager._instances) == 0
         assert len(manager._instances) == 0
 
 
+    # ========================================================================
+    # Tests for sync_from_db config change detection
+    # ========================================================================
+
+    def _make_db_vp(self, **overrides):
+        """Create a mock VirtualPrinter DB object."""
+        defaults = {
+            "id": 1,
+            "name": "TestVP",
+            "enabled": True,
+            "mode": "immediate",
+            "model": "C11",
+            "access_code": "12345678",
+            "serial_suffix": "391800001",
+            "bind_ip": "",
+            "remote_interface_ip": "",
+            "target_printer_id": None,
+            "position": 0,
+        }
+        defaults.update(overrides)
+        vp = MagicMock()
+        for k, v in defaults.items():
+            setattr(vp, k, v)
+        return vp
+
+    def _setup_sync_mocks(self, manager, enabled_vps, tmp_path):
+        """Wire up session_factory mock for sync_from_db."""
+        mock_result = MagicMock()
+        mock_result.scalars.return_value.all.return_value = enabled_vps
+
+        mock_db = AsyncMock()
+        mock_db.execute = AsyncMock(return_value=mock_result)
+        mock_db.__aenter__ = AsyncMock(return_value=mock_db)
+        mock_db.__aexit__ = AsyncMock(return_value=False)
+
+        manager._session_factory = MagicMock(return_value=mock_db)
+        manager._base_dir = tmp_path
+
+    @pytest.mark.asyncio
+    async def test_sync_from_db_restarts_on_mode_change(self, manager, tmp_path):
+        """Verify sync_from_db restarts VP when mode changes."""
+        from backend.app.services.virtual_printer.manager import VirtualPrinterInstance
+
+        inst = VirtualPrinterInstance(
+            vp_id=1,
+            name="TestVP",
+            mode="immediate",
+            model="C11",
+            access_code="12345678",
+            serial_suffix="391800001",
+            base_dir=tmp_path,
+        )
+        inst.stop_server = AsyncMock()
+        manager._instances[1] = inst
+
+        # DB says mode changed to "archive"
+        db_vp = self._make_db_vp(mode="archive")
+        self._setup_sync_mocks(manager, [db_vp], tmp_path)
+
+        with patch.object(manager, "remove_instance", new_callable=AsyncMock) as mock_remove:
+            # Patch VirtualPrinterInstance to prevent actual start
+            with patch("backend.app.services.virtual_printer.manager.VirtualPrinterInstance") as MockInst:
+                mock_new = MagicMock()
+                mock_new.start_server = AsyncMock()
+                MockInst.return_value = mock_new
+
+                await manager.sync_from_db()
+
+            mock_remove.assert_called_once_with(1)
+
+    @pytest.mark.asyncio
+    async def test_sync_from_db_restarts_on_access_code_change(self, manager, tmp_path):
+        """Verify sync_from_db restarts VP when access_code changes."""
+        from backend.app.services.virtual_printer.manager import VirtualPrinterInstance
+
+        inst = VirtualPrinterInstance(
+            vp_id=1,
+            name="TestVP",
+            mode="immediate",
+            model="C11",
+            access_code="12345678",
+            serial_suffix="391800001",
+            base_dir=tmp_path,
+        )
+        inst.stop_server = AsyncMock()
+        manager._instances[1] = inst
+
+        db_vp = self._make_db_vp(access_code="newcode99")
+        self._setup_sync_mocks(manager, [db_vp], tmp_path)
+
+        with patch.object(manager, "remove_instance", new_callable=AsyncMock) as mock_remove:
+            with patch("backend.app.services.virtual_printer.manager.VirtualPrinterInstance") as MockInst:
+                mock_new = MagicMock()
+                mock_new.start_server = AsyncMock()
+                MockInst.return_value = mock_new
+
+                await manager.sync_from_db()
+
+            mock_remove.assert_called_once_with(1)
+
+    @pytest.mark.asyncio
+    async def test_sync_from_db_skips_unchanged_instance(self, manager, tmp_path):
+        """Verify sync_from_db does NOT restart when config is identical."""
+        from backend.app.services.virtual_printer.manager import VirtualPrinterInstance
+
+        inst = VirtualPrinterInstance(
+            vp_id=1,
+            name="TestVP",
+            mode="immediate",
+            model="C11",
+            access_code="12345678",
+            serial_suffix="391800001",
+            base_dir=tmp_path,
+        )
+        manager._instances[1] = inst
+
+        # DB matches running config exactly
+        db_vp = self._make_db_vp()
+        self._setup_sync_mocks(manager, [db_vp], tmp_path)
+
+        with patch.object(manager, "remove_instance", new_callable=AsyncMock) as mock_remove:
+            await manager.sync_from_db()
+
+            mock_remove.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_sync_from_db_restarts_on_bind_ip_change(self, manager, tmp_path):
+        """Verify sync_from_db restarts VP when bind_ip changes."""
+        from backend.app.services.virtual_printer.manager import VirtualPrinterInstance
+
+        inst = VirtualPrinterInstance(
+            vp_id=1,
+            name="TestVP",
+            mode="immediate",
+            model="C11",
+            access_code="12345678",
+            serial_suffix="391800001",
+            bind_ip="192.168.1.10",
+            base_dir=tmp_path,
+        )
+        inst.stop_server = AsyncMock()
+        manager._instances[1] = inst
+
+        db_vp = self._make_db_vp(bind_ip="192.168.1.20")
+        self._setup_sync_mocks(manager, [db_vp], tmp_path)
+
+        with patch.object(manager, "remove_instance", new_callable=AsyncMock) as mock_remove:
+            with patch("backend.app.services.virtual_printer.manager.VirtualPrinterInstance") as MockInst:
+                mock_new = MagicMock()
+                mock_new.start_server = AsyncMock()
+                MockInst.return_value = mock_new
+
+                await manager.sync_from_db()
+
+            mock_remove.assert_called_once_with(1)
+
+    @pytest.mark.asyncio
+    async def test_sync_from_db_restarts_on_model_change(self, manager, tmp_path):
+        """Verify sync_from_db restarts VP when model changes."""
+        from backend.app.services.virtual_printer.manager import VirtualPrinterInstance
+
+        inst = VirtualPrinterInstance(
+            vp_id=1,
+            name="TestVP",
+            mode="immediate",
+            model="C11",
+            access_code="12345678",
+            serial_suffix="391800001",
+            base_dir=tmp_path,
+        )
+        inst.stop_server = AsyncMock()
+        manager._instances[1] = inst
+
+        db_vp = self._make_db_vp(model="C12")
+        self._setup_sync_mocks(manager, [db_vp], tmp_path)
+
+        with patch.object(manager, "remove_instance", new_callable=AsyncMock) as mock_remove:
+            with patch("backend.app.services.virtual_printer.manager.VirtualPrinterInstance") as MockInst:
+                mock_new = MagicMock()
+                mock_new.start_server = AsyncMock()
+                MockInst.return_value = mock_new
+
+                await manager.sync_from_db()
+
+            mock_remove.assert_called_once_with(1)
+
 
 
 class TestFTPSession:
 class TestFTPSession:
     """Tests for FTP session handling."""
     """Tests for FTP session handling."""