Removes periods at the end of project paths.39695-remove-periods-at-ends-of-projects

3 files changed, 172 insertions, 1 deletions

diff --git a/db/migrate/20171101160108_remove_periods_at_end_of_project_paths.rb b/db/migrate/20171101160108_remove_periods_at_end_of_project_paths.rb
new file mode 100644
index 00000000000..5d8f42d6ad0
--- /dev/null
+++ b/db/migrate/20171101160108_remove_periods_at_end_of_project_paths.rb
@@ -0,0 +1,75 @@
+class RemovePeriodsAtEndOfProjectPaths < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  include Gitlab::ShellAdapter
+
+  DOWNTIME = false
+
+  RenameProjectError = Class.new(StandardError)
+
+  def up
+    queries = [
+      "projects.path LIKE '-%'",
+      "projects.path LIKE '%.'",
+      "projects.path LIKE '%.git'",
+      "projects.path LIKE '%.atom'"
+    ]
+
+    queries.each do |relation|
+      Project.includes(:route, namespace: [:route]).where(relation)
+        .find_in_batches(batch_size: 100) do |batch|
+        rename_projects(batch)
+      end
+    end
+  end
+
+  def down
+  end
+
+  private
+
+  def route_exists?(full_path)
+    quoted_path = ActiveRecord::Base.connection.quote_string(full_path)
+
+    ActiveRecord::Base.connection.select_all("SELECT id, path FROM routes WHERE path = '#{quoted_path}'").present?
+  end
+
+  def rename_path(namespace_full_path, path_was)
+    path = path_was.dup
+
+    # Remove periods .git and .atom at the end of a project path
+    path.gsub!(/(\.|\.git|\.atom)+\z/, "")
+    # Remove dashes at the start of the project path.
+    path.gsub!(/\A-+/, "")
+
+    counter = 0
+    while route_exists?("#{namespace_full_path}/#{path}")
+      path = "#{path}#{counter}"
+      counter += 1
+    end
+
+    path
+  end
+
+  def rename_projects(projects)
+    projects.each do |project|
+      id = project.id
+      path_was = project.path
+      namespace_full_path = project.namespace.full_path
+      path = rename_path(namespace_full_path, path_was)
+
+      begin
+        raise RenameProjectError, "Failed to update path." unless rename_project_row(project, path)
+
+        project.rename_repo
+      rescue Exception => e # rubocop: disable Lint/RescueException
+        Rails.logger.error "Exception when renaming project #{id}: #{e.message}"
+        raise
+      end
+    end
+  end
+
+  def rename_project_row(project, path)
+    project.path = path
+    project.save(validate: false) && project.respond_to?(:rename_repo)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 80d8ff92d6e..fca999985ea 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171026082505) do
+ActiveRecord::Schema.define(version: 20171101160108) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/spec/migrations/remove_periods_at_end_of_project_paths_spec.rb b/spec/migrations/remove_periods_at_end_of_project_paths_spec.rb
new file mode 100644
index 00000000000..334ff3f8d30
--- /dev/null
+++ b/spec/migrations/remove_periods_at_end_of_project_paths_spec.rb
@@ -0,0 +1,96 @@
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20171101160108_remove_periods_at_end_of_project_paths.rb')
+
+describe RemovePeriodsAtEndOfProjectPaths, :migration do
+  let!(:path) { 'foo' }
+  let!(:namespace) { create(:namespace) }
+  let!(:valid_project) { create(:project, namespace: namespace, path: path) }
+
+  shared_examples 'invalid project path cleanup' do
+    before do
+      allow_any_instance_of(Project).to receive(:rename_repo).and_return(true)
+    end
+
+    context 'when there is no project collision' do
+      it 'cleans up the project path' do
+        project = build(:project, path: path_was)
+        project.save!(validate: false)
+
+        migrate!
+
+        expect(project.reload.path).to eq(path)
+      end
+    end
+
+    context 'when there is project collision' do
+      it 'cleans up the project path with a number in front to avoid collision' do
+        project = build(:project, namespace: namespace, path: path_was)
+        project.save!(validate: false)
+
+        migrate!
+
+        expect(project.reload.path).to eq("#{path}0")
+      end
+    end
+  end
+
+  describe 'when path starts with -' do
+    it_behaves_like 'invalid project path cleanup' do
+      let!(:path_was) { "-#{path}" }
+    end
+  end
+
+  describe 'when path ends with .' do
+    it_behaves_like 'invalid project path cleanup' do
+      let!(:path_was) { "#{path}." }
+    end
+  end
+
+  describe 'when path ends with .git' do
+    it_behaves_like 'invalid project path cleanup' do
+      let!(:path_was) { "#{path}.git" }
+    end
+  end
+
+  describe 'when path ends with .atom' do
+    it_behaves_like 'invalid project path cleanup' do
+      let!(:path_was) { "#{path}.atom" }
+    end
+  end
+
+  describe 'when path ends with .atom.git' do
+    it_behaves_like 'invalid project path cleanup' do
+      let!(:path_was) { "#{path}.atom.git" }
+    end
+  end
+
+  describe 'when path starts with - and ends with .atom.git' do
+    it_behaves_like 'invalid project path cleanup' do
+      let!(:path_was) { "-#{path}.atom.git" }
+    end
+  end
+
+  describe 'when project path fails to update' do
+    it 'raises StandardError' do
+      project = build(:project, path: 'foo.git')
+      project.save!(validate: false)
+
+      allow_any_instance_of(Project).to receive(:save).and_return(false)
+
+      expect { migrate! }.to raise_error(StandardError)
+      expect(project.reload.path).to eq('foo.git')
+    end
+  end
+
+  describe 'when repo fails to be renamed' do
+    it 'raises StandardError' do
+      project = build(:project, path: 'foo.git')
+      project.save!(validate: false)
+
+      allow_any_instance_of(Project).to receive(:rename_repo).and_raise(StandardError)
+
+      expect { migrate! }.to raise_error(StandardError)
+      expect(project.reload.path).to eq('foo.git')
+    end
+  end
+end