From 1bac04e854b42fc0e47162e251105434d356d2b4 Mon Sep 17 00:00:00 2001 From: Daniel Schierbeck Date: Mon, 29 Aug 2011 13:07:03 +0200 Subject: Optimize the performance of #delegate Remove the use of #__send__ in order to boost performance. This also means that you can no longer delegate to private methods on the target object. --- .../active_support/core_ext/module/delegation.rb | 22 +++++++++++++--------- activesupport/test/core_ext/module_test.rb | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index af92b869fd..0ea58d4224 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -124,23 +124,27 @@ class Module file, line = caller.first.split(':', 2) line = line.to_i - if allow_nil - methods.each do |method| + methods.each do |method| + method = method.to_s + + # Attribute writer methods only accept one argument. Makes sure []= + # methods still accept two arguments. + definition = (method =~ /[^\]]=$/) ? "arg" : "*args, &block" + + if allow_nil module_eval(<<-EOS, file, line - 2) - def #{method_prefix}#{method}(*args, &block) # def customer_name(*args, &block) + def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block) if #{to} || #{to}.respond_to?(:#{method}) # if client || client.respond_to?(:name) - #{to}.__send__(:#{method}, *args, &block) # client.__send__(:name, *args, &block) + #{to}.#{method}(#{definition}) # client.name(*args, &block) end # end end # end EOS - end - else - methods.each do |method| + else exception = %(raise "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}") module_eval(<<-EOS, file, line - 1) - def #{method_prefix}#{method}(*args, &block) # def customer_name(*args, &block) - #{to}.__send__(:#{method}, *args, &block) # client.__send__(:name, *args, &block) + def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block) + #{to}.#{method}(#{definition}) # client.name(*args, &block) rescue NoMethodError # rescue NoMethodError if #{to}.nil? # if client.nil? #{exception} # # add helpful message to the exception diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index 09ca4e7296..6e1b3ca010 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -60,6 +60,14 @@ Tester = Struct.new(:client) do delegate :name, :to => :client, :prefix => false end +class ParameterSet + delegate :[], :[]=, :to => :@params + + def initialize + @params = {:foo => "bar"} + end +end + class Name delegate :upcase, :to => :@full_name @@ -83,6 +91,17 @@ class ModuleTest < ActiveSupport::TestCase assert_equal "Fred", @david.place.name end + def test_delegation_to_index_get_method + @params = ParameterSet.new + assert_equal "bar", @params[:foo] + end + + def test_delegation_to_index_set_method + @params = ParameterSet.new + @params[:foo] = "baz" + assert_equal "baz", @params[:foo] + end + def test_delegation_down_hierarchy assert_equal "CHICAGO", @david.upcase end -- cgit v1.2.3 From 2310db373ba915e77fdc1dcd780068ad98d242cd Mon Sep 17 00:00:00 2001 From: Daniel Schierbeck Date: Thu, 1 Sep 2011 15:07:40 +0200 Subject: Change API docs regarding delegation to non-public methods --- activesupport/lib/active_support/core_ext/module/delegation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index 0ea58d4224..ee8adae1cb 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -1,5 +1,5 @@ class Module - # Provides a delegate class method to easily expose contained objects' methods + # Provides a delegate class method to easily expose contained objects' public methods # as your own. Pass one or more methods (specified as symbols or strings) # and the name of the target object via the :to option (also a symbol # or string). At least one method and the :to option are required. -- cgit v1.2.3 From 3b1537a7d1c19d7f4ab6ae1a405ec4c2c186a55c Mon Sep 17 00:00:00 2001 From: Daniel Schierbeck Date: Thu, 1 Sep 2011 15:23:32 +0200 Subject: Document the changes to delegate in the guides --- guides/source/active_support_core_extensions.textile | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/guides/source/active_support_core_extensions.textile b/guides/source/active_support_core_extensions.textile index 5d0a3f82e8..181dac72cf 100644 --- a/guides/source/active_support_core_extensions.textile +++ b/guides/source/active_support_core_extensions.textile @@ -883,6 +883,26 @@ delegate :size, :to => :attachment, :prefix => :avatar In the previous example the macro generates +avatar_size+ rather than +size+. +WARNING: You can only delegate to public methods on the target object. Trying to delegate to a private or protected method will raise a +NoMethodError+ when the delegate is called. + +If you need to delegate to a private or protected method, you will need to implement the delegation method yourself. It's usually rather simple to do using +__send__+: + + +class Wrapper + def initialize + # Target#zizzle is a private method. + @target = Target.new + end + + def zizzle(*args, &block) + # __send__ circumvents the private/protected mechanism. + @target.__send__(:zizzle, *args, &block) + end +end + + +Since +__send__+ can be used to call private and protected methods, this will not raise a +NoMethodError+. + NOTE: Defined in +active_support/core_ext/module/delegation.rb+ h4. Redefining Methods -- cgit v1.2.3 From 402a119ddb886cc60b6e6c120aff964a6d4780af Mon Sep 17 00:00:00 2001 From: Daniel Schierbeck Date: Tue, 11 Oct 2011 15:38:17 +0200 Subject: Add back the old `deprecate` method as `deprecate!` --- .../active_support/core_ext/module/delegation.rb | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index ee8adae1cb..b927059c6b 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -156,4 +156,56 @@ class Module end end end + + def delegate!(*methods) + ActiveSupport::Deprecation.warn('Delegating to non-public methods is deprecated.', caller) + + options = methods.pop + unless options.is_a?(Hash) && to = options[:to] + raise ArgumentError, "Delegation needs a target. Supply an options hash with a :to key as the last argument (e.g. delegate :hello, :to => :greeter)." + end + prefix, to, allow_nil = options[:prefix], options[:to], options[:allow_nil] + + if prefix == true && to.to_s =~ /^[^a-z_]/ + raise ArgumentError, "Can only automatically set the delegation prefix when delegating to a method." + end + + method_prefix = + if prefix + "#{prefix == true ? to : prefix}_" + else + '' + end + + file, line = caller.first.split(':', 2) + line = line.to_i + + methods.each do |method| + method = method.to_s + + if allow_nil + module_eval(<<-EOS, file, line - 2) + def #{method_prefix}#{method}(*args, &block) # def customer_name(*args, &block) + if #{to} || #{to}.respond_to?(:#{method}) # if client || client.respond_to?(:name) + #{to}.__send__(:#{method}, *args, &block) # client.__send__(:name, *args, &block) + end # end + end # end + EOS + else + exception = %(raise "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}") + + module_eval(<<-EOS, file, line - 1) + def #{method_prefix}#{method}(*args, &block) # def customer_name(*args, &block) + #{to}.__send__(:#{method}, *args, &block) # client.__send__(:name, *args, &block) + rescue NoMethodError # rescue NoMethodError + if #{to}.nil? # if client.nil? + #{exception} # # add helpful message to the exception + else # else + raise # raise + end # end + end # end + EOS + end + end + end end -- cgit v1.2.3 From 5fb6bd8b22327b00061602b64af8c08886abc581 Mon Sep 17 00:00:00 2001 From: Daniel Schierbeck Date: Thu, 12 Apr 2012 14:55:30 +0200 Subject: Remove Module#delegate! --- .../active_support/core_ext/module/delegation.rb | 52 ---------------------- 1 file changed, 52 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index b927059c6b..ee8adae1cb 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -156,56 +156,4 @@ class Module end end end - - def delegate!(*methods) - ActiveSupport::Deprecation.warn('Delegating to non-public methods is deprecated.', caller) - - options = methods.pop - unless options.is_a?(Hash) && to = options[:to] - raise ArgumentError, "Delegation needs a target. Supply an options hash with a :to key as the last argument (e.g. delegate :hello, :to => :greeter)." - end - prefix, to, allow_nil = options[:prefix], options[:to], options[:allow_nil] - - if prefix == true && to.to_s =~ /^[^a-z_]/ - raise ArgumentError, "Can only automatically set the delegation prefix when delegating to a method." - end - - method_prefix = - if prefix - "#{prefix == true ? to : prefix}_" - else - '' - end - - file, line = caller.first.split(':', 2) - line = line.to_i - - methods.each do |method| - method = method.to_s - - if allow_nil - module_eval(<<-EOS, file, line - 2) - def #{method_prefix}#{method}(*args, &block) # def customer_name(*args, &block) - if #{to} || #{to}.respond_to?(:#{method}) # if client || client.respond_to?(:name) - #{to}.__send__(:#{method}, *args, &block) # client.__send__(:name, *args, &block) - end # end - end # end - EOS - else - exception = %(raise "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}") - - module_eval(<<-EOS, file, line - 1) - def #{method_prefix}#{method}(*args, &block) # def customer_name(*args, &block) - #{to}.__send__(:#{method}, *args, &block) # client.__send__(:name, *args, &block) - rescue NoMethodError # rescue NoMethodError - if #{to}.nil? # if client.nil? - #{exception} # # add helpful message to the exception - else # else - raise # raise - end # end - end # end - EOS - end - end - end end -- cgit v1.2.3 From 1d48e5028493b7b628b9e7df0b74a40b44fc1b67 Mon Sep 17 00:00:00 2001 From: Daniel Schierbeck Date: Thu, 12 Apr 2012 14:59:21 +0200 Subject: Add changelog entry --- activesupport/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 8165b89cde..b6c3e91db8 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,5 +1,7 @@ ## Rails 4.0.0 (unreleased) ## +* Make Module#delegate stop using `send` - can no longer delegate to private methods. *dasch* + * AS::Callbacks: deprecate `:rescuable` option. *Bogdan Gusiev* * Adds Integer#ordinal to get the ordinal suffix string of an integer. *Tim Gildea* -- cgit v1.2.3 From 300868d9cbf52bf268ba81ec289a257f8dc186ae Mon Sep 17 00:00:00 2001 From: Daniel Schierbeck Date: Thu, 12 Apr 2012 15:03:19 +0200 Subject: Remove the addition to the guides --- guides/source/active_support_core_extensions.textile | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/guides/source/active_support_core_extensions.textile b/guides/source/active_support_core_extensions.textile index 181dac72cf..5d0a3f82e8 100644 --- a/guides/source/active_support_core_extensions.textile +++ b/guides/source/active_support_core_extensions.textile @@ -883,26 +883,6 @@ delegate :size, :to => :attachment, :prefix => :avatar In the previous example the macro generates +avatar_size+ rather than +size+. -WARNING: You can only delegate to public methods on the target object. Trying to delegate to a private or protected method will raise a +NoMethodError+ when the delegate is called. - -If you need to delegate to a private or protected method, you will need to implement the delegation method yourself. It's usually rather simple to do using +__send__+: - - -class Wrapper - def initialize - # Target#zizzle is a private method. - @target = Target.new - end - - def zizzle(*args, &block) - # __send__ circumvents the private/protected mechanism. - @target.__send__(:zizzle, *args, &block) - end -end - - -Since +__send__+ can be used to call private and protected methods, this will not raise a +NoMethodError+. - NOTE: Defined in +active_support/core_ext/module/delegation.rb+ h4. Redefining Methods -- cgit v1.2.3