Skip to content

Commit

Permalink
Don't expose add_to_transaction
Browse files Browse the repository at this point in the history
`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`.
  • Loading branch information
kamipo committed Apr 14, 2019
1 parent 3e66ba9 commit 533dd8a
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 18 deletions.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/touch_later.rb
Expand Up @@ -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|
Expand Down
20 changes: 7 additions & 13 deletions activerecord/lib/active_record/transactions.rb
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions activerecord/test/cases/transaction_callbacks_test.rb
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/transactions_test.rb
Expand Up @@ -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
Expand Down

0 comments on commit 533dd8a

Please sign in to comment.