Skip to content
Browse files

Multiparameter POLA, time_select fixes. See LH4346

  • Loading branch information...
1 parent 3e9bb54 commit 7d0f26796d4bff7bb3fdc6c105bb3a4f5555b75a Aditya Sanghi committed May 5, 2011
Showing with 187 additions and 29 deletions.
  1. +52 −29 activerecord/lib/active_record/base.rb
  2. +135 −0 activerecord/test/cases/base_test.rb
View
81 activerecord/lib/active_record/base.rb
@@ -1943,60 +1943,83 @@ def execute_callstack_for_multiparameter_attributes(callstack)
errors = []
callstack.each do |name, values_with_empty_parameters|
begin
- 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? }
-
- if values.empty?
- send(name + "=", nil)
- else
-
- value = if Time == klass
- instantiate_time_object(name, values)
- elsif Date == klass
- begin
- values = values_with_empty_parameters.collect do |v| v.nil? ? 1 : v end
- Date.new(*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)
- end
-
- send(name + "=", value)
- end
+ send(name + "=", read_value_from_parameter(name, values_with_empty_parameters))
rescue => ex
- errors << AttributeAssignmentError.new("error on assignment #{values.inspect} to #{name}", ex, name)
+ errors << AttributeAssignmentError.new("error on assignment #{values_with_empty_parameters.values.inspect} to #{name}", ex, name)
end
end
unless errors.empty?
raise MultiparameterAssignmentErrors.new(errors), "#{errors.size} error(s) on assignment of multiparameter attributes"
end
end
+ def read_value_from_parameter(name, values_hash_from_param)
+ klass = (self.class.reflect_on_aggregation(name.to_sym) || column_for_attribute(name)).klass
+ if values_hash_from_param.values.all?{|v|v.nil?}
+ nil
+ elsif klass == Time
+ read_time_parameter_value(name, values_hash_from_param)
+ elsif klass == Date
+ read_date_parameter_value(name, values_hash_from_param)
+ else
+ read_other_parameter_value(klass, name, values_hash_from_param)
+ end
+ end
+
+ def read_time_parameter_value(name, values_hash_from_param)
+ # If Date bits were not provided, error
+ raise "Missing Parameter" if [1,2,3].any?{|position| !values_hash_from_param.has_key?(position)}
+ max_position = extract_max_param_for_multiparameter_attributes(values_hash_from_param, 6)
+ set_values = (1..max_position).collect{|position| values_hash_from_param[position] }
+ # If Date bits were provided but blank, then default to 1
+ # If Time bits are not there, then default to 0
+ [1,1,1,0,0,0].each_with_index{|v,i| set_values[i] = set_values[i].blank? ? v : set_values[i]}
+ instantiate_time_object(name, set_values)
+ end
+
+ def read_date_parameter_value(name, values_hash_from_param)
+ set_values = (1..3).collect{|position| values_hash_from_param[position].blank? ? 1 : values_hash_from_param[position]}
+ begin
+ Date.new(*set_values)
+ rescue ArgumentError => ex # if Date.new raises an exception on an invalid date
+ instantiate_time_object(name, 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_parameter_value(klass, name, values_hash_from_param)
+ max_position = extract_max_param_for_multiparameter_attributes(values_hash_from_param)
+ values = (1..max_position).collect do |position|
+ raise "Missing Parameter" if !values_hash_from_param.has_key?(position)
+ values_hash_from_param[position]
+ end
+ klass.new(*values)
+ end
+
+ def extract_max_param_for_multiparameter_attributes(values_hash_from_param, upper_cap = 100)
+ [values_hash_from_param.keys.max,upper_cap].min
+ end
+
def extract_callstack_for_multiparameter_attributes(pairs)
attributes = { }
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)
multiparameter_name =~ /\([0-9]*([if])\)/ ? value.send("to_" + $1) : value
end
def find_parameter_position(multiparameter_name)
- multiparameter_name.scan(/\(([0-9]*).*\)/).first.first
+ multiparameter_name.scan(/\(([0-9]*).*\)/).first.first.to_i
end
# Returns a comma-separated pair list, like "key1 = val1, key2 = val2".
View
135 activerecord/test/cases/base_test.rb
@@ -575,6 +575,29 @@ def test_multiparameter_attributes_on_time
assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on
end
+ def test_multiparameter_attributes_on_time_with_no_date
+ ex = assert_raise(ActiveRecord::MultiparameterAssignmentErrors) do
+ attributes = {
+ "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00"
+ }
+ topic = Topic.find(1)
+ topic.attributes = attributes
+ end
+ assert_equal("written_on", ex.errors[0].attribute)
+ end
+
+ def test_multiparameter_attributes_on_time_with_invalid_time_params
+ ex = assert_raise(ActiveRecord::MultiparameterAssignmentErrors) do
+ attributes = {
+ "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24",
+ "written_on(4i)" => "2004", "written_on(5i)" => "36", "written_on(6i)" => "64",
+ }
+ topic = Topic.find(1)
+ topic.attributes = attributes
+ end
+ assert_equal("written_on", ex.errors[0].attribute)
+ end
+
def test_multiparameter_attributes_on_time_with_old_date
attributes = {
"written_on(1i)" => "1850", "written_on(2i)" => "6", "written_on(3i)" => "24",
@@ -586,6 +609,82 @@ def test_multiparameter_attributes_on_time_with_old_date
assert_equal "1850-06-24 16:24:00", topic.written_on.to_s(:db)
end
+ def test_multiparameter_attributes_on_time_will_raise_on_big_time_if_missing_date_parts
+ ex = assert_raise(ActiveRecord::MultiparameterAssignmentErrors) do
+ attributes = {
+ "written_on(4i)" => "16", "written_on(5i)" => "24"
+ }
+ topic = Topic.find(1)
+ topic.attributes = attributes
+ end
+ assert_equal("written_on", ex.errors[0].attribute)
+ end
+
+ def test_multiparameter_attributes_on_time_with_raise_on_small_time_if_missing_date_parts
+ ex = assert_raise(ActiveRecord::MultiparameterAssignmentErrors) do
+ attributes = {
+ "written_on(4i)" => "16", "written_on(5i)" => "12", "written_on(6i)" => "02"
+ }
+ topic = Topic.find(1)
+ topic.attributes = attributes
+ end
+ assert_equal("written_on", ex.errors[0].attribute)
+ end
+
+ def test_multiparameter_attributes_on_time_will_ignore_hour_if_missing
+ attributes = {
+ "written_on(1i)" => "2004", "written_on(2i)" => "12", "written_on(3i)" => "12",
+ "written_on(5i)" => "12", "written_on(6i)" => "02"
+ }
+ topic = Topic.find(1)
+ topic.attributes = attributes
+ assert_equal Time.local(2004, 12, 12, 0, 12, 2), topic.written_on
+ end
+
+ def test_multiparameter_attributes_on_time_will_ignore_hour_if_blank
+ attributes = {
+ "written_on(1i)" => "", "written_on(2i)" => "", "written_on(3i)" => "",
+ "written_on(4i)" => "", "written_on(5i)" => "12", "written_on(6i)" => "02"
+ }
+ topic = Topic.find(1)
+ topic.attributes = attributes
+ assert_equal 1, topic.written_on.year
+ assert_equal 1, topic.written_on.month
+ assert_equal 1, topic.written_on.day
+ assert_equal 0, topic.written_on.hour
+ assert_equal 12, topic.written_on.min
+ assert_equal 2, topic.written_on.sec
+ end
+
+ def test_multiparameter_attributes_on_time_will_ignore_date_if_empty
+ attributes = {
+ "written_on(1i)" => "", "written_on(2i)" => "", "written_on(3i)" => "",
+ "written_on(4i)" => "16", "written_on(5i)" => "24"
+ }
+ topic = Topic.find(1)
+ topic.attributes = attributes
+ assert_equal 1, topic.written_on.year
+ assert_equal 1, topic.written_on.month
+ assert_equal 1, topic.written_on.day
+ assert_equal 16, topic.written_on.hour
+ assert_equal 24, topic.written_on.min
+ assert_equal 0, topic.written_on.sec
+ end
+ def test_multiparameter_attributes_on_time_with_seconds_will_ignore_date_if_empty
+ attributes = {
+ "written_on(1i)" => "", "written_on(2i)" => "", "written_on(3i)" => "",
+ "written_on(4i)" => "16", "written_on(5i)" => "12", "written_on(6i)" => "02"
+ }
+ topic = Topic.find(1)
+ topic.attributes = attributes
+ assert_equal 1, topic.written_on.year
+ assert_equal 1, topic.written_on.month
+ assert_equal 1, topic.written_on.day
+ assert_equal 16, topic.written_on.hour
+ assert_equal 12, topic.written_on.min
+ assert_equal 02, topic.written_on.sec
+ end
+
def test_multiparameter_attributes_on_time_with_utc
ActiveRecord::Base.default_timezone = :utc
attributes = {
@@ -692,6 +791,42 @@ def test_multiparameter_assignment_of_aggregation
assert_equal address, customer.address
end
+ def test_multiparameter_assignment_of_aggregation_out_of_order
+ customer = Customer.new
+ address = Address.new("The Street", "The City", "The Country")
+ attributes = { "address(3)" => address.country, "address(2)" => address.city, "address(1)" => address.street }
+ customer.attributes = attributes
+ assert_equal address, customer.address
+ end
+
+ def test_multiparameter_assignment_of_aggregation_with_missing_values
+ ex = assert_raise(ActiveRecord::MultiparameterAssignmentErrors) do
+ customer = Customer.new
+ address = Address.new("The Street", "The City", "The Country")
+ attributes = { "address(2)" => address.city, "address(3)" => address.country }
+ customer.attributes = attributes
+ end
+ assert_equal("address", ex.errors[0].attribute)
+ end
+
+ def test_multiparameter_assignment_of_aggregation_with_blank_values
+ customer = Customer.new
+ address = Address.new("The Street", "The City", "The Country")
+ attributes = { "address(1)" => "", "address(2)" => address.city, "address(3)" => address.country }
+ customer.attributes = attributes
+ assert_equal Address.new(nil, "The City", "The Country"), customer.address
+ end
+
+ def test_multiparameter_assignment_of_aggregation_with_large_index
+ ex = assert_raise(ActiveRecord::MultiparameterAssignmentErrors) do
+ customer = Customer.new
+ address = Address.new("The Street", "The City", "The Country")
+ attributes = { "address(1)" => "The Street", "address(2)" => address.city, "address(3000)" => address.country }
+ customer.attributes = attributes
+ end
+ assert_equal("address", ex.errors[0].attribute)
+ end
+
def test_attributes_on_dummy_time
# Oracle, and Sybase do not have a TIME datatype.
return true if current_adapter?(:OracleAdapter, :SybaseAdapter)

0 comments on commit 7d0f267

Please sign in to comment.
Something went wrong with that request. Please try again.