Merge branch 'fix/rugged-repo-error' into 'master'

Fix broken repo errors in the UI

This should prevent repo errors (or 404s) in the UI, together with https://gitlab.com/gitlab-org/gitlab_git/merge_requests/124

The `exists?` cache is now expired if the repo gets broken. 

Related MR: https://gitlab.com/gitlab-org/gitlab_git/merge_requests/124

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/20501

See merge request !6491

CHANGELOG

@@ -19,6 +19,7 @@ v 8.13.0 (unreleased)
   - Fix resolved discussion display in side-by-side diff view !6575
   - Optimize GitHub importing for speed and memory
   - API: expose pipeline data in builds API (!6502, Guilherme Salazar)
+  - Fix broken repository 500 errors in project list
 
 v 8.12.2 (unreleased)
   - Added University content to doc/university

Gemfile

@@ -51,7 +51,7 @@ gem 'browser', '~> 2.2'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem 'gitlab_git', '~> 10.6.6'
+gem 'gitlab_git', '~> 10.6.7'
 
 # LDAP Auth
 # GitLab fork with several improvements to original library. For full list of changes

Gemfile.lock

@@ -276,7 +276,7 @@ GEM
       diff-lcs (~> 1.1)
       mime-types (>= 1.16, < 3)
       posix-spawn (~> 0.3)
-    gitlab_git (10.6.6)
+    gitlab_git (10.6.7)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
       github-linguist (~> 4.7.0)
@@ -857,7 +857,7 @@ DEPENDENCIES
   github-linguist (~> 4.7.0)
   github-markup (~> 1.4)
   gitlab-flowdock-git-hook (~> 1.0.1)
-  gitlab_git (~> 10.6.6)
+  gitlab_git (~> 10.6.7)
   gitlab_omniauth-ldap (~> 1.2.1)
   gollum-lib (~> 4.2)
   gollum-rugged_adapter (~> 0.4.2)

app/controllers/projects_controller.rb

@@ -324,7 +324,12 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def repo_exists?
-    project.repository_exists? && !project.empty_repo?
+    project.repository_exists? && !project.empty_repo? && project.repo
+
+  rescue Gitlab::Git::Repository::NoRepository
+    project.repository.expire_exists_cache
+
+    false
   end
 
   def project_view_files?

app/services/projects/import_service.rb

@@ -44,6 +44,11 @@ module Projects
       begin
         gitlab_shell.import_repository(project.repository_storage_path, project.path_with_namespace, project.import_url)
       rescue => e
+        # Expire cache to prevent scenarios such as:
+        # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true
+        # 2. Retried import, repo is broken or not imported but +exists?+ still returns true
+        project.repository.before_import if project.repository_exists?
+
         raise Error,  "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}"
       end
     end

spec/controllers/projects_controller_spec.rb

@@ -63,6 +63,28 @@ describe ProjectsController do
       end
     end
 
+    context "project with broken repo" do
+      let(:empty_project) { create(:project_broken_repo, :public) }
+
+      before { sign_in(user) }
+
+      User.project_views.keys.each do |project_view|
+        context "with #{project_view} view set" do
+          before do
+            user.update_attributes(project_view: project_view)
+
+            get :show, namespace_id: empty_project.namespace.path, id: empty_project.path
+          end
+
+          it "renders the empty project view" do
+            allow(Project).to receive(:repo).and_raise(Gitlab::Git::Repository::NoRepository)
+
+            expect(response).to render_template('projects/no_repo')
+          end
+        end
+      end
+    end
+
     context "rendering default project view" do
       render_views
 

spec/factories/projects.rb

@@ -27,6 +27,14 @@ FactoryGirl.define do
       end
     end
 
+    trait :broken_repo do
+      after(:create) do |project|
+        project.create_repository
+
+        FileUtils.rm_r(File.join(project.repository_storage_path, "#{project.path_with_namespace}.git", 'refs'))
+      end
+    end
+
     # Nest Project Feature attributes
     transient do
       wiki_access_level ProjectFeature::ENABLED
@@ -56,6 +64,13 @@ FactoryGirl.define do
     empty_repo
   end
 
+  # Project with broken repository
+  #
+  # Project with an invalid repository state
+  factory :project_broken_repo, parent: :empty_project do
+    broken_repo
+  end
+
   # Project with test repository
   #
   # Test repository source can be found at

spec/services/projects/import_service_spec.rb

@@ -108,6 +108,16 @@ describe Projects::ImportService, services: true do
         expect(result[:status]).to eq :error
         expect(result[:message]).to eq 'Github: failed to connect API'
       end
+
+      it 'expires existence cache after error' do
+        allow_any_instance_of(Project).to receive(:repository_exists?).and_return(true)
+
+        expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).with(project.repository_storage_path, project.path_with_namespace, project.import_url).and_raise(Gitlab::Shell::Error.new('Failed to import the repository'))
+        expect_any_instance_of(Repository).to receive(:expire_emptiness_caches).and_call_original
+        expect_any_instance_of(Repository).to receive(:expire_exists_cache).and_call_original
+
+        subject.execute
+      end
     end
 
     def stub_github_omniauth_provider