From 83b5cc6e5e865415d2e3f2d1401ff61e2ba281e5 Mon Sep 17 00:00:00 2001 From: danny-mhlv Date: Fri, 7 Oct 2022 13:18:50 +0300 Subject: [PATCH] Fixed validation issues. Cleared unnecessary DTOs --- .../controller/papers.controller.ts | 15 +++--- src/core/domain/dtos/index.ts | 4 +- src/core/domain/dtos/request.dto.ts | 48 ----------------- src/core/domain/dtos/search-q.dto.ts | 33 +++++++----- src/core/domain/dtos/search-result.dto.ts | 53 ------------------- src/core/interceptors/page.interceptor.ts | 28 +++------- src/core/pipes/query-str.pipe.ts | 13 ----- src/core/services/common/search.service.ts | 24 ++++++--- src/infrastructure/modules/app.module.ts | 1 - src/main.ts | 2 +- src/test/search.service.spec.ts | 2 +- 11 files changed, 53 insertions(+), 170 deletions(-) delete mode 100644 src/core/domain/dtos/request.dto.ts delete mode 100644 src/core/domain/dtos/search-result.dto.ts delete mode 100644 src/core/pipes/query-str.pipe.ts diff --git a/src/application/controller/papers.controller.ts b/src/application/controller/papers.controller.ts index 50b30e7..bdf507a 100644 --- a/src/application/controller/papers.controller.ts +++ b/src/application/controller/papers.controller.ts @@ -1,11 +1,9 @@ -import { Body, Controller, Get, HttpCode, Param, ParseUUIDPipe, Query, Req, UseFilters, UseInterceptors, UsePipes } from "@nestjs/common"; +import { Controller, Get, HttpCode, Param, ParseUUIDPipe, Query, Req, UseFilters, UseInterceptors } from "@nestjs/common"; import { SearchService } from "../../core/services/common/search.service"; import { PageInterceptor } from "../../core/interceptors/page.interceptor"; import { ApiExtraModels, ApiGatewayTimeoutResponse, ApiOperation, ApiResponse, ApiTags } from "@nestjs/swagger"; -import { RequestDto } from "../../core/domain/dtos/request.dto"; -import { EsHitDto, EsResponseDto, PageDto, PaperDto } from "../../core/domain"; +import { EsHitDto, EsResponseDto, PageDto, PaperDto, SearchQueryDto } from "../../core/domain"; import { HttpExceptionFilter } from "src/core/filters/http-exception.filter"; -import { QueryStringPipe } from "src/core/pipes/query-str.pipe"; /** * /papers/ route controller @@ -15,8 +13,7 @@ import { QueryStringPipe } from "src/core/pipes/query-str.pipe"; version: '1', path: 'papers', }) -@ApiExtraModels(RequestDto, EsHitDto, EsResponseDto) -// @UseInterceptors(CacheInterceptor) +@ApiExtraModels(EsHitDto, EsResponseDto) export class PapersController { constructor(private searchService: SearchService) {} @@ -39,10 +36,10 @@ export class PapersController { description: 'Elasticsearch request timed out' }) @Get('search') - @UseInterceptors(PageInterceptor) @HttpCode(200) - getByContext(@Req() request: RequestDto): Promise { - return this.searchService.findByContext(request.es_query).then( + @UseInterceptors(PageInterceptor) + getByContext(@Query() request: SearchQueryDto): Promise { + return this.searchService.findByContext(request).then( (response) => { return response; }, diff --git a/src/core/domain/dtos/index.ts b/src/core/domain/dtos/index.ts index d24be14..ac35a7b 100644 --- a/src/core/domain/dtos/index.ts +++ b/src/core/domain/dtos/index.ts @@ -3,6 +3,4 @@ export * from './elastic/es-response.dto'; export * from './elastic/es-hit.dto'; export * from './page.dto'; export * from './search-q.dto'; -export * from './search-result.dto'; -export * from './paper.dto'; -export * from './request.dto'; \ No newline at end of file +export * from './paper.dto'; \ No newline at end of file diff --git a/src/core/domain/dtos/request.dto.ts b/src/core/domain/dtos/request.dto.ts deleted file mode 100644 index 973264d..0000000 --- a/src/core/domain/dtos/request.dto.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { ApiExtraModels, ApiProperty, ApiPropertyOptional } from "@nestjs/swagger"; -import { IsDefined, IsNotEmpty, IsOptional } from "class-validator"; -import { EsQueryDto } from "./elastic/es-query.dto"; -import { SearchQueryDto } from "./search-q.dto"; - -/** - * List of allowed properties in this DTO - */ -const allowedProperties = ['query', 'es_query']; - -/** - * Request object, which contains query parameters and Elasticsearch query object - */ -@ApiExtraModels() -export class RequestDto { - /** - * Query parameters object - */ - @IsDefined() - @IsNotEmpty() - @ApiProperty({ - type: SearchQueryDto, - description: 'Actual query with parameters acquired from the request', - example: {} - }) - query: SearchQueryDto; - - /** - * Elasticsearch query object - */ - @IsOptional() - @ApiPropertyOptional({ - type: EsQueryDto, - description: 'Elasticsearch query body constructed by pagination mechanism', - example: {}, - }) - es_query?: EsQueryDto; - - /** - * Constructs an object with provided parameters - * @param query - * @param es_query - */ - constructor(query: SearchQueryDto, es_query: EsQueryDto) { - this.query = query; - this.es_query = es_query; - } -} \ No newline at end of file diff --git a/src/core/domain/dtos/search-q.dto.ts b/src/core/domain/dtos/search-q.dto.ts index ee4d929..0651a03 100644 --- a/src/core/domain/dtos/search-q.dto.ts +++ b/src/core/domain/dtos/search-q.dto.ts @@ -1,10 +1,11 @@ import { ApiExtraModels, ApiPropertyOptional } from "@nestjs/swagger"; -import { IsDefined, IsInt, IsNotEmpty, IsOptional, IsString } from "class-validator"; +import { Type } from "class-transformer"; +import { IsDefined, IsInt, IsNotEmpty, IsOptional, IsString, Min } from "class-validator"; /** * List of allowed properties in this DTO */ -const allowedProperties = ['query', 'pagen', 'limit', 'order']; +const allowedProperties = ['query', 'limit', 'offset', 'order']; /** * Elasticsearch response DTO @@ -28,9 +29,12 @@ export class SearchQueryDto { */ @IsOptional() @IsInt() + @Type(() => Number) + @Min(1) @ApiPropertyOptional({ description: 'Limits the number of displayed elements', example: 10, + required: false }) limit?: number; @@ -39,26 +43,26 @@ export class SearchQueryDto { */ @IsOptional() @IsInt() + @Type(() => Number) + @Min(0) @ApiPropertyOptional({ description: 'Offset from the start of the list of hits', example: 0, + required: false, }) offset?: number; /** * Indicates in which order elements need to be displayed. */ - @IsOptional() - @IsString() - @ApiPropertyOptional({ - description: 'Indicates in which order elements need to be displayed', - example: 'asc', - }) - order?: string; - - /** - * - */ + @IsOptional() + @IsString() + @ApiPropertyOptional({ + description: 'Indicates in which order elements need to be displayed', + example: 'asc', + required: false, + }) + order?: string; /** * Constructs an object with provided parameters @@ -67,9 +71,10 @@ export class SearchQueryDto { * @param limit * @param order */ - constructor(query: string, page: number, limit: number, order: string) { + constructor(query: string = undefined, limit: number = 10, offset: number = 0, order: string = undefined) { this.query = query; this.limit = limit; + this.offset = offset; this.order = order; } } \ No newline at end of file diff --git a/src/core/domain/dtos/search-result.dto.ts b/src/core/domain/dtos/search-result.dto.ts deleted file mode 100644 index c926308..0000000 --- a/src/core/domain/dtos/search-result.dto.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { ApiExtraModels, ApiProperty } from "@nestjs/swagger"; -import { IsArray, IsDefined, IsInt, IsNotEmpty } from "class-validator"; -import { EsResponseDto } from "./elastic/es-response.dto"; - -/** - * List of allowed properties in this DTO - */ -const allowedProperties = ['data', 'status']; - -/** - * Elasticsearch response DTO - */ -@ApiExtraModels() -export class SearchResultDto { - /** - * Status code - */ - @IsDefined() - @IsNotEmpty() - @IsInt() - @ApiProperty({ - description: 'Status code', - example: 200, - }) - statusCode: number; - - /** - * All the data acquired. - */ - @IsDefined() - @IsNotEmpty() - @IsArray() - @ApiProperty({ - description: 'Data acquired from the Elasticsearch', - example: { - took: 1, - timed_out: false, - _shards: {}, - hits: {} - }, - }) - data: EsResponseDto; - - /** - * Constructs an object with provided parameters - * @param code - * @param data - */ - constructor(code: number, data: EsResponseDto) { - this.statusCode = code; - this.data = data; - } -} \ No newline at end of file diff --git a/src/core/interceptors/page.interceptor.ts b/src/core/interceptors/page.interceptor.ts index 73c2089..8a78ded 100644 --- a/src/core/interceptors/page.interceptor.ts +++ b/src/core/interceptors/page.interceptor.ts @@ -2,9 +2,6 @@ import { HttpService } from "@nestjs/axios"; import { CACHE_MANAGER, CallHandler, ExecutionContext, Inject, Injectable, NestInterceptor } from "@nestjs/common"; import { Observable, map, take, switchMap, of } from "rxjs"; import { PageDto } from "../domain/dtos"; -import { EsQueryDto } from "../domain/dtos/elastic/es-query.dto"; -import { RequestDto } from "../domain/dtos/request.dto"; -import { SearchQueryDto } from "../domain/dtos/search-q.dto"; import { EsTime } from "../domain/enums/es-time.enum"; import { Order, toOrder } from "../domain/enums/page-order.enum"; import { EsPit } from "../domain/interfaces/elastic/es-pit.interface"; @@ -43,29 +40,20 @@ export class PageInterceptor implements NestInterceptor { * @returns Page with content and metadata */ async intercept(context: ExecutionContext, next: CallHandler): Promise> { - const request: RequestDto = context.switchToHttp().getRequest(); - const query: SearchQueryDto = request.query; + const query = context.switchToHttp().getRequest().query; - const offset = !query.offset ? 0 : query.offset; - const limit = !query.limit ? 10 : query.limit; - const order = !query.order ? Order.DESC : query.order; + // const offset = !query.offset ? 0 : query.offset; + const offset = query.offset; + // const limit = !query.limit ? 10 : query.limit; + const limit = query.limit; + // const order = !query.order ? Order.DESC : query.order; + const order = query.order; - const prev_page = await this.cacheManager.get('prev_page'); + const prev_page = await this.cacheManager.get('prev_page'); if (prev_page) { if (offset == prev_page[1] && limit == prev_page[2] && order == prev_page[3]) return of(prev_page[0]); } - // Contruct a body for querying Elasticsearch - request.es_query = new EsQueryDto(); - request.es_query.query = { - query_string: { - query: query.query, - default_field: 'content', - } - }; - request.es_query.from = offset; - request.es_query.size = limit; - return next.handle().pipe( switchMap(async (res) => { // Setting the page meta-data diff --git a/src/core/pipes/query-str.pipe.ts b/src/core/pipes/query-str.pipe.ts deleted file mode 100644 index d9bfcca..0000000 --- a/src/core/pipes/query-str.pipe.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { ArgumentMetadata, BadRequestException, ImATeapotException, Injectable, PipeTransform } from "@nestjs/common"; -import { RequestDto } from "../domain"; - -@Injectable() -export class QueryStringPipe implements PipeTransform { - constructor() {} - - transform(value: RequestDto, metadata: ArgumentMetadata): RequestDto { - console.log(value.query.limit) - - return value; - } -} \ No newline at end of file diff --git a/src/core/services/common/search.service.ts b/src/core/services/common/search.service.ts index 4cd131b..b72685e 100644 --- a/src/core/services/common/search.service.ts +++ b/src/core/services/common/search.service.ts @@ -1,7 +1,7 @@ import { HttpService } from "@nestjs/axios"; import { GatewayTimeoutException, Injectable, NotFoundException } from "@nestjs/common"; import { map, take } from "rxjs"; -import { EsResponseDto} from "../../domain/dtos"; +import { EsResponseDto, SearchQueryDto} from "../../domain/dtos"; import { EsQueryDto } from "../../domain/dtos/elastic/es-query.dto"; /** @@ -32,10 +32,9 @@ export class SearchService { * @returns Elasticsearch hits or an error object */ async findByID(uuid: string): Promise { // Should I change 'object' to specific DTO? - let ESQ: EsQueryDto = new EsQueryDto; - - ESQ.size = 1; - ESQ.query = { + const es_query: EsQueryDto = new EsQueryDto(); + es_query.size = 1; + es_query.query = { query_string: { query: ('id:' + uuid), } @@ -44,7 +43,7 @@ export class SearchService { return new Promise((resolve, reject) => { try { (this.httpService.get(`http://${this.ES_IP}:${this.ES_PORT}/_search`, { - data: ESQ, + data: es_query, headers: {'Content-Type': 'application/json'}, })) ?.pipe(take(1), map(axiosRes => axiosRes.data)) @@ -68,7 +67,18 @@ export class SearchService { * @param query, * @returns Elasticsearch hits or an error object */ - async findByContext(es_query: EsQueryDto): Promise { + async findByContext(query: SearchQueryDto): Promise { + // Contruct a body for querying Elasticsearch + const es_query: EsQueryDto = new EsQueryDto(); + es_query.query = { + query_string: { + query: query.query, + default_field: 'content', + } + }; + es_query.from = query.offset; + es_query.size = query.limit; + return new Promise((resolve, reject) => { try { (this.httpService.get(`http://${this.ES_IP}:${this.ES_PORT}/_search`, { diff --git a/src/infrastructure/modules/app.module.ts b/src/infrastructure/modules/app.module.ts index 9b20b70..1eefb80 100644 --- a/src/infrastructure/modules/app.module.ts +++ b/src/infrastructure/modules/app.module.ts @@ -8,7 +8,6 @@ import * as modules from '../../core/modules' import { CommonModule } from './common/common.module'; import { PrometheusModule } from '@willsoto/nestjs-prometheus'; import { SearchModule } from './search.module'; -import { QueryStringPipe } from 'src/core/pipes/query-str.pipe'; /** * application modules list diff --git a/src/main.ts b/src/main.ts index 58e4e19..f413052 100644 --- a/src/main.ts +++ b/src/main.ts @@ -4,7 +4,7 @@ import { NestFactory } from '@nestjs/core'; import { AppModule } from './infrastructure/modules/app.module'; import { SwaggerModule, DocumentBuilder, SwaggerDocumentOptions } from '@nestjs/swagger'; import { ConfigService } from '@nestjs/config'; -import { QueryStringPipe } from './core/pipes/query-str.pipe'; + /** * Main entry point of the application * @returns Nothing diff --git a/src/test/search.service.spec.ts b/src/test/search.service.spec.ts index d773f4f..689c43c 100644 --- a/src/test/search.service.spec.ts +++ b/src/test/search.service.spec.ts @@ -152,7 +152,7 @@ describe('Unit tests for SearchService', () => { } } - searchService.findByContext(es_query); + // searchService.findByContext(es_query); expect(httpGetSpy).toHaveBeenCalledWith<[string, object]>(expect.anything(), { data: es_query, headers: { 'Content-Type': 'application/json' }