Commit f50ce77b authored by Tetiana Chupryna's avatar Tetiana Chupryna

Merge branch 'fix_each_batch_scope_leaking' into 'master'

Fix the leaking scope of `each_batch`

See merge request gitlab-org/gitlab!67459
parents a20fcab4 7ab6ca87
...@@ -91,7 +91,11 @@ module EachBatch ...@@ -91,7 +91,11 @@ module EachBatch
# Any ORDER BYs are useless for this relation and can lead to less # Any ORDER BYs are useless for this relation and can lead to less
# efficient UPDATE queries, hence we get rid of it. # efficient UPDATE queries, hence we get rid of it.
yield relation.except(:order), index relation = relation.except(:order)
# Using unscoped is necessary to prevent leaking the current scope used by
# ActiveRecord to chain `each_batch` method.
unscoped { yield relation, index }
break unless stop break unless stop
end end
......
...@@ -9,6 +9,8 @@ RSpec.describe EachBatch do ...@@ -9,6 +9,8 @@ RSpec.describe EachBatch do
include EachBatch include EachBatch
self.table_name = 'users' self.table_name = 'users'
scope :never_signed_in, -> { where(sign_in_count: 0) }
end end
end end
...@@ -72,5 +74,16 @@ RSpec.describe EachBatch do ...@@ -72,5 +74,16 @@ RSpec.describe EachBatch do
expect(ids).to eq(ids.sort.reverse) expect(ids).to eq(ids.sort.reverse)
end end
describe 'current scope' do
let(:entry) { create(:user, sign_in_count: 1) }
let(:ids_with_new_relation) { model.where(id: entry.id).pluck(:id) }
it 'does not leak current scope to block being executed' do
model.never_signed_in.each_batch(of: 5) do |relation|
expect(ids_with_new_relation).to include(entry.id)
end
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment