diff options
Diffstat (limited to 'activerecord')
| -rw-r--r-- | activerecord/CHANGELOG.md | 32 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/delegation.rb | 25 | ||||
| -rw-r--r-- | activerecord/test/cases/relation/delegation_test.rb | 15 |
3 files changed, 42 insertions, 30 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index d1db4db8d8..c90ed03ac4 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,20 @@ +* `Relation` no longer has mutator methods like `#map!` and `#delete_if`. Convert + to an `Array` by calling `#to_a` before using these methods. + + It intends to prevent odd bugs and confusion in code that call mutator + methods directly on the `Relation`. + + Example: + + # Instead of this + Author.where(name: 'Hank Moody').compact! + + # Now you have to do this + authors = Author.where(name: 'Hank Moody').to_a + authors.compact! + + *Lauro Caetano* + * Better support for `where()` conditions that use a `belongs_to` association name. @@ -80,21 +97,6 @@ *arthurnn* -* Create a whitelist of delegable methods to `Array`. - - Currently `Relation` directly delegates methods to `Array`. With this change, - only the methods present in this whitelist will be delegated. - - The whitelist contains: - - #&, #+, #[], #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 use any other method, instead first call `#to_a` on the association. - - *Lauro Caetano* - * Use the right column to type cast grouped calculations with custom expressions. Fixes #13230. diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 246c5db5bd..21beed332f 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -1,3 +1,4 @@ +require 'set' require 'active_support/concern' require 'active_support/deprecation' @@ -36,18 +37,13 @@ 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. - # 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 - ] + BLACKLISTED_ARRAY_METHODS = [ + :compact!, :flatten!, :reject!, :reverse!, :rotate!, :map!, + :shuffle!, :slice!, :sort!, :sort_by!, :delete_if, + :keep_if, :pop, :shift, :delete_at, :compact + ].to_set # :nodoc: - delegate(*ARRAY_DELEGATES, to: :to_a) + delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, to: :to_a delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, :to => :klass @@ -119,14 +115,21 @@ module ActiveRecord def respond_to?(method, include_private = false) super || @klass.respond_to?(method, include_private) || + array_delegable?(method) || arel.respond_to?(method, include_private) end protected + def array_delegable?(method) + Array.method_defined?(method) && BLACKLISTED_ARRAY_METHODS.exclude?(method) + end + def method_missing(method, *args, &block) if @klass.respond_to?(method) scoping { @klass.public_send(method, *args, &block) } + elsif array_delegable?(method) + to_a.public_send(method, *args, &block) elsif arel.respond_to?(method) arel.public_send(method, *args, &block) else diff --git a/activerecord/test/cases/relation/delegation_test.rb b/activerecord/test/cases/relation/delegation_test.rb index f295ccbc6a..9b2bfed039 100644 --- a/activerecord/test/cases/relation/delegation_test.rb +++ b/activerecord/test/cases/relation/delegation_test.rb @@ -26,15 +26,22 @@ module ActiveRecord end module DelegationWhitelistBlacklistTests - ActiveRecord::Delegation::ARRAY_DELEGATES.each do |method| + 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 + ] + + ARRAY_DELEGATES.each do |method| define_method "test_delegates_#{method}_to_Array" do assert_respond_to target, method end end - [:compact!, :flatten!, :reject!, :reverse!, :rotate!, - :shuffle!, :slice!, :sort!, :sort_by!, :delete_if, - :keep_if, :pop, :shift, :delete_at, :compact].each do |method| + ActiveRecord::Delegation::BLACKLISTED_ARRAY_METHODS.each do |method| define_method "test_#{method}_is_not_delegated_to_Array" do assert_raises(NoMethodError) { call_method(target, method) } end |