mirror of
https://github.com/github/rails.git
synced 2026-04-26 03:00:59 -04:00
Merge pull request #396 from asanghi/lh_4346
Multiparameter POLA (principle of least authority) with respect to time_select fixes. See LH4346
This commit is contained in:
@@ -1943,32 +1943,9 @@ MSG
|
||||
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?
|
||||
@@ -1976,19 +1953,65 @@ MSG
|
||||
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)
|
||||
@@ -1996,7 +2019,7 @@ MSG
|
||||
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".
|
||||
|
||||
@@ -575,6 +575,29 @@ 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
|
||||
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 @@ class BasicsTest < ActiveRecord::TestCase
|
||||
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 @@ class BasicsTest < ActiveRecord::TestCase
|
||||
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)
|
||||
|
||||
Reference in New Issue
Block a user