Merge branch '239518_add_build_info_to_all_packages' into 'master'

Add build info to all package types

See merge request gitlab-org/gitlab!48811

app/services/packages/composer/create_package_service.rb

@@ -16,6 +16,8 @@ module Packages
             composer_json: composer_json
           })
         end
+
+        created_package
       end
 
       private

app/services/packages/conan/create_package_file_service.rb

@@ -12,7 +12,7 @@ module Packages
       end
 
       def execute
-        package.package_files.create!(
+        package_file = package.package_files.build(
           file:      file,
           size:      params['file.size'],
           file_name: params[:file_name],
@@ -25,6 +25,13 @@ module Packages
             conan_file_type: params[:conan_file_type]
           }
         )
+
+        if params[:build].present?
+          package_file.package_file_build_infos << package_file.package_file_build_infos.build(pipeline: params[:build].pipeline)
+        end
+
+        package_file.save!
+        package_file
       end
     end
   end

app/services/packages/create_package_service.rb

@@ -8,9 +8,9 @@ module Packages
       project
         .packages
         .with_package_type(package_type)
-        .safe_find_or_create_by!(name: name, version: version) do |pkg|
-          pkg.creator = package_creator
-          yield pkg if block_given?
+        .safe_find_or_create_by!(name: name, version: version) do |package|
+          package.creator = package_creator
+          add_build_info(package)
         end
     end
 
@@ -18,7 +18,9 @@ module Packages
       project
         .packages
         .with_package_type(package_type)
-        .create!(package_attrs(attrs))
+        .create!(package_attrs(attrs)) do |package|
+          add_build_info(package)
+        end
     end
 
     private
@@ -34,5 +36,11 @@ module Packages
     def package_creator
       current_user if current_user.is_a?(User)
     end
+
+    def add_build_info(package)
+      if params[:build].present?
+        package.build_infos.new(pipeline: params[:build].pipeline)
+      end
+    end
   end
 end

app/services/packages/generic/create_package_file_service.rb

@@ -18,9 +18,12 @@ module Packages
           build: params[:build]
         }
 
-        ::Packages::Generic::FindOrCreatePackageService
+        package = ::Packages::Generic::FindOrCreatePackageService
           .new(project, current_user, package_params)
           .execute
+
+        package.build_infos.safe_find_or_create_by!(pipeline: params[:build].pipeline) if params[:build].present?
+        package
       end
 
       def create_package_file(package)

app/services/packages/generic/find_or_create_package_service.rb

@@ -4,11 +4,7 @@ module Packages
   module Generic
     class FindOrCreatePackageService < ::Packages::CreatePackageService
       def execute
-        find_or_create_package!(::Packages::Package.package_types['generic']) do |package|
-          if params[:build].present?
-            package.build_infos.new(pipeline: params[:build].pipeline)
-          end
-        end
+        find_or_create_package!(::Packages::Package.package_types['generic'])
       end
     end
   end

app/services/packages/maven/find_or_create_package_service.rb

@@ -46,7 +46,7 @@ module Packages
                                                    .execute
         end
 
-        package.build_infos.create!(pipeline: params[:build].pipeline) if params[:build].present?
+        package.build_infos.safe_find_or_create_by!(pipeline: params[:build].pipeline) if params[:build].present?
 
         package
       end

app/services/packages/npm/create_package_service.rb

@@ -17,10 +17,6 @@ module Packages
       def create_npm_package!
         package = create_package!(:npm, name: name, version: version)
 
-        if build.present?
-          package.build_infos.create!(pipeline: build.pipeline)
-        end
-
         ::Packages::CreatePackageFileService.new(package, file_params).execute
         ::Packages::CreateDependencyService.new(package, package_dependencies).execute
         ::Packages::Npm::CreateTagService.new(package, dist_tag).execute
@@ -50,10 +46,6 @@ module Packages
         params[:versions][version]
       end
 
-      def build
-        params[:build]
-      end
-
       def dist_tag
         params['dist-tags'].each_key.first
       end

app/services/packages/pypi/create_package_service.rb

@@ -19,6 +19,8 @@ module Packages
           Packages::Pypi::Metadatum.upsert(meta.attributes)
 
           ::Packages::CreatePackageFileService.new(created_package, file_params).execute
+
+          created_package
         end
       end
 
