diff options
5 files changed, 174 insertions, 94 deletions
diff --git a/lib/gitlab/database/rename_reserved_paths_migration.rb b/lib/gitlab/database/rename_reserved_paths_migration.rb index ad5570b4c72..2cfc01ab2f5 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration.rb +++ b/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 diff --git a/lib/gitlab/database/rename_reserved_paths_migration/migration_classes.rb b/lib/gitlab/database/rename_reserved_paths_migration/migration_classes.rb index a919d250541..d725402ace4 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/migration_classes.rb +++ b/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 diff --git a/lib/gitlab/database/rename_reserved_paths_migration/namespaces.rb b/lib/gitlab/database/rename_reserved_paths_migration/namespaces.rb index f8fbeaa990a..b4f2a67fd06 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/namespaces.rb +++ b/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 diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/namespaces_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/namespaces_spec.rb index ea31e373647..3e5f981c095 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/namespaces_spec.rb +++ b/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 + 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) + describe '#rename_namespaces' do + let!(:top_level_namespace) { create(:namespace, path: 'the-path') } + let!(:child_namespace) do + create(:namespace, path: 'the-path', parent: create(:namespace)) + end - subject.rename_namespace(namespace) + it 'renames top level namespaces the namespace' do + expect(subject).to receive(:rename_namespace). + with(migration_namespace(top_level_namespace)) - expect(project.route.reload.path).to eq("the-path0/subgroup/the-path0") - end + subject.rename_namespaces(['the-path'], type: :top_level) end - end - describe '#rename_namespaces' do - context 'top level namespaces' do - let!(:namespace) { create(:namespace, path: 'the-path') } + it 'renames child namespaces' do + expect(subject).to receive(:rename_namespace). + with(migration_namespace(child_namespace)) - it 'should rename the namespace' do - expect(subject).to receive(:rename_namespace). - with(migration_namespace(namespace)) - - subject.rename_namespaces(['the-path'], type: :top_level) - end + subject.rename_namespaces(['the-path'], type: :wildcard) end end end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration_spec.rb index 8b4ab6703c6..8fc15c8f716 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration_spec.rb +++ b/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 |