notifications refactor

added getMultiple which works with an array of nids

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
 		});

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) {
+			async.map(notifications, function(notification, next) {
-				winston.info('[notifications.get] Could not retrieve nid ' + nid);
+				if (!notification) {
-				return callback(null, null);
+					return next(null, null);
-			}
+				}
 
-			// Backwards compatibility for old notification schema
+				// Backwards compatibility for old notification schema
-			// Remove this block when NodeBB v0.6.0 is released.
+				// Remove this block when NodeBB v0.6.0 is released.
-			if (notification.hasOwnProperty('text')) {
+				if (notification.hasOwnProperty('text')) {
-				notification.bodyShort = notification.text;
+					notification.bodyShort = notification.text;
-				notification.bodyLong = '';
+					notification.bodyLong = '';
-				notification.text = S(notification.text).escapeHTML().s;
+					notification.text = S(notification.text).escapeHTML().s;
-			}
+				}
 
-			notification.bodyShort = S(notification.bodyShort).escapeHTML().s;
+				notification.bodyShort = S(notification.bodyShort).escapeHTML().s;
-			notification.bodyLong = S(notification.bodyLong).escapeHTML().s;
+				notification.bodyLong = S(notification.bodyLong).escapeHTML().s;
 
-			if (notification.from && !notification.image) {
+				if (notification.from && !notification.image) {
-				User.getUserField(notification.from, 'picture', function(err, picture) {
+					User.getUserField(notification.from, 'picture', function(err, picture) {
-					if (err) {
+						if (err) {
-						return callback(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);
 		});
 	};
 

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) {
+				getNotificationsFromSet('uid:' + uid + ':notifications:unread', uid, 0, 9, maxNotifs, next);
-					notif_data.read = false;
-					notif_data.readClass = !notif_data.read ? 'label-warning' : '';
-					next(null, notif_data);
-				}, next);
 			},
 			read: function(next) {
-				getNotifications('uid:' + uid + ':notifications:read', 0, 9, function(notif_data, next) {
+				getNotificationsFromSet('uid:' + uid + ':notifications:read', uid, 0, 9, maxNotifs, next);
-					notif_data.read = true;
-					next(null, notif_data);
-				}, next);
 			}
 		}, function(err, notifications) {
-			function filterDeleted(notifObj) {
-				return !!notifObj;
-			}
-
 			if (err) {
 				return callback(err);
 			}
 
-			notifications.read = notifications.read.filter(filterDeleted);
+			notifications.read = notifications.read.filter(Boolean);
-			notifications.unread = notifications.unread.filter(filterDeleted);
+			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) {
+	function getNotificationsFromSet(set, uid, start, stop, max, callback) {
-		var	now = new Date();
+		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) {
+			UserNotifications.getNotifications(nids, uid, function(err, notifs) {
-				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) {
 				if (err) {
 					return callback(err);
 				}
 
-				notifs = notifs.filter(function(notif) {
+				notifs = notifs.filter(Boolean).sort(function(a, b) {
-					return !!notif;
-				}).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) {
+	UserNotifications.getNotifications = function(nids, uid, callback) {
-		db.isSortedSetMember('uid:' + uid + ':notifications:read', nid, 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) {
+				UserNotifications.getNotifications(nids, uid, callback);
-					notifications.get(nid, next);
-				}, callback);
 			});
 		});
 	};
@@ -212,15 +195,17 @@ var async = require('async'),
 					return uniqueIdsToNids[uniqueId];
 				});
 
-				async.filter(nids, function(nid, next) {
+				UserNotifications.getNotifications(nids, uid, function(err, notifications) {
-					notifications.get(nid, function(err, notifObj) {
+					if (err) {
-						if (err || !notifObj) {
+						return callback(err);
-							return next(false);
+					}
-						}
 
-						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);
 				});
 			});