From 5ae7c92d55040494fcff9f8905a6da5d3deb050b Mon Sep 17 00:00:00 2001 From: barisusakli Date: Sat, 16 Aug 2014 23:25:30 -0400 Subject: [PATCH] notifications refactor added getMultiple which works with an array of nids --- src/controllers/accounts.js | 2 +- src/notifications.js | 72 +++++++------ src/user/notifications.js | 199 +++++++++++++++++------------------- 3 files changed, 134 insertions(+), 139 deletions(-) diff --git a/src/controllers/accounts.js b/src/controllers/accounts.js index 15bfd0a17b..fac96c99a5 100644 --- a/src/controllers/accounts.js +++ b/src/controllers/accounts.js @@ -478,7 +478,7 @@ accountsController.uploadPicture = function (req, res, next) { }; accountsController.getNotifications = function(req, res, next) { - user.notifications.getAll(req.user.uid, null, null, function(err, notifications) { + user.notifications.getAll(req.user.uid, 25, function(err, notifications) { res.render('notifications', { notifications: notifications }); diff --git a/src/notifications.js b/src/notifications.js index b5d61895e5..ecd73e22ed 100644 --- a/src/notifications.js +++ b/src/notifications.js @@ -24,47 +24,57 @@ var async = require('async'), }; Notifications.get = function(nid, callback) { - db.getObject('notifications:' + nid, function(err, notification) { + Notifications.getMultiple([nid], function(err, notifications) { + callback(err, Array.isArray(notifications) && notifications.length ? notifications[0] : null); + }); + }; + + Notifications.getMultiple = function(nids, callback) { + var keys = nids.map(function(nid) { + return 'notifications:' + nid; + }); + + db.getObjects(keys, function(err, notifications) { if (err) { return callback(err); } - if (!notification) { - winston.info('[notifications.get] Could not retrieve nid ' + nid); - return callback(null, null); - } + async.map(notifications, function(notification, next) { + if (!notification) { + return next(null, null); + } - // Backwards compatibility for old notification schema - // Remove this block when NodeBB v0.6.0 is released. - if (notification.hasOwnProperty('text')) { - notification.bodyShort = notification.text; - notification.bodyLong = ''; - notification.text = S(notification.text).escapeHTML().s; - } + // Backwards compatibility for old notification schema + // Remove this block when NodeBB v0.6.0 is released. + if (notification.hasOwnProperty('text')) { + notification.bodyShort = notification.text; + notification.bodyLong = ''; + notification.text = S(notification.text).escapeHTML().s; + } - notification.bodyShort = S(notification.bodyShort).escapeHTML().s; - notification.bodyLong = S(notification.bodyLong).escapeHTML().s; + notification.bodyShort = S(notification.bodyShort).escapeHTML().s; + notification.bodyLong = S(notification.bodyLong).escapeHTML().s; - if (notification.from && !notification.image) { - User.getUserField(notification.from, 'picture', function(err, picture) { - if (err) { - return callback(err); + if (notification.from && !notification.image) { + User.getUserField(notification.from, 'picture', function(err, picture) { + if (err) { + return next(err); + } + notification.image = picture; + next(null, notification); + }); + return; + } else if (notification.image) { + switch(notification.image) { + case 'brand:logo': + notification.image = meta.config['brand:logo'] || nconf.get('relative_path') + '/logo.png'; + break; } - notification.image = picture; - callback(null, notification); - }); - return; - } else if (notification.image) { - switch(notification.image) { - case 'brand:logo': - notification.image = meta.config['brand:logo'] || nconf.get('relative_path') + '/logo.png'; - break; - } - return callback(null, notification); - } + return next(null, notification); + } - callback(null, notification); + }, callback); }); }; diff --git a/src/user/notifications.js b/src/user/notifications.js index f6e85aa05f..38ece77940 100644 --- a/src/user/notifications.js +++ b/src/user/notifications.js @@ -17,86 +17,22 @@ var async = require('async'), (function(UserNotifications) { UserNotifications.get = function(uid, callback) { - function getNotifications(set, start, stop, iterator, done) { - db.getSortedSetRevRange(set, start, stop, function(err, uniqueIds) { - if(err) { - return done(err); - } - - if(!Array.isArray(uniqueIds) || !uniqueIds.length) { - return done(null, []); - } - - if (uniqueIds.length > maxNotifs) { - uniqueIds.length = maxNotifs; - } - - db.getObjectFields('uid:' + uid + ':notifications:uniqueId:nid', uniqueIds, function(err, uniqueIdToNids) { - if (err) { - return done(err); - } - - var nidsToUniqueIds = {}; - var nids = []; - uniqueIds.forEach(function(uniqueId) { - nidsToUniqueIds[uniqueIdToNids[uniqueId]] = uniqueId; - nids.push(uniqueIdToNids[uniqueId]); - }); - - async.map(nids, function(nid, next) { - notifications.get(nid, function(err, notif_data) { - if (err) { - return next(err); - } - - if (!notif_data) { - if (process.env.NODE_ENV === 'development') { - winston.info('[notifications.get] nid ' + nid + ' not found. Removing.'); - } - - db.sortedSetRemove(set, nidsToUniqueIds[nid]); - db.deleteObjectField('uid:' + uid + ':notifications:uniqueId:nid', nidsToUniqueIds[nid]); - return next(); - } - - if (typeof iterator === 'function') { - iterator(notif_data, next); - } else { - next(null, notif_data); - } - }); - }, done); - }); - }); - } - var maxNotifs = 15; async.parallel({ unread: function(next) { - getNotifications('uid:' + uid + ':notifications:unread', 0, 9, function(notif_data, next) { - notif_data.read = false; - notif_data.readClass = !notif_data.read ? 'label-warning' : ''; - next(null, notif_data); - }, next); + getNotificationsFromSet('uid:' + uid + ':notifications:unread', uid, 0, 9, maxNotifs, next); }, read: function(next) { - getNotifications('uid:' + uid + ':notifications:read', 0, 9, function(notif_data, next) { - notif_data.read = true; - next(null, notif_data); - }, next); + getNotificationsFromSet('uid:' + uid + ':notifications:read', uid, 0, 9, maxNotifs, next); } }, function(err, notifications) { - function filterDeleted(notifObj) { - return !!notifObj; - } - if (err) { return callback(err); } - notifications.read = notifications.read.filter(filterDeleted); - notifications.unread = notifications.unread.filter(filterDeleted); + notifications.read = notifications.read.filter(Boolean); + notifications.unread = notifications.unread.filter(Boolean); // Limit the number of notifications to `maxNotifs`, prioritising unread notifications if (notifications.read.length + notifications.unread.length > maxNotifs) { @@ -107,48 +43,71 @@ var async = require('async'), }); }; - UserNotifications.getAll = function(uid, limit, before, callback) { - var now = new Date(); + function getNotificationsFromSet(set, uid, start, stop, max, callback) { + db.getSortedSetRevRange(set, start, stop, function(err, uniqueIds) { + if (err) { + return callback(err); + } + + if(!Array.isArray(uniqueIds) || !uniqueIds.length) { + return callback(null, []); + } + + if (uniqueIds.length > max) { + uniqueIds.length = max; + } + + db.getObjectFields('uid:' + uid + ':notifications:uniqueId:nid', uniqueIds, function(err, uniqueIdToNids) { + if (err) { + return callback(err); + } + + var nidsToUniqueIds = {}; + var nids = []; + uniqueIds.forEach(function(uniqueId) { + nidsToUniqueIds[uniqueIdToNids[uniqueId]] = uniqueId; + nids.push(uniqueIdToNids[uniqueId]); + }); + + UserNotifications.getNotifications(nids, uid, function(err, notifications) { + if (err) { + return callback(err); + } + + notifications.forEach(function(notification, index) { + if (!notification) { + if (process.env.NODE_ENV === 'development') { + winston.info('[notifications.get] nid ' + nids[index] + ' not found. Removing.'); + } + db.sortedSetRemove(set, nidsToUniqueIds[nids[index]]); + db.deleteObjectField('uid:' + uid + ':notifications:uniqueId:nid', nidsToUniqueIds[nids[index]]); + } + }); + + callback(null, notifications); + }); + }); + }); + } + + UserNotifications.getAll = function(uid, limit, callback) { if (!limit || parseInt(limit, 10) <= 0) { limit = 25; } - if (before) { - before = new Date(parseInt(before, 10)); - } db.getObjectValues('uid:' + uid + ':notifications:uniqueId:nid', function(err, nids) { if (err) { return callback(err); } - async.map(nids, function(nid, next) { - notifications.get(nid, function(err, notif_data) { - if (err || !notif_data) { - return next(err); - } - UserNotifications.isNotificationRead(notif_data.nid, uid, function(err, isRead) { - if (err) { - return next(err); - } - - notif_data.read = isRead; - next(null, notif_data); - }); - }); - }, function(err, notifs) { + UserNotifications.getNotifications(nids, uid, function(err, notifs) { if (err) { return callback(err); } - notifs = notifs.filter(function(notif) { - return !!notif; - }).sort(function(a, b) { + notifs = notifs.filter(Boolean).sort(function(a, b) { return parseInt(b.datetime, 10) - parseInt(a.datetime, 10); - }).map(function(notif) { - notif.datetimeISO = utils.toISOString(notif.datetime); - notif.readClass = !notif.read ? 'label-warning' : ''; - return notif; }); callback(null, notifs); @@ -156,8 +115,34 @@ var async = require('async'), }); }; - UserNotifications.isNotificationRead = function(nid, uid, callback) { - db.isSortedSetMember('uid:' + uid + ':notifications:read', nid, callback); + UserNotifications.getNotifications = function(nids, uid, callback) { + notifications.getMultiple(nids, function(err, notifications) { + if (err) { + return callback(err); + } + + var uniqueIds = notifications.map(function(notification) { + return notification ? notification.uniqueId : null; + }); + + db.isSortedSetMembers('uid:' + uid + ':notifications:read', uniqueIds, function(err, hasRead) { + if (err) { + return callback(err); + } + + notifications = notifications.map(function(notification, index) { + if (!notification) { + return null; + } + notification.read = hasRead[index]; + notification.datetimeISO = utils.toISOString(notification.datetime); + notification.readClass = !notification.read ? 'label-warning' : ''; + return notification; + }); + + callback(null, notifications); + }); + }); }; UserNotifications.getDailyUnread = function(uid, callback) { @@ -182,9 +167,7 @@ var async = require('async'), return uniqueIdToNids[uniqueId]; }); - async.map(nids, function(nid, next) { - notifications.get(nid, next); - }, callback); + UserNotifications.getNotifications(nids, uid, callback); }); }); }; @@ -212,15 +195,17 @@ var async = require('async'), return uniqueIdsToNids[uniqueId]; }); - async.filter(nids, function(nid, next) { - notifications.get(nid, function(err, notifObj) { - if (err || !notifObj) { - return next(false); - } + UserNotifications.getNotifications(nids, uid, function(err, notifications) { + if (err) { + return callback(err); + } - next(notifObj[field] === value.toString()); + notifications = notifications.filter(function(notification) { + return !!notification || notification[field] !== value.toString(); + }).map(function(notification) { + return notification.nid; }); - }, function(nids) { + callback(null, nids); }); });