Push multi-parameter assignement into the types

This allows us to remove `Type::Value#klass`, as it was only used for
multi-parameter assignment to reach into the types internals. The
relevant type objects now accept a hash in addition to their previous
accepted arguments to `type_cast_from_user`. This required minor
modifications to the tests, since previously they were relying on the
fact that mulit-parameter assignement was reaching into the internals of
time zone aware attributes. In reaility, changing those properties at
runtime wouldn't change the accessor methods for all other forms of
assignment.

11 files changed, 69 insertions, 108 deletions

diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb
index 1040e6e3bb..39077aea7e 100644
--- a/activerecord/lib/active_record/aggregations.rb
+++ b/activerecord/lib/active_record/aggregations.rb
@@ -245,7 +245,8 @@ module ActiveRecord
           define_method("#{name}=") do |part|
             klass = class_name.constantize
             if part.is_a?(Hash)
-              part = klass.new(*part.values)
+              raise ArgumentError unless part.size == part.keys.max
+              part = klass.new(*part.sort.map(&:last))
             end
 
             unless part.is_a?(klass) || converter.nil? || part.nil?
diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb
index fdc90df5b6..483d4200bd 100644
--- a/activerecord/lib/active_record/attribute_assignment.rb
+++ b/activerecord/lib/active_record/attribute_assignment.rb
@@ -50,7 +50,12 @@ module ActiveRecord
       errors = []
       callstack.each do |name, values_with_empty_parameters|
         begin
-          send("#{name}=", MultiparameterAttribute.new(self, name, values_with_empty_parameters).read_value)
+          if values_with_empty_parameters.each_value.all?(&:nil?)
+            values = nil
+          else
+            values = values_with_empty_parameters
+          end
+          send("#{name}=", values)
         rescue => ex
           errors << AttributeAssignmentError.new("error on assignment #{values_with_empty_parameters.values.inspect} to #{name} (#{ex.message})", ex, name)
         end
@@ -82,100 +87,5 @@ module ActiveRecord
     def find_parameter_position(multiparameter_name)
       multiparameter_name.scan(/\(([0-9]*).*\)/).first.first.to_i
     end
-
-    class MultiparameterAttribute #:nodoc:
-      attr_reader :object, :name, :values, :cast_type
-
-      def initialize(object, name, values)
-        @object = object
-        @name   = name
-        @values = values
-      end
-
-      def read_value
-        return if values.values.compact.empty?
-
-        @cast_type = object.type_for_attribute(name)
-        klass = cast_type.klass
-
-        if klass == Time
-          read_time
-        elsif klass == Date
-          read_date
-        else
-          read_other
-        end
-      end
-
-      private
-
-      def instantiate_time_object(set_values)
-        if object.class.send(:create_time_zone_conversion_attribute?, name, cast_type)
-          Time.zone.local(*set_values)
-        else
-          Time.send(object.class.default_timezone, *set_values)
-        end
-      end
-
-      def read_time
-        # If column is a :time (and not :date or :datetime) there is no need to validate if
-        # there are year/month/day fields
-        if cast_type.type == :time
-          # if the column is a time set the values to their defaults as January 1, 1970, but only if they're nil
-          { 1 => 1970, 2 => 1, 3 => 1 }.each do |key,value|
-            values[key] ||= value
-          end
-        else
-          # else column is a timestamp, so if Date bits were not provided, error
-          validate_required_parameters!([1,2,3])
-
-          # If Date bits were provided but blank, then return nil
-          return if blank_date_parameter?
-        end
-
-        max_position = extract_max_param(6)
-        set_values   = values.values_at(*(1..max_position))
-        # If Time bits are not there, then default to 0
-        (3..5).each { |i| set_values[i] = set_values[i].presence || 0 }
-        instantiate_time_object(set_values)
-      end
-
-      def read_date
-        return if blank_date_parameter?
-        set_values = values.values_at(1,2,3)
-        begin
-          Date.new(*set_values)
-        rescue ArgumentError # if Date.new raises an exception on an invalid date
-          instantiate_time_object(set_values).to_date # we instantiate Time object and convert it back to a date thus using Time's logic in handling invalid dates
-        end
-      end
-
-      def read_other
-        max_position = extract_max_param
-        positions    = (1..max_position)
-        validate_required_parameters!(positions)
-
-        values.slice(*positions)
-      end
-
-      # Checks whether some blank date parameter exists. Note that this is different
-      # than the validate_required_parameters! method, since it just checks for blank
-      # positions instead of missing ones, and does not raise in case one blank position
-      # exists. The caller is responsible to handle the case of this returning true.
-      def blank_date_parameter?
-        (1..3).any? { |position| values[position].blank? }
-      end
-
-      # If some position is not provided, it errors out a missing parameter exception.
-      def validate_required_parameters!(positions)
-        if missing_parameter = positions.detect { |position| !values.key?(position) }
-          raise ArgumentError.new("Missing Parameter - #{name}(#{missing_parameter})")
-        end
-      end
-
-      def extract_max_param(upper_cap = 100)
-        [values.keys.max, upper_cap].min
-      end
-    end
   end
 end
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 75cc88d4ee..565ecddde5 100644
--- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
+++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
@@ -11,6 +11,8 @@ module ActiveRecord
         def type_cast_from_user(value)
           if value.is_a?(Array)
             value.map { |v| type_cast_from_user(v) }
+          elsif value.is_a?(Hash)
+            set_time_zone_without_conversion(super)
           elsif value.respond_to?(:in_time_zone)
             begin
               user_input_in_time_zone(value) || super
@@ -20,6 +22,8 @@ module ActiveRecord
           end
         end
 
+        private
+
         def convert_time_to_time_zone(value)
           if value.is_a?(Array)
             value.map { |v| convert_time_to_time_zone(v) }
