Stop the std_list cache drifting from the database

Source/Process.ts

@@ -45,6 +45,32 @@ function sleep(time: number) {
   return new Promise((resolve) => setTimeout(resolve, time));
 }
 
+// The KV key holding the list of problems that have a std answer. It is a
+// cache of `SELECT problem_id FROM std_answer`, kept so that GetStdList - a
+// hot read - costs no database rows.
+export const StdListKey = "std_list";
+
+// Tolerates the legacy trailing-newline format and any blank lines left behind
+// by earlier writes. An empty string is a legitimately empty cache; a missing
+// key is not, and callers must handle that before getting here.
+export const ParseStdList = (Cached: string): Array<number> => {
+  return Cached.split("\n")
+    .filter((Entry) => Entry.trim() !== "")
+    .map(Number);
+};
+
+// Rewrites the cache from the database. Rebuilding wholesale rather than
+// patching an entry in means a dropped or racing write can only ever cost
+// freshness until the next rebuild, never permanent drift.
+export const RebuildStdList = async (XMOJDatabase: Database, kv: KVNamespace): Promise<string> => {
+  const Rows = ThrowErrorIfFailed(
+    await XMOJDatabase.Select("std_answer", ["problem_id"])
+  ) as Array<Record<string, any>>;
+  const List = Rows.map((Row) => Row["problem_id"]).join("\n");
+  await kv.put(StdListKey, List);
+  return List;
+};
+
 export class Process {
   private AdminUserList: Array<string> = ["chenlangning", "shanwenxiao", "zhuchenrui2","liushangchen"];
   // noinspection JSMismatchedCollectionQueryUpdate
@@ -1192,13 +1218,13 @@ export class Process {
       if (ThrowErrorIfFailed(await this.XMOJDatabase.GetTableSize("std_answer", {
         problem_id: ProblemID
       }))["TableSize"] !== 0) {
-        let currentStdList = await this.kv.get("std_list");
-        console.log(currentStdList.toString().indexOf(Data["ProblemID"].toString()));
-        if (currentStdList.split('\n').some(d => d === Data["ProblemID"])) {
-          currentStdList = currentStdList + Data["ProblemID"] + "\n";
-          this.kv.put("std_list", currentStdList);
+        // This is the hot path - the script calls UploadStd for problems that
+        // already have a std. Only touch the database when the cache is
+        // actually missing this problem, which is the drift we are repairing.
+        const Cached = await this.kv.get(StdListKey);
+        if (Cached === null || Cached === undefined || !ParseStdList(Cached).includes(ProblemID)) {
+          await RebuildStdList(this.XMOJDatabase, this.kv);
         }
-        console.log("ProblemID: " + ProblemID + " already has a std answer, skipping upload.");
         return new Result(true, "此题已经有人上传标程");
       }
       if (await this.GetProblemScoreChecker(ProblemID) !== 100) {
@@ -1289,17 +1315,27 @@ export class Process {
         problem_id: Data["ProblemID"],
         std_code: StdCode
       }));
-      let currentStdList = await this.kv.get("std_list");
-      currentStdList = currentStdList + Data["ProblemID"] + "\n";
-      this.kv.put("std_list", currentStdList);
+      // Rebuild from the database rather than appending to the cached string:
+      // an append races with concurrent uploads (KV has no compare-and-set) and
+      // silently loses entries. Uploads are bounded at one per problem ever, so
+      // the extra scan is affordable here in a way it would not be on reads.
+      await RebuildStdList(this.XMOJDatabase, this.kv);
       return new Result(true, "标程上传成功");
     },
     GetStdList: async (Data: object): Promise<Result> => {
       ThrowErrorIfFailed(this.CheckParams(Data, {}));
       const ResponseData = {
         StdList: new Array<number>()
       };
-      ResponseData.StdList = (await this.kv.get("std_list")).split("\n").map(Number);
+      // A missing key is not an empty list - it means the cache has never been
+      // built, and answering [] would tell the client that no problem has a std
+      // answer. Fill it from the database instead. An empty string is a real
+      // empty cache and is served as-is, so this costs a scan only on a genuine
+      // miss, which the daily rebuild keeps rare.
+      const Cached = await this.kv.get(StdListKey);
+      ResponseData.StdList = ParseStdList(
+        Cached ?? await RebuildStdList(this.XMOJDatabase, this.kv)
+      );
       return new Result(true, "获得标程列表成功", ResponseData);
     },
     GetStd: async (Data: object): Promise<Result> => {

Source/index.ts

@@ -15,7 +15,7 @@
  *     along with XMOJ-bbs.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-import {Process} from "./Process";
+import {Process, RebuildStdList} from "./Process";
 import {Database} from "./Database";
 import {NotificationManager} from "./NotificationManager";
 import type {D1Database, KVNamespace, AnalyticsEngineDataset, DurableObjectNamespace, Ai} from "@cloudflare/workers-types";
@@ -152,11 +152,16 @@ export default {
     let Processor = new Process(RequestData, Environment);
     return addCorsHeaders(await Processor.Process(), origin);
   },
-  async scheduled(Event: any, Environment: { DB: D1Database; }, Context: {
+  async scheduled(Event: any, Environment: { DB: D1Database; kv: KVNamespace; }, Context: {
     waitUntil: (arg0: Promise<void>) => void;
   }) {
     let XMOJDatabase = new Database(Environment.DB);
-    Context.waitUntil(new Promise<void>(async (Resolve) => {
+    // An async function passed as a Promise executor swallows its own
+    // rejection - the constructor discards the returned promise, so a throw
+    // from ThrowErrorIfFailed would leave this pending forever and waitUntil
+    // would hang instead of reporting the failed run. Hand waitUntil the async
+    // call's promise directly so errors propagate.
+    Context.waitUntil((async () => {
       await XMOJDatabase.Delete("short_message", {
         "send_time": {
           "Operator": "<=",
@@ -173,7 +178,10 @@ export default {
           "Value": new Date().getTime() - 1000 * 60 * 60 * 24 * 5
         }
       });
-      Resolve();
-    }));
+      // Reconcile the std list cache against the database. One scan per day
+      // bounds any drift - from a dropped KV write or two uploads racing - to
+      // 24 hours, instead of it persisting forever as it does today.
+      await RebuildStdList(XMOJDatabase, Environment.kv);
+    })());
   },
 };

test/process.test.js

@@ -1,6 +1,6 @@
 const test = require('node:test');
 const assert = require('node:assert');
-const { Process } = require('../Source/Process.ts');
+const { Process, RebuildStdList } = require('../Source/Process.ts');
 const { Result } = require('../Source/Result.ts');
 
 function createProcess(mocks = {}) {
@@ -664,3 +664,196 @@ test('GetPost clears mentions for the reader', async () => {
 
     assert.deepStrictEqual(deleted, [{ table: 'bbs_mention', where: { post_id: 1, to_user_id: 'testuser' } }]);
 });
+
+// --- std_list KV cache ---------------------------------------------------
+// The cache is a denormalised copy of `SELECT problem_id FROM std_answer`.
+// It is rebuilt wholesale from the database rather than patched incrementally,
+// so a dropped or racing write cannot leave it permanently out of sync.
+
+function kvStub(initial) {
+    const store = { std_list: initial };
+    const puts = [];
+    return {
+        store,
+        puts,
+        get: async (key) => (key in store ? store[key] : null),
+        put: async (key, value) => { store[key] = value; puts.push(value); },
+    };
+}
+
+test('RebuildStdList writes every problem_id from the database', async () => {
+    const kv = kvStub('stale\n');
+    const db = {
+        Select: async (table, columns) => {
+            assert.strictEqual(table, 'std_answer');
+            assert.deepStrictEqual(columns, ['problem_id']);
+            return new Result(true, '', [
+                { problem_id: 1000 }, { problem_id: 1001 }, { problem_id: 1002 }
+            ]);
+        }
+    };
+
+    const list = await RebuildStdList(db, kv);
+
+    assert.strictEqual(list, '1000\n1001\n1002');
+    assert.strictEqual(kv.store.std_list, '1000\n1001\n1002');
+});
+
+test('RebuildStdList writes an empty cache when the table is empty', async () => {
+    const kv = kvStub('1000\n1001\n');
+    const db = { Select: async () => new Result(true, '', []) };
+
+    await RebuildStdList(db, kv);
+
+    assert.strictEqual(kv.store.std_list, '');
+});
+
+test('RebuildStdList heals a cache that has drifted from the database', async () => {
+    // 1001 was dropped by a lost write; 9999 was never in the table.
+    const kv = kvStub('1000\n9999\n');
+    const db = {
+        Select: async () => new Result(true, '', [
+            { problem_id: 1000 }, { problem_id: 1001 }, { problem_id: 1002 }
+        ])
+    };
+
+    await RebuildStdList(db, kv);
+
+    assert.strictEqual(kv.store.std_list, '1000\n1001\n1002');
+});
+
+const STD_CODE_MARKER = '/' + '*'.repeat(62);
+
+// Minimal pages that satisfy the XMOJ scraper in UploadStd.
+function stdScraperFetch() {
+    const statusPage = `<table id="problemstatus">
+        <thead><tr><th>#</th><th>SID</th><th>user</th></tr></thead>
+        <tbody>
+          <tr><td>1</td><td>1</td><td>someone</td></tr>
+          <tr><td>2</td><td>555</td><td>std</td></tr>
+        </tbody>
+      </table>[NEXT]`;
+    const sourcePage = `int main(){}\n${STD_CODE_MARKER}\ntrailer\n<!--not cached-->`;
+    return async (url) => new Response(
+        String(url).includes('getsource.php') ? sourcePage : statusPage
+    );
+}
+
+test('UploadStd rebuilds and awaits the cache after inserting a std', async () => {
+    const kv = kvStub('1000\n');
+    const proc = createProcess({
+        db: {
+            GetTableSize: async () => new Result(true, '', { TableSize: 0 }),
+            Insert: async () => new Result(true, '', { InsertID: 1 }),
+            Select: async () => new Result(true, '', [{ problem_id: 1000 }, { problem_id: 1234 }]),
+        }
+    });
+    proc.kv = kv;
+    proc.GetProblemScoreChecker = async () => 100;
+    proc.Fetch = stdScraperFetch();
+
+    const result = await proc.ProcessFunctions['UploadStd']({ ProblemID: 1234 });
+
+    assert.ok(result.Success, result.Message);
+    assert.strictEqual(kv.puts.length, 1, 'cache written exactly once');
+    assert.strictEqual(kv.store.std_list, '1000\n1234',
+        'cache must reflect the database once UploadStd resolves');
+});
+
+test('UploadStd repairs a cache missing an already-uploaded problem', async () => {
+    // The DB already has a std for 1234 but the cache lost it. Re-uploading
+    // must put it back rather than silently doing nothing.
+    const kv = kvStub('1000\n');
+    const proc = createProcess({
+        db: {
+            GetTableSize: async () => new Result(true, '', { TableSize: 1 }),
+            Select: async () => new Result(true, '', [{ problem_id: 1000 }, { problem_id: 1234 }]),
+        }
+    });
+    proc.kv = kv;
+
+    const result = await proc.ProcessFunctions['UploadStd']({ ProblemID: 1234 });
+
+    assert.ok(result.Success);
+    assert.strictEqual(result.Message, '此题已经有人上传标程');
+    assert.strictEqual(kv.store.std_list, '1000\n1234');
+});
+
+test('UploadStd touches neither database nor cache when already in sync', async () => {
+    // The hot path: the script re-uploads a problem that already has a std and
+    // is already cached. This must cost zero database rows and zero KV writes.
+    const kv = kvStub('1000\n1234\n');
+    const select = test.mock.fn(async () => new Result(true, '', []));
+    const proc = createProcess({
+        db: {
+            GetTableSize: async () => new Result(true, '', { TableSize: 1 }),
+            Select: select,
+        }
+    });
+    proc.kv = kv;
+
+    await proc.ProcessFunctions['UploadStd']({ ProblemID: 1234 });
+
+    assert.strictEqual(select.mock.calls.length, 0, 'no database read on the hot path');
+    assert.strictEqual(kv.puts.length, 0, 'no cache write on the hot path');
+    assert.strictEqual(kv.store.std_list, '1000\n1234\n', 'cache left untouched');
+});
+
+test('GetStdList returns no spurious trailing zero', async () => {
+    const proc = createProcess();
+    proc.kv = kvStub('1000\n1001\n1002\n');   // legacy trailing-newline format
+
+    const result = await proc.ProcessFunctions['GetStdList']({});
+
+    assert.ok(result.Success);
+    assert.deepStrictEqual(result.Data.StdList, [1000, 1001, 1002]);
+});
+
+test('GetStdList fills the cache from the database when the key is unset', async () => {
+    // An unset key is not an empty list - answering [] would tell the client
+    // that no problem has a std answer at all.
+    const kv = kvStub(undefined);
+    const proc = createProcess({
+        db: {
+            Select: async () => new Result(true, '', [
+                { problem_id: 1000 }, { problem_id: 1001 }
+            ])
+        }
+    });
+    proc.kv = kv;
+
+    const result = await proc.ProcessFunctions['GetStdList']({});
+
+    assert.ok(result.Success);
+    assert.deepStrictEqual(result.Data.StdList, [1000, 1001]);
+    assert.strictEqual(kv.store.std_list, '1000\n1001', 'cache filled for next time');
+});
+
+test('GetStdList serves an empty cache without touching the database', async () => {
+    // An empty string is a legitimately empty cache (no stds uploaded yet) and
+    // must be distinguished from a missing key.
+    const select = test.mock.fn(async () => new Result(true, '', []));
+    const proc = createProcess({ db: { Select: select } });
+    proc.kv = kvStub('');
+
+    const result = await proc.ProcessFunctions['GetStdList']({});
+
+    assert.ok(result.Success);
+    assert.deepStrictEqual(result.Data.StdList, []);
+    assert.strictEqual(select.mock.calls.length, 0, 'empty cache is valid, no rebuild');
+});
+
+test('UploadStd rebuilds when the cache key is unset entirely', async () => {
+    const kv = kvStub(undefined);
+    const proc = createProcess({
+        db: {
+            GetTableSize: async () => new Result(true, '', { TableSize: 1 }),
+            Select: async () => new Result(true, '', [{ problem_id: 1234 }]),
+        }
+    });
+    proc.kv = kv;
+
+    await proc.ProcessFunctions['UploadStd']({ ProblemID: 1234 });
+
+    assert.strictEqual(kv.store.std_list, '1234');
+});