Merge branch 'cache-refactor' into 'master'

Cache `#can_be_resolved_in_ui?` git operations

Closes gitaly#1051

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

4 files changed, 147 insertions, 32 deletions

diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb
index 0a3ae2c3760..3ccfd9a739d 100644
--- a/lib/gitlab/conflict/file_collection.rb
+++ b/lib/gitlab/conflict/file_collection.rb
@@ -1,14 +1,18 @@
 module Gitlab
   module Conflict
     class FileCollection
+      include Gitlab::RepositoryCacheAdapter
+
       attr_reader :merge_request, :resolver
 
       def initialize(merge_request)
         our_commit = merge_request.source_branch_head.raw
         their_commit = merge_request.target_branch_head.raw
-        target_repo = merge_request.target_project.repository.raw
+        @target_repo = merge_request.target_project.repository
         @source_repo = merge_request.source_project.repository.raw
-        @resolver = Gitlab::Git::Conflict::Resolver.new(target_repo, our_commit.id, their_commit.id)
+        @our_commit_id = our_commit.id
+        @their_commit_id = their_commit.id
+        @resolver = Gitlab::Git::Conflict::Resolver.new(@target_repo.raw, @our_commit_id, @their_commit_id)
         @merge_request = merge_request
       end
 
@@ -30,6 +34,17 @@ module Gitlab
         end
       end
 
+      def can_be_resolved_in_ui?
+        # Try to parse each conflict. If the MR's mergeable status hasn't been
+        # updated, ensure that we don't say there are conflicts to resolve
+        # when there are no conflict files.
+        files.each(&:lines)
+        files.any?
+      rescue Gitlab::Git::CommandError, Gitlab::Git::Conflict::Parser::UnresolvableError, Gitlab::Git::Conflict::Resolver::ConflictSideMissing
+        false
+      end
+      cache_method :can_be_resolved_in_ui?
+
       def file_for_path(old_path, new_path)
         files.find { |file| file.their_path == old_path && file.our_path == new_path }
       end
@@ -56,6 +71,19 @@ Merge branch '#{merge_request.target_branch}' into '#{merge_request.source_branc
 #{conflict_filenames.join("\n")}
 EOM
       end
+
+      private
+
+      def cache
+        @cache ||= begin
+          # Use the commit ids as a namespace so if the MR branches get
+          # updated we instantiate the cache under a different namespace. That
+          # way don't have to worry about explicitly invalidating the cache
+          namespace = "#{@our_commit_id}:#{@their_commit_id}"
+
+          Gitlab::RepositoryCache.new(@target_repo, extra_namespace: namespace)
+        end
+      end
     end
   end
 end
diff --git a/lib/gitlab/repository_cache.rb b/lib/gitlab/repository_cache.rb
new file mode 100644
index 00000000000..b1bf3ca4143
--- /dev/null
+++ b/lib/gitlab/repository_cache.rb
@@ -0,0 +1,33 @@
+# Interface to the Redis-backed cache store
+module Gitlab
+  class RepositoryCache
+    attr_reader :repository, :namespace, :backend
+
+    def initialize(repository, extra_namespace: nil, backend: Rails.cache)
+      @repository = repository
+      @namespace = "#{repository.full_path}:#{repository.project.id}"
+      @namespace += ":#{extra_namespace}" if extra_namespace
+      @backend = backend
+    end
+
+    def cache_key(type)
+      "#{type}:#{namespace}"
+    end
+
+    def expire(key)
+      backend.delete(cache_key(key))
+    end
+
+    def fetch(key, &block)
+      backend.fetch(cache_key(key), &block)
+    end
+
+    def exist?(key)
+      backend.exist?(cache_key(key))
+    end
+
+    def read(key)
+      backend.read(cache_key(key))
+    end
+  end
+end
diff --git a/lib/gitlab/repository_cache_adapter.rb b/lib/gitlab/repository_cache_adapter.rb
new file mode 100644
index 00000000000..7f64a8c9e46
--- /dev/null
+++ b/lib/gitlab/repository_cache_adapter.rb
@@ -0,0 +1,84 @@
+module Gitlab
+  module RepositoryCacheAdapter
+    extend ActiveSupport::Concern
+
+    class_methods do
+      # Wraps around the given method and caches its output in Redis and an instance
+      # variable.
+      #
+      # This only works for methods that do not take any arguments.
+      def cache_method(name, fallback: nil, memoize_only: false)
+        original = :"_uncached_#{name}"
+
+        alias_method(original, name)
+
+        define_method(name) do
+          cache_method_output(name, fallback: fallback, memoize_only: memoize_only) do
+            __send__(original) # rubocop:disable GitlabSecurity/PublicSend
+          end
+        end
+      end
+    end
+
+    # RepositoryCache to be used. Should be overridden by the including class
+    def cache
+      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
+    # the `key` argument.
+    #
+    # This method will return `nil` if the corresponding instance variable is also
+    # set to `nil`. This ensures we don't keep yielding the block when it returns
+    # `nil`.
+    #
+    # key - The name of the key to cache the data in.
+    # fallback - A value to fall back to in the event of a Git error.
+    def cache_method_output(key, fallback: nil, memoize_only: false, &block)
+      ivar = cache_instance_variable_name(key)
+
+      if instance_variable_defined?(ivar)
+        instance_variable_get(ivar)
+      else
+        # If the repository doesn't exist and a fallback was specified we return
+        # that value inmediately. This saves us Rugged/gRPC invocations.
+        return fallback unless fallback.nil? || cache.repository.exists?
+
+        begin
+          value =
+            if memoize_only
+              yield
+            else
+              cache.fetch(key, &block)
+            end
+
+          instance_variable_set(ivar, value)
+        rescue Gitlab::Git::Repository::NoRepository
+          # Even if the above `#exists?` check passes these errors might still
+          # occur (for example because of a non-existing HEAD). We want to
+          # gracefully handle this and not cache anything
+          fallback
+        end
+      end
+    end
+
+    # Expires the caches of a specific set of methods
+    def expire_method_caches(methods)
+      methods.each do |key|
+        cache.expire(key)
+
+        ivar = cache_instance_variable_name(key)
+
+        remove_instance_variable(ivar) if instance_variable_defined?(ivar)
+      end
+    end
+
+    private
+
+    def cache_instance_variable_name(key)
+      :"@#{key.to_s.tr('?!', '')}"
+    end
+  end
+end
diff --git a/lib/repository_cache.rb b/lib/repository_cache.rb
deleted file mode 100644
index 068a95790c0..00000000000
--- a/lib/repository_cache.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-# Interface to the Redis-backed cache store used by the Repository model
-class RepositoryCache
-  attr_reader :namespace, :backend, :project_id
-
-  def initialize(namespace, project_id, backend = Rails.cache)
-    @namespace = namespace
-    @backend = backend
-    @project_id = project_id
-  end
-
-  def cache_key(type)
-    "#{type}:#{namespace}:#{project_id}"
-  end
-
-  def expire(key)
-    backend.delete(cache_key(key))
-  end
-
-  def fetch(key, &block)
-    backend.fetch(cache_key(key), &block)
-  end
-
-  def exist?(key)
-    backend.exist?(cache_key(key))
-  end
-
-  def read(key)
-    backend.read(cache_key(key))
-  end
-end