summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorcharlieablett <cablett@gitlab.com>2019-06-14 10:20:03 +1200
committercharlieablett <cablett@gitlab.com>2019-06-26 11:47:18 +1200
commit426f1c20b19e5bd98ee0ab8bd5f8851b683a79c6 (patch)
tree14dfe36a0f72fa5fee258f7a89f94c74be6720f4
parent4633df7b9e756ae50f35246e9ba93e1c3ca1e01a (diff)
downloadgitlab-ce-426f1c20b19e5bd98ee0ab8bd5f8851b683a79c6.tar.gz
Remove N+1 query for project and group boards
- Add test for N+1 queries - Add destroyable lists scope to Board and List - Preload lists for both project and group boards
-rw-r--r--app/models/board.rb3
-rw-r--r--app/serializers/board_serializer.rb8
-rw-r--r--lib/api/boards.rb2
-rw-r--r--lib/api/boards_responses.rb2
-rw-r--r--lib/api/entities.rb2
-rw-r--r--lib/api/group_boards.rb2
-rw-r--r--spec/support/api/boards_shared_examples.rb10
7 files changed, 25 insertions, 4 deletions
diff --git a/app/models/board.rb b/app/models/board.rb
index e08db764f65..891d26897ed 100644
--- a/app/models/board.rb
+++ b/app/models/board.rb
@@ -5,10 +5,13 @@ class Board < ApplicationRecord
belongs_to :project
has_many :lists, -> { order(:list_type, :position) }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
+ has_many :destroyable_lists, -> { destroyable }, class_name: "List"
validates :project, presence: true, if: :project_needed?
validates :group, presence: true, unless: :project
+ scope :with_associations, -> { preload(:destroyable_lists) }
+
def project_needed?
!group
end
diff --git a/app/serializers/board_serializer.rb b/app/serializers/board_serializer.rb
index 70a4c9ae282..3877a1bdcbc 100644
--- a/app/serializers/board_serializer.rb
+++ b/app/serializers/board_serializer.rb
@@ -2,4 +2,12 @@
class BoardSerializer < BaseSerializer
entity BoardSimpleEntity
+
+ def represent(resource, opts = {})
+ if resource.respond_to?(:with_associations)
+ resource = resource.with_associations
+ end
+
+ super
+ end
end
diff --git a/lib/api/boards.rb b/lib/api/boards.rb
index b7c77730afb..4e31f74f18a 100644
--- a/lib/api/boards.rb
+++ b/lib/api/boards.rb
@@ -27,7 +27,7 @@ module API
end
get '/' do
authorize!(:read_board, user_project)
- present paginate(board_parent.boards), with: Entities::Board
+ present paginate(board_parent.boards.with_associations), with: Entities::Board
end
desc 'Find a project board' do
diff --git a/lib/api/boards_responses.rb b/lib/api/boards_responses.rb
index 86d9b24802f..68497a08fb8 100644
--- a/lib/api/boards_responses.rb
+++ b/lib/api/boards_responses.rb
@@ -11,7 +11,7 @@ module API
end
def board_lists
- board.lists.destroyable
+ board.destroyable_lists
end
def create_list
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index ead01dc53f7..d783591c238 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1101,7 +1101,7 @@ module API
expose :project, using: Entities::BasicProjectDetails
expose :lists, using: Entities::List do |board|
- board.lists.destroyable
+ board.destroyable_lists
end
end
diff --git a/lib/api/group_boards.rb b/lib/api/group_boards.rb
index 9a20ee8c8b9..feb2254963e 100644
--- a/lib/api/group_boards.rb
+++ b/lib/api/group_boards.rb
@@ -37,7 +37,7 @@ module API
use :pagination
end
get '/' do
- present paginate(board_parent.boards), with: Entities::Board
+ present paginate(board_parent.boards.with_associations), with: Entities::Board
end
end
diff --git a/spec/support/api/boards_shared_examples.rb b/spec/support/api/boards_shared_examples.rb
index 592962ebf7c..3abb5096a7a 100644
--- a/spec/support/api/boards_shared_examples.rb
+++ b/spec/support/api/boards_shared_examples.rb
@@ -14,6 +14,16 @@ shared_examples_for 'group and project boards' do |route_definition, ee = false|
end
end
+ it 'avoids N+1 queries' do
+ pat = create(:personal_access_token, user: user)
+ control = ActiveRecord::QueryRecorder.new { get api(root_url, personal_access_token: pat) }
+
+ create(:milestone, "#{board_parent.class.name.underscore}": board_parent)
+ create(:board, "#{board_parent.class.name.underscore}": board_parent)
+
+ expect { get api(root_url, personal_access_token: pat) }.not_to exceed_query_limit(control)
+ end
+
describe "GET #{route_definition}" do
context "when unauthenticated" do
it "returns authentication error" do