Merge branch '329538-rename-delayed-deletion-project' into 'master'

Rename projects set for delayed deletion

See merge request gitlab-org/gitlab!76675

ee/app/services/projects/mark_for_deletion_service.rb

@@ -10,6 +10,8 @@ module Projects
         project,
         current_user,
         { archived: true,
+          name: "#{project.name}-deleted-#{project.id}",
+          path: "#{project.path}-deleted-#{project.id}",
           marked_for_deletion_at: Time.current.utc,
           deleting_user: current_user }
       ).execute

ee/app/services/projects/restore_service.rb

@@ -2,6 +2,10 @@
 
 module Projects
   class RestoreService < BaseService
+    include Gitlab::Utils::StrongMemoize
+
+    DELETED_SUFFIX_REGEX = /-deleted-[a-zA-Z0-9]+\z/.freeze
+
     def execute
       return error(_('Project already deleted')) if project.pending_delete?
 
@@ -10,7 +14,9 @@ module Projects
         current_user,
         { archived: false,
           marked_for_deletion_at: nil,
-          deleting_user: nil }
+          deleting_user: nil,
+          name: updated_value(project.name),
+          path: updated_value(project.path) }
       ).execute
       log_event if result[:status] == :success
 
@@ -30,5 +36,27 @@ module Projects
         custom_message: "Project restored"
       ).for_project.security_event
     end
+
+    private
+
+    def suffix
+      strong_memoize(:suffix) do
+        original_path_taken?(project) ? "-#{SecureRandom.alphanumeric(5)}" : ""
+      end
+    end
+
+    def original_path_taken?(project)
+      existing_project = ::Project.find_by_full_path(original_value(project.full_path))
+
+      existing_project.present? && existing_project.id != project.id
+    end
+
+    def original_value(value)
+      value.sub(DELETED_SUFFIX_REGEX, '')
+    end
+
+    def updated_value(value)
+      "#{original_value(value)}#{suffix}"
+    end
   end
 end

ee/spec/services/projects/mark_for_deletion_service_spec.rb

@@ -9,6 +9,7 @@ RSpec.describe Projects::MarkForDeletionService do
     create(:project,
       :repository,
       namespace: user.namespace,
+      name: 'test project xyz',
       marked_for_deletion_at: marked_for_deletion_at)
   end
 
@@ -18,15 +19,23 @@ RSpec.describe Projects::MarkForDeletionService do
     end
 
     context 'marking project for deletion' do
-      it 'marks project as archived and marked for deletion' do
-        result = described_class.new(project, user).execute
+      subject { described_class.new(project, user).execute }
 
-        expect(result[:status]).to eq(:success)
+      it 'marks project as archived and marked for deletion' do
+        expect(subject[:status]).to eq(:success)
         expect(Project.unscoped.all).to include(project)
         expect(project.archived).to eq(true)
         expect(project.marked_for_deletion_at).not_to be_nil
         expect(project.deleting_user).to eq(user)
       end
+
+      it 'renames project name' do
+        expect { subject }.to change { project.name }.from('test project xyz').to("test project xyz-deleted-#{project.id}")
+      end
+
+      it 'renames project path' do
+        expect { subject }.to change { project.path }.from('test_project_xyz').to("test_project_xyz-deleted-#{project.id}")
+      end
     end
 
     context 'marking project for deletion once again' do
@@ -43,7 +52,7 @@ RSpec.describe Projects::MarkForDeletionService do
     context 'audit events' do
       it 'saves audit event' do
         expect { described_class.new(project, user).execute }
-          .to change { AuditEvent.count }.by(1)
+          .to change { AuditEvent.count }.by(3)
       end
     end
   end

ee/spec/services/projects/restore_service_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe Projects::RestoreService do
   let(:project) do
     create(:project,
       :repository,
+      name: 'project 1-deleted-177483',
       namespace: user.namespace,
       marked_for_deletion_at: 1.day.ago,
       deleting_user: user,
@@ -16,17 +17,71 @@ RSpec.describe Projects::RestoreService do
   end
 
   context 'restoring project' do
-    before do
-      described_class.new(project, user).execute
-    end
+    subject { described_class.new(project, user).execute }
 
     it 'marks project as unarchived and not marked for deletion' do
+      subject
+
       expect(Project.unscoped.all).to include(project)
 
       expect(project.archived).to eq(false)
       expect(project.marked_for_deletion_at).to be_nil
       expect(project.deleting_user).to eq(nil)
     end
+
+    context 'when the original project path is not taken' do
+      it 'renames the project back to its original path' do
+        expect { subject }.to change { project.path }.from("project_1-deleted-177483").to("project_1")
+      end
+
+      it 'renames the project back to its original name' do
+        expect { subject }.to change { project.name }.from("project 1-deleted-177483").to("project 1")
+      end
+    end
+
+    context 'when the original project name has been taken' do
+      before do
+        create(:project, name: 'project 1', namespace: user.namespace, deleting_user: user)
+      end
+
+      it 'renames the project back to its original path with a suffix' do
+        expect { subject }.to change { project.path }.from("project_1-deleted-177483").to(/project_1-[a-zA-Z0-9]{5}/)
+      end
+
+      it 'renames the project back to its original name with a suffix' do
+        expect { subject }.to change { project.name }.from("project 1-deleted-177483").to(/project 1-[a-zA-Z0-9]{5}/)
+      end
+
+      it 'uses the same suffix for both the path and name' do
+        subject
+
+        path_suffix = project.path.split('-')[-1]
+        name_suffix = project.name.split('-')[-1]
+
+        expect(path_suffix).to eq(name_suffix)
+      end
+    end
+
+    context 'when the original project path does not contain the -deleted- suffix' do
+      let(:project) do
+        create(:project,
+               :repository,
+               name: 'a project name',
+               namespace: user.namespace,
+               marked_for_deletion_at: 1.day.ago,
+               deleting_user: user,
+               archived: true,
+               pending_delete: pending_delete)
+      end
+
+      it 'renames the project back to its original path' do
+        expect { subject }.not_to change { project.path }
+      end
+
+      it 'renames the project back to its original name' do
+        expect { subject }.not_to change { project.name }
+      end
+    end
   end
 
   context 'restoring project already in process of removal' do
@@ -41,7 +96,7 @@ RSpec.describe Projects::RestoreService do
   context 'audit events' do
     it 'saves audit event' do
       expect { described_class.new(project, user).execute }
-        .to change { AuditEvent.count }.by(1)
+        .to change { AuditEvent.count }.by(3)
     end
   end
 end