Merge branch 'sh-fix-epics-api-nplus-one' into 'master'

Eliminate N+1 queries in Epics API

Closes #10243

See merge request gitlab-org/gitlab-ee!9897

ee/app/models/ee/epic.rb

@@ -77,6 +77,8 @@ module EE
         reorder('relative_position ASC', 'id DESC')
       end
 
+      scope :with_api_entity_associations, -> { preload(:author, :labels, :group) }
+
       def etag_caching_enabled?
         true
       end

ee/changelogs/unreleased/sh-fix-epics-api-nplus-one.yml

+---
+title: Eliminate N+1 queries in Epics API
+merge_request: 9897
+author:
+type: performance

ee/lib/api/epics.rb

@@ -33,9 +33,10 @@ module API
         use :pagination
       end
       get ':id/(-/)epics' do
-        epics = paginate(find_epics(finder_params: { group_id: user_group.id }))
+        epics = paginate(find_epics(finder_params: { group_id: user_group.id })).with_api_entity_associations
 
-        present epics, with: EE::API::Entities::Epic, user: current_user, epics_metadata: issuable_meta_data(epics, 'Epic')
+        # issuable_metadata is the standard used by the Todo API
+        present epics, with: EE::API::Entities::Epic, user: current_user, issuable_metadata: issuable_meta_data(epics, 'Epic')
       end
 
       desc 'Get details of an epic' do

ee/lib/ee/api/entities.rb

@@ -205,17 +205,17 @@ module EE
           epic.labels.map(&:title).sort
         end
         expose :upvotes do |epic, options|
-          if options[:epics_metadata]
+          if options[:issuable_metadata]
             # Avoids an N+1 query when metadata is included
-            options[:epics_metadata][epic.id].upvotes
+            options[:issuable_metadata][epic.id].upvotes
           else
             epic.upvotes
           end
         end
         expose :downvotes do |epic, options|
-          if options[:epics_metadata]
+          if options[:issuable_metadata]
             # Avoids an N+1 query when metadata is included
-            options[:epics_metadata][epic.id].downvotes
+            options[:issuable_metadata][epic.id].downvotes
           else
             epic.downvotes
           end

ee/spec/requests/api/epics_spec.rb

@@ -4,7 +4,8 @@ describe API::Epics do
   let(:user) { create(:user) }
   let(:group) { create(:group) }
   let(:project) { create(:project, :public, group: group) }
-  let(:epic) { create(:epic, group: group) }
+  let(:label) { create(:label) }
+  let(:epic) { create(:labeled_epic, group: group, labels: [label]) }
   let(:params) { nil }
 
   shared_examples 'error requests' do
@@ -85,6 +86,22 @@ describe API::Epics do
       it 'matches the response schema' do
         expect(response).to match_response_schema('public_api/v4/epics', dir: 'ee')
       end
+
+      it 'avoids N+1 queries', :request_store do
+        epic
+        # Avoid polluting queries with inserts for personal access token
+        pat = create(:personal_access_token, user: user)
+
+        control = ActiveRecord::QueryRecorder.new(skip_cached: false) do
+          get api(url, personal_access_token: pat)
+        end
+
+        label_2 = create(:label)
+        create_list(:labeled_epic, 2, group: group, labels: [label_2])
+
+        expect { get api(url, personal_access_token: pat) }.not_to exceed_all_query_limit(control)
+        expect(response).to have_gitlab_http_status(200)
+      end
     end
 
     context 'with multiple epics' do

ee/spec/requests/api/todos_spec.rb

@@ -5,9 +5,44 @@ describe API::Todos do
   let(:user) { create(:user) }
   let(:epic) { create(:epic, group: group) }
 
-  subject { post api("/groups/#{group.id}/epics/#{epic.iid}/todo", user) }
+  describe 'GET /todos' do
+    let(:author_1) { create(:user) }
+    let!(:pat) { create(:personal_access_token, user: user) }
+
+    before do
+      group.add_developer(user)
+      group.add_developer(author_1)
+
+      create_todo_for_new_epic
+    end
+
+    def create_todo_for_new_epic
+      new_group = create(:group)
+      label = create(:label)
+      new_epic = create(:labeled_epic, group: new_group, labels: [label])
+      new_group.add_developer(author_1)
+      new_group.add_developer(user)
+      create(:todo, project: nil, group: new_group, author: author_1, user: user, target: new_epic)
+    end
+
+    it 'avoids N+1 queries', :request_store do
+      create_todo_for_new_epic
+
+      get api('/todos', personal_access_token: pat)
+
+      control = ActiveRecord::QueryRecorder.new { get api('/todos', personal_access_token: pat) }
+
+      create_todo_for_new_epic
+
+      expect { get api('/todos', personal_access_token: pat) }.not_to exceed_query_limit(control)
+
+      expect(response.status).to eq(200)
+    end
+  end
 
   describe 'POST :id/epics/:epic_iid/todo' do
+    subject { post api("/groups/#{group.id}/epics/#{epic.iid}/todo", user) }
+
     context 'when epics feature is disabled' do
       it 'returns 403 forbidden error' do
         subject