Merge branch 'ee-17361-redirect-renamed-paths' into 'master'

Port of 17361-redirect-renamed-paths to EE

See merge request !1838

app/controllers/application_controller.rb

@@ -56,7 +56,7 @@ class ApplicationController < ActionController::Base
     if current_user
       not_found
     else
-      redirect_to new_user_session_path
+      authenticate_user!
     end
   end
 

app/controllers/concerns/routable_actions.rb

+module RoutableActions
+  extend ActiveSupport::Concern
+
+  def find_routable!(routable_klass, requested_full_path, extra_authorization_proc: nil)
+    routable = routable_klass.find_by_full_path(requested_full_path, follow_redirects: request.get?)
+
+    if routable_authorized?(routable_klass, routable, extra_authorization_proc)
+      ensure_canonical_path(routable, requested_full_path)
+      routable
+    else
+      route_not_found
+      nil
+    end
+  end
+
+  def routable_authorized?(routable_klass, routable, extra_authorization_proc)
+    action = :"read_#{routable_klass.to_s.underscore}"
+    return false unless can?(current_user, action, routable)
+
+    if extra_authorization_proc
+      extra_authorization_proc.call(routable)
+    else
+      true
+    end
+  end
+
+  def ensure_canonical_path(routable, requested_path)
+    return unless request.get?
+
+    canonical_path = routable.full_path
+    if canonical_path != requested_path
+      if canonical_path.casecmp(requested_path) != 0
+        flash[:notice] = "Project '#{requested_path}' was moved to '#{canonical_path}'. Please update any links and bookmarks that may still have the old path."
+      end
+      redirect_to request.original_url.sub(requested_path, canonical_path)
+    end
+  end
+end

app/controllers/groups/application_controller.rb

 class Groups::ApplicationController < ApplicationController
+  include RoutableActions
+
   layout 'group'
 
   skip_before_action :authenticate_user!
@@ -7,29 +9,17 @@ class Groups::ApplicationController < ApplicationController
   private
 
   def group
-    unless @group
-      id = params[:group_id] || params[:id]
-      @group = Group.find_by_full_path(id)
-      @group_merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id).execute
-
-      unless @group && can?(current_user, :read_group, @group)
-        @group = nil
-
-        if current_user.nil?
-          authenticate_user!
-        else
-          render_404
-        end
-      end
-    end
-
-    @group
+    @group ||= find_routable!(Group, params[:group_id] || params[:id])
   end
 
   def group_projects
     @projects ||= GroupProjectsFinder.new(group: group, current_user: current_user).execute
   end
 
+  def group_merge_requests
+    @group_merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id).execute
+  end
+
   def authorize_admin_group!
     unless can?(current_user, :admin_group, group)
       return render_404

app/controllers/groups_controller.rb

@@ -12,8 +12,8 @@ class GroupsController < Groups::ApplicationController
   before_action :authorize_admin_group!, only: [:edit, :update, :destroy, :projects]
   before_action :authorize_create_group!, only: [:new, :create]
 
-  # Load group projects
   before_action :group_projects, only: [:projects, :activity, :issues, :merge_requests]
+  before_action :group_merge_requests, only: [:merge_requests]
   before_action :event_filter, only: [:activity]
 
   before_action :user_actions, only: [:show, :subgroups]

app/controllers/projects/application_controller.rb

 class Projects::ApplicationController < ApplicationController
+  include RoutableActions
+
   skip_before_action :authenticate_user!
+  before_action :redirect_git_extension
   before_action :project
   before_action :repository
   layout 'project'
@@ -8,40 +11,22 @@ class Projects::ApplicationController < ApplicationController
 
   private
 
+  def redirect_git_extension
+    # Redirect from
+    #   localhost/group/project.git
+    # to
+    #   localhost/group/project
+    #
+    redirect_to url_for(params.merge(format: nil)) if params[:format] == 'git'
+  end
+
   def project
-    unless @project
-      namespace = params[:namespace_id]
-      id = params[:project_id] || params[:id]
-
-      # Redirect from
-      #   localhost/group/project.git
-      # to
-      #   localhost/group/project
-      #
-      if params[:format] == 'git'
-        redirect_to request.original_url.gsub(/\.git\/?\Z/, '')
-        return
-      end
-
-      project_path = "#{namespace}/#{id}"
-      @project = Project.find_by_full_path(project_path)
-
-      if can?(current_user, :read_project, @project) && !@project.pending_delete?
-        if @project.path_with_namespace != project_path
-          redirect_to request.original_url.gsub(project_path, @project.path_with_namespace)
-        end
-      else
-        @project = nil
-
-        if current_user.nil?
-          authenticate_user!
-        else
-          render_404
-        end
-      end
-    end
+    return @project if @project
+
+    path = File.join(params[:namespace_id], params[:project_id] || params[:id])
+    auth_proc = ->(project) { !project.pending_delete? }
 
-    @project
+    @project = find_routable!(Project, path, extra_authorization_proc: auth_proc)
   end
 
   def repository

app/controllers/users_controller.rb

 class UsersController < ApplicationController
+  include RoutableActions
+
   skip_before_action :authenticate_user!
   before_action :user, except: [:exists]
-  before_action :authorize_read_user!, only: [:show]
 
   def show
     respond_to do |format|
@@ -91,12 +92,8 @@ class UsersController < ApplicationController
 
   private
 
-  def authorize_read_user!
-    render_404 unless can?(current_user, :read_user, user)
-  end
-
   def user
-    @user ||= User.find_by_username!(params[:username])
+    @user ||= find_routable!(User, params[:username])
   end
 
   def contributed_projects

app/models/concerns/routable.rb

@@ -5,6 +5,7 @@ module Routable
 
   included do
     has_one :route, as: :source, autosave: true, dependent: :destroy
+    has_many :redirect_routes, as: :source, autosave: true, dependent: :destroy
 
     validates_associated :route
     validates :route, presence: true
@@ -26,16 +27,31 @@ module Routable
     #     Klass.find_by_full_path('gitlab-org/gitlab-ce')
     #
     # Returns a single object, or nil.
-    def find_by_full_path(path)
+    def find_by_full_path(path, follow_redirects: false)
       # On MySQL we want to ensure the ORDER BY uses a case-sensitive match so
       # any literal matches come first, for this we have to use "BINARY".
       # Without this there's still no guarantee in what order MySQL will return
       # rows.
+      #
+      # Why do we do this?
+      #
+      # Even though we have Rails validation on Route for unique paths
+      # (case-insensitive), there are old projects in our DB (and possibly
+      # clients' DBs) that have the same path with different cases.
+      # See https://gitlab.com/gitlab-org/gitlab-ce/issues/18603. Also note that
+      # our unique index is case-sensitive in Postgres.
       binary = Gitlab::Database.mysql? ? 'BINARY' : ''
-
       order_sql = "(CASE WHEN #{binary} routes.path = #{connection.quote(path)} THEN 0 ELSE 1 END)"
-
-      where_full_path_in([path]).reorder(order_sql).take
+      found = where_full_path_in([path]).reorder(order_sql).take
+      return found if found
+
+      if follow_redirects
+        if Gitlab::Database.postgresql?
+          joins(:redirect_routes).find_by("LOWER(redirect_routes.path) = LOWER(?)", path)
+        else
+          joins(:redirect_routes).find_by(redirect_routes: { path: path })
+        end
+      end
     end
 
     # Builds a relation to find multiple objects by their full paths.

app/models/redirect_route.rb

+class RedirectRoute < ActiveRecord::Base
+  belongs_to :source, polymorphic: true
+
+  validates :source, presence: true
+
+  validates :path,
+    length: { within: 1..255 },
+    presence: true,
+    uniqueness: { case_sensitive: false }
+
+  scope :matching_path_and_descendants, -> (path) { where('redirect_routes.path = ? OR redirect_routes.path LIKE ?', path, "#{sanitize_sql_like(path)}/%") }
+end

