diff options
17 files changed, 489 insertions, 278 deletions
diff --git a/actionmailer/actionmailer.gemspec b/actionmailer/actionmailer.gemspec index 67ec0d1097..c56b6979ef 100644 --- a/actionmailer/actionmailer.gemspec +++ b/actionmailer/actionmailer.gemspec @@ -21,5 +21,5 @@ Gem::Specification.new do |s| s.add_dependency 'actionpack', version - s.add_dependency 'mail', '~> 2.5.3' + s.add_dependency 'mail', '~> 2.5.4' end diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index 599fff81c2..19cfd7dae1 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -8,7 +8,9 @@ module AbstractController include ActiveSupport::Callbacks included do - define_callbacks :process_action, :terminator => "response_body", :skip_after_callbacks_if_terminated => true + define_callbacks :process_action, + terminator: ->(controller,_) { controller.response_body }, + skip_after_callbacks_if_terminated: true end # Override AbstractController::Base's process_action to run the diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 7318c8b7ec..9279d8bcea 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -33,7 +33,7 @@ module ActionController end def halted_callback(event) - info("Filter chain halted as #{event.payload[:filter]} rendered or redirected") + info("Filter chain halted as #{event.payload[:filter].inspect} rendered or redirected") end def send_file(event) diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 3b79161ad3..e74bc2bb3d 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -10,7 +10,7 @@ class ActionController::Base def before_filters filters = _process_action_callbacks.select { |c| c.kind == :before } - filters.map! { |c| c.instance_variable_get(:@raw_filter) } + filters.map! { |c| c.raw_filter } end end diff --git a/activemodel/lib/active_model/callbacks.rb b/activemodel/lib/active_model/callbacks.rb index 94d2181e4d..8b09f8b203 100644 --- a/activemodel/lib/active_model/callbacks.rb +++ b/activemodel/lib/active_model/callbacks.rb @@ -100,7 +100,7 @@ module ActiveModel def define_model_callbacks(*callbacks) options = callbacks.extract_options! options = { - terminator: "result == false", + terminator: ->(_,result) { result == false }, skip_after_callbacks_if_terminated: true, scope: [:kind, :name], only: [:before, :around, :after] diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index 8de36b4f8b..cabb9482f2 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -22,7 +22,10 @@ module ActiveModel included do include ActiveSupport::Callbacks - define_callbacks :validation, terminator: "result == false", skip_after_callbacks_if_terminated: true, scope: [:kind, :name] + define_callbacks :validation, + terminator: ->(_,result) { result == false }, + skip_after_callbacks_if_terminated: true, + scope: [:kind, :name] end module ClassMethods diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb index 3241a03b53..3e84297cc2 100644 --- a/activemodel/test/cases/validations_test.rb +++ b/activemodel/test/cases/validations_test.rb @@ -166,7 +166,7 @@ class ValidationsTest < ActiveModel::TestCase def test_invalid_validator Topic.validate :i_dont_exist - assert_raise(NameError) do + assert_raises(NoMethodError) do t = Topic.new t.valid? end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index a5955ccba4..77634b40bb 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -10,7 +10,9 @@ module ActiveRecord end included do - define_callbacks :commit, :rollback, :terminator => "result == false", :scope => [:kind, :name] + define_callbacks :commit, :rollback, + terminator: ->(_, result) { result == false }, + scope: [:kind, :name] end # = Active Record Transactions diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb index 187cad9599..c8f56e3c73 100644 --- a/activerecord/test/cases/callbacks_test.rb +++ b/activerecord/test/cases/callbacks_test.rb @@ -43,7 +43,7 @@ class CallbackDeveloper < ActiveRecord::Base end class CallbackDeveloperWithFalseValidation < CallbackDeveloper - before_validation proc { |model| model.history << [:before_validation, :returning_false]; return false } + before_validation proc { |model| model.history << [:before_validation, :returning_false]; false } before_validation proc { |model| model.history << [:before_validation, :should_never_get_here] } end diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index a151aa03b5..c4cfc11372 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -1,9 +1,9 @@ -require 'thread_safe' require 'active_support/concern' require 'active_support/descendants_tracker' require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/kernel/reporting' require 'active_support/core_ext/kernel/singleton_class' +require 'thread' module ActiveSupport # Callbacks are code hooks that are run at key points in an object's lifecycle. @@ -76,8 +76,14 @@ module ActiveSupport # save # end def run_callbacks(kind, &block) - runner_name = self.class.__define_callbacks(kind, self) - send(runner_name, &block) + cbs = send("_#{kind}_callbacks") + if cbs.empty? + yield if block_given? + else + runner = cbs.compile + e = Filters::Environment.new(self, false, nil, block) + runner.call(e).value + end end private @@ -88,59 +94,278 @@ module ActiveSupport def halted_callback_hook(filter) end - class Callback #:nodoc:# - @@_callback_sequence = 0 + module Filters + Environment = Struct.new(:target, :halted, :value, :run_block) - class Basic < Callback + class End + def call(env) + block = env.run_block + env.value = !env.halted && (!block || block.call) + env + end end + ENDING = End.new - class Object < Callback - def duplicates?(other) - false + class Before + def self.build(next_callback, user_callback, user_conditions, chain_config, filter) + halted_lambda = chain_config[:terminator] + + if chain_config.key?(:terminator) && user_conditions.any? + halting_and_conditional(next_callback, user_callback, user_conditions, halted_lambda, filter) + elsif chain_config.key? :terminator + halting(next_callback, user_callback, halted_lambda, filter) + elsif user_conditions.any? + conditional(next_callback, user_callback, user_conditions) + else + simple next_callback, user_callback + end + end + + private + + def self.halting_and_conditional(next_callback, user_callback, user_conditions, halted_lambda, filter) + lambda { |env| + target = env.target + value = env.value + halted = env.halted + + if !halted && user_conditions.all? { |c| c.call(target, value) } + result = user_callback.call target, value + env.halted = halted_lambda.call(target, result) + if env.halted + target.send :halted_callback_hook, filter + end + end + next_callback.call env + } + end + + def self.halting(next_callback, user_callback, halted_lambda, filter) + lambda { |env| + target = env.target + value = env.value + halted = env.halted + + if !halted + result = user_callback.call target, value + env.halted = halted_lambda.call(target, result) + if env.halted + target.send :halted_callback_hook, filter + end + end + next_callback.call env + } + end + + def self.conditional(next_callback, user_callback, user_conditions) + lambda { |env| + target = env.target + value = env.value + + if user_conditions.all? { |c| c.call(target, value) } + user_callback.call target, value + end + next_callback.call env + } + end + + def self.simple(next_callback, user_callback) + lambda { |env| + user_callback.call env.target, env.value + next_callback.call env + } end end - def self.build(chain, filter, kind, options, _klass) - klass = case filter - when Array, Symbol, String - Callback::Basic - else - Callback::Object - end - klass.new chain, filter, kind, options, _klass + class After + def self.build(next_callback, user_callback, user_conditions, chain_config) + if chain_config[:skip_after_callbacks_if_terminated] + if chain_config.key?(:terminator) && user_conditions.any? + halting_and_conditional(next_callback, user_callback, user_conditions) + elsif chain_config.key?(:terminator) + halting(next_callback, user_callback) + elsif user_conditions.any? + conditional next_callback, user_callback, user_conditions + else + simple next_callback, user_callback + end + else + if user_conditions.any? + conditional next_callback, user_callback, user_conditions + else + simple next_callback, user_callback + end + end + end + + private + + def self.halting_and_conditional(next_callback, user_callback, user_conditions) + lambda { |env| + env = next_callback.call env + target = env.target + value = env.value + halted = env.halted + + if !halted && user_conditions.all? { |c| c.call(target, value) } + user_callback.call target, value + end + env + } + end + + def self.halting(next_callback, user_callback) + lambda { |env| + env = next_callback.call env + if !env.halted + user_callback.call env.target, env.value + end + env + } + end + + def self.conditional(next_callback, user_callback, user_conditions) + lambda { |env| + env = next_callback.call env + target = env.target + value = env.value + + if user_conditions.all? { |c| c.call(target, value) } + user_callback.call target, value + end + env + } + end + + def self.simple(next_callback, user_callback) + lambda { |env| + env = next_callback.call env + user_callback.call env.target, env.value + env + } + end end - attr_accessor :chain, :kind, :options, :klass, :raw_filter + class Around + def self.build(next_callback, user_callback, user_conditions, chain_config) + if chain_config.key?(:terminator) && user_conditions.any? + halting_and_conditional(next_callback, user_callback, user_conditions) + elsif chain_config.key? :terminator + halting(next_callback, user_callback) + elsif user_conditions.any? + conditional(next_callback, user_callback, user_conditions) + else + simple(next_callback, user_callback) + end + end - def initialize(chain, filter, kind, options, klass) - @chain, @kind, @klass = chain, kind, klass - deprecate_per_key_option(options) - normalize_options!(options) + private + + def self.halting_and_conditional(next_callback, user_callback, user_conditions) + lambda { |env| + target = env.target + value = env.value + halted = env.halted + + if !halted && user_conditions.all? { |c| c.call(target, value) } + user_callback.call(target, value) { + env = next_callback.call env + env.value + } + env + else + next_callback.call env + end + } + end + + def self.halting(next_callback, user_callback) + lambda { |env| + target = env.target + value = env.value + + if !env.halted + user_callback.call(target, value) { + env = next_callback.call env + env.value + } + env + else + next_callback.call env + end + } + end + + def self.conditional(next_callback, user_callback, user_conditions) + lambda { |env| + target = env.target + value = env.value + + if user_conditions.all? { |c| c.call(target, value) } + user_callback.call(target, value) { + env = next_callback.call env + env.value + } + env + else + next_callback.call env + end + } + end + + def self.simple(next_callback, user_callback) + lambda { |env| + user_callback.call(env.target, env.value) { + env = next_callback.call env + env.value + } + env + } + end + end + end - @raw_filter, @options = filter, options - @key = compute_identifier filter - @source = _compile_source(filter) - recompile_options! + class Callback #:nodoc:# + def self.build(chain, filter, kind, options) + new chain.name, filter, kind, options, chain.config end - def filter - @key + attr_accessor :kind, :options, :name + attr_reader :chain_config + + def initialize(name, filter, kind, options, chain_config) + @chain_config = chain_config + @name = name + @kind = kind + @filter = filter + @options = options + @key = compute_identifier filter + + deprecate_per_key_option(options) + normalize_options!(options) end + def filter; @key; end + def raw_filter; @filter; end + def deprecate_per_key_option(options) if options[:per_key] raise NotImplementedError, ":per_key option is no longer supported. Use generic :if and :unless options instead." end end - def clone(chain, klass) - obj = super() - obj.chain = chain - obj.klass = klass - obj.options = @options.dup - obj.options[:if] = @options[:if].dup - obj.options[:unless] = @options[:unless].dup - obj + def merge(chain, new_options) + _options = { + :if => @options[:if].dup, + :unless => @options[:unless].dup + } + + deprecate_per_key_option new_options + + _options[:if].concat Array(new_options.fetch(:unless, [])) + _options[:unless].concat Array(new_options.fetch(:if, [])) + + self.class.build chain, @filter, @kind, _options end def normalize_options!(options) @@ -148,144 +373,42 @@ module ActiveSupport options[:unless] = Array(options[:unless]) end - def name - chain.name - end - - def next_id - @@_callback_sequence += 1 - end - def matches?(_kind, _filter) @kind == _kind && filter == _filter end def duplicates?(other) - return false unless self.class == other.class - - matches?(other.kind, other.filter) - end - - def _update_filter(filter_options, new_options) - filter_options[:if].concat(Array(new_options[:unless])) if new_options.key?(:unless) - filter_options[:unless].concat(Array(new_options[:if])) if new_options.key?(:if) - end - - def recompile!(_options) - deprecate_per_key_option(_options) - _update_filter(self.options, _options) - - recompile_options! + case @filter + when Symbol, String + matches?(other.kind, other.filter) + else + false + end end # Wraps code with filter - def apply(code) - case @kind + def apply(next_callback) + user_conditions = conditions_lambdas + user_callback = make_lambda @filter + + case kind when :before - <<-RUBY_EVAL - if !halted && #{@compiled_options} - # 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. - result = result = #{@source} - halted = (#{chain.config[:terminator]}) - if halted - halted_callback_hook(#{@raw_filter.inspect.inspect}) - end - end - #{code} - RUBY_EVAL + Filters::Before.build(next_callback, user_callback, user_conditions, chain_config, @filter) when :after - <<-RUBY_EVAL - #{code} - if #{!chain.config[:skip_after_callbacks_if_terminated] || "!halted"} && #{@compiled_options} - #{@source} - end - RUBY_EVAL + Filters::After.build(next_callback, user_callback, user_conditions, chain_config) when :around - name = define_conditional_callback - <<-RUBY_EVAL - #{name}(halted) do - #{code} - value - end - RUBY_EVAL + Filters::Around.build(next_callback, user_callback, user_conditions, chain_config) end end private - def compute_identifier(filter) - case filter - when String, ::Proc - filter.object_id - else - filter - end - end - - # Compile around filters with conditions into proxy methods - # that contain the conditions. - # - # For `set_callback :save, :around, :filter_name, if: :condition`: - # - # def _conditional_callback_save_17 - # if condition - # filter_name do - # yield self - # end - # else - # yield self - # end - # end - 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 - #{@source} do - yield self - end - else - yield self - end - end - RUBY_EVAL - name - end - - # Options support the same options as filters themselves (and support - # symbols, string, procs, and objects), so compile a conditional - # expression based on the options. - def recompile_options! - conditions = ["true"] - - unless options[:if].empty? - conditions << Array(_compile_source(options[:if])) - end - - unless options[:unless].empty? - conditions << Array(_compile_source(options[:unless])).map {|f| "!#{f}"} - end - - @compiled_options = conditions.flatten.join(" && ") - end - - def _method_name_for_object_filter(kind, filter, append_next_id = true) - class_name = filter.kind_of?(Class) ? filter.to_s : filter.class.to_s - class_name.gsub!(/<|>|#/, '') - class_name.gsub!(/\/|:/, "_") - - method_name = "_callback_#{kind}_#{class_name}" - method_name << "_#{next_id}" if append_next_id - method_name + def invert_lambda(l) + lambda { |*args, &blk| !l.call(*args, &blk) } end # Filters support: # - # Arrays:: Used in conditions. This is used to specify - # multiple conditions. Used internally to - # merge conditions from skip_* filters. # Symbols:: A method to call. # Strings:: Some content to evaluate. # Procs:: A proc to call with the object. @@ -294,43 +417,64 @@ module ActiveSupport # All of these objects are compiled into methods and handled # the same after this point: # - # Arrays:: Merged together into a single filter. # Symbols:: Already methods. # Strings:: class_eval'ed into methods. # Procs:: define_method'ed into methods. # Objects:: # a method is created that calls the before_foo method # on the object. - def _compile_source(filter) + def make_lambda(filter) case filter - when Array - filter.map {|f| _compile_source(f)} when Symbol - filter + lambda { |target, _, &blk| target.send filter, &blk } when String - "(#{filter})" + l = eval "lambda { |value| #{filter} }" + lambda { |target, value| target.instance_exec(value, &l) } when ::Proc - method_name = "_callback_#{@kind}_#{next_id}" - @klass.send(:define_method, method_name, &filter) - return method_name if filter.arity <= 0 + if filter.arity > 1 + return lambda { |target, _, &block| + raise ArgumentError unless block + target.instance_exec(target, block, &filter) + } + end - method_name << (filter.arity == 1 ? "(self)" : "(self, ::Proc.new)") + if filter.arity <= 0 + lambda { |target, _| target.instance_exec(&filter) } + else + lambda { |target, _| target.instance_exec(target, &filter) } + end else - method_name = _method_name_for_object_filter(kind, filter) - @klass.send(:define_method, "#{method_name}_object") { filter } - - _normalize_legacy_filter(kind, filter) - scopes = Array(chain.config[:scope]) + scopes = Array(chain_config[:scope]) method_to_call = scopes.map{ |s| public_send(s) }.join("_") - @klass.class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 - def #{method_name}(&blk) - #{method_name}_object.send(:#{method_to_call}, self, &blk) - end - RUBY_EVAL + lambda { |target, _, &blk| + filter.public_send method_to_call, target, &blk + } + end + end + + def compute_identifier(filter) + case filter + when String, ::Proc + filter.object_id + else + filter + end + end - method_name + def conditions_lambdas + conditions = [] + + unless options[:if].empty? + lambdas = Array(options[:if]).map { |c| make_lambda c } + conditions.concat lambdas + end + + unless options[:unless].empty? + lambdas = Array(options[:unless]).map { |c| make_lambda c } + conditions.concat lambdas.map { |l| invert_lambda l } end + conditions end def _normalize_legacy_filter(kind, filter) @@ -354,27 +498,53 @@ module ActiveSupport end # An Array with a compile method. - class CallbackChain < Array #:nodoc:# + class CallbackChain #:nodoc:# + include Enumerable + attr_reader :name, :config def initialize(name, config) @name = name @config = { - :terminator => "false", :scope => [ :kind ] }.merge!(config) + @chain = [] + @callbacks = nil + @mutex = Mutex.new + end + + def each(&block); @chain.each(&block); end + def index(o); @chain.index(o); end + def empty?; @chain.empty?; end + + def insert(index, o) + @callbacks = nil + @chain.insert(index, o) + end + + def delete(o) + @callbacks = nil + @chain.delete(o) + end + + def clear + @callbacks = nil + @chain.clear + self + end + + def initialize_copy(other) + @callbacks = nil + @chain = other.chain.dup + @mutex = Mutex.new end def compile - method = ["value = nil", "halted = false"] - callbacks = "value = !halted && (!block_given? || yield)" - reverse_each do |callback| - callbacks = callback.apply(callbacks) + @callbacks || @mutex.synchronize do + @callbacks ||= @chain.reverse.inject(Filters::ENDING) do |chain, callback| + callback.apply chain + end end - method << callbacks - - method << "value" - method.join("\n") end def append(*callbacks) @@ -385,69 +555,45 @@ module ActiveSupport callbacks.each { |c| prepend_one(c) } end + protected + def chain; @chain; end + private def append_one(callback) + @callbacks = nil remove_duplicates(callback) - push(callback) + @chain.push(callback) end def prepend_one(callback) + @callbacks = nil remove_duplicates(callback) - unshift(callback) + @chain.unshift(callback) end def remove_duplicates(callback) - delete_if { |c| callback.duplicates?(c) } + @callbacks = nil + @chain.delete_if { |c| callback.duplicates?(c) } end end module ClassMethods - # This method defines callback chain method for the given kind - # if it was not yet defined. - # This generated method plays caching role. - def __define_callbacks(kind, object) #:nodoc: - name = __callback_runner_name(kind) - unless object.respond_to?(name, true) - str = object.send("_#{kind}_callbacks").compile - class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 - def #{name}() #{str} end - protected :#{name} - RUBY_EVAL - end - name - end - - def __reset_runner(symbol) - name = __callback_runner_name(symbol) - undef_method(name) if method_defined?(name) - end - - def __callback_runner_name_cache - @__callback_runner_name_cache ||= ThreadSafe::Cache.new {|cache, kind| cache[kind] = __generate_callback_runner_name(kind) } - end - - def __generate_callback_runner_name(kind) - "_run__#{self.name.hash.abs}__#{kind}__callbacks" - end - - def __callback_runner_name(kind) - __callback_runner_name_cache[kind] - end - - # This is used internally to append, prepend and skip callbacks to the - # CallbackChain. - def __update_callbacks(name, filters = [], block = nil) #:nodoc: + def normalize_callback_params(name, filters, block) # :nodoc: type = CALLBACK_FILTER_TYPES.include?(filters.first) ? filters.shift : :before options = filters.last.is_a?(Hash) ? filters.pop : {} filters.unshift(block) if block + [type, filters, options.dup] + end + # This is used internally to append, prepend and skip callbacks to the + # CallbackChain. + def __update_callbacks(name) #:nodoc: ([self] + ActiveSupport::DescendantsTracker.descendants(self)).reverse.each do |target| - chain = target.send("_#{name}_callbacks") - yield target, chain.dup, type, filters, options - target.__reset_runner(name) + chain = target.get_callbacks name + yield target, chain.dup end end @@ -487,16 +633,15 @@ module ActiveSupport # * <tt>:prepend</tt> - If +true+, the callback will be prepended to the # existing chain rather than appended. def set_callback(name, *filter_list, &block) - mapped = nil - - __update_callbacks(name, filter_list, block) do |target, chain, type, filters, options| - mapped ||= filters.map do |filter| - Callback.build(chain, filter, type, options.dup, self) - end + type, filters, options = normalize_callback_params(name, filter_list, block) + self_chain = get_callbacks name + mapped = filters.map do |filter| + Callback.build(self_chain, filter, type, options) + end + __update_callbacks(name) do |target, chain| options[:prepend] ? chain.prepend(*mapped) : chain.append(*mapped) - - target.send("_#{name}_callbacks=", chain) + target.set_callbacks name, chain end end @@ -508,36 +653,34 @@ module ActiveSupport # skip_callback :validate, :before, :check_membership, if: -> { self.age > 18 } # end def skip_callback(name, *filter_list, &block) - __update_callbacks(name, filter_list, block) do |target, chain, type, filters, options| + type, filters, options = normalize_callback_params(name, filter_list, block) + + __update_callbacks(name) do |target, chain| filters.each do |filter| filter = chain.find {|c| c.matches?(type, filter) } if filter && options.any? - new_filter = filter.clone(chain, self) + new_filter = filter.merge(chain, options) chain.insert(chain.index(filter), new_filter) - new_filter.recompile!(options) end chain.delete(filter) end - target.send("_#{name}_callbacks=", chain) + target.set_callbacks name, chain end end # Remove all set callbacks for the given event. def reset_callbacks(symbol) - callbacks = send("_#{symbol}_callbacks") + callbacks = get_callbacks symbol ActiveSupport::DescendantsTracker.descendants(self).each do |target| - chain = target.send("_#{symbol}_callbacks").dup + chain = target.get_callbacks(symbol).dup callbacks.each { |c| chain.delete(c) } - target.send("_#{symbol}_callbacks=", chain) - target.__reset_runner(symbol) + target.set_callbacks symbol, chain end - self.send("_#{symbol}_callbacks=", callbacks.dup.clear) - - __reset_runner(symbol) + self.set_callbacks symbol, callbacks.dup.clear end # Define sets of events in the object lifecycle that support callbacks. @@ -609,11 +752,28 @@ module ActiveSupport # would call <tt>Audit#save</tt>. def define_callbacks(*callbacks) config = callbacks.last.is_a?(Hash) ? callbacks.pop : {} + if config.key?(:terminator) && String === config[:terminator] + ActiveSupport::Deprecation.warn "String based terminators are deprecated, please use a lambda" + value = config[:terminator] + l = class_eval "lambda { |result| #{value} }", __FILE__, __LINE__ + config[:terminator] = lambda { |target, result| target.instance_exec(result, &l) } + end + callbacks.each do |callback| class_attribute "_#{callback}_callbacks" - send("_#{callback}_callbacks=", CallbackChain.new(callback, config)) + set_callbacks callback, CallbackChain.new(callback, config) end end + + protected + + def get_callbacks(name) + send "_#{name}_callbacks" + end + + def set_callbacks(name, callbacks) + send "_#{name}_callbacks=", callbacks + end end end end diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb index dc033ed11b..7c21abea72 100644 --- a/activesupport/lib/active_support/core_ext/string/output_safety.rb +++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb @@ -171,17 +171,15 @@ module ActiveSupport #:nodoc: end UNSAFE_STRING_METHODS.each do |unsafe_method| - if 'String'.respond_to?(unsafe_method) - class_eval <<-EOT, __FILE__, __LINE__ + 1 - def #{unsafe_method}(*args, &block) # def capitalize(*args, &block) - to_str.#{unsafe_method}(*args, &block) # to_str.capitalize(*args, &block) - end # end - - def #{unsafe_method}!(*args) # def capitalize!(*args) - @html_safe = false # @html_safe = false - super # super - end # end - EOT + if String.new.respond_to?(unsafe_method) + define_method(unsafe_method) do |*args, &block| + to_str.send(unsafe_method, *args, &block) + end + + define_method("#{unsafe_method}!") do |*args| + @html_safe = false + super(*args) + end end end end diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 1b20592e4c..9a9ed02bd9 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -229,7 +229,11 @@ module ActiveSupport # Convert to a regular hash with string keys. def to_hash - Hash.new(default).merge!(self) + _new_hash= {} + each do |key, value| + _new_hash[convert_key(key)] = convert_value(value,true) + end + Hash.new(default).merge!(_new_hash) end protected @@ -237,9 +241,9 @@ module ActiveSupport key.kind_of?(Symbol) ? key.to_s : key end - def convert_value(value) + def convert_value(value, _convert_for_to_hash = false) if value.is_a? Hash - value.nested_under_indifferent_access + _convert_for_to_hash ? value.to_hash : value.nested_under_indifferent_access elsif value.is_a?(Array) value = value.dup if value.frozen? value.map! { |e| convert_value(e) } diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 091fe90bcc..5fce1eeafc 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -525,7 +525,7 @@ module CallbacksTest class CallbackTerminator include ActiveSupport::Callbacks - define_callbacks :save, :terminator => "result == :halt" + define_callbacks :save, :terminator => ->(_,result) { result == :halt } set_callback :save, :before, :first set_callback :save, :before, :second @@ -718,7 +718,7 @@ module CallbacksTest def test_termination_invokes_hook terminator = CallbackTerminator.new terminator.save - assert_equal ":second", terminator.halted + assert_equal :second, terminator.halted end def test_block_never_called_if_terminated @@ -802,6 +802,46 @@ module CallbacksTest end end + class CallbackProcTest < ActiveSupport::TestCase + def build_class(callback) + Class.new { + include ActiveSupport::Callbacks + define_callbacks :foo + set_callback :foo, :before, callback + def run; run_callbacks :foo; end + } + end + + def test_proc_arity_0 + calls = [] + klass = build_class(->() { calls << :foo }) + klass.new.run + assert_equal [:foo], calls + end + + def test_proc_arity_1 + calls = [] + klass = build_class(->(o) { calls << o }) + instance = klass.new + instance.run + assert_equal [instance], calls + end + + def test_proc_arity_2 + assert_raises(ArgumentError) do + klass = build_class(->(x,y) { }) + klass.new.run + end + end + + def test_proc_negative_called_with_empty_list + calls = [] + klass = build_class(->(*args) { calls << args }) + klass.new.run + assert_equal [[]], calls + end + end + class ConditionalTests < ActiveSupport::TestCase def build_class(callback) Class.new { @@ -824,8 +864,9 @@ module CallbacksTest include ActiveSupport::Callbacks define_callbacks :foo, :scope => [:name] set_callback :foo, :before, :foo, :if => callback - def foo; end def run; run_callbacks :foo; end + private + def foo; end } object = klass.new object.run diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 30d95b75bc..b385e806bc 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -490,6 +490,10 @@ class HashExtTest < ActiveSupport::TestCase roundtrip = mixed_with_default.with_indifferent_access.to_hash assert_equal @strings, roundtrip assert_equal '1234', roundtrip.default + new_to_hash = @nested_mixed.with_indifferent_access.to_hash + assert_not new_to_hash.instance_of?(HashWithIndifferentAccess) + assert_not new_to_hash["a"].instance_of?(HashWithIndifferentAccess) + assert_not new_to_hash["a"]["b"].instance_of?(HashWithIndifferentAccess) end def test_lookup_returns_the_same_object_that_is_stored_in_hash_indifferent_access diff --git a/guides/code/getting_started/app/assets/javascripts/application.js b/guides/code/getting_started/app/assets/javascripts/application.js index 9e83eb5e7e..5a4fbaa370 100644 --- a/guides/code/getting_started/app/assets/javascripts/application.js +++ b/guides/code/getting_started/app/assets/javascripts/application.js @@ -7,8 +7,7 @@ // It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the // compiled file. // -// WARNING: THE FIRST BLANK LINE MARKS THE END OF WHAT'S TO BE PROCESSED, ANY BLANK LINE SHOULD -// GO AFTER THE REQUIRES BELOW. +// stub path allows dependency to be excluded from the asset bundle. // //= require jquery //= require jquery_ujs diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt index 7342bffd9d..6899dfe9c9 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt @@ -7,8 +7,7 @@ // It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the // compiled file. // -// WARNING: THE FIRST BLANK LINE MARKS THE END OF WHAT'S TO BE PROCESSED, ANY BLANK LINE SHOULD -// GO AFTER THE REQUIRES BELOW. +// stub path allows dependency to be excluded from the asset bundle. // <% unless options[:skip_javascript] -%> //= require <%= options[:javascript] %> diff --git a/railties/lib/rails/generators/rails/plugin_new/templates/rails/javascripts.js b/railties/lib/rails/generators/rails/plugin_new/templates/rails/javascripts.js index 084d5d1c49..f84de00fd3 100644 --- a/railties/lib/rails/generators/rails/plugin_new/templates/rails/javascripts.js +++ b/railties/lib/rails/generators/rails/plugin_new/templates/rails/javascripts.js @@ -7,7 +7,6 @@ // It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the // compiled file. // -// WARNING: THE FIRST BLANK LINE MARKS THE END OF WHAT'S TO BE PROCESSED, ANY BLANK LINE SHOULD -// GO AFTER THE REQUIRES BELOW. +// stub path allows dependency to be excluded from the asset bundle. // //= require_tree . |