diff --git a/test/api/v3/unit/libs/cron.test.js b/test/api/v3/unit/libs/cron.test.js index a7e732442c..6f16f9973f 100644 --- a/test/api/v3/unit/libs/cron.test.js +++ b/test/api/v3/unit/libs/cron.test.js @@ -34,15 +34,6 @@ describe('cron', () => { }; }); - it('updates user.auth.timestamps.loggedin and lastCron', () => { - let now = new Date(); - - cron({user, tasksByType, daysMissed, analytics, now}); - - expect(user.auth.timestamps.loggedin).to.equal(now); - expect(user.lastCron).to.equal(now); - }); - it('updates user.preferences.timezoneOffsetAtLastCron', () => { let timezoneOffsetFromUserPrefs = 1; diff --git a/test/api/v3/unit/middlewares/cronMiddleware.js b/test/api/v3/unit/middlewares/cronMiddleware.js index f4e040a11c..cba4cf83c1 100644 --- a/test/api/v3/unit/middlewares/cronMiddleware.js +++ b/test/api/v3/unit/middlewares/cronMiddleware.js @@ -1,7 +1,6 @@ import { generateRes, generateReq, - generateNext, generateTodo, generateDaily, } from '../../../../helpers/api-unit.helper'; @@ -14,13 +13,12 @@ import analyticsService from '../../../../../website/server/libs/api-v3/analytic import { v4 as generateUUID } from 'uuid'; describe('cron middleware', () => { - let res, req, next; + let res, req; let user; - beforeEach(() => { + beforeEach((done) => { res = generateRes(); req = generateReq(); - next = generateNext(); user = new User({ auth: { local: { @@ -33,24 +31,27 @@ describe('cron middleware', () => { }, }); - user._statsComputed = { - mp: 10, - maxMP: 100, - }; + user.save() + .then(savedUser => { + savedUser._statsComputed = { + mp: 10, + maxMP: 100, + }; - res.locals.user = user; - res.analytics = analyticsService; + res.locals.user = savedUser; + res.analytics = analyticsService; + done(); + }) + .catch(done); }); - it('calls next when user is not attached', () => { + it('calls next when user is not attached', (done) => { res.locals.user = null; - cronMiddleware(req, res, next); - expect(next).to.be.calledOnce; + cronMiddleware(req, res, (err) => done(err)); }); - it('calls next when days have not been missed', () => { - cronMiddleware(req, res, next); - expect(next).to.be.calledOnce; + it('calls next when days have not been missed', (done) => { + cronMiddleware(req, res, (err) => done(err)); }); it('should clear todos older than 30 days for free users', async (done) => { @@ -59,35 +60,37 @@ describe('cron middleware', () => { task.dateCompleted = moment(new Date()).subtract({days: 31}); task.completed = true; await task.save(); + await user.save(); - cronMiddleware(req, res, () => { - Tasks.Task.findOne({_id: task}, function (err, taskFound) { - expect(err).to.not.exist; + cronMiddleware(req, res, (err) => { + Tasks.Task.findOne({_id: task}, function (secondErr, taskFound) { + expect(secondErr).to.not.exist; expect(taskFound).to.not.exist; - done(); + done(err); }); }); }); - it('should not clear todos older than 30 days for subscribed users', (done) => { + it('should not clear todos older than 30 days for subscribed users', async (done) => { user.purchased.plan.customerId = 'subscribedId'; user.purchased.plan.dateUpdated = moment('012013', 'MMYYYY'); user.lastCron = moment(new Date()).subtract({days: 2}); let task = generateTodo(user); task.dateCompleted = moment(new Date()).subtract({days: 31}); task.completed = true; - task.save(); + await task.save(); + await user.save(); - cronMiddleware(req, res, () => { - Tasks.Task.findOne({_id: task}, function (err, taskFound) { - expect(err).to.not.exist; + cronMiddleware(req, res, (err) => { + Tasks.Task.findOne({_id: task}, function (secondErr, taskFound) { + expect(secondErr).to.not.exist; expect(taskFound).to.exist; - done(); + done(err); }); }); }); - it('should clear todos older than 90 days for subscribed users', (done) => { + it('should clear todos older than 90 days for subscribed users', async (done) => { user.purchased.plan.customerId = 'subscribedId'; user.purchased.plan.dateUpdated = moment('012013', 'MMYYYY'); user.lastCron = moment(new Date()).subtract({days: 2}); @@ -95,46 +98,60 @@ describe('cron middleware', () => { let task = generateTodo(user); task.dateCompleted = moment(new Date()).subtract({days: 91}); task.completed = true; - task.save(); + await task.save(); + await user.save(); - cronMiddleware(req, res, () => { - Tasks.Task.findOne({_id: task}, function (err, taskFound) { - expect(err).to.not.exist; + cronMiddleware(req, res, (err) => { + Tasks.Task.findOne({_id: task}, function (secondErr, taskFound) { + expect(secondErr).to.not.exist; expect(taskFound).to.not.exist; - done(); + done(err); }); }); }); - it('should call next is user was not modified after cron', (done) => { + it('should call next if user was not modified after cron', async (done) => { let hpBefore = user.stats.hp; user.lastCron = moment(new Date()).subtract({days: 2}); + await user.save(); - user.save().then(function () { - cronMiddleware(req, res, function () { - expect(hpBefore).to.equal(user.stats.hp); - done(); - }); + cronMiddleware(req, res, (err) => { + expect(hpBefore).to.equal(user.stats.hp); + done(err); }); }); - it('does damage for missing dailies', (done) => { + it('updates user.auth.timestamps.loggedin and lastCron', async (done) => { + user.lastCron = moment(new Date()).subtract({days: 2}); + let now = new Date(); + await user.save(); + + cronMiddleware(req, res, (err) => { + expect(moment(now).isSame(user.lastCron, 'day')); + expect(moment(now).isSame(user.auth.timestamps.loggedin, 'day')); + done(err); + }); + }); + + it('does damage for missing dailies', async (done) => { let hpBefore = user.stats.hp; user.lastCron = moment(new Date()).subtract({days: 2}); let daily = generateDaily(user); daily.startDate = moment(new Date()).subtract({days: 2}); - daily.save(); + await daily.save(); + await user.save(); - cronMiddleware(req, res, () => { + cronMiddleware(req, res, (err) => { expect(user.stats.hp).to.be.lessThan(hpBefore); - done(); + done(err); }); }); - it('updates tasks', (done) => { + it('updates tasks', async (done) => { user.lastCron = moment(new Date()).subtract({days: 2}); let todo = generateTodo(user); let todoValueBefore = todo.value; + await user.save(); cronMiddleware(req, res, () => { Tasks.Task.findOne({_id: todo._id}, function (err, todoFound) { @@ -150,7 +167,7 @@ describe('cron middleware', () => { user.lastCron = moment(new Date()).subtract({days: 2}); let daily = generateDaily(user); daily.startDate = moment(new Date()).subtract({days: 2}); - daily.save(); + await daily.save(); let questKey = 'dilatory'; user.party.quest.key = questKey; diff --git a/website/server/middlewares/api-v3/cron.js b/website/server/middlewares/api-v3/cron.js index ff9338b416..75589e5af3 100644 --- a/website/server/middlewares/api-v3/cron.js +++ b/website/server/middlewares/api-v3/cron.js @@ -10,44 +10,36 @@ import { v4 as uuid } from 'uuid'; const daysSince = common.daysSince; -async function recoverCron (status, req, res, next) { - try { - let user = res.locals.user; +async function recoverCron (status, req, res) { + let user = res.locals.user; - await Bluebird.delay(300); - let reloadedUser = await User.findOne({_id: user._id}).exec(); + await Bluebird.delay(300); + let reloadedUser = await User.findOne({_id: user._id}).exec(); - if (!reloadedUser) { - throw new Error(`User ${user._id} not found while recovering.`); - } else if (reloadedUser._cronSignature !== 'NOT_RUNNING') { - console.log((new Date()).toISOString(), 'RECOVERED FROM CRON - STILL RUNNING', req.originalUrl, req.method); - status.times++; - if (status.times < 4) { - await recoverCron(status, req, res, next); - } else { - throw new Error(`Impossible to recover from cron for user ${user._id}.`); - } + if (!reloadedUser) { + throw new Error(`User ${user._id} not found while recovering.`); + } else if (reloadedUser._cronSignature !== 'NOT_RUNNING') { + status.times++; + + if (status.times < 4) { + await recoverCron(status, req, res); } else { - console.log((new Date()).toISOString(), 'RECOVERED FROM CRON', req.originalUrl, req.method); - res.locals.user = reloadedUser; - return next(); + throw new Error(`Impossible to recover from cron for user ${user._id}.`); } - } catch (err) { - return next(err); + } else { + res.locals.user = reloadedUser; + return null; } } -module.exports = async function cronMiddleware (req, res, next) { +async function cronAsync (req, res) { let user = res.locals.user; - if (!user) return next(); // User might not be available when authentication is not mandatory + if (!user) return null; // User might not be available when authentication is not mandatory let analytics = res.analytics; let now = new Date(); - let _cronSignature = uuid(); try { - console.log((new Date()).toISOString(), 'CHECKING RUN CRON', req.originalUrl, req.method); - // If the user's timezone has changed (due to travel or daylight savings), // cron can be triggered twice in one day, so we check for that and use // both timezones to work out if cron should run. @@ -128,10 +120,10 @@ module.exports = async function cronMiddleware (req, res, next) { if (daysMissed <= 0) { if (user.isModified()) await user.save(); - return next(); + return null; } - console.log((new Date()).toISOString(), 'RUNNING CRON FOR REAL', req.originalUrl, req.method); + let _cronSignature = uuid(); // To avoid double cron we first set _cronSignature to now and then check that it's not changed while processing let userUpdateResult = await User.update({ @@ -142,7 +134,6 @@ module.exports = async function cronMiddleware (req, res, next) { _cronSignature, }, }).exec(); - console.log((new Date()).toISOString(), 'FIRST USER UPDATE?', userUpdateResult, req.originalUrl, req.method); // If the cron signature is already set, cron is running in another request // throw an error and recover later, @@ -208,11 +199,10 @@ module.exports = async function cronMiddleware (req, res, next) { // Reload user res.locals.user = await User.findOne({_id: user._id}).exec(); - return next(); + return null; } catch (err) { // If cron was aborted for a race condition try to recover from it if (err.message === 'CRON_ALREADY_RUNNING') { - console.log((new Date()).toISOString(), 'RECOVERING FROM CRON', req.originalUrl, req.method); // Recovering after abort, wait 300ms and reload user // do it for max 4 times then reset _cronSignature so that it doesn't prevent cron from running // at the next request @@ -220,17 +210,31 @@ module.exports = async function cronMiddleware (req, res, next) { times: 0, }; - recoverCron(recoveryStatus, req, res, next); + recoverCron(recoveryStatus, req, res); } else { // For any other error make sure to reset _cronSignature so that it doesn't prevent cron from running // at the next request - await User.update({ - _id: user._id, - }, { - _cronSignature: 'NOT_RUNNING', - }).exec() - .then(() => next(err)) - .catch(secondError => next(secondError)); + try { + await User.update({ + _id: user._id, + }, { + _cronSignature: 'NOT_RUNNING', + }).exec(); + + throw err; // re-throw original error + } catch (secondErr) { + throw secondErr; + } } } +} + +module.exports = function cronMiddleware (req, res, next) { + cronAsync(req, res) + .then(() => { + next(); + }) + .catch(err => { + next(err); + }); };