app/models/route.rb

@@ -8,29 +8,58 @@ class Route < ActiveRecord::Base
     presence: true,
     uniqueness: { case_sensitive: false }
 
+  after_create :delete_conflicting_redirects
+  after_update :delete_conflicting_redirects, if: :path_changed?
+  after_update :create_redirect_for_old_path
   after_update :rename_descendants
 
   scope :inside_path, -> (path) { where('routes.path LIKE ?', "#{sanitize_sql_like(path)}/%") }
 
   def rename_descendants
-    if path_changed? || name_changed?
-      descendants = self.class.inside_path(path_was)
+    return unless path_changed? || name_changed?
 
-      descendants.each do |route|
-        attributes = {}
+    descendant_routes = self.class.inside_path(path_was)
 
-        if path_changed? && route.path.present?
-          attributes[:path] = route.path.sub(path_was, path)
-        end
+    descendant_routes.each do |route|
+      attributes = {}
 
-        if name_changed? && name_was.present? && route.name.present?
-          attributes[:name] = route.name.sub(name_was, name)
-        end
+      if path_changed? && route.path.present?
+        attributes[:path] = route.path.sub(path_was, path)
+      end
 
-        # Note that update_columns skips validation and callbacks.
-        # We need this to avoid recursive call of rename_descendants method
-        route.update_columns(attributes) unless attributes.empty?
+      if name_changed? && name_was.present? && route.name.present?
+        attributes[:name] = route.name.sub(name_was, name)
+      end
+
+      if attributes.present?
+        old_path = route.path
+
+        # Callbacks must be run manually
+        route.update_columns(attributes)
+
+        # We are not calling route.delete_conflicting_redirects here, in hopes
+        # of avoiding deadlocks. The parent (self, in this method) already
+        # called it, which deletes conflicts for all descendants.
+        route.create_redirect(old_path) if attributes[:path]
       end
     end
   end
+
+  def delete_conflicting_redirects
+    conflicting_redirects.delete_all
+  end
+
+  def conflicting_redirects
+    RedirectRoute.matching_path_and_descendants(path)
+  end
+
+  def create_redirect(path)
+    RedirectRoute.create(source: source, path: path)
+  end
+
+  private
+
+  def create_redirect_for_old_path
+    create_redirect(path_was) if path_changed?
+  end
 end

app/models/user.rb

@@ -353,6 +353,11 @@ class User < ActiveRecord::Base
       find_by(id: Key.unscoped.select(:user_id).where(id: key_id))
     end
 
+    def find_by_full_path(path, follow_redirects: false)
+      namespace = Namespace.find_by_full_path(path, follow_redirects: follow_redirects)
+      namespace&.owner
+    end
+
     def non_ldap
       joins('LEFT JOIN identities ON identities.user_id = users.id')
         .where('identities.provider IS NULL OR identities.provider NOT LIKE ?', 'ldap%')
@@ -380,6 +385,10 @@ class User < ActiveRecord::Base
     end
   end
 
+  def full_path
+    username
+  end
+
   def self.internal_attributes
     [:ghost]
   end

changelogs/unreleased/17361-redirect-renamed-paths.yml

+---
+title: Redirect old links after renaming a user/group/project.
+merge_request: 10370
+author:

db/migrate/20170427215854_create_redirect_routes.rb

+class CreateRedirectRoutes < ActiveRecord::Migration
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    create_table :redirect_routes do |t|
+      t.integer :source_id, null: false
+      t.string :source_type, null: false
+      t.string :path, null: false
+
+      t.timestamps null: false
+    end
+  end
+end

db/migrate/20170503184421_add_index_to_redirect_routes.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddIndexToRedirectRoutes < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(:redirect_routes, :path, unique: true)
+    add_concurrent_index(:redirect_routes, [:source_type, :source_id])
+  end
+
+  def down
+    remove_concurrent_index(:redirect_routes, :path) if index_exists?(:redirect_routes, :path)
+    remove_concurrent_index(:redirect_routes, [:source_type, :source_id]) if index_exists?(:redirect_routes, [:source_type, :source_id])
+  end
+end

db/migrate/20170503185032_index_redirect_routes_path_for_like.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class IndexRedirectRoutesPathForLike < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  INDEX_NAME = 'index_redirect_routes_on_path_text_pattern_ops'
+
+  disable_ddl_transaction!
+
+  def up
+    return unless Gitlab::Database.postgresql?
+
+    unless index_exists?(:redirect_routes, :path, name: INDEX_NAME)
+      execute("CREATE INDEX CONCURRENTLY #{INDEX_NAME} ON redirect_routes (path varchar_pattern_ops);")
+    end
+  end
+
+  def down
+    return unless Gitlab::Database.postgresql?
+
+    if index_exists?(:redirect_routes, :path, name: INDEX_NAME)
+      execute("DROP INDEX CONCURRENTLY #{INDEX_NAME};")
+    end
+  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: 20170503004425) do
+ActiveRecord::Schema.define(version: 20170503185032) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1219,6 +1219,18 @@ ActiveRecord::Schema.define(version: 20170503004425) do
 
   add_index "push_rules", ["project_id"], name: "index_push_rules_on_project_id", using: :btree
 
+  create_table "redirect_routes", force: :cascade do |t|
+    t.integer "source_id", null: false
+    t.string "source_type", null: false
+    t.string "path", null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+  end
+
+  add_index "redirect_routes", ["path"], name: "index_redirect_routes_on_path", unique: true, using: :btree
+  add_index "redirect_routes", ["path"], name: "index_redirect_routes_on_path_text_pattern_ops", using: :btree, opclasses: {"path"=>"varchar_pattern_ops"}
+  add_index "redirect_routes", ["source_type", "source_id"], name: "index_redirect_routes_on_source_type_and_source_id", using: :btree
+
   create_table "releases", force: :cascade do |t|
     t.string "tag"
     t.text "description"

lib/constraints/group_url_constrainer.rb

@@ -4,6 +4,6 @@ class GroupUrlConstrainer
 
     return false unless DynamicPathValidator.valid?(id)
 
-    Group.find_by_full_path(id).present?
+    Group.find_by_full_path(id, follow_redirects: request.get?).present?
   end
 end

lib/constraints/project_url_constrainer.rb

@@ -6,6 +6,6 @@ class ProjectUrlConstrainer
 
     return false unless DynamicPathValidator.valid?(full_path)
 
-    Project.find_by_full_path(full_path).present?
+    Project.find_by_full_path(full_path, follow_redirects: request.get?).present?
   end
 end

lib/constraints/user_url_constrainer.rb

 class UserUrlConstrainer
   def matches?(request)
-    User.find_by_username(request.params[:username]).present?
+    User.find_by_full_path(request.params[:username], follow_redirects: request.get?).present?
   end
 end

lib/tasks/migrate/setup_postgresql.rake

@@ -11,4 +11,5 @@ task setup_postgresql: :environment do
   AddUsersLowerUsernameEmailIndexes.new.up
   AddLowerPathIndexToRoutes.new.up
   IndexRoutesPathForLike.new.up
+  IndexRedirectRoutesPathForLike.new.up
 end

spec/controllers/application_controller_spec.rb

@@ -106,10 +106,9 @@ describe ApplicationController do
       controller.send(:route_not_found)
     end
 
-    it 'does redirect to login page if not authenticated' do
+    it 'does redirect to login page via authenticate_user! if not authenticated' do
       allow(controller).to receive(:current_user).and_return(nil)
-      expect(controller).to receive(:redirect_to)
-      expect(controller).to receive(:new_user_session_path)
+      expect(controller).to receive(:authenticate_user!)
       controller.send(:route_not_found)
     end
   end

