From 8a02fb210f3d4cac96ec8bb7d87cfac8664163d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominique=20J=C3=A4ggi?= Date: Tue, 28 Nov 2023 17:37:41 +0100 Subject: [PATCH] feat: add guards and use spacecat-shared-utils --- package-lock.json | 10 ++- packages/spacecat-shared-dynamo/package.json | 3 +- .../src/modules/getItem.js | 11 ++-- .../src/modules/putItem.js | 6 +- .../src/modules/query.js | 3 + .../src/modules/removeItem.js | 11 ++-- .../src/utils/guards.js | 44 +++++++++++++ .../test/modules/getItem.test.js | 4 +- .../test/modules/putItem.test.js | 2 +- .../test/modules/query.test.js | 14 ++++- .../test/modules/removeItem.test.js | 4 +- .../test/utils/guards.test.js | 63 +++++++++++++++++++ 12 files changed, 148 insertions(+), 27 deletions(-) create mode 100644 packages/spacecat-shared-dynamo/src/utils/guards.js create mode 100644 packages/spacecat-shared-dynamo/test/utils/guards.test.js diff --git a/package-lock.json b/package-lock.json index e6344d0ba..bcc316398 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11873,10 +11873,18 @@ "version": "1.0.0", "license": "Apache-2.0", "dependencies": { + "@adobe/spacecat-shared-utils": "1.0.1", "@aws-sdk/client-dynamodb": "3.454.0", "@aws-sdk/lib-dynamodb": "3.454.0" }, - "devDependencies": {} + "devDependencies": { + "chai": "4.3.10" + } + }, + "packages/spacecat-shared-dynamo/node_modules/@adobe/spacecat-shared-utils": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-utils/-/spacecat-shared-utils-1.0.1.tgz", + "integrity": "sha512-6HiCywan5y9jpbFYkQX87Vg0q5dXoJf2m6MiDvNBEJdREdUxFwE+oYA6Fr86qqd32ECnCeYCFO13HcNRn1RxkQ==" }, "packages/spacecat-shared-example": { "name": "@adobe/spacecat-shared-example", diff --git a/packages/spacecat-shared-dynamo/package.json b/packages/spacecat-shared-dynamo/package.json index 520ec7622..cf2041a63 100644 --- a/packages/spacecat-shared-dynamo/package.json +++ b/packages/spacecat-shared-dynamo/package.json @@ -30,7 +30,8 @@ }, "dependencies": { "@aws-sdk/client-dynamodb": "3.454.0", - "@aws-sdk/lib-dynamodb": "3.454.0" + "@aws-sdk/lib-dynamodb": "3.454.0", + "@adobe/spacecat-shared-utils": "1.0.1" }, "devDependencies": { "chai": "4.3.10" diff --git a/packages/spacecat-shared-dynamo/src/modules/getItem.js b/packages/spacecat-shared-dynamo/src/modules/getItem.js index eaa64a3fd..5a4e24e8a 100644 --- a/packages/spacecat-shared-dynamo/src/modules/getItem.js +++ b/packages/spacecat-shared-dynamo/src/modules/getItem.js @@ -12,6 +12,8 @@ import { performance } from 'perf_hooks'; +import { guardKey, guardTableName } from '../utils/guards.js'; + /** * Retrieves an item from DynamoDB using a table name and key object. * @@ -23,13 +25,8 @@ import { performance } from 'perf_hooks'; * @throws {Error} Throws an error if the DynamoDB get operation fails or input validation fails. */ async function getItem(docClient, tableName, key, log = console) { - if (!tableName || typeof tableName !== 'string') { - throw new Error('Invalid tableName: must be a non-empty string.'); - } - - if (!key || typeof key !== 'object' || !key.partitionKey) { - throw new Error('Invalid key: must be an object with a partitionKey.'); - } + guardTableName(tableName); + guardKey(key); const params = { TableName: tableName, diff --git a/packages/spacecat-shared-dynamo/src/modules/putItem.js b/packages/spacecat-shared-dynamo/src/modules/putItem.js index 39fb28ea9..21cc9be6b 100644 --- a/packages/spacecat-shared-dynamo/src/modules/putItem.js +++ b/packages/spacecat-shared-dynamo/src/modules/putItem.js @@ -12,6 +12,8 @@ import { performance } from 'perf_hooks'; +import { guardTableName } from '../utils/guards.js'; + /** * Inserts or updates an item in a DynamoDB table. * @@ -23,9 +25,7 @@ import { performance } from 'perf_hooks'; * @throws {Error} Throws an error if the DynamoDB put operation fails. */ async function putItem(docClient, tableName, item, log = console) { - if (!tableName || typeof tableName !== 'string') { - throw new Error('Invalid tableName: must be a non-empty string.'); - } + guardTableName(tableName); const params = { TableName: tableName, diff --git a/packages/spacecat-shared-dynamo/src/modules/query.js b/packages/spacecat-shared-dynamo/src/modules/query.js index 0bec8a76d..6738b8cb8 100644 --- a/packages/spacecat-shared-dynamo/src/modules/query.js +++ b/packages/spacecat-shared-dynamo/src/modules/query.js @@ -11,6 +11,7 @@ */ import { performance } from 'perf_hooks'; +import { guardQueryParameters } from '../utils/guards.js'; /** * Queries DynamoDB and automatically handles pagination to retrieve all items. @@ -22,6 +23,8 @@ import { performance } from 'perf_hooks'; * @throws {Error} Throws an error if the DynamoDB query operation fails. */ async function query(docClient, originalParams, log = console) { + guardQueryParameters(originalParams); + let items = []; const params = { ...originalParams }; diff --git a/packages/spacecat-shared-dynamo/src/modules/removeItem.js b/packages/spacecat-shared-dynamo/src/modules/removeItem.js index a89a4ace2..2978278ca 100644 --- a/packages/spacecat-shared-dynamo/src/modules/removeItem.js +++ b/packages/spacecat-shared-dynamo/src/modules/removeItem.js @@ -12,6 +12,8 @@ import { performance } from 'perf_hooks'; +import { guardKey, guardTableName } from '../utils/guards.js'; + /** * Removes an item from a DynamoDB table. * @@ -23,13 +25,8 @@ import { performance } from 'perf_hooks'; * @throws {Error} Throws an error if the DynamoDB delete operation fails or input validation fails. */ async function removeItem(docClient, tableName, key, log = console) { - if (!tableName || typeof tableName !== 'string') { - throw new Error('Invalid tableName: must be a non-empty string.'); - } - - if (!key || typeof key !== 'object' || !key.partitionKey) { - throw new Error('Invalid key: must be an object with a partitionKey.'); - } + guardTableName(tableName); + guardKey(key); const params = { TableName: tableName, diff --git a/packages/spacecat-shared-dynamo/src/utils/guards.js b/packages/spacecat-shared-dynamo/src/utils/guards.js new file mode 100644 index 000000000..90668b1d2 --- /dev/null +++ b/packages/spacecat-shared-dynamo/src/utils/guards.js @@ -0,0 +1,44 @@ +/* + * Copyright 2023 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import { hasText, isObject } from '@adobe/spacecat-shared-utils'; + +const guardTableName = (tableName) => { + if (!hasText(tableName)) { + throw new Error('Table name is required.'); + } +}; + +const guardKey = (key) => { + if (!isObject(key) || !key.partitionKey) { + throw new Error('Key must be an object with a partitionKey.'); + } +}; + +const guardQueryParameters = (params) => { + if (!isObject(params)) { + throw new Error('Query parameters must be an object.'); + } + + const requiredProps = ['TableName', 'KeyConditionExpression', 'ExpressionAttributeValues']; + for (const prop of requiredProps) { + if (!Object.prototype.hasOwnProperty.call(params, prop)) { + throw new Error(`Query parameters is missing required parameter: ${prop}`); + } + } +}; + +export { + guardKey, + guardQueryParameters, + guardTableName, +}; diff --git a/packages/spacecat-shared-dynamo/test/modules/getItem.test.js b/packages/spacecat-shared-dynamo/test/modules/getItem.test.js index 16fdbd285..f89a131b3 100644 --- a/packages/spacecat-shared-dynamo/test/modules/getItem.test.js +++ b/packages/spacecat-shared-dynamo/test/modules/getItem.test.js @@ -45,7 +45,7 @@ describe('getItem', () => { await dynamoDbClient.getItem('', key); expect.fail('getItem did not throw with empty tableName'); } catch (error) { - expect(error.message).to.equal('Invalid tableName: must be a non-empty string.'); + expect(error.message).to.equal('Table name is required.'); } }); @@ -54,7 +54,7 @@ describe('getItem', () => { await dynamoDbClient.getItem('TestTable', null); expect.fail('getItem did not throw with invalid key'); } catch (error) { - expect(error.message).to.equal('Invalid key: must be an object with a partitionKey.'); + expect(error.message).to.equal('Key must be an object with a partitionKey.'); } }); diff --git a/packages/spacecat-shared-dynamo/test/modules/putItem.test.js b/packages/spacecat-shared-dynamo/test/modules/putItem.test.js index 2c32df75a..cbccbdcff 100644 --- a/packages/spacecat-shared-dynamo/test/modules/putItem.test.js +++ b/packages/spacecat-shared-dynamo/test/modules/putItem.test.js @@ -37,7 +37,7 @@ describe('putItem', () => { await dynamoDbClient.putItem('', { someKey: 'someValue' }); expect.fail('putItem did not throw with empty tableName'); } catch (error) { - expect(error.message).to.equal('Invalid tableName: must be a non-empty string.'); + expect(error.message).to.equal('Table name is required.'); } }); diff --git a/packages/spacecat-shared-dynamo/test/modules/query.test.js b/packages/spacecat-shared-dynamo/test/modules/query.test.js index 3424f47df..45c1e8854 100644 --- a/packages/spacecat-shared-dynamo/test/modules/query.test.js +++ b/packages/spacecat-shared-dynamo/test/modules/query.test.js @@ -16,6 +16,14 @@ import { expect } from 'chai'; import { createClient } from '../../src/index.js'; describe('query', () => { + const queryParams = { + TableName: 'TestTable', + KeyConditionExpression: 'partitionKey = :partitionKey', + ExpressionAttributeValues: { + ':partitionKey': 'testPartitionKey', + }, + }; + let dynamoDbClient; let mockDocClient; @@ -35,12 +43,12 @@ describe('query', () => { }); it('queries items from the database', async () => { - const result = await dynamoDbClient.query({ TableName: 'TestTable' }); + const result = await dynamoDbClient.query(queryParams); expect(result).to.be.an('array'); }); it('queries items from the database with pagination', async () => { - const result = await dynamoDbClient.query({ TableName: 'TestTable' }); + const result = await dynamoDbClient.query(queryParams); expect(result).to.have.lengthOf(3); expect(result).to.deep.equal(['item1', 'item2', 'item3']); }); @@ -51,7 +59,7 @@ describe('query', () => { }; try { - await dynamoDbClient.query({ TableName: 'TestTable' }); + await dynamoDbClient.query(queryParams); expect.fail('queryDb did not throw as expected'); } catch (error) { expect(error.message).to.equal('Query failed'); diff --git a/packages/spacecat-shared-dynamo/test/modules/removeItem.test.js b/packages/spacecat-shared-dynamo/test/modules/removeItem.test.js index f1c772f7d..37f94fd63 100644 --- a/packages/spacecat-shared-dynamo/test/modules/removeItem.test.js +++ b/packages/spacecat-shared-dynamo/test/modules/removeItem.test.js @@ -45,7 +45,7 @@ describe('removeItem', () => { await dynamoDbClient.removeItem('', key); expect.fail('removeItem did not throw with empty tableName'); } catch (error) { - expect(error.message).to.equal('Invalid tableName: must be a non-empty string.'); + expect(error.message).to.equal('Table name is required.'); } }); @@ -54,7 +54,7 @@ describe('removeItem', () => { await dynamoDbClient.removeItem('TestTable', null); expect.fail('removeItem did not throw with invalid key'); } catch (error) { - expect(error.message).to.equal('Invalid key: must be an object with a partitionKey.'); + expect(error.message).to.equal('Key must be an object with a partitionKey.'); } }); diff --git a/packages/spacecat-shared-dynamo/test/utils/guards.test.js b/packages/spacecat-shared-dynamo/test/utils/guards.test.js new file mode 100644 index 000000000..814b2c4db --- /dev/null +++ b/packages/spacecat-shared-dynamo/test/utils/guards.test.js @@ -0,0 +1,63 @@ +/* + * Copyright 2023 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +/* eslint-env mocha */ + +import { expect } from 'chai'; +import { guardKey, guardQueryParameters, guardTableName } from '../../src/utils/guards.js'; + +describe('Query Parameter Guards', () => { + // Test guardTableName + describe('guardTableName', () => { + it('should throw an error if tableName is empty', () => { + expect(() => guardTableName('')).to.throw('Table name is required.'); + }); + + it('should not throw an error for valid tableName', () => { + expect(() => guardTableName('validTableName')).to.not.throw(); + }); + }); + + // Test guardKey + describe('guardKey', () => { + it('should throw an error if key is not an object', () => { + expect(() => guardKey('notAnObject')).to.throw('Key must be an object with a partitionKey.'); + }); + + it('should throw an error if key does not have partitionKey', () => { + expect(() => guardKey({})).to.throw('Key must be an object with a partitionKey.'); + }); + + it('should not throw an error for a valid key', () => { + expect(() => guardKey({ partitionKey: 'value' })).to.not.throw(); + }); + }); + + describe('guardQueryParameters', () => { + it('should throw an error if params is not an object', () => { + expect(() => guardQueryParameters('notAnObject')).to.throw('Query parameters must be an object.'); + }); + + it('should throw an error if any required parameter is missing', () => { + expect(() => guardQueryParameters({ TableName: 'table' })).to.throw('Query parameters is missing required parameter: KeyConditionExpression'); + }); + + it('should not throw an error for valid params', () => { + const validParams = { + TableName: 'table', + KeyConditionExpression: 'expression', + ExpressionAttributeValues: {}, + }; + expect(() => guardQueryParameters(validParams)).to.not.throw(); + }); + }); +});