Set _cronSignature to current time instead of uuid (#8565)

* Changes made to satisfy #8163. _cronSignature is set to current time when cron starts so that if cron fails to set _cronSignature to 'NOT_RUNNING' for any reason a new cron can be started after a set amount of time (1 hour for now)

* fix lint errors

* changed cronTimeout to CRON_TIMEOUT

* Changed variable names and comments to be more clear

* Fixed stub for failing test so that it matches new mongo db update call signature

* First pass at unit tests, error messages and some other things need to be determined

* Fixed a tab that snuck in :/

* Fixed lint issues (issues with spaces)

* Fix infix operator spacing

* Created constant. Make sure cron failure test verifies that it is failing for the right reason

* Fixed lint errors

* Removed no longer used uuid import
This commit is contained in:
Zobdek
2017-10-25 16:29:16 -04:00
committed by Sabe Jones
parent 9736ef0d25
commit addee73e4d
2 changed files with 65 additions and 4 deletions

View File

@@ -13,6 +13,9 @@ import analyticsService from '../../../../../website/server/libs/analyticsServic
import * as cronLib from '../../../../../website/server/libs/cron'; import * as cronLib from '../../../../../website/server/libs/cron';
import { v4 as generateUUID } from 'uuid'; import { v4 as generateUUID } from 'uuid';
const CRON_TIMEOUT_WAIT = new Date(60 * 60 * 1000).getTime();
const CRON_TIMEOUT_UNIT = new Date(60 * 1000).getTime();
describe('cron middleware', () => { describe('cron middleware', () => {
let res, req; let res, req;
let user; let user;
@@ -235,7 +238,13 @@ describe('cron middleware', () => {
sandbox.spy(cronLib, 'recoverCron'); sandbox.spy(cronLib, 'recoverCron');
sandbox.stub(User, 'update') sandbox.stub(User, 'update')
.withArgs({ _id: user._id, _cronSignature: 'NOT_RUNNING' }) .withArgs({
_id: user._id,
$or: [
{_cronSignature: 'NOT_RUNNING'},
{_cronSignature: {$lt: sinon.match.number}},
],
})
.returns({ .returns({
exec () { exec () {
return Promise.resolve(updatedUser); return Promise.resolve(updatedUser);
@@ -251,4 +260,48 @@ describe('cron middleware', () => {
}); });
}); });
}); });
it('cronSignature less than an hour ago should error', async () => {
user.lastCron = moment(new Date()).subtract({days: 2});
let now = new Date();
await User.update({
_id: user._id,
}, {
$set: {
_cronSignature: now.getTime() - CRON_TIMEOUT_WAIT + CRON_TIMEOUT_UNIT,
},
}).exec();
await user.save();
let expectedErrMessage = `Impossible to recover from cron for user ${user._id}.`;
await new Promise((resolve, reject) => {
cronMiddleware(req, res, (err) => {
if (!err) return reject(new Error('Cron should have failed.'));
expect(err.message).to.be.equal(expectedErrMessage);
resolve();
});
});
});
it('cronSignature longer than an hour ago should allow cron', async () => {
user.lastCron = moment(new Date()).subtract({days: 2});
let now = new Date();
await User.update({
_id: user._id,
}, {
$set: {
_cronSignature: now.getTime() - CRON_TIMEOUT_WAIT - CRON_TIMEOUT_UNIT,
},
}).exec();
await user.save();
await new Promise((resolve, reject) => {
cronMiddleware(req, res, (err) => {
if (err) return reject(err);
expect(moment(now).isSame(user.auth.timestamps.loggedin, 'day'));
expect(user._cronSignature).to.be.equal('NOT_RUNNING');
resolve();
});
});
});
}); });

View File

@@ -4,15 +4,23 @@ import Bluebird from 'bluebird';
import { model as Group } from '../models/group'; import { model as Group } from '../models/group';
import { model as User } from '../models/user'; import { model as User } from '../models/user';
import { recoverCron, cron } from '../libs/cron'; import { recoverCron, cron } from '../libs/cron';
import { v4 as uuid } from 'uuid';
// Wait this length of time in ms before attempting another cron
const CRON_TIMEOUT_WAIT = new Date(60 * 60 * 1000).getTime();
async function checkForActiveCron (user, now) { async function checkForActiveCron (user, now) {
let _cronSignature = uuid(); // set _cronSignature to current time in ms since epoch time so we can make sure to wait at least CRONT_TIMEOUT_WAIT before attempting another cron
let _cronSignature = now.getTime();
// Calculate how long ago cron must have been attempted to try again
let cronRetryTime = _cronSignature - CRON_TIMEOUT_WAIT;
// To avoid double cron we first set _cronSignature and then check that it's not changed while processing // To avoid double cron we first set _cronSignature and then check that it's not changed while processing
let userUpdateResult = await User.update({ let userUpdateResult = await User.update({
_id: user._id, _id: user._id,
_cronSignature: 'NOT_RUNNING', // Check that in the meantime another cron has not started $or: [ // Make sure last cron was successful or failed before cronRetryTime
{_cronSignature: 'NOT_RUNNING'},
{_cronSignature: {$lt: cronRetryTime}},
],
}, { }, {
$set: { $set: {
_cronSignature, _cronSignature,