Merge branch 'jc-remove-namespace-exists-calls' into 'master'

Migrate namespace exist to repository exist calls

See merge request gitlab-org/gitlab!18146

app/models/project.rb

@@ -2312,7 +2312,7 @@ class Project < ApplicationRecord
   end
 
   def repository_with_same_path_already_exists?
-    gitlab_shell.exists?(repository_storage, "#{disk_path}.git")
+    gitlab_shell.repository_exists?(repository_storage, "#{disk_path}.git")
   end
 
   def set_timestamps_for_create

app/services/projects/destroy_service.rb

@@ -123,11 +123,9 @@ module Projects
       mv_repository(old_path, new_path)
     end
 
-    # rubocop: disable CodeReuse/ActiveRecord
     def repo_exists?(path)
-      gitlab_shell.exists?(project.repository_storage, path + '.git')
+      gitlab_shell.repository_exists?(project.repository_storage, path + '.git')
     end
-    # rubocop: enable CodeReuse/ActiveRecord
 
     def mv_repository(from_path, to_path)
       return true unless repo_exists?(from_path)

app/services/projects/hashed_storage/base_repository_service.rb

@@ -20,16 +20,13 @@ module Projects
 
       protected
 
-      # rubocop: disable CodeReuse/ActiveRecord
       def has_wiki?
-        gitlab_shell.exists?(project.repository_storage, "#{old_wiki_disk_path}.git")
+        gitlab_shell.repository_exists?(project.repository_storage, "#{old_wiki_disk_path}.git")
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
-      # rubocop: disable CodeReuse/ActiveRecord
       def move_repository(from_name, to_name)
-        from_exists = gitlab_shell.exists?(project.repository_storage, "#{from_name}.git")
-        to_exists = gitlab_shell.exists?(project.repository_storage, "#{to_name}.git")
+        from_exists = gitlab_shell.repository_exists?(project.repository_storage, "#{from_name}.git")
+        to_exists = gitlab_shell.repository_exists?(project.repository_storage, "#{to_name}.git")
 
         # If we don't find the repository on either original or target we should log that as it could be an issue if the
         # project was not originally empty.
@@ -46,7 +43,6 @@ module Projects
 
         gitlab_shell.mv_repository(project.repository_storage, from_name, to_name)
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       def rollback_folder_move
         move_repository(new_disk_path, old_disk_path)

ee/app/services/geo/repository_base_sync_service.rb

@@ -207,15 +207,13 @@ module Geo
       @temp_repo ||= ::Repository.new(repository.full_path, repository.project, disk_path: disk_path_temp, repo_type: repository.repo_type)
     end
 
-    # rubocop: disable CodeReuse/ActiveRecord
     def clean_up_temporary_repository
-      exists = gitlab_shell.exists?(project.repository_storage, disk_path_temp + '.git')
+      exists = gitlab_shell.repository_exists?(project.repository_storage, disk_path_temp + '.git')
 
       if exists && !gitlab_shell.remove_repository(project.repository_storage, disk_path_temp)
         raise Gitlab::Shell::Error, "Temporary #{type} can not be removed"
       end
     end
