Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DEV: improve design of site setting default provider 

This refactors it so "Defaults provider" is only responsible for "defaults"

Locale handling and management of locale settings is moved back into
SiteSettingExtension

This eliminates complex state management using DistributedCache and makes
it way easier to test SiteSettingExtension
This commit is contained in:
Sam 2018-06-01 12:22:47 +10:00
parent d2880246cd
commit f331d2603d
7 changed files with 122 additions and 260 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
lib/site_setting_extension.rb
View File

@ -1,3 +1,5 @@
# frozen_string_literal: true
require_dependency 'site_settings/deprecated_settings'
require_dependency 'site_settings/type_supervisor'
require_dependency 'site_settings/defaults_provider'
@ -5,12 +7,54 @@ require_dependency 'site_settings/db_provider'
module SiteSettingExtension
include SiteSettings::DeprecatedSettings
extend Forwardable
def_delegator :defaults, :site_locale, :default_locale
def_delegator :defaults, :site_locale=, :default_locale=
def_delegator :defaults, :has_setting?
def_delegators 'SiteSettings::TypeSupervisor', :types, :supported_types
# support default_locale being set via global settings
# this also adds support for testing the extension and global settings
# for site locale
def self.extended(klass)
if GlobalSetting.respond_to?(:default_locale) && GlobalSetting.default_locale.present?
klass.send :setup_shadowed_methods, :default_locale, GlobalSetting.default_locale
end
end
# we need a default here to support defaults per locale
def default_locale=(val)
val = val.to_s
raise Discourse::InvalidParameters.new(:value) unless LocaleSiteSetting.valid_value?(val)
if val != self.default_locale
add_override!(:default_locale, val)
refresh!
Discourse.request_refresh!
end
end
def default_locale?
true
end
# set up some sort of default so we can look stuff up
def default_locale
# note optimised cause this is called a lot so avoiding .presence which
# adds 2 method calls
locale = current[:default_locale]
if locale && !locale.blank?
locale
else
SiteSettings::DefaultsProvider::DEFAULT_LOCALE
end
end
def has_setting?(v)
defaults.has_setting?(v)
end
def supported_types
SiteSettings::TypeSupervisor.supported_types
end
def types
SiteSettings::TypeSupervisor.types
end
def listen_for_changes=(val)
@listen_for_changes = val
@ -55,11 +99,11 @@ module SiteSettingExtension
end
def refresh_settings
@refresh_settings ||= []
@refresh_settings ||= [:default_locale]
end
def client_settings
@client_settings ||= []
@client_settings ||= [:default_locale]
end
def previews
@ -73,13 +117,17 @@ module SiteSettingExtension
def setting(name_arg, default = nil, opts = {})
name = name_arg.to_sym
if name == :default_locale
raise ArgumentError.new("Other settings depend on default locale, you can not configure it like this")
end
shadowed_val = nil
mutex.synchronize do
defaults.load_setting(
name,
default,
opts.extract!(*SiteSettings::DefaultsProvider::CONSUMED_OPTS)
opts.delete(:locale_default)
)
categories[name] = opts[:category] || :uncategorized
@ -129,7 +177,7 @@ module SiteSettingExtension
def settings_hash
result = {}
defaults.each_key do |s|
defaults.all.keys.each do |s|
result[s] = send(s).to_s
end
result
@ -147,14 +195,28 @@ module SiteSettingExtension
# Retrieve all settings
def all_settings(include_hidden = false)
defaults
locale_setting_hash =
{
setting: 'default_locale',
default: SiteSettings::DefaultsProvider::DEFAULT_LOCALE,
category: 'required',
description: description('default_locale'),
type: SiteSetting.types[SiteSetting.types[:enum]],
preview: nil,
value: self.default_locale,
valid_values: LocaleSiteSetting.values,
translate_names: LocaleSiteSetting.translate_names?
}
defaults.all(default_locale)
.reject { |s, _| !include_hidden && hidden_settings.include?(s) }
.map do |s, v|
value = send(s)
opts = {
setting: s,
description: description(s),
default: defaults[s].to_s,
default: defaults.get(s, default_locale).to_s,
value: value.to_s,
category: categories[s],
preview: previews[s],
@ -162,7 +224,7 @@ module SiteSettingExtension
}.merge(type_supervisor.type_hash(s))
opts
end.unshift(defaults.locale_setting_hash)
end.unshift(locale_setting_hash)
end
def description(setting)
@ -185,7 +247,7 @@ module SiteSettingExtension
[s.name.to_sym, type_supervisor.to_rb_value(s.name, s.value, s.data_type)]
}.to_a.flatten)]
defaults_view = defaults.all
defaults_view = defaults.all(new_hash[:default_locale])
# add locale default and defaults based on default_locale, cause they are cached
new_hash = defaults_view.merge!(new_hash)
@ -242,7 +304,7 @@ module SiteSettingExtension
def remove_override!(name)
provider.destroy(name)
current[name] = defaults[name]
current[name] = defaults.get(name, default_locale)
clear_cache!
end

