fix: #8287, dont readd user after deletion

don't add user uid back to users:* sorted sets if they are deleted
upgrade script to fix users:* sorted sets

src/posts/user.js

@@ -1,16 +1,16 @@
 'use strict';
 
-var async = require('async');
+const async = require('async');
-var validator = require('validator');
+const validator = require('validator');
-var _ = require('lodash');
+const _ = require('lodash');
 
 const db = require('../database');
-var user = require('../user');
+const user = require('../user');
 const topics = require('../topics');
-var groups = require('../groups');
+const groups = require('../groups');
-var meta = require('../meta');
+const meta = require('../meta');
-var plugins = require('../plugins');
+const plugins = require('../plugins');
-var privileges = require('../privileges');
+const privileges = require('../privileges');
 
 module.exports = function (Posts) {
 	Posts.getUserInfoForPosts = async function (uids, uid) {
@@ -158,7 +158,7 @@ module.exports = function (Posts) {
 			db.sortedSetRemoveBulk(bulkRemove),
 			db.sortedSetAddBulk(bulkAdd),
 			user.incrementUserPostCountBy(toUid, pids.length),
-			updateReputation(toUid, repChange),
+			user.incrementUserReputationBy(toUid, repChange),
 			handleMainPidOwnerChange(postData, toUid),
 			reduceCounters(postsByUser),
 			updateTopicPosters(postData, toUid),
@@ -171,7 +171,7 @@ module.exports = function (Posts) {
 			const repChange = posts.reduce((acc, val) => acc + val.votes, 0);
 			await Promise.all([
 				user.incrementUserPostCountBy(uid, -posts.length),
-				updateReputation(uid, -repChange),
+				user.incrementUserReputationBy(uid, -repChange),
 			]);
 		});
 	}
@@ -187,14 +187,6 @@ module.exports = function (Posts) {
 		});
 	}
 
