From 4c23bce69fc6331e977229ad9ca3c9a8f1aadc8a Mon Sep 17 00:00:00 2001 From: danny-mhlv Date: Fri, 7 Oct 2022 15:10:18 +0300 Subject: [PATCH] Fixed problems with reading 'undefined's --- .../controller/papers.controller.ts | 2 +- src/core/domain/enums/page-order.enum.ts | 2 + src/core/interceptors/page.interceptor.ts | 4 +- src/core/services/common/search.service.ts | 14 +-- src/test/search.service.spec.ts | 93 ++++--------------- 5 files changed, 30 insertions(+), 85 deletions(-) diff --git a/src/application/controller/papers.controller.ts b/src/application/controller/papers.controller.ts index bdf507a..60d0f3a 100644 --- a/src/application/controller/papers.controller.ts +++ b/src/application/controller/papers.controller.ts @@ -1,4 +1,4 @@ -import { Controller, Get, HttpCode, Param, ParseUUIDPipe, Query, Req, UseFilters, UseInterceptors } from "@nestjs/common"; +import { Controller, Get, HttpCode, Param, ParseUUIDPipe, Query, 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"; diff --git a/src/core/domain/enums/page-order.enum.ts b/src/core/domain/enums/page-order.enum.ts index 61e5176..9e724bd 100644 --- a/src/core/domain/enums/page-order.enum.ts +++ b/src/core/domain/enums/page-order.enum.ts @@ -19,6 +19,8 @@ export enum Order { * @returns Appropriate enum-member */ export function toOrder(str: string): Order { + if (!str) return Order.DESC; + switch (str) { case 'asc': return Order.ASC; case 'desc': return Order.DESC; diff --git a/src/core/interceptors/page.interceptor.ts b/src/core/interceptors/page.interceptor.ts index 8a78ded..0551733 100644 --- a/src/core/interceptors/page.interceptor.ts +++ b/src/core/interceptors/page.interceptor.ts @@ -1,5 +1,5 @@ import { HttpService } from "@nestjs/axios"; -import { CACHE_MANAGER, CallHandler, ExecutionContext, Inject, Injectable, NestInterceptor } from "@nestjs/common"; +import { BadRequestException, CACHE_MANAGER, CallHandler, ExecutionContext, Inject, Injectable, InternalServerErrorException, NestInterceptor } from "@nestjs/common"; import { Observable, map, take, switchMap, of } from "rxjs"; import { PageDto } from "../domain/dtos"; import { EsTime } from "../domain/enums/es-time.enum"; @@ -63,7 +63,7 @@ export class PageInterceptor implements NestInterceptor { }; // Check if the performed search is a backwards search - let data = res.hits.hits; + let data = res?.hits?.hits; // Omitting the redundant info and leaving only the document data = data.map((el) => el._source); // Change the order if set diff --git a/src/core/services/common/search.service.ts b/src/core/services/common/search.service.ts index b72685e..b037ab5 100644 --- a/src/core/services/common/search.service.ts +++ b/src/core/services/common/search.service.ts @@ -1,5 +1,5 @@ import { HttpService } from "@nestjs/axios"; -import { GatewayTimeoutException, Injectable, NotFoundException } from "@nestjs/common"; +import { GatewayTimeoutException, ImATeapotException, Injectable, NotFoundException } from "@nestjs/common"; import { map, take } from "rxjs"; import { EsResponseDto, SearchQueryDto} from "../../domain/dtos"; import { EsQueryDto } from "../../domain/dtos/elastic/es-query.dto"; @@ -28,7 +28,7 @@ export class SearchService { /** * Finds a paper by its own ID - * @param uuid + * @param uuid String, that represents unique identifier of a paper * @returns Elasticsearch hits or an error object */ async findByID(uuid: string): Promise { // Should I change 'object' to specific DTO? @@ -48,12 +48,12 @@ export class SearchService { })) ?.pipe(take(1), map(axiosRes => axiosRes.data)) .subscribe((res: EsResponseDto) => { - if (!res.hits.hits.length) { - reject(new NotFoundException); - } if (res.timed_out) { reject(new GatewayTimeoutException('Elasticsearch Timed Out')); } + if (!res?.hits?.hits?.length) { + reject(new NotFoundException); + } resolve(res); }); } catch (error) { @@ -65,7 +65,7 @@ export class SearchService { /** * Finds relevant documents by context using the given query string * @param query, - * @returns Elasticsearch hits or an error object + * @returns Elasticsearch response */ async findByContext(query: SearchQueryDto): Promise { // Contruct a body for querying Elasticsearch @@ -86,7 +86,7 @@ export class SearchService { headers: {'Content-Type': 'application/json'}, })) ?.pipe(take(1), map(axiosRes => axiosRes.data)) - .subscribe((res: EsResponseDto) => { + ?.subscribe((res: EsResponseDto) => { if (res.timed_out) { reject(new GatewayTimeoutException('Elasticsearch Timed Out')); } diff --git a/src/test/search.service.spec.ts b/src/test/search.service.spec.ts index 689c43c..2fa1197 100644 --- a/src/test/search.service.spec.ts +++ b/src/test/search.service.spec.ts @@ -3,7 +3,7 @@ import { GatewayTimeoutException, HttpException } from "@nestjs/common"; import { ConfigModule } from "@nestjs/config"; import { Test } from "@nestjs/testing"; import { of } from "rxjs"; -import { EsQueryDto, EsResponseDto } from "src/core/domain"; +import { SearchQueryDto } from "src/core/domain"; import { SearchService } from "src/core/services/common/search.service"; describe('Unit tests for SearchService', () => { @@ -42,26 +42,6 @@ describe('Unit tests for SearchService', () => { expect(httpGetSpy).toHaveBeenCalled(); }); - // it('Should send correct data via HttpService.get() body parameter', () => { - // let httpGetSpy = jest.spyOn(httpService, 'get'); - - // const uuid = 'thisIsUUID_Provided'; - // searchService.findByID(uuid); - // expect(httpGetSpy).toHaveBeenCalledWith<[string, object]>(expect.anything(), { - // data: { - // size: 1, - // query: { - // query_string: { - // query: 'id:' + uuid - // } - // }, - // search_after: undefined, - // sort: undefined, - // }, - // headers: { 'Content-Type': 'application/json' } - // }); - // }); - it('Should call HttpService.get() with correct URI and port number', () => { let httpGetSpy = jest.spyOn(httpService, 'get'); @@ -76,28 +56,6 @@ describe('Unit tests for SearchService', () => { expect(searchService.findByID('')).toBeInstanceOf(Promise); }); - // it('Should return a Promise with EsResponseDto', () => { - // // Axios response mock - // httpService.get = jest.fn().mockReturnValueOnce( - // of({ - // status: undefined, - // statusText: undefined, - // headers: undefined, - // config: undefined, - // data: { - // took: 1, - // timed_out: false, - // hits: { - // total: {}, - // hits: [{}] - // } - // }, - // }) - // ); - - // expect(searchService.findByID('')).resolves.toBeInstanceOf(EsResponseDto) - // }); - // Errors it('Should throw 504 | GatewayTimeoutException', () => { // Axios response mock @@ -136,25 +94,27 @@ describe('Unit tests for SearchService', () => { it('Should touch HttpService.get() method', () => { let httpGetSpy = jest.spyOn(httpService, 'get'); - searchService.findByContext(null); + searchService.findByContext({query: ""}); expect(httpGetSpy).toHaveBeenCalled(); }); it('Should send correct data via HttpService.get() body parameter', () => { let httpGetSpy = jest.spyOn(httpService, 'get'); - let es_query = new EsQueryDto(); - es_query = { - query: { - query_string: { - query: 'thisIsTheQuery!' - } - } - } + const query = new SearchQueryDto('keyword', 1, 32, 'desc'); - // searchService.findByContext(es_query); + searchService.findByContext(query); expect(httpGetSpy).toHaveBeenCalledWith<[string, object]>(expect.anything(), { - data: es_query, + data: { + query: { + query_string: { + query: 'keyword', + default_field: 'content', + } + }, + from: 32, + size: 1, + }, headers: { 'Content-Type': 'application/json' } }); }); @@ -162,7 +122,7 @@ describe('Unit tests for SearchService', () => { it('Should call HttpService.get() with correct URI and port number', () => { let httpGetSpy = jest.spyOn(httpService, 'get'); - searchService.findByContext(null); + searchService.findByContext({query: ""}); expect(httpGetSpy).toHaveBeenCalledWith<[string, object]>( `http://${process.env.ES_CONTAINER_NAME}:${process.env.ES_PORT}/_search`, expect.anything() @@ -170,26 +130,9 @@ describe('Unit tests for SearchService', () => { }); it('Should return a Promise', () => { - expect(searchService.findByContext(null)).toBeInstanceOf(Promise); + expect(searchService.findByContext({query: ""})).toBeInstanceOf(Promise); }); - // it('Should return a Promise with EsResponseDto', () => { - // // Axios response mock - // httpService.get = jest.fn().mockReturnValueOnce( - // of({ - // status: undefined, - // statusText: undefined, - // headers: undefined, - // config: undefined, - // data: { - // dummy: 'dum' - // } - // }) - // ); - - // expect(searchService.findByContext(null)).resolves.toMatchObject(null); - // }); - // Errors it('Should throw 504 | GatewayTimeoutException', () => { // Axios response mock @@ -206,7 +149,7 @@ describe('Unit tests for SearchService', () => { }) ); - searchService.findByContext(null).catch((err) => { + searchService.findByContext({query: ""}).catch((err) => { expect(err).toBeInstanceOf(GatewayTimeoutException); }); }); @@ -216,7 +159,7 @@ describe('Unit tests for SearchService', () => { throw new HttpException({ oops: 'sorry' }, 999); }); - searchService.findByContext(null).catch((err) => { + searchService.findByContext({query: ""}).catch((err) => { expect(err).toBeInstanceOf(HttpException); expect(err.response).toEqual({ oops: 'sorry' }); expect(err.status).toEqual(999);