From 3f8f4308d41c9db4be3ec128441b827a67209464 Mon Sep 17 00:00:00 2001
From: Brian Williams <bwilliams@gitlab.com>
Date: Mon, 20 May 2024 19:08:18 +0000
Subject: [PATCH] Add dependency aggregations finder
This finder implements a more performance version of the query used for
`Group#sbom_occurrences`. Is able to perform aggregations one page at a
time, while the current implementation of `Group#sbom_occurrences` must
read all the records in the group before aggregating them and then
finally applying the limit.
---
..._component_version_id_and_traversal_ids.rb | 19 ++
...index_sbom_occurrences_for_aggregations.rb | 19 ++
db/schema_migrations/20240506202834 | 1 +
db/schema_migrations/20240506202928 | 1 +
db/structure.sql | 4 +
.../groups/dependencies_controller.rb | 21 +-
.../aggregations_finder.rb | 70 ++++
ee/app/models/sbom/occurrence.rb | 15 +
ee/app/serializers/dependency_entity.rb | 10 +-
.../rewrite_sbom_occurrences_query.yml | 9 +
ee/spec/factories/sbom/occurrences.rb | 7 +-
.../user_sees_dependency_list_spec.rb | 47 +++
.../aggregations_finder_spec.rb | 70 ++++
ee/spec/models/sbom/occurrence_spec.rb | 12 +
.../groups/dependencies_controller_spec.rb | 311 ++++++++++--------
ee/spec/serializers/dependency_entity_spec.rb | 31 --
.../project_dependencies_service_spec.rb | 2 +-
.../sbom/sync_archived_status_service_spec.rb | 5 +-
.../sync_project_traversal_ids_worker_spec.rb | 2 +-
.../helpers/database/duplicate_indexes.yml | 3 +
spec/support/matchers/exceed_query_limit.rb | 9 +-
21 files changed, 492 insertions(+), 176 deletions(-)
create mode 100644 db/post_migrate/20240506202834_index_sbom_occurrences_on_component_version_id_and_traversal_ids.rb
create mode 100644 db/post_migrate/20240506202928_index_sbom_occurrences_for_aggregations.rb
create mode 100644 db/schema_migrations/20240506202834
create mode 100644 db/schema_migrations/20240506202928
create mode 100644 ee/app/finders/dependency_management/aggregations_finder.rb
create mode 100644 ee/config/feature_flags/gitlab_com_derisk/rewrite_sbom_occurrences_query.yml
create mode 100644 ee/spec/features/groups/security/user_sees_dependency_list_spec.rb
create mode 100644 ee/spec/finders/dependency_management/aggregations_finder_spec.rb
diff --git a/db/post_migrate/20240506202834_index_sbom_occurrences_on_component_version_id_and_traversal_ids.rb b/db/post_migrate/20240506202834_index_sbom_occurrences_on_component_version_id_and_traversal_ids.rb
new file mode 100644
index 0000000000000..088138b8ff876
--- /dev/null
+++ b/db/post_migrate/20240506202834_index_sbom_occurrences_on_component_version_id_and_traversal_ids.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class IndexSbomOccurrencesOnComponentVersionIdAndTraversalIds < Gitlab::Database::Migration[2.2]
+ INDEX_NAME = 'index_unarchived_occurrences_on_version_id_and_traversal_ids'
+
+ milestone '17.0'
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index :sbom_occurrences, [:component_version_id, :traversal_ids],
+ where: 'archived = false',
+ name: INDEX_NAME
+ end
+
+ def down
+ remove_concurrent_index_by_name :sbom_occurrences, INDEX_NAME
+ end
+end
diff --git a/db/post_migrate/20240506202928_index_sbom_occurrences_for_aggregations.rb b/db/post_migrate/20240506202928_index_sbom_occurrences_for_aggregations.rb
new file mode 100644
index 0000000000000..a01c3029b97e6
--- /dev/null
+++ b/db/post_migrate/20240506202928_index_sbom_occurrences_for_aggregations.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class IndexSbomOccurrencesForAggregations < Gitlab::Database::Migration[2.2]
+ INDEX_NAME = 'index_unarchived_sbom_occurrences_for_aggregations'
+
+ milestone '17.0'
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index :sbom_occurrences, [:traversal_ids, :component_id, :component_version_id],
+ where: 'archived = false',
+ name: INDEX_NAME
+ end
+
+ def down
+ remove_concurrent_index_by_name :sbom_occurrences, INDEX_NAME
+ end
+end
diff --git a/db/schema_migrations/20240506202834 b/db/schema_migrations/20240506202834
new file mode 100644
index 0000000000000..23d7973e89852
--- /dev/null
+++ b/db/schema_migrations/20240506202834
@@ -0,0 +1 @@
+0c07c0a916056fd73717c65348dcefd31deffc1bd291bf938ebee41ec23bf919
\ No newline at end of file
diff --git a/db/schema_migrations/20240506202928 b/db/schema_migrations/20240506202928
new file mode 100644
index 0000000000000..4d021c7ea4282
--- /dev/null
+++ b/db/schema_migrations/20240506202928
@@ -0,0 +1 @@
+10f2898e5a02425aca29e3fde273fc96ca3a862d681f05d777c696e3a1aca5ad
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 9b6edbf6a37ab..4012118ceae3c 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -27637,6 +27637,10 @@ CREATE INDEX index_topics_total_projects_count ON topics USING btree (total_proj
CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects USING btree (project_id);
+CREATE INDEX index_unarchived_occurrences_on_version_id_and_traversal_ids ON sbom_occurrences USING btree (component_version_id, traversal_ids) WHERE (archived = false);
+
+CREATE INDEX index_unarchived_sbom_occurrences_for_aggregations ON sbom_occurrences USING btree (traversal_ids, component_id, component_version_id) WHERE (archived = false);
+
CREATE UNIQUE INDEX index_uniq_ci_runners_on_token ON ci_runners USING btree (token);
CREATE UNIQUE INDEX index_uniq_ci_runners_on_token_encrypted ON ci_runners USING btree (token_encrypted);
diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
index cfadee01e8e6a..11353e3130643 100644
--- a/ee/app/controllers/groups/dependencies_controller.rb
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -3,6 +3,7 @@
module Groups
class DependenciesController < Groups::ApplicationController
include GovernUsageGroupTracking
+ include Gitlab::Utils::StrongMemoize
before_action only: :index do
push_frontend_feature_flag(:group_level_dependencies_filtering_by_component, group)
@@ -27,7 +28,6 @@ def index
render status: :ok
end
format.json do
- dependencies = dependencies_finder.execute.with_component.with_version.with_source.with_project_route
render json: dependencies_serializer.represent(dependencies)
end
end
@@ -72,8 +72,18 @@ def validate_project_ids_limit!
)
end
- def dependencies_finder
- ::Sbom::DependenciesFinder.new(group, params: dependencies_finder_params)
+ def dependencies
+ if using_new_query?
+ ::DependencyManagement::AggregationsFinder.new(group, params: dependencies_finder_params).execute
+ .with_component
+ .with_version
+ else
+ ::Sbom::DependenciesFinder.new(group, params: dependencies_finder_params).execute
+ .with_component
+ .with_version
+ .with_source
+ .with_project_route
+ end
end
def dependencies_finder_params
@@ -125,5 +135,10 @@ def set_below_group_limit
def below_group_limit?
group.count_within_namespaces <= GROUP_COUNT_LIMIT
end
+
+ def using_new_query?
+ ::Feature.enabled?(:rewrite_sbom_occurrences_query, group)
+ end
+ strong_memoize_attr :using_new_query?
end
end
diff --git a/ee/app/finders/dependency_management/aggregations_finder.rb b/ee/app/finders/dependency_management/aggregations_finder.rb
new file mode 100644
index 0000000000000..b6f0b650c2708
--- /dev/null
+++ b/ee/app/finders/dependency_management/aggregations_finder.rb
@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+module DependencyManagement
+ # rubocop:disable CodeReuse/ActiveRecord -- Code won't be reused outside this context
+ class AggregationsFinder
+ DEFAULT_PAGE_SIZE = 20
+ MAX_PAGE_SIZE = 20
+ DISTINCT_BY = %i[component_id component_version_id].freeze
+
+ def initialize(namespace, params: {})
+ @namespace = namespace
+ @params = params
+ end
+
+ def execute
+ Sbom::Occurrence.with(namespaces_cte.to_arel)
+ .select(
+ 'MIN(outer_occurrences.id)::bigint AS id',
+ 'outer_occurrences.component_id',
+ 'outer_occurrences.component_version_id',
+ 'MIN(outer_occurrences.package_manager) AS package_manager',
+ 'MIN(outer_occurrences.input_file_path) AS input_file_path',
+ 'JSONB_AGG(outer_occurrences.licenses->0) AS licenses',
+ 'SUM(counts.occurrence_count)::integer AS occurrence_count',
+ 'SUM(counts.vulnerability_count)::integer AS vulnerability_count',
+ 'SUM(counts.project_count)::integer AS project_count'
+ )
+ .from("(#{outer_occurrences.to_sql}) outer_occurrences, LATERAL (#{counts.to_sql}) counts")
+ .group('outer_occurrences.component_id', 'outer_occurrences.component_version_id')
+ .order('MIN(outer_occurrences.component_id) ASC', 'MIN(outer_occurrences.component_version_id) ASC')
+ end
+
+ private
+
+ attr_reader :namespace, :params
+
+ def namespaces_cte
+ ::Gitlab::SQL::CTE.new(:namespaces, namespace.self_and_descendants.select(:traversal_ids))
+ end
+
+ def inner_occurrences
+ Sbom::Occurrence.where('sbom_occurrences.traversal_ids = namespaces.traversal_ids::bigint[]')
+ .unarchived
+ .order(*DISTINCT_BY)
+ .limit(page_size)
+ .select_distinct(on: DISTINCT_BY)
+ end
+
+ def outer_occurrences
+ Sbom::Occurrence.select_distinct(on: DISTINCT_BY, table_name: '"inner_occurrences"')
+ .from("namespaces, LATERAL (#{inner_occurrences.to_sql}) inner_occurrences")
+ .order('inner_occurrences.component_id ASC', 'inner_occurrences.component_version_id ASC')
+ .limit(page_size)
+ end
+
+ def counts
+ Sbom::Occurrence.select('COUNT(project_id) AS occurrence_count')
+ .select('COUNT(DISTINCT project_id) project_count')
+ .select('SUM(vulnerability_count) vulnerability_count')
+ .for_namespace_and_descendants(namespace)
+ .unarchived
+ .where('sbom_occurrences.component_version_id = outer_occurrences.component_version_id')
+ end
+
+ def page_size
+ [params.fetch(:per_page, DEFAULT_PAGE_SIZE), MAX_PAGE_SIZE].min
+ end
+ end
+ # rubocop:enable CodeReuse/ActiveRecord
+end
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index 6587ace5db522..5f01326d6bc81 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -77,8 +77,13 @@ class Occurrence < ApplicationRecord
where(query_parts.join(' OR '), licenses: Array(ids))
end
+ scope :unarchived, -> { where(archived: false) }
scope :by_project_ids, ->(project_ids) { where(project_id: project_ids) }
scope :by_uuids, ->(uuids) { where(uuid: uuids) }
+ scope :for_namespace_and_descendants, ->(namespace) do
+ where("traversal_ids >= ('{?}')", namespace.traversal_ids)
+ .where("traversal_ids < ('{?}')", namespace.next_traversal_ids)
+ end
scope :filter_by_package_managers, ->(package_managers) do
where(package_manager: package_managers)
@@ -137,6 +142,16 @@ class Occurrence < ApplicationRecord
.where(project_authorizations: { user_id: user.id })
end
+ def self.select_distinct(on:, table_name: quoted_table_name)
+ quote = ->(column) { "#{table_name}.#{connection.quote_column_name(column)}" }
+
+ distinct_values = on.map { |column| quote.call(column) }
+
+ select_values = column_names.map { |column| quote.call(column) }
+
+ select("DISTINCT ON (#{distinct_values.join(', ')}) #{select_values.join(', ')}")
+ end
+
def location
{
blob_path: input_file_blob_path,
diff --git a/ee/app/serializers/dependency_entity.rb b/ee/app/serializers/dependency_entity.rb
index b26b91edcbd89..552893bce4f66 100644
--- a/ee/app/serializers/dependency_entity.rb
+++ b/ee/app/serializers/dependency_entity.rb
@@ -70,7 +70,7 @@ class ProjectEntity < Grape::Entity
end
expose :name, :packager, :version
- expose :location, using: LocationEntity
+ expose :location, using: LocationEntity, if: ->(_) { !using_aggregations? }
expose :vulnerabilities, using: VulnerabilityEntity, if: ->(_) { render_vulnerabilities? }
expose :licenses, using: LicenseEntity, if: ->(_) { can_read_licenses? } do |object|
object[:licenses].presence || [::Gitlab::LicenseScanning::PackageLicenses::UNKNOWN_LICENSE]
@@ -78,7 +78,7 @@ class ProjectEntity < Grape::Entity
expose :occurrence_count, if: ->(_) { group? } do |object|
object.respond_to?(:occurrence_count) ? object.occurrence_count : 1
end
- expose :project, using: ProjectEntity, if: ->(_) { group? }
+ expose :project, using: ProjectEntity, if: ->(_) { group? && !using_aggregations? }
expose :project_count, if: ->(_) { group? } do |object|
object.respond_to?(:project_count) ? object.project_count : 1
end
@@ -125,4 +125,10 @@ def project_level_sbom_occurrences_enabled?
project = request.try(:project)
project.present? && Feature.enabled?(:project_level_sbom_occurrences, project)
end
+
+ def using_aggregations?
+ return false unless group?
+
+ Feature.enabled?(:rewrite_sbom_occurrences_query, request.group)
+ end
end
diff --git a/ee/config/feature_flags/gitlab_com_derisk/rewrite_sbom_occurrences_query.yml b/ee/config/feature_flags/gitlab_com_derisk/rewrite_sbom_occurrences_query.yml
new file mode 100644
index 0000000000000..c8a991c6c8f8c
--- /dev/null
+++ b/ee/config/feature_flags/gitlab_com_derisk/rewrite_sbom_occurrences_query.yml
@@ -0,0 +1,9 @@
+---
+name: rewrite_sbom_occurrences_query
+feature_issue_url: https://gitlab.com/groups/gitlab-org/-/epics/12371
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145704
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/442672
+milestone: '17.0'
+group: group::threat insights
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/ee/spec/factories/sbom/occurrences.rb b/ee/spec/factories/sbom/occurrences.rb
index fc6ec9b679c62..e462807a429e2 100644
--- a/ee/spec/factories/sbom/occurrences.rb
+++ b/ee/spec/factories/sbom/occurrences.rb
@@ -9,6 +9,8 @@
component { component_version&.component || association(:sbom_component) }
source { association :sbom_source, packager_name: packager_name }
source_package { association :sbom_source_package }
+ traversal_ids { project.namespace.traversal_ids }
+ archived { project.archived }
trait :os_occurrence do
source do
@@ -18,10 +20,7 @@
end
trait :with_vulnerabilities do
- transient do
- vulnerability_count { 2 }
- end
-
+ vulnerability_count { 2 }
vulnerabilities { build_list(:vulnerability, vulnerability_count) }
end
diff --git a/ee/spec/features/groups/security/user_sees_dependency_list_spec.rb b/ee/spec/features/groups/security/user_sees_dependency_list_spec.rb
new file mode 100644
index 0000000000000..865d7c1ea348f
--- /dev/null
+++ b/ee/spec/features/groups/security/user_sees_dependency_list_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe "User sees dependency list", :js, feature_category: :vulnerability_management do
+ let_it_be(:owner) { create(:user) }
+ let_it_be(:group) { create(:group) }
+ let_it_be(:project) { create(:project, group: group) }
+ let_it_be(:ci_pipeline) { create(:ee_ci_pipeline, :with_cyclonedx_pypi_only, project: project) }
+
+ before do
+ stub_licensed_features(
+ security_dashboard: true,
+ dependency_scanning: true
+ )
+ end
+
+ before_all do
+ ::Sbom::Ingestion::IngestReportsService.execute(ci_pipeline)
+ group.add_owner(owner)
+ sign_in(owner)
+ end
+
+ it "shows the dependency list" do
+ visit(group_dependencies_path(group))
+
+ # The default sort order of the page is by 'Severity' which orders by
+ # `sbom_occurrences.hightest_severity_level NULL LAST`. All of the test
+ # data is NULL for this field so the order is indeterminate.
+ # So just check for some content and the right number of elements.
+ within_testid("dependencies-table-content") do
+ expect(page).to have_content "beautifulsoup4"
+ expect(page).to have_content "soupsieve"
+ expect(page).to have_selector("tbody tr", count: 12)
+ end
+
+ within(".gl-sorting") do
+ click_on "Severity"
+ find("li", text: "Component name").click
+ end
+
+ within_testid("dependencies-table-content") do
+ expect(find("tbody tr:first-child")).to have_content "beautifulsoup4"
+ expect(find("tbody tr:last-child")).to have_content "soupsieve"
+ end
+ end
+end
diff --git a/ee/spec/finders/dependency_management/aggregations_finder_spec.rb b/ee/spec/finders/dependency_management/aggregations_finder_spec.rb
new file mode 100644
index 0000000000000..3ba3676bf4e5b
--- /dev/null
+++ b/ee/spec/finders/dependency_management/aggregations_finder_spec.rb
@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DependencyManagement::AggregationsFinder, feature_category: :dependency_management do
+ let_it_be(:target_group) { create(:group) }
+ let(:params) { {} }
+ let_it_be(:subgroup) { create(:group, parent: target_group) }
+ let_it_be(:outside_group) { create(:group) }
+ let_it_be(:outside_project) { create(:project, group: outside_group) }
+ let_it_be(:archived_project) { create(:project, :archived, group: target_group) }
+
+ let_it_be(:target_projects) do
+ [
+ create(:project, group: target_group),
+ create(:project, group: subgroup)
+ ]
+ end
+
+ let_it_be(:target_occurrences) do
+ target_projects.map do |project|
+ create(:sbom_occurrence, :with_vulnerabilities, project: project)
+ end
+ end
+
+ before_all do
+ # Records that should not be returned in the results
+ create(:sbom_occurrence, project: outside_project)
+ create(:sbom_occurrence, project: archived_project)
+ end
+
+ describe '#execute' do
+ subject(:execute) { described_class.new(target_group, params: params).execute }
+
+ it 'returns occurrences in target group hierarchy' do
+ expected = target_occurrences.map do |occurrence|
+ an_object_having_attributes(
+ component_id: occurrence.component_id,
+ component_version_id: occurrence.component_version_id,
+ occurrence_count: 1,
+ project_count: 1,
+ vulnerability_count: 2
+ )
+ end
+
+ expect(execute).to match_array(expected)
+ end
+
+ describe 'pagination' do
+ let(:params) { { per_page: 1 } }
+
+ it 'uses per_page to determine page size' do
+ expect(execute.to_a.size).to eq(1)
+ end
+
+ context 'when per_page is over max page size' do
+ let(:params) { { per_page: 2 } }
+ let(:max) { 1 }
+
+ before do
+ stub_const("#{described_class}::MAX_PAGE_SIZE", max)
+ end
+
+ it 'returns max number of items' do
+ expect(execute.to_a.size).to eq(max)
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/models/sbom/occurrence_spec.rb b/ee/spec/models/sbom/occurrence_spec.rb
index ccfac010485e0..ff4cb0f995de1 100644
--- a/ee/spec/models/sbom/occurrence_spec.rb
+++ b/ee/spec/models/sbom/occurrence_spec.rb
@@ -306,6 +306,18 @@
end
end
+ describe '.unarchived' do
+ let_it_be(:unarchived_occurrence) { create(:sbom_occurrence) }
+
+ before_all do
+ create(:sbom_occurrence, project: create(:project, :archived))
+ end
+
+ it 'returns only unarchived occurrences' do
+ expect(described_class.unarchived).to eq([unarchived_occurrence])
+ end
+ end
+
describe '.by_project_ids' do
let_it_be(:occurrence_1) { create(:sbom_occurrence) }
let_it_be(:occurrence_2) { create(:sbom_occurrence) }
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index e28f82b01678e..b68bfd9068e41 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -6,7 +6,10 @@
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
+ let(:using_new_query) { false }
+
before do
+ stub_feature_flags(rewrite_sbom_occurrences_query: using_new_query)
sign_in(user)
end
@@ -127,6 +130,11 @@
let_it_be(:project) { create(:project, group: group) }
let_it_be(:sbom_occurrence_npm) { create(:sbom_occurrence, :mit, :npm, project: project) }
let_it_be(:sbom_occurrence_bundler) { create(:sbom_occurrence, :apache_2, :bundler, project: project) }
+ let_it_be(:archived_occurrence) do
+ create(:sbom_occurrence, project: create(:project, :archived, group: group))
+ end
+
+ let(:using_new_query) { true }
let(:expected_response) do
{
@@ -135,7 +143,6 @@
},
'dependencies' => [
{
- 'location' => sbom_occurrence_npm.location.as_json,
'name' => sbom_occurrence_npm.name,
'packager' => sbom_occurrence_npm.packager,
'version' => sbom_occurrence_npm.version,
@@ -148,13 +155,11 @@
],
'occurrence_count' => 1,
'project_count' => 1,
- "project" => { "full_path" => project.full_path, "name" => project.name },
"component_id" => sbom_occurrence_npm.component_version_id,
"occurrence_id" => sbom_occurrence_npm.id,
"vulnerability_count" => 0
},
{
- 'location' => sbom_occurrence_bundler.location.as_json,
'name' => sbom_occurrence_bundler.name,
'packager' => sbom_occurrence_bundler.packager,
'version' => sbom_occurrence_bundler.version,
@@ -167,7 +172,6 @@
],
'occurrence_count' => 1,
'project_count' => 1,
- "project" => { "full_path" => project.full_path, "name" => project.name },
"component_id" => sbom_occurrence_bundler.component_version_id,
"occurrence_id" => sbom_occurrence_bundler.id,
"vulnerability_count" => 0
@@ -188,187 +192,234 @@
expect(response).to include_pagination_headers
end
- it 'avoids N+1 database queries related to projects and routes', :aggregate_failures do
- control = ActiveRecord::QueryRecorder.new(skip_cached: false) { subject }
+ it 'avoids N+1 database queries', :aggregate_failures do
+ recording = ActiveRecord::QueryRecorder.new { subject }
- project_routes_count = control.log.count do |entry|
- entry[/\Aselect.+routes.+from.+routes.+where.+routes(.+source_id|.+source_type.+project){2}/i]
- end
- project_count = control.log.count do |entry|
- entry[/\Aselect.+projects.+from.+projects.+where.+projects.+id/i]
- end
- component_versions_count = control.log.count do |entry|
- entry[/\Aselect.+sbom_component_versions.+from.+sbom_component_versions/i]
- end
- sbom_sources_count = control.log.count do |entry|
- entry[/\Aselect.+sbom_sources.+from.+sbom_sources/i]
- end
- sbom_components_count = control.log.count do |entry|
- entry[/\Aselect.+sbom_components.+from.+sbom_components/i]
- end
-
- expect(project_routes_count).to eq(1)
- expect(project_count).to eq(1)
- expect(component_versions_count).to eq(1)
- expect(sbom_sources_count).to eq(1)
- expect(sbom_components_count).to eq(1)
+ expect(recording).not_to exceed_all_query_limit(1).for_model(::Sbom::Component)
+ expect(recording).not_to exceed_all_query_limit(1).for_model(::Sbom::ComponentVersion)
+ expect(recording).not_to exceed_all_query_limit(1).for_model(::Sbom::Source)
end
- context 'with sorting params' do
- context 'when sorted by packager in ascending order' do
- let(:params) { { group_id: group.to_param, sort_by: 'packager', sort: 'asc' } }
+ context 'when using old query' do
+ let(:using_new_query) { false }
- it 'returns sorted list' do
- subject
+ let(:expected_response) do
+ {
+ 'report' => {
+ 'status' => 'ok'
+ },
+ 'dependencies' => [
+ {
+ 'location' => sbom_occurrence_npm.location.as_json,
+ 'name' => sbom_occurrence_npm.name,
+ 'packager' => sbom_occurrence_npm.packager,
+ 'version' => sbom_occurrence_npm.version,
+ 'licenses' => [
+ {
+ 'spdx_identifier' => 'MIT',
+ 'name' => 'MIT License',
+ 'url' => 'https://spdx.org/licenses/MIT.html'
+ }
+ ],
+ 'occurrence_count' => 1,
+ 'project_count' => 1,
+ "project" => { "full_path" => project.full_path, "name" => project.name },
+ "component_id" => sbom_occurrence_npm.component_version_id,
+ "occurrence_id" => sbom_occurrence_npm.id,
+ "vulnerability_count" => 0
+ },
+ {
+ 'location' => sbom_occurrence_bundler.location.as_json,
+ 'name' => sbom_occurrence_bundler.name,
+ 'packager' => sbom_occurrence_bundler.packager,
+ 'version' => sbom_occurrence_bundler.version,
+ 'licenses' => [
+ {
+ 'spdx_identifier' => 'Apache-2.0',
+ 'name' => 'Apache 2.0 License',
+ 'url' => 'https://spdx.org/licenses/Apache-2.0.html'
+ }
+ ],
+ 'occurrence_count' => 1,
+ 'project_count' => 1,
+ "project" => { "full_path" => project.full_path, "name" => project.name },
+ "component_id" => sbom_occurrence_bundler.component_version_id,
+ "occurrence_id" => sbom_occurrence_bundler.id,
+ "vulnerability_count" => 0
+ }
+ ]
+ }
+ end
- expect(json_response['dependencies'].first['packager']).to eq('bundler')
- expect(json_response['dependencies'].last['packager']).to eq('npm')
- end
+ before_all do
+ # The old version of the query has a functional bug where it returns records from archived projects.
+ # This oversight is fixed in the new version of the query. Once all tests are migrated over to the
+ # new query, this line should be deleted.
+ archived_occurrence.destroy!
+ end
+
+ it 'returns the expected response' do
+ subject
+
+ expect(json_response).to eq(expected_response)
end
- context 'when sorted by packager in descending order' do
- let(:params) { { group_id: group.to_param, sort_by: 'packager', sort: 'desc' } }
+ context 'with sorting params' do
+ context 'when sorted by packager in ascending order' do
+ let(:params) { { group_id: group.to_param, sort_by: 'packager', sort: 'asc' } }
- it 'returns sorted list' do
- subject
+ it 'returns sorted list' do
+ subject
- expect(json_response['dependencies'].first['packager']).to eq('npm')
- expect(json_response['dependencies'].last['packager']).to eq('bundler')
+ expect(json_response['dependencies'].first['packager']).to eq('bundler')
+ expect(json_response['dependencies'].last['packager']).to eq('npm')
+ end
end
- end
- context 'when sorted by name in ascending order' do
- let(:params) { { group_id: group.to_param, sort_by: 'name', sort: 'asc' } }
+ context 'when sorted by packager in descending order' do
+ let(:params) { { group_id: group.to_param, sort_by: 'packager', sort: 'desc' } }
- it 'returns sorted list' do
- subject
+ it 'returns sorted list' do
+ subject
- expect(json_response['dependencies'].first['name']).to eq(sbom_occurrence_npm.name)
- expect(json_response['dependencies'].last['name']).to eq(sbom_occurrence_bundler.name)
+ expect(json_response['dependencies'].first['packager']).to eq('npm')
+ expect(json_response['dependencies'].last['packager']).to eq('bundler')
+ end
end
- end
- context 'when sorted by name in descending order' do
- let(:params) { { group_id: group.to_param, sort_by: 'name', sort: 'desc' } }
+ context 'when sorted by name in ascending order' do
+ let(:params) { { group_id: group.to_param, sort_by: 'name', sort: 'asc' } }
- it 'returns sorted list' do
- subject
+ it 'returns sorted list' do
+ subject
- expect(json_response['dependencies'].first['name']).to eq(sbom_occurrence_bundler.name)
- expect(json_response['dependencies'].last['name']).to eq(sbom_occurrence_npm.name)
+ expect(json_response['dependencies'].first['name']).to eq(sbom_occurrence_npm.name)
+ expect(json_response['dependencies'].last['name']).to eq(sbom_occurrence_bundler.name)
+ end
end
- end
- context 'when sorted by license in ascending order' do
- let(:params) { { group_id: group.to_param, sort_by: 'license', sort: 'asc' } }
+ context 'when sorted by name in descending order' do
+ let(:params) { { group_id: group.to_param, sort_by: 'name', sort: 'desc' } }
- it 'returns sorted list' do
- subject
+ it 'returns sorted list' do
+ subject
- expect(json_response['dependencies'].first['licenses'][0]['spdx_identifier']).to eq('Apache-2.0')
- expect(json_response['dependencies'].last['licenses'][0]['spdx_identifier']).to eq('MIT')
+ expect(json_response['dependencies'].first['name']).to eq(sbom_occurrence_bundler.name)
+ expect(json_response['dependencies'].last['name']).to eq(sbom_occurrence_npm.name)
+ end
end
- end
- context 'when sorted by license in descending order' do
- let(:params) { { group_id: group.to_param, sort_by: 'license', sort: 'desc' } }
+ context 'when sorted by license in ascending order' do
+ let(:params) { { group_id: group.to_param, sort_by: 'license', sort: 'asc' } }
- it 'returns sorted list' do
- subject
+ it 'returns sorted list' do
+ subject
- expect(json_response['dependencies'].first['licenses'][0]['spdx_identifier']).to eq('MIT')
- expect(json_response['dependencies'].last['licenses'][0]['spdx_identifier']).to eq('Apache-2.0')
+ expect(json_response['dependencies'].first['licenses'][0]['spdx_identifier']).to eq('Apache-2.0')
+ expect(json_response['dependencies'].last['licenses'][0]['spdx_identifier']).to eq('MIT')
+ end
end
- end
- end
- context 'with filtering params' do
- context 'when filtered by package managers' do
- let(:params) { { group_id: group.to_param, package_managers: ['npm'] } }
+ context 'when sorted by license in descending order' do
+ let(:params) { { group_id: group.to_param, sort_by: 'license', sort: 'desc' } }
- it 'returns filtered list' do
- subject
+ it 'returns sorted list' do
+ subject
- expect(json_response['dependencies'].count).to eq(1)
- expect(json_response['dependencies'].pluck('packager')).to eq(['npm'])
+ expect(json_response['dependencies'].first['licenses'][0]['spdx_identifier']).to eq('MIT')
+ expect(json_response['dependencies'].last['licenses'][0]['spdx_identifier']).to eq('Apache-2.0')
+ end
end
end
- context 'when filtered by component_names' do
- let(:params) do
- {
- group_id: group.to_param,
- component_names: [sbom_occurrence_bundler.name]
- }
- end
+ context 'with filtering params' do
+ context 'when filtered by package managers' do
+ let(:params) { { group_id: group.to_param, package_managers: ['npm'] } }
- it 'returns filtered list' do
- subject
+ it 'returns filtered list' do
+ subject
- expect(json_response['dependencies'].count).to eq(1)
- expect(json_response['dependencies'].pluck('name')).to eq([sbom_occurrence_bundler.name])
+ expect(json_response['dependencies'].count).to eq(1)
+ expect(json_response['dependencies'].pluck('packager')).to eq(['npm'])
+ end
end
- context 'when the group hierarchy depth is too high' do
- before do
- stub_const('::Groups::DependenciesController::GROUP_COUNT_LIMIT', 0)
+ context 'when filtered by component_names' do
+ let(:params) do
+ {
+ group_id: group.to_param,
+ component_names: [sbom_occurrence_bundler.name]
+ }
end
- it 'ignores the filter' do
+ it 'returns filtered list' do
subject
- expect(json_response['dependencies'].pluck('name')).to match_array([
- sbom_occurrence_bundler.component_name,
- sbom_occurrence_npm.component_name
- ])
+ expect(json_response['dependencies'].count).to eq(1)
+ expect(json_response['dependencies'].pluck('name')).to eq([sbom_occurrence_bundler.name])
end
- end
- end
- context 'when filtered by licenses' do
- let(:params) do
- {
- group_id: group.to_param,
- licenses: ['Apache-2.0']
- }
- end
+ context 'when the group hierarchy depth is too high' do
+ before do
+ stub_const('::Groups::DependenciesController::GROUP_COUNT_LIMIT', 0)
+ end
- it 'returns a filtered list' do
- subject
+ it 'ignores the filter' do
+ subject
- expect(json_response['dependencies'].count).to eq(1)
- expect(json_response['dependencies'].pluck('name')).to eq([sbom_occurrence_bundler.name])
+ expect(json_response['dependencies'].pluck('name')).to match_array([
+ sbom_occurrence_bundler.component_name,
+ sbom_occurrence_npm.component_name
+ ])
+ end
+ end
end
- end
- context 'when filtered by unknown licenses' do
- let_it_be(:sbom_occurrence_unknown) { create(:sbom_occurrence, project: project) }
+ context 'when filtered by licenses' do
+ let(:params) do
+ {
+ group_id: group.to_param,
+ licenses: ['Apache-2.0']
+ }
+ end
- let(:params) do
- {
- group_id: group.to_param,
- licenses: ['unknown']
- }
+ it 'returns a filtered list' do
+ subject
+
+ expect(json_response['dependencies'].count).to eq(1)
+ expect(json_response['dependencies'].pluck('name')).to eq([sbom_occurrence_bundler.name])
+ end
end
- it 'returns a filtered list' do
- subject
+ context 'when filtered by unknown licenses' do
+ let_it_be(:sbom_occurrence_unknown) { create(:sbom_occurrence, project: project) }
- expect(json_response['dependencies'].pluck('occurrence_id')).to eq([sbom_occurrence_unknown.id])
+ let(:params) do
+ {
+ group_id: group.to_param,
+ licenses: ['unknown']
+ }
+ end
+
+ it 'returns a filtered list' do
+ subject
+
+ expect(json_response['dependencies'].pluck('occurrence_id')).to eq([sbom_occurrence_unknown.id])
+ end
end
- end
- context 'when filtered by projects' do
- let_it_be(:other_project) { create(:project, group: group) }
- let_it_be(:occurrence_from_other_project) { create(:sbom_occurrence, project: other_project) }
+ context 'when filtered by projects' do
+ let_it_be(:other_project) { create(:project, group: group) }
+ let_it_be(:occurrence_from_other_project) { create(:sbom_occurrence, project: other_project) }
- let(:params) { { project_ids: [other_project.id] } }
+ let(:params) { { project_ids: [other_project.id] } }
- it 'returns a filtered list' do
- subject
+ it 'returns a filtered list' do
+ subject
- expect(json_response['dependencies'].count).to eq(1)
- expect(json_response['dependencies'].pluck('name')).to eq([occurrence_from_other_project.name])
+ expect(json_response['dependencies'].count).to eq(1)
+ expect(json_response['dependencies'].pluck('name')).to eq([occurrence_from_other_project.name])
+ end
end
context 'when trying to search for too many projects' do
diff --git a/ee/spec/serializers/dependency_entity_spec.rb b/ee/spec/serializers/dependency_entity_spec.rb
index 15bccb96d5665..e0cea9357fcf2 100644
--- a/ee/spec/serializers/dependency_entity_spec.rb
+++ b/ee/spec/serializers/dependency_entity_spec.rb
@@ -72,27 +72,6 @@
is_expected.to eq(dependency.except(:vulnerabilities, :package_manager, :iid))
end
end
-
- context 'with project' do
- let(:project) { create(:project, :repository, :private, :in_group) }
- let(:dependency) { build(:dependency, project: project) }
-
- before do
- allow(request).to receive(:project).and_return(nil)
- allow(request).to receive(:group).and_return(project.group)
- end
-
- it 'includes project name and full_path' do
- result = subject
-
- expect(result.dig(:project, :full_path)).to eq(project.full_path)
- expect(result.dig(:project, :name)).to eq(project.name)
- end
-
- it 'includes component_id' do
- expect(subject.keys).to include(:component_id)
- end
- end
end
context 'when all required features are unavailable' do
@@ -136,20 +115,10 @@
"name" => sbom_occurrence.name,
"occurrence_count" => 1,
"packager" => sbom_occurrence.packager,
- "project" => {
- "name" => project.name,
- "full_path" => project.full_path
- },
"project_count" => 1,
"version" => sbom_occurrence.version,
"licenses" => sbom_occurrence.licenses,
"component_id" => sbom_occurrence.component_version_id,
- "location" => {
- "ancestors" => sbom_occurrence.ancestors,
- "blob_path" => sbom_occurrence.location[:blob_path],
- "path" => sbom_occurrence.location[:path],
- "top_level" => sbom_occurrence.location[:top_level]
- },
"vulnerability_count" => 0,
"occurrence_id" => sbom_occurrence.id
})
diff --git a/ee/spec/services/dependencies/export_serializers/project_dependencies_service_spec.rb b/ee/spec/services/dependencies/export_serializers/project_dependencies_service_spec.rb
index 7eab1d6e9dc49..f2f7a1e485325 100644
--- a/ee/spec/services/dependencies/export_serializers/project_dependencies_service_spec.rb
+++ b/ee/spec/services/dependencies/export_serializers/project_dependencies_service_spec.rb
@@ -61,7 +61,7 @@ def json_dependency(occurrence)
},
'licenses' => occurrence.licenses,
'vulnerabilities' => vulnerabilities,
- 'vulnerability_count' => 0
+ 'vulnerability_count' => 2
}
end
diff --git a/ee/spec/services/sbom/sync_archived_status_service_spec.rb b/ee/spec/services/sbom/sync_archived_status_service_spec.rb
index b26f5228e5397..aa38549f3ca5c 100644
--- a/ee/spec/services/sbom/sync_archived_status_service_spec.rb
+++ b/ee/spec/services/sbom/sync_archived_status_service_spec.rb
@@ -4,8 +4,8 @@
RSpec.describe Sbom::SyncArchivedStatusService, feature_category: :dependency_management do
let_it_be(:project) { create(:project, :archived) }
- let_it_be(:sbom_occurrence) { create(:sbom_occurrence, project: project) }
- let_it_be(:sbom_occurrence_2) { create(:sbom_occurrence, project: project) }
+ let_it_be(:sbom_occurrence) { create(:sbom_occurrence, archived: false, project: project) }
+ let_it_be(:sbom_occurrence_2) { create(:sbom_occurrence, archived: false, project: project) }
let(:project_id) { project.id }
@@ -13,6 +13,7 @@
it 'updates sbom_occurrences.archived' do
expect { sync }.to change { sbom_occurrence.reload.archived }.from(false).to(true)
+ .and change { sbom_occurrence_2.reload.archived }.from(false).to(true)
end
context 'when project does not exist with id' do
diff --git a/ee/spec/workers/sbom/sync_project_traversal_ids_worker_spec.rb b/ee/spec/workers/sbom/sync_project_traversal_ids_worker_spec.rb
index 81432c5a07ae8..fdf43f3e49b7d 100644
--- a/ee/spec/workers/sbom/sync_project_traversal_ids_worker_spec.rb
+++ b/ee/spec/workers/sbom/sync_project_traversal_ids_worker_spec.rb
@@ -4,7 +4,7 @@
RSpec.describe Sbom::SyncProjectTraversalIdsWorker, feature_category: :dependency_management, type: :worker do
let_it_be(:project) { create(:project) }
- let_it_be(:sbom_occurrence) { create(:sbom_occurrence, project: project) }
+ let_it_be(:sbom_occurrence) { create(:sbom_occurrence, traversal_ids: [], project: project) }
let(:job_args) { project.id }
diff --git a/spec/support/helpers/database/duplicate_indexes.yml b/spec/support/helpers/database/duplicate_indexes.yml
index a010f048453bd..26631b407bb44 100644
--- a/spec/support/helpers/database/duplicate_indexes.yml
+++ b/spec/support/helpers/database/duplicate_indexes.yml
@@ -107,6 +107,9 @@ requirements_management_test_reports:
sbom_component_versions:
index_sbom_component_versions_on_component_id_and_version:
- index_sbom_component_versions_on_component_id
+sbom_occurrences:
+ index_unarchived_occurrences_on_version_id_and_traversal_ids:
+ - index_sbom_occurrences_on_component_version_id
search_namespace_index_assignments:
index_search_namespace_index_assignments_uniqueness_index_type:
- index_search_namespace_index_assignments_on_namespace_id
diff --git a/spec/support/matchers/exceed_query_limit.rb b/spec/support/matchers/exceed_query_limit.rb
index cc912d8de661f..71cdac317e720 100644
--- a/spec/support/matchers/exceed_query_limit.rb
+++ b/spec/support/matchers/exceed_query_limit.rb
@@ -194,7 +194,7 @@ def log_message
#{sections.join("\n\n")}
MSG
else
- @recorder.log_message
+ query_log(@recorder).join("\n\n")
end
end
@@ -348,7 +348,12 @@ def skip_cached
include ExceedQueryLimitHelpers
match do |block|
- verify_count(&block)
+ if block.is_a?(ActiveRecord::QueryRecorder)
+ @recorder = block
+ verify_count
+ else
+ verify_count(&block)
+ end
end
failure_message_when_negated do |actual|
--
GitLab