Add latest changes from gitlab-org/gitlab@15-9-stable-eev15.9.0-rc42

10 files changed, 228 insertions, 68 deletions

diff --git a/spec/services/projects/container_repository/destroy_service_spec.rb b/spec/services/projects/container_repository/destroy_service_spec.rb
index 0ec0aecaa04..fed1d13daa5 100644
--- a/spec/services/projects/container_repository/destroy_service_spec.rb
+++ b/spec/services/projects/container_repository/destroy_service_spec.rb
@@ -5,86 +5,131 @@ require 'spec_helper'
 RSpec.describe Projects::ContainerRepository::DestroyService do
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, :private) }
+  let_it_be(:params) { {} }
 
-  subject { described_class.new(project, user) }
+  subject { described_class.new(project, user, params) }
 
   before do
     stub_container_registry_config(enabled: true)
   end
 
-  context 'when user does not have access to registry' do
-    let!(:repository) { create(:container_repository, :root, project: project) }
+  shared_examples 'returning an error status with message' do |error_message|
+    it 'returns an error status' do
+      response = subject.execute(repository)
 
-    it 'does not delete a repository' do
-      expect { subject.execute(repository) }.not_to change { ContainerRepository.count }
+      expect(response).to include(status: :error, message: error_message)
     end
   end
 
-  context 'when user has access to registry' do
+  shared_examples 'executing with permissions' do
+    let_it_be_with_refind(:repository) { create(:container_repository, :root, project: project) }
+
     before do
-      project.add_developer(user)
+      stub_container_registry_tags(repository: :any, tags: %w[latest stable])
     end
 
-    context 'when root container repository exists' do
-      let!(:repository) { create(:container_repository, :root, project: project) }
+    it 'deletes the repository' do
+      expect_cleanup_tags_service_with(container_repository: repository, return_status: :success)
+      expect { subject.execute(repository) }.to change { ContainerRepository.count }.by(-1)
+    end
+
+    it 'sends disable_timeout = true as part of the params as default' do
+      expect_cleanup_tags_service_with(container_repository: repository, return_status: :success, disable_timeout: true)
+      expect { subject.execute(repository) }.to change { ContainerRepository.count }.by(-1)
+    end
+
+    it 'sends disable_timeout = false as part of the params if it is set to false' do
+      expect_cleanup_tags_service_with(container_repository: repository, return_status: :success, disable_timeout: false)
+      expect { subject.execute(repository, disable_timeout: false) }.to change { ContainerRepository.count }.by(-1)
+    end
 
+    context 'when deleting the tags fails' do
       before do
-        stub_container_registry_tags(repository: :any, tags: %w[latest stable])
+        expect_cleanup_tags_service_with(container_repository: repository, return_status: :error)
+        allow(Gitlab::AppLogger).to receive(:error).and_call_original
       end
 
-      it 'deletes the repository' do
-        expect_cleanup_tags_service_with(container_repository: repository, return_status: :success)
-        expect { subject.execute(repository) }.to change { ContainerRepository.count }.by(-1)
-      end
+      it 'sets status as deleted_failed' do
+        subject.execute(repository)
 
-      it 'sends disable_timeout = true as part of the params as default' do
-        expect_cleanup_tags_service_with(container_repository: repository, return_status: :success, disable_timeout: true)
-        expect { subject.execute(repository) }.to change { ContainerRepository.count }.by(-1)
+        expect(repository).to be_delete_failed
       end
 
-      it 'sends disable_timeout = false as part of the params if it is set to false' do
-        expect_cleanup_tags_service_with(container_repository: repository, return_status: :success, disable_timeout: false)
-        expect { subject.execute(repository, disable_timeout: false) }.to change { ContainerRepository.count }.by(-1)
-      end
+      it 'logs the error' do
+        subject.execute(repository)
 
-      context 'when deleting the tags fails' do
-        it 'sets status as deleted_failed' do
-          expect_cleanup_tags_service_with(container_repository: repository, return_status: :error)
-          allow(Gitlab::AppLogger).to receive(:error).and_call_original
+        expect(Gitlab::AppLogger).to have_received(:error)
+        .with("Container repository with ID: #{repository.id} and path: #{repository.path} failed with message: error in deleting tags")
+      end
 
-          subject.execute(repository)
+      it_behaves_like 'returning an error status with message', 'Deletion failed for container repository'
+    end
 
-          expect(repository).to be_delete_failed
-          expect(Gitlab::AppLogger).to have_received(:error)
-          .with("Container repository with ID: #{repository.id} and path: #{repository.path} failed with message: error in deleting tags")
-        end
+    context 'when destroying the repository fails' do
+      before do
+        expect_cleanup_tags_service_with(container_repository: repository, return_status: :success)
+        allow(repository).to receive(:destroy).and_return(false)
+        allow(repository.errors).to receive(:full_messages).and_return(['Error 1', 'Error 2'])
+        allow(Gitlab::AppLogger).to receive(:error).and_call_original
       end
 
-      context 'when destroying the repository fails' do
-        it 'sets status as deleted_failed' do
-          expect_cleanup_tags_service_with(container_repository: repository, return_status: :success)
-          allow(repository).to receive(:destroy).and_return(false)
-          allow(repository.errors).to receive(:full_messages).and_return(['Error 1', 'Error 2'])
-          allow(Gitlab::AppLogger).to receive(:error).and_call_original
+      it 'sets status as deleted_failed' do
+        subject.execute(repository)
+
+        expect(repository).to be_delete_failed
+      end
 
-          subject.execute(repository)
+      it 'logs the error' do
+        subject.execute(repository)
 
-          expect(repository).to be_delete_failed
-          expect(Gitlab::AppLogger).to have_received(:error)
-          .with("Container repository with ID: #{repository.id} and path: #{repository.path} failed with message: Error 1. Error 2")
-        end
+        expect(Gitlab::AppLogger).to have_received(:error)
+        .with("Container repository with ID: #{repository.id} and path: #{repository.path} failed with message: Error 1. Error 2")
       end
 
-      def expect_cleanup_tags_service_with(container_repository:, return_status:, disable_timeout: true)
-        delete_tags_service = instance_double(Projects::ContainerRepository::CleanupTagsService)
+      it_behaves_like 'returning an error status with message', 'Deletion failed for container repository'
+    end
+  end
+
+  context 'when user has access to registry' do
+    before do
+      project.add_developer(user)
+    end
 
-        expect(Projects::ContainerRepository::CleanupTagsService).to receive(:new).with(
-          container_repository: container_repository,
-          params: described_class::CLEANUP_TAGS_SERVICE_PARAMS.merge('disable_timeout' => disable_timeout)
-        ).and_return(delete_tags_service)
+    it_behaves_like 'executing with permissions'
+  end
 
-        expect(delete_tags_service).to receive(:execute).and_return(status: return_status)
-      end
+  context 'when user does not have access to registry' do
+    let_it_be(:repository) { create(:container_repository, :root, project: project) }
+
+    it 'does not delete a repository' do
+      expect { subject.execute(repository) }.not_to change { ContainerRepository.count }
     end
+
+    it_behaves_like 'returning an error status with message', 'Unauthorized access'
+  end
+
+  context 'when called during project deletion' do
+    let(:user) { nil }
+    let(:params) { { skip_permission_check: true } }
+
+    it_behaves_like 'executing with permissions'
+  end
+
+  context 'when there is no user' do
+    let(:user) { nil }
+    let(:repository) { create(:container_repository, :root, project: project) }
+
+    it_behaves_like 'returning an error status with message', 'Unauthorized access'
+  end
+
+  def expect_cleanup_tags_service_with(container_repository:, return_status:, disable_timeout: true)
+    delete_tags_service = instance_double(Projects::ContainerRepository::CleanupTagsService)
+
+    expect(Projects::ContainerRepository::CleanupTagsService).to receive(:new).with(
+      container_repository: container_repository,
+      params: described_class::CLEANUP_TAGS_SERVICE_PARAMS.merge('disable_timeout' => disable_timeout)
+    ).and_return(delete_tags_service)
+
+    expect(delete_tags_service).to receive(:execute).and_return(status: return_status)
   end
 end
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index f85a8eda7ee..e435db4efa6 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -163,7 +163,7 @@ RSpec.describe Projects::CreateService, '#execute', feature_category: :projects
 
   describe 'after create actions' do
     it 'invalidate personal_projects_count caches' do
-      expect(user).to receive(:invalidate_personal_projects_count)
+      expect(Rails.cache).to receive(:delete).with(['users', user.id, 'personal_projects_count'])
 
       create_project(user, opts)
     end
@@ -947,6 +947,8 @@ RSpec.describe Projects::CreateService, '#execute', feature_category: :projects
     end
 
     it 'schedules authorization update for users with access to group', :sidekiq_inline do
+      stub_feature_flags(do_not_run_safety_net_auth_refresh_jobs: false)
+
       expect(AuthorizedProjectsWorker).not_to(
         receive(:bulk_perform_async)
       )
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index ff2de45661f..0689a65c2f4 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publisher do
+RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publisher, feature_category: :projects do
   include ProjectForksHelper
   include BatchDestroyDependentAssociationsHelper
 
@@ -151,10 +151,22 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi
 
   it_behaves_like 'deleting the project'
 
-  it 'invalidates personal_project_count cache' do
-    expect(user).to receive(:invalidate_personal_projects_count)
+  context 'personal projects count cache' do
+    context 'when the executor is the creator of the project itself' do
+      it 'invalidates personal_project_count cache of the the owner of the personal namespace' do
+        expect(user).to receive(:invalidate_personal_projects_count)
 
