diff options
| -rw-r--r-- | activerecord/CHANGELOG | 3 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 64 | ||||
| -rw-r--r-- | activerecord/test/finder_test.rb | 16 |
3 files changed, 50 insertions, 33 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 52068e461e..606f87f678 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,8 @@ *SVN* +* Support nil and Array in :conditions => { attr => value } hashes. #6548 [Assaf, Jeremy Kemper] + find(:all, :conditions => { :topic_id => [1, 2, 3], :last_read => nil } + * Consistently use LOWER() for uniqueness validations (rather than mixing with UPPER()) so the database can always use a functional index on the lowercased column. #6495 [Si] * SQLite: fix calculations workaround, remove count(distinct) query rewrite, cleanup test connection scripts. [Jeremy Kemper] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 3fc9b4bddb..c09cef1118 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -986,7 +986,7 @@ module ActiveRecord #:nodoc: options.update(:limit => 1) unless options[:include] find_every(options).first end - + def find_every(options) records = scoped?(:find, :include) || options[:include] ? find_with_associations(options) : @@ -996,11 +996,11 @@ module ActiveRecord #:nodoc: records end - + def find_from_ids(ids, options) - expects_array = ids.first.kind_of?(Array) + expects_array = ids.first.kind_of?(Array) return ids.first if expects_array && ids.first.empty? - + ids = ids.flatten.compact.uniq case ids.size @@ -1192,16 +1192,16 @@ module ActiveRecord #:nodoc: attribute_names = extract_attribute_names_from_match(match) super unless all_attributes_exists?(attribute_names) - conditions = construct_conditions_from_arguments(attribute_names, arguments) + attributes = construct_attributes_from_arguments(attribute_names, arguments) case extra_options = arguments[attribute_names.size] when nil - options = { :conditions => conditions } + options = { :conditions => attributes } set_readonly_option!(options) ActiveSupport::Deprecation.silence { send(finder, options) } when Hash - finder_options = extra_options.merge(:conditions => conditions) + finder_options = extra_options.merge(:conditions => attributes) validate_find_options(finder_options) set_readonly_option!(finder_options) @@ -1215,7 +1215,7 @@ module ActiveRecord #:nodoc: else ActiveSupport::Deprecation.silence do - send(deprecated_finder, conditions, *arguments[attribute_names.length..-1]) + send(deprecated_finder, sanitize_sql(attributes), *arguments[attribute_names.length..-1]) end end elsif match = /find_or_(initialize|create)_by_([_a-zA-Z]\w*)/.match(method_id.to_s) @@ -1223,9 +1223,11 @@ module ActiveRecord #:nodoc: attribute_names = extract_attribute_names_from_match(match) super unless all_attributes_exists?(attribute_names) - options = { :conditions => construct_conditions_from_arguments(attribute_names, arguments) } + attributes = construct_attributes_from_arguments(attribute_names, arguments) + options = { :conditions => attributes } set_readonly_option!(options) - find_initial(options) || send(instantiator, construct_attributes_from_arguments(attribute_names, arguments)) + + find_initial(options) || send(instantiator, attributes) else super end @@ -1247,12 +1249,6 @@ module ActiveRecord #:nodoc: match.captures.last.split('_and_') end - def construct_conditions_from_arguments(attribute_names, arguments) - conditions = [] - attribute_names.each_with_index { |name, idx| conditions << "#{table_name}.#{connection.quote_column_name(name)} #{attribute_condition(arguments[idx])} " } - [ conditions.join(" AND "), *arguments[0...attribute_names.length] ] - end - def construct_attributes_from_arguments(attribute_names, arguments) attributes = {} attribute_names.each_with_index { |name, idx| attributes[name] = arguments[idx] } @@ -1275,7 +1271,7 @@ module ActiveRecord #:nodoc: def expand_id_conditions(id_or_conditions) case id_or_conditions when Array, Hash then id_or_conditions - else construct_conditions_from_arguments([primary_key], [id_or_conditions]) + else sanitize_sql(primary_key => id_or_conditions) end end @@ -1377,26 +1373,32 @@ module ActiveRecord #:nodoc: klass.base_class.name end - #Accepts an array, hash, or string of sql conditions and - #deals with them accordingly + # Accepts an array, hash, or string of sql conditions and sanitizes + # them into a valid SQL fragment. # ["name='%s' and group_id='%s'", "foo'bar", 4] returns "name='foo''bar' and group_id='4'" # { :name => "foo'bar", :group_id => 4 } returns "name='foo''bar' and group_id='4'" # "name='foo''bar' and group_id='4'" returns "name='foo''bar' and group_id='4'" def sanitize_sql(condition) - return sanitize_sql_array(condition) if condition.is_a?(Array) - return sanitize_sql_hash(condition) if condition.is_a?(Hash) - condition + case condition + when Array; sanitize_sql_array(condition) + when Hash; sanitize_sql_hash(condition) + else condition + end end - - # Accepts a hash of conditions. The hash has each key/value or attribute/value pair - # sanitized and interpolated into the sql statement. - # { :name => "foo'bar", :group_id => 4 } returns "name='foo''bar' and group_id= 4" - def sanitize_sql_hash(hash) - hash.collect { |attrib, value| - "#{table_name}.#{connection.quote_column_name(attrib)} = #{quote_value(value)}" - }.join(" AND ") + + # Sanitizes a hash of attribute/value pairs into SQL conditions. + # { :name => "foo'bar", :group_id => 4 } + # # => "name='foo''bar' and group_id= 4" + # { :status => nil, :group_id => [1,2,3] } + # # => "status IS NULL and group_id IN (1,2,3)" + def sanitize_sql_hash(attrs) + conditions = attrs.map do |attr, value| + "#{table_name}.#{connection.quote_column_name(attr)} #{attribute_condition(value)}" + end.join(' AND ') + + replace_bind_variables(conditions, attrs.values) end - + # Accepts an array of conditions. The array has each value # sanitized and interpolated into the sql statement. # ["name='%s' and group_id='%s'", "foo'bar", 4] returns "name='foo''bar' and group_id='4'" diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb index ebc21b5e0c..cf2dfcbb24 100644 --- a/activerecord/test/finder_test.rb +++ b/activerecord/test/finder_test.rb @@ -161,10 +161,22 @@ class FinderTest < Test::Unit::TestCase Company.find(:first, :conditions => { :id => 2, :dhh => true }) } end - + def test_hash_condition_find_with_escaped_characters Company.create("name" => "Ain't noth'n like' \#stuff") - assert Company.find(:first, :conditions => { :name => "Ain't noth'n like' \#stuff"}) + assert Company.find(:first, :conditions => { :name => "Ain't noth'n like' \#stuff" }) + end + + def test_hash_condition_find_with_array + p1, p2 = Post.find(:all, :limit => 2, :order => 'id asc') + assert_equal [p1, p2], Post.find(:all, :conditions => { :id => [p1, p2] }, :order => 'id asc') + assert_equal [p1, p2], Post.find(:all, :conditions => { :id => [p1, p2.id] }, :order => 'id asc') + end + + def test_hash_condition_find_with_nil + topic = Topic.find(:first, :conditions => { :last_read => nil } ) + assert_not_nil topic + assert_nil topic.last_read end def test_bind_variables |