Skip to content

Commit 49299c4

Browse files
committed
Be clearer about when transactions are reset by reconnection
This adds a very awkward test-private method that attempts to detect whether a raw connection physically has a transaction open -- which is quite Not Great, but this seems important to get right.
1 parent 8d04150 commit 49299c4

File tree

3 files changed

+61
-3
lines changed

3 files changed

+61
-3
lines changed

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,9 +322,9 @@ def reload_type_map # :nodoc:
322322
def reconnect!
323323
@lock.synchronize do
324324
@raw_connection.reset
325+
super
325326
configure_connection
326327
reload_type_map
327-
super
328328
rescue PG::ConnectionBad
329329
connect
330330
end

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,9 @@ def active?
165165
end
166166

167167
def reconnect!
168+
unless @raw_connection.closed?
169+
@raw_connection.rollback rescue nil
170+
end
168171
super
169172
connect if @raw_connection.closed?
170173
end

activerecord/test/cases/adapter_test.rb

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,20 +384,50 @@ def setup
384384
assert_predicate @connection, :active?
385385
end
386386

387-
test "transaction state is reset after a reconnect" do
387+
test "materialized transaction state is reset after a reconnect" do
388388
@connection.begin_transaction
389389
assert_predicate @connection, :transaction_open?
390+
@connection.materialize_transactions
391+
assert raw_transaction_open?(@connection)
390392
@connection.reconnect!
391393
assert_not_predicate @connection, :transaction_open?
394+
assert_not raw_transaction_open?(@connection)
392395
end
393396

394-
test "transaction state is reset after a disconnect" do
397+
test "materialized transaction state is reset after a disconnect" do
395398
@connection.begin_transaction
396399
assert_predicate @connection, :transaction_open?
400+
@connection.materialize_transactions
401+
assert raw_transaction_open?(@connection)
397402
@connection.disconnect!
398403
assert_not_predicate @connection, :transaction_open?
399404
ensure
400405
@connection.reconnect!
406+
assert_not raw_transaction_open?(@connection)
407+
end
408+
409+
test "unmaterialized transaction state is reset after a reconnect" do
410+
@connection.begin_transaction
411+
assert_predicate @connection, :transaction_open?
412+
assert_not raw_transaction_open?(@connection)
413+
@connection.reconnect!
414+
assert_not_predicate @connection, :transaction_open?
415+
assert_not raw_transaction_open?(@connection)
416+
@connection.materialize_transactions
417+
assert_not raw_transaction_open?(@connection)
418+
end
419+
420+
test "unmaterialized transaction state is reset after a disconnect" do
421+
@connection.begin_transaction
422+
assert_predicate @connection, :transaction_open?
423+
assert_not raw_transaction_open?(@connection)
424+
@connection.disconnect!
425+
assert_not_predicate @connection, :transaction_open?
426+
ensure
427+
@connection.reconnect!
428+
assert_not raw_transaction_open?(@connection)
429+
@connection.materialize_transactions
430+
assert_not raw_transaction_open?(@connection)
401431
end
402432
end
403433

@@ -489,6 +519,31 @@ def test_reset_table_with_non_integer_pk
489519
end
490520

491521
private
522+
def raw_transaction_open?(connection)
523+
case connection.class::ADAPTER_NAME
524+
when "PostgreSQL"
525+
connection.instance_variable_get(:@raw_connection).transaction_status == ::PG::PQTRANS_INTRANS
526+
when "Mysql2"
527+
begin
528+
connection.instance_variable_get(:@raw_connection).query("SAVEPOINT transaction_test")
529+
connection.instance_variable_get(:@raw_connection).query("RELEASE SAVEPOINT transaction_test")
530+
531+
true
532+
rescue
533+
false
534+
end
535+
when "SQLite"
536+
begin
537+
connection.instance_variable_get(:@raw_connection).transaction { nil }
538+
false
539+
rescue
540+
true
541+
end
542+
else
543+
skip
544+
end
545+
end
546+
492547
def reset_fixtures(*fixture_names)
493548
ActiveRecord::FixtureSet.reset_cache
494549

0 commit comments

Comments
 (0)