Skip to content

Commit 48e81d1

Browse files
Merge pull request #98 from jd-apprentice/development
Improvements from sonarqube Suggestions
2 parents bed7efa + 636e239 commit 48e81d1

9 files changed

Lines changed: 110 additions & 68 deletions

File tree

__tests__/user/__tests__/users.test.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
import { app } from '../../../src/app/main';
2-
import { Config } from "../../../src/app/config/config";
3-
import { loadDatabase } from "../../../src/app/db/index";
4-
import generateToken from '../../../src/common/utils/generateToken';
5-
1+
// External Modules
62
import request from 'supertest';
73
import { describe, test, expect, beforeAll, jest, beforeEach } from 'bun:test';
84

5+
// Internal Modules
6+
import generateToken from 'src/common/utils/generateToken';
7+
import { Config } from 'src/app/config/config';
8+
import { loadDatabase } from 'src/app/db';
9+
import { app } from 'src/app/main';
10+
911
const contentTypeKey = 'Content-Type';
1012
const contentTypeValue = /json/;
1113
const httpSuccess = 200;
@@ -46,7 +48,7 @@ describe('INTEGRATION - User Module', () => {
4648
{
4749
statusCode: httpUnauthorized,
4850
error: "Unauthorized",
49-
message: "Invalid token"
51+
message: "Unauthorized"
5052
}
5153
);
5254
})

docker/ARM64.Dockerfile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ FROM base AS release
2222
COPY --from=install /temp/prod/node_modules node_modules
2323
COPY --from=prerelease /bun/dist ./dist
2424

25-
RUN mkdir -p /bun/src/image/assets/images
26-
RUN chown -R bun:bun /bun/src/image/assets/images
27-
RUN chmod -R 600 /bun/src/image/assets/images
25+
RUN mkdir -p /bun/src/image/assets/images && \
26+
chown -R bun:bun /bun/src/image/assets/images && \
27+
chmod -R 600 /bun/src/image/assets/images
2828

2929
USER bun
3030
EXPOSE 4000

docker/Dockerfile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ FROM base AS release
2222
COPY --from=install /temp/prod/node_modules node_modules
2323
COPY --from=prerelease /bun/dist ./dist
2424

25-
RUN mkdir -p /bun/src/image/assets/images
26-
RUN chown -R bun:bun /bun/src/image/assets/images
27-
RUN chmod -R 600 /bun/src/image/assets/images
25+
RUN mkdir -p /bun/src/image/assets/images && \
26+
chown -R bun:bun /bun/src/image/assets/images && \
27+
chmod -R 600 /bun/src/image/assets/images
2828

2929
USER bun
3030
EXPOSE 4000

src/image/image-controller.ts

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,66 @@
11
// External Modules
22
import { Request, Response } from 'express';
3+
import { LogArgument } from 'rollbar';
34

45
// Interal Modules
5-
import clearTemporaryFiles from '../common/utils/clear';
6+
import clearTemporaryFiles from 'src/common/utils/clear';
67
import ImageService from './image-service';
7-
import { Query } from '../common/interfaces/types';
8+
import { Query } from 'src/common/interfaces/types';
9+
import { rollbar } from 'src/app/config/rollbar';
810

