Merge branch 'jej-fix-n+1-queries-milestones-show' into 'master'

Fix N+1 queries on milestone show pages

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/25832

See merge request !8185
This commit is contained in:
Sean McGivern 2016-12-20 19:54:36 +00:00
commit d814533f69
5 changed files with 73 additions and 1 deletions

View file

@ -39,6 +39,8 @@ class Issue < ActiveRecord::Base
scope :created_after, -> (datetime) { where("created_at >= ?", datetime) }
scope :include_associations, -> { includes(:assignee, :labels, project: :namespace) }
attr_spammable :title, spam_title: true
attr_spammable :description, spam_description: true

View file

@ -21,7 +21,7 @@
.tab-content.milestone-content
.tab-pane.active#tab-issues
= render 'shared/milestones/issues_tab', issues: milestone.issues_visible_to_user(current_user), show_project_name: show_project_name, show_full_project_name: show_full_project_name
= render 'shared/milestones/issues_tab', issues: milestone.issues_visible_to_user(current_user).include_associations, show_project_name: show_project_name, show_full_project_name: show_full_project_name
.tab-pane#tab-merge-requests
= render 'shared/milestones/merge_requests_tab', merge_requests: milestone.merge_requests, show_project_name: show_project_name, show_full_project_name: show_full_project_name
.tab-pane#tab-participants

View file

@ -0,0 +1,4 @@
---
title: Fix N+1 queries on milestone show pages
merge_request: 8185
author:

View file

@ -0,0 +1,26 @@
require 'rails_helper'
describe 'Milestone show', feature: true do
let(:user) { create(:user) }
let(:project) { create(:empty_project) }
let(:milestone) { create(:milestone, project: project) }
let(:labels) { create_list(:label, 2, project: project) }
let(:issue_params) { { project: project, assignee: user, author: user, milestone: milestone, labels: labels } }
before do
project.add_user(user, :developer)
login_as(user)
end
def visit_milestone
visit namespace_project_milestone_path(project.namespace, project, milestone)
end
it 'avoids N+1 database queries' do
create(:labeled_issue, issue_params)
control_count = ActiveRecord::QueryRecorder.new { visit_milestone }.count
create_list(:labeled_issue, 10, issue_params)
expect { visit_milestone }.not_to exceed_query_limit(control_count)
end
end

View file

@ -0,0 +1,40 @@
module ActiveRecord
class QueryRecorder
attr_reader :log
def initialize(&block)
@log = []
ActiveSupport::Notifications.subscribed(method(:callback), 'sql.active_record', &block)
end
def callback(name, start, finish, message_id, values)
return if %w(CACHE SCHEMA).include?(values[:name])
@log << values[:sql]
end
def count
@log.count
end
def log_message
@log.join("\n\n")
end
end
end
RSpec::Matchers.define :exceed_query_limit do |expected|
supports_block_expectations
match do |block|
query_count(&block) > expected
end
failure_message_when_negated do |actual|
"Expected a maximum of #{expected} queries, got #{@recorder.count}:\n\n#{@recorder.log_message}"
end
def query_count(&block)
@recorder = ActiveRecord::QueryRecorder.new(&block)
@recorder.count
end
end