Commit bb2bf39d authored by Alejandro Rodríguez's avatar Alejandro Rodríguez

Cache `#can_be_resolved_in_ui?` git operations

parent 5171e2f3
...@@ -17,15 +17,7 @@ module MergeRequests ...@@ -17,15 +17,7 @@ module MergeRequests
return @conflicts_can_be_resolved_in_ui = false unless merge_request.has_complete_diff_refs? return @conflicts_can_be_resolved_in_ui = false unless merge_request.has_complete_diff_refs?
return @conflicts_can_be_resolved_in_ui = false if merge_request.branch_missing? return @conflicts_can_be_resolved_in_ui = false if merge_request.branch_missing?
begin @conflicts_can_be_resolved_in_ui = conflicts.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.
conflicts.files.each(&:lines)
@conflicts_can_be_resolved_in_ui = conflicts.files.length > 0
rescue Gitlab::Git::CommandError, Gitlab::Git::Conflict::Parser::UnresolvableError, Gitlab::Git::Conflict::Resolver::ConflictSideMissing
@conflicts_can_be_resolved_in_ui = false
end
end end
def conflicts def conflicts
......
---
title: Cache MergeRequests can_be_resolved_in_ui? git operations
merge_request: 17589
author:
type: performance
module Gitlab module Gitlab
module Conflict module Conflict
class FileCollection class FileCollection
include Gitlab::RepositoryCacheAdapter
attr_reader :merge_request, :resolver attr_reader :merge_request, :resolver
def initialize(merge_request) def initialize(merge_request)
our_commit = merge_request.source_branch_head.raw our_commit = merge_request.source_branch_head.raw
their_commit = merge_request.target_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 @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 @merge_request = merge_request
end end
...@@ -30,6 +34,17 @@ module Gitlab ...@@ -30,6 +34,17 @@ module Gitlab
end end
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) def file_for_path(old_path, new_path)
files.find { |file| file.their_path == old_path && file.our_path == new_path } files.find { |file| file.their_path == old_path && file.our_path == new_path }
end end
...@@ -56,6 +71,19 @@ Merge branch '#{merge_request.target_branch}' into '#{merge_request.source_branc ...@@ -56,6 +71,19 @@ Merge branch '#{merge_request.target_branch}' into '#{merge_request.source_branc
#{conflict_filenames.join("\n")} #{conflict_filenames.join("\n")}
EOM EOM
end 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 end
end end
...@@ -3,9 +3,10 @@ module Gitlab ...@@ -3,9 +3,10 @@ module Gitlab
class RepositoryCache class RepositoryCache
attr_reader :repository, :namespace, :backend attr_reader :repository, :namespace, :backend
def initialize(repository, backend = Rails.cache) def initialize(repository, extra_namespace: nil, backend: Rails.cache)
@repository = repository @repository = repository
@namespace = "#{repository.full_path}:#{repository.project.id}" @namespace = "#{repository.full_path}:#{repository.project.id}"
@namespace += ":#{extra_namespace}" if extra_namespace
@backend = backend @backend = backend
end end
......
...@@ -10,6 +10,38 @@ describe Gitlab::Conflict::FileCollection do ...@@ -10,6 +10,38 @@ describe Gitlab::Conflict::FileCollection do
end end
end end
describe '#cache' do
it 'specifies a custom namespace with the merge request commit ids' do
our_commit = merge_request.source_branch_head.raw
their_commit = merge_request.target_branch_head.raw
custom_namespace = "#{our_commit.id}:#{their_commit.id}"
expect(file_collection.send(:cache).namespace).to include(custom_namespace)
end
end
describe '#can_be_resolved_in_ui?' do
it 'returns true if conflicts for this collection can be resolved in the UI' do
expect(file_collection.can_be_resolved_in_ui?).to be true
end
it "returns false if conflicts for this collection can't be resolved in the UI" do
expect(file_collection).to receive(:files).and_raise(Gitlab::Git::Conflict::Resolver::ConflictSideMissing)
expect(file_collection.can_be_resolved_in_ui?).to be false
end
it 'caches the result' do
expect(file_collection).to receive(:files).twice.and_call_original
expect(file_collection.can_be_resolved_in_ui?).to be true
expect(file_collection).not_to receive(:files)
expect(file_collection.can_be_resolved_in_ui?).to be true
end
end
describe '#default_commit_message' do describe '#default_commit_message' do
it 'matches the format of the git CLI commit message' do it 'matches the format of the git CLI commit message' do
expect(file_collection.default_commit_message).to eq(<<EOM.chomp) expect(file_collection.default_commit_message).to eq(<<EOM.chomp)
......
...@@ -5,11 +5,25 @@ describe Gitlab::RepositoryCache do ...@@ -5,11 +5,25 @@ describe Gitlab::RepositoryCache do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:repository) { project.repository } let(:repository) { project.repository }
let(:namespace) { "#{repository.full_path}:#{project.id}" } let(:namespace) { "#{repository.full_path}:#{project.id}" }
let(:cache) { described_class.new(repository, backend) } let(:cache) { described_class.new(repository, backend: backend) }
describe '#cache_key' do describe '#cache_key' do
subject { cache.cache_key(:foo) }
it 'includes the namespace' do it 'includes the namespace' do
expect(cache.cache_key(:foo)).to eq "foo:#{namespace}" expect(subject).to eq "foo:#{namespace}"
end
context 'with a given namespace' do
let(:extra_namespace) { 'my:data' }
let(:cache) do
described_class.new(repository, extra_namespace: extra_namespace,
backend: backend)
end
it 'includes the full namespace' do
expect(subject).to eq "foo:#{namespace}:#{extra_namespace}"
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment