From 33c9e65961e67fdff36c854a7ae9f78c3c1ccd0c Mon Sep 17 00:00:00 2001 From: Audrius Vaitonis Date: Mon, 13 May 2024 11:30:48 +0100 Subject: [PATCH] feat(DTFS2-7052): change GET /geospatial/addresses/postcode?postcode= empty response from 200 to 404 --- src/constants/geospatial.constant.ts | 5 +- .../ordnance-survey.service.test.ts | 2 +- .../get-addresses-by-postcode-query.dto.ts | 5 +- .../dto/get-addresses-response.dto.ts | 2 +- .../geospatial/geospatial.controller.test.ts | 14 +++-- .../geospatial/geospatial.controller.ts | 2 +- .../geospatial/geospatial.service.test.ts | 8 ++- src/modules/geospatial/geospatial.service.ts | 6 +- .../get-docs-yaml.api-test.ts.snap | 4 +- .../exposure-period.api-test.ts | 7 +++ .../get-address-by-postcode.api-test.ts | 60 ++++++++++++++++--- .../get-geospatial-addresses-generator.ts | 4 +- 12 files changed, 91 insertions(+), 28 deletions(-) diff --git a/src/constants/geospatial.constant.ts b/src/constants/geospatial.constant.ts index d4d3102f..f3accec2 100644 --- a/src/constants/geospatial.constant.ts +++ b/src/constants/geospatial.constant.ts @@ -4,7 +4,10 @@ export const GEOSPATIAL = { DATASET: 'DPA', }, EXAMPLES: { - POSTCODE: 'SW1A 2AQ', + ENGLISH_POSTCODE: 'SW1A 2AQ', + NORTHERN_IRELAND_POSTCODE: 'BT7 3GG', + WALES_POSTCODE: 'SY23 3AR', + SCOTLAND_POSTCODE: 'EH1 1JF', ORGANISATION_NAME: 'CHURCHILL MUSEUM & CABINET WAR ROOMS', ADDRESS_LINE_1: 'CLIVE STEPS KING CHARLES STREET', LOCALITY: 'LONDON', diff --git a/src/helper-modules/ordnance-survey/ordnance-survey.service.test.ts b/src/helper-modules/ordnance-survey/ordnance-survey.service.test.ts index 2bc641f9..fe3a1dbc 100644 --- a/src/helper-modules/ordnance-survey/ordnance-survey.service.test.ts +++ b/src/helper-modules/ordnance-survey/ordnance-survey.service.test.ts @@ -19,7 +19,7 @@ describe('OrdnanceSurveyService', () => { let configServiceGet: jest.Mock; let service: OrdnanceSurveyService; - const testPostcode = GEOSPATIAL.EXAMPLES.POSTCODE; + const testPostcode = GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE; const testKey = valueGenerator.string({ length: 10 }); const basePath = '/search/places/v1/postcode'; diff --git a/src/modules/geospatial/dto/get-addresses-by-postcode-query.dto.ts b/src/modules/geospatial/dto/get-addresses-by-postcode-query.dto.ts index 26f62be0..55c06260 100644 --- a/src/modules/geospatial/dto/get-addresses-by-postcode-query.dto.ts +++ b/src/modules/geospatial/dto/get-addresses-by-postcode-query.dto.ts @@ -1,15 +1,16 @@ import { ApiProperty } from '@nestjs/swagger'; import { GEOSPATIAL } from '@ukef/constants'; -import { Matches, MaxLength, MinLength } from 'class-validator'; +import { IsString, Matches, MaxLength, MinLength } from 'class-validator'; export class GetAddressesByPostcodeQueryDto { @ApiProperty({ - example: GEOSPATIAL.EXAMPLES.POSTCODE, + example: GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE, description: 'Postcode to search for', minLength: 5, maxLength: 8, pattern: GEOSPATIAL.REGEX.UK_POSTCODE.source, }) + @IsString() @MinLength(5) @MaxLength(8) @Matches(GEOSPATIAL.REGEX.UK_POSTCODE) diff --git a/src/modules/geospatial/dto/get-addresses-response.dto.ts b/src/modules/geospatial/dto/get-addresses-response.dto.ts index 87e997a1..0947e056 100644 --- a/src/modules/geospatial/dto/get-addresses-response.dto.ts +++ b/src/modules/geospatial/dto/get-addresses-response.dto.ts @@ -40,7 +40,7 @@ export class GetAddressesResponseItem { @ApiProperty({ description: 'Postcode', - example: GEOSPATIAL.EXAMPLES.POSTCODE, + example: GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE, nullable: true, }) readonly postalCode: string | null; diff --git a/src/modules/geospatial/geospatial.controller.test.ts b/src/modules/geospatial/geospatial.controller.test.ts index d125cf04..0d951cbf 100644 --- a/src/modules/geospatial/geospatial.controller.test.ts +++ b/src/modules/geospatial/geospatial.controller.test.ts @@ -1,3 +1,4 @@ +import { NotFoundException } from '@nestjs/common'; import { GEOSPATIAL } from '@ukef/constants'; import { GetGeospatialAddressesGenerator } from '@ukef-test/support/generator/get-geospatial-addresses-generator'; import { RandomValueGenerator } from '@ukef-test/support/generator/random-value-generator'; @@ -30,7 +31,7 @@ describe('GeospatialController', () => { }); describe('getAddressesByPostcode()', () => { - const postcode = GEOSPATIAL.EXAMPLES.POSTCODE; + const postcode = GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE; it('returns a single address for the postcode when the service returns a single address', async () => { when(geospatialServiceGetAddressesByPostcode).calledWith(postcode).mockResolvedValueOnce(getAddressesByPostcodeResponse[0]); @@ -50,13 +51,16 @@ describe('GeospatialController', () => { expect(response).toEqual(getAddressesByPostcodeMultipleResponse); }); - it('returns an empty response for the postcode when the service returns an empty response', async () => { - when(geospatialServiceGetAddressesByPostcode).calledWith(postcode).mockResolvedValueOnce([]); + it('passes NotFoundException when it is thrown by geospatialService', async () => { + const errorMessage = valueGenerator.sentence(); + when(geospatialServiceGetAddressesByPostcode).calledWith(postcode).mockRejectedValueOnce(new NotFoundException(errorMessage)); - const response = await controller.getAddressesByPostcode({ postcode }); + const responsePromise = controller.getAddressesByPostcode({ postcode }); expect(geospatialServiceGetAddressesByPostcode).toHaveBeenCalledTimes(1); - expect(response).toEqual([]); + + await expect(responsePromise).rejects.toBeInstanceOf(NotFoundException); + await expect(responsePromise).rejects.toThrow(errorMessage); }); }); }); diff --git a/src/modules/geospatial/geospatial.controller.ts b/src/modules/geospatial/geospatial.controller.ts index 7ded6c31..8fe5b2c0 100644 --- a/src/modules/geospatial/geospatial.controller.ts +++ b/src/modules/geospatial/geospatial.controller.ts @@ -21,7 +21,7 @@ export class GeospatialController { type: [GetAddressesResponseItem], }) @ApiNotFoundResponse({ - description: 'Postcode not found.', + description: 'No addresses found', }) getAddressesByPostcode(@Query() query: GetAddressesByPostcodeQueryDto): Promise { return this.geospatialService.getAddressesByPostcode(query.postcode); diff --git a/src/modules/geospatial/geospatial.service.test.ts b/src/modules/geospatial/geospatial.service.test.ts index 1718118f..67c34630 100644 --- a/src/modules/geospatial/geospatial.service.test.ts +++ b/src/modules/geospatial/geospatial.service.test.ts @@ -1,3 +1,4 @@ +import { NotFoundException } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { OrdnanceSurveyService } from '@ukef/helper-modules/ordnance-survey/ordnance-survey.service'; import { GetGeospatialAddressesGenerator } from '@ukef-test/support/generator/get-geospatial-addresses-generator'; @@ -56,12 +57,13 @@ describe('GeospatialService', () => { expect(response).toEqual(getAddressesByPostcodeMultipleResponse); }); - it('can handle empty backend response', async () => { + it('throws NotFoundException in case of empty backend response', async () => { when(ordnanceSurveyServiceGetAddressesByPostcode).calledWith(postcode).mockResolvedValueOnce(getAddressesOrdnanceSurveyEmptyResponse[0]); - const response = await service.getAddressesByPostcode(postcode); + const responsePromise = service.getAddressesByPostcode(postcode); - expect(response).toEqual([]); + await expect(responsePromise).rejects.toBeInstanceOf(NotFoundException); + await expect(responsePromise).rejects.toThrow('No addresses found'); }); it('returns addressLine1 formatted correctly even if middle value is missing', async () => { diff --git a/src/modules/geospatial/geospatial.service.ts b/src/modules/geospatial/geospatial.service.ts index cf2199a8..ff4cfc9e 100644 --- a/src/modules/geospatial/geospatial.service.ts +++ b/src/modules/geospatial/geospatial.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, NotFoundException } from '@nestjs/common'; import { ENUMS } from '@ukef/constants'; import { GetAddressesOrdnanceSurveyResponse } from '@ukef/helper-modules/ordnance-survey/dto/get-addresses-ordnance-survey-response.dto'; import { OrdnanceSurveyService } from '@ukef/helper-modules/ordnance-survey/ordnance-survey.service'; @@ -13,8 +13,8 @@ export class GeospatialService { const addresses = []; const response: GetAddressesOrdnanceSurveyResponse = await this.ordnanceSurveyService.getAddressesByPostcode(postcode); - if (!response?.results) { - return []; + if (!response?.results?.length) { + throw new NotFoundException('No addresses found'); } response.results.forEach((item) => { diff --git a/test/docs/__snapshots__/get-docs-yaml.api-test.ts.snap b/test/docs/__snapshots__/get-docs-yaml.api-test.ts.snap index 3bcf1ea2..73ac0a10 100644 --- a/test/docs/__snapshots__/get-docs-yaml.api-test.ts.snap +++ b/test/docs/__snapshots__/get-docs-yaml.api-test.ts.snap @@ -515,7 +515,7 @@ paths: items: $ref: '#/components/schemas/GetAddressesResponseItem' '404': - description: Postcode not found. + description: No addresses found tags: - geospatial info: @@ -1174,9 +1174,9 @@ components: example: England enum: - England + - Northern Ireland - Scotland - Wales - - Northern Ireland nullable: true required: - organisationName diff --git a/test/exposure-period/exposure-period.api-test.ts b/test/exposure-period/exposure-period.api-test.ts index 0781ea76..74ec9e40 100644 --- a/test/exposure-period/exposure-period.api-test.ts +++ b/test/exposure-period/exposure-period.api-test.ts @@ -137,4 +137,11 @@ describe('Exposure period', () => { expect(status).toBe(400); expect(body.message).toContain('productgroup must be one of the following values: EW, BS'); }); + + it('returns a 404 response if URL is missing /api/v1 at the beginning', async () => { + const url = '/exposure-period?startdate=2017-03-05&enddate=2017-04-05&productgroup=new'; + const { status, body } = await api.get(url); + expect(status).toBe(404); + expect(body.message).toContain(`Cannot GET ${url}`); + }); }); diff --git a/test/geospatial/get-address-by-postcode.api-test.ts b/test/geospatial/get-address-by-postcode.api-test.ts index 37002809..733054bc 100644 --- a/test/geospatial/get-address-by-postcode.api-test.ts +++ b/test/geospatial/get-address-by-postcode.api-test.ts @@ -13,6 +13,7 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => { const { ordnanceSurveyPaths, + ordnanceSurveyKey, mdmPaths, getAddressesByPostcodeResponse, getAddressesByPostcodeMultipleResponse, @@ -21,12 +22,14 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => { getAddressesOrdnanceSurveyMultipleMatchingAddressesResponse, ordnanceSurveyAuthErrorResponse, } = new GetGeospatialAddressesGenerator(valueGenerator).generate({ - postcode: GEOSPATIAL.EXAMPLES.POSTCODE, + postcode: GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE, key: ENVIRONMENT_VARIABLES.ORDNANCE_SURVEY_KEY, numberToGenerate: 2, }); - const getMdmUrl = (postcode: string) => `/api/v1/geospatial/addresses/postcode?postcode=${encodeURIComponent(postcode)}`; + const getMdmUrl = (postcode: any) => `/api/v1/geospatial/addresses/postcode?postcode=${encodeURIComponent(postcode)}`; + const getOsUrl = (postcode: any) => + `/search/places/v1/postcode?postcode=${encodeURIComponent(postcode)}&lr=${GEOSPATIAL.DEFAULT.RESULT_LANGUAGE}&key=${encodeURIComponent(ordnanceSurveyKey)}`; beforeAll(async () => { api = await Api.create(); @@ -49,10 +52,27 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => { makeRequestWithoutAuth: (incorrectAuth?: IncorrectAuthArg) => api.getWithoutAuth(mdmPaths[0], incorrectAuth?.headerName, incorrectAuth?.headerValue), }); - it('returns a 200 response with the address if it is returned by Ordnance Survey API', async () => { - requestToGetAddressesByPostcode(ordnanceSurveyPaths[0]).reply(200, getAddressesOrdnanceSurveyResponse[0]); + it.each([ + { + postcode: GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE, + description: 'for English postcode', + }, + { + postcode: GEOSPATIAL.EXAMPLES.NORTHERN_IRELAND_POSTCODE, + description: 'for Northern Ireland postcode', + }, + { + postcode: GEOSPATIAL.EXAMPLES.SCOTLAND_POSTCODE, + description: 'for Scotish postcode', + }, + { + postcode: GEOSPATIAL.EXAMPLES.WALES_POSTCODE, + description: 'for Wales postcode', + }, + ])('returns a 200 response $description', async ({ postcode }) => { + requestToGetAddressesByPostcode(getOsUrl(postcode)).reply(200, getAddressesOrdnanceSurveyResponse[0]); - const { status, body } = await api.get(mdmPaths[0]); + const { status, body } = await api.get(getMdmUrl(postcode)); expect(status).toBe(200); expect(body).toStrictEqual(getAddressesByPostcodeResponse[0]); @@ -67,13 +87,17 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => { expect(body).toStrictEqual(getAddressesByPostcodeMultipleResponse); }); - it('returns an empty 200 response if Ordnance Survey API returns a 200 without results', async () => { + it('returns a 404 response if Ordnance Survey API returns a 200 without results', async () => { requestToGetAddressesByPostcode(ordnanceSurveyPaths[0]).reply(200, getAddressesOrdnanceSurveyEmptyResponse[0]); const { status, body } = await api.get(mdmPaths[0]); - expect(status).toBe(200); - expect(body).toStrictEqual([]); + expect(status).toBe(404); + expect(body).toEqual({ + statusCode: 404, + message: 'No addresses found', + error: 'Not Found', + }); }); it('returns a 500 response if Ordnance Survey API returns a status code 401', async () => { @@ -125,6 +149,26 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => { }); it.each([ + { + postcode: false, + expectedError: 'postcode must match /^[A-Za-z]{1,2}[\\dRr][\\dA-Za-z]?\\s?\\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/ regular expression', + }, + { + postcode: 1234567, + expectedError: 'postcode must match /^[A-Za-z]{1,2}[\\dRr][\\dA-Za-z]?\\s?\\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/ regular expression', + }, + { + postcode: null, + expectedError: 'postcode must match /^[A-Za-z]{1,2}[\\dRr][\\dA-Za-z]?\\s?\\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/ regular expression', + }, + { + postcode: {}, + expectedError: 'postcode must match /^[A-Za-z]{1,2}[\\dRr][\\dA-Za-z]?\\s?\\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/ regular expression', + }, + { + postcode: '', + expectedError: 'postcode must be longer than or equal to 5 characters', + }, { postcode: valueGenerator.string({ length: 4 }), expectedError: 'postcode must be longer than or equal to 5 characters', diff --git a/test/support/generator/get-geospatial-addresses-generator.ts b/test/support/generator/get-geospatial-addresses-generator.ts index 347cb262..45bc5e73 100644 --- a/test/support/generator/get-geospatial-addresses-generator.ts +++ b/test/support/generator/get-geospatial-addresses-generator.ts @@ -26,7 +26,7 @@ export class GetGeospatialAddressesGenerator extends AbstractGenerator ({ postcode: postcode || v.POSTCODE }) as GetAddressesByPostcodeQueryDto); @@ -191,6 +191,7 @@ export class GetGeospatialAddressesGenerator extends AbstractGenerator