DEV: Minor refactor of screen-track service (#25699)

Set/Map, async/await, inline vars, updated tests
This commit is contained in:
Jarek Radosz 2024-02-21 17:17:10 +01:00 committed by GitHub
parent 9199c52e5e
commit 428db40deb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 111 additions and 131 deletions

View File

@ -252,28 +252,27 @@ export default MountWidget.extend({
this._currentPercent = null; this._currentPercent = null;
} }
const onscreenPostNumbers = []; const onscreenPostNumbers = new Set();
const readPostNumbers = []; const readPostNumbers = new Set();
const prev = this._previouslyNearby; const newPrev = new Set();
const newPrev = {};
nearby.forEach((idx) => { nearby.forEach((idx) => {
const post = posts.objectAt(idx); const post = posts.objectAt(idx);
const postNumber = post.post_number;
delete prev[postNumber]; this._previouslyNearby.delete(post.post_number);
if (onscreen.includes(idx)) { if (onscreen.includes(idx)) {
onscreenPostNumbers.push(postNumber); onscreenPostNumbers.add(post.post_number);
if (post.read) { if (post.read) {
readPostNumbers.push(postNumber); readPostNumbers.add(post.post_number);
} }
} }
newPrev[postNumber] = post;
newPrev.add(post.post_number, post);
uncloak(post, this); uncloak(post, this);
}); });
Object.values(prev).forEach((node) => cloak(node, this)); Object.values(this._previouslyNearby).forEach((node) => cloak(node, this));
this._previouslyNearby = newPrev; this._previouslyNearby = newPrev;
this.screenTrack.setOnscreen(onscreenPostNumbers, readPostNumbers); this.screenTrack.setOnscreen(onscreenPostNumbers, readPostNumbers);
@ -323,7 +322,7 @@ export default MountWidget.extend({
didInsertElement() { didInsertElement() {
this._super(...arguments); this._super(...arguments);
this._previouslyNearby = {}; this._previouslyNearby = new Set();
this.appEvents.on("post-stream:refresh", this, "_debouncedScroll"); this.appEvents.on("post-stream:refresh", this, "_debouncedScroll");
const opts = { const opts = {

View File

@ -27,16 +27,17 @@ export default class ScreenTrack extends Service {
@service siteSettings; @service siteSettings;
@service topicTrackingState; @service topicTrackingState;
_ajaxFailures = 0;
_consolidatedTimings = []; _consolidatedTimings = [];
_lastTick = null; _lastTick = null;
_lastScrolled = null; _lastScrolled = null;
_lastFlush = 0; _lastFlush = 0;
_timings = {}; _timings = new Map();
_totalTimings = {}; _totalTimings = new Map();
_topicTime = 0; _topicTime = 0;
_onscreen = []; _onscreen = null;
_readOnscreen = []; _readOnscreen = null;
_readPosts = {}; _readPosts = new Set();
_inProgress = false; _inProgress = false;
constructor() { constructor() {
@ -45,9 +46,7 @@ export default class ScreenTrack extends Service {
} }
start(topicId, topicController) { start(topicId, topicController) {
const currentTopicId = this._topicId; if (this._topicId && this._topicId !== topicId) {
if (currentTopicId && currentTopicId !== topicId) {
this.tick(); this.tick();
this.flush(); this.flush();
} }
@ -98,12 +97,12 @@ export default class ScreenTrack extends Service {
this._lastTick = now; this._lastTick = now;
this._lastScrolled = now; this._lastScrolled = now;
this._lastFlush = 0; this._lastFlush = 0;
this._timings = {}; this._timings.clear();
this._totalTimings = {}; this._totalTimings.clear();
this._topicTime = 0; this._topicTime = 0;
this._onscreen = []; this._onscreen = null;
this._readOnscreen = []; this._readOnscreen = null;
this._readPosts = {}; this._readPosts.clear();
this._inProgress = false; this._inProgress = false;
} }
@ -117,13 +116,12 @@ export default class ScreenTrack extends Service {
} }
consolidateTimings(timings, topicTime, topicId) { consolidateTimings(timings, topicTime, topicId) {
let foundIndex = this._consolidatedTimings.findIndex( const foundIndex = this._consolidatedTimings.findIndex(
(elem) => elem.topicId === topicId (elem) => elem.topicId === topicId
); );
if (foundIndex > -1) { if (foundIndex > -1) {
let found = this._consolidatedTimings[foundIndex]; const found = this._consolidatedTimings[foundIndex];
const lastIndex = this._consolidatedTimings.length - 1; const lastIndex = this._consolidatedTimings.length - 1;
if (foundIndex !== lastIndex) { if (foundIndex !== lastIndex) {
@ -132,12 +130,12 @@ export default class ScreenTrack extends Service {
this._consolidatedTimings[lastIndex - 1] = last; this._consolidatedTimings[lastIndex - 1] = last;
} }
const oldTimings = found.timings; Object.keys(found.timings).forEach((id) => {
Object.keys(oldTimings).forEach((id) => {
if (timings[id]) { if (timings[id]) {
oldTimings[id] += timings[id]; found.timings[id] += timings[id];
} }
}); });
found.topicTime += topicTime; found.topicTime += topicTime;
found.timings = { ...timings, ...found.timings }; found.timings = { ...timings, ...found.timings };
} else { } else {
@ -158,7 +156,7 @@ export default class ScreenTrack extends Service {
return getHighestReadCache(topicId); return getHighestReadCache(topicId);
} }
sendNextConsolidatedTiming() { async sendNextConsolidatedTiming() {
if (this._consolidatedTimings.length === 0) { if (this._consolidatedTimings.length === 0) {
return; return;
} }
@ -174,8 +172,6 @@ export default class ScreenTrack extends Service {
return; return;
} }
this._ajaxFailures = this._ajaxFailures || 0;
const { timings, topicTime, topicId } = this._consolidatedTimings.pop(); const { timings, topicTime, topicId } = this._consolidatedTimings.pop();
const data = { const data = {
timings, timings,
@ -185,82 +181,76 @@ export default class ScreenTrack extends Service {
this._inProgress = true; this._inProgress = true;
return ajax("/topics/timings", { try {
data, await ajax("/topics/timings", {
type: "POST", data,
headers: { type: "POST",
"X-SILENCE-LOGGER": "true", headers: {
"Discourse-Background": "true", "X-SILENCE-LOGGER": "true",
}, "Discourse-Background": "true",
}) },
.then(() => {
if (this.isDestroying || this.isDestroyed) {
return;
}
this._ajaxFailures = 0;
const topicController = this._topicController;
if (topicController) {
const postNumbers = Object.keys(timings).map((v) => parseInt(v, 10));
topicController.readPosts(topicId, postNumbers);
const cachedHighestRead = this.highestReadFromCache(topicId);
if (
cachedHighestRead &&
cachedHighestRead <= postNumbers.lastObject
) {
resetHighestReadCache(topicId);
}
}
this.appEvents.trigger("topic:timings-sent", data);
})
.catch((e) => {
if (e.jqXHR && ALLOWED_AJAX_FAILURES.includes(e.jqXHR.status)) {
const delay = AJAX_FAILURE_DELAYS[this._ajaxFailures];
this._ajaxFailures += 1;
if (delay) {
this._blockSendingToServerTill = Date.now() + delay;
// we did not send to the server, got to re-queue it
this.consolidateTimings(timings, topicTime, topicId);
}
}
if (window.console && window.console.warn && e.jqXHR) {
window.console.warn(
`Failed to update topic times for topic ${topicId} due to ${e.jqXHR.status} error`
);
}
})
.finally(() => {
this._inProgress = false;
this._lastFlush = 0;
}); });
if (this.isDestroying || this.isDestroyed) {
return;
}
this._ajaxFailures = 0;
if (this._topicController) {
const postNumbers = Object.keys(timings).map((v) => parseInt(v, 10));
this._topicController.readPosts(topicId, postNumbers);
const cachedHighestRead = this.highestReadFromCache(topicId);
if (cachedHighestRead && cachedHighestRead <= postNumbers.lastObject) {
resetHighestReadCache(topicId);
}
}
this.appEvents.trigger("topic:timings-sent", data);
} catch (e) {
if (e.jqXHR && ALLOWED_AJAX_FAILURES.includes(e.jqXHR.status)) {
const delay = AJAX_FAILURE_DELAYS[this._ajaxFailures];
this._ajaxFailures += 1;
if (delay) {
this._blockSendingToServerTill = Date.now() + delay;
// we did not send to the server, got to re-queue it
this.consolidateTimings(timings, topicTime, topicId);
}
}
if (window.console && window.console.warn && e.jqXHR) {
window.console.warn(
`Failed to update topic times for topic ${topicId} due to ${e.jqXHR.status} error`
);
}
} finally {
this._inProgress = false;
this._lastFlush = 0;
}
} }
flush() { flush() {
const newTimings = {}; const newTimings = {};
const totalTimings = this._totalTimings;
const timings = this._timings; for (const [postNumber, time] of this._timings) {
Object.keys(this._timings).forEach((postNumber) => { if (!this._totalTimings.has(postNumber)) {
const time = timings[postNumber]; this._totalTimings.set(postNumber, 0);
totalTimings[postNumber] = totalTimings[postNumber] || 0; }
if (time > 0 && totalTimings[postNumber] < MAX_TRACKING_TIME) { const totalTiming = this._totalTimings.get(postNumber);
totalTimings[postNumber] += time; if (time > 0 && totalTiming < MAX_TRACKING_TIME) {
this._totalTimings.set(postNumber, totalTiming + time);
newTimings[postNumber] = time; newTimings[postNumber] = time;
} }
timings[postNumber] = 0;
}); this._timings.set(postNumber, 0);
}
const topicId = parseInt(this._topicId, 10); const topicId = parseInt(this._topicId, 10);
let highestSeen = 0;
// Workaround to avoid ignored posts being "stuck unread" // Workaround to avoid ignored posts being "stuck unread"
const controller = this._topicController; const stream = this._topicController?.get("model.postStream");
const stream = controller ? controller.get("model.postStream") : null;
if ( if (
this.currentUser && // Logged in this.currentUser && // Logged in
this.currentUser.get("ignored_users.length") && // At least 1 user is ignored this.currentUser.get("ignored_users.length") && // At least 1 user is ignored
@ -281,10 +271,9 @@ export default class ScreenTrack extends Service {
] = 1; ] = 1;
} }
const newTimingsKeys = Object.keys(newTimings); const highestSeen = Object.keys(newTimings)
newTimingsKeys.forEach((postNumber) => { .map((postNumber) => parseInt(postNumber, 10))
highestSeen = Math.max(highestSeen, parseInt(postNumber, 10)); .reduce((a, b) => Math.max(a, b), 0);
});
const highestSeenByTopic = this.session.get("highestSeenByTopic"); const highestSeenByTopic = this.session.get("highestSeenByTopic");
if ((highestSeenByTopic[topicId] || 0) < highestSeen) { if ((highestSeenByTopic[topicId] || 0) < highestSeen) {
@ -293,7 +282,7 @@ export default class ScreenTrack extends Service {
this.topicTrackingState.updateSeen(topicId, highestSeen); this.topicTrackingState.updateSeen(topicId, highestSeen);
if (newTimingsKeys.length > 0) { if (highestSeen > 0) {
if (this.currentUser) { if (this.currentUser) {
this.consolidateTimings(newTimings, this._topicTime, topicId); this.consolidateTimings(newTimings, this._topicTime, topicId);
@ -301,15 +290,15 @@ export default class ScreenTrack extends Service {
this.sendNextConsolidatedTiming(); this.sendNextConsolidatedTiming();
} }
} else if (this._anonCallback) { } else if (this._anonCallback) {
// Anonymous viewer - save to localStorage
const storage = this.keyValueStore;
// Save total time // Save total time
const existingTime = storage.getInt("anon-topic-time"); const existingTime = this.keyValueStore.getInt("anon-topic-time");
storage.setItem("anon-topic-time", existingTime + this._topicTime); this.keyValueStore.setItem(
"anon-topic-time",
existingTime + this._topicTime
);
// Save unique topic IDs up to a max // Save unique topic IDs up to a max
let topicIds = storage.get("anon-topic-ids"); let topicIds = this.keyValueStore.get("anon-topic-ids");
if (topicIds) { if (topicIds) {
topicIds = topicIds.split(",").map((e) => parseInt(e, 10)); topicIds = topicIds.split(",").map((e) => parseInt(e, 10));
} else { } else {
@ -321,7 +310,7 @@ export default class ScreenTrack extends Service {
topicIds.length < ANON_MAX_TOPIC_IDS topicIds.length < ANON_MAX_TOPIC_IDS
) { ) {
topicIds.push(topicId); topicIds.push(topicId);
storage.setItem("anon-topic-ids", topicIds.join(",")); this.keyValueStore.setItem("anon-topic-ids", topicIds.join(","));
} }
// Inform the observer // Inform the observer
@ -349,15 +338,13 @@ export default class ScreenTrack extends Service {
this._lastFlush += diff; this._lastFlush += diff;
this._lastTick = now; this._lastTick = now;
const totalTimings = this._totalTimings;
const timings = this._timings;
const nextFlush = this.siteSettings.flush_timings_secs * 1000; const nextFlush = this.siteSettings.flush_timings_secs * 1000;
const rush = Object.keys(timings).some((postNumber) => { const rush = [...this._timings.entries()].some(([postNumber, timing]) => {
return ( return (
timings[postNumber] > 0 && timing > 0 &&
!totalTimings[postNumber] && !this._totalTimings.get(postNumber) &&
!this._readPosts[postNumber] !this._readPosts.has(postNumber)
); );
}); });
@ -373,13 +360,15 @@ export default class ScreenTrack extends Service {
if (this.session.hasFocus) { if (this.session.hasFocus) {
this._topicTime += diff; this._topicTime += diff;
this._onscreen.forEach( this._onscreen?.forEach((postNumber) =>
(postNumber) => this._timings.set(
(timings[postNumber] = (timings[postNumber] || 0) + diff) postNumber,
(this._timings.get(postNumber) ?? 0) + diff
)
); );
this._readOnscreen.forEach((postNumber) => { this._readOnscreen?.forEach((postNumber) => {
this._readPosts[postNumber] = true; this._readPosts.add(postNumber);
}); });
} }
} }

View File

@ -22,32 +22,24 @@ module("Unit | Service | screen-track", function (hooks) {
await tracker.sendNextConsolidatedTiming(); await tracker.sendNextConsolidatedTiming();
assert.equal( assert.strictEqual(
tracker.highestReadFromCache(2), tracker.highestReadFromCache(2),
4, 4,
"caches highest read post number for second topic" "caches highest read post number for second topic"
); );
}); });
test("ScreenTrack has appEvents", async function (assert) {
const tracker = this.owner.lookup("service:screen-track");
assert.ok(tracker.appEvents);
});
test("appEvent topic:timings-sent is triggered after posting consolidated timings", async function (assert) { test("appEvent topic:timings-sent is triggered after posting consolidated timings", async function (assert) {
assert.timeout(1000);
const tracker = this.owner.lookup("service:screen-track"); const tracker = this.owner.lookup("service:screen-track");
const appEvents = this.owner.lookup("service:app-events"); const appEvents = this.owner.lookup("service:app-events");
const done = assert.async();
appEvents.on("topic:timings-sent", () => { appEvents.on("topic:timings-sent", () => {
assert.ok(true); assert.step("sent");
done();
}); });
tracker.consolidateTimings({ 1: 10, 2: 5 }, 10, 1); tracker.consolidateTimings({ 1: 10, 2: 5 }, 10, 1);
await tracker.sendNextConsolidatedTiming(); await tracker.sendNextConsolidatedTiming();
await assert.verifySteps(["sent"]);
}); });
}); });