Redirect to the new default branch from a deleted default branch

Renaming default branches is becoming more common; let's make it as
comfortable as we can. For now, only affect showing blobs and trees,
but we may weant to extend this to more actions in the future.

Changelog: added

app/controllers/projects/blob_controller.rb

@@ -23,6 +23,10 @@ class Projects::BlobController < Projects::ApplicationController
   # We need to assign the blob vars before `authorize_edit_tree!` so we can
   # validate access to a specific ref.
   before_action :assign_blob_vars
+
+  # Since BlobController doesn't use assign_ref_vars, we have to call this explicitly
+  before_action :rectify_renamed_default_branch!, only: [:show]
+
   before_action :authorize_edit_tree!, only: [:new, :create, :update, :destroy]
 
   before_action :commit, except: [:new, :create]
@@ -140,11 +144,15 @@ class Projects::BlobController < Projects::ApplicationController
   end
 
   def commit
-    @commit = @repository.commit(@ref)
+    @commit ||= @repository.commit(@ref)
 
     return render_404 unless @commit
   end
 
+  def redirect_renamed_default_branch?
+    action_name == 'show'
+  end
+
   def assign_blob_vars
     @id = params[:id]
     @ref, @path = extract_ref(@id)
@@ -152,6 +160,12 @@ class Projects::BlobController < Projects::ApplicationController
     render_404
   end
 
+  def rectify_renamed_default_branch!
+    @commit ||= @repository.commit(@ref)
+
+    super
+  end
+
   # rubocop: disable CodeReuse/ActiveRecord
   def after_edit_path
     from_merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).find_by(iid: params[:from_merge_request_iid])

app/controllers/projects/tree_controller.rb

@@ -39,6 +39,10 @@ class Projects::TreeController < Projects::ApplicationController
 
   private
 
+  def redirect_renamed_default_branch?
+    action_name == 'show'
+  end
+
   def assign_dir_vars
     @branch_name = params[:branch_name]
 

doc/user/project/repository/branches/default.md

@@ -152,6 +152,20 @@ renames a Git repository's (`example`) default branch.
 1. Update references to the old branch name in related code and scripts that reside outside
    your repository, such as helper utilities and integrations.
 
