Skip to content

Commit d566ba6

Browse files
fehmerfehmer
authored
perf: optimize database access for UserDal (@fehmer) (#5544)
* impr: optimize database access for UserDal (@fehmer) * rename getPartial to getPartialUser
1 parent 63b9f76 commit d566ba6

6 files changed

Lines changed: 212 additions & 64 deletions

File tree

‎backend/__tests__/api/controllers/user.spec.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ describe("user controller test", () => {
195195
});
196196
});
197197
describe("getTestActivity", () => {
198-
const getUserMock = vi.spyOn(UserDal, "getUser");
198+
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
199199
afterAll(() => {
200200
getUserMock.mockReset();
201201
});
@@ -303,7 +303,7 @@ describe("user controller test", () => {
303303
});
304304

305305
describe("toggle ban", () => {
306-
const getUserMock = vi.spyOn(UserDal, "getUser");
306+
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
307307
const setBannedMock = vi.spyOn(UserDal, "setBanned");
308308
const georgeUserBannedMock = vi.spyOn(GeorgeQueue, "userBanned");
309309
const isAdminMock = vi.spyOn(AdminUuids, "isAdmin");
@@ -340,7 +340,10 @@ describe("user controller test", () => {
340340
.expect(200);
341341

342342
//THEN
343-
expect(getUserMock).toHaveBeenLastCalledWith(uid, "toggle ban");
343+
expect(getUserMock).toHaveBeenLastCalledWith(uid, "toggle ban", [
344+
"banned",
345+
"discordId",
346+
]);
344347
expect(setBannedMock).toHaveBeenCalledWith(uid, true);
345348
expect(georgeUserBannedMock).toHaveBeenCalledWith("discordId", true);
346349
});
@@ -392,7 +395,10 @@ describe("user controller test", () => {
392395
.expect(200);
393396

394397
//THEN
395-
expect(getUserMock).toHaveBeenLastCalledWith(uid, "toggle ban");
398+
expect(getUserMock).toHaveBeenLastCalledWith(uid, "toggle ban", [
399+
"banned",
400+
"discordId",
401+
]);
396402
expect(setBannedMock).toHaveBeenCalledWith(uid, false);
397403
expect(georgeUserBannedMock).toHaveBeenCalledWith("discordId", false);
398404
});
@@ -425,7 +431,7 @@ describe("user controller test", () => {
425431
});
426432

427433
describe("delete user", () => {
428-
const getUserMock = vi.spyOn(UserDal, "getUser");
434+
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
429435
const deleteUserMock = vi.spyOn(UserDal, "deleteUser");
430436
const firebaseDeleteUserMock = vi.spyOn(AuthUtils, "deleteUser");
431437
const deleteAllApeKeysMock = vi.spyOn(ApeKeys, "deleteAllApeKeys");
@@ -537,7 +543,7 @@ describe("user controller test", () => {
537543
});
538544
});
539545
describe("link discord", () => {
540-
const getUserMock = vi.spyOn(UserDal, "getUser");
546+
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
541547
const isDiscordIdAvailableMock = vi.spyOn(UserDal, "isDiscordIdAvailable");
542548
const isStateValidForUserMock = vi.spyOn(
543549
DiscordUtils,
@@ -601,7 +607,7 @@ describe("user controller test", () => {
601607
});
602608
});
603609
describe("getCurrentTestActivity", () => {
604-
const getUserMock = vi.spyOn(UserDal, "getUser");
610+
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
605611

606612
afterEach(() => {
607613
getUserMock.mockReset();
@@ -635,7 +641,7 @@ describe("user controller test", () => {
635641
});
636642
});
637643
describe("getStreak", () => {
638-
const getUserMock = vi.spyOn(UserDal, "getUser");
644+
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
639645

640646
afterEach(() => {
641647
getUserMock.mockReset();

‎backend/__tests__/dal/user.spec.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,4 +849,46 @@ describe("UserDal", () => {
849849
expect(year2024[93]).toEqual(2);
850850
});
851851
});
852+
describe("getPartialUser", () => {
853+
it("should throw for unknown user", async () => {
854+
expect(async () =>
855+
UserDAL.getPartialUser("1234", "stack", [])
856+
).rejects.toThrowError("User not found\nStack: stack");
857+
});
858+
859+
it("should get streak", async () => {
860+
//GIVEN
861+
let user = await UserTestData.createUser({
862+
streak: {
863+
hourOffset: 1,
864+
length: 5,
865+
lastResultTimestamp: 4711,
866+
maxLength: 23,
867+
},
868+
});
869+
870+
//WHEN
871+
const partial = await UserDAL.getPartialUser(user.uid, "streak", [
872+
"streak",
873+
]);
874+
875+
//THEN
876+
expect(partial).toStrictEqual({
877+
_id: user._id,
878+
streak: {
879+
hourOffset: 1,
880+
length: 5,
881+
lastResultTimestamp: 4711,
882+
maxLength: 23,
883+
},
884+
});
885+
});
886+
});
887+
describe("updateEmail", () => {
888+
it("throws for nonexisting user", async () => {
889+
expect(async () =>
890+
UserDAL.updateEmail(123, "test@example.com")
891+
).rejects.toThrowError("User not found\nStack: update email");
892+
});
893+
});
852894
});

‎backend/src/api/controllers/quote.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import _ from "lodash";
22
import { v4 as uuidv4 } from "uuid";
3-
import { getUser, updateQuoteRatings } from "../../dal/user";
3+
import { getPartialUser, updateQuoteRatings } from "../../dal/user";
44
import * as ReportDAL from "../../dal/report";
55
import * as NewQuotesDAL from "../../dal/new-quotes";
66
import * as QuoteRatingsDAL from "../../dal/quote-ratings";
@@ -21,7 +21,7 @@ export async function getQuotes(
2121
): Promise<MonkeyResponse> {
2222
const { uid } = req.ctx.decodedToken;
2323
const quoteMod: boolean | undefined | string = (
24-
await getUser(uid, "get quotes")
24+
await getPartialUser(uid, "get quotes", ["quoteMod"])
2525
).quoteMod;
2626
let quoteModString: string;
2727
if (quoteMod === true) {
@@ -63,7 +63,7 @@ export async function approveQuote(
6363
const { uid } = req.ctx.decodedToken;
6464
const { quoteId, editText, editSource } = req.body;
6565

66-
const { name } = await getUser(uid, "approve quote");
66+
const { name } = await getPartialUser(uid, "approve quote", ["name"]);
6767

6868
if (!name) {
6969
throw new MonkeyError(500, "Missing name field");
@@ -103,7 +103,7 @@ export async function submitRating(
103103
const { uid } = req.ctx.decodedToken;
104104
const { quoteId, rating, language } = req.body;
105105

106-
const user = await getUser(uid, "submit rating");
106+
const user = await getPartialUser(uid, "submit rating", ["quoteRatings"]);
107107

108108
const normalizedQuoteId = parseInt(quoteId as string, 10);
109109
const normalizedRating = Math.round(parseInt(rating as string, 10));

‎backend/src/api/controllers/result.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ export async function updateTags(
157157
result.numbers = false;
158158
}
159159

160-
const user = await UserDAL.getUser(uid, "update tags");
160+
const user = await UserDAL.getPartialUser(uid, "update tags", ["tags"]);
161161
const tagPbs = await UserDAL.checkIfTagPb(uid, user, result);
162162
return new MonkeyResponse("Result tags updated", {
163163
tagPbs,

‎backend/src/api/controllers/user.ts‎

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,11 @@ export async function sendVerificationEmail(
9191
throw new MonkeyError(400, "Email already verified");
9292
}
9393

94-
const userInfo = await UserDAL.getUser(uid, "request verification email");
94+
const userInfo = await UserDAL.getPartialUser(
95+
uid,
96+
"request verification email",
97+
["uid", "name", "email"]
98+
);
9599

96100
if (userInfo.email !== email) {
97101
throw new MonkeyError(
@@ -150,9 +154,10 @@ export async function sendForgotPasswordEmail(
150154

151155
try {
152156
const uid = (await FirebaseAdmin().auth().getUserByEmail(email)).uid;
153-
const userInfo = await UserDAL.getUser(
157+
const userInfo = await UserDAL.getPartialUser(
154158
uid,
155-
"request forgot password email"
159+
"request forgot password email",
160+
["name"]
156161
);
157162

158163
const link = await FirebaseAdmin()
@@ -179,7 +184,12 @@ export async function deleteUser(
179184
): Promise<MonkeyResponse> {
180185
const { uid } = req.ctx.decodedToken;
181186

182-
const userInfo = await UserDAL.getUser(uid, "delete user");
187+
const userInfo = await UserDAL.getPartialUser(uid, "delete user", [
188+
"banned",
189+
"name",
190+
"email",
191+
"discordId",
192+
]);
183193

184194
if (userInfo.banned === true) {
185195
await BlocklistDal.add(userInfo);
@@ -215,7 +225,12 @@ export async function resetUser(
215225
): Promise<MonkeyResponse> {
216226
const { uid } = req.ctx.decodedToken;
217227

218-
const userInfo = await UserDAL.getUser(uid, "reset user");
228+
const userInfo = await UserDAL.getPartialUser(uid, "reset user", [
229+
"banned",
230+
"discordId",
231+
"email",
232+
"name",
233+
]);
219234
if (userInfo.banned) {
220235
throw new MonkeyError(403, "Banned users cannot reset their account");
221236
}
@@ -247,7 +262,12 @@ export async function updateName(
247262
const { uid } = req.ctx.decodedToken;
248263
const { name } = req.body;
249264

250-
const user = await UserDAL.getUser(uid, "update name");
265+
const user = await UserDAL.getPartialUser(uid, "update name", [
266+
"name",
267+
"banned",
268+
"needsToChangeName",
269+
"lastNameChange",
270+
]);
251271

252272
if (user.banned) {
253273
throw new MonkeyError(403, "Banned users cannot change their name");
@@ -486,7 +506,10 @@ export async function linkDiscord(
486506
throw new MonkeyError(403, "Invalid user token");
487507
}
488508

489-
const userInfo = await UserDAL.getUser(uid, "link discord");
509+
const userInfo = await UserDAL.getPartialUser(uid, "link discord", [
510+
"banned",
511+
"discordId",
512+
]);
490513
if (userInfo.banned) {
491514
throw new MonkeyError(403, "Banned accounts cannot link with Discord");
492515
}
@@ -538,7 +561,10 @@ export async function unlinkDiscord(
538561
): Promise<MonkeyResponse> {
539562
const { uid } = req.ctx.decodedToken;
540563

541-
const userInfo = await UserDAL.getUser(uid, "unlink discord");
564+
const userInfo = await UserDAL.getPartialUser(uid, "unlink discord", [
565+
"banned",
566+
"discordId",
567+
]);
542568

543569
if (userInfo.banned) {
544570
throw new MonkeyError(403, "Banned accounts cannot unlink Discord");
@@ -822,7 +848,10 @@ export async function updateProfile(
822848
const { uid } = req.ctx.decodedToken;
823849
const { bio, keyboard, socialProfiles, selectedBadgeId } = req.body;
824850

825-
const user = await UserDAL.getUser(uid, "update user profile");
851+
const user = await UserDAL.getPartialUser(uid, "update user profile", [
852+
"banned",
853+
"inventory",
854+
]);
826855

827856
if (user.banned) {
828857
throw new MonkeyError(403, "Banned users cannot update their profile");
@@ -908,7 +937,9 @@ export async function setStreakHourOffset(
908937
const { uid } = req.ctx.decodedToken;
909938
const { hourOffset } = req.body;
910939

911-
const user = await UserDAL.getUser(uid, "update user profile");
940+
const user = await UserDAL.getPartialUser(uid, "update user profile", [
941+
"streak",
942+
]);
912943

913944
if (
914945
user.streak?.hourOffset !== undefined &&
@@ -927,7 +958,10 @@ export async function toggleBan(
927958
): Promise<MonkeyResponse> {
928959
const { uid } = req.body;
929960

930-
const user = await UserDAL.getUser(uid, "toggle ban");
961+
const user = await UserDAL.getPartialUser(uid, "toggle ban", [
962+
"banned",
963+
"discordId",
964+
]);
931965
const discordId = user.discordId;
932966
const discordIdIsValid = discordId !== undefined && discordId !== "";
933967

@@ -1036,7 +1070,10 @@ export async function getTestActivity(
10361070
): Promise<MonkeyResponse> {
10371071
const { uid } = req.ctx.decodedToken;
10381072
const premiumFeaturesEnabled = req.ctx.configuration.users.premium.enabled;
1039-
const user = await UserDAL.getUser(uid, "testActivity");
1073+
const user = await UserDAL.getPartialUser(uid, "testActivity", [
1074+
"testActivity",
1075+
"premium",
1076+
]);
10401077
const userHasPremium = await UserDAL.checkIfUserIsPremium(uid, user);
10411078

10421079
if (!premiumFeaturesEnabled) {
@@ -1063,7 +1100,9 @@ export async function getCurrentTestActivity(
10631100
): Promise<MonkeyResponse> {
10641101
const { uid } = req.ctx.decodedToken;
10651102

1066-
const user = await UserDAL.getUser(uid, "current test activity");
1103+
const user = await UserDAL.getPartialUser(uid, "current test activity", [
1104+
"testActivity",
1105+
]);
10671106
const data = generateCurrentTestActivity(user.testActivity);
10681107
return new MonkeyResponse("Current test activity data retrieved", data);
10691108
}
@@ -1073,7 +1112,7 @@ export async function getStreak(
10731112
): Promise<MonkeyResponse> {
10741113
const { uid } = req.ctx.decodedToken;
10751114

1076-
const user = await UserDAL.getUser(uid, "streak");
1115+
const user = await UserDAL.getPartialUser(uid, "streak", ["streak"]);
10771116

10781117
return new MonkeyResponse("Streak data retrieved", user.streak);
10791118
}

0 commit comments

Comments
 (0)