Merge branch '290288-disable-composer-shas-for-v2' into 'master'

Improve performance for composer v2 clients

See merge request gitlab-org/gitlab!55169

app/presenters/packages/composer/packages_presenter.rb

@@ -5,25 +5,35 @@ module Composer
     class PackagesPresenter
       include API::Helpers::RelatedResourcesHelpers
-      def initialize(group, packages)
+      def initialize(group, packages, is_v2 = false)
         @group = group
         @packages = packages
+        @is_v2 = is_v2
       end
       def root
-        v1_path = expose_path(api_v4_group___packages_composer_package_name_path({ id: @group.id, package_name: '%package%$%hash%', format: '.json' }, true))
         v2_path = expose_path(api_v4_group___packages_composer_p2_package_name_path({ id: @group.id, package_name: '%package%', format: '.json' }, true))
-        {
+        index = {
           'packages' => [],
+          'metadata-url' => v2_path
+        }
+
+        # if the client is composer v2 then we don't want to
+        # include the provider_sha since it is computationally expensive
+        # to compute.
+        return index if @is_v2
+
+        v1_path = expose_path(api_v4_group___packages_composer_package_name_path({ id: @group.id, package_name: '%package%$%hash%', format: '.json' }, true))
+
+        index.merge!(
           'provider-includes' => {
             'p/%hash%.json' => {
               'sha256' => provider_sha
             }
           },
-          'providers-url' => v1_path,
-          'metadata-url' => v2_path
-        }
+          'providers-url' => v1_path
+        )
       end
       def provider

changelogs/unreleased/290288-disable-composer-shas-for-v2.yml

+---
+title: Improve performance for composer v2 clients
+merge_request: 55169
+author:
+type: added

lib/api/composer_packages.rb

@@ -47,8 +47,12 @@ def packages
         end
       end
+      def composer_v2?
+        headers['User-Agent'].to_s.include?('Composer/2')
+      end
+
       def presenter
-        @presenter ||= ::Packages::Composer::PackagesPresenter.new(user_group, packages)
+        @presenter ||= ::Packages::Composer::PackagesPresenter.new(user_group, packages, composer_v2?)
       end
     end
@@ -66,33 +70,25 @@ def presenter
       end
       desc 'Composer packages endpoint at group level'
-
       route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true
-
       get ':id/-/packages/composer/packages' do
         presenter.root
       end
       desc 'Composer packages endpoint at group level for packages list'
-
       params do
         requires :sha, type: String, desc: 'Shasum of current json'
       end
-
       route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true
-
       get ':id/-/packages/composer/p/:sha' do
         presenter.provider
       end
-      desc 'Composer packages endpoint at group level for package versions metadata'
-
+      desc 'Composer v2 packages p2 endpoint at group level for package versions metadata'
       params do
         requires :package_name, type: String, file_path: true, desc: 'The Composer package name'
       end
-
       route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true
-
       get ':id/-/packages/composer/p2/*package_name', requirements: COMPOSER_ENDPOINT_REQUIREMENTS, file_path: true do
         not_found! if packages.empty?
@@ -100,13 +96,10 @@ def presenter
       end
       desc 'Composer packages endpoint at group level for package versions metadata'
-
       params do
         requires :package_name, type: String, file_path: true, desc: 'The Composer package name'
       end
-
       route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true
-
       get ':id/-/packages/composer/*package_name', requirements: COMPOSER_ENDPOINT_REQUIREMENTS, file_path: true do
         not_found! if packages.empty?
         not_found! if params[:sha].blank?
@@ -125,7 +118,6 @@ def presenter
       end
       desc 'Composer packages endpoint for registering packages'
-
       namespace ':id/packages/composer' do
         route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true
@@ -134,7 +126,6 @@ def presenter
           optional :tag, type: String, desc: 'The name of the tag'
           exactly_one_of :tag, :branch
         end
-
         post do
           authorize_create_package!(authorized_user_project)
@@ -159,7 +150,6 @@ def presenter
           requires :sha, type: String, desc: 'Shasum of current json'
           requires :package_name, type: String, file_path: true, desc: 'The Composer package name'
         end
-
         get 'archives/*package_name' do
           metadata = unauthorized_user_project
             .packages

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

+{
+  "type": "object",
+  "required": ["packages", "metadata-url"],
+  "properties": {
+    "packages": {
+      "type": "array",
+      "items": { "type": "integer" }
+    },
+    "metadata-url": {
+      "type": "string"
+    }
+  },
+  "additionalProperties": false
+}

spec/presenters/packages/composer/packages_presenter_spec.rb

@@ -15,7 +15,8 @@
   let(:branch) { project.repository.find_branch('master') }
   let(:packages) { [package1, package2] }
-  let(:presenter) { described_class.new(group, packages) }
+  let(:is_v2) { false }
+  let(:presenter) { described_class.new(group, packages, is_v2) }
   describe '#package_versions' do
     subject { presenter.package_versions }
@@ -84,5 +85,19 @@ def expected_json(package)
     it 'returns the provider json' do
       expect(subject).to match(expected_json)
     end
+
+    context 'with a client version 2' do
+      let(:is_v2) { true }
+      let(:expected_json) do
+        {
+          'packages' => [],
+          'metadata-url' => "prefix/api/v4/group/#{group.id}/-/packages/composer/p2/%package%.json"
+        }
+      end
+
+      it 'returns the provider json' do
+        expect(subject).to match(expected_json)
+      end
+    end
   end
 end

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

@@ -7,20 +7,30 @@
   end
 end
+RSpec.shared_examples 'Composer package index with version' do |schema_path|
+  it 'returns the package index' do
+    subject
+
+    expect(response).to have_gitlab_http_status(status)
+
+    if status == :success
+      expect(response).to match_response_schema(schema_path)
+      expect(json_response).to eq presenter.root
+    end
+  end
+end
+
 RSpec.shared_examples 'Composer package index' do |user_type, status, add_member, include_package|
   include_context 'Composer user type', user_type, add_member do
     let(:expected_packages) { include_package == :include_package ? [package] : [] }
     let(:presenter) { ::Packages::Composer::PackagesPresenter.new(group, expected_packages ) }
-    it 'returns the package index' do
-      subject
-      expect(response).to have_gitlab_http_status(status)
-      if status == :success
-        expect(response).to match_response_schema('public_api/v4/packages/composer/index')
-        expect(json_response).to eq presenter.root
-      end
+    it_behaves_like 'Composer package index with version', 'public_api/v4/packages/composer/index'
+    context 'with version 2' do
+      let(:headers) { super().merge('User-Agent' => 'Composer/2.0.9 (Darwin; 19.6.0; PHP 7.4.8; cURL 7.71.1)') }
+      it_behaves_like 'Composer package index with version', 'public_api/v4/packages/composer/index_v2'
     end
   end
 end