fix(annotations): bookmark and note PUTs land through the sync service (LWW by id)
H3 census bugs #2 and #3: both web PUTs used plain queries that bypassed the sync-aware layer entirely — no LWW, no source bookkeeping, and locator fields the client couldn't even send. The highlight PUT was the only one already routed (via HighlightID); the other two now mirror it: - SaveBookmarkRequest gains BookmarkID, SaveNoteRequest gains NoteID: id-targeted resolution reads the exact row, verifies ownership, LWWs under its stored dedup key, and resurrects tombstones when the edit is newer (same doctrine and comment structure as the highlight path). - Fields the PUT omits are coalesced from the stored row before the full-column sync update — the legacy web PUT sends only title/notes/position, and zeroing cfi/percentage/page on every edit would have been data loss. Verified both directions by TestH3_BookmarkPUTPreservesLocators (omitted survives, explicit lands). - The bookmark PUT's wire DTO gains the create endpoint's locator keys as optional fields (documented additive request keys in contract_pins.md); the note PUT keeps its content+position wire. UpdateMediaBookmark/UpdateMediaNote plain queries are now uncalled (the generated queries stay until the H4 query sweep).
This commit is contained in:
1 parent
c579179fee
commit
7d698ea0d8
4 files changed
+258
-14
No files matched your search
@@ -521,6 +521,10 @@ type SaveNoteRequest struct {
|
||||
ModifiedAt time.Time
|
||||
DeviceSyncData []byte
|
||||
DedupKey string // overrides the computed key for device echoes
|
||||
// NoteID, when valid, targets that exact row (web PUTs edit by id):
|
||||
// the save LWWs against it under its stored dedup key, mirroring
|
||||
// SaveHighlightRequest.HighlightID.
|
||||
NoteID pgtype.UUID
|
||||
}
|
||||
|
||||
type SaveNoteResult struct {
|
||||
@@ -535,6 +539,54 @@ func (s *AnnotationService) SaveNote(ctx context.Context, req SaveNoteRequest) (
|
||||
}
|
||||
|
||||
dedupKey := req.DedupKey
|
||||
|
||||
// Web edits arrive with the row id from the URL: resolve by id and LWW
|
||||
// against that row under its stored key (same doctrine as
|
||||
// SaveHighlight's HighlightID path). Locator columns the note PUT
|
||||
// does not carry are coalesced from the stored row so the full-column
|
||||
// sync update cannot zero them.
|
||||
if req.NoteID.Valid {
|
||||
byID, idErr := s.db.GetMediaNote(ctx, req.NoteID)
|
||||
if idErr != nil && !errors.Is(idErr, pgx.ErrNoRows) {
|
||||
return nil, fmt.Errorf("query note by id: %w", idErr)
|
||||
}
|
||||
if idErr == nil {
|
||||
if byID.UserID != req.UserID || byID.MediaItemID != req.MediaItemID {
|
||||
return nil, fmt.Errorf("note %s belongs to another user or media item", req.NoteID)
|
||||
}
|
||||
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 byID.Deleted.Bool {
|
||||
if !incomingNewerThanTombstone(req.ModifiedAt, byID.DeletedAt, byID.LastModifiedAt) {
|
||||
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)
|
||||
}
|
||||
// No row with that id: fall through to identity-based resolution.
|
||||
}
|
||||
|
||||
if dedupKey == "" {
|
||||
dedupKey = ComputeDedupKey(req.Content, req.EpubcfiLocation, req.Position)
|
||||
}
|
||||
@@ -684,6 +736,11 @@ type SaveBookmarkRequest struct {
|
||||
// DedupKey overrides the computed key for device echoes (see
|
||||
// SaveHighlightRequest).
|
||||
DedupKey string
|
||||
// BookmarkID, when valid, targets that exact row (web PUTs edit by
|
||||
// id): the save LWWs against it under its stored dedup key, mirroring
|
||||
// SaveHighlightRequest.HighlightID — an edit must not re-derive
|
||||
// identity from (possibly edited) content.
|
||||
BookmarkID pgtype.UUID
|
||||
}
|
||||
|
||||
type SaveBookmarkResult struct {
|
||||
@@ -694,6 +751,55 @@ type SaveBookmarkResult struct {
|
||||
|
||||
func (s *AnnotationService) SaveBookmark(ctx context.Context, req SaveBookmarkRequest) (*SaveBookmarkResult, error) {
|
||||
dedupKey := req.DedupKey
|
||||
|
||||
// Web edits arrive with the row id from the URL: resolve by id and LWW
|
||||
// against that row under its stored key (same doctrine as
|
||||
// SaveHighlight's HighlightID path). Fields the edit does not carry
|
||||
// are coalesced from the stored row — the web PUT predates the
|
||||
// locator columns and omits them; zeroing them on update would be
|
||||
// data loss.
|
||||
if req.BookmarkID.Valid {
|
||||
byID, idErr := s.db.GetMediaBookmark(ctx, req.BookmarkID)
|
||||
if idErr != nil && !errors.Is(idErr, pgx.ErrNoRows) {
|
||||
return nil, fmt.Errorf("query bookmark by id: %w", idErr)
|
||||
}
|
||||
if idErr == nil {
|
||||
if byID.UserID != req.UserID || byID.MediaItemID != req.MediaItemID {
|
||||
return nil, fmt.Errorf("bookmark %s belongs to another user or media item", req.BookmarkID)
|
||||
}
|
||||
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 byID.Deleted.Bool {
|
||||
if !incomingNewerThanTombstone(req.ModifiedAt, byID.DeletedAt, byID.LastModifiedAt) {
|
||||
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)
|
||||
}
|
||||
// No row with that id: fall through to identity-based resolution.
|
||||
}
|
||||
|
||||
if dedupKey == "" {
|
||||
dedupKey = ComputeDedupKey(req.Title, req.EpubcfiLocation, req.Position)
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user