diff --git a/app/graphql/types/packages/conan/file_metadatum_type.rb b/app/graphql/types/packages/conan/file_metadatum_type.rb index fcc538ab24545c84c268a48e70dc31b1840eeefe..480fcec8a5ba52ac1cfef13fcbb2f5b7af43400e 100644 --- a/app/graphql/types/packages/conan/file_metadatum_type.rb +++ b/app/graphql/types/packages/conan/file_metadatum_type.rb @@ -14,7 +14,8 @@ class FileMetadatumType < BaseObject field :conan_file_type, ::Types::Packages::Conan::MetadatumFileTypeEnum, null: false, description: 'Type of the Conan file.' field :conan_package_reference, GraphQL::Types::String, null: true, - description: 'Reference of the Conan package.' + description: 'Reference of the Conan package.', + method: :package_reference_value field :id, ::Types::GlobalIDType[::Packages::Conan::FileMetadatum], null: false, description: 'ID of the metadatum.' field :package_revision, GraphQL::Types::String, null: true, description: 'Revision of the package.', diff --git a/app/models/packages/conan/file_metadatum.rb b/app/models/packages/conan/file_metadatum.rb index 79aed19b185e7fb3fcd7c59e4e3bf6290b73029e..c8de1df5effb242a19590576f20141ec0997bdf4 100644 --- a/app/models/packages/conan/file_metadatum.rb +++ b/app/models/packages/conan/file_metadatum.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Packages::Conan::FileMetadatum < ApplicationRecord + ignore_column :conan_package_reference, remove_with: '18.0', remove_after: '2025-04-17' + belongs_to :package_file, inverse_of: :conan_file_metadatum belongs_to :recipe_revision, inverse_of: :file_metadata, class_name: 'Packages::Conan::RecipeRevision' belongs_to :package_revision, inverse_of: :file_metadata, class_name: 'Packages::Conan::PackageRevision' @@ -9,10 +11,8 @@ class Packages::Conan::FileMetadatum < ApplicationRecord DEFAULT_REVISION = '0' validates :package_file, presence: true - validates :conan_package_reference, absence: true, if: :recipe_file? - validates :conan_package_reference, format: { with: Gitlab::Regex.conan_package_reference_regex }, if: :package_file? validates :package_reference, absence: true, if: :recipe_file? - validates :package_reference, presence: true, if: :package_file?, on: :create + validates :package_reference, presence: true, if: :package_file? validate :conan_package_type # package_revision is not supported yet validates :package_revision, absence: true @@ -34,6 +34,10 @@ def package_revision_value DEFAULT_REVISION end + def package_reference_value + package_reference&.reference + end + private def conan_package_type diff --git a/app/models/packages/conan/package_reference.rb b/app/models/packages/conan/package_reference.rb index f9586a8fb06a0bed99a05ba350024d1c8dd4bec5..4cd8754b9a6f65378966b42866fb1dedad57ec78 100644 --- a/app/models/packages/conan/package_reference.rb +++ b/app/models/packages/conan/package_reference.rb @@ -20,7 +20,8 @@ class PackageReference < ApplicationRecord has_many :file_metadata, inverse_of: :package_reference, class_name: 'Packages::Conan::FileMetadatum' validates :package, :project, presence: true - validates :reference, presence: true, bytesize: { maximum: -> { REFERENCE_LENGTH_MAX } } + validates :reference, presence: true, format: { with: Gitlab::Regex.conan_package_reference_regex }, + bytesize: { maximum: -> { REFERENCE_LENGTH_MAX } } validates :reference, uniqueness: { scope: %i[package_id recipe_revision_id] }, on: %i[create update] validates :info, json_schema: { filename: 'conan_package_info', detail_errors: true } diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index 193ffe0534c9223ae2cf662a5f3e6a995b4975e6..f0e76a07e623a141871ac0ad9cb507d35fcff5fd 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -94,9 +94,9 @@ class Packages::PackageFile < ApplicationRecord where('EXISTS (?)', file_metadata.select(1)) end - scope :with_conan_package_reference, ->(conan_package_reference) do - joins(:conan_file_metadatum) - .where(packages_conan_file_metadata: { conan_package_reference: conan_package_reference }) + scope :with_conan_package_reference, ->(reference) do + joins(conan_file_metadatum: :package_reference) + .where(packages_conan_package_references: { reference: reference }) end scope :with_conan_recipe_revision, ->(recipe_revision) do diff --git a/app/presenters/packages/conan/package_presenter.rb b/app/presenters/packages/conan/package_presenter.rb index ecb32a52360019370330b152c505e40039e41cf9..709876176109e12a375843268ef92cf141614935 100644 --- a/app/presenters/packages/conan/package_presenter.rb +++ b/app/presenters/packages/conan/package_presenter.rb @@ -36,7 +36,7 @@ def package_urls next unless package_file.conan_file_metadatum.package_file? && matching_reference?(package_file) options = url_options(package_file).merge( - conan_package_reference: package_file.conan_file_metadatum.conan_package_reference, + conan_package_reference: package_file.conan_file_metadatum.package_reference_value, package_revision: package_file.conan_file_metadatum.package_revision_value ) @@ -85,7 +85,7 @@ def package_files strong_memoize_attr :package_files def matching_reference?(package_file) - package_file.conan_file_metadatum.conan_package_reference == conan_package_reference + package_file.conan_file_metadatum.package_reference_value == conan_package_reference end def conan_package_reference diff --git a/app/services/packages/conan/create_package_file_service.rb b/app/services/packages/conan/create_package_file_service.rb index cedcf3676f295049e48554607bc2a1ef00379473..5e9f4cc216c8cf971d9f4f719b089edc74a438d8 100644 --- a/app/services/packages/conan/create_package_file_service.rb +++ b/app/services/packages/conan/create_package_file_service.rb @@ -19,7 +19,6 @@ def execute file_sha1: params['file.sha1'], file_md5: params['file.md5'], conan_file_metadatum_attributes: { - conan_package_reference: params[:conan_package_reference], conan_file_type: params[:conan_file_type], package_reference_id: package_reference_id } diff --git a/ee/spec/requests/api/conan/v1/project_packages_spec.rb b/ee/spec/requests/api/conan/v1/project_packages_spec.rb index bc54f99e597e6bbb37cd12c0f9b7cbec4d4a199c..499a30e2f91174b51ecab2771a225b7f1831af1d 100644 --- a/ee/spec/requests/api/conan/v1/project_packages_spec.rb +++ b/ee/spec/requests/api/conan/v1/project_packages_spec.rb @@ -19,7 +19,7 @@ "/:package_channel/:recipe_revision/package/:conan_package_reference/:package_revision/:file_name" do let(:url) do "/projects/#{project.id}/packages/conan/v1/files/#{package.conan_recipe_path}" \ - "/#{metadata.recipe_revision_value}/package/#{metadata.conan_package_reference}" \ + "/#{metadata.recipe_revision_value}/package/#{metadata.package_reference_value}" \ "/#{metadata.package_revision_value}/#{package_file.file_name}" end diff --git a/spec/factories/packages/conan/file_metadata.rb b/spec/factories/packages/conan/file_metadata.rb index f445337f4a04b211f17d68d67fee3e48329a0f99..14a6ab146452c6410c5ab1a97ae0c1b8dc7842e0 100644 --- a/spec/factories/packages/conan/file_metadata.rb +++ b/spec/factories/packages/conan/file_metadata.rb @@ -13,7 +13,6 @@ package_file { association(:conan_package_file, :conan_package, without_loaded_metadatum: true) } conan_file_type { 'package_file' } package_reference { association(:conan_package_reference) } - conan_package_reference { package_reference.reference } end end end diff --git a/spec/finders/packages/conan/package_file_finder_spec.rb b/spec/finders/packages/conan/package_file_finder_spec.rb index 283a56696637202e03ac849bfeddb040a30925b5..d12b4dbdec89c067086a7952a83d21d29c20aea2 100644 --- a/spec/finders/packages/conan/package_file_finder_spec.rb +++ b/spec/finders/packages/conan/package_file_finder_spec.rb @@ -20,8 +20,10 @@ end context 'with conan_package_reference' do + let_it_be(:package_file) { package.package_files.find_by(file_name: 'conaninfo.txt') } + let(:params) do - { conan_package_reference: package_file.conan_file_metadatum.conan_package_reference } + { conan_package_reference: package_file.conan_file_metadatum.package_reference_value } end it { expect(subject).to eq(package_file) } @@ -65,7 +67,8 @@ end it 'returns the correct package file' do - expect(package.package_files.last).to eq(recent_package_file_pending_destruction) + # Verify the pending_destruction file is indeed the last one, as only the last file is returned + expect(package.reload.package_files.last).to eq(recent_package_file_pending_destruction) expect(subject).to eq(package_file) end diff --git a/spec/models/packages/conan/file_metadatum_spec.rb b/spec/models/packages/conan/file_metadatum_spec.rb index e882f9c6027e20e57150776e7891508317e031f9..fc83ce0c3ede1d8d59bb0d98670cc6ecf0d6f0d5 100644 --- a/spec/models/packages/conan/file_metadatum_spec.rb +++ b/spec/models/packages/conan/file_metadatum_spec.rb @@ -28,46 +28,6 @@ it { is_expected.to validate_presence_of(:package_file) } it { is_expected.to validate_absence_of(:package_revision) } - describe '#conan_package_reference' do - context 'recipe file' do - let_it_be(:conan_file_metadatum) { build(:conan_file_metadatum, :recipe_file, package_file: package_file) } - - it 'is valid with empty value' do - conan_file_metadatum.conan_package_reference = nil - - expect(conan_file_metadatum).to be_valid - end - - it 'is invalid with value' do - conan_file_metadatum.conan_package_reference = '123456789' - - expect(conan_file_metadatum).to be_invalid - end - end - - context 'package file' do - let_it_be(:conan_file_metadatum) { build(:conan_file_metadatum, :package_file, package_file: package_file) } - - it 'is valid with acceptable value' do - conan_file_metadatum.conan_package_reference = '123456asdf' - - expect(conan_file_metadatum).to be_valid - end - - it 'is invalid with invalid value' do - conan_file_metadatum.conan_package_reference = 'foo@bar' - - expect(conan_file_metadatum).to be_invalid - end - - it 'is invalid when nil' do - conan_file_metadatum.conan_package_reference = nil - - expect(conan_file_metadatum).to be_invalid - end - end - end - describe '#package_reference' do let_it_be(:package_reference) { build(:conan_package_reference) } @@ -88,38 +48,16 @@ end context 'package file' do - context 'on create' do - let(:conan_file_metadatum) { build(:conan_file_metadatum, :package_file, package_file: package_file) } - - it 'is valid when package_reference is present' do - conan_file_metadatum.package_reference = package_reference - - expect(conan_file_metadatum).to be_valid - end - - it 'is invalid when package_reference is absent' do - conan_file_metadatum.package_reference = nil + let(:conan_file_metadatum) { build(:conan_file_metadatum, :package_file, package_file: package_file) } - expect(conan_file_metadatum).to be_invalid - end + it 'is valid when package_reference is present' do + expect(conan_file_metadatum).to be_valid end - context 'on update' do - let_it_be_with_reload(:existing_metadatum) do - create(:conan_file_metadatum, :package_file, package_file: package_file) - end - - it 'is valid when package_reference is absent' do - existing_metadatum.package_reference = nil - - expect(existing_metadatum).to be_valid - end - - it 'is valid when package_reference is present' do - existing_metadatum.package_reference = package_reference + it 'is invalid when package_reference is absent' do + conan_file_metadatum.package_reference = nil - expect(existing_metadatum).to be_valid - end + expect(conan_file_metadatum).to be_invalid end end end @@ -176,4 +114,26 @@ end end end + + describe '#package_reference_value' do + let(:package_reference) { build(:conan_package_reference) } + + subject { conan_file_metadatum.package_reference_value } + + context 'when package_reference is present' do + let(:conan_file_metadatum) do + build(:conan_file_metadatum, :package_file, package_file: package_file, package_reference: package_reference) + end + + it { is_expected.to eq(package_reference.reference) } + end + + context 'when package_reference is nil' do + let(:conan_file_metadatum) do + build(:conan_file_metadatum, :recipe_file, package_file: package_file) + end + + it { is_expected.to be_nil } + end + end end diff --git a/spec/models/packages/conan/package_reference_spec.rb b/spec/models/packages/conan/package_reference_spec.rb index 9c42bb4c7a638eecf8268bb124ed284ecf0c3a19..6ad31a00f495f14b5e66a930ee6a1bd1282b8cec 100644 --- a/spec/models/packages/conan/package_reference_spec.rb +++ b/spec/models/packages/conan/package_reference_spec.rb @@ -97,6 +97,9 @@ expect(package_reference).to be_valid end end + + it { is_expected.to allow_value('abc123').for(:reference) } + it { is_expected.not_to allow_value('abc-123').for(:reference) } end context 'on info' do diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index b858c6e3493604fdd00d5c180e652882dff88194..c5917dab544e9060d746765b32ec42f3e5e74afa 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -165,7 +165,7 @@ subject { described_class.with_conan_package_reference(reference) } context 'with existing reference' do - let(:reference) { matching_package_file.conan_file_metadatum.conan_package_reference } + let(:reference) { matching_package_file.conan_file_metadatum.package_reference.reference } it 'returns package files with matching reference' do expect(subject).to contain_exactly(matching_package_file) diff --git a/spec/requests/api/conan/v1/instance_packages_spec.rb b/spec/requests/api/conan/v1/instance_packages_spec.rb index 5dcea2647fb608e4d8520f4dc1849bc5000e4e0b..8520ee95b4f0976124022ee8ac8d4e968c5cc656 100644 --- a/spec/requests/api/conan/v1/instance_packages_spec.rb +++ b/spec/requests/api/conan/v1/instance_packages_spec.rb @@ -144,7 +144,7 @@ '/:recipe_revision/package/:conan_package_reference/:package_revision/:file_name' do subject(:request) do get api("/packages/conan/v1/files/#{recipe_path}/#{package_file_metadata.recipe_revision_value}/package" \ - "/#{package_file_metadata.conan_package_reference}/#{package_file_metadata.package_revision_value}" \ + "/#{package_file_metadata.package_reference_value}/#{package_file_metadata.package_revision_value}" \ "/#{package_file.file_name}"), headers: headers end diff --git a/spec/requests/api/conan/v1/project_packages_spec.rb b/spec/requests/api/conan/v1/project_packages_spec.rb index b28d310a643f82c007e67e63c7195d0727f5556e..991f70ea7274169c98e4338a326601674eb26a2e 100644 --- a/spec/requests/api/conan/v1/project_packages_spec.rb +++ b/spec/requests/api/conan/v1/project_packages_spec.rb @@ -170,7 +170,7 @@ '/:package_channel/:recipe_revision/package/:conan_package_reference/:package_revision/:file_name' do let(:url) do "/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}" \ - "/#{package_file_metadata.recipe_revision_value}/package/#{package_file_metadata.conan_package_reference}" \ + "/#{package_file_metadata.recipe_revision_value}/package/#{package_file_metadata.package_reference_value}" \ "/#{package_file_metadata.package_revision_value}/#{package_file.file_name}" end diff --git a/spec/requests/api/graphql/packages/conan_spec.rb b/spec/requests/api/graphql/packages/conan_spec.rb index b29195a3fb6504229d2ac1b9e91c78da79e719cf..38c4ae10c7f0acac10d294ee3457ab6d2d393ece 100644 --- a/spec/requests/api/graphql/packages/conan_spec.rb +++ b/spec/requests/api/graphql/packages/conan_spec.rb @@ -46,7 +46,7 @@ expect(first_file_response_metadata).to match( a_graphql_entity_for( first_file.conan_file_metadatum, - :conan_package_reference, + conan_package_reference: first_file.conan_file_metadatum.package_reference_value, package_revision: first_file.conan_file_metadatum.package_revision_value, recipe_revision: first_file.conan_file_metadatum.recipe_revision_value, conan_file_type: first_file.conan_file_metadatum.conan_file_type.upcase diff --git a/spec/services/packages/conan/create_package_file_service_spec.rb b/spec/services/packages/conan/create_package_file_service_spec.rb index 0edaf2acc2796ed8b5a5d654840b7c0a79914699..8d19f406ead297737033dbd98a67e4dc0639e23e 100644 --- a/spec/services/packages/conan/create_package_file_service_spec.rb +++ b/spec/services/packages/conan/create_package_file_service_spec.rb @@ -43,7 +43,6 @@ expect(package_file.conan_file_metadatum.package_revision_value).to eq('0') expect(package_file.conan_file_metadatum.recipe_revision_id).to be_nil expect(package_file.conan_file_metadatum.package_revision_id).to be_nil - expect(package_file.conan_file_metadatum.conan_package_reference).to eq(conan_package_reference) expect(package_file.conan_file_metadatum.package_reference.reference).to eq(conan_package_reference) expect(package_file.conan_file_metadatum.conan_file_type).to eq('package_file') expect(package_file.file.read).to eq('content') @@ -82,7 +81,6 @@ expect(package_file.conan_file_metadatum.package_revision_value).to be_nil expect(package_file.conan_file_metadatum.recipe_revision_id).to be_nil expect(package_file.conan_file_metadatum.package_revision_id).to be_nil - expect(package_file.conan_file_metadatum.conan_package_reference).to be_nil expect(package_file.conan_file_metadatum.package_reference).to be_nil expect(package_file.conan_file_metadatum.conan_file_type).to eq('recipe_file') expect(package_file.file.read).to eq('content') @@ -205,8 +203,7 @@ response = subject expect(response).to be_error - expect(response.message).to include('Validation failed: ' \ - 'Conan file metadatum conan package reference is invalid') + expect(response.message).to include('Validation failed: Reference is invalid') expect(response.reason).to eq(:invalid_package_file) expect { response }.not_to change { Packages::PackageFile.count } expect { response }.not_to change { Packages::Conan::FileMetadatum.count }