spec/controllers/groups_controller_spec.rb

@@ -49,6 +49,26 @@ describe GroupsController do
         expect(assigns(:issues)).to eq [issue_2, issue_1]
       end
     end
+
+    context 'when requesting the canonical path with different casing' do
+      it 'redirects to the correct casing' do
+        get :issues, id: group.to_param.upcase
+
+        expect(response).to redirect_to(issues_group_path(group.to_param))
+        expect(controller).not_to set_flash[:notice]
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { group.redirect_routes.create(path: 'old-path') }
+
+      it 'redirects to the canonical path' do
+        get :issues, id: redirect_route.path
+
+        expect(response).to redirect_to(issues_group_path(group.to_param))
+        expect(controller).to set_flash[:notice].to(/moved/)
+      end
+    end
   end
 
   describe 'GET #merge_requests' do
@@ -74,6 +94,26 @@ describe GroupsController do
         expect(assigns(:merge_requests)).to eq [merge_request_2, merge_request_1]
       end
     end
+
+    context 'when requesting the canonical path with different casing' do
+      it 'redirects to the correct casing' do
+        get :merge_requests, id: group.to_param.upcase
+
+        expect(response).to redirect_to(merge_requests_group_path(group.to_param))
+        expect(controller).not_to set_flash[:notice]
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { group.redirect_routes.create(path: 'old-path') }
+
+      it 'redirects to the canonical path' do
+        get :merge_requests, id: redirect_route.path
+
+        expect(response).to redirect_to(merge_requests_group_path(group.to_param))
+        expect(controller).to set_flash[:notice].to(/moved/)
+      end
+    end
   end
 
   describe 'DELETE #destroy' do
@@ -81,7 +121,7 @@ describe GroupsController do
       it 'returns 404' do
         sign_in(create(:user))
 
-        delete :destroy, id: group.path
+        delete :destroy, id: group.to_param
 
         expect(response.status).to eq(404)
       end
@@ -94,15 +134,39 @@ describe GroupsController do
 
       it 'schedules a group destroy' do
         Sidekiq::Testing.fake! do
-          expect { delete :destroy, id: group.path }.to change(GroupDestroyWorker.jobs, :size).by(1)
+          expect { delete :destroy, id: group.to_param }.to change(GroupDestroyWorker.jobs, :size).by(1)
         end
       end
 
       it 'redirects to the root path' do
-        delete :destroy, id: group.path
+        delete :destroy, id: group.to_param
 
         expect(response).to redirect_to(root_path)
       end
+
+      context 'when requesting the canonical path with different casing' do
+        it 'does not 404' do
+          delete :destroy, id: group.to_param.upcase
+
+          expect(response).not_to have_http_status(404)
+        end
+
+        it 'does not redirect to the correct casing' do
+          delete :destroy, id: group.to_param.upcase
+
+          expect(response).not_to redirect_to(group_path(group.to_param))
+        end
+      end
+
+      context 'when requesting a redirected path' do
+        let(:redirect_route) { group.redirect_routes.create(path: 'old-path') }
+
+        it 'returns not found' do
+          delete :destroy, id: redirect_route.path
+
+          expect(response).to have_http_status(404)
+        end
+      end
     end
   end
 
@@ -111,7 +175,7 @@ describe GroupsController do
       sign_in(user)
     end
 
-    it 'updates the path succesfully' do
+    it 'updates the path successfully' do
       post :update, id: group.to_param, group: { path: 'new_path' }
 
       expect(response).to have_http_status(302)
@@ -125,6 +189,30 @@ describe GroupsController do
       expect(assigns(:group).errors).not_to be_empty
       expect(assigns(:group).path).not_to eq('new_path')
     end
+
+    context 'when requesting the canonical path with different casing' do
+      it 'does not 404' do
+        post :update, id: group.to_param.upcase, group: { path: 'new_path' }
+
+        expect(response).not_to have_http_status(404)
+      end
+
+      it 'does not redirect to the correct casing' do
+        post :update, id: group.to_param.upcase, group: { path: 'new_path' }
+
+        expect(response).not_to redirect_to(group_path(group.to_param))
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { group.redirect_routes.create(path: 'old-path') }
+
+      it 'returns not found' do
+        post :update, id: redirect_route.path, group: { path: 'new_path' }
+
+        expect(response).to have_http_status(404)
+      end
+    end
   end
 
   describe 'POST create' do

spec/controllers/projects_controller_spec.rb

@@ -206,6 +206,7 @@ describe ProjectsController do
 
           expect(assigns(:project)).to eq(public_project)
           expect(response).to redirect_to("/#{public_project.full_path}")
+          expect(controller).not_to set_flash[:notice]
         end
       end
     end
@@ -239,19 +240,33 @@ describe ProjectsController do
         expect(response).to redirect_to(namespace_project_path)
       end
     end
+
+    context 'when requesting a redirected path' do
+      let!(:redirect_route) { public_project.redirect_routes.create!(path: "foo/bar") }
+
+      it 'redirects to the canonical path' do
+        get :show, namespace_id: 'foo', id: 'bar'
+
+        expect(response).to redirect_to(public_project)
+        expect(controller).to set_flash[:notice].to(/moved/)
+      end
+    end
   end
 
   describe "#update" do
     render_views
 
     let(:admin) { create(:admin) }
+    let(:project) { create(:project, :repository) }
+    let(:new_path) { 'renamed_path' }
+    let(:project_params) { { path: new_path } }
+
+    before do
+      sign_in(admin)
+    end
 
     it "sets the repository to the right path after a rename" do
-      project = create(:project, :repository)
-      new_path = 'renamed_path'
-      project_params = { path: new_path }
       controller.instance_variable_set(:@project, project)
-      sign_in(admin)
 
       put :update,
           namespace_id: project.namespace,
@@ -262,6 +277,34 @@ describe ProjectsController do
       expect(assigns(:repository).path).to eq(project.repository.path)
       expect(response).to have_http_status(302)
     end
+
+    context 'when requesting the canonical path' do
+      it "is case-insensitive" do
+        controller.instance_variable_set(:@project, project)
+
+        put :update,
+            namespace_id: 'FOo',
+            id: 'baR',
+            project: project_params
+
+        expect(project.repository.path).to include(new_path)
+        expect(assigns(:repository).path).to eq(project.repository.path)
+        expect(response).to have_http_status(302)
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let!(:redirect_route) { project.redirect_routes.create!(path: "foo/bar") }
+
+      it 'returns not found' do
+        put :update,
+            namespace_id: 'foo',
+            id: 'bar',
+            project: project_params
+
+        expect(response).to have_http_status(404)
+      end
+    end
   end
 
   describe "#destroy" do
@@ -297,6 +340,31 @@ describe ProjectsController do
         expect(merge_request.reload.state).to eq('closed')
       end
     end
+
+    context 'when requesting the canonical path' do
+      it "is case-insensitive" do
+        controller.instance_variable_set(:@project, project)
+        sign_in(admin)
+
+        orig_id = project.id
+        delete :destroy, namespace_id: project.namespace, id: project.path.upcase
+
+        expect { Project.find(orig_id) }.to raise_error(ActiveRecord::RecordNotFound)
+        expect(response).to have_http_status(302)
+        expect(response).to redirect_to(dashboard_projects_path)
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let!(:redirect_route) { project.redirect_routes.create!(path: "foo/bar") }
+
+      it 'returns not found' do
+        sign_in(admin)
+        delete :destroy, namespace_id: 'foo', id: 'bar'
+
+        expect(response).to have_http_status(404)
+      end
+    end
   end
 
   describe 'PUT #new_issue_address' do
@@ -418,6 +486,17 @@ describe ProjectsController do
       expect(parsed_body["Tags"]).to include("v1.0.0")
       expect(parsed_body["Commits"]).to include("123456")
     end
