diff --git a/.github/workflows/run_regression_tests.yml b/.github/workflows/run_regression_tests.yml index f9d3f8ac4..c92462e0f 100644 --- a/.github/workflows/run_regression_tests.yml +++ b/.github/workflows/run_regression_tests.yml @@ -79,8 +79,8 @@ jobs: GITHUB-TOKEN: ${{ steps.generate-token.outputs.token }} run: | if [[ "$TARGET_ENVIRONMENT" != "prod" && "$TARGET_ENVIRONMENT" != "ref" ]]; then - REGRESSION_TEST_REPO_TAG="v3.8.18" # This is the tag or branch of the regression test code to run, usually a version tag like v3.1.0 or a branch name - REGRESSION_TEST_WORKFLOW_TAG="v3.8.18" # This is the tag of the github workflow to run, usually the same as REGRESSION_TEST_REPO_TAG + REGRESSION_TEST_REPO_TAG="v3.8.19" # This is the tag or branch of the regression test code to run, usually a version tag like v3.1.0 or a branch name + REGRESSION_TEST_WORKFLOW_TAG="v3.8.19" # This is the tag of the github workflow to run, usually the same as REGRESSION_TEST_REPO_TAG if [[ -z "$REGRESSION_TEST_REPO_TAG" || -z "$REGRESSION_TEST_WORKFLOW_TAG" ]]; then echo "Error: One or both tag variables are not set" >&2 @@ -121,8 +121,8 @@ jobs: # GITHUB-TOKEN: ${{ steps.generate-token.outputs.token }} # run: | # if [[ "$TARGET_ENVIRONMENT" != "prod" && "$TARGET_ENVIRONMENT" != "ref" ]]; then - # REGRESSION_TEST_REPO_TAG="v3.8.18" # This is the tag or branch of the regression test code to run, usually a version tag like v3.1.0 or a branch name - # REGRESSION_TEST_WORKFLOW_TAG="v3.8.18" # This is the tag of the github workflow to run, usually the same as REGRESSION_TEST_REPO_TAG + # REGRESSION_TEST_REPO_TAG="v3.8.19" # This is the tag or branch of the regression test code to run, usually a version tag like v3.1.0 or a branch name + # REGRESSION_TEST_WORKFLOW_TAG="v3.8.19" # This is the tag of the github workflow to run, usually the same as REGRESSION_TEST_REPO_TAG # if [[ -z "$REGRESSION_TEST_REPO_TAG" || -z "$REGRESSION_TEST_WORKFLOW_TAG" ]]; then # echo "Error: One or both tag variables are not set" >&2 diff --git a/Makefile b/Makefile index 257728965..83e32add7 100644 --- a/Makefile +++ b/Makefile @@ -47,6 +47,7 @@ sam-sync-sandbox: guard-stack_name compile download-get-secrets-layer sam-deploy: guard-AWS_DEFAULT_PROFILE guard-stack_name sam deploy \ + --template-file SAMtemplates/main_template.yaml \ --stack-name $$stack_name \ --parameter-overrides \ EnableSplunk=false \ diff --git a/README.md b/README.md index 032566dda..a73dd7449 100644 --- a/README.md +++ b/README.md @@ -201,7 +201,8 @@ Note - the command will keep running and should not be stopped. You can now call this api - note getMyPrescriptions requires an nhsd-nhslogin-user header ```bash -curl --header "nhsd-nhslogin-user: P9:9446041481" https://${stack_name}.dev.prescriptionsforpatients.national.nhs.uk/Bundle +curl --header "nhsd-nhslogin-user: P9:9446041481" --header "x-request-id: $(uuid)" \ + https://${stack_name}.dev.eps.national.nhs.uk/Bundle ``` You can also use the AWS vscode extension to invoke the API or lambda directly diff --git a/packages/serviceSearchClient/src/live-serviceSearch-client.ts b/packages/serviceSearchClient/src/live-serviceSearch-client.ts index 7ab8cf826..439403548 100644 --- a/packages/serviceSearchClient/src/live-serviceSearch-client.ts +++ b/packages/serviceSearchClient/src/live-serviceSearch-client.ts @@ -1,4 +1,5 @@ import {Logger} from "@aws-lambda-powertools/logger" +import {getSecret} from "@aws-lambda-powertools/parameters/secrets" import axios, {AxiosError, AxiosInstance} from "axios" import axiosRetry from "axios-retry" import {handleUrl} from "./handleUrl" @@ -14,10 +15,24 @@ type Service = { "OrganisationSubType": string } +type Contact = { + "ContactMethodType": string + "ContactValue": string +} + export type ServiceSearchData = { "value": Array } +export type ServiceSearch3Data = { + "@odata.context": string + "value": Array<{ + "@search.score": number + "OrganisationSubType": string + "Contacts": Array + }> +} + export const SERVICE_SEARCH_BASE_QUERY_PARAMS = { "api-version": 2, "searchFields": "ODSCode", @@ -26,26 +41,40 @@ export const SERVICE_SEARCH_BASE_QUERY_PARAMS = { "$top": 1 } -export function getServiceSearchEndpoint(): string { +export function getServiceSearchVersion(logger: Logger | null = null): number { const endpoint = process.env.TargetServiceSearchServer || "service-search" - const baseUrl = `https://${endpoint}` if (endpoint.toLowerCase().includes("api.service.nhs.uk")) { - // service search v3 + logger?.info("Using service search v3 endpoint") SERVICE_SEARCH_BASE_QUERY_PARAMS["api-version"] = 3 - return `${baseUrl}/service-search-api/` + SERVICE_SEARCH_BASE_QUERY_PARAMS["$select"] = "Contacts,OrganisationSubType" + return 3 + } + logger?.warn("Using service search v2 endpoint") + return 2 +} + +export function getServiceSearchEndpoint(logger: Logger | null = null): string { + switch (getServiceSearchVersion(logger)) { + case 3: + return `https://${process.env.TargetServiceSearchServer}/service-search-api/` + case 2: + default: + return `https://${process.env.TargetServiceSearchServer}/service-search` } - // service search v2 - return `${baseUrl}/service-search` } export class LiveServiceSearchClient implements ServiceSearchClient { private readonly axiosInstance: AxiosInstance private readonly logger: Logger - private readonly outboundHeaders: {"apikey": string | undefined, "Subscription-Key": string | undefined} + private readonly outboundHeaders: {"apikey"?: string, "Subscription-Key"?: string} constructor(logger: Logger) { this.logger = logger - + this.logger.info("ServiceSearchClient configured", + { + v2: process.env.ServiceSearchApiKey !== undefined, + v3: process.env.ServiceSearch3ApiKey !== undefined + }) this.axiosInstance = axios.create() axiosRetry(this.axiosInstance, {retries: 3}) @@ -95,15 +124,49 @@ export class LiveServiceSearchClient implements ServiceSearchClient { return Promise.reject(err) }) - this.outboundHeaders = { - "Subscription-Key": process.env.ServiceSearchApiKey, - "apikey": process.env.ServiceSearch3ApiKey + const version = getServiceSearchVersion(this.logger) + if (version === 3) { + this.outboundHeaders = { + "apikey": process.env.ServiceSearch3ApiKey + } + } else { + this.outboundHeaders = { + "Subscription-Key": process.env.ServiceSearchApiKey + } + } + } + + private async loadApiKeyFromSecretsManager(): Promise { + try { + const secretArn = process.env.ServiceSearch3ApiKeyARN + if (!secretArn) { + this.logger.error("ServiceSearch3ApiKeyARN environment variable is not set") + return undefined + } + this.logger.info("Loading ServiceSearch API key from Secrets Manager", {secretArn}) + + const secret = await getSecret(secretArn, { + maxAge: 300 // Cache for 5 minutes + }) + + this.logger.info("Successfully loaded ServiceSearch API key from Secrets Manager") + return secret as string + } catch (error) { + this.logger.error("Failed to load ServiceSearch API key from Secrets Manager", {error}) + return undefined } } async searchService(odsCode: string): Promise { try { - const address = getServiceSearchEndpoint() + // Load API key if not set in environment (secrets layer is failing to load v3 key) + const apiVsn = getServiceSearchVersion(this.logger) + if (apiVsn === 3 && !this.outboundHeaders.apikey) { + this.logger.info("API key not in environment, attempting to load from Secrets Manager") + this.outboundHeaders.apikey = await this.loadApiKeyFromSecretsManager() + } + + const address = getServiceSearchEndpoint(this.logger) const queryParams = {...SERVICE_SEARCH_BASE_QUERY_PARAMS, search: odsCode} this.logger.info(`making request to ${address} with ods code ${odsCode}`, {odsCode: odsCode}) @@ -113,22 +176,14 @@ export class LiveServiceSearchClient implements ServiceSearchClient { timeout: SERVICE_SEARCH_TIMEOUT }) - const serviceSearchResponse: ServiceSearchData = response.data - const services = serviceSearchResponse.value - if (services.length === 0) { - return undefined + this.logger.info(`received response from serviceSearch for ods code ${odsCode}`, + {odsCode: odsCode, status: response.status, data: response.data}) + if (apiVsn === 2) { + return this.handleV2Response(odsCode, response.data) + } else { + return this.handleV3Response(odsCode, response.data) } - this.logger.info(`pharmacy with ods code ${odsCode} is of type ${DISTANCE_SELLING}`, {odsCode: odsCode}) - const service = services[0] - const urlString = service["URL"] - - if (urlString === null) { - this.logger.warn(`ods code ${odsCode} has no URL but is of type ${DISTANCE_SELLING}`, {odsCode: odsCode}) - return undefined - } - const serviceUrl = handleUrl(urlString, odsCode, this.logger) - return serviceUrl } catch (error) { if (axios.isAxiosError(error)) { this.stripApiKeyFromHeaders(error) @@ -157,14 +212,43 @@ export class LiveServiceSearchClient implements ServiceSearchClient { throw error } } + handleV3Response(odsCode: string, data: ServiceSearch3Data): URL | undefined { + const contacts = data.value[0]?.Contacts + const websiteContact = contacts?.find((contact: Contact) => contact.ContactMethodType === "Website") + const websiteUrl = websiteContact?.ContactValue + if (!websiteUrl) { + this.logger.warn(`pharmacy with ods code ${odsCode} has no website`, {odsCode: odsCode}) + return undefined + } + const serviceUrl = handleUrl(websiteUrl, odsCode, this.logger) + return serviceUrl + } + + handleV2Response(odsCode: string, data: ServiceSearchData): URL | undefined { + const services = data.value + if (services.length === 0) { + return undefined + } + + this.logger.info(`pharmacy with ods code ${odsCode} is of type ${DISTANCE_SELLING}`, {odsCode: odsCode}) + const service = services[0] + const urlString = service["URL"] + + if (urlString === null) { + this.logger.warn(`ods code ${odsCode} has no URL but is of type ${DISTANCE_SELLING}`, {odsCode: odsCode}) + return undefined + } + const serviceUrl = handleUrl(urlString, odsCode, this.logger) + return serviceUrl + } stripApiKeyFromHeaders(error: AxiosError) { const headerKeys = ["subscription-key", "apikey"] headerKeys.forEach((key) => { - if (error.response?.headers) { + if (error.response?.headers?.[key]) { delete error.response.headers[key] } - if (error.request?.headers) { + if (error.request?.headers?.[key]) { delete error.request.headers[key] } }) diff --git a/packages/serviceSearchClient/tests/live-serviceSearch-client.test.ts b/packages/serviceSearchClient/tests/live-serviceSearch-client.test.ts index 38f08b620..cc17efdc2 100644 --- a/packages/serviceSearchClient/tests/live-serviceSearch-client.test.ts +++ b/packages/serviceSearchClient/tests/live-serviceSearch-client.test.ts @@ -1,4 +1,4 @@ -import {LiveServiceSearchClient, ServiceSearchData} from "../src/live-serviceSearch-client" +import {LiveServiceSearchClient, ServiceSearchData, ServiceSearch3Data} from "../src/live-serviceSearch-client" import {jest} from "@jest/globals" import MockAdapter from "axios-mock-adapter" import axios, {AxiosError, AxiosRequestConfig, AxiosResponse} from "axios" @@ -29,13 +29,39 @@ describe("live serviceSearch client", () => { }) // Helper function tests - test("getServiceSearchEndpoint returns correct URL", async () => { + test("getServiceSearchEndpoint returns correct URL for v2", async () => { const {getServiceSearchEndpoint} = await import("../src/live-serviceSearch-client.js") const endpoint = getServiceSearchEndpoint() expect(endpoint).toBe(serviceSearchUrl) }) - test("stripApiKeyFromHeaders removes only subscription-key header", () => { + test("getServiceSearchEndpoint returns correct URL for v3", async () => { + process.env.TargetServiceSearchServer = "api.service.nhs.uk" + const {getServiceSearchEndpoint} = await import("../src/live-serviceSearch-client.js") + const endpoint = getServiceSearchEndpoint(logger) + expect(endpoint).toBe("https://api.service.nhs.uk/service-search-api/") + process.env.TargetServiceSearchServer = "live" + }) + + test("getServiceSearchVersion returns 3 and logs info for v3 endpoint", async () => { + process.env.TargetServiceSearchServer = "api.service.nhs.uk" + const infoSpy = jest.spyOn(Logger.prototype, "info") + const {getServiceSearchVersion} = await import("../src/live-serviceSearch-client.js") + const version = getServiceSearchVersion(logger) + expect(version).toBe(3) + expect(infoSpy).toHaveBeenCalledWith("Using service search v3 endpoint") + process.env.TargetServiceSearchServer = "live" + }) + + test("getServiceSearchVersion returns 2 and logs warn for v2 endpoint", async () => { + const warnSpy = jest.spyOn(Logger.prototype, "warn") + const {getServiceSearchVersion} = await import("../src/live-serviceSearch-client.js") + const version = getServiceSearchVersion(logger) + expect(version).toBe(2) + expect(warnSpy).toHaveBeenCalledWith("Using service search v2 endpoint") + }) + + test("stripKeyFromHeaders removes only subscription-key header", () => { const axiosErr: AxiosError = { isAxiosError: true, config: { @@ -68,6 +94,45 @@ describe("live serviceSearch client", () => { expect(axiosErr.response!.headers).toHaveProperty("foo", "bar") }) + test("stripApiKeyFromHeaders removes only apikey header", () => { + const axiosErr: AxiosError = { + isAxiosError: true, + config: { + headers: new axios.AxiosHeaders({"apikey": "secret", keep: "yes"}) + } satisfies AxiosRequestConfig, + response: { + headers: {"apikey": "secret", foo: "bar"}, + data: null, + status: 200, + statusText: "", + config: {headers: new axios.AxiosHeaders()} satisfies AxiosRequestConfig, + request: {} + } satisfies AxiosResponse, + request: { + headers: {"apikey": "secret", keep: "yes"} + }, + toJSON: function (): object { + throw new Error("Function not implemented.") + }, + name: "", + message: "" + } + + expect(axiosErr.config!.headers).toHaveProperty("apikey") + expect(axiosErr.request!.headers).toHaveProperty("apikey") + expect(axiosErr.response!.headers).toHaveProperty("apikey") + + client.stripApiKeyFromHeaders(axiosErr) + + // The config doesn't get touched by the stripping function + expect(axiosErr.config!.headers).toHaveProperty("apikey") + expect(axiosErr.config!.headers).toHaveProperty("keep", "yes") + expect(axiosErr.request!.headers).not.toHaveProperty("apikey") + expect(axiosErr.request!.headers).toHaveProperty("keep", "yes") + expect(axiosErr.response!.headers).not.toHaveProperty("apikey") + expect(axiosErr.response!.headers).toHaveProperty("foo", "bar") + }) + // Test non-Axios exception path test("searchService logs and rethrows non-Axios error", async () => { jest.spyOn(client["axiosInstance"], "get").mockImplementation(() => { @@ -130,6 +195,25 @@ describe("live serviceSearch client", () => { ) }) + // Test AxiosError without response or request (general axios error) + test("searchService logs general axios error when no response or request", async () => { + const axiosErr = { + isAxiosError: true, + message: "generalfail", + config: {headers: {}}, + request: undefined, + response: undefined + } as unknown as AxiosError + + jest.spyOn(client["axiosInstance"], "get").mockRejectedValue(axiosErr) + const errSpy = jest.spyOn(Logger.prototype, "error") + + await expect(client.searchService("y")).rejects.toBe(axiosErr) + expect(errSpy).toHaveBeenCalledWith( + "general error calling serviceSearch", {error: axiosErr} + ) + }) + describe("integration scenarios", () => { const validUrlData: ServiceSearchData = { value: [ @@ -256,4 +340,119 @@ describe("live serviceSearch client", () => { }) }) + + describe("handleV3Response", () => { + test("returns URL from Website contact", () => { + const data: ServiceSearch3Data = { + "@odata.context": "https://api.service.nhs.uk/service-search-api/$metadata#Services", + value: [ + { + "@search.score": 1.0, + OrganisationSubType: "DistanceSelling", + Contacts: [ + {ContactMethodType: "Website", ContactValue: "https://example.com"} + ] + } + ] + } + + const result = client.handleV3Response("TEST123", data) + expect(result).toEqual(new URL("https://example.com")) + }) + + test("returns undefined when response has no Contacts", () => { + const data: ServiceSearch3Data = { + "@odata.context": "https://api.service.nhs.uk/service-search-api/$metadata#Services", + value: [ + { + "@search.score": 1.0, + OrganisationSubType: "DistanceSelling", + Contacts: [] + } + ] + } + + const warnSpy = jest.spyOn(Logger.prototype, "warn") + const result = client.handleV3Response("TEST123", data) + + expect(warnSpy).toHaveBeenCalledWith( + "pharmacy with ods code TEST123 has no website", + {odsCode: "TEST123"} + ) + expect(result).toBeUndefined() + }) + + test("returns undefined when response has no Website contact", () => { + const data: ServiceSearch3Data = { + "@odata.context": "https://api.service.nhs.uk/service-search-api/$metadata#Services", + value: [ + { + "@search.score": 1.0, + OrganisationSubType: "DistanceSelling", + Contacts: [ + {ContactMethodType: "Phone", ContactValue: "01234567890"}, + {ContactMethodType: "Email", ContactValue: "test@example.com"} + ] + } + ] + } + + const warnSpy = jest.spyOn(Logger.prototype, "warn") + const result = client.handleV3Response("TEST123", data) + + expect(warnSpy).toHaveBeenCalledWith( + "pharmacy with ods code TEST123 has no website", + {odsCode: "TEST123"} + ) + expect(result).toBeUndefined() + }) + + test("handles URL without protocol", () => { + const data: ServiceSearch3Data = { + "@odata.context": "https://api.service.nhs.uk/service-search-api/$metadata#Services", + value: [ + { + "@search.score": 1.0, + OrganisationSubType: "DistanceSelling", + Contacts: [ + {ContactMethodType: "Website", ContactValue: "example.com"} + ] + } + ] + } + + const result = client.handleV3Response("TEST123", data) + expect(result).toEqual(new URL("https://example.com")) + }) + + test("returns undefined when value array is empty", () => { + const data: ServiceSearch3Data = { + "@odata.context": "https://api.service.nhs.uk/service-search-api/$metadata#Services", + value: [] + } + + const result = client.handleV3Response("TEST123", data) + expect(result).toBeUndefined() + }) + + test("finds Website contact among multiple contacts", () => { + const data: ServiceSearch3Data = { + "@odata.context": "https://api.service.nhs.uk/service-search-api/$metadata#Services", + value: [ + { + "@search.score": 1.0, + OrganisationSubType: "DistanceSelling", + Contacts: [ + {ContactMethodType: "Phone", ContactValue: "01234567890"}, + {ContactMethodType: "Website", ContactValue: "https://pharmacy.example.com"}, + {ContactMethodType: "Email", ContactValue: "test@example.com"} + ] + } + ] + } + + const result = client.handleV3Response("TEST123", data) + expect(result).toEqual(new URL("https://pharmacy.example.com")) + }) + }) })