MTM-3: view reading list (most-recent-bookmarked first)

[devin-ai-integration]

Summary

Implements MTM-3 — a signed-in user can view their reading list, newest‑bookmarked first — on top of the MTM-6 bookmark persistence foundation. All additive: a query‑service method, a REST path, a GraphQL Query field, a WebSecurityConfig matcher, and two read models. No changes to the per‑article bookmarked field (owned by MTM-4, per D5).

Ordering and cursoring are anchored on article_bookmarks.created_at (D1), independent of the article's own create/update time.

Subtasks

• MTM-15 ArticleQueryService
  • findUserBookmarks(User, Page) → offset/limit, returns ArticleDataList (REST).
  • findUserBookmarksWithCursor(User, CursorPageParameter<DateTime>) → relay cursor (GraphQL), mirrors findRecentArticlesWithCursor (hasExtra trim + reverse‑on‑PREV).
• MTM-16 REST GET /articles/bookmarked (auth required), offset default 0 / limit default 20, envelope { "articles": [...], "articlesCount": <int> }; explicit authenticated() matcher in WebSecurityConfig (since GET /articles/** is otherwise permitAll).
• MTM-17 GraphQL top‑level bookmarkedArticles(first, after, last, before): ArticlesConnection; datafetcher builds the connection exactly like userFavorites (edges/cursors/pageInfo + slug→ArticleData localContext); getCurrentUser().orElseThrow(AuthenticationException::new).
• MTM-18 tests (see below).

Key implementation note — cursor vs. storage format

articles.created_at is persisted as INTEGER epoch‑millis (written via the joda DateTimeHandler), but article_bookmarks.created_at is persisted as TEXT (the column's CURRENT_TIMESTAMP default from MTM-6). Feeding the MTM-6 cursor primitive a DateTime cursor binds it as millis, which can't compare correctly against the TEXT column under SQLite affinity. To keep the client‑facing cursor consistent (DateTimeCursor = millis) while comparing correctly, the cursor is converted to SQLite text form only at the read boundary — MTM-6's mapper and write path are untouched:

// ArticleQueryService.findUserBookmarksWithCursor
ids = articleBookmarksReadService.findUserBookmarkedArticleIdsWithCursor(
        userId, toTextCursor(page));   // DateTime -> "yyyy-MM-dd HH:mm:ss" (UTC)
// edges still expose DateTimeCursor(bookmarkedAt) via findBookmarkDates(...)

BookmarkedArticleData implements Node carries the bookmark DateTime so getCursor() returns DateTimeCursor(bookmarkedAt).

Robustness (from review)

• Skip fillExtraInfo when the page's articles are all deleted — otherwise the favorites/bookmarks mappers render where id in () (invalid SQL → 500). Applies to both offset and cursor paths; covered by a new test.
• Filter out an article whose bookmark was concurrently removed between the id query and the timestamp query (null bookmark time) — avoids an NPE in DateTimeCursor and correctly drops the entry.

Files

• Added: application/data/BookmarkedArticleData.java, application/data/ArticleBookmarkDate.java.
• Changed (additive): application/ArticleQueryService.java, api/ArticlesApi.java, api/security/WebSecurityConfig.java, graphql/ArticleDatafetcher.java (+ widened buildArticlePageInfo to CursorPager<? extends Node>), mybatis/readservice/ArticleBookmarksReadService.java (+3 methods), mapper/ArticleBookmarksReadService.xml, schema/schema.graphqls.
• Tests: ArticleBookmarkQueryServiceTest, BookmarkedArticlesApiTest, ArticleBookmarkDatafetcherTest, additions to ArticleBookmarksReadServiceTest.
• Merged latest feat/reading-list (MTM-2 / MTM-4) so this branch builds against the shared bookmarked flag.

Tests (MTM-18)

• Functional: newest‑bookmarked‑first with controlled created_at; REST {articles, articlesCount}; GraphQL edges/cursors/pageInfo + localContext.
• Pagination: REST offset/limit slices; GraphQL first/after + last/before stable, non‑overlapping; hasNextPage/hasPreviousPage correct.
• Privacy/security: user A never sees user B's bookmarks; anonymous → 401 (REST) / AuthenticationException (GraphQL).
• Edge: empty reading list; bookmark of a missing article filtered; whole page of deleted articles → empty, no crash; ordering anchored on bookmark time even when article create order is reversed.
• Per D5, no assertions on the bookmarked flag.

Verification gate

./gradlew clean build -x test     # BUILD SUCCESSFUL
./gradlew clean test              # all tests pass (see note)
./gradlew spotlessCheck           # BUILD SUCCESSFUL

JaCoCo on new code = 98.8% (340/344 instructions; every new method 100%). CI build + Devin Review checks are green.

Note: ./gradlew clean test is finalized by the project‑wide jacocoTestCoverageVerification rule (bundle min 0.80), which the repo does not meet today — base feat/reading-list is 0.33. That rule is preexisting‑failing and the repo's own configured test command runs -x jacocoTestCoverageVerification. The test task itself is green: run ./gradlew clean test -x jacocoTestCoverageVerification.

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/d1d174cc645441b9b27945249d17f070

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

Devin Review found 3 potential issues.

[devin-ai-integration]

🚩 Bookmark count includes orphaned bookmarks, creating a mismatch with returned articles

In findUserBookmarks at src/main/java/io/spring/application/ArticleQueryService.java:137, the total count comes from countUserBookmarks which counts raw bookmark rows, while findArticlesInIdOrder at line 141 filters out bookmarks whose articles no longer exist. This means articlesCount in the response can be higher than the number of articles actually returned. The test should_drop_bookmarks_for_deleted_articles at src/test/java/io/spring/application/article/ArticleBookmarkQueryServiceTest.java:237-240 explicitly documents and asserts this behavior (count=2 but only 1 article returned), so it appears intentional. However, clients paginating based on articlesCount may observe gaps or confusion, especially if many bookmarked articles are deleted.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

This is intentional, as the test documents. articlesCount is the count of the user's bookmark rows (countUserBookmarks), independent of the page slice — matching the REST envelope semantics used elsewhere (articlesCount = total matching, not the page size). Orphaned bookmarks only arise because article_bookmarks has no FK to articles (MTM-6 foundation); in normal operation a deleted article wouldn't leave a dangling bookmark, so the count and result set agree. Reconciling orphan cleanup / FK semantics is a foundation concern outside MTM-3's additive scope, so I've left the count as the true bookmark total here rather than silently under-reporting.