From cfbd92e5e38194e3b26bbff8f9afb5f496a53b46 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 30 Aug 2023 14:55:20 -0600
Subject: [PATCH 01/32] Add sorting and filtering specs
---
.../groups/dependencies_controller.rb | 10 ++++-
.../groups/dependencies_controller_spec.rb | 43 ++++++++++++++++++-
2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
index 722539b3a1d8c8..d4e6ae41f250e6 100644
--- a/ee/app/controllers/groups/dependencies_controller.rb
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -47,7 +47,15 @@ def dependency_list_params
end
def collect_dependencies
- @collect_dependencies ||= ::Sbom::DependenciesFinder.new(group, params: dependency_list_params).execute
+ query_params = params.permit(
+ :sort_by,
+ :sort,
+ component_names: [],
+ package_managers: []
+ )
+ ::Sbom::DependenciesFinder
+ .new(group, params: query_params)
+ .execute
end
def serialized_dependencies
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index c20a37e1e14f1a..85af132a2034c5 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -200,7 +200,18 @@
end
context 'with sorting params' do
- context 'when sorted by packager' 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
+
+ expect(json_response['dependencies'].first['packager']).to eq('bundler')
+ expect(json_response['dependencies'].last['packager']).to eq('npm')
+ end
+ end
+
+ 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
@@ -211,7 +222,7 @@
end
end
- context 'when sorted by name' do
+ 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
@@ -221,6 +232,17 @@
expect(json_response['dependencies'].last['name']).to eq(sbom_occurrence_bundler.name)
end
end
+
+ 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
+
+ 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 'with filtering params' do
@@ -230,9 +252,26 @@
it 'returns filtered list' do
subject
+ expect(json_response['dependencies'].count).to eq(1)
expect(json_response['dependencies'].pluck('packager')).to eq(['npm'])
end
end
+
+ context 'when filtered by component_names' do
+ let(:params) do
+ {
+ group_id: group.to_param,
+ component_names: [sbom_occurrence_bundler.name]
+ }
+ end
+
+ 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])
+ end
+ end
end
end
--
GitLab
From b50535a0d948764ea0c772faaaa46462a987f9e0 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 30 Aug 2023 14:57:49 -0600
Subject: [PATCH 02/32] Remove params that are not supported
---
ee/app/controllers/groups/dependencies_controller.rb | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
index d4e6ae41f250e6..933d0a4e808ecf 100644
--- a/ee/app/controllers/groups/dependencies_controller.rb
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -42,10 +42,6 @@ def authorize_read_dependency_list!
render_not_authorized
end
- def dependency_list_params
- params.permit(:sort_by, :sort, :component_id, :search, package_managers: [])
- end
-
def collect_dependencies
query_params = params.permit(
:sort_by,
@@ -82,7 +78,7 @@ def locations_info
def dependency_locations
Sbom::DependencyLocationsFinder
- .new(namespace: group, params: dependency_list_params.slice(:component_id, :search))
+ .new(namespace: group, params: params.permit(:component_id, :search))
.execute
end
--
GitLab
From b2a541ecf27e307057bf55ebec9a8110e675d4f8 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 30 Aug 2023 15:13:39 -0600
Subject: [PATCH 03/32] Inline methods
---
.../groups/dependencies_controller.rb | 34 +++++++------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
index 933d0a4e808ecf..8062dc23c99989 100644
--- a/ee/app/controllers/groups/dependencies_controller.rb
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -25,13 +25,19 @@ def index
render status: :ok
end
format.json do
- render json: serialized_dependencies
+ render json: dependencies_serializer
+ .represent(dependencies_finder.execute)
end
end
end
def locations
- render json: locations_info
+ locations_finder = Sbom::DependencyLocationsFinder.new(
+ namespace: group,
+ params: params.permit(:component_id, :search)
+ )
+ render json: ::Sbom::DependencyLocationListEntity
+ .represent(locations_finder.execute)
end
private
@@ -42,23 +48,19 @@ def authorize_read_dependency_list!
render_not_authorized
end
- def collect_dependencies
- query_params = params.permit(
- :sort_by,
+ def dependencies_finder
+ ::Sbom::DependenciesFinder.new(group, params: params.permit(
:sort,
+ :sort_by,
component_names: [],
package_managers: []
- )
- ::Sbom::DependenciesFinder
- .new(group, params: query_params)
- .execute
+ ))
end
- def serialized_dependencies
+ def dependencies_serializer
DependencyListSerializer
.new(project: nil, group: group, user: current_user)
.with_pagination(request, response)
- .represent(collect_dependencies)
end
def render_not_authorized
@@ -72,16 +74,6 @@ def render_not_authorized
end
end
- def locations_info
- ::Sbom::DependencyLocationListEntity.represent(dependency_locations)
- end
-
- def dependency_locations
- Sbom::DependencyLocationsFinder
- .new(namespace: group, params: params.permit(:component_id, :search))
- .execute
- end
-
def set_enable_project_search
@enable_project_search = group.count_within_namespaces <= GROUP_COUNT_LIMIT
end
--
GitLab
From a1c6e23532591621fc7e6b56cb73cd475a839a19 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 30 Aug 2023 16:43:18 -0600
Subject: [PATCH 04/32] Start to sort dependencies by spdx_identifier
---
.../groups/dependencies_controller.rb | 10 ++++-----
ee/app/finders/sbom/dependencies_finder.rb | 2 ++
ee/app/models/sbom/occurrence.rb | 4 ++++
ee/spec/factories/sbom/occurrences.rb | 10 +++++++++
.../finders/sbom/dependencies_finder_spec.rb | 16 +++++++++++---
.../groups/dependencies_controller_spec.rb | 22 +++++++++++++++++++
6 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
index 8062dc23c99989..2e7bd6650facf9 100644
--- a/ee/app/controllers/groups/dependencies_controller.rb
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -32,12 +32,12 @@ def index
end
def locations
- locations_finder = Sbom::DependencyLocationsFinder.new(
- namespace: group,
- params: params.permit(:component_id, :search)
+ render json: ::Sbom::DependencyLocationListEntity.represent(
+ Sbom::DependencyLocationsFinder.new(
+ namespace: group,
+ params: params.permit(:component_id, :search)
+ ).execute
)
- render json: ::Sbom::DependencyLocationListEntity
- .represent(locations_finder.execute)
end
private
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index 50dfc999ffbc25..4cb50f5810a266 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -15,6 +15,8 @@ def execute
sbom_occurrences.order_by_component_name(sort_direction)
when 'packager'
sbom_occurrences.order_by_package_name(sort_direction)
+ when 'spdx_identifier'
+ sbom_occurrences.order_by_spdx_identifier(sort_direction)
else
sbom_occurrences.order_by_id
end
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index 4186cbbfacf6c6..6eceb4f57534c3 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -33,6 +33,10 @@ class Occurrence < ApplicationRecord
order(package_manager: direction)
end
+ scope :order_by_spdx_identifier, ->(_direction) do
+ order(Arel.sql("licenses#>'{0,spdx_identifier}' ASC"))
+ end
+
scope :filter_by_package_managers, ->(package_managers) do
where(package_manager: package_managers)
end
diff --git a/ee/spec/factories/sbom/occurrences.rb b/ee/spec/factories/sbom/occurrences.rb
index 3b8872fc6fcaad..a0f9f4fd22a3b5 100644
--- a/ee/spec/factories/sbom/occurrences.rb
+++ b/ee/spec/factories/sbom/occurrences.rb
@@ -41,6 +41,16 @@
end
end
+ trait :mpl_2 do
+ after(:build) do |occurrence|
+ occurrence.licenses.push({
+ 'spdx_identifier' => 'MPL-2.0',
+ 'name' => 'Mozilla Public License 2.0',
+ 'url' => 'https://spdx.org/licenses/MPL-2.0.html'
+ })
+ end
+ end
+
after(:build) do |occurrence|
occurrence.uuid = Sbom::OccurrenceUUID.generate(
project_id: occurrence.project.id,
diff --git a/ee/spec/finders/sbom/dependencies_finder_spec.rb b/ee/spec/finders/sbom/dependencies_finder_spec.rb
index 24eb20cea1627c..d4510c53352c65 100644
--- a/ee/spec/finders/sbom/dependencies_finder_spec.rb
+++ b/ee/spec/finders/sbom/dependencies_finder_spec.rb
@@ -14,15 +14,15 @@
let_it_be(:component_version_3) { create(:sbom_component_version, component: component_3) }
let_it_be(:occurrence_1) do
- create(:sbom_occurrence, component_version: component_version_1, packager_name: 'nuget', project: project)
+ create(:sbom_occurrence, :mit, component_version: component_version_1, packager_name: 'nuget', project: project)
end
let_it_be(:occurrence_2) do
- create(:sbom_occurrence, component_version: component_version_2, packager_name: 'npm', project: project)
+ create(:sbom_occurrence, :apache_2, component_version: component_version_2, packager_name: 'npm', project: project)
end
let_it_be(:occurrence_3) do
- create(:sbom_occurrence, component_version: component_version_3, source: nil, project: project)
+ create(:sbom_occurrence, :mpl_2, component_version: component_version_3, source: nil, project: project)
end
shared_examples 'filter and sorting' do
@@ -94,6 +94,16 @@
end
end
+ context 'when sorted asc by spdx_identifier' do
+ let_it_be(:params) { { sort: 'asc', sort_by: 'spdx_identifier' } }
+
+ it 'returns sorted results' do
+ expect(dependencies[0].licenses[0]["spdx_identifier"]).to eq('Apache-2.0')
+ expect(dependencies[1].licenses[0]["spdx_identifier"]).to eq('MIT')
+ expect(dependencies[2].licenses[0]["spdx_identifier"]).to eq('MPL-2.0')
+ end
+ end
+
context 'when filtered by package name npm' do
let_it_be(:params) do
{
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index 85af132a2034c5..d230bf59f7bd86 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -243,6 +243,28 @@
expect(json_response['dependencies'].last['name']).to eq(sbom_occurrence_npm.name)
end
end
+
+ context 'when sorted by spdx_identifier in ascending order' do
+ let(:params) { { group_id: group.to_param, sort_by: 'spdx_identifier', sort: 'asc' } }
+
+ 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')
+ end
+ end
+
+ context 'when sorted by spdx_identifier in descending order' do
+ let(:params) { { group_id: group.to_param, sort_by: 'spdx_identifier', sort: 'desc' } }
+
+ 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')
+ end
+ end
end
context 'with filtering params' do
--
GitLab
From 162e6c6afe4ec69c48520c61bd8d5eabab3b75d1 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Thu, 31 Aug 2023 14:51:16 -0600
Subject: [PATCH 05/32] Sort occurrences by licenses
---
ee/app/models/sbom/occurrence.rb | 6 ++--
ee/spec/factories/sbom/occurrences.rb | 4 +++
ee/spec/models/sbom/occurrence_spec.rb | 38 ++++++++++++++++++++++++++
3 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index 6eceb4f57534c3..b866e22b22fff4 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -33,8 +33,10 @@ class Occurrence < ApplicationRecord
order(package_manager: direction)
end
- scope :order_by_spdx_identifier, ->(_direction) do
- order(Arel.sql("licenses#>'{0,spdx_identifier}' ASC"))
+ scope :order_by_spdx_identifier, ->(direction, depth: 1) do
+ 0.upto(depth).inject(self) do |relation, index|
+ relation.order(Arel.sql("licenses#>'{#{index},spdx_identifier}' #{direction}"))
+ end
end
scope :filter_by_package_managers, ->(package_managers) do
diff --git a/ee/spec/factories/sbom/occurrences.rb b/ee/spec/factories/sbom/occurrences.rb
index a0f9f4fd22a3b5..1ff696f95ecc62 100644
--- a/ee/spec/factories/sbom/occurrences.rb
+++ b/ee/spec/factories/sbom/occurrences.rb
@@ -21,6 +21,10 @@
packager_name { 'npm' }
end
+ trait :nuget do
+ packager_name { 'nuget' }
+ end
+
trait :apache_2 do
after(:build) do |occurrence|
occurrence.licenses.push({
diff --git a/ee/spec/models/sbom/occurrence_spec.rb b/ee/spec/models/sbom/occurrence_spec.rb
index 843647237b1b3b..fb56e3f950fb83 100644
--- a/ee/spec/models/sbom/occurrence_spec.rb
+++ b/ee/spec/models/sbom/occurrence_spec.rb
@@ -204,6 +204,44 @@
end
end
+ describe '.order_by_spdx_identifier' do
+ let_it_be(:mit_occurrence) { create(:sbom_occurrence, :mit) }
+ let_it_be(:apache_occurrence) { create(:sbom_occurrence, :apache_2) }
+ let_it_be(:apache_and_mpl_occurrence) { create(:sbom_occurrence, :apache_2, :mpl_2) }
+ let_it_be(:apache_and_mit_occurrence) { create(:sbom_occurrence, :apache_2, :mit) }
+ let_it_be(:mit_and_mpl_occurrence) { create(:sbom_occurrence, :mit, :mpl_2) }
+
+ subject(:relation) { described_class.order_by_spdx_identifier(order) }
+
+ context 'when sorting in ascending order' do
+ let(:order) { 'asc' }
+
+ it 'returns the sorted records' do
+ expect(relation.map(&:licenses)).to eq([
+ apache_and_mit_occurrence.licenses,
+ apache_and_mpl_occurrence.licenses,
+ apache_occurrence.licenses,
+ mit_and_mpl_occurrence.licenses,
+ mit_occurrence.licenses
+ ])
+ end
+ end
+
+ context 'when sorting in descending order' do
+ let(:order) { 'desc' }
+
+ it 'returns the sorted records' do
+ expect(relation.map(&:licenses)).to eq([
+ mit_occurrence.licenses,
+ mit_and_mpl_occurrence.licenses,
+ apache_occurrence.licenses,
+ apache_and_mpl_occurrence.licenses,
+ apache_and_mit_occurrence.licenses
+ ])
+ end
+ end
+ end
+
describe '.filter_by_component_names' do
let_it_be(:occurrence_1) { create(:sbom_occurrence) }
let_it_be(:occurrence_2) { create(:sbom_occurrence) }
--
GitLab
From 0ca971a4f340e420f24ea2ed3d074bfa5b28d5a6 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 6 Sep 2023 15:34:52 -0600
Subject: [PATCH 06/32] Fix N+1 issue
---
ee/app/finders/sbom/dependencies_finder.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index 4cb50f5810a266..c16d251ac2c6e9 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -33,7 +33,7 @@ def filtered_collection
collection = filter_by_component_names(collection) if params[:component_names].present?
- collection
+ collection.with_component.with_version.with_source
end
def filter_by_package_managers(sbom_occurrences)
--
GitLab
From 4b486ce872c8aaea991422cdf523a4e51011e723 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 6 Sep 2023 15:44:24 -0600
Subject: [PATCH 07/32] Eager load Projects
---
ee/app/finders/sbom/dependencies_finder.rb | 2 +-
ee/app/models/sbom/occurrence.rb | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index c16d251ac2c6e9..648da8288f9c70 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -33,7 +33,7 @@ def filtered_collection
collection = filter_by_component_names(collection) if params[:component_names].present?
- collection.with_component.with_version.with_source
+ collection.with_component.with_version.with_source.with_project
end
def filter_by_package_managers(sbom_occurrences)
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index b866e22b22fff4..a2266ad2cfdf60 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -57,6 +57,7 @@ class Occurrence < ApplicationRecord
end
scope :with_component, -> { includes(:component) }
+ scope :with_project, -> { includes(:project) }
scope :with_source, -> { includes(:source) }
scope :with_version, -> { includes(:component_version) }
scope :with_component_source_version_project_and_pipeline, -> do
--
GitLab
From 1ff09f569d71f2a7bf87b38505ddc8758f8dea53 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 6 Sep 2023 16:08:15 -0600
Subject: [PATCH 08/32] Permit the pagination parameters
---
ee/app/controllers/groups/dependencies_controller.rb | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
index 2e7bd6650facf9..a91e8cfa0ca992 100644
--- a/ee/app/controllers/groups/dependencies_controller.rb
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -50,6 +50,8 @@ def authorize_read_dependency_list!
def dependencies_finder
::Sbom::DependenciesFinder.new(group, params: params.permit(
+ :page,
+ :per_page,
:sort,
:sort_by,
component_names: [],
--
GitLab
From b17fed88bf8f7ab88c120fee5186b4049a3c2334 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 6 Sep 2023 16:13:22 -0600
Subject: [PATCH 09/32] rename query string parameter from spdx_identifier to
license
---
ee/app/finders/sbom/dependencies_finder.rb | 2 +-
ee/spec/finders/sbom/dependencies_finder_spec.rb | 14 ++++++++++++--
.../groups/dependencies_controller_spec.rb | 8 ++++----
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index 648da8288f9c70..8313b75f3a1faa 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -15,7 +15,7 @@ def execute
sbom_occurrences.order_by_component_name(sort_direction)
when 'packager'
sbom_occurrences.order_by_package_name(sort_direction)
- when 'spdx_identifier'
+ when 'license'
sbom_occurrences.order_by_spdx_identifier(sort_direction)
else
sbom_occurrences.order_by_id
diff --git a/ee/spec/finders/sbom/dependencies_finder_spec.rb b/ee/spec/finders/sbom/dependencies_finder_spec.rb
index d4510c53352c65..5b5d44635099a4 100644
--- a/ee/spec/finders/sbom/dependencies_finder_spec.rb
+++ b/ee/spec/finders/sbom/dependencies_finder_spec.rb
@@ -94,8 +94,8 @@
end
end
- context 'when sorted asc by spdx_identifier' do
- let_it_be(:params) { { sort: 'asc', sort_by: 'spdx_identifier' } }
+ context 'when sorted asc by license' do
+ let_it_be(:params) { { sort: 'asc', sort_by: 'license' } }
it 'returns sorted results' do
expect(dependencies[0].licenses[0]["spdx_identifier"]).to eq('Apache-2.0')
@@ -104,6 +104,16 @@
end
end
+ context 'when sorted desc by license' do
+ let_it_be(:params) { { sort: 'desc', sort_by: 'license' } }
+
+ it 'returns sorted results' do
+ expect(dependencies[0].licenses[0]["spdx_identifier"]).to eq('MPL-2.0')
+ expect(dependencies[1].licenses[0]["spdx_identifier"]).to eq('MIT')
+ expect(dependencies[2].licenses[0]["spdx_identifier"]).to eq('Apache-2.0')
+ end
+ end
+
context 'when filtered by package name npm' do
let_it_be(:params) do
{
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index d230bf59f7bd86..916b1bd50087dc 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -244,8 +244,8 @@
end
end
- context 'when sorted by spdx_identifier in ascending order' do
- let(:params) { { group_id: group.to_param, sort_by: 'spdx_identifier', sort: 'asc' } }
+ 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
@@ -255,8 +255,8 @@
end
end
- context 'when sorted by spdx_identifier in descending order' do
- let(:params) { { group_id: group.to_param, sort_by: 'spdx_identifier', sort: 'desc' } }
+ 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
--
GitLab
From aca7671ef464e2c5debe267a100cac57f067ae3d Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Thu, 7 Sep 2023 11:27:07 -0600
Subject: [PATCH 10/32] Add spec to reproduce SELECT N+1 issue
---
.../requests/groups/dependencies_controller_spec.rb | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index 916b1bd50087dc..610c7d0e9bc72d 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -179,6 +179,18 @@
expect(json_response).to eq(expected_response)
end
+ it 'loads the data efficiently' do
+ queries = ActiveRecord::QueryRecorder.new do
+ subject
+ end
+
+ expect(queries.occurrences_starting_with('SELECT "projects"').values.sum).to eq(1)
+ expect(queries.occurrences_starting_with('SELECT "sbom_component_versions"').values.sum).to eq(1)
+ expect(queries.occurrences_starting_with('SELECT "sbom_components"').values.sum).to eq(1)
+ expect(queries.occurrences_starting_with('SELECT "sbom_sources"').values.sum).to eq(1)
+ expect(queries.occurrences_starting_with('SELECT sbom_occurrences').values.sum).to eq(1)
+ end
+
it 'includes pagination headers in the response' do
subject
--
GitLab
From e67ebc50dfc0a477ed54e66f5cd145b24f93d973 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Thu, 7 Sep 2023 14:13:07 -0600
Subject: [PATCH 11/32] Use helper to test for N+1
---
.../groups/dependencies_controller_spec.rb | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index 610c7d0e9bc72d..b212e957cc7345 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -179,16 +179,18 @@
expect(json_response).to eq(expected_response)
end
- it 'loads the data efficiently' do
- queries = ActiveRecord::QueryRecorder.new do
- subject
+ it 'loads the data without N+1 queries' do
+ def render
+ get group_dependencies_path(group_id: group.full_path), params: params, as: :json
end
- expect(queries.occurrences_starting_with('SELECT "projects"').values.sum).to eq(1)
- expect(queries.occurrences_starting_with('SELECT "sbom_component_versions"').values.sum).to eq(1)
- expect(queries.occurrences_starting_with('SELECT "sbom_components"').values.sum).to eq(1)
- expect(queries.occurrences_starting_with('SELECT "sbom_sources"').values.sum).to eq(1)
- expect(queries.occurrences_starting_with('SELECT sbom_occurrences').values.sum).to eq(1)
+ control = ActiveRecord::QueryRecorder.new do
+ render
+ end
+
+ create(:sbom_occurrence, project: create(:project, group: group))
+
+ expect { render }.not_to exceed_query_limit(control)
end
it 'includes pagination headers in the response' do
--
GitLab
From 9bf8553516196d6729b90819e85d20c18f98810a Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Thu, 7 Sep 2023 14:54:35 -0600
Subject: [PATCH 12/32] Add index on sbom_occurrences.licenses
---
..._add_index_to_sbom_occurrences_licenses.rb | 23 +++++++++++++++++++
db/schema_migrations/20230907204731 | 1 +
db/structure.sql | 2 ++
3 files changed, 26 insertions(+)
create mode 100644 db/post_migrate/20230907204731_add_index_to_sbom_occurrences_licenses.rb
create mode 100644 db/schema_migrations/20230907204731
diff --git a/db/post_migrate/20230907204731_add_index_to_sbom_occurrences_licenses.rb b/db/post_migrate/20230907204731_add_index_to_sbom_occurrences_licenses.rb
new file mode 100644
index 00000000000000..9ed60941c962cb
--- /dev/null
+++ b/db/post_migrate/20230907204731_add_index_to_sbom_occurrences_licenses.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class AddIndexToSbomOccurrencesLicenses < Gitlab::Database::Migration[2.1]
+ INDEX_NAME = "index_sbom_occurrences_on_licenses_spdx_identifier"
+
+ disable_ddl_transaction!
+
+ def up
+ return if index_exists_by_name?(:sbom_occurrences, INDEX_NAME)
+
+ disable_statement_timeout do
+ execute <<~SQL
+ CREATE INDEX CONCURRENTLY #{INDEX_NAME}
+ ON sbom_occurrences
+ USING BTREE (project_id, (licenses#>'{0,spdx_identifier}'), (licenses#>'{1,spdx_identifier}'))
+ SQL
+ end
+ end
+
+ def down
+ remove_concurrent_index_by_name :sbom_occurrences, INDEX_NAME
+ end
+end
diff --git a/db/schema_migrations/20230907204731 b/db/schema_migrations/20230907204731
new file mode 100644
index 00000000000000..9b10980eae00fb
--- /dev/null
+++ b/db/schema_migrations/20230907204731
@@ -0,0 +1 @@
+e5448d02414f99074a52337c271310277a1d76b386a9e2e1bd3c1b806a70c462
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index d89580e0a432e6..e9ce4f63cfcaa4 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -33610,6 +33610,8 @@ CREATE INDEX index_sbom_occurrences_on_component_id_and_id ON sbom_occurrences U
CREATE INDEX index_sbom_occurrences_on_component_version_id ON sbom_occurrences USING btree (component_version_id);
+CREATE INDEX index_sbom_occurrences_on_licenses_spdx_identifier ON sbom_occurrences USING btree (project_id, ((licenses #> '{0,spdx_identifier}'::text[])), ((licenses #> '{1,spdx_identifier}'::text[])));
+
CREATE INDEX index_sbom_occurrences_on_pipeline_id ON sbom_occurrences USING btree (pipeline_id);
CREATE INDEX index_sbom_occurrences_on_project_id ON sbom_occurrences USING btree (project_id);
--
GitLab
From da0b4604ad644ae6c445b6233b4518219f0d0b27 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Thu, 7 Sep 2023 16:15:29 -0600
Subject: [PATCH 13/32] Add spec to optimize Group#sbom_occurrences query
---
ee/spec/factories/sbom/components.rb | 15 ++++++
ee/spec/models/ee/group_spec.rb | 74 ++++++++++++++++++++++++++++
2 files changed, 89 insertions(+)
diff --git a/ee/spec/factories/sbom/components.rb b/ee/spec/factories/sbom/components.rb
index d1cf8fc456cb42..8ae125b514ba04 100644
--- a/ee/spec/factories/sbom/components.rb
+++ b/ee/spec/factories/sbom/components.rb
@@ -6,5 +6,20 @@
purl_type { :npm }
sequence(:name) { |n| "component-#{n}" }
+
+ trait :bundler do
+ name { "bundler" }
+ purl_type { :gem }
+ end
+
+ trait :caddy do
+ name { "caddy" }
+ purl_type { :golang }
+ end
+
+ trait :webpack do
+ name { "webpack" }
+ purl_type { :npm }
+ end
end
end
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 517b68ff0a8628..f17c2ba71c64fc 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2878,6 +2878,80 @@ def webhook_headers
end
end
end
+
+ context 'with multiple sub groups' do
+ let!(:group_a) { create(:group, parent: group) }
+ let!(:group_a_a) { create(:group, parent: group_a) }
+ let!(:group_a_b) { create(:group, parent: group_a) }
+
+ let!(:group_b) { create(:group, parent: group) }
+ let!(:group_b_a) { create(:group, parent: group_b) }
+
+ let!(:group_c) { create(:group, parent: group) }
+ let!(:group_c_a) { create(:group, parent: group_c) }
+ let!(:group_c_b) { create(:group, parent: group_c) }
+
+ context 'with multiple projects' do
+ let!(:project_a) { create(:project, namespace: group_a) }
+ let!(:project_b_a) { create(:project, namespace: group_b_a) }
+ let!(:project_c) { create(:project, namespace: group_c) }
+ let!(:project_c_b) { create(:project, namespace: group_c_b) }
+
+ context 'with multiple versions of the same component' do
+ let!(:bundler) { create(:sbom_component, :bundler) }
+ let!(:bundler_version_1) { create(:sbom_component_version, component: bundler, version: "1.0.0") }
+ let!(:bundler_version_2) { create(:sbom_component_version, component: bundler, version: "2.0.0") }
+ let!(:bundler_v1) { create(:sbom_occurrence, project: project_a, component: bundler, component_version: bundler_version_1) }
+ let!(:bundler_v2) { create(:sbom_occurrence, project: project_b_a, component: bundler, component_version: bundler_version_2) }
+
+ let!(:webpack) { create(:sbom_component, :webpack) }
+ let!(:webpack_version_4) { create(:sbom_component_version, component: webpack, version: "4.46.0") }
+ let!(:webpack_v4) { create(:sbom_occurrence, project: project_c, component: webpack, component_version: webpack_version_4) }
+ let!(:webpack_unknown) { create(:sbom_occurrence, project: project_c_b, component: webpack, component_version: nil) }
+
+ let!(:caddy) { create(:sbom_component, :caddy) }
+ let!(:caddy_version_1) { create(:sbom_component_version, component: caddy, version: "1.0.0") }
+ let!(:caddy_version_2) { create(:sbom_component_version, component: caddy, version: "2.7.4") }
+ let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_1) }
+ let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_2) }
+
+ it 'returns an occurrence for each version of each component' do
+ expect(subject).to match_array([
+ bundler_v1,
+ bundler_v2,
+ caddy_v1,
+ caddy_v2,
+ webpack_v4,
+ webpack_unknown
+ ])
+ end
+
+ # Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
+ pending 'returns the project count for each version of each component' do
+ expect(subject.pluck(:component_name, :component_version_id, :project_count)).to match_array([
+ [bundler.name, bundler_version_1.id, 1],
+ [bundler.name, bundler_version_2.id, 1],
+ [caddy.name, caddy_version_1.id, 1],
+ [caddy.name, caddy_version_2.id, 1],
+ [webpack.name, webpack_version_4.id, 1],
+ [webpack.name, nil, 2]
+ ])
+ end
+
+ # Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
+ pending 'returns the occurrence count for each version of each component' do
+ expect(subject.pluck(:component_name, :component_version_id, :occurrence_count)).to match_array([
+ [bundler.name, bundler_version_1.id, 1],
+ [bundler.name, bundler_version_2.id, 1],
+ [caddy.name, caddy_version_1.id, 1],
+ [caddy.name, caddy_version_2.id, 1],
+ [webpack.name, webpack_version_4.id, 1],
+ [webpack.name, nil, 2]
+ ])
+ end
+ end
+ end
+ end
end
describe '#reached_project_access_token_limit?' do
--
GitLab
From 06a0de6db19a28c073566734bdc63ea35551f85e Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Fri, 8 Sep 2023 10:42:23 -0600
Subject: [PATCH 14/32] Use Gitlab::Pagination::Keyset::Order to build ordering
syntax
---
ee/app/models/sbom/occurrence.rb | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index a2266ad2cfdf60..a73f31c9b34bdb 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -34,9 +34,17 @@ class Occurrence < ApplicationRecord
end
scope :order_by_spdx_identifier, ->(direction, depth: 1) do
- 0.upto(depth).inject(self) do |relation, index|
- relation.order(Arel.sql("licenses#>'{#{index},spdx_identifier}' #{direction}"))
- end
+ order(Gitlab::Pagination::Keyset::Order.build(
+ 0.upto(depth).map do |index|
+ sql = Arel.sql("(licenses#>'{#{index},spdx_identifier}')::text")
+ Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+ attribute_name: "spdx_identifier_#{index}",
+ order_expression: direction == "desc" ? sql.desc : sql.asc,
+ distinct: false,
+ sql_type: 'text'
+ )
+ end
+ ))
end
scope :filter_by_package_managers, ->(package_managers) do
--
GitLab
From 3ac589d17782d8af157bba5021c6924d51c1f318 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Fri, 8 Sep 2023 11:32:40 -0600
Subject: [PATCH 15/32] Use CTE to optimize the Group#sbom_occurrences query
---
ee/app/finders/sbom/dependencies_finder.rb | 2 +-
ee/app/models/ee/group.rb | 29 ++++------------------
ee/app/models/sbom/occurrence.rb | 16 ++++++++++++
ee/spec/models/ee/group_spec.rb | 8 +++---
4 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index 8313b75f3a1faa..d64fde54ea8fe4 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -27,7 +27,7 @@ def execute
attr_reader :project_or_group, :params
def filtered_collection
- collection = project_or_group.sbom_occurrences
+ collection = project_or_group.sbom_occurrences.with_totals
collection = filter_by_package_managers(collection) if params[:package_managers].present?
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 5b69438637656c..f8f373affd689e 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -840,30 +840,11 @@ def code_owner_approval_required_available?
end
def sbom_occurrences
- Sbom::Occurrence.includes(project: :route).select('sbom_occurrences.*, agg_occurrences.occurrence_count, agg_occurrences.project_count')
- .from('sbom_occurrences')
- .joins(
- <<-SQL
- INNER JOIN (
- SELECT component_id,
- COUNT(DISTINCT id) AS occurrence_count,
- COUNT(DISTINCT project_id) AS project_count
- FROM sbom_occurrences
- WHERE project_id IN (
- SELECT "projects"."id" FROM "projects"
- WHERE "projects"."namespace_id" IN (
- SELECT namespaces.traversal_ids[array_length(namespaces.traversal_ids, 1)] AS id
- FROM "namespaces"
- WHERE "namespaces"."type" = 'Group'
- AND (traversal_ids @> (\'{#{id}}\'))
- )
- )
- GROUP BY component_id
- ) agg_occurrences ON sbom_occurrences.component_id = agg_occurrences.component_id
- SQL
- )
- .where(project_id: all_projects.select(:id))
- .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
+ ::Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
+ scope: Sbom::Occurrence.order_by_id,
+ array_scope: self.all_projects.select(:id),
+ array_mapping_scope: -> (project_id) { Sbom::Occurrence.where(Sbom::Occurrence.arel_table[:project_id].eq(project_id)) }
+ ).execute.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
end
override :reached_project_access_token_limit?
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index a73f31c9b34bdb..1c510150b554d7 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -73,6 +73,22 @@ class Occurrence < ApplicationRecord
end
scope :filter_by_non_nil_component_version, -> { where.not(component_version: nil) }
+ scope :with_totals, -> do
+ select('sbom_occurrences.*, totals.*')
+ .from('sbom_occurrences')
+ .joins(
+ <<-SQL
+ INNER JOIN (
+ SELECT component_id,
+ COUNT(DISTINCT id) AS occurrence_count,
+ COUNT(DISTINCT project_id) AS project_count
+ FROM sbom_occurrences
+ GROUP BY component_id
+ ) totals ON sbom_occurrences.component_id = totals.component_id
+ SQL
+ )
+ end
+
def location
{
blob_path: input_file_blob_path,
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index f17c2ba71c64fc..53ecb40bb8bf13 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2873,8 +2873,8 @@ def webhook_headers
it 'returns occurrences with aggregated ids' do
expect(subject).to eq([sbom_occurrence])
- expect(subject.pluck(:project_count)).to eq([1])
- expect(subject.pluck(:occurrence_count)).to eq([1])
+ expect(subject.with_totals.pluck(:project_count)).to eq([1])
+ expect(subject.with_totals.pluck(:occurrence_count)).to eq([1])
end
end
end
@@ -2928,7 +2928,7 @@ def webhook_headers
# Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
pending 'returns the project count for each version of each component' do
- expect(subject.pluck(:component_name, :component_version_id, :project_count)).to match_array([
+ expect(subject.with_totals.pluck(:component_name, :component_version_id, :project_count)).to match_array([
[bundler.name, bundler_version_1.id, 1],
[bundler.name, bundler_version_2.id, 1],
[caddy.name, caddy_version_1.id, 1],
@@ -2940,7 +2940,7 @@ def webhook_headers
# Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
pending 'returns the occurrence count for each version of each component' do
- expect(subject.pluck(:component_name, :component_version_id, :occurrence_count)).to match_array([
+ expect(subject.with_totals.pluck(:component_name, :component_version_id, :occurrence_count)).to match_array([
[bundler.name, bundler_version_1.id, 1],
[bundler.name, bundler_version_2.id, 1],
[caddy.name, caddy_version_1.id, 1],
--
GitLab
From a38b3e5b7da425afa310dade596d1aab21b03a13 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Fri, 8 Sep 2023 15:34:19 -0600
Subject: [PATCH 16/32] Group by component_version_id
---
ee/app/finders/sbom/dependencies_finder.rb | 4 +-
ee/app/models/ee/group.rb | 7 ++-
ee/app/models/sbom/occurrence.rb | 24 ++++----
ee/spec/models/ee/group_spec.rb | 24 --------
ee/spec/models/sbom/occurrence_spec.rb | 65 ++++++++++++++++++++++
5 files changed, 83 insertions(+), 41 deletions(-)
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index d64fde54ea8fe4..03015ecc03870a 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -27,13 +27,13 @@ def execute
attr_reader :project_or_group, :params
def filtered_collection
- collection = project_or_group.sbom_occurrences.with_totals
+ collection = project_or_group.sbom_occurrences
collection = filter_by_package_managers(collection) if params[:package_managers].present?
collection = filter_by_component_names(collection) if params[:component_names].present?
- collection.with_component.with_version.with_source.with_project
+ collection.with_component.with_version.with_source.with_project.with_totals
end
def filter_by_package_managers(sbom_occurrences)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index f8f373affd689e..692ad6268cc3d4 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -840,11 +840,14 @@ def code_owner_approval_required_available?
end
def sbom_occurrences
- ::Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
+ builder = ::Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
scope: Sbom::Occurrence.order_by_id,
array_scope: self.all_projects.select(:id),
array_mapping_scope: -> (project_id) { Sbom::Occurrence.where(Sbom::Occurrence.arel_table[:project_id].eq(project_id)) }
- ).execute.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
+ )
+ Sbom::Occurrence
+ .where(id: builder.execute)
+ .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
end
override :reached_project_access_token_limit?
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index 1c510150b554d7..a0895b4077c839 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -74,19 +74,17 @@ class Occurrence < ApplicationRecord
scope :filter_by_non_nil_component_version, -> { where.not(component_version: nil) }
scope :with_totals, -> do
- select('sbom_occurrences.*, totals.*')
- .from('sbom_occurrences')
- .joins(
- <<-SQL
- INNER JOIN (
- SELECT component_id,
- COUNT(DISTINCT id) AS occurrence_count,
- COUNT(DISTINCT project_id) AS project_count
- FROM sbom_occurrences
- GROUP BY component_id
- ) totals ON sbom_occurrences.component_id = totals.component_id
- SQL
- )
+ joins(
+ <<-SQL
+ INNER JOIN (
+ SELECT s.component_version_id,
+ COUNT(DISTINCT id) AS occurrence_count,
+ COUNT(DISTINCT project_id) AS project_count
+ FROM sbom_occurrences s
+ GROUP BY s.component_version_id
+ ) totals ON sbom_occurrences.component_version_id = totals.component_version_id
+ SQL
+ ).select('sbom_occurrences.*, totals.*')
end
def location
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 53ecb40bb8bf13..45bfff7291cc7b 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2925,30 +2925,6 @@ def webhook_headers
webpack_unknown
])
end
-
- # Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
- pending 'returns the project count for each version of each component' do
- expect(subject.with_totals.pluck(:component_name, :component_version_id, :project_count)).to match_array([
- [bundler.name, bundler_version_1.id, 1],
- [bundler.name, bundler_version_2.id, 1],
- [caddy.name, caddy_version_1.id, 1],
- [caddy.name, caddy_version_2.id, 1],
- [webpack.name, webpack_version_4.id, 1],
- [webpack.name, nil, 2]
- ])
- end
-
- # Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
- pending 'returns the occurrence count for each version of each component' do
- expect(subject.with_totals.pluck(:component_name, :component_version_id, :occurrence_count)).to match_array([
- [bundler.name, bundler_version_1.id, 1],
- [bundler.name, bundler_version_2.id, 1],
- [caddy.name, caddy_version_1.id, 1],
- [caddy.name, caddy_version_2.id, 1],
- [webpack.name, webpack_version_4.id, 1],
- [webpack.name, nil, 2]
- ])
- end
end
end
end
diff --git a/ee/spec/models/sbom/occurrence_spec.rb b/ee/spec/models/sbom/occurrence_spec.rb
index fb56e3f950fb83..06879f44a62822 100644
--- a/ee/spec/models/sbom/occurrence_spec.rb
+++ b/ee/spec/models/sbom/occurrence_spec.rb
@@ -318,6 +318,71 @@
end
end
+ describe ".with_totals" do
+ let_it_be(:project_a) { create(:project) }
+ let_it_be(:project_b) { create(:project) }
+ let_it_be(:project_c) { create(:project) }
+ let_it_be(:project_d) { create(:project) }
+
+ let_it_be(:bundler) { create(:sbom_component, :bundler) }
+ let_it_be(:bundler_version_1) { create(:sbom_component_version, component: bundler, version: "1.0.0") }
+ let_it_be(:bundler_version_2) { create(:sbom_component_version, component: bundler, version: "2.0.0") }
+ let_it_be(:bundler_v1) do
+ create(:sbom_occurrence, project: project_a, component: bundler, component_version: bundler_version_1)
+ end
+
+ let_it_be(:bundler_v2) do
+ create(:sbom_occurrence, project: project_b, component: bundler, component_version: bundler_version_2)
+ end
+
+ let_it_be(:webpack) { create(:sbom_component, :webpack) }
+ let_it_be(:webpack_version_4) { create(:sbom_component_version, component: webpack, version: "4.46.0") }
+ let_it_be(:webpack_v4) do
+ create(:sbom_occurrence, project: project_c, component: webpack, component_version: webpack_version_4)
+ end
+
+ let_it_be(:webpack_unknown) do
+ create(:sbom_occurrence, project: project_d, component: webpack, component_version: nil)
+ end
+
+ let_it_be(:caddy) { create(:sbom_component, :caddy) }
+ let_it_be(:caddy_version_1) { create(:sbom_component_version, component: caddy, version: "1.0.0") }
+ let_it_be(:caddy_version_2) { create(:sbom_component_version, component: caddy, version: "2.7.4") }
+ let_it_be(:caddy_v1) do
+ create(:sbom_occurrence, project: project_d, component: caddy, component_version: caddy_version_1)
+ end
+
+ let_it_be(:caddy_v2) do
+ create(:sbom_occurrence, project: project_d, component: caddy, component_version: caddy_version_2)
+ end
+
+ subject do
+ described_class
+ .where(project_id: [project_a, project_c, project_d])
+ .with_totals
+ end
+
+ # Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
+ it 'returns the project count for each version of each component' do
+ expect(subject.with_totals.pluck(:component_name, :component_version_id, :project_count)).to match_array([
+ [bundler.name, bundler_version_1.id, 1],
+ [caddy.name, caddy_version_1.id, 1],
+ [caddy.name, caddy_version_2.id, 1],
+ [webpack.name, webpack_version_4.id, 1]
+ ])
+ end
+
+ # Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
+ it 'returns the occurrence count for each version of each component' do
+ expect(subject.with_totals.pluck(:component_name, :component_version_id, :occurrence_count)).to match_array([
+ [bundler.name, bundler_version_1.id, 1],
+ [caddy.name, caddy_version_1.id, 1],
+ [caddy.name, caddy_version_2.id, 1],
+ [webpack.name, webpack_version_4.id, 1]
+ ])
+ end
+ end
+
describe '#name' do
let(:component) { build(:sbom_component, name: 'rails') }
let(:occurrence) { build(:sbom_occurrence, component: component) }
--
GitLab
From 1c67eb0d1b89b8c776cc2ab017dbc4d0372d294b Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Sat, 9 Sep 2023 10:55:12 -0600
Subject: [PATCH 17/32] Generate sbom occurrences query with a recursive cte
---
ee/app/models/ee/group.rb | 15 +++++++++++++++
ee/spec/models/ee/group_spec.rb | 16 ++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 692ad6268cc3d4..dae8a1d2211ce0 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -839,6 +839,21 @@ def code_owner_approval_required_available?
feature_available?(:code_owner_approval_required)
end
+ def sbom_occurrences_cte
+ cte = ::Gitlab::SQL::RecursiveCTE.new(:our_projects)
+ table = Arel::Table.new(:sbom_occurrences)
+
+ cte << Sbom::Occurrence
+ .where(table[:project_id].in(all_projects.select(:id).arel))
+ cte << Sbom::Occurrence
+ .from([table, cte.table])
+ .where(table[:project_id].eq(cte.table[:id]))
+
+ Sbom::Occurrence.with
+ .recursive(cte.to_arel)
+ .from(cte.alias_to(table))
+ end
+
def sbom_occurrences
builder = ::Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
scope: Sbom::Occurrence.order_by_id,
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 45bfff7291cc7b..7c8a31d683fe43 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2915,7 +2915,23 @@ def webhook_headers
let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_1) }
let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_2) }
+ it 'uses a cte' do
+ relation = group.sbom_occurrences_cte
+ puts relation.to_sql
+
+ expect(relation).to match_array([
+ bundler_v1,
+ bundler_v2,
+ caddy_v1,
+ caddy_v2,
+ webpack_v4,
+ webpack_unknown
+ ])
+ end
+
it 'returns an occurrence for each version of each component' do
+ puts subject.to_sql
+
expect(subject).to match_array([
bundler_v1,
bundler_v2,
--
GitLab
From e77e8217217bbaf9e33e75cf867f303d7d0cc6e4 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Sat, 9 Sep 2023 12:39:43 -0600
Subject: [PATCH 18/32] Create a CTE for projects and occurrences
---
ee/app/models/ee/group.rb | 24 ++++++++++++++++++++++--
ee/spec/models/ee/group_spec.rb | 28 ++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index dae8a1d2211ce0..8dec5db8de22ce 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -839,8 +839,28 @@ def code_owner_approval_required_available?
feature_available?(:code_owner_approval_required)
end
+ def our_projects_cte
+ cte = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
+
+ Sbom::Occurrence
+ .with(cte.to_arel)
+ .where(Sbom::Occurrence.arel_table[:project_id].in(cte.table.project(cte.table[:id])))
+ end
+
def sbom_occurrences_cte
- cte = ::Gitlab::SQL::RecursiveCTE.new(:our_projects)
+ table = Arel::Table.new(:sbom_occurrences)
+ cte = ::Gitlab::SQL::CTE.new(
+ :our_occurrences,
+ Sbom::Occurrence.where(table[:project_id].in(all_projects.select(:id).arel))
+ )
+
+ Sbom::Occurrence
+ .with(cte.to_arel)
+ .from(cte.alias_to(table))
+ end
+
+ def sbom_occurrences_recursive_cte
+ cte = ::Gitlab::SQL::RecursiveCTE.new(:our_occurrences)
table = Arel::Table.new(:sbom_occurrences)
cte << Sbom::Occurrence
@@ -857,7 +877,7 @@ def sbom_occurrences_cte
def sbom_occurrences
builder = ::Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
scope: Sbom::Occurrence.order_by_id,
- array_scope: self.all_projects.select(:id),
+ array_scope: all_projects.select(:id),
array_mapping_scope: -> (project_id) { Sbom::Occurrence.where(Sbom::Occurrence.arel_table[:project_id].eq(project_id)) }
)
Sbom::Occurrence
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 7c8a31d683fe43..092924788de572 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2915,6 +2915,20 @@ def webhook_headers
let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_1) }
let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_2) }
+ it 'uses a projects cte' do
+ relation = group.our_projects_cte
+ puts relation.to_sql
+
+ expect(relation).to match_array([
+ bundler_v1,
+ bundler_v2,
+ caddy_v1,
+ caddy_v2,
+ webpack_v4,
+ webpack_unknown
+ ])
+ end
+
it 'uses a cte' do
relation = group.sbom_occurrences_cte
puts relation.to_sql
@@ -2929,6 +2943,20 @@ def webhook_headers
])
end
+ it 'uses a recursive cte' do
+ relation = group.sbom_occurrences_recursive_cte
+ puts relation.to_sql
+
+ expect(relation).to match_array([
+ bundler_v1,
+ bundler_v2,
+ caddy_v1,
+ caddy_v2,
+ webpack_v4,
+ webpack_unknown
+ ])
+ end
+
it 'returns an occurrence for each version of each component' do
puts subject.to_sql
--
GitLab
From 4a2f421bf6fe941f370f400f740ff339cce473d4 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Sat, 9 Sep 2023 12:49:35 -0600
Subject: [PATCH 19/32] Combine CTEs in query
---
ee/app/models/ee/group.rb | 30 ++++++++++--------------------
ee/spec/models/ee/group_spec.rb | 14 --------------
2 files changed, 10 insertions(+), 34 deletions(-)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 8dec5db8de22ce..a79454be3c4f80 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -848,30 +848,20 @@ def our_projects_cte
end
def sbom_occurrences_cte
- table = Arel::Table.new(:sbom_occurrences)
- cte = ::Gitlab::SQL::CTE.new(
+ table = Sbom::Occurrence.arel_table
+ our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
+ our_occurrences = ::Gitlab::SQL::CTE.new(
:our_occurrences,
- Sbom::Occurrence.where(table[:project_id].in(all_projects.select(:id).arel))
+ Sbom::Occurrence
+ .with(our_projects.to_arel)
+ .where(
+ table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
+ )
)
Sbom::Occurrence
- .with(cte.to_arel)
- .from(cte.alias_to(table))
- end
-
- def sbom_occurrences_recursive_cte
- cte = ::Gitlab::SQL::RecursiveCTE.new(:our_occurrences)
- table = Arel::Table.new(:sbom_occurrences)
-
- cte << Sbom::Occurrence
- .where(table[:project_id].in(all_projects.select(:id).arel))
- cte << Sbom::Occurrence
- .from([table, cte.table])
- .where(table[:project_id].eq(cte.table[:id]))
-
- Sbom::Occurrence.with
- .recursive(cte.to_arel)
- .from(cte.alias_to(table))
+ .with(our_occurrences.to_arel)
+ .from(our_occurrences.alias_to(table))
end
def sbom_occurrences
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 092924788de572..60e2d008d9cc0f 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2943,20 +2943,6 @@ def webhook_headers
])
end
- it 'uses a recursive cte' do
- relation = group.sbom_occurrences_recursive_cte
- puts relation.to_sql
-
- expect(relation).to match_array([
- bundler_v1,
- bundler_v2,
- caddy_v1,
- caddy_v2,
- webpack_v4,
- webpack_unknown
- ])
- end
-
it 'returns an occurrence for each version of each component' do
puts subject.to_sql
--
GitLab
From 36f9ed3bcdb3d81f16e90a930d55ea171338bbae Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Sat, 9 Sep 2023 12:51:58 -0600
Subject: [PATCH 20/32] Remove Group#projects_cte method
---
ee/app/models/ee/group.rb | 8 --------
ee/spec/models/ee/group_spec.rb | 14 --------------
2 files changed, 22 deletions(-)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index a79454be3c4f80..6914e9678f65c0 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -839,14 +839,6 @@ def code_owner_approval_required_available?
feature_available?(:code_owner_approval_required)
end
- def our_projects_cte
- cte = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
-
- Sbom::Occurrence
- .with(cte.to_arel)
- .where(Sbom::Occurrence.arel_table[:project_id].in(cte.table.project(cte.table[:id])))
- end
-
def sbom_occurrences_cte
table = Sbom::Occurrence.arel_table
our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 60e2d008d9cc0f..7c8a31d683fe43 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2915,20 +2915,6 @@ def webhook_headers
let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_1) }
let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_2) }
- it 'uses a projects cte' do
- relation = group.our_projects_cte
- puts relation.to_sql
-
- expect(relation).to match_array([
- bundler_v1,
- bundler_v2,
- caddy_v1,
- caddy_v2,
- webpack_v4,
- webpack_unknown
- ])
- end
-
it 'uses a cte' do
relation = group.sbom_occurrences_cte
puts relation.to_sql
--
GitLab
From d985615071ef7c275c538100ab2109fdbbc41adf Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Sat, 9 Sep 2023 13:00:06 -0600
Subject: [PATCH 21/32] Combine methods and add option to switch based on
algorithm
---
ee/app/models/ee/group.rb | 51 ++++++------
ee/spec/models/ee/group_spec.rb | 138 ++++++++++++++------------------
2 files changed, 88 insertions(+), 101 deletions(-)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 6914e9678f65c0..49b23b49e1fa34 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -839,32 +839,33 @@ def code_owner_approval_required_available?
feature_available?(:code_owner_approval_required)
end
- def sbom_occurrences_cte
- table = Sbom::Occurrence.arel_table
- our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
- our_occurrences = ::Gitlab::SQL::CTE.new(
- :our_occurrences,
- Sbom::Occurrence
- .with(our_projects.to_arel)
- .where(
- table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
- )
- )
+ def sbom_occurrences(algorithm: 'in-optimization')
+ if algorithm == 'cte'
+ our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
+ our_occurrences = ::Gitlab::SQL::CTE.new(
+ :our_occurrences,
+ Sbom::Occurrence
+ .with(our_projects.to_arel)
+ .where(
+ Sbom::Occurrence.arel_table[:project_id].in(
+ our_projects.table.project(our_projects.table[:id])
+ )
+ )
+ )
- Sbom::Occurrence
- .with(our_occurrences.to_arel)
- .from(our_occurrences.alias_to(table))
- end
-
- def sbom_occurrences
- builder = ::Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
- scope: Sbom::Occurrence.order_by_id,
- array_scope: all_projects.select(:id),
- array_mapping_scope: -> (project_id) { Sbom::Occurrence.where(Sbom::Occurrence.arel_table[:project_id].eq(project_id)) }
- )
- Sbom::Occurrence
- .where(id: builder.execute)
- .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
+ Sbom::Occurrence
+ .with(our_occurrences.to_arel)
+ .from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
+ else
+ builder = ::Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
+ scope: Sbom::Occurrence.order_by_id,
+ array_scope: all_projects.select(:id),
+ array_mapping_scope: -> (project_id) { Sbom::Occurrence.where(Sbom::Occurrence.arel_table[:project_id].eq(project_id)) }
+ )
+ Sbom::Occurrence
+ .where(id: builder.execute)
+ .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
+ end
end
override :reached_project_access_token_limit?
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 7c8a31d683fe43..079b7cd73ef692 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2858,88 +2858,74 @@ def webhook_headers
end
end
- describe '#sbom_occurrences' do
- subject { group.sbom_occurrences }
+ ['cte', 'in-optimization'].each do |algorithm|
+ describe "#sbom_occurrences (#{algorithm})" do
+ subject { group.sbom_occurrences(algorithm: algorithm) }
- it { is_expected.to be_empty }
+ it { is_expected.to be_empty }
- context 'with project' do
- let!(:project) { create(:project, group: group) }
+ context 'with project' do
+ let!(:project) { create(:project, group: group) }
- it { is_expected.to be_empty }
+ it { is_expected.to be_empty }
+
+ context 'with occurrences' do
+ let!(:sbom_occurrence) { create(:sbom_occurrence, project: project) }
- context 'with occurrences' do
- let!(:sbom_occurrence) { create(:sbom_occurrence, project: project) }
-
- it 'returns occurrences with aggregated ids' do
- expect(subject).to eq([sbom_occurrence])
- expect(subject.with_totals.pluck(:project_count)).to eq([1])
- expect(subject.with_totals.pluck(:occurrence_count)).to eq([1])
- end
- end
- end
-
- context 'with multiple sub groups' do
- let!(:group_a) { create(:group, parent: group) }
- let!(:group_a_a) { create(:group, parent: group_a) }
- let!(:group_a_b) { create(:group, parent: group_a) }
-
- let!(:group_b) { create(:group, parent: group) }
- let!(:group_b_a) { create(:group, parent: group_b) }
-
- let!(:group_c) { create(:group, parent: group) }
- let!(:group_c_a) { create(:group, parent: group_c) }
- let!(:group_c_b) { create(:group, parent: group_c) }
-
- context 'with multiple projects' do
- let!(:project_a) { create(:project, namespace: group_a) }
- let!(:project_b_a) { create(:project, namespace: group_b_a) }
- let!(:project_c) { create(:project, namespace: group_c) }
- let!(:project_c_b) { create(:project, namespace: group_c_b) }
-
- context 'with multiple versions of the same component' do
- let!(:bundler) { create(:sbom_component, :bundler) }
- let!(:bundler_version_1) { create(:sbom_component_version, component: bundler, version: "1.0.0") }
- let!(:bundler_version_2) { create(:sbom_component_version, component: bundler, version: "2.0.0") }
- let!(:bundler_v1) { create(:sbom_occurrence, project: project_a, component: bundler, component_version: bundler_version_1) }
- let!(:bundler_v2) { create(:sbom_occurrence, project: project_b_a, component: bundler, component_version: bundler_version_2) }
-
- let!(:webpack) { create(:sbom_component, :webpack) }
- let!(:webpack_version_4) { create(:sbom_component_version, component: webpack, version: "4.46.0") }
- let!(:webpack_v4) { create(:sbom_occurrence, project: project_c, component: webpack, component_version: webpack_version_4) }
- let!(:webpack_unknown) { create(:sbom_occurrence, project: project_c_b, component: webpack, component_version: nil) }
-
- let!(:caddy) { create(:sbom_component, :caddy) }
- let!(:caddy_version_1) { create(:sbom_component_version, component: caddy, version: "1.0.0") }
- let!(:caddy_version_2) { create(:sbom_component_version, component: caddy, version: "2.7.4") }
- let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_1) }
- let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_2) }
-
- it 'uses a cte' do
- relation = group.sbom_occurrences_cte
- puts relation.to_sql
-
- expect(relation).to match_array([
- bundler_v1,
- bundler_v2,
- caddy_v1,
- caddy_v2,
- webpack_v4,
- webpack_unknown
- ])
+ it 'returns occurrences with aggregated ids' do
+ expect(subject).to eq([sbom_occurrence])
+ expect(subject.with_totals.pluck(:project_count)).to eq([1])
+ expect(subject.with_totals.pluck(:occurrence_count)).to eq([1])
end
+ end
+ end
- it 'returns an occurrence for each version of each component' do
- puts subject.to_sql
-
- expect(subject).to match_array([
- bundler_v1,
- bundler_v2,
- caddy_v1,
- caddy_v2,
- webpack_v4,
- webpack_unknown
- ])
+ context 'with multiple sub groups' do
+ let!(:group_a) { create(:group, parent: group) }
+ let!(:group_a_a) { create(:group, parent: group_a) }
+ let!(:group_a_b) { create(:group, parent: group_a) }
+
+ let!(:group_b) { create(:group, parent: group) }
+ let!(:group_b_a) { create(:group, parent: group_b) }
+
+ let!(:group_c) { create(:group, parent: group) }
+ let!(:group_c_a) { create(:group, parent: group_c) }
+ let!(:group_c_b) { create(:group, parent: group_c) }
+
+ context 'with multiple projects' do
+ let!(:project_a) { create(:project, namespace: group_a) }
+ let!(:project_b_a) { create(:project, namespace: group_b_a) }
+ let!(:project_c) { create(:project, namespace: group_c) }
+ let!(:project_c_b) { create(:project, namespace: group_c_b) }
+
+ context 'with multiple versions of the same component' do
+ let!(:bundler) { create(:sbom_component, :bundler) }
+ let!(:bundler_version_1) { create(:sbom_component_version, component: bundler, version: "1.0.0") }
+ let!(:bundler_version_2) { create(:sbom_component_version, component: bundler, version: "2.0.0") }
+ let!(:bundler_v1) { create(:sbom_occurrence, project: project_a, component: bundler, component_version: bundler_version_1) }
+ let!(:bundler_v2) { create(:sbom_occurrence, project: project_b_a, component: bundler, component_version: bundler_version_2) }
+
+ let!(:webpack) { create(:sbom_component, :webpack) }
+ let!(:webpack_version_4) { create(:sbom_component_version, component: webpack, version: "4.46.0") }
+ let!(:webpack_v4) { create(:sbom_occurrence, project: project_c, component: webpack, component_version: webpack_version_4) }
+ let!(:webpack_unknown) { create(:sbom_occurrence, project: project_c_b, component: webpack, component_version: nil) }
+
+ let!(:caddy) { create(:sbom_component, :caddy) }
+ let!(:caddy_version_1) { create(:sbom_component_version, component: caddy, version: "1.0.0") }
+ let!(:caddy_version_2) { create(:sbom_component_version, component: caddy, version: "2.7.4") }
+ let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_1) }
+ let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_2) }
+
+ it 'returns an occurrence for each version of each component' do
+ expect(subject).to match_array([
+ bundler_v1,
+ bundler_v2,
+ caddy_v1,
+ caddy_v2,
+ webpack_v4,
+ webpack_unknown
+ ])
+ end
end
end
end
--
GitLab
From b710f23d721e77f1071df595de01efc580e2e1a5 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Mon, 11 Sep 2023 13:57:50 -0600
Subject: [PATCH 22/32] Promote window function to top
---
ee/app/models/ee/group.rb | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 49b23b49e1fa34..cad8991a254a2a 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -842,18 +842,12 @@ def code_owner_approval_required_available?
def sbom_occurrences(algorithm: 'in-optimization')
if algorithm == 'cte'
our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
- our_occurrences = ::Gitlab::SQL::CTE.new(
- :our_occurrences,
- Sbom::Occurrence
- .with(our_projects.to_arel)
- .where(
- Sbom::Occurrence.arel_table[:project_id].in(
- our_projects.table.project(our_projects.table[:id])
- )
- )
- )
+ our_occurrences = ::Gitlab::SQL::CTE.new(:our_occurrences, Sbom::Occurrence.where(
+ Sbom::Occurrence.arel_table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
+ ))
Sbom::Occurrence
+ .with(our_projects.to_arel)
.with(our_occurrences.to_arel)
.from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
else
--
GitLab
From b60d4695ae955ed852a25a5610447b6f78c348ae Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Mon, 11 Sep 2023 14:37:27 -0600
Subject: [PATCH 23/32] Pass CTE into .with_totals scope
---
ee/app/finders/sbom/dependencies_finder.rb | 2 +-
ee/app/models/ee/group.rb | 18 ++++++++++------
ee/app/models/sbom/occurrence.rb | 10 ++++-----
ee/spec/models/ee/group_spec.rb | 24 ++++++++--------------
ee/spec/models/sbom/occurrence_spec.rb | 16 +++++++++++----
5 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index 03015ecc03870a..8313b75f3a1faa 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -33,7 +33,7 @@ def filtered_collection
collection = filter_by_component_names(collection) if params[:component_names].present?
- collection.with_component.with_version.with_source.with_project.with_totals
+ collection.with_component.with_version.with_source.with_project
end
def filter_by_package_managers(sbom_occurrences)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index cad8991a254a2a..c63b0fd105a9ae 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -839,17 +839,19 @@ def code_owner_approval_required_available?
feature_available?(:code_owner_approval_required)
end
- def sbom_occurrences(algorithm: 'in-optimization')
- if algorithm == 'cte'
- our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
- our_occurrences = ::Gitlab::SQL::CTE.new(:our_occurrences, Sbom::Occurrence.where(
- Sbom::Occurrence.arel_table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
- ))
+ def sbom_occurrences(algorithm: 'cte')
+ our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
+ our_occurrences = ::Gitlab::SQL::CTE.new(:our_occurrences, Sbom::Occurrence.where(
+ Sbom::Occurrence.arel_table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
+ ))
+ if algorithm == 'cte'
Sbom::Occurrence
.with(our_projects.to_arel)
.with(our_occurrences.to_arel)
.from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
+ .with_totals(our_occurrences)
+ .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
else
builder = ::Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
scope: Sbom::Occurrence.order_by_id,
@@ -857,7 +859,11 @@ def sbom_occurrences(algorithm: 'in-optimization')
array_mapping_scope: -> (project_id) { Sbom::Occurrence.where(Sbom::Occurrence.arel_table[:project_id].eq(project_id)) }
)
Sbom::Occurrence
+ .with(our_projects.to_arel)
+ .with(our_occurrences.to_arel)
+ .from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
.where(id: builder.execute)
+ .with_totals(our_occurrences)
.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
end
end
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index a0895b4077c839..1addeac22f1d1b 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -73,16 +73,16 @@ class Occurrence < ApplicationRecord
end
scope :filter_by_non_nil_component_version, -> { where.not(component_version: nil) }
- scope :with_totals, -> do
+ scope :with_totals, ->(sbom_occurrences_cte) do
joins(
<<-SQL
INNER JOIN (
- SELECT s.component_version_id,
+ SELECT s.component_version_id as _version_id,
COUNT(DISTINCT id) AS occurrence_count,
COUNT(DISTINCT project_id) AS project_count
- FROM sbom_occurrences s
- GROUP BY s.component_version_id
- ) totals ON sbom_occurrences.component_version_id = totals.component_version_id
+ FROM #{sbom_occurrences_cte.table.name} s
+ GROUP BY _version_id
+ ) totals ON sbom_occurrences.component_version_id = totals._version_id
SQL
).select('sbom_occurrences.*, totals.*')
end
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 079b7cd73ef692..d3713db2c6c821 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2858,7 +2858,7 @@ def webhook_headers
end
end
- ['cte', 'in-optimization'].each do |algorithm|
+ %w[cte in-optimization].each do |algorithm|
describe "#sbom_occurrences (#{algorithm})" do
subject { group.sbom_occurrences(algorithm: algorithm) }
@@ -2874,8 +2874,8 @@ def webhook_headers
it 'returns occurrences with aggregated ids' do
expect(subject).to eq([sbom_occurrence])
- expect(subject.with_totals.pluck(:project_count)).to eq([1])
- expect(subject.with_totals.pluck(:occurrence_count)).to eq([1])
+ expect(subject.pluck(:project_count)).to eq([1])
+ expect(subject.pluck(:occurrence_count)).to eq([1])
end
end
end
@@ -2900,21 +2900,16 @@ def webhook_headers
context 'with multiple versions of the same component' do
let!(:bundler) { create(:sbom_component, :bundler) }
- let!(:bundler_version_1) { create(:sbom_component_version, component: bundler, version: "1.0.0") }
- let!(:bundler_version_2) { create(:sbom_component_version, component: bundler, version: "2.0.0") }
- let!(:bundler_v1) { create(:sbom_occurrence, project: project_a, component: bundler, component_version: bundler_version_1) }
- let!(:bundler_v2) { create(:sbom_occurrence, project: project_b_a, component: bundler, component_version: bundler_version_2) }
+ let!(:bundler_v1) { create(:sbom_occurrence, project: project_a, component: bundler, component_version: create(:sbom_component_version, component: bundler, version: "1.0.0")) }
+ let!(:bundler_v2) { create(:sbom_occurrence, project: project_b_a, component: bundler, component_version: create(:sbom_component_version, component: bundler, version: "2.0.0")) }
let!(:webpack) { create(:sbom_component, :webpack) }
- let!(:webpack_version_4) { create(:sbom_component_version, component: webpack, version: "4.46.0") }
- let!(:webpack_v4) { create(:sbom_occurrence, project: project_c, component: webpack, component_version: webpack_version_4) }
+ let!(:webpack_v4) { create(:sbom_occurrence, project: project_c, component: webpack, component_version: create(:sbom_component_version, component: webpack, version: "4.46.0")) }
let!(:webpack_unknown) { create(:sbom_occurrence, project: project_c_b, component: webpack, component_version: nil) }
let!(:caddy) { create(:sbom_component, :caddy) }
- let!(:caddy_version_1) { create(:sbom_component_version, component: caddy, version: "1.0.0") }
- let!(:caddy_version_2) { create(:sbom_component_version, component: caddy, version: "2.7.4") }
- let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_1) }
- let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: caddy_version_2) }
+ let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: create(:sbom_component_version, component: caddy, version: "1.0.0")) }
+ let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: create(:sbom_component_version, component: caddy, version: "2.7.4")) }
it 'returns an occurrence for each version of each component' do
expect(subject).to match_array([
@@ -2922,8 +2917,7 @@ def webhook_headers
bundler_v2,
caddy_v1,
caddy_v2,
- webpack_v4,
- webpack_unknown
+ webpack_v4
])
end
end
diff --git a/ee/spec/models/sbom/occurrence_spec.rb b/ee/spec/models/sbom/occurrence_spec.rb
index 06879f44a62822..698c6b6655e72e 100644
--- a/ee/spec/models/sbom/occurrence_spec.rb
+++ b/ee/spec/models/sbom/occurrence_spec.rb
@@ -357,14 +357,22 @@
end
subject do
+ our_projects = ::Gitlab::SQL::CTE.new(:our_projects,
+ ::Project.where(id: [project_a, project_c, project_d]).select(:id))
+ our_occurrences = ::Gitlab::SQL::CTE.new(:our_occurrences, described_class.where(
+ described_class.arel_table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
+ ))
+
described_class
- .where(project_id: [project_a, project_c, project_d])
- .with_totals
+ .with(our_projects.to_arel)
+ .with(our_occurrences.to_arel)
+ .from(our_occurrences.alias_to(described_class.arel_table))
+ .with_totals(our_occurrences)
end
# Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
it 'returns the project count for each version of each component' do
- expect(subject.with_totals.pluck(:component_name, :component_version_id, :project_count)).to match_array([
+ expect(subject.pluck(:component_name, :component_version_id, :project_count)).to match_array([
[bundler.name, bundler_version_1.id, 1],
[caddy.name, caddy_version_1.id, 1],
[caddy.name, caddy_version_2.id, 1],
@@ -374,7 +382,7 @@
# Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
it 'returns the occurrence count for each version of each component' do
- expect(subject.with_totals.pluck(:component_name, :component_version_id, :occurrence_count)).to match_array([
+ expect(subject.pluck(:component_name, :component_version_id, :occurrence_count)).to match_array([
[bundler.name, bundler_version_1.id, 1],
[caddy.name, caddy_version_1.id, 1],
[caddy.name, caddy_version_2.id, 1],
--
GitLab
From c5db51198094db7eafefe346422b125ee0b0d600 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Mon, 11 Sep 2023 15:07:51 -0600
Subject: [PATCH 24/32] Remove InOperatorOptimization::QueryBuilder
---
ee/app/models/ee/group.rb | 29 +++--------
ee/spec/models/ee/group_spec.rb | 92 ++++++++++++++++-----------------
2 files changed, 52 insertions(+), 69 deletions(-)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index c63b0fd105a9ae..c2177508a2031d 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -839,33 +839,18 @@ def code_owner_approval_required_available?
feature_available?(:code_owner_approval_required)
end
- def sbom_occurrences(algorithm: 'cte')
+ def sbom_occurrences
our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
our_occurrences = ::Gitlab::SQL::CTE.new(:our_occurrences, Sbom::Occurrence.where(
Sbom::Occurrence.arel_table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
))
- if algorithm == 'cte'
- Sbom::Occurrence
- .with(our_projects.to_arel)
- .with(our_occurrences.to_arel)
- .from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
- .with_totals(our_occurrences)
- .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
- else
- builder = ::Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
- scope: Sbom::Occurrence.order_by_id,
- array_scope: all_projects.select(:id),
- array_mapping_scope: -> (project_id) { Sbom::Occurrence.where(Sbom::Occurrence.arel_table[:project_id].eq(project_id)) }
- )
- Sbom::Occurrence
- .with(our_projects.to_arel)
- .with(our_occurrences.to_arel)
- .from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
- .where(id: builder.execute)
- .with_totals(our_occurrences)
- .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
- end
+ Sbom::Occurrence
+ .with(our_projects.to_arel)
+ .with(our_occurrences.to_arel)
+ .from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
+ .with_totals(our_occurrences)
+ .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
end
override :reached_project_access_token_limit?
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index d3713db2c6c821..4f54bc763881f6 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2858,68 +2858,66 @@ def webhook_headers
end
end
- %w[cte in-optimization].each do |algorithm|
- describe "#sbom_occurrences (#{algorithm})" do
- subject { group.sbom_occurrences(algorithm: algorithm) }
+ describe "#sbom_occurrences" do
+ subject { group.sbom_occurrences }
- it { is_expected.to be_empty }
+ it { is_expected.to be_empty }
- context 'with project' do
- let!(:project) { create(:project, group: group) }
+ context 'with project' do
+ let!(:project) { create(:project, group: group) }
- it { is_expected.to be_empty }
+ it { is_expected.to be_empty }
- context 'with occurrences' do
- let!(:sbom_occurrence) { create(:sbom_occurrence, project: project) }
+ context 'with occurrences' do
+ let!(:sbom_occurrence) { create(:sbom_occurrence, project: project) }
- it 'returns occurrences with aggregated ids' do
- expect(subject).to eq([sbom_occurrence])
- expect(subject.pluck(:project_count)).to eq([1])
- expect(subject.pluck(:occurrence_count)).to eq([1])
- end
+ it 'returns occurrences with aggregated ids' do
+ expect(subject).to eq([sbom_occurrence])
+ expect(subject.pluck(:project_count)).to eq([1])
+ expect(subject.pluck(:occurrence_count)).to eq([1])
end
end
+ end
- context 'with multiple sub groups' do
- let!(:group_a) { create(:group, parent: group) }
- let!(:group_a_a) { create(:group, parent: group_a) }
- let!(:group_a_b) { create(:group, parent: group_a) }
+ context 'with multiple sub groups' do
+ let!(:group_a) { create(:group, parent: group) }
+ let!(:group_a_a) { create(:group, parent: group_a) }
+ let!(:group_a_b) { create(:group, parent: group_a) }
- let!(:group_b) { create(:group, parent: group) }
- let!(:group_b_a) { create(:group, parent: group_b) }
+ let!(:group_b) { create(:group, parent: group) }
+ let!(:group_b_a) { create(:group, parent: group_b) }
- let!(:group_c) { create(:group, parent: group) }
- let!(:group_c_a) { create(:group, parent: group_c) }
- let!(:group_c_b) { create(:group, parent: group_c) }
+ let!(:group_c) { create(:group, parent: group) }
+ let!(:group_c_a) { create(:group, parent: group_c) }
+ let!(:group_c_b) { create(:group, parent: group_c) }
- context 'with multiple projects' do
- let!(:project_a) { create(:project, namespace: group_a) }
- let!(:project_b_a) { create(:project, namespace: group_b_a) }
- let!(:project_c) { create(:project, namespace: group_c) }
- let!(:project_c_b) { create(:project, namespace: group_c_b) }
+ context 'with multiple projects' do
+ let!(:project_a) { create(:project, namespace: group_a) }
+ let!(:project_b_a) { create(:project, namespace: group_b_a) }
+ let!(:project_c) { create(:project, namespace: group_c) }
+ let!(:project_c_b) { create(:project, namespace: group_c_b) }
- context 'with multiple versions of the same component' do
- let!(:bundler) { create(:sbom_component, :bundler) }
- let!(:bundler_v1) { create(:sbom_occurrence, project: project_a, component: bundler, component_version: create(:sbom_component_version, component: bundler, version: "1.0.0")) }
- let!(:bundler_v2) { create(:sbom_occurrence, project: project_b_a, component: bundler, component_version: create(:sbom_component_version, component: bundler, version: "2.0.0")) }
+ context 'with multiple versions of the same component' do
+ let!(:bundler) { create(:sbom_component, :bundler) }
+ let!(:bundler_v1) { create(:sbom_occurrence, project: project_a, component: bundler, component_version: create(:sbom_component_version, component: bundler, version: "1.0.0")) }
+ let!(:bundler_v2) { create(:sbom_occurrence, project: project_b_a, component: bundler, component_version: create(:sbom_component_version, component: bundler, version: "2.0.0")) }
- let!(:webpack) { create(:sbom_component, :webpack) }
- let!(:webpack_v4) { create(:sbom_occurrence, project: project_c, component: webpack, component_version: create(:sbom_component_version, component: webpack, version: "4.46.0")) }
- let!(:webpack_unknown) { create(:sbom_occurrence, project: project_c_b, component: webpack, component_version: nil) }
+ let!(:webpack) { create(:sbom_component, :webpack) }
+ let!(:webpack_v4) { create(:sbom_occurrence, project: project_c, component: webpack, component_version: create(:sbom_component_version, component: webpack, version: "4.46.0")) }
+ let!(:webpack_unknown) { create(:sbom_occurrence, project: project_c_b, component: webpack, component_version: nil) }
- let!(:caddy) { create(:sbom_component, :caddy) }
- let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: create(:sbom_component_version, component: caddy, version: "1.0.0")) }
- let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: create(:sbom_component_version, component: caddy, version: "2.7.4")) }
+ let!(:caddy) { create(:sbom_component, :caddy) }
+ let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: create(:sbom_component_version, component: caddy, version: "1.0.0")) }
+ let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: create(:sbom_component_version, component: caddy, version: "2.7.4")) }
- it 'returns an occurrence for each version of each component' do
- expect(subject).to match_array([
- bundler_v1,
- bundler_v2,
- caddy_v1,
- caddy_v2,
- webpack_v4
- ])
- end
+ it 'returns an occurrence for each version of each component' do
+ expect(subject).to match_array([
+ bundler_v1,
+ bundler_v2,
+ caddy_v1,
+ caddy_v2,
+ webpack_v4
+ ])
end
end
end
--
GitLab
From 787b58ec1ff16bea67540e124cdd58a0cf9d23c6 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Mon, 11 Sep 2023 15:20:36 -0600
Subject: [PATCH 25/32] Include project routes
---
ee/app/models/ee/group.rb | 1 +
ee/spec/requests/groups/dependencies_controller_spec.rb | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index c2177508a2031d..b8e13f3c2a1086 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -846,6 +846,7 @@ def sbom_occurrences
))
Sbom::Occurrence
+ .includes(project: :route)
.with(our_projects.to_arel)
.with(our_occurrences.to_arel)
.from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index b212e957cc7345..3700a1cde65e60 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -210,7 +210,7 @@ def render
end
expect(project_routes_count).to eq(1)
- expect(project_count).to eq(1)
+ expect(project_count).to eq(2)
end
context 'with sorting params' do
--
GitLab
From 3bae60fea64d54a32c0f49f5ec63c8e8a5fe9f24 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Mon, 11 Sep 2023 15:52:37 -0600
Subject: [PATCH 26/32] Undo unneccessary change
---
ee/spec/models/ee/group_spec.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 4f54bc763881f6..4b42ec0303e78e 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2858,7 +2858,7 @@ def webhook_headers
end
end
- describe "#sbom_occurrences" do
+ describe '#sbom_occurrences' do
subject { group.sbom_occurrences }
it { is_expected.to be_empty }
--
GitLab
From e92852d8e5042b227a704353a1d41250eadad281 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Tue, 12 Sep 2023 10:45:10 -0600
Subject: [PATCH 27/32] Excluded soft deleted projects
---
ee/app/models/ee/group.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index b8e13f3c2a1086..d8ce062d2ecee6 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -840,7 +840,7 @@ def code_owner_approval_required_available?
end
def sbom_occurrences
- our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects.select(:id))
+ our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects_except_soft_deleted.select(:id))
our_occurrences = ::Gitlab::SQL::CTE.new(:our_occurrences, Sbom::Occurrence.where(
Sbom::Occurrence.arel_table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
))
--
GitLab
From 7d8e8526a8c4f7e8a3bd25abca75e3f9dd33f5eb Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Tue, 12 Sep 2023 15:48:35 -0600
Subject: [PATCH 28/32] Use a single non materialized CTE
---
ee/app/models/ee/group.rb | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index d8ce062d2ecee6..2240dce60c8d64 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -840,14 +840,12 @@ def code_owner_approval_required_available?
end
def sbom_occurrences
- our_projects = ::Gitlab::SQL::CTE.new(:our_projects, all_projects_except_soft_deleted.select(:id))
our_occurrences = ::Gitlab::SQL::CTE.new(:our_occurrences, Sbom::Occurrence.where(
- Sbom::Occurrence.arel_table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
- ))
+ project_id: all_projects_except_soft_deleted.select(:id)
+ ), materialized: false)
Sbom::Occurrence
.includes(project: :route)
- .with(our_projects.to_arel)
.with(our_occurrences.to_arel)
.from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
.with_totals(our_occurrences)
--
GitLab
From 3dcde825a724b14b7ad888aff1cb42a1ea3fee34 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 13 Sep 2023 15:29:07 -0600
Subject: [PATCH 29/32] Remove changes that are not related to sorting by
license
---
ee/app/finders/sbom/dependencies_finder.rb | 2 +-
ee/app/models/ee/group.rb | 34 ++++++---
ee/app/models/sbom/occurrence.rb | 15 ----
ee/spec/factories/sbom/components.rb | 15 ----
ee/spec/models/ee/group_spec.rb | 44 -----------
ee/spec/models/sbom/occurrence_spec.rb | 73 -------------------
.../groups/dependencies_controller_spec.rb | 16 +---
7 files changed, 26 insertions(+), 173 deletions(-)
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index 8313b75f3a1faa..334e14a0be5adc 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -33,7 +33,7 @@ def filtered_collection
collection = filter_by_component_names(collection) if params[:component_names].present?
- collection.with_component.with_version.with_source.with_project
+ collection
end
def filter_by_package_managers(sbom_occurrences)
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 2240dce60c8d64..5b69438637656c 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -840,16 +840,30 @@ def code_owner_approval_required_available?
end
def sbom_occurrences
- our_occurrences = ::Gitlab::SQL::CTE.new(:our_occurrences, Sbom::Occurrence.where(
- project_id: all_projects_except_soft_deleted.select(:id)
- ), materialized: false)
-
- Sbom::Occurrence
- .includes(project: :route)
- .with(our_occurrences.to_arel)
- .from(our_occurrences.alias_to(Sbom::Occurrence.arel_table))
- .with_totals(our_occurrences)
- .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
+ Sbom::Occurrence.includes(project: :route).select('sbom_occurrences.*, agg_occurrences.occurrence_count, agg_occurrences.project_count')
+ .from('sbom_occurrences')
+ .joins(
+ <<-SQL
+ INNER JOIN (
+ SELECT component_id,
+ COUNT(DISTINCT id) AS occurrence_count,
+ COUNT(DISTINCT project_id) AS project_count
+ FROM sbom_occurrences
+ WHERE project_id IN (
+ SELECT "projects"."id" FROM "projects"
+ WHERE "projects"."namespace_id" IN (
+ SELECT namespaces.traversal_ids[array_length(namespaces.traversal_ids, 1)] AS id
+ FROM "namespaces"
+ WHERE "namespaces"."type" = 'Group'
+ AND (traversal_ids @> (\'{#{id}}\'))
+ )
+ )
+ GROUP BY component_id
+ ) agg_occurrences ON sbom_occurrences.component_id = agg_occurrences.component_id
+ SQL
+ )
+ .where(project_id: all_projects.select(:id))
+ .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
end
override :reached_project_access_token_limit?
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index 1addeac22f1d1b..38be2d6e1c8398 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -65,7 +65,6 @@ class Occurrence < ApplicationRecord
end
scope :with_component, -> { includes(:component) }
- scope :with_project, -> { includes(:project) }
scope :with_source, -> { includes(:source) }
scope :with_version, -> { includes(:component_version) }
scope :with_component_source_version_project_and_pipeline, -> do
@@ -73,20 +72,6 @@ class Occurrence < ApplicationRecord
end
scope :filter_by_non_nil_component_version, -> { where.not(component_version: nil) }
- scope :with_totals, ->(sbom_occurrences_cte) do
- joins(
- <<-SQL
- INNER JOIN (
- SELECT s.component_version_id as _version_id,
- COUNT(DISTINCT id) AS occurrence_count,
- COUNT(DISTINCT project_id) AS project_count
- FROM #{sbom_occurrences_cte.table.name} s
- GROUP BY _version_id
- ) totals ON sbom_occurrences.component_version_id = totals._version_id
- SQL
- ).select('sbom_occurrences.*, totals.*')
- end
-
def location
{
blob_path: input_file_blob_path,
diff --git a/ee/spec/factories/sbom/components.rb b/ee/spec/factories/sbom/components.rb
index 8ae125b514ba04..d1cf8fc456cb42 100644
--- a/ee/spec/factories/sbom/components.rb
+++ b/ee/spec/factories/sbom/components.rb
@@ -6,20 +6,5 @@
purl_type { :npm }
sequence(:name) { |n| "component-#{n}" }
-
- trait :bundler do
- name { "bundler" }
- purl_type { :gem }
- end
-
- trait :caddy do
- name { "caddy" }
- purl_type { :golang }
- end
-
- trait :webpack do
- name { "webpack" }
- purl_type { :npm }
- end
end
end
diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb
index 4b42ec0303e78e..517b68ff0a8628 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2878,50 +2878,6 @@ def webhook_headers
end
end
end
-
- context 'with multiple sub groups' do
- let!(:group_a) { create(:group, parent: group) }
- let!(:group_a_a) { create(:group, parent: group_a) }
- let!(:group_a_b) { create(:group, parent: group_a) }
-
- let!(:group_b) { create(:group, parent: group) }
- let!(:group_b_a) { create(:group, parent: group_b) }
-
- let!(:group_c) { create(:group, parent: group) }
- let!(:group_c_a) { create(:group, parent: group_c) }
- let!(:group_c_b) { create(:group, parent: group_c) }
-
- context 'with multiple projects' do
- let!(:project_a) { create(:project, namespace: group_a) }
- let!(:project_b_a) { create(:project, namespace: group_b_a) }
- let!(:project_c) { create(:project, namespace: group_c) }
- let!(:project_c_b) { create(:project, namespace: group_c_b) }
-
- context 'with multiple versions of the same component' do
- let!(:bundler) { create(:sbom_component, :bundler) }
- let!(:bundler_v1) { create(:sbom_occurrence, project: project_a, component: bundler, component_version: create(:sbom_component_version, component: bundler, version: "1.0.0")) }
- let!(:bundler_v2) { create(:sbom_occurrence, project: project_b_a, component: bundler, component_version: create(:sbom_component_version, component: bundler, version: "2.0.0")) }
-
- let!(:webpack) { create(:sbom_component, :webpack) }
- let!(:webpack_v4) { create(:sbom_occurrence, project: project_c, component: webpack, component_version: create(:sbom_component_version, component: webpack, version: "4.46.0")) }
- let!(:webpack_unknown) { create(:sbom_occurrence, project: project_c_b, component: webpack, component_version: nil) }
-
- let!(:caddy) { create(:sbom_component, :caddy) }
- let!(:caddy_v1) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: create(:sbom_component_version, component: caddy, version: "1.0.0")) }
- let!(:caddy_v2) { create(:sbom_occurrence, project: project_c_b, component: caddy, component_version: create(:sbom_component_version, component: caddy, version: "2.7.4")) }
-
- it 'returns an occurrence for each version of each component' do
- expect(subject).to match_array([
- bundler_v1,
- bundler_v2,
- caddy_v1,
- caddy_v2,
- webpack_v4
- ])
- end
- end
- end
- end
end
describe '#reached_project_access_token_limit?' do
diff --git a/ee/spec/models/sbom/occurrence_spec.rb b/ee/spec/models/sbom/occurrence_spec.rb
index 698c6b6655e72e..fb56e3f950fb83 100644
--- a/ee/spec/models/sbom/occurrence_spec.rb
+++ b/ee/spec/models/sbom/occurrence_spec.rb
@@ -318,79 +318,6 @@
end
end
- describe ".with_totals" do
- let_it_be(:project_a) { create(:project) }
- let_it_be(:project_b) { create(:project) }
- let_it_be(:project_c) { create(:project) }
- let_it_be(:project_d) { create(:project) }
-
- let_it_be(:bundler) { create(:sbom_component, :bundler) }
- let_it_be(:bundler_version_1) { create(:sbom_component_version, component: bundler, version: "1.0.0") }
- let_it_be(:bundler_version_2) { create(:sbom_component_version, component: bundler, version: "2.0.0") }
- let_it_be(:bundler_v1) do
- create(:sbom_occurrence, project: project_a, component: bundler, component_version: bundler_version_1)
- end
-
- let_it_be(:bundler_v2) do
- create(:sbom_occurrence, project: project_b, component: bundler, component_version: bundler_version_2)
- end
-
- let_it_be(:webpack) { create(:sbom_component, :webpack) }
- let_it_be(:webpack_version_4) { create(:sbom_component_version, component: webpack, version: "4.46.0") }
- let_it_be(:webpack_v4) do
- create(:sbom_occurrence, project: project_c, component: webpack, component_version: webpack_version_4)
- end
-
- let_it_be(:webpack_unknown) do
- create(:sbom_occurrence, project: project_d, component: webpack, component_version: nil)
- end
-
- let_it_be(:caddy) { create(:sbom_component, :caddy) }
- let_it_be(:caddy_version_1) { create(:sbom_component_version, component: caddy, version: "1.0.0") }
- let_it_be(:caddy_version_2) { create(:sbom_component_version, component: caddy, version: "2.7.4") }
- let_it_be(:caddy_v1) do
- create(:sbom_occurrence, project: project_d, component: caddy, component_version: caddy_version_1)
- end
-
- let_it_be(:caddy_v2) do
- create(:sbom_occurrence, project: project_d, component: caddy, component_version: caddy_version_2)
- end
-
- subject do
- our_projects = ::Gitlab::SQL::CTE.new(:our_projects,
- ::Project.where(id: [project_a, project_c, project_d]).select(:id))
- our_occurrences = ::Gitlab::SQL::CTE.new(:our_occurrences, described_class.where(
- described_class.arel_table[:project_id].in(our_projects.table.project(our_projects.table[:id]))
- ))
-
- described_class
- .with(our_projects.to_arel)
- .with(our_occurrences.to_arel)
- .from(our_occurrences.alias_to(described_class.arel_table))
- .with_totals(our_occurrences)
- end
-
- # Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
- it 'returns the project count for each version of each component' do
- expect(subject.pluck(:component_name, :component_version_id, :project_count)).to match_array([
- [bundler.name, bundler_version_1.id, 1],
- [caddy.name, caddy_version_1.id, 1],
- [caddy.name, caddy_version_2.id, 1],
- [webpack.name, webpack_version_4.id, 1]
- ])
- end
-
- # Related to https://gitlab.com/gitlab-org/gitlab/-/issues/424211
- it 'returns the occurrence count for each version of each component' do
- expect(subject.pluck(:component_name, :component_version_id, :occurrence_count)).to match_array([
- [bundler.name, bundler_version_1.id, 1],
- [caddy.name, caddy_version_1.id, 1],
- [caddy.name, caddy_version_2.id, 1],
- [webpack.name, webpack_version_4.id, 1]
- ])
- end
- end
-
describe '#name' do
let(:component) { build(:sbom_component, name: 'rails') }
let(:occurrence) { build(:sbom_occurrence, component: component) }
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index 3700a1cde65e60..916b1bd50087dc 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -179,20 +179,6 @@
expect(json_response).to eq(expected_response)
end
- it 'loads the data without N+1 queries' do
- def render
- get group_dependencies_path(group_id: group.full_path), params: params, as: :json
- end
-
- control = ActiveRecord::QueryRecorder.new do
- render
- end
-
- create(:sbom_occurrence, project: create(:project, group: group))
-
- expect { render }.not_to exceed_query_limit(control)
- end
-
it 'includes pagination headers in the response' do
subject
@@ -210,7 +196,7 @@ def render
end
expect(project_routes_count).to eq(1)
- expect(project_count).to eq(2)
+ expect(project_count).to eq(1)
end
context 'with sorting params' do
--
GitLab
From 0c0f3a7d56df02c0da9ae106538835d77de7539a Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 13 Sep 2023 16:09:37 -0600
Subject: [PATCH 30/32] Prevent sorting by license on large groups
---
.../dependencies/components/dependencies_actions.vue | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/ee/app/assets/javascripts/dependencies/components/dependencies_actions.vue b/ee/app/assets/javascripts/dependencies/components/dependencies_actions.vue
index d3fce2a689735b..ccd8ffbcee4bd2 100644
--- a/ee/app/assets/javascripts/dependencies/components/dependencies_actions.vue
+++ b/ee/app/assets/javascripts/dependencies/components/dependencies_actions.vue
@@ -24,7 +24,7 @@ export default {
GlSortingItem,
},
mixins: [glFeatureFlagsMixin()],
- inject: ['namespaceType'],
+ inject: ['namespaceType', 'enableProjectSearch'],
props: {
namespace: {
type: String,
@@ -49,9 +49,10 @@ export default {
return this.sortFields[this.sortField];
},
sortFields() {
- const groupFields = this.glFeatures.groupLevelLicenses
- ? SORT_FIELDS_GROUP
- : omit(SORT_FIELDS_GROUP, SORT_FIELD_LICENSE);
+ const groupFields =
+ this.glFeatures.groupLevelLicenses && this.enableProjectSearch
+ ? SORT_FIELDS_GROUP
+ : omit(SORT_FIELDS_GROUP, SORT_FIELD_LICENSE);
return this.isProjectNamespace ? SORT_FIELDS_PROJECT : groupFields;
},
--
GitLab
From 3482582f05cea900d65a6bb8365071689850ee69 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 13 Sep 2023 16:17:59 -0600
Subject: [PATCH 31/32] Inject enableProjectSearch in specs
---
.../dependencies/components/dependencies_actions_spec.js | 1 +
1 file changed, 1 insertion(+)
diff --git a/ee/spec/frontend/dependencies/components/dependencies_actions_spec.js b/ee/spec/frontend/dependencies/components/dependencies_actions_spec.js
index 9530d2eac659c8..e68414c32e8324 100644
--- a/ee/spec/frontend/dependencies/components/dependencies_actions_spec.js
+++ b/ee/spec/frontend/dependencies/components/dependencies_actions_spec.js
@@ -17,6 +17,7 @@ describe('DependenciesActions component', () => {
const objectBasicProp = {
namespaceType: 'project',
+ enableProjectSearch: true,
};
const factory = ({ propsData, provide } = {}) => {
--
GitLab
From 66f402cd3a8770e5d367bfa8fe8e441e0f025c39 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 13 Sep 2023 16:20:40 -0600
Subject: [PATCH 32/32] Add spec to disable sorting by license on large group
hierarchies
---
.../components/dependencies_actions_spec.js | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/ee/spec/frontend/dependencies/components/dependencies_actions_spec.js b/ee/spec/frontend/dependencies/components/dependencies_actions_spec.js
index e68414c32e8324..ce79cee2322a9c 100644
--- a/ee/spec/frontend/dependencies/components/dependencies_actions_spec.js
+++ b/ee/spec/frontend/dependencies/components/dependencies_actions_spec.js
@@ -84,6 +84,33 @@ describe('DependenciesActions component', () => {
);
});
+ describe('with the "enableProjectSearch" set to false', () => {
+ beforeEach(async () => {
+ factory({
+ propsData: { namespace },
+ provide: {
+ namespaceType: 'group',
+ enableProjectSearch: false,
+ glFeatures: { groupLevelLicenses: true },
+ },
+ });
+ store.state[namespace].endpoint = `${TEST_HOST}/dependencies.json`;
+ await nextTick();
+ });
+
+ it('does not dispatch the "license" action', () => {
+ const sortingItems = wrapper.findAllComponents(GlSortingItem).wrappers;
+
+ sortingItems.forEach((item) => {
+ item.vm.$emit('click');
+ });
+
+ expect(store.dispatch.mock.calls).not.toEqual(
+ expect.arrayContaining([[`${namespace}/setSortField`, 'license']]),
+ );
+ });
+ });
+
describe('with the "groupLevelLicenses" feature flag disabled', () => {
beforeEach(async () => {
factory({
--
GitLab