Skip to content

Commit 86383cf

Browse files
byseif21Miodec
andauthored
refactor(backend): improve redis and json.parse type safety with zod (@byseif21, @Miodec) (monkeytypegame#6481)
### Description refactored backend files to enhance type safety and reliability using Zod validation and Redis instead of JSON.parse , I tried to avoid the files that isn't necessary tho so I hope I don't miss any or included unnecessary ones!! didn't fully test only verified code compilation and partial tests without Redis!!. Should Close monkeytypegame#5881 Related to monkeytypegame#6207 --------- Co-authored-by: Miodec <[email protected]>
1 parent d863e8d commit 86383cf

File tree

9 files changed

+213
-103
lines changed

9 files changed

+213
-103
lines changed

backend/src/api/controllers/result.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,9 +606,9 @@ export async function addResult(
606606
badgeId: selectedBadgeId,
607607
lastActivityTimestamp: Date.now(),
608608
isPremium,
609+
timeTypedSeconds: totalDurationTypedSeconds,
609610
},
610611
xpGained: xpGained.xp,
611-
timeTypedSeconds: totalDurationTypedSeconds,
612612
}
613613
);
614614
}

backend/src/dal/new-quotes.ts

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,23 @@ import MonkeyError from "../utils/error";
88
import { compareTwoStrings } from "string-similarity";
99
import { ApproveQuote, Quote } from "@monkeytype/contracts/schemas/quotes";
1010
import { WithObjectId } from "../utils/misc";
11-
12-
type JsonQuote = {
13-
text: string;
14-
britishText?: string;
15-
source: string;
16-
length: number;
17-
id: number;
18-
};
19-
20-
type QuoteData = {
21-
language: string;
22-
quotes: JsonQuote[];
23-
groups: [number, number][];
24-
};
11+
import { parseWithSchema as parseJsonWithSchema } from "@monkeytype/util/json";
12+
import { z } from "zod";
13+
14+
const JsonQuoteSchema = z.object({
15+
text: z.string(),
16+
britishText: z.string().optional(),
17+
approvedBy: z.string().optional(),
18+
source: z.string(),
19+
length: z.number(),
20+
id: z.number(),
21+
});
22+
23+
const QuoteDataSchema = z.object({
24+
language: z.string(),
25+
quotes: z.array(JsonQuoteSchema),
26+
groups: z.array(z.tuple([z.number(), z.number()])),
27+
});
2528

2629
const PATH_TO_REPO = "../../../../monkeytype-new-quotes";
2730

