Merge branch 'bvl-rename-all-reserved-paths' into 'master'

Rename all forbidden paths again

Closes #32625

See merge request !11713

4 files changed, 233 insertions, 17 deletions

diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb
index 5653cfee686..8813f129ef5 100644
--- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb
+++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb
@@ -6,6 +6,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase, :trunca
 
   before do
     allow(migration).to receive(:say)
+    TestEnv.clean_test_path
   end
 
   def migration_namespace(namespace)
@@ -153,6 +154,30 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase, :trunca
     end
   end
 
+  describe '#perform_rename' do
+    describe 'for namespaces' do
+      let(:namespace) { create(:namespace, path: 'the-path') }
+      it 'renames the path' do
+        subject.perform_rename(migration_namespace(namespace), 'the-path', 'renamed')
+
+        expect(namespace.reload.path).to eq('renamed')
+      end
+
+      it 'renames all the routes for the namespace' do
+        child = create(:group, path: 'child', parent: namespace)
+        project = create(:project, namespace: child, path: 'the-project')
+        other_one = create(:namespace, path: 'the-path-is-similar')
+
+        subject.perform_rename(migration_namespace(namespace), 'the-path', 'renamed')
+
+        expect(namespace.reload.route.path).to eq('renamed')
+        expect(child.reload.route.path).to eq('renamed/child')
+        expect(project.reload.route.path).to eq('renamed/child/the-project')
+        expect(other_one.reload.route.path).to eq('the-path-is-similar')
+      end
+    end
+  end
+
   describe '#move_pages' do
     it 'moves the pages directory' do
       expect(subject).to receive(:move_folders)
@@ -203,4 +228,53 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase, :trunca
       expect(File.exist?(expected_file)).to be(true)
     end
   end
+
+  describe '#track_rename', redis: true do
+    it 'tracks a rename in redis' do
+      key = 'rename:FakeRenameReservedPathMigrationV1:namespace'
+
+      subject.track_rename('namespace', 'path/to/namespace', 'path/to/renamed')
+
+      old_path, new_path = [nil, nil]
+      Gitlab::Redis.with do |redis|
+        rename_info = redis.lpop(key)
+        old_path, new_path = JSON.parse(rename_info)
+      end
+
+      expect(old_path).to eq('path/to/namespace')
+      expect(new_path).to eq('path/to/renamed')
+    end
+  end
+
+  describe '#reverts_for_type', redis: true do
+    it 'yields for each tracked rename' do
+      subject.track_rename('project', 'old_path', 'new_path')
+      subject.track_rename('project', 'old_path2', 'new_path2')
+      subject.track_rename('namespace', 'namespace_path', 'new_namespace_path')
+
+      expect { |b| subject.reverts_for_type('project', &b) }
+        .to yield_successive_args(%w(old_path2 new_path2), %w(old_path new_path))
+      expect { |b| subject.reverts_for_type('namespace', &b) }
+        .to yield_with_args('namespace_path', 'new_namespace_path')
+    end
+
+    it 'keeps the revert in redis if it failed' do
+      subject.track_rename('project', 'old_path', 'new_path')
+
+      subject.reverts_for_type('project') do
+        raise 'whatever happens, keep going!'
+      end
+
+      key = 'rename:FakeRenameReservedPathMigrationV1:project'
+      stored_renames = nil
+      rename_count = 0
+      Gitlab::Redis.with do |redis|
+        stored_renames = redis.lrange(key, 0, 1)
+        rename_count = redis.llen(key)
+      end
+
+      expect(rename_count).to eq(1)
+      expect(JSON.parse(stored_renames.first)).to eq(%w(old_path new_path))
+    end
+  end
 end
diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb
index 8125dedd3fc..803e923b4a5 100644
--- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb
+++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb
@@ -3,9 +3,11 @@ require 'spec_helper'
 describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :truncate do
   let(:migration) { FakeRenameReservedPathMigrationV1.new }
   let(:subject) { described_class.new(['the-path'], migration) }
+  let(:namespace) { create(:group, name: 'the-path') }
 
   before do
     allow(migration).to receive(:say)
+    TestEnv.clean_test_path
   end
 
   def migration_namespace(namespace)
@@ -137,8 +139,6 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
   end
 
   describe "#rename_namespace" do
