FEATURE: cache last post number (#15772)

Instead of relaying on /timings request, we should cache last read post number. That should protect from having incorrect unread counter when going back to topic list.

This additional cache is very temporary as once /timings request is finished, serializer will have a correct result.

Simplified flow is:
1. Store in cache information about last seen post number before /timings request is sent
2. When getting back to topic list compare value of last seen post number returned by /latest request and information in cache. If cache number is higher, than use it instead of information returned by /latest. In addition delete cache item as there is high chance that `/timings` request already finished.
3. Optionally, delete cache when timings request is done and topic list was not yet visited.

Keeping cache reasonably small should not affect performance.
This commit is contained in:
Krzysztof Kotlarek 2022-02-10 13:09:28 +11:00 committed by GitHub
parent 8d170ad22d
commit 9610aea189
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 69 additions and 5 deletions

View File

@ -1,8 +1,22 @@
import { Promise } from "rsvp"; import { Promise } from "rsvp";
let model, currentTopicId; let model, currentTopicId;
let lastTopicId, lastHighestRead;
export function setTopicList(incomingModel) { export function setTopicList(incomingModel) {
model = incomingModel; model = incomingModel;
model?.topics?.forEach((topic) => {
let highestRead = getHighestReadCache(topic.id);
if (highestRead && highestRead >= topic.last_read_post_number) {
let count = Math.max(topic.highest_post_number - highestRead, 0);
topic.setProperties({
unread_posts: count,
new_posts: count,
});
resetHighestReadCache();
}
});
currentTopicId = null; currentTopicId = null;
} }
@ -14,6 +28,22 @@ export function previousTopicUrl() {
return urlAt(-1); return urlAt(-1);
} }
export function setHighestReadCache(topicId, postNumber) {
lastTopicId = topicId;
lastHighestRead = postNumber;
}
export function getHighestReadCache(topicId) {
if (topicId === lastTopicId) {
return lastHighestRead;
}
}
export function resetHighestReadCache() {
lastTopicId = undefined;
lastHighestRead = undefined;
}
function urlAt(delta) { function urlAt(delta) {
if (!model || !model.topics) { if (!model || !model.topics) {
return Promise.resolve(null); return Promise.resolve(null);

View File

@ -2,6 +2,11 @@ import Service, { inject as service } from "@ember/service";
import { ajax } from "discourse/lib/ajax"; import { ajax } from "discourse/lib/ajax";
import { bind } from "discourse-common/utils/decorators"; import { bind } from "discourse-common/utils/decorators";
import { isTesting } from "discourse-common/config/environment"; import { isTesting } from "discourse-common/config/environment";
import {
getHighestReadCache,
resetHighestReadCache,
setHighestReadCache,
} from "discourse/lib/topic-list-tracker";
// We use this class to track how long posts in a topic are on the screen. // We use this class to track how long posts in a topic are on the screen.
const PAUSE_UNLESS_SCROLLED = 1000 * 60 * 3; const PAUSE_UNLESS_SCROLLED = 1000 * 60 * 3;
@ -128,9 +133,19 @@ export default class ScreenTrack extends Service {
this._consolidatedTimings.push({ timings, topicTime, topicId }); this._consolidatedTimings.push({ timings, topicTime, topicId });
} }
const highestRead = parseInt(Object.keys(timings).lastObject, 10);
const cachedHighestRead = this.highestReadFromCache(topicId);
if (!cachedHighestRead || cachedHighestRead < highestRead) {
setHighestReadCache(topicId, highestRead);
}
return this._consolidatedTimings; return this._consolidatedTimings;
} }
highestReadFromCache(topicId) {
return getHighestReadCache(topicId);
}
sendNextConsolidatedTiming() { sendNextConsolidatedTiming() {
if (this._consolidatedTimings.length === 0) { if (this._consolidatedTimings.length === 0) {
return; return;
@ -172,11 +187,19 @@ export default class ScreenTrack extends Service {
if (topicController) { if (topicController) {
const postNumbers = Object.keys(timings).map((v) => parseInt(v, 10)); const postNumbers = Object.keys(timings).map((v) => parseInt(v, 10));
topicController.readPosts(topicId, postNumbers); topicController.readPosts(topicId, postNumbers);
const cachedHighestRead = this.highestReadFromCache(topicId);
if (
cachedHighestRead &&
cachedHighestRead <= postNumbers.lastObject
) {
resetHighestReadCache(topicId);
}
} }
this.appEvents.trigger("topic:timings-sent", data); this.appEvents.trigger("topic:timings-sent", data);
}) })
.catch((e) => { .catch((e) => {
if (ALLOWED_AJAX_FAILURES.indexOf(e.jqXHR.status) > -1) { if (e.jqXHR && ALLOWED_AJAX_FAILURES.indexOf(e.jqXHR.status) > -1) {
const delay = AJAX_FAILURE_DELAYS[this._ajaxFailures]; const delay = AJAX_FAILURE_DELAYS[this._ajaxFailures];
this._ajaxFailures += 1; this._ajaxFailures += 1;
@ -187,7 +210,7 @@ export default class ScreenTrack extends Service {
} }
} }
if (window.console && window.console.warn) { if (window.console && window.console.warn && e.jqXHR) {
window.console.warn( window.console.warn(
`Failed to update topic times for topic ${topicId} due to ${e.jqXHR.status} error` `Failed to update topic times for topic ${topicId} due to ${e.jqXHR.status} error`
); );

View File

@ -39,7 +39,10 @@ import { resetCardClickListenerSelector } from "discourse/mixins/card-contents-b
import { resetComposerCustomizations } from "discourse/models/composer"; import { resetComposerCustomizations } from "discourse/models/composer";
import { resetQuickSearchRandomTips } from "discourse/widgets/search-menu-results"; import { resetQuickSearchRandomTips } from "discourse/widgets/search-menu-results";
import sessionFixtures from "discourse/tests/fixtures/session-fixtures"; import sessionFixtures from "discourse/tests/fixtures/session-fixtures";
import { setTopicList } from "discourse/lib/topic-list-tracker"; import {
resetHighestReadCache,
setTopicList,
} from "discourse/lib/topic-list-tracker";
import sinon from "sinon"; import sinon from "sinon";
import siteFixtures from "discourse/tests/fixtures/site-fixtures"; import siteFixtures from "discourse/tests/fixtures/site-fixtures";
import { clearResolverOptions } from "discourse-common/resolver"; import { clearResolverOptions } from "discourse-common/resolver";
@ -160,6 +163,7 @@ function testCleanup(container, app) {
resetOneboxCache(); resetOneboxCache();
resetCustomPostMessageCallbacks(); resetCustomPostMessageCallbacks();
resetUserSearchCache(); resetUserSearchCache();
resetHighestReadCache();
resetCardClickListenerSelector(); resetCardClickListenerSelector();
resetComposerCustomizations(); resetComposerCustomizations();
resetQuickSearchRandomTips(); resetQuickSearchRandomTips();

View File

@ -7,15 +7,22 @@ discourseModule("Unit | Service | screen-track", function () {
tracker.consolidateTimings({ 1: 10, 2: 5 }, 10, 1); tracker.consolidateTimings({ 1: 10, 2: 5 }, 10, 1);
tracker.consolidateTimings({ 1: 5, 3: 1 }, 3, 1); tracker.consolidateTimings({ 1: 5, 3: 1 }, 3, 1);
const consolidated = tracker.consolidateTimings({ 1: 5, 3: 1 }, 3, 2); const consolidated = tracker.consolidateTimings({ 1: 5, 3: 1, 4: 5 }, 3, 2);
assert.deepEqual( assert.deepEqual(
consolidated, consolidated,
[ [
{ timings: { 1: 15, 2: 5, 3: 1 }, topicTime: 13, topicId: 1 }, { timings: { 1: 15, 2: 5, 3: 1 }, topicTime: 13, topicId: 1 },
{ timings: { 1: 5, 3: 1 }, topicTime: 3, topicId: 2 }, { timings: { 1: 5, 3: 1, 4: 5 }, topicTime: 3, topicId: 2 },
], ],
"expecting consolidated timings to match correctly" "expecting consolidated timings to match correctly"
); );
tracker.sendNextConsolidatedTiming();
assert.equal(
tracker.highestReadFromCache(2),
4,
"caches highest read post number for second topic"
);
}); });
}); });