diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d92c9ced..0172fb2ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ All notable changes to Bambuddy will be documented in this file. - **Inventory: `/reset-usage` renamed to `/reset-consumed-counter`; UI label is now "Reset counter"** — The old endpoint name implied that calling it would drop `weight_used` to 0; in practice it only stamps `weight_used_baseline = weight_used` so the Inventory page's "Total Consumed" widget (which renders `weight_used - baseline`) reads 0 going forward, while remaining (`label_weight - weight_used`) is preserved. Calling the endpoint via curl and seeing `weight_used` unchanged in the JSON response is confusing — the name didn't describe what the endpoint actually does. New paths: internal `/api/v1/inventory/spools/{id}/reset-consumed-counter` + `/spools/reset-consumed-counter-bulk`, Spoolman-mode `/api/v1/spoolman/inventory/spools/{id}/reset-consumed-counter` + `/spools/reset-consumed-counter-bulk`. **Behaviour is unchanged in both modes**: internal stamps the baseline directly; Spoolman-mode PATCHes upstream `used_weight=0` and the `_map_spoolman_spool` read mapping at `_spoolman_helpers.py:252-268` reconstructs the same `displayed consumed = 0, remaining unchanged` Bambuddy-visible shape — Bambuddy-side parity between modes (per [[feedback_inventory_modes_parity]]) was already in place before this rename and is preserved. The Spoolman-client method `reset_spool_usage` keeps its name because it describes what's sent upstream to Spoolman, which has not been renamed. **Frontend**: `api.resetSpoolUsage` / `bulkResetSpoolUsage` (and Spoolman variants) renamed to `resetSpoolConsumedCounter` / `bulkResetSpoolConsumedCounter`. Button labels switch from "Reset usage to 0" to "Reset counter" / "Reset all counters" — short and unambiguous; tooltips and confirm-modal bodies still spell out the full semantics ("zero the consumed-grams counter; remaining weight is not changed"). **i18n**: 9 keys renamed (`resetUsage*`, `resetAllUsage*`, `usageReset`, `allUsageReset`, `resetUsageFailed` → `resetConsumedCounter*`, `resetAllConsumedCounters*`, `consumedCounterReset`, `allConsumedCountersReset`, `resetConsumedCounterFailed`); real translations shipped in all 10 non-English locales (de / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW) per the hard-rule against English fallbacks. **Tests**: `test_spool_reset_usage.py` (9 tests, internal mode) and 3 reset-related tests in `test_spoolman_inventory_api.py` updated to hit the new paths; behavioural assertions unchanged. **Breaking change for external API consumers** that already wired `/reset-usage` — no compat shim shipped because the old name actively misled callers; the migration is a one-line URL swap. ### Fixed +- **MakerWorld URL imports into a writable external folder wrote bytes to internal storage, not the NAS (#1645, reported and root-caused by @needo37)** — Reporter linked a writable external SMB folder, selected it as the destination in the MakerWorld import dialog, the import succeeded, the file card appeared in the File Manager under the external folder's view — but `ls` on the NAS turned up nothing, and a `find` across the whole NAS and the container for the original filename matched nothing either. The bytes had landed in Bambuddy's internal `/archive/library/files/.3mf` instead of `/` on the mount. Root cause was the byte-import save helper `save_3mf_bytes_to_library` at `backend/app/api/routes/library.py:422`: it accepts `folder_id` but never loaded the folder or inspected `is_external` / `external_path`, hardcoded the destination to `get_library_files_dir() / `, and left the `LibraryFile` row with `is_external=False`. So the row's `folder_id` pointed at the external folder while its bytes + `is_external` flag both said "managed/internal" — exact same class of bug as #1112 (which got fixed for the multipart-upload and move paths but never applied to the byte-import path). Compounded by the UUID-renamed on-disk copy: searching for the human-readable basename anywhere — NAS or container — never matches. **Fix** is a direct mirror of the multipart-upload path that's done this correctly since #1112: load the target `LibraryFolder` (when `folder_id` is non-None), feed it to the existing `_resolve_upload_destination(target_folder, filename)` helper which already produces `(dest, is_external)` and enforces the 403-read-only / 400-unwritable-or-missing / 409-collision rejections, write the bytes to that destination (real filename for external, UUID for managed), and persist the row via `_stored_file_path(dest, is_external)` + `is_external=is_external`. The route-layer read-only guard at `makerworld.py:256-260` is preserved — it returns the friendlier error before the upstream download burns bandwidth — and `_resolve_upload_destination`'s identical check stays as defence-in-depth for any future caller that skips the route gate. Thumbnails continue to live under the managed `get_library_thumbnails_dir()` regardless of the 3MF's location, matching the upload path. **Tests**: 4 new in `TestImport` (writable external → bytes on mount + `is_external=True` + absolute file_path persisted; read-only external → 403 at route, no download; missing external_path → 400; filename collision → 409 with the pre-existing file's bytes untouched). 21 existing makerworld tests + 72 library-route tests stay green. Ruff clean. + - **X2D archives lose 3MF metadata because FTPS handshake fails on firmware 01.01.00.00 (#1638, reported by @vasmarfas)** — Reporter's first archive entries from a brand-new X2D landed almost empty (only print time visible, no filament weight / layers / MakerWorld link / thumbnail), and Spoolman filament-usage tracking also went silent. The support bundle traces the symptom end-to-end: at print start `backend/app/main.py::on_print_start` tries the usual FTP-download dance for the 3MF, every connect attempt to the printer fails with `[SSL: WRONG_VERSION_NUMBER] wrong version number (_ssl.c:1032)`, and ~2 minutes later `Could not find 3MF file for print: /data/Metadata/plate_1.gcode` → `Created fallback archive N for (no 3MF available)`. The fallback path writes the row with `file_path=""`, `file_size=0`, `content_hash=NULL`, and no layers / filament / model-link fields — exactly the "almost empty card" in the reporter's screenshot. Spoolman tracking and reprint-grouping also degrade from the same root cause: both depend on metadata pulled out of the 3MF by `ThreeMFParser`. The proximate cause is the FTPS handshake: Python 3.13's default `ssl.create_default_context()` negotiates TLS 1.3, and the X2D's implicit-FTPS server on port 990 rejects the ClientHello with `WRONG_VERSION_NUMBER`. This is the same shape of symptom as the P2S 01.02.00.00 FTPS bug from #1401 — handshake / data-channel breakage triggered by the move to Python 3.13's TLS-1.3 default — but the wire-level failure mode is different (P2S completes the handshake and truncates mid-stream with 426; X2D fails the handshake outright). Both are addressed via the per-model registry that #1401 established: `backend/app/services/ftp_profiles.py` gains an `X2D` entry with `cap_tls_v1_2=True` plus a `N6 → X2D` SSDP alias, so the X2D's `ImplicitFTP_TLS` connection caps the SSL context's `maximum_version` to TLS 1.2 and the ClientHello looks like the one the firmware accepted before the Python upgrade. Deliberately conservative — every other model stays on negotiated TLS 1.3, only X2D-tagged sessions flip. **Honest caveat**: this ships as a hypothesis-driven trial rather than a confirmed root-cause fix. The TLS-1.2 cap is the most likely cure given the symptom's family resemblance to #1401, but `WRONG_VERSION_NUMBER` could equally describe the X2D switching to explicit FTPS (AUTH TLS on a plaintext greeting) or moving the FTPS service to a different port — both would need a different code path. The reporter has been asked to test this build; if the cap doesn't clear the error, the registry slot stays useful as a tuning anchor and the next round of diagnostics (`openssl s_client -connect :990 -tls1_2` from a network-adjacent host) will tell us which of (2)/(3) applies. **Tests**: 3 new in `test_ftp_profiles.py` mirroring the existing P2S coverage — `X2D` resolves to `cap_tls_v1_2=True`, `N6` SSDP code aliases to the X2D profile, lowercase `x2d` still hits the cap. Existing P2S + default + unknown-model + frozen-dataclass + non-capped-spot-check (X1C / H2D / P1S / A1) tests stay green. **Verified**: ruff clean; the integration test at `test_cap_tls_v1_2_actually_applied_to_ssl_context` already pins the profile→`ImplicitFTP_TLS`→`ssl_context.maximum_version` wiring so this entry can't silently fail to apply. - **Label printing produced two identical PDFs per click (#1628)** — `LabelTemplatePickerModal.tsx::openBlobInNewTab` called `window.open(url, '_blank', 'noopener,noreferrer')` and treated a `null` return as "popup blocked → fall back to `` click." Per the WindowFeatures spec, `noopener` deliberately forces `window.open` to return `null` even on success, so the `if (!win)` fallback fired on EVERY click. Path 1 (window.open) opened the blob tab — on Linux Chromium without an inline PDF viewer the OS saved a random-named copy (the `zo70GhSL.pdf` / `f7w0OcDi.pdf` files in the reporter's screenshot). Path 2 (fallback) downloaded a second copy named `bambuddy-labels.pdf`. Two identical PDFs per click. Fix: drop `noopener,noreferrer`. The blob is same-origin (created via `URL.createObjectURL` from our own fetch response), the destination is a passive PDF preview tab with no script context to abuse `window.opener`, and `noreferrer` is a no-op for blob URLs. After removal, `window.open` returns a real window reference on success → `if (!win)` only fires on genuine popup-block, single PDF per click. Existing 17 vitest cases in `LabelTemplatePickerModal.test.tsx` still pass; the change is comment + one parameter. diff --git a/backend/app/api/routes/library.py b/backend/app/api/routes/library.py index a4e09060b..4e92d05cd 100644 --- a/backend/app/api/routes/library.py +++ b/backend/app/api/routes/library.py @@ -449,13 +449,21 @@ async def save_3mf_bytes_to_library( if existing_row is not None: return existing_row, True - # Persist bytes to disk under a UUID-scoped filename; keep the original - # extension so downstream logic (ThreeMFParser, thumbnail viewer) works. - ext = os.path.splitext(filename)[1].lower() or ".3mf" - unique_filename = f"{uuid.uuid4().hex}{ext}" - file_path = ( - get_library_files_dir() / unique_filename - ) # SEC-PATH-OK: unique_filename = uuid.uuid4().hex + ext, generated on the previous line + # Resolve target folder so writable-external destinations land on the + # mount with the real filename, instead of being silently misrouted to + # the internal library dir with a UUID name (#1645). Mirrors what the + # multipart-upload path has done since #1112. ``_resolve_upload_destination`` + # also enforces the 403 read-only / 400 unwritable / 409 collision + # rejections — the makerworld route layer already pre-checks read-only, + # but the helper's checks remain as defence-in-depth for any future + # caller that skips that route gate. + target_folder: LibraryFolder | None = None + if folder_id is not None: + folder_q = await db.execute(select(LibraryFolder).where(LibraryFolder.id == folder_id)) + target_folder = folder_q.scalar_one_or_none() + + file_path, is_external = _resolve_upload_destination(target_folder, filename) + ext = file_path.suffix.lower() or ".3mf" with open(file_path, "wb") as fh: fh.write(file_bytes) @@ -486,8 +494,9 @@ async def save_3mf_bytes_to_library( library_file = LibraryFile( folder_id=folder_id, + is_external=is_external, filename=filename, - file_path=to_relative_path(file_path), + file_path=_stored_file_path(file_path, is_external), file_type=classify_file_type(filename), file_size=len(file_bytes), file_hash=file_hash, diff --git a/backend/tests/unit/test_makerworld_routes.py b/backend/tests/unit/test_makerworld_routes.py index 6ba6fdeaf..f1f18aad5 100644 --- a/backend/tests/unit/test_makerworld_routes.py +++ b/backend/tests/unit/test_makerworld_routes.py @@ -382,6 +382,151 @@ class TestImport: assert resp.status_code == 200, resp.text assert resp.json()["profile_id"] == 298919107 + @pytest.mark.asyncio + async def test_import_to_writable_external_writes_bytes_to_mount(self, async_client, db_session, tmp_path): + """#1645: importing into a writable external folder writes the bytes to + ``/`` and tags the row ``is_external=True`` — + same shape as the multipart-upload path (#1112). Previously the bytes + landed in the internal library dir under a UUID name while the row + showed up under the external folder in the UI, leaving a NAS/SMB user + unable to find their file on the mount.""" + ext_dir = tmp_path / "nas-makerworld" + ext_dir.mkdir() + folder = LibraryFolder( + name="NAS Imports", + parent_id=None, + is_external=True, + external_path=str(ext_dir), + external_readonly=False, + ) + db_session.add(folder) + await db_session.commit() + await db_session.refresh(folder) + + svc = _fake_service( + get_design=_default_design(), + get_profile_download=_default_manifest("seed-starter.3mf"), + download_3mf=(self._FAKE_3MF_BYTES, "seed-starter.3mf"), + ) + + with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "profile_id": 298919107, "folder_id": folder.id}, + ) + assert resp.status_code == 200, resp.text + + from sqlalchemy import select + + row = ( + await db_session.execute(select(LibraryFile).where(LibraryFile.id == resp.json()["library_file_id"])) + ).scalar_one() + assert row.folder_id == folder.id + assert row.is_external is True, "Row must be tagged external so re-scan can reconcile it" + # External rows persist the absolute mount path (matches scan + upload paths). + assert row.file_path == str(ext_dir / "seed-starter.3mf") + on_disk = ext_dir / "seed-starter.3mf" + assert on_disk.is_file(), "Bytes must land on the external mount, not in the internal library dir" + assert on_disk.read_bytes() == self._FAKE_3MF_BYTES + + @pytest.mark.asyncio + async def test_import_to_readonly_external_rejected_at_route(self, async_client, db_session, tmp_path): + """The route-layer gate at ``makerworld.py:256-260`` rejects read-only + externals with 403 before any download happens — so MakerWorld + credentials and the upstream download bandwidth aren't wasted.""" + ext_dir = tmp_path / "nas-readonly" + ext_dir.mkdir() + folder = LibraryFolder( + name="NAS read-only", + parent_id=None, + is_external=True, + external_path=str(ext_dir), + external_readonly=True, + ) + db_session.add(folder) + await db_session.commit() + await db_session.refresh(folder) + + svc = _fake_service( + get_design=_default_design(), + get_profile_download=_default_manifest(), + ) + svc.download_3mf = AsyncMock() + + with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "profile_id": 298919107, "folder_id": folder.id}, + ) + assert resp.status_code == 403, resp.text + svc.download_3mf.assert_not_called() + + @pytest.mark.asyncio + async def test_import_to_external_with_missing_path_returns_400(self, async_client, db_session, tmp_path): + """If the external folder's mount has gone away (NAS unplugged, SMB + share down), ``_resolve_upload_destination`` returns 400 before the + write so we don't silently fall back to the internal library dir.""" + missing_dir = tmp_path / "vanished-mount" # NOTE: deliberately not created + folder = LibraryFolder( + name="NAS gone", + parent_id=None, + is_external=True, + external_path=str(missing_dir), + external_readonly=False, + ) + db_session.add(folder) + await db_session.commit() + await db_session.refresh(folder) + + svc = _fake_service( + get_design=_default_design(), + get_profile_download=_default_manifest(), + download_3mf=(self._FAKE_3MF_BYTES, "benchy.3mf"), + ) + + with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "profile_id": 298919107, "folder_id": folder.id}, + ) + assert resp.status_code == 400, resp.text + assert "not accessible" in resp.text.lower() + + @pytest.mark.asyncio + async def test_import_to_external_with_name_collision_returns_409(self, async_client, db_session, tmp_path): + """A user-visible 409 fires when the filename already exists on the + external mount, instead of silently overwriting a file the user put + there outside Bambuddy.""" + ext_dir = tmp_path / "nas-collide" + ext_dir.mkdir() + (ext_dir / "benchy.3mf").write_bytes(b"pre-existing") + + folder = LibraryFolder( + name="NAS collide", + parent_id=None, + is_external=True, + external_path=str(ext_dir), + external_readonly=False, + ) + db_session.add(folder) + await db_session.commit() + await db_session.refresh(folder) + + svc = _fake_service( + get_design=_default_design(), + get_profile_download=_default_manifest("benchy.3mf"), + download_3mf=(self._FAKE_3MF_BYTES, "benchy.3mf"), + ) + + with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "profile_id": 298919107, "folder_id": folder.id}, + ) + assert resp.status_code == 409, resp.text + # Pre-existing file's contents must not be clobbered by the failed write. + assert (ext_dir / "benchy.3mf").read_bytes() == b"pre-existing" + class TestRecentImports: """GET /makerworld/recent-imports — sidebar feed on the MakerWorld page."""