Cache project avatars stored in Git

The avatar logic has been moved from Project to Repository as this makes
caching easier. The logic itself in turn has been changed so that the
logo file names are cached in Redis. This cache is flushed upon pushing
a commit but _only_ if:

1. The commit was pushed to the default branch
2. The commit actually changes any of the logo files

If no branch or commit is given the cache is flushed anyway, this
ensures that calling Repository#expire_cache without any arguments still
flushes the avatar cache (e.g. this is used when removing a project).

Fixes gitlab-org/gitlab-ce#14363

2 files changed, 85 insertions, 5 deletions

diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index fc2ab2d9931..536fe66b21b 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -597,9 +597,9 @@ describe Repository, models: true do
 
   describe '#after_push_commit' do
     it 'flushes the cache' do
-      expect(repository).to receive(:expire_cache).with('master')
+      expect(repository).to receive(:expire_cache).with('master', '123')
 
-      repository.after_push_commit('master')
+      repository.after_push_commit('master', '123')
     end
   end
 
@@ -703,4 +703,81 @@ describe Repository, models: true do
       repository.rm_tag('8.5')
     end
   end
+
+  describe '#avatar' do
+    it 'returns the first avatar file found in the repository' do
+      expect(repository).to receive(:blob_at_branch).
+        with('master', 'logo.png').
+        and_return(true)
+
+      expect(repository.avatar).to eq('logo.png')
+    end
+
+    it 'caches the output' do
+      allow(repository).to receive(:blob_at_branch).
+        with('master', 'logo.png').
+        and_return(true)
+
+      expect(repository.avatar).to eq('logo.png')
+
+      expect(repository).to_not receive(:blob_at_branch)
+      expect(repository.avatar).to eq('logo.png')
+    end
+  end
+
+  describe '#expire_avatar_cache' do
+    let(:cache) { repository.send(:cache) }
+
+    before do
+      allow(repository).to receive(:cache).and_return(cache)
+    end
+
+    context 'without a branch or revision' do
+      it 'flushes the cache' do
+        expect(cache).to receive(:expire).with(:avatar)
+
+        repository.expire_avatar_cache
+      end
+    end
+
+    context 'with a branch' do
+      it 'does not flush the cache if the branch is not the default branch' do
+        expect(cache).not_to receive(:expire)
+
+        repository.expire_avatar_cache('cats')
+      end
+
+      it 'flushes the cache if the branch equals the default branch' do
+        expect(cache).to receive(:expire).with(:avatar)
+
+        repository.expire_avatar_cache(repository.root_ref)
+      end
+    end
+
+    context 'with a branch and revision' do
+      let(:commit) { double(:commit) }
+
+      before do
+        allow(repository).to receive(:commit).and_return(commit)
+      end
+
+      it 'does not flush the cache if the commit does not change any logos' do
+        diff = double(:diff, new_path: 'test.txt')
+
+        expect(commit).to receive(:diffs).and_return([diff])
+        expect(cache).not_to receive(:expire)
+
+        repository.expire_avatar_cache(repository.root_ref, '123')
+      end
+
+      it 'flushes the cache if the commit changes any of the logos' do
+        diff = double(:diff, new_path: Repository::AVATAR_FILES[0])
+
+        expect(commit).to receive(:diffs).and_return([diff])
+        expect(cache).to receive(:expire).with(:avatar)
+
+        repository.expire_avatar_cache(repository.root_ref, '123')
+      end
+    end
+  end
 end
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 145bc937560..b49ca96e8e8 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -29,7 +29,8 @@ describe GitPushService, services: true do
       it { is_expected.to be_truthy }
 
       it 'flushes general cached data' do
-        expect(project.repository).to receive(:expire_cache).with('master')
+        expect(project.repository).to receive(:expire_cache).
+          with('master', newrev)
 
         subject
       end
@@ -46,7 +47,8 @@ describe GitPushService, services: true do
       it { is_expected.to be_truthy }
 
       it 'flushes general cached data' do
-        expect(project.repository).to receive(:expire_cache).with('master')
+        expect(project.repository).to receive(:expire_cache).
+          with('master', newrev)
 
         subject
       end
@@ -65,7 +67,8 @@ describe GitPushService, services: true do
       end
 
       it 'flushes general cached data' do
-        expect(project.repository).to receive(:expire_cache).with('master')
+        expect(project.repository).to receive(:expire_cache).
+          with('master', newrev)
 
         subject
       end