From 4899f07b5538d4a198037c8a71f8f4a63737237d Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 19 Oct 2021 16:42:48 -0400 Subject: [PATCH 01/11] Add version increment to `update` This will set the stage for implementing optimistic locking via conditional expressions. --- src/index.test.ts | 73 ++++++++++++++++++++++++++++++ src/index.ts | 37 +++++++++++++--- test/helpers/TestContext.ts | 5 ++- test/update.locking.test.ts | 88 +++++++++++++++++++++++++++++++++++++ test/update.test.ts | 3 +- 5 files changed, 198 insertions(+), 8 deletions(-) create mode 100644 test/update.locking.test.ts diff --git a/src/index.test.ts b/src/index.test.ts index 9073ec1..6c85cb8 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -559,6 +559,79 @@ test('#generateUpdateParams should generate both update and remove params for do } }); +test('#generateUpdateParams should increment the version number', () => { + { + const options = { + tableName: 'blah2', + key: { + HashKey: 'abc', + }, + data: { + a: 123, + b: 'abc', + c: undefined, + lockVersion: 1, + }, + optimisticLockVersionAttribute: 'lockVersion', + }; + + expect(generateUpdateParams(options)).toEqual({ + TableName: options.tableName, + Key: options.key, + ReturnValues: 'ALL_NEW', + UpdateExpression: + 'add #lockVersion :lockVersionInc set #a0 = :a0, #a1 = :a1 remove #a2', + ExpressionAttributeNames: { + '#a0': 'a', + '#a1': 'b', + '#a2': 'c', + '#lockVersion': 'lockVersion', + }, + ExpressionAttributeValues: { + ':a0': options.data.a, + ':a1': options.data.b, + ':lockVersionInc': 1, + }, + }); + } +}); + +test('#generateUpdateParams should increment the version number even when not supplied', () => { + { + const options = { + tableName: 'blah3', + key: { + HashKey: 'abc', + }, + data: { + a: 123, + b: 'abc', + c: undefined, + }, + optimisticLockVersionAttribute: 'lockVersion', + }; + + expect(generateUpdateParams(options)).toEqual({ + TableName: options.tableName, + Key: options.key, + ReturnValues: 'ALL_NEW', + UpdateExpression: + 'add #lockVersion :lockVersionInc set #a0 = :a0, #a1 = :a1 remove #a2', + ExpressionAttributeNames: { + '#a0': 'a', + '#a1': 'b', + '#a2': 'c', + '#lockVersion': 'lockVersion', + }, + ExpressionAttributeValues: { + ':a0': options.data.a, + ':a1': options.data.b, + ':lockVersionInc': 1, + }, + }); + } +}); + test(`#queryUntilLimitReached should call #query if "filterExpression" not provided`, async () => { const keyConditionExpression = 'id = :id'; const attributeValues = { id: uuid() }; diff --git a/src/index.ts b/src/index.ts index 674ddb7..3dbcd88 100644 --- a/src/index.ts +++ b/src/index.ts @@ -120,7 +120,12 @@ export interface ConditionalOptions { attributeValues?: AttributeValues; } -export type PutOptions = ConditionalOptions; +export interface SaveBehavior { + optimisticLockVersionAttribute?: string; + optimisticLockVersionIncrement?: number; +} + +export type PutOptions = ConditionalOptions & SaveBehavior; export type UpdateOptions = ConditionalOptions; export type DeleteOptions = ConditionalOptions; @@ -131,9 +136,10 @@ export interface GenerateUpdateParamsInput extends UpdateOptions { } export function generateUpdateParams( - options: GenerateUpdateParamsInput + options: GenerateUpdateParamsInput & SaveBehavior ): DocumentClient.UpdateItemInput { const setExpressions: string[] = []; + const addExpressions: string[] = []; const removeExpressions: string[] = []; const expressionAttributeNameMap: AttributeNames = {}; const expressionAttributeValueMap: AttributeValues = {}; @@ -145,12 +151,25 @@ export function generateUpdateParams( conditionExpression, attributeNames, attributeValues, + optimisticLockVersionAttribute: versionAttribute, + optimisticLockVersionIncrement: versionInc, } = options; + if (versionAttribute) { + addExpressions.push(`#${versionAttribute} :${versionAttribute}Inc`); + expressionAttributeNameMap[`#${versionAttribute}`] = versionAttribute; + expressionAttributeValueMap[`:${versionAttribute}Inc`] = versionInc ?? 1; + } + const keys = Object.keys(options.data).sort(); for (let i = 0; i < keys.length; i++) { const name = keys[i]; + if (name === versionAttribute) { + // versionAttribute is a special case and should always be handled + // explicitly as above with the supplied value ignored + continue; + } const valueName = `:a${i}`; const attributeName = `#a${i}`; @@ -178,11 +197,13 @@ export function generateUpdateParams( ? 'remove ' + removeExpressions.join(', ') : undefined; + const addString = + addExpressions.length > 0 ? 'add ' + addExpressions.join(', ') : undefined; return { TableName: tableName, Key: key, ConditionExpression: conditionExpression, - UpdateExpression: [setString, removeString] + UpdateExpression: [addString, setString, removeString] .filter((val) => val !== undefined) .join(' '), ExpressionAttributeNames: { @@ -197,9 +218,10 @@ export function generateUpdateParams( }; } -interface DynamoDbDaoInput { +interface DynamoDbDaoInput { tableName: string; documentClient: DocumentClient; + optimisticLockingAttribute?: keyof NumberPropertiesInType; } function invalidCursorError(cursor: string): Error { @@ -261,10 +283,12 @@ export type NumberPropertiesInType = Pick< export default class DynamoDbDao { public readonly tableName: string; public readonly documentClient: DocumentClient; + public readonly optimisticLockingAttribute?: keyof NumberPropertiesInType; - constructor(options: DynamoDbDaoInput) { + constructor(options: DynamoDbDaoInput) { this.tableName = options.tableName; this.documentClient = options.documentClient; + this.optimisticLockingAttribute = options.optimisticLockingAttribute; } /** @@ -337,6 +361,9 @@ export default class DynamoDbDao { key, data, ...updateOptions, + optimisticLockVersionAttribute: this.optimisticLockingAttribute + ? this.optimisticLockingAttribute.toString() + : undefined, }); const { Attributes: attributes } = await this.documentClient .update(params) diff --git a/test/helpers/TestContext.ts b/test/helpers/TestContext.ts index 97f492a..e710287 100644 --- a/test/helpers/TestContext.ts +++ b/test/helpers/TestContext.ts @@ -43,7 +43,9 @@ export default class TestContext { this.dao = dao; } - static async setup(): Promise { + static async setup( + useOptimisticLocking: boolean = false + ): Promise { const tableName = uuid(); const indexName = uuid(); @@ -98,6 +100,7 @@ export default class TestContext { const dao = new DynamoDbDao({ tableName, documentClient, + optimisticLockingAttribute: useOptimisticLocking ? 'version' : undefined, }); return new TestContext(tableName, indexName, dao); diff --git a/test/update.locking.test.ts b/test/update.locking.test.ts new file mode 100644 index 0000000..23bb381 --- /dev/null +++ b/test/update.locking.test.ts @@ -0,0 +1,88 @@ +import { v4 as uuid } from 'uuid'; +import TestContext, { documentClient } from './helpers/TestContext'; + +let context: TestContext; + +beforeAll(async () => { + context = await TestContext.setup(true); +}); + +afterAll(() => { + if (context) { + return context.teardown(); + } +}); + +let key: any; +let item: any; + +beforeEach(async () => { + const { tableName } = context; + + key = { id: uuid() }; + + const input = { + ...key, + test: uuid(), + }; + + // put data into dynamodb + await documentClient + .put({ + TableName: tableName, + Item: input, + }) + .promise(); + + // ensure it exists + const { Item: storedItem } = await documentClient + .get({ + TableName: tableName, + Key: key, + }) + .promise(); + + // eslint-disable-next-line jest/no-standalone-expect + expect(storedItem).toEqual(input); + item = storedItem; +}); + +test('should set the version number to 1 on first update', async () => { + const { tableName, dao } = context; + const updateData = { test: uuid(), newField: uuid() }; + await dao.update(key, updateData); + + const { Item: updatedItem } = await documentClient + .get({ + TableName: tableName, + Key: key, + }) + .promise(); + + expect(updatedItem).toEqual({ + ...item, + ...updateData, + version: 1, + }); +}); + +test('should increment the version number by 1 on subsequent updates', async () => { + const { tableName, dao } = context; + const updateData = { test: uuid(), newField: uuid() }; + await dao.update(key, updateData); + await dao.update(key, updateData); + await dao.update(key, updateData); + + const { Item: updatedItem } = await documentClient + .get({ + TableName: tableName, + Key: key, + }) + .promise(); + + expect(updatedItem).toEqual({ + ...item, + ...updateData, + version: 3, + }); +}); diff --git a/test/update.test.ts b/test/update.test.ts index df7c24f..f2e3d41 100644 --- a/test/update.test.ts +++ b/test/update.test.ts @@ -1,7 +1,6 @@ -import TestContext, { documentClient } from './helpers/TestContext'; import { v4 as uuid } from 'uuid'; - import reservedWords from './fixtures/reservedWords'; +import TestContext, { documentClient } from './helpers/TestContext'; let context: TestContext; From 53b00203c31e502cfd2acb0a3d7a071722b17f20 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 19 Oct 2021 17:13:27 -0400 Subject: [PATCH 02/11] Implement optimistic lock via version number This enforces the lock (except where ignored) --- src/index.test.ts | 3 +++ src/index.ts | 23 +++++++++++++++++++--- test/update.locking.test.ts | 38 ++++++++++++++++++++++++++++++++++++- 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index 6c85cb8..190330b 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -579,6 +579,7 @@ test('#generateUpdateParams should increment the version number', () => { TableName: options.tableName, Key: options.key, ReturnValues: 'ALL_NEW', + ConditionExpression: '#lockVersion = :expectedlockVersion', UpdateExpression: 'add #lockVersion :lockVersionInc set #a0 = :a0, #a1 = :a1 remove #a2', ExpressionAttributeNames: { @@ -591,6 +592,7 @@ test('#generateUpdateParams should increment the version number', () => { ':a0': options.data.a, ':a1': options.data.b, ':lockVersionInc': 1, + ':expectedlockVersion': 1, }, }); } @@ -617,6 +619,7 @@ test('#generateUpdateParams should increment the version number even when not su ReturnValues: 'ALL_NEW', UpdateExpression: 'add #lockVersion :lockVersionInc set #a0 = :a0, #a1 = :a1 remove #a2', + ConditionExpression: 'attribute_not_exists(lockVersion)', ExpressionAttributeNames: { '#a0': 'a', '#a1': 'b', diff --git a/src/index.ts b/src/index.ts index 3dbcd88..134b27f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -125,8 +125,12 @@ export interface SaveBehavior { optimisticLockVersionIncrement?: number; } -export type PutOptions = ConditionalOptions & SaveBehavior; -export type UpdateOptions = ConditionalOptions; +export interface PublicSaveBehavior { + ignoreOptimisticLocking?: boolean; +} + +export type PutOptions = ConditionalOptions; +export type UpdateOptions = ConditionalOptions & PublicSaveBehavior; export type DeleteOptions = ConditionalOptions; export interface GenerateUpdateParamsInput extends UpdateOptions { @@ -148,17 +152,30 @@ export function generateUpdateParams( tableName, key, data, - conditionExpression, attributeNames, attributeValues, optimisticLockVersionAttribute: versionAttribute, optimisticLockVersionIncrement: versionInc, + ignoreOptimisticLocking: ignoreLocking = false, } = options; + let conditionExpression = options.conditionExpression; + if (versionAttribute) { addExpressions.push(`#${versionAttribute} :${versionAttribute}Inc`); expressionAttributeNameMap[`#${versionAttribute}`] = versionAttribute; expressionAttributeValueMap[`:${versionAttribute}Inc`] = versionInc ?? 1; + + if (!ignoreLocking) { + const lockExpression = data[versionAttribute] + ? `#${versionAttribute} = :expected${versionAttribute}` + : `attribute_not_exists(${versionAttribute})`; + expressionAttributeValueMap[`:expected${versionAttribute}`] = + data[versionAttribute]; + conditionExpression = conditionExpression + ? `(${conditionExpression}) AND ${lockExpression}` + : lockExpression; + } } const keys = Object.keys(options.data).sort(); diff --git a/test/update.locking.test.ts b/test/update.locking.test.ts index 23bb381..37953a6 100644 --- a/test/update.locking.test.ts +++ b/test/update.locking.test.ts @@ -70,8 +70,44 @@ test('should increment the version number by 1 on subsequent updates', async () const { tableName, dao } = context; const updateData = { test: uuid(), newField: uuid() }; await dao.update(key, updateData); + await dao.update(key, { ...updateData, version: 1 }); + await dao.update(key, { ...updateData, version: 2 }); + + const { Item: updatedItem } = await documentClient + .get({ + TableName: tableName, + Key: key, + }) + .promise(); + + expect(updatedItem).toEqual({ + ...item, + ...updateData, + version: 3, + }); +}); + +test('should error if update does not supply the correct version number', async () => { + const { tableName, dao } = context; + const updateData = { test: uuid(), newField: uuid() }; + // sets the initial version to 1 await dao.update(key, updateData); + + await expect(async () => { + // doesn't supply a version, throws error + await dao.update(key, updateData); + }).rejects.toThrow('The conditional request failed'); +}); + +test('should allow update without correct version number if ignore flag is set', async () => { + const { tableName, dao } = context; + const updateData = { test: uuid(), newField: uuid() }; + // sets the initial version to 1 await dao.update(key, updateData); + // Still increments the version + await dao.update(key, updateData, { + ignoreOptimisticLocking: true, + }); const { Item: updatedItem } = await documentClient .get({ @@ -83,6 +119,6 @@ test('should increment the version number by 1 on subsequent updates', async () expect(updatedItem).toEqual({ ...item, ...updateData, - version: 3, + version: 2, }); }); From bd112833959cb4a8817aff895e24ff7b1442722b Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 19 Oct 2021 18:05:35 -0400 Subject: [PATCH 03/11] Add delete support for optimistic locking --- src/index.test.ts | 4 +- src/index.ts | 66 ++++++++++++++++++++------ test/delete.locking.test.ts | 95 +++++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 test/delete.locking.test.ts diff --git a/src/index.test.ts b/src/index.test.ts index 190330b..a02cadd 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -579,7 +579,7 @@ test('#generateUpdateParams should increment the version number', () => { TableName: options.tableName, Key: options.key, ReturnValues: 'ALL_NEW', - ConditionExpression: '#lockVersion = :expectedlockVersion', + ConditionExpression: '#lockVersion = :lockVersion', UpdateExpression: 'add #lockVersion :lockVersionInc set #a0 = :a0, #a1 = :a1 remove #a2', ExpressionAttributeNames: { @@ -592,7 +592,7 @@ test('#generateUpdateParams should increment the version number', () => { ':a0': options.data.a, ':a1': options.data.b, ':lockVersionInc': 1, - ':expectedlockVersion': 1, + ':lockVersion': 1, }, }); } diff --git a/src/index.ts b/src/index.ts index 134b27f..8af0722 100644 --- a/src/index.ts +++ b/src/index.ts @@ -125,14 +125,32 @@ export interface SaveBehavior { optimisticLockVersionIncrement?: number; } -export interface PublicSaveBehavior { +export interface MutateBehavior { ignoreOptimisticLocking?: boolean; } export type PutOptions = ConditionalOptions; -export type UpdateOptions = ConditionalOptions & PublicSaveBehavior; -export type DeleteOptions = ConditionalOptions; +export type UpdateOptions = ConditionalOptions & MutateBehavior; +export type DeleteOptions = ConditionalOptions & MutateBehavior; +export interface AppendOptimisticLockConditionInput { + versionAttribute: string; + versionAttributeValue: any; + conditionExpression: string; +} + +export function appendOptimisticLockCondition({ + versionAttribute, + versionAttributeValue, + conditionExpression, +}: AppendOptimisticLockConditionInput) { + const lockExpression = versionAttributeValue + ? `#${versionAttribute} = :${versionAttribute}` + : `attribute_not_exists(${versionAttribute})`; + return conditionExpression + ? `(${conditionExpression}) AND ${lockExpression}` + : lockExpression; +} export interface GenerateUpdateParamsInput extends UpdateOptions { tableName: string; key: any; @@ -167,14 +185,13 @@ export function generateUpdateParams( expressionAttributeValueMap[`:${versionAttribute}Inc`] = versionInc ?? 1; if (!ignoreLocking) { - const lockExpression = data[versionAttribute] - ? `#${versionAttribute} = :expected${versionAttribute}` - : `attribute_not_exists(${versionAttribute})`; - expressionAttributeValueMap[`:expected${versionAttribute}`] = + conditionExpression = appendOptimisticLockCondition({ + versionAttribute, + versionAttributeValue: data[versionAttribute], + conditionExpression, + }); + expressionAttributeValueMap[`:${versionAttribute}`] = data[versionAttribute]; - conditionExpression = conditionExpression - ? `(${conditionExpression}) AND ${lockExpression}` - : lockExpression; } } @@ -333,16 +350,37 @@ export default class DynamoDbDao { */ async delete( key: KeySchema, - options: DeleteOptions = {} + options: DeleteOptions = {}, + data: Partial = {} ): Promise { + let { attributeNames, attributeValues, conditionExpression } = options; + + if (this.optimisticLockingAttribute && !options.ignoreOptimisticLocking) { + const versionAttribute = this.optimisticLockingAttribute.toString(); + conditionExpression = appendOptimisticLockCondition({ + versionAttribute, + versionAttributeValue: data[versionAttribute], + conditionExpression: conditionExpression, + }); + if (data[versionAttribute]) { + attributeNames = { + ...attributeNames, + [`#${versionAttribute}`]: versionAttribute, + }; + attributeValues = { + ...attributeValues, + [`:${versionAttribute}`]: data[versionAttribute], + }; + } + } const { Attributes: attributes } = await this.documentClient .delete({ TableName: this.tableName, Key: key, ReturnValues: 'ALL_OLD', - ConditionExpression: options.conditionExpression, - ExpressionAttributeNames: options.attributeNames, - ExpressionAttributeValues: options.attributeValues, + ConditionExpression: conditionExpression, + ExpressionAttributeNames: attributeNames, + ExpressionAttributeValues: attributeValues, }) .promise(); diff --git a/test/delete.locking.test.ts b/test/delete.locking.test.ts new file mode 100644 index 0000000..0a42be8 --- /dev/null +++ b/test/delete.locking.test.ts @@ -0,0 +1,95 @@ +import { v4 as uuid } from 'uuid'; +import TestContext from './helpers/TestContext'; + +let context: TestContext; + +beforeAll(async () => { + context = await TestContext.setup(true); +}); + +afterAll(() => { + if (context) { + return context.teardown(); + } +}); + +test('should require version number to remove item from the table', async () => { + const { dao } = context; + + const key = { id: uuid() }; + const updateData = { test: uuid(), newField: uuid() }; + + // put data into dynamodb, which should set the version number + await dao.update(key, updateData); + await dao.update(key, { ...updateData, version: 1 }); + + // ensure it exists + const item = await dao.get(key); + expect(item).toEqual({ + ...key, + ...updateData, + version: 2, + }); + + await dao.delete(key, undefined, { + version: 2, + }); + + expect(await dao.get(key)).toBeUndefined(); +}); + +test('should error when version number is missing when removing item from the table', async () => { + const { tableName, dao } = context; + + const key = { id: uuid() }; + const updateData = { test: uuid(), newField: uuid() }; + + // put data into dynamodb, which should set the version number + await dao.update(key, updateData); + await dao.update(key, { ...updateData, version: 1 }); + + await expect(async () => { + await dao.delete(key); + }).rejects.toThrow('The conditional request failed'); +}); + +test('should error when version number is old when removing item from the table', async () => { + const { tableName, dao } = context; + + const key = { id: uuid() }; + const updateData = { test: uuid(), newField: uuid() }; + + // put data into dynamodb, which should set the version number + await dao.update(key, updateData); + await dao.update(key, { ...updateData, version: 1 }); + + await expect(async () => { + await dao.delete(key, undefined, { + version: 1, + }); + }).rejects.toThrow('The conditional request failed'); +}); + +test('should not require version number to remove item from the table when ignore flag is set', async () => { + const { tableName, dao } = context; + + const key = { id: uuid() }; + const updateData = { test: uuid(), newField: uuid() }; + + // put data into dynamodb, which should set the version number + await dao.update(key, updateData); + + // ensure it exists + const item = await dao.get(key); + expect(item).toEqual({ + ...key, + ...updateData, + version: 1, + }); + + await dao.delete(key, { ignoreOptimisticLocking: true }); + + // ensure it deleted + const deletedItem = await dao.get(key); + expect(deletedItem).toEqual(undefined); +}); From 225ba122d6ae2e65bfd5917bb50bc01c6ea7afd2 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 19 Oct 2021 23:45:02 -0400 Subject: [PATCH 04/11] Add support for optimistic locking with `put` --- src/index.ts | 36 ++++++++-- test/put.locking.test.ts | 146 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 4 deletions(-) create mode 100644 test/put.locking.test.ts diff --git a/src/index.ts b/src/index.ts index 8af0722..e7efc38 100644 --- a/src/index.ts +++ b/src/index.ts @@ -129,7 +129,7 @@ export interface MutateBehavior { ignoreOptimisticLocking?: boolean; } -export type PutOptions = ConditionalOptions; +export type PutOptions = ConditionalOptions & MutateBehavior; export type UpdateOptions = ConditionalOptions & MutateBehavior; export type DeleteOptions = ConditionalOptions & MutateBehavior; @@ -391,13 +391,40 @@ export default class DynamoDbDao { * Creates/Updates an item in the table */ async put(data: DataModel, options: PutOptions = {}): Promise { + let { conditionExpression, attributeNames, attributeValues } = options; + if (this.optimisticLockingAttribute) { + const versionAttribute = this.optimisticLockingAttribute.toString(); + + if (!options.ignoreOptimisticLocking) { + conditionExpression = appendOptimisticLockCondition({ + versionAttribute, + versionAttributeValue: data[versionAttribute], + conditionExpression, + }); + if (data[versionAttribute]) { + attributeNames = { + ...attributeNames, + [`#${versionAttribute}`]: versionAttribute, + }; + attributeValues = { + ...attributeValues, + [`:${versionAttribute}`]: data[versionAttribute], + }; + } + } + + data[versionAttribute] = data[versionAttribute] + ? data[versionAttribute] + 1 + : 1; + } + await this.documentClient .put({ TableName: this.tableName, Item: data, - ConditionExpression: options.conditionExpression, - ExpressionAttributeNames: options.attributeNames, - ExpressionAttributeValues: options.attributeValues, + ConditionExpression: conditionExpression, + ExpressionAttributeNames: attributeNames, + ExpressionAttributeValues: attributeValues, }) .promise(); return data; @@ -705,6 +732,7 @@ export default class DynamoDbDao { .batchWrite({ RequestItems: { [this.tableName]: operations.map((operation) => { + // TODO: optionally add the opt lock here if (isBatchPutOperation(operation)) { return { PutRequest: { diff --git a/test/put.locking.test.ts b/test/put.locking.test.ts new file mode 100644 index 0000000..f066709 --- /dev/null +++ b/test/put.locking.test.ts @@ -0,0 +1,146 @@ +import { v4 as uuid } from 'uuid'; +import TestContext, { documentClient } from './helpers/TestContext'; + +let context: TestContext; + +beforeAll(async () => { + context = await TestContext.setup(true); +}); + +afterAll(() => { + if (context) { + return context.teardown(); + } +}); + +test('should add version number on first put', async () => { + const { tableName, dao } = context; + + const key = { id: uuid() }; + + const input = { + ...key, + test: uuid(), + }; + + // put data into dynamodb + await dao.put(input); + + // ensure it exists + const { Item: item } = await documentClient + .get({ + TableName: tableName, + Key: key, + }) + .promise(); + + expect(item).toEqual({ + ...input, + version: 1, + }); +}); + +test('should throw error if version number is not supplied on second update', async () => { + const { tableName, dao } = context; + + const key = { id: uuid() }; + + const input = { + ...key, + test: uuid(), + }; + + // put data into dynamodb + await dao.put(input); + + expect(1).toEqual(1); + + // ensure it exists + const { Item: item } = await documentClient + .get({ + TableName: tableName, + Key: key, + }) + .promise(); + + expect(item).toEqual({ + ...input, + version: 1, + }); + + await expect(async () => { + await dao.put({ + ...key, + test: uuid(), + }); + }).rejects.toThrow('The conditional request failed'); +}); + +test('should allow multiple puts if version number is incremented', async () => { + const { tableName, dao } = context; + + const key = { id: uuid() }; + + const input = { + ...key, + test: uuid(), + }; + + // put data into dynamodb + await dao.put(input); + await dao.put({ + ...input, + version: 1, + }); + + // ensure it exists + const { Item: item } = await documentClient + .get({ + TableName: tableName, + Key: key, + }) + .promise(); + + expect(item).toEqual({ + ...input, + version: 2, + }); +}); + +test('should allow multiple puts if version number is incremented when multiple conditions exist', async () => { + const { tableName, dao } = context; + + const key = { id: uuid() }; + + const input = { + ...key, + test: uuid(), + }; + + // put data into dynamodb + await dao.put(input); + await dao.put( + { + ...input, + version: 1, + }, + { + attributeNames: { '#test': 'test' }, + attributeValues: { ':test': input.test, ':test2': '2' }, + conditionExpression: '#test = :test or #test = :test2', + } + ); + + // ensure it exists + const { Item: item } = await documentClient + .get({ + TableName: tableName, + Key: key, + }) + .promise(); + + expect(item).toEqual({ + ...input, + version: 2, + }); +}); From 0061c0957583655e1791b914c6a480d99299a46b Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 20 Oct 2021 00:01:58 -0400 Subject: [PATCH 05/11] Refactor conditional lock expression Extract and reduce duplication --- src/index.ts | 85 +++++++++++++++++++++++++++------------------------- 1 file changed, 45 insertions(+), 40 deletions(-) diff --git a/src/index.ts b/src/index.ts index e7efc38..69a2655 100644 --- a/src/index.ts +++ b/src/index.ts @@ -133,24 +133,43 @@ export type PutOptions = ConditionalOptions & MutateBehavior; export type UpdateOptions = ConditionalOptions & MutateBehavior; export type DeleteOptions = ConditionalOptions & MutateBehavior; -export interface AppendOptimisticLockConditionInput { +export interface BuildOptimisticLockOptionsInput extends ConditionalOptions { versionAttribute: string; versionAttributeValue: any; - conditionExpression: string; } -export function appendOptimisticLockCondition({ - versionAttribute, - versionAttributeValue, - conditionExpression, -}: AppendOptimisticLockConditionInput) { +export function buildOptimisticLockOptions( + options: BuildOptimisticLockOptionsInput +): ConditionalOptions { + const { versionAttribute, versionAttributeValue } = options; + let { conditionExpression, attributeNames, attributeValues } = options; + const lockExpression = versionAttributeValue ? `#${versionAttribute} = :${versionAttribute}` : `attribute_not_exists(${versionAttribute})`; - return conditionExpression + + conditionExpression = conditionExpression ? `(${conditionExpression}) AND ${lockExpression}` : lockExpression; + + if (versionAttributeValue) { + attributeNames = { + ...attributeNames, + [`#${versionAttribute}`]: versionAttribute, + }; + attributeValues = { + ...attributeValues, + [`:${versionAttribute}`]: versionAttributeValue, + }; + } + + return { + conditionExpression, + attributeNames, + attributeValues, + }; } + export interface GenerateUpdateParamsInput extends UpdateOptions { tableName: string; key: any; @@ -185,11 +204,11 @@ export function generateUpdateParams( expressionAttributeValueMap[`:${versionAttribute}Inc`] = versionInc ?? 1; if (!ignoreLocking) { - conditionExpression = appendOptimisticLockCondition({ + ({ conditionExpression } = buildOptimisticLockOptions({ versionAttribute, versionAttributeValue: data[versionAttribute], conditionExpression, - }); + })); expressionAttributeValueMap[`:${versionAttribute}`] = data[versionAttribute]; } @@ -357,21 +376,14 @@ export default class DynamoDbDao { if (this.optimisticLockingAttribute && !options.ignoreOptimisticLocking) { const versionAttribute = this.optimisticLockingAttribute.toString(); - conditionExpression = appendOptimisticLockCondition({ - versionAttribute, - versionAttributeValue: data[versionAttribute], - conditionExpression: conditionExpression, - }); - if (data[versionAttribute]) { - attributeNames = { - ...attributeNames, - [`#${versionAttribute}`]: versionAttribute, - }; - attributeValues = { - ...attributeValues, - [`:${versionAttribute}`]: data[versionAttribute], - }; - } + ({ attributeNames, attributeValues, conditionExpression } = + buildOptimisticLockOptions({ + versionAttribute, + versionAttributeValue: data[versionAttribute], + conditionExpression: conditionExpression, + attributeNames, + attributeValues, + })); } const { Attributes: attributes } = await this.documentClient .delete({ @@ -396,21 +408,14 @@ export default class DynamoDbDao { const versionAttribute = this.optimisticLockingAttribute.toString(); if (!options.ignoreOptimisticLocking) { - conditionExpression = appendOptimisticLockCondition({ - versionAttribute, - versionAttributeValue: data[versionAttribute], - conditionExpression, - }); - if (data[versionAttribute]) { - attributeNames = { - ...attributeNames, - [`#${versionAttribute}`]: versionAttribute, - }; - attributeValues = { - ...attributeValues, - [`:${versionAttribute}`]: data[versionAttribute], - }; - } + ({ conditionExpression, attributeNames, attributeValues } = + buildOptimisticLockOptions({ + versionAttribute, + versionAttributeValue: data[versionAttribute], + conditionExpression, + attributeNames, + attributeValues, + })); } data[versionAttribute] = data[versionAttribute] From 9ac66f8af5e39b89a1c09a33b00840e11a76feb7 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 20 Oct 2021 00:32:19 -0400 Subject: [PATCH 06/11] Cast DataModel as map Must cast data to avoid tripping the linter, otherwise, it'll complain about expression of type 'string' can't be used to index type 'unknown'. --- src/index.ts | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/index.ts b/src/index.ts index 69a2655..863c0d6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -170,6 +170,8 @@ export function buildOptimisticLockOptions( }; } +type DataModelAsMap = { [key: string]: any }; + export interface GenerateUpdateParamsInput extends UpdateOptions { tableName: string; key: any; @@ -206,11 +208,12 @@ export function generateUpdateParams( if (!ignoreLocking) { ({ conditionExpression } = buildOptimisticLockOptions({ versionAttribute, - versionAttributeValue: data[versionAttribute], + versionAttributeValue: (data as DataModelAsMap)[versionAttribute], conditionExpression, })); - expressionAttributeValueMap[`:${versionAttribute}`] = - data[versionAttribute]; + expressionAttributeValueMap[`:${versionAttribute}`] = ( + data as DataModelAsMap + )[versionAttribute]; } } @@ -379,7 +382,7 @@ export default class DynamoDbDao { ({ attributeNames, attributeValues, conditionExpression } = buildOptimisticLockOptions({ versionAttribute, - versionAttributeValue: data[versionAttribute], + versionAttributeValue: (data as DataModelAsMap)[versionAttribute], conditionExpression: conditionExpression, attributeNames, attributeValues, @@ -405,21 +408,24 @@ export default class DynamoDbDao { async put(data: DataModel, options: PutOptions = {}): Promise { let { conditionExpression, attributeNames, attributeValues } = options; if (this.optimisticLockingAttribute) { + // Must cast data to avoid tripping the linter, otherwise, it'll complain + // about expression of type 'string' can't be used to index type 'unknown' + const dataAsMap = data as DataModelAsMap; const versionAttribute = this.optimisticLockingAttribute.toString(); if (!options.ignoreOptimisticLocking) { ({ conditionExpression, attributeNames, attributeValues } = buildOptimisticLockOptions({ versionAttribute, - versionAttributeValue: data[versionAttribute], + versionAttributeValue: dataAsMap[versionAttribute], conditionExpression, attributeNames, attributeValues, })); } - data[versionAttribute] = data[versionAttribute] - ? data[versionAttribute] + 1 + dataAsMap[versionAttribute] = dataAsMap[versionAttribute] + ? dataAsMap[versionAttribute] + 1 : 1; } From c20ecd95e5a281753f8a5445585e7ac0a58df69b Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 20 Oct 2021 00:37:45 -0400 Subject: [PATCH 07/11] Remove unnecessary destructure properties --- test/delete.locking.test.ts | 6 +++--- test/update.locking.test.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/delete.locking.test.ts b/test/delete.locking.test.ts index 0a42be8..7e93590 100644 --- a/test/delete.locking.test.ts +++ b/test/delete.locking.test.ts @@ -39,7 +39,7 @@ test('should require version number to remove item from the table', async () => }); test('should error when version number is missing when removing item from the table', async () => { - const { tableName, dao } = context; + const { dao } = context; const key = { id: uuid() }; const updateData = { test: uuid(), newField: uuid() }; @@ -54,7 +54,7 @@ test('should error when version number is missing when removing item from the ta }); test('should error when version number is old when removing item from the table', async () => { - const { tableName, dao } = context; + const { dao } = context; const key = { id: uuid() }; const updateData = { test: uuid(), newField: uuid() }; @@ -71,7 +71,7 @@ test('should error when version number is old when removing item from the table' }); test('should not require version number to remove item from the table when ignore flag is set', async () => { - const { tableName, dao } = context; + const { dao } = context; const key = { id: uuid() }; const updateData = { test: uuid(), newField: uuid() }; diff --git a/test/update.locking.test.ts b/test/update.locking.test.ts index 37953a6..952b89c 100644 --- a/test/update.locking.test.ts +++ b/test/update.locking.test.ts @@ -88,7 +88,7 @@ test('should increment the version number by 1 on subsequent updates', async () }); test('should error if update does not supply the correct version number', async () => { - const { tableName, dao } = context; + const { dao } = context; const updateData = { test: uuid(), newField: uuid() }; // sets the initial version to 1 await dao.update(key, updateData); From 1f513df0f889b426713dd3e05c94085ec449e4e9 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 26 Oct 2021 12:54:56 -0400 Subject: [PATCH 08/11] Type tweaks Appeasing (coercing!) the linter --- src/index.ts | 7 +++---- test/helpers/TestContext.ts | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/index.ts b/src/index.ts index 863c0d6..352245a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -274,7 +274,7 @@ export function generateUpdateParams( }; } -interface DynamoDbDaoInput { +export interface DynamoDbDaoInput { tableName: string; documentClient: DocumentClient; optimisticLockingAttribute?: keyof NumberPropertiesInType; @@ -454,9 +454,8 @@ export default class DynamoDbDao { key, data, ...updateOptions, - optimisticLockVersionAttribute: this.optimisticLockingAttribute - ? this.optimisticLockingAttribute.toString() - : undefined, + optimisticLockVersionAttribute: + this.optimisticLockingAttribute?.toString(), }); const { Attributes: attributes } = await this.documentClient .update(params) diff --git a/test/helpers/TestContext.ts b/test/helpers/TestContext.ts index e710287..024a0c3 100644 --- a/test/helpers/TestContext.ts +++ b/test/helpers/TestContext.ts @@ -2,7 +2,7 @@ import { DynamoDB } from 'aws-sdk'; import { v4 as uuid } from 'uuid'; -import DynamoDbDao from '../../src'; +import DynamoDbDao, { DynamoDbDaoInput } from '../../src'; const { DYNAMODB_ENDPOINT = 'http://localhost:8000' } = process.env; @@ -101,7 +101,7 @@ export default class TestContext { tableName, documentClient, optimisticLockingAttribute: useOptimisticLocking ? 'version' : undefined, - }); + } as DynamoDbDaoInput); return new TestContext(tableName, indexName, dao); } From d40b46c769cbaee5c8bfc198bd5d088969373e3a Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 26 Oct 2021 13:15:22 -0400 Subject: [PATCH 09/11] Bumping package version We'll consider this a non-breaking change, since optimistic locking is completely optional --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 658f97c..a07e709 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@jupiterone/dynamodb-dao", - "version": "1.4.0", + "version": "1.5.0", "description": "DynamoDB Data Access Object (DAO) helper library", "main": "index.js", "types": "index.d.ts", From d14fbc4c348f99b12d001461cdaddd65600b9f22 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 26 Oct 2021 14:04:32 -0400 Subject: [PATCH 10/11] Add readme --- README.md | 26 ++++++++++++++++++++++++++ src/index.ts | 1 - 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d8b8ad2..041d5c0 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,32 @@ const { total } = await myDocumentDao.decr( ); ``` +**Optimistic Locking with Version Numbers** + +For callers who wish to enable an optimistic locking strategy there are two +available toggles: + +1. Provide the attribute you wish to be used to store the version number. This + will enable optimistic locking on the following operations: `put`, `update`, + and `delete`. + + Writes for documents that do not have a version number attribute will + initialize the version number to 1. All subsequent writes will need to + provide the current version number. If an out-of-date version number is + supplied, an error will be thrown. + +2. If you wish to ignore optimistic locking for a save operation, specify + `ignoreOptimisticLocking: true` on your `put`, `update`, or `delete`. + +NOTE: Optimistic locking is NOT supported for `batchWrite` or `batchPut` +operations. Consuming those APIs for data models that do have optimistic locking +enabled may clobber your version data and could produce undesirable effects for +other callers. + +This was modeled after the +[Java Dynamo client](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/DynamoDBMapper.OptimisticLocking.html) +implementation. + ## Developing The test setup requires that [docker-compose]() be installed. To run the tests, diff --git a/src/index.ts b/src/index.ts index 352245a..ac5918c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -742,7 +742,6 @@ export default class DynamoDbDao { .batchWrite({ RequestItems: { [this.tableName]: operations.map((operation) => { - // TODO: optionally add the opt lock here if (isBatchPutOperation(operation)) { return { PutRequest: { From 332e251af7de10ce10d41476c592bee477d3e69f Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Thu, 28 Oct 2021 09:16:01 -0400 Subject: [PATCH 11/11] Add further docs --- CHANGELOG.md | 6 ++++++ README.md | 13 ++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d914d23..894a829 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,3 +13,9 @@ and this project adheres to ### Added - Support `consistentRead` option on `get` API + +## 1.5.0 - 2021-10-27 + +### Added + +- Support optimistic locking for `put`, `update` and `delete` APIs diff --git a/README.md b/README.md index 041d5c0..1d1584b 100644 --- a/README.md +++ b/README.md @@ -151,8 +151,19 @@ available toggles: provide the current version number. If an out-of-date version number is supplied, an error will be thrown. + Example of Dao constructed with optimistic locking enabled. + + ``` + const dao = new DynamoDbDao({ + tableName, + documentClient, + optimisticLockingAttribute: 'version', + }); + ``` + 2. If you wish to ignore optimistic locking for a save operation, specify - `ignoreOptimisticLocking: true` on your `put`, `update`, or `delete`. + `ignoreOptimisticLocking: true` in the options on your `put`, `update`, or + `delete`. NOTE: Optimistic locking is NOT supported for `batchWrite` or `batchPut` operations. Consuming those APIs for data models that do have optimistic locking