+
+    context 'when requesting a redirected path' do
+      let!(:redirect_route) { public_project.redirect_routes.create!(path: "foo/bar") }
+
+      it 'redirects to the canonical path' do
+        get :refs, namespace_id: 'foo', id: 'bar'
+
+        expect(response).to redirect_to(refs_namespace_project_path(namespace_id: public_project.namespace, id: public_project))
+        expect(controller).to set_flash[:notice].to(/moved/)
+      end
+    end
   end
 
   describe 'GET edit' do

spec/controllers/users_controller_spec.rb

@@ -4,15 +4,6 @@ describe UsersController do
   let(:user) { create(:user) }
 
   describe 'GET #show' do
-    it 'is case-insensitive' do
-      user = create(:user, username: 'CamelCaseUser')
-      sign_in(user)
-
-      get :show, username: user.username.downcase
-
-      expect(response).to be_success
-    end
-
     context 'with rendered views' do
       render_views
 
@@ -45,9 +36,9 @@ describe UsersController do
       end
 
       context 'when logged out' do
-        it 'renders 404' do
+        it 'redirects to login page' do
           get :show, username: user.username
-          expect(response).to have_http_status(404)
+          expect(response).to redirect_to new_user_session_path
         end
       end
 
@@ -61,6 +52,58 @@ describe UsersController do
         end
       end
     end
+
+    context 'when requesting the canonical path' do
+      let(:user) { create(:user, username: 'CamelCaseUser') }
+
+      before { sign_in(user) }
+
+      context 'with exactly matching casing' do
+        it 'responds with success' do
+          get :show, username: user.username
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'with different casing' do
+        it 'redirects to the correct casing' do
+          get :show, username: user.username.downcase
+
+          expect(response).to redirect_to(user)
+          expect(controller).not_to set_flash[:notice]
+        end
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+      it 'redirects to the canonical path' do
+        get :show, username: redirect_route.path
+
+        expect(response).to redirect_to(user)
+        expect(controller).to set_flash[:notice].to(/moved/)
+      end
+    end
+
+    context 'when a user by that username does not exist' do
+      context 'when logged out' do
+        it 'redirects to login page' do
+          get :show, username: 'nonexistent'
+          expect(response).to redirect_to new_user_session_path
+        end
+      end
+
+      context 'when logged in' do
+        before { sign_in(user) }
+
+        it 'renders 404' do
+          get :show, username: 'nonexistent'
+          expect(response).to have_http_status(404)
+        end
+      end
+    end
   end
 
   describe 'GET #calendar' do
@@ -88,11 +131,45 @@ describe UsersController do
         expect(assigns(:contributions_calendar).projects.count).to eq(2)
       end
     end
+
+    context 'when requesting the canonical path' do
+      let(:user) { create(:user, username: 'CamelCaseUser') }
+
+      before { sign_in(user) }
+
+      context 'with exactly matching casing' do
+        it 'responds with success' do
+          get :calendar, username: user.username
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'with different casing' do
+        it 'redirects to the correct casing' do
+          get :calendar, username: user.username.downcase
+
+          expect(response).to redirect_to(user_calendar_path(user))
+          expect(controller).not_to set_flash[:notice]
+        end
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+      it 'redirects to the canonical path' do
+        get :calendar, username: redirect_route.path
+
+        expect(response).to redirect_to(user_calendar_path(user))
+        expect(controller).to set_flash[:notice].to(/moved/)
+      end
+    end
   end
 
   describe 'GET #calendar_activities' do
     let!(:project) { create(:empty_project) }
-    let!(:user) { create(:user) }
+    let(:user) { create(:user) }
 
     before do
       allow_any_instance_of(User).to receive(:contributed_projects_ids).and_return([project.id])
@@ -110,6 +187,38 @@ describe UsersController do
       get :calendar_activities, username: user.username
       expect(response).to render_template('calendar_activities')
     end
+
+    context 'when requesting the canonical path' do
+      let(:user) { create(:user, username: 'CamelCaseUser') }
+
+      context 'with exactly matching casing' do
+        it 'responds with success' do
+          get :calendar_activities, username: user.username
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'with different casing' do
+        it 'redirects to the correct casing' do
+          get :calendar_activities, username: user.username.downcase
+
+          expect(response).to redirect_to(user_calendar_activities_path(user))
+          expect(controller).not_to set_flash[:notice]
+        end
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+      it 'redirects to the canonical path' do
+        get :calendar_activities, username: redirect_route.path
+
+        expect(response).to redirect_to(user_calendar_activities_path(user))
+        expect(controller).to set_flash[:notice].to(/moved/)
+      end
+    end
   end
 
   describe 'GET #snippets' do
@@ -132,5 +241,83 @@ describe UsersController do
         expect(JSON.parse(response.body)).to have_key('html')
       end
     end
+
+    context 'when requesting the canonical path' do
+      let(:user) { create(:user, username: 'CamelCaseUser') }
+
+      context 'with exactly matching casing' do
+        it 'responds with success' do
+          get :snippets, username: user.username
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'with different casing' do
+        it 'redirects to the correct casing' do
+          get :snippets, username: user.username.downcase
+
+          expect(response).to redirect_to(user_snippets_path(user))
+          expect(controller).not_to set_flash[:notice]
+        end
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+      it 'redirects to the canonical path' do
+        get :snippets, username: redirect_route.path
+
+        expect(response).to redirect_to(user_snippets_path(user))
+        expect(controller).to set_flash[:notice].to(/moved/)
+      end
+    end
+  end
+
+  describe 'GET #exists' do
+    before do
+      sign_in(user)
+    end
+
+    context 'when user exists' do
+      it 'returns JSON indicating the user exists' do
+        get :exists, username: user.username
+
+        expected_json = { exists: true }.to_json
+        expect(response.body).to eq(expected_json)
+      end
+
+      context 'when the casing is different' do
+        let(:user) { create(:user, username: 'CamelCaseUser') }
+
+        it 'returns JSON indicating the user exists' do
+          get :exists, username: user.username.downcase
+
+          expected_json = { exists: true }.to_json
+          expect(response.body).to eq(expected_json)
+        end
+      end
+    end
+
+    context 'when the user does not exist' do
+      it 'returns JSON indicating the user does not exist' do
+        get :exists, username: 'foo'
+
+        expected_json = { exists: false }.to_json
+        expect(response.body).to eq(expected_json)
+      end
+
+      context 'when a user changed their username' do
+        let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+        it 'returns JSON indicating a user by that username does not exist' do
+          get :exists, username: 'old-username'
+
+          expected_json = { exists: false }.to_json
+          expect(response.body).to eq(expected_json)
+        end
+      end
+    end
   end
 end

spec/features/groups/group_settings_spec.rb