-    let(:namespace) { create(:group, name: 'the-path') }
-
     it 'renames paths & routes for the namespace' do
       expect(subject).to receive(:rename_path_for_routable)
                            .with(namespace)
@@ -149,11 +149,27 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
       expect(namespace.reload.path).to eq('the-path0')
     end
 
+    it 'tracks the rename' do
+      expect(subject).to receive(:track_rename)
+                           .with('namespace', 'the-path', 'the-path0')
+
+      subject.rename_namespace(namespace)
+    end
+
+    it 'renames things related to the namespace' do
+      expect(subject).to receive(:rename_namespace_dependencies)
+                           .with(namespace, 'the-path', 'the-path0')
+
+      subject.rename_namespace(namespace)
+    end
+  end
+
+  describe '#rename_namespace_dependencies' do
     it "moves the the repository for a project in the namespace" do
       create(:project, namespace: namespace, path: "the-path-project")
       expected_repo = File.join(TestEnv.repos_path, "the-path0", "the-path-project.git")
 
-      subject.rename_namespace(namespace)
+      subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0')
 
       expect(File.directory?(expected_repo)).to be(true)
     end
@@ -161,13 +177,13 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
     it "moves the uploads for the namespace" do
       expect(subject).to receive(:move_uploads).with("the-path", "the-path0")
 
-      subject.rename_namespace(namespace)
+      subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0')
     end
 
     it "moves the pages for the namespace" do
       expect(subject).to receive(:move_pages).with("the-path", "the-path0")
 
-      subject.rename_namespace(namespace)
+      subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0')
     end
 
     it 'invalidates the markdown cache of related projects' do
@@ -175,13 +191,13 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
 
       expect(subject).to receive(:remove_cached_html_for_projects).with([project.id])
 
-      subject.rename_namespace(namespace)
+      subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0')
     end
 
     it "doesn't rename users for other namespaces" do
       expect(subject).not_to receive(:rename_user)
 
-      subject.rename_namespace(namespace)
+      subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0')
     end
 
     it 'renames the username of a namespace for a user' do
@@ -189,7 +205,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
 
       expect(subject).to receive(:rename_user).with('the-path', 'the-path0')
 
-      subject.rename_namespace(user.namespace)
+      subject.rename_namespace_dependencies(user.namespace, 'the-path', 'the-path0')
     end
   end
 
@@ -224,4 +240,50 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
       subject.rename_namespaces(type: :child)
     end
   end
+
+  describe '#revert_renames', redis: true do
+    it 'renames the routes back to the previous values' do
+      project = create(:project, path: 'a-project', namespace: namespace)
+      subject.rename_namespace(namespace)
+
+      expect(subject).to receive(:perform_rename)
+                           .with(
+                             kind_of(Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::Namespace),
+                             'the-path0',
+                             'the-path'
+                           ).and_call_original
+
+      subject.revert_renames
+
+      expect(namespace.reload.path).to eq('the-path')
+      expect(namespace.reload.route.path).to eq('the-path')
+      expect(project.reload.route.path).to eq('the-path/a-project')
+    end
+
+    it 'moves the repositories back to their original place' do
+      project = create(:project, path: 'a-project', namespace: namespace)
+      project.create_repository
+      subject.rename_namespace(namespace)
+
+      expected_path = File.join(TestEnv.repos_path, 'the-path', 'a-project.git')
+
+      expect(subject).to receive(:rename_namespace_dependencies)
+                           .with(
+                             kind_of(Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::Namespace),
+                             'the-path0',
+                             'the-path'
+                           ).and_call_original
+
+      subject.revert_renames
+
+      expect(File.directory?(expected_path)).to be_truthy
+    end
+
+    it "doesn't break when the namespace was renamed" do
+      subject.rename_namespace(namespace)
+      namespace.update_attributes!(path: 'renamed-afterwards')
+
+      expect { subject.revert_renames }.not_to raise_error
+    end
+  end
 end
diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb
index 802f77ad430..0e240a5ccf1 100644
--- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb
+++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb
@@ -3,9 +3,15 @@ require 'spec_helper'
 describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :truncate do
   let(:migration) { FakeRenameReservedPathMigrationV1.new }
   let(:subject) { described_class.new(['the-path'], migration) }
+  let(:project) do
+    create(:empty_project,
+           path: 'the-path',
+           namespace: create(:namespace, path: 'known-parent' ))
+  end
 
   before do
     allow(migration).to receive(:say)
