CCM-11602: param validation, max range, and OAS 400 examples

Author: masl2

infrastructure/terraform/components/api/README.md

@@ -12,12 +12,14 @@ No requirements.
 | <a name="input_aws_account_id"></a> [aws\_account\_id](#input\_aws\_account\_id) | The AWS Account ID (numeric) | `string` | n/a | yes |
 | <a name="input_ca_pem_filename"></a> [ca\_pem\_filename](#input\_ca\_pem\_filename) | Filename for the CA truststore file within the s3 bucket | `string` | `null` | no |
 | <a name="input_component"></a> [component](#input\_component) | The variable encapsulating the name of this component | `string` | `"supapi"` | no |
+| <a name="input_default_get_limit"></a> [default\_get\_limit](#input\_default\_get\_limit) | Default limit to apply to GET requests that support pagination | `number` | `2500` | no |
 | <a name="input_default_tags"></a> [default\_tags](#input\_default\_tags) | A map of default tags to apply to all taggable resources within the component | `map(string)` | `{}` | no |
 | <a name="input_enable_backups"></a> [enable\_backups](#input\_enable\_backups) | Enable backups | `bool` | `false` | no |
 | <a name="input_environment"></a> [environment](#input\_environment) | The name of the tfscaffold environment | `string` | n/a | yes |
 | <a name="input_force_lambda_code_deploy"></a> [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 |
 | <a name="input_group"></a> [group](#input\_group) | The group variables are being inherited from (often synonmous with account short-name) | `string` | n/a | yes |
 | <a name="input_kms_deletion_window"></a> [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 |
+| <a name="input_letter_table_ttl_hours"></a> [letter\_table\_ttl\_hours](#input\_letter\_table\_ttl\_hours) | Number of hours to set as TTL on letters table | `number` | `24` | no |
 | <a name="input_log_level"></a> [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 |
 | <a name="input_log_retention_in_days"></a> [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 |
 | <a name="input_manually_configure_mtls_truststore"></a> [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 |

infrastructure/terraform/components/api/module_lambda_get_letters.tf

@@ -37,7 +37,8 @@ module "get_letters" {
 
   lambda_env_vars = {
     LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name,
-    LETTER_TTL_HOURS   = 24
+    LETTER_TTL_HOURS   = var.letter_table_ttl_hours,
+    DEFAULT_LIMIT      = var.default_get_limit,
   }
 }
 

infrastructure/terraform/components/api/variables.tf

@@ -99,9 +99,20 @@ variable "enable_backups" {
   default     = false
 }
 
-
 variable "ca_pem_filename" {
   type        = string
   description = "Filename for the CA truststore file within the s3 bucket"
   default     = null
 }
+
+variable "letter_table_ttl_hours" {
+  type        = number
+  description = "Number of hours to set as TTL on letters table"
+  default     = 24
+}
+
+variable "default_get_limit" {
+  type        = number
+  description = "Default limit to apply to GET requests that support pagination"
+  default     = 2500
+}

internal/datastore/src/types.md

@@ -10,19 +10,19 @@ The schemas are generated from Zod definitions and provide a visual representati
 erDiagram
     Letter {
         string id
-        string supplierId "ref: Supplier"
+        string status "enum: PENDING, ACCEPTED, REJECTED, PRINTED, ENCLOSED, CANCELLED, DISPATCHED, FAILED, RETURNED, DESTROYED, FORWARDED, DELIVERED"
         string specificationId
         string groupId
+        number reasonCode
+        string reasonText
+        string supplierId
         string url "url"
-        string status "enum: PENDING, ACCEPTED, DISPATCHED, FAILED, REJECTED, DELIVERED, CANCELLED"
         string createdAt
         string updatedAt
         string supplierStatus
+        string supplierStatusSk
         number ttl "min: -9007199254740991, max: 9007199254740991"
     }
-    Supplier {
-    }
-    Letter }o--|| Supplier : "supplierId"
 ```
 
 ## MI schema

lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts

@@ -10,6 +10,7 @@ describe('API Lambda handler', () => {
 
   beforeEach(() => {
     jest.resetAllMocks();
+    process.env.DEFAULT_LIMIT = '100';
   });
 
   it('returns 200 OK with basic paginated resources', async () => {
@@ -90,11 +91,11 @@ describe('API Lambda handler', () => {
 
     expect(result).toEqual({
       statusCode: 400,
-      body: "Bad Request: limit parameter is not a number",
+      body: "Invalid Request: limit parameter must be a positive number not greater than 2500",
     });
   });
 
-  it("returns 400 if the limit parameter is not positive", async () => {
+  it("returns 400 if the limit parameter is negative", async () => {
     const event = makeApiGwEvent({
       path: "/letters",
       queryStringParameters: { limit: "-1" },
@@ -105,7 +106,37 @@ describe('API Lambda handler', () => {
 
     expect(result).toEqual({
       statusCode: 400,
-      body: "Bad Request: limit parameter is not positive",
+      body: "Invalid Request: limit parameter must be a positive number not greater than 2500",
+    });
+  });
+
+  it("returns 400 if the limit parameter is out of range", async () => {
+    const event = makeApiGwEvent({
+      path: "/letters",
+      queryStringParameters: { limit: "2501" },
+    });
+    const context = mockDeep<Context>();
+    const callback = jest.fn();
+    const result = await getLetters(event, context, callback);
+
+    expect(result).toEqual({
+      statusCode: 400,
+      body: "Invalid Request: limit parameter must be a positive number not greater than 2500",
+    });
+  });
+
+  it("returns 400 if unknown parameters are present", async () => {
+    const event = makeApiGwEvent({
+      path: "/letters",
+      queryStringParameters: { max: "2000" },
+    });
+    const context = mockDeep<Context>();
+    const callback = jest.fn();
+    const result = await getLetters(event, context, callback);
+
+    expect(result).toEqual({
+      statusCode: 400,
+      body: "Invalid Request: Only 'limit' query parameter is allowed",
     });
   });
 
@@ -117,7 +148,7 @@ describe('API Lambda handler', () => {
 
     expect(result).toEqual({
       statusCode: 400,
-      body: 'Bad Request: Missing supplier ID',
+      body: 'Invalid Request: Missing supplier ID',
     });
   });
 
@@ -129,7 +160,7 @@ describe('API Lambda handler', () => {
 
     expect(result).toEqual({
       statusCode: 400,
-      body: 'Bad Request: Missing supplier ID',
+      body: 'Invalid Request: Missing supplier ID',
     });
   });
 });

lambdas/api-handler/src/handlers/get-letters.ts

@@ -6,6 +6,7 @@ import pino from 'pino';
 
 const letterRepo = createLetterRepository();
 const log = pino();
+const DEFAULT_LIMIT = process.env.DEFAULT_LIMIT || '2500';
 
 export const getLetters: APIGatewayProxyHandler = async (event) => {
   if (event.path === "/letters") {
@@ -17,19 +18,32 @@ export const getLetters: APIGatewayProxyHandler = async (event) => {
       });
       return {
         statusCode: 400,
-        body: "Bad Request: Missing supplier ID",
+        body: "Invalid Request: Missing supplier ID",
       };
     }
 
     // The endpoint should only return pending letters for now
     const status = "PENDING";
 
-    let limit = event.queryStringParameters?.limit;
+    if (
+      event.queryStringParameters &&
+      Object.keys(event.queryStringParameters).some(
+        (key) => key !== "limit"
+      )
+    ) {
+      log.info({
+        description: "Unexpected query parameter(s) present",
+        queryStringParameters: event.queryStringParameters,
+      });
 
-    if (!limit) {
-      limit = "10";
+      return {
+        statusCode: 400,
+        body: "Invalid Request: Only 'limit' query parameter is supported",
+      };
     }
 
+    let limit = event.queryStringParameters?.limit || DEFAULT_LIMIT;
+
     let limitNumber = Number(limit);
 
     if (isNaN(limitNumber)) {
@@ -39,18 +53,18 @@ export const getLetters: APIGatewayProxyHandler = async (event) => {
       });
       return {
         statusCode: 400,
-        body: "Bad Request: limit parameter is not a number",
+        body: "Invalid Request: limit parameter must be a positive number not greater than 2500",
       };
     }
 
-    if (limitNumber < 0) {
+    if (limitNumber < 0 || limitNumber > 2500) {
       log.info({
-        description: "limit parameter is not positive",
+        description: "Limit value is invalid",
         limit,
       });
       return {
         statusCode: 400,
-        body: "Bad Request: limit parameter is not positive",
+        body: "Invalid Request: limit parameter must be a positive number not greater than 2500",
       };
     }
 

specification/api/components/endpoints/listLetters.yml

@@ -11,6 +11,8 @@ description: The key use of this endpoint is to query letters which are ready to
 responses:
   '200':
     $ref: "../responses/getLetters200.yml"
+  '400':
+    $ref: "../responses/errors/listLetters/listLettersBadRequest.yml"
   '404':
     $ref: "../responses/errors/resourceNotFound.yml"
   '429':

specification/api/components/examples/errors/responses/listLetters/limitInvalidValue.json

@@ -0,0 +1,14 @@
+{
+  "errors": [
+    {
+      "code": "NOTIFY_INVALID_REQUEST",
+      "detail": "Invalid Request: limit parameter must be a positive number not greater than 2500",
+      "id": "rrt-1931948104716186917-c-geu2-10664-3111479-3.0",
+      "links": {
+        "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier"
+      },
+      "status": "400",
+      "title": "Invalid request"
+    }
+  ]
+}

specification/api/components/examples/errors/responses/listLetters/unknownParameter.json

@@ -0,0 +1,14 @@
+{
+  "errors": [
+    {
+      "code": "NOTIFY_INVALID_REQUEST",
+      "detail": "Invalid Request: Only 'limit' query parameter is supported",
+      "id": "rrt-1931948104716186917-c-geu2-10664-3111479-3.0",
+      "links": {
+        "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier"
+      },
+      "status": "400",
+      "title": "Invalid request"
+    }
+  ]
+}

specification/api/components/parameters/limit.yml

@@ -1,7 +1,7 @@
 name: limit
 in: query
 description: |-
-  The maximum number of items to return in a single request
+  The maximum number of items to return in a single request, max 2500
 schema:
   type: number
-  example: 100
+  example: 2500