From 643557dabccbb3a503b0867ae44ec5701759d2a8 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 20 Jul 2015 16:55:45 -0700 Subject: Fix 404 error in files view after deleting the last file in a repository Closes #1362 --- CHANGELOG | 1 + app/controllers/projects/tree_controller.rb | 4 +- spec/controllers/projects/tree_controller_spec.rb | 91 +++++++++++++++++++++++ spec/controllers/tree_controller_spec.rb | 68 ----------------- spec/requests/api/branches_spec.rb | 9 ++- spec/support/test_env.rb | 11 ++- 6 files changed, 111 insertions(+), 73 deletions(-) create mode 100644 spec/controllers/projects/tree_controller_spec.rb delete mode 100644 spec/controllers/tree_controller_spec.rb diff --git a/CHANGELOG b/CHANGELOG index 052f4865d69..038a4d95d7e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.14.0 (unreleased) + - 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) 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/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb new file mode 100644 index 00000000000..53915856357 --- /dev/null +++ b/spec/controllers/projects/tree_controller_spec.rb @@ -0,0 +1,91 @@ +require 'spec_helper' + +describe Projects::TreeController do + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + sign_in(user) + + project.team << [user, :master] + controller.instance_variable_set(:@project, project) + end + + describe "GET show" do + # Make sure any errors accessing the tree in our views bubble up to this spec + render_views + + before do + get(:show, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: id) + end + + context "valid branch, no path" do + let(:id) { 'master' } + it { is_expected.to respond_with(:success) } + end + + context "valid branch, valid path" do + let(:id) { 'master/encoding/' } + it { is_expected.to respond_with(:success) } + end + + context "valid branch, invalid path" do + let(:id) { 'master/invalid-path/' } + it { is_expected.to respond_with(:not_found) } + end + + context "invalid branch, valid path" 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 + render_views + + before do + get(:show, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: id) + end + + context 'redirect to blob' do + let(:id) { 'master/README.md' } + it 'redirects' do + redirect_url = "/#{project.path_with_namespace}/blob/master/README.md" + expect(subject). + to redirect_to(redirect_url) + end + end + end +end diff --git a/spec/controllers/tree_controller_spec.rb b/spec/controllers/tree_controller_spec.rb deleted file mode 100644 index e09caf5df13..00000000000 --- a/spec/controllers/tree_controller_spec.rb +++ /dev/null @@ -1,68 +0,0 @@ -require 'spec_helper' - -describe Projects::TreeController do - let(:project) { create(:project) } - let(:user) { create(:user) } - - before 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 - - describe "GET show" do - # Make sure any errors accessing the tree in our views bubble up to this spec - render_views - - before do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: id) - end - - context "valid branch, no path" do - let(:id) { 'master' } - it { is_expected.to respond_with(:success) } - end - - context "valid branch, valid path" do - let(:id) { 'master/encoding/' } - it { is_expected.to respond_with(:success) } - end - - context "valid branch, invalid path" do - let(:id) { 'master/invalid-path/' } - it { is_expected.to respond_with(:not_found) } - end - - context "invalid branch, valid path" do - let(:id) { 'invalid-branch/encoding/' } - it { is_expected.to respond_with(:not_found) } - end - end - - describe 'GET show with blob path' do - render_views - - before do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: id) - end - - context 'redirect to blob' do - let(:id) { 'master/README.md' } - it 'redirects' do - redirect_url = "/#{project.path_with_namespace}/blob/master/README.md" - expect(subject). - to redirect_to(redirect_url) - end - end - end -end 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 # -- cgit v1.2.1