From 188d8bbfea9fd32e076cda89af08d21c15e62315 Mon Sep 17 00:00:00 2001 From: Aakriti Gupta Date: Fri, 18 Jul 2025 13:29:35 +0200 Subject: [PATCH] Create project_repository records only when Git repositories exist --- app/models/project.rb | 9 +++++- app/services/projects/transfer_service.rb | 2 +- .../update_repository_storage_service.rb | 2 +- ee/spec/lib/gitlab/git_access_spec.rb | 1 + .../projects/branches_controller_spec.rb | 4 +++ spec/factories/projects.rb | 2 ++ .../gitlab/repository_cache/preloader_spec.rb | 8 +++++ spec/models/project_repository_spec.rb | 3 +- spec/models/project_spec.rb | 32 +++++++++---------- spec/models/repository_spec.rb | 6 ++++ spec/services/projects/create_service_spec.rb | 6 ++++ .../update_repository_storage_service_spec.rb | 7 ++-- 12 files changed, 58 insertions(+), 24 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 2d41c769cb9560..e43bf2aec8173f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2289,7 +2289,14 @@ def check_repository_path_availability false end - def track_project_repository + # The parameter 'in_transit' is used when we want to update project_repository + # but the Git repository has been moved. + # This occurs during project transfer to another namespace, in Projects::TransferService + # or when repository storage is being updated in Projects::UpdateRepositoryStorageService + + def track_project_repository(in_transit: false) + return unless in_transit || repository_exists? + (project_repository || build_project_repository).tap do |proj_repo| attributes = { shard_name: repository_storage, disk_path: disk_path } diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index b3b8898a261d71..809bfe32c63635 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -235,7 +235,7 @@ def update_namespace_and_visibility(to_namespace) end def update_repository_configuration - project.track_project_repository + project.track_project_repository(in_transit: true) end def ensure_personal_project_owner_membership(project) diff --git a/app/services/projects/update_repository_storage_service.rb b/app/services/projects/update_repository_storage_service.rb index 41f62858786137..3d6e1a6af5f1cd 100644 --- a/app/services/projects/update_repository_storage_service.rb +++ b/app/services/projects/update_repository_storage_service.rb @@ -15,7 +15,7 @@ def track_repository(destination_storage_name) project.swap_pool_repository! # Connect project to the repository from the new shard - project.track_project_repository + project.track_project_repository(in_transit: true) # Link repository from the new shard to pool repository from the new shard project.link_pool_repository diff --git a/ee/spec/lib/gitlab/git_access_spec.rb b/ee/spec/lib/gitlab/git_access_spec.rb index 24bd28ccda9880..36f7f9c5a334fd 100644 --- a/ee/spec/lib/gitlab/git_access_spec.rb +++ b/ee/spec/lib/gitlab/git_access_spec.rb @@ -573,6 +573,7 @@ def download_ability describe 'Geo' do let_it_be(:primary_node) { create(:geo_node, :primary) } let_it_be(:secondary_node) { create(:geo_node) } + let_it_be(:project) { create(:project, :repository) } context 'git pull' do let(:actor) { :geo } diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index 7ae635c38fa29c..5888d292daca6c 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -8,6 +8,10 @@ let(:developer) { create(:user) } before do + # Stub calls to Project#repository_exists? to avoid extra calls to Gitlab::GitalyClient + # that affect caching tests + allow_any_instance_of(Project).to receive(:repository_exists?).and_return(true) + project.add_developer(developer) allow(project).to receive(:branches).and_return(['master', 'foo/bar/baz']) diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 9c454793ec99b1..740912164e4eca 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -322,6 +322,8 @@ branch_name: project.default_branch || 'master' ) end + + project.track_project_repository end end diff --git a/spec/lib/gitlab/repository_cache/preloader_spec.rb b/spec/lib/gitlab/repository_cache/preloader_spec.rb index 7683feb502d51a..2d59bc4acac6e4 100644 --- a/spec/lib/gitlab/repository_cache/preloader_spec.rb +++ b/spec/lib/gitlab/repository_cache/preloader_spec.rb @@ -40,6 +40,14 @@ end context 'when values are not cached' do + before do + # Avoid an extra call to repository.exists? + # during project creation + allow_next_instance_of(Project) do |p| + allow(p).to receive(:track_project_repository).and_return(true) + end + end + it 'reads and writes from cache individually' do described_class.new(repositories).preload( %i[exists? has_visible_content?] diff --git a/spec/models/project_repository_spec.rb b/spec/models/project_repository_spec.rb index 1ca5ae9948d900..12af5c95d72a4c 100644 --- a/spec/models/project_repository_spec.rb +++ b/spec/models/project_repository_spec.rb @@ -15,8 +15,7 @@ describe '.find_project' do it 'finds project by disk path' do - project = create(:project) - project.track_project_repository + project = create(:project_with_repo) expect(described_class.find_project(project.disk_path)).to eq(project) end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8f0cda0bc76743..598c5567df3241 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3891,14 +3891,9 @@ def create_project_statistics_with_size(project, size) context 'when repository is missing' do let(:project) { create(:project) } - it 'sets a default sha1 object format' do - project.track_project_repository - - expect(project.project_repository).to have_attributes( - disk_path: project.disk_path, - shard_name: project.repository_storage, - object_format: 'sha1' - ) + it 'does not create a project_repository record' do + expect { project.track_project_repository }.not_to change(project, :project_repository) + expect(project.project_repository).to be_nil end end @@ -10012,22 +10007,25 @@ def create_hook describe '#repository_object_format' do subject { project.repository_object_format } - let_it_be(:project) { create(:project) } - context 'when project without a repository' do + let_it_be(:project) { create(:project) } + it { is_expected.to be_nil } end - context 'when project with sha1 repository' do - let_it_be(:project_repository) { create(:project_repository, project: project, object_format: 'sha1') } + context 'when project with a repository' do + context 'when project with sha1 repository' do + # :custom_repo uses sha1 object format by default + let_it_be(:project) { create(:project, :custom_repo) } - it { is_expected.to eq 'sha1' } - end + it { is_expected.to eq 'sha1' } + end - context 'when project with sha256 repository' do - let_it_be(:project_repository) { create(:project_repository, project: project, object_format: 'sha256') } + context 'when project with sha256 repository' do + let_it_be(:project) { create(:project, :custom_repo, object_format: 'sha256') } - it { is_expected.to eq 'sha256' } + it { is_expected.to eq 'sha256' } + end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 29440b58331f63..5d0717adca59be 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1960,6 +1960,12 @@ def expect_to_raise_storage_error describe '#exists?' do let(:project) { create(:project, :repository) } + before do + allow_next_instance_of(Project) do |p| + allow(p).to receive(:track_project_repository).and_return(true) + end + end + it 'returns true when a repository exists' do expect(repository.exists?).to be(true) end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index c853bc654f200f..3a2843172aaa14 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -195,6 +195,12 @@ create_project(user, opts) end + it 'creates a project_repository record' do + project = create_project(user, opts) + + expect(project.project_repository).to be_persisted + end + it 'creates associated project settings' do project = create_project(user, opts) diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index d6597e9e9fcc64..d091bfab8676fd 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -281,12 +281,16 @@ before do project.repository.remove + project.project_repository&.destroy! end it 'does not mirror object pool' do expect(project.pool_repository.shard).to eq(shard_source) - subject.execute + begin + subject.execute + rescue Gitlab::Git::Repository::NoRepository + end expect(project.pool_repository.shard).to eq(shard_source) end @@ -411,7 +415,6 @@ expect(result).to be_success expect(project).not_to be_repository_read_only expect(project.repository_storage).to eq(storage_destination) - expect(project.project_repository.shard_name).to eq(storage_destination) end end -- GitLab