Cache output of Repository#exists?

This caches the output of Repository#exists? in Redis while making sure
it's flushed properly when creating new repositories, deleting them,
etc.

For the ProjectWiki tests to work I had to make ProjectWiki#create_repo!
public as testing private methods in RSpec is a bit of a pain.

8 files changed, 130 insertions, 15 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 412c6c6732d..fd36dca0cad 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -876,6 +876,7 @@ class Project < ActiveRecord::Base
     # Forked import is handled asynchronously
     unless forked?
       if gitlab_shell.add_repository(path_with_namespace)
+        repository.after_create
         true
       else
         errors.add(:base, 'Failed to create repository via gitlab-shell')
diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb
index 59b1b86d1fb..7c1a61bb0bf 100644
--- a/app/models/project_wiki.rb
+++ b/app/models/project_wiki.rb
@@ -123,23 +123,27 @@ class ProjectWiki
   end
 
   def repository
-    Repository.new(path_with_namespace, @project)
+    @repository ||= Repository.new(path_with_namespace, @project)
   end
 
   def default_branch
     wiki.class.default_ref
   end
 
-  private
-
   def create_repo!
     if init_repo(path_with_namespace)
-      Gollum::Wiki.new(path_to_repo)
+      wiki = Gollum::Wiki.new(path_to_repo)
     else
       raise CouldNotCreateWikiError
     end
+
+    repository.after_create
+
+    wiki
   end
 
+  private
+
   def init_repo(path_with_namespace)
     gitlab_shell.add_repository(path_with_namespace)
   end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 25d24493f6e..13154eb4205 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -42,12 +42,15 @@ class Repository
   end
 
   def exists?
-    return false unless raw_repository
+    return @exists unless @exists.nil?
 
-    raw_repository.rugged
-    true
-  rescue Gitlab::Git::Repository::NoRepository
-    false
+    @exists = cache.fetch(:exists?) do
+      begin
+        raw_repository && raw_repository.rugged ? true : false
+      rescue Gitlab::Git::Repository::NoRepository
+        false
+      end
+    end
   end
 
   def empty?
@@ -320,12 +323,23 @@ class Repository
     @avatar = nil
   end
 
+  def expire_exists_cache
+    cache.expire(:exists?)
+    @exists = nil
+  end
+
+  # Runs code after a repository has been created.
+  def after_create
+    expire_exists_cache
+  end
+
   # Runs code just before a repository is deleted.
   def before_delete
     expire_cache if exists?
 
     expire_root_ref_cache
     expire_emptiness_caches
+    expire_exists_cache
   end
 
   # Runs code just before the HEAD of a repository is changed.
@@ -351,6 +365,7 @@ class Repository
   # Runs code after a repository has been forked/imported.
   def after_import
     expire_emptiness_caches
+    expire_exists_cache
   end
 
   # Runs code after a new commit has been pushed.
diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb
index 21d311579e3..f9e32337983 100644
--- a/app/workers/repository_fork_worker.rb
+++ b/app/workers/repository_fork_worker.rb
@@ -20,14 +20,15 @@ class RepositoryForkWorker
       return
     end
 
+    project.repository.after_import
+
     unless project.valid_repo?
-      logger.error("Project #{id} had an invalid repository after fork")
+      logger.error("Project #{project_id} had an invalid repository after fork")
       project.update(import_error: "The forked repository is invalid.")
       project.import_fail
       return
     end
 
-    project.repository.after_import
     project.import_finish
   end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index b8b9a455b83..624022c1dda 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -720,4 +720,45 @@ describe Project, models: true do
       expect(described_class.search_by_title('KITTENS')).to eq([project])
     end
   end
