Commit 57d5e537 authored by Nick Thomas's avatar Nick Thomas

Merge branch...

Merge branch '209786-improve-performance-of-diffs_batch-diffs-metadata-source-branch-exists' into 'master'

Improve performance of diffs_batch and diffs_metadata actions for Projects::MergeRequestsController

See merge request gitlab-org/gitlab!34516
parents dba80652 a2f3675d
......@@ -107,6 +107,7 @@ class MergeRequest < ApplicationRecord
after_create :ensure_merge_request_diff
after_update :clear_memoized_shas
after_update :clear_memoized_source_branch_exists
after_update :reload_diff_if_branch_changed
after_commit :ensure_metrics, on: [:create, :update], unless: :importing?
after_commit :expire_etag_cache, unless: :importing?
......@@ -862,6 +863,10 @@ class MergeRequest < ApplicationRecord
clear_memoization(:target_branch_head)
end
def clear_memoized_source_branch_exists
clear_memoization(:source_branch_exists)
end
def reload_diff_if_branch_changed
if (saved_change_to_source_branch? || saved_change_to_target_branch?) &&
(source_branch_head && target_branch_head)
......@@ -1109,9 +1114,17 @@ class MergeRequest < ApplicationRecord
end
def source_branch_exists?
return false unless self.source_project
if Feature.enabled?(:memoize_source_branch_merge_request, project)
strong_memoize(:source_branch_exists) do
next false unless self.source_project
self.source_project.repository.branch_exists?(self.source_branch)
end
else
return false unless self.source_project
self.source_project.repository.branch_exists?(self.source_branch)
self.source_project.repository.branch_exists?(self.source_branch)
end
end
def target_branch_exists?
......
---
title: Further improve the performance for loading large diffs on a Merge request
merge_request: 34516
author:
type: performance
......@@ -1217,6 +1217,59 @@ RSpec.describe MergeRequest do
end
end
describe "#source_branch_exists?" do
let(:merge_request) { subject }
let(:repository) { merge_request.source_project.repository }
context 'when memoize_source_branch_merge_request feature is enabled' do
before do
stub_feature_flags(memoize_source_branch_merge_request: true)
end
context 'when the source project is set' do
it 'memoizes the value and returns the result' do
expect(repository).to receive(:branch_exists?).once.with(merge_request.source_branch).and_return(true)
2.times { expect(merge_request.source_branch_exists?).to eq(true) }
end
end
context 'when the source project is not set' do
before do
merge_request.source_project = nil
end
it 'returns false' do
expect(merge_request.source_branch_exists?).to eq(false)
end
end
end
context 'when memoize_source_branch_merge_request feature is disabled' do
before do
stub_feature_flags(memoize_source_branch_merge_request: false)
end
context 'when the source project is set' do
it 'does not memoize the value and returns the result' do
expect(repository).to receive(:branch_exists?).twice.with(merge_request.source_branch).and_return(true)
2.times { expect(merge_request.source_branch_exists?).to eq(true) }
end
end
context 'when the source project is not set' do
before do
merge_request.source_project = nil
end
it 'returns false' do
expect(merge_request.source_branch_exists?).to eq(false)
end
end
end
end
describe '#default_merge_commit_message' do
it 'includes merge information as the title' do
request = build(:merge_request, source_branch: 'source', target_branch: 'target')
......
......@@ -30,6 +30,7 @@ RSpec.describe MergeRequests::Conflicts::ListService do
it 'returns a falsey value when one of the MR branches is missing' do
merge_request = create_merge_request('conflict-resolvable')
merge_request.project.repository.rm_branch(merge_request.author, 'conflict-resolvable')
merge_request.clear_memoized_source_branch_exists
expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey
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