Based on the comments on my blog post from yesterday, many developers seem to think that adding lock_version to your tables (for optimistic locking) will prevent the problem where two users update a record around the same time, and one update reverts the changes from the other update.
I don't think that's true for how most applications are implemented. Optimistic locking prevents updating a stale object, but it doesn't prevent updating a fresh object with changes based on a stale object. And solving that problem isn't simple (at least, not that I know. I'm really hoping somebody will enlighten me).
I'm going to use the same scenario from my blog post yesterday. I'm not talking about the invalid record problem anymore, only the problem with an update undoing changes from another update.
Here's the schema and model. I added a lock_version column, so Rails will automatically do optimistic locking.
create_table :employees do |t| t.boolean :manager t.integer :salary t.integer :lock_version end class Employee < ActiveRecord::Base # I removed the validations from the other blog post. # They don't matter for this one. end
For the controller, I'm going to use standard CRUD edit and update actions:
class EmployeesController < ApplicationController def edit @employee = Employee.find(params[:id]) end def update @employee = Employee.find(params[:id]) if @employee.update_attributes(params[:employee]) flash[:notice] = 'Employee was successfully updated.' redirect_to(@employee) else render :action => "edit" end end end
The form will have two fields, a checkbox to indicate if the employee is a manager, and a field to enter a salary.
Here's why simply adding a lock_version column doesn't prevent the problem. Let's start with an employee that two users are going to edit.
employee = Employee.create! :manager => false, :salary => 3
User 1 and user 2 both go to the edit page. They're both presented with a form, with the manager checkbox unchecked, and salary set to 3.
User 1 checks the box to make the employee a manager and submits the form. The form parameters that are posted are:
{:employee => {:manager => "1", :salary => "3"}}
The record will be updated, and the lock_version incremented to 2.
Now user 2 changes the employee salary to 2, leaving the check box to indicate the employee is a manager unchecked. She submits the form, and the form parameters are:
{:employee => {:manager => "0", :salary => "2"}}
The update action finds the record (lock version is 2). It calls update_attributes with the form parameters, and the record is updated. The lock_version is incremented to 3.
Even though we had optimistic locking enabled, user 2's change reverted user 1's change, and the employee is no longer a manager.
What happened wasn't that user 2 updated a stale object (optimistic locking does prevent that). User 2 updated a fresh object based on changes to a stale object. The form was rendered with the attributes from lock_version 1, but the update was performed on the object with lock_version 2. This happened because the update action makes another find call, which I think is how most people implement edit/update actions.
We would have the same outcome using pessimistic locking.
There are some solutions, but as I said at the beginning of the post, I don't think it's simple.
I can think of two possible solutions to this problem:
(1) Add lock_version to the form.
(2) Make the update action use the same object as the edit action.
The end result of either of these approaches is the first update will succeed, and the second will fail.
Adding lock_version to the form will fix this problem, and user 2's update would cause a StaleObjectError exception.
When the users goes to the edit page, their form values would both be:
{:employee => {:manager => "0", :salary => "3", :lock_version => "1"}}
User 1 checks the manager check box and submits the form. The posted form parameters are:
{:employee => {:manager => "1", :salary => "3", :lock_version => "1"}}
Having lock_version as part of the parameters will set the lock_version in the call to update_attributes. When the update action does the find to get the employee record to update, it's lock_version will be 1, so in effect it won't change it. The update will succeed, and the lock_version will be 2.
Now user 2 changes the salary to 2 and submits the form. The posted form parameters are:
{:employee => {:manager => "0", :salary => "2", :lock_version => "1"}}
The employee record in the update action will have a lock_version of 2, due to the update from user 1. Now update_attributes is going to change the lock_version from 2 to 1 because of the form parameter. This will cause the update to fail due to optimistic locking. It will preserve user 1's changes and reject user 2's.
Another solution is to keep the object in memory so that the update action will perform the update on the same object that the edit action used to render the form. Updating the controller like this would fix the problem without requiring lock_version as a hidden field on the form.
class EmployeesController < ApplicationController def edit @employee = Employee.find(params[:id]) session[:employee] = @employee end def update @employee = session[:employee] if @employee.update_attributes(params[:employee]) flash[:notice] = 'Employee was successfully updated.' redirect_to(@employee) else render :action => "edit" end end end
This solution has some drawbacks. You can't define instance-specific behavior on the employee instance, or you won't be able to put it in the session. You'll receive a "singleton can't be dumped" error. An action like this blows up:
def action o = Object.new def o.foo "bar" end session[:o] = o end
There's a Railscast titled "Dangers of Model in Session." I haven't seen it, but it probably has some additional reasons not to do this.
Let's add another column to the employees table, employee_of_the_month_votes, an integer. The edit page I've been talking about wouldn't be able to edit this column, instead there might be a separate controller action.
class EmployeesController < ActionController::Base def vote_for_employee_of_the_month employee = Employee.find(params[:id]) employee.vote_for_employee_of_the_month employee.save! end end class Employee < ActiveRecord::Base def vote_for_employee_of_the_month self.employee_of_the_month_votes += 1 end end
With the lock_version column present, this update will increment the lock_version. That means that even if only a single user is changing the employee's management status or salary, the update will fail if between the time when the user opened the edit page, made his changes, and submitted the form, somebody else voted for the employee as employee of the month. Clearly that's not desirable. It's also more likely to happen the longer the user takes to make his edits.
I was talking to Philippe about this, and he summarized it well:
Optimistic locking is great tool, but you have to be aware of what problems it solves and which it does not solve.
I'm not saying that you shouldn't use optimistic locking. But only adding the lock_version column doesn't prevent the problem of one user's changes being overwritten by another user's changes if you're implementing your edit and update actions the common way.
If anybody has a simple solution, I'd definitely be interested in hearing about it. I can only think of some not-so-simple ones that aren't trivial to implement while still providing good usability.
Thanks to Philippe Hanrigou and Josh Schairbaum for giving me feedback on a draft of this post.