Skip to content

Commit 4001b83

Browse files
committed
fix(tag): prevent dupl. tags on concurrent inserts
1 parent 14dedbf commit 4001b83

2 files changed

Lines changed: 26 additions & 1 deletion

File tree

app/models/tag.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,11 @@ def history
9191
class << self
9292
def find_or_create_by_names(name_or_names)
9393
Array(name_or_names).map(&method(:normalize)).uniq { |str| str.mb_chars.downcase.to_s }.map do |normalized_name|
94-
tag = matching_name(normalized_name).first || create(name: normalized_name)
94+
tag = begin
95+
matching_name(normalized_name).first || create!(name: normalized_name)
96+
rescue ActiveRecord::RecordNotUnique
97+
find_normalized(normalized_name)
98+
end
9599

96100
yield tag if block_given?
97101

spec/models/tag_spec.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,27 @@
134134
end
135135
end
136136

137+
describe '.find_or_create_by_names race condition' do
138+
it 'handles simultaneous inserts of the same tag in different cases without error' do
139+
tag_name_upper = 'Rails'
140+
tag_name_lower = 'rails'
141+
142+
threads = []
143+
144+
2.times do |i|
145+
threads << Thread.new do
146+
described_class.find_or_create_by_names(i.zero? ? tag_name_upper : tag_name_lower)
147+
end
148+
end
149+
150+
threads.each(&:join)
151+
152+
tags = described_class.where('lower(name) = ?', tag_name_lower.downcase)
153+
expect(tags.count).to eq(1)
154+
expect(tags.first.name.downcase).to eq(tag_name_lower.downcase)
155+
end
156+
end
157+
137158
describe '.search_for' do
138159
it 'finds tag records with matching names' do
139160
tag = Fabricate(:tag, name: "match")

0 commit comments

Comments
 (0)