Permalink
Browse files

Merge pull request #5100 from paukul/validate_on_condition_on_transac…

…tion_callbacks

Validate :on option on after_commit and after_rollback callbacks
  • Loading branch information...
2 parents 84d38f6 + 5a3dc80 commit e72790c4e7adf5ede082f8c71304c5d9972cf7bc @rafaelfranca rafaelfranca committed Dec 26, 2012
@@ -22,6 +22,11 @@
*Rafael Mendonça França*
+* after_commit and after_rollback now validate the :on option and raise an ArgumentError if
+ it is not one of :create, :destroy or :update
+
+ *Pascal Friederich*
+
* Keep index names when using `alter_table` with sqlite3.
Fix #3489.
@@ -4,6 +4,7 @@ module ActiveRecord
# See ActiveRecord::Transactions::ClassMethods for documentation.
module Transactions
extend ActiveSupport::Concern
+ ACTIONS = [:create, :destroy, :update]
class TransactionError < ActiveRecordError # :nodoc:
end
@@ -224,24 +225,33 @@ def transaction(options = {}, &block)
# Note that transactional fixtures do not play well with this feature. Please
# use the +test_after_commit+ gem to have these hooks fired in tests.
def after_commit(*args, &block)
- options = args.last
- if options.is_a?(Hash) && options[:on]
- options[:if] = Array(options[:if])
- options[:if] << "transaction_include_action?(:#{options[:on]})"
- end
+ set_options_for_callbacks!(args)
set_callback(:commit, :after, *args, &block)
end
# This callback is called after a create, update, or destroy are rolled back.
#
# Please check the documentation of +after_commit+ for options.
def after_rollback(*args, &block)
+ set_options_for_callbacks!(args)
+ set_callback(:rollback, :after, *args, &block)
+ end
+
+ private
+
+ def set_options_for_callbacks!(args)
options = args.last
if options.is_a?(Hash) && options[:on]
+ assert_valid_transaction_action(options[:on])
options[:if] = Array(options[:if])
options[:if] << "transaction_include_action?(:#{options[:on]})"
end
- set_callback(:rollback, :after, *args, &block)
+ end
+
+ def assert_valid_transaction_action(action)
+ unless ACTIONS.include?(action.to_sym)
+ raise ArgumentError, ":on conditions for after_commit and after_rollback callbacks have to be one of #{ACTIONS.join(",")}"
+ end
end
end
@@ -244,6 +244,14 @@ def @first.last_after_transaction_error; @last_transaction_error; end
assert_equal :rollback, @first.last_after_transaction_error
assert_equal [:after_rollback], @second.history
end
+
+ def test_after_rollback_callbacks_should_validate_on_condition
+ assert_raise(ArgumentError) { Topic.send(:after_rollback, :on => :save) }
+ end
+
+ def test_after_commit_callbacks_should_validate_on_condition
+ assert_raise(ArgumentError) { Topic.send(:after_commit, :on => :save) }
+ end
end

0 comments on commit e72790c

Please sign in to comment.