Merge branch '54781-store-disk-path-in-database-for-projects-on-legacy-storage' into 'master'

Storage location for projects on legacy storage

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

4 files changed, 112 insertions, 36 deletions

diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 18b54cce834..475fbe56e4d 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -337,32 +337,40 @@ describe Namespace do
       end
     end
 
-    it 'updates project full path in .git/config for each project inside namespace' do
-      parent = create(:group, name: 'mygroup', path: 'mygroup')
-      subgroup = create(:group, name: 'mysubgroup', path: 'mysubgroup', parent: parent)
-      project_in_parent_group = create(:project, :legacy_storage, :repository, namespace: parent, name: 'foo1')
-      hashed_project_in_subgroup = create(:project, :repository, namespace: subgroup, name: 'foo2')
-      legacy_project_in_subgroup = create(:project, :legacy_storage, :repository, namespace: subgroup, name: 'foo3')
-
-      parent.update(path: 'mygroup_new')
-
-      # Routes are loaded when creating the projects, so we need to manually
-      # reload them for the below code to be aware of the above UPDATE.
-      [
-        project_in_parent_group,
-        hashed_project_in_subgroup,
-        legacy_project_in_subgroup
-      ].each do |project|
-        project.route.reload
+    context 'for each project inside the namespace' do
+      let!(:parent) { create(:group, name: 'mygroup', path: 'mygroup') }
+      let!(:subgroup) { create(:group, name: 'mysubgroup', path: 'mysubgroup', parent: parent) }
+      let!(:project_in_parent_group) { create(:project, :legacy_storage, :repository, namespace: parent, name: 'foo1') }
+      let!(:hashed_project_in_subgroup) { create(:project, :repository, namespace: subgroup, name: 'foo2') }
+      let!(:legacy_project_in_subgroup) { create(:project, :legacy_storage, :repository, namespace: subgroup, name: 'foo3') }
+
+      it 'updates project full path in .git/config' do
+        parent.update(path: 'mygroup_new')
+
+        expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}"
+        expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}"
+        expect(project_rugged(legacy_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}"
       end
 
-      expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}"
-      expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}"
-      expect(project_rugged(legacy_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}"
-    end
+      it 'updates the project storage location' do
+        repository_project_in_parent_group = create(:project_repository, project: project_in_parent_group)
+        repository_hashed_project_in_subgroup = create(:project_repository, project: hashed_project_in_subgroup)
+        repository_legacy_project_in_subgroup = create(:project_repository, project: legacy_project_in_subgroup)
+
+        parent.update(path: 'mygroup_moved')
+
+        expect(repository_project_in_parent_group.reload.disk_path).to eq "mygroup_moved/#{project_in_parent_group.path}"
+        expect(repository_hashed_project_in_subgroup.reload.disk_path).to eq hashed_project_in_subgroup.disk_path
+        expect(repository_legacy_project_in_subgroup.reload.disk_path).to eq "mygroup_moved/mysubgroup/#{legacy_project_in_subgroup.path}"
+      end
+
+      def project_rugged(project)
+        # Routes are loaded when creating the projects, so we need to manually
+        # reload them for the below code to be aware of the above UPDATE.
+        project.route.reload
 
-    def project_rugged(project)
-      rugged_repo(project.repository)
+        rugged_repo(project.repository)
+      end
     end
   end
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index b95095a24cd..a01f76a5bab 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1651,26 +1651,54 @@ describe Project do
   end
 
   describe '#track_project_repository' do
-    let(:project) { create(:project, :repository) }
+    shared_examples 'tracks storage location' do
+      context 'when a project repository entry does not exist' do
+        it 'creates a new entry' do
+          expect { project.track_project_repository }.to change(project, :project_repository)
+        end
+
+        it 'tracks the project storage location' do
+          project.track_project_repository
 
-    it 'creates a project_repository' do
-      project.track_project_repository
+          expect(project.project_repository).to have_attributes(
+            disk_path: project.disk_path,
+            shard_name: project.repository_storage
+          )
+        end
+      end
+
+      context 'when a tracking entry exists' do
+        let!(:project_repository) { create(:project_repository, project: project) }
+        let!(:shard) { create(:shard, name: 'foo') }
+
+        it 'does not create a new entry in the database' do
+          expect { project.track_project_repository }.not_to change(project, :project_repository)
+        end
 
-      expect(project.reload.project_repository).to be_present
-      expect(project.project_repository.disk_path).to eq(project.disk_path)
-      expect(project.project_repository.shard_name).to eq(project.repository_storage)
+        it 'updates the project storage location' do
+          allow(project).to receive(:disk_path).and_return('fancy/new/path')
+          allow(project).to receive(:repository_storage).and_return('foo')
+
+          project.track_project_repository
+
+          expect(project.project_repository).to have_attributes(
+            disk_path: 'fancy/new/path',
+            shard_name: 'foo'
+          )
+        end
+      end
     end
 
-    it 'updates the project_repository' do
-      project.track_project_repository
+    context 'with projects on legacy storage' do
+      let(:project) { create(:project, :repository, :legacy_storage) }
 
-      allow(project).to receive(:disk_path).and_return('@fancy/new/path')
+      it_behaves_like 'tracks storage location'
+    end
 
-      expect do
-        project.track_project_repository
-      end.not_to change(ProjectRepository, :count)
+    context 'with projects on hashed storage' do
+      let(:project) { create(:project, :repository) }
 
-      expect(project.reload.project_repository.disk_path).to eq(project.disk_path)
+      it_behaves_like 'tracks storage location'
     end
   end
 
diff --git a/spec/services/projects/after_rename_service_spec.rb b/spec/services/projects/after_rename_service_spec.rb
index b4718a07204..59c08b30f9f 100644
--- a/spec/services/projects/after_rename_service_spec.rb
+++ b/spec/services/projects/after_rename_service_spec.rb
@@ -99,6 +99,17 @@ describe Projects::AfterRenameService do
 
         expect(rugged_config['gitlab.fullpath']).to eq(project.full_path)
       end
+
+      it 'updates storage location' do
+        allow(project_storage).to receive(:rename_repo).and_return(true)
+
+        described_class.new(project).execute
+
+        expect(project.project_repository).to have_attributes(
+          disk_path: project.disk_path,
+          shard_name: project.repository_storage
+        )
+      end
     end
 
     context 'using hashed storage' do
@@ -193,6 +204,15 @@ describe Projects::AfterRenameService do
 
         expect(rugged_config['gitlab.fullpath']).to eq(project.full_path)
       end
+
+      it 'updates storage location' do
+        described_class.new(project).execute
+
+        expect(project.project_repository).to have_attributes(
+          disk_path: project.disk_path,
+          shard_name: project.repository_storage
+        )
+      end
     end
   end
 end
diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index 132ad9a2646..766276fdba3 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -63,6 +63,15 @@ describe Projects::TransferService do
       expect(rugged_config['gitlab.fullpath']).to eq "#{group.full_path}/#{project.path}"
     end
 
+    it 'updates storage location' do
+      transfer_project(project, user, group)
+
+      expect(project.project_repository).to have_attributes(
+        disk_path: "#{group.full_path}/#{project.path}",
+        shard_name: project.repository_storage
+      )
+    end
+
     context 'new group has a kubernetes cluster' do
       let(:group_cluster) { create(:cluster, :group, :provided_by_gcp) }
       let(:group) { group_cluster.group }
@@ -139,6 +148,17 @@ describe Projects::TransferService do
         expect(service).not_to receive(:execute_system_hooks)
       end
     end
+
+    it 'does not update storage location' do
+      create(:project_repository, project: project)
+
+      attempt_project_transfer
+
+      expect(project.project_repository).to have_attributes(
+        disk_path: project.disk_path,
+        shard_name: project.repository_storage
+      )
+    end
   end
 
   context 'namespace -> no namespace' do