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

Сервер наносит ответный удар #8

Merged
merged 14 commits into from
Nov 4, 2024

Conversation

AdonaiJehosua
Copy link

@AdonaiJehosua AdonaiJehosua commented Oct 21, 2024

@keksobot keksobot changed the title Module6 task1 Сервер наносит ответный удар Oct 21, 2024
// Идем в другую коллекцию
{
'$lookup': {
'from': 'comments', // Искать будем в коллекции комментариев
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Увы, но попытка выровнять комментарии приводит к ошибкам eslint, а они по критериям недопустимы. Ты же используешь eslint, включен в твоей IDE?

private async _initExceptionFilters() {
this.server.use(this.appExceptionFilter.catch.bind(this.appExceptionFilter));
}

public async init() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В стартовых логах в терминале начинает сложно ориентироваться. Можно подумать о том, чтобы сообщение-заголовок группы, например Init controllers, перекрасить в отдельный цвет

@@ -29,6 +37,25 @@ export class RestApplication {
return this.databaseClient.connect(mongoUri);
}

private async _initServer() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_initServer название не отражает действительность. Всё-таки внутри этого метода происходит запуск сервера, а инициализация/создание в конструкторе класса. Я бы предложил название _startServer

) {
this.server = express();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Express по умолчанию добавляет во все ответы заголовок X-Powered-By: Express обозначающий, какой сервер отвечает на запросы. И конечно же не следует предоставлять в публичных api неполезную (с точки зрения бизнес логики и пользователя) информацию. С точки зрения безопасности это может быть важный момент. Так как мы резко уменьшаем коридор поиска лазеек для потенциального злоумышленника, так как сразу говорим ему какой у нас веб-сервер.
Для того, чтобы отключить этот заголовок, следует сразу после создания инстанса express'а вызвать команду, думаю, что ты можешь это сделать прямо в конструкторе:

this.server = express();
this.server.disable('x-powered-by');

@@ -59,7 +59,6 @@ export class ImportCommand implements Command {
hostId: user.id,
bedrooms: offer.bedrooms,
maxAdults: offer.maxAdults,
commentCount: offer.commentCount
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В shared/helpers/offer.ts ts ругается из-за того, что в типизации не почистил следом. Нужно до-урегулировать отличия. В конце концов можно вроде изначально прокидывать 0, потом пусть пересчитывается

@@ -5,45 +5,34 @@ import {Logger} from '../../libs/logger/index.js';
import {CommentEntity} from './comment.entity.js';
import {DocumentType, types} from '@typegoose/typegoose';
import {CreateCommentDto} from './dto/create-comment.dto.js';
import {MAX_COMMENTS_AMOUNT} from '../../const/index.js';
import {OfferService} from '../offer/index.js';

@injectable()
export class DefaultCommentService implements CommentService {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default сбивает с толку. Если есть необходимость различать имя для интерфейса и самого класса, то лучше использовать следующую нотация CommentServiceInterface и CommentService

@nikopol-fw nikopol-fw merged commit 1aded15 into htmlacademy-nodejs-api:master Nov 4, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants