Merge branch 'bvl-authorized-projects-idempotent' into 'master'

Mark the AuthorizedProjectsWorker as idempotent

See merge request gitlab-org/gitlab!26794

app/workers/all_queues.yml

@@ -856,7 +856,7 @@
   :urgency: :high
   :resource_boundary: :unknown
   :weight: 2
-  :idempotent: 
+  :idempotent: true
 - :name: background_migration
   :feature_category: :not_owned
   :has_external_dependencies: 

app/workers/authorized_projects_worker.rb

 # frozen_string_literal: true
 
-class AuthorizedProjectsWorker # rubocop:disable Scalability/IdempotentWorker
+class AuthorizedProjectsWorker
   include ApplicationWorker
   prepend WaitableWorker
 
@@ -8,6 +8,8 @@ class AuthorizedProjectsWorker # rubocop:disable Scalability/IdempotentWorker
   urgency :high
   weight 2
 
+  idempotent!
+
   # This is a workaround for a Ruby 2.3.7 bug. rspec-mocks cannot restore the
   # visibility of prepended modules. See https://github.com/rspec/rspec-mocks/issues/1231
   # for more details.

spec/lib/gitlab/sidekiq_queue_spec.rb

@@ -2,18 +2,18 @@
 
 require 'spec_helper'
 
-describe Gitlab::SidekiqQueue do
+describe Gitlab::SidekiqQueue, :clean_gitlab_redis_queues do
   around do |example|
     Sidekiq::Queue.new('authorized_projects').clear
     Sidekiq::Testing.disable!(&example)
     Sidekiq::Queue.new('authorized_projects').clear
   end
 
-  def add_job(user)
+  def add_job(user, args)
     Sidekiq::Client.push(
       'class' => 'AuthorizedProjectsWorker',
       'queue' => 'authorized_projects',
-      'args' => [user.id],
+      'args' => args,
       'meta.user' => user.username
     )
   end
@@ -24,9 +24,9 @@ describe Gitlab::SidekiqQueue do
       let_it_be(:sidekiq_queue_user) { create(:user) }
 
       before do
-        add_job(create(:user))
-        add_job(sidekiq_queue_user)
-        add_job(sidekiq_queue_user)
+        add_job(create(:user), [1])
+        add_job(sidekiq_queue_user, [2])
+        add_job(sidekiq_queue_user, [3])
       end
 
       context 'when the queue is not processed in time' do
@@ -70,7 +70,7 @@ describe Gitlab::SidekiqQueue do
 
     context 'when there are no valid metadata keys passed' do
       it 'raises NoMetadataError' do
-        add_job(create(:user))
+        add_job(create(:user), [1])
 
         expect { described_class.new('authorized_projects').drop_jobs!({ username: 'sidekiq_queue_user' }, timeout: 1) }
           .to raise_error(described_class::NoMetadataError)

spec/requests/api/admin/sidekiq_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe API::Admin::Sidekiq do
+describe API::Admin::Sidekiq, :clean_gitlab_redis_queues do
   let_it_be(:admin) { create(:admin) }
 
   describe 'DELETE /admin/sidekiq/queues/:queue_name' do
@@ -21,20 +21,20 @@ describe API::Admin::Sidekiq do
         Sidekiq::Queue.new('authorized_projects').clear
       end
 
-      def add_job(user)
+      def add_job(user, args)
         Sidekiq::Client.push(
           'class' => 'AuthorizedProjectsWorker',
           'queue' => 'authorized_projects',
-          'args' => [user.id],
+          'args' => args,
           'meta.user' => user.username
         )
       end
 
       context 'valid request' do
         it 'returns info about the deleted jobs' do
-          add_job(admin)
-          add_job(admin)
-          add_job(create(:user))
+          add_job(admin, [1])
+          add_job(admin, [2])
+          add_job(create(:user), [3])
 
           delete api("/admin/sidekiq/queues/authorized_projects?user=#{admin.username}", admin)
 

spec/requests/api/graphql/mutations/admin/sidekiq_queues/delete_jobs_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe 'Deleting Sidekiq jobs' do
+describe 'Deleting Sidekiq jobs', :clean_gitlab_redis_queues do
   include GraphqlHelpers
 
   let_it_be(:admin) { create(:admin) }
@@ -31,19 +31,19 @@ describe 'Deleting Sidekiq jobs' do
         Sidekiq::Queue.new('authorized_projects').clear
       end
 
-      def add_job(user)
+      def add_job(user, args)
         Sidekiq::Client.push(
           'class' => 'AuthorizedProjectsWorker',
           'queue' => 'authorized_projects',
-          'args' => [user.id],
+          'args' => args,
           'meta.user' => user.username
         )
       end
 
       it 'returns info about the deleted jobs' do
-        add_job(admin)
-        add_job(admin)
-        add_job(create(:user))
+        add_job(admin, [1])
+        add_job(admin, [2])
+        add_job(create(:user), [3])
 
         post_graphql_mutation(mutation, current_user: admin)
 

spec/workers/authorized_projects_worker_spec.rb

@@ -21,5 +21,22 @@ describe AuthorizedProjectsWorker do
         job.perform(-1)
       end
     end
+
+    it_behaves_like "an idempotent worker" do
+      let(:job_args) { user.id }
+
+      it "does not change authorizations when run twice" do
+        group = create(:group)
+        create(:project, namespace: group)
+        group.add_developer(user)
+
+        # Delete the authorization created by the after save hook of the member
+        # created above.
+        user.project_authorizations.delete_all
+
+        expect { job.perform(user.id) }.to change { user.project_authorizations.reload.size }.by(1)
+        expect { job.perform(user.id) }.not_to change { user.project_authorizations.reload.size }
+      end
+    end
   end
 end