From 00776bdd8e2662a0a5f70f5a3460ee6ee6380cfd Mon Sep 17 00:00:00 2001
From: Ben Lubar <ben.lubar+github@gmail.com>
Date: Thu, 8 Feb 2018 09:50:12 -0600
Subject: [PATCH] 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 |  2 +-
 src/topics/bookmarks.js    | 52 +++++++++++++++++++++++++-------------
 2 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/public/src/client/topic.js b/public/src/client/topic.js
index 54052d47c1..63c63e3051 100644
--- a/public/src/client/topic.js
+++ b/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,
diff --git a/src/topics/bookmarks.js b/src/topics/bookmarks.js
index 975a0d54f8..d1782b6713 100644
--- a/src/topics/bookmarks.js
+++ b/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);
 			},