fix(sync): echo dedup + color semantics + classification for KOReader round-trips
Echo duplication: devices push their full annotation list on every sync, and an echo of a web-created annotation computed a different dedup key than the original (device locators differ from web locators) — every pull→push cycle minted a duplicate row, and cleaning those up on the web tombstoned them back to the device, deleting the just-applied copies. That was the "web highlights never appear on KOReader" experience. GetMetadata now serves each annotation's dedup_key; the device stores it on the applied entry and echoes it in pushes; SaveHighlight/SaveBookmark/SaveNote accept a DedupKey override so echoes converge onto the original row (verified: pull → echo push creates no rows, LWW skips identical content). Color semantics (per user preference): devices render their own default and cannot round-trip web colors, so GetMetadata no longer serves colors at all — every highlight syncs regardless of its web color and the device draws its default. An echo carries no color; ingest then PRESERVES the stored web color (existingHighlightColor lookup by dedup key) so round-trips never change it. A non-empty device color means the user edited the highlight there: it maps name→hex (green→#a5d6a7, default yellow) and wins. Verified: echo kept #ffd54f; a simulated device edit with "green" updated the web row to #a5d6a7. Classification: KOReader auto-fills text="in Chapter X" on page bookmarks (ReaderAnnotation:updateItemByXPointer), so the plugin's text-presence classification turned every echoed bookmark into a junk highlight on the web. v2 classification now keys off the drawer field (present = highlight/note, absent = bookmark with its label in note).
This commit is contained in:
@@ -106,6 +106,24 @@ func extendCFIByLength(cfi, text string) string {
|
||||
return cfi[:i+1] + strconv.Itoa(off+utf16len) + ")"
|
||||
}
|
||||
|
||||
// existingHighlightColor returns the stored color of the highlight matching
|
||||
// the dedup key ("" when none) so device echoes that carry no color never
|
||||
// clobber the web color.
|
||||
func (h *KOReaderHandler) existingHighlightColor(ctx context.Context, mediaItemID, userID pgtype.UUID, dedupKey string) string {
|
||||
if dedupKey == "" {
|
||||
return ""
|
||||
}
|
||||
existing, err := h.db.GetMediaHighlightByDedupKey(ctx, database.GetMediaHighlightByDedupKeyParams{
|
||||
UserID: userID,
|
||||
MediaItemID: mediaItemID,
|
||||
DedupKey: pgtype.Text{String: dedupKey, Valid: true},
|
||||
})
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
return existing.Color.String
|
||||
}
|
||||
|
||||
// deriveAnnotationPercentage computes a percentage for device-pushed
|
||||
// annotations when the client didn't send one (thin clients skip their own
|
||||
// per-annotation page lookups; arithmetic is only free on paging documents).
|
||||
@@ -211,6 +229,7 @@ type KOReaderBookmark struct {
|
||||
Type string `json:"type,omitempty"`
|
||||
Percentage *float64 `json:"percentage,omitempty"`
|
||||
BookSHA256 string `json:"book_sha256,omitempty"`
|
||||
DedupKey string `json:"dedup_key,omitempty"`
|
||||
}
|
||||
|
||||
type KOReaderHighlight struct {
|
||||
@@ -225,6 +244,7 @@ type KOReaderHighlight struct {
|
||||
Color string `json:"color,omitempty"`
|
||||
Percentage *float64 `json:"percentage,omitempty"`
|
||||
BookSHA256 string `json:"book_sha256,omitempty"`
|
||||
DedupKey string `json:"dedup_key,omitempty"`
|
||||
}
|
||||
|
||||
type KOReaderNote struct {
|
||||
@@ -238,6 +258,7 @@ type KOReaderNote struct {
|
||||
Type string `json:"type,omitempty"`
|
||||
Percentage *float64 `json:"percentage,omitempty"`
|
||||
BookSHA256 string `json:"book_sha256,omitempty"`
|
||||
DedupKey string `json:"dedup_key,omitempty"`
|
||||
}
|
||||
|
||||
type KOReaderSyncResponse struct {
|
||||
@@ -604,19 +625,40 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
|
||||
"page": hl.Page,
|
||||
})
|
||||
|
||||
// Color semantics: devices render their own default and cannot
|
||||
// round-trip web colors. An echo carries NO color — preserve the
|
||||
// stored (web) color so round-trips never change it. A non-empty
|
||||
// color means the user edited the highlight on the device: map the
|
||||
// device color name and let it win.
|
||||
color := ""
|
||||
if hl.Color != "" {
|
||||
color = mapColorFromKOReader(hl.Color)
|
||||
}
|
||||
dedupKey := hl.DedupKey
|
||||
if dedupKey == "" {
|
||||
dedupKey = wsync.ComputeDedupKey(hl.Text, epubcfiStart, startPos)
|
||||
}
|
||||
if color == "" {
|
||||
color = h.existingHighlightColor(ctx, mediaItemID, userID, dedupKey)
|
||||
}
|
||||
if color == "" {
|
||||
color = "#ffd54f"
|
||||
}
|
||||
|
||||
h.annotationSvc.SaveHighlight(ctx, wsync.SaveHighlightRequest{
|
||||
MediaItemID: mediaItemID,
|
||||
UserID: userID,
|
||||
SelectionText: hl.Text,
|
||||
StartPosition: startPos,
|
||||
EndPosition: endPos,
|
||||
Color: mapColorFromKOReader(hl.Color),
|
||||
Color: color,
|
||||
NoteText: hl.Notes,
|
||||
PercentageStart: pctStart,
|
||||
EpubcfiStart: epubcfiStart,
|
||||
EpubcfiEnd: epubcfiEnd,
|
||||
Source: "koreader",
|
||||
DeviceSyncData: deviceData,
|
||||
DedupKey: dedupKey,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -640,18 +682,25 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
|
||||
"page": note.Page,
|
||||
})
|
||||
|
||||
dedupKey := note.DedupKey
|
||||
if dedupKey == "" {
|
||||
dedupKey = wsync.ComputeDedupKey(note.Text, epubcfiStart, startPos)
|
||||
}
|
||||
|
||||
h.annotationSvc.SaveHighlight(ctx, wsync.SaveHighlightRequest{
|
||||
MediaItemID: mediaItemID,
|
||||
UserID: userID,
|
||||
SelectionText: note.Text,
|
||||
StartPosition: startPos,
|
||||
EndPosition: endPos,
|
||||
Color: h.existingHighlightColor(ctx, mediaItemID, userID, dedupKey),
|
||||
NoteText: note.Notes,
|
||||
PercentageStart: pctStart,
|
||||
EpubcfiStart: epubcfiStart,
|
||||
EpubcfiEnd: epubcfiEnd,
|
||||
Source: "koreader",
|
||||
DeviceSyncData: deviceData,
|
||||
DedupKey: dedupKey,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -669,6 +718,11 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
|
||||
"page": bookmark.Page,
|
||||
})
|
||||
|
||||
dedupKey := bookmark.DedupKey
|
||||
if dedupKey == "" {
|
||||
dedupKey = wsync.ComputeDedupKey(bookmark.Text, "", position)
|
||||
}
|
||||
|
||||
h.annotationSvc.SaveBookmark(ctx, wsync.SaveBookmarkRequest{
|
||||
MediaItemID: mediaItemID,
|
||||
UserID: userID,
|
||||
@@ -677,6 +731,7 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
|
||||
ChapterNumber: int32(bookmark.Chapter),
|
||||
Source: "koreader",
|
||||
DeviceSyncData: deviceData,
|
||||
DedupKey: dedupKey,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -935,11 +990,15 @@ func (h *KOReaderHandler) GetMetadata(c *echo.Context) error {
|
||||
pos1 = extendXPointerByLength(pos0, ann.SelectionText)
|
||||
}
|
||||
highlight := KOReaderHighlight{
|
||||
Text: ann.SelectionText,
|
||||
Pos0: pos0,
|
||||
Pos1: pos1,
|
||||
Color: mapColorToKOReader(ann.Color.String),
|
||||
Text: ann.SelectionText,
|
||||
Pos0: pos0,
|
||||
Pos1: pos1,
|
||||
// No color served: devices render their own default and
|
||||
// cannot round-trip web colors — the web color only changes
|
||||
// when the highlight is edited on the device (push carries
|
||||
// the device color, ingested with the name→hex map).
|
||||
Datetime: ann.CreatedAt.Time.Format(time.RFC3339),
|
||||
DedupKey: ann.DedupKey.String,
|
||||
}
|
||||
if ann.NoteText.Valid && ann.NoteText.String != "" {
|
||||
highlight.Notes = ann.NoteText.String
|
||||
@@ -955,6 +1014,7 @@ func (h *KOReaderHandler) GetMetadata(c *echo.Context) error {
|
||||
Text: ann.SelectionText,
|
||||
Pos0: pos0,
|
||||
Datetime: ann.CreatedAt.Time.Format(time.RFC3339),
|
||||
DedupKey: ann.DedupKey.String,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -974,6 +1034,7 @@ func (h *KOReaderHandler) GetMetadata(c *echo.Context) error {
|
||||
Pos0: pos0,
|
||||
Pos1: pos0,
|
||||
Datetime: bm.CreatedAt.Time.Format(time.RFC3339),
|
||||
DedupKey: bm.DedupKey.String,
|
||||
}
|
||||
if bm.Notes.Valid && bm.Notes.String != "" {
|
||||
koreaderBookmark.Notes = bm.Notes.String
|
||||
@@ -1126,14 +1187,6 @@ var koreaderColorFromName = map[string]string{
|
||||
"red": "#f48fb1",
|
||||
}
|
||||
|
||||
var koreaderColorFromHex = map[string]string{
|
||||
"#ffd54f": "yellow",
|
||||
"#a5d6a7": "green",
|
||||
"#90caf9": "blue",
|
||||
"#ce93d8": "purple",
|
||||
"#f48fb1": "purple",
|
||||
}
|
||||
|
||||
// mapColorFromKOReader normalizes a device color name to a web hex
|
||||
// swatch (default yellow) when ingesting device pushes.
|
||||
func mapColorFromKOReader(name string) string {
|
||||
@@ -1143,15 +1196,6 @@ func mapColorFromKOReader(name string) string {
|
||||
return "#ffd54f"
|
||||
}
|
||||
|
||||
// mapColorToKOReader normalizes a web hex swatch to a KOReader color
|
||||
// name (default yellow) when serving to devices.
|
||||
func mapColorToKOReader(hex string) string {
|
||||
if name, ok := koreaderColorFromHex[strings.ToLower(strings.TrimSpace(hex))]; ok {
|
||||
return name
|
||||
}
|
||||
return "yellow"
|
||||
}
|
||||
|
||||
// 1. A device-native CRE xpointer ("/body/...") in startPosition wins —
|
||||
// round-trip identical for KOReader-pushed annotations (converting the
|
||||
// stored CFI instead could drift and duplicate on the device).
|
||||
|
||||
@@ -23,8 +23,8 @@ import (
|
||||
const TombstoneTTL = 30 * 24 * time.Hour
|
||||
|
||||
type AnnotationService struct {
|
||||
db *database.Queries
|
||||
connMgr *ConnectionManager
|
||||
db *database.Queries
|
||||
connMgr *ConnectionManager
|
||||
settings *database.SettingsRegistry
|
||||
}
|
||||
|
||||
@@ -75,6 +75,11 @@ type SaveHighlightRequest struct {
|
||||
Source string
|
||||
ModifiedAt time.Time
|
||||
DeviceSyncData json.RawMessage
|
||||
// DedupKey overrides the computed key when the client echoes back an
|
||||
// annotation it received from us (device echoes carry device-native
|
||||
// locators, so the computed key would never match the original row and
|
||||
// every pull→push cycle would mint a duplicate).
|
||||
DedupKey string
|
||||
}
|
||||
|
||||
type SaveHighlightResult struct {
|
||||
@@ -84,7 +89,10 @@ type SaveHighlightResult struct {
|
||||
}
|
||||
|
||||
func (s *AnnotationService) SaveHighlight(ctx context.Context, req SaveHighlightRequest) (*SaveHighlightResult, error) {
|
||||
dedupKey := ComputeDedupKey(req.SelectionText, req.EpubcfiStart, req.StartPosition)
|
||||
dedupKey := req.DedupKey
|
||||
if dedupKey == "" {
|
||||
dedupKey = ComputeDedupKey(req.SelectionText, req.EpubcfiStart, req.StartPosition)
|
||||
}
|
||||
|
||||
existing, err := s.db.GetMediaHighlightByDedupKey(ctx, database.GetMediaHighlightByDedupKeyParams{
|
||||
UserID: req.UserID,
|
||||
@@ -124,22 +132,22 @@ func (s *AnnotationService) createHighlight(
|
||||
deviceData := mergeDeviceSyncData(nil, req.Source, req.DeviceSyncData)
|
||||
|
||||
highlight, err := s.db.CreateMediaHighlightFull(ctx, database.CreateMediaHighlightFullParams{
|
||||
MediaItemID: req.MediaItemID,
|
||||
UserID: req.UserID,
|
||||
SelectionText: req.SelectionText,
|
||||
StartPosition: pgText(req.StartPosition),
|
||||
EndPosition: pgText(req.EndPosition),
|
||||
Color: pgText(req.Color),
|
||||
NoteText: pgText(req.NoteText),
|
||||
PercentageStart: pgFloat8(req.PercentageStart),
|
||||
PercentageEnd: pgFloat8(req.PercentageEnd),
|
||||
EpubcfiStart: pgText(req.EpubcfiStart),
|
||||
EpubcfiEnd: pgText(req.EpubcfiEnd),
|
||||
ChapterReference: pgInt4(req.ChapterReference),
|
||||
DedupKey: pgtype.Text{String: dedupKey, Valid: true},
|
||||
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
|
||||
MediaItemID: req.MediaItemID,
|
||||
UserID: req.UserID,
|
||||
SelectionText: req.SelectionText,
|
||||
StartPosition: pgText(req.StartPosition),
|
||||
EndPosition: pgText(req.EndPosition),
|
||||
Color: pgText(req.Color),
|
||||
NoteText: pgText(req.NoteText),
|
||||
PercentageStart: pgFloat8(req.PercentageStart),
|
||||
PercentageEnd: pgFloat8(req.PercentageEnd),
|
||||
EpubcfiStart: pgText(req.EpubcfiStart),
|
||||
EpubcfiEnd: pgText(req.EpubcfiEnd),
|
||||
ChapterReference: pgInt4(req.ChapterReference),
|
||||
DedupKey: pgtype.Text{String: dedupKey, Valid: true},
|
||||
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
|
||||
LastModifiedSource: pgtype.Text{String: req.Source, Valid: req.Source != ""},
|
||||
DeviceSyncData: deviceData,
|
||||
DeviceSyncData: deviceData,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("create highlight: %w", err)
|
||||
@@ -179,20 +187,20 @@ func (s *AnnotationService) applyLWW(
|
||||
deviceData := mergeDeviceSyncData(existing.DeviceSyncData, req.Source, req.DeviceSyncData)
|
||||
|
||||
highlight, err := s.db.UpdateMediaHighlightForSync(ctx, database.UpdateMediaHighlightForSyncParams{
|
||||
ID: existing.ID,
|
||||
SelectionText: req.SelectionText,
|
||||
StartPosition: pgText(req.StartPosition),
|
||||
EndPosition: pgText(req.EndPosition),
|
||||
Color: pgText(req.Color),
|
||||
NoteText: pgText(req.NoteText),
|
||||
PercentageStart: pgFloat8(req.PercentageStart),
|
||||
PercentageEnd: pgFloat8(req.PercentageEnd),
|
||||
EpubcfiStart: pgText(req.EpubcfiStart),
|
||||
EpubcfiEnd: pgText(req.EpubcfiEnd),
|
||||
ChapterReference: pgInt4(req.ChapterReference),
|
||||
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
|
||||
ID: existing.ID,
|
||||
SelectionText: req.SelectionText,
|
||||
StartPosition: pgText(req.StartPosition),
|
||||
EndPosition: pgText(req.EndPosition),
|
||||
Color: pgText(req.Color),
|
||||
NoteText: pgText(req.NoteText),
|
||||
PercentageStart: pgFloat8(req.PercentageStart),
|
||||
PercentageEnd: pgFloat8(req.PercentageEnd),
|
||||
EpubcfiStart: pgText(req.EpubcfiStart),
|
||||
EpubcfiEnd: pgText(req.EpubcfiEnd),
|
||||
ChapterReference: pgInt4(req.ChapterReference),
|
||||
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
|
||||
LastModifiedSource: pgtype.Text{String: req.Source, Valid: req.Source != ""},
|
||||
DeviceSyncData: deviceData,
|
||||
DeviceSyncData: deviceData,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("update highlight: %w", err)
|
||||
@@ -335,6 +343,7 @@ type SaveNoteRequest struct {
|
||||
Source string
|
||||
ModifiedAt time.Time
|
||||
DeviceSyncData []byte
|
||||
DedupKey string // overrides the computed key for device echoes
|
||||
}
|
||||
|
||||
type SaveNoteResult struct {
|
||||
@@ -348,7 +357,10 @@ func (s *AnnotationService) SaveNote(ctx context.Context, req SaveNoteRequest) (
|
||||
return nil, errors.New("invalid user_id or media_item_id")
|
||||
}
|
||||
|
||||
dedupKey := ComputeDedupKey(req.Content, req.EpubcfiLocation, req.Position)
|
||||
dedupKey := req.DedupKey
|
||||
if dedupKey == "" {
|
||||
dedupKey = ComputeDedupKey(req.Content, req.EpubcfiLocation, req.Position)
|
||||
}
|
||||
|
||||
existing, err := s.db.GetMediaNoteByDedupKey(ctx, database.GetMediaNoteByDedupKeyParams{
|
||||
UserID: req.UserID,
|
||||
@@ -485,6 +497,9 @@ type SaveBookmarkRequest struct {
|
||||
Source string
|
||||
ModifiedAt time.Time
|
||||
DeviceSyncData json.RawMessage
|
||||
// DedupKey overrides the computed key for device echoes (see
|
||||
// SaveHighlightRequest).
|
||||
DedupKey string
|
||||
}
|
||||
|
||||
type SaveBookmarkResult struct {
|
||||
@@ -494,7 +509,10 @@ type SaveBookmarkResult struct {
|
||||
}
|
||||
|
||||
func (s *AnnotationService) SaveBookmark(ctx context.Context, req SaveBookmarkRequest) (*SaveBookmarkResult, error) {
|
||||
dedupKey := ComputeDedupKey(req.Title, req.EpubcfiLocation, req.Position)
|
||||
dedupKey := req.DedupKey
|
||||
if dedupKey == "" {
|
||||
dedupKey = ComputeDedupKey(req.Title, req.EpubcfiLocation, req.Position)
|
||||
}
|
||||
|
||||
existing, err := s.db.GetMediaBookmarkByDedupKey(ctx, database.GetMediaBookmarkByDedupKeyParams{
|
||||
UserID: req.UserID,
|
||||
@@ -530,21 +548,21 @@ func (s *AnnotationService) createBookmark(ctx context.Context, req SaveBookmark
|
||||
deviceData := mergeDeviceSyncData(nil, req.Source, req.DeviceSyncData)
|
||||
|
||||
bm, err := s.db.CreateMediaBookmarkFull(ctx, database.CreateMediaBookmarkFullParams{
|
||||
MediaItemID: req.MediaItemID,
|
||||
UserID: req.UserID,
|
||||
PageNumber: pgInt4(req.PageNumber),
|
||||
ChapterNumber: pgInt4(req.ChapterNumber),
|
||||
CfiPosition: pgText(req.CFIPosition),
|
||||
Title: req.Title,
|
||||
Position: pgText(req.Position),
|
||||
Notes: pgText(req.Notes),
|
||||
MediaItemID: req.MediaItemID,
|
||||
UserID: req.UserID,
|
||||
PageNumber: pgInt4(req.PageNumber),
|
||||
ChapterNumber: pgInt4(req.ChapterNumber),
|
||||
CfiPosition: pgText(req.CFIPosition),
|
||||
Title: req.Title,
|
||||
Position: pgText(req.Position),
|
||||
Notes: pgText(req.Notes),
|
||||
PercentageLocation: pgFloat8(req.PercentageLoc),
|
||||
EpubcfiLocation: pgText(req.EpubcfiLocation),
|
||||
ChapterReference: pgInt4(req.ChapterReference),
|
||||
DedupKey: pgtype.Text{String: dedupKey, Valid: true},
|
||||
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
|
||||
EpubcfiLocation: pgText(req.EpubcfiLocation),
|
||||
ChapterReference: pgInt4(req.ChapterReference),
|
||||
DedupKey: pgtype.Text{String: dedupKey, Valid: true},
|
||||
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
|
||||
LastModifiedSource: pgtype.Text{String: req.Source, Valid: req.Source != ""},
|
||||
DeviceSyncData: deviceData,
|
||||
DeviceSyncData: deviceData,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("create bookmark: %w", err)
|
||||
@@ -573,19 +591,19 @@ func (s *AnnotationService) applyBookmarkLWW(ctx context.Context, req SaveBookma
|
||||
deviceData := mergeDeviceSyncData(existing.DeviceSyncData, req.Source, req.DeviceSyncData)
|
||||
|
||||
bm, err := s.db.UpdateMediaBookmarkForSync(ctx, database.UpdateMediaBookmarkForSyncParams{
|
||||
ID: existing.ID,
|
||||
PageNumber: pgInt4(req.PageNumber),
|
||||
ChapterNumber: pgInt4(req.ChapterNumber),
|
||||
CfiPosition: pgText(req.CFIPosition),
|
||||
Title: req.Title,
|
||||
Position: pgText(req.Position),
|
||||
Notes: pgText(req.Notes),
|
||||
ID: existing.ID,
|
||||
PageNumber: pgInt4(req.PageNumber),
|
||||
ChapterNumber: pgInt4(req.ChapterNumber),
|
||||
CfiPosition: pgText(req.CFIPosition),
|
||||
Title: req.Title,
|
||||
Position: pgText(req.Position),
|
||||
Notes: pgText(req.Notes),
|
||||
PercentageLocation: pgFloat8(req.PercentageLoc),
|
||||
EpubcfiLocation: pgText(req.EpubcfiLocation),
|
||||
ChapterReference: pgInt4(req.ChapterReference),
|
||||
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
|
||||
EpubcfiLocation: pgText(req.EpubcfiLocation),
|
||||
ChapterReference: pgInt4(req.ChapterReference),
|
||||
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
|
||||
LastModifiedSource: pgtype.Text{String: req.Source, Valid: req.Source != ""},
|
||||
DeviceSyncData: deviceData,
|
||||
DeviceSyncData: deviceData,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("update bookmark: %w", err)
|
||||
|
||||
Reference in New Issue
Block a user