Merge pull request #588 from Shopify/pm/only-deep-symbolize-keys-when…

…-needed

Only deep_symbolize_keys when needed

lib/i18n/backend/base.rb

@@ -221,27 +221,28 @@ def deep_interpolate(locale, data, values = EMPTY_HASH)
         def load_file(filename)
           type = File.extname(filename).tr('.', '').downcase
           raise UnknownFileType.new(type, filename) unless respond_to?(:"load_#{type}", true)
-          data = send(:"load_#{type}", filename)
+          data, keys_symbolized = send(:"load_#{type}", filename)
           unless data.is_a?(Hash)
             raise InvalidLocaleData.new(filename, 'expects it to return a hash, but does not')
           end
-          data.each { |locale, d| store_translations(locale, d || {}) }
+          data.each { |locale, d| store_translations(locale, d || {}, skip_symbolize_keys: keys_symbolized) }
         end
 
         # Loads a plain Ruby translations file. eval'ing the file must yield
         # a Hash containing translation data with locales as toplevel keys.
         def load_rb(filename)
-          eval(IO.read(filename), binding, filename)
+          translations = eval(IO.read(filename), binding, filename)
+          [translations, false]
         end
 
         # Loads a YAML translations file. The data must have locales as
         # toplevel keys.
         def load_yml(filename)
           begin
             if YAML.respond_to?(:unsafe_load_file) # Psych 4.0 way
-              YAML.unsafe_load_file(filename)
+              [YAML.unsafe_load_file(filename), false]
             else
-              YAML.load_file(filename)
+              [YAML.load_file(filename), false]
             end
           rescue TypeError, ScriptError, StandardError => e
             raise InvalidLocaleData.new(filename, e.inspect)
@@ -253,7 +254,12 @@ def load_yml(filename)
         # toplevel keys.
         def load_json(filename)
           begin
-            ::JSON.parse(File.read(filename))
+            # Use #load_file as a proxy for a version of JSON where symbolize_names and freeze are supported.
+            if JSON.respond_to?(:load_file)
+              [::JSON.load_file(filename, symbolize_names: true, freeze: true), true]
+            else
+              [::JSON.parse(File.read(filename)), false]
+            end
           rescue TypeError, StandardError => e
             raise InvalidLocaleData.new(filename, e.inspect)
           end

lib/i18n/backend/gettext.rb

@@ -41,7 +41,7 @@ def set_comment(msgid_or_sym, comment)
         def load_po(filename)
           locale = ::File.basename(filename, '.po').to_sym
           data = normalize(locale, parse(filename))
-          { locale => data }
+          [{ locale => data }, false]
         end
 
         def parse(filename)

lib/i18n/backend/simple.rb

@@ -38,7 +38,7 @@ def store_translations(locale, data, options = EMPTY_HASH)
           end
           locale = locale.to_sym
           translations[locale] ||= Concurrent::Hash.new
-          data = Utils.deep_symbolize_keys(data)
+          data = Utils.deep_symbolize_keys(data) unless options.fetch(:skip_symbolize_keys, false)
           Utils.deep_merge!(translations[locale], data)
         end
 

test/backend/simple_test.rb

@@ -86,18 +86,22 @@ def setup
   end
 
   test "simple load_rb: loads data from a Ruby file" do
-    data = I18n.backend.send(:load_rb, "#{locales_dir}/en.rb")
+    data, _ = I18n.backend.send(:load_rb, "#{locales_dir}/en.rb")
     assert_equal({ :en => { :fuh => { :bah => 'bas' } } }, data)
   end
 
   test "simple load_yml: loads data from a YAML file" do
-    data = I18n.backend.send(:load_yml, "#{locales_dir}/en.yml")
+    data, _ = I18n.backend.send(:load_yml, "#{locales_dir}/en.yml")
     assert_equal({ 'en' => { 'foo' => { 'bar' => 'baz' } } }, data)
   end
 
   test "simple load_json: loads data from a JSON file" do
-    data = I18n.backend.send(:load_json, "#{locales_dir}/en.json")
-    assert_equal({ 'en' => { 'foo' => { 'bar' => 'baz' } } }, data)
+    data, _ = I18n.backend.send(:load_json, "#{locales_dir}/en.json")
+    assert_equal({ :en => { :foo => { :bar => 'baz' } } }, data)
+
+    if JSON.respond_to?(:load_file)
+      assert_predicate data.dig(:en, :foo, :bar), :frozen?
+    end
   end
 
   test "simple load_translations: loads data from known file formats" do
@@ -160,6 +164,20 @@ def setup
     assert_equal 'foo', I18n.t(:'1')
   end
 
+  test "simple store_translations: store translations doesn't deep symbolize keys if skip_symbolize_keys is true" do
+    data = { :foo => {'bar' => 'barfr', 'baz' => 'bazfr'} }
+
+    # symbolized by default
+    store_translations(:fr, data)
+    assert_equal Hash[:foo, {:bar => 'barfr', :baz => 'bazfr'}], translations[:fr]
+
+    I18n.backend.reload!
+
+    # not deep symbolized when configured
+    store_translations(:fr, data, skip_symbolize_keys: true)
+    assert_equal Hash[:foo, {'bar' => 'barfr', 'baz' => 'bazfr'}], translations[:fr]
+  end
+
   # reloading translations
 
   test "simple reload_translations: unloads translations" do

test/test_helper.rb

@@ -45,8 +45,8 @@ def translations
     I18n.backend.instance_variable_get(:@translations)
   end
 
-  def store_translations(locale, data)
-    I18n.backend.store_translations(locale, data)
+  def store_translations(locale, data, options = I18n::EMPTY_HASH)
+    I18n.backend.store_translations(locale, data, options)
   end
 
   def locales_dir