From fed8cc6d53388ae1a0acdfe666f3328545ae4fdf Mon Sep 17 00:00:00 2001
From: barisusakli <barisusakli@gmail.com>
Date: Tue, 29 Jul 2014 21:51:46 -0400
Subject: [PATCH] optimize privileges and assorted fixes.

* new methods
privileges.categories.filter
privileges.topics.filter
privileges.posts.filter

they take a list of ids and a privilege, and return the filtered list of
ids, faster than doing  async.filter and calling the db for each id.

* remove event listeners on recent page before adding
* group.exists works for both single group names and arrays
* helpers.allowedTo works for both a single cid and an array of cids
* moved filter:topic.post hook right before topic creation.
* moved filter:topic.reply hook right before topic reply.
---
 public/src/forum/recent.js   |   2 +-
 src/categories.js            |  44 ++++++++-------
 src/groups.js                |   6 +-
 src/posts.js                 |  22 ++++----
 src/privileges/categories.js |  74 ++++++++++++++++++-------
 src/privileges/helpers.js    | 104 +++++++++++++++++++++--------------
 src/privileges/posts.js      |  25 +++++++++
 src/privileges/topics.js     |  30 ++++++++++
 src/search.js                |  10 ++--
 src/topics.js                |  15 +++--
 src/topics/create.js         |  25 ++++-----
 src/topics/unread.js         |  11 ++--
 src/user/notifications.js    |   2 -
 13 files changed, 242 insertions(+), 128 deletions(-)

diff --git a/public/src/forum/recent.js b/public/src/forum/recent.js
index 3f0885690d..b4a27e664b 100644
--- a/public/src/forum/recent.js
+++ b/public/src/forum/recent.js
@@ -56,7 +56,7 @@ define('forum/recent', ['forum/infinitescroll'], function(infinitescroll) {
 	Recent.watchForNewPosts = function () {
 		newPostCount = 0;
 		newTopicCount = 0;
-
+		Recent.removeListeners();
 		socket.on('event:new_topic', onNewTopic);
 		socket.on('event:new_post', onNewPost);
 	};
diff --git a/src/categories.js b/src/categories.js
index 80ed12e621..ceae7d8f01 100644
--- a/src/categories.js
+++ b/src/categories.js
@@ -198,19 +198,21 @@ var db = require('./database'),
 				return callback(null, []);
 			}
 
-			Categories.getCategories(cids, uid, function(err, categories) {
+			privileges.categories.filter('find', cids, uid, function(err, cids) {
 				if (err) {
 					return callback(err);
 				}
-				async.filter(categories, function (category, next) {
-					if (category.disabled) {
-						return next(false);
+
+				Categories.getCategories(cids, uid, function(err, categories) {
+					if (err) {
+						return callback(err);
 					}
-					privileges.categories.can('find', category.cid, uid, function(err, findable) {
-						next(!err && findable);
+
+					categories = categories.filter(function(category) {
+						return !category.disabled;
 					});
-				}, function(visibleCategories) {
-					callback(null, visibleCategories);
+
+					callback(null, categories);
 				});
 			});
 		});
@@ -218,18 +220,15 @@ var db = require('./database'),
 
 	Categories.getModerators = function(cid, callback) {
 		Groups.get('cid:' + cid + ':privileges:mods', {}, function(err, groupObj) {
-			if (!err) {
-				if (groupObj.members && groupObj.members.length) {
-					user.getMultipleUserFields(groupObj.members, ['uid', 'username', 'userslug', 'picture'], function(err, moderators) {
-						callback(err, moderators);
-					});
-				} else {
-					callback(null, []);
-				}
-			} else {
-				// Probably no mods
-				callback(null, []);
+			if (err) {
+				return callback(err);
 			}
+
+			if (!Array.isArray(groupObj) || !groupObj.members.length) {
+				return callback(null, []);
+			}
+
+			user.getMultipleUserFields(groupObj.members, ['uid', 'username', 'userslug', 'picture'], callback);
 		});
 	};
 
@@ -309,11 +308,14 @@ var db = require('./database'),
 	};
 
 	Categories.getCategories = function(cids, uid, callback) {
-
-		if (!Array.isArray(cids) || cids.length === 0) {
+		if (!Array.isArray(cids)) {
 			return callback(new Error('[[error:invalid-cid]]'));
 		}
 
+		if (!cids.length) {
+			 return callback(null, []);
+		}
+
 		async.parallel({
 			categories: function(next) {
 				Categories.getCategoriesData(cids, next);
diff --git a/src/groups.js b/src/groups.js
index 294e8c134f..8164ae457c 100644
--- a/src/groups.js
+++ b/src/groups.js
@@ -183,7 +183,11 @@
 	};
 
 	Groups.exists = function(name, callback) {
-		db.isSetMember('groups', name, callback);
+		if (Array.isArray(name)) {
+			db.isSetMembers('groups', name, callback);
+		} else {
+			db.isSetMember('groups', name, callback);
+		}
 	};
 
 	Groups.create = function(name, description, callback) {
diff --git a/src/posts.js b/src/posts.js
index c272dfb78b..df218c14c8 100644
--- a/src/posts.js
+++ b/src/posts.js
@@ -178,11 +178,14 @@ var async = require('async'),
 				return callback(err);
 			}
 
-			async.filter(pids, function(pid, next) {
-				privileges.posts.can('read', pid, uid, function(err, canRead) {
-					next(!err && canRead);
-				});
-			}, function(pids) {
+			if (!Array.isArray(pids) || !pids.length) {
+				return callback(null, []);
+			}
+
+			privileges.posts.filter('read', pids, uid, function(err, pids) {
+				if (err) {
+					return callback(err);
+				}
 				Posts.getPostSummaryByPids(pids, {stripTags: true}, callback);
 			});
 		});
@@ -487,11 +490,10 @@ var async = require('async'),
 				return callback(err);
 			}
 
-			async.filter(pids, function(pid, next) {
-				privileges.posts.can('read', pid, callerUid, function(err, canRead) {
-					next(!err && canRead);
-				});
-			}, function(pids) {
+			privileges.posts.filter('read', pids, callerUid, function(err, pids) {
+				if (err) {
+					return callback(err);
+				}
 				getPostsFromSet('uid:' + uid + ':posts', pids, callback);
 			});
 		});
diff --git a/src/privileges/categories.js b/src/privileges/categories.js
index 7509808f60..11b4767b30 100644
--- a/src/privileges/categories.js
+++ b/src/privileges/categories.js
@@ -4,6 +4,7 @@
 var async = require('async'),
 
 	user = require('../user'),
+	categories = require('../categories'),
 	groups = require('../groups'),
 	helpers = require('./helpers');
 
@@ -43,17 +44,54 @@ module.exports = function(privileges) {
 	};
 
 	privileges.categories.can = function(privilege, cid, uid, callback) {
-		helpers.some([
-			function(next) {
-				helpers.allowedTo(privilege, uid, cid, next);
+		categories.getCategoryField(cid, 'disabled', function(err, disabled) {
+			if (err) {
+				return callback(err);
+			}
+
+			if (parseInt(disabled, 10) === 1) {
+				return callback(null, false);
+			}
+
+			helpers.some([
+				function(next) {
+					helpers.allowedTo(privilege, uid, cid, next);
+				},
+				function(next) {
+					user.isModerator(uid, cid, next);
+				},
+				function(next) {
+					user.isAdministrator(uid, next);
+				}
+			], callback);
+		});
+	};
+
+	privileges.categories.filter = function(privilege, cids, uid, callback) {
+		async.parallel({
+			allowedTo: function(next) {
+				helpers.allowedTo(privilege, uid, cids, next);
 			},
-			function(next) {
-				user.isModerator(uid, cid, next);
+			isModerators: function(next) {
+				user.isModerator(uid, cids, next);
 			},
-			function(next) {
+			isAdmin: function(next) {
 				user.isAdministrator(uid, next);
 			}
-		], callback);
+		}, function(err, results) {
+			if (err) {
+				return callback(err);
+			}
+
+			if (results.isAdmin) {
+				return callback(null, cids);
+			}
+
+			cids = cids.filter(function(cid, index) {
+				return results.allowedTo[index] || results.isModerators[index];
+			});
+			callback(null, cids);
+		});
 	};
 
 	privileges.categories.canMoveAllTopics = function(currentCid, targetCid, uid, callback) {
@@ -78,15 +116,15 @@ module.exports = function(privileges) {
 
 	privileges.categories.userPrivileges = function(cid, uid, callback) {
 		async.parallel({
-			find: async.apply(helpers.isMember, groups.isMember, 'cid:' + cid + ':privileges:find', uid),
+			find: async.apply(groups.isMember, uid, 'cid:' + cid + ':privileges:find'),
 			read: function(next) {
-				helpers.isMember(groups.isMember, 'cid:' + cid + ':privileges:read', uid, next);
+				groups.isMember(uid, 'cid:' + cid + ':privileges:read', next);
 			},
 			'topics:create': function(next) {
-				helpers.isMember(groups.isMember, 'cid:' + cid + ':privileges:topics:create', uid, next);
+				groups.isMember(uid, 'cid:' + cid + ':privileges:topics:create', next);
 			},
 			'topics:reply': function(next) {
-				helpers.isMember(groups.isMember, 'cid:' + cid + ':privileges:topics:reply', uid, next);
+				groups.isMember(uid, 'cid:' + cid + ':privileges:topics:reply', next);
 			},
 			mods: function(next) {
 				user.isModerator(uid, cid, next);
@@ -96,21 +134,15 @@ module.exports = function(privileges) {
 
 	privileges.categories.groupPrivileges = function(cid, groupName, callback) {
 		async.parallel({
-			'groups:find': async.apply(helpers.isMember, groups.isMember, 'cid:' + cid + ':privileges:groups:find', groupName),
+			'groups:find': async.apply(groups.isMember, groupName, 'cid:' + cid + ':privileges:groups:find'),
 			'groups:read': function(next) {
-				helpers.isMember(groups.isMember, 'cid:' + cid + ':privileges:groups:read', groupName, function(err, isMember){
-					next(err, !!isMember);
-				});
+				groups.isMember(groupName, 'cid:' + cid + ':privileges:groups:read', next);
 			},
 			'groups:topics:create': function(next) {
-				helpers.isMember(groups.isMember, 'cid:' + cid + ':privileges:groups:topics:create', groupName, function(err, isMember){
-					next(err, !!isMember);
-				});
+				groups.isMember(groupName, 'cid:' + cid + ':privileges:groups:topics:create', next);
 			},
 			'groups:topics:reply': function(next) {
-				helpers.isMember(groups.isMember, 'cid:' + cid + ':privileges:groups:topics:reply', groupName, function(err, isMember){
-					next(err, !!isMember);
-				});
+				groups.isMember(groupName, 'cid:' + cid + ':privileges:groups:topics:reply', next);
 			}
 		}, callback);
 	};
diff --git a/src/privileges/helpers.js b/src/privileges/helpers.js
index 642cf36b26..778f3a84df 100644
--- a/src/privileges/helpers.js
+++ b/src/privileges/helpers.js
@@ -2,7 +2,7 @@
 'use strict';
 
 var async = require('async'),
-
+	db = require('../database'),
 	meta = require('../meta'),
 	user = require('../user'),
 	groups = require('../groups'),
@@ -20,67 +20,87 @@ helpers.some = function(tasks, callback) {
 	});
 };
 
-helpers.allowedTo = function(privilege, uid, cid, callback) {
-	categories.getCategoryField(cid, 'disabled', function(err, disabled) {
+helpers.allowedTo = function(privilege, uid, cids, callback) {
+
+	if (!Array.isArray(cids)) {
+		cids = [cids];
+	}
+
+	if (parseInt(uid, 10) === 0) {
+		return isGuestAllowedTo(privilege, cids, callback);
+	}
+
+	var userKeys = [], groupKeys = [];
+	for (var i=0; i<cids.length; ++i) {
+		userKeys.push('cid:' + cids[i] + ':privileges:' + privilege);
+		groupKeys.push('cid:' + cids[i] + ':privileges:groups:' + privilege);
+	}
+
+	async.parallel({
+		userPrivilegeExists: function(next) {
+			groups.exists(userKeys, next);
+		},
+		groupPrivilegeExists: function(next) {
+			groups.exists(groupKeys, next);
+		},
+		hasUserPrivilege: function(next) {
+			groups.isMemberOfGroups(uid, userKeys, next);
+		},
+		hasGroupPrivilege: function(next) {
+			async.map(groupKeys, function(groupKey, next) {
+				groups.isMemberOfGroupList(uid, groupKey, next);
+			}, next);
+		}
+	}, function(err, results) {
 		if (err) {
 			return callback(err);
 		}
 
-		if (parseInt(disabled, 10) === 1) {
-			return callback(null, false);
+		var result = [];
+		for (var i=0; i<cids.length; ++i) {
+			result.push((!results.userPrivilegeExists[i] && !results.groupPrivilegeExists[i]) || results.hasUserPrivilege[i] || results.hasGroupPrivilege[i]);
 		}
 
-		if (parseInt(uid, 10) === 0) {
-			return isGuestAllowedTo(privilege, cid, callback);
+		if (result.length === 1) {
+			result = result[0];
 		}
 
-		async.parallel({
-			hasUserPrivilege: function(next) {
-				helpers.isMember(groups.isMember, 'cid:' + cid + ':privileges:' + privilege, uid, next);
-			},
-			hasGroupPrivilege: function(next) {
-				helpers.isMember(groups.isMemberOfGroupList, 'cid:' + cid + ':privileges:groups:' + privilege, uid, next);
-			}
-		}, function(err, results) {
-			if (err) {
-				return callback(err);
-			}
-
-			callback(null, (results.hasUserPrivilege === null && results.hasGroupPrivilege === null) || results.hasUserPrivilege || results.hasGroupPrivilege);
-		});
+		callback(null, result);
 	});
 };
 
-function isGuestAllowedTo(privilege, cid, callback) {
-	async.parallel([
-		function(next) {
-			groups.exists('cid:' + cid + ':privileges:' + privilege, function(err, exists) {
-				next(err, !err ? !exists : false);
-			});
+function isGuestAllowedTo(privilege, cids, callback) {
+	var userKeys = [], groupKeys = [];
+	for (var i=0; i<cids.length; ++i) {
+		userKeys.push('cid:' + cids[i] + ':privileges:' + privilege);
+		groupKeys.push('cid:' + cids[i] + ':privileges:groups:' + privilege);
+	}
+
+	async.parallel({
+		userPrivilegeExists: function(next) {
+			groups.exists(userKeys, next);
 		},
-		function(next) {
-			helpers.isMember(groups.isMember, 'cid:' + cid + ':privileges:groups:' + privilege, 'guests', function(err, isMember) {
-				next(err, privilege !== 'find' && privilege !== 'read' ? isMember === true : isMember !== false);
-			});
+		hasGroupPrivilege: function(next) {
+			groups.isMemberOfGroups('guests', groupKeys, next);
 		}
-	], function(err, results) {
-		callback(err, results[0] && (results[1] || results[1] === null));
-	});
-}
-
-helpers.isMember = function(method, group, uid, callback) {
-	groups.exists(group, function(err, exists) {
+	}, function(err, results) {
 		if (err) {
 			return callback(err);
 		}
 
-		if (!exists) {
-			return callback(null, null);
+		var result = [];
+		for (var i = 0; i<cids.length; ++i) {
+			var groupPriv = privilege !== 'find' && privilege !== 'read' ? results.hasGroupPrivilege[i] === true : results.hasGroupPrivilege[i] !== false;
+			result.push(!results.userPrivilegeExists[i] && groupPriv);
+		}
+
+		if (result.length === 1) {
+			result = result[0];
 		}
 
-		method(uid, group, callback);
+		callback(null, result);
 	});
-};
+}
 
 helpers.hasEnoughReputationFor = function(privilege, uid, callback) {
 	if (parseInt(meta.config['privileges:disabled'], 10)) {
diff --git a/src/privileges/posts.js b/src/privileges/posts.js
index df73afe97b..71df14ae36 100644
--- a/src/privileges/posts.js
+++ b/src/privileges/posts.js
@@ -76,6 +76,31 @@ module.exports = function(privileges) {
 		});
 	};
 
+	privileges.posts.filter = function(privilege, pids, uid, callback) {
+		posts.getCidsByPids(pids, function(err, cids) {
+			if (err) {
+				return callback(err);
+			}
+
+			pids = pids.map(function(pid, index) {
+				return {pid: pid, cid: cids[index]};
+			});
+
+			privileges.categories.filter(privilege, cids, uid, function(err, cids) {
+				if (err) {
+					return callback(err);
+				}
+
+				pids = pids.filter(function(post) {
+					return cids.indexOf(post.cid) !== -1;
+				}).map(function(post) {
+					return post.pid;
+				});
+				callback(null, pids);
+			});
+		});
+	};
+
 	privileges.posts.canEdit = function(pid, uid, callback) {
 		helpers.some([
 			function(next) {
diff --git a/src/privileges/topics.js b/src/privileges/topics.js
index ef0a2cb23d..f263f3823b 100644
--- a/src/privileges/topics.js
+++ b/src/privileges/topics.js
@@ -3,6 +3,7 @@
 
 var async = require('async'),
 
+	db = require('../database'),
 	topics = require('../topics'),
 	user = require('../user'),
 	helpers = require('./helpers'),
@@ -69,6 +70,35 @@ module.exports = function(privileges) {
 		});
 	};
 
+	privileges.topics.filter = function(privilege, tids, uid, callback) {
+		var keys = tids.map(function(tid) {
+			return 'topic:' + tid;
+		});
+
+		db.getObjectsFields(keys, ['tid', 'cid'], function(err, topics) {
+			if (err) {
+				return callback(err);
+			}
+
+			var cids = topics.map(function(topic) {
+				return topic.cid;
+			});
+
+			privileges.categories.filter(privilege, cids, uid, function(err, cids) {
+				if (err) {
+					return callback(err);
+				}
+
+				tids = topics.filter(function(topic) {
+					return cids.indexOf(topic.cid) !== -1;
+				}).map(function(topic) {
+					return topic.tid;
+				});
+				callback(null, tids);
+			});
+		});
+	};
+
 	privileges.topics.canEdit = function(tid, uid, callback) {
 		helpers.some([
 			function(next) {
diff --git a/src/search.js b/src/search.js
index 8d10ede1c2..808958ad73 100644
--- a/src/search.js
+++ b/src/search.js
@@ -45,11 +45,11 @@ search.search = function(term, uid, callback) {
 				}
 			});
 
-			async.filter(mainPids, function(pid, next) {
-				privileges.posts.can('read', pid, uid, function(err, canRead) {
-					next(!err && canRead);
-				});
-			}, function(pids) {
+			privileges.posts.filter('read', mainPids, uid, function(err, pids) {
+				if (err) {
+					return callback(err);
+				}
+
 				posts.getPostSummaryByPids(pids, {stripTags: true, parse: false}, function(err, posts) {
 					if (err) {
 						return callback(err);
diff --git a/src/topics.js b/src/topics.js
index 66d2ee3d02..0214f912bf 100644
--- a/src/topics.js
+++ b/src/topics.js
@@ -132,11 +132,11 @@ var async = require('async'),
 			return callback(null, returnTopics);
 		}
 
-		async.filter(tids, function(tid, next) {
-			privileges.topics.can('read', tid, uid, function(err, canRead) {
-				next(!err && canRead);
-			});
-		}, function(tids) {
+		privileges.topics.filter('read', tids, uid, function(err, tids) {
+			if (err) {
+				return callback(err);
+			}
+
 			Topics.getTopicsByTids(tids, uid, function(err, topicData) {
 				if(err) {
 					return callback(err);
@@ -185,6 +185,9 @@ var async = require('async'),
 			}
 
 			function isTopicVisible(topicData, topicInfo) {
+				if (parseInt(topicInfo.categoryData.disabled, 10) === 1) {
+					return false;
+				}
 				var deleted = parseInt(topicData.deleted, 10) !== 0;
 				return !deleted || (deleted && topicInfo.privileges.view_deleted) || parseInt(topicData.uid, 10) === parseInt(uid, 10);
 			}
@@ -206,7 +209,7 @@ var async = require('async'),
 					if (categoryCache[topicData.cid]) {
 						return next(null, categoryCache[topicData.cid]);
 					}
-					categories.getCategoryFields(topicData.cid, ['name', 'slug', 'icon', 'bgColor', 'color'], next);
+					categories.getCategoryFields(topicData.cid, ['name', 'slug', 'icon', 'bgColor', 'color', 'disabled'], next);
 				},
 				user: function(next) {
 					if (userCache[topicData.uid]) {
diff --git a/src/topics/create.js b/src/topics/create.js
index fcf3829f80..01a334b128 100644
--- a/src/topics/create.js
+++ b/src/topics/create.js
@@ -92,16 +92,6 @@ module.exports = function(Topics) {
 		}
 
 		async.waterfall([
-			function(next) {
-				plugins.fireHook('filter:topic.post', data, function(err, filteredData) {
-					if (err) {
-						return next(err);
-					}
-
-					content = filteredData.content || data.content;
-					next();
-				});
-			},
 			function(next) {
 				categories.exists(cid, next);
 			},
@@ -120,6 +110,13 @@ module.exports = function(Topics) {
 			function(next) {
 				user.isReadyToPost(uid, next);
 			},
+			function(next) {
+				plugins.fireHook('filter:topic.post', data, next);
+			},
+			function(filteredData, next) {
+				content = filteredData.content || data.content;
+				next();
+			},
 			function(next) {
 				Topics.create({uid: uid, title: title, cid: cid, thumb: data.thumb, tags: data.tags}, next);
 			},
@@ -161,10 +158,6 @@ module.exports = function(Topics) {
 
 		async.waterfall([
 			function(next) {
-				plugins.fireHook('filter:topic.reply', data, next);
-			},
-			function(filteredData, next) {
-				content = filteredData.content || data.content;
 				threadTools.exists(tid, next);
 			},
 			function(topicExists, next) {
@@ -191,6 +184,10 @@ module.exports = function(Topics) {
 				user.isReadyToPost(uid, next);
 			},
 			function(next) {
+				plugins.fireHook('filter:topic.reply', data, next);
+			},
+			function(filteredData, next) {
+				content = filteredData.content || data.content;
 				if (content) {
 					content = content.trim();
 				}
diff --git a/src/topics/unread.js b/src/topics/unread.js
index 7316bf7759..d33603108a 100644
--- a/src/topics/unread.js
+++ b/src/topics/unread.js
@@ -44,15 +44,16 @@ module.exports = function(Topics) {
 					if(err) {
 						return callback(err);
 					}
+
 					var newtids = tids.filter(function(tid, index, self) {
 						return !read[index];
 					});
 
-					async.filter(newtids, function(tid, next) {
-						privileges.topics.can('read', tid, uid, function(err, canRead) {
-							next(!err && canRead);
-						});
-					}, function(newtids) {
+					privileges.topics.filter('read', newtids, uid, function(err, newTids) {
+						if(err) {
+							return callback(err);
+						}
+
 						unreadTids.push.apply(unreadTids, newtids);
 
 						start = stop + 1;
diff --git a/src/user/notifications.js b/src/user/notifications.js
index 7f1bb54b5e..f6c8832dbe 100644
--- a/src/user/notifications.js
+++ b/src/user/notifications.js
@@ -17,8 +17,6 @@ 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) {