perf: optimize database access for UserDal (@fehmer) (#5544)

* impr: optimize database access for UserDal (@fehmer)

* rename getPartial to getPartialUser
This commit is contained in:
Christian Fehmer 2024-07-01 14:37:12 +02:00 committed by GitHub
parent 63b9f7605f
commit d566ba6468
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 212 additions and 64 deletions

View file

@ -195,7 +195,7 @@ describe("user controller test", () => {
});
});
describe("getTestActivity", () => {
const getUserMock = vi.spyOn(UserDal, "getUser");
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
afterAll(() => {
getUserMock.mockReset();
});
@ -303,7 +303,7 @@ describe("user controller test", () => {
});
describe("toggle ban", () => {
const getUserMock = vi.spyOn(UserDal, "getUser");
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
const setBannedMock = vi.spyOn(UserDal, "setBanned");
const georgeUserBannedMock = vi.spyOn(GeorgeQueue, "userBanned");
const isAdminMock = vi.spyOn(AdminUuids, "isAdmin");
@ -340,7 +340,10 @@ describe("user controller test", () => {
.expect(200);
//THEN
expect(getUserMock).toHaveBeenLastCalledWith(uid, "toggle ban");
expect(getUserMock).toHaveBeenLastCalledWith(uid, "toggle ban", [
"banned",
"discordId",
]);
expect(setBannedMock).toHaveBeenCalledWith(uid, true);
expect(georgeUserBannedMock).toHaveBeenCalledWith("discordId", true);
});
@ -392,7 +395,10 @@ describe("user controller test", () => {
.expect(200);
//THEN
expect(getUserMock).toHaveBeenLastCalledWith(uid, "toggle ban");
expect(getUserMock).toHaveBeenLastCalledWith(uid, "toggle ban", [
"banned",
"discordId",
]);
expect(setBannedMock).toHaveBeenCalledWith(uid, false);
expect(georgeUserBannedMock).toHaveBeenCalledWith("discordId", false);
});
@ -425,7 +431,7 @@ describe("user controller test", () => {
});
describe("delete user", () => {
const getUserMock = vi.spyOn(UserDal, "getUser");
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
const deleteUserMock = vi.spyOn(UserDal, "deleteUser");
const firebaseDeleteUserMock = vi.spyOn(AuthUtils, "deleteUser");
const deleteAllApeKeysMock = vi.spyOn(ApeKeys, "deleteAllApeKeys");
@ -537,7 +543,7 @@ describe("user controller test", () => {
});
});
describe("link discord", () => {
const getUserMock = vi.spyOn(UserDal, "getUser");
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
const isDiscordIdAvailableMock = vi.spyOn(UserDal, "isDiscordIdAvailable");
const isStateValidForUserMock = vi.spyOn(
DiscordUtils,
@ -601,7 +607,7 @@ describe("user controller test", () => {
});
});
describe("getCurrentTestActivity", () => {
const getUserMock = vi.spyOn(UserDal, "getUser");
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
afterEach(() => {
getUserMock.mockReset();
@ -635,7 +641,7 @@ describe("user controller test", () => {
});
});
describe("getStreak", () => {
const getUserMock = vi.spyOn(UserDal, "getUser");
const getUserMock = vi.spyOn(UserDal, "getPartialUser");
afterEach(() => {
getUserMock.mockReset();

View file

@ -849,4 +849,46 @@ describe("UserDal", () => {
expect(year2024[93]).toEqual(2);
});
});
describe("getPartialUser", () => {
it("should throw for unknown user", async () => {
expect(async () =>
UserDAL.getPartialUser("1234", "stack", [])
).rejects.toThrowError("User not found\nStack: stack");
});
it("should get streak", async () => {
//GIVEN
let user = await UserTestData.createUser({
streak: {
hourOffset: 1,
length: 5,
lastResultTimestamp: 4711,
maxLength: 23,
},
});
//WHEN
const partial = await UserDAL.getPartialUser(user.uid, "streak", [
"streak",
]);
//THEN
expect(partial).toStrictEqual({
_id: user._id,
streak: {
hourOffset: 1,
length: 5,
lastResultTimestamp: 4711,
maxLength: 23,
},
});
});
});
describe("updateEmail", () => {
it("throws for nonexisting user", async () => {
expect(async () =>
UserDAL.updateEmail(123, "test@example.com")
).rejects.toThrowError("User not found\nStack: update email");
});
});
});

View file

@ -1,6 +1,6 @@
import _ from "lodash";
import { v4 as uuidv4 } from "uuid";
import { getUser, updateQuoteRatings } from "../../dal/user";
import { getPartialUser, updateQuoteRatings } from "../../dal/user";
import * as ReportDAL from "../../dal/report";
import * as NewQuotesDAL from "../../dal/new-quotes";
import * as QuoteRatingsDAL from "../../dal/quote-ratings";
@ -21,7 +21,7 @@ export async function getQuotes(
): Promise<MonkeyResponse> {
const { uid } = req.ctx.decodedToken;
const quoteMod: boolean | undefined | string = (
await getUser(uid, "get quotes")
await getPartialUser(uid, "get quotes", ["quoteMod"])
).quoteMod;
let quoteModString: string;
if (quoteMod === true) {
@ -63,7 +63,7 @@ export async function approveQuote(
const { uid } = req.ctx.decodedToken;
const { quoteId, editText, editSource } = req.body;
const { name } = await getUser(uid, "approve quote");
const { name } = await getPartialUser(uid, "approve quote", ["name"]);
if (!name) {
throw new MonkeyError(500, "Missing name field");
@ -103,7 +103,7 @@ export async function submitRating(
const { uid } = req.ctx.decodedToken;
const { quoteId, rating, language } = req.body;
const user = await getUser(uid, "submit rating");
const user = await getPartialUser(uid, "submit rating", ["quoteRatings"]);
const normalizedQuoteId = parseInt(quoteId as string, 10);
const normalizedRating = Math.round(parseInt(rating as string, 10));

View file

@ -157,7 +157,7 @@ export async function updateTags(
result.numbers = false;
}
const user = await UserDAL.getUser(uid, "update tags");
const user = await UserDAL.getPartialUser(uid, "update tags", ["tags"]);
const tagPbs = await UserDAL.checkIfTagPb(uid, user, result);
return new MonkeyResponse("Result tags updated", {
tagPbs,

View file

@ -91,7 +91,11 @@ export async function sendVerificationEmail(
throw new MonkeyError(400, "Email already verified");
}
const userInfo = await UserDAL.getUser(uid, "request verification email");
const userInfo = await UserDAL.getPartialUser(
uid,
"request verification email",
["uid", "name", "email"]
);
if (userInfo.email !== email) {
throw new MonkeyError(
@ -150,9 +154,10 @@ export async function sendForgotPasswordEmail(
try {
const uid = (await FirebaseAdmin().auth().getUserByEmail(email)).uid;
const userInfo = await UserDAL.getUser(
const userInfo = await UserDAL.getPartialUser(
uid,
"request forgot password email"
"request forgot password email",
["name"]
);
const link = await FirebaseAdmin()
@ -179,7 +184,12 @@ export async function deleteUser(
): Promise<MonkeyResponse> {
const { uid } = req.ctx.decodedToken;
const userInfo = await UserDAL.getUser(uid, "delete user");
const userInfo = await UserDAL.getPartialUser(uid, "delete user", [
"banned",
"name",
"email",
"discordId",
]);
if (userInfo.banned === true) {
await BlocklistDal.add(userInfo);
@ -215,7 +225,12 @@ export async function resetUser(
): Promise<MonkeyResponse> {
const { uid } = req.ctx.decodedToken;
const userInfo = await UserDAL.getUser(uid, "reset user");
const userInfo = await UserDAL.getPartialUser(uid, "reset user", [
"banned",
"discordId",
"email",
"name",
]);
if (userInfo.banned) {
throw new MonkeyError(403, "Banned users cannot reset their account");
}
@ -247,7 +262,12 @@ export async function updateName(
const { uid } = req.ctx.decodedToken;
const { name } = req.body;
const user = await UserDAL.getUser(uid, "update name");
const user = await UserDAL.getPartialUser(uid, "update name", [
"name",
"banned",
"needsToChangeName",
"lastNameChange",
]);
if (user.banned) {
throw new MonkeyError(403, "Banned users cannot change their name");
@ -486,7 +506,10 @@ export async function linkDiscord(
throw new MonkeyError(403, "Invalid user token");
}
const userInfo = await UserDAL.getUser(uid, "link discord");
const userInfo = await UserDAL.getPartialUser(uid, "link discord", [
"banned",
"discordId",
]);
if (userInfo.banned) {
throw new MonkeyError(403, "Banned accounts cannot link with Discord");
}
@ -538,7 +561,10 @@ export async function unlinkDiscord(
): Promise<MonkeyResponse> {
const { uid } = req.ctx.decodedToken;
const userInfo = await UserDAL.getUser(uid, "unlink discord");
const userInfo = await UserDAL.getPartialUser(uid, "unlink discord", [
"banned",
"discordId",
]);
if (userInfo.banned) {
throw new MonkeyError(403, "Banned accounts cannot unlink Discord");
@ -822,7 +848,10 @@ export async function updateProfile(
const { uid } = req.ctx.decodedToken;
const { bio, keyboard, socialProfiles, selectedBadgeId } = req.body;
const user = await UserDAL.getUser(uid, "update user profile");
const user = await UserDAL.getPartialUser(uid, "update user profile", [
"banned",
"inventory",
]);
if (user.banned) {
throw new MonkeyError(403, "Banned users cannot update their profile");
@ -908,7 +937,9 @@ export async function setStreakHourOffset(
const { uid } = req.ctx.decodedToken;
const { hourOffset } = req.body;
const user = await UserDAL.getUser(uid, "update user profile");
const user = await UserDAL.getPartialUser(uid, "update user profile", [
"streak",
]);
if (
user.streak?.hourOffset !== undefined &&
@ -927,7 +958,10 @@ export async function toggleBan(
): Promise<MonkeyResponse> {
const { uid } = req.body;
const user = await UserDAL.getUser(uid, "toggle ban");
const user = await UserDAL.getPartialUser(uid, "toggle ban", [
"banned",
"discordId",
]);
const discordId = user.discordId;
const discordIdIsValid = discordId !== undefined && discordId !== "";
@ -1036,7 +1070,10 @@ export async function getTestActivity(
): Promise<MonkeyResponse> {
const { uid } = req.ctx.decodedToken;
const premiumFeaturesEnabled = req.ctx.configuration.users.premium.enabled;
const user = await UserDAL.getUser(uid, "testActivity");
const user = await UserDAL.getPartialUser(uid, "testActivity", [
"testActivity",
"premium",
]);
const userHasPremium = await UserDAL.checkIfUserIsPremium(uid, user);
if (!premiumFeaturesEnabled) {
@ -1063,7 +1100,9 @@ export async function getCurrentTestActivity(
): Promise<MonkeyResponse> {
const { uid } = req.ctx.decodedToken;
const user = await UserDAL.getUser(uid, "current test activity");
const user = await UserDAL.getPartialUser(uid, "current test activity", [
"testActivity",
]);
const data = generateCurrentTestActivity(user.testActivity);
return new MonkeyResponse("Current test activity data retrieved", data);
}
@ -1073,7 +1112,7 @@ export async function getStreak(
): Promise<MonkeyResponse> {
const { uid } = req.ctx.decodedToken;
const user = await UserDAL.getUser(uid, "streak");
const user = await UserDAL.getPartialUser(uid, "streak", ["streak"]);
return new MonkeyResponse("Streak data retrieved", user.streak);
}

View file

@ -179,8 +179,7 @@ export async function updateQuoteRatings(
uid: string,
quoteRatings: SharedTypes.UserQuoteRatings
): Promise<boolean> {
await getUser(uid, "update quote ratings");
await getUsersCollection().updateOne({ uid }, { $set: { quoteRatings } });
await updateUser("update quote ratings", { uid }, { $set: { quoteRatings } });
return true;
}
@ -188,8 +187,8 @@ export async function updateEmail(
uid: string,
email: string
): Promise<boolean> {
await getUser(uid, "update email"); // To make sure that the user exists
await getUsersCollection().updateOne({ uid }, { $set: { email } });
await updateUser("update email", { uid }, { $set: { email } });
return true;
}
@ -202,6 +201,26 @@ export async function getUser(
return user;
}
/**
* Get user document only containing requested fields
* @param uid user id
* @param stack stack description used in the error
* @param fields list of fields
* @returns partial DBUser only containing requested fields
* @throws MonkeyError if user does not exist
*/
export async function getPartialUser<K extends keyof MonkeyTypes.DBUser>(
uid: string,
stack: string,
fields: K[]
): Promise<Pick<MonkeyTypes.DBUser, K>> {
const projection = new Map(fields.map((it) => [it, 1]));
const results = await getUsersCollection().findOne({ uid }, { projection });
if (results === null) throw new MonkeyError(404, "User not found", stack);
return results;
}
export async function findByName(
name: string
): Promise<MonkeyTypes.DBUser | undefined> {
@ -247,7 +266,8 @@ export async function addResultFilterPreset(
): Promise<ObjectId> {
// ensure limit not reached
const filtersCount = (
(await getUser(uid, "Add Result filter")).resultFilterPresets ?? []
(await getPartialUser(uid, "Add Result filter", ["resultFilterPresets"]))
.resultFilterPresets ?? []
).length;
if (filtersCount >= maxFiltersPerUser) {
@ -269,7 +289,9 @@ export async function removeResultFilterPreset(
uid: string,
_id: string
): Promise<void> {
const user = await getUser(uid, "remove result filter");
const user = await getPartialUser(uid, "remove result filter", [
"resultFilterPresets",
]);
const filterId = new ObjectId(_id);
if (
user.resultFilterPresets === undefined ||
@ -292,7 +314,7 @@ export async function addTag(
uid: string,
name: string
): Promise<MonkeyTypes.DBUserTag> {
const user = await getUser(uid, "add tag");
const user = await getPartialUser(uid, "add tag", ["tags"]);
if ((user?.tags?.length ?? 0) >= 15) {
throw new MonkeyError(400, "You can only have up to 15 tags");
@ -323,7 +345,7 @@ export async function addTag(
}
export async function getTags(uid: string): Promise<MonkeyTypes.DBUserTag[]> {
const user = await getUser(uid, "get tags");
const user = await getPartialUser(uid, "get tags", ["tags"]);
return user.tags ?? [];
}
@ -333,7 +355,7 @@ export async function editTag(
_id: string,
name: string
): Promise<void> {
const user = await getUser(uid, "edit tag");
const user = await getPartialUser(uid, "edit tag", ["tags"]);
if (
user.tags === undefined ||
user.tags.filter((t) => t._id.toHexString() === _id).length === 0
@ -350,7 +372,7 @@ export async function editTag(
}
export async function removeTag(uid: string, _id: string): Promise<void> {
const user = await getUser(uid, "remove tag");
const user = await getPartialUser(uid, "remove tag", ["tags"]);
if (
user.tags === undefined ||
user.tags.filter((t) => t._id.toHexString() === _id).length === 0
@ -367,7 +389,7 @@ export async function removeTag(uid: string, _id: string): Promise<void> {
}
export async function removeTagPb(uid: string, _id: string): Promise<void> {
const user = await getUser(uid, "remove tag pb");
const user = await getPartialUser(uid, "remove tag pb", ["tags"]);
if (
user.tags === undefined ||
user.tags.filter((t) => t._id.toHexString() === _id).length === 0
@ -400,7 +422,7 @@ export async function updateLbMemory(
language: string,
rank: number
): Promise<void> {
const user = await getUser(uid, "update lb memory");
const user = await getPartialUser(uid, "update lb memory", ["lbMemory"]);
if (user.lbMemory === undefined) user.lbMemory = {};
if (user.lbMemory[mode] === undefined) user.lbMemory[mode] = {};
if (user.lbMemory[mode]?.[mode2] === undefined) {
@ -419,7 +441,7 @@ export async function updateLbMemory(
export async function checkIfPb(
uid: string,
user: MonkeyTypes.DBUser,
user: Pick<MonkeyTypes.DBUser, "personalBests" | "lbPersonalBests">,
result: Result
): Promise<boolean> {
const { mode } = result;
@ -462,7 +484,7 @@ export async function checkIfPb(
export async function checkIfTagPb(
uid: string,
user: MonkeyTypes.DBUser,
user: Pick<MonkeyTypes.DBUser, "tags">,
result: Result
): Promise<string[]> {
if (user.tags === undefined || user.tags.length === 0) {
@ -511,8 +533,8 @@ export async function checkIfTagPb(
}
export async function resetPb(uid: string): Promise<void> {
await getUser(uid, "reset pb");
await getUsersCollection().updateOne(
await updateUser(
"reset pb",
{ uid },
{
$set: {
@ -579,16 +601,17 @@ export async function linkDiscord(
}
export async function unlinkDiscord(uid: string): Promise<void> {
await getUser(uid, "unlink discord");
await getUsersCollection().updateOne(
await updateUser(
"unlink discord",
{ uid },
{ $unset: { discordId: "", discordAvatar: "" } }
);
}
export async function incrementBananas(uid: string, wpm): Promise<void> {
const user = await getUser(uid, "increment bananas");
const user = await getPartialUser(uid, "increment bananas", [
"personalBests",
]);
let best60: number | undefined;
const personalBests60 = user.personalBests?.time["60"];
@ -644,7 +667,7 @@ export async function addTheme(
uid: string,
theme
): Promise<{ _id: ObjectId; name: string }> {
const user = await getUser(uid, "add theme");
const user = await getPartialUser(uid, "add theme", ["customThemes"]);
if ((user.customThemes ?? []).length >= 10) {
throw new MonkeyError(409, "Too many custom themes");
@ -671,7 +694,7 @@ export async function addTheme(
}
export async function removeTheme(uid: string, _id): Promise<void> {
const user = await getUser(uid, "remove theme");
const user = await getPartialUser(uid, "remove theme", ["customThemes"]);
if (themeDoesNotExist(user.customThemes, _id)) {
throw new MonkeyError(404, "Custom theme not found");
@ -687,7 +710,7 @@ export async function removeTheme(uid: string, _id): Promise<void> {
}
export async function editTheme(uid: string, _id, theme): Promise<void> {
const user = await getUser(uid, "edit theme");
const user = await getPartialUser(uid, "edit theme", ["customThemes"]);
if (themeDoesNotExist(user.customThemes, _id)) {
throw new MonkeyError(404, "Custom Theme not found");
@ -710,7 +733,7 @@ export async function editTheme(uid: string, _id, theme): Promise<void> {
export async function getThemes(
uid: string
): Promise<MonkeyTypes.DBCustomTheme[]> {
const user = await getUser(uid, "get themes");
const user = await getPartialUser(uid, "get themes", ["customThemes"]);
return user.customThemes ?? [];
}
@ -719,7 +742,9 @@ export async function getPersonalBests(
mode: string,
mode2?: string
): Promise<SharedTypes.PersonalBest> {
const user = await getUser(uid, "get personal bests");
const user = await getPartialUser(uid, "get personal bests", [
"personalBests",
]);
if (mode2 !== undefined) {
return user.personalBests?.[mode]?.[mode2];
@ -731,7 +756,11 @@ export async function getPersonalBests(
export async function getStats(
uid: string
): Promise<Record<string, number | undefined>> {
const user = await getUser(uid, "get stats");
const user = await getPartialUser(uid, "get stats", [
"startedTests",
"completedTests",
"timeTyping",
]);
return {
startedTests: user.startedTests,
@ -743,7 +772,9 @@ export async function getStats(
export async function getFavoriteQuotes(
uid
): Promise<MonkeyTypes.DBUser["favoriteQuotes"]> {
const user = await getUser(uid, "get favorite quotes");
const user = await getPartialUser(uid, "get favorite quotes", [
"favoriteQuotes",
]);
return user.favoriteQuotes ?? {};
}
@ -754,7 +785,9 @@ export async function addFavoriteQuote(
quoteId: string,
maxQuotes: number
): Promise<void> {
const user = await getUser(uid, "add favorite quote");
const user = await getPartialUser(uid, "add favorite quote", [
"favoriteQuotes",
]);
if (user.favoriteQuotes) {
if (user.favoriteQuotes[language]?.includes(quoteId)) {
@ -790,7 +823,9 @@ export async function removeFavoriteQuote(
language: string,
quoteId: string
): Promise<void> {
const user = await getUser(uid, "remove favorite quote");
const user = await getPartialUser(uid, "remove favorite quote", [
"favoriteQuotes",
]);
if (!user.favoriteQuotes?.[language]?.includes(quoteId)) {
return;
@ -807,7 +842,10 @@ export async function recordAutoBanEvent(
maxCount: number,
maxHours: number
): Promise<boolean> {
const user = await getUser(uid, "record auto ban event");
const user = await getPartialUser(uid, "record auto ban event", [
"banned",
"autoBanTimestamps",
]);
let ret = false;
@ -875,7 +913,7 @@ export async function updateProfile(
export async function getInbox(
uid: string
): Promise<MonkeyTypes.DBUser["inbox"]> {
const user = await getUser(uid, "get inventory");
const user = await getPartialUser(uid, "get inbox", ["inbox"]);
return user.inbox ?? [];
}
@ -984,7 +1022,10 @@ export async function updateInbox(
mailToRead: string[],
mailToDelete: string[]
): Promise<void> {
const user = await getUser(uid, "update inbox");
const user = await getPartialUser(uid, "update inbox", [
"inbox",
"inventory",
]);
const inbox = user.inbox ?? [];
@ -1024,7 +1065,7 @@ export async function updateStreak(
uid: string,
timestamp: number
): Promise<number> {
const user = await getUser(uid, "calculate streak");
const user = await getPartialUser(uid, "calculate streak", ["streak"]);
const streak: SharedTypes.UserStreak = {
lastResultTimestamp: user.streak?.lastResultTimestamp ?? 0,
length: user.streak?.length ?? 0,
@ -1080,14 +1121,16 @@ export async function setBanned(uid: string, banned: boolean): Promise<void> {
export async function checkIfUserIsPremium(
uid: string,
userInfoOverride?: MonkeyTypes.DBUser
userInfoOverride?: Pick<MonkeyTypes.DBUser, "premium">
): Promise<boolean> {
const premiumFeaturesEnabled = (await getCachedConfiguration(true)).users
.premium.enabled;
if (premiumFeaturesEnabled !== true) {
return false;
}
const user = userInfoOverride ?? (await getUser(uid, "checkIfUserIsPremium"));
const user =
userInfoOverride ??
(await getPartialUser(uid, "checkIfUserIsPremium", ["premium"]));
const expirationDate = user.premium?.expirationTimestamp;
if (expirationDate === undefined) return false;
@ -1098,9 +1141,10 @@ export async function checkIfUserIsPremium(
export async function logIpAddress(
uid: string,
ip: string,
userInfoOverride?: MonkeyTypes.DBUser
userInfoOverride?: Pick<MonkeyTypes.DBUser, "ips">
): Promise<void> {
const user = userInfoOverride ?? (await getUser(uid, "logIpAddress"));
const user =
userInfoOverride ?? (await getPartialUser(uid, "logIpAddress", ["ips"]));
const currentIps = user.ips ?? [];
const ipIndex = currentIps.indexOf(ip);
if (ipIndex !== -1) {
@ -1112,3 +1156,20 @@ export async function logIpAddress(
}
await getUsersCollection().updateOne({ uid }, { $set: { ips: currentIps } });
}
/**
* Update user document. Requires the user to exist
* @param stack stack description used in the error
* @param filter user filter
* @param update update document
* @throws MonkeyError if user does not exist
*/
async function updateUser(
stack: string,
filter: { uid: string },
update: UpdateFilter<MonkeyTypes.DBUser>
): Promise<void> {
const result = await getUsersCollection().updateOne(filter, update);
if (result.matchedCount !== 1)
throw new MonkeyError(404, "User not found", stack);
}