Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce middleware that resets current local thread's config after each request #382

Merged
merged 1 commit into from Oct 14, 2017
Merged

Introduce middleware that resets current local thread's config after each request #382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/i18n.rb
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@ module I18n
autoload :Gettext, 'i18n/gettext'
autoload :Locale, 'i18n/locale'
autoload :Tests, 'i18n/tests'
autoload :Middleware, 'i18n/middleware'

RESERVED_KEYS = [:scope, :default, :separator, :resolve, :object, :fallback, :fallback_in_progress, :format, :cascade, :throw, :raise, :deep_interpolation]
RESERVED_KEYS_PATTERN = /%\{(#{RESERVED_KEYS.join("|")})\}/
Expand Down
15 changes: 15 additions & 0 deletions lib/i18n/middleware.rb
View file
Open in desktop
@@ -0,0 +1,15 @@
module I18n
class Middleware

def initialize(app)
@app = app
end

def call(env)
@app.call(env)
ensure
Thread.current[:i18n_config] = I18n::Config.new
Copy link
Collaborator

@radar radar Oct 1, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this will cause the translations to be reloaded upon every single request. For applications with large amounts of translations, this can lead to an unnecessary slow down on every request.

Since your issue seems to be just that the default locale is not used again for I18n.locale, why not have here instead:

I18n.locale = I18n.default_locale

?

I think this would accomplish what you wish.

Copy link
Contributor Author

@shayonj shayonj Oct 1, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@radar thanks for the review, I don't think it will cause the translations to be re-loaded, as Backend::Simple is initialized as a class variable and once rails is initialized, the same object is persisted/re-used:

https://github.com/svenfuchs/i18n/blob/6ab51c78be833eb011d5b4d3f5b2c54e74847512/lib/i18n/config.rb#L19

You can test this by going to rails console and checking that backend's object_id is always the same:

2.3.3 :003 > I18n::Config.new.backend.object_id
 => 70171618106860 
2.3.3 :004 > I18n::Config.new.backend.object_id
 => 70171618106860

That said, since the main issue is of TLS in a multi-threaded environemt, updating Thread.current itself made sense :). Lmk, if I missed something.

end

end
end
24 changes: 24 additions & 0 deletions test/i18n/middleware_test.rb
View file
Open in desktop
@@ -0,0 +1,24 @@
require 'test_helper'

class I18nMiddlewareTest < I18n::TestCase
def setup
super
I18n.default_locale = :fr
@app = DummyRackApp.new
@middleware = I18n::Middleware.new(@app)
end

test "middleware initializes new config object after request" do
old_i18n_config_object_id = Thread.current[:i18n_config].object_id
@middleware.call({})

updated_i18n_config_object_id = Thread.current[:i18n_config].object_id
assert_not_equal updated_i18n_config_object_id, old_i18n_config_object_id
end

test "succesfully resets i18n locale to default locale by defining new config" do
@middleware.call({})

assert_equal :fr, I18n.locale
end
end
6 changes: 6 additions & 0 deletions test/test_helper.rb
View file
Open in desktop
Expand Up @@ -53,3 +53,9 @@ def locales_dir
File.dirname(__FILE__) + '/test_data/locales'
end
end

class DummyRackApp
def call(env)
I18n.locale = :es
end
end