+require 'spec_helper'
+
+feature 'Edit group settings', feature: true do
+  given(:user)  { create(:user) }
+  given(:group) { create(:group, path: 'foo') }
+
+  background do
+    group.add_owner(user)
+    login_as(user)
+  end
+
+  describe 'when the group path is changed' do
+    let(:new_group_path) { 'bar' }
+    let(:old_group_full_path) { "/#{group.path}" }
+    let(:new_group_full_path) { "/#{new_group_path}" }
+
+    scenario 'the group is accessible via the new path' do
+      update_path(new_group_path)
+      visit new_group_full_path
+      expect(current_path).to eq(new_group_full_path)
+      expect(find('h1.group-title')).to have_content(new_group_path)
+    end
+
+    scenario 'the old group path redirects to the new path' do
+      update_path(new_group_path)
+      visit old_group_full_path
+      expect(current_path).to eq(new_group_full_path)
+      expect(find('h1.group-title')).to have_content(new_group_path)
+    end
+
+    context 'with a subgroup' do
+      given!(:subgroup) { create(:group, parent: group, path: 'subgroup') }
+      given(:old_subgroup_full_path) { "/#{group.path}/#{subgroup.path}" }
+      given(:new_subgroup_full_path) { "/#{new_group_path}/#{subgroup.path}" }
+
+      scenario 'the subgroup is accessible via the new path' do
+        update_path(new_group_path)
+        visit new_subgroup_full_path
+        expect(current_path).to eq(new_subgroup_full_path)
+        expect(find('h1.group-title')).to have_content(subgroup.path)
+      end
+
+      scenario 'the old subgroup path redirects to the new path' do
+        update_path(new_group_path)
+        visit old_subgroup_full_path
+        expect(current_path).to eq(new_subgroup_full_path)
+        expect(find('h1.group-title')).to have_content(subgroup.path)
+      end
+    end
+
+    context 'with a project' do
+      given!(:project) { create(:project, group: group, path: 'project') }
+      given(:old_project_full_path) { "/#{group.path}/#{project.path}" }
+      given(:new_project_full_path) { "/#{new_group_path}/#{project.path}" }
+      
+      before(:context) { TestEnv.clean_test_path }
+      after(:example) { TestEnv.clean_test_path }
+
+      scenario 'the project is accessible via the new path' do
+        update_path(new_group_path)
+        visit new_project_full_path
+        expect(current_path).to eq(new_project_full_path)
+        expect(find('h1.project-title')).to have_content(project.name)
+      end
+
+      scenario 'the old project path redirects to the new path' do
+        update_path(new_group_path)
+        visit old_project_full_path
+        expect(current_path).to eq(new_project_full_path)
+        expect(find('h1.project-title')).to have_content(project.name)
+      end
+    end
+  end
+end
+
+def update_path(new_group_path)
+  visit edit_group_path(group)
+  fill_in 'group_path', with: new_group_path
+  click_button 'Save group'
+end

spec/features/profiles/account_spec.rb

+require 'rails_helper'
+
+feature 'Profile > Account', feature: true do
+  given(:user) { create(:user, username: 'foo') }
+
+  before do
+    login_as(user)
+  end
+
+  describe 'Change username' do
+    given(:new_username) { 'bar' }
+    given(:new_user_path) { "/#{new_username}" }
+    given(:old_user_path) { "/#{user.username}" }
+
+    scenario 'the user is accessible via the new path' do
+      update_username(new_username)
+      visit new_user_path
+      expect(current_path).to eq(new_user_path)
+      expect(find('.user-info')).to have_content(new_username)
+    end
+
+    scenario 'the old user path redirects to the new path' do
+      update_username(new_username)
+      visit old_user_path
+      expect(current_path).to eq(new_user_path)
+      expect(find('.user-info')).to have_content(new_username)
+    end
+
+    context 'with a project' do
+      given!(:project) { create(:project, namespace: user.namespace, path: 'project') }
+      given(:new_project_path) { "/#{new_username}/#{project.path}" }
+      given(:old_project_path) { "/#{user.username}/#{project.path}" }
+
+      before(:context) { TestEnv.clean_test_path }
+      after(:example) { TestEnv.clean_test_path }
+
+      scenario 'the project is accessible via the new path' do
+        update_username(new_username)
+        visit new_project_path
+        expect(current_path).to eq(new_project_path)
+        expect(find('h1.project-title')).to have_content(project.name)
+      end
+
+      scenario 'the old project path redirects to the new path' do
+        update_username(new_username)
+        visit old_project_path
+        expect(current_path).to eq(new_project_path)
+        expect(find('h1.project-title')).to have_content(project.name)
+      end
+    end
+  end
+end
+
+def update_username(new_username)
+  allow(user.namespace).to receive(:move_dir)
+  visit profile_account_path
+  fill_in 'user_username', with: new_username
+  click_button 'Update username'
+end

spec/features/projects/features_visibility_spec.rb

@@ -68,20 +68,23 @@ describe 'Edit Project Settings', feature: true do
   end
 
   describe 'project features visibility pages' do
-    before do
-      @tools =
-        {
-          builds: namespace_project_pipelines_path(project.namespace, project),
-          issues: namespace_project_issues_path(project.namespace, project),
-          wiki: namespace_project_wiki_path(project.namespace, project, :home),
-          snippets: namespace_project_snippets_path(project.namespace, project),
-          merge_requests: namespace_project_merge_requests_path(project.namespace, project),
-        }
+    let(:tools) do
+      {
+        builds: namespace_project_pipelines_path(project.namespace, project),
+        issues: namespace_project_issues_path(project.namespace, project),
+        wiki: namespace_project_wiki_path(project.namespace, project, :home),
+        snippets: namespace_project_snippets_path(project.namespace, project),
+        merge_requests: namespace_project_merge_requests_path(project.namespace, project),
+      }
     end
 
     context 'normal user' do
+      before do
+        login_as(member)
+      end
+
       it 'renders 200 if tool is enabled' do
-        @tools.each do |method_name, url|
+        tools.each do |method_name, url|
           project.project_feature.update_attribute("#{method_name}_access_level", ProjectFeature::ENABLED)
           visit url
           expect(page.status_code).to eq(200)
@@ -89,7 +92,7 @@ describe 'Edit Project Settings', feature: true do
       end
 
       it 'renders 404 if feature is disabled' do
-        @tools.each do |method_name, url|
+        tools.each do |method_name, url|
           project.project_feature.update_attribute("#{method_name}_access_level", ProjectFeature::DISABLED)
           visit url
           expect(page.status_code).to eq(404)
@@ -99,21 +102,21 @@ describe 'Edit Project Settings', feature: true do
       it 'renders 404 if feature is enabled only for team members' do
         project.team.truncate
 
-        @tools.each do |method_name, url|
+        tools.each do |method_name, url|
           project.project_feature.update_attribute("#{method_name}_access_level", ProjectFeature::PRIVATE)
           visit url
           expect(page.status_code).to eq(404)
         end
       end
 
-      it 'renders 200 if users is member of group' do
+      it 'renders 200 if user is member of group' do
         group = create(:group)
         project.group = group
         project.save
 
         group.add_owner(member)
 
-        @tools.each do |method_name, url|
+        tools.each do |method_name, url|
           project.project_feature.update_attribute("#{method_name}_access_level", ProjectFeature::PRIVATE)
           visit url
           expect(page.status_code).to eq(200)
@@ -128,7 +131,7 @@ describe 'Edit Project Settings', feature: true do
       end
 
       it 'renders 404 if feature is disabled' do
-        @tools.each do |method_name, url|
+        tools.each do |method_name, url|
           project.project_feature.update_attribute("#{method_name}_access_level", ProjectFeature::DISABLED)
           visit url
           expect(page.status_code).to eq(404)
@@ -138,7 +141,7 @@ describe 'Edit Project Settings', feature: true do
       it 'renders 200 if feature is enabled only for team members' do
         project.team.truncate
 
-        @tools.each do |method_name, url|
+        tools.each do |method_name, url|
           project.project_feature.update_attribute("#{method_name}_access_level", ProjectFeature::PRIVATE)
           visit url
           expect(page.status_code).to eq(200)

spec/features/projects/project_settings_spec.rb

 require 'spec_helper'
 
 describe 'Edit Project Settings', feature: true do
+  include Select2Helper
+
   let(:user) { create(:user) }
-  let(:project) { create(:empty_project, path: 'gitlab', name: 'sample') }
+  let(:project) { create(:empty_project, namespace: user.namespace, path: 'gitlab', name: 'sample') }
 
   before do
     login_as(user)
-    project.team << [user, :master]
   end
 
-  describe 'Project settings', js: true do
+  describe 'Project settings section', js: true do
     it 'shows errors for invalid project name' do
       visit edit_namespace_project_path(project.namespace, project)
