summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2015-07-22 01:01:03 +0000
committerStan Hu <stanhu@gmail.com>2015-07-22 01:01:03 +0000
commitf593b621e3d4b3da31a2e48dd53a96d60dde6cd0 (patch)
treeb553985902225a726d011faea559fd83bdc4b56d
parent815cf7f84309f28af3561e1c9b49d1fa35719c7f (diff)
parent643557dabccbb3a503b0867ae44ec5701759d2a8 (diff)
downloadgitlab-ce-f593b621e3d4b3da31a2e48dd53a96d60dde6cd0.tar.gz
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
-rw-r--r--CHANGELOG2
-rw-r--r--app/controllers/projects/tree_controller.rb4
-rw-r--r--spec/controllers/projects/tree_controller_spec.rb (renamed from spec/controllers/tree_controller_spec.rb)29
-rw-r--r--spec/requests/api/branches_spec.rb9
-rw-r--r--spec/support/test_env.rb11
5 files changed, 47 insertions, 8 deletions
diff --git a/CHANGELOG b/CHANGELOG
index dab74b654b0..d26ac5bbfab 100644
--- a/CHANGELOG
+++ b/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)
diff --git a/app/controllers/projects/tree_controller.rb b/app/controllers/projects/tree_controller.rb
index b659e15f242..92e4bc16d9d 100644
--- a/app/controllers/projects/tree_controller.rb
+++ b/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
diff --git a/spec/controllers/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb
index e09caf5df13..53915856357 100644
--- a/spec/controllers/tree_controller_spec.rb
+++ b/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
diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb
index cb6e5e89625..bb3862c9eb2 100644
--- a/spec/requests/api/branches_spec.rb
+++ b/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
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 8bdd6b43cdd..dcf2a9e2ce5 100644
--- a/spec/support/test_env.rb
+++ b/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
#