Skip to content

Commit 608ebcc

Browse files
committed
Deprecate and replace set_state method
`set_state` was directly setting the transaction state instance variable. It's better to set the state via specific methods (`rollback!` and `commit!` respectively. While undocumented and untested, it's possible someone is using `set_state` in their app or gem so I've added a deprecation notice to it. No where in the app do we use `nullify!` but I wanted to keep existing behavior while replacing the method with a better pattern.
1 parent c197418 commit 608ebcc

File tree

3 files changed

+73
-6
lines changed

3 files changed

+73
-6
lines changed

activerecord/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
* Deprecate `set_state` method in `TransactionState`
2+
3+
Deprecated the `set_state` method in favor of setting the state via specific methods. If you need to mark the
4+
state of the transaction you can now use `rollback!`, `commit!` or `nullify!` instead of
5+
`set_state(:rolledback)`, `set_state(:committed)`, or `set_state(nil)`.
6+
7+
*Eileen M. Uchitelle*, *Aaron Patterson*
8+
19
* Deprecate delegating to `arel` in `Relation`.
210

311
*Ryuta Kamizono*

activerecord/lib/active_record/connection_adapters/abstract/transaction.rb

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
module ActiveRecord
22
module ConnectionAdapters
33
class TransactionState
4-
VALID_STATES = Set.new([:committed, :rolledback, nil])
5-
64
def initialize(state = nil)
75
@state = state
86
end
@@ -24,10 +22,33 @@ def completed?
2422
end
2523

2624
def set_state(state)
27-
unless VALID_STATES.include?(state)
25+
ActiveSupport::Deprecation.warn(<<-MSG.squish)
26+
The set_state method is deprecated and will be removed in
27+
Rails 5.2. Please use rollback! or commit! to set transaction
28+
state directly.
29+
MSG
30+
case state
31+
when :rolledback
32+
rollback!
33+
when :committed
34+
commit!
35+
when nil
36+
nullify!
37+
else
2838
raise ArgumentError, "Invalid transaction state: #{state}"
2939
end
30-
@state = state
40+
end
41+
42+
def rollback!
43+
@state = :rolledback
44+
end
45+
46+
def commit!
47+
@state = :committed
48+
end
49+
50+
def nullify!
51+
@state = nil
3152
end
3253
end
3354

@@ -57,7 +78,7 @@ def add_record(record)
5778
end
5879

5980
def rollback
60-
@state.set_state(:rolledback)
81+
@state.rollback!
6182
end
6283

6384
def rollback_records
@@ -72,7 +93,7 @@ def rollback_records
7293
end
7394

7495
def commit
75-
@state.set_state(:committed)
96+
@state.commit!
7697
end
7798

7899
def before_commit_records

activerecord/test/cases/transactions_test.rb

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,44 @@ def test_transactions_state_from_commit
725725
assert transaction.state.committed?
726726
end
727727

728+
def test_set_state_method_is_deprecated
729+
connection = Topic.connection
730+
transaction = ActiveRecord::ConnectionAdapters::TransactionManager.new(connection).begin_transaction
731+
732+
transaction.commit
733+
734+
assert_deprecated do
735+
transaction.state.set_state(:rolledback)
736+
end
737+
end
738+
739+
def test_mark_transaction_state_as_committed
740+
connection = Topic.connection
741+
transaction = ActiveRecord::ConnectionAdapters::TransactionManager.new(connection).begin_transaction
742+
743+
transaction.rollback
744+
745+
assert_equal :committed, transaction.state.commit!
746+
end
747+
748+
def test_mark_transaction_state_as_rolledback
749+
connection = Topic.connection
750+
transaction = ActiveRecord::ConnectionAdapters::TransactionManager.new(connection).begin_transaction
751+
752+
transaction.commit
753+
754+
assert_equal :rolledback, transaction.state.rollback!
755+
end
756+
757+
def test_mark_transaction_state_as_nil
758+
connection = Topic.connection
759+
transaction = ActiveRecord::ConnectionAdapters::TransactionManager.new(connection).begin_transaction
760+
761+
transaction.commit
762+
763+
assert_equal nil, transaction.state.nullify!
764+
end
765+
728766
def test_transaction_rollback_with_primarykeyless_tables
729767
connection = ActiveRecord::Base.connection
730768
connection.create_table(:transaction_without_primary_keys, force: true, id: false) do |t|

0 commit comments

Comments
 (0)