feat(metadata): wire provider IDs end-to-end to populate hardcover_book_id, google_id, audible_id, asin (bookshelf-yco0j) #1075

Merged

zombor merged 2 commits from bd-bookshelf-yco0j into main 2026-07-10 01:41:03 +00:00

Conversation 3 Commits 2 Files changed 14 +734 -141

zombor commented 2026-07-10 01:02:08 +00:00

Owner

Copy link

Summary

• Previously metadata fetches never wrote book_metadata.hardcover_book_id (or google_id, audible_id, asin from background fetches). The Hardcover sync (bookshelf-oi1l2, PR #1070) filters on hardcover_book_id IS NOT NULL, so sync was a no-op for all existing books.
• Add HardcoverBookID, GoogleBooksID, AudibleID, AmazonASIN fields to metadata.Metadata DTO
• Each provider now sets the appropriate field: Hardcover → HardcoverBookID, Google Books → GoogleBooksID, Audnexus → AudibleID + AmazonASIN
• Add SetHardcoverBookID, SetAudibleID, SetAsin SQL functions in metadata_extra.go (all honour per-field *_locked flags via IF(COALESCE(...)) guard); extend UpdateBookProviderIDs to also write hardcover_book_id
• Wire all 4 setters through PersistMetadataDeps, SaveMetadataDeps, and both call sites (persistInTxWithComic via persistProviderIDs helper, interactive persistMatchedProviderID multi-setter for Hardcover)
• Extract applyProviderIDs, persistProviderIDs, callSetter, nullF64Ptr helpers to keep funlen/gocyclo under gates — no new .golangci.yml exclusions
• 100% internal/ coverage, no schema migrations, no Grimmory-table columns added

Test plan

• make build passes
• make coverage passes (100% coverage gate)
• make lint passes (no new exclusions; pre-existing foreign-worktree errcheck errors only)
• Provider tests verify each provider populates the new DTO fields
• Persist tests verify setters called with correct values, guarded by AllFieldsLocked, error paths propagated
• GetMetadataForShow test verifies HardcoverBookID populated from provider IDs row
• Interactive save test verifies Hardcover save writes both hardcover_id and hardcover_book_id

Closes bead bookshelf-yco0j on merge.

## Summary - Previously metadata fetches never wrote `book_metadata.hardcover_book_id` (or `google_id`, `audible_id`, `asin` from background fetches). The Hardcover sync (bookshelf-oi1l2, PR #1070) filters on `hardcover_book_id IS NOT NULL`, so sync was a no-op for all existing books. - Add `HardcoverBookID`, `GoogleBooksID`, `AudibleID`, `AmazonASIN` fields to `metadata.Metadata` DTO - Each provider now sets the appropriate field: Hardcover → `HardcoverBookID`, Google Books → `GoogleBooksID`, Audnexus → `AudibleID` + `AmazonASIN` - Add `SetHardcoverBookID`, `SetAudibleID`, `SetAsin` SQL functions in `metadata_extra.go` (all honour per-field `*_locked` flags via `IF(COALESCE(...))` guard); extend `UpdateBookProviderIDs` to also write `hardcover_book_id` - Wire all 4 setters through `PersistMetadataDeps`, `SaveMetadataDeps`, and both call sites (`persistInTxWithComic` via `persistProviderIDs` helper, interactive `persistMatchedProviderID` multi-setter for Hardcover) - Extract `applyProviderIDs`, `persistProviderIDs`, `callSetter`, `nullF64Ptr` helpers to keep funlen/gocyclo under gates — no new `.golangci.yml` exclusions - 100% `internal/` coverage, no schema migrations, no Grimmory-table columns added ## Test plan - [x] `make build` passes - [x] `make coverage` passes (100% coverage gate) - [x] `make lint` passes (no new exclusions; pre-existing foreign-worktree errcheck errors only) - [x] Provider tests verify each provider populates the new DTO fields - [x] Persist tests verify setters called with correct values, guarded by `AllFieldsLocked`, error paths propagated - [x] `GetMetadataForShow` test verifies `HardcoverBookID` populated from provider IDs row - [x] Interactive save test verifies Hardcover save writes both `hardcover_id` and `hardcover_book_id` Closes bead bookshelf-yco0j on merge.

zombor added 1 commit 2026-07-10 01:02:08 +00:00

feat(metadata): wire provider IDs end-to-end so fetches populate hardcover_book_id, google_id, audible_id, asin (bookshelf-yco0j) ... d84bb2c739

Previously metadata fetches never wrote book_metadata.hardcover_book_id (or
google_id, audible_id, asin from background fetches). The Hardcover sync
(oi1l2) filters on hardcover_book_id IS NOT NULL, so sync was a no-op.

- Add HardcoverBookID/GoogleBooksID/AudibleID/AmazonASIN fields to Metadata DTO
- Hardcover provider sets HardcoverBookID from b.ID (same numeric string as ProviderResourceID)
- Google Books provider sets GoogleBooksID from item.ID
- Audnexus provider sets AudibleID and AmazonASIN from b.ASIN
- Add SetHardcoverBookID/SetAudibleID/SetAsin SQL functions to metadata_extra.go
  (SetGoogleID already existed); extend UpdateBookProviderIDs to write hardcover_book_id
- Wire all 4 setters through PersistMetadataDeps/SaveMetadataDeps and their
  callers (persistProviderIDs helper, persistMatchedProviderID multi-setter)
- Wire.go updated for all 3 call sites (non-tx, tx, rawSaveMeta)
- Extract applyProviderIDs/persistProviderIDs/callSetter/nullF64Ptr helpers to
  keep function lengths and CC under the lint gates
- 100% coverage; no new golangci.yml exclusions; no schema changes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor commented 2026-07-10 01:13:07 +00:00

Author

Owner

Copy link

Security Review — PR #1075 (bookshelf-yco0j)

Persist provider IDs (hardcover_book_id/google_id/audible_id/asin) from metadata fetch + manual save. Read-only review of the diff (origin/main...origin/bd-bookshelf-yco0j).

Findings

1. SQL injection — CLEAN. All new writes are parameterized. UpdateBookProviderIDs uses ? placeholders for every value (internal/db/sqlc/metadata_extra.go:147-171), and the new SetHardcoverBookID/SetAudibleID/SetAsin helpers (metadata_extra.go:236-280) each do SET bm.<col> = ? with ExecContext(ctx, query, <id>, bookID). No string interpolation of provider- or user-supplied ids into SQL anywhere.

2. Lock bypass / integrity — CLEAN. The IF(COALESCE(all_fields_locked,0)=0 AND COALESCE(hardcover_book_id_locked,0)=0, ?, bm.hardcover_book_id) guard is correct (AND of both unlocked → write, else keep existing). The three new Set* helpers replicate the same guard as WHERE predicates (AND all_fields_locked=0 AND <field>_locked=0 AND b.deleted=0) — a fetch cannot clobber a user-locked id. persistProviderIDs additionally short-circuits when existing.AllFieldsLocked in Go (metadata_store.go:1097); per-field locks are enforced at the SQL layer, no bypass. The form-mirror p.HardcoverBookID = nullStr(req.HardcoverID) (metadata_store.go:672) writes both columns from one field but each column's own lock still gates its write — correct.

3. Manual form input — CLEAN. No new user-input surface: dto.go is unchanged (the hardcover_id/audible_id/asin form fields pre-exist) and there are zero changes to handler.go/routes.go — the metadata-edit endpoint's existing auth gating is untouched. Values flow to varchar(100) columns via parameterized writes; over-length input yields a DB error, not injection (no security impact).

4. Secrets/PII — CLEAN. Persisted values are opaque provider book IDs (Hardcover numeric id, Google volume id, Audible/Amazon ASIN) — not secrets or PII. The non-fatal Warn log (metadata_service.go:632) carries only book_id/provider/error/trace_id. No API keys/tokens persisted or logged.

Notes

[MINOR] internal/db/sqlc/metadata_extra.go:122 — UpdateBookProviderIDsParams struct fields are not gofmt-aligned (HardcoverBookID is longer than its neighbors, which were not re-aligned). Cosmetic; gofmt/lint in CI will flag it. Run gofmt -w.

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## Security Review — PR #1075 (bookshelf-yco0j) Persist provider IDs (`hardcover_book_id`/`google_id`/`audible_id`/`asin`) from metadata fetch + manual save. Read-only review of the diff (`origin/main...origin/bd-bookshelf-yco0j`). ### Findings **1. SQL injection — CLEAN.** All new writes are parameterized. `UpdateBookProviderIDs` uses `?` placeholders for every value (`internal/db/sqlc/metadata_extra.go:147-171`), and the new `SetHardcoverBookID`/`SetAudibleID`/`SetAsin` helpers (`metadata_extra.go:236-280`) each do `SET bm.<col> = ?` with `ExecContext(ctx, query, <id>, bookID)`. No string interpolation of provider- or user-supplied ids into SQL anywhere. **2. Lock bypass / integrity — CLEAN.** The `IF(COALESCE(all_fields_locked,0)=0 AND COALESCE(hardcover_book_id_locked,0)=0, ?, bm.hardcover_book_id)` guard is correct (AND of both unlocked → write, else keep existing). The three new `Set*` helpers replicate the same guard as WHERE predicates (`AND all_fields_locked=0 AND <field>_locked=0 AND b.deleted=0`) — a fetch cannot clobber a user-locked id. `persistProviderIDs` additionally short-circuits when `existing.AllFieldsLocked` in Go (`metadata_store.go:1097`); per-field locks are enforced at the SQL layer, no bypass. The form-mirror `p.HardcoverBookID = nullStr(req.HardcoverID)` (`metadata_store.go:672`) writes both columns from one field but each column's own lock still gates its write — correct. **3. Manual form input — CLEAN.** No new user-input surface: `dto.go` is unchanged (the `hardcover_id`/`audible_id`/`asin` form fields pre-exist) and there are zero changes to `handler.go`/`routes.go` — the metadata-edit endpoint's existing auth gating is untouched. Values flow to varchar(100) columns via parameterized writes; over-length input yields a DB error, not injection (no security impact). **4. Secrets/PII — CLEAN.** Persisted values are opaque provider book IDs (Hardcover numeric id, Google volume id, Audible/Amazon ASIN) — not secrets or PII. The non-fatal Warn log (`metadata_service.go:632`) carries only `book_id`/`provider`/`error`/`trace_id`. No API keys/tokens persisted or logged. ### Notes [MINOR] internal/db/sqlc/metadata_extra.go:122 — `UpdateBookProviderIDsParams` struct fields are not gofmt-aligned (`HardcoverBookID` is longer than its neighbors, which were not re-aligned). Cosmetic; `gofmt`/lint in CI will flag it. Run `gofmt -w`. REVIEW VERDICT: 0 blocker, 0 major, 1 minor

zombor commented 2026-07-10 01:13:30 +00:00

Author

Owner

Copy link

Security Review — PR #1075 (bookshelf-yco0j)

Persist provider IDs (hardcover_book_id/google_id/audible_id/asin) from metadata fetch + manual save. Read-only review of the diff (origin/main...origin/bd-bookshelf-yco0j).

Findings

1. SQL injection — CLEAN. All new writes are parameterized. UpdateBookProviderIDs uses ? placeholders for every value (internal/db/sqlc/metadata_extra.go:147-171), and the new SetHardcoverBookID/SetAudibleID/SetAsin helpers (metadata_extra.go:236-280) each do SET bm.<col> = ? with ExecContext(ctx, query, <id>, bookID). No string interpolation of provider- or user-supplied ids into SQL anywhere.

2. Lock bypass / integrity — CLEAN. The IF(COALESCE(all_fields_locked,0)=0 AND COALESCE(hardcover_book_id_locked,0)=0, ?, bm.hardcover_book_id) guard is correct (AND of both unlocked → write, else keep existing). The three new Set* helpers replicate the same guard as WHERE predicates (AND all_fields_locked=0 AND <field>_locked=0 AND b.deleted=0) — a fetch cannot clobber a user-locked id. persistProviderIDs additionally short-circuits when existing.AllFieldsLocked in Go (metadata_store.go:1097); per-field locks are enforced at the SQL layer, no bypass. The form-mirror p.HardcoverBookID = nullStr(req.HardcoverID) (metadata_store.go:672) writes both columns from one field but each column's own lock still gates its write — correct.

3. Manual form input — CLEAN. No new user-input surface: dto.go is unchanged (the hardcover_id/audible_id/asin form fields pre-exist) and there are zero changes to handler.go/routes.go — the metadata-edit endpoint's existing auth gating is untouched. Values flow to varchar(100) columns via parameterized writes; over-length input yields a DB error, not injection (no security impact).

4. Secrets/PII — CLEAN. Persisted values are opaque provider book IDs (Hardcover numeric id, Google volume id, Audible/Amazon ASIN) — not secrets or PII. The non-fatal Warn log (metadata_service.go:632) carries only book_id/provider/error/trace_id. No API keys/tokens persisted or logged.

Notes

[MINOR] internal/db/sqlc/metadata_extra.go:122 — UpdateBookProviderIDsParams struct fields are not gofmt-aligned (HardcoverBookID is longer than its neighbors, which were not re-aligned). Cosmetic; gofmt/lint in CI will flag it. Run gofmt -w.

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## Security Review — PR #1075 (bookshelf-yco0j) Persist provider IDs (`hardcover_book_id`/`google_id`/`audible_id`/`asin`) from metadata fetch + manual save. Read-only review of the diff (`origin/main...origin/bd-bookshelf-yco0j`). ### Findings **1. SQL injection — CLEAN.** All new writes are parameterized. `UpdateBookProviderIDs` uses `?` placeholders for every value (`internal/db/sqlc/metadata_extra.go:147-171`), and the new `SetHardcoverBookID`/`SetAudibleID`/`SetAsin` helpers (`metadata_extra.go:236-280`) each do `SET bm.<col> = ?` with `ExecContext(ctx, query, <id>, bookID)`. No string interpolation of provider- or user-supplied ids into SQL anywhere. **2. Lock bypass / integrity — CLEAN.** The `IF(COALESCE(all_fields_locked,0)=0 AND COALESCE(hardcover_book_id_locked,0)=0, ?, bm.hardcover_book_id)` guard is correct (AND of both unlocked → write, else keep existing). The three new `Set*` helpers replicate the same guard as WHERE predicates (`AND all_fields_locked=0 AND <field>_locked=0 AND b.deleted=0`) — a fetch cannot clobber a user-locked id. `persistProviderIDs` additionally short-circuits when `existing.AllFieldsLocked` in Go (`metadata_store.go:1097`); per-field locks are enforced at the SQL layer, no bypass. The form-mirror `p.HardcoverBookID = nullStr(req.HardcoverID)` (`metadata_store.go:672`) writes both columns from one field but each column's own lock still gates its write — correct. **3. Manual form input — CLEAN.** No new user-input surface: `dto.go` is unchanged (the `hardcover_id`/`audible_id`/`asin` form fields pre-exist) and there are zero changes to `handler.go`/`routes.go` — the metadata-edit endpoint's existing auth gating is untouched. Values flow to varchar(100) columns via parameterized writes; over-length input yields a DB error, not injection (no security impact). **4. Secrets/PII — CLEAN.** Persisted values are opaque provider book IDs (Hardcover numeric id, Google volume id, Audible/Amazon ASIN) — not secrets or PII. The non-fatal Warn log (`metadata_service.go:632`) carries only `book_id`/`provider`/`error`/`trace_id`. No API keys/tokens persisted or logged. ### Notes [MINOR] internal/db/sqlc/metadata_extra.go:122 — `UpdateBookProviderIDsParams` struct fields are not gofmt-aligned (`HardcoverBookID` is longer than its neighbors, which were not re-aligned). Cosmetic; `gofmt`/lint in CI will flag it. Run `gofmt -w`. REVIEW VERDICT: 0 blocker, 0 major, 1 minor

zombor commented 2026-07-10 01:13:36 +00:00

Author

Owner

Copy link

Security Review — PR #1075 (bookshelf-yco0j)

Persist provider IDs (hardcover_book_id/google_id/audible_id/asin) from metadata fetch + manual save. Read-only review of the diff (origin/main...origin/bd-bookshelf-yco0j).

Findings

1. SQL injection — CLEAN. All new writes are parameterized. UpdateBookProviderIDs uses ? placeholders for every value (internal/db/sqlc/metadata_extra.go:147-171), and the new SetHardcoverBookID/SetAudibleID/SetAsin helpers (metadata_extra.go:236-280) each do SET bm.<col> = ? with ExecContext(ctx, query, <id>, bookID). No string interpolation of provider- or user-supplied ids into SQL anywhere.

2. Lock bypass / integrity — CLEAN. The IF(COALESCE(all_fields_locked,0)=0 AND COALESCE(hardcover_book_id_locked,0)=0, ?, bm.hardcover_book_id) guard is correct (AND of both unlocked → write, else keep existing). The three new Set* helpers replicate the same guard as WHERE predicates (AND all_fields_locked=0 AND <field>_locked=0 AND b.deleted=0) — a fetch cannot clobber a user-locked id. persistProviderIDs additionally short-circuits when existing.AllFieldsLocked in Go (metadata_store.go:1097); per-field locks are enforced at the SQL layer, no bypass. The form-mirror p.HardcoverBookID = nullStr(req.HardcoverID) (metadata_store.go:672) writes both columns from one field but each column's own lock still gates its write — correct.

3. Manual form input — CLEAN. No new user-input surface: dto.go is unchanged (the hardcover_id/audible_id/asin form fields pre-exist) and there are zero changes to handler.go/routes.go — the metadata-edit endpoint's existing auth gating is untouched. Values flow to varchar(100) columns via parameterized writes; over-length input yields a DB error, not injection (no security impact).

4. Secrets/PII — CLEAN. Persisted values are opaque provider book IDs (Hardcover numeric id, Google volume id, Audible/Amazon ASIN) — not secrets or PII. The non-fatal Warn log (metadata_service.go:632) carries only book_id/provider/error/trace_id. No API keys/tokens persisted or logged.

Notes

[MINOR] internal/db/sqlc/metadata_extra.go:122 — UpdateBookProviderIDsParams struct fields are not gofmt-aligned (HardcoverBookID is longer than its neighbors, which were not re-aligned). Cosmetic; gofmt/lint in CI will flag it. Run gofmt -w.

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## Security Review — PR #1075 (bookshelf-yco0j) Persist provider IDs (`hardcover_book_id`/`google_id`/`audible_id`/`asin`) from metadata fetch + manual save. Read-only review of the diff (`origin/main...origin/bd-bookshelf-yco0j`). ### Findings **1. SQL injection — CLEAN.** All new writes are parameterized. `UpdateBookProviderIDs` uses `?` placeholders for every value (`internal/db/sqlc/metadata_extra.go:147-171`), and the new `SetHardcoverBookID`/`SetAudibleID`/`SetAsin` helpers (`metadata_extra.go:236-280`) each do `SET bm.<col> = ?` with `ExecContext(ctx, query, <id>, bookID)`. No string interpolation of provider- or user-supplied ids into SQL anywhere. **2. Lock bypass / integrity — CLEAN.** The `IF(COALESCE(all_fields_locked,0)=0 AND COALESCE(hardcover_book_id_locked,0)=0, ?, bm.hardcover_book_id)` guard is correct (AND of both unlocked → write, else keep existing). The three new `Set*` helpers replicate the same guard as WHERE predicates (`AND all_fields_locked=0 AND <field>_locked=0 AND b.deleted=0`) — a fetch cannot clobber a user-locked id. `persistProviderIDs` additionally short-circuits when `existing.AllFieldsLocked` in Go (`metadata_store.go:1097`); per-field locks are enforced at the SQL layer, no bypass. The form-mirror `p.HardcoverBookID = nullStr(req.HardcoverID)` (`metadata_store.go:672`) writes both columns from one field but each column's own lock still gates its write — correct. **3. Manual form input — CLEAN.** No new user-input surface: `dto.go` is unchanged (the `hardcover_id`/`audible_id`/`asin` form fields pre-exist) and there are zero changes to `handler.go`/`routes.go` — the metadata-edit endpoint's existing auth gating is untouched. Values flow to varchar(100) columns via parameterized writes; over-length input yields a DB error, not injection (no security impact). **4. Secrets/PII — CLEAN.** Persisted values are opaque provider book IDs (Hardcover numeric id, Google volume id, Audible/Amazon ASIN) — not secrets or PII. The non-fatal Warn log (`metadata_service.go:632`) carries only `book_id`/`provider`/`error`/`trace_id`. No API keys/tokens persisted or logged. ### Notes [MINOR] internal/db/sqlc/metadata_extra.go:122 — `UpdateBookProviderIDsParams` struct fields are not gofmt-aligned (`HardcoverBookID` is longer than its neighbors, which were not re-aligned). Cosmetic; `gofmt`/lint in CI will flag it. Run `gofmt -w`. REVIEW VERDICT: 0 blocker, 0 major, 1 minor

zombor added 1 commit 2026-07-10 01:18:07 +00:00

fix(yco0j): gofmt alignment on provider-ID structs and test file ... c9b740f635

Run gofmt -w on the three files touched by the feat commit that had
alignment nits flagged by the code review.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor force-pushed bd-bookshelf-yco0j from c9b740f635 to 1d11d9baed 2026-07-10 01:33:42 +00:00 Compare

zombor merged commit 1c8c41b997 into main 2026-07-10 01:41:03 +00:00

zombor referenced this pull request from a commit 2026-07-10 01:41:08 +00:00

Merge pull request 'feat(metadata): wire provider IDs end-to-end to populate hardcover_book_id, google_id, audible_id, asin (bookshelf-yco0j)' (#1075) from bd-bookshelf-yco0j into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!1075

No description provided.