diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-03-07 13:46:43 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-03-07 13:46:43 +0000 |
commit | b809f434699f3bc2adb84d0cb59f575cb44b5984 (patch) | |
tree | f200aa6028a0fd278729b9923d1af5ee33a835d3 | |
parent | 1da5a103ace9c346758e9525d7f0fa55bab13599 (diff) | |
parent | c277aac9dc1a72eb8b4584ad217996a9d7daaf14 (diff) | |
download | gitlab-ce-b809f434699f3bc2adb84d0cb59f575cb44b5984.tar.gz |
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
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 2 | ||||
-rw-r--r-- | app/models/environment.rb | 4 | ||||
-rw-r--r-- | app/models/merge_request.rb | 20 | ||||
-rw-r--r-- | app/serializers/merge_request_widget_entity.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/mr-commit-optimization.yml | 5 | ||||
-rw-r--r-- | spec/models/environment_spec.rb | 6 | ||||
-rw-r--r-- | spec/serializers/merge_request_widget_entity_spec.rb | 6 |
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 |