From e168c5fde3a66ac13c445c17dd2a8776f86bfe57 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 19 Nov 2015 16:36:59 -0500 Subject: [PATCH] PERF: Much more performant, multisite aware I18n overrides --- config/initializers/i18n.rb | 1 + .../20140120155706_add_lounge_category.rb | 2 +- .../20140122043508_add_meta_category.rb | 2 +- .../20140227201005_add_staff_category.rb | 2 +- db/migrate/20141014191645_fix_tos_name.rb | 2 +- ...50728210202_migrate_old_moderator_posts.rb | 2 +- ...20150729150523_migrate_auto_close_posts.rb | 2 +- lib/freedom_patches/translate_accelerator.rb | 52 +++++++++++++++++-- lib/i18n/backend/discourse_i18n.rb | 34 ++++++------ spec/components/discourse_i18n_spec.rb | 45 ++++++++++++---- 10 files changed, 108 insertions(+), 36 deletions(-) diff --git a/config/initializers/i18n.rb b/config/initializers/i18n.rb index d9084154b32..5d1ecc6b006 100644 --- a/config/initializers/i18n.rb +++ b/config/initializers/i18n.rb @@ -3,5 +3,6 @@ require 'i18n/backend/discourse_i18n' I18n.backend = I18n::Backend::DiscourseI18n.new I18n.config.missing_interpolation_argument_handler = proc { throw(:exception) } +I18n.reload! MessageBus.subscribe("/i18n-flush") { I18n.reload! } diff --git a/db/migrate/20140120155706_add_lounge_category.rb b/db/migrate/20140120155706_add_lounge_category.rb index b0dd781e1bd..b43b8b97e10 100644 --- a/db/migrate/20140120155706_add_lounge_category.rb +++ b/db/migrate/20140120155706_add_lounge_category.rb @@ -2,7 +2,7 @@ class AddLoungeCategory < ActiveRecord::Migration def up return if Rails.env.test? - I18n.backend.overrides_disabled do + I18n.overrides_disabled do result = Category.exec_sql "SELECT 1 FROM site_settings where name = 'lounge_category_id'" if result.count == 0 description = I18n.t('vip_category_description') diff --git a/db/migrate/20140122043508_add_meta_category.rb b/db/migrate/20140122043508_add_meta_category.rb index 782519663bb..1e17de6f463 100644 --- a/db/migrate/20140122043508_add_meta_category.rb +++ b/db/migrate/20140122043508_add_meta_category.rb @@ -2,7 +2,7 @@ class AddMetaCategory < ActiveRecord::Migration def up return if Rails.env.test? - I18n.backend.overrides_disabled do + I18n.overrides_disabled do result = Category.exec_sql "SELECT 1 FROM site_settings where name = 'meta_category_id'" if result.count == 0 description = I18n.t('meta_category_description') diff --git a/db/migrate/20140227201005_add_staff_category.rb b/db/migrate/20140227201005_add_staff_category.rb index 3d3cccdec64..c8b7075b303 100644 --- a/db/migrate/20140227201005_add_staff_category.rb +++ b/db/migrate/20140227201005_add_staff_category.rb @@ -2,7 +2,7 @@ class AddStaffCategory < ActiveRecord::Migration def up return if Rails.env.test? - I18n.backend.overrides_disabled do + I18n.overrides_disabled do result = Category.exec_sql "SELECT 1 FROM site_settings where name = 'staff_category_id'" if result.count == 0 description = I18n.t('staff_category_description') diff --git a/db/migrate/20141014191645_fix_tos_name.rb b/db/migrate/20141014191645_fix_tos_name.rb index 1c2f3d5ab64..56059dd95f9 100644 --- a/db/migrate/20141014191645_fix_tos_name.rb +++ b/db/migrate/20141014191645_fix_tos_name.rb @@ -1,6 +1,6 @@ class FixTosName < ActiveRecord::Migration def up - I18n.backend.overrides_disabled do + I18n.overrides_disabled do execute ActiveRecord::Base.sql_fragment('UPDATE user_fields SET name = ? WHERE name = ?', I18n.t('terms_of_service.title'), I18n.t("terms_of_service.signup_form_message")) end diff --git a/db/migrate/20150728210202_migrate_old_moderator_posts.rb b/db/migrate/20150728210202_migrate_old_moderator_posts.rb index 47cc9654320..a741854b0a1 100644 --- a/db/migrate/20150728210202_migrate_old_moderator_posts.rb +++ b/db/migrate/20150728210202_migrate_old_moderator_posts.rb @@ -1,7 +1,7 @@ class MigrateOldModeratorPosts < ActiveRecord::Migration def migrate_key(action_code) - I18n.backend.overrides_disabled do + I18n.overrides_disabled do text = I18n.t("topic_statuses.#{action_code.gsub('.', '_')}") execute "UPDATE posts SET action_code = '#{action_code}', raw = '', cooked = '', post_type = 3 where post_type = 2 AND raw = #{ActiveRecord::Base.connection.quote(text)}" diff --git a/db/migrate/20150729150523_migrate_auto_close_posts.rb b/db/migrate/20150729150523_migrate_auto_close_posts.rb index 798338b2934..d00ec9623ad 100644 --- a/db/migrate/20150729150523_migrate_auto_close_posts.rb +++ b/db/migrate/20150729150523_migrate_auto_close_posts.rb @@ -1,6 +1,6 @@ class MigrateAutoClosePosts < ActiveRecord::Migration def up - I18n.backend.overrides_disabled do + I18n.overrides_disabled do strings = [] %w(days hours lastpost_days lastpost_hours lastpost_minutes).map do |k| strings << I18n.t("topic_statuses.autoclosed_enabled_#{k}.one") diff --git a/lib/freedom_patches/translate_accelerator.rb b/lib/freedom_patches/translate_accelerator.rb index 604b85d04cc..68ac930c951 100644 --- a/lib/freedom_patches/translate_accelerator.rb +++ b/lib/freedom_patches/translate_accelerator.rb @@ -19,6 +19,10 @@ module I18n def reload! @loaded_locales = [] @cache = nil + + @overrides_enabled = true + @overrides_by_site = {} + reload_no_cache! end @@ -48,18 +52,60 @@ module I18n load_locale(locale) unless @loaded_locales.include?(locale) end - def translate(key, *args) - load_locale(config.locale) unless @loaded_locales.include?(config.locale) + # In some environments such as migrations we don't want to use overrides. + # Use this to disable them over a block of ruby code + def overrides_disabled + @overrides_enabled = false + yield + ensure + @overrides_enabled = true + end + + def translate_no_override(key, *args) return translate_no_cache(key, *args) if args.length > 0 @cache ||= LruRedux::ThreadSafeCache.new(LRU_CACHE_SIZE) - k = "#{key}#{config.locale}#{config.backend.object_id}#{RailsMultisite::ConnectionManagement.current_db}" + k = "#{key}#{config.locale}#{config.backend.object_id}" @cache.getset(k) do translate_no_cache(key).freeze end end + def translate(key, *args) + load_locale(config.locale) unless @loaded_locales.include?(config.locale) + + if @overrides_enabled + site = RailsMultisite::ConnectionManagement.current_db + + by_site = @overrides_by_site[site] + + by_locale = nil + unless by_site + by_site = @overrides_by_site[site] = {} + + # Load overrides + TranslationOverride.where(locale: locale).pluck(:translation_key, :value).each do |tuple| + by_locale = by_site[locale] ||= {} + by_locale[tuple[0]] = tuple[1] + end + end + + by_locale = by_site[config.locale] + if by_locale + if args.size > 0 && args[0].is_a?(Hash) + args[0][:overrides] = by_locale + return backend.translate(config.locale, key, args[0]) + end + + if result = by_locale[key] + return result + end + end + end + translate_no_override(key, *args) + end + alias_method :t, :translate end end diff --git a/lib/i18n/backend/discourse_i18n.rb b/lib/i18n/backend/discourse_i18n.rb index 02567a46f44..6abe29ca2ba 100644 --- a/lib/i18n/backend/discourse_i18n.rb +++ b/lib/i18n/backend/discourse_i18n.rb @@ -6,10 +6,6 @@ module I18n include I18n::Backend::Fallbacks include I18n::Backend::Pluralization - def initialize - @overrides_enabled = true - end - def available_locales # in case you are wondering this is: # Dir.glob( File.join(Rails.root, 'config', 'locales', 'client.*.yml') ) @@ -29,22 +25,10 @@ module I18n return site_overrides[locale] if site_overrides[locale] locale_overrides = site_overrides[locale] = {} - TranslationOverride.where(locale: locale).pluck(:translation_key, :value).each do |tuple| - locale_overrides[tuple[0]] = tuple[1] - end locale_overrides end - # In some environments such as migrations we don't want to use overrides. - # Use this to disable them over a block of ruby code - def overrides_disabled - @overrides_enabled = false - yield - ensure - @overrides_enabled = true - end - # force explicit loading def load_translations(*filenames) unless filenames.empty? @@ -56,8 +40,22 @@ module I18n [locale, SiteSetting.default_locale.to_sym, :en].uniq.compact end - def translate(locale, key, options = {}) - (@overrides_enabled && overrides_for(locale)[key]) || super(locale, key, options) + def lookup(locale, key, scope = [], options = {}) + + # Support interpolation and pluralization of overrides + if options[:overrides] + if options[:count] + result = {} + options[:overrides].each do |k, v| + result[k.split('.').last.to_sym] = v if k != key && k.start_with?(key.to_s) + end + return result if result.size > 0 + end + + return options[:overrides][key] if options[:overrides][key] + end + + super(locale, key, scope, options) end def exists?(locale, key) diff --git a/spec/components/discourse_i18n_spec.rb b/spec/components/discourse_i18n_spec.rb index 6f2f02813a3..551c05aa344 100644 --- a/spec/components/discourse_i18n_spec.rb +++ b/spec/components/discourse_i18n_spec.rb @@ -7,17 +7,22 @@ describe I18n::Backend::DiscourseI18n do let(:backend) { I18n::Backend::DiscourseI18n.new } before do - backend.reload! - backend.store_translations(:en, :foo => 'Foo in :en', :bar => 'Bar in :en') + I18n.reload! + backend.store_translations(:en, :foo => 'Foo in :en', :bar => 'Bar in :en', :wat => "Hello %{count}") backend.store_translations(:en, :items => {:one => 'one item', :other => "%{count} items" }) backend.store_translations(:de, :bar => 'Bar in :de') backend.store_translations(:'de-AT', :baz => 'Baz in :de-AT') end + after do + I18n.reload! + end + it 'translates the basics as expected' do expect(backend.translate(:en, 'foo')).to eq("Foo in :en") expect(backend.translate(:en, 'items', count: 1)).to eq("one item") expect(backend.translate(:en, 'items', count: 3)).to eq("3 items") + expect(backend.translate(:en, 'wat', count: 3)).to eq("Hello 3") end describe '#exists?' do @@ -53,16 +58,38 @@ describe I18n::Backend::DiscourseI18n do end describe 'with overrides' do - before do + it 'returns the overriden key' do TranslationOverride.upsert!('en', 'foo', 'Overwritten foo') - end - - it 'returns the overrided key' do - expect(backend.translate(:en, 'foo')).to eq('Overwritten foo') + expect(I18n.translate('foo')).to eq('Overwritten foo') TranslationOverride.upsert!('en', 'foo', 'new value') - backend.reload! - expect(backend.translate(:en, 'foo')).to eq('new value') + I18n.reload! + expect(I18n.translate('foo')).to eq('new value') + end + + it 'supports disabling' do + TranslationOverride.upsert!('en', 'foo', 'meep') + + I18n.overrides_disabled do + expect(I18n.translate('foo')).to eq('meep') + end + end + + it 'supports interpolation' do + TranslationOverride.upsert!('en', 'foo', 'hello %{world}') + expect(I18n.translate('foo', world: 'foo')).to eq('hello foo') + end + + it 'supports interpolation named count' do + TranslationOverride.upsert!('en', 'wat', 'goodbye %{count}') + expect(I18n.translate('wat', count: 123)).to eq('goodbye 123') + end + + it 'supports one and other' do + TranslationOverride.upsert!('en', 'items.one', 'one fish') + TranslationOverride.upsert!('en', 'items.other', '%{count} fishies') + expect(I18n.translate('items', count: 13)).to eq('13 fishies') + expect(I18n.translate('items', count: 1)).to eq('one fish') end end