diff --git a/src/helper-modules/ordnance-survey/dto/get-addresses-ordnance-survey-response.dto.ts b/src/helper-modules/ordnance-survey/dto/get-addresses-ordnance-survey-response.dto.ts index f1c5b1f5..e500d8d9 100644 --- a/src/helper-modules/ordnance-survey/dto/get-addresses-ordnance-survey-response.dto.ts +++ b/src/helper-modules/ordnance-survey/dto/get-addresses-ordnance-survey-response.dto.ts @@ -1,4 +1,4 @@ -export type GetAddressOrdnanceSurveyResponse = { +export type GetAddressesOrdnanceSurveyResponse = { header: { uri: string; query: string; @@ -12,14 +12,14 @@ export type GetAddressOrdnanceSurveyResponse = { lastupdate: string; output_srs: string; }; - results?: GetAddressOrdnanceSurveyResponseItem[]; + results?: GetAddressesOrdnanceSurveyResponseItem[]; }; -interface GetAddressOrdnanceSurveyResponseItem { - DPA: GetAddressOrdnanceSurveyResponseAddress; +interface GetAddressesOrdnanceSurveyResponseItem { + DPA: GetAddressesOrdnanceSurveyResponseAddress; } -interface GetAddressOrdnanceSurveyResponseAddress { +interface GetAddressesOrdnanceSurveyResponseAddress { UPRN: string; UDPRN: string; ADDRESS: string; diff --git a/src/helper-modules/ordnance-survey/known-errors.ts b/src/helper-modules/ordnance-survey/known-errors.ts deleted file mode 100644 index 48c9d029..00000000 --- a/src/helper-modules/ordnance-survey/known-errors.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { NotFoundException } from '@nestjs/common'; -import { AxiosError } from 'axios'; - -export type KnownErrors = KnownError[]; - -type KnownError = { caseInsensitiveSubstringToFind: string; throwError: (error: AxiosError) => never }; - -export const getAddressNotFoundKnownOrdnanceSurveyError = (): KnownError => ({ - caseInsensitiveSubstringToFind: 'Address not found', - throwError: (error) => { - throw new NotFoundException('Address not found.', error); - }, -}); 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 e3cfceca..611cc5b1 100644 --- a/src/helper-modules/ordnance-survey/ordnance-survey.service.test.ts +++ b/src/helper-modules/ordnance-survey/ordnance-survey.service.test.ts @@ -4,8 +4,8 @@ import { RandomValueGenerator } from '@ukef-test/support/generator/random-value- import { AxiosError } from 'axios'; import { when } from 'jest-when'; import { of, throwError } from 'rxjs'; -import expectedResponse = require('./examples/example-response-for-search-places-v1-postcode.json'); -import noResultsResponse = require('./examples/example-response-for-search-places-v1-postcode-no-results.json'); +import expectedResponseData = require('./examples/example-response-for-search-places-v1-postcode.json'); +import noResultsResponseData = require('./examples/example-response-for-search-places-v1-postcode-no-results.json'); import { GEOSPATIAL } from '@ukef/constants'; @@ -23,6 +23,14 @@ describe('OrdnanceSurveyService', () => { const testKey = valueGenerator.string({ length: 10 }); const basePath = '/search/places/v1/postcode'; + const expectedResponse = of({ + data: expectedResponseData, + status: 200, + statusText: 'OK', + config: undefined, + headers: undefined, + }); + beforeEach(() => { const httpService = new HttpService(); const configService = new ConfigService(); @@ -40,26 +48,11 @@ describe('OrdnanceSurveyService', () => { const expectedHttpServiceGetArgs: [string, object] = [expectedPath, { headers: { 'Content-Type': 'application/json' } }]; - it('sends a GET to the Ordnance Survey API /search endpoint with the specified request', async () => { - when(httpServiceGet) - .calledWith(...expectedHttpServiceGetArgs) - .mockReturnValueOnce( - of({ - data: expectedResponse, - status: 200, - statusText: 'OK', - config: undefined, - headers: undefined, - }), - ); - - await service.getAddressesByPostcode(testPostcode); - - expect(httpServiceGet).toHaveBeenCalledTimes(1); - expect(httpServiceGet).toHaveBeenCalledWith(...expectedHttpServiceGetArgs); - }); - - it.each([ + describe.each([ + { + postcode: testPostcode, + expectedUrlQueryPart: `?postcode=${encodeURIComponent(testPostcode)}`, + }, { postcode: 'W1A 1AA', expectedUrlQueryPart: `?postcode=W1A%201AA`, @@ -68,34 +61,40 @@ describe('OrdnanceSurveyService', () => { postcode: 'W1A1AA', expectedUrlQueryPart: '?postcode=W1A1AA', }, - ])('call Ordnance Survey API with correct and safe query parameters "$expectedUrlQueryPart"', async ({ postcode, expectedUrlQueryPart }) => { - const expectedPath = `${basePath}${expectedUrlQueryPart}&lr=EN&key=${encodeURIComponent(testKey)}`; - const expectedHttpServiceGetArgs: [string, object] = [expectedPath, { headers: { 'Content-Type': 'application/json' } }]; + ])('test postcode $postcode', ({ postcode, expectedUrlQueryPart }) => { + it('call Ordnance Survey with the correct arguments', async () => { + const expectedPath = `${basePath}${expectedUrlQueryPart}&lr=EN&key=${encodeURIComponent(testKey)}`; + const expectedHttpServiceGetArgs: [string, object] = [expectedPath, { headers: { 'Content-Type': 'application/json' } }]; - when(httpServiceGet) - .calledWith(...expectedHttpServiceGetArgs) - .mockReturnValueOnce( - of({ - data: expectedResponse, - status: 200, - statusText: 'OK', - config: undefined, - headers: undefined, - }), - ); + when(httpServiceGet) + .calledWith(...expectedHttpServiceGetArgs) + .mockReturnValueOnce(expectedResponse); + await service.getAddressesByPostcode(postcode); - await service.getAddressesByPostcode(postcode); + expect(httpServiceGet).toHaveBeenCalledTimes(1); + expect(httpServiceGet).toHaveBeenCalledWith(...expectedHttpServiceGetArgs); + }); - expect(httpServiceGet).toHaveBeenCalledTimes(1); - expect(httpServiceGet).toHaveBeenCalledWith(...expectedHttpServiceGetArgs); + it('call Ordnance Survey returns expectedResponse', async () => { + const expectedPath = `${basePath}${expectedUrlQueryPart}&lr=EN&key=${encodeURIComponent(testKey)}`; + const expectedHttpServiceGetArgs: [string, object] = [expectedPath, { headers: { 'Content-Type': 'application/json' } }]; + + when(httpServiceGet) + .calledWith(...expectedHttpServiceGetArgs) + .mockReturnValueOnce(expectedResponse); + + const response = await service.getAddressesByPostcode(postcode); + + expect(response).toBe(expectedResponseData); + }); }); - it('no results - returns 200 without results', async () => { + it('returns a 200 response without results when Ordnance Survey returns no results', async () => { when(httpServiceGet) .calledWith(...expectedHttpServiceGetArgs) .mockReturnValueOnce( of({ - data: noResultsResponse, + data: noResultsResponseData, status: 200, statusText: 'OK', config: undefined, @@ -107,7 +106,7 @@ describe('OrdnanceSurveyService', () => { expect(httpServiceGet).toHaveBeenCalledTimes(1); expect(httpServiceGet).toHaveBeenCalledWith(...expectedHttpServiceGetArgs); - expect(results).toBe(noResultsResponse); + expect(results).toBe(noResultsResponseData); }); it('throws an OrdnanceSurveyException if the request to Ordnance Survey fails', async () => { diff --git a/src/helper-modules/ordnance-survey/ordnance-survey.service.ts b/src/helper-modules/ordnance-survey/ordnance-survey.service.ts index a079baab..f8ad1a78 100644 --- a/src/helper-modules/ordnance-survey/ordnance-survey.service.ts +++ b/src/helper-modules/ordnance-survey/ordnance-survey.service.ts @@ -5,9 +5,8 @@ import { KEY as ORDNANCE_SURVEY_CONFIG_KEY, OrdnanceSurveyConfig } from '@ukef/c import { GEOSPATIAL } from '@ukef/constants'; import { HttpClient } from '@ukef/modules/http/http.client'; -import { GetAddressOrdnanceSurveyResponse } from './dto/get-addresses-ordnance-survey-response.dto'; -// import { getCustomersNotFoundKnownOrdnanceSurveyError } from './known-errors'; -import { createWrapOrdnanceSurveyHttpGetErrorCallback } from './wrap-ordnance-survey-http-error-callback'; +import { GetAddressesOrdnanceSurveyResponse } from './dto/get-addresses-ordnance-survey-response.dto'; +import { OrdnanceSurveyException } from './exception/ordnance-survey.exception'; @Injectable() export class OrdnanceSurveyService { @@ -20,16 +19,15 @@ export class OrdnanceSurveyService { this.key = key; } - async getAddressesByPostcode(postcode): Promise { + async getAddressesByPostcode(postcode: string): Promise { const path = `/search/places/v1/postcode?postcode=${encodeURIComponent(postcode)}&lr=${GEOSPATIAL.DEFAULT.RESULT_LANGUAGE}&key=${encodeURIComponent(this.key)}`; - const { data } = await this.httpClient.get({ + const { data } = await this.httpClient.get({ path, headers: { 'Content-Type': 'application/json' }, - onError: createWrapOrdnanceSurveyHttpGetErrorCallback({ - messageForUnknownError: `Failed to get response from Ordnance Survey API.`, - knownErrors: [], - // knownErrors: [getCustomersNotFoundKnownOrdnanceSurveyError()], // TODO: should we change 200 no results to 404? - }), + onError: (error: Error) => { + console.error('Http call error happened, error %o', error); + throw new OrdnanceSurveyException('Failed to get response from Ordnance Survey API.', error); + }, }); return data; } diff --git a/src/helper-modules/ordnance-survey/wrap-ordnance-survey-http-error-callback.ts b/src/helper-modules/ordnance-survey/wrap-ordnance-survey-http-error-callback.ts deleted file mode 100644 index cf1e4c1a..00000000 --- a/src/helper-modules/ordnance-survey/wrap-ordnance-survey-http-error-callback.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { AxiosError } from 'axios'; -import { ObservableInput, throwError } from 'rxjs'; - -import { OrdnanceSurveyException } from './exception/ordnance-survey.exception'; -import { KnownErrors } from './known-errors'; - -type AcbsHttpErrorCallback = (error: Error) => ObservableInput; - -export const createWrapOrdnanceSurveyHttpGetErrorCallback = - ({ messageForUnknownError, knownErrors }: { messageForUnknownError: string; knownErrors: KnownErrors }): AcbsHttpErrorCallback => - (error: Error) => { - let errorString; - if (error instanceof AxiosError && error.response) { - if (typeof error.response.data === 'object') { - errorString = JSON.stringify(error.response.data); - } - if (typeof error.response.data === 'string') { - errorString = error.response.data; - } - if (errorString) { - const errorStringInLowerCase = errorString.toLowerCase(); - - knownErrors.forEach(({ caseInsensitiveSubstringToFind, throwError }) => { - if (errorStringInLowerCase.includes(caseInsensitiveSubstringToFind.toLowerCase())) { - return throwError(error); - } - }); - } - } - - return throwError(() => new OrdnanceSurveyException(messageForUnknownError, error)); - }; diff --git a/src/modules/geospatial/dto/get-address-by-postcode-query.dto.ts b/src/modules/geospatial/dto/get-addresses-by-postcode-query.dto.ts similarity index 90% rename from src/modules/geospatial/dto/get-address-by-postcode-query.dto.ts rename to src/modules/geospatial/dto/get-addresses-by-postcode-query.dto.ts index 6202fdd1..b344a25b 100644 --- a/src/modules/geospatial/dto/get-address-by-postcode-query.dto.ts +++ b/src/modules/geospatial/dto/get-addresses-by-postcode-query.dto.ts @@ -4,7 +4,7 @@ import { Matches, MaxLength, MinLength } from 'class-validator'; const UK_POSTCODE = /^[A-Za-z]{1,2}[\dRr][\dA-Za-z]?\s?\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/; -export class GetAddressByPostcodeQueryDto { +export class GetAddressesByPostcodeQueryDto { @ApiProperty({ example: GEOSPATIAL.EXAMPLES.POSTCODE, description: 'Postcode to search for', diff --git a/src/modules/geospatial/geospatial.controller.ts b/src/modules/geospatial/geospatial.controller.ts index 5b7fe670..95813d93 100644 --- a/src/modules/geospatial/geospatial.controller.ts +++ b/src/modules/geospatial/geospatial.controller.ts @@ -1,7 +1,7 @@ import { Controller, Get, Query } from '@nestjs/common'; import { ApiNotFoundResponse, ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; -import { GetAddressByPostcodeQueryDto } from './dto/get-address-by-postcode-query.dto'; +import { GetAddressesByPostcodeQueryDto } from './dto/get-addresses-by-postcode-query.dto'; import { GetAddressesResponse, GetAddressesResponseItem } from './dto/get-addresses-response.dto'; import { GeospatialService } from './geospatial.service'; @@ -23,7 +23,7 @@ export class GeospatialController { @ApiNotFoundResponse({ description: 'Customer not found.', }) - getAddressesByPostcode(@Query() query: GetAddressByPostcodeQueryDto): Promise { + getAddressesByPostcode(@Query() query: GetAddressesByPostcodeQueryDto): Promise { return this.geospatialService.getAddressesByPostcode(query.postcode); } } diff --git a/src/modules/geospatial/geospatial.service.ts b/src/modules/geospatial/geospatial.service.ts index 2e07562b..b566e7f0 100644 --- a/src/modules/geospatial/geospatial.service.ts +++ b/src/modules/geospatial/geospatial.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; import { ENUMS } from '@ukef/constants'; -import { GetAddressOrdnanceSurveyResponse } from '@ukef/helper-modules/ordnance-survey/dto/get-addresses-ordnance-survey-response.dto'; +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'; import { GetAddressesResponse } from './dto/get-addresses-response.dto'; @@ -11,7 +11,7 @@ export class GeospatialService { async getAddressesByPostcode(postcode: string): Promise { const addresses = []; - const response: GetAddressOrdnanceSurveyResponse = await this.ordnanceSurveyService.getAddressesByPostcode(postcode); + const response: GetAddressesOrdnanceSurveyResponse = await this.ordnanceSurveyService.getAddressesByPostcode(postcode); if (!response?.results) { return []; diff --git a/test/support/generator/get-geospatial-addresses-generator.ts b/test/support/generator/get-geospatial-addresses-generator.ts index c0ed8415..6a8c8e22 100644 --- a/test/support/generator/get-geospatial-addresses-generator.ts +++ b/test/support/generator/get-geospatial-addresses-generator.ts @@ -1,7 +1,7 @@ import { ENUMS, GEOSPATIAL } from '@ukef/constants'; -import { GetAddressOrdnanceSurveyResponse } from '@ukef/helper-modules/ordnance-survey/dto/get-addresses-ordnance-survey-response.dto'; +import { GetAddressesOrdnanceSurveyResponse } from '@ukef/helper-modules/ordnance-survey/dto/get-addresses-ordnance-survey-response.dto'; import { OrdnanceSurveyAuthErrorResponse } from '@ukef/helper-modules/ordnance-survey/dto/ordnance-survey-auth-error-response.dto'; -import { GetAddressByPostcodeQueryDto } from '@ukef/modules/geospatial/dto/get-address-by-postcode-query.dto'; +import { GetAddressesByPostcodeQueryDto } from '@ukef/modules/geospatial/dto/get-addresses-by-postcode-query.dto'; import { GetAddressesResponse } from '@ukef/modules/geospatial/dto/get-addresses-response.dto'; import { AbstractGenerator } from './abstract-generator'; @@ -28,19 +28,19 @@ export class GetGeospatialAddressesGenerator extends AbstractGenerator ({ postcode: postcode || v.POSTCODE }) as GetAddressByPostcodeQueryDto); + const requests: GetAddressesByPostcodeQueryDto[] = values.map((v) => ({ postcode: postcode || v.POSTCODE }) as GetAddressesByPostcodeQueryDto); - const ordnanceSurveyPath: string[] = values.map((v) => { + const ordnanceSurveyPaths: string[] = values.map((v) => { const usePostcode = postcode || v.POSTCODE; return `/search/places/v1/postcode?postcode=${encodeURIComponent(usePostcode)}&lr=${GEOSPATIAL.DEFAULT.RESULT_LANGUAGE}&key=${encodeURIComponent(useKey)}`; }); - const mdmPath: string[] = values.map((v) => { + const mdmPaths: string[] = values.map((v) => { const usePostcode = postcode || v.POSTCODE; return `/api/v1/geospatial/addresses/postcode?postcode=${usePostcode}`; }); - const getAddressByPostcodeResponse: GetAddressesResponse[] = values.map((v) => [ + const getAddressesByPostcodeResponse: GetAddressesResponse[] = values.map((v) => [ { organisationName: v.ORGANISATION_NAME, addressLine1: `${v.BUILDING_NAME} ${v.BUILDING_NUMBER} ${v.THOROUGHFARE_NAME}`, @@ -52,9 +52,9 @@ export class GetGeospatialAddressesGenerator extends AbstractGenerator response[0]); + const getAddressesByPostcodeMultipleResponse = getAddressesByPostcodeResponse.map((response) => response[0]); - const getAddressOrdnanceSurveyResponse: GetAddressOrdnanceSurveyResponse[] = values.map((v) => ({ + const getAddressesOrdnanceSurveyResponse: GetAddressesOrdnanceSurveyResponse[] = values.map((v) => ({ header: { uri: 'test', query: 'test', @@ -109,7 +109,7 @@ export class GetGeospatialAddressesGenerator extends AbstractGenerator ({ + const getAddressesOrdnanceSurveyEmptyResponse: GetAddressesOrdnanceSurveyResponse = { header: { uri: 'test', query: 'test', @@ -176,9 +176,9 @@ export class GetGeospatialAddressesGenerator extends AbstractGenerator