diff --git a/.env.template b/.env.template index 9f135763f..6bc94ea30 100644 --- a/.env.template +++ b/.env.template @@ -1,15 +1,26 @@ -PR_NUMBER=prxx # remove if needs to run against main -GITHUB_TOKEN= # Your github Personal Access Token (PAT) +# Your github Personal Access Token (PAT) +GITHUB_TOKEN= +# Apigee proxy name to be used for test execution +# nhs-notify-supplier--internal-dev--nhs-notify-supplier-PR-XX +PROXY_NAME= -# The variables below are used for End to End tests -PROXY_NAME= # information about the proxy name can be found in the tests/e2e-tests/README.md +# APIM env to run e2e tests against, other options are: ref, int, prod +API_ENVIRONMENT=internal-dev +# Used for component and e2e tests +# Account group controls default account id mapping for tests. +# If omitted, tests default to dev: +# TARGET_ACCOUNT_GROUP=nhs-notify-supplier-api-dev +# Mapping used by tests: +# nhs-notify-supplier-api-dev -> 820178564574 +# nhs-notify-supplier-api-nonprod -> 885964308133 +# nhs-notify-supplier-api-prod -> blocked (tests are intentionally disabled for prod) +TARGET_ACCOUNT_GROUP=nhs-notify-supplier-api-dev - -# * nhs-notify-supplier--internal-dev--nhs-notify-supplier -# * nhs-notify-supplier--internal-dev--nhs-notify-supplier-PR-XX -# * nhs-notify-supplier--ref--nhs-notify-supplier -- ref env +# Resource namespace used to resolve AWS resource names for tests (main, pr123) +# remove if needs to run against main +TARGET_ENVIRONMENT=prxx # API Keys # ======== @@ -25,9 +36,9 @@ export STATUS_ENDPOINT_API_KEY=xxx # Private Keys # ============ -# private key used to generate authentication for tests ran against the internal-dev and internal-qa -export NON_PROD_PRIVATE_KEY=xxx # path to the private key file -# private key used to generate authentication for tests ran against the int environment +# private key path used to generate authentication for tests ran against the internal-dev and internal-qa +export NON_PROD_PRIVATE_KEY=xxx +# private key path used to generate authentication for tests ran against the int environment export INTEGRATION_PRIVATE_KEY=xxx -# private key used to generate authentication for tests ran against the prod environment +# private key path used to generate authentication for tests ran against the prod environment export PRODUCTION_PRIVATE_KEY=xxx diff --git a/.github/actions/acceptance-tests-component/action.yml b/.github/actions/acceptance-tests-components/action.yml similarity index 84% rename from .github/actions/acceptance-tests-component/action.yml rename to .github/actions/acceptance-tests-components/action.yml index 810c3b3a3..833966699 100644 --- a/.github/actions/acceptance-tests-component/action.yml +++ b/.github/actions/acceptance-tests-components/action.yml @@ -14,6 +14,10 @@ inputs: description: Name of the component under test required: true + targetAccountGroup: + description: Name of the account group under test (e.g. nhs-notify-supplier-api-dev) + required: true + runs: using: "composite" @@ -39,5 +43,6 @@ runs: shell: bash env: TARGET_ENVIRONMENT: ${{ inputs.targetEnvironment }} + TARGET_ACCOUNT_GROUP: ${{ inputs.targetAccountGroup }} run: | make test-${{ inputs.testType }} diff --git a/.github/actions/acceptance-tests-e2e/action.yml b/.github/actions/acceptance-tests-e2e/action.yml index f536842d4..ae5e2e95c 100644 --- a/.github/actions/acceptance-tests-e2e/action.yml +++ b/.github/actions/acceptance-tests-e2e/action.yml @@ -5,6 +5,9 @@ inputs: targetEnvironment: description: Name of the environment under test required: true + targetAccountGroup: + description: Name of the account group under test + required: true runs: using: "composite" @@ -65,6 +68,7 @@ runs: env: TARGET_ENVIRONMENT: ${{ inputs.targetEnvironment }} PR_NUMBER: ${{ steps.set_pr_number.outputs.pr_number }} + TARGET_ACCOUNT_GROUP: ${{ inputs.targetAccountGroup }} run: | echo "$DEV_E2E_KEYS_PRIVATE" > "${GITHUB_WORKSPACE}/internal-dev-test-1.pem" chmod 600 "${GITHUB_WORKSPACE}/internal-dev-test-1.pem" diff --git a/.github/actions/acceptance-tests/action.yml b/.github/actions/acceptance-tests/action.yml index dcf3ddb29..23370bcff 100644 --- a/.github/actions/acceptance-tests/action.yml +++ b/.github/actions/acceptance-tests/action.yml @@ -11,8 +11,7 @@ inputs: required: true targetAccountGroup: - description: Name of the account group under test - default: nhs-notify-template-management-dev + description: Name of the account group under test (e.g. nhs-notify-supplier-api-dev) required: true targetComponent: @@ -24,16 +23,18 @@ runs: steps: - - name: Run component tests + - name: Run components tests (sandbox and component tests) if: ${{ inputs.testType != 'e2e' }} - uses: ./.github/actions/acceptance-tests-component + uses: ./.github/actions/acceptance-tests-components with: testType: ${{ inputs.testType }} targetEnvironment: ${{ inputs.targetEnvironment }} targetComponent: ${{ inputs.targetComponent }} + targetAccountGroup: ${{ inputs.targetAccountGroup }} - name: Run e2e tests - if: ${{ inputs.testType == 'e2e' && inputs.targetEnvironment == 'main' }} + if: ${{ inputs.testType == 'e2e' }} uses: ./.github/actions/acceptance-tests-e2e with: targetEnvironment: ${{ inputs.targetEnvironment }} + targetAccountGroup: ${{ inputs.targetAccountGroup }} diff --git a/.github/actions/build-proxies/action.yml b/.github/actions/build-proxies/action.yml index 728edf4bc..23c5005c3 100644 --- a/.github/actions/build-proxies/action.yml +++ b/.github/actions/build-proxies/action.yml @@ -118,4 +118,5 @@ runs: --apimEnvironment "${{ env.APIM_ENV }}" \ --boundedContext "notify-supplier" \ --targetDomain "$TARGET_DOMAIN" \ - --version "${{ inputs.version }}" + --version "${{ inputs.version }}" \ + --internalRef "feature/CCM-17012" # TO BE REMOVED - used to trigger workflow until internal branch merges diff --git a/.github/workflows/deploy-dynamic-env-proxy.yaml b/.github/workflows/deploy-dynamic-env-proxy.yaml index 5c4607ae0..a61b3035d 100644 --- a/.github/workflows/deploy-dynamic-env-proxy.yaml +++ b/.github/workflows/deploy-dynamic-env-proxy.yaml @@ -29,7 +29,8 @@ jobs: - name: Resolve nodejs version id: toolversions - run: echo "nodejs_version=$(grep '^nodejs\s' .tool-versions | cut -f2 -d' ')" >> + run: + echo "nodejs_version=$(grep '^nodejs\s' .tool-versions | cut -f2 -d' ')" >> "$GITHUB_OUTPUT" - name: "Check if pull request exists for this branch and set diff --git a/Makefile b/Makefile index c7915eb31..a0368e2f0 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,7 @@ # the project as automated steps to be executed on locally and in the CD pipeline. include scripts/init.mk +-include .env # Load environment variables from .env file if it exists # ============================================================================== @@ -130,11 +131,14 @@ ${VERBOSE}.SILENT: \ # E2E Test commands # ##################### +# https://pytest-xdist.readthedocs.io/en/stable/known-limitations.html#output-stdout-and-stderr-from-workers means pytest won't print to stdout even with -s +PYTEST_WORKERS := 4 # set to 0 to see stdout/stderr when debugging e2e tests + TEST_CMD := APIGEE_ACCESS_TOKEN="$(APIGEE_ACCESS_TOKEN)" \ STATUS_ENDPOINT_API_KEY="$(STATUS_ENDPOINT_API_KEY)" \ PYTHONPATH=. poetry run pytest --disable-warnings -vv \ --color=yes \ - -n 4 \ + -n $(PYTEST_WORKERS) \ --api-name=nhs-notify-supplier \ --proxy-name="$(PROXY_NAME)" \ -s \ @@ -145,7 +149,6 @@ TEST_CMD := APIGEE_ACCESS_TOKEN="$(APIGEE_ACCESS_TOKEN)" \ --only-rerun 'AssertionError: Unexpected 502' \ --junitxml=test-report.xml - .internal-dev-test: @cd tests/e2e-tests && \ $(TEST_CMD) \ @@ -161,7 +164,7 @@ TEST_CMD := APIGEE_ACCESS_TOKEN="$(APIGEE_ACCESS_TOKEN)" \ PROD_CMD := APIGEE_ACCESS_TOKEN="$(APIGEE_ACCESS_TOKEN)" \ PYTHONPATH=. poetry run pytest --disable-warnings -vv \ --color=yes \ - -n 4 \ + -n $(PYTEST_WORKERS) \ --api-name=nhs-notify-supplier \ --proxy-name="$(PROXY_NAME)" \ -s \ diff --git a/infrastructure/terraform/components/api/README.md b/infrastructure/terraform/components/api/README.md index 23356b0a4..3a6e442c4 100644 --- a/infrastructure/terraform/components/api/README.md +++ b/infrastructure/terraform/components/api/README.md @@ -19,6 +19,7 @@ No requirements. | [csoc\_log\_forwarding](#input\_csoc\_log\_forwarding) | Enable forwarding of API Gateway logs to CSOC | `bool` | `true` | no | | [default\_tags](#input\_default\_tags) | A map of default tags to apply to all taggable resources within the component | `map(string)` | `{}` | no | | [disable\_gateway\_execute\_endpoint](#input\_disable\_gateway\_execute\_endpoint) | Disable the execution endpoint for the API Gateway | `bool` | `true` | no | +| [download\_url\_ttl\_seconds](#input\_download\_url\_ttl\_seconds) | TTL in seconds for generated download URLs | `number` | `60` | no | | [enable\_alarms](#input\_enable\_alarms) | Enable CloudWatch alarms for this deployed environment | `bool` | `true` | no | | [enable\_api\_data\_trace](#input\_enable\_api\_data\_trace) | Enable API Gateway data trace logging | `bool` | `false` | no | | [enable\_backups](#input\_enable\_backups) | Enable backups | `bool` | `false` | no | @@ -33,14 +34,18 @@ No requirements. | [eventpub\_data\_plane\_bus\_arn](#input\_eventpub\_data\_plane\_bus\_arn) | ARN of the EventBridge data plane bus for eventpub | `string` | `""` | no | | [force\_destroy](#input\_force\_destroy) | Flag to force deletion of S3 buckets | `bool` | `false` | no | | [force\_lambda\_code\_deploy](#input\_force\_lambda\_code\_deploy) | If the lambda package in s3 has the same commit id tag as the terraform build branch, the lambda will not update automatically. Set to True if making changes to Lambda code from on the same commit for example during development | `bool` | `false` | no | -| [group](#input\_group) | The group variables are being inherited from (often synonmous with account short-name) | `string` | n/a | yes | +| [group](#input\_group) | The account group short-name | `string` | n/a | yes | | [kms\_deletion\_window](#input\_kms\_deletion\_window) | When a kms key is deleted, how long should it wait in the pending deletion state? | `string` | `"30"` | no | | [letter\_event\_source](#input\_letter\_event\_source) | Source value to use for the letter status event updates | `string` | `"/data-plane/supplier-api/nhs-supplier-api-prod/main/update-status"` | no | +| [letter\_queue\_ttl\_hours](#input\_letter\_queue\_ttl\_hours) | TTL in hours for letter queue records | `number` | `168` | no | +| [letter\_queue\_visibility\_timeout](#input\_letter\_queue\_visibility\_timeout) | Visibility timeout in seconds for processing queued letter updates | `number` | `300` | no | | [letter\_table\_ttl\_hours](#input\_letter\_table\_ttl\_hours) | Number of hours to set as TTL on letters table | `number` | `24` | no | +| [letter\_ttl\_hours](#input\_letter\_ttl\_hours) | TTL in hours for letter records | `number` | `12960` | no | | [log\_level](#input\_log\_level) | The log level to be used in lambda functions within the component. Any log with a lower severity than the configured value will not be logged: https://docs.python.org/3/library/logging.html#levels | `string` | `"INFO"` | no | | [log\_retention\_in\_days](#input\_log\_retention\_in\_days) | The retention period in days for the Cloudwatch Logs events to be retained, default of 0 is indefinite | `number` | `0` | no | | [manually\_configure\_mtls\_truststore](#input\_manually\_configure\_mtls\_truststore) | Manually manage the truststore used for API Gateway mTLS (e.g. for prod environment) | `bool` | `false` | no | | [max\_get\_limit](#input\_max\_get\_limit) | Default limit to apply to GET requests that support pagination | `number` | `2500` | no | +| [mi\_ttl\_hours](#input\_mi\_ttl\_hours) | TTL in hours for MI records | `number` | `2160` | no | | [parent\_acct\_environment](#input\_parent\_acct\_environment) | Name of the environment responsible for the acct resources used, affects things like DNS zone. Useful for named dev environments | `string` | `"main"` | no | | [project](#input\_project) | The name of the tfscaffold project | `string` | n/a | yes | | [region](#input\_region) | The AWS Region | `string` | n/a | yes | diff --git a/infrastructure/terraform/components/api/locals.tf b/infrastructure/terraform/components/api/locals.tf index 6975e8d4e..5a37f1f12 100644 --- a/infrastructure/terraform/components/api/locals.tf +++ b/infrastructure/terraform/components/api/locals.tf @@ -23,15 +23,15 @@ locals { common_lambda_env_vars = { APIM_CORRELATION_HEADER = "nhsd-correlation-id", - DOWNLOAD_URL_TTL_SECONDS = 60 - EVENT_SOURCE = "/data-plane/supplier-api/${var.group}/${var.environment}/letters" - LETTER_TTL_HOURS = 12960, # 18 months * 30 days * 24 hours + DOWNLOAD_URL_TTL_SECONDS = var.download_url_ttl_seconds, + EVENT_SOURCE = "/data-plane/supplier-api/${var.group}/${var.environment}/letters", + LETTER_TTL_HOURS = var.letter_ttl_hours, LETTER_QUEUE_TABLE_NAME = aws_dynamodb_table.letter_queue.name, - LETTER_QUEUE_TTL_HOURS = 168 # 7 days * 24 hours - LETTER_QUEUE_VISIBILITY_TIMEOUT = 300, # 5 minutes * 60 seconds + LETTER_QUEUE_TTL_HOURS = var.letter_queue_ttl_hours, + LETTER_QUEUE_VISIBILITY_TIMEOUT = var.letter_queue_visibility_timeout, LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name, MI_TABLE_NAME = aws_dynamodb_table.mi.name, - MI_TTL_HOURS = 2160 # 90 days * 24 hours + MI_TTL_HOURS = var.mi_ttl_hours, SNS_TOPIC_ARN = "${module.eventsub.sns_topic.arn}", SUPPLIER_CONFIG_TABLE_NAME = aws_dynamodb_table.supplier-configuration.name, SUPPLIER_QUOTAS_TABLE_NAME = aws_dynamodb_table.supplier-quotas.name, diff --git a/infrastructure/terraform/components/api/variables.tf b/infrastructure/terraform/components/api/variables.tf index 0c64ff283..efea09a15 100644 --- a/infrastructure/terraform/components/api/variables.tf +++ b/infrastructure/terraform/components/api/variables.tf @@ -24,7 +24,7 @@ variable "region" { variable "group" { type = string - description = "The group variables are being inherited from (often synonmous with account short-name)" + description = "The account group short-name" } ## @@ -111,6 +111,36 @@ variable "max_get_limit" { default = 2500 } +variable "download_url_ttl_seconds" { + type = number + description = "TTL in seconds for generated download URLs" + default = 60 +} + +variable "letter_ttl_hours" { + type = number + description = "TTL in hours for letter records" + default = 12960 +} + +variable "letter_queue_ttl_hours" { + type = number + description = "TTL in hours for letter queue records" + default = 168 +} + +variable "letter_queue_visibility_timeout" { + type = number + description = "Visibility timeout in seconds for processing queued letter updates" + default = 300 +} + +variable "mi_ttl_hours" { + type = number + description = "TTL in hours for MI records" + default = 2160 +} + variable "parent_acct_environment" { type = string description = "Name of the environment responsible for the acct resources used, affects things like DNS zone. Useful for named dev environments" diff --git a/lambdas/api-handler/src/services/letter-operations.ts b/lambdas/api-handler/src/services/letter-operations.ts index b96b3f8db..a29017f65 100644 --- a/lambdas/api-handler/src/services/letter-operations.ts +++ b/lambdas/api-handler/src/services/letter-operations.ts @@ -41,7 +41,6 @@ function mapPendingLetterToLetterBase(pending: PendingLetterBase): LetterBase { export const getPendingLetters = async ( supplierId: string, - limit: number, letterQueueRepo: LetterQueueRepository, visibilityTimeout: number, diff --git a/package-lock.json b/package-lock.json index b997674e0..78538f2be 100644 --- a/package-lock.json +++ b/package-lock.json @@ -116,7 +116,7 @@ }, "internal/events": { "name": "@nhsdigital/nhs-notify-event-schemas-supplier-api", - "version": "1.0.18", + "version": "1.0.19", "license": "MIT", "dependencies": { "@asyncapi/bundler": "^0.6.4", diff --git a/scripts/utilities/letter-test-data/src/cli/index.ts b/scripts/utilities/letter-test-data/src/cli/index.ts index a8ddc8ffb..0b6c32371 100644 --- a/scripts/utilities/letter-test-data/src/cli/index.ts +++ b/scripts/utilities/letter-test-data/src/cli/index.ts @@ -229,6 +229,9 @@ async function main() { await letterRepository.unsafePutLetterBatch(letterDtos); console.log(`Created batch ${batchId} of ${letterDtos.length} letters`); + console.log( + `LETTER_IDS:${JSON.stringify(letterDtos.map(({ id }) => id))}`, + ); }, ) .demandCommand(1) diff --git a/tests/component-tests/apiGateway-tests/get-letters.spec.ts b/tests/component-tests/apiGateway-tests/get-letters.spec.ts index d01a97d43..b48544916 100644 --- a/tests/component-tests/apiGateway-tests/get-letters.spec.ts +++ b/tests/component-tests/apiGateway-tests/get-letters.spec.ts @@ -1,11 +1,15 @@ import { expect, test } from "@playwright/test"; -import { SUPPLIER_LETTERS } from "../../constants/api-constants"; import { createHeaderWithNoCorrelationId, createInvalidRequestHeaders, createValidRequestHeaders, } from "../../constants/request-headers"; import getRestApiGatewayBaseUrl from "../../helpers/aws-gateway-helper"; +import { + getLettersWithRetry, + isErrorResponse, + isGetLettersResponse, +} from "../../helpers/generate-fetch-test-data"; let baseUrl: string; @@ -15,31 +19,36 @@ test.beforeAll(async () => { test.describe("API Gateway Tests To Get List Of Pending Letters", () => { test("GET /letters should return 200 and list items", async ({ request }) => { - const header = createValidRequestHeaders(); - const response = await request.get(`${baseUrl}/${SUPPLIER_LETTERS}`, { - headers: header, - params: { - limit: "2", + const headers = createValidRequestHeaders(); + const { responseBody, statusCode } = await getLettersWithRetry( + request, + baseUrl, + headers, + { + lettersLimit: "2", }, - }); + ); - expect(response.status()).toBe(200); - const responseBody = await response.json(); + expect(statusCode).toBe(200); + if (!isGetLettersResponse(responseBody)) { + throw new Error("Expected GetLettersResponse body for 200 status"); + } expect(responseBody.data.length).toBeGreaterThanOrEqual(1); }); test("GET /letters with invalid authentication should return 403", async ({ request, }) => { - const header = createInvalidRequestHeaders(); - const response = await request.get(`${baseUrl}/${SUPPLIER_LETTERS}`, { - headers: header, - params: { - limit: "2", + const headers = createInvalidRequestHeaders(); + const { responseBody, statusCode } = await getLettersWithRetry( + request, + baseUrl, + headers, + { + waitForVisibilityTimeout: false, }, - }); - expect(response.status()).toBe(403); - const responseBody = await response.json(); + ); + expect(statusCode).toBe(403); expect(responseBody).toMatchObject({ Message: "User is not authorized to access this resource with an explicit deny in an identity-based policy", @@ -49,15 +58,19 @@ test.describe("API Gateway Tests To Get List Of Pending Letters", () => { test("GET /letters with empty correlationId should return 500", async ({ request, }) => { - const header = createHeaderWithNoCorrelationId(); - const response = await request.get(`${baseUrl}/${SUPPLIER_LETTERS}`, { - headers: header, - params: { - limit: "2", + const headers = createHeaderWithNoCorrelationId(); + const { responseBody, statusCode } = await getLettersWithRetry( + request, + baseUrl, + headers, + { + waitForVisibilityTimeout: false, }, - }); - expect(response.status()).toBe(500); - const responseBody = await response.json(); + ); + expect(statusCode).toBe(500); + if (!isErrorResponse(responseBody)) { + throw new Error("Expected ErrorResponse body for 500 status"); + } expect(responseBody.errors[0].code).toBe("NOTIFY_INTERNAL_SERVER_ERROR"); expect(responseBody.errors[0].detail).toBe("Unexpected error"); }); @@ -65,15 +78,17 @@ test.describe("API Gateway Tests To Get List Of Pending Letters", () => { test("GET /letters with invalid query param return 400", async ({ request, }) => { - const header = createValidRequestHeaders(); - const response = await request.get(`${baseUrl}/${SUPPLIER_LETTERS}`, { - headers: header, - params: { - limit: "?", + const headers = createValidRequestHeaders(); + const { responseBody, statusCode } = await getLettersWithRetry( + request, + baseUrl, + headers, + { + lettersLimit: "?", + waitForVisibilityTimeout: false, }, - }); - expect(response.status()).toBe(400); - const responseBody = await response.json(); + ); + expect(statusCode).toBe(400); expect(responseBody).toMatchObject({ errors: [ { diff --git a/tests/component-tests/integration-tests/urgent-letter-priority.spec.ts b/tests/component-tests/integration-tests/urgent-letter-priority.spec.ts index 2a2f2858c..8b72dbbae 100644 --- a/tests/component-tests/integration-tests/urgent-letter-priority.spec.ts +++ b/tests/component-tests/integration-tests/urgent-letter-priority.spec.ts @@ -1,7 +1,6 @@ import { expect, test } from "@playwright/test"; import getRestApiGatewayBaseUrl from "tests/helpers/aws-gateway-helper"; import { pollForLetterStatus } from "tests/helpers/poll-for-letters-helper"; -import { getLettersFromQueueViaIndex } from "tests/helpers/generate-fetch-test-data"; import { getVariantsWithUrgency, sendEventsForVariants, @@ -10,12 +9,12 @@ import { verifyIndexPositionOfLetterVariants, } from "tests/helpers/urgent-letter-priority-helper"; import { createValidRequestHeaders } from "tests/constants/request-headers"; -import { SUPPLIER_LETTERS } from "tests/constants/api-constants"; import { supplierDataSetup } from "tests/helpers/suppliers-setup-helper"; +import { logger } from "tests/helpers/pino-logger"; import { - GetLettersResponse, - GetLettersResponseSchema, -} from "../../../lambdas/api-handler/src/contracts/letters"; + getLettersWithRetry, + isGetLettersResponse, +} from "tests/helpers/generate-fetch-test-data"; let baseUrl: string; @@ -35,6 +34,8 @@ test.describe("Urgent Letter Priority Tests", () => { const urgencyNineLetterIds = await sendEventsForVariants(variantsUrgencyNine); + logger.info({ urgencyNineLetterIds, urgencyTenLetterIds }); + await Promise.all( [...urgencyNineLetterIds, ...urgencyTenLetterIds].map(async (domainId) => pollForLetterStatus(request, supplier, domainId, baseUrl), @@ -44,29 +45,24 @@ test.describe("Urgent Letter Priority Tests", () => { await verifyAllocationLogsContainPriority(urgencyNineLetterIds, 9); await verifyAllocationLogsContainPriority(urgencyTenLetterIds, 10); - const lettersFromQueue = await getLettersFromQueueViaIndex(supplier); - - const letterIdsFromQueue = lettersFromQueue.map( - (letter) => letter.letterId, + const headers = createValidRequestHeaders(supplier); + const { responseBody, statusCode } = await getLettersWithRetry( + request, + baseUrl, + headers, ); - const header = createValidRequestHeaders(supplier); - const response = await request.get(`${baseUrl}/${SUPPLIER_LETTERS}`, { - headers: header, - }); + expect(statusCode).toBe(200); + if (!isGetLettersResponse(responseBody)) { + throw new Error("Expected GetLettersResponse body for 200 status"); + } - expect(response.status()).toBe(200); - const responseBody = await response.json(); expect(responseBody.data.length).toBeGreaterThanOrEqual(1); - const getLettersResponse: GetLettersResponse = - GetLettersResponseSchema.parse(responseBody); - - const letterIds = getLettersResponse.data.map((letter) => letter.id); - expect(letterIds).toEqual(letterIdsFromQueue); + const letterIds = responseBody.data.map((letter) => letter.id); verifyIndexPositionOfLetterVariants( - letterIdsFromQueue, + letterIds, urgencyTenLetterIds, urgencyNineLetterIds, ); diff --git a/tests/component-tests/letterQueue-tests/queue-operations.spec.ts b/tests/component-tests/letterQueue-tests/queue-operations.spec.ts index 1810f273c..ef96ef586 100644 --- a/tests/component-tests/letterQueue-tests/queue-operations.spec.ts +++ b/tests/component-tests/letterQueue-tests/queue-operations.spec.ts @@ -11,11 +11,16 @@ import { supplierIdFromSupplierAllocatorLog, } from "tests/helpers/aws-cloudwatch-helper"; import getRestApiGatewayBaseUrl from "tests/helpers/aws-gateway-helper"; -import { SUPPLIER_LETTERS } from "tests/constants/api-constants"; +import { + SUPPLIER_LETTERS, + VISIBILITY_TIMEOUT_SECONDS, +} from "tests/constants/api-constants"; import { supplierDataSetup } from "tests/helpers/suppliers-setup-helper"; import { checkLetterQueueTable, getLetterFromQueueById, + getLettersWithRetry, + isGetLettersResponse, } from "tests/helpers/generate-fetch-test-data"; import { createValidRequestHeaders } from "../../constants/request-headers"; import { @@ -132,17 +137,21 @@ test.describe("Letter Queue Tests", () => { ); // call get letters endpoint which should update the visibility timestamp - const header = createValidRequestHeaders(supplierId); - const getLettersResponse = await request.get( - `${baseUrl}/${SUPPLIER_LETTERS}`, - { - headers: header, - }, + const headers = createValidRequestHeaders(supplierId); + const { responseBody, statusCode } = await getLettersWithRetry( + request, + baseUrl, + headers, ); - expect(getLettersResponse.status()).toBe(200); + expect(statusCode).toBe(200); + if (!isGetLettersResponse(responseBody)) { + throw new Error("Expected GetLettersResponse body for 200 status"); + } + expect(responseBody.data.length).toBeGreaterThanOrEqual(1); + const currentTimeWithTimeOut = Math.floor( - (Date.now() + 5 * 60 * 1000) / 1000, + (Date.now() + VISIBILITY_TIMEOUT_SECONDS * 1000) / 1000, ); logger.info( @@ -158,15 +167,15 @@ test.describe("Letter Queue Tests", () => { Math.abs(visibilityTimestampAfterGet - currentTimeWithTimeOut), ).toBeLessThanOrEqual(1); - const getLettersWithInVisibility = await request.get( - `${baseUrl}/${SUPPLIER_LETTERS}`, - { - headers: header, - }, - ); + const { responseBody: secondResponseBody, statusCode: secondStatusCode } = + await getLettersWithRetry(request, baseUrl, headers, { + waitForVisibilityTimeout: false, + }); - expect(getLettersWithInVisibility.status()).toBe(200); - const responseBody = await getLettersWithInVisibility.json(); - expect(responseBody.data).toHaveLength(0); + expect(secondStatusCode).toBe(200); + if (!isGetLettersResponse(secondResponseBody)) { + throw new Error("Expected GetLettersResponse body for 200 status"); + } + expect(secondResponseBody.data).toHaveLength(0); }); }); diff --git a/tests/constants/api-constants.ts b/tests/constants/api-constants.ts index 608fb75d8..9d66106c2 100644 --- a/tests/constants/api-constants.ts +++ b/tests/constants/api-constants.ts @@ -5,14 +5,50 @@ export const AWS_REGION = "eu-west-2"; export const envName = process.env.TARGET_ENVIRONMENT ?? "main"; export const API_NAME = `nhs-${envName}-supapi`; export const LETTERSTABLENAME = `nhs-${envName}-supapi-letters`; -export const SUPPLIERID = "TestSupplier1"; +export const SUPPLIERID = "supplier1"; export const MI_ENDPOINT = "mi"; export const SUPPLIERTABLENAME = `nhs-${envName}-supapi-suppliers`; -export const UPSERT_LETTER_LAMBDA_ARN = `arn:aws:lambda:eu-west-2:820178564574:function:nhs-${envName}-supapi-upsertletter`; export const DATA = "data"; export const EVENT_SUBSCRIPTION_TOPIC_NAME = `nhs-${envName}-supapi-eventsub`; -export const AWS_ACCOUNT_ID = process.env.AWS_ACCOUNT_ID ?? "820178564574"; + +export const DEFAULT_TARGET_ACCOUNT_GROUP = "nhs-notify-supplier-api-dev"; +export const PROD_TARGET_ACCOUNT_GROUP = "nhs-notify-supplier-api-prod"; +export const TARGET_ACCOUNT_GROUP = + process.env.TARGET_ACCOUNT_GROUP ?? DEFAULT_TARGET_ACCOUNT_GROUP; + +export const ACCOUNT_GROUP_TO_AWS_ACCOUNT_ID = new Map([ + ["nhs-notify-supplier-api-dev", "820178564574"], + ["nhs-notify-supplier-api-nonprod", "885964308133"], +]); + +function resolveAwsAccountId(): string { + if (TARGET_ACCOUNT_GROUP === PROD_TARGET_ACCOUNT_GROUP) { + throw new Error( + `TARGET_ACCOUNT_GROUP='${TARGET_ACCOUNT_GROUP}' points to production. Test execution against production is blocked.`, + ); + } + + const mappedAccountId = + ACCOUNT_GROUP_TO_AWS_ACCOUNT_ID.get(TARGET_ACCOUNT_GROUP); + if (mappedAccountId) { + return mappedAccountId; + } + + throw new Error( + `No AWS account mapping configured for TARGET_ACCOUNT_GROUP='${TARGET_ACCOUNT_GROUP}'. Add a mapping in tests/constants/api-constants.ts.`, + ); +} + +export const AWS_ACCOUNT_ID = resolveAwsAccountId(); +export const UPSERT_LETTER_LAMBDA_ARN = `arn:aws:lambda:eu-west-2:${AWS_ACCOUNT_ID}:function:nhs-${envName}-supapi-upsertletter`; export const EVENT_SUBSCRIPTION_TOPIC_ARN = process.env.EVENT_SUBSCRIPTION_TOPIC_ARN ?? `arn:aws:sns:${AWS_REGION}:${AWS_ACCOUNT_ID}:${EVENT_SUBSCRIPTION_TOPIC_NAME}`; export const LETTERQUEUE_TABLENAME = `nhs-${envName}-supapi-letter-queue`; +export const GET_LETTERS_MAX_RETRIES = 3; +export const DEV_VISIBILITY_TIMEOUT_SECONDS = 10; +export const DEFAULT_VISIBILITY_TIMEOUT_SECONDS = 300; +export const VISIBILITY_TIMEOUT_SECONDS = + TARGET_ACCOUNT_GROUP === DEFAULT_TARGET_ACCOUNT_GROUP + ? DEV_VISIBILITY_TIMEOUT_SECONDS + : DEFAULT_VISIBILITY_TIMEOUT_SECONDS; diff --git a/tests/e2e-tests/api/data/test_get_letter_data.py b/tests/e2e-tests/api/data/test_get_letter_data.py index 42bfc0bef..94b865079 100644 --- a/tests/e2e-tests/api/data/test_get_letter_data.py +++ b/tests/e2e-tests/api/data/test_get_letter_data.py @@ -11,28 +11,17 @@ @pytest.mark.devtest @pytest.mark.inttest @pytest.mark.prodtest -def test_200_get_letter_status(url, authentication_secret): +def test_200_get_letter_data(url, authentication_secret): headers = Generators.generate_valid_headers(authentication_secret) ids = get_pending_letter_ids(url, headers, LETTERS_ENDPOINT, limit=1) - get_letter_data = requests.get(f"{url}/{LETTERS_ENDPOINT}/{ids[0]}/data", headers=headers) + print(f"calling GET {url}{LETTERS_ENDPOINT}/{ids[0]}/data with headers {headers}") + get_letter_data = requests.get(f"{url}{LETTERS_ENDPOINT}/{ids[0]}/data", headers=headers) ErrorHandler.handle_retry(get_letter_data) assert get_letter_data.status_code == 200, f"Response: {get_letter_data.status_code}: {get_letter_data.text}" assert get_letter_data.headers.get("Content-Type") == "application/pdf" -@pytest.mark.test -@pytest.mark.devtest -@pytest.mark.inttest -@pytest.mark.prodtest -def test_404_letter_does_not_exist(url, authentication_secret): - headers = Generators.generate_valid_headers(authentication_secret) - get_message_response = requests.get(f"{url}/{LETTERS_ENDPOINT}/xx", headers=headers) - - ErrorHandler.handle_retry(get_message_response) - assert get_message_response.status_code == 404 - assert get_message_response.json().get("errors")[0].get("detail") == "No resource found with that ID" - @pytest.mark.test @pytest.mark.devtest @pytest.mark.inttest @@ -40,7 +29,9 @@ def test_404_letter_does_not_exist(url, authentication_secret): def test_404_letter_does_not_exist(url, authentication_secret): letter_id = uuid.uuid4().hex headers = Generators.generate_valid_headers(authentication_secret) - get_message_response = requests.get(f"{url}/{LETTERS_ENDPOINT}/{letter_id}/data", headers=headers) + + print(f"calling GET {url}{LETTERS_ENDPOINT}/{letter_id}/data with headers {headers}") + get_message_response = requests.get(f"{url}{LETTERS_ENDPOINT}/{letter_id}/data", headers=headers) ErrorHandler.handle_retry(get_message_response) assert get_message_response.status_code == 404 diff --git a/tests/e2e-tests/api/headers/test_x_request_id.py b/tests/e2e-tests/api/headers/test_x_request_id.py index 827b8549d..7adaa7dcb 100644 --- a/tests/e2e-tests/api/headers/test_x_request_id.py +++ b/tests/e2e-tests/api/headers/test_x_request_id.py @@ -20,7 +20,7 @@ def test_header_letters_endpoint( ): auth_header = {"apikey": authentication_secret.value} if authentication_secret.auth_type == "apikey" \ else {"Authorization": authentication_secret.value} - resp = getattr(requests, method)(f"{url}/{endpoints}", headers={ + resp = getattr(requests, method)(f"{url}{endpoints}", headers={ **auth_header, "X-Request-ID": None }) @@ -38,7 +38,7 @@ def test_header_mi_endpoint( ): auth_header = {"apikey": authentication_secret.value} if authentication_secret.auth_type == "apikey" \ else {"Authorization": authentication_secret.value} - resp = getattr(requests, "post")(f"{url}/{MI_ENDPOINT}", headers={ + resp = getattr(requests, "post")(f"{url}{MI_ENDPOINT}", headers={ **auth_header, "X-Request-ID": "" }) diff --git a/tests/e2e-tests/api/letters/test_get_letter_status.py b/tests/e2e-tests/api/letters/test_get_letter_status.py index c55af8820..9cfa22175 100644 --- a/tests/e2e-tests/api/letters/test_get_letter_status.py +++ b/tests/e2e-tests/api/letters/test_get_letter_status.py @@ -16,19 +16,21 @@ def test_200_get_letter_status(url, authentication_secret): ids = get_pending_letter_ids(url, headers, LETTERS_ENDPOINT, limit=1) letter_id = ids[0] - get_message_response = requests.get(f"{url}/{LETTERS_ENDPOINT}/{letter_id}", headers=headers) + print(f"calling GET {url}{LETTERS_ENDPOINT}/{letter_id} with headers {headers}") + get_message_response = requests.get(f"{url}{LETTERS_ENDPOINT}/{letter_id}", headers=headers) ErrorHandler.handle_retry(get_message_response) assert get_message_response.status_code == 200, f"Response: {get_message_response.status_code}: {get_message_response.text}" - @pytest.mark.test @pytest.mark.devtest @pytest.mark.inttest @pytest.mark.prodtest def test_404_letter_does_not_exist(url, authentication_secret): headers = Generators.generate_valid_headers(authentication_secret) - get_message_response = requests.get(f"{url}/{LETTERS_ENDPOINT}/xx", headers=headers) + + print(f"calling GET {url}{LETTERS_ENDPOINT}/xx with headers {headers}") + get_message_response = requests.get(f"{url}{LETTERS_ENDPOINT}/xx", headers=headers) ErrorHandler.handle_retry(get_message_response) assert get_message_response.status_code == 404, f"Response: {get_message_response.status_code}: {get_message_response.text}" diff --git a/tests/e2e-tests/api/letters/test_get_list_of_letters.py b/tests/e2e-tests/api/letters/test_get_list_of_letters.py index 275d255a6..1c67edb91 100644 --- a/tests/e2e-tests/api/letters/test_get_list_of_letters.py +++ b/tests/e2e-tests/api/letters/test_get_list_of_letters.py @@ -12,5 +12,6 @@ def test_200_get_letters(url, authentication_secret): headers = Generators.generate_valid_headers(authentication_secret) ids = get_pending_letter_ids(url, headers, LETTERS_ENDPOINT, limit=1) + assert ids, "Expected at least one PENDING letter" assert len(ids) == 1 diff --git a/tests/e2e-tests/api/letters/test_multiple_letter_status.py b/tests/e2e-tests/api/letters/test_multiple_letter_status.py index 744fbb8d4..51fe72ddf 100644 --- a/tests/e2e-tests/api/letters/test_multiple_letter_status.py +++ b/tests/e2e-tests/api/letters/test_multiple_letter_status.py @@ -18,8 +18,9 @@ def test_202_with_valid_headers(url, authentication_secret): ids = get_pending_letter_ids(url, headers, LETTERS_ENDPOINT, limit=2) data = Generators.generate_multiple_valid_request(ids) + print(f"calling POST {url}{LETTERS_ENDPOINT} with headers {headers} and body {data}") update_letter_status = requests.post( - f"{url}/{LETTERS_ENDPOINT}", + f"{url}{LETTERS_ENDPOINT}", headers=headers, json=data, ) @@ -37,8 +38,9 @@ def test_400_duplicates_in_request_body(url, authentication_secret): ids = get_pending_letter_ids(url, headers, LETTERS_ENDPOINT, limit=2) data = Generators.generate_duplicate_request(ids) + print(f"calling POST {url}{LETTERS_ENDPOINT} with headers {headers} and body {data}") update_letter_status = requests.post( - f"{url}/{LETTERS_ENDPOINT}", + f"{url}{LETTERS_ENDPOINT}", headers=headers, json=data, ) @@ -57,8 +59,9 @@ def test_400_invalid_status_in_request_body(url, authentication_secret): ids = get_pending_letter_ids(url, headers, LETTERS_ENDPOINT, limit=3) data = Generators.generate_invalid_status_request(ids) + print(f"calling POST {url}{LETTERS_ENDPOINT} with headers {headers} and body {data}") update_letter_status = requests.post( - f"{url}/{LETTERS_ENDPOINT}", + f"{url}{LETTERS_ENDPOINT}", headers=headers, json=data, ) diff --git a/tests/e2e-tests/api/letters/test_update_letter_status.py b/tests/e2e-tests/api/letters/test_update_letter_status.py index 28a61c3ff..291c84848 100644 --- a/tests/e2e-tests/api/letters/test_update_letter_status.py +++ b/tests/e2e-tests/api/letters/test_update_letter_status.py @@ -19,8 +19,10 @@ def test_202_with_valid_headers(url, authentication_secret): letter_id = ids[0] data = Generators.generate_valid_message_body("ACCEPTED", letter_id) + + print(f"calling PATCH {url}{LETTERS_ENDPOINT}/{letter_id} with headers {headers} and body {data}") update_letter_status = requests.patch( - f"{url}/{LETTERS_ENDPOINT}/{letter_id}", + f"{url}{LETTERS_ENDPOINT}/{letter_id}", headers=headers, json=data, ) @@ -28,6 +30,10 @@ def test_202_with_valid_headers(url, authentication_secret): ErrorHandler.handle_retry(update_letter_status) assert update_letter_status.status_code == 202, f"Response: {update_letter_status.status_code}: {update_letter_status.text}" +@pytest.mark.test +@pytest.mark.devtest +@pytest.mark.inttest +@pytest.mark.prodtest def test_202_with_rejected_status(url, authentication_secret): headers = Generators.generate_valid_headers(authentication_secret) @@ -35,8 +41,10 @@ def test_202_with_rejected_status(url, authentication_secret): letter_id = ids[0] data = Generators.generate_valid_message_rejected("REJECTED", letter_id) + + print(f"calling PATCH {url}{LETTERS_ENDPOINT}/{letter_id} with headers {headers} and body {data}") update_letter_status = requests.patch( - f"{url}/{LETTERS_ENDPOINT}/{letter_id}", + f"{url}{LETTERS_ENDPOINT}/{letter_id}", headers=headers, json=data, ) @@ -55,8 +63,10 @@ def test_400_with_invalid_status(url, authentication_secret): letter_id = ids[0] data = Generators.generate_valid_message_body("", letter_id) + + print(f"calling PATCH {url}{LETTERS_ENDPOINT}/{letter_id} with headers {headers} and body {data}") update_letter_status = requests.patch( - f"{url}/{LETTERS_ENDPOINT}/{letter_id}", + f"{url}{LETTERS_ENDPOINT}/{letter_id}", headers=headers, json=data, ) @@ -75,8 +85,10 @@ def test_400_id_mismatch_with_request(url, authentication_secret): letter_id = ids[0] data = Generators.generate_valid_message_body("ACCEPTED", "letter1") + + print(f"calling PATCH {url}{LETTERS_ENDPOINT}/{letter_id} with headers {headers} and body {data}") update_letter_status = requests.patch( - f"{url}/{LETTERS_ENDPOINT}/{letter_id}", + f"{url}{LETTERS_ENDPOINT}/{letter_id}", headers=headers, json=data, ) diff --git a/tests/e2e-tests/lib/constants.py b/tests/e2e-tests/lib/constants.py index 79822086f..2d9fb7ea7 100644 --- a/tests/e2e-tests/lib/constants.py +++ b/tests/e2e-tests/lib/constants.py @@ -1,5 +1,36 @@ +import os + VALID_ENDPOINT_LETTERS= ["/letters"] METHODS = ["get", "post"] DEFAULT_CONTENT_TYPE = "application/vnd.api+json" LETTERS_ENDPOINT = "/letters" MI_ENDPOINT = "/mi" + +DEFAULT_TARGET_ACCOUNT_GROUP = "nhs-notify-supplier-api-dev" +PROD_TARGET_ACCOUNT_GROUP = "nhs-notify-supplier-api-prod" +ACCOUNT_GROUP_TO_AWS_ACCOUNT_ID = { + "nhs-notify-supplier-api-dev": "820178564574", + "nhs-notify-supplier-api-nonprod": "885964308133", +} + + +def resolve_aws_account_id() -> str: + target_account_group = os.environ.get( + "TARGET_ACCOUNT_GROUP", + DEFAULT_TARGET_ACCOUNT_GROUP, + ) + if target_account_group == PROD_TARGET_ACCOUNT_GROUP: + raise RuntimeError( + f"TARGET_ACCOUNT_GROUP='{target_account_group}' points to production. " + "Test execution against production is blocked." + ) + + mapped_account_id = ACCOUNT_GROUP_TO_AWS_ACCOUNT_ID.get(target_account_group) + if mapped_account_id: + return mapped_account_id + + raise RuntimeError( + "No AWS account mapping configured for " + f"TARGET_ACCOUNT_GROUP='{target_account_group}'. " + "Add a mapping in tests/e2e-tests/lib/constants.py." + ) diff --git a/tests/e2e-tests/lib/letters.py b/tests/e2e-tests/lib/letters.py index cabd8f458..700541adb 100644 --- a/tests/e2e-tests/lib/letters.py +++ b/tests/e2e-tests/lib/letters.py @@ -2,23 +2,27 @@ import subprocess import pathlib import time +import json import requests +from lib.constants import resolve_aws_account_id from lib.errorhandler import ErrorHandler _REPO_ROOT = pathlib.Path(__file__).resolve().parents[3] _CLI_WORKSPACE = "nhs-notify-supplier-api-letter-test-data-utility" -_SUPPLIER_ID = "TestSupplier1" +_SUPPLIER_ID = "supplier1" # This should be the same id registered in the Apigee App to which the proxy will be associated -def create_test_data(count: int = 10) -> None: +def create_test_data(count: int = 10) -> list[str]: """Seed PENDING letters by delegating to the shared letter-test-data CLI. Mirrors createTestData() in tests/helpers/generate-fetch-test-data.ts so both test suites seed data through the same tool. + + Returns a list of letter IDs created by the CLI. """ environment = os.environ.get("TARGET_ENVIRONMENT", "main") - aws_account_id = os.environ.get("AWS_ACCOUNT_ID", "820178564574") + aws_account_id = resolve_aws_account_id() cmd = [ "npm", @@ -38,6 +42,7 @@ def create_test_data(count: int = 10) -> None: "--test-letter", "test-letter-standard", ] + print(f"Creating test data by running CLI command: {' '.join(cmd)}") result = subprocess.run(cmd, cwd=_REPO_ROOT, capture_output=True, text=True) if result.returncode != 0: raise RuntimeError( @@ -46,6 +51,22 @@ def create_test_data(count: int = 10) -> None: f"stderr:\n{result.stderr}" ) + ids_prefix = "LETTER_IDS:" + for line in result.stdout.splitlines(): + if line.startswith(ids_prefix): + payload = line[len(ids_prefix):] + data = json.loads(payload) + if isinstance(data, list): + res = [str(item) for item in data] + print(f"The following letter IDs were created: {res}") + return res + + raise RuntimeError( + "create-letter-batch CLI completed successfully but did not output LETTER_IDS.\n" + f"stdout:\n{result.stdout}\n" + f"stderr:\n{result.stderr}" + ) + def get_pending_letter_ids( url: str, @@ -57,28 +78,37 @@ def get_pending_letter_ids( retries: int = 5, ) -> list: """Injects the given number of pending letters as test data, then waits for them to become - visible via the letters endpoint. Retries to account for other tests running in parallel stealing the letters + visible via the getLetters endpoint. + + Because the getLetters endpoint increases the visibility timeout, if it is called immediately again before the letter's visibility timeout expires, the same letter will not be returned in the response. + + Retries to account for other tests running in parallel stealing the letters. Returns a list of letter ID strings. - Raises TimeoutError if fewer than `limit` letters are returned after all retries are exhausted. + + Raises TimeoutError if the expected number of pending letters do not appear within the timeout period. """ for _ in range(retries): - create_test_data(limit) + letterIds = create_test_data(limit) deadline = time.monotonic() + timeout_s data = [] while time.monotonic() < deadline: + # Retrieves letters based on the supplier registered in the Apigee App response = requests.get( - f"{url}/{letters_endpoint}?limit={limit}", headers=headers + f"{url}{letters_endpoint}", headers=headers ) ErrorHandler.handle_retry(response) response.raise_for_status() data.extend(response.json().get("data", [])) - if len(data) >= limit: - return [item.get("id") for item in data] + idsFound = [ item.get("id") for item in data ] + if set(letterIds).issubset(idsFound): + print(f"Found all created letter IDs: {letterIds}") + return letterIds + print(f"Expected letter IDs {letterIds} not found in response. Retrying in {interval_s} seconds...") time.sleep(interval_s) raise TimeoutError( f"Timed out after {retries} retries waiting for {limit} PENDING letter(s) at " - f"{url}/{letters_endpoint}" + f"{url}{letters_endpoint}" ) diff --git a/tests/helpers/generate-fetch-test-data.ts b/tests/helpers/generate-fetch-test-data.ts index 60aa046ae..3f0e4e5ab 100644 --- a/tests/helpers/generate-fetch-test-data.ts +++ b/tests/helpers/generate-fetch-test-data.ts @@ -5,15 +5,25 @@ import { GetCommand, QueryCommand, } from "@aws-sdk/lib-dynamodb"; +import { APIRequestContext } from "@playwright/test"; import z from "zod"; import { + AWS_ACCOUNT_ID, + GET_LETTERS_MAX_RETRIES, LETTERQUEUE_TABLENAME, LETTERSTABLENAME, SUPPLIERTABLENAME, + SUPPLIER_LETTERS, + VISIBILITY_TIMEOUT_SECONDS, envName, } from "../constants/api-constants"; import { createSupplierData, runCreateLetter } from "./pnpm-helpers"; import { logger } from "./pino-logger"; +import { + GetLettersResponse, + GetLettersResponseSchema, +} from "../../lambdas/api-handler/src/contracts/letters"; +import { ErrorResponse } from "../../lambdas/api-handler/src/contracts/errors"; const ddb = new DynamoDBClient({}); const docClient = DynamoDBDocumentClient.from(ddb); @@ -53,7 +63,7 @@ export async function createTestData( filter: "nhs-notify-supplier-api-letter-test-data-utility", supplierId, environment: envName, - awsAccountId: "820178564574", + awsAccountId: AWS_ACCOUNT_ID, groupId: "TestGroupID", specificationId: "TestSpecificationID", status: "PENDING", @@ -92,6 +102,111 @@ const delay = (ms: number) => setTimeout(resolve, ms); }); +type FetchLettersWithRetryOptions = { + lettersLimit?: string; + waitForVisibilityTimeout?: boolean; +}; + +type GetLettersResponseBody = + | GetLettersResponse + | ErrorResponse + | Record; + +type FetchLettersWithRetryResult = { + statusCode: number; + responseBody: GetLettersResponseBody; +}; + +export function isGetLettersResponse( + responseBody: GetLettersResponseBody, +): responseBody is GetLettersResponse { + return GetLettersResponseSchema.safeParse(responseBody).success; +} + +export function isErrorResponse( + responseBody: GetLettersResponseBody, +): responseBody is ErrorResponse { + return ( + typeof responseBody === "object" && + Array.isArray((responseBody as ErrorResponse).errors) + ); +} + +function parseGetLettersResponseBody( + parsedBody: unknown, +): GetLettersResponseBody { + const parsedGetLettersResponse = + GetLettersResponseSchema.safeParse(parsedBody); + if (parsedGetLettersResponse.success) { + return parsedGetLettersResponse.data; + } + + if (isErrorResponse(parsedBody as GetLettersResponseBody)) { + return parsedBody as ErrorResponse; + } + + return parsedBody as Record; +} + +function shouldRetryGetLettersRequest( + waitForVisibilityTimeout: boolean, + statusCode: number, + responseBody: GetLettersResponseBody, +): boolean { + const dataIsEmpty = + isGetLettersResponse(responseBody) && + Array.isArray(responseBody.data) && + responseBody.data.length === 0; + + return waitForVisibilityTimeout && statusCode === 200 && dataIsEmpty; +} + +export async function getLettersWithRetry( + request: APIRequestContext, + baseUrl: string, + headers: Record, + options?: FetchLettersWithRetryOptions, +): Promise { + const limit = options?.lettersLimit; + const waitForVisibilityTimeout = options?.waitForVisibilityTimeout ?? true; + + const executeGetLettersRequest = + limit === undefined + ? () => + request.get(`${baseUrl}/${SUPPLIER_LETTERS}`, { + headers, + }) + : () => + request.get(`${baseUrl}/${SUPPLIER_LETTERS}`, { + headers, + params: { + limit, + }, + }); + + for (let attempt = 0; attempt <= GET_LETTERS_MAX_RETRIES; attempt++) { + const response = await executeGetLettersRequest(); + const statusCode = response.status(); + + const parsedBody = (await response.json()) as unknown; + const responseBody = parseGetLettersResponseBody(parsedBody); + + const shouldRetry = shouldRetryGetLettersRequest( + waitForVisibilityTimeout, + statusCode, + responseBody, + ); + + if (!shouldRetry || attempt === GET_LETTERS_MAX_RETRIES) { + return { statusCode, responseBody }; + } + + await delay(VISIBILITY_TIMEOUT_SECONDS * 1000); + } + + throw new Error("Unexpectedly exhausted GET /letters retries"); +} + export async function waitForLetterStatus( supplierId: string, id: string, diff --git a/tests/performance/testCases/send-prepared-letter-request.spec.ts b/tests/performance/testCases/send-prepared-letter-request.spec.ts index 4381105f6..c678cdaff 100644 --- a/tests/performance/testCases/send-prepared-letter-request.spec.ts +++ b/tests/performance/testCases/send-prepared-letter-request.spec.ts @@ -6,7 +6,7 @@ import { expect, test } from "@playwright/test"; import { snsClient } from "tests/helpers/aws-sns-helper"; import { retrieveKinesisRecordsAtTimestamp } from "tests/helpers/aws-kinesis-helper"; import { logger } from "tests/helpers/pino-logger"; -import { envName } from "tests/constants/api-constants"; +import { AWS_ACCOUNT_ID, envName } from "tests/constants/api-constants"; import { randomUUID } from "node:crypto"; import PREPARED_LETTER from "../../resources/prepared-letter.json"; @@ -15,8 +15,8 @@ test.describe("Performance test checking how long it takes letter requests from const MESSAGES_TO_SEND = 2500; const FIVE_MINUTES = 1000 * 60 * 5; const BATCH_SIZE = 10; - const SNS_ARN = `arn:aws:sns:eu-west-2:820178564574:nhs-${envName}-supapi-eventsub`; - const KINESIS_STREAM_ARN = `arn:aws:kinesis:eu-west-2:820178564574:stream/nhs-${envName}-supapi-letter-change-stream`; + const SNS_ARN = `arn:aws:sns:eu-west-2:${AWS_ACCOUNT_ID}:nhs-${envName}-supapi-eventsub`; + const KINESIS_STREAM_ARN = `arn:aws:kinesis:eu-west-2:${AWS_ACCOUNT_ID}:stream/nhs-${envName}-supapi-letter-change-stream`; test.setTimeout(FIVE_MINUTES); const startTime = Date.now();