From 5c7dd5481519b9f28b8c0cf42216788fe3175cf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Thu, 11 Oct 2018 16:00:22 -0400 Subject: [PATCH] fix zsets and update them on post move, closes #6823 (#6827) --- src/categories/data.js | 12 +- src/posts/delete.js | 41 ++----- src/topics/data.js | 15 ++- src/topics/fork.js | 50 ++++---- src/topics/posts.js | 2 +- src/topics/recent.js | 54 +++++++-- .../1.10.2/fix_category_post_zsets.js | 60 ++++++++++ .../1.10.2/fix_category_topic_zsets.js | 39 +++++++ test/topics.js | 110 +++++++++++++++++- 9 files changed, 304 insertions(+), 79 deletions(-) create mode 100644 src/upgrades/1.10.2/fix_category_post_zsets.js create mode 100644 src/upgrades/1.10.2/fix_category_topic_zsets.js diff --git a/src/categories/data.js b/src/categories/data.js index a1a9d5c587..8eb24b2179 100644 --- a/src/categories/data.js +++ b/src/categories/data.js @@ -28,9 +28,15 @@ module.exports = function (Categories) { return; } - category.name = validator.escape(String(category.name || '')); - category.disabled = category.hasOwnProperty('disabled') ? parseInt(category.disabled, 10) === 1 : undefined; - category.isSection = category.hasOwnProperty('isSection') ? parseInt(category.isSection, 10) === 1 : undefined; + if (category.hasOwnProperty('name')) { + category.name = validator.escape(String(category.name || '')); + } + if (category.hasOwnProperty('disabled')) { + category.disabled = parseInt(category.disabled, 10) === 1; + } + if (category.hasOwnProperty('isSection')) { + category.isSection = parseInt(category.isSection, 10) === 1; + } if (category.hasOwnProperty('icon')) { category.icon = category.icon || 'hidden'; diff --git a/src/posts/delete.js b/src/posts/delete.js index 44ffb12d87..14cd9dd3b1 100644 --- a/src/posts/delete.js +++ b/src/posts/delete.js @@ -31,7 +31,7 @@ module.exports = function (Posts) { postData.cid = topicData.cid; async.parallel([ function (next) { - updateTopicTimestamp(topicData, next); + topics.updateLastPostTimeFromLastPid(postData.tid, next); }, function (next) { db.sortedSetRemove('cid:' + topicData.cid + ':pids', pid, next); @@ -68,7 +68,7 @@ module.exports = function (Posts) { postData.cid = topicData.cid; async.parallel([ function (next) { - updateTopicTimestamp(topicData, next); + topics.updateLastPostTimeFromLastPid(topicData.tid, next); }, function (next) { db.sortedSetAdd('cid:' + topicData.cid + ':pids', postData.timestamp, pid, next); @@ -85,35 +85,6 @@ module.exports = function (Posts) { ], callback); }; - function updateTopicTimestamp(topicData, callback) { - var timestamp; - async.waterfall([ - function (next) { - topics.getLatestUndeletedPid(topicData.tid, next); - }, - function (pid, next) { - if (!parseInt(pid, 10)) { - return callback(); - } - Posts.getPostField(pid, 'timestamp', next); - }, - function (_timestamp, next) { - timestamp = _timestamp; - if (!parseInt(timestamp, 10)) { - return callback(); - } - topics.updateTimestamp(topicData.tid, timestamp, next); - }, - function (next) { - if (parseInt(topicData.pinned, 10) !== 1) { - db.sortedSetAdd('cid:' + topicData.cid + ':tids', timestamp, topicData.tid, next); - } else { - next(); - } - }, - ], callback); - } - Posts.purge = function (pid, uid, callback) { async.waterfall([ function (next) { @@ -194,10 +165,14 @@ module.exports = function (Posts) { topics.updateTeaser(postData.tid, next); }, function (next) { - updateTopicTimestamp(topicData, next); + topics.updateLastPostTimeFromLastPid(postData.tid, next); }, function (next) { - db.sortedSetIncrBy('cid:' + topicData.cid + ':tids:posts', -1, postData.tid, next); + if (parseInt(topicData.pinned, 10) !== 1) { + db.sortedSetIncrBy('cid:' + topicData.cid + ':tids:posts', -1, postData.tid, next); + } else { + next(); + } }, function (next) { db.sortedSetIncrBy('tid:' + postData.tid + ':posters', -1, postData.uid, next); diff --git a/src/topics/data.js b/src/topics/data.js index 36a0d36206..80f4969103 100644 --- a/src/topics/data.js +++ b/src/topics/data.js @@ -92,12 +92,19 @@ module.exports = function (Topics) { if (!topic) { return; } + if (topic.hasOwnProperty('title')) { + topic.titleRaw = topic.title; + topic.title = String(topic.title); + } - topic.titleRaw = topic.title; - topic.title = String(topic.title); escapeTitle(topic); - topic.timestampISO = utils.toISOString(topic.timestamp); - topic.lastposttimeISO = utils.toISOString(topic.lastposttime); + if (topic.hasOwnProperty('timestamp')) { + topic.timestampISO = utils.toISOString(topic.timestamp); + } + if (topic.hasOwnProperty('lastposttime')) { + topic.lastposttimeISO = utils.toISOString(topic.lastposttime); + } + if (topic.hasOwnProperty('upvotes')) { topic.upvotes = parseInt(topic.upvotes, 10) || 0; } diff --git a/src/topics/fork.js b/src/topics/fork.js index 75591207a3..e1e3b51339 100644 --- a/src/topics/fork.js +++ b/src/topics/fork.js @@ -68,7 +68,7 @@ module.exports = function (Topics) { }, next); }, function (next) { - Topics.updateTimestamp(tid, Date.now(), next); + Topics.updateLastPostTime(tid, Date.now(), next); }, function (next) { plugins.fireHook('action:topic.fork', { tid: tid, fromTid: fromTid, uid: uid }); @@ -106,7 +106,7 @@ module.exports = function (Topics) { function (next) { async.parallel([ function (next) { - updateCategoryPostCount(postData.tid, tid, next); + updateCategory(postData, tid, next); }, function (next) { Topics.decreasePostCount(postData.tid, next); @@ -124,8 +124,8 @@ module.exports = function (Topics) { }, function (results, next) { async.parallel([ - async.apply(updateRecentTopic, tid), - async.apply(updateRecentTopic, postData.tid), + async.apply(Topics.updateLastPostTimeFromLastPid, tid), + async.apply(Topics.updateLastPostTimeFromLastPid, postData.tid), ], function (err) { next(err); }); @@ -137,40 +137,42 @@ module.exports = function (Topics) { ], callback); }; - function updateCategoryPostCount(oldTid, tid, callback) { + function updateCategory(postData, toTid, callback) { + var topicData; async.waterfall([ function (next) { - Topics.getTopicsFields([oldTid, tid], ['cid'], next); + Topics.getTopicsFields([postData.tid, toTid], ['cid', 'pinned'], next); }, - function (topicData, next) { + function (_topicData, next) { + topicData = _topicData; if (!topicData[0].cid || !topicData[1].cid) { return callback(); } + var tasks = []; + if (parseInt(topicData[0].pinned, 10) !== 1) { + tasks.push(async.apply(db.sortedSetIncrBy, 'cid:' + topicData[0].cid + ':tids:posts', -1, postData.tid)); + } + if (parseInt(topicData[1].pinned, 10) !== 1) { + tasks.push(async.apply(db.sortedSetIncrBy, 'cid:' + topicData[1].cid + ':tids:posts', 1, toTid)); + } else { + next(); + } + async.series(tasks, function (err) { + next(err); + }); + }, + function (next) { if (parseInt(topicData[0].cid, 10) === parseInt(topicData[1].cid, 10)) { return callback(); } + async.parallel([ async.apply(db.incrObjectFieldBy, 'category:' + topicData[0].cid, 'post_count', -1), async.apply(db.incrObjectFieldBy, 'category:' + topicData[1].cid, 'post_count', 1), + async.apply(db.sortedSetRemove, 'cid:' + topicData[0].cid + ':pids', postData.pid), + async.apply(db.sortedSetAdd, 'cid:' + topicData[1].cid + ':pids', postData.timestamp, postData.pid), ], next); }, ], callback); } - - function updateRecentTopic(tid, callback) { - async.waterfall([ - function (next) { - Topics.getLatestUndeletedPid(tid, next); - }, - function (pid, next) { - if (!pid) { - return callback(); - } - posts.getPostField(pid, 'timestamp', next); - }, - function (timestamp, next) { - Topics.updateTimestamp(tid, timestamp, next); - }, - ], callback); - } }; diff --git a/src/topics/posts.js b/src/topics/posts.js index a3bc448d75..eb31fc0171 100644 --- a/src/topics/posts.js +++ b/src/topics/posts.js @@ -19,7 +19,7 @@ module.exports = function (Topics) { Topics.increasePostCount(postData.tid, next); }, function (next) { - Topics.updateTimestamp(postData.tid, postData.timestamp, next); + Topics.updateLastPostTime(postData.tid, postData.timestamp, next); }, function (next) { Topics.addPostToTopic(postData.tid, postData, next); diff --git a/src/topics/recent.js b/src/topics/recent.js index 834c0c5f52..7b5fa00cf2 100644 --- a/src/topics/recent.js +++ b/src/topics/recent.js @@ -3,12 +3,14 @@ 'use strict'; var async = require('async'); +var winston = require('winston'); var db = require('../database'); var plugins = require('../plugins'); var privileges = require('../privileges'); var user = require('../user'); var meta = require('../meta'); +var posts = require('../posts'); module.exports = function (Topics) { var terms = { @@ -227,34 +229,62 @@ module.exports = function (Topics) { db.getSortedSetRevRangeByScore(set, start, count, '+inf', Date.now() - since, callback); }; - Topics.updateTimestamp = function (tid, timestamp, callback) { + Topics.updateLastPostTimeFromLastPid = function (tid, callback) { + async.waterfall([ + function (next) { + Topics.getLatestUndeletedPid(tid, next); + }, + function (pid, next) { + if (!parseInt(pid, 10)) { + return callback(); + } + posts.getPostField(pid, 'timestamp', next); + }, + function (timestamp, next) { + if (!parseInt(timestamp, 10)) { + return callback(); + } + Topics.updateLastPostTime(tid, timestamp, next); + }, + ], callback); + }; + + Topics.updateLastPostTime = function (tid, lastposttime, callback) { async.parallel([ function (next) { - var topicData; async.waterfall([ function (next) { - Topics.getTopicFields(tid, ['cid', 'deleted'], next); - }, - function (_topicData, next) { - topicData = _topicData; - db.sortedSetAdd('cid:' + topicData.cid + ':tids:lastposttime', timestamp, tid, next); + Topics.getTopicFields(tid, ['cid', 'deleted', 'pinned'], next); }, - function (next) { - if (parseInt(topicData.deleted, 10) === 1) { - return next(); + function (topicData, next) { + var tasks = [ + async.apply(db.sortedSetAdd, 'cid:' + topicData.cid + ':tids:lastposttime', lastposttime, tid), + ]; + + if (parseInt(topicData.deleted, 10) !== 1) { + tasks.push(async.apply(Topics.updateRecent, tid, lastposttime)); } - Topics.updateRecent(tid, timestamp, next); + + if (parseInt(topicData.pinned, 10) !== 1) { + tasks.push(async.apply(db.sortedSetAdd, 'cid:' + topicData.cid + ':tids', lastposttime, tid)); + } + async.series(tasks, next); }, ], next); }, function (next) { - Topics.setTopicField(tid, 'lastposttime', timestamp, next); + Topics.setTopicField(tid, 'lastposttime', lastposttime, next); }, ], function (err) { callback(err); }); }; + Topics.updateTimestamp = function (tid, lastposttime, callback) { + winston.warn('[deprecated] Topics.updateTimestamp is deprecated please use Topics.updateLastPostTime'); + Topics.updateLastPostTime(tid, lastposttime, callback); + }; + Topics.updateRecent = function (tid, timestamp, callback) { callback = callback || function () {}; diff --git a/src/upgrades/1.10.2/fix_category_post_zsets.js b/src/upgrades/1.10.2/fix_category_post_zsets.js new file mode 100644 index 0000000000..216026f753 --- /dev/null +++ b/src/upgrades/1.10.2/fix_category_post_zsets.js @@ -0,0 +1,60 @@ +'use strict'; + +var db = require('../../database'); + +var async = require('async'); +var batch = require('../../batch'); + +module.exports = { + name: 'Fix category post zsets', + timestamp: Date.UTC(2018, 9, 10), + method: function (callback) { + const progress = this.progress; + + db.getSortedSetRange('categories:cid', 0, -1, function (err, cids) { + if (err) { + return callback(err); + } + var keys = cids.map(function (cid) { + return 'cid:' + cid + ':pids'; + }); + var posts = require('../../posts'); + batch.processSortedSet('posts:pid', function (postData, next) { + async.eachSeries(postData, function (postData, next) { + progress.incr(); + var pid = postData.value; + var timestamp = postData.score; + var cid; + async.waterfall([ + function (next) { + posts.getCidByPid(pid, next); + }, + function (_cid, next) { + cid = _cid; + db.isMemberOfSortedSets(keys, pid, next); + }, + function (isMembers, next) { + var memberCids = []; + isMembers.forEach(function (isMember, index) { + if (isMember) { + memberCids.push(cids[index]); + } + }); + if (memberCids.length > 1) { + async.waterfall([ + async.apply(db.sortedSetRemove, memberCids.map(cid => 'cid:' + cid + ':pids'), pid), + async.apply(db.sortedSetAdd, 'cid:' + cid + ':pids', timestamp, pid), + ], next); + } else { + next(); + } + }, + ], next); + }, next); + }, { + progress: progress, + withScores: true, + }, callback); + }); + }, +}; diff --git a/src/upgrades/1.10.2/fix_category_topic_zsets.js b/src/upgrades/1.10.2/fix_category_topic_zsets.js new file mode 100644 index 0000000000..11b7b1a9da --- /dev/null +++ b/src/upgrades/1.10.2/fix_category_topic_zsets.js @@ -0,0 +1,39 @@ +'use strict'; + +var db = require('../../database'); + +var async = require('async'); +var batch = require('../../batch'); + +module.exports = { + name: 'Fix category topic zsets', + timestamp: Date.UTC(2018, 9, 11), + method: function (callback) { + const progress = this.progress; + + var topics = require('../../topics'); + batch.processSortedSet('topics:tid', function (tids, next) { + async.eachSeries(tids, function (tid, next) { + progress.incr(); + + async.waterfall([ + function (next) { + db.getObjectFields('topic:' + tid, ['cid', 'pinned', 'postcount'], next); + }, + function (topicData, next) { + if (parseInt(topicData.pinned, 10) === 1) { + return setImmediate(next); + } + + db.sortedSetAdd('cid:' + topicData.cid + ':tids:posts', topicData.postcount, tid, next); + }, + function (next) { + topics.updateLastPostTimeFromLastPid(tid, next); + }, + ], next); + }, next); + }, { + progress: progress, + }, callback); + }, +}; diff --git a/test/topics.js b/test/topics.js index 1ca506ef72..1a8c7bcfc2 100644 --- a/test/topics.js +++ b/test/topics.js @@ -403,6 +403,112 @@ describe('Topic\'s', function () { }); }); + it('should properly update sets when post is moved', function (done) { + var movedPost; + var previousPost; + var topic2LastReply; + var tid1; + var tid2; + var cid1 = topic.categoryId; + var cid2; + function checkCidSets(post1, post2, callback) { + async.waterfall([ + function (next) { + async.parallel({ + topicData: function (next) { + topics.getTopicsFields([tid1, tid2], ['lastposttime', 'postcount'], next); + }, + scores1: function (next) { + db.sortedSetsScore([ + 'cid:' + cid1 + ':tids', + 'cid:' + cid1 + ':tids:lastposttime', + 'cid:' + cid1 + ':tids:posts', + ], tid1, next); + }, + scores2: function (next) { + db.sortedSetsScore([ + 'cid:' + cid2 + ':tids', + 'cid:' + cid2 + ':tids:lastposttime', + 'cid:' + cid2 + ':tids:posts', + ], tid2, next); + }, + }, next); + }, + function (results, next) { + assert.equal(results.topicData[0].lastposttime, results.scores1[0]); + assert.equal(results.topicData[1].lastposttime, results.scores2[0]); + assert.equal(results.topicData[0].lastposttime, results.scores1[1]); + assert.equal(results.topicData[1].lastposttime, results.scores2[1]); + assert.equal(results.topicData[0].postcount, results.scores1[2]); + assert.equal(results.topicData[1].postcount, results.scores2[2]); + assert.equal(results.topicData[0].lastposttime, post1.timestamp); + assert.equal(results.topicData[1].lastposttime, post2.timestamp); + next(); + }, + ], callback); + } + + async.waterfall([ + function (next) { + categories.create({ + name: 'move to this category', + description: 'Test category created by testing script', + }, next); + }, + function (category, next) { + cid2 = category.cid; + topics.post({ uid: adminUid, title: 'topic1', content: 'topic 1 mainPost', cid: cid1 }, next); + }, + function (result, next) { + tid1 = result.topicData.tid; + topics.reply({ uid: adminUid, content: 'topic 1 reply 1', tid: tid1 }, next); + }, + function (postData, next) { + previousPost = postData; + topics.reply({ uid: adminUid, content: 'topic 1 reply 2', tid: tid1 }, next); + }, + function (postData, next) { + movedPost = postData; + topics.post({ uid: adminUid, title: 'topic2', content: 'topic 2 mainpost', cid: cid2 }, next); + }, + function (results, next) { + tid2 = results.topicData.tid; + topics.reply({ uid: adminUid, content: 'topic 2 reply 1', tid: tid2 }, next); + }, + function (postData, next) { + topic2LastReply = postData; + checkCidSets(movedPost, postData, next); + }, + function (next) { + db.isMemberOfSortedSets(['cid:' + cid1 + ':pids', 'cid:' + cid2 + ':pids'], movedPost.pid, next); + }, + function (isMember, next) { + assert.deepEqual(isMember, [true, false]); + categories.getCategoriesFields([cid1, cid2], ['post_count'], next); + }, + function (categoryData, next) { + assert.equal(categoryData[0].post_count, 4); + assert.equal(categoryData[1].post_count, 2); + topics.movePostToTopic(movedPost.pid, tid2, next); + }, + function (next) { + checkCidSets(previousPost, topic2LastReply, next); + }, + function (next) { + db.isMemberOfSortedSets(['cid:' + cid1 + ':pids', 'cid:' + cid2 + ':pids'], movedPost.pid, next); + }, + function (isMember, next) { + assert.deepEqual(isMember, [false, true]); + categories.getCategoriesFields([cid1, cid2], ['post_count'], next); + }, + function (categoryData, next) { + assert.equal(categoryData[0].post_count, 3); + assert.equal(categoryData[1].post_count, 3); + next(); + }, + ], done); + }); + it('should purge the topic', function (done) { socketTopics.purge({ uid: 1 }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { assert.ifError(err); @@ -794,8 +900,8 @@ describe('Topic\'s', function () { request(nconf.get('url') + '/api/topic/' + topicData.slug + '/-1', { json: true }, function (err, res, body) { assert.ifError(err); assert.equal(res.statusCode, 200); - assert.equal(res.headers['x-redirect'], '/topic/13/topic-for-controller-test'); - assert.equal(body, '/topic/13/topic-for-controller-test'); + assert.equal(res.headers['x-redirect'], '/topic/15/topic-for-controller-test'); + assert.equal(body, '/topic/15/topic-for-controller-test'); done(); }); });