@@ -86,7 +89,10 @@ export async function add(
8689
let similarityScore = -1;
8790
if (existsSync(fileDir)) {
8891
const quoteFile = await readFile(fileDir);
89-
const quoteFileJSON = JSON.parse(quoteFile.toString()) as QuoteData;
92+
const quoteFileJSON = parseJsonWithSchema(
93+
quoteFile.toString(),
94+
QuoteDataSchema
95+
);
9096
quoteFileJSON.quotes.every((old) => {
9197
if (compareTwoStrings(old.text, quote.text) > 0.9) {
9298
duplicateId = old.id;
@@ -156,6 +162,7 @@ export async function approve(
156162
source: editSource ?? targetQuote.source,
157163
length: targetQuote.text.length,
158164
approvedBy: name,
165+
id: -1,
159166
};
160167
let message = "";
161168

@@ -170,7 +177,10 @@ export async function approve(
170177
await git.pull("upstream", "master");
171178
if (existsSync(fileDir)) {
172179
const quoteFile = await readFile(fileDir);
173-
const quoteObject = JSON.parse(quoteFile.toString()) as QuoteData;
180+
const quoteObject = parseJsonWithSchema(
181+
quoteFile.toString(),
182+
QuoteDataSchema
183+
);
174184
quoteObject.quotes.every((old) => {
175185
if (compareTwoStrings(old.text, quote.text) > 0.8) {
176186
throw new MonkeyError(409, "Duplicate quote");
@@ -183,7 +193,12 @@ export async function approve(
183193
}
184194
});
185195
quote.id = maxid + 1;
186-
quoteObject.quotes.push(quote as JsonQuote);
196+
197+
if (quote.id === -1) {
198+
throw new MonkeyError(500, "Failed to get max id");
199+
}
200+
201+
quoteObject.quotes.push(quote);
187202
writeFileSync(fileDir, JSON.stringify(quoteObject, null, 2));
188203
message = `Added quote to ${language}.json.`;
189204
} else {

backend/src/dal/user.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,11 +1103,7 @@ export async function updateStreak(
11031103
if (isYesterday(streak.lastResultTimestamp, streak.hourOffset ?? 0)) {
11041104
streak.length += 1;
11051105
} else if (!isToday(streak.lastResultTimestamp, streak.hourOffset ?? 0)) {
1106-
void addImportantLog(
1107-
"streak_lost",
1108-
JSON.parse(JSON.stringify(streak)) as Record<string, unknown>,
1109-
uid
1110-
);
1106+
void addImportantLog("streak_lost", streak, uid);
11111107
streak.length = 1;
11121108
}
11131109

backend/src/init/redis.ts

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,47 @@
11
import fs from "fs";
22
import _ from "lodash";
33
import { join } from "path";
4-
import IORedis from "ioredis";
4+
import IORedis, { Redis } from "ioredis";
55
import Logger from "../utils/logger";
66
import { isDevEnvironment } from "../utils/misc";
77
import { getErrorMessage } from "../utils/error";
88

9+
// Define Redis connection with custom methods for type safety
10+
export type RedisConnectionWithCustomMethods = Redis & {
11+
addResult: (
12+
keyCount: number,
13+
scoresKey: string,
14+
resultsKey: string,
15+
maxResults: number,
16+
expirationTime: number,
17+
uid: string,
18+
score: number,
19+
data: string
20+
) => Promise<number>;
21+
addResultIncrement: (
22+
keyCount: number,
23+
scoresKey: string,
24+
resultsKey: string,
25+
expirationTime: number,
26+
uid: string,
27+
score: number,
28+
data: string
29+
) => Promise<number>;
30+
getResults: (
31+
keyCount: number,
32+
scoresKey: string,
33+
resultsKey: string,
34+
minRank: number,
35+
maxRank: number,
36+
withScores: string
37+
) => Promise<[string[], string[]]>;
38+
purgeResults: (
39+
keyCount: number,
40+
uid: string,
41+
namespace: string
42+
) => Promise<void>;
43+
};
44+
945
let connection: IORedis.Redis;
1046
let connected = false;
1147

@@ -73,11 +109,11 @@ export function isConnected(): boolean {
73109
return connected;
74110
}
75111

76-
export function getConnection(): IORedis.Redis | undefined {
112+
export function getConnection(): RedisConnectionWithCustomMethods | null {
77113
const status = connection?.status;
78114
if (connection === undefined || status !== "ready") {
79-
return undefined;
115+
return null;
80116
}
81117

82-
return connection;
118+
return connection as RedisConnectionWithCustomMethods;
83119
}

backend/src/server.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ async function bootServer(port: number): Promise<Server> {
4747

4848
Logger.info("Initializing queues...");
4949
queues.forEach((queue) => {
50-
queue.init(connection);
50+
queue.init(connection ?? undefined);
5151
});
5252
Logger.success(
5353
`Queues initialized: ${queues
@@ -57,11 +57,11 @@ async function bootServer(port: number): Promise<Server> {
5757

5858
Logger.info("Initializing workers...");
5959
workers.forEach(async (worker) => {
60-
await worker(connection).run();
60+
await worker(connection ?? undefined).run();
6161
});
6262
Logger.success(
6363
`Workers initialized: ${workers
64-
.map((worker) => worker(connection).name)
64+
.map((worker) => worker(connection ?? undefined).name)
6565
.join(", ")}`
6666
);
6767
}

backend/src/services/weekly-xp-leaderboard.ts

Lines changed: 62 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,20 @@
11
import { Configuration } from "@monkeytype/contracts/schemas/configuration";
22
import * as RedisClient from "../init/redis";
33
import LaterQueue from "../queues/later-queue";
4-
import { XpLeaderboardEntry } from "@monkeytype/contracts/schemas/leaderboards";
4+
import {
5+
RedisXpLeaderboardEntry,
6+
RedisXpLeaderboardEntrySchema,
7+
RedisXpLeaderboardScore,
8+
XpLeaderboardEntry,
9+
} from "@monkeytype/contracts/schemas/leaderboards";
510
import { getCurrentWeekTimestamp } from "@monkeytype/util/date-and-time";
611
import MonkeyError from "../utils/error";
712
import { omit } from "lodash";
13+
import { parseWithSchema as parseJsonWithSchema } from "@monkeytype/util/json";
814

915
type AddResultOpts = {
10-
entry: Pick<
11-
XpLeaderboardEntry,
12-
| "uid"
13-
| "name"
14-
| "discordId"
15-
| "discordAvatar"
16-
| "badgeId"
17-
| "lastActivityTimestamp"
18-
| "isPremium"
19-
>;
20-
xpGained: number;
21-
timeTypedSeconds: number;
16+
entry: RedisXpLeaderboardEntry;
17+
xpGained: RedisXpLeaderboardScore;
2218
};
2319

2420
const weeklyXpLeaderboardLeaderboardNamespace =
@@ -59,7 +55,7 @@ export class WeeklyXpLeaderboard {
5955
weeklyXpLeaderboardConfig: Configuration["leaderboards"]["weeklyXp"],
6056
opts: AddResultOpts
6157
): Promise<number> {
62-
const { entry, xpGained, timeTypedSeconds } = opts;
58+
const { entry, xpGained } = opts;
6359

6460
const connection = RedisClient.getConnection();
6561
if (!connection || !weeklyXpLeaderboardConfig.enabled) {
@@ -89,25 +85,28 @@ export class WeeklyXpLeaderboard {
8985

9086
const currentEntryTimeTypedSeconds =
9187
currentEntry !== null
92-
? (JSON.parse(currentEntry) as { timeTypedSeconds: number | undefined })
88+
? parseJsonWithSchema(currentEntry, RedisXpLeaderboardEntrySchema)
9389
?.timeTypedSeconds
9490
: undefined;
9591

9692
const totalTimeTypedSeconds =
97-
timeTypedSeconds + (currentEntryTimeTypedSeconds ?? 0);
93+
entry.timeTypedSeconds + (currentEntryTimeTypedSeconds ?? 0);
9894

9995
const [rank] = await Promise.all([
100-
// @ts-expect-error we are doing some weird file to function mapping, thats why its any
101-
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
10296
connection.addResultIncrement(
10397
2,
10498
weeklyXpLeaderboardScoresKey,
10599
weeklyXpLeaderboardResultsKey,
106100
weeklyXpLeaderboardExpirationTimeInSeconds,
107101
entry.uid,
108102
xpGained,
109-
JSON.stringify({ ...entry, timeTypedSeconds: totalTimeTypedSeconds })
110-
) as Promise<number>,
103+
JSON.stringify(
104+
RedisXpLeaderboardEntrySchema.parse({
105+
...entry,
106+
timeTypedSeconds: totalTimeTypedSeconds,
107+
})
108+
)
109+
),
111110
LaterQueue.scheduleForNextWeek(
112111
"weekly-xp-leaderboard-results",
113112
"weekly-xp"
@@ -138,10 +137,8 @@ export class WeeklyXpLeaderboard {
138137
const { weeklyXpLeaderboardScoresKey, weeklyXpLeaderboardResultsKey } =
139138
this.getThisWeeksXpLeaderboardKeys();
140139

141-
// @ts-expect-error we are doing some weird file to function mapping, thats why its any
142-
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
143140
const [results, scores] = (await connection.getResults(
144-
2, // How many of the arguments are redis keys (https://redis.io/docs/manual/programmability/lua-api/)
141+
2,
145142
weeklyXpLeaderboardScoresKey,
146143
weeklyXpLeaderboardResultsKey,
147144
minRank,
@@ -163,14 +160,32 @@ export class WeeklyXpLeaderboard {
163160

164161
const resultsWithRanks: XpLeaderboardEntry[] = results.map(
165162
(resultJSON: string, index: number) => {
166-
//TODO parse with zod?
167-
const parsed = JSON.parse(resultJSON) as XpLeaderboardEntry;
168-
169-
return {
170-
...parsed,
171-
rank: minRank + index + 1,
172-
totalXp: parseInt(scores[index] as string, 10),
173-
};
163+
try {
164+
const parsed = parseJsonWithSchema(
165+
resultJSON,
166+
RedisXpLeaderboardEntrySchema
167+
);
168+
const scoreValue = scores[index];
169+
170+
if (typeof scoreValue !== "string") {
171+
throw new Error(
172+
`Invalid score value at index ${index}: ${scoreValue}`
173+
);
174+
}
175+
176+
return {
177+
...parsed,
178+
rank: minRank + index + 1,
179+
totalXp: parseInt(scoreValue, 10),
180+
};
181+
} catch (error) {
182+
throw new MonkeyError(
183+
500,
184+
`Failed to parse leaderboard entry at index ${index}: ${
185+
error instanceof Error ? error.message : String(error)
186+
}`
187+
);
188+
}
174189
}
175190
);
176191

@@ -187,15 +202,12 @@ export class WeeklyXpLeaderboard {
187202
): Promise<XpLeaderboardEntry | null> {
188203
const connection = RedisClient.getConnection();
189204
if (!connection || !weeklyXpLeaderboardConfig.enabled) {
190-
throw new MonkeyError(500, "Redis connnection is unavailable");
205+
throw new MonkeyError(500, "Redis connection is unavailable");
191206
}
192207

193208
const { weeklyXpLeaderboardScoresKey, weeklyXpLeaderboardResultsKey } =
194209
this.getThisWeeksXpLeaderboardKeys();
195210

196-
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
197-
connection.set;
198-
199211
const [[, rank], [, totalXp], [, _count], [, result]] = (await connection
200212
.multi()
201213
.zrevrank(weeklyXpLeaderboardScoresKey, uid)
@@ -213,11 +225,21 @@ export class WeeklyXpLeaderboard {
213225
return null;
214226
}
215227

216-
//TODO parse with zod?
217-
const parsed = JSON.parse((result as string) ?? "null") as Omit<
218-
XpLeaderboardEntry,
219-
"rank" | "count" | "totalXp"
220-
>;
228+
// safely parse the result with error handling
229+
let parsed: RedisXpLeaderboardEntry;
230+
try {
231+
parsed = parseJsonWithSchema(
232+
result ?? "null",
233+
RedisXpLeaderboardEntrySchema
234+
);
235+
} catch (error) {
236+
throw new MonkeyError(
237+
500,
238+
`Failed to parse leaderboard entry: ${
239+
error instanceof Error ? error.message : String(error)
240+
}`
241+
);
242+
}
221243

222244
return {
223245
...parsed,
@@ -261,8 +283,6 @@ export async function purgeUserFromXpLeaderboards(
261283
return;
262284
}
263285

264-
// @ts-expect-error we are doing some weird file to function mapping, thats why its any
265-
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
266286
await connection.purgeResults(
267287
0,
268288
uid,

0 commit comments

Comments
 (0)