Bookhoard/bookhoard
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 12 Wiki Activity

fix(bookmarks): upsert on title conflict so position upgrades don't 500

Browse Source 
Bookmark dedup is keyed on hash(title + position bucket), but the table
also enforces UNIQUE(media_item_id, user_id, title). When a client re-
saves the same bookmark title with a changed position form - e.g. the
Android app upgrading a percentage-only row to an EPUB CFI, or a web and
app bookmark landing on the same 'Bookmark at 44%' title - the dedup-key
lookup misses and the INSERT violates the title constraint, returning
HTTP 500 and failing the sync.

A title collision on the same (user, item) is by definition the same
bookmark slot, so take the LWW semantics all the way: ON CONFLICT DO
UPDATE replaces position/cfi_position/page/chapter/percentage, refreshes
dedup_key and timestamps, merges device_sync_data, and - matching
UpdateMediaBookmarkForSync - clears deleted/deleted_at so a re-create
resurrects a tombstoned title slot instead of leaving an invisible row
holding it.

Device sync flows are unaffected: KOReader/Kobo pushes that carry their
own dedup-key echoes never reach the INSERT, and same-key saves still go
through applyBookmarkLWW with its tombstone freshness checks.
This commit is contained in:
john-okeefe
2026-08-30 20:57:15 -04:00
parent 5c5593644d
commit 27b3dcb69f
5 changed files with 50 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/database/db.go
+1 -1
View File
@@ -1,6 +1,6 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
// sqlc v1.30.0
// sqlc v1.31.1
package database
internal/database/models.go
+1 -1
View File
@@ -1,6 +1,6 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
// sqlc v1.30.0
// sqlc v1.31.1
package database
internal/database/querier.go
+5 -1
View File
@@ -1,6 +1,6 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
// sqlc v1.30.0
// sqlc v1.31.1
package database
@@ -60,6 +60,10 @@ type Querier interface {
// Libraries queries
CreateLibrary(ctx context.Context, arg CreateLibraryParams) (Libraries, error)
CreateMediaBookmark(ctx context.Context, arg CreateMediaBookmarkParams) (MediaBookmarks, error)
// The dedup key can change while the title stays the same (e.g. a client
// upgrading a bookmark from percentage-only to a CFI position), so the
// title UNIQUE constraint races the dedup-key lookup. Upsert on the title
// slot: the conflicting row is by definition the same bookmark.
CreateMediaBookmarkFull(ctx context.Context, arg CreateMediaBookmarkFullParams) (MediaBookmarks, error)
// Media Highlights queries
CreateMediaHighlight(ctx context.Context, arg CreateMediaHighlightParams) (MediaHighlights, error)
internal/database/queries.sql.go
+22 -2
View File
@@ -1,6 +1,6 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
// sqlc v1.30.0
// sqlc v1.31.1
// source: queries.sql
package database
@@ -683,7 +683,23 @@ INSERT INTO media_bookmarks (
device_sync_data
) VALUES (
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
) RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
) ON CONFLICT (media_item_id, user_id, title) DO UPDATE SET
page_number = EXCLUDED.page_number,
chapter_number = EXCLUDED.chapter_number,
cfi_position = EXCLUDED.cfi_position,
position = EXCLUDED.position,
notes = EXCLUDED.notes,
percentage_location = EXCLUDED.percentage_location,
epubcfi_location = EXCLUDED.epubcfi_location,
chapter_reference = EXCLUDED.chapter_reference,
dedup_key = EXCLUDED.dedup_key,
last_modified_at = EXCLUDED.last_modified_at,
last_modified_source = EXCLUDED.last_modified_source,
device_sync_data = EXCLUDED.device_sync_data,
created_at = created_at,
deleted = FALSE,
deleted_at = NULL
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
`
type CreateMediaBookmarkFullParams struct {
@@ -704,6 +720,10 @@ type CreateMediaBookmarkFullParams struct {
DeviceSyncData []byte `db:"device_sync_data" json:"device_sync_data"`
}
// The dedup key can change while the title stays the same (e.g. a client
// upgrading a bookmark from percentage-only to a CFI position), so the
// title UNIQUE constraint races the dedup-key lookup. Upsert on the title
// slot: the conflicting row is by definition the same bookmark.
func (q *Queries) CreateMediaBookmarkFull(ctx context.Context, arg CreateMediaBookmarkFullParams) (MediaBookmarks, error) {
row := q.db.QueryRow(ctx, CreateMediaBookmarkFull,
arg.MediaItemID,
internal/database/queries/queries.sql
+21 -1
View File
@@ -893,6 +893,10 @@ ORDER BY deleted ASC, deleted_at DESC NULLS LAST
LIMIT 1;
-- name: CreateMediaBookmarkFull :one
-- The dedup key can change while the title stays the same (e.g. a client
-- upgrading a bookmark from percentage-only to a CFI position), so the
-- title UNIQUE constraint races the dedup-key lookup. Upsert on the title
-- slot: the conflicting row is by definition the same bookmark.
INSERT INTO media_bookmarks (
media_item_id, user_id, page_number, chapter_number,
cfi_position, title, position, notes,
@@ -901,7 +905,23 @@ INSERT INTO media_bookmarks (
device_sync_data
) VALUES (
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
) RETURNING *;
) ON CONFLICT (media_item_id, user_id, title) DO UPDATE SET
page_number = EXCLUDED.page_number,
chapter_number = EXCLUDED.chapter_number,
cfi_position = EXCLUDED.cfi_position,
position = EXCLUDED.position,
notes = EXCLUDED.notes,
percentage_location = EXCLUDED.percentage_location,
epubcfi_location = EXCLUDED.epubcfi_location,
chapter_reference = EXCLUDED.chapter_reference,
dedup_key = EXCLUDED.dedup_key,
last_modified_at = EXCLUDED.last_modified_at,
last_modified_source = EXCLUDED.last_modified_source,
device_sync_data = EXCLUDED.device_sync_data,
created_at = created_at,
deleted = FALSE,
deleted_at = NULL
RETURNING *;
-- name: UpdateMediaBookmarkForSync :one
UPDATE media_bookmarks SET