Commit 4cd14988 authored by Luke Duncalfe's avatar Luke Duncalfe

Merge branch 'philipcunningham-validate-dast-profiles-resolver-performance-321215' into 'master'

Extract Dast::Profile GraphQL resolver

See merge request gitlab-org/gitlab!60554
parents 6ba95dc2 93c75443
......@@ -63,6 +63,7 @@ module EE
field :dast_profiles,
::Types::Dast::ProfileType.connection_type,
null: true,
resolver: ::Resolvers::AppSec::Dast::ProfileResolver,
description: 'DAST Profiles associated with the project.'
field :dast_site_profile,
......@@ -154,10 +155,6 @@ module EE
}
end
def dast_profiles
Dast::ProfilesFinder.new(project_id: object.id).execute
end
def dast_scanner_profiles
DastScannerProfilesFinder.new(project_ids: [object.id]).execute
end
......
# frozen_string_literal: true
module Resolvers
module AppSec
module Dast
class ProfileResolver < BaseResolver
include LooksAhead
alias_method :project, :object
type ::Types::Dast::ProfileType.connection_type, null: true
def resolve_with_lookahead(**args)
apply_lookahead(find_dast_profiles(args))
end
private
def preloads
{
dast_site_profile: [{ dast_site_profile: [:dast_site, :secret_variables] }],
dast_scanner_profile: [:dast_scanner_profile]
}
end
def find_dast_profiles(args)
::Dast::ProfilesFinder.new(project_id: project.id).execute
end
end
end
end
end
---
title: Fix GraphQL N+1 performance issue when loading a Project's DAST Profiles
merge_request: 60554
author:
type: performance
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Resolvers::AppSec::Dast::ProfileResolver do
include GraphqlHelpers
before do
stub_licensed_features(security_on_demand_scans: true)
end
let_it_be(:project) { create(:project) }
let_it_be(:developer) { create(:user, developer_projects: [project]) }
let_it_be(:dast_profile1) { create(:dast_profile, project: project) }
let_it_be(:dast_profile2) { create(:dast_profile, project: project) }
let(:current_user) { developer }
specify do
expect(described_class).to have_nullable_graphql_type(Types::Dast::ProfileType.connection_type)
end
context 'when resolving multiple DAST profiles' do
subject { sync(dast_profiles) }
it { is_expected.to contain_exactly(dast_profile1, dast_profile2) }
context 'when the feature is disabled' do
before do
stub_licensed_features(security_on_demand_scans: false)
end
it { is_expected.to be_empty }
end
context 'when the user does not have access' do
let(:current_user) { create(:user) }
it { is_expected.to be_empty }
end
end
private
def dast_profiles
resolve(described_class, obj: project, ctx: { current_user: current_user })
end
end
......@@ -12,15 +12,17 @@ RSpec.describe 'Query.project(fullPath).dastProfiles' do
let_it_be(:dast_profile3) { create(:dast_profile, project: project) }
let_it_be(:dast_profile4) { create(:dast_profile, project: project) }
subject do
let(:query) do
fields = all_graphql_fields_for('DastProfile')
query = graphql_query_for(
graphql_query_for(
:project,
{ full_path: project.full_path },
query_nodes(:dast_profiles, fields)
)
end
subject do
post_graphql(
query,
current_user: current_user,
......@@ -77,6 +79,22 @@ RSpec.describe 'Query.project(fullPath).dastProfiles' do
expect(graphql_data_at(:project, :dast_profiles, :nodes, 0, 'branch')).to eq('name' => 'master', 'exists' => true)
end
it 'avoids N+1 queries' do
control = ActiveRecord::QueryRecorder.new do
post_graphql(
query,
current_user: current_user,
variables: {
fullPath: project.full_path
}
)
end
create_list(:dast_profile, 2, project: project)
expect { subject }.not_to exceed_query_limit(control)
end
end
def pagination_query(arguments)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment