Make renaming records in the database reusable

So we can use it for projects

lib/gitlab/database/rename_reserved_paths_migration.rb

@@ -15,11 +15,38 @@ module Gitlab
         rename_namespaces(paths, type: :top_level)
       end
 
+      def rename_path_for_routable(routable)
+        old_path = routable.path
+        old_full_path = routable.full_path
+        # Only remove the last occurrence of the path name to get the parent namespace path
+        namespace_path = remove_last_occurrence(old_full_path, old_path)
+        new_path = rename_path(namespace_path, old_path)
+        new_full_path = join_routable_path(namespace_path, new_path)
+
+        # skips callbacks & validations
+        routable.class.where(id: routable).
+          update_all(path: new_path)
+
+        rename_routes(old_full_path, new_full_path)
+
+        [old_full_path, new_full_path]
+      end
+
+      def rename_routes(old_full_path, new_full_path)
+        replace_statement = replace_sql(Route.arel_table[:path],
+                                        old_full_path,
+                                        new_full_path)
+
+        update_column_in_batches(:routes, :path, replace_statement)  do |table, query|
+          query.where(MigrationClasses::Route.arel_table[:path].matches("#{old_full_path}%"))
+        end
+      end
+
       def rename_path(namespace_path, path_was)
         counter = 0
         path = "#{path_was}#{counter}"
 
-        while route_exists?(File.join(namespace_path, path))
+        while route_exists?(join_routable_path(namespace_path, path))
           counter += 1
           path = "#{path_was}#{counter}"
         end
@@ -27,6 +54,18 @@ module Gitlab
         path
       end
 
+      def remove_last_occurrence(string, pattern)
+        string.reverse.sub(pattern.reverse, "").reverse
+      end
+
+      def join_routable_path(namespace_path, top_level)
+        if namespace_path.present?
+          File.join(namespace_path, top_level)
+        else
+          top_level
+        end
+      end
+
       def route_exists?(full_path)
         MigrationClasses::Route.where(Route.arel_table[:path].matches(full_path)).any?
       end

lib/gitlab/database/rename_reserved_paths_migration/migration_classes.rb

@@ -2,26 +2,7 @@ module Gitlab
   module Database
     module RenameReservedPathsMigration
       module MigrationClasses
-        class User < ActiveRecord::Base
-          self.table_name = 'users'
-        end
-
-        class Namespace < ActiveRecord::Base
-          self.table_name = 'namespaces'
-          belongs_to :parent,
-                     class_name: "#{MigrationClasses.name}::Namespace"
-          has_one :route, as: :source
-          has_many :children,
-                   class_name: "#{MigrationClasses.name}::Namespace",
-                   foreign_key: :parent_id
-          belongs_to :owner,
-                     class_name: "#{MigrationClasses.name}::User"
-
-          # Overridden to have the correct `source_type` for the `route` relation
-          def self.name
-            'Namespace'
-          end
-
+        module Routable
           def full_path
             if route && route.path.present?
               @full_path ||= route.path
@@ -66,17 +47,45 @@ module Gitlab
           end
         end
 
+        class User < ActiveRecord::Base
+          self.table_name = 'users'
+        end
+
+        class Namespace < ActiveRecord::Base
+          include MigrationClasses::Routable
+          self.table_name = 'namespaces'
+          belongs_to :parent,
+                     class_name: "#{MigrationClasses.name}::Namespace"
+          has_one :route, as: :source
+          has_many :children,
+                   class_name: "#{MigrationClasses.name}::Namespace",
+                   foreign_key: :parent_id
+          belongs_to :owner,
+                     class_name: "#{MigrationClasses.name}::User"
+
+          # Overridden to have the correct `source_type` for the `route` relation
+          def self.name
+            'Namespace'
+          end
+        end
+
         class Route < ActiveRecord::Base
           self.table_name = 'routes'
           belongs_to :source, polymorphic: true
         end
 
         class Project < ActiveRecord::Base
+          include MigrationClasses::Routable
           self.table_name = 'projects'
 
           def repository_storage_path
             Gitlab.config.repositories.storages[repository_storage]['path']
           end
+
+          # Overridden to have the correct `source_type` for the `route` relation
+          def self.name
+            'Project'
+          end
         end
       end
     end

lib/gitlab/database/rename_reserved_paths_migration/namespaces.rb

@@ -16,32 +16,11 @@ module Gitlab
                        elsif type == :top_level
                          MigrationClasses::Namespace.where(parent_id: nil)
                        end
