mirror of
https://github.com/monkeytypegame/monkeytype.git
synced 2025-09-06 22:56:49 +08:00
refactor: result migration and tests cleanup (@miodec) (#6929)
move all legacy test value migration into a central place, update the tests to only check if that was called, test replaceLegacyValues
This commit is contained in:
Jack
GitHub
parent
4d46c62982
commit
7487e53c67
6 changed files with 215 additions and 182 deletions
|
|
@ -3,6 +3,7 @@ import * as ResultDal from "../../../src/dal/result";
|
|||
import { ObjectId } from "mongodb";
|
||||
import * as UserDal from "../../../src/dal/user";
|
||||
import { DBResult } from "../../../src/utils/result";
|
||||
import * as ResultUtils from "../../../src/utils/result";
|
||||
|
||||
let uid: string;
|
||||
const timestamp = Date.now() - 60000;
|
||||
|
|
@ -64,11 +65,14 @@ async function createDummyData(
|
|||
}
|
||||
}
|
||||
describe("ResultDal", () => {
|
||||
const replaceLegacyValuesMock = vi.spyOn(ResultUtils, "replaceLegacyValues");
|
||||
|
||||
beforeEach(() => {
|
||||
uid = new ObjectId().toHexString();
|
||||
});
|
||||
afterEach(async () => {
|
||||
if (uid) await ResultDal.deleteAll(uid);
|
||||
replaceLegacyValuesMock.mockClear();
|
||||
});
|
||||
describe("getResults", () => {
|
||||
it("should read lastest 10 results ordered by timestamp", async () => {
|
||||
|
|
@ -135,84 +139,52 @@ describe("ResultDal", () => {
|
|||
expect(it.tags).toContain("old");
|
||||
});
|
||||
});
|
||||
it("should convert legacy values", async () => {
|
||||
it("should call replaceLegacyValues", async () => {
|
||||
//GIVEN
|
||||
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
|
||||
await createDummyData(uid, 1);
|
||||
|
||||
//WHEN
|
||||
const results = await ResultDal.getResults(uid);
|
||||
await ResultDal.getResults(uid);
|
||||
|
||||
//THEN
|
||||
expect(results[0]?.funbox).toEqual(["58008", "read_ahead"]);
|
||||
expect(replaceLegacyValuesMock).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
describe("getResult", () => {
|
||||
it("should convert legacy values", async () => {
|
||||
it("should call replaceLegacyValues", async () => {
|
||||
//GIVEN
|
||||
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
|
||||
await createDummyData(uid, 1);
|
||||
const resultId = (await ResultDal.getLastResult(uid))._id.toHexString();
|
||||
|
||||
//WHEN
|
||||
const result = await ResultDal.getResult(uid, resultId);
|
||||
await ResultDal.getResult(uid, resultId);
|
||||
|
||||
//THEN
|
||||
expect(result?.funbox).toEqual(["58008", "read_ahead"]);
|
||||
expect(replaceLegacyValuesMock).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
describe("getLastResult", () => {
|
||||
it("should convert legacy values", async () => {
|
||||
it("should call replaceLegacyValues", async () => {
|
||||
//GIVEN
|
||||
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
|
||||
await createDummyData(uid, 1);
|
||||
|
||||
//WHEN
|
||||
const result = await ResultDal.getLastResult(uid);
|
||||
await ResultDal.getLastResult(uid);
|
||||
|
||||
//THEN
|
||||
expect(result?.funbox).toEqual(["58008", "read_ahead"]);
|
||||
expect(replaceLegacyValuesMock).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
describe("getResultByTimestamp", () => {
|
||||
it("should convert legacy values", async () => {
|
||||
it("should call replaceLegacyValues", async () => {
|
||||
//GIVEN
|
||||
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
|
||||
await createDummyData(uid, 1);
|
||||
|
||||
//WHEN
|
||||
const result = await ResultDal.getResultByTimestamp(uid, timestamp);
|
||||
await ResultDal.getResultByTimestamp(uid, timestamp);
|
||||
|
||||
//THEN
|
||||
expect(result?.funbox).toEqual(["58008", "read_ahead"]);
|
||||
});
|
||||
});
|
||||
describe("converts legacy values", () => {
|
||||
it("should convert funbox as string", async () => {
|
||||
//GIVEN
|
||||
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
|
||||
|
||||
//WHEN
|
||||
const read = await ResultDal.getLastResult(uid);
|
||||
|
||||
//THEN
|
||||
expect(read.funbox).toEqual(["58008", "read_ahead"]);
|
||||
});
|
||||
it("should convert funbox 'none'", async () => {
|
||||
//GIVEN
|
||||
await createDummyData(uid, 1, { funbox: "none" as any });
|
||||
|
||||
//WHEN
|
||||
const read = await ResultDal.getLastResult(uid);
|
||||
|
||||
//THEN
|
||||
expect(read.funbox).toEqual([]);
|
||||
});
|
||||
it("should not convert funbox as array", async () => {
|
||||
//GIVEN
|
||||
await createDummyData(uid, 1, { funbox: ["58008", "read_ahead"] });
|
||||
|
||||
//WHEN
|
||||
const read = await ResultDal.getLastResult(uid);
|
||||
|
||||
//THEN
|
||||
expect(read.funbox).toEqual(["58008", "read_ahead"]);
|
||||
expect(replaceLegacyValuesMock).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -271,44 +271,6 @@ describe("result controller test", () => {
|
|||
validationErrors: ["Unrecognized key(s) in object: 'extra'"],
|
||||
});
|
||||
});
|
||||
it("should get results with legacy values", async () => {
|
||||
//GIVEN
|
||||
const resultOne = givenDbResult(uid, {
|
||||
charStats: undefined,
|
||||
incorrectChars: 5,
|
||||
correctChars: 12,
|
||||
});
|
||||
const resultTwo = givenDbResult(uid, {
|
||||
charStats: undefined,
|
||||
incorrectChars: 7,
|
||||
correctChars: 15,
|
||||
});
|
||||
resultMock.mockResolvedValue([resultOne, resultTwo]);
|
||||
|
||||
//WHEN
|
||||
const { body } = await mockApp
|
||||
.get("/results")
|
||||
.set("Authorization", `Bearer ${uid}`)
|
||||
.send()
|
||||
.expect(200);
|
||||
|
||||
//THEN
|
||||
|
||||
expect(body.message).toEqual("Results retrieved");
|
||||
expect(body.data[0]).toMatchObject({
|
||||
_id: resultOne._id.toHexString(),
|
||||
charStats: [12, 5, 0, 0],
|
||||
});
|
||||
expect(body.data[0]).not.toHaveProperty("correctChars");
|
||||
expect(body.data[0]).not.toHaveProperty("incorrectChars");
|
||||
|
||||
expect(body.data[1]).toMatchObject({
|
||||
_id: resultTwo._id.toHexString(),
|
||||
charStats: [15, 7, 0, 0],
|
||||
});
|
||||
expect(body.data[1]).not.toHaveProperty("correctChars");
|
||||
expect(body.data[1]).not.toHaveProperty("incorrectChars");
|
||||
});
|
||||
it("should be rate limited", async () => {
|
||||
await expect(
|
||||
mockApp.get("/results").set("Authorization", `Bearer ${uid}`)
|
||||
|
|
@ -362,31 +324,6 @@ describe("result controller test", () => {
|
|||
.send()
|
||||
.expect(200);
|
||||
});
|
||||
it("should get last result with legacy values", async () => {
|
||||
//GIVEN
|
||||
const result = givenDbResult(uid, {
|
||||
charStats: undefined,
|
||||
incorrectChars: 5,
|
||||
correctChars: 12,
|
||||
});
|
||||
getResultMock.mockResolvedValue(result);
|
||||
|
||||
//WHEN
|
||||
const { body } = await mockApp
|
||||
.get(`/results/id/${result._id}`)
|
||||
.set("Authorization", `Bearer ${uid}`)
|
||||
.send()
|
||||
.expect(200);
|
||||
|
||||
//THEN
|
||||
expect(body.message).toEqual("Result retrieved");
|
||||
expect(body.data).toMatchObject({
|
||||
_id: result._id.toHexString(),
|
||||
charStats: [12, 5, 0, 0],
|
||||
});
|
||||
expect(body.data).not.toHaveProperty("correctChars");
|
||||
expect(body.data).not.toHaveProperty("incorrectChars");
|
||||
});
|
||||
it("should rate limit get result with ape key", async () => {
|
||||
//GIVEN
|
||||
const result = givenDbResult(uid, {
|
||||
|
|
@ -443,31 +380,6 @@ describe("result controller test", () => {
|
|||
.send()
|
||||
.expect(200);
|
||||
});
|
||||
it("should get last result with legacy values", async () => {
|
||||
//GIVEN
|
||||
const result = givenDbResult(uid, {
|
||||
charStats: undefined,
|
||||
incorrectChars: 5,
|
||||
correctChars: 12,
|
||||
});
|
||||
getLastResultMock.mockResolvedValue(result);
|
||||
|
||||
//WHEN
|
||||
const { body } = await mockApp
|
||||
.get("/results/last")
|
||||
.set("Authorization", `Bearer ${uid}`)
|
||||
.send()
|
||||
.expect(200);
|
||||
|
||||
//THEN
|
||||
expect(body.message).toEqual("Result retrieved");
|
||||
expect(body.data).toMatchObject({
|
||||
_id: result._id.toHexString(),
|
||||
charStats: [12, 5, 0, 0],
|
||||
});
|
||||
expect(body.data).not.toHaveProperty("correctChars");
|
||||
expect(body.data).not.toHaveProperty("incorrectChars");
|
||||
});
|
||||
it("should rate limit get last result with ape key", async () => {
|
||||
//GIVEN
|
||||
const result = givenDbResult(uid, {
|
||||
|
|
|
|||
154
backend/__tests__/utils/result.spec.ts
Normal file
154
backend/__tests__/utils/result.spec.ts
Normal file
|
|
@ -0,0 +1,154 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { replaceLegacyValues, DBResult } from "../../src/utils/result";
|
||||
|
||||
describe("Result Utils", () => {
|
||||
describe("replaceLegacyValues", () => {
|
||||
describe("legacy charStats conversion", () => {
|
||||
it.each([
|
||||
{
|
||||
description:
|
||||
"should convert correctChars and incorrectChars to charStats",
|
||||
correctChars: 95,
|
||||
incorrectChars: 5,
|
||||
expectedCharStats: [95, 5, 0, 0],
|
||||
},
|
||||
{
|
||||
description: "should handle zero values for legacy chars",
|
||||
correctChars: 0,
|
||||
incorrectChars: 0,
|
||||
expectedCharStats: [0, 0, 0, 0],
|
||||
},
|
||||
{
|
||||
description: "should handle large values for legacy chars",
|
||||
correctChars: 9999,
|
||||
incorrectChars: 1234,
|
||||
expectedCharStats: [9999, 1234, 0, 0],
|
||||
},
|
||||
])(
|
||||
"$description",
|
||||
({ correctChars, incorrectChars, expectedCharStats }) => {
|
||||
const resultWithLegacyChars: DBResult = {
|
||||
correctChars,
|
||||
incorrectChars,
|
||||
} as any;
|
||||
|
||||
const result = replaceLegacyValues(resultWithLegacyChars);
|
||||
|
||||
expect(result.charStats).toEqual(expectedCharStats);
|
||||
expect(result.correctChars).toBeUndefined();
|
||||
expect(result.incorrectChars).toBeUndefined();
|
||||
}
|
||||
);
|
||||
|
||||
it("should prioritise charStats when legacy data exists", () => {
|
||||
const resultWithBothFormats: DBResult = {
|
||||
charStats: [80, 4, 2, 1],
|
||||
correctChars: 95,
|
||||
incorrectChars: 5,
|
||||
} as any;
|
||||
|
||||
const result = replaceLegacyValues(resultWithBothFormats);
|
||||
|
||||
// Should convert legacy values and overwrite existing charStats
|
||||
expect(result.charStats).toEqual([80, 4, 2, 1]);
|
||||
// Legacy values should be removed after conversion
|
||||
expect(result.correctChars).toBeUndefined();
|
||||
expect(result.incorrectChars).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
description:
|
||||
"should not convert when only one legacy property is present",
|
||||
input: { correctChars: 95 },
|
||||
expectedCharStats: undefined,
|
||||
expectedCorrectChars: 95,
|
||||
expectedIncorrectChars: undefined,
|
||||
},
|
||||
{
|
||||
description: "should not convert when only incorrectChars is present",
|
||||
input: { incorrectChars: 5 },
|
||||
expectedCharStats: undefined,
|
||||
expectedCorrectChars: undefined,
|
||||
expectedIncorrectChars: 5,
|
||||
},
|
||||
])(
|
||||
"$description",
|
||||
({
|
||||
input,
|
||||
expectedCharStats,
|
||||
expectedCorrectChars,
|
||||
expectedIncorrectChars,
|
||||
}) => {
|
||||
const result = replaceLegacyValues(input as any);
|
||||
|
||||
// Should not convert since both properties are required
|
||||
expect(result.charStats).toBe(expectedCharStats);
|
||||
expect(result.correctChars).toBe(expectedCorrectChars);
|
||||
expect(result.incorrectChars).toBe(expectedIncorrectChars);
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
describe("legacy funbox conversion", () => {
|
||||
it.each([
|
||||
{
|
||||
description: "should convert string funbox to array",
|
||||
input: "memory#mirror",
|
||||
expected: ["memory", "mirror"],
|
||||
},
|
||||
{
|
||||
description: "should convert single funbox string to array",
|
||||
input: "memory",
|
||||
expected: ["memory"],
|
||||
},
|
||||
{
|
||||
description: "should convert 'none' funbox to empty array",
|
||||
input: "none",
|
||||
expected: [],
|
||||
},
|
||||
{
|
||||
description: "should handle complex funbox combinations",
|
||||
input: "memory#mirror#arrows#58008",
|
||||
expected: ["memory", "mirror", "arrows", "58008"],
|
||||
},
|
||||
])("$description", ({ input, expected }) => {
|
||||
const resultWithStringFunbox: DBResult = {
|
||||
funbox: input as any,
|
||||
} as any;
|
||||
|
||||
const result = replaceLegacyValues(resultWithStringFunbox);
|
||||
|
||||
expect(result.funbox).toEqual(expected);
|
||||
});
|
||||
});
|
||||
|
||||
it("should convert all legacy data at once", () => {
|
||||
const resultWithBothLegacy: DBResult = {
|
||||
correctChars: 100,
|
||||
incorrectChars: 8,
|
||||
funbox: "memory#mirror" as any,
|
||||
} as any;
|
||||
|
||||
const result = replaceLegacyValues(resultWithBothLegacy);
|
||||
|
||||
expect(result.charStats).toEqual([100, 8, 0, 0]);
|
||||
expect(result.correctChars).toBeUndefined();
|
||||
expect(result.incorrectChars).toBeUndefined();
|
||||
expect(result.funbox).toEqual(["memory", "mirror"]);
|
||||
});
|
||||
|
||||
describe("no legacy values", () => {
|
||||
it("should return result unchanged when no legacy values present", () => {
|
||||
const modernResult: DBResult = {
|
||||
charStats: [95, 5, 2, 1],
|
||||
funbox: ["memory"],
|
||||
} as any;
|
||||
|
||||
const result = replaceLegacyValues(modernResult);
|
||||
|
||||
expect(result).toEqual(modernResult);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -1,6 +1,10 @@
|
|||
import * as ResultDAL from "../../dal/result";
|
||||
import * as PublicDAL from "../../dal/public";
|
||||
import { isDevEnvironment, replaceObjectId } from "../../utils/misc";
|
||||
import {
|
||||
isDevEnvironment,
|
||||
replaceObjectId,
|
||||
replaceObjectIds,
|
||||
} from "../../utils/misc";
|
||||
import objectHash from "object-hash";
|
||||
import Logger from "../../utils/logger";
|
||||
import "dotenv/config";
|
||||
|
|
@ -26,11 +30,7 @@ import _, { omit } from "lodash";
|
|||
import * as WeeklyXpLeaderboard from "../../services/weekly-xp-leaderboard";
|
||||
import { UAParser } from "ua-parser-js";
|
||||
import { canFunboxGetPb } from "../../utils/pb";
|
||||
import {
|
||||
buildDbResult,
|
||||
DBResult,
|
||||
replaceLegacyValues,
|
||||
} from "../../utils/result";
|
||||
import { buildDbResult } from "../../utils/result";
|
||||
import { Configuration } from "@monkeytype/schemas/configuration";
|
||||
import { addImportantLog, addLog } from "../../dal/logs";
|
||||
import {
|
||||
|
|
@ -47,11 +47,9 @@ import {
|
|||
import {
|
||||
CompletedEvent,
|
||||
KeyStats,
|
||||
Result,
|
||||
PostResultResponse,
|
||||
XpBreakdown,
|
||||
} from "@monkeytype/schemas/results";
|
||||
import { Mode } from "@monkeytype/schemas/shared";
|
||||
import {
|
||||
isSafeNumber,
|
||||
mapRange,
|
||||
|
|
@ -133,7 +131,8 @@ export async function getResults(
|
|||
},
|
||||
uid
|
||||
);
|
||||
return new MonkeyResponse("Results retrieved", results.map(convertResult));
|
||||
|
||||
return new MonkeyResponse("Results retrieved", replaceObjectIds(results));
|
||||
}
|
||||
|
||||
export async function getResultById(
|
||||
|
|
@ -143,7 +142,7 @@ export async function getResultById(
|
|||
const { resultId } = req.params;
|
||||
|
||||
const result = await ResultDAL.getResult(uid, resultId);
|
||||
return new MonkeyResponse("Result retrieved", convertResult(result));
|
||||
return new MonkeyResponse("Result retrieved", replaceObjectId(result));
|
||||
}
|
||||
|
||||
export async function getLastResult(
|
||||
|
|
@ -151,7 +150,7 @@ export async function getLastResult(
|
|||
): Promise<GetLastResultResponse> {
|
||||
const { uid } = req.ctx.decodedToken;
|
||||
const result = await ResultDAL.getLastResult(uid);
|
||||
return new MonkeyResponse("Result retrieved", convertResult(result));
|
||||
return new MonkeyResponse("Result retrieved", replaceObjectId(result));
|
||||
}
|
||||
|
||||
export async function deleteAll(req: MonkeyRequest): Promise<MonkeyResponse> {
|
||||
|
|
@ -841,7 +840,3 @@ async function calculateXp(
|
|||
breakdown,
|
||||
};
|
||||
}
|
||||
|
||||
function convertResult(db: DBResult): Result<Mode> {
|
||||
return replaceObjectId(replaceLegacyValues(db));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,10 +7,8 @@ import {
|
|||
} from "mongodb";
|
||||
import MonkeyError from "../utils/error";
|
||||
import * as db from "../init/db";
|
||||
|
||||
import { getUser, getTags } from "./user";
|
||||
import { DBResult } from "../utils/result";
|
||||
import { FunboxName } from "@monkeytype/schemas/configs";
|
||||
import { DBResult, replaceLegacyValues } from "../utils/result";
|
||||
import { tryCatch } from "@monkeytype/util/trycatch";
|
||||
|
||||
export const getResultCollection = (): Collection<DBResult> =>
|
||||
|
|
@ -66,7 +64,7 @@ export async function getResult(uid: string, id: string): Promise<DBResult> {
|
|||
uid,
|
||||
});
|
||||
if (!result) throw new MonkeyError(404, "Result not found");
|
||||
return convert(result);
|
||||
return replaceLegacyValues(result);
|
||||
}
|
||||
|
||||
export async function getLastResult(uid: string): Promise<DBResult> {
|
||||
|
|
@ -76,7 +74,7 @@ export async function getLastResult(uid: string): Promise<DBResult> {
|
|||
);
|
||||
|
||||
if (lastResult === null) throw new MonkeyError(404, "No last result found");
|
||||
return convert(lastResult);
|
||||
return replaceLegacyValues(lastResult);
|
||||
}
|
||||
|
||||
export async function getLastResultTimestamp(uid: string): Promise<number> {
|
||||
|
|
@ -97,7 +95,8 @@ export async function getResultByTimestamp(
|
|||
timestamp: number
|
||||
): Promise<DBResult | null> {
|
||||
const result = await getResultCollection().findOne({ uid, timestamp });
|
||||
return convert(result);
|
||||
if (result === null) return null;
|
||||
return replaceLegacyValues(result);
|
||||
}
|
||||
|
||||
type GetResultsOpts = {
|
||||
|
|
@ -140,26 +139,5 @@ export async function getResults(
|
|||
|
||||
const results = await query.toArray();
|
||||
if (results === undefined) throw new MonkeyError(404, "Result not found");
|
||||
return convert(results);
|
||||
}
|
||||
|
||||
function convert<T extends DBResult | DBResult[] | null>(results: T): T {
|
||||
if (results === null) return results;
|
||||
|
||||
const migrate = (result: DBResult): DBResult => {
|
||||
if (typeof result.funbox === "string") {
|
||||
if (result.funbox === "none") {
|
||||
result.funbox = [];
|
||||
} else {
|
||||
result.funbox = (result.funbox as string).split("#") as FunboxName[];
|
||||
}
|
||||
}
|
||||
return result;
|
||||
};
|
||||
|
||||
if (Array.isArray(results)) {
|
||||
return results.map(migrate) as T;
|
||||
} else {
|
||||
return migrate(results) as T;
|
||||
}
|
||||
return results.map(replaceLegacyValues);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { CompletedEvent, Result } from "@monkeytype/schemas/results";
|
|||
import { Mode } from "@monkeytype/schemas/shared";
|
||||
import { ObjectId } from "mongodb";
|
||||
import { WithObjectId } from "./misc";
|
||||
import { FunboxName } from "@monkeytype/schemas/configs";
|
||||
|
||||
export type DBResult = WithObjectId<Result<Mode>> & {
|
||||
//legacy values
|
||||
|
|
@ -77,9 +78,30 @@ export function replaceLegacyValues(result: DBResult): DBResult {
|
|||
result.correctChars !== undefined &&
|
||||
result.incorrectChars !== undefined
|
||||
) {
|
||||
result.charStats = [result.correctChars, result.incorrectChars, 0, 0];
|
||||
delete result.correctChars;
|
||||
delete result.incorrectChars;
|
||||
//super edge case but just in case
|
||||
if (result.charStats !== undefined) {
|
||||
result.charStats = [
|
||||
result.charStats[0],
|
||||
result.charStats[1],
|
||||
result.charStats[2],
|
||||
result.charStats[3],
|
||||
];
|
||||
delete result.correctChars;
|
||||
delete result.incorrectChars;
|
||||
} else {
|
||||
result.charStats = [result.correctChars, result.incorrectChars, 0, 0];
|
||||
delete result.correctChars;
|
||||
delete result.incorrectChars;
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof result.funbox === "string") {
|
||||
if (result.funbox === "none") {
|
||||
result.funbox = [];
|
||||
} else {
|
||||
result.funbox = (result.funbox as string).split("#") as FunboxName[];
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue