diff options
-rw-r--r-- | app/models/application_record.rb | 8 | ||||
-rw-r--r-- | doc/development/sql.md | 34 | ||||
-rw-r--r-- | spec/models/application_record_spec.rb | 10 |
3 files changed, 25 insertions, 27 deletions
diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 29696ab276f..c4e310e638d 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -6,4 +6,12 @@ class ApplicationRecord < ActiveRecord::Base def self.id_in(ids) where(id: ids) end + + def self.safe_find_or_create_by(*args) + transaction(requires_new: true) do + find_or_create_by(*args) + end + rescue ActiveRecord::RecordNotUnique + retry + end end diff --git a/doc/development/sql.md b/doc/development/sql.md index 06005a0a6f8..47519d39e74 100644 --- a/doc/development/sql.md +++ b/doc/development/sql.md @@ -256,32 +256,12 @@ violation, for example. Using transactions does not solve this problem. -The following pattern should be used to avoid the problem: +To solve this we've added the `ApplicationRecord.safe_find_or_create_by`. -```ruby -Project.transaction do - begin - User.find_or_create_by(username: "foo") - rescue ActiveRecord::RecordNotUnique - retry - end -end -``` - -If the above block is run inside a transaction and hits the race -condition, the transaction is aborted and we cannot simply retry (any -further queries inside the aborted transaction are going to fail). We -can employ [nested transactions](http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Nested+transactions) -here to only rollback the "inner transaction". Note that `requires_new: true` is required here. +This method can be used just as you would the normal +`find_or_create_by` but it wraps the call in a *new* transaction and +retries if it were to fail because of an +`ActiveRecord::RecordNotUnique` error. -```ruby -Project.transaction do - begin - User.transaction(requires_new: true) do - User.find_or_create_by(username: "foo") - end - rescue ActiveRecord::RecordNotUnique - retry - end -end -``` +To be able to use this method, make sure the model you want to use +this on inherits from `ApplicationRecord`. diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index 68aed387bfc..ca23f581fdc 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -10,4 +10,14 @@ describe ApplicationRecord do expect(User.id_in(records.last(2).map(&:id))).to eq(records.last(2)) end end + + describe '#safe_find_or_create_by' do + it 'creates the user avoiding race conditions' do + expect(Suggestion).to receive(:find_or_create_by).and_raise(ActiveRecord::RecordNotUnique) + allow(Suggestion).to receive(:find_or_create_by).and_call_original + + expect { Suggestion.safe_find_or_create_by(build(:suggestion).attributes) } + .to change { Suggestion.count }.by(1) + end + end end |