clean up and refactor active record container backend

fixes dirty tracking and creating empty translation hashes on read

fixes shioyama#540

lib/mobility/backends/active_record/container.rb

@@ -22,7 +22,11 @@ class ActiveRecord::Container
       # @param [Hash] options
       # @return [String,Integer,Boolean] Value of translation
       def read(locale, _ = nil)
-        model_translations(locale)[attribute]
+        locale_translations = model_translations(locale)
+
+        return unless locale_translations
+
+        locale_translations[attribute.to_s]
       end
 
       # @note Translation may be a string, integer, boolean, hash or array
@@ -33,7 +37,7 @@ def read(locale, _ = nil)
       # @return [String,Integer,Boolean] Updated value
       def write(locale, value, _ = nil)
         set_attribute_translation(locale, value)
-        model_translations(locale)[attribute]
+        read(locale)
       end
       # @!endgroup
 
@@ -42,8 +46,7 @@ class << self
         # @option options [Symbol] column_name (:translations) Name of column on which to store translations
         # @raise [InvalidColumnType] if the type of the container column is not json or jsonb
         def configure(options)
-          options[:column_name] ||= :translations
-          options[:column_name] = options[:column_name].to_sym
+          options[:column_name] = options[:column_name]&.to_sym || :translations
         end
         # @!endgroup
 
@@ -84,8 +87,6 @@ def each_locale
       end
 
       setup do |_attributes, options|
-        store options[:column_name], coder: Coder
-
         # Fix for duping depth-2 jsonb column in AR < 5.0
         if ::ActiveRecord::VERSION::STRING < '5.0'
           column_name = options[:column_name]
@@ -107,32 +108,22 @@ def initialize_dup(source)
       private
 
       def model_translations(locale)
-        model[column_name][locale] ||= {}
-      end
+        translations = model[column_name]
 
-      def set_attribute_translation(locale, value)
-        translations = model[column_name] || {}
-        translations[locale.to_s] ||= {}
-        translations[locale.to_s][attribute] = value
-        model[column_name] = translations
-      end
+        return unless translations
 
-      class Coder
-        def self.dump(obj)
-          if obj.is_a? ::Hash
-            obj.inject({}) do |translations, (locale, value)|
-              value.each do |k, v|
-                (translations[locale] ||= {})[k] = v if v.present?
-              end
-              translations
-            end
-          else
-            raise ArgumentError, "Attribute is supposed to be a Hash, but was a #{obj.class}. -- #{obj.inspect}"
-          end
-        end
+        translations[locale.to_s]
+      end
 
-        def self.load(obj)
-          obj
+      def set_attribute_translation(locale, value)
+        locale_translations = model_translations(locale)
+
+        if locale_translations
+          locale_translations[attribute.to_s] = value
+          locale_translations.compact!
+          model[column_name].compact!
+        elsif value
+          (model[column_name] ||= {})[locale.to_s] = { attribute.to_s => value }
         end
       end
 

spec/mobility/backends/active_record/container_spec.rb

@@ -20,6 +20,19 @@
     include_accessor_examples 'ContainerPost'
     include_dup_examples 'ContainerPost'
     include_cache_key_examples 'ContainerPost'
+
+    it 'does not change translations on access' do
+      post = ContainerPost.new
+
+      expect { post.title }.not_to change { post.translations }.from({})
+    end
+
+    it 'does not mix up dirty tracking on access' do
+      post = ContainerPost.new
+
+      expect { post.title }.not_to change { post.changes }.from({})
+      expect(post.changed?).to be(false)
+    end
   end
 
   context "with query plugin" do