DEV: Various behind-the-scenes improvements to PresenceChannel (#14518)

DEV: Various behind-the-scenes improvements to PresenceChannel (#14518)

  • Allow the /presence/get endpoint to return multiple channels in a single request (limited to 50)
  • When multiple presence channels are initialized in a single Ember runloop, batch them into a single GET request
  • Introduce the presence-pretender to allow easy testing of PresenceChannel-related features
  • Introduce a use_cache boolean (default true) on the the server-side PresenceChannel initializer. Useful during testing.
diff --git a/app/assets/javascripts/discourse/app/services/presence.js b/app/assets/javascripts/discourse/app/services/presence.js
index 325fc20..85ae22b 100644
--- a/app/assets/javascripts/discourse/app/services/presence.js
+++ b/app/assets/javascripts/discourse/app/services/presence.js
@@ -2,7 +2,7 @@ import Service from "@ember/service";
 import EmberObject, { computed, defineProperty } from "@ember/object";
 import { readOnly } from "@ember/object/computed";
 import { ajax } from "discourse/lib/ajax";
-import { cancel, debounce, later, throttle } from "@ember/runloop";
+import { cancel, debounce, later, next, once, throttle } from "@ember/runloop";
 import Session from "discourse/models/session";
 import { Promise } from "rsvp";
 import { isTesting } from "discourse-common/config/environment";
@@ -12,6 +12,8 @@ const PRESENCE_INTERVAL_S = 30;
 const PRESENCE_DEBOUNCE_MS = isTesting() ? 0 : 500;
 const PRESENCE_THROTTLE_MS = isTesting() ? 0 : 5000;
 
+const PRESENCE_GET_RETRY_MS = 5000;
+
 function createPromiseProxy() {
   const promiseProxy = {};
   promiseProxy.promise = new Promise((resolve, reject) => {
@@ -121,21 +123,7 @@ class PresenceChannelState extends EmberObject {
     }
 
     if (!initialData) {
-      try {
-        initialData = await ajax("/presence/get", {
-          data: {
-            channel: this.name,
-          },
-        });
-      } catch (e) {
-        if (e.jqXHR?.status === 404) {
-          throw new PresenceChannelNotFound(
-            `PresenceChannel '${this.name}' not found`
-          );
-        } else {
-          throw e;
-        }
-      }
+      initialData = await this.presenceService._getInitialData(this.name);
     }
 
     this.set("count", initialData.count);
@@ -231,6 +219,7 @@ export default class PresenceService extends Service {
     this._presenceChannelStates = EmberObject.create();
     this._presentProxies = {};
     this._subscribedProxies = {};
+    this._initialDataRequests = {};
     window.addEventListener("beforeunload", () => {
       this._beaconLeaveAll();
     });
@@ -244,6 +233,64 @@ export default class PresenceService extends Service {
     });
   }
 
+  _getInitialData(channelName) {
+    let promiseProxy = this._initialDataRequests[channelName];
+    if (!promiseProxy) {
+      promiseProxy = this._initialDataRequests[
+        channelName
+      ] = createPromiseProxy();
+    }
+
+    once(this, this._makeInitialDataRequest);
+
+    return promiseProxy.promise;
+  }
+
+  async _makeInitialDataRequest() {
+    if (this._initialDataAjax) {
+      // try again next runloop
+      next(this, () => once(this, this._makeInitialDataRequest));
+    }
+
+    if (Object.keys(this._initialDataRequests).length === 0) {
+      // Nothing to request
+      return;
+    }
+
+    this._initialDataAjax = ajax("/presence/get", {
+      data: {
+        channels: Object.keys(this._initialDataRequests).slice(0, 50),
+      },
+    });
+
+    let result;
+    try {
+      result = await this._initialDataAjax;
+    } catch (e) {
+      later(this, this._makeInitialDataRequest, PRESENCE_GET_RETRY_MS);
+      throw e;
+    } finally {
+      this._initialDataAjax = null;
+    }
+
+    for (const channel in result) {
+      if (!result.hasOwnProperty(channel)) {
+        continue;
+      }
+
+      const state = result[channel];
+      if (state) {
+        this._initialDataRequests[channel].resolve(state);
+      } else {
+        const error = new PresenceChannelNotFound(
+          `PresenceChannel '${channel}' not found`
+        );
+        this._initialDataRequests[channel].reject(error);
+      }
+      delete this._initialDataRequests[channel];
+    }
+  }
+
   _addPresent(channelProxy) {
     let present = this._presentProxies[channelProxy.name];
     if (!present) {
@@ -459,7 +506,11 @@ export default class PresenceService extends Service {
     } else if (this._queuedEvents.length > 0) {
       this._cancelTimer();
       debounce(this, this._throttledUpdateServer, PRESENCE_DEBOUNCE_MS);
-    } else if (!this._nextUpdateTimer && !isTesting()) {
+    } else if (
+      !this._nextUpdateTimer &&
+      this._presentChannels.size > 0 &&
+      !isTesting()
+    ) {
       this._nextUpdateTimer = later(
         this,
         this._throttledUpdateServer,
diff --git a/app/assets/javascripts/discourse/tests/helpers/presence-pretender.js b/app/assets/javascripts/discourse/tests/helpers/presence-pretender.js
new file mode 100644
index 0000000..eb356a1
--- /dev/null
+++ b/app/assets/javascripts/discourse/tests/helpers/presence-pretender.js
@@ -0,0 +1,84 @@
+import { publishToMessageBus } from "discourse/tests/helpers/qunit-helpers";
+import User from "discourse/models/user";
+import { settled } from "@ember/test-helpers";
+
+let channels = {};
+
+export default function (helper) {
+  this.post("/presence/update", (request) => {
+    const params = new URLSearchParams(request.requestBody);
+    const presentChannels = params.getAll("present_channels[]");
+    const leaveChannels = params.getAll("leave_channels[]");
+
+    const user = User.current();
+    if (!user) {
+      return helper.response(403, {});
+    }
+    const userInfo = {
+      id: user.id,
+      username: user.username,
+      name: user.name,
+      avatar_template: "/letter_avatar_proxy/v4/letter/b/35a633/{size}.png",
+    };
+
+    presentChannels.forEach((c) => joinChannel(c, userInfo));
+    leaveChannels.forEach((c) => leaveChannel(c, userInfo));
+
+    return helper.response({ success: "OK" });
+  });
+  this.get("/presence/get", (request) => {
+    const channelNames = request.queryParams.channels;
+    const response = {};
+    channelNames.forEach((c) => (response[c] = getChannelInfo(c)));
+    return helper.response(response);
+  });
+}
+
+export function getChannelInfo(name) {
+  channels[name] ||= { count: 0, users: [], last_message_id: 0 };
+  return channels[name];
+}
+
+export function joinChannel(name, user) {
+  const channel = getChannelInfo(name);
+  if (!channel.users.any((u) => u.id === user.id)) {
+    channel.users.push(user);
+    channel.count += 1;
+    channel.last_message_id += 1;
+    publishToMessageBus(
+      `/presence${name}`,
+      {
+        entering_users: [user],
+      },
+      0,
+      channel.last_message_id
+    );
+  }
+  return settled();
+}
+
+export function leaveChannel(name, user) {
+  const channel = getChannelInfo(name);
+  if (channel.users.any((u) => u.id === user.id)) {
+    channel.users = channel.users.reject((u) => u.id === user.id);
+    channel.count -= 1;
+    channel.last_message_id += 1;
+    publishToMessageBus(
+      `/presence${name}`,
+      {
+        leaving_user_ids: [user.id],
+      },
+      0,
+      channel.last_message_id
+    );
+  }
+  return settled();
+}
+
+export function presentUserIds(channelName) {
+  return getChannelInfo(channelName).users.map((u) => u.id);
+}
+
+export function clearState() {
+  channels = {};
+}
diff --git a/app/assets/javascripts/discourse/tests/setup-tests.js b/app/assets/javascripts/discourse/tests/setup-tests.js
index 6aa4fb8..b038718 100644
--- a/app/assets/javascripts/discourse/tests/setup-tests.js
+++ b/app/assets/javascripts/discourse/tests/setup-tests.js
@@ -33,6 +33,7 @@ import { registerObjects } from "discourse/pre-initializers/inject-discourse-obj
 import sinon from "sinon";
 import { run } from "@ember/runloop";
 import { isLegacyEmber } from "discourse-common/config/environment";
+import { clearState as clearPresenceState } from "discourse/tests/helpers/presence-pretender";
 
 const Plugin = $.fn.modal;
 const Modal = Plugin.Constructor;
@@ -308,6 +309,7 @@ function setupTestsCommon(application, container, config) {
   QUnit.testDone(function () {
     sinon.restore();
     resetPretender();
+    clearPresenceState();
 
     // Destroy any modals
     $(".modal-backdrop").remove();

[... diff too long, it was truncated ...]

GitHub sha: a55642a30a166f9f4c8293e8a63604daba7c9e60

This commit appears in #14518 which was approved by eviltrout and tgxworld. It was merged by davidtaylorhq.