diff --git a/db/post_migrate/20240619114215_index_unarchived_sbom_occurrences_for_aggregations_license.rb b/db/post_migrate/20240619114215_index_unarchived_sbom_occurrences_for_aggregations_license.rb
new file mode 100644
index 0000000000000000000000000000000000000000..8b6e166879a6875c3a6e106869b62edc3f6f5513
--- /dev/null
+++ b/db/post_migrate/20240619114215_index_unarchived_sbom_occurrences_for_aggregations_license.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class IndexUnarchivedSbomOccurrencesForAggregationsLicense < Gitlab::Database::Migration[2.2]
+ INDEX_NAME = 'index_unarchived_occurrences_for_aggregations_license'
+
+ milestone '17.2'
+
+ disable_ddl_transaction!
+
+ # rubocop:disable Migration/PreventIndexCreation -- Once complete, this index unblocks the removal of other indexes https://gitlab.com/gitlab-org/gitlab/-/issues/442486
+ def up
+ add_concurrent_index :sbom_occurrences,
+ "traversal_ids, (licenses -> 0 ->> 'spdx_identifier'), component_id, component_version_id",
+ where: 'archived = false',
+ name: INDEX_NAME
+ end
+ # rubocop:enable Migration/PreventIndexCreation
+
+ def down
+ remove_concurrent_index_by_name :sbom_occurrences, INDEX_NAME
+ end
+end
diff --git a/db/schema_migrations/20240619114215 b/db/schema_migrations/20240619114215
new file mode 100644
index 0000000000000000000000000000000000000000..28f9b18273b312ef60a456c113a91dd073dd5738
--- /dev/null
+++ b/db/schema_migrations/20240619114215
@@ -0,0 +1 @@
+4ebdcd532d085ccd3a0bb11aa13ff0013230060b8f1ba50d1fafb8855a07b525
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index cb0402099167ea21df4f76ecb3ff7dcf0ad2f3c2..8eb09bfa5148ac6cc34b955bda3136fd48cf71d9 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -29022,6 +29022,8 @@ CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects U
CREATE INDEX index_unarchived_occurrences_for_aggregations_component_name ON sbom_occurrences USING btree (traversal_ids, component_name, component_id, component_version_id) WHERE (archived = false);
+CREATE INDEX index_unarchived_occurrences_for_aggregations_license ON sbom_occurrences USING btree (traversal_ids, (((licenses -> 0) ->> 'spdx_identifier'::text)), component_id, component_version_id) WHERE (archived = false);
+
CREATE INDEX index_unarchived_occurrences_for_aggregations_package_manager ON sbom_occurrences USING btree (traversal_ids, package_manager, component_id, component_version_id) WHERE (archived = false);
CREATE INDEX index_unarchived_occurrences_for_aggregations_severity ON sbom_occurrences USING btree (traversal_ids, highest_severity, component_id, component_version_id) WHERE (archived = false);
diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
index 4521cee0424a68e1a360485a59b70282afa930e6..fdde450e2b93e4bc56f5344cac45a684dec409df 100644
--- a/ee/app/controllers/groups/dependencies_controller.rb
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -141,6 +141,8 @@ def map_sort_by(sort_by)
:component_name
when 'packager'
:package_manager
+ when 'license'
+ :licenses
when 'severity'
:highest_severity
else
diff --git a/ee/app/finders/dependency_management/aggregations_finder.rb b/ee/app/finders/dependency_management/aggregations_finder.rb
index 7bd8d8642a6d63ed4f062727130fb71defbad174..b42dcebd64ca1ef574d4f43ad7a0c2bef1c64a7f 100644
--- a/ee/app/finders/dependency_management/aggregations_finder.rb
+++ b/ee/app/finders/dependency_management/aggregations_finder.rb
@@ -8,7 +8,7 @@ class AggregationsFinder
DEFAULT_PAGE_SIZE = 20
MAX_PAGE_SIZE = 20
DEFAULT_SORT_COLUMNS = %i[component_id component_version_id].freeze
- SUPPORTED_SORT_COLUMNS = %i[component_name highest_severity package_manager].freeze
+ SUPPORTED_SORT_COLUMNS = %i[component_name highest_severity package_manager licenses].freeze
def initialize(namespace, params: {})
@namespace = namespace
@@ -16,7 +16,7 @@ def initialize(namespace, params: {})
end
def execute
- group_columns = distinct_columns.map { |column| "outer_occurrences.#{column}" }
+ group_columns = distinct_columns.map { |column| column_expression(column, 'outer_occurrences') }
Sbom::Occurrence.with(namespaces_cte.to_arel)
.select(
@@ -38,10 +38,6 @@ def execute
attr_reader :namespace, :params
- def distinct_columns
- orderings.keys
- end
-
def keyset_order(column_expression_evaluator:, order_expression_evaluator:)
order_definitions = orderings.map do |column, direction|
column_expression = column_expression_evaluator.call(column)
@@ -61,8 +57,8 @@ def keyset_order(column_expression_evaluator:, order_expression_evaluator:)
def outer_order
keyset_order(
- column_expression_evaluator: ->(column) { Sbom::Occurrence.arel_table.alias('outer_occurrences')[column] },
- order_expression_evaluator: ->(column) { Arel.sql("MIN(outer_occurrences.#{column})") }
+ column_expression_evaluator: ->(column) { column_expression(column, 'outer_occurrences') },
+ order_expression_evaluator: ->(column) { sql_min(column, 'outer_occurrences') }
)
end
@@ -74,12 +70,13 @@ def inner_occurrences
Sbom::Occurrence.where('sbom_occurrences.traversal_ids = namespaces.traversal_ids::bigint[]')
.unarchived
.order(inner_order)
- .select_distinct(on: distinct_columns)
+ .select(distinct(on: distinct_columns))
.keyset_paginate(cursor: cursor, per_page: page_size)
end
def inner_order
- evaluator = ->(column) { Sbom::Occurrence.arel_table[column] }
+ evaluator = ->(column) { column_expression(column) }
+
keyset_order(
column_expression_evaluator: evaluator,
order_expression_evaluator: evaluator
@@ -88,10 +85,11 @@ def inner_order
def outer_occurrences
order = orderings.map do |column, direction|
- "inner_occurrences.#{column} #{direction.to_s.upcase}"
+ column_expression = column_expression(column, 'inner_occurrences')
+ direction == :desc ? column_expression.desc : column_expression.asc
end
- Sbom::Occurrence.select_distinct(on: distinct_columns, table_name: '"inner_occurrences"')
+ Sbom::Occurrence.select(distinct(on: distinct_columns, table_name: 'inner_occurrences'))
.from("namespaces, LATERAL (#{inner_occurrences.to_sql}) inner_occurrences")
.order(*order)
.limit(page_size + 1)
@@ -114,6 +112,34 @@ def cursor
params[:cursor]
end
+ def distinct_columns
+ orderings.keys
+ end
+
+ def column_expression(column, table_name = 'sbom_occurrences')
+ if column == :licenses
+ Sbom::Occurrence.connection.quote_table_name(table_name)
+ .then { |table_name| Arel.sql("(#{table_name}.\"licenses\" -> 0 ->> 'spdx_identifier')::text") }
+ else
+ Sbom::Occurrence.arel_table.alias(table_name)[column]
+ end
+ end
+
+ def distinct(on:, table_name: 'sbom_occurrences')
+ select_values = Sbom::Occurrence.column_names.map do |column|
+ Sbom::Occurrence.connection.quote_table_name("#{table_name}.#{column}")
+ end
+ distinct_values = on.map { |column| column_expression(column, table_name) }
+
+ distinct_sql = Arel::Nodes::DistinctOn.new(distinct_values).to_sql
+
+ "#{distinct_sql} #{select_values.join(', ')}"
+ end
+
+ def sql_min(column, table_name = 'sbom_occurrences')
+ Arel::Nodes::NamedFunction.new('MIN', [column_expression(column, table_name)])
+ end
+
def orderings
default_orderings = DEFAULT_SORT_COLUMNS.index_with { sort_direction }
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index 6a1f700985c65fc3dd34637ae34f37b524575d64..73b338910cd963f195d108517ba5a22ddabfa287 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -146,16 +146,6 @@ 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/spec/finders/dependency_management/aggregations_finder_spec.rb b/ee/spec/finders/dependency_management/aggregations_finder_spec.rb
index 493eb704e4306d319b7b56a1a2192852c867e1ac..541d72071351aaaabdbb397eb470c62e37dd3019 100644
--- a/ee/spec/finders/dependency_management/aggregations_finder_spec.rb
+++ b/ee/spec/finders/dependency_management/aggregations_finder_spec.rb
@@ -69,18 +69,19 @@
describe 'sorting' do
let_it_be(:project) { target_projects.first }
- let_it_be(:occurrence_1) { occurrence(packager: :npm, name: 'c', severity: 'low') }
- let_it_be(:occurrence_2) { occurrence(packager: :bundler, name: 'b', severity: 'medium') }
- let_it_be(:occurrence_3) { occurrence(packager: :nuget, name: 'a', severity: 'high') }
- let_it_be(:occurrence_4) { occurrence(packager: :yarn, name: 'd', severity: 'critical') }
+ let_it_be(:occurrence_1) { occurrence(traits: [:npm], name: 'c', severity: 'low') }
+ let_it_be(:occurrence_2) { occurrence(traits: [:mit, :apache_2, :bundler], name: 'b', severity: 'medium') }
+ let_it_be(:occurrence_3) { occurrence(traits: [:mpl_2, :nuget], name: 'a', severity: 'high') }
+ let_it_be(:occurrence_4) { occurrence(traits: [:apache_2, :yarn], name: 'd', severity: 'critical') }
before_all do
Sbom::Occurrence.id_in(target_occurrences.map(&:id)).delete_all
end
- def occurrence(packager:, name:, severity:)
- component = create(:sbom_component, name: name)
- create(:sbom_occurrence, packager, component: component, highest_severity: severity, project: project)
+ def occurrence(name:, severity:, traits: [])
+ params = { component: create(:sbom_component, name: name), highest_severity: severity, project: project }
+
+ create(:sbom_occurrence, *traits, **params)
end
shared_examples 'can sort in both asc and desc order' do |sort_by|
@@ -127,6 +128,18 @@ def occurrence(packager:, name:, severity:)
end
end
+ context 'when sorting by license id' do
+ it_behaves_like 'can sort in both asc and desc order', :licenses do
+ let_it_be(:blank_license_array) { occurrence_1 }
+ let_it_be(:mit_apache) { occurrence_2 }
+ let_it_be(:mpl) { occurrence_3 }
+ let_it_be(:apache) { occurrence_4 }
+
+ let(:expected_asc) { [apache, mit_apache, mpl, blank_license_array] }
+ let(:expected_desc) { [blank_license_array, mpl, mit_apache, apache] }
+ end
+ end
+
context 'when sorting by package manager' do
it_behaves_like 'can sort in both asc and desc order', :package_manager do
let_it_be(:npm) { occurrence_1 }
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index 16078ae49dbf30b3eef6ac63b1c72b75c6fe2988..2abede4a9a1d3737cc320816f37cbbd78d0317ee 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -254,6 +254,28 @@
expect(recording).not_to exceed_all_query_limit(1).for_model(::Sbom::Source)
end
+ 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
+
+ expect(json_response['dependencies'].first['licenses'].first['spdx_identifier']).to eq('Apache-2.0')
+ expect(json_response['dependencies'].last['licenses'].first['spdx_identifier']).to eq('MIT')
+ end
+ end
+
+ context 'when sorted by license in descending order' do
+ let(:params) { { group_id: group.to_param, sort_by: 'license', sort: 'desc' } }
+
+ it 'returns sorted list' do
+ subject
+
+ expect(json_response['dependencies'].first['licenses'].first['spdx_identifier']).to eq('MIT')
+ expect(json_response['dependencies'].last['licenses'].first['spdx_identifier']).to eq('Apache-2.0')
+ end
+ end
+
context 'when sorted by name in ascending order' do
let(:params) { { group_id: group.to_param, sort_by: 'name', sort: 'asc' } }