Bookmark optimization (#6315)

* Set the user's bookmark if their current bookmark is past the end of the topic.

* Optimize forked topic bookmark updating.

Remove support for updating bookmarks for users who sort by votes.

Don't even consider updating bookmarks for users who have not read the posts being removed.

Only compute post indices once per fork operation instead of once per user that has ever read the topic.

public/src/client/topic.js

@@ -231,7 +231,7 @@ define('forum/topic', [
 		var bookmarkKey = 'topic:' + ajaxify.data.tid + ':bookmark';
 		var currentBookmark = ajaxify.data.bookmark || storage.getItem(bookmarkKey);
 
-		if (ajaxify.data.postcount > ajaxify.data.bookmarkThreshold && (!currentBookmark || parseInt(index, 10) > parseInt(currentBookmark, 10))) {
+		if (ajaxify.data.postcount > ajaxify.data.bookmarkThreshold && (!currentBookmark || parseInt(index, 10) > parseInt(currentBookmark, 10) || ajaxify.data.postcount < parseInt(currentBookmark, 10))) {
 			if (app.user.uid) {
 				socket.emit('topics.bookmark', {
 					tid: ajaxify.data.tid,

src/topics/bookmarks.js

@@ -4,7 +4,7 @@
 var async = require('async');
 
 var db = require('../database');
-var posts = require('../posts');
+var user = require('../user');
 
 module.exports = function (Topics) {
 	Topics.getUserBookmark = function (tid, uid, callback) {
@@ -34,7 +34,9 @@ module.exports = function (Topics) {
 	};
 
 	Topics.updateTopicBookmarks = function (tid, pids, callback) {
+		var minIndex;
 		var maxIndex;
+		var postIndices;
 
 		async.waterfall([
 			function (next) {
@@ -42,38 +44,54 @@ module.exports = function (Topics) {
 			},
 			function (postcount, next) {
 				maxIndex = postcount;
-				Topics.getTopicBookmarks(tid, next);
+
+				db.sortedSetRanks('tid:' + tid + ':posts', pids, next);
 			},
-			function (bookmarks, next) {
-				var forkedPosts = pids.map(function (pid) {
-					return { pid: pid, tid: tid };
+			function (indices, next) {
+				postIndices = indices.map(function (i) {
+					return i === null ? 0 : i + 1;
 				});
+				minIndex = Math.min.apply(Math, postIndices);
 
+				Topics.getTopicBookmarks(tid, next);
+			},
+			function (bookmarks, next) {
 				var uidData = bookmarks.map(function (bookmark) {
 					return {
 						uid: bookmark.value,
-						bookmark: bookmark.score,
+						bookmark: parseInt(bookmark.score, 10),
 					};
+				}).filter(function (data) {
+					return data.bookmark >= minIndex;
 				});
 
 				async.eachLimit(uidData, 50, function (data, next) {
-					posts.getPostIndices(forkedPosts, data.uid, function (err, postIndices) {
-						if (err) {
-							return next(err);
+					var bookmark = data.bookmark;
+					bookmark = Math.min(bookmark, maxIndex);
+
+					postIndices.forEach(function (i) {
+						if (i < data.bookmark) {
+							bookmark -= 1;
 						}
+					});
 
-						var bookmark = data.bookmark;
-						bookmark = bookmark < maxIndex ? bookmark : maxIndex;
+					// make sure the bookmark is valid if we removed the last post
+					bookmark = Math.min(bookmark, maxIndex - pids.length);
 
-						for (var i = 0; i < postIndices.length && postIndices[i] < data.bookmark; i += 1) {
-							bookmark -= 1;
+					if (bookmark === data.bookmark) {
+						return next();
+					}
+
+					user.getSettings(data.uid, function (err, settings) {
+						if (err) {
+							return next(err);
 						}
 
-						if (parseInt(bookmark, 10) !== parseInt(data.bookmark, 10)) {
-							Topics.setUserBookmark(tid, data.uid, bookmark, next);
-						} else {
-							next();
+						if (settings.topicPostSort === 'most_votes') {
+							return next();
 						}
+
+						Topics.setUserBookmark(tid, data.uid, bookmark, next);
 					});
 				}, next);
 			},