Merge pull request #15591 from sgrif/sg-rm-write-attribute

Make `_before_type_cast` actually be before type cast

15 files changed, 64 insertions, 64 deletions

diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
index c1c3987cf5..684be2e845 100644
--- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
+++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
@@ -1,21 +1,22 @@
 module ActiveRecord
   module AttributeMethods
     module TimeZoneConversion
-      class Type # :nodoc:
-        delegate :type, :type_cast_for_database, to: :@column
-
-        def initialize(column)
-          @column = column
+      class Type < SimpleDelegator # :nodoc:
+        def type_cast(value)
+          convert_time_to_time_zone(super)
         end
 
-        def type_cast(value)
-          value = @column.type_cast(value)
-          convert_value_to_time_zone(value)
+        def type_cast_from_user(value)
+          if value.is_a?(Array)
+            value.map { |v| type_cast_from_user(v) }
+          elsif value.respond_to?(:in_time_zone)
+            value.in_time_zone
+          end
         end
 
-        def convert_value_to_time_zone(value)
+        def convert_time_to_time_zone(value)
           if value.is_a?(Array)
-            value.map { |v| convert_value_to_time_zone(v) }
+            value.map { |v| convert_time_to_time_zone(v) }
           elsif value.acts_like?(:time)
             value.in_time_zone
           else
@@ -35,26 +36,6 @@ module ActiveRecord
       end
 
       module ClassMethods