+    TestEnv.clean_test_path
   end
 
   describe '#projects_for_paths' do
@@ -47,12 +53,6 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :tr
   end
 
   describe '#rename_project' do
-    let(:project) do
-      create(:empty_project,
-             path: 'the-path',
-             namespace: create(:namespace, path: 'known-parent' ))
-    end
-
     it 'renames path & route for the project' do
       expect(subject).to receive(:rename_path_for_routable)
                            .with(project)
@@ -63,27 +63,42 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :tr
       expect(project.reload.path).to eq('the-path0')
     end
 
+    it 'tracks the rename' do
+      expect(subject).to receive(:track_rename)
+                           .with('project', 'known-parent/the-path', 'known-parent/the-path0')
+
+      subject.rename_project(project)
+    end
+
+    it 'renames the folders for the project' do
+      expect(subject).to receive(:move_project_folders).with(project, 'known-parent/the-path', 'known-parent/the-path0')
+
+      subject.rename_project(project)
+    end
+  end
+
+  describe '#move_project_folders' do
     it 'moves the wiki & the repo' do
       expect(subject).to receive(:move_repository)
                            .with(project, 'known-parent/the-path.wiki', 'known-parent/the-path0.wiki')
       expect(subject).to receive(:move_repository)
                            .with(project, 'known-parent/the-path', 'known-parent/the-path0')
 
-      subject.rename_project(project)
+      subject.move_project_folders(project, 'known-parent/the-path', 'known-parent/the-path0')
     end
 
     it 'moves uploads' do
       expect(subject).to receive(:move_uploads)
                            .with('known-parent/the-path', 'known-parent/the-path0')
 
-      subject.rename_project(project)
+      subject.move_project_folders(project, 'known-parent/the-path', 'known-parent/the-path0')
     end
 
     it 'moves pages' do
       expect(subject).to receive(:move_pages)
                            .with('known-parent/the-path', 'known-parent/the-path0')
 
-      subject.rename_project(project)
+      subject.move_project_folders(project, 'known-parent/the-path', 'known-parent/the-path0')
     end
   end
 
@@ -99,4 +114,47 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :tr
       expect(File.directory?(expected_path)).to be(true)
     end
   end
+
+  describe '#revert_renames', redis: true do
+    it 'renames the routes back to the previous values' do
+      subject.rename_project(project)
+
+      expect(subject).to receive(:perform_rename)
+                           .with(
+                             kind_of(Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::Project),
+                             'known-parent/the-path0',
+                             'known-parent/the-path'
+                           ).and_call_original
+
+      subject.revert_renames
+
+      expect(project.reload.path).to eq('the-path')
+      expect(project.route.path).to eq('known-parent/the-path')
+    end
+
+    it 'moves the repositories back to their original place' do
+      project.create_repository
+      subject.rename_project(project)
+
+      expected_path = File.join(TestEnv.repos_path, 'known-parent', 'the-path.git')
+
+      expect(subject).to receive(:move_project_folders)
+                           .with(
+                             kind_of(Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::Project),
+                             'known-parent/the-path0',
+                             'known-parent/the-path'
+                           ).and_call_original
+
+      subject.revert_renames
+
+      expect(File.directory?(expected_path)).to be_truthy
+    end
+
+    it "doesn't break when the project was renamed" do
+      subject.rename_project(project)
+      project.update_attributes!(path: 'renamed-afterwards')
+
+      expect { subject.revert_renames }.not_to raise_error
+    end
+  end
 end
diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb
index 1d5e58855c1..7695b95dc57 100644
--- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb
+++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb
@@ -51,4 +51,26 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1, :truncate do
       subject.rename_root_paths('the-path')
     end
   end
+
+  describe '#revert_renames' do
+    it 'renames namespaces' do
+      rename_namespaces = double
+      expect(described_class::RenameNamespaces)
+        .to receive(:new).with([], subject)
+              .and_return(rename_namespaces)
+      expect(rename_namespaces).to receive(:revert_renames)
+
+      subject.revert_renames
+    end
+
+    it 'renames projects' do
+      rename_projects = double
+      expect(described_class::RenameProjects)
+        .to receive(:new).with([], subject)
+              .and_return(rename_projects)
+      expect(rename_projects).to receive(:revert_renames)
+
+      subject.revert_renames
+    end
+  end
 end