Merge branch 'refresh-authorizations-fork-join' into 'master'

Fix race conditions for AuthorizedProjectsWorker

Closes #26194 and #26310

See merge request !8701

8 files changed, 135 insertions, 1 deletions

diff --git a/spec/lib/gitlab/job_waiter_spec.rb b/spec/lib/gitlab/job_waiter_spec.rb
new file mode 100644
index 00000000000..780f5b1f8d7
--- /dev/null
+++ b/spec/lib/gitlab/job_waiter_spec.rb
@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+describe Gitlab::JobWaiter do
+  describe '#wait' do
+    let(:waiter) { described_class.new(%w(a)) }
+    it 'returns when all jobs have been completed' do
+      expect(Gitlab::SidekiqStatus).to receive(:all_completed?).with(%w(a)).
+        and_return(true)
+
+      expect(waiter).not_to receive(:sleep)
+
+      waiter.wait
+    end
+
+    it 'sleeps between checking the job statuses' do
+      expect(Gitlab::SidekiqStatus).to receive(:all_completed?).
+        with(%w(a)).
+        and_return(false, true)
+
+      expect(waiter).to receive(:sleep).with(described_class::INTERVAL)
+
+      waiter.wait
+    end
+
+    it 'returns when timing out' do
+      expect(waiter).not_to receive(:sleep)
+      waiter.wait(0)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/sidekiq_status/client_middleware_spec.rb b/spec/lib/gitlab/sidekiq_status/client_middleware_spec.rb
new file mode 100644
index 00000000000..287bf62d9bd
--- /dev/null
+++ b/spec/lib/gitlab/sidekiq_status/client_middleware_spec.rb
@@ -0,0 +1,12 @@
+require 'spec_helper'
+
+describe Gitlab::SidekiqStatus::ClientMiddleware do
+  describe '#call' do
+    it 'tracks the job in Redis' do
+      expect(Gitlab::SidekiqStatus).to receive(:set).with('123')
+
+      described_class.new.
+        call('Foo', { 'jid' => '123' }, double(:queue), double(:pool)) { nil }
+    end
+  end
+end
diff --git a/spec/lib/gitlab/sidekiq_status/server_middleware_spec.rb b/spec/lib/gitlab/sidekiq_status/server_middleware_spec.rb
new file mode 100644
index 00000000000..80728197b8c
--- /dev/null
+++ b/spec/lib/gitlab/sidekiq_status/server_middleware_spec.rb
@@ -0,0 +1,14 @@
+require 'spec_helper'
+
+describe Gitlab::SidekiqStatus::ServerMiddleware do
+  describe '#call' do
+    it 'stops tracking of a job upon completion' do
+      expect(Gitlab::SidekiqStatus).to receive(:unset).with('123')
+
+      ret = described_class.new.
+        call(double(:worker), { 'jid' => '123' }, double(:queue)) { 10 }
+
+      expect(ret).to eq(10)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/sidekiq_status_spec.rb b/spec/lib/gitlab/sidekiq_status_spec.rb
new file mode 100644
index 00000000000..0aa36a3416b
--- /dev/null
+++ b/spec/lib/gitlab/sidekiq_status_spec.rb
@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+describe Gitlab::SidekiqStatus do
+  describe '.set', :redis do
+    it 'stores the job ID' do
+      described_class.set('123')
+
+      key = described_class.key_for('123')
+
+      Sidekiq.redis do |redis|
+        expect(redis.exists(key)).to eq(true)
+        expect(redis.ttl(key) > 0).to eq(true)
+      end
+    end
+  end
+
+  describe '.unset', :redis do
+    it 'removes the job ID' do
+      described_class.set('123')
+      described_class.unset('123')
+
+      key = described_class.key_for('123')
+
+      Sidekiq.redis do |redis|
+        expect(redis.exists(key)).to eq(false)
+      end
+    end
+  end
+
+  describe '.all_completed?', :redis do
+    it 'returns true if all jobs have been completed' do
+      expect(described_class.all_completed?(%w(123))).to eq(true)
+    end
+
+    it 'returns false if a job has not yet been completed' do
+      described_class.set('123')
+
+      expect(described_class.all_completed?(%w(123 456))).to eq(false)
+    end
+  end
+
+  describe '.key_for' do
+    it 'returns the key for a job ID' do
+      key = described_class.key_for('123')
+
+      expect(key).to be_an_instance_of(String)
+      expect(key).to include('123')
+    end
+  end
+end
diff --git a/spec/services/user_project_access_changed_service_spec.rb b/spec/services/user_project_access_changed_service_spec.rb
new file mode 100644
index 00000000000..b4efe7de431
--- /dev/null
+++ b/spec/services/user_project_access_changed_service_spec.rb
@@ -0,0 +1,12 @@
+require 'spec_helper'
+
+describe UserProjectAccessChangedService do
+  describe '#execute' do
+    it 'schedules the user IDs' do
+      expect(AuthorizedProjectsWorker).to receive(:bulk_perform_and_wait).
+        with([[1], [2]])
+
+      described_class.new([1, 2]).execute
+    end
+  end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 6ee3307512d..4c5972d8111 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -6,7 +6,6 @@ ENV["RAILS_ENV"] ||= 'test'
 require File.expand_path("../../config/environment", __FILE__)
 require 'rspec/rails'
 require 'shoulda/matchers'
-require 'sidekiq/testing/inline'
 require 'rspec/retry'
 
 if ENV['CI'] && !ENV['NO_KNAPSACK']
diff --git a/spec/support/sidekiq.rb b/spec/support/sidekiq.rb
new file mode 100644
index 00000000000..575d3451150
--- /dev/null
+++ b/spec/support/sidekiq.rb
@@ -0,0 +1,5 @@
+require 'sidekiq/testing/inline'
+
+Sidekiq::Testing.server_middleware do |chain|
+  chain.add Gitlab::SidekiqStatus::ServerMiddleware
+end
diff --git a/spec/workers/authorized_projects_worker_spec.rb b/spec/workers/authorized_projects_worker_spec.rb
index b6591f272f6..97c4bfcd248 100644
--- a/spec/workers/authorized_projects_worker_spec.rb
+++ b/spec/workers/authorized_projects_worker_spec.rb
@@ -3,6 +3,18 @@ require 'spec_helper'
 describe AuthorizedProjectsWorker do
   let(:worker) { described_class.new }
 
+  describe '.bulk_perform_and_wait' do
+    it 'schedules the ids and waits for the jobs to complete' do
+      project = create(:project)
+
+      project.owner.project_authorizations.delete_all
+
+      described_class.bulk_perform_and_wait([[project.owner.id]])
+
+      expect(project.owner.project_authorizations.count).to eq(1)
+    end
+  end
+
   describe '#perform' do
     it "refreshes user's authorized projects" do
       user = create(:user)