From d49f862b9a6877f02d6fbf90f276a345abfa4372 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Tue, 19 Feb 2013 14:36:42 -0500 Subject: [PATCH] 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. --- activerecord/CHANGELOG.md | 6 +-- .../active_record/relation/query_methods.rb | 38 +++++++++++++------ activerecord/test/cases/relations_test.rb | 16 ++++++++ 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 @@ def create_with_value # :nodoc: # # 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 @@ def includes!(*args) # :nodoc: # 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 @@ def eager_load!(*args) # :nodoc: # 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 @@ def preload!(*args) # :nodoc: # 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 @@ def select!(*fields) # :nodoc: # User.group('name AS grouped_name, age') # => [#, #, #] def group(*args) - check_empty_arguments("group", *args) + check_empty_arguments("group", args) spawn.group!(*args) end @@ -269,7 +269,7 @@ def group!(*args) # :nodoc: # 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 @@ def order!(*args) # :nodoc: # # 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 @@ def reorder!(*args) # :nodoc: # 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 @@ def where!(opts = :chain, *rest) # :nodoc: # # 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 @@ def validate_order_args(args) 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 @@ def test_joins_with_string_array 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 -- GitLab