Commit b809f434 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'mr-commit-optimization' into 'master'

Use persisted/memoized value for MRs sha's instead of doing git lookups

See merge request gitlab-org/gitlab-ce!17555
parents 1da5a103 c277aac9
...@@ -187,7 +187,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -187,7 +187,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
begin begin
@merge_request.environments_for(current_user).map do |environment| @merge_request.environments_for(current_user).map do |environment|
project = environment.project project = environment.project
deployment = environment.first_deployment_for(@merge_request.diff_head_commit) deployment = environment.first_deployment_for(@merge_request.diff_head_sha)
stop_url = stop_url =
if environment.stop_action? && can?(current_user, :create_deployment, environment) if environment.stop_action? && can?(current_user, :create_deployment, environment)
......
...@@ -99,8 +99,8 @@ class Environment < ActiveRecord::Base ...@@ -99,8 +99,8 @@ class Environment < ActiveRecord::Base
folder_name == "production" folder_name == "production"
end end
def first_deployment_for(commit) def first_deployment_for(commit_sha)
ref = project.repository.ref_name_for_sha(ref_path, commit.sha) ref = project.repository.ref_name_for_sha(ref_path, commit_sha)
return nil unless ref return nil unless ref
......
...@@ -375,15 +375,27 @@ class MergeRequest < ActiveRecord::Base ...@@ -375,15 +375,27 @@ class MergeRequest < ActiveRecord::Base
end end
def diff_start_sha def diff_start_sha
diff_start_commit.try(:sha) if persisted?
merge_request_diff.start_commit_sha
else
target_branch_head.try(:sha)
end
end end
def diff_base_sha def diff_base_sha
diff_base_commit.try(:sha) if persisted?
merge_request_diff.base_commit_sha
else
branch_merge_base_commit.try(:sha)
end
end end
def diff_head_sha def diff_head_sha
diff_head_commit.try(:sha) if persisted?
merge_request_diff.head_commit_sha
else
source_branch_head.try(:sha)
end
end end
# When importing a pull request from GitHub, the old and new branches may no # When importing a pull request from GitHub, the old and new branches may no
...@@ -646,7 +658,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -646,7 +658,7 @@ class MergeRequest < ActiveRecord::Base
!ProtectedBranch.protected?(source_project, source_branch) && !ProtectedBranch.protected?(source_project, source_branch) &&
!source_project.root_ref?(source_branch) && !source_project.root_ref?(source_branch) &&
Ability.allowed?(current_user, :push_code, source_project) && Ability.allowed?(current_user, :push_code, source_project) &&
diff_head_commit == source_branch_head diff_head_sha == source_branch_head.try(:sha)
end end
def should_remove_source_branch? def should_remove_source_branch?
......
...@@ -38,7 +38,7 @@ class MergeRequestWidgetEntity < IssuableEntity ...@@ -38,7 +38,7 @@ class MergeRequestWidgetEntity < IssuableEntity
# Diff sha's # Diff sha's
expose :diff_head_sha do |merge_request| expose :diff_head_sha do |merge_request|
merge_request.diff_head_sha if merge_request.diff_head_commit merge_request.diff_head_sha.presence
end end
expose :merge_commit_message expose :merge_commit_message
......
---
title: Use persisted/memoized value for MRs shas instead of doing git lookups
merge_request: 17555
author:
type: performance
...@@ -142,15 +142,15 @@ describe Environment do ...@@ -142,15 +142,15 @@ describe Environment do
let(:commit) { project.commit.parent } let(:commit) { project.commit.parent }
it 'returns deployment id for the environment' do it 'returns deployment id for the environment' do
expect(environment.first_deployment_for(commit)).to eq deployment1 expect(environment.first_deployment_for(commit.id)).to eq deployment1
end end
it 'return nil when no deployment is found' do it 'return nil when no deployment is found' do
expect(environment.first_deployment_for(head_commit)).to eq nil expect(environment.first_deployment_for(head_commit.id)).to eq nil
end end
it 'returns a UTF-8 ref' do it 'returns a UTF-8 ref' do
expect(environment.first_deployment_for(commit).ref).to be_utf8 expect(environment.first_deployment_for(commit.id).ref).to be_utf8
end end
end end
......
...@@ -147,9 +147,9 @@ describe MergeRequestWidgetEntity do ...@@ -147,9 +147,9 @@ describe MergeRequestWidgetEntity do
allow(resource).to receive(:diff_head_sha) { 'sha' } allow(resource).to receive(:diff_head_sha) { 'sha' }
end end
context 'when no diff head commit' do context 'when diff head commit is empty' do
it 'returns nil' do it 'returns nil' do
allow(resource).to receive(:diff_head_commit) { nil } allow(resource).to receive(:diff_head_sha) { '' }
expect(subject[:diff_head_sha]).to be_nil expect(subject[:diff_head_sha]).to be_nil
end end
...@@ -157,8 +157,6 @@ describe MergeRequestWidgetEntity do ...@@ -157,8 +157,6 @@ describe MergeRequestWidgetEntity do
context 'when diff head commit present' do context 'when diff head commit present' do
it 'returns diff head commit short id' do it 'returns diff head commit short id' do
allow(resource).to receive(:diff_head_commit) { double }
expect(subject[:diff_head_sha]).to eq('sha') expect(subject[:diff_head_sha]).to eq('sha')
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