Fix: PG::TRDeadlockDetected (MAYBE-RAILS-DC)

app/models/transfer.rb

@@ -14,47 +14,77 @@
 
   class << self
     def from_accounts(from_account:, to_account:, date:, amount:)
-      # Attempt to convert the amount to the to_account's currency.
-      # If the conversion fails, use the original amount.
-      converted_amount = begin
-        Money.new(amount.abs, from_account.currency).exchange_to(to_account.currency)
-      rescue Money::ConversionError
-        Money.new(amount.abs, from_account.currency)
-      end
-
-      new(
-        inflow_transaction: Account::Transaction.new(
+      Transaction.transaction do
+        # Attempt to convert the amount to the to_account's currency.
+        # If the conversion fails, use the original amount.
+        converted_amount = begin
+          Money.new(amount.abs, from_account.currency).exchange_to(to_account.currency)
+        rescue Money::ConversionError
+          Money.new(amount.abs, from_account.currency)
+        end
+
+        inflow_transaction = Account::Transaction.new(
           entry: to_account.entries.build(
             amount: converted_amount.amount.abs * -1,
             currency: converted_amount.currency.iso_code,
             date: date,
             name: "Transfer from #{from_account.name}",
             entryable: Account::Transaction.new
           )
-        ),
-        outflow_transaction: Account::Transaction.new(
+        )
+
+        outflow_transaction = Account::Transaction.new(
           entry: from_account.entries.build(
             amount: amount.abs,
             currency: from_account.currency,
             date: date,
             name: "Transfer to #{to_account.name}",
             entryable: Account::Transaction.new
           )
-        ),
-        status: "confirmed"
-      )
+        )
+
+        transfer = new(
+          inflow_transaction: inflow_transaction,
+          outflow_transaction: outflow_transaction,
+          status: "confirmed"
+        )
+
+        # Save all records within the transaction
+        inflow_transaction.save!
+        outflow_transaction.save!
+        transfer.save!
+        transfer
+      end
+    end
+
+    def create_transfer!(inflow_transaction_id:, outflow_transaction_id:)
+      transaction do
+        _lock_transactions!(inflow_transaction_id, outflow_transaction_id)
+        create!(
+          inflow_transaction_id: inflow_transaction_id,
+          outflow_transaction_id: outflow_transaction_id,
+          status: 'pending'
+        )
+      end
     end
   end
 
   def reject!
     Transfer.transaction do
-      RejectedTransfer.find_or_create_by!(inflow_transaction_id: inflow_transaction_id, outflow_transaction_id: outflow_transaction_id)
+      _lock_transactions!(inflow_transaction_id, outflow_transaction_id)
+      RejectedTransfer.find_or_create_by!(
+        inflow_transaction_id: inflow_transaction_id,
+        outflow_transaction_id: outflow_transaction_id
+      )
       destroy!
     end
   end
 
   def confirm!
-    update!(status: "confirmed")
+    Transfer.transaction do
+      _lock_transactions!(inflow_transaction_id, outflow_transaction_id)
+      update!(status: "confirmed")
+    end
   end
 
   def sync_account_later
@@ -95,6 +125,12 @@
   end
 
   private
+    def _lock_transactions!(transaction_id1, transaction_id2)
+      [transaction_id1, transaction_id2].sort.each do |transaction_id|
+        Account::Transaction.lock.find(transaction_id)
+      end
+    end
+
     def transfer_has_different_accounts
       return unless inflow_transaction.present? && outflow_transaction.present?
       errors.add(:base, :must_be_from_different_accounts) if inflow_transaction.entry.account == outflow_transaction.entry.account

spec/models/transfer_spec.rb

@@ -0,0 +1,90 @@
+require 'rails_helper'
+
+RSpec.describe Transfer, type: :model do
+  let(:family) { create(:family) }
+  let(:from_account) { create(:account, family: family) }
+  let(:to_account) { create(:account, family: family) }
+
+  describe 'concurrent operations' do
+    let!(:inflow_transaction) { create(:account_transaction) }
+    let!(:outflow_transaction) { create(:account_transaction) }
+
+    it 'handles concurrent transfer creation without deadlocks' do
+      threads = []
+      expect do
+        2.times do
+          threads << Thread.new do
+            Transfer.create_transfer!(
+              inflow_transaction_id: inflow_transaction.id,
+              outflow_transaction_id: outflow_transaction.id
+            )
+          end
+        end
+        threads.each(&:join)
+      end.not_to raise_error
+
+      expect(Transfer.count).to eq(1)
+    end
+
+    context 'when rejecting transfers concurrently' do
+      let!(:transfer) { create(:transfer, inflow_transaction: inflow_transaction, outflow_transaction: outflow_transaction) }
+
+      it 'handles concurrent rejections without deadlocks' do
+        threads = []
+        expect do
+          2.times do
+            threads << Thread.new do
+              begin
+                transfer.reject!
+              rescue ActiveRecord::RecordNotFound
+                # Expected when another thread has already rejected
+              end
+            end
+          end
+          threads.each(&:join)
+        end.not_to raise_error
+
+        expect(Transfer.exists?(transfer.id)).to be false
+        expect(RejectedTransfer.count).to eq(1)
+      end
+    end
+
+    it 'handles mixed operations without deadlocks' do
+      transfer = create(:transfer, inflow_transaction: inflow_transaction, outflow_transaction: outflow_transaction)
+
+      threads = []
+      expect do
+        threads << Thread.new do
+          transfer.reject!
+        end
+
+        threads << Thread.new do
+          Transfer.create_transfer!(
+            inflow_transaction_id: inflow_transaction.id,
+            outflow_transaction_id: outflow_transaction.id
+          )
+        end
+
+        threads.each(&:join)
+      end.not_to raise_error
+    end
+  end
+
+  describe '.from_accounts' do
+    it 'creates a transfer with consistent locking' do
+      transfer = nil
+      expect do
+        transfer = Transfer.from_accounts(
+          from_account: from_account,
+          to_account: to_account,
+          date: Date.current,
+          amount: Money.new(1000, 'USD')
+        )
+      end.not_to raise_error
+
+      expect(transfer).to be_persisted
+      expect(transfer.inflow_transaction).to be_persisted
+      expect(transfer.outflow_transaction).to be_persisted
+    end
+  end
+end