-
       fill_in 'project_name_edit', with: 'foo&bar'
-
       click_button 'Save changes'
-
       expect(page).to have_field 'project_name_edit', with: 'foo&bar'
       expect(page).to have_content "Name can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'."
       expect(page).to have_button 'Save changes'
     end
 
-    scenario 'shows a successful notice when the project is updated' do
+    it 'shows a successful notice when the project is updated' do
       visit edit_namespace_project_path(project.namespace, project)
-
       fill_in 'project_name_edit', with: 'hello world'
-
       click_button 'Save changes'
-
       expect(page).to have_content "Project 'hello world' was successfully updated."
     end
   end
 
-  describe 'Rename repository' do
-    it 'shows errors for invalid project path/name' do
-      visit edit_namespace_project_path(project.namespace, project)
-
-      fill_in 'project_name', with: 'foo&bar'
-      fill_in 'Path', with: 'foo&bar'
+  describe 'Rename repository section' do
+    context 'with invalid characters' do
+      it 'shows errors for invalid project path/name' do
+        rename_project(project, name: 'foo&bar', path: 'foo&bar')
+        expect(page).to have_field 'Project name', with: 'foo&bar'
+        expect(page).to have_field 'Path', with: 'foo&bar'
+        expect(page).to have_content "Name can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'."
+        expect(page).to have_content "Path can contain only letters, digits, '_', '-' and '.'. Cannot start with '-', end in '.git' or end in '.atom'"
+      end
+    end
 
-      click_button 'Rename project'
+    context 'when changing project name' do
+      it 'renames the repository' do
+        rename_project(project, name: 'bar')
+        expect(find('h1.title')).to have_content(project.name)
+      end
+
+      context 'with emojis' do
+        it 'shows error for invalid project name' do
+          rename_project(project, name: '🚀 foo bar ☁️')
+          expect(page).to have_field 'Project name', with: '🚀 foo bar ☁️'
+          expect(page).not_to have_content "Name can contain only letters, digits, emojis '_', '.', dash and space. It must start with letter, digit, emoji or '_'."
+        end
+      end
+    end
 
-      expect(page).to have_field 'Project name', with: 'foo&bar'
-      expect(page).to have_field 'Path', with: 'foo&bar'
-      expect(page).to have_content "Name can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'."
-      expect(page).to have_content "Path can contain only letters, digits, '_', '-' and '.'. Cannot start with '-', end in '.git' or end in '.atom'"
+    context 'when changing project path' do
+      # Not using empty project because we need a repo to exist
+      let(:project) { create(:project, namespace: user.namespace, name: 'gitlabhq') }
+
+      before(:context) { TestEnv.clean_test_path }
+      after(:example) { TestEnv.clean_test_path }
+
+      specify 'the project is accessible via the new path' do
+        rename_project(project, path: 'bar')
+        new_path = namespace_project_path(project.namespace, 'bar')
+        visit new_path
+        expect(current_path).to eq(new_path)
+        expect(find('h1.title')).to have_content(project.name)
+      end
+
+      specify 'the project is accessible via a redirect from the old path' do
+        old_path = namespace_project_path(project.namespace, project)
+        rename_project(project, path: 'bar')
+        new_path = namespace_project_path(project.namespace, 'bar')
+        visit old_path
+        expect(current_path).to eq(new_path)
+        expect(find('h1.title')).to have_content(project.name)
+      end
+
+      context 'and a new project is added with the same path' do
+        it 'overrides the redirect' do
+          old_path = namespace_project_path(project.namespace, project)
+          rename_project(project, path: 'bar')
+          new_project = create(:empty_project, namespace: user.namespace, path: 'gitlabhq', name: 'quz')
+          visit old_path
+          expect(current_path).to eq(old_path)
+          expect(find('h1.title')).to have_content(new_project.name)
+        end
+      end
     end
   end
 
-  describe 'Rename repository name with emojis' do
-    it 'shows error for invalid project name' do
-      visit edit_namespace_project_path(project.namespace, project)
-
-      fill_in 'project_name', with: '🚀 foo bar ☁️'
+  describe 'Transfer project section', js: true do
+    # Not using empty project because we need a repo to exist
+    let!(:project) { create(:project, namespace: user.namespace, name: 'gitlabhq') }
+    let!(:group) { create(:group) }
+
+    before(:context) { TestEnv.clean_test_path }
+    before(:example) { group.add_owner(user) }
+    after(:example) { TestEnv.clean_test_path }
+
+    specify 'the project is accessible via the new path' do
+      transfer_project(project, group)
+      new_path = namespace_project_path(group, project)
+      visit new_path
+      expect(current_path).to eq(new_path)
+      expect(find('h1.title')).to have_content(project.name)
+    end
 
-      click_button 'Rename project'
+    specify 'the project is accessible via a redirect from the old path' do
+      old_path = namespace_project_path(project.namespace, project)
+      transfer_project(project, group)
+      new_path = namespace_project_path(group, project)
+      visit old_path
+      expect(current_path).to eq(new_path)
+      expect(find('h1.title')).to have_content(project.name)
+    end
 
-      expect(page).to have_field 'Project name', with: '🚀 foo bar ☁️'
-      expect(page).not_to have_content "Name can contain only letters, digits, emojis '_', '.', dash and space. It must start with letter, digit, emoji or '_'."
+    context 'and a new project is added with the same path' do
+      it 'overrides the redirect' do
+        old_path = namespace_project_path(project.namespace, project)
+        transfer_project(project, group)
+        new_project = create(:empty_project, namespace: user.namespace, path: 'gitlabhq', name: 'quz')
+        visit old_path
+        expect(current_path).to eq(old_path)
+        expect(find('h1.title')).to have_content(new_project.name)
+      end
     end
   end
 end
+
+def rename_project(project, name: nil, path: nil)
+  visit edit_namespace_project_path(project.namespace, project)
+  fill_in('project_name', with: name) if name
+  fill_in('Path', with: path) if path
+  click_button('Rename project')
+  wait_for_edit_project_page_reload
+  project.reload
+end
+
+def transfer_project(project, namespace)
+  visit edit_namespace_project_path(project.namespace, project)
+  select2(namespace.id, from: '#new_namespace_id')
+  click_button('Transfer project')
+  confirm_transfer_modal
+  wait_for_edit_project_page_reload
+  project.reload
+end
+
+def confirm_transfer_modal
+  fill_in('confirm_name_input', with: project.path)
+  click_button 'Confirm'
+end
+
+def wait_for_edit_project_page_reload
+  expect(find('.project-edit-container')).to have_content('Rename repository')
+end

spec/lib/constraints/group_url_constrainer_spec.rb

@@ -29,9 +29,37 @@ describe GroupUrlConstrainer, lib: true do
 
       it { expect(subject.matches?(request)).to be_falsey }
     end
+
+    context 'when the request matches a redirect route' do
+      context 'for a root group' do
+        let!(:redirect_route) { group.redirect_routes.create!(path: 'gitlabb') }
+
+        context 'and is a GET request' do
+          let(:request) { build_request(redirect_route.path) }
+
+          it { expect(subject.matches?(request)).to be_truthy }
+        end
+
+        context 'and is NOT a GET request' do
+          let(:request) { build_request(redirect_route.path, 'POST') }
+
+          it { expect(subject.matches?(request)).to be_falsey }
+        end
+      end
+
+      context 'for a nested group' do
+        let!(:nested_group) { create(:group, path: 'nested', parent: group) }
+        let!(:redirect_route) { nested_group.redirect_routes.create!(path: 'gitlabb/nested') }
+        let(:request) { build_request(redirect_route.path) }
+
+        it { expect(subject.matches?(request)).to be_truthy }
+      end
+    end
   end
 
