Merge branch '219171-package-n2-bug' into 'master'

Fix package API N^2 query

See merge request gitlab-org/gitlab!40770

app/finders/packages/group_packages_finder.rb

@@ -22,6 +22,9 @@ module Packages
 
     def packages_for_group_projects
       packages = ::Packages::Package
+        .including_build_info
+        .including_project_route
+        .including_tags
         .for_projects(group_projects_visible_to_current_user)
         .processed
         .has_version

app/finders/packages/package_finder.rb

@@ -9,6 +9,9 @@ module Packages
     def execute
       @project
         .packages
+        .including_build_info
+        .including_project_route
+        .including_tags
         .processed
         .find(@package_id)
     end

app/finders/packages/packages_finder.rb

@@ -13,7 +13,12 @@ module Packages
     end
 
     def execute
-      packages = project.packages.processed.has_version
+      packages = project.packages
+                        .including_build_info
+                        .including_project_route
+                        .including_tags
+                        .processed
+                        .has_version
       packages = filter_by_package_type(packages)
       packages = filter_by_package_name(packages)
       packages = order_packages(packages)

app/models/packages/package.rb

@@ -51,6 +51,9 @@ class Packages::Package < ApplicationRecord
   scope :with_version, ->(version) { where(version: version) }
   scope :without_version_like, -> (version) { where.not(arel_table[:version].matches(version)) }
   scope :with_package_type, ->(package_type) { where(package_type: package_type) }
+  scope :including_build_info, -> { includes(build_info: { pipeline: :user }) }
+  scope :including_project_route, -> { includes(project: { namespace: :route }) }
+  scope :including_tags, -> { includes(:tags) }
 
   scope :with_conan_channel, ->(package_channel) do
     joins(:conan_metadatum).where(packages_conan_metadata: { package_channel: package_channel })
@@ -143,6 +146,8 @@ class Packages::Package < ApplicationRecord
 
   def versions
     project.packages
+           .including_build_info
+           .including_tags
            .with_name(name)
            .where.not(version: version)
            .with_package_type(package_type)

changelogs/unreleased/219171-package-n2-bug.yml

+---
+title: Fix package API query performance when pipelines and multiple versions are present
+merge_request: 40770
+author:
+type: performance

lib/api/entities/package.rb

@@ -28,7 +28,7 @@ module API
 
       expose :pipeline, if: ->(package) { package.build_info }, using: Package::Pipeline
 
-      expose :versions, using: ::API::Entities::PackageVersion
+      expose :versions, using: ::API::Entities::PackageVersion, unless: ->(_, opts) { opts[:collection] }
 
       private
 

spec/fixtures/api/schemas/public_api/v4/packages/collection_package.json

+{
+  "type": "object",
+  "required": [
+    "name",
+    "version",
+    "package_type",
+    "_links"
+  ],
+  "properties": {
+    "name": {
+      "type": "string"
+    },
+    "version": {
+      "type": "string"
+    },
+    "package_type": {
+      "type": "string"
+    },
+    "_links": {
+      "type": "object",
+      "required": [
+        "web_path"
+      ],
+      "properties": {
+        "details": {
+          "type": "string"
+        }
+      }
+    },
+    "created_at": {
+      "type": "string"
+    }
+  }
+}
\ No newline at end of file

spec/fixtures/api/schemas/public_api/v4/packages/packages.json

 {
   "type": "array",
-  "items": { "$ref": "./package.json" }
+  "items": { "$ref": "./collection_package.json" }
 }

spec/requests/api/group_packages_spec.rb

@@ -141,5 +141,7 @@ RSpec.describe API::GroupPackages do
 
       it_behaves_like 'returning response status', :bad_request
     end
+
+    it_behaves_like 'does not cause n^2 queries'
   end
 end

spec/requests/api/project_packages_spec.rb

@@ -104,6 +104,8 @@ RSpec.describe API::ProjectPackages do
           expect(json_response.first['name']).to include(package2.name)
         end
       end
+
+      it_behaves_like 'does not cause n^2 queries'
     end
   end
 
@@ -127,6 +129,22 @@ RSpec.describe API::ProjectPackages do
     end
 
     context 'without the need for a license' do
+      context 'with build info' do
+        it 'does not result in additional queries' do
+          control = ActiveRecord::QueryRecorder.new do
+            get api(package_url, user)
+          end
+
+          pipeline = create(:ci_pipeline, user: user)
+          create(:ci_build, user: user, pipeline: pipeline)
+          create(:package_build_info, package: package1, pipeline: pipeline)
+
+          expect do
+            get api(package_url, user)
+          end.not_to exceed_query_limit(control)
+        end
+      end
+
       context 'project is public' do
         it 'returns 200 and the package information' do
           subject

spec/support/shared_examples/requests/api/packages_shared_examples.rb

@@ -41,3 +41,32 @@ RSpec.shared_examples 'deploy token for package uploads' do
     end
   end
 end
+
+RSpec.shared_examples 'does not cause n^2 queries' do
+  it 'avoids N^2 database queries' do
+    # we create a package to set the baseline for expected queries from 1 package
+    create(
+      :npm_package,
+      name: "@#{project.root_namespace.path}/my-package",
+      project: project,
+      version: "0.0.1"
+    )
+
+    control = ActiveRecord::QueryRecorder.new do
+      get api(url)
+    end
+
+    5.times do |n|
+      create(
+        :npm_package,
+        name: "@#{project.root_namespace.path}/my-package",
+        project: project,
+        version: "#{n}.0.0"
+      )
+    end
+
+    expect do
+      get api(url)
+    end.not_to exceed_query_limit(control)
+  end
+end