fix(sync): resurrect tombstoned annotations when a newer save re-creates them
Deleting a bookmark/highlight/note and then re-adding the same content at the same position (same dedup key — e.g. the reader's auto-titled 'Bookmark at X%') was silently swallowed: the save hit the tombstone branch, returned 201 with the deleted row, and the list (which filters deleted) stayed empty. Bookmarks were further blocked by the UNIQUE(media_item_id, user_id, title) slot the tombstoned row holds, and notes had no TTL escape at all. Tombstones now only block saves that predate them (stale replays from a device that still has the annotation). A save whose modification time is newer than max(deleted_at, last_modified_at) — a deliberate re-create from the web or a device — resurrects the row via the LWW update queries, which now clear deleted/deleted_at.
This commit is contained in:
@@ -11334,7 +11334,9 @@ UPDATE media_bookmarks SET
|
|||||||
last_modified_at = $11,
|
last_modified_at = $11,
|
||||||
last_modified_source = $12,
|
last_modified_source = $12,
|
||||||
device_sync_data = $13,
|
device_sync_data = $13,
|
||||||
created_at = created_at
|
created_at = created_at,
|
||||||
|
deleted = FALSE,
|
||||||
|
deleted_at = NULL
|
||||||
WHERE id = $1
|
WHERE id = $1
|
||||||
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
|
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
|
||||||
`
|
`
|
||||||
@@ -11474,7 +11476,9 @@ UPDATE media_highlights SET
|
|||||||
last_modified_at = $12,
|
last_modified_at = $12,
|
||||||
last_modified_source = $13,
|
last_modified_source = $13,
|
||||||
device_sync_data = $14,
|
device_sync_data = $14,
|
||||||
updated_at = NOW()
|
updated_at = NOW(),
|
||||||
|
deleted = FALSE,
|
||||||
|
deleted_at = NULL
|
||||||
WHERE id = $1
|
WHERE id = $1
|
||||||
RETURNING id, media_item_id, user_id, selection_text, start_position, end_position, color, note_id, created_at, updated_at, percentage_start, percentage_end, character_start, character_end, epubcfi_start, epubcfi_end, chapter_reference, paragraph_start, paragraph_end, panel_number, device_sync_data, dedup_key, last_modified_at, last_modified_source, note_text, deleted, deleted_at
|
RETURNING id, media_item_id, user_id, selection_text, start_position, end_position, color, note_id, created_at, updated_at, percentage_start, percentage_end, character_start, character_end, epubcfi_start, epubcfi_end, chapter_reference, paragraph_start, paragraph_end, panel_number, device_sync_data, dedup_key, last_modified_at, last_modified_source, note_text, deleted, deleted_at
|
||||||
`
|
`
|
||||||
@@ -12152,7 +12156,9 @@ UPDATE media_notes SET
|
|||||||
last_modified_at = $10,
|
last_modified_at = $10,
|
||||||
last_modified_source = $11,
|
last_modified_source = $11,
|
||||||
device_sync_data = $12,
|
device_sync_data = $12,
|
||||||
updated_at = NOW()
|
updated_at = NOW(),
|
||||||
|
deleted = FALSE,
|
||||||
|
deleted_at = NULL
|
||||||
WHERE id = $1
|
WHERE id = $1
|
||||||
RETURNING id, media_item_id, user_id, content, position, created_at, updated_at, percentage_location, character_start, character_end, epubcfi_location, chapter_reference, paragraph_reference, device_sync_data, dedup_key, last_modified_at, last_modified_source, deleted, deleted_at
|
RETURNING id, media_item_id, user_id, content, position, created_at, updated_at, percentage_location, character_start, character_end, epubcfi_location, chapter_reference, paragraph_reference, device_sync_data, dedup_key, last_modified_at, last_modified_source, deleted, deleted_at
|
||||||
`
|
`
|
||||||
|
|||||||
@@ -802,7 +802,9 @@ UPDATE media_highlights SET
|
|||||||
last_modified_at = $12,
|
last_modified_at = $12,
|
||||||
last_modified_source = $13,
|
last_modified_source = $13,
|
||||||
device_sync_data = $14,
|
device_sync_data = $14,
|
||||||
updated_at = NOW()
|
updated_at = NOW(),
|
||||||
|
deleted = FALSE,
|
||||||
|
deleted_at = NULL
|
||||||
WHERE id = $1
|
WHERE id = $1
|
||||||
RETURNING *;
|
RETURNING *;
|
||||||
|
|
||||||
@@ -857,7 +859,9 @@ UPDATE media_notes SET
|
|||||||
last_modified_at = $10,
|
last_modified_at = $10,
|
||||||
last_modified_source = $11,
|
last_modified_source = $11,
|
||||||
device_sync_data = $12,
|
device_sync_data = $12,
|
||||||
updated_at = NOW()
|
updated_at = NOW(),
|
||||||
|
deleted = FALSE,
|
||||||
|
deleted_at = NULL
|
||||||
WHERE id = $1
|
WHERE id = $1
|
||||||
RETURNING *;
|
RETURNING *;
|
||||||
|
|
||||||
@@ -913,7 +917,9 @@ UPDATE media_bookmarks SET
|
|||||||
last_modified_at = $11,
|
last_modified_at = $11,
|
||||||
last_modified_source = $12,
|
last_modified_source = $12,
|
||||||
device_sync_data = $13,
|
device_sync_data = $13,
|
||||||
created_at = created_at
|
created_at = created_at,
|
||||||
|
deleted = FALSE,
|
||||||
|
deleted_at = NULL
|
||||||
WHERE id = $1
|
WHERE id = $1
|
||||||
RETURNING *;
|
RETURNING *;
|
||||||
|
|
||||||
|
|||||||
@@ -100,10 +100,12 @@ func (s *AnnotationService) SaveHighlight(ctx context.Context, req SaveHighlight
|
|||||||
}
|
}
|
||||||
|
|
||||||
if existing.Deleted.Bool {
|
if existing.Deleted.Bool {
|
||||||
if existing.DeletedAt.Valid && time.Since(existing.DeletedAt.Time) < s.tombstoneTTL() {
|
if !incomingNewerThanTombstone(req.ModifiedAt, existing.DeletedAt, existing.LastModifiedAt) {
|
||||||
return &SaveHighlightResult{Highlight: existing, Outcome: SaveOutcomeDeleted}, nil
|
return &SaveHighlightResult{Highlight: existing, Outcome: SaveOutcomeDeleted}, nil
|
||||||
}
|
}
|
||||||
return s.createHighlight(ctx, req, dedupKey)
|
// Newer than the tombstone: a deliberate re-create. Resurrect via the
|
||||||
|
// LWW update (which clears deleted/deleted_at).
|
||||||
|
return s.applyLWW(ctx, req, existing, dedupKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
return s.applyLWW(ctx, req, existing, dedupKey)
|
return s.applyLWW(ctx, req, existing, dedupKey)
|
||||||
@@ -361,7 +363,11 @@ func (s *AnnotationService) SaveNote(ctx context.Context, req SaveNoteRequest) (
|
|||||||
}
|
}
|
||||||
|
|
||||||
if existing.Deleted.Valid && existing.Deleted.Bool {
|
if existing.Deleted.Valid && existing.Deleted.Bool {
|
||||||
return &SaveNoteResult{Note: existing, Outcome: SaveOutcomeDeleted}, nil
|
if !incomingNewerThanTombstone(req.ModifiedAt, existing.DeletedAt, existing.LastModifiedAt) {
|
||||||
|
return &SaveNoteResult{Note: existing, Outcome: SaveOutcomeDeleted}, nil
|
||||||
|
}
|
||||||
|
// Newer than the tombstone: a deliberate re-create. Resurrect.
|
||||||
|
return s.applyNoteLWW(ctx, req, existing, dedupKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
return s.applyNoteLWW(ctx, req, existing, dedupKey)
|
return s.applyNoteLWW(ctx, req, existing, dedupKey)
|
||||||
@@ -504,10 +510,13 @@ func (s *AnnotationService) SaveBookmark(ctx context.Context, req SaveBookmarkRe
|
|||||||
}
|
}
|
||||||
|
|
||||||
if existing.Deleted.Bool {
|
if existing.Deleted.Bool {
|
||||||
if existing.DeletedAt.Valid && time.Since(existing.DeletedAt.Time) < s.tombstoneTTL() {
|
if !incomingNewerThanTombstone(req.ModifiedAt, existing.DeletedAt, existing.LastModifiedAt) {
|
||||||
return &SaveBookmarkResult{Bookmark: existing, Outcome: SaveOutcomeDeleted}, nil
|
return &SaveBookmarkResult{Bookmark: existing, Outcome: SaveOutcomeDeleted}, nil
|
||||||
}
|
}
|
||||||
return s.createBookmark(ctx, req, dedupKey)
|
// Newer than the tombstone: a deliberate re-create. Resurrect via the
|
||||||
|
// LWW update instead of INSERT (the tombstoned row still holds the
|
||||||
|
// UNIQUE(media_item_id, user_id, title) slot).
|
||||||
|
return s.applyBookmarkLWW(ctx, req, existing, dedupKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
return s.applyBookmarkLWW(ctx, req, existing, dedupKey)
|
return s.applyBookmarkLWW(ctx, req, existing, dedupKey)
|
||||||
@@ -717,6 +726,23 @@ func ComputeDedupKey(selectionText, epubcfiStart, startPosition string) string {
|
|||||||
return hex.EncodeToString(h.Sum(nil))
|
return hex.EncodeToString(h.Sum(nil))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// incomingNewerThanTombstone reports whether an incoming save should
|
||||||
|
// resurrect a tombstoned annotation. A save carrying a modification time
|
||||||
|
// newer than the tombstone (e.g. the user deliberately re-adding on the web,
|
||||||
|
// or a device that genuinely re-created it) wins; a save with a missing or
|
||||||
|
// older timestamp is treated as a stale replay from a client that still has
|
||||||
|
// the deleted annotation, and the tombstone stands.
|
||||||
|
func incomingNewerThanTombstone(incoming time.Time, deletedAt, lastModifiedAt pgtype.Timestamptz) bool {
|
||||||
|
if incoming.IsZero() {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
tombstone := deletedAt.Time
|
||||||
|
if lastModifiedAt.Valid && lastModifiedAt.Time.After(tombstone) {
|
||||||
|
tombstone = lastModifiedAt.Time
|
||||||
|
}
|
||||||
|
return incoming.After(tombstone)
|
||||||
|
}
|
||||||
|
|
||||||
func normalizeText(s string) string {
|
func normalizeText(s string) string {
|
||||||
fields := strings.Fields(strings.ToLower(s))
|
fields := strings.Fields(strings.ToLower(s))
|
||||||
return strings.Join(fields, " ")
|
return strings.Join(fields, " ")
|
||||||
|
|||||||
@@ -341,3 +341,31 @@ func pgHighlights(text, color, note string, pctStart, pctEnd float64) database.M
|
|||||||
PercentageEnd: pgtype.Float8{Float64: pctEnd, Valid: pctEnd != 0},
|
PercentageEnd: pgtype.Float8{Float64: pctEnd, Valid: pctEnd != 0},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestIncomingNewerThanTombstone(t *testing.T) {
|
||||||
|
base := time.Date(2026, 8, 14, 12, 0, 0, 0, time.UTC)
|
||||||
|
delAt := pgtype.Timestamptz{Time: base, Valid: true}
|
||||||
|
lastMod := pgtype.Timestamptz{Time: base.Add(-time.Minute), Valid: true}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
incoming time.Time
|
||||||
|
deleted pgtype.Timestamptz
|
||||||
|
lastMod pgtype.Timestamptz
|
||||||
|
want bool
|
||||||
|
}{
|
||||||
|
{"newer than tombstone resurrects", base.Add(time.Hour), delAt, lastMod, true},
|
||||||
|
{"older than tombstone is a stale replay", base.Add(-time.Hour), delAt, lastMod, false},
|
||||||
|
{"missing timestamp never resurrects", time.Time{}, delAt, lastMod, false},
|
||||||
|
{"exactly equal does not resurrect", base, delAt, lastMod, false},
|
||||||
|
{"last_modified newer than deleted_at wins", base.Add(30 * time.Minute), delAt, pgtype.Timestamptz{Time: base.Add(90 * time.Minute), Valid: true}, false},
|
||||||
|
{"invalid timestamps compare against deleted_at", base.Add(time.Hour), delAt, pgtype.Timestamptz{}, true},
|
||||||
|
}
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
if got := incomingNewerThanTombstone(tt.incoming, tt.deleted, tt.lastMod); got != tt.want {
|
||||||
|
t.Errorf("incomingNewerThanTombstone() = %v, want %v", got, tt.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user