kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add preload in issues controller

Browse source
This commit is contained in:
Chantal Rollison 2018-09-10 10:26:33 -07:00
parent a88004c876
commit c871faa3e4
7 changed files with 141 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
app/controllers/boards/issues_controller.rb
View file

@ -18,10 +18,15 @@ module Boards
list_service = Boards::Issues::ListService.new(board_parent, current_user, filter_params)
issues = list_service.execute
issues = issues.page(params[:page]).per(params[:per] || 20).without_count
make_sure_position_is_set(issues) if Gitlab::Database.read_write?
issues = issues.preload(:project,
:milestone,
Issue.move_to_end(issues) if Gitlab::Database.read_write?
issues = issues.preload(:milestone,
:assignees,
project: [
:route,
{
namespace: [:route]
}
],
labels: [:priorities],
notes: [:award_emoji, :author]
)
@ -60,12 +65,6 @@ module Boards
render json: data
end
def make_sure_position_is_set(issues)
issues.each do |issue|
issue.move_to_end && issue.save unless issue.relative_position
end
end
def issue
@issue ||= issues_finder.find(params[:id])
end

79
app/models/concerns/relative_positioning.rb
View file

@ -12,6 +12,49 @@ module RelativePositioning
after_save :save_positionable_neighbours
end
class_methods do
def move_to_end(objects)
parent_ids = objects.map(&:parent_ids).flatten.uniq
max_relative_position = in_parents(parent_ids).maximum(:relative_position) || START_POSITION
objects = objects.reject(&:relative_position)
self.transaction do
objects.each do |object|
relative_position = position_between(max_relative_position, MAX_POSITION)
object.relative_position = relative_position
max_relative_position = relative_position
object.save
end
end
end
# This method takes two integer values (positions) and
# calculates the position between them. The range is huge as
# the maximum integer value is 2147483647. We are incrementing position by IDEAL_DISTANCE * 2 every time
# when we have enough space. If distance is less then IDEAL_DISTANCE we are calculating an average number
def position_between(pos_before, pos_after)
pos_before ||= MIN_POSITION
pos_after ||= MAX_POSITION
pos_before, pos_after = [pos_before, pos_after].sort
halfway = (pos_after + pos_before) / 2
distance_to_halfway = pos_after - halfway
if distance_to_halfway < IDEAL_DISTANCE
halfway
else
if pos_before == MIN_POSITION
pos_after - IDEAL_DISTANCE
elsif pos_after == MAX_POSITION
pos_before + IDEAL_DISTANCE
else
halfway
end
end
end
end
def min_relative_position
self.class.in_parents(parent_ids).minimum(:relative_position)
end
@ -57,7 +100,7 @@ module RelativePositioning
@positionable_neighbours = [before] # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
self.relative_position = position_between(before.relative_position, after.relative_position)
self.relative_position = self.class.position_between(before.relative_position, after.relative_position)
end
def move_after(before = self)
@ -72,7 +115,7 @@ module RelativePositioning
pos_after = issue_to_move.relative_position
end
self.relative_position = position_between(pos_before, pos_after)
self.relative_position = self.class.position_between(pos_before, pos_after)
end
def move_before(after = self)
@ -87,15 +130,15 @@ module RelativePositioning
pos_before = issue_to_move.relative_position
end
self.relative_position = position_between(pos_before, pos_after)
self.relative_position = self.class.position_between(pos_before, pos_after)
end
def move_to_end
self.relative_position = position_between(max_relative_position || START_POSITION, MAX_POSITION)
self.relative_position = self.class.position_between(max_relative_position || START_POSITION, MAX_POSITION)
end
def move_to_start
self.relative_position = position_between(min_relative_position || START_POSITION, MIN_POSITION)
self.relative_position = self.class.position_between(min_relative_position || START_POSITION, MIN_POSITION)
end
# Indicates if there is an issue that should be shifted to free the place
@ -112,32 +155,6 @@ module RelativePositioning
private
# This method takes two integer values (positions) and
# calculates the position between them. The range is huge as
# the maximum integer value is 2147483647. We are incrementing position by IDEAL_DISTANCE * 2 every time
# when we have enough space. If distance is less then IDEAL_DISTANCE we are calculating an average number
def position_between(pos_before, pos_after)
pos_before ||= MIN_POSITION
pos_after ||= MAX_POSITION
pos_before, pos_after = [pos_before, pos_after].sort
halfway = (pos_after + pos_before) / 2
distance_to_halfway = pos_after - halfway
if distance_to_halfway < IDEAL_DISTANCE
halfway
else
if pos_before == MIN_POSITION
pos_after - IDEAL_DISTANCE
elsif pos_after == MAX_POSITION
pos_before + IDEAL_DISTANCE
else
halfway
end
end
end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def save_positionable_neighbours
return unless @positionable_neighbours

1
app/models/list.rb
View file

@ -15,6 +15,7 @@ class List < ActiveRecord::Base
scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) }
scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) }
scope :preload_associations, -> { preload(:board, :label) }
class << self
def destroyable_types

