Fetch an entire discussion page in one D1 query

Source/Process.ts

@@ -669,13 +669,38 @@ export class Process {
           LockTime: 0
         }
       };
-      const Post = ThrowErrorIfFailed(await this.XMOJDatabase.Select("bbs_post", [], {
-        post_id: Data["PostID"]
-      }));
-      if (Post.toString() == "") {
+      // Post metadata, board name, lock state, total reply count and the
+      // requested page of replies all come back in one round trip instead of
+      // five sequential ones - the round trips dominated the time to open a
+      // discussion. The page CTE is bound before PageCount is known, so the
+      // offset is clamped here and the rows are simply discarded when the
+      // range check below rejects the page.
+      const Offset = Math.max(0, (Data["Page"] - 1) * 15);
+      const Rows: Array<Record<string, any>> = (await this.RawDatabase.prepare(
+        "WITH post AS (" +
+        "  SELECT p.user_id AS post_user_id, p.problem_id AS problem_id, p.title AS title, " +
+        "         p.post_time AS post_time, p.board_id AS board_id, " +
+        "         b.board_name AS board_name, l.lock_person AS lock_person, l.lock_time AS lock_time " +
+        "  FROM bbs_post p " +
+        "  LEFT JOIN bbs_board b ON b.board_id = p.board_id " +
+        "  LEFT JOIN bbs_lock l ON l.post_id = p.post_id " +
+        "  WHERE p.post_id = ?" +
+        "), page AS (" +
+        "  SELECT reply_id, user_id AS reply_user_id, content, reply_time, edit_time, edit_person " +
+        "  FROM bbs_reply WHERE post_id = ? ORDER BY reply_time ASC LIMIT ? OFFSET ?" +
+        ") " +
+        "SELECT post.*, " +
+        "(SELECT COUNT(*) FROM bbs_reply WHERE post_id = ?) AS reply_count, " +
+        "page.reply_id AS reply_id, page.reply_user_id AS reply_user_id, page.content AS content, " +
+        "page.reply_time AS reply_time, page.edit_time AS edit_time, page.edit_person AS edit_person " +
+        "FROM post LEFT JOIN page ON 1 = 1;"
+      ).bind(Data["PostID"], Data["PostID"], 15, Offset, Data["PostID"]).all())["results"];
+
+      if (Rows.length === 0) {
         return new Result(false, "该讨论不存在");
       }
-      ResponseData.PageCount = Math.ceil(ThrowErrorIfFailed(await this.XMOJDatabase.GetTableSize("bbs_reply", {post_id: Data["PostID"]}))["TableSize"] / 15);
+      const Post = Rows[0];
+      ResponseData.PageCount = Math.ceil(Post["reply_count"] / 15);
       if (ResponseData.PageCount === 0) {
         return new Result(true, "获得讨论成功", ResponseData);
       }
@@ -686,34 +711,28 @@ export class Process {
         post_id: Data["PostID"],
         to_user_id: this.Username
       });
-      ResponseData.UserID = Post[0]["user_id"];
-      ResponseData.ProblemID = Post[0]["problem_id"];
-      ResponseData.Title = Post[0]["title"];
-      ResponseData.PostTime = Post[0]["post_time"];
-      ResponseData.BoardID = Post[0]["board_id"];
-      ResponseData.BoardName = ThrowErrorIfFailed(await this.XMOJDatabase.Select("bbs_board", ["board_name"], {board_id: Post[0]["board_id"]}))[0]["board_name"];
+      ResponseData.UserID = Post["post_user_id"];
+      ResponseData.ProblemID = Post["problem_id"];
+      ResponseData.Title = Post["title"];
+      ResponseData.PostTime = Post["post_time"];
+      ResponseData.BoardID = Post["board_id"];
+      ResponseData.BoardName = Post["board_name"];
 
