From e70ed31a45af5feb32027d61ae4280a309f5f3d3 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 28 Dec 2022 13:00:08 +0000 Subject: [PATCH] PERF: Ignore repeated PresenceChannel leave/join calls (#19638) If a consumer is calling leave or join on a channel repeatedly, that should not trigger additional HTTP requests --- .../discourse/app/services/presence.js | 12 ++++--- .../tests/unit/services/presence-test.js | 33 +++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/app/services/presence.js b/app/assets/javascripts/discourse/app/services/presence.js index e0ecb6527fd..91becb949c6 100644 --- a/app/assets/javascripts/discourse/app/services/presence.js +++ b/app/assets/javascripts/discourse/app/services/presence.js @@ -66,14 +66,18 @@ class PresenceChannel extends EmberObject.extend(Evented) { } this.setProperties({ activeOptions }); - await this.presenceService._enter(this); - this.set("present", true); + if (!this.present) { + await this.presenceService._enter(this); + this.set("present", true); + } } // Mark the current user as leaving this channel async leave() { - await this.presenceService._leave(this); - this.set("present", false); + if (this.present) { + await this.presenceService._leave(this); + this.set("present", false); + } } async subscribe(initialData = null) { 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 61b749c35a5..1b332e0524d 100644 --- a/app/assets/javascripts/discourse/tests/unit/services/presence-test.js +++ b/app/assets/javascripts/discourse/tests/unit/services/presence-test.js @@ -292,6 +292,39 @@ acceptance("Presence - Entering and Leaving", function (needs) { ); }); + test("join should be a no-op if already present", async function (assert) { + const presenceService = this.container.lookup("service:presence"); + const channel = presenceService.getChannel("/test/ch1"); + + await channel.enter(); + assert.strictEqual(requests.length, 1, "updated the server for enter"); + + await channel.enter(); + assert.strictEqual( + requests.length, + 1, + "does not update the server unnecessarily" + ); + }); + + test("leave should be a no-op if not present", async function (assert) { + const presenceService = this.container.lookup("service:presence"); + const channel = presenceService.getChannel("/test/ch1"); + + await channel.enter(); + assert.strictEqual(requests.length, 1, "updated the server for enter"); + + await channel.leave(); + assert.strictEqual(requests.length, 2, "updated the server for leave"); + + await channel.leave(); + assert.strictEqual( + requests.length, + 2, + "did not update the server unnecessarily" + ); + }); + test("raises an error when entering a non-existent channel", async function (assert) { const presenceService = this.container.lookup("service:presence"); const channel = presenceService.getChannel("/blah/does-not-exist");