@@ -29,6 +33,10 @@ module ActiveRecord
             value
           end
         end
+
+        def set_time_zone_without_conversion(value)
+          ::Time.zone.local_to_utc(value).in_time_zone
+        end
       end
 
       extend ActiveSupport::Concern
diff --git a/activerecord/lib/active_record/type.rb b/activerecord/lib/active_record/type.rb
index 250e8d5b23..5678b9c84c 100644
--- a/activerecord/lib/active_record/type.rb
+++ b/activerecord/lib/active_record/type.rb
@@ -1,3 +1,4 @@
+require 'active_record/type/helpers'
 require 'active_record/type/decorator'
 require 'active_record/type/mutable'
 require 'active_record/type/numeric'
diff --git a/activerecord/lib/active_record/type/date.rb b/activerecord/lib/active_record/type/date.rb
index d90a6069b7..3ceab59ebb 100644
--- a/activerecord/lib/active_record/type/date.rb
+++ b/activerecord/lib/active_record/type/date.rb
@@ -1,14 +1,12 @@
 module ActiveRecord
   module Type
     class Date < Value # :nodoc:
+      include Helpers::AcceptsMultiparameterTime.new
+
       def type
         :date
       end
 
-      def klass
-        ::Date
-      end
-
       def type_cast_for_schema(value)
         "'#{value.to_s(:db)}'"
       end
@@ -41,6 +39,11 @@ module ActiveRecord
           ::Date.new(year, mon, mday) rescue nil
         end
       end
+
+      def value_from_multiparameter_assignment(*)
+        time = super
+        time && time.to_date
+      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/type/date_time.rb b/activerecord/lib/active_record/type/date_time.rb
index 0a737815bc..922f0d8fef 100644
--- a/activerecord/lib/active_record/type/date_time.rb
+++ b/activerecord/lib/active_record/type/date_time.rb
@@ -2,6 +2,9 @@ module ActiveRecord
   module Type
     class DateTime < Value # :nodoc:
       include TimeValue
+      include Helpers::AcceptsMultiparameterTime.new(
+        defaults: { 4 => 0, 5 => 0 }
+      )
 
       def type
         :datetime
@@ -42,6 +45,14 @@ module ActiveRecord
 
         new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction, :offset))
       end
+
+      def value_from_multiparameter_assignment(values_hash)
+        missing_parameter = (1..3).detect { |key| !values_hash.key?(key) }
+        if missing_parameter
+          raise ArgumentError, missing_parameter
+        end
+        super
+      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/type/helpers.rb b/activerecord/lib/active_record/type/helpers.rb
new file mode 100644
index 0000000000..c22869c330
--- /dev/null
+++ b/activerecord/lib/active_record/type/helpers.rb
@@ -0,0 +1 @@
+require 'active_record/type/helpers/accepts_multiparameter_time'
diff --git a/activerecord/lib/active_record/type/helpers/accepts_multiparameter_time.rb b/activerecord/lib/active_record/type/helpers/accepts_multiparameter_time.rb
new file mode 100644
index 0000000000..c528d410d3
--- /dev/null
+++ b/activerecord/lib/active_record/type/helpers/accepts_multiparameter_time.rb
@@ -0,0 +1,30 @@
+module ActiveRecord
+  module Type
+    module Helpers
+      class AcceptsMultiparameterTime < Module
+        def initialize(defaults: {})
+          define_method(:type_cast_from_user) do |value|
+            if value.is_a?(Hash)
+              value_from_multiparameter_assignment(value)
+            else
+              super(value)
+            end
+          end
+
+          define_method(:value_from_multiparameter_assignment) do |values_hash|
+            defaults.each do |k, v|
+              values_hash[k] ||= v
+            end
+            return unless values_hash[1] && values_hash[2] && values_hash[3]
+            values = values_hash.sort.map(&:last)
+            ::Time.send(
+              ActiveRecord::Base.default_timezone,
+              *values
+            )
+          end
+          private :value_from_multiparameter_assignment
+        end
+      end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/type/time.rb b/activerecord/lib/active_record/type/time.rb
index cab1c7bf1e..930fb69da0 100644
--- a/activerecord/lib/active_record/type/time.rb
+++ b/activerecord/lib/active_record/type/time.rb
@@ -2,6 +2,9 @@ module ActiveRecord
   module Type
     class Time < Value # :nodoc:
       include TimeValue
+      include Helpers::AcceptsMultiparameterTime.new(
+        defaults: { 1 => 1970, 2 => 1, 3 => 1, 4 => 0, 5 => 0 }
+      )
 
       def type
         :time
diff --git a/activerecord/lib/active_record/type/time_value.rb b/activerecord/lib/active_record/type/time_value.rb
index 8d9ac25643..53188d4d69 100644
--- a/activerecord/lib/active_record/type/time_value.rb
+++ b/activerecord/lib/active_record/type/time_value.rb
@@ -1,10 +1,6 @@
 module ActiveRecord
   module Type
     module TimeValue # :nodoc:
-      def klass
-        ::Time
-      end
-
       def type_cast_for_schema(value)
         "'#{value.to_s(:db)}'"
       end
diff --git a/activerecord/lib/active_record/type/value.rb b/activerecord/lib/active_record/type/value.rb
index b800e33e6f..9027925072 100644
--- a/activerecord/lib/active_record/type/value.rb
+++ b/activerecord/lib/active_record/type/value.rb
@@ -64,9 +64,6 @@ module ActiveRecord
         false
       end
 
-      def klass # :nodoc:
-      end
-
       # Determines whether a value has changed for dirty checking. +old_value+
       # and +new_value+ will always be type-cast. Types should not need to
       # override this method.