Merge branch 'mmj-improve-authorized-projects-worker' into 'master'

Fix common errors in AuthorizedProjectsWorker

See merge request gitlab-org/gitlab!74055

app/models/project_authorization.rb

@@ -24,8 +24,9 @@ class ProjectAuthorization < ApplicationRecord
       end
 
       connection.execute <<-EOF.strip_heredoc
-      INSERT INTO project_authorizations (user_id, project_id, access_level)
-      VALUES #{tuples.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')}
+        INSERT INTO project_authorizations (user_id, project_id, access_level)
+        VALUES #{tuples.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')}
+        ON CONFLICT DO NOTHING
       EOF
     end
   end

app/models/user.rb

@@ -1020,8 +1020,10 @@ class User < ApplicationRecord
   end
   # rubocop: enable CodeReuse/ServiceClass
 
-  def remove_project_authorizations(project_ids)
-    project_authorizations.where(project_id: project_ids).delete_all
+  def remove_project_authorizations(project_ids, per_batch = 1000)
+    project_ids.each_slice(per_batch) do |project_ids_batch|
+      project_authorizations.where(project_id: project_ids_batch).delete_all
+    end
   end
 
   def authorized_projects(min_access_level = nil)

app/services/users/refresh_authorized_projects_service.rb

@@ -67,10 +67,8 @@ module Users
     def update_authorizations(remove = [], add = [])
       log_refresh_details(remove, add)
 
-      User.transaction do
-        user.remove_project_authorizations(remove) unless remove.empty?
-        ProjectAuthorization.insert_authorizations(add) unless add.empty?
-      end
+      user.remove_project_authorizations(remove) unless remove.empty?
+      ProjectAuthorization.insert_authorizations(add) unless add.empty?
 
       # Since we batch insert authorization rows, Rails' associations may get
       # out of sync. As such we force a reload of the User object.

spec/models/project_authorization_spec.rb

@@ -3,9 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe ProjectAuthorization do
-  let(:user) { create(:user) }
-  let(:project1) { create(:project) }
-  let(:project2) { create(:project) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project1) { create(:project) }
+  let_it_be(:project2) { create(:project) }
+  let_it_be(:project3) { create(:project) }
 
   describe '.insert_authorizations' do
     it 'inserts the authorizations' do
@@ -23,5 +24,19 @@ RSpec.describe ProjectAuthorization do
 
       expect(user.project_authorizations.count).to eq(2)
     end
+
+    it 'skips duplicates and inserts the remaining rows without error' do
+      create(:project_authorization, user: user, project: project1, access_level: Gitlab::Access::MAINTAINER)
+
+      rows = [
+        [user.id, project1.id, Gitlab::Access::MAINTAINER],
+        [user.id, project2.id, Gitlab::Access::MAINTAINER],
+        [user.id, project3.id, Gitlab::Access::MAINTAINER]
+      ]
+
+      described_class.insert_authorizations(rows)
+
+      expect(user.project_authorizations.pluck(:user_id, :project_id, :access_level)).to match_array(rows)
+    end
   end
 end

spec/models/user_spec.rb

@@ -4147,6 +4147,23 @@ RSpec.describe User do
     end
   end
 
+  describe '#remove_project_authorizations' do
+    let_it_be(:project1) { create(:project) }
+    let_it_be(:project2) { create(:project) }
+    let_it_be(:project3) { create(:project) }
+    let_it_be(:user) { create(:user) }
+
+    it 'removes the project authorizations of the user, in specified projects' do
+      create(:project_authorization, user: user, project: project1)
+      create(:project_authorization, user: user, project: project2)
+      create(:project_authorization, user: user, project: project3)
+
+      user.remove_project_authorizations([project1.id, project2.id])
+
+      expect(user.project_authorizations.pluck(:project_id)).to match_array([project3.id])
+    end
+  end
+
   describe '#access_level=' do
     let(:user) { build(:user) }