From 533dd8a681a3ea7d12acdfc6a2d6a95891282b49 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 14 Apr 2019 17:51:27 +0900 Subject: [PATCH] Don't expose `add_to_transaction` `add_to_transaction` was added at da840d1, but it should not be called by except internal, since `remember_transaction_record_state` should be called only once before saving. And also, currently `add_to_transaction` doesn't always add the record to transaction since da8de91, that is the reason hard to use that even in internal. Even if `add_to_transaction` ensure to add the record to transaction, that is an internal concern, people don't need to explicitly call `add_to_transaction`. --- activerecord/lib/active_record/touch_later.rb | 2 +- .../lib/active_record/transactions.rb | 20 +++++++------------ .../test/cases/transaction_callbacks_test.rb | 6 +++--- activerecord/test/cases/transactions_test.rb | 2 +- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/activerecord/lib/active_record/touch_later.rb b/activerecord/lib/active_record/touch_later.rb index 5dc88fb26ce98..980e42664bac2 100644 --- a/activerecord/lib/active_record/touch_later.rb +++ b/activerecord/lib/active_record/touch_later.rb @@ -22,7 +22,7 @@ def touch_later(*names) # :nodoc: @_touch_time = current_time_from_proper_timezone surreptitiously_touch @_defer_touch_attrs - self.class.connection.add_transaction_record self + add_to_transaction # touch the parents as we are not calling the after_save callbacks self.class.reflect_on_all_associations(:belongs_to).each do |r| diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 333f1a5435b54..00a62a70d63ee 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -355,18 +355,6 @@ def rolledback!(force_restore_state: false, should_run_callbacks: true) #:nodoc: clear_transaction_record_state end - # Add the record to the current transaction so that the #after_rollback and #after_commit callbacks - # can be called. - def add_to_transaction - if has_transactional_callbacks? - self.class.connection.add_transaction_record(self) - else - sync_with_transaction_state - set_transaction_state(self.class.connection.transaction_state) - end - remember_transaction_record_state - end - # Executes +method+ within a transaction and captures its return value as a # status flag. If the status is true the transaction is committed, otherwise # a ROLLBACK is issued. In any case the status flag is returned. @@ -387,7 +375,7 @@ def with_transaction_returning_status ensure if has_transactional_callbacks? && (@_new_record_before_last_commit && !new_record? || _trigger_update_callback || _trigger_destroy_callback) - self.class.connection.add_transaction_record(self) + add_to_transaction end end status @@ -459,6 +447,12 @@ def transaction_include_any_action?(actions) end end + # Add the record to the current transaction so that the #after_rollback and #after_commit + # callbacks can be called. + def add_to_transaction + self.class.connection.add_transaction_record(self) + end + def set_transaction_state(state) @transaction_state = state end diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index cd73f6082e7aa..53fe31e087182 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -624,7 +624,7 @@ def test_commit_run_transactions_callbacks_with_explicit_enrollment @topic.content = "foo" @topic.save! end - @topic.class.connection.add_transaction_record(@topic) + @topic.send(:add_to_transaction) end assert_equal [:before_commit, :after_commit], @topic.history end @@ -634,7 +634,7 @@ def test_commit_run_transactions_callbacks_with_nested_transactions @topic.transaction(requires_new: true) do @topic.content = "foo" @topic.save! - @topic.class.connection.add_transaction_record(@topic) + @topic.send(:add_to_transaction) end end assert_equal [:before_commit, :after_commit], @topic.history @@ -655,7 +655,7 @@ def test_rollback_run_transactions_callbacks_with_explicit_enrollment @topic.content = "foo" @topic.save! end - @topic.class.connection.add_transaction_record(@topic) + @topic.send(:add_to_transaction) raise ActiveRecord::Rollback end assert_equal [:rollback], @topic.history diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index d5a4f123766aa..7bad3de3434f4 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -65,7 +65,7 @@ def transaction_with_return def test_add_to_null_transaction topic = Topic.new - topic.add_to_transaction + topic.send(:add_to_transaction) end def test_successful_with_return