Commit 4741a12a authored by Alan (Maciej) Paruszewski's avatar Alan (Maciej) Paruszewski Committed by Peter Leitzen

Include subgroups in GraphQL field for vulnerabilityGrades for Groups

This change adds ability to include subgroups when fetching
vulnerabilityGrades for a group.
parent 429a74c6
...@@ -8725,7 +8725,12 @@ type Group { ...@@ -8725,7 +8725,12 @@ type Group {
""" """
Represents vulnerable project counts for each grade Represents vulnerable project counts for each grade
""" """
vulnerabilityGrades: [VulnerableProjectsByGrade!]! vulnerabilityGrades(
"""
Include grades belonging to subgroups
"""
includeSubgroups: Boolean = false
): [VulnerableProjectsByGrade!]!
""" """
Vulnerability scanners reported on the project vulnerabilties of the group and its subgroups Vulnerability scanners reported on the project vulnerabilties of the group and its subgroups
......
...@@ -23634,7 +23634,16 @@ ...@@ -23634,7 +23634,16 @@
"name": "vulnerabilityGrades", "name": "vulnerabilityGrades",
"description": "Represents vulnerable project counts for each grade", "description": "Represents vulnerable project counts for each grade",
"args": [ "args": [
{
"name": "includeSubgroups",
"description": "Include grades belonging to subgroups",
"type": {
"kind": "SCALAR",
"name": "Boolean",
"ofType": null
},
"defaultValue": "false"
}
], ],
"type": { "type": {
"kind": "NON_NULL", "kind": "NON_NULL",
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
query groupVulnerabilityGrades($fullPath: ID!) { query groupVulnerabilityGrades($fullPath: ID!) {
group(fullPath: $fullPath) { group(fullPath: $fullPath) {
vulnerabilityGrades { vulnerabilityGrades(includeSubgroups: true) {
grade grade
projects { projects {
nodes { nodes {
......
...@@ -65,9 +65,7 @@ module EE ...@@ -65,9 +65,7 @@ module EE
[::Types::VulnerableProjectsByGradeType], [::Types::VulnerableProjectsByGradeType],
null: false, null: false,
description: 'Represents vulnerable project counts for each grade', description: 'Represents vulnerable project counts for each grade',
resolve: -> (obj, _args, ctx) { resolver: ::Resolvers::VulnerabilitiesGradeResolver
::Gitlab::Graphql::Aggregations::VulnerabilityStatistics::LazyAggregate.new(ctx, obj)
}
end end
end end
end end
......
# frozen_string_literal: true
module Resolvers
class VulnerabilitiesGradeResolver < VulnerabilitiesBaseResolver
type [::Types::VulnerableProjectsByGradeType], null: true
argument :include_subgroups, GraphQL::BOOLEAN_TYPE,
required: false,
default_value: false,
description: 'Include grades belonging to subgroups'
def resolve(**args)
::Gitlab::Graphql::Aggregations::VulnerabilityStatistics::LazyAggregate
.new(context, vulnerable, include_subgroups: args[:include_subgroups])
end
end
end
...@@ -19,9 +19,11 @@ module Vulnerabilities ...@@ -19,9 +19,11 @@ module Vulnerabilities
vulnerable.projects.with_vulnerability_statistics.inc_routes.where(id: project_ids) vulnerable.projects.with_vulnerability_statistics.inc_routes.where(id: project_ids)
end end
def self.grades_for(vulnerables) def self.grades_for(vulnerables, include_subgroups: false)
projects = include_subgroups ? vulnerables.map(&:all_projects) : vulnerables.map(&:projects)
::Vulnerabilities::Statistic ::Vulnerabilities::Statistic
.for_project(vulnerables.map(&:projects).reduce(&:or)) .for_project(projects.reduce(&:or))
.group(:letter_grade) .group(:letter_grade)
.select(:letter_grade, 'array_agg(project_id) project_ids') .select(:letter_grade, 'array_agg(project_id) project_ids')
.then do |statistics| .then do |statistics|
......
---
title: Include subgroups in GraphQL field for vulnerabilityGrades for Groups
merge_request: 45518
author:
type: changed
...@@ -7,27 +7,27 @@ module Gitlab ...@@ -7,27 +7,27 @@ module Gitlab
class LazyAggregate class LazyAggregate
attr_reader :vulnerable, :lazy_state attr_reader :vulnerable, :lazy_state
def initialize(query_ctx, vulnerable) def initialize(query_ctx, vulnerable, include_subgroups: false)
@vulnerable = vulnerable.respond_to?(:sync) ? vulnerable.sync : vulnerable @vulnerable = vulnerable.respond_to?(:sync) ? vulnerable.sync : vulnerable
@include_subgroups = include_subgroups
# Initialize the loading state for this query, # Initialize the loading state for this query,
# or get the previously-initiated state # or get the previously-initiated state
@lazy_state = query_ctx[:lazy_vulnerability_statistics_aggregate] ||= { @lazy_state = query_ctx[:lazy_vulnerability_statistics_aggregate] ||= {
pending_vulnerables: Set.new, pending_vulnerables: { true => Set.new, false => Set.new },
loaded_objects: {} loaded_objects: { true => {}, false => {} }
} }
# Register this ID to be loaded later: # Register this ID to be loaded later:
@lazy_state[:pending_vulnerables] << vulnerable @lazy_state[:pending_vulnerables][@include_subgroups] << vulnerable
end end
# Return the loaded record, hitting the database if needed # Return the loaded record, hitting the database if needed
def execute def execute
# Check if the record was already loaded # Check if the record was already loaded
if @lazy_state[:pending_vulnerables].present? if @lazy_state[:pending_vulnerables][@include_subgroups].present?
load_records_into_loaded_objects load_records_into_loaded_objects
end end
@lazy_state[:loaded_objects][@vulnerable] @lazy_state[:loaded_objects][@include_subgroups][@vulnerable]
end end
private private
...@@ -35,14 +35,17 @@ module Gitlab ...@@ -35,14 +35,17 @@ module Gitlab
def load_records_into_loaded_objects def load_records_into_loaded_objects
# The record hasn't been loaded yet, so # The record hasn't been loaded yet, so
# hit the database with all pending IDs to prevent N+1 # hit the database with all pending IDs to prevent N+1
pending_vulnerables = @lazy_state[:pending_vulnerables].to_a @lazy_state[:pending_vulnerables].each do |include_subgroups, pending_vulnerables|
grades = ::Vulnerabilities::ProjectsGrade.grades_for(pending_vulnerables) next if pending_vulnerables.blank?
pending_vulnerables.each do |vulnerable| grades = ::Vulnerabilities::ProjectsGrade.grades_for(pending_vulnerables, include_subgroups: include_subgroups)
@lazy_state[:loaded_objects][vulnerable] = grades[vulnerable]
end
@lazy_state[:pending_vulnerables].clear pending_vulnerables.each do |vulnerable|
@lazy_state[:loaded_objects][include_subgroups][vulnerable] = grades[vulnerable]
end
@lazy_state[:pending_vulnerables][include_subgroups].clear
end
end end
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Resolvers::VulnerabilitiesGradeResolver do
include GraphqlHelpers
subject { resolve(described_class, obj: group, args: args, ctx: { current_user: user }).execute }
let_it_be(:group) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:project) { create(:project, namespace: group) }
let_it_be(:project_in_subgroup) { create(:project, namespace: subgroup) }
let_it_be(:user) { create(:user) }
let_it_be(:vulnerability_statistic_1) { create(:vulnerability_statistic, :grade_f, project: project) }
let_it_be(:vulnerability_statistic_2) { create(:vulnerability_statistic, :grade_d, project: project_in_subgroup) }
describe '#resolve' do
let(:args) { { include_subgroups: include_subgroups } }
context 'when include_subgroups is set to true' do
let(:include_subgroups) { true }
it 'returns project grades for projects in group and its subgroups' do
expect(subject.map(&:grade)).to match_array(%w[d f])
end
end
context 'when include_subgroups is set to true' do
let(:include_subgroups) { false }
it 'returns project grades for projects in group only' do
expect(subject.map(&:grade)).to match_array(%w[f])
end
end
end
end
...@@ -7,29 +7,37 @@ RSpec.describe Gitlab::Graphql::Aggregations::VulnerabilityStatistics::LazyAggre ...@@ -7,29 +7,37 @@ RSpec.describe Gitlab::Graphql::Aggregations::VulnerabilityStatistics::LazyAggre
{} {}
end end
let(:vulnerable) { create(:group) } let_it_be(:vulnerable) { create(:group) }
let(:blocks_vulnerable) { 18 } let_it_be(:other_vulnerable) { create(:group) }
let(:blocking_vulnerable) { 38 } let(:include_subgroups) { true }
describe '#initialize' do describe '#initialize' do
it 'adds the vulnerable to the lazy state' do it 'adds the vulnerable to the lazy state' do
subject = described_class.new(query_ctx, vulnerable) subject = described_class.new(query_ctx, vulnerable, include_subgroups: true)
expect(subject.lazy_state[:pending_vulnerables]).to match [vulnerable] expect(subject.lazy_state[:pending_vulnerables]).to eq(true => Set.new([vulnerable]), false => Set.new)
expect(subject.vulnerable).to match vulnerable expect(subject.vulnerable).to match vulnerable
end end
end end
describe '#execute' do describe '#execute' do
subject { described_class.new(query_ctx, vulnerable) } let(:fake_data) do
{
vulnerable => [::Vulnerabilities::ProjectsGrade.new(vulnerable, 'a', [])],
other_vulnerable => [::Vulnerabilities::ProjectsGrade.new(other_vulnerable, 'b', [])]
}
end
subject { described_class.new(query_ctx, vulnerable, include_subgroups: include_subgroups) }
before do before do
subject.instance_variable_set(:@lazy_state, fake_state) subject.instance_variable_set(:@lazy_state, fake_state)
allow(::Vulnerabilities::ProjectsGrade).to receive(:grades_for).and_return(fake_data)
end end
context 'if the record has already been loaded' do context 'if the record has already been loaded' do
let(:fake_state) do let(:fake_state) do
{ pending_vulnerables: Set.new, loaded_objects: { vulnerable => [::Vulnerabilities::ProjectsGrade.new(vulnerable, 'a', [])] } } { pending_vulnerables: { true => Set.new, false => Set.new }, loaded_objects: { true => { vulnerable => [::Vulnerabilities::ProjectsGrade.new(vulnerable, 'a', [])] }, false => {} } }
end end
it 'does not make the query again' do it 'does not make the query again' do
...@@ -40,24 +48,14 @@ RSpec.describe Gitlab::Graphql::Aggregations::VulnerabilityStatistics::LazyAggre ...@@ -40,24 +48,14 @@ RSpec.describe Gitlab::Graphql::Aggregations::VulnerabilityStatistics::LazyAggre
end end
context 'if the record has not been loaded' do context 'if the record has not been loaded' do
let(:other_vulnerable) { create(:group) } let(:include_subgroups) { true }
let(:fake_state) do let(:fake_state) do
{ pending_vulnerables: Set.new([vulnerable, other_vulnerable]), loaded_objects: {} } { pending_vulnerables: { true => Set.new([vulnerable]), false => Set.new([other_vulnerable]) }, loaded_objects: { true => {}, false => {} } }
end
let(:fake_data) do
{
vulnerable => [::Vulnerabilities::ProjectsGrade.new(vulnerable, 'a', [])],
other_vulnerable => [::Vulnerabilities::ProjectsGrade.new(other_vulnerable, 'b', [])]
}
end
before do
allow(::Vulnerabilities::ProjectsGrade).to receive(:grades_for).and_return(fake_data)
end end
it 'makes the query' do it 'makes the query' do
expect(::Vulnerabilities::ProjectsGrade).to receive(:grades_for).with([vulnerable, other_vulnerable]) expect(::Vulnerabilities::ProjectsGrade).to receive(:grades_for).with([vulnerable], include_subgroups: true)
expect(::Vulnerabilities::ProjectsGrade).to receive(:grades_for).with([other_vulnerable], include_subgroups: false)
subject.execute subject.execute
end end
...@@ -65,7 +63,7 @@ RSpec.describe Gitlab::Graphql::Aggregations::VulnerabilityStatistics::LazyAggre ...@@ -65,7 +63,7 @@ RSpec.describe Gitlab::Graphql::Aggregations::VulnerabilityStatistics::LazyAggre
it 'clears the pending IDs' do it 'clears the pending IDs' do
subject.execute subject.execute
expect(subject.lazy_state[:pending_vulnerables]).to be_empty expect(subject.lazy_state[:pending_vulnerables].values).to all(be_empty)
end end
end end
end end
......
...@@ -4,38 +4,64 @@ require 'spec_helper' ...@@ -4,38 +4,64 @@ require 'spec_helper'
RSpec.describe Vulnerabilities::ProjectsGrade do RSpec.describe Vulnerabilities::ProjectsGrade do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:project_1) { create(:project, group: group) } let_it_be(:project_1) { create(:project, group: group) }
let_it_be(:project_2) { create(:project, group: group) } let_it_be(:project_2) { create(:project, group: group) }
let_it_be(:project_3) { create(:project, group: group) } let_it_be(:project_3) { create(:project, group: group) }
let_it_be(:project_4) { create(:project, group: group) } let_it_be(:project_4) { create(:project, group: group) }
let_it_be(:project_5) { create(:project, group: group) } let_it_be(:project_5) { create(:project, group: group) }
let_it_be(:project_6) { create(:project, group: subgroup) }
let_it_be(:vulnerability_statistic_1) { create(:vulnerability_statistic, :grade_a, project: project_1) } let_it_be(:vulnerability_statistic_1) { create(:vulnerability_statistic, :grade_a, project: project_1) }
let_it_be(:vulnerability_statistic_2) { create(:vulnerability_statistic, :grade_b, project: project_2) } let_it_be(:vulnerability_statistic_2) { create(:vulnerability_statistic, :grade_b, project: project_2) }
let_it_be(:vulnerability_statistic_3) { create(:vulnerability_statistic, :grade_b, project: project_3) } let_it_be(:vulnerability_statistic_3) { create(:vulnerability_statistic, :grade_b, project: project_3) }
let_it_be(:vulnerability_statistic_4) { create(:vulnerability_statistic, :grade_c, project: project_4) } let_it_be(:vulnerability_statistic_4) { create(:vulnerability_statistic, :grade_c, project: project_4) }
let_it_be(:vulnerability_statistic_5) { create(:vulnerability_statistic, :grade_f, project: project_5) } let_it_be(:vulnerability_statistic_5) { create(:vulnerability_statistic, :grade_f, project: project_5) }
let_it_be(:vulnerability_statistic_6) { create(:vulnerability_statistic, :grade_d, project: project_6) }
describe '.grades_for' do describe '.grades_for' do
let(:compare_key) { ->(projects_grade) { [projects_grade.grade, projects_grade.project_ids.sort] } } let(:compare_key) { ->(projects_grade) { [projects_grade.grade, projects_grade.project_ids.sort] } }
let(:include_subgroups) { false }
subject(:projects_grades) { described_class.grades_for([vulnerable]) } subject(:projects_grades) { described_class.grades_for([vulnerable], include_subgroups: include_subgroups) }
context 'when the given vulnerable is a Group' do context 'when the given vulnerable is a Group' do
let(:vulnerable) { group } let(:vulnerable) { group }
let(:expected_projects_grades) do
{ context 'when subgroups are not included' do
vulnerable => [ let(:expected_projects_grades) do
described_class.new(vulnerable, 'a', [project_1.id]), {
described_class.new(vulnerable, 'b', [project_2.id, project_3.id]), vulnerable => [
described_class.new(vulnerable, 'c', [project_4.id]), described_class.new(vulnerable, 'a', [project_1.id]),
described_class.new(vulnerable, 'f', [project_5.id]) described_class.new(vulnerable, 'b', [project_2.id, project_3.id]),
] described_class.new(vulnerable, 'c', [project_4.id]),
} described_class.new(vulnerable, 'f', [project_5.id])
]
}
end
it 'returns the letter grades for given vulnerable' do
expect(projects_grades[vulnerable].map(&compare_key)).to match_array(expected_projects_grades[vulnerable].map(&compare_key))
end
end end
it 'returns the letter grades for given vulnerable' do context 'when subgroups are included' do
expect(projects_grades[vulnerable].map(&compare_key)).to match_array(expected_projects_grades[vulnerable].map(&compare_key)) let(:include_subgroups) { true }
let(:expected_projects_grades) do
{
vulnerable => [
described_class.new(vulnerable, 'a', [project_1.id]),
described_class.new(vulnerable, 'b', [project_2.id, project_3.id]),
described_class.new(vulnerable, 'c', [project_4.id]),
described_class.new(vulnerable, 'd', [project_6.id]),
described_class.new(vulnerable, 'f', [project_5.id])
]
}
end
it 'returns the letter grades for given vulnerable' do
expect(projects_grades[vulnerable].map(&compare_key)).to match_array(expected_projects_grades[vulnerable].map(&compare_key))
end
end end
end end
......
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