+
+  describe '#create_repository' do
+    let(:project) { create(:project) }
+    let(:shell) { Gitlab::Shell.new }
+
+    before do
+      allow(project).to receive(:gitlab_shell).and_return(shell)
+    end
+
+    context 'using a regular repository' do
+      it 'creates the repository' do
+        expect(shell).to receive(:add_repository).
+          with(project.path_with_namespace).
+          and_return(true)
+
+        expect(project.repository).to receive(:after_create)
+
+        expect(project.create_repository).to eq(true)
+      end
+
+      it 'adds an error if the repository could not be created' do
+        expect(shell).to receive(:add_repository).
+          with(project.path_with_namespace).
+          and_return(false)
+
+        expect(project.repository).not_to receive(:after_create)
+
+        expect(project.create_repository).to eq(false)
+        expect(project.errors).not_to be_empty
+      end
+    end
+
+    context 'using a forked repository' do
+      it 'does nothing' do
+        expect(project).to receive(:forked?).and_return(true)
+        expect(shell).not_to receive(:add_repository)
+
+        project.create_repository
+      end
+    end
+  end
 end
diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb
index a2085df5bcd..532e3f013fd 100644
--- a/spec/models/project_wiki_spec.rb
+++ b/spec/models/project_wiki_spec.rb
@@ -244,6 +244,18 @@ describe ProjectWiki, models: true do
     end
   end
 
+  describe '#create_repo!' do
+    it 'creates a repository' do
+      expect(subject).to receive(:init_repo).
+        with(subject.path_with_namespace).
+        and_return(true)
+
+      expect(subject.repository).to receive(:after_create)
+
+      expect(subject.create_repo!).to be_an_instance_of(Gollum::Wiki)
+    end
+  end
+
   private
 
   def create_temp_repo(path)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index a57229a4fdf..7eac70ae948 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -537,6 +537,12 @@ describe Repository, models: true do
 
         repository.before_delete
       end
+
+      it 'flushes the exists cache' do
+        expect(repository).to receive(:expire_exists_cache)
+
+        repository.before_delete
+      end
     end
 
     describe 'when a repository exists' do
@@ -593,6 +599,12 @@ describe Repository, models: true do
 
       repository.after_import
     end
+
+    it 'flushes the exists cache' do
+      expect(repository).to receive(:expire_exists_cache)
+
+      repository.after_import
+    end
   end
 
   describe '#after_push_commit' do
@@ -619,6 +631,14 @@ describe Repository, models: true do
     end
   end
 
+  describe '#after_create' do
+    it 'flushes the exists cache' do
+      expect(repository).to receive(:expire_exists_cache)
+
+      repository.after_create
+    end
+  end
+
   describe "#main_language" do
     it 'shows the main language of the project' do
       expect(repository.main_language).to eq("Ruby")
@@ -781,6 +801,16 @@ describe Repository, models: true do
     end
   end
 
+  describe '#expire_exists_cache' do
+    let(:cache) { repository.send(:cache) }
+
+    it 'expires the cache' do
+      expect(cache).to receive(:expire).with(:exists?)
+
+      repository.expire_exists_cache
+    end
+  end
+
   describe '#build_cache' do
     let(:cache) { repository.send(:cache) }
 
diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb
index 172537474ee..4ef05eb29d2 100644
--- a/spec/workers/repository_fork_worker_spec.rb
+++ b/spec/workers/repository_fork_worker_spec.rb
@@ -3,12 +3,17 @@ require 'spec_helper'
 describe RepositoryForkWorker do
   let(:project) { create(:project) }
   let(:fork_project) { create(:project, forked_from_project: project) }
+  let(:shell) { Gitlab::Shell.new }
 
   subject { RepositoryForkWorker.new }
 
+  before do
+    allow(subject).to receive(:gitlab_shell).and_return(shell)
+  end
+
   describe "#perform" do
     it "creates a new repository from a fork" do
-      expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).with(
+      expect(shell).to receive(:fork_repository).with(
         project.path_with_namespace,
         fork_project.namespace.path
       ).and_return(true)
@@ -19,20 +24,26 @@ describe RepositoryForkWorker do
         fork_project.namespace.path)
     end
 
-    it 'flushes the empty caches' do
-      expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).
+    it 'flushes various caches' do
+      expect(shell).to receive(:fork_repository).
         with(project.path_with_namespace, fork_project.namespace.path).
         and_return(true)
 
       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.perform(project.id, project.path_with_namespace,
                       fork_project.namespace.path)
     end
 
     it "handles bad fork" do
-      expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_return(false)
+      expect(shell).to receive(:fork_repository).and_return(false)
+
+      expect(subject.logger).to receive(:error)
+
       subject.perform(
         project.id,
         project.path_with_namespace,