Merge branch '44725-expire_correct_methods_after_change_head' into 'master'

Expire correct method caches after HEAD changed

Closes #44725

See merge request gitlab-org/gitlab-ce!20102

app/models/repository.rb

@@ -283,6 +283,10 @@ class Repository
     )
   end
 
+  def cached_methods
+    CACHED_METHODS
+  end
+
   def expire_tags_cache
     expire_method_caches(%i(tag_names tag_count))
     @tags = nil
@@ -423,7 +427,7 @@ class Repository
 
   # Runs code after the HEAD of a repository is changed.
   def after_change_head
-    expire_method_caches(METHOD_CACHES_FOR_FILE_TYPES.keys)
+    expire_all_method_caches
   end
 
   # Runs code after a repository has been forked/imported.

changelogs/unreleased/44725-expire_correct_methods_after_change_head.yml

+---
+title: Expire correct method caches after HEAD changed
+merge_request:
+author:
+type: fixed

lib/gitlab/repository_cache_adapter.rb

@@ -25,6 +25,11 @@ module Gitlab
       raise NotImplementedError
     end
 
+    # List of cached methods. Should be overridden by the including class
+    def cached_methods
+      raise NotImplementedError
+    end
+
     # Caches the supplied block both in a cache and in an instance variable.
     #
     # The cache key and instance variable are named the same way as the value of
@@ -67,6 +72,11 @@ module Gitlab
     # Expires the caches of a specific set of methods
     def expire_method_caches(methods)
       methods.each do |key|
+        unless cached_methods.include?(key.to_sym)
+          Rails.logger.error "Requested to expire non-existent method '#{key}' for Repository"
+          next
+        end
+
         cache.expire(key)
 
         ivar = cache_instance_variable_name(key)

spec/lib/gitlab/repository_cache_adapter_spec.rb

@@ -67,10 +67,18 @@ describe Gitlab::RepositoryCacheAdapter do
 
   describe '#expire_method_caches' do
     it 'expires the caches of the given methods' do
-      expect(cache).to receive(:expire).with(:readme)
+      expect(cache).to receive(:expire).with(:rendered_readme)
       expect(cache).to receive(:expire).with(:gitignore)
 
-      repository.expire_method_caches(%i(readme gitignore))
+      repository.expire_method_caches(%i(rendered_readme gitignore))
+    end
+
+    it 'does not expire caches for non-existent methods' do
+      expect(cache).not_to receive(:expire).with(:nonexistent)
+      expect(Rails.logger).to(
+        receive(:error).with("Requested to expire non-existent method 'nonexistent' for Repository"))
+
+      repository.expire_method_caches(%i(nonexistent))
     end
   end
 end

spec/models/repository_spec.rb

@@ -1689,19 +1689,29 @@ describe Repository do
   end
 
   describe '#after_change_head' do
-    it 'flushes the readme cache' do
+    it 'flushes the method caches' do
       expect(repository).to receive(:expire_method_caches).with([
-        :readme,
+        :size,
+        :commit_count,
+        :rendered_readme,
+        :contribution_guide,
         :changelog,
-        :license,
-        :contributing,
+        :license_blob,
+        :license_key,
         :gitignore,
-        :koding,
-        :gitlab_ci,
+        :koding_yml,
+        :gitlab_ci_yml,
+        :branch_names,
+        :tag_names,
+        :branch_count,
+        :tag_count,
         :avatar,
-        :issue_template,
-        :merge_request_template,
-        :xcode_config
+        :exists?,
+        :root_ref,
+        :has_visible_content?,
+        :issue_template_names,
+        :merge_request_template_names,
+        :xcode_project?
       ])
 
       repository.after_change_head