diff options
| -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-- | changelogs/unreleased/mr-commit-optimization.yml | 5 | ||||
| -rw-r--r-- | spec/models/environment_spec.rb | 6 |
5 files changed, 27 insertions, 10 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 f78c21aebe5..6f7dabdfefe 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/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 6f24a039998..16fdd154e7f 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 |