Fix incorrectly renamed routes

3 files changed, 178 insertions, 1 deletions

diff --git a/db/post_migrate/20170518231126_fix_wrongly_renamed_routes.rb b/db/post_migrate/20170518231126_fix_wrongly_renamed_routes.rb
new file mode 100644
index 00000000000..c78beda9d21
--- /dev/null
+++ b/db/post_migrate/20170518231126_fix_wrongly_renamed_routes.rb
@@ -0,0 +1,104 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class FixWronglyRenamedRoutes < ActiveRecord::Migration
+  include Gitlab::Database::RenameReservedPathsMigration::V1
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  DISALLOWED_ROOT_PATHS = %w[
+    -
+    abuse_reports
+    api
+    autocomplete
+    explore
+    health_check
+    import
+    invites
+    jwt
+    koding
+    member
+    notification_settings
+    oauth
+    sent_notifications
+    unicorn_test
+    uploads
+    users
+  ]
+
+  FIXED_PATHS = DISALLOWED_ROOT_PATHS.map { |p| "#{p}0" }
+
+  class Route < Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::Route
+    self.table_name = 'routes'
+  end
+
+  def routes
+    @routes ||= Route.arel_table
+  end
+
+  def namespaces
+    @namespaces ||= Arel::Table.new(:namespaces)
+  end
+
+  def wildcard_collection(collection)
+    collection.map { |word| "#{word}%" }
+  end
+
+  # The routes that got incorrectly renamed before, still have a namespace that
+  # contains the correct path.
+  # This query fetches all rows from the `routes` table that meet the following
+  # conditions using `api` as an example:
+  # - route.path ILIKE `api0%`
+  # - route.source_type = `Namespace`
+  # - namespace.parent_id IS NULL
+  # - namespace.path ILIKE `api%`
+  # - NOT(namespace.path ILIKE `api0%`)
+  # This gives us all root-routes, that were renamed, but their namespace was not.
+  #
+  def wrongly_renamed
+    Route.joins("INNER JOIN namespaces ON routes.source_id = namespaces.id")
+      .where(
+        routes[:source_type].eq('Namespace')
+          .and(namespaces[:parent_id].eq(nil))
+      )
+      .where(namespaces[:path].matches_any(wildcard_collection(DISALLOWED_ROOT_PATHS)))
+      .where.not(namespaces[:path].matches_any(wildcard_collection(FIXED_PATHS)))
+      .where(routes[:path].matches_any(wildcard_collection(FIXED_PATHS)))
+  end
+
+  # Using the query above, we just fetch the `route.path` & the `namespace.path`
+  # `route.path` is the part of the route that is now incorrect
+  # `namespace.path` is what it should be
+  # We can use `route.path` to find all the namespaces that need to be fixed
+  # And we can use `namespace.path` to apply the correct name.
+  #
+  def paths_and_corrections
+    connection.select_all(
+      wrongly_renamed.select(routes[:path], namespaces[:path].as('namespace_path')).to_sql
+    )
+  end
+
+  # This can be used to limit the `update_in_batches` call to all routes for a
+  # single namespace, note the `/` that's what went wrong in the initial migration.
+  #
+  def routes_in_namespace_query(namespace)
+    routes[:path].matches_any([namespace, "#{namespace}/%"])
+  end
+
+  def up
+    paths_and_corrections.each do |root_namespace|
+      wrong_path = root_namespace['path']
+      correct_path = root_namespace['namespace_path']
+      replace_statement = replace_sql(Route.arel_table[:path], wrong_path, correct_path)
+
+      update_column_in_batches(:routes, :path, replace_statement) do |table, query|
+        query.where(routes_in_namespace_query(wrong_path))
+      end
+    end
+  end
+
+  def down
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 8c7da682807..d14126401c9 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: 20170518200835) do
+ActiveRecord::Schema.define(version: 20170518231126) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/spec/migrations/fix_wrongly_renamed_routes_spec.rb b/spec/migrations/fix_wrongly_renamed_routes_spec.rb
new file mode 100644
index 00000000000..148290b0e7d
--- /dev/null
+++ b/spec/migrations/fix_wrongly_renamed_routes_spec.rb
@@ -0,0 +1,73 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170518231126_fix_wrongly_renamed_routes.rb')
+
+describe FixWronglyRenamedRoutes, truncate: true do
+  let(:subject) { described_class.new }
+  let(:broken_namespace) do
+    namespace = create(:group, name: 'apiis')
+    namespace.route.update_attribute(:path, 'api0is')
+    namespace
+  end
+
+  describe '#wrongly_renamed' do
+    it "includes routes that have names that don't match their namespace" do
+      broken_namespace
+      _other_namespace = create(:group, name: 'api0')
+
+      expect(subject.wrongly_renamed.map(&:id))
+        .to contain_exactly(broken_namespace.route.id)
+    end
+  end
+
+  describe "#paths_and_corrections" do
+    it 'finds the wrong path and gets the correction from the namespace' do
+      broken_namespace
+      namespace = create(:group, name: 'uploads-test')
+      namespace.route.update_attribute(:path, 'uploads0-test')
+
+      expected_result = [
+        { 'namespace_path' => 'apiis', 'path' => 'api0is' },
+        { 'namespace_path' => 'uploads-test', 'path' => 'uploads0-test' }
+      ]
+
+      expect(subject.paths_and_corrections).to include(*expected_result)
+    end
+  end
+
+  describe '#routes_in_namespace_query' do
+    it 'includes only the required routes' do
+      namespace = create(:group, path: 'hello')
+      project = create(:empty_project, namespace: namespace)
+      _other_namespace = create(:group, path: 'hello0')
+
+      result = Route.where(subject.routes_in_namespace_query('hello'))
+
+      expect(result).to contain_exactly(namespace.route, project.route)
+    end
+  end
+
+  describe '#up' do
+    let(:broken_project) do
+      project = create(:empty_project, namespace: broken_namespace, path: 'broken-project')
+      project.route.update_attribute(:path, 'api0is/broken-project')
+      project
+    end
+
+    it 'renames incorrectly named routes' do
+      broken_project
+
+      subject.up
+
+      expect(broken_project.route.reload.path).to eq('apiis/broken-project')
+      expect(broken_namespace.route.reload.path).to eq('apiis')
+    end
+
+    it "doesn't touch namespaces that look like something that should be renamed" do
+      namespace = create(:group, path: 'api0')
+
+      subject.up
+
+      expect(namespace.route.reload.path).to eq('api0')
+    end
+  end
+end