-	async function updateReputation(uid, change) {
-		if (!change) {
-			return;
-		}
-		const newReputation = await user.incrementUserFieldBy(uid, 'reputation', change);
-		await db.sortedSetAdd('users:reputation', newReputation, uid);
-	}
-
 	async function handleMainPidOwnerChange(postData, toUid) {
 		const tids = _.uniq(postData.map(p => p.tid));
 		const topicData = await topics.getTopicsFields(tids, ['mainPid', 'timestamp']);
@@ -230,7 +222,10 @@ module.exports = function (Posts) {
 	async function reduceTopicCounts(postsByUser) {
 		await async.eachSeries(Object.keys(postsByUser), async function (uid) {
 			const posts = postsByUser[uid];
-			await user.incrementUserFieldBy(uid, 'topiccount', -posts.length);
+			const exists = await user.exists(uid);
+			if (exists) {
+				await user.incrementUserFieldBy(uid, 'topiccount', -posts.length);
+			}
 		});
 	}
 };

src/posts/votes.js

@@ -179,10 +179,7 @@ module.exports = function (Posts) {
 		}
 
 		const postData = await Posts.getPostFields(pid, ['pid', 'uid', 'tid']);
-		const newReputation = await user[type === 'upvote' ? 'incrementUserFieldBy' : 'decrementUserFieldBy'](postData.uid, 'reputation', 1);
+		const newReputation = user.incrementUserReputationBy(postData.uid, type === 'upvote' ? 1 : -1);
-		if (parseInt(postData.uid, 10)) {
-			await db.sortedSetAdd('users:reputation', newReputation, postData.uid);
-		}
 
 		await adjustPostVotes(postData, uid, type, unvote);
 

src/upgrades/1.13.3/fix_users_sorted_sets.js

@@ -0,0 +1,58 @@
+'use strict';
+
+const db = require('../../database');
+const batch = require('../../batch');
+
+module.exports = {
+	name: 'Fix user sorted sets',
+	timestamp: Date.UTC(2020, 4, 2),
+	method: async function (callback) {
+		const progress = this.progress;
+		const nextUid = await db.getObjectField('global', 'nextUid');
+		const allUids = [];
+		for (let i = 1; i <= nextUid; i++) {
+			allUids.push(i);
+		}
+
+		progress.total = nextUid;
+		let totalUserCount = 0;
+
+		await db.delete('user:null');
+		await db.sortedSetsRemove([
+			'users:joindate',
+			'users:reputation',
+			'users:postcount',
+		], 'null');
+
+		await batch.processArray(allUids, async function (uids) {
+			progress.incr(uids.length);
+			const userData = await db.getObjects(uids.map(id => 'user:' + id));
+
+			await Promise.all(userData.map(async function (userData, index) {
+				if (!userData || !userData.uid) {
+					await db.sortedSetsRemove([
+						'users:joindate',
+						'users:reputation',
+						'users:postcount',
+					], uids[index]);
+					if (userData && !userData.uid) {
+						await db.delete('user:' + uids[index]);
+					}
+					return;
+				}
+				totalUserCount += 1;
+				await db.sortedSetAddBulk([
+					['users:joindate', userData.joindate, uids[index]],
+					['users:reputation', userData.reputation, uids[index]],
+					['users:postcount', userData.postcount, uids[index]],
+				]);
+			}));
+		}, {
+			progress: progress,
+			batch: 500,
+		});
+
+		await db.setObjectField('global', 'userCount', totalUserCount);
+		callback();
+	},
+};

src/user/index.js

@@ -41,7 +41,7 @@ require('./blocks')(User);
 require('./uploads')(User);
 
 User.exists = async function (uid) {
-	return await db.exists('user:' + uid);
+	return await db.isSortedSetMember('users:joindate', uid);
 };
 
 User.existsBySlug = async function (userslug) {

src/user/posts.js

@@ -67,12 +67,26 @@ module.exports = function (User) {
 	};
 
 	User.incrementUserPostCountBy = async function (uid, value) {
-		const newpostcount = await User.incrementUserFieldBy(uid, 'postcount', value);
+		return await incrementUserFieldAndSetBy(uid, 'postcount', 'users:postcount', value);
-		if (parseInt(uid, 10) <= 0) {
+	};
+
+	User.incrementUserReputationBy = async function (uid, value) {
+		return await incrementUserFieldAndSetBy(uid, 'reputation', 'users:reputation', value);
+	};
+
+	async function incrementUserFieldAndSetBy(uid, field, set, value) {
+		value = parseInt(value, 10);
+		if (!value || !field || !(parseInt(uid, 10) > 0)) {
 			return;
 		}
-		await db.sortedSetAdd('users:postcount', newpostcount, uid);
+		const exists = await User.exists(uid);
-	};
+		if (!exists) {
+			return;
+		}
+		const newValue = await User.incrementUserFieldBy(uid, field, value);
+		await db.sortedSetAdd(set, newValue, uid);
+		return newValue;
+	}
 
 	User.getPostIds = async function (uid, start, stop) {
 		const pids = await db.getSortedSetRevRange('uid:' + uid + ':posts', start, stop);

test/user.js

@@ -11,6 +11,7 @@ var db = require('./mocks/databasemock');
 var User = require('../src/user');
 var Topics = require('../src/topics');
 var Categories = require('../src/categories');
+var Posts = require('../src/posts');
 var Password = require('../src/password');
 var groups = require('../src/groups');
 var helpers = require('./helpers');
@@ -412,6 +413,40 @@ describe('User', function () {
 				});
 			});
 		});
+
+		it('should not re-add user to users:postcount if post is deleted after user deletion', async function () {
+			const uid = await User.create({ username: 'olduserwithposts' });
+			assert(await db.isSortedSetMember('users:postcount', uid));
+
+			const result = await Topics.post({
+				uid: uid,
+				title: 'old user topic',
+				content: 'old user topic post content',
+				cid: testCid,
+			});
+			assert.equal(await db.sortedSetScore('users:postcount', uid), 1);
+			await User.deleteAccount(uid);
+			assert(!await db.isSortedSetMember('users:postcount', uid));
+			await Posts.purge(result.postData.pid, 1);
+			assert(!await db.isSortedSetMember('users:postcount', uid));
+		});
+
+		it('should not re-add user to users:reputation if post is upvoted after user deletion', async function () {
+			const uid = await User.create({ username: 'olduserwithpostsupvote' });
+			assert(await db.isSortedSetMember('users:reputation', uid));
+
+			const result = await Topics.post({
+				uid: uid,
+				title: 'old user topic',
+				content: 'old user topic post content',
+				cid: testCid,
+			});
+			assert.equal(await db.sortedSetScore('users:reputation', uid), 0);
+			await User.deleteAccount(uid);
+			assert(!await db.isSortedSetMember('users:reputation', uid));
+			await Posts.upvote(result.postData.pid, 1);
+			assert(!await db.isSortedSetMember('users:reputation', uid));
+		});
 	});
 
 	describe('passwordReset', function () {