Merge branch 'expire-fork-import-caches' into 'master'

Expire caches after forking/importing a repository

cc @dblessing @DouweM 

Related issue: gitlab-org/gitlab-ce#13505

See merge request !2838

app/models/repository.rb

@@ -238,6 +238,15 @@ class Repository
     expire_branch_cache(branch_name)
   end
 
+  # Expires _all_ caches, including those that would normally only be expired
+  # under specific conditions.
+  def expire_all_caches!
+    expire_cache
+    expire_root_ref_cache
+    expire_emptiness_caches
+    expire_has_visible_content_cache
+  end
+
   def expire_branch_cache(branch_name = nil)
     # When we push to the root branch we have to flush the cache for all other
     # branches as their statistics are based on the commits relative to the
@@ -258,6 +267,14 @@ class Repository
     @root_ref = nil
   end
 
+  # Expires the cache(s) used to determine if a repository is empty or not.
+  def expire_emptiness_caches
+    cache.expire(:empty?)
+    @empty = nil
+
+    expire_has_visible_content_cache
+  end
+
   def expire_has_visible_content_cache
     cache.expire(:has_visible_content?)
     @has_visible_content = nil

app/services/projects/destroy_service.rb

@@ -16,11 +16,15 @@ module Projects
       return false unless can?(current_user, :remove_project, project)
 
       project.team.truncate
-      project.repository.expire_cache unless project.empty_repo?
 
       repo_path = project.path_with_namespace
       wiki_path = repo_path + '.wiki'
 
+      # Flush the cache for both repositories. This has to be done _before_
+      # removing the physical repositories as some expiration code depends on
+      # Git data (e.g. a list of branch names).
+      flush_caches(project, wiki_path)
+
       Project.transaction do
         project.destroy!
 
@@ -70,5 +74,13 @@ module Projects
     def removal_path(path)
       "#{path}+#{project.id}#{DELETED_FLAG}"
     end
+
+    def flush_caches(project, wiki_path)
+      project.repository.expire_all_caches! if project.repository.exists?
+
+      wiki_repo = Repository.new(wiki_path, project)
+
+      wiki_repo.expire_all_caches! if wiki_repo.exists?
+    end
   end
 end

app/workers/repository_fork_worker.rb

@@ -27,6 +27,7 @@ class RepositoryForkWorker
       return
     end
 
+    project.repository.expire_emptiness_caches
     project.import_finish
   end
 end

app/workers/repository_import_worker.rb

@@ -18,6 +18,7 @@ class RepositoryImportWorker
       return
     end
 
+    project.repository.expire_emptiness_caches
     project.import_finish
   end
 end

spec/models/repository_spec.rb

@@ -355,6 +355,17 @@ describe Repository, models: true do
     end
   end
 
+  describe '#expire_emptiness_caches' do
+    let(:cache) { repository.send(:cache) }
+
+    it 'expires the caches' do
+      expect(cache).to receive(:expire).with(:empty?)
+      expect(repository).to receive(:expire_has_visible_content_cache)
+
+      repository.expire_emptiness_caches
+    end
+  end
+
   describe :skip_merged_commit do
     subject { repository.commits(Gitlab::Git::BRANCH_REF_PREFIX + "'test'", nil, 100, 0, true).map{ |k| k.id } }
 

spec/workers/repository_fork_worker_spec.rb

@@ -19,6 +19,18 @@ describe RepositoryForkWorker do
         fork_project.namespace.path)
     end
 
+    it 'flushes the empty caches' do
+      expect_any_instance_of(Gitlab::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
+
+      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)
       subject.perform(

spec/workers/repository_import_worker_spec.rb

+require 'spec_helper'
+
+describe RepositoryImportWorker do
+  let(:project) { create(:project) }
+
+  subject { described_class.new }
+
+  describe '#perform' do
+    it 'imports a project' do
+      expect_any_instance_of(Projects::ImportService).to receive(:execute).
+        and_return({ status: :ok })
+
+      expect_any_instance_of(Repository).to receive(:expire_emptiness_caches)
+      expect_any_instance_of(Project).to receive(:import_finish)
+
+      subject.perform(project.id)
+    end
+  end
+end