Add a bunch of Relation -> Array delegate methods to the whitelist. This won't last - aim to switch back to a blacklist for mutator methods.

2 files changed, 26 insertions, 32 deletions

diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb
index 603e5a9df5..87f5e8e684 100644
--- a/activerecord/lib/active_record/relation/delegation.rb
+++ b/activerecord/lib/active_record/relation/delegation.rb
@@ -36,10 +36,18 @@ module ActiveRecord
     # may vary depending on the klass of a relation, so we create a subclass of Relation
     # for each different klass, and the delegations are compiled into that subclass only.
 
-    delegate :&, :+, :[], :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, :to => :to_a
+    # TODO: This is not going to work. Brittle, painful. We'll switch to a blacklist
+    # to disallow mutator methods like map!, pop, and delete_if instead.
+    ARRAY_DELEGATES = [
+      :+, :-, :|, :&, :[],
+      :all?, :collect, :detect, :each, :each_cons, :each_with_index,
+      :exclude?, :find_all, :flat_map, :group_by, :include?, :length,
+      :map, :none?, :one?, :partition, :reject, :reverse,
+      :sample, :second, :sort, :sort_by, :third,
+      :to_ary, :to_set, :to_xml, :to_yaml
+    ]
+
+    delegate *ARRAY_DELEGATES, to: :to_a
 
     delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key,
              :connection, :columns_hash, :to => :klass
diff --git a/activerecord/test/cases/relation/delegation_test.rb b/activerecord/test/cases/relation/delegation_test.rb
index b56dd5e5db..f295ccbc6a 100644
--- a/activerecord/test/cases/relation/delegation_test.rb
+++ b/activerecord/test/cases/relation/delegation_test.rb
@@ -25,16 +25,9 @@ module ActiveRecord
     end
   end
 
-  class DelegationAssociationTest < DelegationTest
-    def target
-      Post.first.comments
-    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
+  module DelegationWhitelistBlacklistTests
+    ActiveRecord::Delegation::ARRAY_DELEGATES.each do |method|
+      define_method "test_delegates_#{method}_to_Array" do
         assert_respond_to target, method
       end
     end
@@ -42,34 +35,27 @@ module ActiveRecord
     [:compact!, :flatten!, :reject!, :reverse!, :rotate!,
      :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
+      define_method "test_#{method}_is_not_delegated_to_Array" do
         assert_raises(NoMethodError) { call_method(target, method) }
       end
     end
   end
 
-  class DelegationRelationTest < DelegationTest
-    fixtures :comments
+  class DelegationAssociationTest < DelegationTest
+    include DelegationWhitelistBlacklistTests
 
     def target
-      Comment.all
+      Post.first.comments
     end
+  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
+  class DelegationRelationTest < DelegationTest
+    include DelegationWhitelistBlacklistTests
 
-    [:compact!, :flatten!, :reject!, :reverse!, :rotate!,
-     :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
+    fixtures :comments
+
+    def target
+      Comment.all
     end
   end
 end