Fix package file cleanup worker with PyPI files

PyPI files have an additional model validator that ensures that files
are unique (by file names) within the same package.
The cleanup worker should not hit such issues. It was updated to update
the file status *without* running validations nor callbacks.

Changelog: fixed

app/models/packages/package_file.rb

@@ -34,7 +34,7 @@ class Packages::PackageFile < ApplicationRecord
   validates :file, presence: true
   validates :file_name, presence: true
 
-  validates :file_name, uniqueness: { scope: :package }, if: -> { package&.pypi? }
+  validates :file_name, uniqueness: { scope: :package }, if: -> { !pending_destruction? && package&.pypi? }
 
   scope :recent, -> { order(id: :desc) }
   scope :limit_recent, ->(limit) { recent.limit(limit) }

app/workers/concerns/packages/cleanup_artifact_worker.rb

@@ -14,7 +14,7 @@ module Packages
 
         artifact.destroy!
       rescue StandardError
-        artifact&.error!
+        artifact&.update_column(:status, :error)
       end
 
       after_destroy
@@ -48,7 +48,7 @@ module Packages
           to_delete = next_item
 
           if to_delete
-            to_delete.processing!
+            to_delete.update_column(:status, :processing)
             log_cleanup_item(to_delete)
           end
 

spec/models/packages/package_file_spec.rb

@@ -23,6 +23,28 @@ RSpec.describe Packages::PackageFile, type: :model do
 
   describe 'validations' do
     it { is_expected.to validate_presence_of(:package) }
+
+    context 'with pypi package' do
+      let_it_be(:package) { create(:pypi_package) }
+
+      let(:package_file) { package.package_files.first }
+      let(:status) { :default }
+      let(:file) { fixture_file_upload('spec/fixtures/dk.png') }
+
+      subject { package.package_files.create!(file: file, file_name: package_file.file_name, status: status) }
+
+      it 'can not save a duplicated file' do
+        expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: File name has already been taken")
+      end
+
+      context 'with a pending destruction package duplicated file' do
+        let(:status) { :pending_destruction }
+
+        it 'can save it' do
+          expect { subject }.to change { package.package_files.count }.from(1).to(2)
+        end
+      end
+    end
   end
 
   context 'with package filenames' do

spec/workers/packages/cleanup_package_file_worker_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Packages::CleanupPackageFileWorker do
-  let_it_be(:package) { create(:package) }
+  let_it_be_with_reload(:package) { create(:package) }
 
   let(:worker) { described_class.new }
 
@@ -23,7 +23,23 @@ RSpec.describe Packages::CleanupPackageFileWorker do
         expect(worker).to receive(:log_extra_metadata_on_done).twice
 
         expect { subject }.to change { Packages::PackageFile.count }.by(-1)
-          .and not_change { Packages::Package.count }
+                                .and not_change { Packages::Package.count }
+        expect { package_file2.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      context 'with a duplicated PyPI package file' do
+        let_it_be_with_reload(:duplicated_package_file) { create(:package_file, package: package) }
+
+        before do
+          package.update!(package_type: :pypi, version: '1.2.3')
+          duplicated_package_file.update_column(:file_name, package_file2.file_name)
+        end
+
+        it 'deletes one of the duplicates' do
+          expect { subject }.to change { Packages::PackageFile.count }.by(-1)
+                                  .and not_change { Packages::Package.count }
+          expect { package_file2.reload }.to raise_error(ActiveRecord::RecordNotFound)
+        end
       end
     end