Permalink
Browse files

add strict argument checking to ActiveRecord callbacks

This ends up adding it to all save-related callbacks defined in `ActiveRecord::DefineCallbacks`, including e.g. `after_create`. Which should be fine: they didn't support `:on` in the first place.
  • Loading branch information...
seanlinsley committed Oct 18, 2017
1 parent 9732195 commit dfb0e4b3dc94860e6d484385d31fd399d33dac92
Showing with 32 additions and 7 deletions.
  1. +9 −7 activemodel/lib/active_model/callbacks.rb
  2. +23 −0 activerecord/test/cases/callbacks_test.rb
@@ -127,26 +127,28 @@ def define_model_callbacks(*callbacks)
private

def _define_before_model_callback(klass, callback)
klass.define_singleton_method("before_#{callback}") do |*args, &block|
set_callback(:"#{callback}", :before, *args, &block)
klass.define_singleton_method("before_#{callback}") do |*args, **options, &block|
options.assert_valid_keys :if, :unless, :prepend
set_callback(:"#{callback}", :before, *args, **options, &block)
end
end

def _define_around_model_callback(klass, callback)
klass.define_singleton_method("around_#{callback}") do |*args, &block|
set_callback(:"#{callback}", :around, *args, &block)
klass.define_singleton_method("around_#{callback}") do |*args, **options, &block|
options.assert_valid_keys :if, :unless, :prepend
set_callback(:"#{callback}", :around, *args, **options, &block)
end
end

def _define_after_model_callback(klass, callback)
klass.define_singleton_method("after_#{callback}") do |*args, &block|
options = args.extract_options!
klass.define_singleton_method("after_#{callback}") do |*args, **options, &block|
options.assert_valid_keys :if, :unless, :prepend
options[:prepend] = true
conditional = ActiveSupport::Callbacks::Conditionals::Value.new { |v|
v != false
}
options[:if] = Array(options[:if]) << conditional
set_callback(:"#{callback}", :after, *(args << options), &block)
set_callback(:"#{callback}", :after, *args, **options, &block)
end
end
end
@@ -476,4 +476,27 @@ def test_inheritance_of_callbacks
child.save
assert child.after_save_called
end

def test_on_isnt_allowed
exception = assert_raises ArgumentError do
Class.new(ActiveRecord::Base) do
before_save(on: :create) {}
end
end
assert_equal "Unknown key: :on. Valid keys are: :if, :unless, :prepend", exception.message

exception = assert_raises ArgumentError do
Class.new(ActiveRecord::Base) do
around_save(on: :create) {}
end
end
assert_equal "Unknown key: :on. Valid keys are: :if, :unless, :prepend", exception.message

exception = assert_raises ArgumentError do
Class.new(ActiveRecord::Base) do
after_save(on: :create) {}
end
end
assert_equal "Unknown key: :on. Valid keys are: :if, :unless, :prepend", exception.message
end
end

0 comments on commit dfb0e4b

Please sign in to comment.