From af4b8d0e21f823463a1f8a613aedf9bc845a62e1 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 30 Nov 2021 04:38:19 +0000 Subject: [PATCH] DEV: Automatically leave PresenceChannels when in the background (#15047) * DEV: Improve PresenceChannel state storage Replaces some objects with Maps, and removes the redundant _presentChannels Set. * DEV: Automatically leave PresenceChannels when in the background If a tab has been in the background for 10s, or there has been no user activity for 60s, then the user will be removed from all PresenceChannels until activity resumes. Developers can opt-out of this by passing `{onlyWhileActive: false}` to the `enter` method. --- .../discourse/app/services/presence.js | 96 +++++++++++++------ .../tests/unit/services/presence-test.js | 52 ++++++++++ 2 files changed, 119 insertions(+), 29 deletions(-) diff --git a/app/assets/javascripts/discourse/app/services/presence.js b/app/assets/javascripts/discourse/app/services/presence.js index 0915ce2684..e0b8dae8e4 100644 --- a/app/assets/javascripts/discourse/app/services/presence.js +++ b/app/assets/javascripts/discourse/app/services/presence.js @@ -15,6 +15,10 @@ import Session from "discourse/models/session"; import { Promise } from "rsvp"; import { isLegacyEmber, isTesting } from "discourse-common/config/environment"; import User from "discourse/models/user"; +import userPresent, { + onPresenceChange, + removeOnPresenceChange, +} from "discourse/lib/user-presence"; const PRESENCE_INTERVAL_S = 30; const PRESENCE_DEBOUNCE_MS = isTesting() ? 0 : 500; @@ -22,6 +26,11 @@ const PRESENCE_THROTTLE_MS = isTesting() ? 0 : 1000; const PRESENCE_GET_RETRY_MS = 5000; +const USER_PRESENCE_ARGS = { + userUnseenTime: 60000, + browserHiddenTime: 10000, +}; + function createPromiseProxy() { const promiseProxy = {}; promiseProxy.promise = new Promise((resolve, reject) => { @@ -51,7 +60,11 @@ class PresenceChannel extends EmberObject { } // Mark the current user as 'present' in this channel - async enter() { + // By default, the user will temporarily 'leave' the channel when + // the current tab is in the background, or has no interaction for more than 60 seconds. + // To override this behaviour, set onlyWhileActive: false + async enter({ onlyWhileActive = true } = {}) { + this.setProperties({ onlyWhileActive }); await this.presenceService._enter(this); this.set("present", true); } @@ -221,20 +234,32 @@ class PresenceChannelState extends EmberObject { export default class PresenceService extends Service { init() { super.init(...arguments); - this._presentChannels = new Set(); this._queuedEvents = []; this._presenceChannelStates = EmberObject.create(); - this._presentProxies = {}; - this._subscribedProxies = {}; - this._initialDataRequests = {}; + this._presentProxies = new Map(); + this._subscribedProxies = new Map(); + this._initialDataRequests = new Map(); - this._beforeUnloadCallback = () => this._beaconLeaveAll(); - window.addEventListener("beforeunload", this._beforeUnloadCallback); + if (this.currentUser) { + this._beforeUnloadCallback = () => this._beaconLeaveAll(); + window.addEventListener("beforeunload", this._beforeUnloadCallback); + + this._presenceChangeCallback = () => this._throttledUpdateServer(); + onPresenceChange({ + ...USER_PRESENCE_ARGS, + callback: this._presenceChangeCallback, + }); + } + } + + get _presentChannels() { + return new Set(this._presentProxies.keys()); } willDestroy() { super.willDestroy(...arguments); window.removeEventListener("beforeunload", this._beforeUnloadCallback); + removeOnPresenceChange(this._presenceChangeCallback); } // Get a PresenceChannel object representing a single channel @@ -305,34 +330,39 @@ export default class PresenceService extends Service { } _addPresent(channelProxy) { - let present = this._presentProxies[channelProxy.name]; + let present = this._presentProxies.get(channelProxy.name); if (!present) { - present = this._presentProxies[channelProxy.name] = new Set(); + present = new Set(); + this._presentProxies.set(channelProxy.name, present); } present.add(channelProxy); return present.size; } _removePresent(channelProxy) { - let present = this._presentProxies[channelProxy.name]; + let present = this._presentProxies.get(channelProxy.name); present?.delete(channelProxy); + if (present?.size === 0) { + this._presentProxies.delete(channelProxy.name); + } return present?.size || 0; } _addSubscribed(channelProxy) { - let subscribed = this._subscribedProxies[channelProxy.name]; + let subscribed = this._subscribedProxies.get(channelProxy.name); if (!subscribed) { - subscribed = this._subscribedProxies[channelProxy.name] = new Set(); + subscribed = new Set(); + this._subscribedProxies.set(channelProxy.name, subscribed); } subscribed.add(channelProxy); return subscribed.size; } _removeSubscribed(channelProxy) { - let subscribed = this._subscribedProxies[channelProxy.name]; + let subscribed = this._subscribedProxies.get(channelProxy.name); subscribed?.delete(channelProxy); if (subscribed?.size === 0) { - delete this._subscribedProxies[channelProxy.name]; + this._subscribedProxies.delete(channelProxy.name); } return subscribed?.size || 0; } @@ -342,18 +372,15 @@ export default class PresenceService extends Service { throw "Must be logged in to enter presence channel"; } - this._addPresent(channelProxy); - - const channelName = channelProxy.name; - if (this._presentChannels.has(channelName)) { + const newCount = this._addPresent(channelProxy); + if (newCount > 1) { return; } const promiseProxy = createPromiseProxy(); - this._presentChannels.add(channelName); this._queuedEvents.push({ - channel: channelName, + channel: channelProxy.name, type: "enter", promiseProxy, }); @@ -373,16 +400,10 @@ export default class PresenceService extends Service { return; } - const channelName = channelProxy.name; - if (!this._presentChannels.has(channelName)) { - return; - } - const promiseProxy = createPromiseProxy(); - this._presentChannels.delete(channelName); this._queuedEvents.push({ - channel: channelName, + channel: channelProxy.name, type: "leave", promiseProxy, }); @@ -464,14 +485,31 @@ export default class PresenceService extends Service { this._queuedEvents = []; try { + const presentChannels = []; const channelsToLeave = queue .filter((e) => e.type === "leave") .map((e) => e.channel); + const userIsPresent = userPresent(USER_PRESENCE_ARGS); + for (const [channelName, proxies] of this._presentProxies) { + if ( + !userIsPresent && + Array.from(proxies).every((p) => p.onlyWhileActive) + ) { + channelsToLeave.push(channelName); + } else { + presentChannels.push(channelName); + } + } + + if (queue.length === 0 && presentChannels.length === 0) { + return; + } + const response = await ajax("/presence/update", { data: { client_id: this.messageBus.clientId, - present_channels: [...this._presentChannels], + present_channels: presentChannels, leave_channels: channelsToLeave, }, type: "POST", @@ -539,7 +577,7 @@ export default class PresenceService extends Service { debounce(this, this._throttledUpdateServer, PRESENCE_DEBOUNCE_MS); } else if ( !this._nextUpdateTimer && - this._presentChannels.size > 0 && + this._presentChannels.length > 0 && !isTesting() ) { this._nextUpdateTimer = later( diff --git a/app/assets/javascripts/discourse/tests/unit/services/presence-test.js b/app/assets/javascripts/discourse/tests/unit/services/presence-test.js index d5b0d97f7d..f8f63a61c5 100644 --- a/app/assets/javascripts/discourse/tests/unit/services/presence-test.js +++ b/app/assets/javascripts/discourse/tests/unit/services/presence-test.js @@ -4,6 +4,7 @@ import { } from "discourse/tests/helpers/qunit-helpers"; import { test } from "qunit"; import { PresenceChannelNotFound } from "discourse/services/presence"; +import { setTestPresence } from "discourse/lib/user-presence"; function usersFixture() { return [ @@ -326,4 +327,55 @@ acceptance("Presence - Entering and Leaving", function (needs) { "service shows absent" ); }); + + test("handles the onlyWhileActive flag", async function (assert) { + const presenceService = this.container.lookup("service:presence"); + const channel = presenceService.getChannel("/test/ch1"); + await channel.enter(); + requests.pop(); // Throw away this request + + const channel2 = presenceService.getChannel("/test/ch2"); + await channel2.enter({ onlyWhileActive: false }); + + assert.strictEqual(requests.length, 1, "updated the server"); + let presentChannels = requests.pop().getAll("present_channels[]"); + assert.deepEqual( + presentChannels, + ["/test/ch1", "/test/ch2"], + "included both channels when active" + ); + + setTestPresence(false); + await presenceService._updateServer(); + assert.strictEqual( + requests.length, + 1, + "updated the server after going idle" + ); + let request = requests.pop(); + assert.deepEqual( + request.getAll("present_channels[]"), + ["/test/ch2"], + "ch2 remained present" + ); + assert.ok( + request.getAll("leave_channels[]").includes("/test/ch1"), + "left ch1" + ); + + await channel2.leave(); + assert.strictEqual(requests.length, 1, "updated the server"); + request = requests.pop(); + assert.ok( + request.getAll("leave_channels[]").includes("/test/ch2"), + "left ch2" + ); + + await presenceService._updateServer(); + assert.strictEqual( + requests.length, + 0, + "skips sending empty updates to the server" + ); + }); });