fix: Fix missing bookmarks during pagination if they got created in the same second. Fixes #140

3 files changed, 102 insertions, 8 deletions

diff --git a/packages/shared/types/bookmarks.ts b/packages/shared/types/bookmarks.ts
index 1511db48..10dba0c8 100644
--- a/packages/shared/types/bookmarks.ts
+++ b/packages/shared/types/bookmarks.ts
@@ -91,6 +91,11 @@ export type ZNewBookmarkRequest = z.infer<typeof zNewBookmarkRequestSchema>;
 export const DEFAULT_NUM_BOOKMARKS_PER_PAGE = 20;
 export const MAX_NUM_BOOKMARKS_PER_PAGE = 100;
 
+const zCursorV2 = z.object({
+  createdAt: z.date(),
+  id: z.string(),
+});
+
 export const zGetBookmarksRequestSchema = z.object({
   ids: z.array(z.string()).optional(),
   archived: z.boolean().optional(),
@@ -98,13 +103,17 @@ export const zGetBookmarksRequestSchema = z.object({
   tagId: z.string().optional(),
   listId: z.string().optional(),
   limit: z.number().max(MAX_NUM_BOOKMARKS_PER_PAGE).optional(),
-  cursor: z.date().nullish(),
+  cursor: zCursorV2.or(z.date()).nullish(),
+  // TODO: Remove this field once all clients are updated to use the new cursor structure.
+  // This is done for backward comptability. If a client doesn't send this field, we'll assume it's an old client
+  // and repsond with the old cursor structure. Once all clients are updated, we can remove this field and drop the old cursor structure.
+  useCursorV2: z.boolean().optional(),
 });
 export type ZGetBookmarksRequest = z.infer<typeof zGetBookmarksRequestSchema>;
 
 export const zGetBookmarksResponseSchema = z.object({
   bookmarks: z.array(zBookmarkSchema),
-  nextCursor: z.date().nullable(),
+  nextCursor: zCursorV2.or(z.date()).nullable(),
 });
 export type ZGetBookmarksResponse = z.infer<typeof zGetBookmarksResponseSchema>;
 
diff --git a/packages/trpc/routers/bookmarks.test.ts b/packages/trpc/routers/bookmarks.test.ts
index d739da0c..c9627626 100644
--- a/packages/trpc/routers/bookmarks.test.ts
+++ b/packages/trpc/routers/bookmarks.test.ts
@@ -1,5 +1,7 @@
 import { assert, beforeEach, describe, expect, test } from "vitest";
 
+import { bookmarks } from "@hoarder/db/schema";
+
 import type { CustomTestContext } from "../testUtils";
 import { defaultBeforeEach } from "../testUtils";
 
@@ -250,4 +252,62 @@ describe("Bookmark Routes", () => {
     });
     expect(bookmark3User1.alreadyExists).toEqual(false);
   });
+
+  // Ensure that the pagination returns all the results
+  test<CustomTestContext>("pagination", async ({ apiCallers, db }) => {
+    const user = await apiCallers[0].users.whoami();
+    let now = 100_000;
+
+    const bookmarkWithDate = (date_ms: number) => ({
+      userId: user.id,
+      createdAt: new Date(date_ms),
+    });
+
+    // One normal bookmark
+    const values = [bookmarkWithDate(now)];
+    // 10 with a second in between
+    for (let i = 0; i < 10; i++) {
+      now -= 1000;
+      values.push(bookmarkWithDate(now));
+    }
+    // Another ten but at the same second
+    for (let i = 0; i < 10; i++) {
+      values.push(bookmarkWithDate(now));
+    }
+    // And then another one with a second afterards
+    for (let i = 0; i < 10; i++) {
+      now -= 1000;
+      values.push(bookmarkWithDate(now));
+    }
+    // In total, we should have 31 bookmarks
+
+    const inserted = await db.insert(bookmarks).values(values).returning();
+
+    const validateWithLimit = async (limit: number) => {
+      const results: string[] = [];
+      let cursor = undefined;
+
+      // To avoid running the test forever
+      let i = 0;
+
+      do {
+        const res = await apiCallers[0].bookmarks.getBookmarks({
+          limit,
+          cursor,
+          useCursorV2: true,
+        });
+        results.push(...res.bookmarks.map((b) => b.id));
+        cursor = res.nextCursor;
+        i++;
+      } while (cursor && i < 100);
+
+      expect(results.sort()).toEqual(inserted.map((b) => b.id).sort());
+    };
+
+    await validateWithLimit(1);
+    await validateWithLimit(2);
+    await validateWithLimit(3);
+    await validateWithLimit(10);
+    await validateWithLimit(100);
+  });
 });
