From c5850422f0b42cb40408b26e0de94907c34c5a0c Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Mon, 7 Aug 2017 15:15:32 +0900 Subject: [PATCH] FIX: SiteSettings defaults cache leaking across multisite. --- lib/site_setting_extension.rb | 16 ++++--- lib/site_settings/defaults_provider.rb | 44 +++++++++---------- .../site_settings/defaults_provider_spec.rb | 2 +- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 410f22bc17c..025d126f59c 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -66,9 +66,11 @@ module SiteSettingExtension def setting(name_arg, default = nil, opts = {}) name = name_arg.to_sym mutex.synchronize do - defaults.load_setting(name, - default, - opts.extract!(*SiteSettings::DefaultsProvider::CONSUMED_OPTS)) + defaults.load_setting( + name, + default, + opts.extract!(*SiteSettings::DefaultsProvider::CONSUMED_OPTS) + ) categories[name] = opts[:category] || :uncategorized @@ -97,8 +99,10 @@ module SiteSettingExtension previews[name] = opts[:preview] end - type_supervisor.load_setting(name, - opts.extract!(*SiteSettings::TypeSupervisor::CONSUMED_OPTS)) + type_supervisor.load_setting( + name, + opts.extract!(*SiteSettings::TypeSupervisor::CONSUMED_OPTS) + ) setup_methods(name) end @@ -164,7 +168,7 @@ module SiteSettingExtension defaults_view = defaults.all # add locale default and defaults based on default_locale, cause they are cached - new_hash = defaults_view.merge(new_hash) + new_hash = defaults_view.merge!(new_hash) # add shadowed shadowed_settings.each { |ss| new_hash[ss] = GlobalSetting.send(ss) } diff --git a/lib/site_settings/defaults_provider.rb b/lib/site_settings/defaults_provider.rb index 920ba5cca9c..c155a01ef1f 100644 --- a/lib/site_settings/defaults_provider.rb +++ b/lib/site_settings/defaults_provider.rb @@ -12,11 +12,10 @@ class SiteSettings::DefaultsProvider def initialize(site_setting) @site_setting = site_setting @site_setting.refresh_settings << DEFAULT_LOCALE_KEY - - @cached = {} @defaults = {} @defaults[DEFAULT_LOCALE.to_sym] = {} - @site_locale = {} + + @site_locales = {} refresh_site_locale! end @@ -31,7 +30,6 @@ class SiteSettings::DefaultsProvider @defaults[locale][name] = v end end - refresh_cache! end def db_all @@ -39,12 +37,14 @@ class SiteSettings::DefaultsProvider end def all - @cached + @defaults[DEFAULT_LOCALE.to_sym].merge(@defaults[self.site_locale.to_sym] || {}) end def get(name) - @cached[name.to_sym] + @defaults.dig(self.site_locale.to_sym, name.to_sym) || + @defaults.dig(DEFAULT_LOCALE.to_sym, name.to_sym) end + alias [] get # Used to override site settings in dev/test env def set_regardless_of_locale(name, value) @@ -53,34 +53,34 @@ class SiteSettings::DefaultsProvider @defaults.each { |_, hash| hash.delete(name) } @defaults[DEFAULT_LOCALE.to_sym][name] = value value, type = @site_setting.type_supervisor.to_db_value(name, value) - @cached[name] = @site_setting.type_supervisor.to_rb_value(name, value, type) + @defaults[self.site_locale.to_sym][name] = @site_setting.type_supervisor.to_rb_value(name, value, type) else raise ArgumentError.new("No setting named '#{name}' exists") end end - alias [] get - def site_locale - @site_locale[current_db] + @site_locales[current_db] end def site_locale=(val) val = val.to_s raise Discourse::InvalidParameters.new(:value) unless LocaleSiteSetting.valid_value?(val) - if val != @site_locale[current_db] + if val != @site_locales[current_db] @site_setting.provider.save(DEFAULT_LOCALE_KEY, val, SiteSetting.types[:string]) refresh_site_locale! @site_setting.refresh! Discourse.request_refresh! end - @site_locale[current_db] + @site_locales[current_db] end - def each - @cached.each { |k, v| yield k.to_sym, v } + def each(&block) + self.all.each do |key, value| + block.call(key.to_sym, value) + end end def locale_setting_hash @@ -91,7 +91,7 @@ class SiteSettings::DefaultsProvider description: @site_setting.description(DEFAULT_LOCALE_KEY), type: SiteSetting.types[SiteSetting.types[:enum]], preview: nil, - value: @site_locale[current_db], + value: @site_locales[current_db], valid_values: LocaleSiteSetting.values, translate_names: LocaleSiteSetting.translate_names? } @@ -99,7 +99,7 @@ class SiteSettings::DefaultsProvider def refresh_site_locale! RailsMultisite::ConnectionManagement.each_connection do |db| - @site_locale[db] = + @site_locales[db] = if GlobalSetting.respond_to?(DEFAULT_LOCALE_KEY) && (global_val = GlobalSetting.send(DEFAULT_LOCALE_KEY)) && !global_val.blank? @@ -110,8 +110,7 @@ class SiteSettings::DefaultsProvider DEFAULT_LOCALE end - refresh_cache! - @site_locale[db] + @site_locales[db] end end @@ -121,12 +120,9 @@ class SiteSettings::DefaultsProvider private - def has_key?(key) - @cached.key?(key) || key == DEFAULT_LOCALE_KEY - end - - def refresh_cache! - @cached = @defaults[DEFAULT_LOCALE.to_sym].merge(@defaults.fetch(@site_locale[current_db].to_sym, {})) + def has_key?(name) + @defaults[self.site_locale.to_sym]&.key?(name) || + @defaults[DEFAULT_LOCALE.to_sym].key?(name) || name == DEFAULT_LOCALE_KEY end def current_db diff --git a/spec/components/site_settings/defaults_provider_spec.rb b/spec/components/site_settings/defaults_provider_spec.rb index 01239fbff9b..465e177377d 100644 --- a/spec/components/site_settings/defaults_provider_spec.rb +++ b/spec/components/site_settings/defaults_provider_spec.rb @@ -40,7 +40,7 @@ describe SiteSettings::DefaultsProvider do end describe 'expose default cache according to locale' do - before(:each) do + before do settings.setting(:test_override, 'default', locale_default: { zh_CN: 'cn' }) settings.setting(:test_default, 'test', regex: '^\S+$') settings.refresh!