@@ -32,6 +34,7 @@ module Packages
 
       def file_params
         {
+          build: params[:build],
           file: params[:content],
           file_name: params[:content].original_filename,
           file_md5: params[:md5_digest],

changelogs/unreleased/239518_add_build_info_to_all_packages.yml

+---
+title: Create package build_info records for Conan, NuGet, PyPI, and Composer packages
+  and package files
+merge_request: 48811
+author:
+type: added

lib/api/composer_packages.rb

@@ -132,7 +132,7 @@ module API
           track_package_event('push_package', :composer)
 
           ::Packages::Composer::CreatePackageService
-            .new(authorized_user_project, current_user, declared_params)
+            .new(authorized_user_project, current_user, declared_params.merge(build: current_authenticated_job))
             .execute
 
           created!

lib/api/helpers/packages/conan/api_helpers.rb

@@ -164,7 +164,11 @@ module API
           end
 
           def find_or_create_package
-            package || ::Packages::Conan::CreatePackageService.new(project, current_user, params).execute
+            package || ::Packages::Conan::CreatePackageService.new(
+              project,
+              current_user,
+              params.merge(build: current_authenticated_job)
+            ).execute
           end
 
           def track_push_package_event
@@ -184,7 +188,11 @@ module API
           def create_package_file_with_type(file_type, current_package)
             unless params[:file].size == 0 # rubocop: disable Style/ZeroLengthPredicate
               # conan sends two upload requests, the first has no file, so we skip record creation if file.size == 0
-              ::Packages::Conan::CreatePackageFileService.new(current_package, params[:file], params.merge(conan_file_type: file_type)).execute
+              ::Packages::Conan::CreatePackageFileService.new(
+                current_package,
+                params[:file],
+                params.merge(conan_file_type: file_type, build: current_authenticated_job)
+              ).execute
             end
           end
 

lib/api/nuget_packages.rb

@@ -101,11 +101,16 @@ module API
             file_name: PACKAGE_FILENAME
           )
 
-          package = ::Packages::Nuget::CreatePackageService.new(authorized_user_project, current_user)
-                                                           .execute
-
-          package_file = ::Packages::CreatePackageFileService.new(package, file_params)
-                                                             .execute
+          package = ::Packages::Nuget::CreatePackageService.new(
+            authorized_user_project,
+            current_user,
+            declared_params.merge(build: current_authenticated_job)
+          ).execute
+
+          package_file = ::Packages::CreatePackageFileService.new(
+            package,
+            file_params.merge(build: current_authenticated_job)
+          ).execute
 
           track_package_event('push_package', :nuget)
 

lib/api/pypi_packages.rb

@@ -127,7 +127,7 @@ module API
           track_package_event('push_package', :pypi)
 
           ::Packages::Pypi::CreatePackageService
-            .new(authorized_user_project, current_user, declared_params)
+            .new(authorized_user_project, current_user, declared_params.merge(build: current_authenticated_job))
             .execute
 
           created!

spec/services/packages/composer/create_package_service_spec.rb

@@ -41,6 +41,8 @@ RSpec.describe Packages::Composer::CreatePackageService do
         it_behaves_like 'assigns the package creator' do
           let(:package) { created_package }
         end
+
+        it_behaves_like 'assigns build to package'
       end
 
       context 'with a tag' do
@@ -62,6 +64,8 @@ RSpec.describe Packages::Composer::CreatePackageService do
         it_behaves_like 'assigns the package creator' do
           let(:package) { created_package }
         end
+
+        it_behaves_like 'assigns build to package'
       end
     end
 

spec/services/packages/conan/create_package_file_service_spec.rb

@@ -5,11 +5,12 @@ RSpec.describe Packages::Conan::CreatePackageFileService do
   include WorkhorseHelpers
 
   let_it_be(:package) { create(:conan_package) }
+  let_it_be(:user) { create(:user) }
 
   describe '#execute' do
     let(:file_name) { 'foo.tgz' }
 
-    subject { described_class.new(package, file, params) }
+    subject { described_class.new(package, file, params).execute }
 
     shared_examples 'a valid package_file' do
       let(:params) do
@@ -27,7 +28,7 @@ RSpec.describe Packages::Conan::CreatePackageFileService do
       end
 
       it 'creates a new package file' do
-        package_file = subject.execute
+        package_file = subject
 
         expect(package_file).to be_valid
         expect(package_file.file_name).to eq(file_name)
@@ -40,6 +41,8 @@ RSpec.describe Packages::Conan::CreatePackageFileService do
         expect(package_file.conan_file_metadatum.conan_file_type).to eq('package_file')
         expect(package_file.file.read).to eq('content')
       end
+
+      it_behaves_like 'assigns build to package file'
     end
 
     shared_examples 'a valid recipe_file' do
@@ -56,7 +59,7 @@ RSpec.describe Packages::Conan::CreatePackageFileService do
       end
 
       it 'creates a new recipe file' do
-        package_file = subject.execute
+        package_file = subject
 
         expect(package_file).to be_valid
         expect(package_file.file_name).to eq(file_name)
@@ -69,6 +72,8 @@ RSpec.describe Packages::Conan::CreatePackageFileService do
         expect(package_file.conan_file_metadatum.conan_file_type).to eq('recipe_file')
         expect(package_file.file.read).to eq('content')
       end
+
+      it_behaves_like 'assigns build to package file'
     end
 
     context 'with temp file' do
