docs: add testing guide and progress routes analysis documentation
This commit is contained in:
@@ -0,0 +1,423 @@
|
||||
# Progress Routes Analysis & Thoughts
|
||||
|
||||
## Overview
|
||||
|
||||
This document explores the current state of progress tracking in Bookmann, the migration from legacy media-item-specific routes to universal cross-device progress, and considerations for the future.
|
||||
|
||||
---
|
||||
|
||||
## Current State
|
||||
|
||||
### Legacy Routes (Marked as Deprecated)
|
||||
|
||||
Located in `internal/handlers/ebook.go:114-117`:
|
||||
|
||||
```go
|
||||
// Legacy progress routes (deprecated - use universal progress instead)
|
||||
g.GET("/api/media-items/:id/progress", h.GetMediaReadingProgress)
|
||||
g.PUT("/api/media-items/:id/progress", h.UpdateMediaReadingProgress)
|
||||
g.DELETE("/api/media-items/:id/progress", h.DeleteMediaReadingProgress)
|
||||
```
|
||||
|
||||
**Purpose**: These routes handle progress tracking for a specific media item from the `media_items` table.
|
||||
|
||||
**Data Source**: Likely queries the `reading_progress` table filtered by `media_item_id`.
|
||||
|
||||
**Current Status**: Explicitly marked as "legacy" and "deprecated" in code comments.
|
||||
|
||||
---
|
||||
|
||||
### Universal Progress Routes (Phase 1 Implementation)
|
||||
|
||||
Located in `internal/handlers/ebook.go:119-122`:
|
||||
|
||||
```go
|
||||
// Universal Progress routes (Phase 1)
|
||||
g.GET("/api/progress/:id", h.GetUniversalProgress)
|
||||
g.POST("/api/progress/:id", h.UpdateUniversalProgress)
|
||||
g.GET("/api/progress/:id/history", h.GetProgressHistory)
|
||||
```
|
||||
|
||||
**Purpose**: These routes provide "universal" progress tracking that works across devices and media types.
|
||||
|
||||
**Data Source**: Uses enhanced `reading_progress` table with additional fields:
|
||||
- `percentage` - Universal percentage (0-1)
|
||||
- `character_offset` - Character-based positioning
|
||||
- `epubcfi` - EPUB Canonical Fragment Identifier
|
||||
- `chapter` + `chapter_progress` - Chapter-based tracking
|
||||
- Viewport coordinates (viewport_x, viewport_y, zoom_level)
|
||||
- Scroll positions (scroll_position_x, scroll_position_y)
|
||||
- Panel number for comics/manga
|
||||
- Reading mode indicator
|
||||
|
||||
**Device Sync Metadata**:
|
||||
- `last_sync_device` - Which device last updated
|
||||
- `last_sync_source` - Source type (koreader, kobo, web, etc.)
|
||||
- `last_sync_timestamp` - When sync occurred
|
||||
- `conflict_detected` - Boolean flag for conflicts
|
||||
- `conflict_resolved` - Boolean flag for resolution status
|
||||
|
||||
---
|
||||
|
||||
## Why the Migration Happened
|
||||
|
||||
### 1. **Cross-Platform Kindle Ecosystem Vision**
|
||||
|
||||
Bookmann aims to replace the Kindle ecosystem, which requires:
|
||||
- Syncing progress across multiple devices (Kindle, Kobo, phone, web)
|
||||
- Handling different progress formats (page numbers, percentages, CFI, character offsets)
|
||||
- Maintaining reading state across different device types
|
||||
- Supporting offline reading with sync queues
|
||||
|
||||
### 2. **Format Diversity**
|
||||
|
||||
Different e-readers and formats use different progress indicators:
|
||||
|
||||
| Format/Device | Progress Type | Example |
|
||||
|---------------|---------------|---------|
|
||||
| EPUB (KOReader) | EPUBCFI | `epubcfi(/6/4[chap1ref]!/4/2/1:0)` |
|
||||
| EPUB (Kobo) | Page # + Total | `page 234 of 456` |
|
||||
| PDF | Page # | `page 45` |
|
||||
| Web Reader | Percentage | `0.45 (45%)` |
|
||||
| TXT/Mobi | Character Offset | `offset 12345` |
|
||||
| Comics/Manga | Panel # | `panel 7` |
|
||||
| Kindle | Location # | `location 1234` |
|
||||
|
||||
The legacy `media-items/:id/progress` routes couldn't handle this diversity.
|
||||
|
||||
### 3. **Device Sync Architecture**
|
||||
|
||||
Universal progress enables:
|
||||
- Real-time sync via WebSocket (`/ws/sync`)
|
||||
- Offline queue support (`/api/queue/*`)
|
||||
- Conflict detection and resolution
|
||||
- Checkpoint mode for battery optimization
|
||||
- Progress history tracking
|
||||
|
||||
---
|
||||
|
||||
## Current Database Schema
|
||||
|
||||
From `database/schema/schema.sql:130-158`:
|
||||
|
||||
```sql
|
||||
CREATE TABLE reading_progress (
|
||||
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||
media_item_id UUID NOT NULL REFERENCES media_items(id) ON DELETE CASCADE,
|
||||
user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE,
|
||||
|
||||
-- Legacy fields
|
||||
current_page INTEGER DEFAULT 0,
|
||||
total_pages INTEGER,
|
||||
last_read_at TIMESTAMP WITH TIME ZONE DEFAULT NOW(),
|
||||
|
||||
-- Universal Progress Tracking (Phase 1)
|
||||
percentage FLOAT CHECK (percentage >= 0 AND percentage <= 1),
|
||||
character_offset BIGINT,
|
||||
epubcfi TEXT,
|
||||
chapter INTEGER,
|
||||
chapter_progress FLOAT CHECK (chapter_progress >= 0 AND chapter_progress <= 1),
|
||||
viewport_x FLOAT DEFAULT 0,
|
||||
viewport_y FLOAT DEFAULT 0,
|
||||
zoom_level FLOAT DEFAULT 1.0,
|
||||
scroll_position_x FLOAT DEFAULT 0,
|
||||
scroll_position_y FLOAT DEFAULT 0,
|
||||
panel_number INTEGER,
|
||||
reading_mode VARCHAR(20),
|
||||
|
||||
-- Device Sync Metadata (Phase 1)
|
||||
last_sync_device VARCHAR(50),
|
||||
last_sync_source VARCHAR(20),
|
||||
last_sync_timestamp TIMESTAMP WITH TIME ZONE,
|
||||
conflict_detected BOOLEAN DEFAULT FALSE,
|
||||
conflict_resolved BOOLEAN DEFAULT TRUE,
|
||||
|
||||
UNIQUE(media_item_id, user_id)
|
||||
);
|
||||
```
|
||||
|
||||
**Backward Compatibility View** (Line 160-168):
|
||||
|
||||
```sql
|
||||
CREATE VIEW ebook_reading_progress AS
|
||||
SELECT rp.*,
|
||||
mi.id as ebook_id -- Map media_item_id to ebook_id for compatibility
|
||||
FROM reading_progress rp
|
||||
JOIN media_items mi ON rp.media_item_id = mi.id
|
||||
JOIN libraries l ON mi.library_id = l.id
|
||||
JOIN library_types lt ON l.library_type_id = lt.id
|
||||
WHERE lt.name = 'ebooks';
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## The Migration Challenge
|
||||
|
||||
### Issue: Two Parallel Systems
|
||||
|
||||
Currently, **both** systems exist side-by-side:
|
||||
|
||||
1. **Legacy routes** (`/api/media-items/:id/progress`)
|
||||
- Likely use simple `current_page` / `total_pages` fields
|
||||
- Media-item scoped
|
||||
- No device sync metadata
|
||||
|
||||
2. **Universal routes** (`/api/progress/:id`)
|
||||
- Use rich progress tracking with multiple formats
|
||||
- Device-aware
|
||||
- Include sync metadata
|
||||
|
||||
### Question: What Does `:id` Mean?
|
||||
|
||||
**Legacy**: `:id` = `media_item_id` (UUID of the book)
|
||||
|
||||
**Universal**: `:id` = ??? (Could be same media_item_id, or could be a different identifier)
|
||||
|
||||
**Ambiguity**: The routes use the same parameter name but might mean different things.
|
||||
|
||||
### Potential Problems
|
||||
|
||||
1. **Data Duplication**: If both systems write to `reading_progress` table, they might overwrite each other
|
||||
2. **Client Confusion**: Which endpoint should clients use?
|
||||
3. **Migration Path**: How do existing clients using legacy endpoints transition?
|
||||
4. **API Consistency**: Having two different endpoints for similar functionality is confusing
|
||||
|
||||
---
|
||||
|
||||
## Observations & Concerns
|
||||
|
||||
### 1. **Incomplete Migration**
|
||||
|
||||
The legacy routes are marked as deprecated but **still active**. This suggests:
|
||||
- Migration is ongoing, not complete
|
||||
- Some clients might still depend on legacy routes
|
||||
- Fear of breaking existing integrations
|
||||
|
||||
### 2. **Backward Compatibility View**
|
||||
|
||||
The `ebook_reading_progress` view exists to maintain compatibility with the old `ebooks` table. This adds:
|
||||
- Query overhead (JOINs to filter by library type)
|
||||
- Developer confusion (which table/view to query?)
|
||||
- Technical debt (maintaining two ways to access data)
|
||||
|
||||
### 3. **Route Naming Inconsistency**
|
||||
|
||||
- Legacy: `/api/media-items/:id/progress` (RESTful, nested under media-item)
|
||||
- Universal: `/api/progress/:id` (flat structure, not nested)
|
||||
|
||||
**Question**: Should universal progress be under `/api/media-items/:id/universal-progress` for consistency?
|
||||
|
||||
### 4. **HTTP Method Mismatch**
|
||||
|
||||
Legacy routes use:
|
||||
- `PUT /api/media-items/:id/progress` (update progress)
|
||||
|
||||
Universal routes use:
|
||||
- `POST /api/progress/:id` (update progress)
|
||||
|
||||
**REST convention**: `PUT` is idempotent, `POST` is not. For progress updates, `PUT` might be more appropriate since setting the same progress twice should have the same effect.
|
||||
|
||||
### 5. **Missing Delete Operation**
|
||||
|
||||
Universal routes don't have a `DELETE /api/progress/:id` endpoint. Legacy does:
|
||||
- `DELETE /api/media-items/:id/progress` (clear progress)
|
||||
|
||||
**Question**: Should there be a way to reset progress via universal routes?
|
||||
|
||||
---
|
||||
|
||||
## Potential Future Directions
|
||||
|
||||
### Option 1: Full Migration (Clean Break)
|
||||
|
||||
**Action**: Remove all legacy routes and views.
|
||||
|
||||
**Steps**:
|
||||
1. Deprecate legacy routes in API documentation (return `Warning` header)
|
||||
2. Add a 6-month migration timeline
|
||||
3. Remove `/api/media-items/:id/progress` routes
|
||||
4. Drop `ebook_reading_progress` view
|
||||
5. Update all clients to use universal routes
|
||||
|
||||
**Pros**:
|
||||
- Cleaner API surface
|
||||
- Single source of truth
|
||||
- Less maintenance burden
|
||||
- Clearer documentation
|
||||
|
||||
**Cons**:
|
||||
- Breaking change for existing clients
|
||||
- Mobile apps might need updates
|
||||
- External integrations could break
|
||||
|
||||
### Option 2: Compatibility Layer (Adapter Pattern)
|
||||
|
||||
**Action**: Keep legacy routes but make them thin wrappers around universal routes.
|
||||
|
||||
**Implementation**:
|
||||
```go
|
||||
// Legacy route calls universal route internally
|
||||
func (h *Handler) GetMediaReadingProgress(c echo.Context) error {
|
||||
mediaItemID := c.Param("id")
|
||||
// Extract user_id from JWT
|
||||
// Call h.GetUniversalProgress with same IDs
|
||||
// Transform response if needed
|
||||
}
|
||||
```
|
||||
|
||||
**Pros**:
|
||||
- No breaking changes
|
||||
- Gradual migration path
|
||||
- Single implementation (universal routes)
|
||||
|
||||
**Cons**:
|
||||
- Maintains API surface area
|
||||
- Slight performance overhead (function call)
|
||||
- Still confusing to have two endpoints
|
||||
|
||||
### Option 3: Unified Endpoint (Best of Both)
|
||||
|
||||
**Action**: Create a single endpoint that handles both use cases.
|
||||
|
||||
**Proposed**:
|
||||
```
|
||||
GET /api/media-items/:id/progress?format=universal
|
||||
PUT /api/media-items/:id/progress?format=universal
|
||||
DELETE /api/media-items/:id/progress
|
||||
```
|
||||
|
||||
The `format` query parameter determines:
|
||||
- `format=simple` (default): Returns basic page/percentage (legacy behavior)
|
||||
- `format=universal`: Returns full device-aware progress with metadata
|
||||
|
||||
**Pros**:
|
||||
- Single endpoint
|
||||
- Backward compatible
|
||||
- Clear migration path via query parameter
|
||||
- RESTful structure (nested under media-items)
|
||||
|
||||
**Cons**:
|
||||
- More complex handler logic
|
||||
- Need to maintain both formats in response
|
||||
|
||||
### Option 4: Versioned API (Cleanest Long-Term)
|
||||
|
||||
**Action**: Use API versioning to separate old and new.
|
||||
|
||||
**Proposed**:
|
||||
```
|
||||
# v1 (Legacy)
|
||||
GET /api/v1/media-items/:id/progress
|
||||
PUT /api/v1/media-items/:id/progress
|
||||
DELETE /api/v1/media-items/:id/progress
|
||||
|
||||
# v2 (Universal)
|
||||
GET /api/v2/media-items/:id/progress
|
||||
PUT /api/v2/media-items/:id/progress
|
||||
GET /api/v2/media-items/:id/progress/history
|
||||
```
|
||||
|
||||
**Pros**:
|
||||
- Clean separation
|
||||
- Can deprecate v1 independently
|
||||
- Standard industry practice
|
||||
- Clear migration documentation
|
||||
|
||||
**Cons**:
|
||||
- Need to implement version routing
|
||||
- More upfront work
|
||||
- Maintenance of two versions temporarily
|
||||
|
||||
---
|
||||
|
||||
## Unanswered Questions for Discussion
|
||||
|
||||
1. **Are any clients currently using the legacy progress routes?**
|
||||
- If yes, which ones? (mobile app, web app, third-party integrations?)
|
||||
- Can they be updated easily?
|
||||
|
||||
2. **What does the `:id` parameter represent in universal progress routes?**
|
||||
- Is it still `media_item_id`?
|
||||
- Or is it a `reading_progress` record ID?
|
||||
- Need to check implementation to confirm
|
||||
|
||||
3. **Why was `/api/progress/:id` chosen instead of `/api/media-items/:id/universal-progress`?**
|
||||
- Flat structure vs nested structure design decision
|
||||
- Might indicate plans for progress to exist independently of media items?
|
||||
|
||||
4. **Is the legacy route implementation actually different, or just deprecated?**
|
||||
- Need to read the handler implementations to compare
|
||||
- They might be calling the same underlying code
|
||||
|
||||
5. **Should we maintain progress deletion functionality?**
|
||||
- Universal routes don't have DELETE
|
||||
- Is deleting progress a necessary feature?
|
||||
|
||||
6. **What's the timeline for removing legacy routes?**
|
||||
- Already marked deprecated, but when can we delete them?
|
||||
- Need to coordinate with mobile app releases
|
||||
|
||||
7. **How does the backward compatibility view affect performance?**
|
||||
- The `ebook_reading_progress` view requires JOINs
|
||||
- Is it used anywhere, or can it be dropped?
|
||||
|
||||
---
|
||||
|
||||
## Recommendations
|
||||
|
||||
### Immediate Actions (Discussion Phase)
|
||||
|
||||
1. **Audit Current Usage**
|
||||
- Search codebase for references to legacy routes
|
||||
- Check if any external documentation mentions these endpoints
|
||||
- Identify all clients (web, mobile, third-party)
|
||||
|
||||
2. **Compare Implementations**
|
||||
- Read handler code for both legacy and universal routes
|
||||
- Document differences in behavior
|
||||
- Determine if they're truly different or just deprecated wrappers
|
||||
|
||||
3. **Clarify API Contract**
|
||||
- Define what `:id` means in universal routes
|
||||
- Document expected request/response formats
|
||||
- Add examples for different device types
|
||||
|
||||
4. **Performance Analysis**
|
||||
- Query database to see how many records use legacy fields vs universal
|
||||
- Check if backward compatibility view is actually used
|
||||
- Benchmark query performance with/without views
|
||||
|
||||
### Future Considerations
|
||||
|
||||
1. **Choose a Migration Strategy**
|
||||
- Review Options 1-4 above
|
||||
- Consider breaking changes vs compatibility
|
||||
- Plan timeline based on client usage
|
||||
|
||||
2. **API Versioning Decision**
|
||||
- Decide if we want `/api/v1/` and `/api/v2/` structure
|
||||
- Or use different approach (headers, content negotiation)
|
||||
|
||||
3. **Documentation Updates**
|
||||
- Update API_REFERENCE.md with clear deprecation notices
|
||||
- Add migration guide for clients
|
||||
- Document best practices for progress tracking
|
||||
|
||||
4. **Test Coverage**
|
||||
- Ensure both legacy and universal routes have comprehensive tests
|
||||
- Add integration tests for cross-device sync scenarios
|
||||
- Test conflict resolution workflows
|
||||
|
||||
---
|
||||
|
||||
## Next Steps for Discussion
|
||||
|
||||
1. **Review handler implementations** to understand actual differences
|
||||
2. **Check client usage** (web app, mobile apps, Bruno tests)
|
||||
3. **Decide on migration timeline** and breaking change tolerance
|
||||
4. **Choose unified strategy** (Options 1-4 or hybrid)
|
||||
5. **Plan implementation** with backward compatibility in mind
|
||||
|
||||
---
|
||||
|
||||
*Document created for future discussion. No changes to be made without review.*
|
||||
Reference in New Issue
Block a user