Categories optimize (#6999)

* WIP

* fix category page

* fix counts, and copyPrivileges

* fix lint

* more fixes,

* redis fix

* fix test

* fix  category test

* remove getParentsAndChildren

src/categories/data.js

@@ -7,7 +7,7 @@ var db = require('../database');
 
 const intFields = [
 	'cid', 'parentCid', 'disabled', 'isSection', 'order',
-	'topic_count', 'post_count',
+	'topic_count', 'post_count', 'numRecentReplies',
 ];
 
 module.exports = function (Categories) {

src/categories/index.js

@@ -47,14 +47,26 @@ Categories.getCategoryById = function (data, callback) {
 				isIgnored: function (next) {
 					Categories.isIgnored([data.cid], data.uid, next);
 				},
+				parent: function (next) {
+					if (category.parentCid) {
+						Categories.getCategoryData(category.parentCid, next);
+					} else {
+						next();
+					}
+				},
+				children: function (next) {
+					getChildrenTree(category, data.uid, next);
+				},
 			}, next);
 		},
 		function (results, next) {
 			category.topics = results.topics.topics;
 			category.nextStart = results.topics.nextStart;
-			category.isIgnored = results.isIgnored[0];
 			category.topic_count = results.topicCount;
+			category.isIgnored = results.isIgnored[0];
+			category.parent = results.parent;
 
+			calculateTopicPostCount(category);
 			plugins.fireHook('filter:category.get', { category: category, uid: data.uid }, next);
 		},
 		function (data, next) {
@@ -123,7 +135,6 @@ Categories.getCategories = function (cids, uid, callback) {
 		return callback(null, []);
 	}
 	uid = parseInt(uid, 10);
-	let results;
 	async.waterfall([
 		function (next) {
 			async.parallel({
@@ -138,19 +149,14 @@ Categories.getCategories = function (cids, uid, callback) {
 				},
 			}, next);
 		},
-		function (_results, next) {
-			results = _results;
-			Categories.getParentsAndChildren(results.categories, uid, next);
-		},
-		function (categories, next) {
-			categories.forEach(function (category, i) {
+		function (results, next) {
+			results.categories.forEach(function (category, i) {
 				if (category) {
 					category.tagWhitelist = results.tagWhitelist[i];
 					category['unread-class'] = (category.topic_count === 0 || (results.hasRead[i] && uid !== 0)) ? '' : 'unread';
-					calculateTopicPostCount(category);
 				}
 			});
-			next(null, categories);
+			next(null, results.categories);
 		},
 	], callback);
 };
@@ -209,31 +215,6 @@ Categories.getParents = function (cids, callback) {
 	], callback);
 };
 
-Categories.getParentsAndChildren = function (categoryData, uid, callback) {
-	const parentCids = categoryData.filter(c => c && c.parentCid).map(c => c.parentCid);
-	async.waterfall([
-		function (next) {
-			async.parallel({
-				parents: function (next) {
-					Categories.getCategoriesData(parentCids, next);
-				},
-				children: function (next) {
-					async.each(categoryData, function (category, next) {
-						getChildrenRecursive(category, uid, next);
-					}, next);
-				},
-			}, next);
-		},
-		function (results, next) {
-			const cidToParent = _.zipObject(parentCids, results.parents);
-			categoryData.forEach(function (category) {
-				category.parent = cidToParent[category.parentCid];
-			});
-			next(null, categoryData);
-		},
-	], callback);
-};
-
 Categories.getChildren = function (cids, uid, callback) {
 	var categories;
 	async.waterfall([
@@ -243,7 +224,7 @@ Categories.getChildren = function (cids, uid, callback) {
 		function (categoryData, next) {
 			categories = categoryData.map((category, index) => ({ cid: cids[index], parentCid: category.parentCid }));
 			async.each(categories, function (category, next) {
-				getChildrenRecursive(category, uid, next);
+				getChildrenTree(category, uid, next);
 			}, next);
 		},
 		function (next) {
@@ -252,10 +233,11 @@ Categories.getChildren = function (cids, uid, callback) {
 	], callback);
 };
 
-function getChildrenRecursive(category, uid, callback) {
+function getChildrenTree(category, uid, callback) {
+	let children;
 	async.waterfall([
 		function (next) {
-			db.getSortedSetRange('cid:' + category.cid + ':children', 0, -1, next);
+			Categories.getChildrenCids(category.cid, next);
 		},
 		function (children, next) {
 			privileges.categories.filterCids('find', children, uid, next);
@@ -268,34 +250,27 @@ function getChildrenRecursive(category, uid, callback) {
 			}
 			Categories.getCategoriesData(children, next);
 		},
-		function (children, next) {
-			children = children.filter(Boolean);
-			category.children = children;
-
-			var cids = children.map(child => child.cid);
+		function (_children, next) {
+			children = _children.filter(Boolean);
 
+			const cids = children.map(child => child.cid);
 			Categories.hasReadCategories(cids, uid, next);
 		},
 		function (hasRead, next) {
 			hasRead.forEach(function (read, i) {
-				var child = category.children[i];
+				const child = children[i];
 				child['unread-class'] = (child.topic_count === 0 || (read && uid !== 0)) ? '' : 'unread';
 			});
-
-			async.each(category.children, function (child, next) {
-				if (parseInt(category.parentCid, 10) === parseInt(child.cid, 10)) {
-					return next();
-				}
-				getChildrenRecursive(child, uid, next);
-			}, next);
+			Categories.getTree([category].concat(children), category.parentCid);
+			next();
 		},
 	], callback);
 }
 
 Categories.getChildrenCids = function (rootCid, callback) {
 	var allCids = [];
-	function recursive(currentCid, callback) {
-		db.getSortedSetRange('cid:' + currentCid + ':children', 0, -1, function (err, childrenCids) {
+	function recursive(keys, callback) {
+		db.getSortedSetRange(keys, 0, -1, function (err, childrenCids) {
 			if (err) {
 				return callback(err);
 			}
@@ -303,14 +278,13 @@ Categories.getChildrenCids = function (rootCid, callback) {
 			if (!childrenCids.length) {
 				return callback();
 			}
-			async.eachSeries(childrenCids, function (childCid, next) {
-				allCids.push(parseInt(childCid, 10));
-				recursive(childCid, next);
-			}, callback);
+			const keys = childrenCids.map(cid => 'cid:' + cid + ':children');
+			childrenCids.forEach(cid => allCids.push(parseInt(cid, 10)));
+			recursive(keys, callback);
 		});
 	}
 
-	recursive(rootCid, function (err) {
+	recursive('cid:' + rootCid + ':children', function (err) {
 		callback(err, _.uniq(allCids));
 	});
 };
@@ -318,9 +292,7 @@ Categories.getChildrenCids = function (rootCid, callback) {
 Categories.flattenCategories = function (allCategories, categoryData) {
 	categoryData.forEach(function (category) {
 		if (category) {
-			if (!category.parent) {
-				allCategories.push(category);
-			}
+			allCategories.push(category);
 
 			if (Array.isArray(category.children) && category.children.length) {
 				Categories.flattenCategories(allCategories, category.children);
@@ -336,30 +308,45 @@ Categories.flattenCategories = function (allCategories, categoryData) {
  * @param parentCid {number} start from 0 to build full tree
  */
 Categories.getTree = function (categories, parentCid) {
-	var tree = [];
-
-	categories.forEach(function (category) {
-		if (category) {
-			if (!category.hasOwnProperty('parentCid') || category.parentCid === null) {
-				category.parentCid = 0;
-			}
+	const cids = categories.map(category => category.cid);
+	const cidToCategory = _.zipObject(cids, _.cloneDeep(categories));
+	const tree = buildRecursive(categories, parentCid || 0);
+
+	function buildRecursive(categories, parentCid) {
+		var tree = [];
+
+		categories.forEach(function (category) {
+			if (category) {
+				category.children = category.children || [];
+				if (!category.cid) {
+					return;
+				}
+				if (!category.hasOwnProperty('parentCid') || category.parentCid === null) {
+					category.parentCid = 0;
+				}
 
-			if (category.parentCid === parentCid) {
-				tree.push(category);
-				category.children = Categories.getTree(categories, category.cid);
+				if (category.parentCid === parentCid) {
+					tree.push(category);
+					category.parent = cidToCategory[parentCid];
+					category.children = buildRecursive(categories, category.cid);
+				}
 			}
-		}
-	});
+		});
+		tree.sort((a, b) => a.order - b.order);
+		return tree;
+	}
 
+	categories.forEach(c => calculateTopicPostCount(c));
 	return tree;
 };
 
 Categories.buildForSelect = function (uid, privilege, callback) {
 	async.waterfall([
 		function (next) {
-			Categories.getCategoriesByPrivilege('cid:0:children', uid, privilege, next);
+			Categories.getCategoriesByPrivilege('categories:cid', uid, privilege, next);
 		},
 		function (categories, next) {
+			categories = Categories.getTree(categories);
 			Categories.buildForSelectCategories(categories, next);
 		},
 	], callback);
@@ -373,10 +360,11 @@ Categories.buildForSelectCategories = function (categories, callback) {
 		category.text = level + bullet + category.name;
 		category.depth = depth;
 		categoriesData.push(category);
-
-		category.children.forEach(function (child) {
-			recursive(child, categoriesData, '    ' + level, depth + 1);
-		});
+		if (Array.isArray(category.children)) {
+			category.children.forEach(function (child) {
+				recursive(child, categoriesData, '    ' + level, depth + 1);
+			});
+		}
 	}
 
 	var categoriesData = [];

src/controllers/admin/categories.js

@@ -14,6 +14,7 @@ categoriesController.get = function (req, res, callback) {
 		function (next) {
 			async.parallel({
 				category: async.apply(categories.getCategories, [req.params.category_id], req.uid),
+				parent: async.apply(categories.getParent, [req.params.category_id]),
 				allCategories: async.apply(categories.buildForSelect, req.uid, 'read'),
 			}, next);
 		},
@@ -23,7 +24,7 @@ categoriesController.get = function (req, res, callback) {
 			if (!category) {
 				return callback();
 			}
-
+			category.parent = data.parent[0];
 			data.allCategories.forEach(function (category) {
 				if (category) {
 					category.selected = parseInt(category.cid, 10) === parseInt(req.params.category_id, 10);

src/controllers/admin/privileges.js

@@ -23,12 +23,13 @@ privilegesController.get = function (req, res, callback) {
 				allCategories: function (next) {
 					async.waterfall([
 						function (next) {
-							db.getSortedSetRange('cid:0:children', 0, -1, next);
+							db.getSortedSetRange('categories:cid', 0, -1, next);
 						},
 						function (cids, next) {
 							categories.getCategories(cids, req.uid, next);
 						},
 						function (categoriesData, next) {
+							categoriesData = categories.getTree(categoriesData);
 							categories.buildForSelectCategories(categoriesData, next);
 						},
 					], next);

src/controllers/categories.js

@@ -19,22 +19,21 @@ categoriesController.list = function (req, res, next) {
 	}];
 
 	var categoryData;
+	let tree;
 	async.waterfall([
 		function (next) {
-			categories.getCategoriesByPrivilege('cid:0:children', req.uid, 'find', next);
+			categories.getCategoriesByPrivilege('categories:cid', req.uid, 'find', next);
 		},
 		function (_categoryData, next) {
 			categoryData = _categoryData;
 
-			var allCategories = [];
-			categories.flattenCategories(allCategories, categoryData);
-
-			categories.getRecentTopicReplies(allCategories, req.uid, next);
+			tree = categories.getTree(categoryData, 0);
+			categories.getRecentTopicReplies(categoryData, req.uid, next);
 		},
 		function () {
 			var data = {
 				title: meta.config.homePageTitle || '[[pages:home]]',
-				categories: categoryData,
+				categories: tree,
 			};
 
 			if (req.originalUrl.startsWith(nconf.get('relative_path') + '/api/categories') || req.originalUrl.startsWith(nconf.get('relative_path') + '/categories')) {

src/controllers/helpers.js

@@ -288,7 +288,7 @@ function getCategoryData(cids, uid, selectedCid, callback) {
 			}
 
 			var categoriesData = [];
-			var tree = categories.getTree(categoryData, 0);
+			var tree = categories.getTree(categoryData);
 
 			tree.forEach(function (category) {
 				recursive(category, categoriesData, '');

src/database/redis/sorted.js

@@ -1,6 +1,7 @@
 'use strict';
 
 module.exports = function (redisClient, module) {
+	var _ = require('lodash');
 	var utils = require('../../utils');
 
 	var helpers = module.helpers.redis;
@@ -28,7 +29,33 @@ module.exports = function (redisClient, module) {
 
 	function sortedSetRange(method, key, start, stop, withScores, callback) {
 		if (Array.isArray(key)) {
-			return module.sortedSetUnion({ method: method, sets: key, start: start, stop: stop, withScores: withScores }, callback);
+			const batch = redisClient.batch();
+			key.forEach((key) => {
+				batch[method]([key, start, stop, 'WITHSCORES']);
+			});
+			batch.exec(function (err, data) {
+				if (err) {
+					return callback(err);
+				}
+				data = _.flatten(data);
+				var objects = [];
+				for (var i = 0; i < data.length; i += 2) {
+					objects.push({ value: data[i], score: parseFloat(data[i + 1]) });
+				}
+
+				objects.sort((a, b) => {
+					if (method === 'zrange') {
+						return a.score - b.score;
+					}
+					return b.score - a.score;
+				});
+				if (withScores) {
+					return callback(null, objects);
+				}
+				objects = objects.map(item => item.value);
+				callback(null, objects);
+			});
+			return;
 		}
 
 		var params = [key, start, stop];

src/install.js

@@ -392,14 +392,14 @@ function giveGlobalPrivileges(next) {
 
 function createCategories(next) {
 	var Categories = require('./categories');
-
-	Categories.getAllCategories(0, function (err, categoryData) {
+	var db = require('./database');
+	db.getSortedSetRange('categories:cid', 0, -1, function (err, cids) {
 		if (err) {
 			return next(err);
 		}
 
-		if (Array.isArray(categoryData) && categoryData.length) {
-			console.log('Categories OK. Found ' + categoryData.length + ' categories.');
+		if (Array.isArray(cids) && cids.length) {
+			console.log('Categories OK. Found ' + cids.length + ' categories.');
 			return next();
 		}
 

src/socket.io/admin/categories.js

@@ -105,12 +105,12 @@ Categories.getPrivilegeSettings = function (socket, cid, callback) {
 Categories.copyPrivilegesToChildren = function (socket, cid, callback) {
 	async.waterfall([
 		function (next) {
-			categories.getCategories([cid], socket.uid, next);
+			categories.getChildren([cid], socket.uid, next);
 		},
-		function (categories, next) {
-			var category = categories[0];
+		function (children, next) {
+			children = children[0];
 
-			async.eachSeries(category.children, function (child, next) {
+			async.eachSeries(children, function (child, next) {
 				copyPrivilegesToChildrenRecursive(cid, child, next);
 			}, next);
 		},

src/socket.io/categories.js

@@ -139,12 +139,13 @@ SocketCategories.getMoveCategories = function (socket, data, callback) {
 				categories: function (next) {
 					async.waterfall([
 						function (next) {
-							db.getSortedSetRange('cid:0:children', 0, -1, next);
+							db.getSortedSetRange('categories:cid', 0, -1, next);
 						},
 						function (cids, next) {
 							categories.getCategories(cids, socket.uid, next);
 						},
 						function (categoriesData, next) {
+							categoriesData = categories.getTree(categoriesData);
 							categories.buildForSelectCategories(categoriesData, next);
 						},
 					], next);

test/database/sorted.js

@@ -147,6 +147,24 @@ describe('Sorted Set methods', function () {
 				done();
 			});
 		});
+
+		it('should return duplicates if two sets have same elements', function (done) {
+			async.waterfall([
+				function (next) {
+					db.sortedSetAdd('dupezset1', [1, 2], ['value 1', 'value 2'], next);
+				},
+				function (next) {
+					db.sortedSetAdd('dupezset2', [2, 3], ['value 2', 'value 3'], next);
+				},
+				function (next) {
+					db.getSortedSetRange(['dupezset1', 'dupezset2'], 0, -1, next);
+				},
+				function (data, next) {
+					assert.deepStrictEqual(data, ['value 1', 'value 2', 'value 2', 'value 3']);
+					next();
+				},
+			], done);
+		});
 	});
 
 	describe('getSortedSetRevRange()', function () {