aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwangjohn <wangjohn@mit.edu>2013-02-19 14:36:42 -0500
committerwangjohn <wangjohn@mit.edu>2013-02-20 10:53:25 -0500
commitd49f862b9a6877f02d6fbf90f276a345abfa4372 (patch)
tree446855c56bbebc67ee9e812ceb4c44e447be63e8
parent3a0b6c8e135e268c1550f93db1b63ba27457dec2 (diff)
downloadrails-d49f862b9a6877f02d6fbf90f276a345abfa4372.tar.gz
rails-d49f862b9a6877f02d6fbf90f276a345abfa4372.tar.bz2
rails-d49f862b9a6877f02d6fbf90f276a345abfa4372.zip
Added comments about the check_empty_arguments method which is called
for query methods in a where_clause. Also, modified the CHANGELOG entry because it had false information and added tests.
-rw-r--r--activerecord/CHANGELOG.md6
-rw-r--r--activerecord/lib/active_record/relation/query_methods.rb38
-rw-r--r--activerecord/test/cases/relations_test.rb16
3 files changed, 45 insertions, 15 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index a888b79391..60ebe2ff7c 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -6,13 +6,11 @@
*Yves Senn*
* ActiveRecord now raises an error when blank arguments are passed to query
- methods for which blank arguments do not make sense. This also occurs for
- nil-like objects in arguments.
+ methods for which blank arguments do not make sense.
Example:
- Post.limit() # => raises error
- Post.include([]) # => raises error
+ Post.includes() # => raises error
*John Wang*
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 85534608ac..5358b43728 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -108,7 +108,7 @@ module ActiveRecord
#
# User.includes(:posts).where('posts.name = ?', 'example').references(:posts)
def includes(*args)
- check_empty_arguments("includes", *args)
+ check_empty_arguments("includes", args)
spawn.includes!(*args)
end
@@ -126,7 +126,7 @@ module ActiveRecord
# FROM "users" LEFT OUTER JOIN "posts" ON "posts"."user_id" =
# "users"."id"
def eager_load(*args)
- check_empty_arguments("eager_load", *args)
+ check_empty_arguments("eager_load", args)
spawn.eager_load!(*args)
end
@@ -140,7 +140,7 @@ module ActiveRecord
# User.preload(:posts)
# => SELECT "posts".* FROM "posts" WHERE "posts"."user_id" IN (1, 2, 3)
def preload(*args)
- check_empty_arguments("preload", *args)
+ check_empty_arguments("preload", args)
spawn.preload!(*args)
end
@@ -158,7 +158,7 @@ module ActiveRecord
# User.includes(:posts).where("posts.name = 'foo'").references(:posts)
# # => Query now knows the string references posts, so adds a JOIN
def references(*args)
- check_empty_arguments("references", *args)
+ check_empty_arguments("references", args)
spawn.references!(*args)
end
@@ -238,7 +238,7 @@ module ActiveRecord
# User.group('name AS grouped_name, age')
# => [#<User id: 3, name: "Foo", age: 21, ...>, #<User id: 2, name: "Oscar", age: 21, ...>, #<User id: 5, name: "Foo", age: 23, ...>]
def group(*args)
- check_empty_arguments("group", *args)
+ check_empty_arguments("group", args)
spawn.group!(*args)
end
@@ -269,7 +269,7 @@ module ActiveRecord
# User.order(:name, email: :desc)
# => SELECT "users".* FROM "users" ORDER BY "users"."name" ASC, "users"."email" DESC
def order(*args)
- check_empty_arguments("order", *args)
+ check_empty_arguments("order", args)
spawn.order!(*args)
end
@@ -295,7 +295,7 @@ module ActiveRecord
#
# generates a query with 'ORDER BY name ASC, id ASC'.
def reorder(*args)
- check_empty_arguments("reorder", *args)
+ check_empty_arguments("reorder", args)
spawn.reorder!(*args)
end
@@ -318,8 +318,8 @@ module ActiveRecord
# User.joins("LEFT JOIN bookmarks ON bookmarks.bookmarkable_type = 'Post' AND bookmarks.user_id = users.id")
# => SELECT "users".* FROM "users" LEFT JOIN bookmarks ON bookmarks.bookmarkable_type = 'Post' AND bookmarks.user_id = users.id
def joins(*args)
- check_empty_arguments("joins", *args)
- spawn.joins!(*args.flatten)
+ check_empty_arguments("joins", args)
+ spawn.joins!(*args.compact.flatten)
end
def joins!(*args) # :nodoc:
@@ -483,7 +483,7 @@ module ActiveRecord
#
# Order.having('SUM(price) > 30').group('user_id')
def having(opts, *rest)
- check_empty_arguments("having", opts)
+ opts.blank? ? self : spawn.having!(opts, *rest)
spawn.having!(opts, *rest)
end
@@ -921,7 +921,23 @@ module ActiveRecord
end
end
- def check_empty_arguments(method_name, *args)
+ # Checks to make sure that the arguments are not blank. Note that if some
+ # blank-like object were initially passed into the query method, then this
+ # method will not raise an error.
+ #
+ # Example:
+ #
+ # Post.references() # => raises an error
+ # Post.references([]) # => does not raise an error
+ #
+ # This particular method should be called with a method_name and the args
+ # passed into that method as an input. For example:
+ #
+ # def references(*args)
+ # check_empty_arguments("references", args)
+ # ...
+ # end
+ def check_empty_arguments(method_name, args)
if args.blank?
raise ArgumentError, "The method .#{method_name}() must contain arguments."
end
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 379c0c0758..8298d7534c 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -321,6 +321,22 @@ class RelationTest < ActiveRecord::TestCase
assert_equal 1, person_with_reader_and_post.size
end
+ def test_no_arguments_to_query_methods_raise_errors
+ assert_raises(ArgumentError) { Topic.references() }
+ assert_raises(ArgumentError) { Topic.includes() }
+ assert_raises(ArgumentError) { Topic.preload() }
+ assert_raises(ArgumentError) { Topic.group() }
+ assert_raises(ArgumentError) { Topic.reorder() }
+ end
+
+ def test_blank_like_arguments_to_query_methods_dont_raise_errors
+ assert_nothing_raised { Topic.references([]) }
+ assert_nothing_raised { Topic.includes([]) }
+ assert_nothing_raised { Topic.preload([]) }
+ assert_nothing_raised { Topic.group([]) }
+ assert_nothing_raised { Topic.reorder([]) }
+ end
+
def test_scoped_responds_to_delegated_methods
relation = Topic.all