diff options
author | Rafael Mendonça França <rafaelmfranca@gmail.com> | 2013-12-12 14:42:39 -0800 |
---|---|---|
committer | Rafael Mendonça França <rafaelmfranca@gmail.com> | 2013-12-12 14:42:39 -0800 |
commit | 81433addc2951457e7e201297ae3cd9e83a2affb (patch) | |
tree | 0383c58cee0132b8a862b61bff8b9168aadac460 /activerecord/test | |
parent | 94cd08be38dc086714be8f6cbe17e77f778e1ecc (diff) | |
parent | 1244aa7c5f871da5e907a39242b63a7aee3308a3 (diff) | |
download | rails-81433addc2951457e7e201297ae3cd9e83a2affb.tar.gz rails-81433addc2951457e7e201297ae3cd9e83a2affb.tar.bz2 rails-81433addc2951457e7e201297ae3cd9e83a2affb.zip |
Merge pull request #12590 from laurocaetano/whitelist-to-delegate-array-methods
Create a whitelist of methods to be delegated to Array.
Diffstat (limited to 'activerecord/test')
-rw-r--r-- | activerecord/test/cases/relation/delegation_test.rb | 91 | ||||
-rw-r--r-- | activerecord/test/cases/relations_test.rb | 63 |
2 files changed, 79 insertions, 75 deletions
diff --git a/activerecord/test/cases/relation/delegation_test.rb b/activerecord/test/cases/relation/delegation_test.rb index 13677797cf..b56dd5e5db 100644 --- a/activerecord/test/cases/relation/delegation_test.rb +++ b/activerecord/test/cases/relation/delegation_test.rb @@ -6,22 +6,21 @@ module ActiveRecord class DelegationTest < ActiveRecord::TestCase fixtures :posts - def assert_responds(target, method) - assert target.respond_to?(method) - assert_nothing_raised do - method_arity = target.to_a.method(method).arity + def call_method(target, method) + method_arity = target.to_a.method(method).arity - if method_arity.zero? - target.send(method) - elsif method_arity < 0 - if method == :shuffle! - target.send(method) - else - target.send(method, 1) - end + if method_arity.zero? + target.public_send(method) + elsif method_arity < 0 + if method == :shuffle! + target.public_send(method) else - raise NotImplementedError + target.public_send(method, 1) end + elsif method_arity == 1 + target.public_send(method, 1) + else + raise NotImplementedError end end end @@ -31,31 +30,20 @@ module ActiveRecord Post.first.comments end - [:map, :collect].each do |method| - test "##{method} is delegated" do - assert_responds(target, method) - assert_equal(target.pluck(:body), target.send(method) {|post| post.body }) - end - - test "##{method}! is not delegated" do - assert_deprecated do - assert_responds(target, "#{method}!") - end + [:&, :+, :[], :all?, :collect, :detect, :each, :each_cons, + :each_with_index, :flat_map, :group_by, :include?, :length, + :map, :none?, :one?, :reverse, :sample, :second, :sort, :sort_by, + :to_ary, :to_set, :to_xml, :to_yaml].each do |method| + test "association delegates #{method} to Array" do + assert_respond_to target, method end end [:compact!, :flatten!, :reject!, :reverse!, :rotate!, - :shuffle!, :slice!, :sort!, :sort_by!].each do |method| - test "##{method} delegation is deprecated" do - assert_deprecated do - assert_responds(target, method) - end - end - end - - [:select!, :uniq!].each do |method| - test "##{method} is implemented" do - assert_responds(target, method) + :shuffle!, :slice!, :sort!, :sort_by!, :delete_if, + :keep_if, :pop, :shift, :delete_at, :compact].each do |method| + test "#{method} is not delegated to Array" do + assert_raises(NoMethodError) { call_method(target, method) } end end end @@ -64,36 +52,23 @@ module ActiveRecord fixtures :comments def target - Comment.where(body: 'Normal type') + Comment.all end - [:map, :collect].each do |method| - test "##{method} is delegated" do - assert_responds(target, method) - assert_equal(target.pluck(:body), target.send(method) {|post| post.body }) - end - - test "##{method}! is not delegated" do - assert_deprecated do - assert_responds(target, "#{method}!") - end + [:&, :+, :[], :all?, :collect, :detect, :each, :each_cons, + :each_with_index, :flat_map, :group_by, :include?, :length, + :map, :none?, :one?, :reverse, :sample, :second, :sort, :sort_by, + :to_ary, :to_set, :to_xml, :to_yaml].each do |method| + test "relation delegates #{method} to Array" do + assert_respond_to target, method end end [:compact!, :flatten!, :reject!, :reverse!, :rotate!, - :shuffle!, :slice!, :sort!, :sort_by!].each do |method| - test "##{method} delegation is deprecated" do - assert_deprecated do - assert_responds(target, method) - end - end - end - - [:select!, :uniq!].each do |method| - test "##{method} triggers an immutable error" do - assert_raises ActiveRecord::ImmutableRelation do - assert_responds(target, method) - end + :shuffle!, :slice!, :sort!, :sort_by!, :delete_if, + :keep_if, :pop, :shift, :delete_at, :compact].each do |method| + test "#{method} is not delegated to Array" do + assert_raises(NoMethodError) { call_method(target, method) } end end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 09724d9884..2ccf4c7578 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1506,26 +1506,55 @@ class RelationTest < ActiveRecord::TestCase assert !Post.all.respond_to?(:by_lifo) end - class OMGTopic < ActiveRecord::Base - self.table_name = 'topics' + test "merge collapses wheres from the LHS only" do + left = Post.where(title: "omg").where(comments_count: 1) + right = Post.where(title: "wtf").where(title: "bbq") - def self.__omg__ - "omgtopic" - end + expected = [left.where_values[1]] + right.where_values + merged = left.merge(right) + + assert_equal expected, merged.where_values + assert !merged.to_sql.include?("omg") + assert merged.to_sql.include?("wtf") + assert merged.to_sql.include?("bbq") end - test "delegations do not clash across classes" do - begin - class ::Array - def __omg__ - "array" - end - end + def test_merging_removes_rhs_bind_parameters + left = Post.where(id: Arel::Nodes::BindParam.new('?')) + column = Post.columns_hash['id'] + left.bind_values += [[column, 20]] + right = Post.where(id: 10) - assert_equal "array", Topic.all.__omg__ - assert_equal "omgtopic", OMGTopic.all.__omg__ - ensure - Array.send(:remove_method, :__omg__) - end + merged = left.merge(right) + assert_equal [], merged.bind_values + end + + def test_merging_keeps_lhs_bind_parameters + column = Post.columns_hash['id'] + binds = [[column, 20]] + + right = Post.where(id: Arel::Nodes::BindParam.new('?')) + right.bind_values += binds + left = Post.where(id: 10) + + merged = left.merge(right) + assert_equal binds, merged.bind_values + end + + def test_merging_reorders_bind_params + post = Post.first + id_column = Post.columns_hash['id'] + title_column = Post.columns_hash['title'] + + bv = Post.connection.substitute_at id_column, 0 + + right = Post.where(id: bv) + right.bind_values += [[id_column, post.id]] + + left = Post.where(title: bv) + left.bind_values += [[title_column, post.title]] + + merged = left.merge(right) + assert_equal post, merged.first end end |