ruby-i18n / i18n

Internationalization (i18n) library for Ruby
MIT License
988 stars 411 forks source link

[BUG] `with_locale` does not clean up correctly #703

Open sander-deryckere opened 2 months ago

sander-deryckere commented 2 months ago

What I tried to do

Background

In our project, we have certain translated masterdata that can be maintained by users. So the translation strings are stored in the database. But we want to use the current locale from I18n to render the correct strings, together with all the fixed translations.

The default locale is set on the installation level, and can also be configured by an admin.

For this, we have a number of specs that test the difference between only a default locale being present, or a (when executed in the context of a user), the correct user-specific locale being used.

While running these specs, we noticed that after calling code in a with_locale block, the translations started to use wrong locales, and didn't respond to updates to the default_locale setting.

Conclusion

Apparently, the default_locale is stored as locale after running a with_locale block, causing further modifications of default_locale to have no effect as long as the I18n class stays in memory.

See the annotated sequence of setting locales at the bottom of the ticket.

What I expected to happen

I expected the locale to be reset to an empty value after the with_locale block was executed. So it would continue to render strings in the default_locale, even if that value is updated.

According to the documentation (https://guides.rubyonrails.org/i18n.html#managing-the-locale-across-requests):

I18n.locale can leak into subsequent requests served by the same thread/process if it is not consistently set in every controller. For example executing I18n.locale = :es in one POST requests will have effects for all later requests to controllers that don't set the locale, but only in that particular thread/process. For that reason, instead of I18n.locale = you can use I18n.with_locale which does not have this leak issue.

Since the system locale rarely changes, and the I18n object doesn't survive that long due to threading and containers, it's unlikely to affect our production environment. But it was an annoying spec failure to debug 😅 .

What actually happened

The old default_locale was stored as locale, making it impossible for default_locales to have an effect.

Versions of i18n, rails, and anything else you think is necessary

Latest


Bonus points for providing an application or a small code example which reproduces the issue.

Thanks! :heart:

# Start with a default locale, and no config. `locale` falls back to the default locale
(ruby) I18n.locale
:en
(ruby) I18n.config
#<I18n::Config:0x0000ffff9a492a88>
(ruby) I18n.default_locale
:en
# Execute a block, using a `with_locale` wrapper, the block is expected to clean itself up
(ruby) I18n.with_locale(:nl) { puts "current state: #{I18n.config}\n #{I18n.default_locale}\n #{I18n.locale}" }
current state: #<I18n::Config:0x0000ffff9a492a88>
 en
 nl
nil
# Check the resulting state, `I18n.locale` is set back to the default locale, but materialized in the config
(ruby) I18n.config
#<I18n::Config:0x0000ffff9a492a88 @locale=:en>
(ruby) I18n.locale
:en
# Change the default locale to something else, this no longer has an effect since the `with_locale` call materialized the old default locale
(ruby) I18n.default_locale = :nl
:nl
(ruby) I18n.locale
:en
sander-deryckere commented 2 months ago

This is the monkey-patch I came up with to fix the behaviour:

module I18n
  def self.with_locale(tmp_locale = nil)
    if tmp_locale == nil
      yield
    else
      current_locale = self.config.raw_locale
      self.locale = tmp_locale
      begin
        yield
      ensure
        self.config.raw_locale = current_locale
      end
    end
  end

  class Config
    def raw_locale
      @locale
    end

    def raw_locale=(locale)
      @locale = locale
    end
  end
end