From db128166cea27ca28ebec3166eb2009c0fd3da76 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Mon, 30 Apr 2018 10:21:45 -0700 Subject: [PATCH 1/2] demonstrate redis cleanSnapshots 'del' issue with failing test --- test/storeTest.js | 117 ++++++++++++++++++++++++++++++---------------- 1 file changed, 78 insertions(+), 39 deletions(-) diff --git a/test/storeTest.js b/test/storeTest.js index a7370418..27e4b7d8 100644 --- a/test/storeTest.js +++ b/test/storeTest.js @@ -3038,53 +3038,92 @@ types.forEach(function (type) { } }; - beforeEach(function (done) { - async.series([ - function (callback) { - store.addSnapshot(snap1, callback); - }, - function (callback) { - store.addSnapshot(snap2, callback); - }, - function (callback) { - store.addSnapshot(snap3, callback); - }, - function (callback) { - store.addSnapshot(snap4, callback); - }, - function (callback) { - store.addSnapshot(snap5, callback); - }, - function (callback) { - store.addSnapshot(snap6, callback); - }, - function (callback) { - store.addSnapshot(snap7, callback); - }, - function (callback) { - store.addSnapshot(snap8, callback); - } - ], done); - }); - describe('with an aggregateId being used only in one context and aggregate', function () { - it('it should clean oldest snapshots', function (done) { + describe('having fewer snapshots than the threshold', function() { + + beforeEach(function (done) { + async.series([ + function (callback) { + store.addSnapshot(snap1, callback); + }, + function (callback) { + store.addSnapshot(snap2, callback); + }, + function (callback) { + store.addSnapshot(snap3, callback); + }, + function (callback) { + store.addSnapshot(snap4, callback); + } + ], done); + }); + + it('can be called without error', function(done) { + + store.cleanSnapshots({ + aggregateId: '920193847', + aggregate: 'myCoolAggregate', + context: 'myCoolContext' + }, function (err, cleanedCount) { + expect(err).not.to.be.ok(); + expect(cleanedCount).to.equal(0); + done(); + }); + + }) + }) + + describe('having more snapshots than the threshold', function() { + + beforeEach(function (done) { + async.series([ + function (callback) { + store.addSnapshot(snap1, callback); + }, + function (callback) { + store.addSnapshot(snap2, callback); + }, + function (callback) { + store.addSnapshot(snap3, callback); + }, + function (callback) { + store.addSnapshot(snap4, callback); + }, + function (callback) { + store.addSnapshot(snap5, callback); + }, + function (callback) { + store.addSnapshot(snap6, callback); + }, + function (callback) { + store.addSnapshot(snap7, callback); + }, + function (callback) { + store.addSnapshot(snap8, callback); + } + ], done); + }); + + it('it should clean oldest snapshots', function (done) { + + store.cleanSnapshots({ + aggregateId: '920193847', + aggregate: 'myCoolAggregate', + context: 'myCoolContext' + }, function (err, cleanedCount) { + expect(err).not.to.be.ok(); + expect(cleanedCount).to.equal(3); + done(); + }); - store.cleanSnapshots({ - aggregateId: '920193847', - aggregate: 'myCoolAggregate', - context: 'myCoolContext' - }, function (err, cleanedCount) { - expect(err).not.to.be.ok(); - expect(cleanedCount).to.equal(3); - done(); }); }); + }); - }); + }) }); From 5989ac3458b31a88c0909d34e847bf276648796c Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Mon, 30 Apr 2018 10:24:52 -0700 Subject: [PATCH 2/2] Ensure redis store.cleanSnapshots does not error when number of snapshots is below threshold. --- lib/databases/redis.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/databases/redis.js b/lib/databases/redis.js index 929c81f7..32e89560 100644 --- a/lib/databases/redis.js +++ b/lib/databases/redis.js @@ -589,9 +589,15 @@ _.extend(Redis.prototype, { var keysToDelete = keys .sort() .slice(0, -1 * self.options.maxSnapshotsCount) - .concat(callback); - self.client.del.apply(self.client, keysToDelete); + if (keysToDelete.length === 0) { + if (callback) callback(null, 0); + return; + } + + var delArgs = keysToDelete.concat(callback); + + self.client.del.apply(self.client, delArgs); }); },