-    destroy_project(project, user, {})
+        destroy_project(project, user, {})
+      end
+    end
+
+    context 'when the executor is the instance administrator', :enable_admin_mode do
+      it 'invalidates personal_project_count cache of the the owner of the personal namespace' do
+        expect(user).to receive(:invalidate_personal_projects_count)
+
+        destroy_project(project, create(:admin), {})
+      end
+    end
   end
 
   context 'with running pipelines' do
@@ -331,18 +343,30 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi
       end
 
       context 'when image repository deletion succeeds' do
-        it 'removes tags' do
-          expect_any_instance_of(Projects::ContainerRepository::CleanupTagsService)
-            .to receive(:execute).and_return({ status: :success })
+        it 'returns true' do
+          expect_next_instance_of(Projects::ContainerRepository::CleanupTagsService) do |instance|
+            expect(instance).to receive(:execute).and_return(status: :success)
+          end
 
-          destroy_project(project, user)
+          expect(destroy_project(project, user)).to be true
+        end
+      end
+
+      context 'when image repository deletion raises an error' do
+        it 'returns false' do
+          expect_next_instance_of(Projects::ContainerRepository::CleanupTagsService) do |service|
+            expect(service).to receive(:execute).and_raise(RuntimeError)
+          end
+
+          expect(destroy_project(project, user)).to be false
         end
       end
 
       context 'when image repository deletion fails' do
-        it 'raises an exception' do
-          expect_any_instance_of(Projects::ContainerRepository::CleanupTagsService)
-            .to receive(:execute).and_raise(RuntimeError)
+        it 'returns false' do
+          expect_next_instance_of(Projects::ContainerRepository::DestroyService) do |service|
+            expect(service).to receive(:execute).and_return({ status: :error })
+          end
 
           expect(destroy_project(project, user)).to be false
         end
@@ -369,8 +393,9 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi
 
       context 'when image repository tags deletion succeeds' do
         it 'removes tags' do
-          expect_any_instance_of(ContainerRepository)
-            .to receive(:delete_tags!).and_return(true)
+          expect_next_instance_of(Projects::ContainerRepository::DestroyService) do |service|
+            expect(service).to receive(:execute).and_return({ status: :sucess })
+          end
 
           destroy_project(project, user)
         end
@@ -378,13 +403,27 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi
 
       context 'when image repository tags deletion fails' do
         it 'raises an exception' do
-          expect_any_instance_of(ContainerRepository)
-            .to receive(:delete_tags!).and_return(false)
+          expect_next_instance_of(Projects::ContainerRepository::DestroyService) do |service|
+            expect(service).to receive(:execute).and_return({ status: :error })
+          end
 
           expect(destroy_project(project, user)).to be false
         end
       end
     end
+
+    context 'when there are no tags for legacy root repository' do
+      before do
+        stub_container_registry_tags(repository: project.full_path,
+                                     tags: [])
+      end
+
+      it 'does not try to destroy the repository' do
+        expect(Projects::ContainerRepository::DestroyService).not_to receive(:new)
+
+        destroy_project(project, user)
+      end
+    end
   end
 
   context 'for a forked project with LFS objects' do
diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb
index 65d3085a850..eae898b4f68 100644
--- a/spec/services/projects/group_links/create_service_spec.rb
+++ b/spec/services/projects/group_links/create_service_spec.rb
@@ -58,6 +58,8 @@ RSpec.describe Projects::GroupLinks::CreateService, '#execute' do
       end
 
       it 'schedules authorization update for users with access to group' do
+        stub_feature_flags(do_not_run_safety_net_auth_refresh_jobs: false)
+
         expect(AuthorizedProjectsWorker).not_to(
           receive(:bulk_perform_async)
         )
diff --git a/spec/services/projects/group_links/destroy_service_spec.rb b/spec/services/projects/group_links/destroy_service_spec.rb
index 5d07fd52230..89865d6bc3b 100644
--- a/spec/services/projects/group_links/destroy_service_spec.rb
+++ b/spec/services/projects/group_links/destroy_service_spec.rb
@@ -28,6 +28,8 @@ RSpec.describe Projects::GroupLinks::DestroyService, '#execute' do
     end
 
     it 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' do
