diff options
author | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-04-25 13:38:38 +0300 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-04-25 13:38:38 +0300 |
commit | 8becca4f342e18a887ea9d434e15f54165fd22b0 (patch) | |
tree | d7d0d679097ee5bd93db7880fd84438ad6326b4a | |
parent | 0ef18bb51604c175632cd96ffe0b220860e5c11f (diff) | |
parent | fe374fa7e76ab528a7e30b633387a776a5e1b148 (diff) |
Merge branch 'security-approval-race-condition-11-8' into '11-8-stable'
Add ApplicationRecord#safe_ensure_unique method
See merge request gitlab/gitlabhq!3057
-rw-r--r-- | app/models/application_record.rb | 17 | ||||
-rw-r--r-- | spec/models/application_record_spec.rb | 19 |
2 files changed, 33 insertions, 3 deletions
diff --git a/app/models/application_record.rb b/app/models/application_record.rb index a3d662d8250..289864bbaa4 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -7,6 +7,19 @@ class ApplicationRecord < ActiveRecord::Base where(id: ids) end + def self.safe_ensure_unique(retries: 0) + transaction(requires_new: true) do + yield + end + rescue ActiveRecord::RecordNotUnique + if retries > 0 + retries -= 1 + retry + end + + false + end + def self.safe_find_or_create_by!(*args) safe_find_or_create_by(*args).tap do |record| record.validate! unless record.persisted? @@ -14,10 +27,8 @@ class ApplicationRecord < ActiveRecord::Base end def self.safe_find_or_create_by(*args) - transaction(requires_new: true) do + safe_ensure_unique(retries: 1) do find_or_create_by(*args) end - rescue ActiveRecord::RecordNotUnique - retry end end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index fd25132ed3a..cc90a998d3f 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -11,6 +11,25 @@ describe ApplicationRecord do end end + describe '.safe_ensure_unique' do + let(:model) { build(:suggestion) } + let(:klass) { model.class } + + before do + allow(model).to receive(:save).and_raise(ActiveRecord::RecordNotUnique) + end + + it 'returns false when ActiveRecord::RecordNotUnique is raised' do + expect(model).to receive(:save).once + expect(klass.safe_ensure_unique { model.save }).to be_falsey + end + + it 'retries based on retry count specified' do + expect(model).to receive(:save).exactly(3).times + expect(klass.safe_ensure_unique(retries: 2) { model.save }).to be_falsey + 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) |