-          namespaces.where(path: paths.map(&:downcase))
+          namespaces.where('lower(path) in (?)', paths.map(&:downcase))
         end
 
         def rename_namespace(namespace)
-          old_path = namespace.path
-          old_full_path = namespace.full_path
-          # Only remove the last occurrence of the path name to get the parent namespace path
-          namespace_path = remove_last_occurrence(old_full_path, old_path)
-          new_path = rename_path(namespace_path, old_path)
-          new_full_path = if namespace_path.present?
-                            File.join(namespace_path, new_path)
-                          else
-                            new_path
-                          end
-
-          # skips callbacks & validations
-          MigrationClasses::Namespace.where(id: namespace).
-            update_all(path: new_path)
-
-          replace_statement = replace_sql(Route.arel_table[:path],
-                                          old_full_path,
-                                          new_full_path)
-
-          update_column_in_batches(:routes, :path, replace_statement)  do |table, query|
-            query.where(MigrationClasses::Route.arel_table[:path].matches("#{old_full_path}%"))
-          end
+          old_full_path, new_full_path = rename_path_for_routable(namespace)
 
           move_repositories(namespace, old_full_path, new_full_path)
           move_namespace_folders(uploads_dir, old_full_path, new_full_path) if file_storage?
@@ -92,10 +71,6 @@ module Gitlab
           ids
         end
 
-        def remove_last_occurrence(string, pattern)
-          string.reverse.sub(pattern.reverse, "").reverse
-        end
-
         def file_storage?
           CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
         end

spec/lib/gitlab/database/rename_reserved_paths_migration/namespaces_spec.rb

@@ -25,7 +25,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::Namespaces, :truncate d
   describe '#namespaces_for_paths' do
     context 'for wildcard namespaces' do
       it 'only returns child namespaces with the correct path' do
-        _root_namespace = create(:namespace, path: 'the-path')
+        _root_namespace = create(:namespace, path: 'THE-path')
         _other_path = create(:namespace,
                              path: 'other',
                              parent: create(:namespace))
@@ -33,13 +33,13 @@ describe Gitlab::Database::RenameReservedPathsMigration::Namespaces, :truncate d
                            path: 'the-path',
                            parent: create(:namespace))
 
-        found_ids = subject.namespaces_for_paths(['the-path'], type: :wildcard).
-                      pluck(:id)
+        found_ids = subject.namespaces_for_paths(['the-PATH'], type: :wildcard).
+                      map(&:id)
         expect(found_ids).to contain_exactly(namespace.id)
       end
     end
 
-    context 'for top level namespaces' do
+    context 'for top levelnamespaces' do
       it 'only returns child namespaces with the correct path' do
         root_namespace = create(:namespace, path: 'the-path')
         _other_path = create(:namespace, path: 'other')
@@ -48,7 +48,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::Namespaces, :truncate d
                            parent: create(:namespace))
 
         found_ids = subject.namespaces_for_paths(['the-path'], type: :top_level).
-                      pluck(:id)
+                      map(&:id)
         expect(found_ids).to contain_exactly(root_namespace.id)
       end
     end
@@ -127,35 +127,15 @@ describe Gitlab::Database::RenameReservedPathsMigration::Namespaces, :truncate d
     end
   end
 
-  describe "#remove_last_ocurrence" do
-    it "removes only the last occurance of a string" do
-      input = "this/is/a-word-to-replace/namespace/with/a-word-to-replace"
-
-      expect(subject.remove_last_occurrence(input, "a-word-to-replace"))
-        .to eq("this/is/a-word-to-replace/namespace/with/")
-    end
-  end
-
   describe "#rename_namespace" do
     let(:namespace) { create(:namespace, path: 'the-path') }
-    it "renames namespaces called the-path" do
-      subject.rename_namespace(namespace)
-
-      expect(namespace.reload.path).to eq("the-path0")
-    end
-
-    it "renames the route to the namespace" do
-      subject.rename_namespace(namespace)
-
-      expect(Namespace.find(namespace.id).full_path).to eq("the-path0")
-    end
 
-    it "renames the route for projects of the namespace" do
-      project = create(:project, path: "project-path", namespace: namespace)
+    it 'renames paths & routes for the namesapce' do
+      expect(subject).to receive(:rename_path_for_routable).
+                           with(namespace).
+                           and_call_original
 
       subject.rename_namespace(namespace)
-
-      expect(project.route.reload.path).to eq("the-path0/project-path")
     end
 
     it "moves the the repository for a project in the namespace" do
@@ -180,29 +160,26 @@ describe Gitlab::Database::RenameReservedPathsMigration::Namespaces, :truncate d
 
       subject.rename_namespace(namespace)
     end
