Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update batchPut to work with timestamps #449

Merged
merged 6 commits into from
Oct 18, 2018
Merged

Update batchPut to work with timestamps #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
32a6866
Added tests for batchPut with timestamps
dolsem Oct 17, 2018
5c79d34
Updated batchPut to work with timestamps
dolsem Oct 17, 2018
6e7781e
Added tests for batchPut with updateExpires option
dolsem Oct 17, 2018
f7c8b0b
Added updateExpires to batchPut options
dolsem Oct 17, 2018
6c359e7
Updated documentation with newly added options for batchPut
dolsem Oct 17, 2018
edf897b
Updated style for #449
dolsem Oct 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/_docs/model.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ Dog.batchPut([

Overwrite existing item. Defaults to true.

**updateExpires**: boolean

Update the `expires` timestamp if exists. Defaults to false.

**updateTimestamps**: boolean

Should update the `updatedAt` timestamp if exists. Defaults to false.

**condition**: string

An expression for a conditional update. See
Expand Down
7 changes: 6 additions & 1 deletion lib/Model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1270,10 +1270,15 @@ Model.batchPut = function(NewModel, items, options, next) {
const schema = NewModel.$__.schema;
const newModel$ = NewModel.$__;

const toDynamoOptions = {
updateTimestamps: options.updateTimestamps || false,
updateExpires: options.updateExpires || false
};

const batchRequests = toBatchChunks(newModel$.name, items, MAX_BATCH_WRITE_SIZE, function(item) {
return {
PutRequest: {
Item: schema.toDynamo(item)
Item: schema.toDynamo(item, toDynamoOptions)
}
};
});
Expand Down
234 changes: 234 additions & 0 deletions test/Model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2137,6 +2137,240 @@ describe('Model', function (){
});
});
});

it('Update without updateTimestamps (default)', async function () {
const cats = [...Array(10)]
.map((_, i) => new Cats.Cat4({ id: i + 1, name: 'Tom_' + i }));

const result = await Cats.Cat4.batchPut(cats);
should.exist(result);

const timestamps = {};
cats.forEach((cat, i) => {
const { id, myLittleUpdatedAt } = cat;
cat.name = 'John_' + i;
timestamps[id] = new Date(myLittleUpdatedAt);
});

await new Promise(resolve => setTimeout(resolve, 1000));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized one more thing I forgot to mention. Why not make this something much shorter? Like 5 or even 1? Is there a specific reason why we have to wait a full second here? Waiting just a few milliseconds should ensure that the timestamps are later than the original value, correct?

If we decide to edit it we have to make sure we do it throughout these new tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I tried making it shorter, and the tests would sometimes fail. Seems like when timestamps get updated milliseconds get stripped away either on the original objects after they saved or on the results of batchGet, not sure which one right now. I didn't have time to investigate this, so I don't know if this is the result of floor, ceiling, or round, so to be safe I made it wait a full second. This made the tests deterministic, no chance of failure anymore. I might be mistaken, but I think the same timeout is used for other tests around timestamps in the test suit too.

const result2 = await Cats.Cat4.batchPut(cats);
should.exist(result2);
Object.getOwnPropertyNames(result2.UnprocessedItems).length.should.eql(0);

const updatedCats = await Cats.Cat4.batchGet(cats);
should.exist(updatedCats);
updatedCats.length.should.eql(cats.length);
updatedCats.forEach(cat => {
cat.myLittleUpdatedAt.should.eql(timestamps[cat.id]);
});
});

it('Update with updateTimestamps set to false', async function () {
const cats = [...Array(10)]
.map((_, i) => new Cats.Cat4({ id: i + 1, name: 'Tom_' + i }));

const result = await Cats.Cat4.batchPut(cats);
should.exist(result);

const timestamps = {};
cats.forEach((cat, i) => {
const { id, myLittleUpdatedAt } = cat;
cat.name = 'John_' + i;
timestamps[id] = new Date(myLittleUpdatedAt);
});

await new Promise(resolve => setTimeout(resolve, 1000));
const result2 = await Cats.Cat4.batchPut(cats, { updateTimestamps: false });
should.exist(result2);
Object.getOwnPropertyNames(result2.UnprocessedItems).length.should.eql(0);

const updatedCats = await Cats.Cat4.batchGet(cats);
should.exist(updatedCats);
updatedCats.length.should.eql(cats.length);
updatedCats.forEach(cat => {
cat.myLittleUpdatedAt.should.eql(timestamps[cat.id]);
});
});

it('Update with updateTimestamps set to true', async function () {
const cats = [...Array(10)]
.map((_, i) => new Cats.Cat4({ id: i + 1, name: 'Tom_' + i }));

const result = await Cats.Cat4.batchPut(cats);
should.exist(result);

const timestamps = {};
cats.forEach((cat, i) => {
const { id, myLittleUpdatedAt } = cat;
cat.name = 'John_' + i;
timestamps[id] = new Date(myLittleUpdatedAt);
});

await new Promise(resolve => setTimeout(resolve, 1000));
const result2 = await Cats.Cat4.batchPut(cats, { updateTimestamps: true });
should.exist(result2);
Object.getOwnPropertyNames(result2.UnprocessedItems).length.should.eql(0);

const updatedCats = await Cats.Cat4.batchGet(cats);
should.exist(updatedCats);
updatedCats.length.should.eql(cats.length);
updatedCats.forEach(cat => {
cat.myLittleUpdatedAt.should.be.greaterThan(timestamps[cat.id]);
});
});

