Merge branch 'pedropombeiro/334173-fix-tags-retrieval' into 'master'

Only retrieve tag_list if requested in GraphQL query

See merge request gitlab-org/gitlab!64598

app/finders/ci/runners_finder.rb

@@ -19,8 +19,9 @@ module Ci
       filter_by_runner_type!
       filter_by_tag_list!
       sort!
+      request_tag_list!
 
-      @runners.with_tags
+      @runners
 
     rescue Gitlab::Access::AccessDeniedError
       Ci::Runner.none
@@ -73,6 +74,10 @@ module Ci
       @runners = @runners.order_by(sort_key)
     end
 
+    def request_tag_list!
+      @runners = @runners.with_tags if !@params[:preload].present? || @params.dig(:preload, :tag_name)
+    end
+
     def filter_by!(scope_name, available_scopes)
       scope = @params[scope_name]
 

app/graphql/resolvers/ci/runners_resolver.rb

@@ -3,6 +3,8 @@
 module Resolvers
   module Ci
     class RunnersResolver < BaseResolver
+      include LooksAhead
+
       type Types::Ci::RunnerType.connection_type, null: true
 
       argument :status, ::Types::Ci::RunnerStatusEnum,
@@ -25,10 +27,11 @@ module Resolvers
                required: false,
                description: 'Sort order of results.'
 
-      def resolve(**args)
-        ::Ci::RunnersFinder
-          .new(current_user: current_user, params: runners_finder_params(args))
-          .execute
+      def resolve_with_lookahead(**args)
+        apply_lookahead(
+          ::Ci::RunnersFinder
+            .new(current_user: current_user, params: runners_finder_params(args))
+            .execute)
       end
 
       private
@@ -39,7 +42,10 @@ module Resolvers
           type_type: params[:type],
           tag_name: params[:tag_list],
           search: params[:search],
-          sort: params[:sort]&.to_s
+          sort: params[:sort]&.to_s,
+          preload: {
+            tag_name: node_selection&.selects?(:tag_list)
+          }
         }.compact
       end
     end

spec/finders/ci/runners_finder_spec.rb

@@ -7,12 +7,29 @@ RSpec.describe Ci::RunnersFinder do
     let_it_be(:admin) { create(:user, :admin) }
 
     describe '#execute' do
-      context 'with empty params' do
-        it 'returns all runners' do
-          runner1 = create :ci_runner, active: true
-          runner2 = create :ci_runner, active: false
+      context 'with 2 runners' do
+        let_it_be(:runner1) { create(:ci_runner, active: true) }
+        let_it_be(:runner2) { create(:ci_runner, active: false) }
+
+        context 'with empty params' do
+          it 'returns all runners' do
+            expect(Ci::Runner).to receive(:with_tags).and_call_original
+            expect(described_class.new(current_user: admin, params: {}).execute).to match_array [runner1, runner2]
+          end
+        end
 
-          expect(described_class.new(current_user: admin, params: {}).execute).to match_array [runner1, runner2]
+        context 'with preload param set to :tag_name true' do
+          it 'requests tags' do
+            expect(Ci::Runner).to receive(:with_tags).and_call_original
+            expect(described_class.new(current_user: admin, params: { preload: { tag_name: true } }).execute).to match_array [runner1, runner2]
+          end
+        end
+
+        context 'with preload param set to :tag_name false' do
+          it 'does not request tags' do
+            expect(Ci::Runner).not_to receive(:with_tags)
+            expect(described_class.new(current_user: admin, params: { preload: { tag_name: false } }).execute).to match_array [runner1, runner2]
+          end
         end
       end
 

spec/requests/api/graphql/ci/runner_spec.rb

@@ -85,6 +85,26 @@ RSpec.describe 'Query.runner(id)' do
 
   describe 'for active runner' do
     it_behaves_like 'runner details fetch', :active_runner
+
+    context 'when tagList is not requested' do
+      let(:query) do
+        wrap_fields(query_graphql_path(query_path, 'id'))
+      end
+
+      let(:query_path) do
+        [
+          [:runner, { id: active_runner.to_global_id.to_s }]
+        ]
+      end
+
+      it 'does not retrieve tagList' do
+        post_graphql(query, current_user: user)
+
+        runner_data = graphql_data_at(:runner)
+        expect(runner_data).not_to be_nil
+        expect(runner_data).not_to include('tagList')
+      end
+    end
   end
 
   describe 'for inactive runner' do