From e14e6c1e471aabbff1a2468cf40d50e982d165c4 Mon Sep 17 00:00:00 2001 From: Devon Jones Date: Tue, 7 Jul 2026 12:58:57 -0600 Subject: [PATCH 1/2] feat: Thingiverse v1 write-API client (openforge_catalog-gn2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - openforge/thingiverse/client.py: ThingiverseClient over the v1 write API on api.thingiverse.com β€” create/get/update/delete/publish thing, upload_file (multipart) + finalize_files (pending->committed), and get_thing_files with normalize_hash() (base64 OR hex MD5 -> hex, to match the catalog's file_md5). Auth via TokenManager.write_token(). - openforge/thingiverse/auth.py: add write_token() and persist the opaque 'token' (session_token) across JWT refresh (was dropped). - docs/thingiverse-api-v2-private.md: the reverse-engineered contract, confirmed live (create/delete round-tripped; upload+finalize shapes from swagger; hash=MD5 validated against thing:7364110 aztlan). - 22 client tests (100% cov) + 5 auth write-token tests. Contract confirmed live against devonjones's account; probe things created during discovery were deleted. Co-Authored-By: Claude Fable 5 πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .beads/issues.jsonl | 8 +- .beads/last-touched | 2 +- docs/thingiverse-api-v2-private.md | 109 ++++++++++++++ openforge/thingiverse/auth.py | 36 ++++- openforge/thingiverse/client.py | 203 +++++++++++++++++++++++++ tests/test_thingiverse_auth.py | 32 ++++ tests/test_thingiverse_client.py | 230 +++++++++++++++++++++++++++++ 7 files changed, 612 insertions(+), 8 deletions(-) create mode 100644 docs/thingiverse-api-v2-private.md create mode 100644 openforge/thingiverse/client.py create mode 100644 tests/test_thingiverse_client.py diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 056c680..80dd390 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -5,16 +5,16 @@ {"id":"openforge_catalog-5gb","title":"Frontend: Admin camera position controls UI","description":"Build admin interface to preview and set default camera angles for models. Shows live preview of each angle, allows selecting which is default. Integrates with admin set camera position API. Consider batch operations for setting defaults across similar models.","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-21T09:58:45.023938263-07:00","updated_at":"2025-12-21T09:58:45.023938263-07:00","dependencies":[{"issue_id":"openforge_catalog-5gb","depends_on_id":"openforge_catalog-bll","type":"parent-child","created_at":"2025-12-21T09:59:19.862145404-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-5gb","depends_on_id":"openforge_catalog-hqe","type":"blocks","created_at":"2025-12-21T09:59:36.561390068-07:00","created_by":"daemon"}]} {"id":"openforge_catalog-5na","title":"Phased rollout: Test subset before bulk backfill","description":"Implement safe migration strategy: (1) Generate sprite sheets for small test set (~10-20 diverse models). (2) Deploy frontend with fallback to legacy single thumbnails. (3) Validate with users - test interactive controls, image quality, performance. (4) Iterate on camera positions/layout based on feedback. (5) Only after validation, run bulk backfill for all 8000+ STL files. Include ability to pause/resume bulk processing. Track which models have sprite sheets vs legacy thumbnails.","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-21T10:13:57.517770238-07:00","updated_at":"2025-12-21T10:13:57.517770238-07:00","dependencies":[{"issue_id":"openforge_catalog-5na","depends_on_id":"openforge_catalog-bll","type":"parent-child","created_at":"2025-12-21T10:14:18.760726353-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-5na","depends_on_id":"openforge_catalog-4ji","type":"blocks","created_at":"2025-12-21T10:14:23.673079682-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-5na","depends_on_id":"openforge_catalog-3cx","type":"blocks","created_at":"2025-12-21T10:14:24.071569003-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-5na","depends_on_id":"openforge_catalog-dd4","type":"blocks","created_at":"2025-12-21T10:14:24.494018624-07:00","created_by":"daemon"}]} {"id":"openforge_catalog-5tg","title":"Backend API: Get thumbnail variants endpoint","description":"Create Flask endpoint to return sprite sheet info for a model. Returns: (1) Sprite sheet URL, (2) Grid layout (rows, cols, tile dimensions), (3) Angle metadata (name, position in grid, camera coords), (4) Default angle index. Frontend uses this to display correct region from sprite sheet. Handles models with legacy single thumbnail gracefully.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-21T09:58:32.287223899-07:00","updated_at":"2025-12-21T14:00:25.231595972-07:00","closed_at":"2025-12-21T14:00:25.231595972-07:00","close_reason":"Completed: Created GET /api/blueprints/\u003cblueprint_id\u003e/thumbnail-variants endpoint. Returns sprite sheet URL, grid layout (rows/cols/tile_size), angle metadata with camera positions, and default angle index. Gracefully handles legacy single thumbnails. All 319 tests passing.","dependencies":[{"issue_id":"openforge_catalog-5tg","depends_on_id":"openforge_catalog-bll","type":"parent-child","created_at":"2025-12-21T09:59:18.52350897-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-5tg","depends_on_id":"openforge_catalog-ubp","type":"blocks","created_at":"2025-12-21T09:59:35.26824895-07:00","created_by":"daemon"}]} -{"id":"openforge_catalog-750","title":"Postgres migration: thingiverse sync-state tables","description":"New schema version adding: thingiverse_thing (blueprint_id \u003c-\u003e thing_id, status, published_at, last_synced_at) and thingiverse_file (thing ref, catalog file ref + local hash, remote file id + remote hash, last_synced_at) β€” the diff ledger for the sync engine. Follow existing openforge/db/schema/version_NN.py migration pattern. If the HAR ticket proves the remote hash is SHA-256 (catalog stores MD5), include storage for the second hash algorithm.","notes":"SCOPE NOTES: (1) thingiverse_file must carry a file-type/role discriminator (model/image/zip/other) since things contain mixed file types. (2) This runs against the LOCAL db; prod gets a subset via the fixture path (see openforge_catalog-8yb) β€” keep the mapping (blueprint_id, thing_id, public_url) cleanly separable from local-only operational state (hash ledger, sync timestamps).","status":"in_progress","priority":2,"issue_type":"task","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:25.104764326-06:00","created_by":"Devon Jones","updated_at":"2026-07-06T14:40:56.467812707-06:00","dependencies":[{"issue_id":"openforge_catalog-750","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:52.476138903-06:00","created_by":"Devon Jones"}]} +{"id":"openforge_catalog-750","title":"Postgres migration: thingiverse sync-state tables","description":"New schema version adding: thingiverse_thing (blueprint_id \u003c-\u003e thing_id, status, published_at, last_synced_at) and thingiverse_file (thing ref, catalog file ref + local hash, remote file id + remote hash, last_synced_at) β€” the diff ledger for the sync engine. Follow existing openforge/db/schema/version_NN.py migration pattern. If the HAR ticket proves the remote hash is SHA-256 (catalog stores MD5), include storage for the second hash algorithm.","notes":"FROM hnr FINDINGS: Thingiverse's file hash is content-MD5 (32-hex), confirmed empirically. Catalog already stores file_md5, so thingiverse_files.local_md5 is the comparison column and local_sha256 is UNNECESSARY (dead/harmless column). If a follow-up migration touches this table, drop local_sha256.","status":"in_progress","priority":2,"issue_type":"task","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:25.104764326-06:00","created_by":"Devon Jones","updated_at":"2026-07-07T09:21:58.974547055-06:00","dependencies":[{"issue_id":"openforge_catalog-750","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:52.476138903-06:00","created_by":"Devon Jones"}]} {"id":"openforge_catalog-7dg","title":"Auth/token manager for Thingiverse v2 (login + refresh)","description":"Python module: interactive 'login' does POST /v2/auth/login {usernameOrEmail, password} -\u003e AuthTokensResponse (handle 202 2FA via /v2/auth/2fa/login), persists refresh token securely (env file/keyring, never in repo); runtime path auto-refreshes access JWT via POST /v2/auth/refresh. Password is never stored. Buildable and testable today against the live API β€” verify a Devon-scoped JWT via GET /v2/users/me. Note: THINGIVERSE_APP_TOKEN in ~/.profile.d/099.api_keys.sh is v1-only and scoped to the wrong account (Joaquin Munguia, id 5577639) β€” do not use.","status":"closed","priority":1,"issue_type":"feature","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:24.708129858-06:00","created_by":"Devon Jones","updated_at":"2026-07-06T14:39:30.409750275-06:00","closed_at":"2026-07-06T14:39:30.409750275-06:00","close_reason":"Merged in PR #224 to test. TokenManager (login/2FA/refresh/atomic 0600 persistence/server-side logout) + tv_auth CLI + 53 tests at 100% coverage. Live-verified: devonjones (id 47139) login, status, and refresh rotation against the real v2 API. Unblocks gn2 together with hnr (HAR capture).","dependencies":[{"issue_id":"openforge_catalog-7dg","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:52.071225891-06:00","created_by":"Devon Jones"}]} {"id":"openforge_catalog-8yb","title":"Propagate thingiverse sync data to AWS prod (fixture + API upsert path)","description":"The tool runs against the LOCAL Postgres, but prod (AWS Lambda + serverless Postgres) is populated by replaying fixture JSON through the Flask API (bin/upload_fixture with OPENFORGE_BASE_URL, same pattern as blueprint fixtures). Thingiverse data (blueprint_id \u003c-\u003e thing_id mapping, public thing URLs) must ride that path so the app can render Thingiverse links in prod and support deeper integration later. Deliverables: fixture format for thingiverse mappings (exported from the local sync tables), an authenticated API upsert endpoint in the Flask app, and upload_fixture support. Note: prod likely only needs the mapping/links, not the per-file hash ledger β€” that is local operational state; decide the split when designing the fixture.","notes":"DESIGN NOTE from PR #225 review: the schema allows one blueprint to appear in MULTIPLE things (partial unique index is per-thing only, and test_same_source_allowed_in_different_things pins that behavior). The prod fixture mapping is therefore 1:N β€” (blueprint_id, thing_id, public_url) derived per thingiverse_files row, not a unique-per-blueprint map. The app's Thingiverse links UI must handle a blueprint linking to several things.","status":"open","priority":2,"issue_type":"feature","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:55:33.474857151-06:00","created_by":"Devon Jones","updated_at":"2026-07-06T14:51:24.318740515-06:00","dependencies":[{"issue_id":"openforge_catalog-8yb","depends_on_id":"openforge_catalog-750","type":"blocks","created_at":"2026-07-06T11:55:47.512062985-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-8yb","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:55:47.882392741-06:00","created_by":"Devon Jones"}]} -{"id":"openforge_catalog-ad5","title":"Sync engine: diff local files vs remote hashes, reconcile","description":"For each managed thing: diff manifest-directed local files against remote files using hash comparison (via thingiverse_file ledger) -\u003e upload new, replace changed, delete removed, patch drifted metadata. Idempotent; dry-run mode showing planned actions before mutating. Depends on hash-algorithm verification: if remote is SHA-256, compute+store SHA-256 alongside catalog MD5s.","notes":"PRIOR ART (old v1 tool): tv_update_description did bulk find/replace of boilerplate across all things matching a name filter, with --dry-run (desc_update.json shows real mass migrations happened). tv_tag_dl mirrored all things to local JSON with 3s sleeps (self-rate-limiting; v1 had practical rate limits). Lessons: (1) metadata drift detection should cover the shared-boilerplate case so a boilerplate change re-syncs every managed thing's description; (2) throttle API calls; (3) dry-run is a first-class requirement, already in scope.","status":"open","priority":1,"issue_type":"feature","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:26.401028262-06:00","created_by":"Devon Jones","updated_at":"2026-07-06T12:00:34.191543122-06:00","dependencies":[{"issue_id":"openforge_catalog-ad5","depends_on_id":"openforge_catalog-gn2","type":"blocks","created_at":"2026-07-06T11:42:43.166238543-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-ad5","depends_on_id":"openforge_catalog-750","type":"blocks","created_at":"2026-07-06T11:42:43.606420026-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-ad5","depends_on_id":"openforge_catalog-d7a","type":"blocks","created_at":"2026-07-06T11:42:44.00974978-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-ad5","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:53.860069002-06:00","created_by":"Devon Jones"}]} +{"id":"openforge_catalog-ad5","title":"Sync engine: diff local files vs remote hashes, reconcile","description":"For each managed thing: diff manifest-directed local files against remote files using hash comparison (via thingiverse_file ledger) -\u003e upload new, replace changed, delete removed, patch drifted metadata. Idempotent; dry-run mode showing planned actions before mutating. Depends on hash-algorithm verification: if remote is SHA-256, compute+store SHA-256 alongside catalog MD5s.","notes":"VALIDATED on real data (thing:7364110 aztlan): 31/31 STLs' Thingiverse hashes match catalog file_md5 exactly. TWO REQUIREMENTS: (1) Hash encoding VARIES - old things=hex MD5 (32ch), recent=base64 MD5 (24ch, e.g. lma97RnU7yEQDMZcDe1vEA==). Normalize before compare: 24-char/b64 -\u003e decode to hex; 32-char -\u003e lowercase. (2) Thingiverse SANITIZES filenames on upload (strips #, comma, etc): catalog 'aztlan#snakehole.A.openforge,side.stl' -\u003e TV 'aztlansnakehole.A.openforgeside.stl'. Match files by MD5, NEVER by name. Zips are aggregate bundles (file_type=zip, not blueprint-backed).","status":"open","priority":1,"issue_type":"feature","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:26.401028262-06:00","created_by":"Devon Jones","updated_at":"2026-07-07T12:16:40.504894238-06:00","dependencies":[{"issue_id":"openforge_catalog-ad5","depends_on_id":"openforge_catalog-gn2","type":"blocks","created_at":"2026-07-06T11:42:43.166238543-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-ad5","depends_on_id":"openforge_catalog-750","type":"blocks","created_at":"2026-07-06T11:42:43.606420026-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-ad5","depends_on_id":"openforge_catalog-d7a","type":"blocks","created_at":"2026-07-06T11:42:44.00974978-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-ad5","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:53.860069002-06:00","created_by":"Devon Jones"}]} {"id":"openforge_catalog-aq5","title":"Add general-reviewer to the agent pack (no Gemini on this repo)","description":"Gemini Code Assist/Cursor aren't installed on this repo, so the pack needs a formalized whole-PR generalist reviewer that ALWAYS spawns in every pr-review-loop round, covering what the specialists don't own (logic correctness, domain/design soundness, contract fidelity, docs accuracy). Previously run ad-hoc as the Claude fallback: the invalid-ruff-flag catch was on PR #223 (the pack's own creation PR); the JWT AttributeError catch was PR #224; the multi-accept AND-vs-ANY bug was PR #226.","status":"closed","priority":2,"issue_type":"task","owner":"devon.jones@gmail.com","created_at":"2026-07-06T17:33:30.159979202-06:00","created_by":"Devon Jones","updated_at":"2026-07-06T17:45:19.960182349-06:00","closed_at":"2026-07-06T17:45:19.960182349-06:00","close_reason":"Merged in PR #227. general-reviewer formalized in the pack; dogfooded on its own PR and caught a misattribution in the PR body."} {"id":"openforge_catalog-bll","title":"Multi-Angle Thumbnail Viewing System","description":"Enable users to view STL models from 10 isometric camera angles with interactive controls. Admins can set default viewing angles per model. System generates 10 isometric-perspective angles (6 primary directions + 4 diagonal corners) during file scan using stl-thumb --cam-pos, combined into single sprite sheet PNG. Frontend has drag-to-rotate and keyboard navigation. All angles maintain 3/4 view for depth, not flat orthographic.","status":"open","priority":1,"issue_type":"epic","created_at":"2025-12-21T09:57:50.607001989-07:00","updated_at":"2025-12-21T10:09:20.457266019-07:00"} {"id":"openforge_catalog-d7a","title":"Manifest schema + metadata assembler","description":"Define per-thing manifest format (YAML): explicit file list for the thing plus field overrides. Assembler builds the thing payload with defaults pulled from catalog Postgres blueprint records (name, description, tag hierarchy -\u003e Thingiverse tags, category, license, images) and applies manifest overrides on top. Zero-manual-entry defaults, curated where it matters. Payload field names/shape finalized against the HAR contract.","notes":"PRIOR ART (old v1 tool): bin/new.*.json templates were per-product-line defaults (openforge2, openlock, construction_kit, miniature, tools): license, category, tags, boilerplate description. Key design lesson: descriptions = per-thing content + SHARED BOILERPLATE FOOTER (Patreon/tutorials/catalog links) that changes over time. Model boilerplate as a shared block composed at assembly time (stored once, referenced by manifests), not baked into each manifest β€” enables bulk boilerplate updates via re-sync. Old templates: ~/Dropbox/projects/Hardware/objects/OpenForgeDevelopment/openforge/bin/new.*.json","status":"in_progress","priority":2,"issue_type":"feature","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:25.496132115-06:00","created_by":"Devon Jones","updated_at":"2026-07-06T15:00:55.159469636-06:00","dependencies":[{"issue_id":"openforge_catalog-d7a","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:52.892986073-06:00","created_by":"Devon Jones"}]} {"id":"openforge_catalog-dd4","title":"Integration: Wire scanner pipeline for multi-angle generation","description":"Wire multi-angle sprite sheet generation into file scanning pipeline, BUT ONLY AFTER phased rollout validation. Scanner should: (1) Support both legacy single thumbnails AND new sprite sheets during transition. (2) Have feature flag to enable/disable sprite generation. (3) Track which models have sprites vs legacy. (4) Handle failures gracefully - don't block scan if sprite generation fails. (5) Be pausable/resumable for bulk backfill. Only enable bulk processing after small test set is validated with users.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-21T09:58:49.223198843-07:00","updated_at":"2025-12-21T14:35:20.810908751-07:00","closed_at":"2025-12-21T14:35:20.810908751-07:00","close_reason":"Completed: Integrated sprite sheet generation into scanner pipeline with feature flag (ENABLE_SPRITE_THUMBNAILS). Created create_and_upload_thumbnail() helper that handles both sprite sheets and legacy thumbnails. Updated scanner.py and incremental.py to use new function. Gracefully handles failures without blocking scans. Supports transition period with both formats. All 319 tests passing.","dependencies":[{"issue_id":"openforge_catalog-dd4","depends_on_id":"openforge_catalog-bll","type":"parent-child","created_at":"2025-12-21T09:59:20.290625315-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-dd4","depends_on_id":"openforge_catalog-ubp","type":"blocks","created_at":"2025-12-21T09:59:37.011721674-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-dd4","depends_on_id":"openforge_catalog-5tg","type":"blocks","created_at":"2025-12-21T09:59:37.450897079-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-dd4","depends_on_id":"openforge_catalog-hqe","type":"blocks","created_at":"2025-12-21T09:59:37.840969584-07:00","created_by":"daemon"}]} -{"id":"openforge_catalog-gn2","title":"Thingiverse v2 API client (write endpoints from HAR contract)","description":"Python client wrapping the v2 API using the auth/token manager: create thing, patch metadata, upload model file, replace file, delete file, list files w/ hashes, publish. Read endpoints (things/{id}/complete etc.) from the public spec (docs/thingiverse-api-v2.md); write endpoints implemented per the HAR-derived contract. Stub the write calls if started before the HAR ticket completes.","status":"open","priority":1,"issue_type":"feature","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:25.975705436-06:00","created_by":"Devon Jones","updated_at":"2026-07-06T11:42:25.975705436-06:00","dependencies":[{"issue_id":"openforge_catalog-gn2","depends_on_id":"openforge_catalog-hnr","type":"blocks","created_at":"2026-07-06T11:42:42.234078586-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-gn2","depends_on_id":"openforge_catalog-7dg","type":"blocks","created_at":"2026-07-06T11:42:42.679151017-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-gn2","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:53.406992128-06:00","created_by":"Devon Jones"}]} -{"id":"openforge_catalog-hnr","title":"Capture HAR of Thingiverse SPA create/upload/edit flow and extract API contract","description":"Devon captures a HAR (Network tab, preserve log, Fetch/XHR, 'Save all as HAR with content') of a complete flow: create draft thing, set name/description/tags/category/license, upload image, upload STL, replace the STL, delete a file, publish. HAR goes to scratchpad only β€” contains live JWT/refresh/cookies, never commit; rotate tokens after. From the HAR, extract and document: create-thing endpoint+payload, model-file upload mechanism (direct POST vs presigned URL β€” matters for Lambda), file object shape incl. hash field, metadata patch, and file delete. Also verify hash algorithm: hash the exact uploaded STL locally with MD5 and SHA-256 and match against the hash Thingiverse reports (Devon guesses SHA-256). Output: docs/thingiverse-api-v2-private.md contract doc.","notes":"SCOPE ADDITION: things will contain photos, zip files, and other non-STL files. During the HAR capture, upload at least one photo, one zip, and one STL, and capture whether they all go through the same upload endpoint/mechanism or different ones (the public spec only shows an image upload route). Verify whether the hash field is reported for ALL file types or only models.","status":"open","priority":1,"issue_type":"task","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:24.225732535-06:00","created_by":"Devon Jones","updated_at":"2026-07-06T11:55:48.284424468-06:00","dependencies":[{"issue_id":"openforge_catalog-hnr","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:51.659042706-06:00","created_by":"Devon Jones"}]} +{"id":"openforge_catalog-gn2","title":"Thingiverse v2 API client (write endpoints from HAR contract)","description":"Python client wrapping the v2 API using the auth/token manager: create thing, patch metadata, upload model file, replace file, delete file, list files w/ hashes, publish. Read endpoints (things/{id}/complete etc.) from the public spec (docs/thingiverse-api-v2.md); write endpoints implemented per the HAR-derived contract. Stub the write calls if started before the HAR ticket completes.","notes":"Hash field comes base64-encoded on recent uploads (24-char) and hex on older ones (32-char) - client must expose a normalized-hex form. Remote filenames are sanitized (no #/comma/etc) so never key on name. Confirmed the v1 read shape on a real published thing (thing:7364110): license=human string, is_published=1, creator.name, File.hash present.","status":"in_progress","priority":1,"issue_type":"feature","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:25.975705436-06:00","created_by":"Devon Jones","updated_at":"2026-07-07T12:47:35.456889575-06:00","dependencies":[{"issue_id":"openforge_catalog-gn2","depends_on_id":"openforge_catalog-hnr","type":"blocks","created_at":"2026-07-06T11:42:42.234078586-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-gn2","depends_on_id":"openforge_catalog-7dg","type":"blocks","created_at":"2026-07-06T11:42:42.679151017-06:00","created_by":"Devon Jones"},{"issue_id":"openforge_catalog-gn2","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:53.406992128-06:00","created_by":"Devon Jones"}]} +{"id":"openforge_catalog-hnr","title":"Capture HAR of Thingiverse SPA create/upload/edit flow and extract API contract","description":"Devon captures a HAR (Network tab, preserve log, Fetch/XHR, 'Save all as HAR with content') of a complete flow: create draft thing, set name/description/tags/category/license, upload image, upload STL, replace the STL, delete a file, publish. HAR goes to scratchpad only β€” contains live JWT/refresh/cookies, never commit; rotate tokens after. From the HAR, extract and document: create-thing endpoint+payload, model-file upload mechanism (direct POST vs presigned URL β€” matters for Lambda), file object shape incl. hash field, metadata patch, and file delete. Also verify hash algorithm: hash the exact uploaded STL locally with MD5 and SHA-256 and match against the hash Thingiverse reports (Devon guesses SHA-256). Output: docs/thingiverse-api-v2-private.md contract doc.","notes":"LARGELY SOLVED via v1 swagger (thingiverse.com/swagger/docs/openapi.yaml) + live probes. (1) HASH = content-MD5, confirmed empirically (downloaded a file, MD5 matches the File.hash field exactly). Catalog already stores file_md5 -\u003e sync compares directly, NO SHA-256 needed (local_sha256 column is dead/harmless). (2) WRITE AUTH SOLVED: use the 'token' field from POST /v2/auth/login as Bearer (32-char opaque, devonjones-scoped id 47139), against api.thingiverse.com DIRECTLY (www origin is Cloudflare-challenged). The env THINGIVERSE_APP_TOKEN authenticates as Joaquin (wrong account) - do not use. (3) Write surface is v1: POST /things/ (create), POST /files/0/uploadFile (multipart -\u003e {id}), POST /files/0/FinalizeFiles, GET /things/{id}/files (hash), POST /things/{id}/publish. Full contract: docs/thingiverse-api-v2-private.md. REMAINING: confirm POST /things/ payload on first real create; Devon notes v1 can't set all fields he wants (open request to Thingiverse for v2 write).","status":"closed","priority":1,"issue_type":"task","owner":"devon.jones@gmail.com","created_at":"2026-07-06T11:42:24.225732535-06:00","created_by":"Devon Jones","updated_at":"2026-07-07T12:47:34.953583534-06:00","closed_at":"2026-07-07T12:47:34.953583534-06:00","close_reason":"Contract captured and validated end-to-end. Write auth (login token field, devonjones-scoped, api.thingiverse.com direct), file hash = MD5 (hex or base64 depending on upload path), write endpoints (v1: POST /things/, uploadFile+FinalizeFiles, publish), filename sanitization all documented in docs/thingiverse-api-v2-private.md and validated against thing:7364110 (31/31 STLs match catalog file_md5).","dependencies":[{"issue_id":"openforge_catalog-hnr","depends_on_id":"openforge_catalog-4kx","type":"parent-child","created_at":"2026-07-06T11:42:51.659042706-06:00","created_by":"Devon Jones"}]} {"id":"openforge_catalog-hqe","title":"Backend API: Admin set default camera position","description":"Create Flask endpoint for admins to set/update the default camera position for individual models. Accepts model_id and camera position (x,y,z). Requires admin authentication. Triggers thumbnail regeneration if needed.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-21T09:58:36.544944445-07:00","updated_at":"2025-12-21T14:01:55.202764217-07:00","closed_at":"2025-12-21T14:01:55.202764217-07:00","close_reason":"Completed: Created PATCH /api/blueprints/\u003cblueprint_id\u003e/thumbnail-variants/default-angle endpoint. Admin-only, requires authentication and CSRF protection. Validates angle index (0-9), updates sprite_metadata.default_angle in database. Returns error for legacy single thumbnails. All 319 tests passing.","dependencies":[{"issue_id":"openforge_catalog-hqe","depends_on_id":"openforge_catalog-bll","type":"parent-child","created_at":"2025-12-21T09:59:18.965408581-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-hqe","depends_on_id":"openforge_catalog-jnl","type":"blocks","created_at":"2025-12-21T09:59:35.746577657-07:00","created_by":"daemon"}]} {"id":"openforge_catalog-hr4","title":"Admin UI: Manually trigger sprite generation per model","description":"Add admin interface to manually trigger sprite sheet generation for individual models. Useful for: (1) Testing different camera positions on specific models. (2) Regenerating sprites after adjusting defaults. (3) Fixing failed generations. Shows progress, preview of result, ability to approve/reject before replacing existing thumbnail. Can mark models as 'sprite sheet validated' to track rollout progress.","status":"open","priority":3,"issue_type":"task","created_at":"2025-12-21T10:14:01.648534686-07:00","updated_at":"2025-12-21T10:14:01.648534686-07:00","dependencies":[{"issue_id":"openforge_catalog-hr4","depends_on_id":"openforge_catalog-bll","type":"parent-child","created_at":"2025-12-21T10:14:19.184588659-07:00","created_by":"daemon"},{"issue_id":"openforge_catalog-hr4","depends_on_id":"openforge_catalog-4ji","type":"blocks","created_at":"2025-12-21T10:14:24.909562746-07:00","created_by":"daemon"}]} {"id":"openforge_catalog-jnl","title":"Database schema for camera positions and thumbnail variants","description":"Add database schema to store: (1) Sprite sheet metadata - grid dimensions (rows, cols), tile size, angle positions in grid. (2) Camera position per angle (x,y,z coords). (3) Default angle index per model. (4) Sprite sheet file path/URL in R2. Consider if this extends existing thumbnail tables or needs new camera_angles table. Must support models with legacy single thumbnail during migration.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-21T09:58:19.570114017-07:00","updated_at":"2025-12-21T13:07:55.263321459-07:00","closed_at":"2025-12-21T13:07:55.263321459-07:00","close_reason":"Completed: Added sprite_metadata JSONB column to images table with GIN index. Migration version 16 created and tested. All tests passing.","dependencies":[{"issue_id":"openforge_catalog-jnl","depends_on_id":"openforge_catalog-bll","type":"parent-child","created_at":"2025-12-21T09:59:17.171165836-07:00","created_by":"daemon"}]} diff --git a/.beads/last-touched b/.beads/last-touched index 71f6977..c04b471 100644 --- a/.beads/last-touched +++ b/.beads/last-touched @@ -1 +1 @@ -openforge_catalog-aq5 +openforge_catalog-gn2 diff --git a/docs/thingiverse-api-v2-private.md b/docs/thingiverse-api-v2-private.md new file mode 100644 index 0000000..832cbbc --- /dev/null +++ b/docs/thingiverse-api-v2-private.md @@ -0,0 +1,109 @@ +# Thingiverse write-API contract (working notes for `hnr`) + +Reverse-engineered by (a) driving the live SPA as devonjones with rodney + +fetch/XHR interception, (b) the v1 OpenAPI at +`https://www.thingiverse.com/swagger/docs/openapi.yaml` (+ its `$ref` +sub-files under `/swagger/docs/`), and (c) direct API probes with a live +token. **The write surface is the v1 API**, not v2. + +## Auth for writes β€” SOLVED + +- `POST https://api.thingiverse.com/v2/auth/login` `{usernameOrEmail, password}` + β†’ `200 {message, token, jwt:{access, refresh}}`. +- **Use the `token` field (32-char opaque) as `Authorization: Bearer `.** + Verified it authenticates as **devonjones (Devon Jones, id 47139)** and is + the same token the SPA uses for write calls. The `jwt.access` is for v2 + READ endpoints only (the opaque token 401s on v2 `/users/me`; the JWT is + not what writes use). +- **Hit `api.thingiverse.com` directly, not `www.thingiverse.com/api/`.** + The `www` origin is behind a Cloudflare bot challenge (returns a "Just a + moment…" 403 to non-browser clients); `api.thingiverse.com` is not + challenged and accepts the token fine. +- **The env `THINGIVERSE_APP_TOKEN` is the wrong credential**: it's also a + 32-char opaque token but authenticates as **Joaquin Munguia (id 5577639)**, + not devonjones β€” writes with it would land on the wrong account. Do not use + it. (Open item: reconcile why app:1197 issued a Joaquin-scoped token β€” see + `hnr` notes; not a blocker since the login `token` works.) + +## File hash β€” SOLVED: it's content-MD5 + +- The v1 `File` schema has a `hash` field ("The hash of the file"). +- Empirically: 3DBenchy's files report 32-char hex hashes; downloaded the + 74 KB multipart STL (file 1475623) and its computed **MD5 exactly matches** + the reported `hash` (`15a6feb55745618e878202d862c44b4d`). +- **Implication:** the catalog already stores `file_md5` (per-blueprint) and + computes MD5 in the scanner β€” the sync engine compares catalog `file_md5` + directly against Thingiverse's `hash`. **No SHA-256 needed.** The + `thingiverse_files.local_sha256` column added defensively in migration v17 + is unnecessary (harmless dead column; drop in a follow-up or leave). + +## Write endpoints (v1, host `api.thingiverse.com`) + +From the swagger path list; payload shapes to be confirmed on first real use. + +| Purpose | Endpoint | +|---|---| +| Create thing | `POST /things/` (v1 create; old tooling payload `{name, license, category, tags[], description}`) | +| Update thing | `PATCH /things/{thing_id}` | +| Upload a file | `POST /files/0/uploadFile` (multipart/form-data β†’ `{"id": }`); the `0` is the thing sentinel | +| Finalize upload | `POST /files/0/FinalizeFiles` (the `Finalize`/`/files/{id}/finalize` variants are DEPRECATED) | +| Thing's files (hash lives here) | `GET /things/{thing_id}/files`, `GET /things/{thing_id}/files/{file_id}` | +| Images | `GET/POST /things/{thing_id}/images`, `/images/{image_id}` | +| Publish | `POST /things/{thing_id}/publish` | +| Delete/move/copy | `POST /thingops/{ids}/remove|move|copy` | +| Download | `GET /files/{file_id}/download` | + +Observed upload flow in the SPA: `POST /files/0/uploadFile` (multipart, one +per file) β†’ returns `{id}` β†’ files attach to the draft. A **finalize** step +(`/files/0/FinalizeFiles`) commits them. Create-thing (`POST /things/`) was +NOT captured live (the generic editor stalled on a Thingiverse-side 500), so +confirm its exact payload against the swagger `resources/thing.yaml#/Root` +and on first real create. + +## Known limitation (from Devon) + +The **v1 write API can't modify every field** Devon needs on a thing; he has +an open request to Thingiverse for the missing fields (likely the reason the +custom v2 surface exists). Plan: build `gn2` against the v1 write endpoints +that work today; leave the not-yet-modifiable fields for when Thingiverse +ships v2 write support. + +## Remaining to confirm on first real create (do with a throwaway draft) + +- Exact `POST /things/` request/response (thing_id, initial status). +- The `uploadFile` multipart field names + how a file is bound to a thing + (thingId in path is `0` pre-save β€” how the finalize associates them). +- Publish payload and the draftβ†’published transition. + +## Local test hashes (for any future round) + +| file | md5 | size | +|---|---|---| +| test_model.stl (battle-axe) | `2d56b05653ee9ad44205c53a09657285` | 170807 | +| test_photo.png | `1c2d59d649820c7c302ec3d5da755991` | 76 | +| test_bundle.zip | `db28a8cdbe01b03852dd6945a2e47afe` | 203 | + +## VALIDATED on real data (thing:7364110 "Aztlan Snakeholes", devonjones) + +Cross-checked 33 live files against the catalog's aztlan fixture: **31/31 +individual STLs matched by MD5 exactly** (the 2 non-matches are `.zip` +bundles, which the catalog doesn't track as blueprints). End-to-end proof +the sync comparison works. Two hard requirements surfaced: + +1. **Hash encoding VARIES.** Older things return hex MD5 (`b8ae14b6…`, + 32 chars); this recent set returns **base64 MD5** (`lma97RnU7yEQDMZcDe1vEA==`, + 24 chars = 16 bytes). The sync engine must normalize before comparing to + the catalog's hex `file_md5`: + `norm = hexlify(b64decode(h)) if len(h)==24 and h.endswith("=") else h.lower()` + (more robustly: 24-char/base64 β†’ decodeβ†’hex; 32-char hex β†’ lowercase). +2. **Thingiverse sanitizes filenames on upload** β€” `#`, `,` (and likely + `+`/`%`) are stripped/changed: catalog `aztlan#snakehole.A.openforge,side.stl` + β†’ Thingiverse `aztlansnakehole.A.openforgeside.stl`. **Files cannot be + matched by name across the boundary β€” match by MD5.** (The + `thingiverse_files` ledger already keys on hash/remote_file_id, so this is + fine β€” but the assembler/sync must NOT assume the remote name equals the + local name.) + +Also confirmed on a real thing: `license` = human string +("Creative Commons - Attribution - Non-Commercial - Share Alike"), +`is_published` = 1, `creator.name` = devonjones β€” matches the v1 schema. diff --git a/openforge/thingiverse/auth.py b/openforge/thingiverse/auth.py index 50194cd..354fbce 100644 --- a/openforge/thingiverse/auth.py +++ b/openforge/thingiverse/auth.py @@ -241,6 +241,23 @@ def is_logged_in(self) -> bool: """Report whether tokens are stored (not whether they're valid).""" return self.token_file.exists() + def write_token(self) -> str: + """Return the opaque token used to authorize v1 write-API calls. + + This is the `token` field from login (see + docs/thingiverse-api-v2-private.md), distinct from the access JWT. + + Raises: + NotLoggedIn: If no write token is stored β€” it is captured only + at login (not refresh), so a re-login is required + """ + token = self._load_tokens().get("session_token") + if not token: + raise NotLoggedIn( + "no write token stored; log in again (captured only at login)" + ) + return token + def close(self): """Release the HTTP session's connection pool. @@ -289,10 +306,16 @@ def _handle_token_response(self, resp: requests.Response, action: str) -> Dict: "refresh": refresh, "stored_at": int(time.time()), } - # The session token accompanies the JWTs on login responses; keep - # it in case v1-style endpoints need it later. - if "token" in body: + # The `token` field is the opaque write token used against the v1 + # write API (docs/thingiverse-api-v2-private.md). It's only present + # on login responses, NOT on refresh β€” carry the existing one + # forward on refresh so a JWT refresh doesn't drop the write token. + if body.get("token"): stored["session_token"] = body["token"] + else: + existing = self._peek_tokens() + if existing.get("session_token"): + stored["session_token"] = existing["session_token"] self._store_tokens(stored) logger.info("%s succeeded, tokens stored", action) return stored @@ -317,6 +340,13 @@ def _store_tokens(self, tokens: Dict): os.unlink(tmp_path) raise + def _peek_tokens(self) -> Dict: + """Read stored tokens, returning {} instead of raising if absent.""" + try: + return self._load_tokens() + except NotLoggedIn: + return {} + def _load_tokens(self) -> Dict: """Read stored tokens. diff --git a/openforge/thingiverse/client.py b/openforge/thingiverse/client.py new file mode 100644 index 0000000..a19523f --- /dev/null +++ b/openforge/thingiverse/client.py @@ -0,0 +1,203 @@ +"""Thingiverse write-API client. + +Wraps the v1 write endpoints (create/update/delete thing, upload + +finalize files, publish, read a thing's files) against +`https://api.thingiverse.com` directly, authorized with the opaque write +token from `TokenManager.write_token()`. + +The full contract this implements β€” endpoints, payloads, the +uploadβ†’finalize flow, and the hash-encoding quirk β€” is documented in +docs/thingiverse-api-v2-private.md and was confirmed live against +devonjones's account. + +Notes: +- Auth is the opaque `token` from login, NOT the access JWT. +- Target `api.thingiverse.com` directly; the `www.thingiverse.com/api` + origin is Cloudflare-challenged for non-browser clients. +- File `hash` comes back as hex MD5 on older uploads and base64 MD5 on + newer ones; `normalize_hash()` converts both to lowercase hex to match + the catalog's `file_md5`. +""" + +import base64 +import binascii +import logging +from pathlib import Path +from typing import Dict, List, Optional + +import requests + +from openforge.thingiverse.auth import TokenManager + +logger = logging.getLogger(__name__) + +API_BASE = "https://api.thingiverse.com" +REQUEST_TIMEOUT = 120 # STL uploads are large +USER_AGENT = "openforge-catalog-tools" + + +class ThingiverseAPIError(Exception): + """A Thingiverse API call returned a non-2xx response. + + Attributes: + status: HTTP status code + detail: The API's error message, if any + """ + + def __init__(self, status: int, detail: str, method: str, path: str): + self.status = status + self.detail = detail + msg = f"{method} {path} -> HTTP {status}" + if detail: + msg += f": {detail}" + super().__init__(msg) + + +def normalize_hash(h: Optional[str]) -> Optional[str]: + """Normalize a Thingiverse file hash to lowercase hex MD5. + + Recent uploads return base64-encoded MD5 (24 chars ending in `=`); + older ones return 32-char hex. Both decode to the same MD5 the catalog + stores as `file_md5`. Returns None for a falsy input. + """ + if not h: + return None + if len(h) == 24 and h.endswith("="): + try: + return binascii.hexlify(base64.b64decode(h, validate=True)).decode() + except (binascii.Error, ValueError): + return h.lower() + return h.lower() + + +class ThingiverseClient: + """Client for the Thingiverse write API. + + Args: + token_manager: Provides the write token (and is re-consulted per + request so a re-login mid-run is picked up). + session: HTTP session (injectable for tests). + """ + + def __init__( + self, + token_manager: TokenManager, + session: Optional[requests.Session] = None, + ): + self.tokens = token_manager + if session is None: + session = requests.Session() + session.headers["User-Agent"] = USER_AGENT + self.session = session + + # -- things --------------------------------------------------------- + + def create_thing( + self, + name: str, + license: str, + category: str, + description: Optional[str] = None, + tags: Optional[List[str]] = None, + ) -> Dict: + """Create a draft thing. Returns the created thing (incl. `id`). + + `license` is a slug (e.g. "cc"); `category` is a display name + (e.g. "Toy & Game Accessories"). name/license/category are required + by the API. + """ + payload = {"name": name, "license": license, "category": category} + if description is not None: + payload["description"] = description + if tags is not None: + payload["tags"] = tags + return self._request("POST", "/things/", json=payload) + + def get_thing(self, thing_id: int) -> Dict: + """Fetch a thing by id.""" + return self._request("GET", f"/things/{thing_id}") + + def update_thing(self, thing_id: int, **fields) -> Dict: + """Patch fields on an existing thing.""" + return self._request("PATCH", f"/things/{thing_id}", json=fields) + + def delete_thing(self, thing_id: int) -> Dict: + """Delete a thing. Returns `{"ok": "ok"}`.""" + return self._request("DELETE", f"/things/{thing_id}") + + def publish_thing(self, thing_id: int) -> Dict: + """Publish a draft thing.""" + return self._request("POST", f"/things/{thing_id}/publish") + + # -- files ---------------------------------------------------------- + + def get_thing_files(self, thing_id: int) -> List[Dict]: + """List a thing's files, each with a normalized-hex `md5` added. + + The raw `hash` field is preserved; `md5` is `normalize_hash(hash)` + so callers can compare directly to the catalog's `file_md5`. + """ + data = self._request("GET", f"/things/{thing_id}/files") + files = data if isinstance(data, list) else data.get("files", []) + for f in files: + f["md5"] = normalize_hash(f.get("hash")) + return files + + def upload_file(self, thing_id: int, path: Path) -> Dict: + """Upload a file to a thing as a pending upload. Returns `{id}`. + + The upload is PENDING until finalize_files() commits it β€” an + un-finalized file does not appear in get_thing_files(). + """ + path = Path(path) + with open(path, "rb") as fh: + files = {"file": (path.name, fh)} + return self._request("POST", f"/files/{thing_id}/uploadFile", files=files) + + def finalize_files(self, thing_id: int, file_ids: List[int]) -> Dict: + """Commit pending uploads, associating them with the thing. + + Ranks are assigned by list order (10, 20, …) to preserve intended + file ordering. + """ + pending = [{"id": fid, "rank": (i + 1) * 10} for i, fid in enumerate(file_ids)] + payload = { + "pending_uploads": pending, + "target_id": thing_id, + "target_type": "thing", + } + return self._request("POST", f"/files/{thing_id}/FinalizeFiles", json=payload) + + # -- internals ------------------------------------------------------ + + def _request(self, method: str, path: str, **kwargs) -> Dict: + """Issue an authenticated request; raise on non-2xx, return JSON.""" + headers = {"Authorization": f"Bearer {self.tokens.write_token()}"} + resp = self.session.request( + method, + f"{API_BASE}{path}", + headers=headers, + timeout=REQUEST_TIMEOUT, + **kwargs, + ) + if not 200 <= resp.status_code < 300: + detail = "" + try: + detail = resp.json().get("error", "") + except (ValueError, AttributeError): + pass + raise ThingiverseAPIError(resp.status_code, detail, method, path) + try: + return resp.json() + except ValueError: + return {} + + def close(self): + """Release the HTTP session.""" + self.session.close() + + def __enter__(self) -> "ThingiverseClient": + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.close() diff --git a/tests/test_thingiverse_auth.py b/tests/test_thingiverse_auth.py index 6f0209d..b34abd7 100644 --- a/tests/test_thingiverse_auth.py +++ b/tests/test_thingiverse_auth.py @@ -210,6 +210,38 @@ def test_no_token_file_raises_not_logged_in(self, manager): with pytest.raises(NotLoggedIn): manager.access_token() + def test_write_token_stored_at_login(self, manager, session): + access = make_jwt(exp=int(time.time()) + 3600) + session.queue(FakeResponse(200, login_body(access, "r", token="wtok"))) + manager.login("devon", "pw") + assert manager.write_token() == "wtok" + + def test_write_token_survives_refresh(self, manager, session, token_file): + # login captures the write token; a later JWT refresh (whose + # response has no `token`) must NOT drop it + expired = make_jwt(exp=int(time.time()) - 100) + fresh = make_jwt(exp=int(time.time()) + 3600) + session.queue(FakeResponse(200, login_body(expired, "r1", token="wtok"))) + manager.login("devon", "pw") + session.queue(FakeResponse(200, {"access": fresh, "refresh": "r2"})) + manager.access_token() # triggers refresh + + assert manager.write_token() == "wtok" + assert json.loads(token_file.read_text())["session_token"] == "wtok" + + def test_write_token_missing_raises_not_logged_in(self, manager, session): + # a login response without a `token` field -> no write token stored + access = make_jwt(exp=int(time.time()) + 3600) + body = {"message": "ok", "jwt": {"access": access, "refresh": "r"}} + session.queue(FakeResponse(200, body)) + manager.login("devon", "pw") + with pytest.raises(NotLoggedIn, match="no write token"): + manager.write_token() + + def test_write_token_no_login_raises(self, manager): + with pytest.raises(NotLoggedIn): + manager.write_token() + def test_corrupt_token_file_raises_not_logged_in(self, manager, token_file): token_file.parent.mkdir(parents=True) token_file.write_text("{not json") diff --git a/tests/test_thingiverse_client.py b/tests/test_thingiverse_client.py new file mode 100644 index 0000000..ad2a715 --- /dev/null +++ b/tests/test_thingiverse_client.py @@ -0,0 +1,230 @@ +"""Tests for openforge.thingiverse.client (ThingiverseClient).""" + +import base64 +import binascii + +import pytest + +from openforge.thingiverse.auth import NotLoggedIn +from openforge.thingiverse.client import ( + API_BASE, + ThingiverseAPIError, + ThingiverseClient, + normalize_hash, +) + +MD5_HEX = "2d56b05653ee9ad44205c53a09657285" +MD5_B64 = base64.b64encode(binascii.unhexlify(MD5_HEX)).decode() # ends with '=' + + +class FakeResponse: + def __init__(self, status_code, body=None): + self.status_code = status_code + self._body = body if body is not None else {} + + def json(self): + if isinstance(self._body, Exception): + raise self._body + return self._body + + +class FakeSession: + def __init__(self): + self.responses = [] + self.calls = [] + self.headers = {} + self.closed = False + + def queue(self, resp): + self.responses.append(resp) + return self + + def request(self, method, url, headers=None, timeout=None, **kwargs): + self.calls.append({"method": method, "url": url, "headers": headers, **kwargs}) + return self.responses.pop(0) + + def close(self): + self.closed = True + + +class FakeTokenManager: + def __init__(self, token="write-tok"): + self._token = token + self.calls = 0 + + def write_token(self): + self.calls += 1 + if self._token is None: + raise NotLoggedIn("no write token") + return self._token + + +@pytest.fixture +def session(): + return FakeSession() + + +@pytest.fixture +def client(session): + return ThingiverseClient(FakeTokenManager(), session=session) + + +class TestNormalizeHash: + def test_base64_md5_to_hex(self): + assert normalize_hash(MD5_B64) == MD5_HEX + + def test_hex_passthrough_lowercased(self): + assert normalize_hash(MD5_HEX.upper()) == MD5_HEX + + def test_none_and_empty(self): + assert normalize_hash(None) is None + assert normalize_hash("") is None + + def test_invalid_base64_falls_back_to_lower(self): + junk = "!!!!!!!!!!!!!!!!!!!!!!==" # 24 chars, ends '=', not base64 + assert normalize_hash(junk) == junk.lower() + + +class TestThings: + def test_create_thing_payload(self, client, session): + session.queue(FakeResponse(200, {"id": 999, "name": "X"})) + thing = client.create_thing( + "My Thing", license="cc", category="Toy & Game Accessories" + ) + assert thing["id"] == 999 + call = session.calls[0] + assert call["method"] == "POST" + assert call["url"] == f"{API_BASE}/things/" + assert call["headers"]["Authorization"] == "Bearer write-tok" + assert call["json"] == { + "name": "My Thing", + "license": "cc", + "category": "Toy & Game Accessories", + } + + def test_create_thing_optional_fields(self, client, session): + session.queue(FakeResponse(200, {"id": 1})) + client.create_thing("N", "cc", "Cat", description="hi", tags=["a", "b"]) + assert session.calls[0]["json"]["description"] == "hi" + assert session.calls[0]["json"]["tags"] == ["a", "b"] + + def test_get_thing(self, client, session): + session.queue(FakeResponse(200, {"id": 5, "name": "T"})) + assert client.get_thing(5)["name"] == "T" + assert session.calls[0]["url"] == f"{API_BASE}/things/5" + + def test_update_thing_patches_fields(self, client, session): + session.queue(FakeResponse(200, {"id": 5})) + client.update_thing(5, description="new", name="renamed") + call = session.calls[0] + assert call["method"] == "PATCH" + assert call["json"] == {"description": "new", "name": "renamed"} + + def test_delete_thing(self, client, session): + session.queue(FakeResponse(200, {"ok": "ok"})) + assert client.delete_thing(7)["ok"] == "ok" + assert session.calls[0]["method"] == "DELETE" + assert session.calls[0]["url"] == f"{API_BASE}/things/7" + + def test_publish_thing(self, client, session): + session.queue(FakeResponse(200, {"ok": "ok"})) + client.publish_thing(7) + assert session.calls[0]["url"] == f"{API_BASE}/things/7/publish" + + +class TestFiles: + def test_get_thing_files_adds_normalized_md5(self, client, session): + session.queue( + FakeResponse( + 200, + [ + {"id": 1, "name": "a.stl", "hash": MD5_B64}, + {"id": 2, "name": "b.stl", "hash": MD5_HEX}, + ], + ) + ) + files = client.get_thing_files(9) + assert files[0]["md5"] == MD5_HEX # base64 decoded + assert files[1]["md5"] == MD5_HEX # hex preserved + # raw hash preserved too + assert files[0]["hash"] == MD5_B64 + + def test_get_thing_files_handles_wrapped_list(self, client, session): + session.queue(FakeResponse(200, {"files": [{"id": 1, "hash": MD5_HEX}]})) + files = client.get_thing_files(9) + assert files[0]["md5"] == MD5_HEX + + def test_upload_file_multipart(self, client, session, tmp_path): + stl = tmp_path / "model.stl" + stl.write_bytes(b"solid x\nendsolid x\n") + session.queue(FakeResponse(200, {"id": 44950762})) + + result = client.upload_file(9, stl) + + assert result["id"] == 44950762 + call = session.calls[0] + assert call["method"] == "POST" + assert call["url"] == f"{API_BASE}/files/9/uploadFile" + assert "files" in call # multipart, not json + assert call["files"]["file"][0] == "model.stl" + + def test_finalize_files_payload_and_ranks(self, client, session): + session.queue(FakeResponse(200, {"ok": "ok"})) + client.finalize_files(9, [111, 222, 333]) + call = session.calls[0] + assert call["url"] == f"{API_BASE}/files/9/FinalizeFiles" + assert call["json"] == { + "pending_uploads": [ + {"id": 111, "rank": 10}, + {"id": 222, "rank": 20}, + {"id": 333, "rank": 30}, + ], + "target_id": 9, + "target_type": "thing", + } + + +class TestErrorsAndAuth: + def test_non_2xx_raises_with_detail(self, client, session): + session.queue(FakeResponse(400, {"error": "license is required"})) + with pytest.raises(ThingiverseAPIError) as exc: + client.create_thing("N", "", "Cat") + assert exc.value.status == 400 + assert "license is required" in str(exc.value) + + def test_non_2xx_non_json_body(self, client, session): + session.queue(FakeResponse(429, ValueError("cloudflare html"))) + with pytest.raises(ThingiverseAPIError) as exc: + client.get_thing(1) + assert exc.value.status == 429 + + def test_empty_body_returns_empty_dict(self, client, session): + session.queue(FakeResponse(200, ValueError("no body"))) + assert client.get_thing(1) == {} + + def test_write_token_consulted_per_request(self, session): + tm = FakeTokenManager() + client = ThingiverseClient(tm, session=session) + session.queue(FakeResponse(200, {"id": 1})) + session.queue(FakeResponse(200, {"id": 1})) + client.get_thing(1) + client.get_thing(1) + assert tm.calls == 2 # re-consulted each request (picks up re-login) + + def test_not_logged_in_propagates(self, session): + client = ThingiverseClient(FakeTokenManager(token=None), session=session) + with pytest.raises(NotLoggedIn): + client.get_thing(1) + + def test_context_manager_closes(self, session): + with ThingiverseClient(FakeTokenManager(), session=session) as c: + assert c.session is session + assert session.closed + + def test_default_session_is_requests_session(self): + import requests + + client = ThingiverseClient(FakeTokenManager()) + assert isinstance(client.session, requests.Session) + assert client.session.headers["User-Agent"] == "openforge-catalog-tools" + client.close() From 3764cbcbde5781edd70973e8418eb17b8f116818 Mon Sep 17 00:00:00 2001 From: Devon Jones Date: Tue, 7 Jul 2026 13:10:07 -0600 Subject: [PATCH 2/2] fix: address round-1 review on the Thingiverse client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - wire logger into _request (debug method/path/status, never token; warn on unparseable 2xx body) and normalize_hash (warn on undecodable hash) β€” resolves the dead-logger + silent-fallback forensics findings - add delete_file(thing_id, file_id) (DELETE .../files/{id}) for the sync engine's file-granular removal - document finalize_files rank scope (call-scoped) and comment the tolerated non-JSON error-body except and _peek_tokens swallow - correct docs/thingiverse-api-v2-private.md endpoint table (DELETE /things/{id}; /files/{thing_id}/uploadFile; per-file delete; thingops is collection-scoped) - tests: delete_file, log-on-empty-body, log-on-bad-hash (100% branch) Part of openforge_catalog-gn2 Co-Authored-By: Claude Fable 5 πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- docs/thingiverse-api-v2-private.md | 27 ++++++++++++++++----------- openforge/thingiverse/auth.py | 7 ++++++- openforge/thingiverse/client.py | 27 +++++++++++++++++++++++++-- tests/test_thingiverse_client.py | 25 +++++++++++++++++++++---- 4 files changed, 68 insertions(+), 18 deletions(-) diff --git a/docs/thingiverse-api-v2-private.md b/docs/thingiverse-api-v2-private.md index 832cbbc..f3e0774 100644 --- a/docs/thingiverse-api-v2-private.md +++ b/docs/thingiverse-api-v2-private.md @@ -41,24 +41,29 @@ token. **The write surface is the v1 API**, not v2. From the swagger path list; payload shapes to be confirmed on first real use. +All confirmed live except where noted (βœ“ = round-tripped against the API). + | Purpose | Endpoint | |---|---| -| Create thing | `POST /things/` (v1 create; old tooling payload `{name, license, category, tags[], description}`) | +| Create thing βœ“ | `POST /things/` `{name, license (slug e.g. "cc"), category (display name)}` β†’ `{id, …}` (name/license/category required) | | Update thing | `PATCH /things/{thing_id}` | -| Upload a file | `POST /files/0/uploadFile` (multipart/form-data β†’ `{"id": }`); the `0` is the thing sentinel | -| Finalize upload | `POST /files/0/FinalizeFiles` (the `Finalize`/`/files/{id}/finalize` variants are DEPRECATED) | -| Thing's files (hash lives here) | `GET /things/{thing_id}/files`, `GET /things/{thing_id}/files/{file_id}` | +| Delete thing βœ“ | `DELETE /things/{thing_id}` β†’ `{"ok":"ok"}` | +| Upload a file βœ“ | `POST /files/{thing_id}/uploadFile` (multipart, field name `file`) β†’ `{"id": }` β€” a PENDING upload. (The SPA uses the `0` sentinel pre-save; a real `thing_id` works and is what the tool uses.) | +| Finalize uploads βœ“ | `POST /files/{thing_id}/FinalizeFiles` `{pending_uploads:[{id, rank}], target_id: , target_type:"thing"}` β€” commits pendings so they appear in the file list. (The `/files/{id}/finalize` singular variant is DEPRECATED.) | +| Thing's files (hash lives here) βœ“ | `GET /things/{thing_id}/files`, `GET /things/{thing_id}/files/{file_id}` | +| Delete a file | `DELETE /things/{thing_id}/files/{file_id}` (file-granular removal for sync) | | Images | `GET/POST /things/{thing_id}/images`, `/images/{image_id}` | | Publish | `POST /things/{thing_id}/publish` | -| Delete/move/copy | `POST /thingops/{ids}/remove|move|copy` | | Download | `GET /files/{file_id}/download` | -Observed upload flow in the SPA: `POST /files/0/uploadFile` (multipart, one -per file) β†’ returns `{id}` β†’ files attach to the draft. A **finalize** step -(`/files/0/FinalizeFiles`) commits them. Create-thing (`POST /things/`) was -NOT captured live (the generic editor stalled on a Thingiverse-side 500), so -confirm its exact payload against the swagger `resources/thing.yaml#/Root` -and on first real create. +Note: `POST /thingops/{ids}/remove|move|copy` is **collection**-scoped (it +requires a `collection_id`), NOT thing deletion β€” use `DELETE /things/{id}`. + +Confirmed upload flow: `POST /files/{thing_id}/uploadFile` (multipart, one per +file) β†’ `{id}` (pending, does NOT yet appear in the file list) β†’ collect the +ids β†’ `POST /files/{thing_id}/FinalizeFiles` with `pending_uploads` + +`target_id`/`target_type` to commit. Rank is call-scoped (each finalize +restarts numbering), so finalize a thing's uploads in one call. ## Known limitation (from Devon) diff --git a/openforge/thingiverse/auth.py b/openforge/thingiverse/auth.py index 354fbce..7f6a84b 100644 --- a/openforge/thingiverse/auth.py +++ b/openforge/thingiverse/auth.py @@ -341,7 +341,12 @@ def _store_tokens(self, tokens: Dict): raise def _peek_tokens(self) -> Dict: - """Read stored tokens, returning {} instead of raising if absent.""" + """Read stored tokens, returning {} if absent or unreadable. + + Used only to carry an existing session_token forward on refresh; a + missing-or-corrupt file just means "no prior token to carry" (the + caller is about to write a fresh, valid token file regardless). + """ try: return self._load_tokens() except NotLoggedIn: diff --git a/openforge/thingiverse/client.py b/openforge/thingiverse/client.py index a19523f..c6cee48 100644 --- a/openforge/thingiverse/client.py +++ b/openforge/thingiverse/client.py @@ -66,6 +66,9 @@ def normalize_hash(h: Optional[str]) -> Optional[str]: try: return binascii.hexlify(base64.b64decode(h, validate=True)).decode() except (binascii.Error, ValueError): + # A hash we can't decode would silently fail (or coincidentally + # collide) in the sync diff β€” surface it instead of hiding it. + logger.warning("unrecognized file hash %r; using as-is", h) return h.lower() return h.lower() @@ -154,11 +157,22 @@ def upload_file(self, thing_id: int, path: Path) -> Dict: files = {"file": (path.name, fh)} return self._request("POST", f"/files/{thing_id}/uploadFile", files=files) + def delete_file(self, thing_id: int, file_id: int) -> Dict: + """Delete a single file from a thing. + + This is the file-granular removal the sync engine uses to drop a + file that's no longer in the manifest (vs. delete_thing()). + """ + return self._request("DELETE", f"/things/{thing_id}/files/{file_id}") + def finalize_files(self, thing_id: int, file_ids: List[int]) -> Dict: """Commit pending uploads, associating them with the thing. - Ranks are assigned by list order (10, 20, …) to preserve intended - file ordering. + Ranks are assigned by list order (10, 20, …). Rank numbering is + CALL-SCOPED β€” a later finalize restarts at 10 rather than + continuing β€” so callers adding files incrementally should pass the + full intended set in one call. The sync engine finalizes a thing's + uploads together, so this holds. """ pending = [{"id": fid, "rank": (i + 1) * 10} for i, fid in enumerate(file_ids)] payload = { @@ -180,9 +194,13 @@ def _request(self, method: str, path: str, **kwargs) -> Dict: timeout=REQUEST_TIMEOUT, **kwargs, ) + # Log method/path/status only β€” never the headers or the token. + logger.debug("%s %s -> %s", method, path, resp.status_code) if not 200 <= resp.status_code < 300: detail = "" try: + # Error bodies are {"error": "..."}; a non-JSON body (e.g. a + # Cloudflare 429 HTML page) yields no detail, not a crash. detail = resp.json().get("error", "") except (ValueError, AttributeError): pass @@ -190,6 +208,11 @@ def _request(self, method: str, path: str, **kwargs) -> Dict: try: return resp.json() except ValueError: + # Some write endpoints legitimately return an empty body. Log it + # so a truncated/corrupt 2xx body (which would otherwise read as + # "zero files" downstream and trigger spurious re-uploads) is + # visible rather than silently masked. + logger.warning("2xx body from %s %s did not parse as JSON", method, path) return {} def close(self): diff --git a/tests/test_thingiverse_client.py b/tests/test_thingiverse_client.py index ad2a715..53d53cd 100644 --- a/tests/test_thingiverse_client.py +++ b/tests/test_thingiverse_client.py @@ -80,9 +80,13 @@ def test_none_and_empty(self): assert normalize_hash(None) is None assert normalize_hash("") is None - def test_invalid_base64_falls_back_to_lower(self): + def test_invalid_base64_falls_back_to_lower_and_warns(self, caplog): + import logging + junk = "!!!!!!!!!!!!!!!!!!!!!!==" # 24 chars, ends '=', not base64 - assert normalize_hash(junk) == junk.lower() + with caplog.at_level(logging.WARNING): + assert normalize_hash(junk) == junk.lower() + assert "unrecognized file hash" in caplog.text class TestThings: @@ -132,6 +136,15 @@ def test_publish_thing(self, client, session): assert session.calls[0]["url"] == f"{API_BASE}/things/7/publish" +class TestFileDelete: + def test_delete_file(self, client, session): + session.queue(FakeResponse(200, {"ok": "ok"})) + client.delete_file(7, 555) + call = session.calls[0] + assert call["method"] == "DELETE" + assert call["url"] == f"{API_BASE}/things/7/files/555" + + class TestFiles: def test_get_thing_files_adds_normalized_md5(self, client, session): session.queue( @@ -198,9 +211,13 @@ def test_non_2xx_non_json_body(self, client, session): client.get_thing(1) assert exc.value.status == 429 - def test_empty_body_returns_empty_dict(self, client, session): + def test_empty_body_returns_empty_dict_and_warns(self, client, session, caplog): + import logging + session.queue(FakeResponse(200, ValueError("no body"))) - assert client.get_thing(1) == {} + with caplog.at_level(logging.WARNING): + assert client.get_thing(1) == {} + assert "did not parse as JSON" in caplog.text def test_write_token_consulted_per_request(self, session): tm = FakeTokenManager()