Skip to content

Commit

Permalink
Revert "Reduce likelyhood of overwriting changes during consecutive u…
Browse files Browse the repository at this point in the history
…pdates"
  • Loading branch information
mappies authored Nov 27, 2023
1 parent 6615066 commit 1bddcce
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 77 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "nodenamo",
"version": "1.7.4",
"version": "1.7.2",
"description": "A powerful ORM for DynamoDb",
"main": "dist/index.js",
"typings": "dist/index",
Expand Down
72 changes: 1 addition & 71 deletions spec/unit/dynamodbManagerUpdate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ describe('DynamoDbManager.Update()', function ()
let called:boolean;
let deleted:boolean;
let deleted2:boolean;
let desiredObjectCreatedFromStronglyConsistentRead:boolean;

beforeEach(()=>
{
Expand All @@ -32,11 +31,6 @@ describe('DynamoDbManager.Update()', function ()
called = false;
deleted = false;
deleted2 = false;
desiredObjectCreatedFromStronglyConsistentRead = false;
});

afterEach(()=>{
desiredObjectCreatedFromStronglyConsistentRead = true;
});

it('update() - no keys', async () =>
Expand All @@ -57,13 +51,13 @@ describe('DynamoDbManager.Update()', function ()
order:number;
};

setupStronglyConsistentRead({hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'});
let findResponse = getMockedQueryResponse({Items: <any>[{hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'}, {hash: 'entity#1', range: 'order', id:1, name:'Some Two', created:'created', order:'order'}]});
mockedClient.setup(q => q.query(It.is(p => !!p.TableName && p.IndexName === Const.IdIndexName && p.KeyConditionExpression === '#objid = :objid' && p.ExpressionAttributeNames['#objid'] === Const.IdColumn && p.ExpressionAttributeValues[':objid'] === 'entity#1'))).callback(()=>called=true).returns(()=>findResponse.object);

let manager = new DynamoDbManager(mockedClient.object);

await manager.update(Entity, 1, {name: 'New Two'}, undefined, mockedTransaction.object);

assert.isTrue(called);
});

Expand All @@ -85,7 +79,6 @@ describe('DynamoDbManager.Update()', function ()
order:number;
};

setupStronglyConsistentRead({hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'});
let findResponse = getMockedQueryResponse({Items: <any>[{hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'}, {hash: 'entity#1', range: 'order', id:1, name:'Some Two', created:'created', order:'order'}]});
mockedClient.setup(q => q.query(It.is(p => !!p.TableName && p.IndexName === Const.IdIndexName && p.KeyConditionExpression === '#objid = :objid' && p.ExpressionAttributeNames['#objid'] === Const.IdColumn && p.ExpressionAttributeValues[':objid'] === 'entity#1'))).callback(()=>called=true).returns(()=>findResponse.object);
mockedTransaction.setup(t => t.add(It.is(t => !!t.Put && !!t.Put.TableName && t.Put.Item.hash === 'entity#1' && t.Put.Item.range === 'new created' && t.Put.Item.name === 'New Two' && t.Put.Item.created === 'new created' && t.Put.Item.order === 'new order'))).callback(()=>put=true);
Expand Down Expand Up @@ -118,7 +111,6 @@ describe('DynamoDbManager.Update()', function ()
created:number;
};

setupStronglyConsistentRead({hash: 'entity#nodenamo', range: 'created#1', id:1, name:'Some One', created:'created', order:'order'});
let findResponse = getMockedQueryResponse({Items: <any>[
{hash: 'entity#nodenamo', range: 'created#1', id:1, name:'Some One', created:'created', order:'order'},
{hash: 'entity#Some One', range: 'created', id:1, name:'Some One', created:'created', order:'order'},
Expand Down Expand Up @@ -158,7 +150,6 @@ describe('DynamoDbManager.Update()', function ()
order:number;
};

setupStronglyConsistentRead({hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'});
let findResponse = getMockedQueryResponse({Items: <any>[{hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'}, {hash: 'entity#1', range: 'order', id:1, name:'Some Two', created:'created', order:'order'}]});
mockedClient.setup(q => q.query(It.is(p => !!p.TableName && p.IndexName === Const.IdIndexName && p.KeyConditionExpression === '#objid = :objid' && p.ExpressionAttributeNames['#objid'] === Const.IdColumn && p.ExpressionAttributeValues[':objid'] === 'entity#1'))).callback(()=>called=true).returns(()=>findResponse.object);
mockedTransaction.setup(t => t.add(It.is(t => !!t.Put && !!t.Put.TableName && t.Put.Item.hash === 'entity#1' && t.Put.Item.range === 'created' && !t.Put.ConditionExpression && t.Put.Item.name === 'New Two' && t.Put.Item.created === 'created' && t.Put.Item.order === 'order'))).callback(()=>put=true);
Expand All @@ -174,43 +165,6 @@ describe('DynamoDbManager.Update()', function ()
assert.isFalse(deleted);
});

it('update() - delta change, change from strongly consistent read reflected in transaction - when missing from getOneRows', async () =>
{
@DBTable()
class Entity
{
@DBColumn({hash:true})
id:number;

@DBColumn()
name:string;

@DBColumn({range:true})
created:number;

@DBColumn({range:true})
order:number;

@DBColumn()
recentlyUpdatedField:number;
};

setupStronglyConsistentRead({hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order', recentlyUpdatedField:1}); //recentlyUpdatedField exists in strongly consistent read
let findResponse = getMockedQueryResponse({Items: <any>[{hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'}, {hash: 'entity#1', range: 'order', id:1, name:'Some Two', created:'created', order:'order'}]});//recentlyUpdatedField does NOT exist in eventually consistent read
mockedClient.setup(q => q.query(It.is(p => !!p.TableName && p.IndexName === Const.IdIndexName && p.KeyConditionExpression === '#objid = :objid' && p.ExpressionAttributeNames['#objid'] === Const.IdColumn && p.ExpressionAttributeValues[':objid'] === 'entity#1'))).callback(()=>called=true).returns(()=>findResponse.object);
mockedTransaction.setup(t => t.add(It.is(t => !!t.Put && !!t.Put.TableName && t.Put.Item.hash === 'entity#1' && t.Put.Item.range === 'created' && !t.Put.ConditionExpression && t.Put.Item.name === 'New Two' && t.Put.Item.created === 'created' && t.Put.Item.order === 'order' && t.Put.Item.recentlyUpdatedField === 1))).callback(()=>put=true); //recentlyUpdatedField included in update payload
mockedTransaction.setup(t => t.add(It.is(t => !!t.Put && !!t.Put.TableName && t.Put.Item.hash === 'entity#1' && t.Put.Item.range === 'order' && !t.Put.ConditionExpression && t.Put.Item.name === 'New Two' && t.Put.Item.created === 'created' && t.Put.Item.order === 'order' && t.Put.Item.recentlyUpdatedField === 1))).callback(()=>put2=true);
mockedTransaction.setup(t => t.add(It.is(t => !!t.Delete))).callback(()=>deleted=true);

let manager = new DynamoDbManager(mockedClient.object);
await manager.update(Entity, 1, {name: 'New Two'}, undefined, mockedTransaction.object);

assert.isTrue(called);
assert.isTrue(put);
assert.isTrue(put2);
assert.isFalse(deleted);
});

it('update() - key changed', async () =>
{
@DBTable()
Expand All @@ -229,7 +183,6 @@ describe('DynamoDbManager.Update()', function ()
order:number;
};

setupStronglyConsistentRead({hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'});
let findResponse = getMockedQueryResponse({Items: <any>[{hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'}, {hash: 'entity#1', range: 'order', id:1, name:'Some Two', created:'created', order:'order'}]});
mockedClient.setup(q => q.query(It.is(p => !!p.TableName && p.IndexName === Const.IdIndexName && p.KeyConditionExpression === '#objid = :objid' && p.ExpressionAttributeNames['#objid'] === Const.IdColumn && p.ExpressionAttributeValues[':objid'] === 'entity#1'))).callback(()=>called=true).returns(()=>findResponse.object);
mockedTransaction.setup(t => t.add(It.is(t => !!t.Put && !!t.Put.TableName && t.Put.Item.hash === 'entity#1a' && t.Put.Item.range === 'created' && !!t.Put.ConditionExpression && t.Put.Item.id === '1a'))).callback(()=>put=true);
Expand Down Expand Up @@ -267,7 +220,6 @@ describe('DynamoDbManager.Update()', function ()

let condition = {conditionExpression: 'condition', expressionAttributeNames: {'#n': 'name'}, expressionAttributeValues: {':v': true}};

setupStronglyConsistentRead({hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'});
let findResponse = getMockedQueryResponse({Items: <any>[{hash: 'entity#1', range: 'created', id:1, name:'Some One', created:'created', order:'order'}, {hash: 'entity#1', range: 'order', id:1, name:'Some Two', created:'created', order:'order'}]});
mockedClient.setup(q => q.query(It.is(p => !!p.TableName && p.IndexName === Const.IdIndexName && p.KeyConditionExpression === '#objid = :objid' && p.ExpressionAttributeNames['#objid'] === Const.IdColumn && p.ExpressionAttributeValues[':objid'] === 'entity#1'))).callback(()=>called=true).returns(()=>findResponse.object);
mockedTransaction.setup(t => t.add(It.is(t => !!t.Put && !!t.Put.TableName && t.Put.Item.hash === 'entity#1' && t.Put.Item.range === 'created' && t.Put.Item.name === 'New Two' && t.Put.ConditionExpression === 'condition' && t.Put.ExpressionAttributeNames['#n'] === 'name' && t.Put.ExpressionAttributeValues[':v'] === 'true'))).callback(()=>put=true);
Expand Down Expand Up @@ -295,7 +247,6 @@ describe('DynamoDbManager.Update()', function ()
name:string;
};

setupStronglyConsistentRead({hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'});
let findResponse = getMockedQueryResponse({Items: <any>[
{hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'},
]});
Expand Down Expand Up @@ -327,7 +278,6 @@ describe('DynamoDbManager.Update()', function ()
name:string;
};

setupStronglyConsistentRead({hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'});
let findResponse = getMockedQueryResponse({Items: <any>[
{hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'},
]});
Expand Down Expand Up @@ -359,7 +309,6 @@ describe('DynamoDbManager.Update()', function ()
name:string;
};

setupStronglyConsistentRead({hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'});
let findResponse = getMockedQueryResponse({Items: <any>[
{hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'},
]});
Expand Down Expand Up @@ -391,7 +340,6 @@ describe('DynamoDbManager.Update()', function ()
name:string;
};

setupStronglyConsistentRead({hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'});
let findResponse = getMockedQueryResponse({Items: <any>[
{hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'},
]});
Expand Down Expand Up @@ -423,7 +371,6 @@ describe('DynamoDbManager.Update()', function ()
name:string;
};

setupStronglyConsistentRead({hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'});
let findResponse = getMockedQueryResponse({Items: <any>[
{hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'},
]});
Expand Down Expand Up @@ -515,7 +462,6 @@ describe('DynamoDbManager.Update()', function ()
name:string;
};

setupStronglyConsistentRead({id:1, name:'Some One'});
let findResponse = getMockedQueryResponse({Items: <any>[
{hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'},
]});
Expand Down Expand Up @@ -556,7 +502,6 @@ describe('DynamoDbManager.Update()', function ()
@DBColumn()
name:string;
};
setupStronglyConsistentRead({hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'});

let findResponse = getMockedQueryResponse({Items: <any>[
{hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'},
Expand Down Expand Up @@ -593,7 +538,6 @@ describe('DynamoDbManager.Update()', function ()
@DBColumn()
name:string;
};
setupStronglyConsistentRead({hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'});

let findResponse = getMockedQueryResponse({Items: <any>[
{hash: 'hash', range: 'range', id:1, objver: 1, name:'Some One'},
Expand All @@ -617,20 +561,6 @@ describe('DynamoDbManager.Update()', function ()
assert.isDefined(error);
assert.isTrue(error.message.includes('An object with the same ID or hash-range key already exists in \'Entity\' table'));
});

function setupStronglyConsistentRead(expectedItem:object)
{
let stronglyConsistentResponse = getMockedGetResponse(
{Item:<any>expectedItem}
);
mockedClient.setup(q => q.get(It.is(p =>
!!p.TableName
&& p.Key[Const.HashColumn] === 'entity#1'
&& p.Key[Const.RangeColumn] === 'nodenamo'
&& p.ConsistentRead === true)))
.callback(()=>desiredObjectCreatedFromStronglyConsistentRead=true)
.returns(()=>stronglyConsistentResponse.object);
}
});

function getMockedGetResponse(response:GetItemOutput): IMock<Request<GetItemOutput, AWSError>>
Expand Down
13 changes: 8 additions & 5 deletions src/managers/dynamodbManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,10 @@ export class DynamoDbManager implements IDynamoDbManager
[Const.RangeColumn]: attributeValues[':range']
},
ConsistentRead: stronglyConsistent
};
};

let response = await this.client.get(query).promise();

if(response.Item)
{
return EntityFactory.create(type, response.Item);
Expand Down Expand Up @@ -254,7 +256,7 @@ export class DynamoDbManager implements IDynamoDbManager
let versioningRequired = tableVersioning || (params && params.versionCheck);

//Calculate new representations
let [rows, idRow] = await Promise.all([this.getById(id, type),this.getOne(type,id,{stronglyConsistent:true})]);
let rows = await this.getById(id, type);

if(rows.length === 0)
{
Expand Down Expand Up @@ -282,8 +284,8 @@ export class DynamoDbManager implements IDynamoDbManager
//Must assign data to `instance` so it has @DBColumn() and @DBTable() metadata.
//EntityFactory is used here to convert an object with custom column names to an object with real property names/
//And because of an object created by EntityFactory.create() will not have Const.VersionColumn property, we have to re-add it here as well.
let desiredObject = Object.assign(instance, Object.assign(Object.assign({}, EntityFactory.create(type, idRow)), noUndefinedValuesObject));
desiredObject[Const.VersionColumn] = idRow[Const.VersionColumn];
let desiredObject = Object.assign(instance, Object.assign(Object.assign({}, EntityFactory.create(type, rows[0])), noUndefinedValuesObject));
desiredObject[Const.VersionColumn] = rows[0][Const.VersionColumn];

//If versionCheck, use the version from the original object.
//Else, RepresentationFactory.get() will increment the version from DB.
Expand Down Expand Up @@ -381,6 +383,7 @@ export class DynamoDbManager implements IDynamoDbManager
}

if(!autoCommit) return;

try
{
await transaction.commit();
Expand All @@ -393,7 +396,7 @@ export class DynamoDbManager implements IDynamoDbManager
let currentObject:T;
try
{
currentObject = await this.getOne(type, id, {stronglyConsistent:true});
currentObject = await this.getOne(type, id);
}
catch(e2)
{
Expand Down

0 comments on commit 1bddcce

Please sign in to comment.