small refactoring

This commit is contained in:
StarAppeal
2025-09-18 23:05:29 +02:00
parent ded120513e
commit 2fa3a3de78
9 changed files with 61 additions and 112 deletions
+3 -5
View File
@@ -1,9 +1,8 @@
import "dotenv/config";
import mongoose, {Schema} from "mongoose";
import bcrypt from "bcrypt";
import mongoose, {Schema, Document} from "mongoose";
import {PasswordUtils} from "../../utils/passwordUtils";
export interface IUser {
export interface IUser extends Document {
name: string;
password?: string;
uuid: string;
@@ -148,8 +147,7 @@ userSchema.pre("findOneAndUpdate", async function (next) {
if (isBcryptHash(newPassword)) return next();
try {
const saltRounds = 10;
const hashed = await bcrypt.hash(newPassword, saltRounds);
const hashed = await PasswordUtils.hashPassword(newPassword);
if (update.password) update.password = hashed;
if (update.$set?.password) update.$set.password = hashed;
return next();
+21 -19
View File
@@ -1,6 +1,6 @@
import {IUser, SpotifyConfig, UserModel} from "../../models/user";
import {connectToDatabase} from "./database.service";
import {UpdateQuery} from "mongoose";
import {Document, UpdateQuery} from "mongoose";
export class UserService {
private static _instance: UserService;
@@ -22,20 +22,6 @@ export class UserService {
}).exec();
}
public async updateUser(user: IUser): Promise<IUser | null> {
const anyUser = user as any;
const targetId: string | undefined = anyUser?.id?.toString?.() ?? anyUser?._id?.toString?.();
if (!targetId) {
throw new Error("updateUser requires user.id or user._id");
}
const { id, _id, ...rest } = anyUser;
return this.updateUserById(targetId, rest as Partial<IUser>);
}
public async getAllUsers(): Promise<IUser[]> {
return await UserModel.find({}, {spotifyConfig: 0, lastState: 0}).exec();
}
@@ -66,11 +52,27 @@ export class UserService {
return await UserModel.findOne({uuid}, {spotifyConfig: 1}).exec().then(user => user?.spotifyConfig);
}
public async createUser(user: IUser): Promise<IUser> {
const createdUser = await UserModel.create(user);
public async createUser(userData: Omit<IUser, keyof Document>): Promise<IUser> {
try {
const newUser = await UserModel.create(userData);
const {password, ...rest} = createdUser.toObject();
return rest as IUser;
const userObject = newUser.toObject();
delete userObject.password;
return userObject as IUser;
} catch (error: any) {
if (error.code === 11000 && error.keyPattern?.uuid) {
throw new Error("User with that uuid already exists");
}
if (error.name === 'ValidationError') {
throw new Error(`ValidationError: ${error.message}`);
}
console.error("Error creating user:", error);
throw new Error("User could not be created.");
}
}
public async existsUserByName(name: string): Promise<boolean> {
+3 -2
View File
@@ -7,6 +7,7 @@ import {PasswordUtils} from "../utils/passwordUtils";
import {asyncHandler} from "./middleware/asyncHandler";
import {validateBody, v} from "./middleware/validate";
import {ok, badRequest, unauthorized, created, conflict, notFound} from "./utils/responses";
import {Document} from "mongoose";
export class RestAuth {
private readonly userService: UserService;
@@ -44,7 +45,7 @@ export class RestAuth {
}
const hashedPassword = await PasswordUtils.hashPassword(password);
const newUser: IUser = {
const newUser: Omit<IUser, keyof Document> = {
name: username,
password: hashedPassword,
uuid: crypto.randomUUID(),
@@ -84,7 +85,7 @@ export class RestAuth {
const jwtToken = new JwtAuthenticator(process.env.SECRET_KEY!)
.generateToken({
username: user.name,
id: (user as any).id?.toString?.() ?? (user as any)._id?.toString?.(),
id: user.id,
uuid: user.uuid
});
+18 -18
View File
@@ -17,9 +17,9 @@ export class RestUser {
public createRouter() {
const router = express.Router();
router.get("/",isAdmin(this.userService), asyncHandler(async (_req, res) => {
router.get("/", isAdmin(this.userService), asyncHandler(async (_req, res) => {
const users = await this.userService.getAllUsers();
return ok(res, { users });
return ok(res, {users});
}));
router.get("/me", asyncHandler(async (req, res) => {
@@ -30,10 +30,10 @@ export class RestUser {
router.put(
"/me/spotify",
validateBody({
accessToken: { required: true, validator: v.isString({ nonEmpty: true }) },
refreshToken: { required: true, validator: v.isString({ nonEmpty: true }) },
scope: { required: true, validator: v.isString({ nonEmpty: true }) },
expirationDate: { required: true, validator: v.isString({ nonEmpty: true }) },
accessToken: {required: true, validator: v.isString({nonEmpty: true})},
refreshToken: {required: true, validator: v.isString({nonEmpty: true})},
scope: {required: true, validator: v.isString({nonEmpty: true})},
expirationDate: {required: true, validator: v.isString({nonEmpty: true})},
}),
asyncHandler(async (req, res) => {
const user = await this.userService.getUserByUUID(req.payload.uuid);
@@ -41,19 +41,19 @@ export class RestUser {
return badRequest(res, "User not found");
}
const { accessToken, refreshToken, scope, expirationDate } = req.body as {
const {accessToken, refreshToken, scope, expirationDate} = req.body as {
accessToken: string; refreshToken: string; scope: string; expirationDate: string;
};
user.spotifyConfig = {
const spotifyConfig = {
accessToken,
refreshToken,
scope,
expirationDate: new Date(expirationDate),
};
await this.userService.updateUser(user);
return ok(res, { message: "Spotify Config erfolgreich geändert" });
await this.userService.updateUserById(user.id, {spotifyConfig: spotifyConfig});
return ok(res, {message: "Spotify Config erfolgreich geändert"});
})
);
@@ -64,14 +64,14 @@ export class RestUser {
}
const updated = await this.userService.clearSpotifyConfigByUUID(req.payload.uuid);
return ok(res, { user: updated });
return ok(res, {user: updated});
}));
router.put(
"/me/password",
validateBody({
password: { required: true, validator: v.isString({ nonEmpty: true, min: 8 }) },
passwordConfirmation: { required: true, validator: v.isString({ nonEmpty: true, min: 8 }) },
password: {required: true, validator: v.isString({nonEmpty: true, min: 8})},
passwordConfirmation: {required: true, validator: v.isString({nonEmpty: true, min: 8})},
}),
asyncHandler(async (req, res) => {
const user = await this.userService.getUserByUUID(req.payload.uuid);
@@ -79,7 +79,7 @@ export class RestUser {
return badRequest(res, "User not found");
}
const { password, passwordConfirmation } = req.body as { password: string; passwordConfirmation: string };
const {password, passwordConfirmation} = req.body as { password: string; passwordConfirmation: string };
if (password !== passwordConfirmation) {
return badRequest(res, "Passwörter stimmen nicht überein");
@@ -90,17 +90,17 @@ export class RestUser {
return badRequest(res, passwordValidation.message ?? "Invalid password");
}
user.password = await PasswordUtils.hashPassword(password);
const newPassword = await PasswordUtils.hashPassword(password);
await this.userService.updateUser(user);
return ok(res, { message: "Passwort erfolgreich geändert" });
await this.userService.updateUserById(user.id, {password: newPassword});
return ok(res, {message: "Passwort erfolgreich geändert"});
})
);
router.get(
"/:id",
validateParams({
id: { required: true, validator: v.isString({ nonEmpty: true }) },
id: {required: true, validator: v.isString({nonEmpty: true})},
}),
isAdmin(this.userService),
asyncHandler(async (req, res) => {
@@ -49,14 +49,14 @@ export class GetSingleSpotifyUpdateEvent extends CustomWebsocketEventUserService
console.log("Token expired");
const token = await new SpotifyTokenService().refreshToken(spotifyConfig.refreshToken);
user.spotifyConfig = {
const newSpotifyConfig = {
// use old refresh token because you don't get a new one
refreshToken: user.spotifyConfig!.refreshToken,
accessToken: token.access_token,
expirationDate: new Date(Date.now() + token.expires_in * 1000),
scope: token.scope,
};
await this.userService.updateUser(user);
await this.userService.updateUserById(user.id, {spotifyConfig: newSpotifyConfig});
console.log("Token refreshed and database updated");
}
const musicData = await getCurrentlyPlaying(user.spotifyConfig!.accessToken);
+8 -42
View File
@@ -32,7 +32,7 @@ describe("UserService", () => {
describe("create (singleton)", () => {
it("should create a singleton instance", async () => {
const instance1 = userService; // Bereits im beforeEach erstellt
const instance1 = userService;
const instance2 = await UserService.create();
expect(instance1).toBe(instance2);
@@ -60,7 +60,7 @@ describe("UserService", () => {
expect(mockedUserModel.findByIdAndUpdate).toHaveBeenCalledWith(
userId,
updateData,
{ new: true, projection: { password: 0 } }
{ new: true }
);
expect(result).toEqual(updatedUser);
});
@@ -74,42 +74,6 @@ describe("UserService", () => {
});
});
describe("updateUser", () => {
it("should update user using id field", async () => {
const user = { id: "507f1f77bcf86cd799439011", name: "Test User" };
mockedUserModel.findByIdAndUpdate.mockReturnValue(createMockMongooseQuery(user) as any);
const result = await userService.updateUser(user as any);
expect(mockedUserModel.findByIdAndUpdate).toHaveBeenCalledWith(
user.id,
{ name: "Test User" },
expect.any(Object)
);
expect(result).toEqual(user);
});
it("should update user using _id field", async () => {
const user = { _id: "507f1f77bcf86cd799439011", name: "Test User" };
mockedUserModel.findByIdAndUpdate.mockReturnValue(createMockMongooseQuery(user) as any);
const result = await userService.updateUser(user as any);
expect(mockedUserModel.findByIdAndUpdate).toHaveBeenCalledWith(
user._id,
{ name: "Test User" },
expect.any(Object)
);
expect(result).toEqual(user);
});
it("should throw error if user has no id or _id", async () => {
await expect(userService.updateUser({ name: "Test User" } as any)).rejects.toThrow(
"updateUser requires user.id or user._id"
);
});
});
describe("getAllUsers", () => {
it("should return all users without sensitive fields", async () => {
const users = [{ name: "User1" }, { name: "User2" }];
@@ -225,16 +189,18 @@ describe("UserService", () => {
describe("existsUserByName", () => {
it("should return true if user exists", async () => {
mockedUserModel.findOne.mockReturnValue(createMockMongooseQuery({ _id: "some-id" }) as any);
// @ts-ignore
mockedUserModel.countDocuments.mockReturnValue(1);
const result = await userService.existsUserByName("ExistingUser");
expect(mockedUserModel.findOne).toHaveBeenCalledWith({ name: "ExistingUser" });
expect(mockedUserModel.countDocuments).toHaveBeenCalledWith({ name: "ExistingUser" });
expect(result).toBe(true);
});
it("should return false if user does not exist", async () => {
mockedUserModel.findOne.mockReturnValue(createMockMongooseQuery(null) as any);
// @ts-ignore
mockedUserModel.countDocuments.mockReturnValue(0);
const result = await userService.existsUserByName("NonExistentUser");
@@ -252,7 +218,7 @@ describe("UserService", () => {
expect(mockedUserModel.findOneAndUpdate).toHaveBeenCalledWith(
{ uuid: "uuid-123" },
{ $unset: { spotifyConfig: 1 } },
{ new: true, projection: { password: 0 } }
{ new: true }
);
expect(result).toEqual(updatedUser);
});
-1
View File
@@ -49,7 +49,6 @@ export const createMockUserService = () => ({
getAllUsers: vi.fn(),
getUserByUUID: vi.fn(),
getUserById: vi.fn(),
updateUser: vi.fn(),
updateUserById: vi.fn(),
getUserByName: vi.fn(),
getSpotifyConfigByUUID: vi.fn(),
-17
View File
@@ -202,23 +202,6 @@ describe("RestAuth", () => {
expect(authTokenCookie).toContain("Expires=Thu, 01 Jan 1970 00:00:00 GMT");
});
it("should handle user with _id instead of id", async () => {
const mockUser = {name: "testuser", password: "hashed", uuid: "uuid-123", _id: "user-id-123"};
const mockToken = "jwt-token-123";
mockUserService.getUserAuthByName.mockResolvedValue(mockUser);
mockPasswordUtils.comparePassword.mockResolvedValue(true);
mockJwtAuthenticator.generateToken.mockReturnValue(mockToken);
await request(app).post("/auth/login").send(validLoginData).expect(200);
expect(mockJwtAuthenticator.generateToken).toHaveBeenCalledWith({
username: "testuser",
id: "user-id-123",
uuid: "uuid-123",
});
});
it("should return not found when user does not exist", async () => {
mockUserService.getUserAuthByName.mockResolvedValue(null);
const response = await request(app).post("/auth/login").send(validLoginData).expect(404);
+6 -6
View File
@@ -73,7 +73,7 @@ describe("RestUser", () => {
};
mockedUserService.getUserByUUID.mockResolvedValue(mockUser);
mockedUserService.updateUser.mockResolvedValue(mockUser);
mockedUserService.updateUserById.mockResolvedValue(mockUser);
const response = await request(testEnv.app)
.put("/user/me/spotify")
@@ -82,8 +82,8 @@ describe("RestUser", () => {
expect(response.body.data.message).toBe("Spotify Config erfolgreich geändert");
expect(mockedUserService.getUserByUUID).toHaveBeenCalledWith("test-user-uuid");
expect(mockedUserService.updateUser).toHaveBeenCalledWith({
...mockUser,
expect(mockedUserService.updateUserById).toHaveBeenCalledWith(
mockUser.id, {
spotifyConfig: {
accessToken: "access-token-123",
refreshToken: "refresh-token-123",
@@ -219,7 +219,7 @@ describe("RestUser", () => {
mockedUserService.getUserByUUID.mockResolvedValue(mockUser);
vi.mocked(PasswordUtils.validatePassword).mockReturnValue({valid: true});
vi.mocked(PasswordUtils.hashPassword).mockResolvedValue("new-hashed-password");
mockedUserService.updateUser.mockResolvedValue(mockUser);
mockedUserService.updateUserById.mockResolvedValue(mockUser);
const response = await request(testEnv.app)
.put("/user/me/password")
@@ -229,8 +229,8 @@ describe("RestUser", () => {
expect(response.body.data.message).toBe("Passwort erfolgreich geändert");
expect(PasswordUtils.validatePassword).toHaveBeenCalledWith("newpassword123");
expect(PasswordUtils.hashPassword).toHaveBeenCalledWith("newpassword123");
expect(mockedUserService.updateUser).toHaveBeenCalledWith({
...mockUser,
expect(mockedUserService.updateUserById).toHaveBeenCalledWith(
mockUser.id, {
password: "new-hashed-password"
});
});