-
-    context "the-path namespace -> subgroup -> the-path0 project" do
-      it "updates the route of the project correctly" do
-        subgroup = create(:group, path: "subgroup", parent: namespace)
-        project = create(:project, path: "the-path0", namespace: subgroup)
-
-        subject.rename_namespace(namespace)
-
-        expect(project.route.reload.path).to eq("the-path0/subgroup/the-path0")
-      end
-    end
   end
 
   describe '#rename_namespaces' do
-    context 'top level namespaces' do
-      let!(:namespace) { create(:namespace, path: 'the-path') }
+    let!(:top_level_namespace) { create(:namespace, path: 'the-path') }
+    let!(:child_namespace) do
+      create(:namespace, path: 'the-path', parent: create(:namespace))
+    end
 
-      it 'should rename the namespace' do
+    it 'renames top level namespaces the namespace' do
       expect(subject).to receive(:rename_namespace).
-                             with(migration_namespace(namespace))
+                           with(migration_namespace(top_level_namespace))
 
       subject.rename_namespaces(['the-path'], type: :top_level)
     end
+
+    it 'renames child namespaces' do
+      expect(subject).to receive(:rename_namespace).
+                           with(migration_namespace(child_namespace))
+
+      subject.rename_namespaces(['the-path'], type: :wildcard)
     end
   end
 end

spec/lib/gitlab/database/rename_reserved_paths_migration_spec.rb

@@ -7,6 +7,10 @@ describe Gitlab::Database::RenameReservedPathsMigration do
     )
   end
 
+  before do
+    allow(subject).to receive(:say)
+  end
+
   describe '#rename_wildcard_paths' do
     it 'should rename namespaces' do
       expect(subject).to receive(:rename_namespaces).
@@ -26,4 +30,80 @@ describe Gitlab::Database::RenameReservedPathsMigration do
       subject.rename_root_paths('the-path')
     end
   end
+
+  describe "#remove_last_ocurrence" do
+    it "removes only the last occurance of a string" do
+      input = "this/is/a-word-to-replace/namespace/with/a-word-to-replace"
+
+      expect(subject.remove_last_occurrence(input, "a-word-to-replace"))
+        .to eq("this/is/a-word-to-replace/namespace/with/")
+    end
+  end
+
+  describe '#rename_path_for_routable' do
+    context 'for namespaces' do
+      let(:namespace) { create(:namespace, path: 'the-path') }
+      it "renames namespaces called the-path" do
+        subject.rename_path_for_routable(namespace)
+
+        expect(namespace.reload.path).to eq("the-path0")
+      end
+
+      it "renames the route to the namespace" do
+        subject.rename_path_for_routable(namespace)
+
+        expect(Namespace.find(namespace.id).full_path).to eq("the-path0")
+      end
+
+      it "renames the route for projects of the namespace" do
+        project = create(:project, path: "project-path", namespace: namespace)
+
+        subject.rename_path_for_routable(namespace)
+
+        expect(project.route.reload.path).to eq("the-path0/project-path")
+      end
+
+      it 'returns the old & the new path' do
+        old_path, new_path = subject.rename_path_for_routable(namespace)
+
+        expect(old_path).to eq('the-path')
+        expect(new_path).to eq('the-path0')
+      end
+
+      context "the-path namespace -> subgroup -> the-path0 project" do
+        it "updates the route of the project correctly" do
+          subgroup = create(:group, path: "subgroup", parent: namespace)
+          project = create(:project, path: "the-path0", namespace: subgroup)
+
+          subject.rename_path_for_routable(namespace)
+
+          expect(project.route.reload.path).to eq("the-path0/subgroup/the-path0")
+        end
+      end
+    end
+
+    context 'for projects' do
+      let(:parent) { create(:namespace, path: 'the-parent') }
+      let(:project) { create(:empty_project, path: 'the-path', namespace: parent) }
+
+      it 'renames the project called `the-path`' do
+        subject.rename_path_for_routable(project)
+
+        expect(project.reload.path).to eq('the-path0')
+      end
+
+      it 'renames the route for the project' do
+        subject.rename_path_for_routable(project)
+
+        expect(project.reload.route.path).to eq('the-parent/the-path0')
+      end
+
+      it 'returns the old & new path' do
+        old_path, new_path = subject.rename_path_for_routable(project)
+
+        expect(old_path).to eq('the-parent/the-path')
+        expect(new_path).to eq('the-parent/the-path0')
+      end
+    end
+  end
 end