Merge branch 'sk/216910-validate-package-type' into 'master'

Validate package types in package metadatum models

See merge request gitlab-org/gitlab!32091

changelogs/unreleased/sk-216910-validate-package-type.yml

+---
+title: Validate package types in package metadatum models
+merge_request: 32091
+author: Sashi Kumar
+type: other

ee/app/models/packages/conan/file_metadatum.rb

@@ -14,10 +14,19 @@ class Packages::Conan::FileMetadatum < ApplicationRecord
 
   validates :conan_package_reference, absence: true, if: :recipe_file?
   validates :conan_package_reference, format: { with: Gitlab::Regex.conan_package_reference_regex }, if: :package_file?
+  validate :conan_package_type
 
   enum conan_file_type: { recipe_file: 1, package_file: 2 }
 
   RECIPE_FILES = %w[conanfile.py conanmanifest.txt].freeze
   PACKAGE_FILES = %w[conaninfo.txt conanmanifest.txt conan_package.tgz].freeze
   PACKAGE_BINARY = 'conan_package.tgz'
+
+  private
+
+  def conan_package_type
+    unless package_file&.package&.conan?
+      errors.add(:base, _('Package type must be Conan'))
+    end
+  end
 end

ee/app/models/packages/conan/metadatum.rb

@@ -34,7 +34,7 @@ class Packages::Conan::Metadatum < ApplicationRecord
   private
 
   def conan_package_type
-    unless package && package.conan?
+    unless package&.conan?
       errors.add(:base, _('Package type must be Conan'))
     end
   end

ee/app/models/packages/maven/metadatum.rb

@@ -21,7 +21,7 @@ class Packages::Maven::Metadatum < ApplicationRecord
   private
 
   def maven_package_type
-    unless package && package.maven?
+    unless package&.maven?
       errors.add(:base, _('Package type must be Maven'))
     end
   end

ee/app/models/packages/pypi/metadatum.rb

@@ -6,4 +6,14 @@ class Packages::Pypi::Metadatum < ApplicationRecord
   belongs_to :package, -> { where(package_type: :pypi) }, inverse_of: :pypi_metadatum
 
   validates :package, presence: true
+
+  validate :pypi_package_type
+
+  private
+
+  def pypi_package_type
+    unless package&.pypi?
+      errors.add(:base, _('Package type must be PyPi'))
+    end
+  end
 end

ee/spec/factories/packages.rb

@@ -76,6 +76,10 @@ FactoryBot.define do
     factory :conan_package do
       conan_metadatum
 
