Rename projects set for delayed deletion

Projects that are set for delayed deletion
will now have -deleted-:project_id
appended to their name while they are
awaiting deletion.

If they are restored, the path will be reset
unless the path has been taken in the meantime.
In this case, a random alphanumeric string will be
appended to avoid collisions.

Changelog: added

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