@@ -123,7 +128,7 @@ RSpec.describe Packages::Conan::CreatePackageFileService do
       end
 
       it 'raises an error' do
-        expect { subject.execute }.to raise_error(ActiveRecord::RecordInvalid)
+        expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
       end
     end
   end

spec/services/packages/conan/create_package_service_spec.rb

@@ -30,6 +30,7 @@ RSpec.describe Packages::Conan::CreatePackageService do
       end
 
       it_behaves_like 'assigns the package creator'
+      it_behaves_like 'assigns build to package'
     end
 
     context 'invalid params' do

spec/services/packages/create_package_file_service_spec.rb

@@ -4,10 +4,11 @@ require 'spec_helper'
 RSpec.describe Packages::CreatePackageFileService do
   let_it_be(:package) { create(:maven_package) }
   let_it_be(:user) { create(:user) }
-
-  subject { described_class.new(package, params) }
+  let(:service) { described_class.new(package, params) }
 
   describe '#execute' do
+    subject { service.execute }
+
     context 'with valid params' do
       let(:params) do
         {
@@ -17,11 +18,13 @@ RSpec.describe Packages::CreatePackageFileService do
       end
 
       it 'creates a new package file' do
-        package_file = subject.execute
+        package_file = subject
 
         expect(package_file).to be_valid
         expect(package_file.file_name).to eq('foo.jar')
       end
+
+      it_behaves_like 'assigns build to package file'
     end
 
     context 'file is missing' do
@@ -32,17 +35,7 @@ RSpec.describe Packages::CreatePackageFileService do
       end
 
       it 'raises an error' do
-        expect { subject.execute }.to raise_error(ActiveRecord::RecordInvalid)
-      end
-    end
-
-    context 'with a build' do
-      let_it_be(:pipeline) { create(:ci_pipeline, user: user) }
-      let(:build) { double('build', pipeline: pipeline) }
-      let(:params) { { file: Tempfile.new, file_name: 'foo.jar', build: build } }
-
-      it 'creates a build_info' do
-        expect { subject.execute }.to change { Packages::PackageFileBuildInfo.count }.by(1)
+        expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
       end
     end
   end

spec/services/packages/generic/create_package_file_service_spec.rb

@@ -23,6 +23,8 @@ RSpec.describe Packages::Generic::CreatePackageFileService do
       }
     end
 
+    subject { described_class.new(project, user, params).execute }
+
     before do
       FileUtils.touch(temp_file)
     end
@@ -41,9 +43,7 @@ RSpec.describe Packages::Generic::CreatePackageFileService do
       expect(::Packages::Generic::FindOrCreatePackageService).to receive(:new).with(project, user, package_params).and_return(package_service)
       expect(package_service).to receive(:execute).and_return(package)
 
-      service = described_class.new(project, user, params)
-
-      expect { service.execute }.to change { package.package_files.count }.by(1)
+      expect { subject }.to change { package.package_files.count }.by(1)
         .and change { Packages::PackageFileBuildInfo.count }.by(1)
 
       package_file = package.package_files.last
@@ -54,5 +54,7 @@ RSpec.describe Packages::Generic::CreatePackageFileService do
         expect(package_file.file_sha256).to eq(sha256)
       end
     end
+
+    it_behaves_like 'assigns build to package file'
   end
 end

spec/services/packages/nuget/create_package_service_spec.rb

@@ -31,5 +31,6 @@ RSpec.describe Packages::Nuget::CreatePackageService do
     end
 
     it_behaves_like 'assigns the package creator'
+    it_behaves_like 'assigns build to package'
   end
 end

spec/services/packages/pypi/create_package_service_spec.rb

@@ -51,6 +51,8 @@ RSpec.describe Packages::Pypi::CreatePackageService do
       let(:package) { created_package }
     end
 
+    it_behaves_like 'assigns build to package'
+
     context 'with an existing package' do
       before do
         described_class.new(project, user, params).execute

spec/support/shared_examples/services/packages_shared_examples.rb

@@ -14,6 +14,24 @@ RSpec.shared_examples 'assigns build to package' do
   end
 end
 
+RSpec.shared_examples 'assigns build to package file' do
+  context 'with build info' do
+    let(:job) { create(:ci_build, user: user) }
+    let(:params) { super().merge(build: job) }
+
+    it 'assigns the pipeline to the package' do
+      package_file = subject
+
+      expect(package_file.package_file_build_infos).to be_present
+      expect(package_file.pipelines.first).to eq job.pipeline
+    end
+
+    it 'creates a new PackageFileBuildInfo record' do
+      expect { subject }.to change { Packages::PackageFileBuildInfo.count }.by(1)
+    end
+  end
+end
+
 RSpec.shared_examples 'assigns the package creator' do
   it 'assigns the package creator' do
     subject