Make `_before_type_cast` actually be before type cast

- The following is now true for all types, all the time
  - `model.attribute_before_type_cast == given_value`
  - `model.attribute == model.save_and_reload.attribute`
  - `model.attribute == model.dup.attribute`
  - `model.attribute == YAML.load(YAML.dump(model)).attribute`
- Removes the remaining types implementing `type_cast_for_write`
- Simplifies the implementation of time zone aware attributes
- Brings tz aware attributes closer to being implemented as an attribute
  decorator
- Adds additional point of control for custom types

activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb

 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 @@ def convert_value_to_time_zone(value)
       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 @@ def create_time_zone_conversion_attribute?(name, column)
             (: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

activerecord/lib/active_record/attribute_methods/write.rb

@@ -70,7 +70,7 @@ def write_attribute_with_type_cast(attr_name, value, should_type_cast)
         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 @@ def write_attribute_with_type_cast(attr_name, value, should_type_cast)
         # 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

activerecord/lib/active_record/connection_adapters/column.rb

@@ -17,7 +17,7 @@ module Format
 
       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
 

activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb

@@ -7,7 +7,11 @@ def type
             :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
 

activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb

@@ -7,7 +7,11 @@ def type
             :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
 

activerecord/lib/active_record/model_schema.rb

@@ -51,6 +51,8 @@ module ModelSchema
       self.pluralize_table_names = true
 
       self.inheritance_column = 'type'
+
+      delegate :type_for_attribute, to: :class
     end
 
     module ClassMethods
@@ -221,6 +223,10 @@ def column_types # :nodoc:
         @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 @@ def column_defaults
       # 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
 

activerecord/lib/active_record/type/binary.rb

@@ -9,7 +9,16 @@ def binary?
         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
 

activerecord/lib/active_record/type/serialized.rb

@@ -17,15 +17,17 @@ def type_cast(value)
         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

activerecord/lib/active_record/type/value.rb

@@ -23,8 +23,12 @@ def type_cast(value)
         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 @@ def serialized?
       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

activerecord/test/cases/adapters/postgresql/composite_test.rb

@@ -90,7 +90,11 @@ def type_cast(value)
       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

activerecord/test/cases/adapters/postgresql/hstore_test.rb

@@ -106,6 +106,7 @@ def change
 
     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)

activerecord/test/cases/adapters/postgresql/json_test.rb

@@ -68,6 +68,7 @@ def test_change_table_supports_json
 
   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)

activerecord/test/cases/attribute_decorators_test.rb

@@ -15,6 +15,8 @@ def initialize(delegate, decoration = "decorated!")
       def type_cast(value)
         "#{super} #{@decoration}"
       end
+
+      alias type_cast_from_user type_cast
     end
 
     setup do

activerecord/test/cases/dirty_test.rb

@@ -616,17 +616,15 @@ def test_datetime_attribute_doesnt_change_if_zone_is_modified_in_string
     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

activerecord/test/cases/reflection_test.rb

@@ -96,7 +96,7 @@ def test_non_existent_columns_are_identity_types
     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