From bbade5051357d60321291c8ac79d4ce5045e99ba Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Fri, 15 Sep 2023 09:54:21 +0000
Subject: [PATCH] Fix query to remove ambiguous column reference errors

```bash
RSpec::Retry: 2nd try ./ee/spec/services/dependencies/export_serializers/group_dependencies_service_spec.rb:33
    when the group has dependencies
      example at ./ee/spec/services/dependencies/export_serializers/group_dependencies_service_spec.rb:43 (FAILED - 2)

1st Try error in ./ee/spec/services/dependencies/export_serializers/group_dependencies_service_spec.rb:43:
PG::AmbiguousColumn: ERROR:  column reference "id" is ambiguous
LINE 1: ... AND "projects"."pending_delete" = FALSE)) SELECT id FROM "o...
```

```sql
WITH "our_occurrences" AS MATERIALIZED (
  SELECT "sbom_occurrences".*
  FROM "sbom_occurrences"
  WHERE "sbom_occurrences"."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 @> ('{3456}'))
    )
    AND "projects"."marked_for_deletion_at" IS NULL
    AND "projects"."pending_delete" = FALSE
  )
)
SELECT id
FROM "our_occurrences" AS "sbom_occurrences"
LEFT OUTER JOIN "projects" ON "projects"."id" = "sbom_occurrences"."project_id"
LEFT OUTER JOIN "routes" ON "routes"."source_type" = 'Project' AND "routes"."source_id" = "projects"."id"
INNER JOIN (
  SELECT component_id,
        COUNT(DISTINCT id) AS occurrence_count,
        COUNT(DISTINCT project_id) AS project_count
  FROM our_occurrences
  GROUP BY component_id
) agg_occurrences ON sbom_occurrences.component_id = agg_occurrences.component_id
ORDER BY "sbom_occurrences"."id" ASC
LIMIT 1000;
```
---
 ee/app/models/ee/group.rb                     | 19 +++-----
 ee/spec/factories/sbom/components.rb          | 15 ++++++
 ee/spec/models/ee/group_spec.rb               | 47 +++++++++++++++++++
 .../groups/dependencies_controller_spec.rb    |  2 +-
 4 files changed, 70 insertions(+), 13 deletions(-)

diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 5b69438637656..bb71338f412e1 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -840,29 +840,24 @@ 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)
+      ))
+
       Sbom::Occurrence.includes(project: :route).select('sbom_occurrences.*, agg_occurrences.occurrence_count, agg_occurrences.project_count')
-      .from('sbom_occurrences')
+      .with(our_occurrences.to_arel)
+      .where(Sbom::Occurrence.arel_table[:id].in(our_occurrences.table.project(our_occurrences.table[:id])))
       .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}}\'))
-              )
-            )
+            FROM #{our_occurrences.table.name}
             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
 
diff --git a/ee/spec/factories/sbom/components.rb b/ee/spec/factories/sbom/components.rb
index d1cf8fc456cb4..8ae125b514ba0 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 517b68ff0a862..6c64ce7db8b2c 100644
--- a/ee/spec/models/ee/group_spec.rb
+++ b/ee/spec/models/ee/group_spec.rb
@@ -2878,6 +2878,53 @@ def webhook_headers
         end
       end
     end
+
+    context 'with multiple sub groups' do
+      let_it_be(:group) { create(:group) }
+      let_it_be(:group_a) { create(:group, parent: group) }
+      let_it_be(:group_a_a) { create(:group, parent: group_a) }
+
+      let_it_be(:project) { create(:project, namespace: group) }
+      let_it_be(:project_a) { create(:project, namespace: group_a) }
+      let_it_be(:project_a_a) { create(:project, namespace: group_a_a) }
+
+      let_it_be(:bundler) { create(:sbom_component, :bundler) }
+      let_it_be(:bundler_v1) { create(:sbom_component_version, component: bundler, version: "1.0.0") }
+
+      let_it_be(:webpack) { create(:sbom_component, :webpack) }
+      let_it_be(:webpack_v4) { create(:sbom_component_version, component: webpack, version: "4.46.0") }
+
+      let_it_be(:caddy) { create(:sbom_component, :caddy) }
+      let_it_be(:caddy_v1) { create(:sbom_component_version, component: caddy, version: "1.0.0") }
+
+      let_it_be(:bundler_in_root_project) { create(:sbom_occurrence, project: project, component: bundler, component_version: bundler_v1) }
+      let_it_be(:webpack_in_project_a) { create(:sbom_occurrence, project: project_a, component: webpack, component_version: webpack_v4) }
+      let_it_be(:caddy_in_project_a_a) { create(:sbom_occurrence, project: project_a_a, component: caddy, component_version: caddy_v1) }
+
+      it 'returns an occurrence for each version of each component' do
+        expect(subject).to match_array([
+          bundler_in_root_project,
+          caddy_in_project_a_a,
+          webpack_in_project_a
+        ])
+      end
+
+      it 'returns the project count for each component' do
+        expect(subject.pluck(:component_name, :component_version_id, :project_count)).to match_array([
+          [bundler.name, bundler_v1.id, 1],
+          [caddy.name, caddy_v1.id, 1],
+          [webpack.name, webpack_v4.id, 1]
+        ])
+      end
+
+      it 'returns the occurrence count for each component' do
+        expect(subject.pluck(:component_name, :component_version_id, :occurrence_count)).to match_array([
+          [bundler.name, bundler_v1.id, 1],
+          [caddy.name, caddy_v1.id, 1],
+          [webpack.name, webpack_v4.id, 1]
+        ])
+      end
+    end
   end
 
   describe '#reached_project_access_token_limit?' do
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index c20a37e1e14f1..5b0a8e2d196d6 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -196,7 +196,7 @@
               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