+      stub_feature_flags(do_not_run_safety_net_auth_refresh_jobs: false)
+
       expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to(
         receive(:bulk_perform_in)
           .with(1.hour,
diff --git a/spec/services/projects/group_links/update_service_spec.rb b/spec/services/projects/group_links/update_service_spec.rb
index 20616890ebd..1acbb770763 100644
--- a/spec/services/projects/group_links/update_service_spec.rb
+++ b/spec/services/projects/group_links/update_service_spec.rb
@@ -42,6 +42,8 @@ RSpec.describe Projects::GroupLinks::UpdateService, '#execute' do
     end
 
     it 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' do
+      stub_feature_flags(do_not_run_safety_net_auth_refresh_jobs: false)
+
       expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to(
         receive(:bulk_perform_in)
           .with(1.hour,
diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb
index 2c1ebe27014..be059aec697 100644
--- a/spec/services/projects/import_export/export_service_spec.rb
+++ b/spec/services/projects/import_export/export_service_spec.rb
@@ -2,11 +2,12 @@
 
 require 'spec_helper'
 
-RSpec.describe Projects::ImportExport::ExportService do
+RSpec.describe Projects::ImportExport::ExportService, feature_category: :importers do
   describe '#execute' do
     let_it_be(:user) { create(:user) }
+    let_it_be(:group) { create(:group) }
+    let_it_be_with_reload(:project) { create(:project, group: group) }
 
-    let(:project) { create(:project) }
     let(:shared) { project.import_export_shared }
     let!(:after_export_strategy) { Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy.new }
 
@@ -220,5 +221,21 @@ RSpec.describe Projects::ImportExport::ExportService do
         expect { service.execute }.to raise_error(Gitlab::ImportExport::Error).with_message(expected_message)
       end
     end
+
+    it "avoids N+1 when exporting project members" do
+      group.add_owner(user)
+      group.add_maintainer(create(:user))
+      project.add_maintainer(create(:user))
+
+      # warm up
+      service.execute
+
+      control = ActiveRecord::QueryRecorder.new { service.execute }
+
+      group.add_maintainer(create(:user))
+      project.add_maintainer(create(:user))
+
+      expect { service.execute }.not_to exceed_query_limit(control)
+    end
   end
 end
diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb
index 38ab7b6e2ee..97a3b338069 100644
--- a/spec/services/projects/import_service_spec.rb
+++ b/spec/services/projects/import_service_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Projects::ImportService do
+RSpec.describe Projects::ImportService, feature_category: :importers do
   let!(:project) { create(:project) }
   let(:user) { project.creator }
 
diff --git a/spec/services/projects/protect_default_branch_service_spec.rb b/spec/services/projects/protect_default_branch_service_spec.rb
index c8aa421cdd4..9f9e89ff8f8 100644
--- a/spec/services/projects/protect_default_branch_service_spec.rb
+++ b/spec/services/projects/protect_default_branch_service_spec.rb
@@ -233,6 +233,38 @@ RSpec.describe Projects::ProtectDefaultBranchService do
     end
   end
 
+  describe '#protected_branch_exists?' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, group: group) }
+
+    let(:default_branch) { "default-branch" }
+
+    before do
+      allow(project).to receive(:default_branch).and_return(default_branch)
+      create(:protected_branch, project: nil, group: group, name: default_branch)
+    end
+
+    context 'when feature flag `group_protected_branches` disabled' do
+      before do
+        stub_feature_flags(group_protected_branches: false)
+      end
+
+      it 'return false' do
+        expect(service.protected_branch_exists?).to eq(false)
+      end
+    end
+
+    context 'when feature flag `group_protected_branches` enabled' do
+      before do
+        stub_feature_flags(group_protected_branches: true)
+      end
+
+      it 'return true' do
+        expect(service.protected_branch_exists?).to eq(true)
+      end
+    end
+  end
+
   describe '#default_branch' do
     it 'returns the default branch of the project' do
       allow(project)
diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index 5171836f917..32818535146 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -126,6 +126,12 @@ RSpec.describe Projects::TransferService do
       expect(project.namespace).to eq(user.namespace)
     end
 
+    it 'invalidates personal_project_count cache of the the owner of the personal namespace' do
+      expect(user).to receive(:invalidate_personal_projects_count)
+
+      execute_transfer
+    end
+
     context 'the owner of the namespace does not have a direct membership in the project residing in the group' do
       it 'creates a project membership record for the owner of the namespace, with OWNER access level, after the transfer' do
         execute_transfer
@@ -161,6 +167,17 @@ RSpec.describe Projects::TransferService do
     end
   end
 
+  context 'personal namespace -> group', :enable_admin_mode do
+    let(:executor) { create(:admin) }
+
+    it 'invalidates personal_project_count cache of the the owner of the personal namespace' \
+       'that previously held the project' do
+      expect(user).to receive(:invalidate_personal_projects_count)
+
+      execute_transfer
+    end
+  end
+
   context 'when transfer succeeds' do
     before do
       group.add_owner(user)
@@ -645,6 +662,8 @@ RSpec.describe Projects::TransferService do
     end
 
     it 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' do
+      stub_feature_flags(do_not_run_safety_net_auth_refresh_jobs: false)
+
       user_ids = [user.id, member_of_old_group.id, member_of_new_group.id].map { |id| [id] }
 
       expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to(