From 2fd086533b2e4f30ecdfc038eb2c8b388cb3cdc5 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 26 Jun 2026 03:14:27 +0000 Subject: [PATCH 1/3] MTM-3: view reading list (most-recent-bookmarked first) Add current-user reading-list read paths over the MTM-6 bookmark primitive, newest-bookmarked-first with bookmark-time cursoring (D1): - ArticleQueryService.findUserBookmarks (offset/limit) and findUserBookmarksWithCursor (relay cursor); cursor anchored on article_bookmarks.created_at. - REST GET /articles/bookmarked (auth required) + WebSecurityConfig matcher. - GraphQL Query.bookmarkedArticles datafetcher + SDL. - New read-service methods (offset/limit ids, count, bookmark dates) + ArticleBookmarkDate / BookmarkedArticleData read models. --- src/main/java/io/spring/api/ArticlesApi.java | 8 + .../api/security/WebSecurityConfig.java | 2 + .../application/ArticleQueryService.java | 73 ++++++ .../application/data/ArticleBookmarkDate.java | 14 + .../data/BookmarkedArticleData.java | 25 ++ .../io/spring/graphql/ArticleDatafetcher.java | 52 +++- .../ArticleBookmarksReadService.java | 10 + .../mapper/ArticleBookmarksReadService.xml | 19 ++ src/main/resources/schema/schema.graphqls | 1 + .../spring/api/BookmarkedArticlesApiTest.java | 82 ++++++ .../ArticleBookmarkQueryServiceTest.java | 242 ++++++++++++++++++ .../ArticleBookmarkDatafetcherTest.java | 139 ++++++++++ .../ArticleBookmarksReadServiceTest.java | 42 +++ 13 files changed, 708 insertions(+), 1 deletion(-) create mode 100644 src/main/java/io/spring/application/data/ArticleBookmarkDate.java create mode 100644 src/main/java/io/spring/application/data/BookmarkedArticleData.java create mode 100644 src/test/java/io/spring/api/BookmarkedArticlesApiTest.java create mode 100644 src/test/java/io/spring/application/article/ArticleBookmarkQueryServiceTest.java create mode 100644 src/test/java/io/spring/graphql/ArticleBookmarkDatafetcherTest.java diff --git a/src/main/java/io/spring/api/ArticlesApi.java b/src/main/java/io/spring/api/ArticlesApi.java index 50584bd6d..6a26b045a 100644 --- a/src/main/java/io/spring/api/ArticlesApi.java +++ b/src/main/java/io/spring/api/ArticlesApi.java @@ -45,6 +45,14 @@ public ResponseEntity getFeed( return ResponseEntity.ok(articleQueryService.findUserFeed(user, new Page(offset, limit))); } + @GetMapping(path = "bookmarked") + public ResponseEntity getBookmarkedArticles( + @RequestParam(value = "offset", defaultValue = "0") int offset, + @RequestParam(value = "limit", defaultValue = "20") int limit, + @AuthenticationPrincipal User user) { + return ResponseEntity.ok(articleQueryService.findUserBookmarks(user, new Page(offset, limit))); + } + @GetMapping public ResponseEntity getArticles( @RequestParam(value = "offset", defaultValue = "0") int offset, diff --git a/src/main/java/io/spring/api/security/WebSecurityConfig.java b/src/main/java/io/spring/api/security/WebSecurityConfig.java index 3786959ef..f0ef6aec9 100644 --- a/src/main/java/io/spring/api/security/WebSecurityConfig.java +++ b/src/main/java/io/spring/api/security/WebSecurityConfig.java @@ -54,6 +54,8 @@ protected void configure(HttpSecurity http) throws Exception { .permitAll() .antMatchers(HttpMethod.GET, "/articles/feed") .authenticated() + .antMatchers(HttpMethod.GET, "/articles/bookmarked") + .authenticated() .antMatchers(HttpMethod.POST, "/users", "/users/login") .permitAll() .antMatchers(HttpMethod.GET, "/articles/**", "/profiles/**", "/tags") diff --git a/src/main/java/io/spring/application/ArticleQueryService.java b/src/main/java/io/spring/application/ArticleQueryService.java index 959e8c638..3f051003d 100644 --- a/src/main/java/io/spring/application/ArticleQueryService.java +++ b/src/main/java/io/spring/application/ArticleQueryService.java @@ -2,10 +2,13 @@ import static java.util.stream.Collectors.toList; +import io.spring.application.data.ArticleBookmarkDate; import io.spring.application.data.ArticleData; import io.spring.application.data.ArticleDataList; import io.spring.application.data.ArticleFavoriteCount; +import io.spring.application.data.BookmarkedArticleData; import io.spring.core.user.User; +import io.spring.infrastructure.mybatis.readservice.ArticleBookmarksReadService; import io.spring.infrastructure.mybatis.readservice.ArticleFavoritesReadService; import io.spring.infrastructure.mybatis.readservice.ArticleReadService; import io.spring.infrastructure.mybatis.readservice.UserRelationshipQueryService; @@ -14,10 +17,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import lombok.AllArgsConstructor; import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import org.springframework.stereotype.Service; @Service @@ -26,6 +32,7 @@ public class ArticleQueryService { private ArticleReadService articleReadService; private UserRelationshipQueryService userRelationshipQueryService; private ArticleFavoritesReadService articleFavoritesReadService; + private ArticleBookmarksReadService articleBookmarksReadService; public Optional findById(String id, User user) { ArticleData articleData = articleReadService.findById(id); @@ -97,6 +104,72 @@ public CursorPager findUserFeedWithCursor( } } + public CursorPager findUserBookmarksWithCursor( + User currentUser, CursorPageParameter page) { + List ids = + articleBookmarksReadService.findUserBookmarkedArticleIdsWithCursor( + currentUser.getId(), toTextCursor(page)); + if (ids.size() == 0) { + return new CursorPager<>(new ArrayList<>(), page.getDirection(), false); + } + boolean hasExtra = ids.size() > page.getLimit(); + if (hasExtra) { + ids.remove(page.getLimit()); + } + if (!page.isNext()) { + Collections.reverse(ids); + } + + List articles = findArticlesInIdOrder(ids); + fillExtraInfo(articles, currentUser); + + Map bookmarkedAt = bookmarkDates(currentUser.getId(), ids); + List data = + articles.stream() + .map(article -> new BookmarkedArticleData(article, bookmarkedAt.get(article.getId()))) + .collect(toList()); + return new CursorPager<>(data, page.getDirection(), hasExtra); + } + + public ArticleDataList findUserBookmarks(User currentUser, Page page) { + List ids = + articleBookmarksReadService.findUserBookmarkedArticleIds(currentUser.getId(), page); + int count = articleBookmarksReadService.countUserBookmarks(currentUser.getId()); + if (ids.size() == 0) { + return new ArticleDataList(new ArrayList<>(), count); + } + List articles = findArticlesInIdOrder(ids); + fillExtraInfo(articles, currentUser); + return new ArticleDataList(articles, count); + } + + private List findArticlesInIdOrder(List ids) { + Map byId = + articleReadService.findArticles(ids).stream() + .collect(Collectors.toMap(ArticleData::getId, article -> article)); + return ids.stream().map(byId::get).filter(Objects::nonNull).collect(toList()); + } + + /** + * {@code article_bookmarks.created_at} is persisted as a SQLite text timestamp (the column's + * {@code CURRENT_TIMESTAMP} default), whereas a {@link DateTime} cursor binds as epoch millis. + * Convert the cursor to the same text form so the bookmark-time comparison (D1) is correct; the + * opaque cursor exposed to clients remains the {@link DateTimeCursor} (millis). + */ + private CursorPageParameter toTextCursor(CursorPageParameter page) { + String cursor = + page.getCursor() == null + ? null + : page.getCursor().withZone(DateTimeZone.UTC).toString("yyyy-MM-dd HH:mm:ss"); + return new CursorPageParameter<>(cursor, page.getLimit(), page.getDirection()); + } + + private Map bookmarkDates(String userId, List ids) { + return articleBookmarksReadService.findBookmarkDates(userId, ids).stream() + .collect( + Collectors.toMap(ArticleBookmarkDate::getArticleId, ArticleBookmarkDate::getCreatedAt)); + } + public ArticleDataList findRecentArticles( String tag, String author, String favoritedBy, Page page, User currentUser) { List articleIds = articleReadService.queryArticles(tag, author, favoritedBy, page); diff --git a/src/main/java/io/spring/application/data/ArticleBookmarkDate.java b/src/main/java/io/spring/application/data/ArticleBookmarkDate.java new file mode 100644 index 000000000..1b5e47b6b --- /dev/null +++ b/src/main/java/io/spring/application/data/ArticleBookmarkDate.java @@ -0,0 +1,14 @@ +package io.spring.application.data; + +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import org.joda.time.DateTime; + +@Getter +@Setter +@NoArgsConstructor +public class ArticleBookmarkDate { + private String articleId; + private DateTime createdAt; +} diff --git a/src/main/java/io/spring/application/data/BookmarkedArticleData.java b/src/main/java/io/spring/application/data/BookmarkedArticleData.java new file mode 100644 index 000000000..9acf0c18d --- /dev/null +++ b/src/main/java/io/spring/application/data/BookmarkedArticleData.java @@ -0,0 +1,25 @@ +package io.spring.application.data; + +import io.spring.application.DateTimeCursor; +import io.spring.application.Node; +import io.spring.application.PageCursor; +import lombok.AllArgsConstructor; +import lombok.Getter; +import org.joda.time.DateTime; + +/** + * Read-model node for a current user's bookmarked article. Carries the bookmark's {@code + * created_at} so cursor pagination is anchored on bookmark time (newest-bookmarked first) rather + * than the article's own create/update time. + */ +@Getter +@AllArgsConstructor +public class BookmarkedArticleData implements Node { + private ArticleData article; + private DateTime bookmarkedAt; + + @Override + public PageCursor getCursor() { + return new DateTimeCursor(bookmarkedAt); + } +} diff --git a/src/main/java/io/spring/graphql/ArticleDatafetcher.java b/src/main/java/io/spring/graphql/ArticleDatafetcher.java index 37c82939a..9d854f578 100644 --- a/src/main/java/io/spring/graphql/ArticleDatafetcher.java +++ b/src/main/java/io/spring/graphql/ArticleDatafetcher.java @@ -15,7 +15,9 @@ import io.spring.application.CursorPager; import io.spring.application.CursorPager.Direction; import io.spring.application.DateTimeCursor; +import io.spring.application.Node; import io.spring.application.data.ArticleData; +import io.spring.application.data.BookmarkedArticleData; import io.spring.application.data.CommentData; import io.spring.core.user.User; import io.spring.core.user.UserRepository; @@ -23,6 +25,7 @@ import io.spring.graphql.DgsConstants.COMMENT; import io.spring.graphql.DgsConstants.PROFILE; import io.spring.graphql.DgsConstants.QUERY; +import io.spring.graphql.exception.AuthenticationException; import io.spring.graphql.types.Article; import io.spring.graphql.types.ArticleEdge; import io.spring.graphql.types.ArticlesConnection; @@ -135,6 +138,53 @@ public DataFetcherResult userFeed( .build(); } + @DgsQuery(field = QUERY.BookmarkedArticles) + public DataFetcherResult getBookmarkedArticles( + @InputArgument("first") Integer first, + @InputArgument("after") String after, + @InputArgument("last") Integer last, + @InputArgument("before") String before, + DgsDataFetchingEnvironment dfe) { + if (first == null && last == null) { + throw new IllegalArgumentException("first 和 last 必须只存在一个"); + } + + User current = SecurityUtil.getCurrentUser().orElseThrow(AuthenticationException::new); + + CursorPager articles; + if (first != null) { + articles = + articleQueryService.findUserBookmarksWithCursor( + current, + new CursorPageParameter<>(DateTimeCursor.parse(after), first, Direction.NEXT)); + } else { + articles = + articleQueryService.findUserBookmarksWithCursor( + current, + new CursorPageParameter<>(DateTimeCursor.parse(before), last, Direction.PREV)); + } + graphql.relay.PageInfo pageInfo = buildArticlePageInfo(articles); + ArticlesConnection articlesConnection = + ArticlesConnection.newBuilder() + .pageInfo(pageInfo) + .edges( + articles.getData().stream() + .map( + b -> + ArticleEdge.newBuilder() + .cursor(b.getCursor().toString()) + .node(buildArticleResult(b.getArticle())) + .build()) + .collect(Collectors.toList())) + .build(); + return DataFetcherResult.newResult() + .data(articlesConnection) + .localContext( + articles.getData().stream() + .collect(Collectors.toMap(b -> b.getArticle().getSlug(), b -> b.getArticle()))) + .build(); + } + @DgsData(parentType = PROFILE.TYPE_NAME, field = PROFILE.Favorites) public DataFetcherResult userFavorites( @InputArgument("first") Integer first, @@ -356,7 +406,7 @@ public DataFetcherResult
findArticleBySlug(@InputArgument("slug") Strin .build(); } - private DefaultPageInfo buildArticlePageInfo(CursorPager articles) { + private DefaultPageInfo buildArticlePageInfo(CursorPager articles) { return new DefaultPageInfo( articles.getStartCursor() == null ? null diff --git a/src/main/java/io/spring/infrastructure/mybatis/readservice/ArticleBookmarksReadService.java b/src/main/java/io/spring/infrastructure/mybatis/readservice/ArticleBookmarksReadService.java index f783fa400..8d76e50fe 100644 --- a/src/main/java/io/spring/infrastructure/mybatis/readservice/ArticleBookmarksReadService.java +++ b/src/main/java/io/spring/infrastructure/mybatis/readservice/ArticleBookmarksReadService.java @@ -1,6 +1,8 @@ package io.spring.infrastructure.mybatis.readservice; import io.spring.application.CursorPageParameter; +import io.spring.application.Page; +import io.spring.application.data.ArticleBookmarkDate; import io.spring.core.user.User; import java.util.List; import java.util.Set; @@ -15,4 +17,12 @@ public interface ArticleBookmarksReadService { List findUserBookmarkedArticleIdsWithCursor( @Param("userId") String userId, @Param("page") CursorPageParameter page); + + List findUserBookmarkedArticleIds( + @Param("userId") String userId, @Param("page") Page page); + + int countUserBookmarks(@Param("userId") String userId); + + List findBookmarkDates( + @Param("userId") String userId, @Param("ids") List ids); } diff --git a/src/main/resources/mapper/ArticleBookmarksReadService.xml b/src/main/resources/mapper/ArticleBookmarksReadService.xml index 56fbf46cb..1d36a751a 100644 --- a/src/main/resources/mapper/ArticleBookmarksReadService.xml +++ b/src/main/resources/mapper/ArticleBookmarksReadService.xml @@ -35,4 +35,23 @@ limit #{page.queryLimit} + + + diff --git a/src/main/resources/schema/schema.graphqls b/src/main/resources/schema/schema.graphqls index a3f6be557..7944f281a 100644 --- a/src/main/resources/schema/schema.graphqls +++ b/src/main/resources/schema/schema.graphqls @@ -12,6 +12,7 @@ type Query { ): ArticlesConnection me: User feed(first: Int, after: String, last: Int, before: String): ArticlesConnection + bookmarkedArticles(first: Int, after: String, last: Int, before: String): ArticlesConnection profile(username: String!): ProfilePayload tags: [String] } diff --git a/src/test/java/io/spring/api/BookmarkedArticlesApiTest.java b/src/test/java/io/spring/api/BookmarkedArticlesApiTest.java new file mode 100644 index 000000000..77545a59f --- /dev/null +++ b/src/test/java/io/spring/api/BookmarkedArticlesApiTest.java @@ -0,0 +1,82 @@ +package io.spring.api; + +import static io.restassured.module.mockmvc.RestAssuredMockMvc.given; +import static io.spring.TestHelper.articleDataFixture; +import static java.util.Arrays.asList; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +import io.restassured.module.mockmvc.RestAssuredMockMvc; +import io.spring.JacksonCustomizations; +import io.spring.api.security.WebSecurityConfig; +import io.spring.application.ArticleQueryService; +import io.spring.application.Page; +import io.spring.application.article.ArticleCommandService; +import io.spring.application.data.ArticleDataList; +import io.spring.core.article.ArticleRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.test.web.servlet.MockMvc; + +@WebMvcTest(ArticlesApi.class) +@Import({WebSecurityConfig.class, JacksonCustomizations.class}) +public class BookmarkedArticlesApiTest extends TestWithCurrentUser { + @MockBean private ArticleRepository articleRepository; + + @MockBean private ArticleQueryService articleQueryService; + + @MockBean private ArticleCommandService articleCommandService; + + @Autowired private MockMvc mvc; + + @Override + @BeforeEach + public void setUp() throws Exception { + super.setUp(); + RestAssuredMockMvc.mockMvc(mvc); + } + + @Test + public void should_get_401_without_login() throws Exception { + RestAssuredMockMvc.when().get("/articles/bookmarked").prettyPeek().then().statusCode(401); + } + + @Test + public void should_get_bookmarked_articles_with_envelope() throws Exception { + ArticleDataList articleDataList = + new ArticleDataList( + asList(articleDataFixture("1", user), articleDataFixture("2", user)), 2); + when(articleQueryService.findUserBookmarks(eq(user), eq(new Page(0, 20)))) + .thenReturn(articleDataList); + + given() + .header("Authorization", "Token " + token) + .when() + .get("/articles/bookmarked") + .prettyPeek() + .then() + .statusCode(200) + .body("articlesCount", org.hamcrest.Matchers.is(2)) + .body("articles.size()", org.hamcrest.Matchers.is(2)); + } + + @Test + public void should_pass_offset_and_limit_through() throws Exception { + when(articleQueryService.findUserBookmarks(eq(user), eq(new Page(5, 10)))) + .thenReturn(new ArticleDataList(asList(articleDataFixture("1", user)), 11)); + + given() + .header("Authorization", "Token " + token) + .when() + .get("/articles/bookmarked?offset=5&limit=10") + .prettyPeek() + .then() + .statusCode(200) + .body("articlesCount", org.hamcrest.Matchers.is(11)) + .body("articles.size()", org.hamcrest.Matchers.is(1)); + } +} diff --git a/src/test/java/io/spring/application/article/ArticleBookmarkQueryServiceTest.java b/src/test/java/io/spring/application/article/ArticleBookmarkQueryServiceTest.java new file mode 100644 index 000000000..dfefadb4f --- /dev/null +++ b/src/test/java/io/spring/application/article/ArticleBookmarkQueryServiceTest.java @@ -0,0 +1,242 @@ +package io.spring.application.article; + +import io.spring.application.ArticleQueryService; +import io.spring.application.CursorPageParameter; +import io.spring.application.CursorPager; +import io.spring.application.CursorPager.Direction; +import io.spring.application.DateTimeCursor; +import io.spring.application.Page; +import io.spring.application.data.ArticleData; +import io.spring.application.data.ArticleDataList; +import io.spring.application.data.BookmarkedArticleData; +import io.spring.core.article.Article; +import io.spring.core.article.ArticleRepository; +import io.spring.core.user.User; +import io.spring.core.user.UserRepository; +import io.spring.infrastructure.DbTestBase; +import io.spring.infrastructure.repository.MyBatisArticleRepository; +import io.spring.infrastructure.repository.MyBatisUserRepository; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; +import javax.sql.DataSource; +import org.joda.time.DateTime; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Import; +import org.springframework.jdbc.core.JdbcTemplate; + +@Import({ArticleQueryService.class, MyBatisUserRepository.class, MyBatisArticleRepository.class}) +public class ArticleBookmarkQueryServiceTest extends DbTestBase { + @Autowired private ArticleQueryService queryService; + + @Autowired private UserRepository userRepository; + + @Autowired private ArticleRepository articleRepository; + + @Autowired private DataSource dataSource; + + private JdbcTemplate jdbcTemplate; + + private User user; + private User other; + + @BeforeEach + public void setUp() { + jdbcTemplate = new JdbcTemplate(dataSource); + jdbcTemplate.execute( + "create table if not exists article_bookmarks (" + + "article_id varchar(255) not null," + + "user_id varchar(255) not null," + + "created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP," + + "primary key (article_id, user_id))"); + jdbcTemplate.execute("delete from article_bookmarks"); + + user = new User("reader@example.com", "reader", "123", "", ""); + userRepository.save(user); + other = new User("other@example.com", "other", "123", "", ""); + userRepository.save(other); + } + + /** + * Persists an article whose own create/update time is intentionally unrelated to the bookmark + * time, so the tests can prove ordering/cursoring is anchored on bookmark time only. + */ + private Article article(String title, DateTime articleCreatedAt) { + Article article = + new Article(title, "desc", "body", Arrays.asList("java"), user.getId(), articleCreatedAt); + articleRepository.save(article); + return article; + } + + private void bookmark(String articleId, String userId, String createdAt) { + jdbcTemplate.update( + "insert into article_bookmarks (article_id, user_id, created_at) values (?, ?, ?)", + articleId, + userId, + createdAt); + } + + private List ids(List articles) { + return articles.stream().map(ArticleData::getId).collect(Collectors.toList()); + } + + @Test + public void should_return_bookmarks_newest_bookmarked_first() { + // article create order is the REVERSE of bookmark order on purpose. + Article a1 = article("oldest-article", new DateTime("2020-01-01T00:00:00Z")); + Article a2 = article("middle-article", new DateTime("2020-02-01T00:00:00Z")); + Article a3 = article("newest-article", new DateTime("2020-03-01T00:00:00Z")); + + bookmark(a1.getId(), user.getId(), "2021-05-01 10:00:03"); // bookmarked last + bookmark(a2.getId(), user.getId(), "2021-05-01 10:00:02"); + bookmark(a3.getId(), user.getId(), "2021-05-01 10:00:01"); // bookmarked first + + ArticleDataList list = queryService.findUserBookmarks(user, new Page(0, 20)); + + Assertions.assertEquals(3, list.getCount()); + Assertions.assertEquals( + Arrays.asList(a1.getId(), a2.getId(), a3.getId()), ids(list.getArticleDatas())); + } + + @Test + public void should_paginate_bookmarks_with_offset_and_limit() { + Article a1 = article("a1", new DateTime("2020-03-01T00:00:00Z")); + Article a2 = article("a2", new DateTime("2020-02-01T00:00:00Z")); + Article a3 = article("a3", new DateTime("2020-01-01T00:00:00Z")); + + bookmark(a1.getId(), user.getId(), "2021-05-01 10:00:01"); + bookmark(a2.getId(), user.getId(), "2021-05-01 10:00:02"); + bookmark(a3.getId(), user.getId(), "2021-05-01 10:00:03"); + + ArticleDataList firstPage = queryService.findUserBookmarks(user, new Page(0, 2)); + Assertions.assertEquals(3, firstPage.getCount()); + Assertions.assertEquals( + Arrays.asList(a3.getId(), a2.getId()), ids(firstPage.getArticleDatas())); + + ArticleDataList secondPage = queryService.findUserBookmarks(user, new Page(2, 2)); + Assertions.assertEquals(3, secondPage.getCount()); + Assertions.assertEquals(Arrays.asList(a1.getId()), ids(secondPage.getArticleDatas())); + } + + @Test + public void should_never_return_other_users_bookmarks() { + Article mine = article("mine", new DateTime("2020-01-01T00:00:00Z")); + Article theirs = article("theirs", new DateTime("2020-01-02T00:00:00Z")); + + bookmark(mine.getId(), user.getId(), "2021-05-01 10:00:01"); + bookmark(theirs.getId(), other.getId(), "2021-05-01 10:00:09"); + + ArticleDataList list = queryService.findUserBookmarks(user, new Page(0, 20)); + Assertions.assertEquals(1, list.getCount()); + Assertions.assertEquals(Arrays.asList(mine.getId()), ids(list.getArticleDatas())); + } + + @Test + public void should_return_empty_reading_list() { + ArticleDataList list = queryService.findUserBookmarks(user, new Page(0, 20)); + Assertions.assertEquals(0, list.getCount()); + Assertions.assertTrue(list.getArticleDatas().isEmpty()); + + CursorPager cursor = + queryService.findUserBookmarksWithCursor( + user, new CursorPageParameter<>(null, 20, Direction.NEXT)); + Assertions.assertTrue(cursor.getData().isEmpty()); + Assertions.assertNull(cursor.getStartCursor()); + Assertions.assertFalse(cursor.hasNext()); + } + + @Test + public void should_cursor_bookmarks_newest_first_anchored_on_bookmark_time() { + Article a1 = article("oldest-article", new DateTime("2020-01-01T00:00:00Z")); + Article a2 = article("middle-article", new DateTime("2020-02-01T00:00:00Z")); + Article a3 = article("newest-article", new DateTime("2020-03-01T00:00:00Z")); + + // bookmark order is independent of article create time. + bookmark(a1.getId(), user.getId(), "2021-05-01 10:00:03"); + bookmark(a2.getId(), user.getId(), "2021-05-01 10:00:02"); + bookmark(a3.getId(), user.getId(), "2021-05-01 10:00:01"); + + CursorPager first = + queryService.findUserBookmarksWithCursor( + user, new CursorPageParameter<>(null, 2, Direction.NEXT)); + + Assertions.assertEquals(2, first.getData().size()); + Assertions.assertEquals(a1.getId(), first.getData().get(0).getArticle().getId()); + Assertions.assertEquals(a2.getId(), first.getData().get(1).getArticle().getId()); + Assertions.assertTrue(first.hasNext()); + + // the emitted cursor must be the bookmark created_at, not the article updatedAt. + long expectedEndMillis = new DateTime("2021-05-01T10:00:02Z").getMillis(); + Assertions.assertEquals(String.valueOf(expectedEndMillis), first.getEndCursor().toString()); + } + + @Test + public void should_page_with_after_cursor_without_overlap() { + Article a1 = article("oldest-article", new DateTime("2020-01-01T00:00:00Z")); + Article a2 = article("middle-article", new DateTime("2020-02-01T00:00:00Z")); + Article a3 = article("newest-article", new DateTime("2020-03-01T00:00:00Z")); + + bookmark(a1.getId(), user.getId(), "2021-05-01 10:00:03"); + bookmark(a2.getId(), user.getId(), "2021-05-01 10:00:02"); + bookmark(a3.getId(), user.getId(), "2021-05-01 10:00:01"); + + CursorPager first = + queryService.findUserBookmarksWithCursor( + user, new CursorPageParameter<>(null, 2, Direction.NEXT)); + + CursorPager next = + queryService.findUserBookmarksWithCursor( + user, + new CursorPageParameter<>( + DateTimeCursor.parse(first.getEndCursor().toString()), 2, Direction.NEXT)); + + Assertions.assertEquals(1, next.getData().size()); + Assertions.assertEquals(a3.getId(), next.getData().get(0).getArticle().getId()); + Assertions.assertFalse(next.hasNext()); + } + + @Test + public void should_page_backwards_with_before_cursor() { + Article a1 = article("a1", new DateTime("2020-01-01T00:00:00Z")); + Article a2 = article("a2", new DateTime("2020-02-01T00:00:00Z")); + Article a3 = article("a3", new DateTime("2020-03-01T00:00:00Z")); + Article a4 = article("a4", new DateTime("2020-04-01T00:00:00Z")); + + bookmark(a1.getId(), user.getId(), "2021-05-01 10:00:04"); + bookmark(a2.getId(), user.getId(), "2021-05-01 10:00:03"); + bookmark(a3.getId(), user.getId(), "2021-05-01 10:00:02"); + bookmark(a4.getId(), user.getId(), "2021-05-01 10:00:01"); + + // before the oldest-bookmarked (a4) -> walking back toward newer bookmarks, newest-first. + CursorPager prev = + queryService.findUserBookmarksWithCursor( + user, + new CursorPageParameter<>( + DateTimeCursor.parse( + String.valueOf(new DateTime("2021-05-01T10:00:01Z").getMillis())), + 2, + Direction.PREV)); + + Assertions.assertEquals(2, prev.getData().size()); + Assertions.assertEquals(a2.getId(), prev.getData().get(0).getArticle().getId()); + Assertions.assertEquals(a3.getId(), prev.getData().get(1).getArticle().getId()); + Assertions.assertTrue(prev.hasPrevious()); + Assertions.assertFalse(prev.hasNext()); + } + + @Test + public void should_drop_bookmarks_for_deleted_articles() { + Article a1 = article("kept", new DateTime("2020-01-01T00:00:00Z")); + + bookmark(a1.getId(), user.getId(), "2021-05-01 10:00:01"); + bookmark("missing-article-id", user.getId(), "2021-05-01 10:00:02"); + + ArticleDataList list = queryService.findUserBookmarks(user, new Page(0, 20)); + // count reflects raw bookmarks; the dangling one is filtered from the result set. + Assertions.assertEquals(2, list.getCount()); + Assertions.assertEquals(Arrays.asList(a1.getId()), ids(list.getArticleDatas())); + } +} diff --git a/src/test/java/io/spring/graphql/ArticleBookmarkDatafetcherTest.java b/src/test/java/io/spring/graphql/ArticleBookmarkDatafetcherTest.java new file mode 100644 index 000000000..104f140da --- /dev/null +++ b/src/test/java/io/spring/graphql/ArticleBookmarkDatafetcherTest.java @@ -0,0 +1,139 @@ +package io.spring.graphql; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +import graphql.execution.DataFetcherResult; +import io.spring.TestHelper; +import io.spring.application.ArticleQueryService; +import io.spring.application.CursorPager; +import io.spring.application.CursorPager.Direction; +import io.spring.application.data.ArticleData; +import io.spring.application.data.BookmarkedArticleData; +import io.spring.core.user.User; +import io.spring.core.user.UserRepository; +import io.spring.graphql.exception.AuthenticationException; +import io.spring.graphql.types.ArticleEdge; +import io.spring.graphql.types.ArticlesConnection; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.joda.time.DateTime; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.authentication.AnonymousAuthenticationToken; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.context.SecurityContextHolder; + +@ExtendWith(MockitoExtension.class) +public class ArticleBookmarkDatafetcherTest { + + @Mock private ArticleQueryService articleQueryService; + + @Mock private UserRepository userRepository; + + private ArticleDatafetcher articleDatafetcher; + + private User user; + + @BeforeEach + public void setUp() { + articleDatafetcher = new ArticleDatafetcher(articleQueryService, userRepository); + user = new User("reader@example.com", "reader", "123", "", ""); + } + + @AfterEach + public void tearDown() { + SecurityContextHolder.clearContext(); + } + + private void authenticate(User current) { + SecurityContextHolder.getContext() + .setAuthentication( + new UsernamePasswordAuthenticationToken(current, null, Collections.emptyList())); + } + + private void anonymous() { + SecurityContextHolder.getContext() + .setAuthentication( + new AnonymousAuthenticationToken( + "key", "anonymous", AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS"))); + } + + @Test + public void should_build_connection_with_bookmark_time_cursors() { + authenticate(user); + + ArticleData a1 = TestHelper.articleDataFixture("1", user); + ArticleData a2 = TestHelper.articleDataFixture("2", user); + DateTime t1 = new DateTime("2021-05-01T10:00:02Z"); + DateTime t2 = new DateTime("2021-05-01T10:00:01Z"); + List data = + Arrays.asList(new BookmarkedArticleData(a1, t1), new BookmarkedArticleData(a2, t2)); + CursorPager pager = new CursorPager<>(data, Direction.NEXT, true); + + when(articleQueryService.findUserBookmarksWithCursor(eq(user), any())).thenReturn(pager); + + DataFetcherResult result = + articleDatafetcher.getBookmarkedArticles(10, null, null, null, null); + + ArticlesConnection connection = result.getData(); + Assertions.assertEquals(2, connection.getEdges().size()); + + ArticleEdge firstEdge = connection.getEdges().get(0); + Assertions.assertEquals(String.valueOf(t1.getMillis()), firstEdge.getCursor()); + Assertions.assertEquals(a1.getSlug(), firstEdge.getNode().getSlug()); + Assertions.assertEquals( + String.valueOf(t2.getMillis()), connection.getEdges().get(1).getCursor()); + + Assertions.assertTrue(connection.getPageInfo().isHasNextPage()); + Assertions.assertFalse(connection.getPageInfo().isHasPreviousPage()); + Assertions.assertEquals( + String.valueOf(t2.getMillis()), connection.getPageInfo().getEndCursor().toString()); + + // localContext is keyed by slug -> ArticleData, like the other article connections. + @SuppressWarnings("unchecked") + java.util.Map localContext = + (java.util.Map) result.getLocalContext(); + Assertions.assertTrue(localContext.containsKey(a1.getSlug())); + Assertions.assertTrue(localContext.containsKey(a2.getSlug())); + } + + @Test + public void should_use_last_before_for_backward_paging() { + authenticate(user); + + CursorPager pager = + new CursorPager<>(Collections.emptyList(), Direction.PREV, false); + when(articleQueryService.findUserBookmarksWithCursor(eq(user), any())).thenReturn(pager); + + DataFetcherResult result = + articleDatafetcher.getBookmarkedArticles(null, null, 10, null, null); + + Assertions.assertNotNull(result.getData()); + Assertions.assertTrue(result.getData().getEdges().isEmpty()); + } + + @Test + public void should_throw_when_anonymous() { + anonymous(); + Assertions.assertThrows( + AuthenticationException.class, + () -> articleDatafetcher.getBookmarkedArticles(10, null, null, null, null)); + } + + @Test + public void should_require_first_or_last() { + authenticate(user); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> articleDatafetcher.getBookmarkedArticles(null, null, null, null, null)); + } +} diff --git a/src/test/java/io/spring/infrastructure/bookmark/ArticleBookmarksReadServiceTest.java b/src/test/java/io/spring/infrastructure/bookmark/ArticleBookmarksReadServiceTest.java index 375c496d1..504b08326 100644 --- a/src/test/java/io/spring/infrastructure/bookmark/ArticleBookmarksReadServiceTest.java +++ b/src/test/java/io/spring/infrastructure/bookmark/ArticleBookmarksReadServiceTest.java @@ -138,4 +138,46 @@ public void should_return_empty_for_user_without_bookmarks() { "nobody", new CursorPageParameter<>(null, 20, Direction.NEXT)); Assertions.assertTrue(result.isEmpty()); } + + @Test + public void should_return_bookmarked_ids_ordered_by_created_at_desc_with_offset_limit() { + String userId = "offset-user"; + insertBookmark("a1", userId, "2020-01-01 00:00:01"); + insertBookmark("a2", userId, "2020-01-01 00:00:02"); + insertBookmark("a3", userId, "2020-01-01 00:00:03"); + insertBookmark("a4", "someone-else", "2020-01-01 00:00:09"); + + List firstPage = + readService.findUserBookmarkedArticleIds(userId, new io.spring.application.Page(0, 2)); + Assertions.assertEquals(Arrays.asList("a3", "a2"), firstPage); + + List secondPage = + readService.findUserBookmarkedArticleIds(userId, new io.spring.application.Page(2, 2)); + Assertions.assertEquals(Arrays.asList("a1"), secondPage); + } + + @Test + public void should_count_only_current_users_bookmarks() { + insertBookmark("a1", "count-user", "2020-01-01 00:00:01"); + insertBookmark("a2", "count-user", "2020-01-01 00:00:02"); + insertBookmark("a3", "another-user", "2020-01-01 00:00:03"); + + Assertions.assertEquals(2, readService.countUserBookmarks("count-user")); + Assertions.assertEquals(0, readService.countUserBookmarks("nobody")); + } + + @Test + public void should_return_bookmark_dates_for_current_user_only() { + insertBookmark("a1", "dates-user", "2020-01-01 00:00:01"); + insertBookmark("a2", "dates-user", "2020-01-01 00:00:02"); + insertBookmark("a2", "other-user", "2020-06-06 06:06:06"); + + List dates = + readService.findBookmarkDates("dates-user", Arrays.asList("a1", "a2")); + + Assertions.assertEquals(2, dates.size()); + Assertions.assertTrue(dates.stream().allMatch(d -> d.getCreatedAt() != null)); + Assertions.assertTrue(dates.stream().anyMatch(d -> d.getArticleId().equals("a1"))); + Assertions.assertTrue(dates.stream().anyMatch(d -> d.getArticleId().equals("a2"))); + } } From eebb5c9e62699b06a25925746029a0d48bfdbc42 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 26 Jun 2026 03:23:59 +0000 Subject: [PATCH 2/3] MTM-3: guard fillExtraInfo against empty article list When every article on a bookmark page has been deleted, findArticlesInIdOrder returns empty while bookmark ids are non-empty; calling fillExtraInfo then renders an invalid 'where id in ()' query. Skip fillExtraInfo for the empty case in both the offset/limit and cursor paths; add a regression test. --- .../spring/application/ArticleQueryService.java | 8 ++++++-- .../ArticleBookmarkQueryServiceTest.java | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/spring/application/ArticleQueryService.java b/src/main/java/io/spring/application/ArticleQueryService.java index 3f051003d..f7306e2bf 100644 --- a/src/main/java/io/spring/application/ArticleQueryService.java +++ b/src/main/java/io/spring/application/ArticleQueryService.java @@ -121,7 +121,9 @@ public CursorPager findUserBookmarksWithCursor( } List articles = findArticlesInIdOrder(ids); - fillExtraInfo(articles, currentUser); + if (!articles.isEmpty()) { + fillExtraInfo(articles, currentUser); + } Map bookmarkedAt = bookmarkDates(currentUser.getId(), ids); List data = @@ -139,7 +141,9 @@ public ArticleDataList findUserBookmarks(User currentUser, Page page) { return new ArticleDataList(new ArrayList<>(), count); } List articles = findArticlesInIdOrder(ids); - fillExtraInfo(articles, currentUser); + if (!articles.isEmpty()) { + fillExtraInfo(articles, currentUser); + } return new ArticleDataList(articles, count); } diff --git a/src/test/java/io/spring/application/article/ArticleBookmarkQueryServiceTest.java b/src/test/java/io/spring/application/article/ArticleBookmarkQueryServiceTest.java index dfefadb4f..a738d9ccd 100644 --- a/src/test/java/io/spring/application/article/ArticleBookmarkQueryServiceTest.java +++ b/src/test/java/io/spring/application/article/ArticleBookmarkQueryServiceTest.java @@ -239,4 +239,21 @@ public void should_drop_bookmarks_for_deleted_articles() { Assertions.assertEquals(2, list.getCount()); Assertions.assertEquals(Arrays.asList(a1.getId()), ids(list.getArticleDatas())); } + + @Test + public void should_return_empty_when_every_bookmarked_article_is_deleted() { + bookmark("missing-1", user.getId(), "2021-05-01 10:00:01"); + bookmark("missing-2", user.getId(), "2021-05-01 10:00:02"); + + // REST offset/limit path: no fillExtraInfo over an empty list (no empty IN () SQL). + ArticleDataList list = queryService.findUserBookmarks(user, new Page(0, 20)); + Assertions.assertTrue(list.getArticleDatas().isEmpty()); + Assertions.assertEquals(2, list.getCount()); + + // Cursor path: same guard. + CursorPager page = + queryService.findUserBookmarksWithCursor( + user, new CursorPageParameter<>(null, 20, Direction.NEXT)); + Assertions.assertTrue(page.getData().isEmpty()); + } } From 22e1adebad1c405644791b94146ef86e86f66232 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 26 Jun 2026 03:32:36 +0000 Subject: [PATCH 3/3] MTM-3: filter out vanished bookmarks; merge feat/reading-list (MTM-2/MTM-4) - Merge updated base so this branch builds against MTM-4's bookmarked flag. - Skip articles whose bookmark was concurrently removed (null bookmark timestamp) to avoid an NPE when building the cursor. --- src/main/java/io/spring/application/ArticleQueryService.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/io/spring/application/ArticleQueryService.java b/src/main/java/io/spring/application/ArticleQueryService.java index 7f6e025c0..76877e52c 100644 --- a/src/main/java/io/spring/application/ArticleQueryService.java +++ b/src/main/java/io/spring/application/ArticleQueryService.java @@ -128,6 +128,7 @@ public CursorPager findUserBookmarksWithCursor( Map bookmarkedAt = bookmarkDates(currentUser.getId(), ids); List data = articles.stream() + .filter(article -> bookmarkedAt.get(article.getId()) != null) .map(article -> new BookmarkedArticleData(article, bookmarkedAt.get(article.getId()))) .collect(toList()); return new CursorPager<>(data, page.getDirection(), hasExtra);