-  def build_request(path)
-    double(:request, params: { id: path })
+  def build_request(path, method = 'GET')
+    double(:request,
+      'get?': (method == 'GET'),
+      params: { id: path })
   end
 end

spec/lib/constraints/project_url_constrainer_spec.rb

@@ -24,9 +24,26 @@ describe ProjectUrlConstrainer, lib: true do
         it { expect(subject.matches?(request)).to be_falsey }
       end
     end
+
+    context 'when the request matches a redirect route' do
+      let(:old_project_path) { 'old_project_path' }
+      let!(:redirect_route) { project.redirect_routes.create!(path: "#{namespace.full_path}/#{old_project_path}") }
+
+      context 'and is a GET request' do
+        let(:request) { build_request(namespace.full_path, old_project_path) }
+        it { expect(subject.matches?(request)).to be_truthy }
+      end
+
+      context 'and is NOT a GET request' do
+        let(:request) { build_request(namespace.full_path, old_project_path, 'POST') }
+        it { expect(subject.matches?(request)).to be_falsey }
+      end
+    end
   end
 
-  def build_request(namespace, project)
-    double(:request, params: { namespace_id: namespace, id: project })
+  def build_request(namespace, project, method = 'GET')
+    double(:request,
+      'get?': (method == 'GET'),
+      params: { namespace_id: namespace, id: project })
   end
 end

spec/lib/constraints/user_url_constrainer_spec.rb

@@ -15,9 +15,26 @@ describe UserUrlConstrainer, lib: true do
 
       it { expect(subject.matches?(request)).to be_falsey }
     end
+
+    context 'when the request matches a redirect route' do
+      let(:old_project_path) { 'old_project_path' }
+      let!(:redirect_route) { user.namespace.redirect_routes.create!(path: 'foo') }
+
+      context 'and is a GET request' do
+        let(:request) { build_request(redirect_route.path) }
+        it { expect(subject.matches?(request)).to be_truthy }
+      end
+
+      context 'and is NOT a GET request' do
+        let(:request) { build_request(redirect_route.path, 'POST') }
+        it { expect(subject.matches?(request)).to be_falsey }
+      end
+    end
   end
 
-  def build_request(username)
-    double(:request, params: { username: username })
+  def build_request(username, method = 'GET')
+    double(:request,
+      'get?': (method == 'GET'),
+      params: { username: username })
   end
 end

spec/lib/gitlab/import_export/all_models.yml

@@ -244,6 +244,7 @@ project:
 - path_locks
 - approver_groups
 - route
+- redirect_routes
 - statistics
 - container_repositories
 - uploads

spec/models/concerns/routable_spec.rb

@@ -9,6 +9,7 @@ describe Group, 'Routable' do
 
   describe 'Associations' do
     it { is_expected.to have_one(:route).dependent(:destroy) }
+    it { is_expected.to have_many(:redirect_routes).dependent(:destroy) }
   end
 
   describe 'Callbacks' do
@@ -35,10 +36,53 @@ describe Group, 'Routable' do
   describe '.find_by_full_path' do
     let!(:nested_group) { create(:group, parent: group) }
 
-    it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) }
-    it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) }
-    it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) }
-    it { expect(described_class.find_by_full_path('unknown')).to eq(nil) }
+    context 'without any redirect routes' do
+      it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) }
+      it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) }
+      it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) }
+      it { expect(described_class.find_by_full_path('unknown')).to eq(nil) }
+    end
+
+    context 'with redirect routes' do
+      let!(:group_redirect_route) { group.redirect_routes.create!(path: 'bar') }
+      let!(:nested_group_redirect_route) { nested_group.redirect_routes.create!(path: nested_group.path.sub('foo', 'bar')) }
+
+      context 'without follow_redirects option' do
+        context 'with the given path not matching any route' do
+          it { expect(described_class.find_by_full_path('unknown')).to eq(nil) }
+        end
+
+        context 'with the given path matching the canonical route' do
+          it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) }
+          it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) }
+          it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) }
+        end
+
+        context 'with the given path matching a redirect route' do
+          it { expect(described_class.find_by_full_path(group_redirect_route.path)).to eq(nil) }
+          it { expect(described_class.find_by_full_path(group_redirect_route.path.upcase)).to eq(nil) }
+          it { expect(described_class.find_by_full_path(nested_group_redirect_route.path)).to eq(nil) }
+        end
+      end
+
+      context 'with follow_redirects option set to true' do
+        context 'with the given path not matching any route' do
+          it { expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to eq(nil) }
+        end
+
+        context 'with the given path matching the canonical route' do
+          it { expect(described_class.find_by_full_path(group.to_param, follow_redirects: true)).to eq(group) }
+          it { expect(described_class.find_by_full_path(group.to_param.upcase, follow_redirects: true)).to eq(group) }
+          it { expect(described_class.find_by_full_path(nested_group.to_param, follow_redirects: true)).to eq(nested_group) }
+        end
+
+        context 'with the given path matching a redirect route' do
+          it { expect(described_class.find_by_full_path(group_redirect_route.path, follow_redirects: true)).to eq(group) }
+          it { expect(described_class.find_by_full_path(group_redirect_route.path.upcase, follow_redirects: true)).to eq(group) }
+          it { expect(described_class.find_by_full_path(nested_group_redirect_route.path, follow_redirects: true)).to eq(nested_group) }
+        end
+      end
+    end
   end
 
   describe '.where_full_path_in' do

spec/models/redirect_route_spec.rb

+require 'rails_helper'
+
+describe RedirectRoute, models: true do
+  let(:group) { create(:group) }
+  let!(:redirect_route) { group.redirect_routes.create(path: 'gitlabb') }
+
+  describe 'relationships' do
+    it { is_expected.to belong_to(:source) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:source) }
+    it { is_expected.to validate_presence_of(:path) }
+    it { is_expected.to validate_uniqueness_of(:path) }
+  end
+
+  describe '.matching_path_and_descendants' do
+    let!(:redirect2) { group.redirect_routes.create(path: 'gitlabb/test') }
+    let!(:redirect3) { group.redirect_routes.create(path: 'gitlabb/test/foo') }
+    let!(:redirect4) { group.redirect_routes.create(path: 'gitlabb/test/foo/bar') }
+    let!(:redirect5) { group.redirect_routes.create(path: 'gitlabb/test/baz') }
+
+    it 'returns correct routes' do
+      expect(RedirectRoute.matching_path_and_descendants('gitlabb/test')).to match_array([redirect2, redirect3, redirect4, redirect5])
+    end
+  end
+end

spec/models/route_spec.rb

 require 'spec_helper'
 
 describe Route, models: true do
-  let!(:group) { create(:group, path: 'git_lab', name: 'git_lab') }
-  let!(:route) { group.route }
+  let(:group) { create(:group, path: 'git_lab', name: 'git_lab') }
+  let(:route) { group.route }
 
   describe 'relationships' do
     it { is_expected.to belong_to(:source) }
   end
 
   describe 'validations' do
+    before { route }
     it { is_expected.to validate_presence_of(:source) }
     it { is_expected.to validate_presence_of(:path) }
     it { is_expected.to validate_uniqueness_of(:path) }
   end
 
+  describe 'callbacks' do
+    context 'after update' do
+      it 'calls #create_redirect_for_old_path' do
+        expect(route).to receive(:create_redirect_for_old_path)
+        route.update_attributes(path: 'foo')
+      end
+
+      it 'calls #delete_conflicting_redirects' do
+        expect(route).to receive(:delete_conflicting_redirects)
+        route.update_attributes(path: 'foo')
+      end
+    end
+
+    context 'after create' do
+      it 'calls #delete_conflicting_redirects' do
+        route.destroy
+        new_route = Route.new(source: group, path: group.path)
+        expect(new_route).to receive(:delete_conflicting_redirects)
+        new_route.save!
+      end
+    end
+  end
+
   describe '.inside_path' do
     let!(:nested_group) { create(:group, path: 'test', name: 'test', parent: group) }
     let!(:deep_nested_group) { create(:group, path: 'foo', name: 'foo', parent: nested_group) }
