Merge branch 'sh-optimize-todos-api' into 'master'

Significantly reduce N+1 queries in /api/v4/todos endpoint

Closes #40378

See merge request gitlab-org/gitlab-ce!25711

4 files changed, 35 insertions, 4 deletions

diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 5176e9713c1..2cd0d93b205 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -883,7 +883,8 @@ module API
       expose :target_type
 
       expose :target do |todo, options|
-        todo_target_class(todo.target_type).represent(todo.target, options)
+        todo_options = options.fetch(todo.target_type, {})
+        todo_target_class(todo.target_type).represent(todo.target, todo_options)
       end
 
       expose :target_url do |todo, options|
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index d59d2f5a098..b2ec4ed898e 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -28,7 +28,7 @@ module API
         args[:scope] = args[:scope].underscore if args[:scope]
 
         issues = IssuesFinder.new(current_user, args).execute
-          .preload(:assignees, :labels, :notes, :timelogs, :project, :author, :closed_by)
+                   .with_api_entity_associations
         issues.reorder(order_options_with_tie_breaker)
       end
       # rubocop: enable CodeReuse/ActiveRecord
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 6518ebbcff5..98dcc388f44 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -48,7 +48,7 @@ module API
         return merge_requests if args[:view] == 'simple'
 
         merge_requests
-          .preload(:notes, :author, :assignee, :milestone, :latest_merge_request_diff, :labels, :timelogs, metrics: [:latest_closed_by, :merged_by])
+          .with_api_entity_associations
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
diff --git a/lib/api/todos.rb b/lib/api/todos.rb
index 64ac8ece56c..d2196f05173 100644
--- a/lib/api/todos.rb
+++ b/lib/api/todos.rb
@@ -6,6 +6,8 @@ module API
 
     before { authenticate! }
 
+    helpers ::Gitlab::IssuableMetadata
+
     ISSUABLE_TYPES = {
       'merge_requests' => ->(iid) { find_merge_request_with_access(iid) },
       'issues' => ->(iid) { find_project_issue(iid) }
@@ -42,6 +44,30 @@ module API
         def find_todos
           TodosFinder.new(current_user, params).execute
         end
+
+        def issuable_and_awardable?(type)
+          obj_type = Object.const_get(type)
+
+          (obj_type < Issuable) && (obj_type < Awardable)
+        rescue NameError
+          false
+        end
+
+        def batch_load_issuable_metadata(todos, options)
+          # This should be paginated and will cause Rails to SELECT for all the Todos
+          todos_by_type = todos.group_by(&:target_type)
+
+          todos_by_type.keys.each do |type|
+            next unless issuable_and_awardable?(type)
+
+            collection = todos_by_type[type]
+
+            next unless collection
+
+            targets = collection.map(&:target)
+            options[type] = { issuable_metadata: issuable_meta_data(targets, type) }
+          end
+        end
       end
 
       desc 'Get a todo list' do
@@ -51,7 +77,11 @@ module API
         use :pagination
       end
       get do
-        present paginate(find_todos), with: Entities::Todo, current_user: current_user
+        todos = paginate(find_todos.with_api_entity_associations)
+        options = { with: Entities::Todo, current_user: current_user }
+        batch_load_issuable_metadata(todos, options)
+
+        present todos, options
       end
 
       desc 'Mark a todo as done' do