diff --git a/internal/sync/annotations.go b/internal/sync/annotations.go index 2ad0f12b..b012fc91 100644 --- a/internal/sync/annotations.go +++ b/internal/sync/annotations.go @@ -95,6 +95,18 @@ type SaveHighlightResult struct { Conflict bool } +// coalesce returns fallback when v is the zero value — the LWW-apply +// "an omitted field preserves the stored value" primitive. Zeroes matter +// here because pgFloat8/pgInt4 turn them into NULLs: an uncoalesced +// partial payload would erase stored locators on every re-save. +func coalesce[T comparable](v, fallback T) T { + var zero T + if v == zero { + return fallback + } + return v +} + // 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 { @@ -238,35 +250,23 @@ func (s *AnnotationService) applyLWW( // xpointer) and may carry no CFI either: keep the stored ones so // device-native serve-back and round-trip identity survive a web-side // note/color edit instead of being wiped to empty. - startPosition := req.StartPosition - if startPosition == "" { - startPosition = existing.StartPosition.String - } - endPosition := req.EndPosition - if endPosition == "" { - endPosition = existing.EndPosition.String - } - epubcfiStart := req.EpubcfiStart - if epubcfiStart == "" { - epubcfiStart = existing.EpubcfiStart.String - } - epubcfiEnd := req.EpubcfiEnd - if epubcfiEnd == "" { - epubcfiEnd = existing.EpubcfiEnd.String - } + startPosition := coalesce(req.StartPosition, existing.StartPosition.String) + endPosition := coalesce(req.EndPosition, existing.EndPosition.String) + epubcfiStart := coalesce(req.EpubcfiStart, existing.EpubcfiStart.String) + epubcfiEnd := coalesce(req.EpubcfiEnd, existing.EpubcfiEnd.String) highlight, err := s.db.UpdateMediaHighlightForSync(ctx, database.UpdateMediaHighlightForSyncParams{ ID: existing.ID, SelectionText: req.SelectionText, StartPosition: pgText(startPosition), EndPosition: pgText(endPosition), - Color: pgText(req.Color), - NoteText: pgText(req.NoteText), - PercentageStart: pgFloat8(req.PercentageStart), - PercentageEnd: pgFloat8(req.PercentageEnd), + Color: pgText(coalesce(req.Color, existing.Color.String)), + NoteText: pgText(coalesce(req.NoteText, existing.NoteText.String)), + PercentageStart: pgFloat8(coalesce(req.PercentageStart, existing.PercentageStart.Float64)), + PercentageEnd: pgFloat8(coalesce(req.PercentageEnd, existing.PercentageEnd.Float64)), EpubcfiStart: pgText(epubcfiStart), EpubcfiEnd: pgText(epubcfiEnd), - ChapterReference: pgInt4(req.ChapterReference), + ChapterReference: pgInt4(coalesce(req.ChapterReference, existing.ChapterReference.Int32)), LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true}, LastModifiedSource: pgtype.Text{String: req.Source, Valid: req.Source != ""}, DeviceSyncData: deviceData, @@ -561,24 +561,6 @@ func (s *AnnotationService) SaveNote(ctx context.Context, req SaveNoteRequest) ( if dedupKey == "" { dedupKey = byID.DedupKey.String } - if req.EpubcfiLocation == "" { - req.EpubcfiLocation = byID.EpubcfiLocation.String - } - if req.PercentageLocation == 0 { - req.PercentageLocation = byID.PercentageLocation.Float64 - } - if req.CharacterStart == 0 { - req.CharacterStart = byID.CharacterStart.Int32 - } - if req.CharacterEnd == 0 { - req.CharacterEnd = byID.CharacterEnd.Int32 - } - if req.ChapterReference == 0 { - req.ChapterReference = byID.ChapterReference.Int32 - } - if req.ParagraphReference == 0 { - req.ParagraphReference = byID.ParagraphReference.Int32 - } if tombstoneWins(byID.Deleted.Bool, byID.DeletedAt, byID.LastModifiedAt, req.ModifiedAt) { return &SaveNoteResult{Note: byID, Outcome: SaveOutcomeDeleted}, nil } @@ -663,17 +645,21 @@ func (s *AnnotationService) applyNoteLWW(ctx context.Context, req SaveNoteReques deviceData := mergeDeviceSyncData(existing.DeviceSyncData, req.Source, req.DeviceSyncData) + // Coalesce omitted fields with the stored row (same doctrine as the + // highlight apply): a partial payload — a web PUT carrying only + // content/position, a device echo without percentages — must not + // wipe the locators it didn't mention (see coalesce). note, err := s.db.UpdateMediaNoteForSync(ctx, database.UpdateMediaNoteForSyncParams{ ID: existing.ID, UserID: req.UserID, Content: req.Content, - Position: pgText(req.Position), - PercentageLocation: pgFloat8(req.PercentageLocation), - CharacterStart: pgInt4(req.CharacterStart), - CharacterEnd: pgInt4(req.CharacterEnd), - EpubcfiLocation: pgText(req.EpubcfiLocation), - ChapterReference: pgInt4(req.ChapterReference), - ParagraphReference: pgInt4(req.ParagraphReference), + Position: pgText(coalesce(req.Position, existing.Position.String)), + PercentageLocation: pgFloat8(coalesce(req.PercentageLocation, existing.PercentageLocation.Float64)), + CharacterStart: pgInt4(coalesce(req.CharacterStart, existing.CharacterStart.Int32)), + CharacterEnd: pgInt4(coalesce(req.CharacterEnd, existing.CharacterEnd.Int32)), + EpubcfiLocation: pgText(coalesce(req.EpubcfiLocation, existing.EpubcfiLocation.String)), + ChapterReference: pgInt4(coalesce(req.ChapterReference, existing.ChapterReference.Int32)), + ParagraphReference: pgInt4(coalesce(req.ParagraphReference, existing.ParagraphReference.Int32)), LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true}, LastModifiedSource: pgtype.Text{String: req.Source, Valid: req.Source != ""}, DeviceSyncData: deviceData, @@ -778,24 +764,6 @@ func (s *AnnotationService) SaveBookmark(ctx context.Context, req SaveBookmarkRe if dedupKey == "" { dedupKey = byID.DedupKey.String } - if req.CFIPosition == "" { - req.CFIPosition = byID.CfiPosition.String - } - if req.EpubcfiLocation == "" { - req.EpubcfiLocation = byID.EpubcfiLocation.String - } - if req.PercentageLoc == 0 { - req.PercentageLoc = byID.PercentageLocation.Float64 - } - if req.PageNumber == 0 { - req.PageNumber = byID.PageNumber.Int32 - } - if req.ChapterNumber == 0 { - req.ChapterNumber = byID.ChapterNumber.Int32 - } - if req.ChapterReference == 0 { - req.ChapterReference = byID.ChapterReference.Int32 - } if tombstoneWins(byID.Deleted.Bool, byID.DeletedAt, byID.LastModifiedAt, req.ModifiedAt) { return &SaveBookmarkResult{Bookmark: byID, Outcome: SaveOutcomeDeleted}, nil } @@ -898,16 +866,18 @@ func (s *AnnotationService) applyBookmarkLWW(ctx context.Context, req SaveBookma } bm, err := s.db.UpdateMediaBookmarkForSync(ctx, database.UpdateMediaBookmarkForSyncParams{ - ID: existing.ID, - PageNumber: pgInt4(req.PageNumber), - ChapterNumber: pgInt4(req.ChapterNumber), - CfiPosition: pgText(cfiPosition), - Title: req.Title, - Position: pgText(position), - Notes: pgText(req.Notes), - PercentageLocation: pgFloat8(req.PercentageLoc), - EpubcfiLocation: pgText(req.EpubcfiLocation), - ChapterReference: pgInt4(req.ChapterReference), + ID: existing.ID, + PageNumber: pgInt4(coalesce(req.PageNumber, existing.PageNumber.Int32)), + ChapterNumber: pgInt4(coalesce(req.ChapterNumber, existing.ChapterNumber.Int32)), + CfiPosition: pgText(cfiPosition), + Title: req.Title, + Position: pgText(position), + Notes: pgText(req.Notes), + // Locator fields a partial payload omits must survive: zeroes + // become NULLs (pgFloat8/pgInt4) and would erase stored values. + PercentageLocation: pgFloat8(coalesce(req.PercentageLoc, existing.PercentageLocation.Float64)), + EpubcfiLocation: pgText(coalesce(req.EpubcfiLocation, existing.EpubcfiLocation.String)), + ChapterReference: pgInt4(coalesce(req.ChapterReference, existing.ChapterReference.Int32)), LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true}, LastModifiedSource: pgtype.Text{String: req.Source, Valid: req.Source != ""}, DeviceSyncData: deviceData,