100
lib/site_settings/defaults_provider.rb
View File

@ -1,31 +1,23 @@
# frozen_string_literal: true
module SiteSettings; end
# A cache for providing default value based on site locale
class SiteSettings::DefaultsProvider
include Enumerable
CONSUMED_OPTS = %i[default locale_default].freeze
DEFAULT_LOCALE_KEY = :default_locale
DEFAULT_LOCALE = 'en'.freeze
DEFAULT_CATEGORY = 'required'.freeze
@@site_locales ||= DistributedCache.new('site_locales')
DEFAULT_LOCALE = 'en'
def initialize(site_setting)
@site_setting = site_setting
@site_setting.refresh_settings << DEFAULT_LOCALE_KEY
@defaults = {}
@defaults[DEFAULT_LOCALE.to_sym] = {}
refresh_site_locale!
end
def load_setting(name_arg, value, opts = {})
def load_setting(name_arg, value, locale_defaults)
name = name_arg.to_sym
@defaults[DEFAULT_LOCALE.to_sym][name] = value
if (locale_default = opts[:locale_default])
locale_default.each do |locale, v|
if (locale_defaults)
locale_defaults.each do |locale, v|
locale = locale.to_sym
@defaults[locale] ||= {}
@defaults[locale][name] = v
@ -34,15 +26,19 @@ class SiteSettings::DefaultsProvider
end
def db_all
@site_setting.provider.all.delete_if { |s| s.name.to_sym == DEFAULT_LOCALE_KEY }
@site_setting.provider.all
end
def all
@defaults[DEFAULT_LOCALE.to_sym].merge(@defaults[self.site_locale.to_sym] || {})
def all(locale = nil)
if locale
@defaults[DEFAULT_LOCALE.to_sym].merge(@defaults[locale.to_sym] || {})
else
@defaults[DEFAULT_LOCALE.to_sym].dup
end
end
def get(name)
@defaults.dig(self.site_locale.to_sym, name.to_sym) ||
def get(name, locale = DEFAULT_LOCALE)
@defaults.dig(locale.to_sym, name.to_sym) ||
@defaults.dig(DEFAULT_LOCALE.to_sym, name.to_sym)
end
alias [] get
@ -50,81 +46,25 @@ class SiteSettings::DefaultsProvider
# Used to override site settings in dev/test env
def set_regardless_of_locale(name, value)
name = name.to_sym
if @site_setting.has_setting?(name)
if name == :default_locale || @site_setting.has_setting?(name)
@defaults.each { |_, hash| hash.delete(name) }
@defaults[DEFAULT_LOCALE.to_sym][name] = value
value, type = @site_setting.type_supervisor.to_db_value(name, value)
@defaults[self.site_locale.to_sym] ||= {}
@defaults[self.site_locale.to_sym][name] = @site_setting.type_supervisor.to_rb_value(name, value, type)
@defaults[SiteSetting.default_locale.to_sym] ||= {}
@defaults[SiteSetting.default_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
def site_locale
@@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_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_locales[current_db]
end
def each(&block)
self.all.each do |key, value|
block.call(key.to_sym, value)
end
end
def locale_setting_hash
{
setting: DEFAULT_LOCALE_KEY,
default: DEFAULT_LOCALE,
category: DEFAULT_CATEGORY,
description: @site_setting.description(DEFAULT_LOCALE_KEY),
type: SiteSetting.types[SiteSetting.types[:enum]],
preview: nil,
value: @@site_locales[current_db],
valid_values: LocaleSiteSetting.values,
translate_names: LocaleSiteSetting.translate_names?
}
end
def refresh_site_locale!
RailsMultisite::ConnectionManagement.each_connection do |db|
@@site_locales[db] =
if GlobalSetting.respond_to?(DEFAULT_LOCALE_KEY) &&
(global_val = GlobalSetting.send(DEFAULT_LOCALE_KEY)) &&
!global_val.blank?
global_val
elsif (db_val = @site_setting.provider.find(DEFAULT_LOCALE_KEY))
db_val.value.to_s
else
DEFAULT_LOCALE
end
@@site_locales[db]
end
end
def has_setting?(name)
has_key?(name.to_sym) || has_key?("#{name.to_s}?".to_sym)
has_key?(name.to_sym) || has_key?("#{name.to_s}?".to_sym) || name.to_sym == :default_locale
end
private
def has_key?(name)
@defaults[self.site_locale.to_sym]&.key?(name) ||
@defaults[DEFAULT_LOCALE.to_sym].key?(name) || name == DEFAULT_LOCALE_KEY
@defaults[DEFAULT_LOCALE.to_sym].key?(name)
end
def current_db

9
spec/components/site_setting_extension_spec.rb
View File

@ -505,6 +505,15 @@ describe SiteSettingExtension do
end
describe "shadowed_by_global" do
context "default_locale" do
it "supports adding a default locale via a global" do
global_setting :default_locale, 'zh_CN'
settings.default_locale = 'en'
expect(settings.default_locale).to eq('zh_CN')
end
end
context "without global setting" do
before do
settings.setting(:trout_api_key, 'evil', shadowed_by_global: true)

165
spec/components/site_settings/defaults_provider_spec.rb
View File

@ -26,20 +26,7 @@ describe SiteSettings::DefaultsProvider do
new_settings(provider_local)
end
describe 'inserts default_locale into refresh' do
it 'when initialize' do
expect(settings.refresh_settings.include?(SiteSettings::DefaultsProvider::DEFAULT_LOCALE_KEY)).to be_truthy
end
end
describe '.db_all' do
it 'collects values from db except default locale' do
settings.provider.save(SiteSettings::DefaultsProvider::DEFAULT_LOCALE_KEY,
'en',
SiteSetting.types[:string])
expect(settings.defaults.db_all).to eq([])
end
it 'can collect values from db' do
settings.provider.save('try_a', 1, SiteSetting.types[:integer])
settings.provider.save('try_b', 2, SiteSetting.types[:integer])
@ -55,11 +42,9 @@ describe SiteSettings::DefaultsProvider do
end
describe '.all' do
it 'returns all values according to the current locale' do
it 'returns all values according to locale' do
expect(settings.defaults.all).to eq(test_override: 'default', test_default: 'test')
settings.defaults.site_locale = 'zh_CN'
settings.defaults.refresh_site_locale!
expect(settings.defaults.all).to eq(test_override: 'cn', test_default: 'test')
expect(settings.defaults.all('zh_CN')).to eq(test_override: 'cn', test_default: 'test')
end
end
@ -72,11 +57,6 @@ describe SiteSettings::DefaultsProvider do
expect(settings.defaults.get('test_override')).to eq 'default'
end
it 'returns the default value according to current locale' do
expect(settings.defaults.get(:test_override)).to eq 'default'
settings.defaults.site_locale = 'zh_CN'
expect(settings.defaults.get(:test_override)).to eq 'cn'
end
end
describe '.set_regardless_of_locale' do
@ -85,8 +65,7 @@ describe SiteSettings::DefaultsProvider do
it 'sets the default value to a site setting regardless the locale' do
settings.defaults.set_regardless_of_locale(:test_override, val)
expect(settings.defaults.get(:test_override)).to eq val
settings.defaults.site_locale = 'zh_CN'
expect(settings.defaults.get(:test_override)).to eq val
expect(settings.defaults.get(:test_override, 'zh_CN')).to eq val
end
it 'handles the string' do
@ -111,143 +90,13 @@ describe SiteSettings::DefaultsProvider do
}.to raise_error(Discourse::InvalidParameters)
end
end
describe '.each' do
it 'yields the pair of site settings' do
expect { |b| settings.defaults.each(&b) }.to yield_successive_args([:test_override, 'default'], [:test_default, 'test'])
settings.defaults.site_locale = 'zh_CN'
expect { |b| settings.defaults.each(&b) }.to yield_successive_args([:test_override, 'cn'], [:test_default, 'test'])
end
end
end
describe '.site_locale' do
it 'returns the current site locale' do
expect(settings.defaults.site_locale).to eq 'en'
end
context 'when locale is set in the db' do
let(:db_val) { 'zr' }
let(:global_val) { 'gr' }
before do
settings.provider.save(SiteSettings::DefaultsProvider::DEFAULT_LOCALE_KEY,
db_val,
SiteSetting.types[:string])
settings.defaults.refresh_site_locale!
end
it 'should load from database' do
expect(settings.defaults.site_locale).to eq db_val
end
it 'prioritizes GlobalSetting than value from db' do
GlobalSetting.stubs(:default_locale).returns(global_val)
settings.defaults.refresh_site_locale!
expect(settings.defaults.site_locale).to eq global_val
end
it 'ignores blank GlobalSetting' do
GlobalSetting.stubs(:default_locale).returns('')
settings.defaults.refresh_site_locale!
expect(settings.defaults.site_locale).to eq db_val
end
end
end
describe '.site_locale=' do
it 'should store site locale in a distributed cache' do
expect(settings.defaults.class.class_variable_get(:@@site_locales))
.to be_a(DistributedCache)
end
it 'changes and store the current site locale' do
settings.defaults.site_locale = 'zh_CN'
expect(settings.defaults.site_locale).to eq('zh_CN')
end
it 'changes and store the current site locale' do
expect { settings.defaults.site_locale = 'random' }.to raise_error(Discourse::InvalidParameters)
expect(settings.defaults.site_locale).to eq 'en'
end
it "don't change when it's shadowed" do
GlobalSetting.stubs(:default_locale).returns('shadowed')
settings.defaults.site_locale = 'zh_CN'
expect(settings.defaults.site_locale).to eq 'shadowed'
end
it 'refresh_site_locale! when called' do
settings.defaults.expects(:refresh_site_locale!)
settings.defaults.site_locale = 'zh_CN'
end
it 'refreshes the client when changed' do
Discourse.expects(:request_refresh!).once
settings.defaults.site_locale = 'zh_CN'
end
it "doesn't refresh the client when changed" do
Discourse.expects(:request_refresh!).never
settings.defaults.site_locale = 'en'
end
end
describe '.locale_setting_hash' do
it 'returns the hash for client display' do
result = settings.defaults.locale_setting_hash
expect(result[:setting]).to eq(SiteSettings::DefaultsProvider::DEFAULT_LOCALE_KEY)
expect(result[:default]).to eq(SiteSettings::DefaultsProvider::DEFAULT_LOCALE)
expect(result[:type]).to eq(SiteSetting.types[SiteSetting.types[:enum]])
expect(result[:preview]).to be_nil
expect(result[:value]).to eq(SiteSettings::DefaultsProvider::DEFAULT_LOCALE)
expect(result[:category]).to eq(SiteSettings::DefaultsProvider::DEFAULT_CATEGORY)
expect(result[:valid_values]).to eq(LocaleSiteSetting.values)
expect(result[:translate_names]).to eq(LocaleSiteSetting.translate_names?)
expect(result[:description]).not_to be_nil
end
end
describe '.load_setting' do
it 'adds a setting to the cache' do
settings.defaults.load_setting('new_a', 1)
it 'adds a setting to the cache correctly' do
settings.defaults.load_setting('new_a', 1, zh_CN: 7)
expect(settings.defaults[:new_a]).to eq 1
end
it 'takes care of locale default' do
settings.defaults.load_setting(:new_b, 1, locale_default: { zh_CN: 2, zh_TW: 2 })
expect(settings.defaults[:new_b]).to eq 1
end
end
describe '.refresh_site_locale!' do
it 'loads the change to locale' do
expect(settings.defaults.site_locale).to eq 'en'
settings.provider.save(SiteSettings::DefaultsProvider::DEFAULT_LOCALE_KEY,
'zh_CN',
SiteSetting.types[:string])
settings.defaults.refresh_site_locale!
expect(settings.defaults.site_locale).to eq 'zh_CN'
end
it 'loads from GlobalSettings' do
expect(settings.defaults.site_locale).to eq 'en'
GlobalSetting.stubs(:default_locale).returns('fr')
settings.defaults.refresh_site_locale!
expect(settings.defaults.site_locale).to eq 'fr'
end
it 'prioritized GlobalSettings than db' do
expect(settings.defaults.site_locale).to eq 'en'
settings.provider.save(SiteSettings::DefaultsProvider::DEFAULT_LOCALE_KEY,
'zh_CN',
SiteSetting.types[:string])
GlobalSetting.stubs(:default_locale).returns('fr')
settings.defaults.refresh_site_locale!
expect(settings.defaults.site_locale).to eq 'fr'
expect(settings.defaults.get(:new_a, 'zh_CN')).to eq 7
end
end
@ -266,7 +115,7 @@ describe SiteSettings::DefaultsProvider do
end
it 'default_locale always exists' do
expect(settings.defaults.has_setting?(SiteSettings::DefaultsProvider::DEFAULT_LOCALE_KEY)).to be_truthy
expect(settings.defaults.has_setting?(:default_locale)).to be_truthy
end
it 'returns false when the key is not exist' do

