summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlejandro Rodríguez <alejorro70@gmail.com>2018-03-06 22:25:12 -0300
committerAlejandro Rodríguez <alejorro70@gmail.com>2018-03-08 11:54:21 -0300
commitbb2bf39ddfc05db887d4a02ad3a38dbf81beed1a (patch)
treee175678908f5b0f1d0d469adb0859ecd88dd3d4e
parent5171e2f3d4fdc681a58e11f9615afa968324a278 (diff)
downloadgitlab-ce-bb2bf39ddfc05db887d4a02ad3a38dbf81beed1a.tar.gz
Cache `#can_be_resolved_in_ui?` git operationscache-refactor
-rw-r--r--app/services/merge_requests/conflicts/list_service.rb10
-rw-r--r--changelogs/unreleased/cache-refactor.yml5
-rw-r--r--lib/gitlab/conflict/file_collection.rb32
-rw-r--r--lib/gitlab/repository_cache.rb3
-rw-r--r--spec/lib/gitlab/conflict/file_collection_spec.rb32
-rw-r--r--spec/lib/gitlab/repository_cache_spec.rb18
6 files changed, 86 insertions, 14 deletions
diff --git a/app/services/merge_requests/conflicts/list_service.rb b/app/services/merge_requests/conflicts/list_service.rb
index ca9a33678e4..72cbc49adb2 100644
--- a/app/services/merge_requests/conflicts/list_service.rb
+++ b/app/services/merge_requests/conflicts/list_service.rb
@@ -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 if merge_request.branch_missing?
- begin
- # 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
+ @conflicts_can_be_resolved_in_ui = conflicts.can_be_resolved_in_ui?
end
def conflicts
diff --git a/changelogs/unreleased/cache-refactor.yml b/changelogs/unreleased/cache-refactor.yml
new file mode 100644
index 00000000000..dec7a0392a5
--- /dev/null
+++ b/changelogs/unreleased/cache-refactor.yml
@@ -0,0 +1,5 @@
+---
+title: Cache MergeRequests can_be_resolved_in_ui? git operations
+merge_request: 17589
+author:
+type: performance
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
index b7650f6ed81..b1bf3ca4143 100644
--- a/lib/gitlab/repository_cache.rb
+++ b/lib/gitlab/repository_cache.rb
@@ -3,9 +3,10 @@ module Gitlab
class RepositoryCache
attr_reader :repository, :namespace, :backend
- def initialize(repository, backend = Rails.cache)
+ 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
diff --git a/spec/lib/gitlab/conflict/file_collection_spec.rb b/spec/lib/gitlab/conflict/file_collection_spec.rb
index 5944ce8049a..c93912db411 100644
--- a/spec/lib/gitlab/conflict/file_collection_spec.rb
+++ b/spec/lib/gitlab/conflict/file_collection_spec.rb
@@ -10,6 +10,38 @@ describe Gitlab::Conflict::FileCollection do
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
it 'matches the format of the git CLI commit message' do
expect(file_collection.default_commit_message).to eq(<<EOM.chomp)
diff --git a/spec/lib/gitlab/repository_cache_spec.rb b/spec/lib/gitlab/repository_cache_spec.rb
index f4789421663..fc259cf1208 100644
--- a/spec/lib/gitlab/repository_cache_spec.rb
+++ b/spec/lib/gitlab/repository_cache_spec.rb
@@ -5,11 +5,25 @@ describe Gitlab::RepositoryCache do
let(:project) { create(:project) }
let(:repository) { project.repository }
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
+ subject { cache.cache_key(:foo) }
+
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