From cce7c221c472f8cb64fe9995b9cd9d81f5aa5890 Mon Sep 17 00:00:00 2001 From: Moaz Khalifa <mkhalifa@gitlab.com> Date: Tue, 3 Oct 2023 09:44:00 +0000 Subject: [PATCH] Fix ci pipeline reference when pushing duplicate NuGet package Pushing a NuGet package from a CI pipeline doesn't link to the CI pipeline properly when the package is a duplicate. This mr is fixing this behavior by moving the ci pipeline reference from the tmp package to the existing one. Changelog: fixed --- .../update_package_from_metadata_service.rb | 34 ++++++++----------- spec/factories/packages/packages.rb | 6 ++++ spec/models/packages/build_info_spec.rb | 2 +- ...date_package_from_metadata_service_spec.rb | 3 +- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/app/services/packages/nuget/update_package_from_metadata_service.rb b/app/services/packages/nuget/update_package_from_metadata_service.rb index 258f8c8f6aaf1..df0d2a851a3b9 100644 --- a/app/services/packages/nuget/update_package_from_metadata_service.rb +++ b/app/services/packages/nuget/update_package_from_metadata_service.rb @@ -54,13 +54,15 @@ def process_package_update update_linked_package end - update_package(target_package) + build_infos = package_to_destroy&.build_infos || [] + + update_package(target_package, build_infos) ::Packages::UpdatePackageFileService.new(@package_file, package_id: target_package.id, file_name: package_filename) .execute package_to_destroy&.destroy! end - def update_package(package) + def update_package(package, build_infos) return if symbol_package? ::Packages::Nuget::SyncMetadatumService @@ -71,6 +73,7 @@ def update_package(package) .new(package, package_tags) .execute + package.build_infos << build_infos if build_infos.any? rescue StandardError => e raise InvalidMetadataError, e.message end @@ -81,18 +84,6 @@ def valid_metadata? fields.all?(&:present?) end - def link_to_existing_package - package_to_destroy = @package_file.package - # Updating package_id updates the path where the file is stored. - # We must pass the file again so that CarrierWave can handle the update - @package_file.update!( - package_id: existing_package.id, - file: @package_file.file - ) - package_to_destroy.destroy! - existing_package - end - def update_linked_package @package_file.package.update!( name: package_name, @@ -106,12 +97,15 @@ def update_linked_package end def existing_package - @package_file.project.packages - .nuget - .with_name(package_name) - .with_version(package_version) - .not_pending_destruction - .first + ::Packages::Nuget::PackageFinder + .new( + nil, + @package_file.project, + package_name: package_name, + package_version: package_version + ) + .execute + .first end strong_memoize_attr :existing_package diff --git a/spec/factories/packages/packages.rb b/spec/factories/packages/packages.rb index caec7580e465a..dc16b4d75f5e3 100644 --- a/spec/factories/packages/packages.rb +++ b/spec/factories/packages/packages.rb @@ -214,6 +214,12 @@ create :package_file, :snupkg, package: package, file_name: "#{package.name}.#{package.version}.snupkg" end end + + trait :with_build do + after :create do |package| + create(:package_build_info, package: package) + end + end end factory :pypi_package do diff --git a/spec/models/packages/build_info_spec.rb b/spec/models/packages/build_info_spec.rb index db8ac605d7262..9bb8062005a6a 100644 --- a/spec/models/packages/build_info_spec.rb +++ b/spec/models/packages/build_info_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Packages::BuildInfo, type: :model do +RSpec.describe Packages::BuildInfo, type: :model, feature_category: :package_registry do describe 'relationships' do it { is_expected.to belong_to(:package) } it { is_expected.to belong_to(:pipeline) } diff --git a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb index 0459588bf8d8b..6f1d285a97b4e 100644 --- a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb +++ b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_redis_shared_state, feature_category: :package_registry do include ExclusiveLeaseHelpers - let!(:package) { create(:nuget_package, :processing, :with_symbol_package) } + let!(:package) { create(:nuget_package, :processing, :with_symbol_package, :with_build) } let(:package_file) { package.package_files.first } let(:service) { described_class.new(package_file) } let(:package_name) { 'DummyProject.DummyPackage' } @@ -101,6 +101,7 @@ .and change { Packages::DependencyLink.count }.by(0) .and change { Packages::Nuget::DependencyLinkMetadatum.count }.by(0) .and change { ::Packages::Nuget::Metadatum.count }.by(1) + .and change { existing_package.build_infos.count }.by(1) expect(package_file.reload.file_name).to eq(package_file_name) expect(package_file.package).to eq(existing_package) end -- GitLab