911
class ImageController {
1012
/**
11-
* @description Upload a file to cloudinary then saves the url on mongodb
12-
* @param { Request } req.body - tag, source, is_nsfw
13-
* @param { Request } file.path - path to the file
14-
* @returns { Promise<Response> } A success message with a Json response format
13+
* @description Uploads a file to Cloudinary and saves the URL in MongoDB
14+
* @param { Request } req - The request object containing the file and additional data
15+
* @returns { Promise<Response> } A JSON response with a success message
1516
*/
1617
async uploadFile(req: Request, res: Response): Promise<Response> {
1718
try {
1819
const { file } = req;
1920
const { tag, source, is_nsfw } = req.body;
20-
const response = await ImageService.cloudinaryUpload(file?.path);
21-
const { public_id, secure_url } = response;
22-
ImageService.upload({
21+
22+
if (!file) {
23+
return res.status(400).json({ error: 'File is required' });
24+
}
25+
26+
const uploadResult = await ImageService.cloudinaryUpload(file.path);
27+
const { public_id, secure_url } = uploadResult;
28+
29+
await ImageService.upload({
2330
source,
2431
is_nsfw,
2532
id: public_id,
2633
url: secure_url,
2734
tag,
2835
});
29-
clearTemporaryFiles(file?.path ?? 'image/assets/images');
30-
return res.json({ url: 'Imagen guardada correctamente' });
36+
37+
clearTemporaryFiles(file.path ?? 'image/assets/images');
38+
return res.json({ message: 'Image saved successfully', url: secure_url });
3139
} catch (error: unknown) {
32-
return res.json({ message: (<Error>error).message });
40+
rollbar.error(error as LogArgument);
41+
return res
42+
.status(500)
43+
.json({ error: 'An error occurred while uploading the image' });
3344
}
3445
}
3546

3647
/**
3748
* @description Get a random waifu from the collection!
38-
* @param { Response } res object with the waifu
49+
* @param { Response } res - object with the waifu
3950
* @query size - number of items to retrieve
4051
* @query tag_id - tag to filter
4152
* @returns { Promise<Response> } An url with the waifu image hosted in cloudinary
4253
*/
4354
async getRandomImage(req: Request, res: Response): Promise<Response> {
4455
try {
45-
const { size, tag_id } = req.query as unknown as Query;
46-
const getImages = await ImageService.getImage(size, tag_id);
47-
return res.json(getImages);
48-
} catch {
49-
return res.json({ message: 'No se pudo encontrar alguna imagen' });
56+
const { size, tag_id } = req.query as Query;
57+
const images = await ImageService.getImage(size, tag_id);
58+
return res.json(images);
59+
} catch (error: unknown) {
60+
rollbar.error(error as LogArgument);
61+
return res
62+
.status(500)
63+
.json({ error: 'An error occurred while getting the image' });
5064
}
5165
}
5266

@@ -58,11 +72,12 @@ class ImageController {
5872
*/
5973
async getImages(req: Request, res: Response): Promise<Response> {
6074
try {
61-
const { tag_id } = req.query as unknown as Query;
75+
const { tag_id } = req.query as Query;
6276
const images = await ImageService.getAllImages(tag_id);
6377
return res.json(images);
6478
} catch (error: unknown) {
65-
return res.json({ message: (<Error>error).message });
79+
rollbar.error(error as LogArgument);
80+
return res.json({ error });
6681
}
6782
}
6883
}

src/image/image-repository.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22
import Image from './schema/image-schema';
33
import Tag from '../tag/schema/tag-schema';
44
import { hasTag } from '../common/utils/ref';
5-
import { IImage } from './interfaces/image-interface';
5+
import { ImageProp } from './interfaces/image-interface';
66

77
class ImageRepository {
88
/**
99
* @description Creates a new Image in the database
10-
* @param {IImage} image - the image to create in the database
11-
* @return { Promise<IImage> } - A new image created
10+
* @param { ImageProp } image - the image to create in the database
11+
* @return { Promise<ImageProp> } - A new image created
1212
*/
13-
async create(image: IImage): Promise<IImage> {
13+
async create(image: ImageProp): Promise<ImageProp> {
1414
const tagExists = await Tag.findOne({ tag_id: { $eq: image.tag } });
1515
const _idTag = tagExists?._id;
1616
return Image.create({
@@ -22,9 +22,9 @@ class ImageRepository {
2222
/**
2323
* @description Get all images from the database
2424
* @param tag_id - the id from the tag to retrieve
25-
* @return { Promise<IImage[]> } An array of images
25+
* @return { Promise<ImageProp[]> } An array of images
2626
*/
27-
async findImages(tag_id?: number): Promise<IImage[]> {
27+
async findImages(tag_id?: number): Promise<ImageProp[]> {
2828
const images = await Image.find().populate(hasTag(tag_id));
2929
return images.filter(image => image.tag !== null);
3030
}

src/image/image-service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class ImageService {
8080
tag: image.tag,
8181
};
8282
});
83-
const randomArray = urls.sort(randomUrls);
83+
const randomArray = urls.toSorted(randomUrls);
8484
const sizeArray = randomArray.slice(0, size);
8585
const randomUrl = urls[Math.floor(Math.random() * urls.length)];
8686
return size === undefined || null ? randomUrl : sizeArray;

src/image/interfaces/image-interface.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,3 @@ export interface ImageTypeResponse extends ImageType {
2020
}
2121

2222
export type ImageProp = Omit<ImageProps, "save">;
23-
export type IImage = ImageProp;

src/user/interfaces/user-interface.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import { Boom } from "@hapi/boom";
2+
import { NextFunction, Response } from "express";
3+
14
export interface UsernameType {
25
username: string;
36
}
@@ -22,3 +25,5 @@ export type UserPicture = Omit<
2225
IUser,
2326
"username" | "password" | "save" | "isAdmin"
2427
>;
28+
29+
export type MiddlewareUser = Boom | NextFunction | Response | void;

src/user/user-middleware.ts

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,84 +1,105 @@
11
// External Modules
2-
import boom, { Boom } from '@hapi/boom';
2+
import boom from '@hapi/boom';
33
import { NextFunction, Request, Response } from 'express';
44
import bcrypt from 'bcrypt';
55
import jwt from 'jsonwebtoken';
6+
import { LogArgument } from 'rollbar';
67

78
// Internal Modules
89
import User from './schema/user-schema';
9-
import { Config } from '../app/config/config';
10-
import { UsernameType } from './interfaces/user-interface';
10+
import { Config } from 'src/app/config/config';
11+
import { rollbar } from 'src/app/config/rollbar';
12+
import { MiddlewareUser } from './interfaces/user-interface';
1113

1214
/**
13-
* @description Validates if the user exist in the database
15+
* Checks if a user with the given username already exists in the database
16+
* @param {Object} req.body - request body containing the username
1417
* @param {string} req.body.username - the username to check
15-
* @returns {Response<Boom | NextFunction>} Response with the next function
18+
* @param {Response} res - response object
19+
* @param {NextFunction} next - next function
20+
* @returns {Promise<MiddlewareUser | NextFunction>}
1621
*/
1722
const userExists = async (
1823
req: Request,
1924
res: Response,
2025
next: NextFunction,
21-
): Promise<Boom | NextFunction | Response | unknown> => {
22-
const { username }: UsernameType = req.body;
23-
const user: UsernameType | null = await User.findOne({
24-
username: { $eq: username },
25-
});
26-
return user ? res.json(boom.conflict('User already exists')) : next();
26+
): Promise<MiddlewareUser> => {
27+
const { username } = req.body;
28+
29+
try {
30+
const user = await User.findOne({ username: { $eq: username } });
31+
32+
if (user) {
33+
return res.status(409).json({ error: 'User already exists' });
34+
}
35+
36+
return next();
37+
} catch (error: unknown) {
38+
rollbar.error(error as LogArgument);
39+
return res.status(500).json({ error });
40+
}
2741
};
2842

2943
/**
3044
* @description Validates the user username & password of the request body
3145
* @param {string} req.body.username - the username to validate
3246
* @param {string} req.body.password - the password to validate
33-
* @returns {Response<Boom | NextFunction} if the username && password match then next() else return an error
47+
* @returns {Promise<MiddlewareUser | NextFunction>} if the username & password match, then next() else return an error
3448
*/
3549
const validateUser = async (
3650
req: Request,
3751
res: Response,
3852
next: NextFunction,
39-
): Promise<Boom | NextFunction | Response | unknown> => {
53+
): Promise<MiddlewareUser> => {
4054
try {
4155
const { username, password } = req.body;
42-
const userExists = await User.findOne({ username: { $eq: username } });
43-
const user = userExists?.username;
44-
const pass = userExists?.password;
45-
const isMatch = pass && (await bcrypt.compare(password, pass)); // Compare the password with the hash password
46-
const isValid = user && isMatch;
47-
return isValid ? next() : res.json(boom.badRequest('Invalid credentials'));
56+
const user = await User.findOne({ username: { $eq: username } });
57+
58+
if (user?.password && (await bcrypt.compare(password, user.password))) {
59+
return next();
60+
}
61+
62+
return res.status(401).json(boom.unauthorized('Invalid credentials'));
4863
} catch (error) {
49-
return res.status(400) && res.json(boom.badRequest('Something went wrong'));
64+
return res.status(400).json(boom.badRequest('User not found'));
5065
}
5166
};
5267

5368
/**
5469
* @description Validate if the user is administrator
5570
* @param {string} req.body.username - the username to validate
56-
* @returns {Promise<Boom | NextFunction | Response | unknown>} if the user is admin then next() else return an error
71+
* @returns {Promise<MiddlewareUser | NextFunction>} if the user is admin then next() else return an error
5772
*/
5873
const isAdmin = async (
5974
req: Request,
6075
res: Response,
6176
next: NextFunction,
62-
): Promise<Boom | NextFunction | Response | unknown> => {
77+
): Promise<MiddlewareUser> => {
78+
const { username } = req.body;
79+
6380
try {
64-
const { username } = req.body;
6581
const user = await User.findOne({ username: { $eq: username } });
66-
return user?.isAdmin ? next() : res.json(boom.unauthorized('Not admin'));
82+
83+
if (user?.isAdmin) {
84+
return next();
85+
}
86+
87+
return res.status(401).json(boom.unauthorized('Not admin'));
6788
} catch (error) {
68-
return res.status(400) && res.json(boom.badRequest('User not found'));
89+
return res.status(400).json(boom.badRequest('User not found'));
6990
}
7091
};
7192

7293
/**
73-
* @description Validate the jwt
94+
* @description Validate the JWT
7495
* @param {Authorization} req.headers - Authorization header with the token
7596
* @returns {Response<Boom | NextFunction>} Authorization error or next
7697
*/
7798
const validateToken = async (
7899
req: Request,
79100
res: Response,
80101
next: NextFunction,
81-
): Promise<Boom | NextFunction | Response | unknown> => {
102+
): Promise<MiddlewareUser> => {
82103
const { secret } = Config.jwt;
83104
try {
84105
const { authorization } = req.headers;
@@ -88,7 +109,7 @@ const validateToken = async (
88109
return decoded ? next() : res.json(boom.unauthorized());
89110
}
90111
} catch (error) {
91-
return res.status(401).json(boom.unauthorized('Invalid token'));
112+
return res.status(401).json(boom.unauthorized());
92113
}
93114
};
94115

0 commit comments

Comments
 (0)