Merge branch '194104-epics-iid-too-many-queries' into 'master'

Optimize AvailableLabelsService for multiple labels

See merge request gitlab-org/gitlab!59032

app/services/labels/available_labels_service.rb

@@ -14,15 +14,16 @@ module Labels
 
       return [] unless labels
 
-      labels = labels.split(',') if labels.is_a?(String)
+      labels = labels.split(',').map(&:strip) if labels.is_a?(String)
+      existing_labels = LabelsFinder.new(current_user, finder_params(labels)).execute.index_by(&:title)
 
       labels.map do |label_name|
         label = Labels::FindOrCreateService.new(
           current_user,
           parent,
           include_ancestor_groups: true,
-          title: label_name.strip,
-          available_labels: available_labels
+          title: label_name,
+          existing_labels_by_title: existing_labels
         ).execute(find_only: find_only)
 
         label
@@ -45,18 +46,19 @@ module Labels
 
     private
 
-    def finder_params
-      params = { include_ancestor_groups: true }
+    def finder_params(titles = nil)
+      finder_params = { include_ancestor_groups: true }
+      finder_params[:title] = titles if titles
 
       case parent
       when Group
-        params[:group_id] = parent.id
-        params[:only_group_labels] = true
+        finder_params[:group_id] = parent.id
+        finder_params[:only_group_labels] = true
       when Project
-        params[:project_id] = parent.id
+        finder_params[:project_id] = parent.id
       end
 
-      params
+      finder_params
     end
   end
 end

app/services/labels/find_or_create_service.rb

@@ -6,6 +6,7 @@ module Labels
       @current_user = current_user
       @parent = parent
       @available_labels = params.delete(:available_labels)
+      @existing_labels_by_title = params.delete(:existing_labels_by_title) || {}
       @params = params.dup.with_indifferent_access
     end
 
@@ -16,7 +17,7 @@ module Labels
 
     private
 
-    attr_reader :current_user, :parent, :params, :skip_authorization
+    attr_reader :current_user, :parent, :params, :skip_authorization, :existing_labels_by_title
 
     def available_labels
       @available_labels ||= LabelsFinder.new(
@@ -29,9 +30,8 @@ module Labels
 
     # Only creates the label if current_user can do so, if the label does not exist
     # and the user can not create the label, nil is returned
-    # rubocop: disable CodeReuse/ActiveRecord
     def find_or_create_label(find_only: false)
-      new_label = available_labels.find_by(title: title)
+      new_label = find_existing_label(title)
 
       return new_label if find_only
 
@@ -42,6 +42,11 @@ module Labels
 
       new_label
     end
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def find_existing_label(title)
+      existing_labels_by_title[title] || available_labels.find_by(title: title)
+    end
     # rubocop: enable CodeReuse/ActiveRecord
 
     def title

changelogs/unreleased/194104-epics-iid-too-many-queries.yml

+---
+title: Optimize AvailableLabelsService for multiple labels search
+merge_request: 59032
+author:
+type: performance

spec/services/labels/available_labels_service_spec.rb

@@ -36,6 +36,15 @@ RSpec.describe Labels::AvailableLabelsService do
           expect(result).to include(project_label, group_label)
           expect(result).not_to include(other_project_label, other_group_label)
         end
+
+        it 'do not cause additional query for finding labels' do
+          label_titles = [project_label.title]
+          control_count = ActiveRecord::QueryRecorder.new { described_class.new(user, project, labels: label_titles).find_or_create_by_titles }
+
+          new_label = create(:label, project: project)
+          label_titles = [project_label.title, new_label.title]
+          expect { described_class.new(user, project, labels: label_titles).find_or_create_by_titles }.not_to exceed_query_limit(control_count)
+        end
       end
     end