Merge branch 'sh-fix-backfill-project-repo-migration' into 'master'

Fix duplicate project disk path in BackfillLegacyProjectRepositories

Closes #56061

See merge request gitlab-org/gitlab-ce!24213

4 files changed, 33 insertions, 6 deletions

diff --git a/changelogs/unreleased/sh-fix-backfill-project-repo-migration.yml b/changelogs/unreleased/sh-fix-backfill-project-repo-migration.yml
new file mode 100644
index 00000000000..d1d4412eb50
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-backfill-project-repo-migration.yml
@@ -0,0 +1,5 @@
+---
+title: Fix duplicate project disk path in BackfillLegacyProjectRepositories
+merge_request: 24213
+author:
+type: changed
diff --git a/lib/gitlab/background_migration/backfill_project_repositories.rb b/lib/gitlab/background_migration/backfill_project_repositories.rb
index aaf520d70f6..c8d83cc1803 100644
--- a/lib/gitlab/background_migration/backfill_project_repositories.rb
+++ b/lib/gitlab/background_migration/backfill_project_repositories.rb
@@ -83,7 +83,7 @@ module Gitlab
         extend ActiveSupport::Concern
 
         def full_path
-          @full_path ||= build_full_path
+          route&.path || build_full_path
         end
 
         def build_full_path
@@ -99,7 +99,12 @@ module Gitlab
         end
       end
 
-      # Namespace model.
+      # Route model
+      class Route < ActiveRecord::Base
+        belongs_to :source, inverse_of: :route, polymorphic: true
+      end
+
+      # Namespace model
       class Namespace < ActiveRecord::Base
         self.table_name = 'namespaces'
         self.inheritance_column = nil
@@ -108,6 +113,8 @@ module Gitlab
 
         belongs_to :parent, class_name: 'Namespace', inverse_of: 'namespaces'
 
+        has_one :route, -> { where(source_type: 'Namespace') }, inverse_of: :source, foreign_key: :source_id
+
         has_many :projects, inverse_of: :parent
         has_many :namespaces, inverse_of: :parent
       end
@@ -134,6 +141,7 @@ module Gitlab
 
         belongs_to :parent, class_name: 'Namespace', foreign_key: :namespace_id, inverse_of: 'projects'
 
+        has_one :route, -> { where(source_type: 'Project') }, inverse_of: :source, foreign_key: :source_id
         has_one :project_repository, inverse_of: :project
 
         delegate :disk_path, to: :storage
@@ -194,6 +202,8 @@ module Gitlab
       def project_repositories(start_id, stop_id)
         projects
           .without_project_repository
+          .includes(:route, parent: [:route]).references(:routes)
+          .includes(:parent).references(:namespaces)
           .where(id: start_id..stop_id)
           .map { |project| build_attributes_for_project(project) }
           .compact
diff --git a/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb
index 53c071f0268..f4a6f4be754 100644
--- a/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb
+++ b/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb
@@ -34,6 +34,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
     let!(:project_hashed_storage_2) { create(:project, name: 'bar', path: 'bar', namespace: group, storage_version: 2) }
     let!(:project_legacy_storage_3) { create(:project, name: 'baz', path: 'baz', namespace: group, storage_version: 0) }
     let!(:project_legacy_storage_4) { create(:project, name: 'zoo', path: 'zoo', namespace: group, storage_version: nil) }
+    let!(:project_legacy_storage_5) { create(:project, name: 'test', path: 'test', namespace: group, storage_version: nil) }
 
     describe '.on_hashed_storage' do
       it 'finds projects with repository on hashed storage' do
@@ -47,7 +48,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
       it 'finds projects with repository on legacy storage' do
         projects = described_class.on_legacy_storage.pluck(:id)
 
-        expect(projects).to match_array([project_legacy_storage_3.id, project_legacy_storage_4.id])
+        expect(projects).to match_array([project_legacy_storage_3.id, project_legacy_storage_4.id, project_legacy_storage_5.id])
       end
     end
 
@@ -58,7 +59,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
 
         projects = described_class.without_project_repository.pluck(:id)
 
-        expect(projects).to contain_exactly(project_hashed_storage_2.id, project_legacy_storage_4.id)
+        expect(projects).to contain_exactly(project_hashed_storage_2.id, project_legacy_storage_4.id, project_legacy_storage_5.id)
       end
     end
 
@@ -78,14 +79,23 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
           expect(project.disk_path).to eq(project_legacy_storage_3.disk_path)
         end
 
+        it 'returns the correct disk_path using the route entry' do
+          project_legacy_storage_5.route.update(path: 'zoo/new-test')
+          project = described_class.find(project_legacy_storage_5.id)
+
+          expect(project.disk_path).to eq('zoo/new-test')
+        end
+
         it 'raises OrphanedNamespaceError when any parent namespace does not exist' do
           subgroup = create(:group, parent: group)
           project_orphaned_namespace = create(:project, name: 'baz', path: 'baz', namespace: subgroup, storage_version: nil)
           subgroup.update_column(:parent_id, Namespace.maximum(:id).succ)
 
           project = described_class.find(project_orphaned_namespace.id)
+          project.route.destroy
+          subgroup.route.destroy
 
-          expect { project.disk_path }
+          expect { project.reload.disk_path }
             .to raise_error(Gitlab::BackgroundMigration::BackfillProjectRepositories::OrphanedNamespaceError)
         end
       end
diff --git a/spec/support/shared_examples/lib/gitlab/background_migration/backfill_project_repositories_examples.rb b/spec/support/shared_examples/lib/gitlab/background_migration/backfill_project_repositories_examples.rb
index 1f688c0f9d3..dcf7c1a90c2 100644
--- a/spec/support/shared_examples/lib/gitlab/background_migration/backfill_project_repositories_examples.rb
+++ b/spec/support/shared_examples/lib/gitlab/background_migration/backfill_project_repositories_examples.rb
@@ -32,11 +32,13 @@ shared_examples 'backfill migration for project repositories' do |storage|
 
     it 'inserts rows in a single query' do
       projects.create!(name: 'foo', path: 'foo', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
+      group2 = namespaces.create!(name: 'gro', path: 'gro')
 
       control_count = ActiveRecord::QueryRecorder.new { described_class.new.perform(1, projects.last.id) }
 
       projects.create!(name: 'bar', path: 'bar', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
-      projects.create!(name: 'zoo', path: 'zoo', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
+      projects.create!(name: 'top', path: 'top', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
+      projects.create!(name: 'zoo', path: 'zoo', namespace_id: group2.id, storage_version: storage_version, repository_storage: shard.name)
 
       expect { described_class.new.perform(1, projects.last.id) }.not_to exceed_query_limit(control_count)
     end