Permalink
Browse files

conditions in callbacks return consistent lambdas

  • Loading branch information...
tenderlove committed May 8, 2013
1 parent 1f11dbf commit 6189e2714b730d4ae0da11884b8082cbee0fb44c
Showing with 43 additions and 9 deletions.
  1. +43 −9 activesupport/lib/active_support/callbacks.rb
@@ -183,7 +183,7 @@ def apply(code)
case @kind
when :before
<<-RUBY_EVAL
- if !halted && #{@compiled_options}
+ if !halted && #{@compiled_options}(value)
# This double assignment is to prevent warnings in 1.9.3 as
# the `result` variable is not always used except if the
# terminator code refers to it.
@@ -198,14 +198,14 @@ def apply(code)
when :after
<<-RUBY_EVAL
#{code}
- if #{!chain.config[:skip_after_callbacks_if_terminated] || "!halted"} && #{@compiled_options}
+ if #{!chain.config[:skip_after_callbacks_if_terminated] || "!halted"} && #{@compiled_options}(value)
#{@source}
end
RUBY_EVAL
when :around
name = define_conditional_callback
<<-RUBY_EVAL
- #{name}(halted) do
+ #{name}(halted, value) do
#{code}
value
end
@@ -241,8 +241,8 @@ def compute_identifier(filter)
def define_conditional_callback
name = "_conditional_callback_#{@kind}_#{next_id}"
@klass.class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
- def #{name}(halted)
- if #{@compiled_options} && !halted
+ def #{name}(halted, value)
+ if #{@compiled_options}(value) && !halted
#{@source} do
yield self
end
@@ -258,17 +258,24 @@ def #{name}(halted)
# symbols, string, procs, and objects), so compile a conditional
# expression based on the options.
def recompile_options!
- conditions = ["true"]
+ conditions = []
unless options[:if].empty?
- conditions << Array(_compile_source(options[:if]))
+ conditions.concat Array(_compile_options(options[:if]))
end
unless options[:unless].empty?
- conditions << Array(_compile_source(options[:unless])).map {|f| "!#{f}"}
+ conditions.concat Array(_compile_options(options[:unless])).map { |f|
+ lambda { |*args,&blk| !f.call(*args, &blk) }
+ }
end
- @compiled_options = conditions.flatten.join(" && ")
+ method_name = "_callback_#{@kind}_#{next_id}"
+ @klass.send(:define_method, method_name) do |*args,&block|
+ conditions.all? { |c| c.call(self, *args, &block) }
+ end
+
+ @compiled_options = method_name
end
def _method_name_for_object_filter(kind, filter, append_next_id = true)
@@ -333,6 +340,33 @@ def #{method_name}(&blk)
end
end
+ def _compile_options(filter)
+ case filter
+ when Array
+ filter.map {|f| _compile_options(f)}
+ when Symbol
+ lambda { |target, value| target.send filter }
+ when String
+ l = eval "lambda { |value| #{filter} }"

This comment has been minimized.

Show comment
Hide comment
@mbj

mbj Aug 18, 2014

In my strong opinion this is an antifeature. Very easy to misuse, regardless how safe it looks in todays call sides. Some future refactoring can expose it somewhere unintended. And whoever, for what (possibly braindead) reasons builds callbacks dynamically on user input can fail today.

Ruby is strong enough without eval.

http://postmodern.github.io/2013/03/07/its-simple-we-kill-eval.html

@mbj

mbj Aug 18, 2014

In my strong opinion this is an antifeature. Very easy to misuse, regardless how safe it looks in todays call sides. Some future refactoring can expose it somewhere unintended. And whoever, for what (possibly braindead) reasons builds callbacks dynamically on user input can fail today.

Ruby is strong enough without eval.

http://postmodern.github.io/2013/03/07/its-simple-we-kill-eval.html

+ lambda { |target,value| target.instance_exec(value, &l) }
+ when ::Proc
+ method_name = "_callback_#{@kind}_#{next_id}"
+ @klass.send(:define_method, method_name, &filter)
+
+ if filter.arity <= 0
+ return lambda { |target, _| target.instance_exec(&filter) }
+ end
+
+ if filter.arity == 1
+ lambda { |target, _| target.send method_name, target }
+ else
+ lambda { |target, _,&blk| target.send method_name, target, &blk }
+ end
+ else
+ raise
+ end
+ end
+
def _normalize_legacy_filter(kind, filter)
if !filter.respond_to?(kind) && filter.respond_to?(:filter)
message = "Filter object with #filter method is deprecated. Define method corresponding " \

0 comments on commit 6189e27

Please sign in to comment.