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

Get rid of namespace exist? calls entirely

Closes #33463

See merge request gitlab-org/gitlab!18611

lib/gitlab/gitaly_client/namespace_service.rb

@@ -7,12 +7,6 @@ module Gitlab
         @storage = storage
       end
 
-      def exists?(name)
-        request = Gitaly::NamespaceExistsRequest.new(storage_name: @storage, name: name)
-
-        gitaly_client_call(:namespace_exists, request, timeout: GitalyClient.fast_timeout).exists
-      end
-
       def add(name)
         request = Gitaly::AddNamespaceRequest.new(storage_name: @storage, name: name)
 

lib/gitlab/shell.rb

@@ -285,18 +285,6 @@ module Gitlab
       end
     end
 
-    # Check if such directory exists in repositories.
-    #
-    # Usage:
-    #   exists?(storage, 'gitlab')
-    #   exists?(storage, 'gitlab/cookies.git')
-    #
-    # rubocop: disable CodeReuse/ActiveRecord
-    def exists?(storage, dir_name)
-      Gitlab::GitalyClient::NamespaceService.new(storage).exists?(dir_name)
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
     def repository_exists?(storage, dir_name)
       Gitlab::Git::Repository.new(storage, dir_name, nil, nil).exists?
     rescue GRPC::Internal

spec/factories/projects.rb

@@ -234,10 +234,7 @@ FactoryBot.define do
 
     trait :broken_repo do
       after(:create) do |project|
-        raise "Failed to create repository!" unless project.create_repository
-
-        project.gitlab_shell.rm_directory(project.repository_storage,
-                                          File.join("#{project.disk_path}.git", 'refs'))
+        TestEnv.rm_storage_dir(project.repository_storage, "#{project.disk_path}.git/refs")
       end
     end
 

spec/lib/gitlab/bare_repository_import/importer_spec.rb

@@ -90,7 +90,7 @@ describe Gitlab::BareRepositoryImport::Importer, :seed_helper do
         hook_path = File.join(repo_path, 'hooks')
 
         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)
+        expect(TestEnv.storage_dir_exists?(project.repository_storage, hook_path)).to be(true)
       end
 
       context 'hashed storage enabled' do

spec/lib/gitlab/shell_spec.rb

@@ -310,18 +310,18 @@ describe Gitlab::Shell do
       let(:disk_path) { "#{project.disk_path}.git" }
 
       it 'returns true when the command succeeds' do
-        expect(gitlab_shell.exists?(project.repository_storage, disk_path)).to be(true)
+        expect(TestEnv.storage_dir_exists?(project.repository_storage, disk_path)).to be(true)
 
         expect(gitlab_shell.remove_repository(project.repository_storage, project.disk_path)).to be(true)
 
-        expect(gitlab_shell.exists?(project.repository_storage, disk_path)).to be(false)
+        expect(TestEnv.storage_dir_exists?(project.repository_storage, disk_path)).to be(false)
       end
 
       it 'keeps the namespace directory' do
         gitlab_shell.remove_repository(project.repository_storage, project.disk_path)
 
-        expect(gitlab_shell.exists?(project.repository_storage, disk_path)).to be(false)
-        expect(gitlab_shell.exists?(project.repository_storage, project.disk_path.gsub(project.name, ''))).to be(true)
+        expect(TestEnv.storage_dir_exists?(project.repository_storage, disk_path)).to be(false)
+        expect(TestEnv.storage_dir_exists?(project.repository_storage, project.disk_path.gsub(project.name, ''))).to be(true)
       end
     end
 
@@ -332,18 +332,18 @@ describe Gitlab::Shell do
         old_path = project2.disk_path
         new_path = "project/new_path"
 
-        expect(gitlab_shell.exists?(project2.repository_storage, "#{old_path}.git")).to be(true)
-        expect(gitlab_shell.exists?(project2.repository_storage, "#{new_path}.git")).to be(false)
+        expect(TestEnv.storage_dir_exists?(project2.repository_storage, "#{old_path}.git")).to be(true)
+        expect(TestEnv.storage_dir_exists?(project2.repository_storage, "#{new_path}.git")).to be(false)
 
         expect(gitlab_shell.mv_repository(project2.repository_storage, old_path, new_path)).to be_truthy
 
-        expect(gitlab_shell.exists?(project2.repository_storage, "#{old_path}.git")).to be(false)
-        expect(gitlab_shell.exists?(project2.repository_storage, "#{new_path}.git")).to be(true)
+        expect(TestEnv.storage_dir_exists?(project2.repository_storage, "#{old_path}.git")).to be(false)
+        expect(TestEnv.storage_dir_exists?(project2.repository_storage, "#{new_path}.git")).to be(true)
       end
 
       it 'returns false when the command fails' do
         expect(gitlab_shell.mv_repository(project2.repository_storage, project2.disk_path, '')).to be_falsy