@@ -37,7 +61,7 @@ describe Route, models: true do
       context 'when route name is set' do
         before { route.update_attributes(path: 'bar') }
 
-        it "updates children routes with new path" do
+        it 'updates children routes with new path' do
           expect(described_class.exists?(path: 'bar')).to be_truthy
           expect(described_class.exists?(path: 'bar/test')).to be_truthy
           expect(described_class.exists?(path: 'bar/test/foo')).to be_truthy
@@ -56,10 +80,24 @@ describe Route, models: true do
           expect(route.update_attributes(path: 'bar')).to be_truthy
         end
       end
+
+      context 'when conflicting redirects exist' do
+        let!(:conflicting_redirect1) { route.create_redirect('bar/test') }
+        let!(:conflicting_redirect2) { route.create_redirect('bar/test/foo') }
+        let!(:conflicting_redirect3) { route.create_redirect('gitlab-org') }
+
+        it 'deletes the conflicting redirects' do
+          route.update_attributes(path: 'bar')
+
+          expect(RedirectRoute.exists?(path: 'bar/test')).to be_falsey
+          expect(RedirectRoute.exists?(path: 'bar/test/foo')).to be_falsey
+          expect(RedirectRoute.exists?(path: 'gitlab-org')).to be_truthy
+        end
+      end
     end
 
     context 'name update' do
-      it "updates children routes with new path" do
+      it 'updates children routes with new path' do
         route.update_attributes(name: 'bar')
 
         expect(described_class.exists?(name: 'bar')).to be_truthy
@@ -77,4 +115,72 @@ describe Route, models: true do
       end
     end
   end
+
+  describe '#create_redirect_for_old_path' do
+    context 'if the path changed' do
+      it 'creates a RedirectRoute for the old path' do
+        redirect_scope = route.source.redirect_routes.where(path: 'git_lab')
+        expect(redirect_scope.exists?).to be_falsey
+        route.path = 'new-path'
+        route.save!
+        expect(redirect_scope.exists?).to be_truthy
+      end
+    end
+  end
+
+  describe '#create_redirect' do
+    it 'creates a RedirectRoute with the same source' do
+      redirect_route = route.create_redirect('foo')
+      expect(redirect_route).to be_a(RedirectRoute)
+      expect(redirect_route).to be_persisted
+      expect(redirect_route.source).to eq(route.source)
+      expect(redirect_route.path).to eq('foo')
+    end
+  end
+
+  describe '#delete_conflicting_redirects' do
+    context 'when a redirect route with the same path exists' do
+      let!(:redirect1) { route.create_redirect(route.path) }
+
+      it 'deletes the redirect' do
+        route.delete_conflicting_redirects
+        expect(route.conflicting_redirects).to be_empty
+      end
+
+      context 'when redirect routes with paths descending from the route path exists' do
+        let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
+        let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
+        let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
+        let!(:other_redirect) { route.create_redirect("other") }
+
+        it 'deletes all redirects with paths that descend from the route path' do
+          route.delete_conflicting_redirects
+          expect(route.conflicting_redirects).to be_empty
+        end
+      end
+    end
+  end
+
+  describe '#conflicting_redirects' do
+    context 'when a redirect route with the same path exists' do
+      let!(:redirect1) { route.create_redirect(route.path) }
+
+      it 'returns the redirect route' do
+        expect(route.conflicting_redirects).to be_an(ActiveRecord::Relation)
+        expect(route.conflicting_redirects).to match_array([redirect1])
+      end
+
+      context 'when redirect routes with paths descending from the route path exists' do
+        let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
+        let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
+        let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
+        let!(:other_redirect) { route.create_redirect("other") }
+
+        it 'returns the redirect routes' do
+          expect(route.conflicting_redirects).to be_an(ActiveRecord::Relation)
+          expect(route.conflicting_redirects).to match_array([redirect1, redirect2, redirect3, redirect4])
+        end
+      end
+    end
+  end
 end

spec/models/user_spec.rb

@@ -873,6 +873,65 @@ describe User, models: true do
     end
   end
 
+  describe '.find_by_full_path' do
+    let!(:user) { create(:user) }
+
+    context 'with a route matching the given path' do
+      let!(:route) { user.namespace.route }
+
+      it 'returns the user' do
+        expect(User.find_by_full_path(route.path)).to eq(user)
+      end
+
+      it 'is case-insensitive' do
+        expect(User.find_by_full_path(route.path.upcase)).to eq(user)
+        expect(User.find_by_full_path(route.path.downcase)).to eq(user)
+      end
+    end
+
+    context 'with a redirect route matching the given path' do
+      let!(:redirect_route) { user.namespace.redirect_routes.create(path: 'foo') }
+
+      context 'without the follow_redirects option' do
+        it 'returns nil' do
+          expect(User.find_by_full_path(redirect_route.path)).to eq(nil)
+        end
+      end
+
+      context 'with the follow_redirects option set to true' do
+        it 'returns the user' do
+          expect(User.find_by_full_path(redirect_route.path, follow_redirects: true)).to eq(user)
+        end
+
+        it 'is case-insensitive' do
+          expect(User.find_by_full_path(redirect_route.path.upcase, follow_redirects: true)).to eq(user)
+          expect(User.find_by_full_path(redirect_route.path.downcase, follow_redirects: true)).to eq(user)
+        end
+      end
+    end
+
+    context 'without a route or a redirect route matching the given path' do
+      context 'without the follow_redirects option' do
+        it 'returns nil' do
+          expect(User.find_by_full_path('unknown')).to eq(nil)
+        end
+      end
+      context 'with the follow_redirects option set to true' do
+        it 'returns nil' do
+          expect(User.find_by_full_path('unknown', follow_redirects: true)).to eq(nil)
+        end
+      end
+    end
+
+    context 'with a group route matching the given path' do
+      let!(:group) { create(:group, path: 'group_path') }
+
+      it 'returns nil' do
+        expect(User.find_by_full_path('group_path')).to eq(nil)
+      end
+    end
+  end
+
   describe 'all_ssh_keys' do
     it { is_expected.to have_many(:keys).dependent(:destroy) }
 

spec/routing/project_routing_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe 'project routing' do
   before do
     allow(Project).to receive(:find_by_full_path).and_return(false)
-    allow(Project).to receive(:find_by_full_path).with('gitlab/gitlabhq').and_return(true)
+    allow(Project).to receive(:find_by_full_path).with('gitlab/gitlabhq', any_args).and_return(true)
   end
 
   # Shared examples for a resource inside a Project
@@ -93,13 +93,13 @@ describe 'project routing' do
       end
 
       context 'name with dot' do
-        before { allow(Project).to receive(:find_by_full_path).with('gitlab/gitlabhq.keys').and_return(true) }
+        before { allow(Project).to receive(:find_by_full_path).with('gitlab/gitlabhq.keys', any_args).and_return(true) }
 
         it { expect(get('/gitlab/gitlabhq.keys')).to route_to('projects#show', namespace_id: 'gitlab', id: 'gitlabhq.keys') }
       end
 
       context 'with nested group' do
-        before { allow(Project).to receive(:find_by_full_path).with('gitlab/subgroup/gitlabhq').and_return(true) }
+        before { allow(Project).to receive(:find_by_full_path).with('gitlab/subgroup/gitlabhq', any_args).and_return(true) }
 
         it { expect(get('/gitlab/subgroup/gitlabhq')).to route_to('projects#show', namespace_id: 'gitlab/subgroup', id: 'gitlabhq') }
       end