Refactor of ranking-handling in pb document handling

server/package.json

@@ -52,7 +52,7 @@
         "express": "^4.17.1",
         "express-async-errors": "^3.1.1",
         "express-session": "^1.17.1",
-        "fast-json-stable-hash": "^1.0.1",
+        "fast-json-stable-hash": "^1.0.2",
         "kamaitachi-common": "github:zkldi/Kamaitachi-common",
         "mongodb": "^3.6.6",
         "monk": "^7.3.4",

server/pnpm-lock.yaml

@@ -30,7 +30,7 @@ specifiers:
   express-async-errors: ^3.1.1
   express-request-mock: ^3.1.0
   express-session: ^1.17.1
-  fast-json-stable-hash: ^1.0.1
+  fast-json-stable-hash: ^1.0.2
   kamaitachi-common: github:zkldi/Kamaitachi-common
   mongodb: ^3.6.6
   monk: ^7.3.4
@@ -61,8 +61,8 @@ dependencies:
   express: 4.17.1
   express-async-errors: 3.1.1_express@4.17.1
   express-session: 1.17.1
-  fast-json-stable-hash: 1.0.1
-  kamaitachi-common: github.com/zkldi/Kamaitachi-common/b63c3213ba997b06d87f72e8dc18dde061502e6c
+  fast-json-stable-hash: 1.0.2
+  kamaitachi-common: github.com/zkldi/Kamaitachi-common/a3a77f6c5e8c870a01652c92515dfe7f8030863d
   mongodb: 3.6.6
   monk: 7.3.4
   multer: 1.4.2
@@ -1875,8 +1875,8 @@ packages:
       picomatch: 2.2.3
     dev: true
 
-  /fast-json-stable-hash/1.0.1:
-    resolution: {integrity: sha512-WaR/iEiaii1b9sWC9yYauW/QXDCRIcWbVhS2a+rXFxyeYBLTVQxG6QXe2EDFduSosl4tL319Q50Qyq87l+ZX1A==}
+  /fast-json-stable-hash/1.0.2:
+    resolution: {integrity: sha512-p7Sr7IjiqJkFuRx1P6tYVTDwBf+ZfXyU2l3bv5XkBhiey5G3fHzBUJiEnpIlAZjGedXpEJM/V2SV+hD0pOnmmg==}
     dev: false
 
   /fast-json-stable-stringify/2.1.0:
@@ -4638,8 +4638,8 @@ packages:
       '@types/yoga-layout': 1.9.2
     dev: true
 
