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

refactor(annotations): one LWW decision core + one tombstone gate + typed queue decoders

Browse Source 
The consolidation half of H3 — shared LOGIC with thin per-type remains:

- lwwDecision: the three compareIncoming* functions were the same
  skeleton (client timestamp -> strictly-newer wins; no timestamp ->
  any content difference wins) drifting apart only in per-type
  equality and the schema-driven fallback timestamp (bookmarks carry
  no updated_at). The decision now lives in one function; each compare
  keeps only its equality expression.
- idTargetOwnership + tombstoneWins: the id-targeted resolution blocks
  (HighlightID/BookmarkID/NoteID) shared the ownership check and the
  deleted-row gate — now one gate each instead of three copies of the
  resurrect semantics.
- queue.go: syncNote/syncHighlight/syncBookmark decoded device payloads
  through three 10-key if-type-assertion chains; typed map accessors
  (mapStr/mapF64/mapI32/mapDeviceSyncData) replace them — same
  missing-key-is-zero semantics, declarative field lists. The progress
  decoder is untouched (H1 territory, separately tested).

Per-type remains are the things that genuinely cannot merge: request
structs, dedup triples, the per-table sqlc param builders, and the
KOReader/Kobo payload adapters. Pins, H1/H3 tests, both unit suites
green.
This commit is contained in:
John O'Keefe committed 2026-10-17 15:56:24 -04:00
1 parent 7d698ea0d8
commit 11609d13d1
2 files changed
+120 -161
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/sync/annotations.go
+59 -62
View File
@@ -95,6 +95,22 @@ type SaveHighlightResult struct {
Conflict bool Conflict bool
} }
// idTargetOwnership verifies an id-targeted edit's ownership: the URL row
// must belong to the requesting user and media item.
func idTargetOwnership(rowUser, rowItem, reqUser, reqItem pgtype.UUID, kind string, id pgtype.UUID) error {
if rowUser != reqUser || rowItem != reqItem {
return fmt.Errorf("%s %s belongs to another user or media item", kind, id)
}
return nil
}
// tombstoneWins is the shared tombstone gate for id-targeted edits: a
// deleted row stays deleted unless the edit is newer than the tombstone
// (a deliberate re-create — the LWW update then resurrects the row).
func tombstoneWins(deleted bool, deletedAt, lastModifiedAt pgtype.Timestamptz, modifiedAt time.Time) bool {
return deleted && !incomingNewerThanTombstone(modifiedAt, deletedAt, lastModifiedAt)
}
func (s *AnnotationService) SaveHighlight(ctx context.Context, req SaveHighlightRequest) (*SaveHighlightResult, error) { func (s *AnnotationService) SaveHighlight(ctx context.Context, req SaveHighlightRequest) (*SaveHighlightResult, error) {
dedupKey := req.DedupKey dedupKey := req.DedupKey
@@ -108,18 +124,14 @@ func (s *AnnotationService) SaveHighlight(ctx context.Context, req SaveHighlight
return nil, fmt.Errorf("query highlight by id: %w", idErr) return nil, fmt.Errorf("query highlight by id: %w", idErr)
} }
if idErr == nil { if idErr == nil {
if byID.UserID != req.UserID || byID.MediaItemID != req.MediaItemID { if err := idTargetOwnership(byID.UserID, byID.MediaItemID, req.UserID, req.MediaItemID, "highlight", req.HighlightID); err != nil {
return nil, fmt.Errorf("highlight %s belongs to another user or media item", req.HighlightID) return nil, err
} }
if dedupKey == "" { if dedupKey == "" {
dedupKey = byID.DedupKey.String dedupKey = byID.DedupKey.String
} }
if byID.Deleted.Bool { if tombstoneWins(byID.Deleted.Bool, byID.DeletedAt, byID.LastModifiedAt, req.ModifiedAt) {
if !incomingNewerThanTombstone(req.ModifiedAt, byID.DeletedAt, byID.LastModifiedAt) { return &SaveHighlightResult{Highlight: byID, Outcome: SaveOutcomeDeleted}, nil
return &SaveHighlightResult{Highlight: byID, Outcome: SaveOutcomeDeleted}, nil
}
// Newer than the tombstone: a deliberate re-create. Resurrect
// via the LWW update (which clears deleted/deleted_at).
} }
return s.applyLWW(ctx, req, byID, dedupKey) return s.applyLWW(ctx, req, byID, dedupKey)
} }
@@ -275,22 +287,14 @@ func (s *AnnotationService) applyLWW(
} }
func (s *AnnotationService) compareIncoming(req SaveHighlightRequest, existing database.MediaHighlights) (incomingNewer bool, contentChanged bool) { func (s *AnnotationService) compareIncoming(req SaveHighlightRequest, existing database.MediaHighlights) (incomingNewer bool, contentChanged bool) {
if req.ModifiedAt.IsZero() { contentSame := strings.EqualFold(req.SelectionText, existing.SelectionText) &&
contentSame := strings.EqualFold(req.SelectionText, existing.SelectionText) && textEq(req.Color, existing.Color) &&
textEq(req.Color, existing.Color) && textEq(req.NoteText, existing.NoteText) &&
textEq(req.NoteText, existing.NoteText) && floatEq(req.PercentageStart, existing.PercentageStart) &&
floatEq(req.PercentageStart, existing.PercentageStart) && floatEq(req.PercentageEnd, existing.PercentageEnd) &&
floatEq(req.PercentageEnd, existing.PercentageEnd) && locatorRefresher(req.EpubcfiStart, existing.EpubcfiStart) &&
locatorRefresher(req.EpubcfiStart, existing.EpubcfiStart) && locatorRefresher(req.EpubcfiEnd, existing.EpubcfiEnd)
locatorRefresher(req.EpubcfiEnd, existing.EpubcfiEnd) return lwwDecision(req.ModifiedAt, existing.LastModifiedAt, existing.UpdatedAt, contentSame)
return !contentSame, !contentSame
}
existingMod := existing.LastModifiedAt
if !existingMod.Valid {
existingMod = existing.UpdatedAt
}
return req.ModifiedAt.After(existingMod.Time), true
} }
// locatorRefresher reports whether an incoming locator leaves the stored // locatorRefresher reports whether an incoming locator leaves the stored
@@ -551,8 +555,8 @@ func (s *AnnotationService) SaveNote(ctx context.Context, req SaveNoteRequest) (
return nil, fmt.Errorf("query note by id: %w", idErr) return nil, fmt.Errorf("query note by id: %w", idErr)
} }
if idErr == nil { if idErr == nil {
if byID.UserID != req.UserID || byID.MediaItemID != req.MediaItemID { if err := idTargetOwnership(byID.UserID, byID.MediaItemID, req.UserID, req.MediaItemID, "note", req.NoteID); err != nil {
return nil, fmt.Errorf("note %s belongs to another user or media item", req.NoteID) return nil, err
} }
if dedupKey == "" { if dedupKey == "" {
dedupKey = byID.DedupKey.String dedupKey = byID.DedupKey.String
@@ -575,12 +579,8 @@ func (s *AnnotationService) SaveNote(ctx context.Context, req SaveNoteRequest) (
if req.ParagraphReference == 0 { if req.ParagraphReference == 0 {
req.ParagraphReference = byID.ParagraphReference.Int32 req.ParagraphReference = byID.ParagraphReference.Int32
} }
if byID.Deleted.Bool { if tombstoneWins(byID.Deleted.Bool, byID.DeletedAt, byID.LastModifiedAt, req.ModifiedAt) {
if !incomingNewerThanTombstone(req.ModifiedAt, byID.DeletedAt, byID.LastModifiedAt) { return &SaveNoteResult{Note: byID, Outcome: SaveOutcomeDeleted}, nil
return &SaveNoteResult{Note: byID, Outcome: SaveOutcomeDeleted}, nil
}
// Newer than the tombstone: a deliberate re-create. Resurrect
// via the LWW update (which clears deleted/deleted_at).
} }
return s.applyNoteLWW(ctx, req, byID, dedupKey) return s.applyNoteLWW(ctx, req, byID, dedupKey)
} }
@@ -692,20 +692,28 @@ func (s *AnnotationService) applyNoteLWW(ctx context.Context, req SaveNoteReques
return &SaveNoteResult{Note: note, Outcome: SaveOutcomeUpdated, Conflict: conflict}, nil return &SaveNoteResult{Note: note, Outcome: SaveOutcomeUpdated, Conflict: conflict}, nil
} }
func (s *AnnotationService) compareIncomingNote(req SaveNoteRequest, existing database.MediaNotes) (incomingNewer bool, contentChanged bool) { // lwwDecision is the shared LWW decision core for all three annotation
if req.ModifiedAt.IsZero() { // stacks: with a client timestamp, strictly-newer wins (and content is
contentSame := strings.EqualFold(req.Content, existing.Content) && // treated as changed — the update re-lands everything); without one, any
textEq(req.Position, existing.Position) // content difference is "newer" and identical content resolves to skip.
// The fallback timestamp is schema-driven per stack (rows with an
// updated_at fall back to it; bookmarks only carry created_at).
func lwwDecision(modifiedAt time.Time, existingLastMod, existingFallback pgtype.Timestamptz, contentSame bool) (incomingNewer bool, contentChanged bool) {
if modifiedAt.IsZero() {
return !contentSame, !contentSame return !contentSame, !contentSame
} }
existingMod := existing.LastModifiedAt existingMod := existingLastMod
if !existingMod.Valid { if !existingMod.Valid {
existingMod = existing.UpdatedAt existingMod = existingFallback
} }
if !existingMod.Valid { return modifiedAt.After(existingMod.Time), true
return true, true }
}
return req.ModifiedAt.After(existingMod.Time), true // compareIncomingNote is compareIncoming for the note stack.
func (s *AnnotationService) compareIncomingNote(req SaveNoteRequest, existing database.MediaNotes) (incomingNewer bool, contentChanged bool) {
contentSame := strings.EqualFold(req.Content, existing.Content) &&
textEq(req.Position, existing.Position)
return lwwDecision(req.ModifiedAt, existing.LastModifiedAt, existing.UpdatedAt, contentSame)
} }
func (s *AnnotationService) TombstoneNoteByID(ctx context.Context, id, userID pgtype.UUID) error { func (s *AnnotationService) TombstoneNoteByID(ctx context.Context, id, userID pgtype.UUID) error {
@@ -764,8 +772,8 @@ func (s *AnnotationService) SaveBookmark(ctx context.Context, req SaveBookmarkRe
return nil, fmt.Errorf("query bookmark by id: %w", idErr) return nil, fmt.Errorf("query bookmark by id: %w", idErr)
} }
if idErr == nil { if idErr == nil {
if byID.UserID != req.UserID || byID.MediaItemID != req.MediaItemID { if err := idTargetOwnership(byID.UserID, byID.MediaItemID, req.UserID, req.MediaItemID, "bookmark", req.BookmarkID); err != nil {
return nil, fmt.Errorf("bookmark %s belongs to another user or media item", req.BookmarkID) return nil, err
} }
if dedupKey == "" { if dedupKey == "" {
dedupKey = byID.DedupKey.String dedupKey = byID.DedupKey.String
@@ -788,12 +796,8 @@ func (s *AnnotationService) SaveBookmark(ctx context.Context, req SaveBookmarkRe
if req.ChapterReference == 0 { if req.ChapterReference == 0 {
req.ChapterReference = byID.ChapterReference.Int32 req.ChapterReference = byID.ChapterReference.Int32
} }
if byID.Deleted.Bool { if tombstoneWins(byID.Deleted.Bool, byID.DeletedAt, byID.LastModifiedAt, req.ModifiedAt) {
if !incomingNewerThanTombstone(req.ModifiedAt, byID.DeletedAt, byID.LastModifiedAt) { return &SaveBookmarkResult{Bookmark: byID, Outcome: SaveOutcomeDeleted}, nil
return &SaveBookmarkResult{Bookmark: byID, Outcome: SaveOutcomeDeleted}, nil
}
// Newer than the tombstone: a deliberate re-create. Resurrect
// via the LWW update (which clears deleted/deleted_at).
} }
return s.applyBookmarkLWW(ctx, req, byID, dedupKey) return s.applyBookmarkLWW(ctx, req, byID, dedupKey)
} }
@@ -923,18 +927,11 @@ func (s *AnnotationService) applyBookmarkLWW(ctx context.Context, req SaveBookma
} }
func (s *AnnotationService) compareIncomingBookmark(req SaveBookmarkRequest, existing database.MediaBookmarks) (incomingNewer bool, contentChanged bool) { func (s *AnnotationService) compareIncomingBookmark(req SaveBookmarkRequest, existing database.MediaBookmarks) (incomingNewer bool, contentChanged bool) {
if req.ModifiedAt.IsZero() { contentSame := strings.EqualFold(req.Title, existing.Title) &&
contentSame := strings.EqualFold(req.Title, existing.Title) && textEq(req.Notes, existing.Notes) &&
textEq(req.Notes, existing.Notes) && locatorRefresher(req.CFIPosition, existing.CfiPosition) &&
locatorRefresher(req.CFIPosition, existing.CfiPosition) && locatorRefresher(req.Position, existing.Position)
locatorRefresher(req.Position, existing.Position) return lwwDecision(req.ModifiedAt, existing.LastModifiedAt, existing.CreatedAt, contentSame)
return !contentSame, !contentSame
}
existingMod := existing.LastModifiedAt
if !existingMod.Valid {
existingMod = existing.CreatedAt
}
return req.ModifiedAt.After(existingMod.Time), true
} }
func (s *AnnotationService) TombstoneBookmarkByID(ctx context.Context, userID, bookmarkID pgtype.UUID, source string) error { func (s *AnnotationService) TombstoneBookmarkByID(ctx context.Context, userID, bookmarkID pgtype.UUID, source string) error {
internal/sync/queue.go
+61 -99
View File
@@ -457,32 +457,42 @@ func (p *SyncQueueProcessor) syncProgress(ctx context.Context, userID pgtype.UUI
return err return err
} }
// queue-map accessors: syncData entries arrive as JSON-decoded
// interface{} values; a missing key or wrong type is simply the zero
// value (the decoders only carry fields the device actually pushed).
func mapStr(m map[string]interface{}, key string) string {
v, _ := m[key].(string)
return v
}
func mapF64(m map[string]interface{}, key string) float64 {
v, _ := m[key].(float64)
return v
}
func mapI32(m map[string]interface{}, key string) int32 {
return int32(mapF64(m, key))
}
func mapDeviceSyncData(m map[string]interface{}) json.RawMessage {
if v, ok := m["device_sync_data"].(map[string]interface{}); ok {
raw, _ := json.Marshal(v)
return raw
}
return nil
}
func (p *SyncQueueProcessor) syncNote(ctx context.Context, userID pgtype.UUID, mediaItemID pgtype.UUID, syncData map[string]interface{}) error { func (p *SyncQueueProcessor) syncNote(ctx context.Context, userID pgtype.UUID, mediaItemID pgtype.UUID, syncData map[string]interface{}) error {
req := SaveNoteRequest{ req := SaveNoteRequest{
MediaItemID: mediaItemID, MediaItemID: mediaItemID,
UserID: userID, UserID: userID,
} Content: mapStr(syncData, "content"),
Position: mapStr(syncData, "position"),
if v, ok := syncData["content"].(string); ok { Source: mapStr(syncData, "source"),
req.Content = v EpubcfiLocation: mapStr(syncData, "epubcfi_location"),
} PercentageLocation: mapF64(syncData, "percentage_location"),
if v, ok := syncData["position"].(string); ok { ChapterReference: mapI32(syncData, "chapter_reference"),
req.Position = v DeviceSyncData: mapDeviceSyncData(syncData),
}
if v, ok := syncData["source"].(string); ok {
req.Source = v
}
if v, ok := syncData["epubcfi_location"].(string); ok {
req.EpubcfiLocation = v
}
if v, ok := syncData["percentage_location"].(float64); ok {
req.PercentageLocation = v
}
if v, ok := syncData["chapter_reference"].(float64); ok {
req.ChapterReference = int32(v)
}
if v, ok := syncData["device_sync_data"].(map[string]interface{}); ok {
req.DeviceSyncData, _ = json.Marshal(v)
} }
_, err := p.annotationSvc.SaveNote(ctx, req) _, err := p.annotationSvc.SaveNote(ctx, req)
@@ -491,45 +501,20 @@ func (p *SyncQueueProcessor) syncNote(ctx context.Context, userID pgtype.UUID, m
func (p *SyncQueueProcessor) syncHighlight(ctx context.Context, userID pgtype.UUID, mediaItemID pgtype.UUID, syncData map[string]interface{}) error { func (p *SyncQueueProcessor) syncHighlight(ctx context.Context, userID pgtype.UUID, mediaItemID pgtype.UUID, syncData map[string]interface{}) error {
req := SaveHighlightRequest{ req := SaveHighlightRequest{
MediaItemID: mediaItemID, MediaItemID: mediaItemID,
UserID: userID, UserID: userID,
} SelectionText: mapStr(syncData, "selection_text"),
StartPosition: mapStr(syncData, "start_position"),
if v, ok := syncData["selection_text"].(string); ok { EndPosition: mapStr(syncData, "end_position"),
req.SelectionText = v Color: mapStr(syncData, "color"),
} NoteText: mapStr(syncData, "note_text"),
if v, ok := syncData["start_position"].(string); ok { PercentageStart: mapF64(syncData, "percentage_start"),
req.StartPosition = v PercentageEnd: mapF64(syncData, "percentage_end"),
} EpubcfiStart: mapStr(syncData, "epubcfi_start"),
if v, ok := syncData["end_position"].(string); ok { EpubcfiEnd: mapStr(syncData, "epubcfi_end"),
req.EndPosition = v ChapterReference: mapI32(syncData, "chapter_reference"),
} Source: mapStr(syncData, "source"),
if v, ok := syncData["color"].(string); ok { DeviceSyncData: mapDeviceSyncData(syncData),
req.Color = v
}
if v, ok := syncData["note_text"].(string); ok {
req.NoteText = v
}
if v, ok := syncData["source"].(string); ok {
req.Source = v
}
if v, ok := syncData["epubcfi_start"].(string); ok {
req.EpubcfiStart = v
}
if v, ok := syncData["epubcfi_end"].(string); ok {
req.EpubcfiEnd = v
}
if v, ok := syncData["percentage_start"].(float64); ok {
req.PercentageStart = v
}
if v, ok := syncData["percentage_end"].(float64); ok {
req.PercentageEnd = v
}
if v, ok := syncData["chapter_reference"].(float64); ok {
req.ChapterReference = int32(v)
}
if v, ok := syncData["device_sync_data"].(map[string]interface{}); ok {
req.DeviceSyncData, _ = json.Marshal(v)
} }
_, err := p.annotationSvc.SaveHighlight(ctx, req) _, err := p.annotationSvc.SaveHighlight(ctx, req)
@@ -538,42 +523,19 @@ func (p *SyncQueueProcessor) syncHighlight(ctx context.Context, userID pgtype.UU
func (p *SyncQueueProcessor) syncBookmark(ctx context.Context, userID pgtype.UUID, mediaItemID pgtype.UUID, syncData map[string]interface{}) error { func (p *SyncQueueProcessor) syncBookmark(ctx context.Context, userID pgtype.UUID, mediaItemID pgtype.UUID, syncData map[string]interface{}) error {
req := SaveBookmarkRequest{ req := SaveBookmarkRequest{
MediaItemID: mediaItemID, MediaItemID: mediaItemID,
UserID: userID, UserID: userID,
} Title: mapStr(syncData, "title"),
Position: mapStr(syncData, "position"),
if v, ok := syncData["title"].(string); ok { Notes: mapStr(syncData, "notes"),
req.Title = v Source: mapStr(syncData, "source"),
} CFIPosition: mapStr(syncData, "cfi_position"),
if v, ok := syncData["position"].(string); ok { EpubcfiLocation: mapStr(syncData, "epubcfi_location"),
req.Position = v PercentageLoc: mapF64(syncData, "percentage_loc"),
} PageNumber: mapI32(syncData, "page_number"),
if v, ok := syncData["notes"].(string); ok { ChapterNumber: mapI32(syncData, "chapter_number"),
req.Notes = v ChapterReference: mapI32(syncData, "chapter_reference"),
} DeviceSyncData: mapDeviceSyncData(syncData),
if v, ok := syncData["source"].(string); ok {
req.Source = v
}
if v, ok := syncData["cfi_position"].(string); ok {
req.CFIPosition = v
}
if v, ok := syncData["epubcfi_location"].(string); ok {
req.EpubcfiLocation = v
}
if v, ok := syncData["percentage_loc"].(float64); ok {
req.PercentageLoc = v
}
if v, ok := syncData["page_number"].(float64); ok {
req.PageNumber = int32(v)
}
if v, ok := syncData["chapter_number"].(float64); ok {
req.ChapterNumber = int32(v)
}
if v, ok := syncData["chapter_reference"].(float64); ok {
req.ChapterReference = int32(v)
}
if v, ok := syncData["device_sync_data"].(map[string]interface{}); ok {
req.DeviceSyncData, _ = json.Marshal(v)
} }
_, err := p.annotationSvc.SaveBookmark(ctx, req) _, err := p.annotationSvc.SaveBookmark(ctx, req)