-      const Locked = ThrowErrorIfFailed(await this.XMOJDatabase.Select("bbs_lock", [], {
-        post_id: Data["PostID"]
-      }));
-      if (Locked.toString() !== "") {
+      if (Post["lock_person"] !== null && Post["lock_person"] !== undefined) {
         ResponseData.Lock.Locked = true;
-        ResponseData.Lock.LockPerson = Locked[0]["lock_person"];
-        ResponseData.Lock.LockTime = Locked[0]["lock_time"];
+        ResponseData.Lock.LockPerson = Post["lock_person"];
+        ResponseData.Lock.LockTime = Post["lock_time"];
       }
 
-      const Reply = ThrowErrorIfFailed(await this.XMOJDatabase.Select("bbs_reply", [], {post_id: Data["PostID"]}, {
-        Order: "reply_time",
-        OrderIncreasing: true,
-        Limit: 15,
-        Offset: (Data["Page"] - 1) * 15
-      }));
-      for (const ReplyItem of Reply) {
+      for (const ReplyItem of Rows) {
+        if (ReplyItem["reply_id"] === null || ReplyItem["reply_id"] === undefined) {
+          continue;
+        }
         let processedContent: string = ReplyItem["content"];
         processedContent = processedContent.replace(/xmoj-bbs\.tech/g, "xmoj-bbs.me");
         ResponseData.Reply.push({
           ReplyID: ReplyItem["reply_id"],
-          UserID: ReplyItem["user_id"],
+          UserID: ReplyItem["reply_user_id"],
           Content: processedContent,
           ReplyTime: ReplyItem["reply_time"],
           EditTime: ReplyItem["edit_time"],

test/process.test.js

@@ -503,3 +503,164 @@ test('GetUserSettings fails when stored settings JSON is valid but not an object
     assert.strictEqual(result.Success, false);
     assert.strictEqual(result.Message, '设置数据损坏');
 });
+
+function stubGetPostQuery(proc, rows) {
+    const calls = [];
+    proc.RawDatabase = {
+        prepare: (query) => ({
+            bind: (...args) => ({
+                all: async () => {
+                    calls.push({ query, args });
+                    return { results: rows };
+                }
+            })
+        })
+    };
+    return calls;
+}
+
+function postRow(overrides = {}) {
+    return Object.assign({
+        post_user_id: 'alice',
+        problem_id: 1000,
+        title: 'Post one',
+        post_time: 111,
+        board_id: 2,
+        board_name: '学术版',
+        lock_person: null,
+        lock_time: null,
+        reply_count: 2,
+        reply_id: null,
+        reply_user_id: null,
+        content: null,
+        reply_time: null,
+        edit_time: null,
+        edit_person: null
+    }, overrides);
+}
+
+test('GetPost fetches the whole discussion in a single query', async () => {
+    const proc = createProcess();
+    const calls = stubGetPostQuery(proc, [
+        postRow({ reply_id: 1, reply_user_id: 'u1', content: 'hello', reply_time: 1001 }),
+        postRow({ reply_id: 2, reply_user_id: 'u2', content: 'world', reply_time: 1002 })
+    ]);
+
+    const result = await proc.ProcessFunctions['GetPost']({ PostID: 1, Page: 1 });
+
+    assert.ok(result.Success);
+    assert.strictEqual(result.Message, '获得讨论成功');
+    assert.strictEqual(calls.length, 1, 'expected exactly one SQL query');
+    assert.deepStrictEqual(calls[0].args, [1, 1, 15, 0, 1]);
+    assert.strictEqual(result.Data.UserID, 'alice');
+    assert.strictEqual(result.Data.ProblemID, 1000);
+    assert.strictEqual(result.Data.Title, 'Post one');
+    assert.strictEqual(result.Data.PostTime, 111);
+    assert.strictEqual(result.Data.BoardID, 2);
+    assert.strictEqual(result.Data.BoardName, '学术版');
+    assert.strictEqual(result.Data.PageCount, 1);
+    assert.deepStrictEqual(result.Data.Lock, { Locked: false, LockPerson: '', LockTime: 0 });
+    assert.deepStrictEqual(result.Data.Reply, [
+        { ReplyID: 1, UserID: 'u1', Content: 'hello', ReplyTime: 1001, EditTime: null, EditPerson: null },
+        { ReplyID: 2, UserID: 'u2', Content: 'world', ReplyTime: 1002, EditTime: null, EditPerson: null }
+    ]);
+});
+
+test('GetPost binds the offset for the requested page', async () => {
+    const proc = createProcess();
+    const calls = stubGetPostQuery(proc, [
+        postRow({ reply_count: 20, reply_id: 16, reply_user_id: 'u16', content: 'x', reply_time: 1016 })
+    ]);
+
+    const result = await proc.ProcessFunctions['GetPost']({ PostID: 7, Page: 2 });
+
+    assert.ok(result.Success);
+    assert.deepStrictEqual(calls[0].args, [7, 7, 15, 15, 7]);
+    assert.strictEqual(result.Data.PageCount, 2);
+    assert.strictEqual(result.Data.Reply.length, 1);
+});
+
+test('GetPost reports a locked discussion', async () => {
+    const proc = createProcess();
+    stubGetPostQuery(proc, [
+        postRow({ lock_person: 'admin', lock_time: 999, reply_count: 1, reply_id: 1, reply_user_id: 'u1', content: 'hi', reply_time: 1001 })
+    ]);
+
+    const result = await proc.ProcessFunctions['GetPost']({ PostID: 1, Page: 1 });
+
+    assert.ok(result.Success);
+    assert.deepStrictEqual(result.Data.Lock, { Locked: true, LockPerson: 'admin', LockTime: 999 });
+});
+
+test('GetPost rewrites legacy domain in reply content', async () => {
+    const proc = createProcess();
+    stubGetPostQuery(proc, [
+        postRow({ reply_count: 1, reply_id: 1, reply_user_id: 'u1', content: 'see https://xmoj-bbs.tech/a and xmoj-bbs.tech/b', reply_time: 1001 })
+    ]);
+
+    const result = await proc.ProcessFunctions['GetPost']({ PostID: 1, Page: 1 });
+
+    assert.strictEqual(result.Data.Reply[0].Content, 'see https://xmoj-bbs.me/a and xmoj-bbs.me/b');
+});
+
+test('GetPost fails when the discussion does not exist', async () => {
+    const proc = createProcess();
+    stubGetPostQuery(proc, []);
+
+    const result = await proc.ProcessFunctions['GetPost']({ PostID: 999, Page: 1 });
+
+    assert.strictEqual(result.Success, false);
+    assert.strictEqual(result.Message, '该讨论不存在');
+});
+
+test('GetPost returns an empty discussion when it has no replies', async () => {
+    const proc = createProcess();
+    stubGetPostQuery(proc, [postRow({ reply_count: 0 })]);
+
+    const result = await proc.ProcessFunctions['GetPost']({ PostID: 2, Page: 1 });
+
+    assert.ok(result.Success);
+    assert.strictEqual(result.Data.PageCount, 0);
+    assert.deepStrictEqual(result.Data.Reply, []);
+    assert.strictEqual(result.Data.Title, '', 'metadata is withheld for an empty discussion, as before');
+});
+
+test('GetPost rejects a page outside the available range', async () => {
+    const proc = createProcess();
+    const calls = stubGetPostQuery(proc, [postRow({ reply_count: 2 })]);
+
+    const result = await proc.ProcessFunctions['GetPost']({ PostID: 1, Page: 5 });
+
+    assert.strictEqual(result.Success, false);
+    assert.strictEqual(result.Message, '参数页数不在范围1~1内');
+    assert.strictEqual(calls.length, 1);
+});
+
+test('GetPost clamps a negative offset for a non-positive page', async () => {
+    const proc = createProcess();
+    const calls = stubGetPostQuery(proc, [postRow({ reply_count: 2 })]);
+
+    const result = await proc.ProcessFunctions['GetPost']({ PostID: 1, Page: 0 });
+
+    assert.strictEqual(result.Success, false);
+    assert.strictEqual(calls[0].args[3], 0, 'offset must never be negative');
+});
+
+test('GetPost clears mentions for the reader', async () => {
+    const deleted = [];
+    const proc = createProcess({
+        db: {
+            Delete: async (table, where) => {
+                deleted.push({ table, where });
+                return new Result(true, '');
+            }
+        }
+    });
+    stubGetPostQuery(proc, [
+        postRow({ reply_count: 1, reply_id: 1, reply_user_id: 'u1', content: 'hi', reply_time: 1001 })
+    ]);
+
+    await proc.ProcessFunctions['GetPost']({ PostID: 1, Page: 1 });
+
+    assert.deepStrictEqual(deleted, [{ table: 'bbs_mention', where: { post_id: 1, to_user_id: 'testuser' } }]);
+});