16
spec/controllers/admin/site_settings_controller_spec.rb
View File

@ -12,14 +12,18 @@ describe Admin::SiteSettingsController do
end
context 'index' do
it 'returns success' do
it 'returns valid info' do
get :index, format: :json
expect(response).to be_successful
end
json = ::JSON.parse(response.body)
expect(json).to be_present
expect(response.status).to eq(200)
expect(json["site_settings"].length).to be > 100
it 'returns JSON' do
get :index, format: :json
expect(::JSON.parse(response.body)).to be_present
locale = json["site_settings"].select do |s|
s["setting"] == "default_locale"
end
expect(locale.length).to eq(1)
end
end

1
spec/multisite/jobs_spec.rb
View File

@ -5,7 +5,6 @@ RSpec.describe "Running Sidekiq Jobs in Multisite" do
before do
conn.config_filename = "spec/fixtures/multisite/two_dbs.yml"
SiteSetting.defaults.refresh_site_locale!
end
after do

1
spec/rails_helper.rb
View File

@ -153,7 +153,6 @@ RSpec.configure do |config|
SiteSetting.provider.all.each do |setting|
SiteSetting.remove_override!(setting.name)
end
SiteSetting.defaults.site_locale = SiteSettings::DefaultsProvider::DEFAULT_LOCALE
# very expensive IO operations
SiteSetting.automatically_download_gravatars = false