refactor(progress): ProgressService required at construction; delete the 9 unreachable erase-fallbacks
Batch H1 step 3a. The ceiling census (exhaustive UpdateUniversalProgress
grep) found 9 nil-guarded `if progressSvc != nil { funnel } else { raw
erase-write }` fallbacks — the survey's "four" under-counted kobo.go,
which has FOUR sites (Markup ReadingSync, Markup last-read-place,
AnalyticsGettests, SyncFromServer), plus one each in media.go, queue.go
and koreader.go. All unreachable: the service is created unconditionally
at main.go:127 and wired into every handler and the queue processor; the
test harness wires them too.
The erasure pattern is now unrepresentable:
- NewMediaHandler / NewKOReaderHandler / NewKoboHandler / both
NewSyncQueueProcessor flavors take the ProgressService as a REQUIRED
constructor arg; the SetProgressService setters are gone.
- Every fallback body (raw UpdateUniversalProgress with explicit-NULL
pgtypes for viewport/scroll/panel/reading_mode/zoom and no
audio_offset_seconds/context_text) is deleted; each write funnels
through SaveProgress with the LWW seed, so fields the writer doesn't
know survive.
- Removed the now-dead koreader pg-type helpers (textPtrToPgText,
intPtrToPgInt4, int64PtrToPgInt8) and their self-tests.
Behavior deltas (stored-row content, no wire shape changes —
KOReader/Kobo response pins stayed green):
- KOReader fallback used to write explicit NULL viewports and drop the
context_text the koplugin sends; the funneled mainline already maps
book.ContextText.
- Queue flushes no longer overwrite chapter_progress with the book
percentage (bogus data; preserved via the seed).
- A web PUT without percentage no longer writes 0% via the deleted
fallback; percentage seeds from the stored row.
Contract suite: KOReader×4 + Kobo×3 pins green.
This commit is contained in:
John O'Keefe committed
1 parent
28bd323505
commit
e83c00f869
9 files changed
+163
-419
No files matched your search
+61
-125
@@ -36,19 +36,16 @@ type LibraryPathResolver interface {
|
||||
ResolveMediaPath(ctx context.Context, libraryID pgtype.UUID, relativePath string) (string, error)
|
||||
}
|
||||
|
||||
func NewKOReaderHandler(db *database.Queries, connManager *wsync.ConnectionManager, queue *wsync.SyncQueueProcessor) *KOReaderHandler {
|
||||
func NewKOReaderHandler(db *database.Queries, connManager *wsync.ConnectionManager, queue *wsync.SyncQueueProcessor, progressSvc *wsync.ProgressService) *KOReaderHandler {
|
||||
return &KOReaderHandler{
|
||||
db: db,
|
||||
connManager: connManager,
|
||||
queue: queue,
|
||||
progressSvc: progressSvc,
|
||||
bookResolver: services.NewBookResolver(db),
|
||||
}
|
||||
}
|
||||
|
||||
func (h *KOReaderHandler) SetProgressService(svc *wsync.ProgressService) {
|
||||
h.progressSvc = svc
|
||||
}
|
||||
|
||||
func (h *KOReaderHandler) SetAnnotationService(svc *wsync.AnnotationService) {
|
||||
h.annotationSvc = svc
|
||||
}
|
||||
@@ -878,138 +875,77 @@ func (h *KOReaderHandler) updateProgressForBook(c *echo.Context, deviceID pgtype
|
||||
deviceModel = "KOReader Device"
|
||||
}
|
||||
|
||||
if h.progressSvc != nil {
|
||||
epubcfi := book.Epubcfi
|
||||
// No position pointer from the device, or a conversion that
|
||||
// declined below: the write is percentage-only by intent. The
|
||||
// service then clears any inherited stale anchor instead of
|
||||
// re-pairing it with the new percentage.
|
||||
clearLocator := epubcfi == nil
|
||||
if epubcfi != nil && wsync.IsCREXPointer(*epubcfi) {
|
||||
// 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
|
||||
}
|
||||
contextText := ""
|
||||
if book.ContextText != nil {
|
||||
contextText = *book.ContextText
|
||||
}
|
||||
loc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, *epubcfi, pct, contextText, ec.mediaItem.FormatGroup, ec.epubPath, "")
|
||||
switch {
|
||||
case strings.HasPrefix(loc.CFI, "epubcfi(") &&
|
||||
(loc.Precision == "structural" || loc.Precision == "exact"):
|
||||
converted := loc.CFI
|
||||
epubcfi = &converted
|
||||
log.Printf("Bookhoard: CRE→CFI converted progress (%s) to %s", loc.Precision, converted)
|
||||
case loc.CFI != "" && loc.CFI != *epubcfi &&
|
||||
(loc.Precision == "element" || loc.Precision == "section"):
|
||||
// Fragment-ID positions resolve to a section href: keep
|
||||
// serving it (legacy behavior).
|
||||
converted := loc.CFI
|
||||
epubcfi = &converted
|
||||
log.Printf("Bookhoard: CRE→CFI converted progress (%s) to href %s", loc.Precision, converted)
|
||||
default:
|
||||
// Low-confidence (percentage/fallback): store no
|
||||
// canonical locator — the row's percentage restores
|
||||
// approximately instead of a confidently-wrong CFI,
|
||||
// and the device keeps its own native position.
|
||||
log.Printf("Bookhoard: CRE→CFI conversion low-confidence (%s) for %s; storing percentage only", loc.Precision, *epubcfi)
|
||||
epubcfi = nil
|
||||
clearLocator = true
|
||||
}
|
||||
epubcfi := book.Epubcfi
|
||||
// No position pointer from the device, or a conversion that
|
||||
// declined below: the write is percentage-only by intent. The
|
||||
// service then clears any inherited stale anchor instead of
|
||||
// re-pairing it with the new percentage.
|
||||
clearLocator := epubcfi == nil
|
||||
if epubcfi != nil && wsync.IsCREXPointer(*epubcfi) {
|
||||
// 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
|
||||
}
|
||||
contextText := ""
|
||||
if book.ContextText != nil {
|
||||
contextText = *book.ContextText
|
||||
}
|
||||
loc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, *epubcfi, pct, contextText, ec.mediaItem.FormatGroup, ec.epubPath, "")
|
||||
switch {
|
||||
case strings.HasPrefix(loc.CFI, "epubcfi(") &&
|
||||
(loc.Precision == "structural" || loc.Precision == "exact"):
|
||||
converted := loc.CFI
|
||||
epubcfi = &converted
|
||||
log.Printf("Bookhoard: CRE→CFI converted progress (%s) to %s", loc.Precision, converted)
|
||||
case loc.CFI != "" && loc.CFI != *epubcfi &&
|
||||
(loc.Precision == "element" || loc.Precision == "section"):
|
||||
// Fragment-ID positions resolve to a section href: keep
|
||||
// serving it (legacy behavior).
|
||||
converted := loc.CFI
|
||||
epubcfi = &converted
|
||||
log.Printf("Bookhoard: CRE→CFI converted progress (%s) to href %s", loc.Precision, converted)
|
||||
default:
|
||||
// Low-confidence (percentage/fallback): store no
|
||||
// canonical locator — the row's percentage restores
|
||||
// approximately instead of a confidently-wrong CFI,
|
||||
// and the device keeps its own native position.
|
||||
log.Printf("Bookhoard: CRE→CFI conversion low-confidence (%s) for %s; storing percentage only", loc.Precision, *epubcfi)
|
||||
epubcfi = nil
|
||||
clearLocator = true
|
||||
}
|
||||
}
|
||||
|
||||
saveReq := wsync.SaveProgressRequest{
|
||||
MediaItemID: mediaItemID,
|
||||
UserID: userID,
|
||||
Source: "koreader",
|
||||
DeviceID: deviceID,
|
||||
Percentage: &book.Percentage,
|
||||
Epubcfi: epubcfi,
|
||||
ContextText: book.ContextText,
|
||||
Chapter: book.Chapter,
|
||||
CharacterOffset: book.Character,
|
||||
CurrentPage: book.Page,
|
||||
TotalPages: book.TotalPages,
|
||||
DeviceType: "koreader",
|
||||
DeviceName: deviceModel,
|
||||
Broadcast: true,
|
||||
ClearLocator: clearLocator,
|
||||
}
|
||||
|
||||
_, err := h.progressSvc.SaveProgress(ctx, saveReq)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
h.processBookAnnotations(ctx, deviceID, userID, mediaItemID, book)
|
||||
return nil
|
||||
}
|
||||
|
||||
_, err := h.db.UpdateUniversalProgress(ctx, database.UpdateUniversalProgressParams{
|
||||
saveReq := wsync.SaveProgressRequest{
|
||||
MediaItemID: mediaItemID,
|
||||
UserID: userID,
|
||||
Percentage: pgtype.Float8{Float64: book.Percentage, Valid: true},
|
||||
Epubcfi: textPtrToPgText(book.Epubcfi),
|
||||
Chapter: intPtrToPgInt4(book.Chapter),
|
||||
ChapterProgress: pgtype.Float8{Float64: book.Percentage, Valid: true},
|
||||
CharacterOffset: int64PtrToPgInt8(book.Character),
|
||||
CurrentPage: intPtrToPgInt4(book.Page),
|
||||
TotalPages: intPtrToPgInt4(book.TotalPages),
|
||||
LastSyncDevice: pgtype.Text{String: "koreader", Valid: true},
|
||||
LastSyncSource: pgtype.Text{String: "koreader", Valid: true},
|
||||
ViewportY: pgtype.Float8{},
|
||||
ScrollPositionX: pgtype.Float8{},
|
||||
ScrollPositionY: pgtype.Float8{},
|
||||
PanelNumber: pgtype.Int4{},
|
||||
ReadingMode: pgtype.Text{},
|
||||
ZoomLevel: pgtype.Float8{},
|
||||
})
|
||||
if err != nil {
|
||||
Source: "koreader",
|
||||
DeviceID: deviceID,
|
||||
Percentage: &book.Percentage,
|
||||
Epubcfi: epubcfi,
|
||||
ContextText: book.ContextText,
|
||||
Chapter: book.Chapter,
|
||||
CharacterOffset: book.Character,
|
||||
CurrentPage: book.Page,
|
||||
TotalPages: book.TotalPages,
|
||||
DeviceType: "koreader",
|
||||
DeviceName: deviceModel,
|
||||
Broadcast: true,
|
||||
ClearLocator: clearLocator,
|
||||
}
|
||||
|
||||
if _, err := h.progressSvc.SaveProgress(ctx, saveReq); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
h.connManager.BroadcastProgressUpdate(
|
||||
mediaItemID.Bytes,
|
||||
book.Percentage,
|
||||
wsync.SourceDevice{
|
||||
ID: uuid.UUID(deviceID.Bytes).String(),
|
||||
Name: deviceModel,
|
||||
Type: "koreader",
|
||||
},
|
||||
)
|
||||
|
||||
h.processBookAnnotations(ctx, deviceID, userID, mediaItemID, book)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func textPtrToPgText(s *string) pgtype.Text {
|
||||
if s != nil {
|
||||
return pgtype.Text{String: *s, Valid: true}
|
||||
}
|
||||
return pgtype.Text{}
|
||||
}
|
||||
|
||||
func intPtrToPgInt4(i *int) pgtype.Int4 {
|
||||
if i != nil {
|
||||
return pgtype.Int4{Int32: int32(*i), Valid: true}
|
||||
}
|
||||
return pgtype.Int4{}
|
||||
}
|
||||
|
||||
func int64PtrToPgInt8(i *int64) pgtype.Int8 {
|
||||
if i != nil {
|
||||
return pgtype.Int8{Int64: *i, Valid: true}
|
||||
}
|
||||
return pgtype.Int8{}
|
||||
}
|
||||
|
||||
type KOReaderResolveResponse struct {
|
||||
BookUUID string `json:"book_uuid"`
|
||||
SHA256 string `json:"sha256,omitempty"`
|
||||
|
||||
Reference in new issue
Block a user