Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat/added validation for userId #4

Merged
merged 2 commits into from
Nov 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion config/custom-environment-variables.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,8 @@
}
},
"kafkaProducer": {},
"outputTopic": "KAFKA_OUTPUT_TOPIC"
"outputTopic": "KAFKA_OUTPUT_TOPIC",
"application": {
"userValidation": "USER_ID_DOMAIN"
}
}
5 changes: 4 additions & 1 deletion config/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,8 @@
},
"kafkaConsumer": {},
"kafkaProducer": {},
"outputTopic": "topic"
"outputTopic": "topic",
"application": {
"userValidation": "@mycompany.net"
}
}
5 changes: 4 additions & 1 deletion config/test.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@
},
"kafkaConsumer": {},
"kafkaProducer": {},
"outputTopic": "testTopic"
"outputTopic": "testTopic",
"application": {
"userValidation": "@mycompany.net"
}
}
8 changes: 8 additions & 0 deletions src/common/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,11 @@ export class NotFoundError extends Error implements HttpError {
super(message);
}
}

export class BadRequestError extends Error implements HttpError {
public readonly status = httpStatus.BAD_REQUEST;

public constructor(message: string) {
super(message);
}
}
10 changes: 8 additions & 2 deletions src/feedback/models/feedbackManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Producer } from 'kafkajs';
import { SERVICES } from '../../common/constants';
import { FeedbackResponse, GeocodingResponse, IConfig } from '../../common/interfaces';
import { RedisClient } from '../../redis';
import { NotFoundError } from '../../common/errors';
import { NotFoundError, BadRequestError } from '../../common/errors';
import { IFeedbackModel } from './feedback';

@injectable()
Expand All @@ -19,6 +19,12 @@ export class FeedbackManager {
public async createFeedback(feedback: IFeedbackModel): Promise<FeedbackResponse> {
const requestId = feedback.request_id;
const userId = feedback.user_id;
const userValidation = this.config.get<string>('application.userValidation');

if (!userId.endsWith(userValidation)) {
throw new BadRequestError(`user_id not valid. valid user_id ends with "${userValidation}"`);
}

const feedbackResponse: FeedbackResponse = {
requestId: requestId,
chosenResultId: feedback.chosen_result_id,
Expand All @@ -44,7 +50,7 @@ export class FeedbackManager {
this.logger.error({ msg: `Redis Error: ${(error as Error).message}` });
throw error;
}
throw new NotFoundError('the current request was not found');
throw new NotFoundError('The current request was not found');
}

public async send(message: FeedbackResponse): Promise<void> {
Expand Down
17 changes: 15 additions & 2 deletions tests/integration/feedback/feedback.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('feedback', function () {
const feedbackModel: IFeedbackModel = {
request_id: redisKey,
chosen_result_id: 3,
user_id: 'user1',
user_id: 'user1@mycompany.net',
};
const response = await requestSender.createFeedback(feedbackModel);

Expand All @@ -60,6 +60,19 @@ describe('feedback', function () {
const feedbackModel: any = {
request_id: '4ca82def-e73f-4b57-989b-3e285034b971',
chosen_result_id: '1',
user_id: '[email protected]',
};
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
const response = await requestSender.createFeedback(feedbackModel);

expect(response.status).toBe(httpStatusCodes.BAD_REQUEST);
});

it('should return 400 status code because user_id is not valid', async function () {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const feedbackModel: any = {
request_id: '4ca82def-e73f-4b57-989b-3e285034b971',
chosen_result_id: 1,
user_id: 'user1',
};
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
Expand All @@ -74,7 +87,7 @@ describe('feedback', function () {
const feedbackModel: IFeedbackModel = {
request_id: '4ca82def-e73f-4b57-989b-3e285034b971',
chosen_result_id: 1,
user_id: 'user1',
user_id: 'user1@mycompany.net',
};
const response = await requestSender.createFeedback(feedbackModel);

Expand Down
29 changes: 24 additions & 5 deletions tests/unit/feedback/models/feedbackModel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { createClient } from 'redis';
import { Producer } from 'kafkajs';
import { FeedbackManager } from '../../../../src/feedback/models/feedbackManager';
import { IFeedbackModel } from '../../../../src/feedback/models/feedback';
import { NotFoundError } from '../../../../src/common/errors';
import { BadRequestError, NotFoundError } from '../../../../src/common/errors';
import { RedisClient } from '../../../../src/redis';

const mockProducer = {
Expand Down Expand Up @@ -47,7 +47,7 @@ describe('FeedbackManager', () => {
it('should create feedback without errors', async function () {
const requestId = '417a4635-0c59-4b5c-877c-45b4bbaaac7a';
const chosenResultId = 3;
const userId = 'user1';
const userId = 'user1@mycompany.net';

const feedbackRequest: IFeedbackModel = { request_id: requestId, chosen_result_id: chosenResultId, user_id: userId };
(mockedRedis.get as jest.Mock).mockResolvedValue('{ "geocodingResponse": "completed" }');
Expand All @@ -62,15 +62,30 @@ describe('FeedbackManager', () => {
expect(mockedRedis.get).toHaveBeenCalledTimes(1);
});

it('should not create feedback when request_id is not found', async function () {
it('should not create feedback when user_id is not valid', async function () {
const feedbackRequest: IFeedbackModel = { request_id: '417a4635-0c59-4b5c-877c-45b4bbaaac7a', chosen_result_id: 3, user_id: 'user1' };
const feedback = feedbackManager.createFeedback(feedbackRequest);

await expect(feedback).rejects.toThrow(BadRequestError);
});

it('should not create feedback when request_id is not found', async function () {
const feedbackRequest: IFeedbackModel = {
request_id: '417a4635-0c59-4b5c-877c-45b4bbaaac7a',
chosen_result_id: 3,
user_id: '[email protected]',
};
const feedback = feedbackManager.createFeedback(feedbackRequest);

await expect(feedback).rejects.toThrow(NotFoundError);
});

it('should not be able to upload feedback to kafka', async function () {
const feedbackRequest: IFeedbackModel = { request_id: '417a4635-0c59-4b5c-877c-45b4bbaaac7a', chosen_result_id: 3, user_id: 'user1' };
const feedbackRequest: IFeedbackModel = {
request_id: '417a4635-0c59-4b5c-877c-45b4bbaaac7a',
chosen_result_id: 3,
user_id: '[email protected]',
};
(mockedRedis.get as jest.Mock).mockResolvedValue('{ "geocodingResponse": "completed" }');
mockProducer.send.mockRejectedValue(new Error('Kafka error'));

Expand All @@ -80,7 +95,11 @@ describe('FeedbackManager', () => {
});

it('should not be able to upload feedback to kafka because redis is unavailable', async function () {
const feedbackRequest: IFeedbackModel = { request_id: '417a4635-0c59-4b5c-877c-45b4bbaaac7a', chosen_result_id: 3, user_id: 'user1' };
const feedbackRequest: IFeedbackModel = {
request_id: '417a4635-0c59-4b5c-877c-45b4bbaaac7a',
chosen_result_id: 3,
user_id: '[email protected]',
};
(mockedRedis.get as jest.Mock).mockRejectedValue(new Error('Redis error'));

const feedback = feedbackManager.createFeedback(feedbackRequest);
Expand Down