-        expect(gitlab_shell.exists?(project2.repository_storage, "#{project2.disk_path}.git")).to be(true)
+        expect(TestEnv.storage_dir_exists?(project2.repository_storage, "#{project2.disk_path}.git")).to be(true)
       end
     end
 
@@ -403,47 +403,23 @@ describe Gitlab::Shell do
       it 'creates a namespace' do
         subject.add_namespace(storage, "mepmep")
 
-        expect(subject.exists?(storage, "mepmep")).to be(true)
+        expect(TestEnv.storage_dir_exists?(storage, "mepmep")).to be(true)
       end
     end
 
-    describe '#exists?' do
-      context 'when the namespace does not exist' do
+    describe '#repository_exists?' do
+      context 'when the repository does not exist' do
         it 'returns false' do
-          expect(subject.exists?(storage, "non-existing")).to be(false)
+          expect(subject.repository_exists?(storage, "non-existing.git")).to be(false)
         end
       end
 
-      context 'when the namespace exists' do
+      context 'when the repository exists' do
         it 'returns true' do
-          subject.add_namespace(storage, "mepmep")
-
-          expect(subject.exists?(storage, "mepmep")).to be(true)
-        end
-      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") }
+          project = create(:project, :repository, :legacy_storage)
 
-        it { is_expected.to be_falsey }
+          expect(subject.repository_exists?(storage, project.repository.disk_path + ".git")).to be(true)
         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
 
@@ -452,7 +428,7 @@ describe Gitlab::Shell do
         subject.add_namespace(storage, "mepmep")
         subject.rm_namespace(storage, "mepmep")
 
-        expect(subject.exists?(storage, "mepmep")).to be(false)
+        expect(TestEnv.storage_dir_exists?(storage, "mepmep")).to be(false)
       end
     end
 
@@ -461,8 +437,8 @@ describe Gitlab::Shell do
         subject.add_namespace(storage, "mepmep")
         subject.mv_namespace(storage, "mepmep", "2mep")
 
-        expect(subject.exists?(storage, "mepmep")).to be(false)
-        expect(subject.exists?(storage, "2mep")).to be(true)
+        expect(TestEnv.storage_dir_exists?(storage, "mepmep")).to be(false)
+        expect(TestEnv.storage_dir_exists?(storage, "2mep")).to be(true)
       end
     end
   end

spec/models/project_spec.rb

@@ -4284,22 +4284,25 @@ describe Project do
 
   describe '#check_repository_path_availability' do
     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 }
+      it 'returns false when repository already exists' do
+        expect(project.check_repository_path_availability).to be_falsey
+      end
     end
 
     context 'when the repository does not exist' do
-      it { is_expected.to be_truthy }
+      it 'returns false when repository already exists' do
+        expect(project.check_repository_path_availability).to be_truthy
+      end
 
       it 'skips gitlab-shell exists?' do
         project.skip_disk_validation = true
 
         expect(project.gitlab_shell).not_to receive(:repository_exists?)
-        is_expected.to be_truthy
+        expect(project.check_repository_path_availability).to be_truthy
       end
     end
   end

spec/services/groups/destroy_service_spec.rb

@@ -55,8 +55,8 @@ describe Groups::DestroyService do
         end
 
         it 'verifies that paths have been deleted' do