2
app/services/boards/lists/list_service.rb
View file

@ -6,7 +6,7 @@ module Boards
def execute(board)
board.lists.create(list_type: :backlog) unless board.lists.backlog.exists?
board.lists
board.lists.preload_associations
end
end
end

5
changelogs/unreleased/ccr-43034_issues_controller_100_queries.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Add preload for routes and namespaces for issues controller.
merge_request: 21651
author:
type: performance

49
spec/controllers/boards/issues_controller_spec.rb
View file

@ -30,6 +30,15 @@ describe Boards::IssuesController do
context 'when list id is present' do
context 'with valid list id' do
let(:group) { create(:group, :private, projects: [project]) }
let(:group_board) { create(:board, group: group) }
let!(:list3) { create(:list, board: group_board, label: development, position: 2) }
let(:sub_group_1) { create(:group, :private, parent: group) }
before do
group.add_maintainer(user)
end
it 'returns issues that have the list label applied' do
issue = create(:labeled_issue, project: project, labels: [planning])
create(:labeled_issue, project: project, labels: [planning])
@ -56,6 +65,39 @@ describe Boards::IssuesController do
expect { list_issues(user: user, board: board, list: list2) }.not_to exceed_query_limit(control_count)
end
it 'avoids N+1 database queries when adding a project', :request_store do
create(:labeled_issue, project: project, labels: [development])
control_count = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: group_board, list: list3) }.count
2.times do
p = create(:project, group: group)
create(:labeled_issue, project: p, labels: [development])
end
project_2 = create(:project, group: group)
create(:labeled_issue, project: project_2, labels: [development], assignees: [johndoe])
# because each issue without relative_position must be updated with
# a different value, we have 8 extra queries per issue
expect { list_issues(user: user, board: group_board, list: list3) }.not_to exceed_query_limit(control_count + (2 * 8 - 1))
end
it 'avoids N+1 database queries when adding a subgroup, project, and issue', :nested_groups do
create(:project, group: sub_group_1)
create(:labeled_issue, project: project, labels: [development])
control_count = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: group_board, list: list3) }.count
project_2 = create(:project, group: group)
2.times do
p = create(:project, group: sub_group_1)
create(:labeled_issue, project: p, labels: [development])
end
create(:labeled_issue, project: project_2, labels: [development], assignees: [johndoe])
expect { list_issues(user: user, board: group_board, list: list3) }.not_to exceed_query_limit(control_count + (2 * 8 - 1))
end
end
context 'with invalid list id' do
@ -102,12 +144,15 @@ describe Boards::IssuesController do
sign_in(user)
params = {
namespace_id: project.namespace.to_param,
project_id: project,
board_id: board.to_param,
list_id: list.try(:to_param)
}
unless board.try(:parent)&.is_a?(Group)
params[:namespace_id] = project.namespace.to_param
params[:project_id] = project
end
get :index, params.compact
end
end

34
spec/models/concerns/relative_positioning_spec.rb
View file

@ -6,9 +6,13 @@ describe RelativePositioning do
let(:issue1) { create(:issue, project: project) }
let(:new_issue) { create(:issue, project: project) }
before do
[issue, issue1].each do |issue|
issue.move_to_end && issue.save
describe '.move_to_end' do
it 'moves the object to the end' do
Issue.move_to_end([issue, issue1])
expect(issue1.prev_relative_position).to eq issue.relative_position
expect(issue.prev_relative_position).to eq nil
expect(issue1.next_relative_position).to eq nil
end
end
@ -59,6 +63,12 @@ describe RelativePositioning do
end
describe '#move_to_end' do
before do
[issue, issue1].each do |issue|
issue.move_to_end && issue.save
end
end
it 'moves issue to the end' do
new_issue.move_to_end
@ -67,6 +77,12 @@ describe RelativePositioning do
end
describe '#shift_after?' do
before do
[issue, issue1].each do |issue|
issue.move_to_end && issue.save
end
end
it 'returns true' do
issue.update(relative_position: issue1.relative_position - 1)
@ -81,6 +97,12 @@ describe RelativePositioning do
end
describe '#shift_before?' do
before do
[issue, issue1].each do |issue|
issue.move_to_end && issue.save
end
end
it 'returns true' do
issue.update(relative_position: issue1.relative_position + 1)
@ -95,6 +117,12 @@ describe RelativePositioning do
end
describe '#move_between' do
before do
[issue, issue1].each do |issue|
issue.move_to_end && issue.save
end
end
it 'positions issue between two other' do
new_issue.move_between(issue, issue1)