-        protected
-        # Defined for all +datetime+ attributes when +time_zone_aware_attributes+ are enabled.
-        # This enhanced write method will automatically convert the time passed to it to the zone stored in Time.zone.
-        def define_method_attribute=(attr_name)
-          if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name])
-            method_body, line = <<-EOV, __LINE__ + 1
-              def #{attr_name}=(time)
-                time_with_zone = convert_value_to_time_zone(time)
-                previous_time = attribute_changed?("#{attr_name}") ? changed_attributes["#{attr_name}"] : read_attribute(:#{attr_name})
-                write_attribute(:#{attr_name}, time)
-                #{attr_name}_will_change! if previous_time != time_with_zone
-                @attributes["#{attr_name}"] = time_with_zone
-              end
-            EOV
-            generated_attribute_methods.module_eval(method_body, __FILE__, line)
-          else
-            super
-          end
-        end
-
         private
         def create_time_zone_conversion_attribute?(name, column)
           time_zone_aware_attributes &&
@@ -62,16 +43,6 @@ module ActiveRecord
             (:datetime == column.type)
         end
       end
-
-      private
-
-      def convert_value_to_time_zone(value)
-        if value.is_a?(Array)
-          value.map { |v| convert_value_to_time_zone(v) }
-        elsif value.respond_to?(:in_time_zone)
-          value.in_time_zone
-        end
-      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb
index 5203b30462..b72a6219b0 100644
--- a/activerecord/lib/active_record/attribute_methods/write.rb
+++ b/activerecord/lib/active_record/attribute_methods/write.rb
@@ -70,7 +70,7 @@ module ActiveRecord
         attr_name = attr_name.to_s
         attr_name = self.class.primary_key if attr_name == 'id' && self.class.primary_key
         @attributes.delete(attr_name)
-        column = column_for_attribute(attr_name)
+        column = type_for_attribute(attr_name)
 
         unless has_attribute?(attr_name) || self.class.columns_hash.key?(attr_name)
           raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{attr_name}'"
@@ -80,13 +80,11 @@ module ActiveRecord
         # so we don't attempt to typecast multiple times.
         if column.binary?
           @attributes[attr_name] = value
+        elsif should_type_cast
+          @attributes[attr_name] = column.type_cast_from_user(value)
         end
 
-        if should_type_cast
-          @raw_attributes[attr_name] = column.type_cast_for_write(value)
-        else
-          @raw_attributes[attr_name] = value
-        end
+        @raw_attributes[attr_name] = value
       end
     end
   end
diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb
index 23434df1fe..22decbc2da 100644
--- a/activerecord/lib/active_record/connection_adapters/column.rb
+++ b/activerecord/lib/active_record/connection_adapters/column.rb
@@ -17,7 +17,7 @@ module ActiveRecord
 
       delegate :type, :precision, :scale, :limit, :klass, :accessor,
         :text?, :number?, :binary?, :serialized?, :changed?,
-        :type_cast, :type_cast_for_write, :type_cast_for_database,
+        :type_cast, :type_cast_from_user, :type_cast_for_database,
         :type_cast_for_schema,
         to: :cast_type
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb
index a65ca83f77..0a48a14b06 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb
@@ -7,7 +7,11 @@ module ActiveRecord
             :hstore
           end
 
-          def type_cast_for_write(value)
+          def type_cast_from_user(value)
+            type_cast(type_cast_for_database(value))
+          end
+
+          def type_cast_for_database(value)
             ConnectionAdapters::PostgreSQLColumn.hstore_to_string(value)
           end
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb
index c87422fe32..c64cf27797 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb
@@ -7,7 +7,11 @@ module ActiveRecord
             :json
           end
 
-          def type_cast_for_write(value)
+          def type_cast_from_user(value)
+            type_cast(type_cast_for_database(value))
+          end
+
+          def type_cast_for_database(value)
             ConnectionAdapters::PostgreSQLColumn.json_to_string(value)
           end
 
diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb
index baf2b5fbf8..f96f77f696 100644
--- a/activerecord/lib/active_record/model_schema.rb
+++ b/activerecord/lib/active_record/model_schema.rb
@@ -51,6 +51,8 @@ module ActiveRecord
       self.pluralize_table_names = true
 
       self.inheritance_column = 'type'
+
+      delegate :type_for_attribute, to: :class
     end
 
     module ClassMethods
@@ -221,6 +223,10 @@ module ActiveRecord
         @column_types ||= decorate_columns(columns_hash.dup)
       end
 
+      def type_for_attribute(attr_name) # :nodoc:
+        column_types.fetch(attr_name) { column_for_attribute(attr_name) }
+      end
+
       def decorate_columns(columns_hash) # :nodoc:
         return if columns_hash.empty?
 
@@ -245,7 +251,7 @@ module ActiveRecord
       # are the default values suitable for use in `@raw_attriubtes`
       def raw_column_defaults # :nodoc:
         @raw_column_defauts ||= Hash[column_defaults.map { |name, default|
-          [name, columns_hash[name].type_cast_for_write(default)]
+          [name, columns_hash[name].type_cast_for_database(default)]
         }]
       end
 
diff --git a/activerecord/lib/active_record/type/binary.rb b/activerecord/lib/active_record/type/binary.rb
index bc93f6e1bf..3bf29b5026 100644
--- a/activerecord/lib/active_record/type/binary.rb
+++ b/activerecord/lib/active_record/type/binary.rb
@@ -9,7 +9,16 @@ module ActiveRecord
         true
       end
 
+      def type_cast(value)
+        if value.is_a?(Data)
+          value.to_s
+        else
+          super
+        end
+      end
+
       def type_cast_for_database(value)
+        return if value.nil?
         Data.new(super)
       end
 
diff --git a/activerecord/lib/active_record/type/serialized.rb b/activerecord/lib/active_record/type/serialized.rb
index 0866383de9..a1205fa819 100644
--- a/activerecord/lib/active_record/type/serialized.rb
+++ b/activerecord/lib/active_record/type/serialized.rb
@@ -17,15 +17,17 @@ module ActiveRecord
         end
       end
 
-      def type_cast_for_write(value)
+      def type_cast_from_user(value)
+        type_cast(type_cast_for_database(value))
+      end
+
+      def type_cast_for_database(value)
         return if value.nil?
         unless is_default_value?(value)
           super coder.dump(value)
         end
       end
 
-      alias type_cast_for_database type_cast_for_write
-
       def serialized?
         true
       end
diff --git a/activerecord/lib/active_record/type/value.rb b/activerecord/lib/active_record/type/value.rb
index 1c41b28646..deacc398ef 100644
--- a/activerecord/lib/active_record/type/value.rb
+++ b/activerecord/lib/active_record/type/value.rb
@@ -23,8 +23,12 @@ module ActiveRecord
         cast_value(value) unless value.nil?
       end
 
+      def type_cast_from_user(value)
+        type_cast(value)
+      end
+
       def type_cast_for_database(value)
-        type_cast_for_write(value)
+        value
       end
 
       def type_cast_for_schema(value)
@@ -50,10 +54,6 @@ module ActiveRecord
       def klass # :nodoc:
       end
 
-      def type_cast_for_write(value) # :nodoc:
-        value
-      end
-
       # +old_value+ will always be type-cast.
       # +new_value+ will come straight from the database
       # or from assignment, so it could be anything. Types
diff --git a/activerecord/test/cases/adapters/postgresql/composite_test.rb b/activerecord/test/cases/adapters/postgresql/composite_test.rb
index a925263098..0b48fe9af8 100644
--- a/activerecord/test/cases/adapters/postgresql/composite_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/composite_test.rb
@@ -90,7 +90,11 @@ class PostgresqlCompositeWithCustomOIDTest < ActiveRecord::TestCase
       end
     end
 
-    def type_cast_for_write(value)
+    def type_cast_from_user(value)
+      value
+    end
+
+    def type_cast_for_database(value)
       return if value.nil?
       "(#{value.city},#{value.street})"
     end
diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb
index a6482786c7..0b8c9fc052 100644
--- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb
@@ -106,6 +106,7 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase
 
     def test_cast_value_on_write
       x = Hstore.new tags: {"bool" => true, "number" => 5}
+      assert_equal({"bool" => true, "number" => 5}, x.tags_before_type_cast)
       assert_equal({"bool" => "true", "number" => "5"}, x.tags)
       x.save
       assert_equal({"bool" => "true", "number" => "5"}, x.reload.tags)
diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb
index 61d4e2b8ae..eab6049956 100644
--- a/activerecord/test/cases/adapters/postgresql/json_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/json_test.rb
@@ -68,6 +68,7 @@ class PostgresqlJSONTest < ActiveRecord::TestCase
 
   def test_cast_value_on_write
     x = JsonDataType.new payload: {"string" => "foo", :symbol => :bar}
+    assert_equal({"string" => "foo", :symbol => :bar}, x.payload_before_type_cast)
     assert_equal({"string" => "foo", "symbol" => "bar"}, x.payload)
     x.save
     assert_equal({"string" => "foo", "symbol" => "bar"}, x.reload.payload)
diff --git a/activerecord/test/cases/attribute_decorators_test.rb b/activerecord/test/cases/attribute_decorators_test.rb
index f17cc02f53..416eb650f6 100644
--- a/activerecord/test/cases/attribute_decorators_test.rb
+++ b/activerecord/test/cases/attribute_decorators_test.rb
@@ -15,6 +15,8 @@ module ActiveRecord
       def type_cast(value)
         "#{super} #{@decoration}"
       end
+
+      alias type_cast_from_user type_cast
     end
 
     setup do
diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index 2beac84fb1..87f24e32b2 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -616,17 +616,15 @@ class DirtyTest < ActiveRecord::TestCase
     end
   end
 
-  test "defaults with type that implements `type_cast_for_write`" do
+  test "defaults with type that implements `type_cast_for_database`" do
     type = Class.new(ActiveRecord::Type::Value) do
       def type_cast(value)
         value.to_i
       end
 
-      def type_cast_for_write(value)
+      def type_cast_for_database(value)
         value.to_s
       end
-
-      alias type_cast_for_database type_cast_for_write
     end
 
     model_class = Class.new(ActiveRecord::Base) do
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index 793e193329..70d52634f7 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -96,7 +96,7 @@ class ReflectionTest < ActiveRecord::TestCase
     object = Object.new
 
     assert_equal object, column.type_cast(object)
-    assert_equal object, column.type_cast_for_write(object)
+    assert_equal object, column.type_cast_from_user(object)
     assert_equal object, column.type_cast_for_database(object)
   end