views:

644

answers:

2

ActiveRecord's validates_uniqueness_of is vulnerable to race conditions. To really ensure uniqueness requires additional safeguards. One suggestion from the ActiveRecord RDocs is to create a unique index on the database, for example by including in your migrations:

add_index :recipes, :name, :unique => true

This will ensure at the database level that the name is unique. But a disadvantage to this approach is that the ActiveRecord::StatementInvalid exception returned on attempting to save a duplicate is not very useful. One cannot be sure when catching this exception that the error was generated by a duplicate record and not just by broken SQL.

One solution, as the RDocs suggest, is to parse the message that comes with the exception and try to detect words like "duplicate" or "unique", but this is kludgy and the message is database backend specific. For SqlLite3, my understanding is that the message is totally generic and cannot be parsed this way at all.

Given that this is a fundamental problem for ActiveRecord users, it would be nice to know if there is any standard approach to handling these exceptions. I will offer my suggestion below; please comment or provide alternatives; thanks!

+5  A: 

Parsing the error message is not so bad, but feels kludgy. A suggestion I ran across (don't remember where) that seems appealing is that in the rescue block you can check the database to see if there is in fact a duplicate record. If there is, then chances are the StatementInvalid is because of the duplicate and you can handle it accordingly. If there isn't, then the StatementInvalid must be from something else, and you must handle it differently.

So the basic idea, assuming a unique index on recipe.name as above:

begin
  recipe.save!
rescue ActiveRecord::StatementInvalid
  if Recipe.count(:conditions => {:name => recipe.name}) > 0
    # It's a duplicate
  else
    # Not a duplicate; something else went wrong
  end
end

I attempted to automate this checking with the following:

class ActiveRecord::Base
  def violates_unique_index?(opts={})
    raise unless connection
    unique_indexes = connection.indexes(self.class.table_name).select{|i|i.unique}
    unique_indexes.each do |ui|
      conditions = {}
      ui.columns.each do |col|
        conditions[col] = send(col)
      end
      next if conditions.values.any?{|c|c.nil?} and !opts[:unique_includes_nil]
      return true if self.class.count(:conditions => conditions) > 0
    end
    return false
  end
end

So now you should be able to use generic_record.violates_unique_index? in your rescue block to decide how to handle StatementInvalid.

Hope that is helpful! Other approaches?

Chinasaur
I implemented this and seems to get the job done. Tempted to take out validates_uniqueness since this essentially does the same thing more efficiently. Will post any relevant updates.
Chinasaur
+1  A: 

Is this really such a big problem?

If you use a unique index together with a validates_uniqueness_of constraint, then

  • Data integrity will be maintained
  • You will at worst only get an error when two separate requests try to insert a non-unique row simultaneously

So unless you have an app which does many potential duplicate inserts (in which case I would look at redesigning that) I see this rarely being a problem in practice.

DanSingerman
Right, you will only get the exception rarely. But I still thought it would be nice to handle it :).One question stemming from your answer: if I can avoid it, do I even want to use `validates_uniqueness_of`? If my answer works, I'd be tempted to at least throw an :if on it...
Chinasaur