Merge branch 'fix-404-empty-repo' into 'master'

Fix 404 error in files view after deleting the last file in a repository

Here's the logic:

1. In `TreeController`, `require_non_empty_project` will prevent `show` from being called if the project is  empty. That means all calls to `show` will be guaranteed to have at least 1 commit.
2. If the ref name is not valid, then return a 404. This ensures that at least the controller is looking at a valid branch/tag/SHA ID.
3. This leaves a number of cases:

```
3a. Valid ref, valid directory
3b. Valid ref, valid filename
3c. Valid ref, invalid path
3d. Valid ref, no files
```

Case 3a: The tree will not be `empty?` and will pass through the whole function.

Case 3b: The tree will be `empty?` but the `blob_at` will resolve properly and trigger a redirect to the file.

Case 3c: In this case, a path is given. Return 404 if it cannot be resolved neither as a tree nor a blob.

Case 3d: In this case, no path is given. If the tree is empty, this means it's an empty branch and just fall through.

Example broken branch: https://gitlab.com/gitlab-org/gitlab-test/tree/empty-branch

Closes #1362

See merge request !1010

CHANGELOG

@@ -2,6 +2,8 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.14.0 (unreleased)
   - Fix full screen mode for snippet comments (Daniel Gerhardt)
+  - Fix 404 error in files view after deleting the last file in a repository (Stan Hu)
+  - Remove repository graph log to fix slow cache updates after push event (Stan Hu)
   - Fix label read access for unauthenticated users (Daniel Gerhardt)
   - Fix access to disabled features for unauthenticated users (Daniel Gerhardt)
   - Fix OAuth provider bug where GitLab would not go return to the redirect_uri after sign-in (Stan Hu)

app/controllers/projects/tree_controller.rb

@@ -7,13 +7,15 @@ class Projects::TreeController < Projects::ApplicationController
   before_action :authorize_download_code!
 
   def show
+    return not_found! unless @repository.commit(@ref)
+
     if tree.entries.empty?
       if @repository.blob_at(@commit.id, @path)
         redirect_to(
           namespace_project_blob_path(@project.namespace, @project,
                                       File.join(@ref, @path))
         ) and return
-      else
+      elsif @path.present?
         return not_found!
       end
     end

spec/controllers/projects/tree_controller_spec.rb

@@ -8,9 +8,6 @@ describe Projects::TreeController do
     sign_in(user)
 
     project.team << [user, :master]
-
-    allow(project).to receive(:branches).and_return(['master', 'foo/bar/baz'])
-    allow(project).to receive(:tags).and_return(['v1.0.0', 'v2.0.0'])
     controller.instance_variable_set(:@project, project)
   end
 
@@ -44,6 +41,32 @@ describe Projects::TreeController do
       let(:id) { 'invalid-branch/encoding/' }
       it { is_expected.to respond_with(:not_found) }
     end
+
+    context "valid empty branch, invalid path" do
+      let(:id) { 'empty-branch/invalid-path/' }
+      it { is_expected.to respond_with(:not_found) }
+    end
+
+    context "valid empty branch" do
+      let(:id) { 'empty-branch' }
+      it { is_expected.to respond_with(:success) }
+    end
+
+    context "invalid SHA commit ID" do
+      let(:id) { 'ff39438/.gitignore' }
+      it { is_expected.to respond_with(:not_found) }
+    end
+
+    context "valid SHA commit ID" do
+      let(:id) { '6d39438' }
+      it { is_expected.to respond_with(:success) }
+    end
+
+    context "valid SHA commit ID with path" do
+      let(:id) { '6d39438/.gitignore' }
+      it { expect(response.status).to eq(302) }
+    end
+
   end
 
   describe 'GET show with blob path' do

spec/requests/api/branches_spec.rb

@@ -13,11 +13,18 @@ describe API::API, api: true  do
   let!(:branch_sha) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' }
 
   describe "GET /projects/:id/repository/branches" do
+    before do
+      # Ensure that repository.branch_names is cleared from the cache at start to ensure
+      # the list matches reality
+      Rails.cache.clear
+    end
+
     it "should return an array of project branches" do
       get api("/projects/#{project.id}/repository/branches", user)
       expect(response.status).to eq(200)
       expect(json_response).to be_an Array
-      expect(json_response.first['name']).to eq(project.repository.branch_names.first)
+      branch_names = json_response.map { |x| x['name'] }
+      expect(branch_names).to match_array(project.repository.branch_names)
     end
   end
 

spec/support/test_env.rb

@@ -5,6 +5,7 @@ module TestEnv
 
   # When developing the seed repository, comment out the branch you will modify.
   BRANCH_SHA = {
+    'empty-branch'     => '7efb185',
     'flatten-dir'      => 'e56497b',
     'feature'          => '0b4bc9a',
     'feature_conflict' => 'bb5206f',
@@ -14,9 +15,13 @@ module TestEnv
     'master'           => '5937ac0'
   }
 
-  FORKED_BRANCH_SHA = BRANCH_SHA.merge({
-    'add-submodule-version-bump' => '3f547c08'
-  })
+  # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily
+  # need to keep all the branches in sync.
+  # We currently only need a subset of the branches
+  FORKED_BRANCH_SHA = {
+    'add-submodule-version-bump' => '3f547c08',
+    'master' => '5937ac0'
+  }
 
   # Test environment
   #