From e49f33317d596220b1168bcc72672b9016d0167a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Tue, 16 May 2017 17:14:50 -0400 Subject: [PATCH] digest cleanup and tests --- src/user/digest.js | 208 ++++++++++++++++++++++----------------------- src/user/jobs.js | 61 +++++++------ test/user.js | 24 ++++++ 3 files changed, 156 insertions(+), 137 deletions(-) diff --git a/src/user/digest.js b/src/user/digest.js index bf11e23a65..6381a8e262 100644 --- a/src/user/digest.js +++ b/src/user/digest.js @@ -12,121 +12,117 @@ var plugins = require('../plugins'); var emailer = require('../emailer'); var utils = require('../utils'); -(function (Digest) { - Digest.execute = function (interval, callback) { - callback = callback || function () {}; +var Digest = module.exports; - var digestsDisabled = parseInt(meta.config.disableEmailSubscriptions, 10) === 1; - if (digestsDisabled) { - winston.info('[user/jobs] Did not send digests (' + interval + ') because subscription system is disabled.'); - return callback(); - } +Digest.execute = function (interval, callback) { + callback = callback || function () {}; - if (!interval) { - // interval is one of: day, week, month, or year - interval = 'day'; - } - var subscribers; - async.waterfall([ - function (next) { - async.parallel({ - topics: async.apply(topics.getLatestTopics, 0, 0, 9, interval), - subscribers: async.apply(Digest.getSubscribers, interval), - }, next); - }, - function (data, next) { - subscribers = data.subscribers; - if (!data.subscribers.length) { - return callback(); + var digestsDisabled = parseInt(meta.config.disableEmailSubscriptions, 10) === 1; + if (digestsDisabled) { + winston.info('[user/jobs] Did not send digests (' + interval + ') because subscription system is disabled.'); + return callback(); + } + + var subscribers; + async.waterfall([ + function (next) { + async.parallel({ + topics: async.apply(topics.getLatestTopics, 0, 0, 9, interval), + subscribers: async.apply(Digest.getSubscribers, interval), + }, next); + }, + function (data, next) { + subscribers = data.subscribers; + if (!data.subscribers.length) { + return callback(); + } + + // Fix relative paths in topic data + data.topics.topics = data.topics.topics.map(function (topicObj) { + var user = topicObj.hasOwnProperty('teaser') && topicObj.teaser !== undefined ? topicObj.teaser.user : topicObj.user; + if (user && user.picture && utils.isRelativeUrl(user.picture)) { + user.picture = nconf.get('base_url') + user.picture; } - // Fix relative paths in topic data - data.topics.topics = data.topics.topics.map(function (topicObj) { - var user = topicObj.hasOwnProperty('teaser') && topicObj.teaser !== undefined ? topicObj.teaser.user : topicObj.user; - if (user && user.picture && utils.isRelativeUrl(user.picture)) { - user.picture = nconf.get('base_url') + user.picture; - } + return topicObj; + }); - return topicObj; - }); + data.interval = interval; + Digest.send(data, next); + }, + ], function (err) { + if (err) { + winston.error('[user/jobs] Could not send digests (' + interval + '): ' + err.message); + } else { + winston.info('[user/jobs] Digest (' + interval + ') scheduling completed. ' + subscribers.length + ' email(s) sent.'); + } - data.interval = interval; - Digest.send(data, next); - }, - ], function (err) { - if (err) { - winston.error('[user/jobs] Could not send digests (' + interval + '): ' + err.message); - } else { - winston.info('[user/jobs] Digest (' + interval + ') scheduling completed. ' + subscribers.length + ' email(s) sent.'); - } + callback(err); + }); +}; - callback(err); - }); - }; +Digest.getSubscribers = function (interval, callback) { + async.waterfall([ + function (next) { + db.getSortedSetRange('digest:' + interval + ':uids', 0, -1, next); + }, + function (subscribers, next) { + plugins.fireHook('filter:digest.subscribers', { + interval: interval, + subscribers: subscribers, + }, next); + }, + function (results, next) { + next(null, results.subscribers); + }, + ], callback); +}; - Digest.getSubscribers = function (interval, callback) { - async.waterfall([ - function (next) { - db.getSortedSetRange('digest:' + interval + ':uids', 0, -1, next); - }, - function (subscribers, next) { - plugins.fireHook('filter:digest.subscribers', { - interval: interval, - subscribers: subscribers, - }, next); - }, - function (results, next) { - next(null, results.subscribers); - }, - ], callback); - }; +Digest.send = function (data, callback) { + if (!data || !data.subscribers || !data.subscribers.length) { + return callback(); + } + var now = new Date(); - Digest.send = function (data, callback) { - if (!data || !data.subscribers || !data.subscribers.length) { - return callback(); - } - var now = new Date(); + async.waterfall([ + function (next) { + user.getUsersFields(data.subscribers, ['uid', 'username', 'userslug', 'lastonline'], next); + }, + function (users, next) { + async.eachLimit(users, 100, function (userObj, next) { + async.waterfall([ + function (next) { + user.notifications.getDailyUnread(userObj.uid, next); + }, + function (notifications, next) { + notifications = notifications.filter(Boolean); + // If there are no notifications and no new topics, don't bother sending a digest + if (!notifications.length && !data.topics.topics.length) { + return next(); + } - async.waterfall([ - function (next) { - user.getUsersFields(data.subscribers, ['uid', 'username', 'userslug', 'lastonline'], next); - }, - function (users, next) { - async.eachLimit(users, 100, function (userObj, next) { - async.waterfall([ - function (next) { - user.notifications.getDailyUnread(userObj.uid, next); - }, - function (notifications, next) { - notifications = notifications.filter(Boolean); - // If there are no notifications and no new topics, don't bother sending a digest - if (!notifications.length && !data.topics.topics.length) { - return next(); + notifications.forEach(function (notification) { + if (notification.image && !notification.image.startsWith('http')) { + notification.image = nconf.get('url') + notification.image; } + }); - notifications.forEach(function (notification) { - if (notification.image && !notification.image.startsWith('http')) { - notification.image = nconf.get('url') + notification.image; - } - }); - - emailer.send('digest', userObj.uid, { - subject: '[' + meta.config.title + '] [[email:digest.subject, ' + (now.getFullYear() + '/' + (now.getMonth() + 1) + '/' + now.getDate()) + ']]', - username: userObj.username, - userslug: userObj.userslug, - url: nconf.get('url'), - site_title: meta.config.title || meta.config.browserTitle || 'NodeBB', - notifications: notifications, - recent: data.topics.topics, - interval: data.interval, - }); - next(); - }, - ], next); - }, next); - }, - ], function (err) { - callback(err); - }); - }; -}(module.exports)); + emailer.send('digest', userObj.uid, { + subject: '[' + meta.config.title + '] [[email:digest.subject, ' + (now.getFullYear() + '/' + (now.getMonth() + 1) + '/' + now.getDate()) + ']]', + username: userObj.username, + userslug: userObj.userslug, + url: nconf.get('url'), + site_title: meta.config.title || meta.config.browserTitle || 'NodeBB', + notifications: notifications, + recent: data.topics.topics, + interval: data.interval, + }); + next(); + }, + ], next); + }, next); + }, + ], function (err) { + callback(err); + }); +}; diff --git a/src/user/jobs.js b/src/user/jobs.js index 1eeeb5650e..7950e3fdcb 100644 --- a/src/user/jobs.js +++ b/src/user/jobs.js @@ -10,7 +10,7 @@ var jobs = {}; module.exports = function (User) { User.startJobs = function (callback) { winston.verbose('[user/jobs] (Re-)starting user jobs...'); - var terminated = 0; + var started = 0; var digestHour = parseInt(meta.config.digestHour, 10); @@ -21,37 +21,12 @@ module.exports = function (User) { digestHour = 0; } - // Terminate any active cron jobs - for (var jobId in jobs) { - if (jobs.hasOwnProperty(jobId)) { - winston.verbose('[user/jobs] Terminating job (' + jobId + ')'); - jobs[jobId].stop(); - delete jobs[jobId]; - terminated += 1; - } - } - winston.verbose('[user/jobs] ' + terminated + ' jobs terminated'); - - jobs['digest.daily'] = new cronJob('0 ' + digestHour + ' * * *', function () { - winston.verbose('[user/jobs] Digest job (daily) started.'); - User.digest.execute('day'); - }, null, true); - winston.verbose('[user/jobs] Starting job (digest.daily)'); - started += 1; - - jobs['digest.weekly'] = new cronJob('0 ' + digestHour + ' * * 0', function () { - winston.verbose('[user/jobs] Digest job (weekly) started.'); - User.digest.execute('week'); - }, null, true); - winston.verbose('[user/jobs] Starting job (digest.weekly)'); - started += 1; + User.stopJobs(); - jobs['digest.monthly'] = new cronJob('0 ' + digestHour + ' 1 * *', function () { - winston.verbose('[user/jobs] Digest job (monthly) started.'); - User.digest.execute('month'); - }, null, true); - winston.verbose('[user/jobs] Starting job (digest.monthly)'); - started += 1; + startDigestJob('digest.daily', '0 ' + digestHour + ' * * *', 'day'); + startDigestJob('digest.weekly', '0 ' + digestHour + ' * * 0', 'week'); + startDigestJob('digest.monthly', '0 ' + digestHour + ' 1 * *', 'month'); + started += 3; jobs['reset.clean'] = new cronJob('0 0 * * *', User.reset.clean, null, true); winston.verbose('[user/jobs] Starting job (reset.clean)'); @@ -63,5 +38,29 @@ module.exports = function (User) { callback(); } }; + + function startDigestJob(name, cronString, term) { + jobs[name] = new cronJob(cronString, function () { + winston.verbose('[user/jobs] Digest job (' + name + ') started.'); + User.digest.execute(term); + }, null, true); + winston.verbose('[user/jobs] Starting job (' + name + ')'); + } + + User.stopJobs = function () { + var terminated = 0; + // Terminate any active cron jobs + for (var jobId in jobs) { + if (jobs.hasOwnProperty(jobId)) { + winston.verbose('[user/jobs] Terminating job (' + jobId + ')'); + jobs[jobId].stop(); + delete jobs[jobId]; + terminated += 1; + } + } + if (terminated > 0) { + winston.verbose('[user/jobs] ' + terminated + ' jobs terminated'); + } + }; }; diff --git a/test/user.js b/test/user.js index 87bcf39e70..f7d9986336 100644 --- a/test/user.js +++ b/test/user.js @@ -1301,6 +1301,30 @@ describe('User', function () { }); }); + describe('user jobs', function () { + it('should start user jobs', function (done) { + User.startJobs(function (err) { + assert.ifError(err); + done(); + }); + }); + + it('should stop user jobs', function (done) { + User.stopJobs(); + done(); + }); + + it('should send digetst', function (done) { + db.sortedSetAdd('digest:day:uids', [Date.now(), Date.now()], [1, 2], function (err) { + assert.ifError(err); + User.digest.execute('day', function (err) { + assert.ifError(err); + done(); + }); + }); + }); + }); + after(function (done) { db.emptydb(done);