From ed851dbfff9da5f0f0e5dc30dbde904be78c3553 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 16 Aug 2017 11:38:30 +0900 Subject: [PATCH] FIX: Avoid publishing a gigantic payload. * Certain sites have way too many categories. --- .../subscribe-user-notifications.js.es6 | 4 ++++ app/models/group.rb | 12 +++++++++--- lib/discourse.rb | 8 ++++++-- spec/models/group_spec.rb | 14 ++++++++++++++ 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 b/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 index 8efabdb327e..6840111a936 100644 --- a/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 +++ b/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 @@ -95,6 +95,10 @@ export default { bus.subscribe("/client_settings", data => Ember.set(siteSettings, data.name, data.value)); + bus.subscribe("/refresh_client", data => { + Discourse.set("assetVersion", data); + }); + if (!Ember.testing) { if (!site.mobileView) { bus.subscribe(alertChannel(user), data => onNotification(data, user)); diff --git a/app/models/group.rb b/app/models/group.rb index 2d4d82f5814..c1f1fce081c 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -414,12 +414,18 @@ class Group < ActiveRecord::Base users.pluck(:username).join(",") end + PUBLISH_CATEGORIES_LIMIT = 10 + def add(user) self.users.push(user) unless self.users.include?(user) - MessageBus.publish('/categories', { - categories: ActiveModel::ArraySerializer.new(self.categories).as_json - }, user_ids: [user.id]) + if self.categories.count < PUBLISH_CATEGORIES_LIMIT + MessageBus.publish('/categories', { + categories: ActiveModel::ArraySerializer.new(self.categories).as_json + }, user_ids: [user.id]) + else + Discourse.request_refresh!(user_ids: [user.id]) + end self end diff --git a/lib/discourse.rb b/lib/discourse.rb index cd6a27e4401..40a00038015 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -296,12 +296,16 @@ module Discourse last_read_only[$redis.namespace] = nil end - def self.request_refresh! + def self.request_refresh!(user_ids: nil) # Causes refresh on next click for all clients # # This is better than `MessageBus.publish "/file-change", ["refresh"]` because # it spreads the refreshes out over a time period - MessageBus.publish '/global/asset-version', 'clobber' + if user_ids + MessageBus.publish("/refresh_client", 'clobber', user_ids: user_id) + else + MessageBus.publish('/global/asset-version', 'clobber') + end end def self.git_version diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 081577ca1c4..9eee947198b 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -539,6 +539,20 @@ describe Group do expect(message.data[:categories].first[:id]).to eq(category.id) expect(message.user_ids).to eq([user.id]) end + + describe "when group belongs to more than #{Group::PUBLISH_CATEGORIES_LIMIT} categories" do + it "should publish a message to refresh the user's client" do + (Group::PUBLISH_CATEGORIES_LIMIT + 1).times do + group.categories << Fabricate(:category) + end + + message = MessageBus.track_publish { group.add(user) }.first + + expect(message.data).to eq('clobber') + expect(message.channel).to eq('/refresh_client') + expect(message.user_ids).to eq([user.id]) + end + end end end