diff --git a/cmd/server/tests/contract_pins_test.go b/cmd/server/tests/contract_pins_test.go index 4eefbead..be95e42d 100644 --- a/cmd/server/tests/contract_pins_test.go +++ b/cmd/server/tests/contract_pins_test.go @@ -1310,3 +1310,81 @@ func TestH3_BookmarkPUTPreservesLocators(t *testing.T) { require.Equal(t, "web", getStringField(t, noteRow, "last_modified_source"), "the note PUT must maintain source bookkeeping via the sync service") } + +// TestOriginWriterAttribution locks the client-origin labeling: every +// web-API write endpoint accepts an optional `origin` key and records it +// as the writer (last_modified_source / last_sync_source) when it names a +// known first-party client ("android", "ios"). Absent or unknown values +// stay "web", and a paired X-Device-Token still outranks origin on the +// progress push. The web SPA sends no origin — its rows are unchanged. +func TestOriginWriterAttribution(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + setup := setupTestServer(t) + fx := createContractFixture(t, setup) + + t.Run("progress PUT honors origin, defaults to web", func(t *testing.T) { + url := progressURL(setup.Server.URL, fx.itemID) + resp := doReq(t, "PUT", url, map[string]interface{}{ + "percentage": 0.3, "origin": "android", + }, setup.RegularToken) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + putRow := decodeJSON(t, resp) + require.Equal(t, "android", getStringField(t, putRow, "last_sync_source")) + + resp2 := doReq(t, "PUT", url, map[string]interface{}{ + "percentage": 0.31, + }, setup.RegularToken) + defer resp2.Body.Close() + require.Equal(t, http.StatusOK, resp2.StatusCode) + require.Equal(t, "web", getStringField(t, decodeJSON(t, resp2), "last_sync_source")) + + // Unknown labels normalize to web (the label keys conflict claims + // and device_sync_data — free-form client strings would pollute). + resp3 := doReq(t, "PUT", url, map[string]interface{}{ + "percentage": 0.32, "origin": "fridge", + }, setup.RegularToken) + defer resp3.Body.Close() + require.Equal(t, http.StatusOK, resp3.StatusCode) + require.Equal(t, "web", getStringField(t, decodeJSON(t, resp3), "last_sync_source")) + }) + + t.Run("bookmark create and PUT honor origin", func(t *testing.T) { + url := annotationURL(setup.Server.URL, fx.itemID, "bookmarks", "") + resp := doReq(t, "POST", url, map[string]interface{}{ + "title": "origin bookmark", "position": "p1", "origin": "android", + }, setup.RegularToken) + defer resp.Body.Close() + require.Equal(t, http.StatusCreated, resp.StatusCode) + row := decodeJSON(t, resp) + id, _ := row["id"].(string) + require.NotEmpty(t, id) + require.Equal(t, "android", getStringField(t, row, "last_modified_source")) + require.Equal(t, "android", getStringField(t, row, "origin_source")) + + put := doReq(t, "PUT", annotationURL(setup.Server.URL, fx.itemID, "bookmarks", id), map[string]interface{}{ + "title": "origin bookmark renamed", "origin": "android", + }, setup.RegularToken) + defer put.Body.Close() + require.Equal(t, http.StatusOK, put.StatusCode) + require.Equal(t, "android", getStringField(t, decodeJSON(t, put), "last_modified_source")) + }) + + t.Run("note and highlight creates honor origin", func(t *testing.T) { + noteResp := doReq(t, "POST", annotationURL(setup.Server.URL, fx.itemID, "notes", ""), map[string]interface{}{ + "content": "origin note", "position": "p2", "origin": "android", + }, setup.RegularToken) + defer noteResp.Body.Close() + require.Equal(t, http.StatusCreated, noteResp.StatusCode) + require.Equal(t, "android", getStringField(t, decodeJSON(t, noteResp), "last_modified_source")) + + hlResp := doReq(t, "POST", annotationURL(setup.Server.URL, fx.itemID, "highlights", ""), map[string]interface{}{ + "selection_text": "origin highlight", "origin": "ios", + }, setup.RegularToken) + defer hlResp.Body.Close() + require.Equal(t, http.StatusCreated, hlResp.StatusCode) + require.Equal(t, "ios", getStringField(t, decodeJSON(t, hlResp), "last_modified_source")) + }) +} diff --git a/docs/contract_pins.md b/docs/contract_pins.md index 91b42199..727d8a82 100644 --- a/docs/contract_pins.md +++ b/docs/contract_pins.md @@ -92,3 +92,13 @@ cover_image_path, has_conflict, format_group, badge`. search row gained the two columns it lagged (EXPLAIN plan-identical — both ride the already-scanned media_items side), and the endpoint serializes the row directly, so they emit. No key removed or renamed. +- **Origin writer attribution (post-hardening parity)** — every web-API + write endpoint now parses an ADDITIVE optional request key `origin` + ("android" | "ios"; anything else stays "web") and records it as the + writer: progress PUT → `last_sync_source`, bookmark/note/highlight + create+PUT → `last_modified_source` (and `origin_source` on bookmark + creates, now normalized through the same gate). Precedence on progress: + a paired X-Device-Token still outranks origin. The web SPA sends no + origin — its rows are unchanged. Request-parse widening documented per + rule 2; response shapes untouched; locked by + TestOriginWriterAttribution. diff --git a/internal/handlers/media.go b/internal/handlers/media.go index a37a04a8..189fb7e2 100644 --- a/internal/handlers/media.go +++ b/internal/handlers/media.go @@ -107,16 +107,35 @@ type UpdateMediaItemRequest struct { Abridged bool `form:"abridged" json:"abridged"` } +// originSource normalizes a client-supplied origin label into the writer +// Source ("android"/"ios"); anything absent or unknown stays "web". The +// web API serves several first-party clients, and the label is what +// last_modified_source / last_sync_source record — a client that +// identifies itself gets honest attribution. Keep the set tight: the +// label also keys device_sync_data and progress-conflict claims. +func originSource(origin string) string { + switch origin { + case "android", "ios": + return origin + } + return "web" +} + // CreateMediaNoteRequest represents the request for creating a media note type CreateMediaNoteRequest struct { Content string `json:"content" validate:"required,min=1,max=100000"` Position string `json:"position"` + // Optional client label recorded as the writer (see originSource). + Origin string `json:"origin"` } // UpdateMediaNoteRequest represents the request for updating a media note type UpdateMediaNoteRequest struct { Content string `json:"content" validate:"required,min=1,max=100000"` Position string `json:"position"` + // Optional client label ("android", "ios") recorded as the writer in + // last_modified_source; absent/unknown stays "web". + Origin string `json:"origin"` } // CreateMediaHighlightRequest represents the request for creating a media highlight @@ -132,6 +151,8 @@ type CreateMediaHighlightRequest struct { PercentageStart float64 `json:"percentage_start"` PercentageEnd float64 `json:"percentage_end"` ChapterReference int32 `json:"chapter_reference"` + // Optional client label recorded as the writer (see originSource). + Origin string `json:"origin"` } // UpdateMediaHighlightRequest represents the request for updating a media highlight @@ -147,6 +168,8 @@ type UpdateMediaHighlightRequest struct { PercentageStart float64 `json:"percentage_start"` PercentageEnd float64 `json:"percentage_end"` ChapterReference int32 `json:"chapter_reference"` + // Optional client label recorded as the writer (see originSource). + Origin string `json:"origin"` } // CreateMediaBookmarkRequest represents the request for creating a media bookmark @@ -177,6 +200,9 @@ type UpdateMediaBookmarkRequest struct { ChapterNumber int32 `json:"chapter_number"` Percentage float64 `json:"percentage"` ChapterReference int32 `json:"chapter_reference"` + // Optional client label recorded as the writer in + // last_modified_source (see originSource). + Origin string `json:"origin"` } type MediaHandler struct { @@ -1270,6 +1296,11 @@ func (mh *MediaHandler) UpdateMediaReadingProgress(c *echo.Context) error { req.MediaItemID = pgtype.UUID{Bytes: mediaUUID, Valid: true} req.UserID = pgtype.UUID{Bytes: userUUID, Valid: true} req.Source = deviceSource + if deviceSource == "web" { + // No paired device claimed this push: an explicit client origin + // ("android"/"ios") is the next-best attribution. + req.Source = originSource(req.Origin) + } req.DeviceID = deviceID req.DeviceType = deviceType req.DeviceName = deviceLabel @@ -1829,7 +1860,7 @@ func (mh *MediaHandler) CreateMediaNote(c *echo.Context) error { UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, Content: req.Content, Position: req.Position, - Source: "web", + Source: originSource(req.Origin), ModifiedAt: time.Now(), }) if err != nil { @@ -1895,7 +1926,7 @@ func (mh *MediaHandler) UpdateMediaNote(c *echo.Context) error { UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, Content: req.Content, Position: req.Position, - Source: "web", + Source: originSource(req.Origin), ModifiedAt: time.Now(), NoteID: pgtype.UUID{Bytes: noteUUID, Valid: true}, }) @@ -1995,7 +2026,7 @@ func (mh *MediaHandler) CreateMediaHighlight(c *echo.Context) error { PercentageStart: req.PercentageStart, PercentageEnd: req.PercentageEnd, ChapterReference: req.ChapterReference, - Source: "web", + Source: originSource(req.Origin), ModifiedAt: time.Now(), }) if err != nil { @@ -2067,7 +2098,7 @@ func (mh *MediaHandler) UpdateMediaHighlight(c *echo.Context) error { PercentageStart: req.PercentageStart, PercentageEnd: req.PercentageEnd, ChapterReference: req.ChapterReference, - Source: "web", + Source: originSource(req.Origin), ModifiedAt: time.Now(), // The PUT targets this exact row (from the URL): identity must // not be re-derived from content — a device echo has usually @@ -2156,10 +2187,7 @@ func (mh *MediaHandler) CreateMediaBookmark(c *echo.Context) error { // The sync-aware path (dedup + LWW + tombstones) is the only path: the // annotation service is a required constructor dependency. - origin := req.Origin - if origin == "" { - origin = "web" - } + origin := originSource(req.Origin) result, err := mh.annotationSvc.SaveBookmark(c.Request().Context(), wsync.SaveBookmarkRequest{ MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true}, UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, @@ -2171,7 +2199,7 @@ func (mh *MediaHandler) CreateMediaBookmark(c *echo.Context) error { CFIPosition: req.CfiPosition, PercentageLoc: req.Percentage, ChapterReference: req.ChapterReference, - Source: "web", + Source: originSource(req.Origin), OriginSource: origin, ModifiedAt: time.Now(), }) @@ -2222,7 +2250,7 @@ func (mh *MediaHandler) UpdateMediaBookmark(c *echo.Context) error { CFIPosition: req.CfiPosition, PercentageLoc: req.Percentage, ChapterReference: req.ChapterReference, - Source: "web", + Source: originSource(req.Origin), ModifiedAt: time.Now(), BookmarkID: pgtype.UUID{Bytes: bookmarkUUID, Valid: true}, }) diff --git a/internal/sync/progress.go b/internal/sync/progress.go index ceed3ae7..cd1e533d 100644 --- a/internal/sync/progress.go +++ b/internal/sync/progress.go @@ -319,6 +319,10 @@ type SaveProgressRequest struct { ScrollY *float64 `json:"scroll_position_y"` PanelNumber *int `json:"panel_number"` ReadingMode *string `json:"reading_mode"` + // Optional client label ("android"/"ios") — the web handler normalizes + // it into Source (writer attribution) unless a paired device token + // claimed the push. Absent/unknown stays "web". + Origin string `json:"origin"` // Audiobook locator (seconds into the current chapter) AudioOffsetSeconds *int `json:"audio_offset_seconds"`