docs: add PROGRESS_MIGRATION.md with full plan, bug list, and execution order
Documents the ProgressService migration including: data loss bug analysis, handler-by-handler migration plan, route changes, test strategy, and known issues for future work (conflict_detected column never set to true, offline detector not started, server-side CFI generation needs Go EPUB parser).
This commit is contained in:
@@ -0,0 +1,447 @@
|
||||
# Universal Progress Service Migration Plan
|
||||
|
||||
## Goal
|
||||
|
||||
Consolidate all progress-saving handlers into a single `ProgressService` that:
|
||||
- Merges new data with existing progress (preventing data loss across client switches)
|
||||
- Enriches progress with computed fields (e.g., character_offset from percentage)
|
||||
- Detects conflicts between different sync sources
|
||||
- Broadcasts updates via WebSocket
|
||||
- Is called by all clients: web reader, KOReader, Kobo
|
||||
|
||||
## Architecture
|
||||
|
||||
```
|
||||
Client Request → HTTP Handler (thin) → ProgressService.SaveProgress()
|
||||
↓
|
||||
1. Read existing progress from DB
|
||||
2. Merge new data over existing (keep unset fields)
|
||||
3. Enrich (compute missing fields)
|
||||
4. Conflict detection
|
||||
5. Upsert enriched progress to DB
|
||||
6. WebSocket broadcast
|
||||
```
|
||||
|
||||
## Current State: Three Separate Handlers Writing Progress
|
||||
|
||||
| Route | Handler | Auth | What it does |
|
||||
|---|---|---|---|
|
||||
| `PUT /api/media-items/:id/progress` | `MediaHandler.UpdateMediaReadingProgress` | JWT | Raw upsert, 4 fields only (percentage, current_page, total_pages, epubcfi). ALL other fields set to NULL. |
|
||||
| `POST /api/progress/:id` | `Handler.UpdateUniversalProgress` (ScannerHandler) | JWT | Page→percentage conversion, WebSocket broadcast. Still nulls unset fields. |
|
||||
| `POST /api/sync/koreader/progress` | `KOReaderHandler.SyncProgress` | Device token | Book resolution (UUID→hash→path→title), conflict detection, checkpoint mode, WebSocket broadcast. Sets chapter/character_offset but nulls viewport/zoom/panel. |
|
||||
| `POST /api/sync/kobo/:token/markup` | `KoboHandler.Markup` | Device token | ContentId mapping, ReadingSync + BookmarkSync. `last-read-place` only sets epubcfi/chapter, NULLs everything else. |
|
||||
| `POST /api/sync/kobo/:token/v1/analytics/gettests` | `KoboHandler.AnalyticsGettests` | Device token | Same as ReadingSync in Markup. |
|
||||
| `POST /api/sync/kobo/:token/sync-from-server` | `KoboHandler.SyncFromServer` | Device token | Same pattern, `last_sync_source = "bookhoard"`. |
|
||||
|
||||
### Critical Bug in Current Code (Data Loss)
|
||||
|
||||
`UpdateUniversalProgress` SQL uses `ON CONFLICT DO UPDATE SET ... = EXCLUDED.*` — it replaces ALL fields. Any field passed as `Valid: false` (NULL) overwrites whatever was previously stored.
|
||||
|
||||
**This means every cross-client save loses data.** Examples:
|
||||
- KOReader saves character_offset → web reader saves → character_offset becomes NULL
|
||||
- Kobo saves percentage → Kobo sends last-read-place → percentage becomes NULL
|
||||
- KOReader saves chapter → web reader saves → chapter becomes NULL
|
||||
|
||||
The merge approach in this migration fixes this.
|
||||
|
||||
## Read Routes
|
||||
|
||||
| Route | Handler | Returns |
|
||||
|---|---|---|
|
||||
| `GET /api/media-items/:id/progress` | `MediaHandler.GetMediaReadingProgress` | Raw `ReadingProgress` struct |
|
||||
| `GET /api/progress/:id` | `Handler.GetUniversalProgress` | Enriched with `format_group`, `total_characters`, `chapter_count` from media_items JOIN |
|
||||
| `GET /api/progress/:id/history` | `Handler.GetProgressHistory` | Reading history array |
|
||||
| `GET /progress` (frontend) | Inline in `frontend.go` | Progress overview page, calls `GetAllProgressData` |
|
||||
|
||||
## Existing Bugs to Fix During Migration
|
||||
|
||||
### KOReader handler (`internal/handlers/koreader.go`)
|
||||
1. **Line ~556:** `UpdateDeviceLastSync` called with zero UUID `pgtype.UUID{Bytes: [16]byte{}, Valid: false}` instead of actual device ID. The correct call already exists in `SyncProgress` at line ~201. Remove the duplicate.
|
||||
2. **Line ~549:** `SourceDevice.ID` set to `uuid.UUID(userID.Bytes).String()` (user ID) instead of device ID. Device ID is available from the device context but not passed through to `updateProgressForBook`.
|
||||
3. **`ChapterProgress` always set to `book.Percentage`** (overall book progress), not chapter-relative. Fix: only set if KOReader provides it explicitly, otherwise preserve existing value via merge.
|
||||
4. **Dead code:** `conflicts` response field is initialized but never populated. This is intentional — conflicts are only shown in web UI, not returned to devices. No change needed.
|
||||
|
||||
### Kobo handler (`internal/handlers/kobo.go`)
|
||||
5. **`last-read-place` (line ~487):** `epubcfi` passed as `Valid: true` even when empty string (BookmarkId doesn't start with `epubcfi(`). Fix: only set `Valid: true` if non-empty after stripping.
|
||||
6. **`calculateFileSHA256` function:** Defined but never called. Dead code — remove.
|
||||
7. **`parseKoboDeviceHeader` function:** Defined but never called in kobo.go (may be used by middleware). Verify before removing.
|
||||
8. **`GetLibrary` bookmark_count:** Counts ALL annotations (highlights + notes + bookmarks), not just bookmarks. Known issue, fix separately.
|
||||
|
||||
### Media handler (`internal/handlers/media.go`)
|
||||
9. **`UpdateMediaReadingProgress`:** Sets `CharacterOffset`, `Chapter`, `ChapterProgress`, `ViewportX/Y`, `ZoomLevel`, `ScrollPositionX/Y`, `PanelNumber`, `ReadingMode` all to `Valid: false` — nulls them. Fixed by merge approach.
|
||||
|
||||
### Universal progress handler (`internal/handlers/progress.go`)
|
||||
10. **`UpdateUniversalProgress`:** Also sets `ChapterProgress = percentage` (book-wide, not chapter-relative). Same bug as KOReader. Fixed by merge approach.
|
||||
|
||||
### Sync infrastructure
|
||||
11. **`OfflineDetector`** (`internal/sync/offline.go`): Fully implemented but never started in `cmd/server/main.go`. Not part of this migration, but noted.
|
||||
12. **Queue processor `syncNote`/`syncHighlight`** (`internal/sync/queue.go`): Stub methods, not implemented. Not part of this migration.
|
||||
13. **`reading_progress.conflict_detected` column:** Never set to `true` by any handler. The `sync_conflicts` table records conflicts, but the boolean on the progress row stays false. The SQL upsert doesn't include this column in the `DO UPDATE SET` clause. Schema fix needed separately.
|
||||
|
||||
## New Code: ProgressService
|
||||
|
||||
### Location: `internal/sync/progress.go` (add to existing file)
|
||||
|
||||
### Struct
|
||||
|
||||
```go
|
||||
type ProgressService struct {
|
||||
db *database.Queries
|
||||
connManager *ConnectionManager
|
||||
}
|
||||
|
||||
func NewProgressService(db *database.Queries, connManager *ConnectionManager) *ProgressService
|
||||
```
|
||||
|
||||
### Input Struct
|
||||
|
||||
```go
|
||||
type SaveProgressRequest struct {
|
||||
MediaItemID pgtype.UUID
|
||||
UserID pgtype.UUID
|
||||
Source string // "web", "koreader", "kobo", "bookhoard"
|
||||
DeviceID pgtype.UUID // for conflict detection context
|
||||
|
||||
// All pointer fields — nil means "don't change existing value"
|
||||
Percentage *float64
|
||||
Epubcfi *string
|
||||
CharacterOffset *int64
|
||||
Chapter *int
|
||||
ChapterProgress *float64
|
||||
CurrentPage *int
|
||||
TotalPages *int
|
||||
ViewportX *float64
|
||||
ViewportY *float64
|
||||
ZoomLevel *float64
|
||||
ScrollX *float64
|
||||
ScrollY *float64
|
||||
PanelNumber *int
|
||||
ReadingMode *string
|
||||
|
||||
// For broadcast and conflict detection
|
||||
DeviceType string
|
||||
DeviceName string
|
||||
}
|
||||
```
|
||||
|
||||
### SaveProgress Logic (pseudocode)
|
||||
|
||||
```
|
||||
func SaveProgress(ctx, req) (ReadingProgress, error):
|
||||
|
||||
// 1. Get media item metadata (for enrichment)
|
||||
mediaItem = db.GetMediaItem(ctx, req.MediaItemID)
|
||||
|
||||
// 2. Read existing progress
|
||||
existing, err = db.GetReadingProgress(ctx, {MediaItemID, UserID})
|
||||
if err == pgx.ErrNoRows:
|
||||
existing = empty defaults
|
||||
else if err != nil:
|
||||
return err
|
||||
|
||||
// 3. Merge: build UpdateUniversalProgressParams by starting with
|
||||
// existing values, then overwriting with any non-nil fields from req
|
||||
params = buildParamsFromExisting(existing)
|
||||
params = mergeRequestOverParams(params, req)
|
||||
|
||||
// 4. Enrich missing fields
|
||||
if params.Percentage.Valid && !params.CharacterOffset.Valid && mediaItem.TotalCharacters.Valid && mediaItem.TotalCharacters.Int64 > 0:
|
||||
charOffset = PercentageToCharacter(params.Percentage.Float64, mediaItem.TotalCharacters.Int64)
|
||||
params.CharacterOffset = {Int64: charOffset, Valid: true}
|
||||
|
||||
if params.Percentage.Valid && !params.CurrentPage.Valid && params.TotalPages.Valid:
|
||||
page = PercentageToPage(params.Percentage.Float64, int(params.TotalPages.Int32))
|
||||
params.CurrentPage = {Int32: int32(page), Valid: true}
|
||||
|
||||
// (Future: generate CFI from character_offset using EPUB parser)
|
||||
|
||||
// 5. Set sync metadata
|
||||
params.MediaItemID = req.MediaItemID
|
||||
params.UserID = req.UserID
|
||||
params.LastSyncDevice = {String: req.DeviceType, Valid: true}
|
||||
params.LastSyncSource = {String: req.Source, Valid: true}
|
||||
|
||||
// 6. Conflict detection
|
||||
if existing exists AND existing.LastSyncSource.Valid:
|
||||
if existing.LastSyncSource.String != req.Source AND existing.LastSyncTimestamp.Valid:
|
||||
if time.Since(existing.LastSyncTimestamp.Time) < 5*time.Minute:
|
||||
pctDiff = abs(params.Percentage.Float64 - existing.Percentage.Float64)
|
||||
if pctDiff > 0.01:
|
||||
// Record conflict
|
||||
conflictData = buildConflictData(existing, req)
|
||||
db.CreateSyncConflict(ctx, {MediaItemID, UserID, "progress", conflictData})
|
||||
connManager.BroadcastConflictNotification(mediaItemID, "detection", "")
|
||||
|
||||
// 7. Upsert
|
||||
result, err = db.UpdateUniversalProgress(ctx, params)
|
||||
if err != nil:
|
||||
return err
|
||||
|
||||
// 8. Broadcast
|
||||
deviceName = req.DeviceName
|
||||
if deviceName == "": deviceName = req.DeviceType + " Device"
|
||||
connManager.BroadcastProgressUpdate(
|
||||
mediaItemID,
|
||||
params.Percentage.Float64,
|
||||
SourceDevice{ID: req.DeviceID, Name: deviceName, Type: req.Source},
|
||||
)
|
||||
|
||||
return result, nil
|
||||
```
|
||||
|
||||
### Merge Logic Detail
|
||||
|
||||
The `buildParamsFromExisting` function reads every field from the existing `ReadingProgress` row into `UpdateUniversalProgressParams`.
|
||||
|
||||
The `mergeRequestOverParams` function only overwrites a field if the corresponding pointer in `SaveProgressRequest` is non-nil.
|
||||
|
||||
This ensures:
|
||||
- Web reader sends percentage + epubcfi + current_page + total_pages → character_offset from KOReader's last save is preserved
|
||||
- KOReader sends percentage + character_offset + chapter → epubcfi from web reader's last save is preserved
|
||||
- Kobo sends only percentage → everything else preserved
|
||||
- Kobo sends epubcfi + chapter (last-read-place) → percentage from previous ReadingSync preserved
|
||||
|
||||
## File Changes
|
||||
|
||||
### Modified Files
|
||||
|
||||
| File | Change |
|
||||
|---|---|
|
||||
| `internal/sync/progress.go` | Add `ProgressService` struct, `NewProgressService`, `SaveProgress`, merge/enrich helpers |
|
||||
| `internal/handlers/media.go` | `UpdateMediaReadingProgress`: parse richer request, call `ProgressService.SaveProgress`. `GetMediaReadingProgress`: use `GetUniversalProgress` query for enriched response. `MediaHandler` struct: add `progressService` field. Constructor: accept `ProgressService`. |
|
||||
| `internal/handlers/koreader.go` | `updateProgressForBook`: replace raw upsert with `ProgressService.SaveProgress` call. Fix `SourceDevice.ID` bug (use device ID). Remove duplicate `UpdateDeviceLastSync` with zero UUID. `enqueueProgressForBook`: update `ProgressUpdate` struct if needed. `KOReaderHandler` struct: add `progressService` field. Constructor: accept `ProgressService`. |
|
||||
| `internal/handlers/kobo.go` | `Markup` ReadingSync: call `ProgressService.SaveProgress`. `Markup` last-read-place: call `ProgressService.SaveProgress` (merge preserves percentage). `AnalyticsGettests`: same. `SyncFromServer`: same. `KoboHandler` struct: add `progressService` field. Constructor: accept `ProgressService`. |
|
||||
| `internal/handlers/progress.go` | Remove `UpdateUniversalProgress` method. Keep `GetUniversalProgress`, `GetAllProgressData`, `GetProgressHistory`. |
|
||||
| `internal/sync/queue.go` | `syncProgress` method: call `ProgressService.SaveProgress` instead of raw `db.UpdateUniversalProgress`. `SyncQueueProcessor` struct: add `progressService` field. |
|
||||
| `internal/router/media.go` | Add `GET /media-items/:id/progress/history` route. Remove "Legacy" comment from progress routes. |
|
||||
| `internal/router/progress.go` | **DELETE THIS FILE** — routes moved to media.go or removed. |
|
||||
| `internal/router/router.go` | Remove `registerProgressRoutes` call. Create `ProgressService` and inject into `MediaHandler`, `KOReaderHandler`, `KoboHandler`, `SyncQueueProcessor`. |
|
||||
| `cmd/server/main.go` | Create `ProgressService` after `connManager` and `queueProcessor` creation. Pass to handler constructors. |
|
||||
| `web/src/reader/reader.ts` | `saveProgress`: send richer payload (add chapter, chapter_progress, reading_mode, zoom_level, etc.) |
|
||||
|
||||
### Deleted Files
|
||||
|
||||
| File | Why |
|
||||
|---|---|
|
||||
| `internal/router/progress.go` | All routes moved to `media.go` or removed |
|
||||
|
||||
### Dead Code to Remove
|
||||
|
||||
| What | Where |
|
||||
|---|---|
|
||||
| `UpdateReadingProgress` query | `queries/queries.sql` (source) + `queries.sql.go` + `querier.go` (generated) |
|
||||
| `registerProgressRoutes` function | `router/progress.go` (file deleted) |
|
||||
| `calculateFileSHA256` function | `handlers/kobo.go` — never called |
|
||||
| `parseKoboDeviceHeader` function | `handlers/kobo.go` — verify it's not used by middleware before removing |
|
||||
|
||||
## Route Changes
|
||||
|
||||
### Before
|
||||
|
||||
| Method | Route | Handler | Auth |
|
||||
|---|---|---|---|
|
||||
| GET | `/api/media-items/:id/progress` | `MediaHandler.GetMediaReadingProgress` | JWT |
|
||||
| PUT | `/api/media-items/:id/progress` | `MediaHandler.UpdateMediaReadingProgress` | JWT |
|
||||
| DELETE | `/api/media-items/:id/progress` | `MediaHandler.DeleteMediaReadingProgress` | JWT |
|
||||
| GET | `/api/progress/:id` | `Handler.GetUniversalProgress` | JWT |
|
||||
| POST | `/api/progress/:id` | `Handler.UpdateUniversalProgress` | JWT |
|
||||
| GET | `/api/progress/:id/history` | `Handler.GetProgressHistory` | JWT |
|
||||
| POST | `/api/sync/koreader/progress` | `KOReaderHandler.SyncProgress` | Device |
|
||||
| GET | `/api/sync/koreader/metadata/:uuid` | `KOReaderHandler.GetMetadata` | Device |
|
||||
| GET | `/api/sync/koreader/library` | `KOReaderHandler.GetLibrary` | Device |
|
||||
| POST | `/api/sync/koreader/bookmarks` | `KOReaderHandler.SyncBookmarks` | Device |
|
||||
| POST | `/api/sync/kobo/:token/markup` | `KoboHandler.Markup` | Device |
|
||||
| POST | `/api/sync/kobo/:token/bookmark` | `KoboHandler.Bookmark` | Device |
|
||||
| POST | `/api/sync/kobo/:token/v1/analytics/gettests` | `KoboHandler.AnalyticsGettests` | Device |
|
||||
| GET | `/api/sync/kobo/:token/v1/initialization` | `KoboHandler.Initialization` | Device |
|
||||
| POST | `/api/sync/kobo/:token/sync-from-server` | `KoboHandler.SyncFromServer` | Device |
|
||||
|
||||
### After
|
||||
|
||||
| Method | Route | Handler | Auth | Change |
|
||||
|---|---|---|---|---|
|
||||
| GET | `/api/media-items/:id/progress` | `MediaHandler.GetMediaReadingProgress` | JWT | Enhanced response (adds format_group, total_characters) |
|
||||
| PUT | `/api/media-items/:id/progress` | `MediaHandler.UpdateMediaReadingProgress` | JWT | Now calls ProgressService, richer request |
|
||||
| DELETE | `/api/media-items/:id/progress` | `MediaHandler.DeleteMediaReadingProgress` | JWT | No change |
|
||||
| GET | `/api/media-items/:id/progress/history` | `MediaHandler.GetProgressHistory` | JWT | **NEW** (moved from /progress/:id/history) |
|
||||
| ~~GET~~ | ~~`/api/progress/:id`~~ | ~~removed~~ | | **REMOVED** |
|
||||
| ~~POST~~ | ~~`/api/progress/:id`~~ | ~~removed~~ | | **REMOVED** |
|
||||
| ~~GET~~ | ~~`/api/progress/:id/history`~~ | ~~removed~~ | | **MOVED** to media-items |
|
||||
| POST | `/api/sync/koreader/progress` | `KOReaderHandler.SyncProgress` | Device | Internally uses ProgressService |
|
||||
| GET | `/api/sync/koreader/metadata/:uuid` | `KOReaderHandler.GetMetadata` | Device | No change |
|
||||
| GET | `/api/sync/koreader/library` | `KOReaderHandler.GetLibrary` | Device | No change |
|
||||
| POST | `/api/sync/koreader/bookmarks` | `KOReaderHandler.SyncBookmarks` | Device | No change |
|
||||
| POST | `/api/sync/kobo/:token/markup` | `KoboHandler.Markup` | Device | Internally uses ProgressService |
|
||||
| POST | `/api/sync/kobo/:token/bookmark` | `KoboHandler.Bookmark` | Device | No change (bookmark-only, no progress) |
|
||||
| POST | `/api/sync/kobo/:token/v1/analytics/gettests` | `KoboHandler.AnalyticsGettests` | Device | Internally uses ProgressService |
|
||||
| GET | `/api/sync/kobo/:token/v1/initialization` | `KoboHandler.Initialization` | Device | No change |
|
||||
| POST | `/api/sync/kobo/:token/sync-from-server` | `KoboHandler.SyncFromServer` | Device | Internally uses ProgressService |
|
||||
|
||||
**All device-facing URLs are unchanged.** KOReader and Kobo firmware expect exact paths.
|
||||
|
||||
## Conflict Rules (Preserved From Current Behavior)
|
||||
|
||||
- Conflict detected ONLY when:
|
||||
1. Existing progress has `last_sync_source` that differs from current source
|
||||
2. `last_sync_timestamp` is within 5 minutes
|
||||
3. Absolute percentage difference > 0.01 (1%)
|
||||
- On conflict: record in `sync_conflicts` table, broadcast WebSocket notification
|
||||
- Current client's data ALWAYS wins (overwrite, don't merge with conflicting data)
|
||||
- Conflict details NOT returned to device caller (only shown in web UI)
|
||||
- Same-source rapid syncs never trigger conflicts (built-in debouncing for web, same-source check in handler)
|
||||
|
||||
## Enrichment Rules
|
||||
|
||||
After merge, compute missing fields:
|
||||
|
||||
| Condition | Enrichment |
|
||||
|---|---|
|
||||
| Has percentage, missing character_offset, media has total_characters | `character_offset = PercentageToCharacter(pct, totalChars)` |
|
||||
| Has percentage, missing current_page, has total_pages | `current_page = PercentageToPage(pct, totalPages)` |
|
||||
| Has current_page + total_pages, missing percentage | `percentage = PageToPercentage(page, totalPages)` |
|
||||
| Has character_offset + total_characters, missing percentage | `percentage = CharacterToPercentage(char, totalChars)` |
|
||||
| Missing chapter_progress | Preserve existing value (never compute from book-wide percentage) |
|
||||
|
||||
**All enrichment is only computed when source data is valid and non-zero. Silently skip if insufficient data.**
|
||||
|
||||
## Kobo Special Cases
|
||||
|
||||
### `last-read-place` (in Markup BookmarkSync)
|
||||
- Only provides: `epubcfi` (parsed from BookmarkId), `chapter`, `chapter_progress = 0.5`
|
||||
- Does NOT provide: `percentage`, `current_page`, `total_pages`, `character_offset`
|
||||
- **Before migration:** These fields get NULLed (data loss bug)
|
||||
- **After migration:** Merge preserves existing values, only overwrites epubcfi/chapter/chapter_progress
|
||||
|
||||
### `ReadingSync` (in Markup)
|
||||
- Only provides: `percentage` (from PercentRead/100)
|
||||
- Does NOT provide: `epubcfi`, `chapter`, `character_offset`, etc.
|
||||
- **Before migration:** These fields get NULLed (data loss bug)
|
||||
- **After migration:** Merge preserves existing values, enrichment may compute character_offset from percentage
|
||||
|
||||
### `SyncFromServer`
|
||||
- `last_sync_source = "bookhoard"` (NOT "kobo") — this must be preserved
|
||||
- No WebSocket broadcast — this must be preserved
|
||||
|
||||
## KOReader Special Cases
|
||||
|
||||
### Book resolution (stays in handler, NOT in ProgressService)
|
||||
The 4-priority resolution chain is KOReader-specific:
|
||||
1. UUID match (confidence 1.0)
|
||||
2. SHA-256 match (confidence 0.9)
|
||||
3. File path match via alias or DB lookup (confidence 0.7)
|
||||
4. Title + Author match (confidence 0.5/0.4)
|
||||
|
||||
This logic stays in `KOReaderHandler.resolveBookToMediaItem`. Only the final progress write goes through `ProgressService`.
|
||||
|
||||
### Checkpoint mode
|
||||
- `enqueueProgressForBook` creates `ProgressUpdate` struct → queue channel
|
||||
- Queue processor's `syncProgress` calls `ProgressService.SaveProgress` instead of raw upsert
|
||||
- `ProgressService` needs to be injected into `SyncQueueProcessor`
|
||||
|
||||
### Device file aliases
|
||||
- `createDeviceFileAlias` stays in `KOReaderHandler` — it's book resolution, not progress writing
|
||||
|
||||
### Bulk sync
|
||||
- `SyncProgress` loops over books, resolves each, calls `ProgressService.SaveProgress` per book
|
||||
- If one book fails, others continue (current behavior, must preserve)
|
||||
- Error from `SaveProgress` causes the book to not be counted in `booksSynced`
|
||||
|
||||
## Execution Order
|
||||
|
||||
Each step is independently deployable. If a step breaks, previous steps are safe.
|
||||
|
||||
1. **Add `ProgressService` to `internal/sync/progress.go`** — additive, nothing breaks
|
||||
2. **Write tests for `ProgressService.SaveProgress`** — verify merge, enrichment, conflict detection
|
||||
3. **Update `cmd/server/main.go` and `internal/router/router.go`** — create `ProgressService`, inject into handlers. Pass as new parameter to constructors.
|
||||
4. **Update `MediaHandler`** — accept `ProgressService`, use it in `UpdateMediaReadingProgress`. Accept richer request body.
|
||||
5. **Update `KOReaderHandler`** — accept `ProgressService`, use in `updateProgressForBook`. Fix bugs.
|
||||
6. **Update `KoboHandler`** — accept `ProgressService`, use in Markup/AnalyticsGettests/SyncFromServer.
|
||||
7. **Update queue processor** — `syncProgress` calls `ProgressService.SaveProgress`
|
||||
8. **Update `reader.ts`** — send richer payload
|
||||
9. **Move routes** — add history route to media.go, remove progress.go
|
||||
10. **Delete dead code** — `UpdateReadingProgress` query, `calculateFileSHA256`, etc.
|
||||
11. **Run all tests**
|
||||
12. **Rebuild frontend** (`npm run build:ts`)
|
||||
13. **Rebuild app** (`make rebuild-app`)
|
||||
14. **Manual test**: web reader → KOReader sync → Kobo sync → back to web reader
|
||||
|
||||
## Dependency Injection Changes
|
||||
|
||||
### Current (cmd/server/main.go)
|
||||
```
|
||||
connManager = sync.NewConnectionManager()
|
||||
queueProcessor = sync.NewSyncQueueProcessor(queries)
|
||||
mediaHandler = handlers.NewMediaHandler(queries, libraryService, worker)
|
||||
koreaderHandler = handlers.NewKOReaderHandler(queries, connManager, queueProcessor)
|
||||
koboHandler = handlers.NewKoboHandler(queries, connManager)
|
||||
```
|
||||
|
||||
### After
|
||||
```
|
||||
connManager = sync.NewConnectionManager()
|
||||
progressService = sync.NewProgressService(queries, connManager)
|
||||
queueProcessor = sync.NewSyncQueueProcessor(queries, progressService)
|
||||
mediaHandler = handlers.NewMediaHandler(queries, libraryService, worker, progressService)
|
||||
koreaderHandler = handlers.NewKOReaderHandler(queries, connManager, queueProcessor, progressService)
|
||||
koboHandler = handlers.NewKoboHandler(queries, connManager, progressService)
|
||||
```
|
||||
|
||||
## Tests to Write/Update
|
||||
|
||||
1. **`internal/sync/progress_test.go`** — add tests for:
|
||||
- `SaveProgress` with no existing data (fresh insert)
|
||||
- `SaveProgress` merge: KOReader data preserved when web saves
|
||||
- `SaveProgress` merge: web data preserved when KOReader saves
|
||||
- `SaveProgress` enrichment: character_offset computed from percentage
|
||||
- `SaveProgress` enrichment: current_page computed from percentage + total_pages
|
||||
- `SaveProgress` conflict detection: triggered when different source within 5 min
|
||||
- `SaveProgress` conflict detection: NOT triggered for same source
|
||||
- `SaveProgress` conflict detection: NOT triggered after 5 min window
|
||||
2. **Run existing tests** — `ConvertProgress`, `MergeProgress`, `PageToPercentage`, etc. must still pass
|
||||
|
||||
## Functionality That Must Not Be Touched
|
||||
|
||||
- KOReader bookmark/highlight/note sync (`SyncBookmarks`) — annotation creation, not progress
|
||||
- Kobo bookmark creation in `Bookmark` handler — annotation creation
|
||||
- Kobo library initialization — read-only
|
||||
- Kobo ContentId mapping helpers — book resolution
|
||||
- KOReader book resolution logic — book resolution
|
||||
- KOReader metadata/library retrieval — read-only
|
||||
- WebSocket connection management — infrastructure
|
||||
- Offline detection — infrastructure (not started anyway)
|
||||
- Scanner/worker functionality on `Handler` struct
|
||||
- Frontend progress overview page
|
||||
- Frontend book detail page progress display
|
||||
- All annotation-related queries and handlers
|
||||
|
||||
## Context: The Bigger Picture
|
||||
|
||||
This migration is the foundation for the "universal sync engine" — the core purpose of the Bookhoard project. The goal is seamless reading progress sync across all devices:
|
||||
- Web reader (foliate-js based)
|
||||
- KOReader (crengine based, running on Kindle/Kobo/Android/desktop)
|
||||
- Kobo (stock firmware)
|
||||
|
||||
The `ProgressService` is designed to eventually support:
|
||||
- Server-side CFI generation from crengine data (EPUB parser needed)
|
||||
- Server-side crengine XPointer generation from CFI (EPUB parser needed)
|
||||
- Bidirectional exact position sync between any two clients
|
||||
|
||||
Current `percentage` is the universal fallback. CFI is exact for EPUB. Character offset bridges the gap for crengine. The `ProgressService` enrichment step is where future CFI generation will be added.
|
||||
|
||||
## Database Schema (unchanged)
|
||||
|
||||
The `reading_progress` table already has all needed fields. No schema changes required.
|
||||
|
||||
Key columns:
|
||||
- `percentage` FLOAT (0.0-1.0) — universal progress
|
||||
- `epubcfi` TEXT — exact EPUB position
|
||||
- `character_offset` BIGINT — crengine position
|
||||
- `chapter` INTEGER — chapter number
|
||||
- `chapter_progress` FLOAT — within-chapter progress
|
||||
- `current_page` / `total_pages` INTEGER — page display
|
||||
- `viewport_x/y`, `zoom_level`, `scroll_position_x/y` — fixed-layout state
|
||||
- `panel_number` — comic panel
|
||||
- `reading_mode` — reading mode identifier
|
||||
- `last_sync_device` / `last_sync_source` — sync metadata
|
||||
- `last_sync_timestamp` — for conflict detection
|
||||
- `conflict_detected` / `conflict_resolved` — conflict flags
|
||||
Reference in New Issue
Block a user