it('Update without updateExpires (default)', async function () {
const cats = [
new Cats.Cat11({
id: 1,
name: 'Crookshanks',
vet: { name: 'theVet', address: 'Diagon Alley' },
ears: [{ name: 'left' }, { name: 'right' }],
legs: ['front right', 'front left', 'back right', 'back left'],
more: { favorites: { food: 'fish' } },
array: [{ one: '1' }],
validated: 'valid'
}),
new Cats.Cat11({
id: 2,
name: 'Behemoth',
vet: { name:'Mikhail Bulgakov', address:'Moscow' },
ears: [{ name: 'left' }, { name: 'right' }],
legs: ['front right', 'front left', 'back right', 'back left'],
more: { favorites: { drink: 'pure alcohol' } },
array: [{ one: '1' }],
validated: 'valid'
})
];

const result = await Cats.Cat11.batchPut(cats);
should.exist(result);

const savedCats = await Cats.Cat11.batchGet(cats);
should.exist(savedCats);
savedCats.length.should.eql(cats.length);

const originalExpires = {};
savedCats.forEach(cat => {
cat.array.push({ two: '2' });
originalExpires[cat.id] = cat.expires;
});

await new Promise(resolve => setTimeout(resolve, 1000));
const result2 = await Cats.Cat11.batchPut(savedCats);
should.exist(result2);
Object.getOwnPropertyNames(result2.UnprocessedItems).length.should.eql(0);

const updatedCats = await Cats.Cat11.batchGet(cats);
should.exist(updatedCats);
updatedCats.length.should.eql(cats.length);
updatedCats.forEach((cat) => {
cat.array.length.should.eql(2);
cat.expires.should.eql(originalExpires[cat.id]);
});
});

it('Update with updateExpires set to false', async function () {
const cats = [
new Cats.Cat11({
id: 1,
name: 'Crookshanks',
vet: { name: 'theVet', address: 'Diagon Alley' },
ears: [{ name: 'left' }, { name: 'right' }],
legs: ['front right', 'front left', 'back right', 'back left'],
more: { favorites: { food: 'fish' } },
array: [{ one: '1' }],
validated: 'valid'
}),
new Cats.Cat11({
id: 2,
name: 'Behemoth',
vet: { name:'Mikhail Bulgakov', address:'Moscow' },
ears: [{ name: 'left' }, { name: 'right' }],
legs: ['front right', 'front left', 'back right', 'back left'],
more: { favorites: { drink: 'pure alcohol' } },
array: [{ one: '1' }],
validated: 'valid'
})
];

const result = await Cats.Cat11.batchPut(cats);
should.exist(result);

const savedCats = await Cats.Cat11.batchGet(cats);
should.exist(savedCats);
savedCats.length.should.eql(cats.length);

const originalExpires = {};
savedCats.forEach(cat => {
cat.array.push({ two: '2' });
originalExpires[cat.id] = cat.expires;
});

await new Promise(resolve => setTimeout(resolve, 1000));
const result2 = await Cats.Cat11.batchPut(savedCats, { updateExpires: false });
should.exist(result2);
Object.getOwnPropertyNames(result2.UnprocessedItems).length.should.eql(0);

const updatedCats = await Cats.Cat11.batchGet(cats);
should.exist(updatedCats);
updatedCats.length.should.eql(cats.length);
updatedCats.forEach((cat) => {
cat.array.length.should.eql(2);
cat.expires.should.eql(originalExpires[cat.id]);
});
});

it('Update with updateExpires set to true', async function () {
const cats = [
new Cats.Cat11({
id: 1,
name: 'Crookshanks',
vet: { name: 'theVet', address: 'Diagon Alley' },
ears: [{ name: 'left' }, { name: 'right' }],
legs: ['front right', 'front left', 'back right', 'back left'],
more: { favorites: { food: 'fish' } },
array: [{ one: '1' }],
validated: 'valid'
}),
new Cats.Cat11({
id: 2,
name: 'Behemoth',
vet: { name:'Mikhail Bulgakov', address:'Moscow' },
ears: [{ name: 'left' }, { name: 'right' }],
legs: ['front right', 'front left', 'back right', 'back left'],
more: { favorites: { drink: 'pure alcohol' } },
array: [{ one: '1' }],
validated: 'valid'
})
];

const result = await Cats.Cat11.batchPut(cats);
should.exist(result);

const savedCats = await Cats.Cat11.batchGet(cats);
should.exist(savedCats);
savedCats.length.should.eql(cats.length);

const originalExpires = {};
savedCats.forEach(cat => {
cat.array.push({ two: '2' });
originalExpires[cat.id] = cat.expires;
});

await new Promise(resolve => setTimeout(resolve, 1000));
const result2 = await Cats.Cat11.batchPut(savedCats, { updateExpires: true });
should.exist(result2);
Object.getOwnPropertyNames(result2.UnprocessedItems).length.should.eql(0);

const updatedCats = await Cats.Cat11.batchGet(cats);
should.exist(updatedCats);
updatedCats.length.should.eql(cats.length);
updatedCats.forEach((cat) => {
cat.array.length.should.eql(2);
cat.expires.should.be.greaterThan(originalExpires[cat.id]);
});
});
});

describe('Model.batchDelete', function (){
Expand Down