+## Default branch rename redirect
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/329100) in GitLab 14.1
+
+URLs for specific files or directories in a project embed the project's default
+branch name, and are often found in documentation or browser bookmarks. When you
+[update the default branch name in your repository](#update-the-default-branch-name-in-your-repository),
+these URLs change, and must be updated.
+
+To ease the transition period, whenever the default branch for a project is
+changed, GitLab records the name of the old default branch. If that branch is
+deleted, attempts to view a file or directory on it are redirected to the
+current default branch, instead of displaying the "not found" page.
+
 ## Resources
 
 - [Discussion of default branch renaming](https://lore.kernel.org/git/pull.656.v4.git.1593009996.gitgitgadget@gmail.com/)

lib/extracts_path.rb

@@ -26,17 +26,17 @@ module ExtractsPath
   # Automatically renders `not_found!` if a valid tree path could not be
   # resolved (e.g., when a user inserts an invalid path or ref).
   #
+  # Automatically redirects to the current default branch if the ref matches a
+  # previous default branch that has subsequently been deleted.
+  #
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   override :assign_ref_vars
   def assign_ref_vars
     super
 
-    if @path.empty? && !@commit && @id.ends_with?('.atom')
-      @id = @ref = extract_ref_without_atom(@id)
-      @commit = @repo.commit(@ref)
+    rectify_atom!
 
-      request.format = :atom if @commit
-    end
+    rectify_renamed_default_branch! && return
 
     raise InvalidPathError unless @commit
 
@@ -59,6 +59,42 @@ module ExtractsPath
 
   private
 
+  # Override in controllers to determine which actions are subject to the redirect
+  def redirect_renamed_default_branch?
+    false
+  end
+
+  # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  def rectify_atom!
+    return if @commit
+    return unless @id.ends_with?('.atom')
+    return unless @path.empty?
+
+    @id = @ref = extract_ref_without_atom(@id)
+    @commit = @repo.commit(@ref)
+
+    request.format = :atom if @commit
+  end
+  # rubocop:enable Gitlab/ModuleWithInstanceVariables
+
+  # For GET/HEAD requests, if the ref doesn't exist in the repository, check
+  # whether we're trying to access a renamed default branch. If we are, we can
+  # redirect to the current default branch instead of rendering a 404.
+  # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  def rectify_renamed_default_branch!
+    return unless redirect_renamed_default_branch?
+    return if @commit
+    return unless @id && @ref && repository_container.respond_to?(:previous_default_branch)
+    return unless repository_container.previous_default_branch == @ref
+    return unless request.get? || request.head?
+
+    flash[:notice] = _('The default branch for this project has been changed. Please update your bookmarks.')
+    redirect_to url_for(id: @id.sub(/\A#{Regexp.escape(@ref)}/, repository_container.default_branch))
+
+    true
+  end
+  # rubocop:enable Gitlab/ModuleWithInstanceVariables
+
   override :repository_container
   def repository_container
     @project

locale/gitlab.pot

@@ -32554,6 +32554,9 @@ msgstr ""
 msgid "The default CI/CD configuration file and path for new projects."
 msgstr ""
 
+msgid "The default branch for this project has been changed. Please update your bookmarks."
+msgstr ""
+
 msgid "The dependency list details information about the components used within your project."
 msgstr ""
 

spec/controllers/projects/blob_controller_spec.rb

@@ -5,7 +5,8 @@ require 'spec_helper'
 RSpec.describe Projects::BlobController do
   include ProjectForksHelper
 
-  let(:project) { create(:project, :public, :repository) }
+  let(:project) { create(:project, :public, :repository, previous_default_branch: previous_default_branch) }
+  let(:previous_default_branch) { nil }
 
   describe "GET show" do
     def request
@@ -42,6 +43,20 @@ RSpec.describe Projects::BlobController do
         it { is_expected.to respond_with(:not_found) }
       end
 
+      context "renamed default branch, valid file" do
+        let(:id) { 'old-default-branch/README.md' }
+        let(:previous_default_branch) { 'old-default-branch' }
+
+        it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/README.md") }
+      end
+
+      context "renamed default branch, invalid file" do
+        let(:id) { 'old-default-branch/invalid-path.rb' }
+        let(:previous_default_branch) { 'old-default-branch' }
+
+        it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/invalid-path.rb") }
+      end
+
       context "binary file" do
         let(:id) { 'binary-encoding/encoding/binary-1.bin' }
 

spec/controllers/projects/tree_controller_spec.rb

@@ -3,8 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe Projects::TreeController do
-  let(:project) { create(:project, :repository) }
-  let(:user)    { create(:user) }
+  let(:project) { create(:project, :repository, previous_default_branch: previous_default_branch) }
+  let(:previous_default_branch) { nil }
+  let(:user) { create(:user) }
 
   before do
     sign_in(user)
@@ -55,6 +56,20 @@ RSpec.describe Projects::TreeController do
       it { is_expected.to respond_with(:not_found) }
     end
 
+    context "renamed default branch, valid file" do
+      let(:id) { 'old-default-branch/encoding/' }
+      let(:previous_default_branch) { 'old-default-branch' }
+
+      it { is_expected.to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/encoding/") }
+    end
+
+    context "renamed default branch, invalid file" do
+      let(:id) { 'old-default-branch/invalid-path/' }
+      let(:previous_default_branch) { 'old-default-branch' }
+
+      it { is_expected.to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/invalid-path/") }
+    end
+
     context "valid empty branch, invalid path" do
       let(:id) { 'empty-branch/invalid-path/' }
 

spec/lib/extracts_path_spec.rb

@@ -7,10 +7,17 @@ RSpec.describe ExtractsPath do
   include RepoHelpers
   include Gitlab::Routing
 
+  # Make url_for work
+  def default_url_options
+    { controller: 'projects/blob', action: 'show', namespace_id: @project.namespace.path, project_id: @project.path }
+  end
+
   let_it_be(:owner) { create(:user) }
   let_it_be(:container) { create(:project, :repository, creator: owner) }
 
   let(:request) { double('request') }
+  let(:flash) { {} }
+  let(:redirect_renamed_default_branch?) { true }
 
   before do
     @project = container
@@ -18,11 +25,14 @@ RSpec.describe ExtractsPath do
 
     allow(container.repository).to receive(:ref_names).and_return(ref_names)
     allow(request).to receive(:format=)
+    allow(request).to receive(:get?)
+    allow(request).to receive(:head?)
   end
 
   describe '#assign_ref_vars' do
     let(:ref) { sample_commit[:id] }
-    let(:params) { { path: sample_commit[:line_code_path], ref: ref } }
+    let(:path) { sample_commit[:line_code_path] }
+    let(:params) { { path: path, ref: ref } }
 
     it_behaves_like 'assigns ref vars'
 
@@ -126,6 +136,66 @@ RSpec.describe ExtractsPath do
         expect(@commit).to be_nil
       end
     end
+
+    context 'ref points to a previous default branch' do
+      let(:ref) { 'develop' }
+
+      before do
+        @project.update!(previous_default_branch: ref)
+
+        allow(@project).to receive(:default_branch).and_return('foo')
+      end
+
+      it 'redirects to the new default branch for a GET request' do
+        allow(request).to receive(:get?).and_return(true)
+
+        expect(self).to receive(:redirect_to).with("http://localhost/#{@project.full_path}/-/blob/foo/#{path}")
+        expect(self).not_to receive(:render_404)
+
+        assign_ref_vars
+
+        expect(@commit).to be_nil
+        expect(flash[:notice]).to match(/default branch/)
+      end
+
+      it 'redirects to the new default branch for a HEAD request' do
+        allow(request).to receive(:head?).and_return(true)
+
+        expect(self).to receive(:redirect_to).with("http://localhost/#{@project.full_path}/-/blob/foo/#{path}")
+        expect(self).not_to receive(:render_404)
+
+        assign_ref_vars
+
+        expect(@commit).to be_nil
+        expect(flash[:notice]).to match(/default branch/)
+      end
+
+      it 'returns 404 for any other request type' do
+        expect(self).not_to receive(:redirect_to)
+        expect(self).to receive(:render_404)
+
+        assign_ref_vars
+
+        expect(@commit).to be_nil
+        expect(flash).to be_empty
+      end
+
+      context 'redirect behaviour is disabled' do
+        let(:redirect_renamed_default_branch?) { false }
+
+        it 'returns 404 for a GET request' do
+          allow(request).to receive(:get?).and_return(true)
+
+          expect(self).not_to receive(:redirect_to)
+          expect(self).to receive(:render_404)
+
+          assign_ref_vars
+
+          expect(@commit).to be_nil
+          expect(flash).to be_empty
+        end
+      end
+    end
   end
 
   it_behaves_like 'extracts refs'