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

3 files changed, 35 insertions, 8 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index ab4913e99a8..412c6c6732d 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -571,10 +571,7 @@ class Project < ActiveRecord::Base
   end
 
   def avatar_in_git
-    @avatar_file ||= 'logo.png' if repository.blob_at_branch('master', 'logo.png')
-    @avatar_file ||= 'logo.jpg' if repository.blob_at_branch('master', 'logo.jpg')
-    @avatar_file ||= 'logo.gif' if repository.blob_at_branch('master', 'logo.gif')
-    @avatar_file
+    repository.avatar
   end
 
   def avatar_url
diff --git a/app/models/repository.rb b/app/models/repository.rb
index e555e97689d..036919c27b2 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -3,6 +3,10 @@ require 'securerandom'
 class Repository
   class CommitError < StandardError; end
 
+  # Files to use as a project avatar in case no avatar was uploaded via the web
+  # UI.
+  AVATAR_FILES = %w{logo.png logo.jpg logo.gif}
+
   include Gitlab::ShellAdapter
 
   attr_accessor :path_with_namespace, :project
@@ -241,12 +245,13 @@ class Repository
     @branches = nil
   end
 
-  def expire_cache(branch_name = nil)
+  def expire_cache(branch_name = nil, revision = nil)
     cache_keys.each do |key|
       cache.expire(key)
     end
 
     expire_branch_cache(branch_name)
+    expire_avatar_cache(branch_name, revision)
 
     # This ensures this particular cache is flushed after the first commit to a
     # new repository.
@@ -316,6 +321,23 @@ class Repository
     cache.expire(:branch_names)
   end
 
+  def expire_avatar_cache(branch_name = nil, revision = nil)
+    # Avatars are pulled from the default branch, thus if somebody pushes to a
+    # different branch there's no need to expire anything.
+    return if branch_name && branch_name != root_ref
+
+    # We don't want to flush the cache if the commit didn't actually make any
+    # changes to any of the possible avatar files.
+    if revision && commit = self.commit(revision)
+      return unless commit.diffs.
+        any? { |diff| AVATAR_FILES.include?(diff.new_path) }
+    end
+
+    cache.expire(:avatar)
+
+    @avatar = nil
+  end
+
   # Runs code just before a repository is deleted.
   def before_delete
     expire_cache if exists?
@@ -350,8 +372,8 @@ class Repository
   end
 
   # Runs code after a new commit has been pushed.
-  def after_push_commit(branch_name)
-    expire_cache(branch_name)
+  def after_push_commit(branch_name, revision)
+    expire_cache(branch_name, revision)
   end
 
   # Runs code after a new branch has been created.
@@ -857,6 +879,14 @@ class Repository
     end
   end
 
+  def avatar
+    @avatar ||= cache.fetch(:avatar) do
+      AVATAR_FILES.find do |file|
+        blob_at_branch('master', file)
+      end
+    end
+  end
+
   private
 
   def cache
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index d840ab5e340..14e2a2c0699 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -17,7 +17,7 @@ class GitPushService < BaseService
   #  6. Checks if the project's main language has changed
   #
   def execute
-    @project.repository.after_push_commit(branch_name)
+    @project.repository.after_push_commit(branch_name, params[:newrev])
 
     if push_remove_branch?
       @project.repository.after_remove_branch