-    # rubocop: enable CodeReuse/ActiveRecord
 
     def set_temp_repository_as_main
       log_info(

ee/spec/services/geo/repository_destroy_service_spec.rb

@@ -37,11 +37,11 @@ describe Geo::RepositoryDestroyService do
       it 'removes the repository from disk' do
         project.delete
 
-        expect(project.gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
+        expect(project.gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
 
         service.execute
 
-        expect(project.gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
+        expect(project.gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
       end
 
       it 'cleans up deleted repositories' do
@@ -75,11 +75,11 @@ describe Geo::RepositoryDestroyService do
       it 'removes the repository from disk' do
         project.delete
 
-        expect(project.gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
+        expect(project.gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
 
         service.execute
 
-        expect(project.gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
+        expect(project.gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
       end
 
       it 'cleans up deleted repositories' do

ee/spec/services/geo/repository_sync_service_spec.rb

@@ -359,7 +359,7 @@ describe Geo::RepositorySyncService do
 
         expect(subject).to receive(:fetch_geo_mirror)
         expect(subject).to receive(:clean_up_temporary_repository).twice.and_call_original
-        expect(subject.gitlab_shell).to receive(:exists?).twice.with(project.repository_storage, /.git$/)
+        expect(subject.gitlab_shell).to receive(:repository_exists?).twice.with(project.repository_storage, /.git$/)
 
         subject.execute
       end

ee/spec/services/projects/update_repository_storage_service_spec.rb

@@ -29,7 +29,7 @@ describe Projects::UpdateRepositoryStorageService do
           subject.execute('test_second_storage')
           expect(project).not_to be_repository_read_only
           expect(project.repository_storage).to eq('test_second_storage')
-          expect(gitlab_shell.exists?('default', old_path)).to be(false)
+          expect(gitlab_shell.repository_exists?('default', old_path)).to be(false)
           expect(project.project_repository.shard_name).to eq('test_second_storage')
         end
       end
@@ -95,8 +95,8 @@ describe Projects::UpdateRepositoryStorageService do
 
           expect(project).not_to be_repository_read_only
           expect(project.repository_storage).to eq('test_second_storage')
-          expect(gitlab_shell.exists?('default', old_path)).to be(false)
-          expect(gitlab_shell.exists?('default', old_wiki_path)).to be(false)
+          expect(gitlab_shell.repository_exists?('default', old_path)).to be(false)
+          expect(gitlab_shell.repository_exists?('default', old_wiki_path)).to be(false)
         end
       end
 

ee/spec/support/services/base_sync_service_shared_examples.rb

@@ -42,7 +42,7 @@ shared_examples 'cleans temporary repositories' do
       allow(subject).to receive(:gitlab_shell).and_return(gitlab_shell)
       allow(subject).to receive(:fetch_geo_mirror)
 
-      expect(gitlab_shell).to receive(:exists?).and_return(true)
+      expect(gitlab_shell).to receive(:repository_exists?).and_return(true)
       expect(gitlab_shell).to receive(:remove_repository).with(project.repository_storage, temp_repo_path)
 
       subject.execute

lib/gitlab/shell.rb

@@ -297,6 +297,12 @@ module Gitlab
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
+    def repository_exists?(storage, dir_name)
+      Gitlab::Git::Repository.new(storage, dir_name, nil, nil).exists?
+    rescue GRPC::Internal
+      false
+    end
+
     def hooks_path
       File.join(gitlab_shell_path, 'hooks')
     end

spec/controllers/profiles_controller_spec.rb

@@ -153,7 +153,7 @@ describe ProfilesController, :request_store do
         user.reload
 
         expect(response.status).to eq(302)
-        expect(gitlab_shell.exists?(project.repository_storage, "#{new_username}/#{project.path}.git")).to be_truthy
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_username}/#{project.path}.git")).to be_truthy
       end
     end
 
@@ -171,7 +171,7 @@ describe ProfilesController, :request_store do
         user.reload
 
         expect(response.status).to eq(302)
-        expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
         expect(before_disk_path).to eq(project.disk_path)
       end
     end

spec/lib/gitlab/bare_repository_import/importer_spec.rb

@@ -89,7 +89,7 @@ describe Gitlab::BareRepositoryImport::Importer, :seed_helper do
         repo_path = "#{project.disk_path}.git"
         hook_path = File.join(repo_path, 'hooks')
 
-        expect(gitlab_shell.exists?(project.repository_storage, repo_path)).to be(true)
+        expect(gitlab_shell.repository_exists?(project.repository_storage, repo_path)).to be(true)
         expect(gitlab_shell.exists?(project.repository_storage, hook_path)).to be(true)
       end
 
@@ -145,8 +145,8 @@ describe Gitlab::BareRepositoryImport::Importer, :seed_helper do
 
       project = Project.find_by_full_path("#{admin.full_path}/#{project_path}")
 
-      expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.git')).to be(true)
-      expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true)
+      expect(gitlab_shell.repository_exists?(project.repository_storage, project.disk_path + '.git')).to be(true)
+      expect(gitlab_shell.repository_exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true)
     end
 
     context 'with a repository already on disk' do
@@ -186,7 +186,7 @@ describe Gitlab::BareRepositoryImport::Importer, :seed_helper do
 
       project = Project.find_by_full_path(project_path)
 
-      expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true)
+      expect(gitlab_shell.repository_exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true)
     end
   end
 

spec/lib/gitlab/shell_spec.rb

@@ -422,6 +422,30 @@ describe Gitlab::Shell do
       end
     end
 
+    describe '#repository_exists?' do
+      context 'when the storage path does not exist' do
+        subject { described_class.new.repository_exists?(storage, "non-existing.git") }
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'when the repository does not exist' do
+        let(:project) { create(:project, :repository, :legacy_storage) }
+
+        subject { described_class.new.repository_exists?(storage, "#{project.repository.disk_path}-some-other-repo.git") }
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'when the repository exists' do
+        let(:project) { create(:project, :repository, :legacy_storage) }
+
+        subject { described_class.new.repository_exists?(storage, "#{project.repository.disk_path}.git") }
+
+        it { is_expected.to be_truthy }
+      end
+    end
+
     describe '#remove' do
       it 'removes the namespace' do
         subject.add_namespace(storage, "mepmep")

spec/models/namespace_spec.rb

@@ -250,7 +250,7 @@ describe Namespace do
       it "moves dir if path changed" do
         namespace.update(path: namespace.full_path + '_new')
 
-        expect(gitlab_shell.exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy
       end
 
       context 'when #write_projects_repository_config raises an error' do
@@ -358,7 +358,7 @@ describe Namespace do
         namespace.update(path: namespace.full_path + '_new')
 
         expect(before_disk_path).to eq(project.disk_path)
-        expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
       end
     end
 

spec/models/project_spec.rb

@@ -4192,13 +4192,24 @@ describe Project do
   end
 
   describe '#check_repository_path_availability' do
-    let(:project) { build(:project) }
+    let(:project) { build(:project, :repository, :legacy_storage) }
+    subject { project.check_repository_path_availability }
+
+    context 'when the repository already exists' do
+      let(:project) { create(:project, :repository, :legacy_storage) }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when the repository does not exist' do
+      it { is_expected.to be_truthy }
 
       it 'skips gitlab-shell exists?' do
         project.skip_disk_validation = true
 
-      expect(project.gitlab_shell).not_to receive(:exists?)
-      expect(project.check_repository_path_availability).to be_truthy
+        expect(project.gitlab_shell).not_to receive(:repository_exists?)
+        is_expected.to be_truthy
+      end
     end
   end
 

spec/services/groups/destroy_service_spec.rb

@@ -119,7 +119,7 @@ describe Groups::DestroyService do
       let!(:project) { create(:project, :legacy_storage, :empty_repo, namespace: group) }
 
       it 'removes repository' do
-        expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
       end
     end
 
@@ -127,7 +127,7 @@ describe Groups::DestroyService do
       let!(:project) { create(:project, :empty_repo, namespace: group) }
 
       it 'removes repository' do
-        expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
       end
     end
   end

spec/services/projects/destroy_service_spec.rb

@@ -24,8 +24,8 @@ describe Projects::DestroyService do
     it 'deletes the project' do
       expect(Project.unscoped.all).not_to include(project)
 
-      expect(project.gitlab_shell.exists?(project.repository_storage, path + '.git')).to be_falsey
-      expect(project.gitlab_shell.exists?(project.repository_storage, remove_path + '.git')).to be_falsey
+      expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_falsey
+      expect(project.gitlab_shell.repository_exists?(project.repository_storage, remove_path + '.git')).to be_falsey
     end
   end
 

spec/services/projects/hashed_storage/migrate_repository_service_spec.rb

@@ -48,8 +48,8 @@ describe Projects::HashedStorage::MigrateRepositoryService do
       it 'renames project and wiki repositories' do
         service.execute
 
-        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy
-        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy
       end
 
       it 'updates project to be hashed and not read-only' do
@@ -84,8 +84,8 @@ describe Projects::HashedStorage::MigrateRepositoryService do
 
         service.execute
 
-        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey
-        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey
         expect(project.repository_read_only?).to be_falsey
       end
 

spec/services/projects/hashed_storage/rollback_repository_service_spec.rb

@@ -48,8 +48,8 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis
       it 'renames project and wiki repositories' do
         service.execute
 
-        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy
-        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy
       end
 
       it 'updates project to be legacy and not read-only' do
@@ -84,8 +84,8 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis
 
         service.execute
 
-        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey
-        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey
+        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey
         expect(project.repository_read_only?).to be_falsey
       end
 

spec/services/projects/transfer_service_spec.rb

@@ -103,7 +103,7 @@ describe Projects::TransferService do
     it 'rolls back repo location' do
       attempt_project_transfer
 
-      expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be(true)
+      expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be(true)
       expect(original_path).to eq current_path
     end
 

spec/services/users/destroy_service_spec.rb

@@ -183,7 +183,7 @@ describe Users::DestroyService do
           let!(:project) { create(:project, :empty_repo, :legacy_storage, namespace: user.namespace) }
 
           it 'removes repository' do
-            expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
+            expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
           end
         end
 
@@ -191,7 +191,7 @@ describe Users::DestroyService do
           let!(:project) { create(:project, :empty_repo, namespace: user.namespace) }
 
           it 'removes repository' do
-            expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
+            expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
           end
         end
       end