From 1f5c5a28bdb3026136f481c663ecdf8123cb171b Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sat, 22 Aug 2026 13:16:43 -0400 Subject: [PATCH] feat(sync): propagate KOReader annotation deletions + history API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KOReader push (processBookAnnotations) accepts deleted_highlights and deleted_bookmarks arrays of dedup keys and tombstones the matching rows, after the upserts so a key present in both lists resolves to 'deleted' (the newer intent). Deletions remain soft: rows stay restorable from the history and echo to other devices as tombstones on their next pull. A stale device replay of the annotation cannot resurrect the tombstone — device pushes carry no modification timestamp, so the save loses to the delete. Absence from these arrays is never a delete, keeping category toggles safe. New annotation-history endpoints (annotation_history.go, media.go): GET /api/media-items/:id/annotations/deleted POST /api/media-items/:id/annotations/:annotationId/restore DELETE /api/media-items/:id/annotations/:annotationId All scoped to the authenticated user and the route's book; the DELETE is the permanent purge (annotation_type required in query or body). MediaDetail gains DeletedAnnotations, populated by the book page route via the shared DeletedAnnotationsForBook builder, so the server-rendered history ships with the page instead of requiring a client round-trip. Binding tests cover the plugin's exact wire shape and the legacy plugin case (arrays omitted -> empty). --- internal/handlers/annotation_history.go | 167 ++++++++++++++++++++++++ internal/handlers/koreader.go | 71 +++++++--- internal/handlers/koreader_test.go | 52 ++++++++ internal/handlers/media_detail.go | 4 + internal/router/frontend.go | 15 ++- internal/router/media.go | 7 + 6 files changed, 291 insertions(+), 25 deletions(-) create mode 100644 internal/handlers/annotation_history.go create mode 100644 internal/handlers/koreader_test.go diff --git a/internal/handlers/annotation_history.go b/internal/handlers/annotation_history.go new file mode 100644 index 0000000..f5274a6 --- /dev/null +++ b/internal/handlers/annotation_history.go @@ -0,0 +1,167 @@ +package handlers + +import ( + "context" + "net/http" + "time" + + "bookhoard/internal/database" + wsync "bookhoard/internal/sync" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgtype" + "github.com/labstack/echo/v5" +) + +// DeletedAnnotationResponse is one entry of the deleted-annotation history +// for a book (the book page's "recently deleted" list). Restoring returns the +// row to the active set; purging removes it permanently. +type DeletedAnnotationResponse struct { + ID string `json:"id"` + AnnotationType string `json:"annotation_type"` + DisplayText string `json:"display_text"` + SecondaryText string `json:"secondary_text,omitempty"` + Color string `json:"color,omitempty"` + DeletedAt time.Time `json:"deleted_at"` + CreatedAt time.Time `json:"created_at"` +} + +// DeletedAnnotationsForBook builds the deleted-annotation history for a user +// and book. Shared by the JSON API and the book page's server-rendered modal. +func DeletedAnnotationsForBook(ctx context.Context, db *database.Queries, userID, mediaItemID pgtype.UUID) []DeletedAnnotationResponse { + rows, err := db.ListDeletedAnnotationsForBook(ctx, database.ListDeletedAnnotationsForBookParams{ + MediaItemID: mediaItemID, + UserID: userID, + }) + if err != nil { + return []DeletedAnnotationResponse{} + } + + response := make([]DeletedAnnotationResponse, 0, len(rows)) + for _, row := range rows { + entry := DeletedAnnotationResponse{ + ID: uuid.UUID(row.ID.Bytes).String(), + AnnotationType: row.AnnotationType, + DisplayText: row.DisplayText, + SecondaryText: row.SecondaryText.String, + Color: row.Color.String, + } + if row.DeletedAt.Valid { + entry.DeletedAt = row.DeletedAt.Time + } + if row.CreatedAt.Valid { + entry.CreatedAt = row.CreatedAt.Time + } + response = append(response, entry) + } + return response +} + +// GetDeletedAnnotations handles GET /api/media-items/:id/annotations/deleted +func (mh *MediaHandler) GetDeletedAnnotations(c *echo.Context) error { + userUUID, mediaUUID, err := mh.parseUserAndMediaIDs(c) + if err != nil { + return err + } + + response := DeletedAnnotationsForBook(c.Request().Context(), mh.db, userUUID, mediaUUID) + + return c.JSON(http.StatusOK, map[string]interface{}{ + "deleted_annotations": response, + "total": len(response), + }) +} + +// RestoreDeletedAnnotation handles POST /api/media-items/:id/annotations/:annotationId/restore +// Body/query: annotation_type=highlight|note|bookmark +func (mh *MediaHandler) RestoreDeletedAnnotation(c *echo.Context) error { + userUUID, mediaUUID, annotationUUID, kind, errResp := mh.parseAnnotationHistoryRequest(c, true) + if errResp != nil { + return errResp + } + + if mh.annotationSvc == nil { + return c.JSON(http.StatusServiceUnavailable, map[string]string{"error": "annotation service unavailable"}) + } + + restored, err := mh.annotationSvc.RestoreAnnotationByID(c.Request().Context(), kind, userUUID, mediaUUID, annotationUUID) + if err != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to restore annotation"}) + } + if !restored { + return c.JSON(http.StatusNotFound, map[string]string{"error": "deleted annotation not found"}) + } + + return c.JSON(http.StatusOK, map[string]interface{}{"restored": true}) +} + +// PurgeDeletedAnnotation handles DELETE /api/media-items/:id/annotations/:annotationId +// Query: annotation_type=highlight|note|bookmark. Permanent — removes the +// tombstoned row from the history. +func (mh *MediaHandler) PurgeDeletedAnnotation(c *echo.Context) error { + userUUID, mediaUUID, annotationUUID, kind, errResp := mh.parseAnnotationHistoryRequest(c, false) + if errResp != nil { + return errResp + } + + if mh.annotationSvc == nil { + return c.JSON(http.StatusServiceUnavailable, map[string]string{"error": "annotation service unavailable"}) + } + + purged, err := mh.annotationSvc.PurgeAnnotationByID(c.Request().Context(), kind, userUUID, mediaUUID, annotationUUID) + if err != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to purge annotation"}) + } + if !purged { + return c.JSON(http.StatusNotFound, map[string]string{"error": "deleted annotation not found"}) + } + + return c.JSON(http.StatusOK, map[string]interface{}{"purged": true}) +} + +// parseUserAndMediaIDs extracts the authenticated user and the media item +// from the route. A non-nil error has already been written as the response. +func (mh *MediaHandler) parseUserAndMediaIDs(c *echo.Context) (pgtype.UUID, pgtype.UUID, error) { + userID := c.Get("user_id").(string) + userUUID, err := uuid.Parse(userID) + if err != nil { + return pgtype.UUID{}, pgtype.UUID{}, c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid user"}) + } + mediaID := c.Param("id") + mediaIDUUID, err := uuid.Parse(mediaID) + if err != nil { + return pgtype.UUID{}, pgtype.UUID{}, c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid media item id"}) + } + return pgtype.UUID{Bytes: userUUID, Valid: true}, pgtype.UUID{Bytes: mediaIDUUID, Valid: true}, nil +} + +// parseAnnotationHistoryRequest extracts user, media item, annotation ID, and +// the annotation_type (from query param or JSON body — restore posts a body, +// purge uses a query param). A non-nil error has already been written. +func (mh *MediaHandler) parseAnnotationHistoryRequest(c *echo.Context, allowBody bool) (pgtype.UUID, pgtype.UUID, pgtype.UUID, string, error) { + userUUID, mediaUUID, err := mh.parseUserAndMediaIDs(c) + if err != nil { + return pgtype.UUID{}, pgtype.UUID{}, pgtype.UUID{}, "", err + } + + annotationID := c.Param("annotationId") + annotationUUID, err := uuid.Parse(annotationID) + if err != nil { + return pgtype.UUID{}, pgtype.UUID{}, pgtype.UUID{}, "", c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid annotation id"}) + } + + kind := c.QueryParam("annotation_type") + if kind == "" && allowBody { + var body struct { + AnnotationType string `json:"annotation_type"` + } + if c.Bind(&body) == nil { + kind = body.AnnotationType + } + } + if !wsync.ValidAnnotationKind(kind) { + return pgtype.UUID{}, pgtype.UUID{}, pgtype.UUID{}, "", c.JSON(http.StatusBadRequest, map[string]string{"error": "annotation_type must be highlight, note, or bookmark"}) + } + + return userUUID, mediaUUID, pgtype.UUID{Bytes: annotationUUID, Valid: true}, kind, nil +} diff --git a/internal/handlers/koreader.go b/internal/handlers/koreader.go index d2a2826..2ea2a2c 100644 --- a/internal/handlers/koreader.go +++ b/internal/handlers/koreader.go @@ -158,24 +158,35 @@ type KOReaderProgressRequest struct { } type KOReaderBookProgress struct { - UUID string `json:"uuid,omitempty"` - SHA256 string `json:"sha256,omitempty"` - Title string `json:"title,omitempty"` - Authors []string `json:"authors,omitempty"` - Progress float64 `json:"progress"` - Percentage float64 `json:"percentage"` - LastRead string `json:"last_read,omitempty"` - FilePath string `json:"file_path,omitempty"` - DeviceInfo KOReaderDeviceInfo `json:"device_info,omitempty"` - Bookmarks []KOReaderBookmark `json:"bookmarks,omitempty"` - Highlights []KOReaderHighlight `json:"highlights,omitempty"` - Notes []KOReaderNote `json:"notes,omitempty"` - Chapter *int `json:"chapter,omitempty"` - Character *int64 `json:"character,omitempty"` - Epubcfi *string `json:"epubcfi,omitempty"` - ContextText *string `json:"context_text,omitempty"` - Page *int `json:"page,omitempty"` - TotalPages *int `json:"total_pages,omitempty"` + UUID string `json:"uuid,omitempty"` + SHA256 string `json:"sha256,omitempty"` + Title string `json:"title,omitempty"` + Authors []string `json:"authors,omitempty"` + Progress float64 `json:"progress"` + Percentage float64 `json:"percentage"` + LastRead string `json:"last_read,omitempty"` + FilePath string `json:"file_path,omitempty"` + DeviceInfo KOReaderDeviceInfo `json:"device_info,omitempty"` + Bookmarks []KOReaderBookmark `json:"bookmarks,omitempty"` + Highlights []KOReaderHighlight `json:"highlights,omitempty"` + Notes []KOReaderNote `json:"notes,omitempty"` + Chapter *int `json:"chapter,omitempty"` + Character *int64 `json:"character,omitempty"` + Epubcfi *string `json:"epubcfi,omitempty"` + ContextText *string `json:"context_text,omitempty"` + Page *int `json:"page,omitempty"` + TotalPages *int `json:"total_pages,omitempty"` + // Device-side deletions, reported by dedup key. Keys refer to annotations + // the device previously received from the server (or echoes of its own + // pushes); the device only flags a deletion after observing the key in a + // pull, so absence from these arrays is never interpreted as deletion. + DeletedHighlights []KOReaderDeletedAnnotation `json:"deleted_highlights,omitempty"` + DeletedBookmarks []KOReaderDeletedAnnotation `json:"deleted_bookmarks,omitempty"` +} + +// KOReaderDeletedAnnotation identifies a deleted annotation by dedup key. +type KOReaderDeletedAnnotation struct { + DedupKey string `json:"dedup_key"` } type KOReaderDeviceInfo struct { @@ -734,6 +745,30 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID, DedupKey: dedupKey, }) } + + // Device-reported deletions: tombstone by dedup key. Tombstoned rows stay + // in the history (restorable from the book page) and are echoed to other + // devices as tombstones on their next pull. A device replay that pushes a + // stale copy of the annotation cannot resurrect the tombstone (its save + // carries no modification timestamp newer than the delete). Deletions run + // after the upserts purely so a key present in both lists resolves to + // "deleted" — the newer intent. + for _, del := range book.DeletedHighlights { + if del.DedupKey == "" { + continue + } + if err := h.annotationSvc.TombstoneHighlight(ctx, userID, mediaItemID, del.DedupKey, "koreader"); err != nil { + log.Printf("KOReader: tombstone highlight by dedup key failed: %v", err) + } + } + for _, del := range book.DeletedBookmarks { + if del.DedupKey == "" { + continue + } + if err := h.annotationSvc.TombstoneBookmarkByDedupKey(ctx, userID, mediaItemID, del.DedupKey, "koreader"); err != nil { + log.Printf("KOReader: tombstone bookmark by dedup key failed: %v", err) + } + } } func (h *KOReaderHandler) updateProgressForBook(c *echo.Context, deviceID pgtype.UUID, userID pgtype.UUID, mediaItemID pgtype.UUID, book KOReaderBookProgress) error { diff --git a/internal/handlers/koreader_test.go b/internal/handlers/koreader_test.go new file mode 100644 index 0000000..ee5cd86 --- /dev/null +++ b/internal/handlers/koreader_test.go @@ -0,0 +1,52 @@ +package handlers + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +// The device pushes deletions as dedup-key arrays on the progress request. +// Verify the wire shape the plugin sends (lua json.encode of +// { deleted_highlights = { { dedup_key = "..." } } }) binds correctly. +func TestKOReaderProgressRequest_DeletedAnnotationsBinding(t *testing.T) { + payload := `{ + "books": [{ + "sha256": "d1b1c6123d6206017b40798744ed994f00803b97d22ce51bea32e95e1ce7a164", + "title": "1984", + "percentage": 0.42, + "deleted_highlights": [ + { "dedup_key": "abc123" }, + { "dedup_key": "def456" } + ], + "deleted_bookmarks": [ + { "dedup_key": "789xyz" } + ] + }] + }` + + var req KOReaderProgressRequest + err := json.Unmarshal([]byte(payload), &req) + assert.NoError(t, err) + assert.Len(t, req.Books, 1) + + book := req.Books[0] + assert.Len(t, book.DeletedHighlights, 2) + assert.Equal(t, "abc123", book.DeletedHighlights[0].DedupKey) + assert.Equal(t, "def456", book.DeletedHighlights[1].DedupKey) + assert.Len(t, book.DeletedBookmarks, 1) + assert.Equal(t, "789xyz", book.DeletedBookmarks[0].DedupKey) +} + +// A request without the arrays (older plugins) must bind with them empty — +// deletion propagation is strictly opt-in per push. +func TestKOReaderProgressRequest_DeletedAnnotationsOmitted(t *testing.T) { + payload := `{"books": [{"sha256": "x", "title": "t", "percentage": 0.1}]}` + + var req KOReaderProgressRequest + err := json.Unmarshal([]byte(payload), &req) + assert.NoError(t, err) + assert.Empty(t, req.Books[0].DeletedHighlights) + assert.Empty(t, req.Books[0].DeletedBookmarks) +} diff --git a/internal/handlers/media_detail.go b/internal/handlers/media_detail.go index c9f472e..cc7e0ed 100644 --- a/internal/handlers/media_detail.go +++ b/internal/handlers/media_detail.go @@ -18,4 +18,8 @@ type MediaDetail struct { // Computed counts NotesCount int `json:"notes_count"` HighlightsCount int `json:"highlights_count"` + + // Deleted-annotation history (tombstoned rows, newest first) — the book + // page's "recently deleted" list with restore/permanent-delete actions. + DeletedAnnotations []DeletedAnnotationResponse `json:"deleted_annotations"` } diff --git a/internal/router/frontend.go b/internal/router/frontend.go index 556ef69..cdae66c 100644 --- a/internal/router/frontend.go +++ b/internal/router/frontend.go @@ -1329,13 +1329,14 @@ func registerFrontendRoutes(cfg *Config) { // Assemble response (no field duplication!) detail := handlers.MediaDetail{ - MediaItems: mediaItem, // Embedded - ALL fields available - Rating: rating, - Collections: collections, - ReadingProgress: progress, - ActiveConflict: activeConflict, - NotesCount: len(notes), - HighlightsCount: len(highlights), + MediaItems: mediaItem, // Embedded - ALL fields available + Rating: rating, + Collections: collections, + ReadingProgress: progress, + ActiveConflict: activeConflict, + NotesCount: len(notes), + HighlightsCount: len(highlights), + DeletedAnnotations: handlers.DeletedAnnotationsForBook(c.Request().Context(), cfg.Queries, pgUserID, pgMediaUUID), } // Render template diff --git a/internal/router/media.go b/internal/router/media.go index bbaf03c..e71e2a6 100644 --- a/internal/router/media.go +++ b/internal/router/media.go @@ -47,6 +47,13 @@ func registerMediaRoutes(cfg *Config) { protected.PUT("/media-items/:id/bookmarks/:bookmarkId", cfg.MediaHandler.UpdateMediaBookmark) protected.DELETE("/media-items/:id/bookmarks/:bookmarkId", cfg.MediaHandler.DeleteMediaBookmark) + // Deleted-annotation history (all authenticated users): tombstoned + // highlights/notes/bookmarks restorable or permanently removable from the + // book page's "recently deleted" list. + protected.GET("/media-items/:id/annotations/deleted", cfg.MediaHandler.GetDeletedAnnotations) + protected.POST("/media-items/:id/annotations/:annotationId/restore", cfg.MediaHandler.RestoreDeletedAnnotation) + protected.DELETE("/media-items/:id/annotations/:annotationId", cfg.MediaHandler.PurgeDeletedAnnotation) + // Admin-only media routes admin.POST("/media-items", cfg.MediaHandler.CreateMediaItem) admin.PUT("/media-items/:id", cfg.MediaHandler.UpdateMediaItem)