Merge branch 'bvl-fix-already-renamed-paths' into 'master'

Fix incorrect renaming migrations

Closes #32526 and #32530

See merge request !11515

db/post_migrate/20170518200835_rename_users_with_renamed_namespace.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RenameUsersWithRenamedNamespace < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  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
+  ]
+
+  def up
+    DISALLOWED_ROOT_PATHS.each do |path|
+      users = Arel::Table.new(:users)
+      namespaces = Arel::Table.new(:namespaces)
+      predicate = namespaces[:owner_id].eq(users[:id])
+                    .and(namespaces[:type].eq(nil))
+                    .and(users[:username].matches(path))
+      update_sql = if Gitlab::Database.postgresql?
+                     "UPDATE users SET username = namespaces.path "\
+                     "FROM namespaces WHERE #{predicate.to_sql}"
+                   else
+                     "UPDATE users INNER JOIN namespaces "\
+                     "ON namespaces.owner_id = users.id "\
+                     "SET username = namespaces.path "\
+                     "WHERE #{predicate.to_sql}"
+                   end
+
+      connection.execute(update_sql)
+    end
+  end
+
+  def down
+  end
+end

db/post_migrate/20170518231126_fix_wrongly_renamed_routes.rb

+# 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

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: 20170516183131) do
+ActiveRecord::Schema.define(version: 20170518231126) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

spec/migrations/fix_wrongly_renamed_routes_spec.rb

+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

spec/migrations/rename_users_with_renamed_namespace_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170518200835_rename_users_with_renamed_namespace.rb')
+
+describe RenameUsersWithRenamedNamespace, truncate: true do
+  it 'renames a user that had their namespace renamed to the namespace path' do
+    other_user = create(:user, username: 'kodingu')
+    other_user1 = create(:user, username: 'api0')
+
+    user = create(:user, username: "Users0")
+    user.update_attribute(:username, 'Users')
+    user1 = create(:user, username: "import0")
+    user1.update_attribute(:username, 'import')
+
+    described_class.new.up
+
+    expect(user.reload.username).to eq('Users0')
+    expect(user1.reload.username).to eq('import0')
+
+    expect(other_user.reload.username).to eq('kodingu')
+    expect(other_user1.reload.username).to eq('api0')
+  end
+end