From be310ecac28e98b5ddf6831c7e09ef6c65e2f240 Mon Sep 17 00:00:00 2001
From: Aditya Sanghi <asanghi@me.com>
Date: Wed, 13 Oct 2010 19:57:21 +0530
Subject: [PATCH] Multiparameter attribute fixes for POLA failures

---
 activerecord/lib/active_record/base.rb |   28 ++++++++++++++++++++--------
 activerecord/test/cases/base_test.rb   |   22 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 78b3507..2c5084f 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1736,23 +1736,35 @@ MSG
             klass = (self.class.reflect_on_aggregation(name.to_sym) || column_for_attribute(name)).klass
             # in order to allow a date to be set without a year, we must keep the empty values.
             # Otherwise, we wouldn't be able to distinguish it from a date with an empty day.
-            values = values_with_empty_parameters.reject { |v| v.nil? }
+            # values_with_empty_parameters = {"1" => 2009, "2" => 12, "3" => 31}
+            values = values_with_empty_parameters.values.reject(&:nil?)
 
             if values.empty?
               send(name + "=", nil)
             else
 
+              # Ensure there are no missing bits which cause POLA failures
+              # set_values = [2009, 2, 31]
+              # In case values_with_empty_parameters = {"4" => 23, "5" => 11, "6" => 11}
+              # set_values = [nil, nil, nil, 23, 11, 11]
+              set_values = (1..values_with_empty_parameters.keys.max.to_i).to_a.collect do |position|
+                values_with_empty_parameters[position.to_s]
+              end
+
               value = if Time == klass
-                instantiate_time_object(name, values)
+                # Default the date component to if not given to 0001-01-01
+                [2001,1,1].each_with_index{|v,i| set_values[i] = set_values[i].blank? ? v : set_values[i]}
+                instantiate_time_object(name, set_values)
               elsif Date == klass
                 begin
-                  values = values_with_empty_parameters.collect do |v| v.nil? ? 1 : v end
-                  Date.new(*values)
+                  # Default the date component to if not given to 0001-01-01
+                  [1,1,1].each_with_index{|v,i| set_values[i] = set_values[i].blank? ? v : set_values[i]}
+                  Date.new(*set_values)
                 rescue ArgumentError => ex # if Date.new raises an exception on an invalid date
                   instantiate_time_object(name, values).to_date # we instantiate Time object and convert it back to a date thus using Time's logic in handling invalid dates
                 end
               else
-                klass.new(*values)
+                klass.new(*set_values)
               end
 
               send(name + "=", value)
@@ -1772,13 +1784,13 @@ MSG
         for pair in pairs
           multiparameter_name, value = pair
           attribute_name = multiparameter_name.split("(").first
-          attributes[attribute_name] = [] unless attributes.include?(attribute_name)
+          attributes[attribute_name] = {} unless attributes.include?(attribute_name)
 
           parameter_value = value.empty? ? nil : type_cast_attribute_value(multiparameter_name, value)
-          attributes[attribute_name] << [ find_parameter_position(multiparameter_name), parameter_value ]
+          attributes[attribute_name][find_parameter_position(multiparameter_name)] ||= parameter_value
         end
 
-        attributes.each { |name, values| attributes[name] = values.sort_by{ |v| v.first }.collect { |v| v.last } }
+        attributes
       end
 
       def type_cast_attribute_value(multiparameter_name, value)
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 16fd9a7..a44999a 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -489,6 +489,28 @@ class BasicsTest < ActiveRecord::TestCase
     assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on
   end
 
+  def test_multiparameter_attributes_on_time_with_no_date
+    attributes = {
+      "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00"
+    }
+    topic = Topic.find(1)
+    topic.attributes = attributes
+    assert_equal Time.local(2001, 1, 1, 16, 24, 0), topic.written_on
+  end
+
+  def test_multiparameter_attributes_on_time_with_invalid_time_params
+    begin
+      attributes = {
+        "written_on(4i)" => "2004", "written_on(5i)" => "36", "written_on(6i)" => "64",
+      }
+      topic = Topic.find(1)
+      topic.attributes = attributes
+    rescue ActiveRecord::MultiparameterAssignmentErrors => ex
+      assert_equal(1, ex.errors.size)
+      assert_equal("written_on", ex.errors[0].attribute)
+    end
+  end
+
   def test_multiparameter_attributes_on_time_with_old_date
     attributes = {
       "written_on(1i)" => "1850", "written_on(2i)" => "6", "written_on(3i)" => "24",
-- 
1.7.1