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

feat(koreader): one conversion route for every feature; bookmarks get web CFIs

Browse Source 
Progress, highlights, notes, and bookmarks entered position conversion
through three different doors: progress converted inline with an
uncached converter, annotations through the facade, bookmarks not at
all (the raw xpointer was stored verbatim, cfi_position stayed empty,
and the web drawer's goToBookmark silently no-ops on cfi-less entries).

Unify on the facade (ConvertToCanonical/ConvertFromCanonical):

- annotationEpub context resolved once per push: media item + EPUB path
  shared by every annotation instead of re-fetched per entry
- progress forward: the inline block becomes one facade call;
  non-reflowable formats pass through unchanged, and the cached
  converter stops re-parsing the book on every sync
- progress reverse: convertCFIToXPointer delegates to reverseConvertCFI,
  keeping the stored percentage in play for the fallback ladder
- bookmarks (bulk progress and /sync-bookmarks): pos0 resolves
  structural-only — bookmark text is a display label, never book text,
  so no context is supplied; webUsableCFI stores the result only for
  structural/exact epubcfi landings, discarding href/percentage results
  rather than storing dead drawer links. Also records percentage_location
  and origin_source on the legacy endpoint.
- percentages thread through: highlights/notes/bookmarks pass the device
  percentage or the derived section percentage instead of a hardcoded 0,
  so the last-resort fallback lands near the true position instead of
  the document start
- extendCFIByLength end-derivation now also fires on structural starts
  (it had silently stopped matching when the structural rung began
  landing starts with precision 'structural' rather than 'exact')

Tests: the drop-cap xpointer through the facade with empty context (the
bookmark scenario) must land structurally, not doc-start; webUsableCFI
table covers the store/discard gate.
This commit is contained in:
john-okeefe
2026-09-09 09:04:23 -04:00
parent 5a6c361c11
commit 9899ba6e9f
3 changed files with 216 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/handlers/koreader.go
+135 -100
View File
@@ -50,35 +50,88 @@ func (h *KOReaderHandler) SetAnnotationService(svc *wsync.AnnotationService) {
h.annotationSvc = svc
}
func (h *KOReaderHandler) convertHighlightPositions(ctx context.Context, mediaItemID pgtype.UUID, pos0, pos1, contextText string) (string, string) {
if pos0 == "" || h.libraryService == nil {
return "", ""
}
// annotationEpub carries the per-book context every locator conversion
// needs: the media item (format gating) and the resolved EPUB path. It is
// resolved once per request so all annotations in a push share one
// converter-cache entry instead of re-resolving (and re-parsing the book)
// per annotation.
type annotationEpub struct {
mediaItem *database.MediaItems
epubPath string
}
func (ec annotationEpub) convertible() bool {
return ec.mediaItem != nil && ec.epubPath != ""
}
func (h *KOReaderHandler) loadAnnotationEpub(ctx context.Context, mediaItemID pgtype.UUID) annotationEpub {
var ec annotationEpub
mediaItem, err := h.db.GetMediaItem(ctx, mediaItemID)
if err != nil {
return ec
}
ec.mediaItem = &mediaItem
if h.libraryService != nil {
if epubPath, err := h.libraryService.ResolveMediaPath(ctx, mediaItem.LibraryID, mediaItem.FilePath); err == nil && epubPath != "" {
ec.epubPath = epubPath
}
}
return ec
}
// convertHighlightPositions resolves a device annotation's pos0/pos1
// locators to canonical CFIs through the shared facade. contextText is the
// selection's own text — the ideal anchor for the converter's verification
// and text-search rungs. percentage anchors the last-resort fallback so a
// failed conversion degrades to the neighborhood of the true position
// rather than the document start.
func (h *KOReaderHandler) convertHighlightPositions(ec annotationEpub, pos0, pos1, contextText string, percentage float64) (string, string) {
if pos0 == "" || !ec.convertible() {
return "", ""
}
epubPath, err := h.libraryService.ResolveMediaPath(ctx, mediaItem.LibraryID, mediaItem.FilePath)
if err != nil || epubPath == "" {
return "", ""
}
// The annotation's own text is the ideal anchor for the converter's
// text-search path: clients (thin, underpowered) send only raw
// locators, the server resolves them against the actual book.
startLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos0, 0, contextText, mediaItem.FormatGroup, epubPath, "")
endLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos1, 0, "", mediaItem.FormatGroup, epubPath, "")
startLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos0, percentage, contextText, ec.mediaItem.FormatGroup, ec.epubPath, "")
endLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos1, percentage, "", ec.mediaItem.FormatGroup, ec.epubPath, "")
endCFI := endLoc.CFI
// The end conversion carries no context text, so unless it resolved
// exactly it degenerates to a percentage fallback anchored at the
// document start — useless as a range end. When the START resolved
// exactly, derive the end from it: same node, character offset
// advanced by the selection's UTF-16 length (the CFI offset unit).
if endLoc.Precision != "exact" && startLoc.Precision == "exact" && contextText != "" {
// structurally/exactly, derive the end from it: same node, character
// offset advanced by the selection's UTF-16 length (the CFI offset
// unit).
if endLoc.Precision != "exact" && endLoc.Precision != "structural" &&
(startLoc.Precision == "exact" || startLoc.Precision == "structural") && contextText != "" {
endCFI = extendCFIByLength(startLoc.CFI, contextText)
}
return startLoc.CFI, endCFI
}
// convertBookmarkPosition resolves a device bookmark's locator to the
// canonical CFI for the web reader. Structural-only by design: bookmark
// text is a display label ("in <chapter>" auto-fill or a user note), never
// book text, so no context is supplied and only a structural/exact landing
// is trusted — lower rungs would store a confident-looking guess the web
// drawer would present as a real destination.
func (h *KOReaderHandler) convertBookmarkPosition(ec annotationEpub, position string, percentage float64) string {
if position == "" || !ec.convertible() || !wsync.IsCREXPointer(position) {
return ""
}
loc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, position, percentage, "", ec.mediaItem.FormatGroup, ec.epubPath, "")
return webUsableCFI(loc)
}
// webUsableCFI keeps only high-confidence conversions: the web reader
// navigates bookmarks by CFI, so href/percentage/fallback results are
// discarded instead of stored as dead links.
func webUsableCFI(loc wsync.CanonicalLocator) string {
if loc.Precision != "structural" && loc.Precision != "exact" {
return ""
}
if !strings.HasPrefix(loc.CFI, "epubcfi(") || !strings.HasSuffix(loc.CFI, ")") {
return ""
}
return loc.CFI
}
// extendCFIByLength advances a point CFI's trailing character offset by the
// UTF-16 length of text (EPUB CFI character offsets are UTF-16 code units).
// Selections spanning multiple text nodes produce an out-of-range offset —
@@ -127,22 +180,19 @@ func (h *KOReaderHandler) existingHighlightColor(ctx context.Context, mediaItemI
// 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).
func (h *KOReaderHandler) deriveAnnotationPercentage(ctx context.Context, mediaItemID pgtype.UUID, pos0 string, page int) float64 {
mediaItem, err := h.db.GetMediaItem(ctx, mediaItemID)
if err != nil {
func (h *KOReaderHandler) deriveAnnotationPercentage(ec annotationEpub, pos0 string, page int) float64 {
if ec.mediaItem == nil {
return 0
}
formatGroup := wsync.FormatGroup(mediaItem.FormatGroup)
formatGroup := wsync.FormatGroup(ec.mediaItem.FormatGroup)
if formatGroup == wsync.FormatGroupFixedLayout || formatGroup == wsync.FormatGroupComicArchive {
if page > 0 && mediaItem.PageCount.Valid && mediaItem.PageCount.Int32 > 0 {
return float64(page) / float64(mediaItem.PageCount.Int32)
if page > 0 && ec.mediaItem.PageCount.Valid && ec.mediaItem.PageCount.Int32 > 0 {
return float64(page) / float64(ec.mediaItem.PageCount.Int32)
}
return 0
}
if wsync.IsCREXPointer(pos0) && h.libraryService != nil {
if epubPath, err := h.libraryService.ResolveMediaPath(ctx, mediaItem.LibraryID, mediaItem.FilePath); err == nil && epubPath != "" {
return wsync.NewCFIConverter(epubPath).SectionPercentage(pos0)
}
if wsync.IsCREXPointer(pos0) && ec.epubPath != "" {
return wsync.SectionPercentageCached(ec.epubPath, pos0)
}
return 0
}
@@ -614,21 +664,23 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
if h.annotationSvc == nil {
return
}
ec := h.loadAnnotationEpub(ctx, mediaItemID)
for _, hl := range book.Highlights {
startPos := hl.Pos0
endPos := hl.Pos1
// The highlight's own text anchors the conversion exactly.
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ctx, mediaItemID, startPos, endPos, hl.Text)
pctStart := 0.0
if hl.Percentage != nil {
pctStart = *hl.Percentage
}
if pctStart == 0 {
pctStart = h.deriveAnnotationPercentage(ctx, mediaItemID, startPos, int(hl.Page))
pctStart = h.deriveAnnotationPercentage(ec, startPos, int(hl.Page))
}
// The highlight's own text anchors the conversion exactly.
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ec, startPos, endPos, hl.Text, pctStart)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": hl.Datetime,
"pos0": hl.Pos0,
@@ -676,16 +728,17 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
for _, note := range book.Notes {
startPos := note.Pos0
endPos := note.Pos1
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ctx, mediaItemID, startPos, endPos, note.Text)
pctStart := 0.0
if note.Percentage != nil {
pctStart = *note.Percentage
}
if pctStart == 0 {
pctStart = h.deriveAnnotationPercentage(ctx, mediaItemID, startPos, int(note.Page))
pctStart = h.deriveAnnotationPercentage(ec, startPos, int(note.Page))
}
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ec, startPos, endPos, note.Text, pctStart)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": note.Datetime,
"pos0": note.Pos0,
@@ -723,6 +776,19 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
position = fmt.Sprintf("page:%d", bookmark.Page)
}
pctLoc := 0.0
if bookmark.Percentage != nil {
pctLoc = *bookmark.Percentage
}
if pctLoc == 0 {
pctLoc = h.deriveAnnotationPercentage(ec, position, int(bookmark.Page))
}
// Device-native xpointer → canonical CFI so the web drawer can
// navigate KOReader-created bookmarks (page-only positions have no
// convertible locator; the drawer's page fallback covers those).
cfiPosition := h.convertBookmarkPosition(ec, position, pctLoc)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": bookmark.Datetime,
"pos0": bookmark.Pos0,
@@ -739,6 +805,8 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
UserID: userID,
Title: bookmark.Text,
Position: position,
CFIPosition: cfiPosition,
PercentageLoc: pctLoc,
ChapterNumber: int32(bookmark.Chapter),
Source: "koreader",
OriginSource: "koreader",
@@ -784,29 +852,11 @@ func (h *KOReaderHandler) updateProgressForBook(c *echo.Context, deviceID pgtype
if h.progressSvc != nil {
epubcfi := book.Epubcfi
if epubcfi != nil && wsync.IsCREXPointer(*epubcfi) {
log.Printf("Bookhoard: CRE→CFI attempting conversion for %s", *epubcfi)
mediaItem, err := h.db.GetMediaItem(ctx, mediaItemID)
if err != nil {
log.Printf("Bookhoard: CRE→CFI failed to get media item: %v", err)
} else if mediaItem.FormatGroup == string(wsync.FormatGroupFixedLayout) ||
mediaItem.FormatGroup == string(wsync.FormatGroupComicArchive) {
// Image-based fixed content (fixed-layout comic EPUBs, PDF,
// comic archives) has no extractable text, so CRE→CFI conversion
// cannot succeed. The page index (page/total_pages) is the
// canonical locator. Keep the incoming xpointer for device-native
// restore; the web reader restores by page.
log.Printf("Bookhoard: CRE→CFI skipped for %s format", mediaItem.FormatGroup)
} else if h.libraryService == nil {
log.Printf("Bookhoard: CRE→CFI libraryService is nil, skipping conversion")
} else {
epubPath, resolveErr := h.libraryService.ResolveMediaPath(ctx, mediaItem.LibraryID, mediaItem.FilePath)
if resolveErr != nil {
log.Printf("Bookhoard: CRE→CFI failed to resolve media path: %v", resolveErr)
} else if epubPath == "" {
log.Printf("Bookhoard: CRE→CFI resolved empty epub path for %s", mediaItem.FilePath)
} else {
log.Printf("Bookhoard: CRE→CFI resolved epub path: %s", epubPath)
converter := wsync.NewCFIConverter(epubPath)
// Same facade every annotation uses: cached converter,
// structural-first resolution, guarded fallbacks. The facade
// passes non-reflowable formats through untouched.
ec := h.loadAnnotationEpub(ctx, mediaItemID)
if ec.convertible() {
pct := 0.0
if book.Percentage >= 0 {
pct = book.Percentage
@@ -815,22 +865,11 @@ func (h *KOReaderHandler) updateProgressForBook(c *echo.Context, deviceID pgtype
if book.ContextText != nil {
contextText = *book.ContextText
}
result, convErr := converter.ConvertCREToStandard(*epubcfi, pct, contextText)
if convErr != nil {
log.Printf("Bookhoard: CRE→CFI conversion error: %v", convErr)
} else if result != nil {
if result.EPUBCFI != "" {
convertedCFI := result.EPUBCFI
epubcfi = &convertedCFI
log.Printf("Bookhoard: CRE→CFI converted to epubcfi: %s", convertedCFI)
} else if result.Href != "" {
convertedHref := result.Href
epubcfi = &convertedHref
log.Printf("Bookhoard: CRE→CFI converted to href: %s", convertedHref)
} else {
log.Printf("Bookhoard: CRE→CFI conversion: %s precision for %s", result.Precision, *epubcfi)
}
}
loc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, *epubcfi, pct, contextText, ec.mediaItem.FormatGroup, ec.epubPath, "")
if loc.CFI != "" && loc.CFI != *epubcfi {
converted := loc.CFI
epubcfi = &converted
log.Printf("Bookhoard: CRE→CFI converted progress (%s) to %s", loc.Precision, converted)
}
}
}
@@ -1169,40 +1208,20 @@ func (h *KOReaderHandler) GetMetadata(c *echo.Context) error {
}
func (h *KOReaderHandler) convertCFIToXPointer(c *echo.Context, mediaItem database.MediaItems, progress database.GetUniversalProgressRow, progressData *KOReaderProgressData) {
if h.libraryService == nil {
log.Printf("Bookhoard: CFI→CRE libraryService is nil, skipping reverse conversion")
return
}
epubPath, err := h.libraryService.ResolveMediaPath(c.Request().Context(), mediaItem.LibraryID, mediaItem.FilePath)
if err != nil {
log.Printf("Bookhoard: CFI→CRE failed to resolve media path: %v", err)
return
}
if epubPath == "" {
log.Printf("Bookhoard: CFI→CRE resolved empty epub path for %s", mediaItem.FilePath)
return
}
converter := wsync.NewCFIConverter(epubPath)
contextText := ""
if progress.ContextText.Valid {
contextText = progress.ContextText.String
}
pct := progress.Percentage.Float64
result, err := converter.ConvertStandardToCRE(progress.Epubcfi.String, pct, contextText)
if err != nil {
log.Printf("Bookhoard: CFI→CRE conversion error: %v", err)
return
}
if result != nil && result.XPointer != "" {
progressData.KoreaderXPointer = &result.XPointer
log.Printf("Bookhoard: CFI→CRE converted to XPointer: %s", result.XPointer)
// Same facade path annotations use on serve: structural resolution
// first, guarded text search only as fallback, cached converter. The
// stored percentage anchors the reverse fallback ladder.
if xp := h.reverseConvertCFI(c, mediaItem, progress.Epubcfi.String, contextText, progress.Percentage.Float64); xp != "" {
progressData.KoreaderXPointer = &xp
log.Printf("Bookhoard: CFI→CRE converted to XPointer: %s", xp)
}
}
func (h *KOReaderHandler) reverseConvertCFI(c *echo.Context, mediaItem database.MediaItems, epubcfi string, contextText string) string {
func (h *KOReaderHandler) reverseConvertCFI(c *echo.Context, mediaItem database.MediaItems, epubcfi string, contextText string, percentage float64) string {
if h.libraryService == nil || epubcfi == "" {
return ""
}
@@ -1210,7 +1229,7 @@ func (h *KOReaderHandler) reverseConvertCFI(c *echo.Context, mediaItem database.
if err != nil || epubPath == "" {
return ""
}
loc := wsync.ConvertFromCanonical(wsync.LocatorSourceKOReader, epubcfi, 0, contextText, mediaItem.FormatGroup, epubPath, "")
loc := wsync.ConvertFromCanonical(wsync.LocatorSourceKOReader, epubcfi, percentage, contextText, mediaItem.FormatGroup, epubPath, "")
if loc.Position != "" && loc.Position != epubcfi {
return loc.Position
}
@@ -1317,7 +1336,7 @@ func (h *KOReaderHandler) koreaderPos0(c *echo.Context, mediaItem database.Media
cfi = strings.TrimPrefix(startPosition, "cfi:")
}
if cfi != "" && wsync.IsStandardEPUBCFI(cfi) {
if converted := h.reverseConvertCFI(c, mediaItem, cfi, contextText); converted != "" {
if converted := h.reverseConvertCFI(c, mediaItem, cfi, contextText, 0); converted != "" {
return converted
}
// Conversion failed; fall through so numeric positions still work.
@@ -1483,6 +1502,17 @@ func (h *KOReaderHandler) SyncBookmarks(c *echo.Context) error {
}
if h.annotationSvc != nil {
ec := h.loadAnnotationEpub(ctx, mediaItemID)
pctLoc := 0.0
if bookmark.Percentage != nil {
pctLoc = *bookmark.Percentage
}
if pctLoc == 0 {
pctLoc = h.deriveAnnotationPercentage(ec, position, int(bookmark.Page))
}
cfiPosition := h.convertBookmarkPosition(ec, position, pctLoc)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": bookmark.Datetime,
"pos0": bookmark.Pos0,
@@ -1494,8 +1524,11 @@ func (h *KOReaderHandler) SyncBookmarks(c *echo.Context) error {
UserID: pgUserID,
Title: bookmark.Text,
Position: position,
CFIPosition: cfiPosition,
PercentageLoc: pctLoc,
ChapterNumber: int32(bookmark.Chapter),
Source: "koreader",
OriginSource: "koreader",
DeviceSyncData: deviceData,
})
if err == nil && result.Outcome != wsync.SaveOutcomeDeleted {
@@ -1589,13 +1622,15 @@ func (h *KOReaderHandler) SyncBookmarks(c *echo.Context) error {
}
if h.annotationSvc != nil {
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ctx, mediaItemID, highlight.Pos0, highlight.Pos1, highlight.Text)
ec := h.loadAnnotationEpub(ctx, mediaItemID)
pctStart := 0.0
if highlight.Percentage != nil {
pctStart = *highlight.Percentage
}
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ec, highlight.Pos0, highlight.Pos1, highlight.Text, pctStart)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": highlight.Datetime,
"pos0": highlight.Pos0,
internal/handlers/koreader_test.go
+52
View File
@@ -4,6 +4,8 @@ import (
"encoding/json"
"testing"
wsync "bookhoard/internal/sync"
"github.com/stretchr/testify/assert"
)
@@ -50,3 +52,53 @@ func TestKOReaderProgressRequest_DeletedAnnotationsOmitted(t *testing.T) {
assert.Empty(t, req.Books[0].DeletedHighlights)
assert.Empty(t, req.Books[0].DeletedBookmarks)
}
// The web drawer navigates bookmarks by CFI, so only high-confidence
// conversions may be stored: href/percentage/fallback results would
// become dead links. This is the sole gate for KOReader→web bookmark
// positions (bookmark text is a label, never book text, so the
// conversion runs structural-only with empty context).
func TestWebUsableCFI(t *testing.T) {
cases := []struct {
name string
loc wsync.CanonicalLocator
expected string
}{
{
name: "structural landing stored",
loc: wsync.CanonicalLocator{CFI: "epubcfi(/6/52!/4/28/2/1:0)", Precision: "structural", Percentage: 0.52},
expected: "epubcfi(/6/52!/4/28/2/1:0)",
},
{
name: "exact text-search landing stored",
loc: wsync.CanonicalLocator{CFI: "epubcfi(/6/52!/4/28/2/1:0)", Precision: "exact", Percentage: 0.52},
expected: "epubcfi(/6/52!/4/28/2/1:0)",
},
{
name: "percentage guess discarded",
loc: wsync.CanonicalLocator{CFI: "epubcfi(/6/52!/4/2/1:0)", Precision: "percentage", Percentage: 0.52},
expected: "",
},
{
name: "fallback passthrough (raw xpointer) discarded",
loc: wsync.CanonicalLocator{CFI: "/body/DocFragment[2]/body/p[3]", Precision: "fallback", Percentage: 0.52},
expected: "",
},
{
name: "section href discarded (not a CFI)",
loc: wsync.CanonicalLocator{CFI: "ch10.xhtml", Precision: "section", Percentage: 0.52},
expected: "",
},
{
name: "structural but non-CFI value discarded",
loc: wsync.CanonicalLocator{CFI: "ch10.xhtml#h1", Precision: "structural", Percentage: 0.52},
expected: "",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, webUsableCFI(tc.loc))
})
}
}
internal/sync/cfi_converter_test.go
+21
View File
@@ -666,3 +666,24 @@ func TestReverseIgnoresSingleCharContext(t *testing.T) {
t.Errorf("single-char reverse context must fall back to percentage, got %s", reverse.Precision)
}
}
// The bookmark route supplies no context (bookmark text is a display
// label, never book text), so the facade must still resolve the drop-cap
// xpointer structurally instead of collapsing to the document start.
func TestConvertToCanonical_DropCapEmptyContext(t *testing.T) {
epubPath := writeDropCapEPUB(t)
xp := "/body/DocFragment[2]/body/p[3]/span[1]/text().0"
loc := ConvertToCanonical(LocatorSourceKOReader, xp, 0.52, "", string(FormatGroupReflowable), epubPath, "")
t.Logf("facade drop-cap (no context) → %s (%s)", loc.CFI, loc.Precision)
if loc.Precision != "structural" && loc.Precision != "exact" {
t.Fatalf("expected structural/exact precision, got %s (%s)", loc.Precision, loc.CFI)
}
if !strings.HasPrefix(loc.CFI, "epubcfi(") {
t.Fatalf("expected standard epubcfi, got %s", loc.CFI)
}
if strings.HasSuffix(loc.CFI, "/4/2/1:0)") {
t.Errorf("empty-context conversion collapsed to doc start: %s", loc.CFI)
}
}