-
Notifications
You must be signed in to change notification settings - Fork 148
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.
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
lookup method not reusing @translations #110
Comments
Hi @dvkch, I think the problem here is that the protected If you have an idea how to do that, feel free to open a PR |
I’ll draw up a pour when I’ll be at a computer. I reimplemented the same behavior as this library, but replacing the locale and value field by a jsonb value field, storing a hash of locale to value. I did not have to implement the Translation.lookup method and instead the backend lookup basically turned into |
Correct me if I am wrong, so you are basically keeping one translation row per locale and all translations for the given locale are stored in the jsonb field? |
The opposite. I have a row per key and the value jsonb field contains values for each locale. This allowed me to add a unique index to the key column and easily create an ActiveAdmin page for my Translation model, allowing me to show/edit all localizations for a key at once. This is why I cannot easily send you a PR since the internals are a bit different. But here is the backend class, it is still very similar : require 'i18n/backend/base'
# heavily inspired from https://github.com/svenfuchs/i18n-active_record/, but adapted to work nicely with Mobility gem
module I18n
module Backend
class ActiveRecord
module Implementation
include Base, Flatten
def available_locales
::Translation.available_locales
end
def store_translations(locale, data, options = {})
escape = options.fetch(:escape, true)
create_only = options.fetch(:create_only, false)
Translation.transaction do
flatten_translations(locale, data, escape, false).each do |key, value|
# cleanup conflicts, e.g.: can't have "common.actions" defined if "common.actions.new" is being written...
conflicting_translations = ::Translation.where(key: conflicting_keys(key))
conflicting_translations.destroy_all
# ... and vice versa
conflicting_translations = ::Translation.where('key LIKE ?', key.to_s + '.%')
conflicting_translations.destroy_all
# create new translation
translation = ::Translation.find_or_initialize_by(key: key)
next if create_only && translation.value(locale: locale, fallback: false).present?
translation.update("value_#{locale}" => value)
end
end
reload!
end
def reload!
@translations = nil
self
end
def initialized?
!@translations.nil?
end
def init_translations
if Translation.table_exists?
@translations = ::Translation.to_hash
else
@translations = {}
end
end
def translations(do_init: false)
init_translations if do_init || !initialized?
@translations ||= {}
end
protected
def lookup(locale, key, scope = [], options = EMPTY_HASH)
# flatten the key, e.g.: key="actions.new", scope=["common"] => common.actions.new
key = normalize_flat_keys(locale, key, scope, options[:separator])
# remove leading and trailing dots
key = key.delete_prefix('.').delete_suffix('.')
# fetch results
keys = [locale.to_sym] + key.split(I18n::Backend::Flatten::FLATTEN_SEPARATOR).map(&:to_sym)
translations.dig(*keys)
end
# For a key :'foo.bar.baz' return ['foo', 'foo.bar', 'foo.bar.baz']
def expand_keys(key)
key.to_s.split(FLATTEN_SEPARATOR).inject([]) do |keys, key|
keys << [keys.last, key].compact.join(FLATTEN_SEPARATOR)
end
end
def conflicting_keys(key)
expand_keys(key) - [key.to_s]
end
end
include Implementation
end
end
end As you can see the lookup method can easily use the The Translation model looks like this (I removed some other features that are not related to this issue): class Translation < ApplicationRecord
translates :value # Mobility gem
after_commit :reload_translations
scope :locale, ->(locale) { Mobility.with_locale(locale) { i18n.where.not(value: nil) } }
def self.available_locales
Translation.select('DISTINCT jsonb_object_keys(value_i18n) AS locale').to_a.map(&:locale)
end
def self.to_hash
all.each.with_object({}) do |t, hash|
locales = t.value_i18n.keys
locales.each do |locale|
keys = [locale.to_sym] + t.key.split(I18n::Backend::Flatten::FLATTEN_SEPARATOR).map(&:to_sym)
keys.each.with_index.inject(hash) do |iterator, (key, index)|
if index == keys.size - 1
iterator[key] = t.value(locale: locale, fallback: false)
else
iterator[key] ||= {}
end
iterator[key]
end
end
end
end
validates :key, presence: true, uniqueness: true
protected
def reload_translations
backend = I18n.backend
backend = backend.backends.find { |b| b.is_a?(I18n::Backend::ActiveRecord) } if backend.is_a?(I18n::Backend::Chain)
backend.reload!
end
end I think this is a nice readability improvement and the developer can now assume the primary source of information is the |
Hm, I see what you mean. I haven't thought about all edge case but it might a good addition that can be used to solve performance-related issues. |
Absolutely. There may be edge cases, I haven't been able to fall into some for now, and your test suite works with my fork, adapted for the parts that have been changed on my end of course. |
Great, hiding the feature behind a configuration option will allow us to minimize regressions. |
I'll get to it as soon as I have the time :) i'm not very well versed in gem editing and packaging, plus my use case is a bit different so I'd rather not break too much things 😬 |
Sure! If you need any help just let me know |
I've been keeping up with this as I noticed our application doing lots of queries to the db for translations. So I understand it correctly, are translations looked up in the db every time a call to I18n.t occurs, or is there currently some caching that occurs so that the db request only happens once per I18n.t('something')? If there is a cache, when would that cache be busted so that a new translation from the db could be loaded? |
Yes, that's correct, we hit DB every time the There is a PR that adds |
This has been addressed here #122 |
Hi,
First of all, thanks for this great work, it's a very nice gem seems to cover a lot of cases nicely.
I am currently looking at performance issues and was wondering why the protected
lookup
method is not reusing@translations
? I imagine there is a balance between hitting the DB or parsing a huge in-memory hash, but that could reduce the DB workload in cases where tens or hundreds of translations are being pulled at the same time.Would love to have your input on this!
The text was updated successfully, but these errors were encountered: