Merge branch 'security-approval-race-condition' into 'master'
Add ApplicationRecord#safe_ensure_unique method See merge request gitlab/gitlabhq!3054
This commit is contained in:
commit
fee6e478d6
|
@ -17,6 +17,19 @@ class ApplicationRecord < ActiveRecord::Base
|
|||
where(nil).pluck(self.primary_key)
|
||||
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?
|
||||
|
@ -24,10 +37,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
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue