patternrubyrailsMinor
Transactions and locking in Rails 3
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
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
In
In
Also, since
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:
For further reference: Ruby on Rails documentation on Transactions.
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
endIn
tran.rb:def save_and_update_user_balances
transaction do
begin
save!
update_user_balances!
true
rescue
false
end
end
endIn
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
endFor 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
enddef save_and_update_user_balances
transaction do
begin
save!
update_user_balances!
true
rescue
false
end
end
enddef 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
endContext
StackExchange Code Review Q#2073, answer score: 3
Revisions (0)
No revisions yet.