Skip to content

Commit

Permalink
Fix routes N+1 in Issues::ReferencedMergeRequestsService#execute
Browse files Browse the repository at this point in the history
Sorting here needs the project routes to be loaded, including the namespace
routes.
  • Loading branch information
smcgivern committed Aug 16, 2018
1 parent 61f6dab commit cae147c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/banzai/reference_parser/merge_request_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ def records_for_nodes(nodes)
# Eager loading these ensures we don't end up running dozens of
# queries in this process.
target_project: [
{ namespace: :owner },
{ namespace: [:owner, :route] },
{ group: [:owners, :group_members] },
:invited_groups,
:project_members,
:project_feature
:project_feature,
:route
]
}),
self.class.data_attribute
Expand Down
13 changes: 13 additions & 0 deletions spec/services/issues/referenced_merge_requests_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@ def create_closing_mr(attributes = {})
expect(mrs).to eq([closing_mr, referencing_mr, closing_mr_other_project, referencing_mr_other_project])
expect(closed_by_mrs).to eq([closing_mr, closing_mr_other_project])
end

context 'performance' do
it 'does not run extra queries when extra namespaces are included', :use_clean_rails_memory_store_caching do
service.execute(issue) # warm cache
control_count = ActiveRecord::QueryRecorder.new { service.execute(issue) }.count

third_project = create(:project, :public)
create_closing_mr(source_project: third_project)
service.execute(issue) # warm cache

expect { service.execute(issue) }.not_to exceed_query_limit(control_count)
end
end
end

describe '#referenced_merge_requests' do
Expand Down

0 comments on commit cae147c

Please sign in to comment.