spikerj/spikersoft-issues
1
0
Fork 0
Code Issues 196 Pull Requests Actions Packages Projects Releases Wiki Activity

[Bug][Prod][Reader] Every bookmark operation 500s — UserBookmark.UserId is ObjectId but requests are keyed by Keycloak username #617

New Issue
Closed
opened 2026-07-16 18:28:41 +00:00 by spikerj · 1 comment
spikerj commented 2026-07-16 18:28:41 +00:00
Owner
Copy Link
Copy Source

Summary

All three bookmark endpoints (GET/POST /api/reader/bookmarks*, DELETE /api/reader/bookmarks/{id}) throw FormatException for any real user, because the handlers ObjectId.Parse the caller identity — which is User.Identity.Name = Keycloak preferred_username, a human string like joey.

Root cause

  • SpikerSoft.Data/Mongos/UserBookmark.cs: UserId declared ObjectId (UpvotedBy likewise List<ObjectId>).
  • CreateBookmarkCommandHandler.cs:17, GetBookBookmarksQueryHandler.cs:16, DeleteBookmarkCommandHandler.cs:23 all call ObjectId.Parse(request.UserId) on the username.
  • Meanwhile ReadingProgress.UserId and Book.UploadedBy are already plain username strings — bookmarks were the odd one out.
  • Unit tests hid the bug by passing ObjectId-shaped strings as user ids.

Fix

Re-key UserBookmark.UserId (and UpvotedBy) to username strings, matching ReadingProgress; remove the parses; migration script mongodb-scripts/2026-07-migrate-userbookmarks-userid.js (existing ObjectId-keyed rows were unreachable — read path threw before any real user could create data); tests now use realistic usernames; added {bookId, page, isPublic} index for the upcoming social-bookmarks feature.

Also riding along in the same PR (small, verified):

  • EpubReaderService now opens EPUBs with the same lenient EpubReaderOptions as the MetadataExtractor, so a malformed EPUB that ingested successfully can't produce a different reading-order count in the live reader (page-number drift between BookPages and /epub-page).
  • GET /api/book/{id}/file now sets enableRangeProcessing: true — pdf.js range requests were silently unsupported, forcing whole-file downloads.

Discovered during the reader/Kavita-parity planning session (2026-07-16).

## Summary All three bookmark endpoints (`GET/POST /api/reader/bookmarks*`, `DELETE /api/reader/bookmarks/{id}`) throw `FormatException` for any real user, because the handlers `ObjectId.Parse` the caller identity — which is `User.Identity.Name` = Keycloak `preferred_username`, a human string like `joey`. ## Root cause - `SpikerSoft.Data/Mongos/UserBookmark.cs`: `UserId` declared `ObjectId` (`UpvotedBy` likewise `List<ObjectId>`). - `CreateBookmarkCommandHandler.cs:17`, `GetBookBookmarksQueryHandler.cs:16`, `DeleteBookmarkCommandHandler.cs:23` all call `ObjectId.Parse(request.UserId)` on the username. - Meanwhile `ReadingProgress.UserId` and `Book.UploadedBy` are already plain username strings — bookmarks were the odd one out. - Unit tests hid the bug by passing ObjectId-shaped strings as user ids. ## Fix Re-key `UserBookmark.UserId` (and `UpvotedBy`) to username strings, matching `ReadingProgress`; remove the parses; migration script `mongodb-scripts/2026-07-migrate-userbookmarks-userid.js` (existing ObjectId-keyed rows were unreachable — read path threw before any real user could create data); tests now use realistic usernames; added `{bookId, page, isPublic}` index for the upcoming social-bookmarks feature. Also riding along in the same PR (small, verified): - `EpubReaderService` now opens EPUBs with the same **lenient** `EpubReaderOptions` as the MetadataExtractor, so a malformed EPUB that ingested successfully can't produce a different reading-order count in the live reader (page-number drift between `BookPages` and `/epub-page`). - `GET /api/book/{id}/file` now sets `enableRangeProcessing: true` — pdf.js range requests were silently unsupported, forcing whole-file downloads. Discovered during the reader/Kavita-parity planning session (2026-07-16).
spikerj commented 2026-07-17 01:59:15 +00:00
Author
Owner
Copy Link
Copy Source

Resolved in spikersoft-backend PR #326 (merged to master, 4d22c0bb — PR-0 of the reader plan): UserBookmark re-keyed to Keycloak username strings, all three bookmark endpoints no longer ObjectId.Parse the caller identity. The API has built + deployed green multiple times since the merge (most recently on the #337 merge commit), so the fix is live. Closing.

Resolved in spikersoft-backend PR #326 (merged to `master`, 4d22c0bb — PR-0 of the reader plan): `UserBookmark` re-keyed to Keycloak username strings, all three bookmark endpoints no longer `ObjectId.Parse` the caller identity. The API has built + deployed green multiple times since the merge (most recently on the #337 merge commit), so the fix is live. Closing.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#617