diff options
author | Sean McGivern <sean@gitlab.com> | 2018-08-16 12:08:00 +0100 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-08-21 12:40:44 +0100 |
commit | 22d8fbacaf153c0b29738e812a22764129483eee (patch) | |
tree | 4d9cbad46a5b44e3e71505dd6f0ab5bd66ff0bc8 /spec | |
parent | c73da6c1e73f04ece18b5fca5ccd67bf918682f8 (diff) | |
download | gitlab-ce-22d8fbacaf153c0b29738e812a22764129483eee.tar.gz |
Fix authors N+1 in Issues::ReferencedMergeRequestsService
`#referenced_merge_requests` preloaded too many associations. Award emoji, for
instance, are completely unnecessary here.
`#closed_by_merge_requests` had the opposite problem: `#all_references` needs
each item's author, and these weren't preloaded.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/services/issues/referenced_merge_requests_service_spec.rb | 26 |
1 files changed, 25 insertions, 1 deletions
diff --git a/spec/services/issues/referenced_merge_requests_service_spec.rb b/spec/services/issues/referenced_merge_requests_service_spec.rb index a10893e502f..0afc8c300f9 100644 --- a/spec/services/issues/referenced_merge_requests_service_spec.rb +++ b/spec/services/issues/referenced_merge_requests_service_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper.rb' describe Issues::ReferencedMergeRequestsService do def create_referencing_mr(attributes = {}) create(:merge_request, attributes).tap do |merge_request| - create(:note, :system, project: project, noteable: issue, note: merge_request.to_reference(full: true)) + create(:note, :system, project: project, noteable: issue, author: user, note: merge_request.to_reference(full: true)) end end @@ -54,6 +54,18 @@ describe Issues::ReferencedMergeRequestsService do expect(service.referenced_merge_requests(issue)).not_to include(closing_mr_other_project) expect(service.referenced_merge_requests(issue)).not_to include(referencing_mr_other_project) end + + context 'performance' do + it 'does not run a query for each note author', :use_clean_rails_memory_store_caching do + service.referenced_merge_requests(issue) # warm cache + control_count = ActiveRecord::QueryRecorder.new { service.referenced_merge_requests(issue) }.count + + create(:note, project: project, noteable: issue, author: create(:user)) + service.referenced_merge_requests(issue) # warm cache + + expect { service.referenced_merge_requests(issue) }.not_to exceed_query_limit(control_count) + end + end end describe '#closed_by_merge_requests' do @@ -68,5 +80,17 @@ describe Issues::ReferencedMergeRequestsService do it 'returns an empty array when the current issue is closed already' do expect(service.closed_by_merge_requests(closed_issue)).to eq([]) end + + context 'performance' do + it 'does not run a query for each note author', :use_clean_rails_memory_store_caching do + service.closed_by_merge_requests(issue) # warm cache + control_count = ActiveRecord::QueryRecorder.new { service.closed_by_merge_requests(issue) }.count + + create(:note, :system, project: project, noteable: issue, author: create(:user)) + service.closed_by_merge_requests(issue) # warm cache + + expect { service.closed_by_merge_requests(issue) }.not_to exceed_query_limit(control_count) + end + end end end |