HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyrailsMinor

Transactions and locking in Rails 3

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
andtransactionsrailslocking

Problem

I am new to Rails and have a system that needs to process transactions.

A user can enter a transaction to which one or more users are tied. These users owe some amount of money to the person making the transaction. For example, Bill might buy lunch for 4 friends and the bill is $125. They decide to split the bill 5 ways, so each owes $25. Bill would enter a total of $125 and enter each friend (including himself) as owing $25 to the transaction.

I have code in my controller and in my model to accomplish this goal, but I don't really know if I am using transactions and locking correctly. Also, is this the intended way to have this information in the controller?

I am using a transaction since all of these actions must occur together or fail (atomicity) and I need locking in case multiple users try to submit at the same time (isolation).

Maybe I should let the db on the backend handle locking? Does it do that already - say, MySQL?

trans_controller.rb

```
class TransController < ApplicationController
# POST trans/
def create
@title = "Create Transaction"
trans_successful = false

# Add the transaction from the client
@tran = Tran.new(params[:tran])

# Update the current user
@tran.submitting_user_id = current_user.id

# Update the data to the database
# This call commits the transaction and transaction users
# It also calls a method to update the balances of each user since that isn't
# part of the regular commit (why isn't it?)
begin
@tran.transaction do
@tran.save!
@tran.update_user_balances
trans_successful = true
end
rescue

end

# Save the transaction
if trans_successful
flash[:success] = 'Transaction was successfully created.'
redirect_to trans_path
else
flash.now[:error] = @tran.errors.full_messages.to_senten

Solution

I think it's a bad practice to use transactions in Controllers.
You should add a method on the Tran model instead (for example, save_and_update_user_balances):

def create
    @title = "Create Transaction"

    # Add the transaction from the client
    @tran = Tran.new(params[:tran])

    # Update the current user
    @tran.submitting_user_id = current_user.id

    # Update the data to the database
    # This call commits the transaction and transaction users 
    # It also calls a method to update the balances of each user since that isn't
    # part of the regular commit (why isn't it?)
    if(@tran.save_and_update_user_balances)
      redirect_to trans_path, success: 'Transaction was successfully created.'
    rescue
      flash.now[:error] = @tran.errors.full_messages.to_sentence          
      render 'new'
    end
end


In tran.rb:

def save_and_update_user_balances
  transaction do
    begin
      save! 
      update_user_balances!
      true
    rescue
      false
    end
  end
end


In update_user_balances! use update_attributes! and force to fail (and rollback) if an error happens.

Also, since update_user_balances! is called inside a transaction, you don't need to create another transaction, or lock. This is because of the way ActiveRecord works:


Though the transaction class method is called on some Active Record class, the objects within the transaction block need not all be instances of that class. This is because transactions are per-database connection, not per-model.

So the method should be:

def update_user_balances!
    self.buying_user.update_attributes! current_balance: self.buying_user.current_balance - self.total

    # Add an offsetting transaction_user for this record
    # Note: You could improve this by using a relationship to create it, like this?
    # transaction_users.create!(amount: -1 * self.total, user_id: buying_user_id)
    TransactionUser.create!(:amount => -1 * self.total, :user_id => self.buying_user_id, :tran => self)

    # Loop through each transaction user and update their balances.
    transaction_users.each do |tu|
        tu.user.update_attributes! current_balance: tu.user.current_balance + tu.amount
    end
end


For further reference: Ruby on Rails documentation on Transactions.

Code Snippets

def create
    @title = "Create Transaction"

    # Add the transaction from the client
    @tran = Tran.new(params[:tran])

    # Update the current user
    @tran.submitting_user_id = current_user.id

    # Update the data to the database
    # This call commits the transaction and transaction users 
    # It also calls a method to update the balances of each user since that isn't
    # part of the regular commit (why isn't it?)
    if(@tran.save_and_update_user_balances)
      redirect_to trans_path, success: 'Transaction was successfully created.'
    rescue
      flash.now[:error] = @tran.errors.full_messages.to_sentence          
      render 'new'
    end
end
def save_and_update_user_balances
  transaction do
    begin
      save! 
      update_user_balances!
      true
    rescue
      false
    end
  end
end
def update_user_balances!
    self.buying_user.update_attributes! current_balance: self.buying_user.current_balance - self.total

    # Add an offsetting transaction_user for this record
    # Note: You could improve this by using a relationship to create it, like this?
    # transaction_users.create!(amount: -1 * self.total, user_id: buying_user_id)
    TransactionUser.create!(:amount => -1 * self.total, :user_id => self.buying_user_id, :tran => self)

    # Loop through each transaction user and update their balances.
    transaction_users.each do |tu|
        tu.user.update_attributes! current_balance: tu.user.current_balance + tu.amount
    end
end

Context

StackExchange Code Review Q#2073, answer score: 3

Revisions (0)

No revisions yet.