summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/controllers/projects/merge_requests_controller.rb2
-rw-r--r--app/models/environment.rb4
-rw-r--r--app/models/merge_request.rb20
-rw-r--r--app/serializers/merge_request_widget_entity.rb2
-rw-r--r--changelogs/unreleased/mr-commit-optimization.yml5
-rw-r--r--spec/models/environment_spec.rb6
-rw-r--r--spec/serializers/merge_request_widget_entity_spec.rb6
7 files changed, 30 insertions, 15 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index a1af125547c..54e7d81de6a 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -187,7 +187,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
begin
@merge_request.environments_for(current_user).map do |environment|
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 =
if environment.stop_action? && can?(current_user, :create_deployment, environment)
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 582a7818502..2b0a88ac5b4 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -99,8 +99,8 @@ class Environment < ActiveRecord::Base
folder_name == "production"
end
- def first_deployment_for(commit)
- ref = project.repository.ref_name_for_sha(ref_path, commit.sha)
+ def first_deployment_for(commit_sha)
+ ref = project.repository.ref_name_for_sha(ref_path, commit_sha)
return nil unless ref
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 5bec68ce4f6..9a7e66a9cbb 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -375,15 +375,27 @@ class MergeRequest < ActiveRecord::Base
end
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
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
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
# When importing a pull request from GitHub, the old and new branches may no
@@ -646,7 +658,7 @@ class MergeRequest < ActiveRecord::Base
!ProtectedBranch.protected?(source_project, source_branch) &&
!source_project.root_ref?(source_branch) &&
Ability.allowed?(current_user, :push_code, source_project) &&
- diff_head_commit == source_branch_head
+ diff_head_sha == source_branch_head.try(:sha)
end
def should_remove_source_branch?
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index 4e8ef320af2..a3ebec0efc6 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -38,7 +38,7 @@ class MergeRequestWidgetEntity < IssuableEntity
# Diff sha's
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
expose :merge_commit_message
diff --git a/changelogs/unreleased/mr-commit-optimization.yml b/changelogs/unreleased/mr-commit-optimization.yml
new file mode 100644
index 00000000000..522d8951b18
--- /dev/null
+++ b/changelogs/unreleased/mr-commit-optimization.yml
@@ -0,0 +1,5 @@
+---
+title: Use persisted/memoized value for MRs shas instead of doing git lookups
+merge_request: 17555
+author:
+type: performance
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index ceb570ac777..412eca4a56b 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -142,15 +142,15 @@ describe Environment do
let(:commit) { project.commit.parent }
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
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
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
diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
index 80a271ba7fb..d2072198d83 100644
--- a/spec/serializers/merge_request_widget_entity_spec.rb
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -147,9 +147,9 @@ describe MergeRequestWidgetEntity do
allow(resource).to receive(:diff_head_sha) { 'sha' }
end
- context 'when no diff head commit' do
+ context 'when diff head commit is empty' 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
end
@@ -157,8 +157,6 @@ describe MergeRequestWidgetEntity do
context 'when diff head commit present' 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')
end
end