Commit 3f8caeda authored by Manoj M J's avatar Manoj M J Committed by Adam Hegyi

Fix common errors in AuthorizedProjectsWorker

This MR aims at removing the
most commonly occurring errors in
AuthorizedProjectsWorker

Changelog: fixed
parent aa8bd302
......@@ -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
......
......@@ -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)
......
......@@ -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.
......
......@@ -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
......@@ -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) }
......
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