-  github.com/zkldi/Kamaitachi-common/b63c3213ba997b06d87f72e8dc18dde061502e6c:
-    resolution: {tarball: https://codeload.github.com/zkldi/Kamaitachi-common/tar.gz/b63c3213ba997b06d87f72e8dc18dde061502e6c}
+  github.com/zkldi/Kamaitachi-common/a3a77f6c5e8c870a01652c92515dfe7f8030863d:
+    resolution: {tarball: https://codeload.github.com/zkldi/Kamaitachi-common/tar.gz/a3a77f6c5e8c870a01652c92515dfe7f8030863d}
     name: kamaitachi-common
     version: 0.1.0
     dependencies:

server/scripts/set-indexes.ts

@@ -35,6 +35,7 @@ const indexes: Partial<Record<ValidDatabases, Index[]>> = {
     "score-pbs": [
         index({ chartID: 1, userID: 1 }, UNIQUE),
         index({ chartID: 1, "scoreData.percent": 1 }),
+        index({ checksum: 1 }, UNIQUE),
     ],
     "charts-iidx": [
         index({ chartID: 1 }, UNIQUE),

server/src/core/goal-core.ts

@@ -0,0 +1 @@
+// idk what needs to go here just yet - need to write the goal support in score import - zkldi

server/src/score-import/framework/goals/goals.ts

@@ -0,0 +1,12 @@
+import { integer, Game, Playtypes } from "kamaitachi-common";
+import { KtLogger } from "../../../types";
+
+/**
+ * Update a user's progress on goals.
+ */
+async function ProcessUsersGoals(
+    game: Game,
+    playtypes: Playtypes[Game][],
+    userID: integer,
+    logger: KtLogger
+) {}

server/src/score-import/framework/pb/create-pb-doc.test.ts

@@ -7,7 +7,7 @@ import {
     TestingIIDXSPScore,
     TestingIIDXSPScorePB,
 } from "../../../test-utils/test-data";
-import { CreatePBDoc, GetRankingInfo } from "./create-pb-doc";
+import { CreatePBDoc } from "./create-pb-doc";
 import deepmerge from "deepmerge";
 import CreateLogCtx from "../../../logger";
 import { KtLogger } from "../../../types";
@@ -17,81 +17,6 @@ let IIDXScore = TestingIIDXSPScore;
 
 const logger = CreateLogCtx("create-pb-doc.test.ts");
 
-t.test("#GetRankingInfo", (t) => {
-    t.beforeEach(ResetDBState);
-
-    let chartID = Testing511SPA.chartID;
-
-    t.test("Ranking info for one score", async (t) => {
-        await db["score-pbs"].insert([
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 50 }, userID: 1 }),
-        ]);
-
-        let { outOf, ranking } = await GetRankingInfo(chartID, 1, 50);
-
-        t.equal(outOf, 1, "Should correctly determine we're out of 1 score.");
-        t.equal(ranking, 1, "Should correctly determine we're #1.");
-
-        t.end();
-    });
-
-    t.test("Ranking info for multiple scores", async (t) => {
-        await db["score-pbs"].insert([
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 50 }, userID: 1 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 60 }, userID: 2 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 70 }, userID: 3 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 80 }, userID: 4 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 40 }, userID: 5 }),
-        ]);
-
-        let { outOf, ranking } = await GetRankingInfo(chartID, 1, 50);
-
-        t.equal(outOf, 5, "Should correctly determine we're out of 5 scores.");
-        t.equal(ranking, 4, "Should correctly determine we're #4/5.");
-
-        t.end();
-    });
-
-    t.test("#1 in Ranking Info for scores", async (t) => {
-        // insert some fake scores - our percent is 50.
-
-        await db["score-pbs"].insert([
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 90 }, userID: 1 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 60 }, userID: 2 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 70 }, userID: 3 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 80 }, userID: 4 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 40 }, userID: 5 }),
-        ]);
-
-        let { outOf, ranking } = await GetRankingInfo(chartID, 1, 90);
-
-        t.equal(outOf, 5, "Should correctly determine we're out of 5 scores.");
-        t.equal(ranking, 1, "Should correctly determine we're #1/5.");
-
-        t.end();
-    });
-
-    t.test("#Last in Ranking Info for scores", async (t) => {
-        // insert some fake scores - our percent is 50.
-
-        await db["score-pbs"].insert([
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 0 }, userID: 1 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 60 }, userID: 2 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 70 }, userID: 3 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 80 }, userID: 4 }),
-            deepmerge(TestingIIDXSPScorePB, { scoreData: { percent: 40 }, userID: 5 }),
-        ]);
-
-        let { outOf, ranking } = await GetRankingInfo(chartID, 1, 0);
-
-        t.equal(outOf, 5, "Should correctly determine we're out of 5 scores.");
-        t.equal(ranking, 5, "Should correctly determine we're #5/5.");
-
-        t.end();
-    });
-    t.end();
-});
-
 t.test("#CreatePBDoc", (t) => {
     t.beforeEach(ResetDBState);
     t.beforeEach(() => {
@@ -105,10 +30,10 @@ t.test("#CreatePBDoc", (t) => {
         chartID,
         userID: 1,
         songID: 1,
-        outOf: 1,
-        ranking: 1,
+        // rankingInfo -- is not present because it is not added until post-processing.
         highlight: false,
         isPrimary: true,
+        timeAchieved: null,
         game: "iidx",
         playtype: "SP",
         comments: [],

server/src/score-import/framework/pb/create-pb-doc.ts

@@ -1,8 +1,10 @@
 import db from "../../../db/db";
-import { integer, ScoreDocument, PBScoreDocument } from "kamaitachi-common";
+import { integer, PBScoreDocument, ScoreDocument } from "kamaitachi-common";
 
 import { KtLogger } from "../../../types";
 import { IIDXMergeFn } from "./game-specific-merge";
+import { PBScoreDocumentNoRank } from "./process-pbs";
+import { BulkWriteUpdateOneOperation } from ".pnpm/@types+mongodb@3.6.12/node_modules/@types/mongodb";
 
 export async function CreatePBDoc(userID: integer, chartID: string, logger: KtLogger) {
     let scorePB = await db.scores.findOne(
@@ -45,44 +47,60 @@ export async function CreatePBDoc(userID: integer, chartID: string, logger: KtLo
         return;
     }
 
+    // finally, return our full pbDoc, that does NOT have the ranking props.
+    // (We will add those later)
     return pbDoc;
 }
 
-export async function GetRankingInfo(
-    chartID: string,
-    userID: integer,
-    percent: number
-): Promise<{ outOf: number; ranking: number }> {
-    let res = await db["score-pbs"].aggregate([
-        // exclude the requesting user because we cannot know whether they already have a pb on this chart
-        // or not - this means we can exec the same logic regardless of whether they already have a pb or not.
-        { $match: { chartID, userID: { $ne: userID } } },
+/**
+ * Updates users' rankings on a given chart.
+ */
+export async function UpdateChartRanking(chartID: string) {
+    let scores = await db["score-pbs"].find(
+        { chartID },
         {
-            $group: {
-                _id: null,
-                outOf: { $sum: 1 },
-                ranking: { $sum: { $cond: [{ $gte: ["$scoreData.percent", percent] }, 1, 0] } },
+            sort: {
+                "scoreData.percent": -1,
             },
-        },
-        // { $project: { outOf: 1, ranking: 1 } },
-    ]);
+        }
+    );
 
-    if (!res[0]) {
-        return { outOf: 1, ranking: 1 };
+    let bwrite: BulkWriteUpdateOneOperation<PBScoreDocument>[] = [];
+
+    let rank = 0;
+    // lazy sentinel value
+    let lastScorePercent = -Infinity;
+
+    for (let i = 0; i < scores.length; i++) {
+        let score = scores[i];
+
+        if (lastScorePercent !== score.scoreData.percent) {
+            rank++;
+            // doesn't matter whether this is inside or outside the loop
+            lastScorePercent = score.scoreData.percent;
+        }
+
+        bwrite.push({
+            updateOne: {
+                filter: { chartID: score.chartID, userID: score.userID },
+                update: {
+                    $set: {
+                        rankingInfo: {
+                            rank,
+                            outOf: scores.length,
+                        },
+                    },
+                },
+            },
+        });
     }
 
-    let { outOf, ranking } = res[0];
-
-    // add one to both stats to account for not including the requesting user
-    // if the field is undefined, there's no other scores to compare to.
-    outOf++;
-    ranking++;
-
-    return { outOf, ranking };
+    await db["score-pbs"].bulkWrite(bwrite, { ordered: false });
 }
 
 // Explicit acknowledgement that typing this properly simply takes too much time
 // This is a function that is aptly described below when you see how its called.
+// They return true on success, false on failure, and mutate their arguments.
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const GAME_SPECIFIC_MERGE_FNS: Record<string, any> = {
     iidx: IIDXMergeFn,
@@ -93,25 +111,29 @@ async function MergeScoreLampIntoPB(
     scorePB: ScoreDocument,
     lampPB: ScoreDocument,
     logger: KtLogger
-): Promise<PBScoreDocument | void> {
-    let { outOf, ranking } = await GetRankingInfo(
-        scorePB.chartID,
-        userID,
-        scorePB.scoreData.percent
+): Promise<PBScoreDocumentNoRank | void> {
+    // since time cannot be negative, this is a rough hack
+    // to resolve nullable timeAchieveds without hitting NaN.
+    let timeAchieved: number | null = Math.max(
+        scorePB.timeAchieved ?? -1,
+        lampPB.timeAchieved ?? -1
     );
 
-    const pbDoc: PBScoreDocument = {
+    if (timeAchieved === -1) {
+        timeAchieved = null;
+    }
+
+    const pbDoc: PBScoreDocumentNoRank = {
         composedFrom: {
             scorePB: scorePB.scoreID,
             lampPB: lampPB.scoreID,
         },
         chartID: scorePB.chartID,
         comments: [scorePB.comment, lampPB.comment].filter((e) => e !== null) as string[],
-        userID: scorePB.userID,
+        userID,
         songID: scorePB.songID,
-        outOf,
-        ranking,
         highlight: scorePB.highlight || lampPB.highlight,
+        timeAchieved,
         game: scorePB.game,
         playtype: scorePB.playtype,
         isPrimary: scorePB.isPrimary,
@@ -139,8 +161,7 @@ async function MergeScoreLampIntoPB(
 
         // If the mergeFn returns false, this means something has gone
         // rather wrong. We just return undefined here, which in turn
-        // tells our calling code to skip this PB. This typically results in a
-        // severe-level warning
+        // tells our calling code to skip this PB entirely.
         if (success === false) {
             return;
         }

server/src/score-import/framework/pb/process-pbs.ts

@@ -1,7 +1,9 @@
 import { integer, PBScoreDocument } from "kamaitachi-common";
 import db from "../../../db/db";
 import { KtLogger } from "../../../types";
-import { CreatePBDoc } from "./create-pb-doc";
+import { CreatePBDoc, UpdateChartRanking } from "./create-pb-doc";
+
+export type PBScoreDocumentNoRank = Omit<PBScoreDocument, "rankingData">;
 
 export async function ProcessPBs(
     userID: integer,
@@ -9,7 +11,7 @@ export async function ProcessPBs(
     logger: KtLogger
 ): Promise<void> {
     if (chartIDs.size === 0) {
-        return; // ez
+        return;
     }
 
     let promises = [];
@@ -18,21 +20,27 @@ export async function ProcessPBs(
         promises.push(CreatePBDoc(userID, chartID, logger));
     }
 
-    let pbDocs = (await Promise.all(promises)).filter((e) => !!e) as PBScoreDocument[];
+    let pbDocsReturn = await Promise.all(promises);
 
-    if (pbDocs.length === 0) {
-        let toStr = "";
-        for (const c of chartIDs) {
-            toStr += `${c},`;
+    let pbDocs: PBScoreDocumentNoRank[] = [];
+
+    for (const doc of pbDocsReturn) {
+        if (!doc) {
+            continue;
         }
 
-        logger.warn(
-            `Skipping PB processing as pbDocs is an empty array. This was probably caused by a previous severe-level warning.`,
-            { userID, chartIDs: toStr }
-        );
+        pbDocs.push(doc);
+    }
+
+    if (pbDocsReturn.length === 0) {
         return;
     }
 
+    // so here's the kinda awkward part - for the time between this operation
+    // and the next one - THE SCORE PBS ARE IN THE DATABASE WITHOUT RANKINGINFO.
+    // this *is* bad behaviour, but I don't have a nice way to fix it.
+    // This should be fixed in the future to avoid crashes between these two
+    // calls - but that is unlikely.
     await db["score-pbs"].bulkWrite(
         pbDocs.map((e) => ({
             updateOne: {
@@ -40,9 +48,15 @@ export async function ProcessPBs(
                 update: { $set: e },
                 upsert: true,
             },
-        }))
+        })),
+        {
+            ordered: false,
+        }
     );
 
-    // originally we returned nUpserted from this function, but it's not
-    // very useful to anyone, tbh.
+    // now that everything has been updated or inserted, we can refresh
+    // the chart rankings.
+    await Promise.all(pbDocs.map((e) => UpdateChartRanking(e.chartID)));
+
+    // and we're done!
 }