Skip to content

Commit ccd5542

Browse files
oelisonChristian OelschlegelClearlyClaire
authored andcommitted
fix(tag): prevent dupl. tags on concurrent inserts (mastodon#35792)
Co-authored-by: Christian Oelschlegel <oelschle@sciphy.de> Co-authored-by: Claire <claire.github-309c@sitedethib.com>
1 parent 4effc36 commit ccd5542

2 files changed

Lines changed: 32 additions & 2 deletions

File tree

app/models/tag.rb

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,17 @@ def history
9090

9191
class << self
9292
def find_or_create_by_names(name_or_names)
93-
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)
93+
names = Array(name_or_names).map { |str| [normalize(str), str] }.uniq(&:first)
94+
95+
names.map do |(normalized_name, display_name)|
96+
tag = begin
97+
matching_name(normalized_name).first || create!(
98+
name: normalized_name,
99+
display_name: display_name.gsub(HASHTAG_INVALID_CHARS_RE, '')
100+
)
101+
rescue ActiveRecord::RecordNotUnique
102+
find_normalized(normalized_name)
103+
end
95104

96105
yield tag if block_given?
97106

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)