+      transient do
+        without_package_files { false }
+      end
+
       after :build do |package|
         package.conan_metadatum.package_username = Packages::Conan::Metadatum.package_username_from(
           full_path: package.project.full_path
@@ -86,12 +90,14 @@ FactoryBot.define do
       version { '1.0.0' }
       package_type { :conan }
 
-      after :create do |package|
-        create :conan_package_file, :conan_recipe_file, package: package
-        create :conan_package_file, :conan_recipe_manifest, package: package
-        create :conan_package_file, :conan_package_info, package: package
-        create :conan_package_file, :conan_package_manifest, package: package
-        create :conan_package_file, :conan_package, package: package
+      after :create do |package, evaluator|
+        unless evaluator.without_package_files
+          create :conan_package_file, :conan_recipe_file, package: package
+          create :conan_package_file, :conan_recipe_manifest, package: package
+          create :conan_package_file, :conan_package_info, package: package
+          create :conan_package_file, :conan_package_manifest, package: package
+          create :conan_package_file, :conan_package, package: package
+        end
       end
 
       trait(:without_loaded_metadatum) do
@@ -107,9 +113,17 @@ FactoryBot.define do
     package
 
     factory :conan_package_file do
+      package { create(:conan_package, without_package_files: true) }
+
+      transient do
+        without_loaded_metadatum { false }
+      end
+
       trait(:conan_recipe_file) do
-        after :create do |package_file|
-          create :conan_file_metadatum, :recipe_file, package_file: package_file
+        after :create do |package_file, evaluator|
+          unless evaluator.without_loaded_metadatum
+            create :conan_file_metadatum, :recipe_file, package_file: package_file
+          end
         end
 
         file { fixture_file_upload('ee/spec/fixtures/conan/recipe_files/conanfile.py') }
@@ -120,8 +134,10 @@ FactoryBot.define do
       end
 
       trait(:conan_recipe_manifest) do
-        after :create do |package_file|
-          create :conan_file_metadatum, :recipe_file, package_file: package_file
+        after :create do |package_file, evaluator|
+          unless evaluator.without_loaded_metadatum
+            create :conan_file_metadatum, :recipe_file, package_file: package_file
+          end
         end
 
         file { fixture_file_upload('ee/spec/fixtures/conan/recipe_files/conanmanifest.txt') }
@@ -132,8 +148,10 @@ FactoryBot.define do
       end
 
       trait(:conan_package_manifest) do
-        after :create do |package_file|
-          create :conan_file_metadatum, :package_file, package_file: package_file
+        after :create do |package_file, evaluator|
+          unless evaluator.without_loaded_metadatum
+            create :conan_file_metadatum, :package_file, package_file: package_file
+          end
         end
 
         file { fixture_file_upload('ee/spec/fixtures/conan/package_files/conanmanifest.txt') }
@@ -144,8 +162,10 @@ FactoryBot.define do
       end
 
       trait(:conan_package_info) do
-        after :create do |package_file|
-          create :conan_file_metadatum, :package_file, package_file: package_file
+        after :create do |package_file, evaluator|
+          unless evaluator.without_loaded_metadatum
+            create :conan_file_metadatum, :package_file, package_file: package_file
+          end
         end
 
         file { fixture_file_upload('ee/spec/fixtures/conan/package_files/conaninfo.txt') }
@@ -156,8 +176,10 @@ FactoryBot.define do
       end
 
       trait(:conan_package) do
-        after :create do |package_file|
-          create :conan_file_metadatum, :package_file, package_file: package_file
+        after :create do |package_file, evaluator|
+          unless evaluator.without_loaded_metadatum
+            create :conan_file_metadatum, :package_file, package_file: package_file
+          end
         end
 
         file { fixture_file_upload('ee/spec/fixtures/conan/package_files/conan_package.tgz') }
@@ -238,7 +260,7 @@ FactoryBot.define do
   end
 
   factory :conan_metadatum, class: 'Packages::Conan::Metadatum' do
-    association :package, factory: [:conan_package, :without_loaded_metadatum]
+    association :package, factory: [:conan_package, :without_loaded_metadatum], without_package_files: true
     package_username { 'username' }
     package_channel { 'stable' }
   end
@@ -257,7 +279,7 @@ FactoryBot.define do
   end
 
   factory :conan_file_metadatum, class: 'Packages::Conan::FileMetadatum' do
-    package_file
+    package_file { create(:conan_package_file, :conan_recipe_file, without_loaded_metadatum: true) }
     recipe_revision { '0' }
 
     trait(:recipe_file) do
@@ -265,6 +287,7 @@ FactoryBot.define do
     end
 
     trait(:package_file) do
+      package_file { create(:conan_package_file, :conan_package, without_loaded_metadatum: true) }
       conan_file_type { 'package_file' }
       package_revision { '0' }
       conan_package_reference { '123456789' }

ee/spec/models/packages/conan/file_metadatum_spec.rb

@@ -8,11 +8,7 @@ RSpec.describe Packages::Conan::FileMetadatum, type: :model do
   end
 
   describe 'validations' do
-    let(:package_file) do
-      create(:package_file,
-             file: fixture_file_upload('ee/spec/fixtures/conan/recipe_files/conanfile.py'),
-             file_name: 'conanfile.py')
-    end
+    let(:package_file) { create(:conan_package_file, :conan_recipe_file) }
 
     it { is_expected.to validate_presence_of(:package_file) }
     it { is_expected.to validate_presence_of(:recipe_revision) }
@@ -95,5 +91,16 @@ RSpec.describe Packages::Conan::FileMetadatum, type: :model do
         end
       end
     end
+
+    describe '#conan_package_type' do
+      it 'validates package of type conan' do
+        package = build('package')
+        package_file = build('package_file', package: package)
+        conan_file_metadatum = build('conan_file_metadatum', package_file: package_file)
+
+        expect(conan_file_metadatum).not_to be_valid
+        expect(conan_file_metadatum.errors.to_a).to contain_exactly('Package type must be Conan')
+      end
+    end
   end
 end

ee/spec/models/packages/conan/metadatum_spec.rb

@@ -46,12 +46,12 @@ RSpec.describe Packages::Conan::Metadatum, type: :model do
     end
 
     describe '#conan_package_type' do
-      it "will not allow a package with a different package_type" do
+      it 'will not allow a package with a different package_type' do
         package = build('package')
         conan_metadatum = build('conan_metadatum', package: package)
 
         expect(conan_metadatum).not_to be_valid
-        expect(conan_metadatum.errors.to_a).to include("Package type must be Conan")
+        expect(conan_metadatum.errors.to_a).to include('Package type must be Conan')
       end
     end
   end

ee/spec/models/packages/maven/metadatum_spec.rb

@@ -28,12 +28,12 @@ RSpec.describe Packages::Maven::Metadatum, type: :model do
     end
 
     describe '#maven_package_type' do
-      it "will not allow a package with a different package_type" do
+      it 'will not allow a package with a different package_type' do
         package = build('conan_package')
         maven_metadatum = build('maven_metadatum', package: package)
 
         expect(maven_metadatum).not_to be_valid
-        expect(maven_metadatum.errors.to_a).to include("Package type must be Maven")
+        expect(maven_metadatum.errors.to_a).to include('Package type must be Maven')
       end
     end
   end

ee/spec/models/packages/package_file_spec.rb

@@ -42,12 +42,12 @@ RSpec.describe Packages::PackageFile, type: :model do
 
   describe '.with_conan_package_reference' do
     let_it_be(:non_matching_package_file) { create(:package_file, :nuget) }
-    let_it_be(:package_file) { create(:conan_package_file, :conan_package) }
-    let_it_be(:reference) { package_file.conan_file_metadatum.conan_package_reference}
+    let_it_be(:metadatum) { create(:conan_file_metadatum, :package_file) }
+    let_it_be(:reference) { metadatum.conan_package_reference}
 
     it 'returns matching packages' do
       expect(described_class.with_conan_package_reference(reference))
-        .to eq([package_file])
+        .to eq([metadatum.package_file])
     end
   end
 

ee/spec/models/packages/pypi/metadatum_spec.rb

@@ -8,5 +8,15 @@ RSpec.describe Packages::Pypi::Metadatum, type: :model do
 
   describe 'validations' do
     it { is_expected.to validate_presence_of(:package) }
+
+    describe '#pypi_package_type' do
+      it 'will not allow a package with a different package_type' do
+        package = build('package')
+        pypi_metadatum = build('pypi_metadatum', package: package)
+
+        expect(pypi_metadatum).not_to be_valid
+        expect(pypi_metadatum.errors.to_a).to include('Package type must be PyPi')
+      end
+    end
   end
 end

locale/gitlab.pot

@@ -14971,6 +14971,9 @@ msgstr ""
 msgid "Package type must be NuGet"
 msgstr ""
 
+msgid "Package type must be PyPi"
+msgstr ""
+
 msgid "Package was removed"
 msgstr ""