clean up and refactor active record container backend

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
 
@@ -78,14 +81,12 @@ def get_column_type
 
       # @!macro backend_iterator
       def each_locale
-        model[column_name].each do |l, v|
-          yield l.to_sym if v.present?
+        model[column_name].each_key do |l|
+          yield l.to_sym
         end
       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,23 @@ def initialize_dup(source)
       private
 
       def model_translations(locale)
-        model[column_name][locale] ||= {}
+        model[column_name][locale.to_s]
       end
 
       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
+        locale_translations = model_translations(locale)
 
-      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
+        if locale_translations
+          if value.nil?
+            locale_translations.delete(attribute.to_s)
+
+            # delete empty locale hash if last attribute was just deleted
+            model[column_name].delete(locale.to_s) if locale_translations.empty?
           else
-            raise ArgumentError, "Attribute is supposed to be a Hash, but was a #{obj.class}. -- #{obj.inspect}"
+            locale_translations[attribute.to_s] = value
           end
-        end
-
-        def self.load(obj)
-          obj
+        elsif !value.nil?
+          model[column_name][locale.to_s] = { attribute.to_s => value }
         end
       end
 

spec/mobility/backends/active_record/container_spec.rb

@@ -20,6 +20,36 @@
     include_accessor_examples 'ContainerPost'
     include_dup_examples 'ContainerPost'
     include_cache_key_examples 'ContainerPost'
+
+    it 'does not change translations and dirty tracking' do
+      post = ContainerPost.create!
+
+      aggregate_failures "on access" do
+        expect { post.title }
+          .to not_change { post.translations }.from({})
+          .and not_change { post.changes }.from({})
+          .and not_change { post.changed? }.from(false)
+      end
+
+      aggregate_failures "on reload" do
+        expect { post.reload }
+          .to not_change { post.translations }.from({})
+          .and not_change { post.changes }.from({})
+          .and not_change { post.changed? }.from(false)
+      end
+    end
+
+    it 'deletes locale hash if last attribute is removed' do
+      post = ContainerPost.create!
+
+      ::Mobility.with_locale(:en) { post.title = 'Title en' }
+      ::Mobility.with_locale(:de) { post.title = 'Title de' }
+
+      expect { post.title = nil }
+        .to change { post.translations }
+        .from({ "en" => { "title" => "Title en" }, "de" => { "title" => "Title de" }})
+        .to({ "de" => { "title" => "Title de" }})
+    end
   end
 
   context "with query plugin" do

spec/support/matchers/not_change.rb

@@ -0,0 +1 @@
+RSpec::Matchers.define_negated_matcher :not_change, :change