From addee73e4d2bb89215386bfb5e537e42b23ec25d Mon Sep 17 00:00:00 2001
From: Zobdek <kyleverma@gmail.com>
Date: Wed, 25 Oct 2017 16:29:16 -0400
Subject: [PATCH] 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
---
 .../api/v3/unit/middlewares/cronMiddleware.js | 55 ++++++++++++++++++-
 website/server/middlewares/cron.js            | 14 ++++-
 2 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/test/api/v3/unit/middlewares/cronMiddleware.js b/test/api/v3/unit/middlewares/cronMiddleware.js
index 107abd9c13..c00be210b2 100644
--- a/test/api/v3/unit/middlewares/cronMiddleware.js
+++ b/test/api/v3/unit/middlewares/cronMiddleware.js
@@ -13,6 +13,9 @@ import analyticsService from '../../../../../website/server/libs/analyticsServic
 import * as cronLib from '../../../../../website/server/libs/cron';
 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', () => {
   let res, req;
   let user;
@@ -235,7 +238,13 @@ describe('cron middleware', () => {
     sandbox.spy(cronLib, 'recoverCron');
 
     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({
         exec () {
           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();
+      });
+    });
+  });
 });
diff --git a/website/server/middlewares/cron.js b/website/server/middlewares/cron.js
index 5bf2eb26b0..8b5abb970e 100644
--- a/website/server/middlewares/cron.js
+++ b/website/server/middlewares/cron.js
@@ -4,15 +4,23 @@ import Bluebird from 'bluebird';
 import { model as Group } from '../models/group';
 import { model as User } from '../models/user';
 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) {
-  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
   let userUpdateResult = await User.update({
     _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: {
       _cronSignature,