-          expect(gitlab_shell.exists?(project.repository_storage, group.path)).to be_falsey
-          expect(gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey
+          expect(TestEnv.storage_dir_exists?(project.repository_storage, group.path)).to be_falsey
+          expect(TestEnv.storage_dir_exists?(project.repository_storage, remove_path)).to be_falsey
         end
       end
     end
@@ -73,13 +73,13 @@ describe Groups::DestroyService do
 
       after do
         # Clean up stale directories
-        gitlab_shell.rm_namespace(project.repository_storage, group.path)
-        gitlab_shell.rm_namespace(project.repository_storage, remove_path)
+        TestEnv.rm_storage_dir(project.repository_storage, group.path)
+        TestEnv.rm_storage_dir(project.repository_storage, remove_path)
       end
 
       it 'verifies original paths and projects still exist' do
-        expect(gitlab_shell.exists?(project.repository_storage, group.path)).to be_truthy
-        expect(gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey
+        expect(TestEnv.storage_dir_exists?(project.repository_storage, group.path)).to be_truthy
+        expect(TestEnv.storage_dir_exists?(project.repository_storage, remove_path)).to be_falsey
         expect(Project.unscoped.count).to eq(1)
         expect(Group.unscoped.count).to eq(2)
       end

spec/services/projects/after_rename_service_spec.rb

@@ -222,7 +222,7 @@ describe Projects::AfterRenameService do
 
   def expect_repository_exist(full_path_with_extension)
     expect(
-      gitlab_shell.exists?(
+      TestEnv.storage_dir_exists?(
         project.repository_storage,
         full_path_with_extension
       )

spec/services/projects/destroy_service_spec.rb

@@ -346,21 +346,21 @@ describe Projects::DestroyService do
     let(:path) { project.disk_path + '.git' }
 
     before do
-      expect(project.gitlab_shell.exists?(project.repository_storage, path)).to be_truthy
-      expect(project.gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey
+      expect(TestEnv.storage_dir_exists?(project.repository_storage, path)).to be_truthy
+      expect(TestEnv.storage_dir_exists?(project.repository_storage, remove_path)).to be_falsey
 
       # Dont run sidekiq to check if renamed repository exists
       Sidekiq::Testing.fake! { destroy_project(project, user, {}) }
 
-      expect(project.gitlab_shell.exists?(project.repository_storage, path)).to be_falsey
-      expect(project.gitlab_shell.exists?(project.repository_storage, remove_path)).to be_truthy
+      expect(TestEnv.storage_dir_exists?(project.repository_storage, path)).to be_falsey
+      expect(TestEnv.storage_dir_exists?(project.repository_storage, remove_path)).to be_truthy
     end
 
     it 'restores the repositories' do
       Sidekiq::Testing.fake! { described_class.new(project, user).attempt_repositories_rollback }
 
-      expect(project.gitlab_shell.exists?(project.repository_storage, path)).to be_truthy
-      expect(project.gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey
+      expect(TestEnv.storage_dir_exists?(project.repository_storage, path)).to be_truthy
+      expect(TestEnv.storage_dir_exists?(project.repository_storage, remove_path)).to be_falsey
     end
   end
 

spec/services/projects/update_service_spec.rb

@@ -151,7 +151,7 @@ describe Projects::UpdateService do
 
     context 'when we update project but not enabling a wiki' do
       it 'does not try to create an empty wiki' do
-        Gitlab::Shell.new.rm_directory(project.repository_storage, project.wiki.path)
+        TestEnv.rm_storage_dir(project.repository_storage, project.wiki.path)
 
         result = update_project(project, user, { name: 'test1' })
 
@@ -172,7 +172,7 @@ describe Projects::UpdateService do
     context 'when enabling a wiki' do
       it 'creates a wiki' do
         project.project_feature.update(wiki_access_level: ProjectFeature::DISABLED)
-        Gitlab::Shell.new.rm_directory(project.repository_storage, project.wiki.path)
+        TestEnv.rm_storage_dir(project.repository_storage, project.wiki.path)
 
         result = update_project(project, user, project_feature_attributes: { wiki_access_level: ProjectFeature::ENABLED })
 

spec/support/helpers/test_env.rb

@@ -243,6 +243,22 @@ module TestEnv
     FileUtils.chmod_R 0755, target_repo_path
   end
 
+  def rm_storage_dir(storage, dir)
+    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      repos_path = Gitlab.config.repositories.storages[storage].legacy_disk_path
+      target_repo_refs_path = File.join(repos_path, dir)
+      FileUtils.remove_dir(target_repo_refs_path)
+    end
+  rescue Errno::ENOENT
+  end
+
+  def storage_dir_exists?(storage, dir)
+    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      repos_path = Gitlab.config.repositories.storages[storage].legacy_disk_path
+      File.exist?(File.join(repos_path, dir))
+    end
+  end
+
   def create_bare_repository(path)
     FileUtils.mkdir_p(path)
 

spec/workers/repository_check/single_repository_worker_spec.rb

@@ -68,7 +68,7 @@ describe RepositoryCheck::SingleRepositoryWorker do
 
   it 'creates missing wikis' do
     project = create(:project, :wiki_enabled)
-    Gitlab::Shell.new.rm_directory(project.repository_storage, project.wiki.path)
+    TestEnv.rm_storage_dir(project.repository_storage, project.wiki.path)
 
     subject.perform(project.id)
 
@@ -77,12 +77,12 @@ describe RepositoryCheck::SingleRepositoryWorker do
 
   it 'does not create a wiki if the main repo does not exist at all' do
     project = create(:project, :repository)
-    Gitlab::Shell.new.rm_directory(project.repository_storage, project.path)
-    Gitlab::Shell.new.rm_directory(project.repository_storage, project.wiki.path)
+    TestEnv.rm_storage_dir(project.repository_storage, project.path)
+    TestEnv.rm_storage_dir(project.repository_storage, project.wiki.path)
 
     subject.perform(project.id)
 
-    expect(Gitlab::Shell.new.exists?(project.repository_storage, project.wiki.path)).to eq(false)
+    expect(TestEnv.storage_dir_exists?(project.repository_storage, project.wiki.path)).to eq(false)
   end
 
   def create_push_event(project)