diff --git a/packages/trpc/routers/bookmarks.ts b/packages/trpc/routers/bookmarks.ts
index a7db564b..0a9747c9 100644
--- a/packages/trpc/routers/bookmarks.ts
+++ b/packages/trpc/routers/bookmarks.ts
@@ -1,5 +1,5 @@
 import { experimental_trpcMiddleware, TRPCError } from "@trpc/server";
-import { and, desc, eq, exists, inArray, lte, or } from "drizzle-orm";
+import { and, desc, eq, exists, inArray, lt, lte, or } from "drizzle-orm";
 import invariant from "tiny-invariant";
 import { z } from "zod";
 
@@ -516,11 +516,21 @@ export const bookmarksAppRouter = router({
                       ),
                   )
                 : undefined,
-              input.cursor ? lte(bookmarks.createdAt, input.cursor) : undefined,
+              input.cursor
+                ? input.cursor instanceof Date
+                  ? lte(bookmarks.createdAt, input.cursor)
+                  : or(
+                      lt(bookmarks.createdAt, input.cursor.createdAt),
+                      and(
+                        eq(bookmarks.createdAt, input.cursor.createdAt),
+                        lte(bookmarks.id, input.cursor.id),
+                      ),
+                    )
+                : undefined,
             ),
           )
           .limit(input.limit + 1)
-          .orderBy(desc(bookmarks.createdAt)),
+          .orderBy(desc(bookmarks.createdAt), desc(bookmarks.id)),
       );
       // TODO: Consider not inlining the tags in the response of getBookmarks as this query is getting kinda expensive
       const results = await ctx.db
@@ -532,7 +542,7 @@ export const bookmarksAppRouter = router({
         .leftJoin(bookmarkLinks, eq(bookmarkLinks.id, sq.id))
         .leftJoin(bookmarkTexts, eq(bookmarkTexts.id, sq.id))
         .leftJoin(bookmarkAssets, eq(bookmarkAssets.id, sq.id))
-        .orderBy(desc(sq.createdAt));
+        .orderBy(desc(sq.createdAt), desc(sq.id));
 
       const bookmarksRes = results.reduce<Record<string, ZBookmark>>(
         (acc, row) => {
@@ -578,10 +588,25 @@ export const bookmarksAppRouter = router({
 
       const bookmarksArr = Object.values(bookmarksRes);
 
+      bookmarksArr.sort((a, b) => {
+        if (a.createdAt != b.createdAt) {
+          return b.createdAt.getTime() - a.createdAt.getTime();
+        } else {
+          return b.id.localeCompare(a.id);
+        }
+      });
+
       let nextCursor = null;
       if (bookmarksArr.length > input.limit) {
-        const nextItem = bookmarksArr.pop();
-        nextCursor = nextItem?.createdAt ?? null;
+        const nextItem = bookmarksArr.pop()!;
+        if (input.useCursorV2) {
+          nextCursor = {
+            id: nextItem.id,
+            createdAt: nextItem.createdAt,
+          };
+        } else {
+          nextCursor = nextItem.createdAt;
+        }
       }
 
       return { bookmarks: bookmarksArr, nextCursor };