diff --git a/app/graphql/gql/subscriptions/config_updates.rb b/app/graphql/gql/subscriptions/config_updates.rb index b53181c39a16..dfa44404cdee 100644 --- a/app/graphql/gql/subscriptions/config_updates.rb +++ b/app/graphql/gql/subscriptions/config_updates.rb @@ -15,13 +15,14 @@ def self.authorize(...) end def update - setting = object + return no_update if !object.frontend + return no_update if object.preferences[:authentication] && !context.current_user? - if !setting.frontend || (setting.preferences[:authentication] && !context.current_user?) - return no_update - end + # Some setting values use interpolation to reference other settings. + # This is applied in `Setting.get`, thus direct reading of the value should be avoided. + value = Setting.get(object.name) - { setting: { key: setting.name, value: setting.state_current[:value] } } + { setting: { key: object.name, value: value } } end end end diff --git a/app/models/setting.rb b/app/models/setting.rb index a7eeb6132d52..19dcb98516bd 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -7,9 +7,9 @@ class Setting < ApplicationModel store :preferences before_validation :state_check before_create :set_initial - after_create :reset_change_id, :reset_cache, :check_broadcast - after_update :reset_change_id, :reset_cache, :check_broadcast - after_commit :check_refresh + after_create :reset_change_id + after_update :reset_change_id + after_commit :reset_cache, :broadcast_frontend, :check_refresh validates_with Setting::Validator @@ -139,11 +139,9 @@ def self.load(force = false) # set initial value in state_initial def set_initial self.state_initial = state_current - true end def reset_change_id - @@current[name] = state_current[:value] change_id = SecureRandom.uuid logger.debug { "Setting.reset_change_id: set new cache, #{change_id}" } Rails.cache.write('Setting::ChangeId', change_id, { expires_in: 24.hours }) @@ -152,12 +150,11 @@ def reset_change_id end def reset_cache - return true if preferences[:cache].blank? + return if preferences[:cache].blank? - preferences[:cache].each do |key| + Array(preferences[:cache]).each do |key| Rails.cache.delete(key) end - true end # check if cache is still valid @@ -180,21 +177,19 @@ def self.cache_valid? # convert state into hash to be able to store it as store def state_check - return true if state.nil? # allow false value - return true if state.try(:key?, :value) + return if state.nil? # allow false value + return if state.try(:key?, :value) self.state_current = { value: state } - true end # Notify clients about config changes. - def check_broadcast - return true if frontend != true + def broadcast_frontend + return if !frontend - value = state_current - if state_current.key?(:value) - value = state_current[:value] - end + # Some setting values use interpolation to reference other settings. + # This is applied in `Setting.get`, thus direct reading of the value should be avoided. + value = self.class.get(name) Sessions.broadcast( { @@ -203,8 +198,8 @@ def check_broadcast }, preferences[:authentication] ? 'authenticated' : 'public' ) + Gql::Subscriptions::ConfigUpdates.trigger(self) - true end # NB: Force users to reload on SAML credentials config changes diff --git a/spec/graphql/gql/subscriptions/config_updates_spec.rb b/spec/graphql/gql/subscriptions/config_updates_spec.rb index 6c7cb0c4add3..5cc39e888a65 100644 --- a/spec/graphql/gql/subscriptions/config_updates_spec.rb +++ b/spec/graphql/gql/subscriptions/config_updates_spec.rb @@ -3,6 +3,7 @@ require 'rails_helper' RSpec.describe Gql::Subscriptions::ConfigUpdates, type: :graphql do + let(:setting) { build(:setting, name: 'broadcast_test', state: setting_value, frontend: true) } let(:subscription) do <<~QUERY @@ -23,8 +24,8 @@ 'data' => { 'configUpdates' => { 'setting' => { - 'key' => 'product_name', - 'value' => 'subscription_test' + 'key' => 'broadcast_test', + 'value' => expected_value } } } @@ -33,9 +34,25 @@ } end - it 'broadcasts config update events' do - gql.execute(subscription, context: { channel: mock_channel }) - Setting.set('product_name', 'subscription_test') - expect(mock_channel.mock_broadcasted_messages).to eq([expected_msg]) + context 'when using static value' do + let(:setting_value) { 'subscription_test' } + let(:expected_value) { setting_value } + + it 'broadcasts config update events' do + gql.execute(subscription, context: { channel: mock_channel }) + setting.save + expect(mock_channel.mock_broadcasted_messages).to eq([expected_msg]) + end + end + + context 'when using interpolated value' do + let(:setting_value) { 'test #{config.fqdn}' } # rubocop:disable Lint/InterpolationCheck + let(:expected_value) { "test #{Setting.get('fqdn')}" } + + it 'broadcasts config update events with interpolated string' do + gql.execute(subscription, context: { channel: mock_channel }) + setting.save + expect(mock_channel.mock_broadcasted_messages).to eq([expected_msg]) + end end end diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index 8e14cfd36589..387da762e91d 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -12,6 +12,18 @@ .to change { described_class.get(setting.name) }.to('foo') end end + + context 'when interpolated value was set and cache is still valid' do + it 'stores interpolated value' do + create(:setting, name: 'broadcast_test', state: 'test') + described_class.send(:load) # prewarm cache + + described_class.set('broadcast_test', 'test #{config.fqdn}') # rubocop:disable Lint/InterpolationCheck + + expect(described_class.get('broadcast_test')) + .to eq("test #{described_class.get('fqdn')}") + end + end end describe '.set' do @@ -57,34 +69,84 @@ end end - describe 'check_broadcast' do + describe 'broadcast_frontend' do + subject(:setting) do + build(:setting, name: 'broadcast_test', state: value, frontend: frontend) + .tap { |setting| setting.preferences = { authentication: true } if authentication_required } + end + + let(:value) { 'foo' } + let(:frontend) { true } + let(:authentication_required) { false } + context 'when setting is non-frontend' do - subject(:setting) { build(:setting, name: 'broadcast_test', state: 'foo', frontend: false) } + let(:frontend) { false } it 'does not broadcast' do allow(Sessions).to receive(:broadcast) setting.save expect(Sessions).not_to have_received(:broadcast) end + + it 'does not trigger subscription' do + allow(Gql::Subscriptions::ConfigUpdates).to receive(:trigger) + setting.save + expect(Gql::Subscriptions::ConfigUpdates).not_to have_received(:trigger).with(setting) + end end context 'when setting is public' do - subject(:setting) { build(:setting, name: 'broadcast_test', state: 'foo', frontend: true) } - it 'broadcasts to public' do allow(Sessions).to receive(:broadcast) setting.save - expect(Sessions).to have_received(:broadcast).with({ data: { name: 'broadcast_test', value: 'foo' }, event: 'config_update' }, 'public') + expect(Sessions).to have_received(:broadcast) + .with({ data: { name: 'broadcast_test', value: 'foo' }, event: 'config_update' }, 'public') + end + + it 'triggers subscription' do + allow(Gql::Subscriptions::ConfigUpdates).to receive(:trigger) + setting.save + expect(Gql::Subscriptions::ConfigUpdates).to have_received(:trigger).with(setting) end end context 'when setting requires authentication' do - subject(:setting) { build(:setting, name: 'broadcast_test', state: 'foo', frontend: true, preferences: { authentication: true }) } + let(:authentication_required) { true } it 'broadcasts to authenticated only' do allow(Sessions).to receive(:broadcast) setting.save - expect(Sessions).to have_received(:broadcast).with({ data: { name: 'broadcast_test', value: 'foo' }, event: 'config_update' }, 'authenticated') + expect(Sessions).to have_received(:broadcast) + .with({ data: { name: 'broadcast_test', value: 'foo' }, event: 'config_update' }, 'authenticated') + end + + it 'triggers subscription' do + allow(Gql::Subscriptions::ConfigUpdates).to receive(:trigger) + setting.save + expect(Gql::Subscriptions::ConfigUpdates).to have_received(:trigger).with(setting) + end + end + + context 'when setting uses interpolation' do + let(:value) { 'test #{config.fqdn}' } # rubocop:disable Lint/InterpolationCheck + + it 'broadcasts to authenticated only' do + allow(Sessions).to receive(:broadcast) + + setting.save + + expect(Sessions) + .to have_received(:broadcast) + .with( + { data: { name: 'broadcast_test', value: "test #{described_class.get('fqdn')}" }, event: 'config_update' }, + 'public' + ) + end + + it 'triggers subscription' do + allow(Gql::Subscriptions::ConfigUpdates).to receive(:trigger) + setting.save + expect(Gql::Subscriptions::ConfigUpdates).to have_received(:trigger).with(setting) end end end