diff --git a/app/models/packages/conan/file_metadatum.rb b/app/models/packages/conan/file_metadatum.rb
index 39dbbcf9d091d48718348d502f6d1e8cd5228a2f..fe7ec6fee1766faa12b0f2611aa7562884032fec 100644
--- a/app/models/packages/conan/file_metadatum.rb
+++ b/app/models/packages/conan/file_metadatum.rb
@@ -16,6 +16,8 @@ class Packages::Conan::FileMetadatum < ApplicationRecord
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
validate :conan_package_type
# recipe_revision and package_revision are not supported yet
validates :recipe_revision, absence: true
diff --git a/app/models/packages/conan/package_reference.rb b/app/models/packages/conan/package_reference.rb
index 99a07300bca7f3ef7128b4840ec4abfae0747a03..f9586a8fb06a0bed99a05ba350024d1c8dd4bec5 100644
--- a/app/models/packages/conan/package_reference.rb
+++ b/app/models/packages/conan/package_reference.rb
@@ -5,7 +5,7 @@ module Conan
class PackageReference < ApplicationRecord
include ShaAttribute
- REFERENCE_LENGTH_MAX = 20
+ REFERENCE_LENGTH_MAX = 40
MAX_INFO_SIZE = 20_000
sha_attribute :reference
@@ -20,11 +20,16 @@ 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 } },
- uniqueness: { scope: [:package_id, :recipe_revision_id] }
+ validates :reference, presence: true, 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 }
validate :ensure_info_size
+ def self.for_package_id_and_reference(package_id, reference)
+ where(package_id: package_id, reference: reference)
+ end
+
private
def ensure_info_size
diff --git a/app/models/packages/conan/package_revision.rb b/app/models/packages/conan/package_revision.rb
index 45e6fc3d64a6cbe2511e095fa7573ab70f7f77ce..5e7973419948e911ff07f913a83ad42af14345d8 100644
--- a/app/models/packages/conan/package_revision.rb
+++ b/app/models/packages/conan/package_revision.rb
@@ -5,7 +5,7 @@ module Conan
class PackageRevision < ApplicationRecord
include ShaAttribute
- REVISION_LENGTH_MAX = 20
+ REVISION_LENGTH_MAX = 40
sha_attribute :revision
diff --git a/app/models/packages/conan/recipe_revision.rb b/app/models/packages/conan/recipe_revision.rb
index fd677aba71eff553aaeada15f83267142a3baae9..3f5d915ad74b93369e009502b088955242215ebd 100644
--- a/app/models/packages/conan/recipe_revision.rb
+++ b/app/models/packages/conan/recipe_revision.rb
@@ -5,7 +5,7 @@ module Conan
class RecipeRevision < ApplicationRecord
include ShaAttribute
- REVISION_LENGTH_MAX = 20
+ REVISION_LENGTH_MAX = 40
sha_attribute :revision
diff --git a/app/services/packages/conan/create_package_file_service.rb b/app/services/packages/conan/create_package_file_service.rb
index 0ca3747a323e40d672217fc196c656721c2e42da..cedcf3676f295049e48554607bc2a1ef00379473 100644
--- a/app/services/packages/conan/create_package_file_service.rb
+++ b/app/services/packages/conan/create_package_file_service.rb
@@ -3,8 +3,6 @@
module Packages
module Conan
class CreatePackageFileService
- attr_reader :package, :file, :params
-
def initialize(package, file, params)
@package = package
@file = file
@@ -12,24 +10,42 @@ def initialize(package, file, params)
end
def execute
- package_file = package.package_files.build(
- file: file,
- size: params['file.size'],
- file_name: params[:file_name],
- 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]
- }
- )
-
- if params[:build].present?
- package_file.package_file_build_infos << package_file.package_file_build_infos.build(pipeline: params[:build].pipeline)
+ package_file = nil
+ ApplicationRecord.transaction do
+ package_file = package.package_files.build(
+ file: file,
+ size: params['file.size'],
+ file_name: params[:file_name],
+ 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
+ }
+ )
+
+ 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!
end
- package_file.save!
- package_file
+ ServiceResponse.success(payload: { package_file: package_file })
+ rescue ActiveRecord::RecordInvalid => e
+ ServiceResponse.error(message: e.message, reason: :invalid_package_file)
+ end
+
+ private
+
+ attr_reader :package, :file, :params
+
+ def package_reference_id
+ return unless params[:conan_package_reference].present?
+
+ package_reference_result = ::Packages::Conan::UpsertPackageReferenceService.new(package, params[:conan_package_reference]).execute!
+ package_reference_result[:package_reference_id]
end
end
end
diff --git a/app/services/packages/conan/upsert_package_reference_service.rb b/app/services/packages/conan/upsert_package_reference_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..59f61ba31daf86cefc34934ca9b930da49a7a2a0
--- /dev/null
+++ b/app/services/packages/conan/upsert_package_reference_service.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+module Packages
+ module Conan
+ class UpsertPackageReferenceService
+ include Gitlab::Utils::StrongMemoize
+
+ def initialize(package, package_reference_value)
+ @package = package
+ @package_reference_value = package_reference_value
+ end
+
+ def execute!
+ # We use a different validation context
+ # so that the uniqueness model validation on
+ # [reference, package_id, recipe_revision_id]
+ # is skipped.
+ package_reference.validate!(:upsert)
+
+ ServiceResponse.success(payload: { package_reference_id: upsert_package_reference[0]['id'] })
+ end
+
+ private
+
+ attr_reader :package, :package_reference_value
+
+ def package_reference
+ ::Packages::Conan::PackageReference.new(
+ package_id: package.id,
+ reference: package_reference_value,
+ project_id: package.project_id
+ )
+ end
+ strong_memoize_attr :package_reference
+
+ def upsert_package_reference
+ ::Packages::Conan::PackageReference
+ .upsert(
+ package_reference.attributes.slice('package_id', 'project_id', 'reference'),
+ unique_by: %i[package_id reference]
+ )
+ end
+ end
+ end
+end
diff --git a/db/migrate/20241105181558_add_uniq_index_package_reference.rb b/db/migrate/20241105181558_add_uniq_index_package_reference.rb
new file mode 100644
index 0000000000000000000000000000000000000000..290338e4127d624761d59a8a04c0e59aab200e11
--- /dev/null
+++ b/db/migrate/20241105181558_add_uniq_index_package_reference.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class AddUniqIndexPackageReference < Gitlab::Database::Migration[2.2]
+ milestone '17.7'
+
+ INDEX_NAME = 'uniq_index_pkg_refs_on_ref_and_pkg_id_when_rev_is_null'
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index :packages_conan_package_references,
+ [:package_id, :reference],
+ unique: true,
+ name: INDEX_NAME,
+ where: 'recipe_revision_id IS NULL'
+ end
+
+ def down
+ remove_concurrent_index_by_name :packages_conan_package_references, INDEX_NAME
+ end
+end
diff --git a/db/migrate/20241129142800_add_check_constraint_to_conan_file_metadata.rb b/db/migrate/20241129142800_add_check_constraint_to_conan_file_metadata.rb
new file mode 100644
index 0000000000000000000000000000000000000000..716fa13c742e56cb0698a9875c1f6093e3e7da7a
--- /dev/null
+++ b/db/migrate/20241129142800_add_check_constraint_to_conan_file_metadata.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class AddCheckConstraintToConanFileMetadata < Gitlab::Database::Migration[2.2]
+ milestone '17.7'
+
+ disable_ddl_transaction!
+
+ CONSTRAINT_NAME = 'check_conan_file_metadata_ref_null_for_recipe_files'
+ RECIPE_FILE = 1
+
+ def up
+ add_check_constraint :packages_conan_file_metadata,
+ "NOT (conan_file_type = #{RECIPE_FILE} AND package_reference_id IS NOT NULL)", CONSTRAINT_NAME
+ end
+
+ def down
+ remove_check_constraint :packages_conan_file_metadata, CONSTRAINT_NAME
+ end
+end
diff --git a/db/schema_migrations/20241105181558 b/db/schema_migrations/20241105181558
new file mode 100644
index 0000000000000000000000000000000000000000..5a1518bb451d138c97add5b591b70b1dbb91b511
--- /dev/null
+++ b/db/schema_migrations/20241105181558
@@ -0,0 +1 @@
+75ba7fa0b8cbb9ffbbc131e72ef208c3bbc4a2165508d624f1827e881f900329
\ No newline at end of file
diff --git a/db/schema_migrations/20241129142800 b/db/schema_migrations/20241129142800
new file mode 100644
index 0000000000000000000000000000000000000000..5b1961807fb71c16dd17de61923f94a81e6ffe7c
--- /dev/null
+++ b/db/schema_migrations/20241129142800
@@ -0,0 +1 @@
+5fa507e11997131856fd0b9c2b0408e86bb20719c0896fd1966d83b856a1cb81
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 07e590b0ae0d309645887fb51563abbd67f6c08b..d0d698ce3a133495673f53ad33ef245d22d8caca 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -16156,7 +16156,8 @@ CREATE TABLE packages_conan_file_metadata (
conan_file_type smallint NOT NULL,
recipe_revision_id bigint,
package_revision_id bigint,
- package_reference_id bigint
+ package_reference_id bigint,
+ CONSTRAINT check_conan_file_metadata_ref_null_for_recipe_files CHECK ((NOT ((conan_file_type = 1) AND (package_reference_id IS NOT NULL))))
);
CREATE SEQUENCE packages_conan_file_metadata_id_seq
@@ -33564,6 +33565,8 @@ CREATE UNIQUE INDEX uniq_idx_subscription_seat_assignments_on_namespace_and_user
CREATE UNIQUE INDEX uniq_idx_user_add_on_assignments_on_add_on_purchase_and_user ON subscription_user_add_on_assignments USING btree (add_on_purchase_id, user_id);
+CREATE UNIQUE INDEX uniq_index_pkg_refs_on_ref_and_pkg_id_when_rev_is_null ON packages_conan_package_references USING btree (package_id, reference) WHERE (recipe_revision_id IS NULL);
+
CREATE UNIQUE INDEX uniq_pkgs_deb_grp_architectures_on_distribution_id_and_name ON packages_debian_group_architectures USING btree (distribution_id, name);
CREATE UNIQUE INDEX uniq_pkgs_deb_grp_components_on_distribution_id_and_name ON packages_debian_group_components USING btree (distribution_id, name);
diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb
index 435b2690184a2cbdddb8a6126a96d8b1a7357a58..9aa478756f737b76ab40361599f3a495e5126ebd 100644
--- a/lib/api/helpers/packages/conan/api_helpers.rb
+++ b/lib/api/helpers/packages/conan/api_helpers.rb
@@ -230,7 +230,12 @@ def upload_package_file(file_type)
track_push_package_event unless params[:file].empty_size?
- create_package_file_with_type(file_type, current_package)
+ service_response = create_package_file_with_type(file_type, current_package)
+ return unless service_response
+
+ bad_request!(service_response.message) if service_response.error?
+
+ service_response[:package_file]
rescue ObjectStorage::RemoteStoreError => e
Gitlab::ErrorTracking.track_exception(e, file_name: params[:file_name], project_id: project.id)
diff --git a/spec/factories/packages/conan/file_metadata.rb b/spec/factories/packages/conan/file_metadata.rb
index 89cd05bdd5ddaa508ba954a32e94cfeffae754e7..f445337f4a04b211f17d68d67fee3e48329a0f99 100644
--- a/spec/factories/packages/conan/file_metadata.rb
+++ b/spec/factories/packages/conan/file_metadata.rb
@@ -12,7 +12,8 @@
trait(:package_file) do
package_file { association(:conan_package_file, :conan_package, without_loaded_metadatum: true) }
conan_file_type { 'package_file' }
- conan_package_reference { '123456789' }
+ package_reference { association(:conan_package_reference) }
+ conan_package_reference { package_reference.reference }
end
end
end
diff --git a/spec/factories/packages/conan/package_references.rb b/spec/factories/packages/conan/package_references.rb
index 791e6818a5bff15f53750e10c0c6e8219c8e6c39..792cd43b097db614c66252b26296c35214138186 100644
--- a/spec/factories/packages/conan/package_references.rb
+++ b/spec/factories/packages/conan/package_references.rb
@@ -4,7 +4,7 @@
factory :conan_package_reference, class: 'Packages::Conan::PackageReference' do
package { association(:conan_package) }
project { association(:project) }
- recipe_revision { association(:conan_recipe_revision) }
+ recipe_revision { association(:conan_recipe_revision, package: package, project: project) }
info do
{
settings: { os: 'Linux', arch: 'x86_64' },
@@ -13,6 +13,6 @@
otherProperties: 'some_value'
}
end
- sequence(:reference) { |n| Digest::SHA1.digest(n.to_s) } # rubocop:disable Fips/SHA1 -- The conan registry is not FIPS compliant
+ sequence(:reference) { |n| Digest::SHA1.hexdigest(n.to_s) } # rubocop:disable Fips/SHA1 -- The conan registry is not FIPS compliant
end
end
diff --git a/spec/factories/packages/conan/package_revision.rb b/spec/factories/packages/conan/package_revision.rb
index 7c8a6bcdcda31eefa66c99faa6d8c5e47388d994..2b7135589f4d3a595d8e18d0875839f10e72e9bb 100644
--- a/spec/factories/packages/conan/package_revision.rb
+++ b/spec/factories/packages/conan/package_revision.rb
@@ -5,6 +5,6 @@
package { association(:conan_package) }
association :project
package_reference { association(:conan_package_reference) }
- sequence(:revision) { |n| Digest::SHA1.digest(n.to_s) } # rubocop:disable Fips/SHA1 -- The conan registry is not FIPS compliant
+ sequence(:revision) { |n| Digest::SHA1.hexdigest(n.to_s) } # rubocop:disable Fips/SHA1 -- The conan registry is not FIPS compliant
end
end
diff --git a/spec/factories/packages/conan/packages.rb b/spec/factories/packages/conan/packages.rb
index e5ad7d35621aeef389e2ccb161798a99796bb760..1f9d5ed85eeadd5b9b7f3c58450818695655b5e9 100644
--- a/spec/factories/packages/conan/packages.rb
+++ b/spec/factories/packages/conan/packages.rb
@@ -10,6 +10,14 @@
transient do
without_package_files { false }
+ package_references { ['1234567890abcdef1234567890abcdef12345678'] }
+ end
+
+ conan_package_references do
+ package_references.map do |ref|
+ association(:conan_package_reference,
+ reference: ref, package: instance)
+ end
end
after :build do |package|
@@ -20,10 +28,17 @@
after :create do |package, evaluator|
unless evaluator.without_package_files
- %i[conan_recipe_file conan_recipe_manifest conan_package_info conan_package_manifest
- conan_package].each do |file|
+ %i[conan_recipe_file conan_recipe_manifest].each do |file|
create :conan_package_file, file, package: package
end
+
+ package_file_traits = %i[conan_package_info conan_package_manifest conan_package]
+ package.conan_package_references.each do |reference|
+ package_file_traits.each do |file|
+ create :conan_package_file, file, package: package,
+ conan_package_reference: reference
+ end
+ end
end
end
diff --git a/spec/factories/packages/conan/recipe_revisions.rb b/spec/factories/packages/conan/recipe_revisions.rb
index 155558f460f7557c099ff7eb54f2573a9239be5c..a82a4bd798b8171cb1ae4339d468a4a5be7ac02a 100644
--- a/spec/factories/packages/conan/recipe_revisions.rb
+++ b/spec/factories/packages/conan/recipe_revisions.rb
@@ -4,6 +4,6 @@
factory :conan_recipe_revision, class: 'Packages::Conan::RecipeRevision' do
package { association(:conan_package) }
association :project
- sequence(:revision) { |n| Digest::SHA1.digest(n.to_s) } # rubocop:disable Fips/SHA1 -- The conan registry is not FIPS compliant
+ sequence(:revision) { |n| Digest::SHA1.hexdigest(n.to_s) } # rubocop:disable Fips/SHA1 -- The conan registry is not FIPS compliant
end
end
diff --git a/spec/factories/packages/package_files.rb b/spec/factories/packages/package_files.rb
index aa4bf58a663d3bb5f3b0e6c596255f2588501169..822fd80f3e254827862e0ac10e07434ee64335b6 100644
--- a/spec/factories/packages/package_files.rb
+++ b/spec/factories/packages/package_files.rb
@@ -25,6 +25,7 @@
transient do
without_loaded_metadatum { false }
+ conan_package_reference { package.conan_package_references.first }
end
trait(:conan_recipe_file) do
@@ -58,7 +59,8 @@
trait(:conan_package_manifest) do
after :create do |package_file, evaluator|
unless evaluator.without_loaded_metadatum
- create :conan_file_metadatum, :package_file, package_file: package_file
+ create :conan_file_metadatum, :package_file,
+ { package_file: package_file, package_reference: evaluator.conan_package_reference }.compact
end
end
@@ -72,7 +74,8 @@
trait(:conan_package_info) do
after :create do |package_file, evaluator|
unless evaluator.without_loaded_metadatum
- create :conan_file_metadatum, :package_file, package_file: package_file
+ create :conan_file_metadatum, :package_file,
+ { package_file: package_file, package_reference: evaluator.conan_package_reference }.compact
end
end
@@ -86,7 +89,8 @@
trait(:conan_package) do
after :create do |package_file, evaluator|
unless evaluator.without_loaded_metadatum
- create :conan_file_metadatum, :package_file, package_file: package_file
+ create :conan_file_metadatum, :package_file,
+ { package_file: package_file, package_reference: evaluator.conan_package_reference }.compact
end
end
diff --git a/spec/models/packages/conan/file_metadatum_spec.rb b/spec/models/packages/conan/file_metadatum_spec.rb
index efae7cc570ad7cbc1afd0653e3f378595cfbcc41..15d6f509e7ccbd7892685e9664836e1175c6812f 100644
--- a/spec/models/packages/conan/file_metadatum_spec.rb
+++ b/spec/models/packages/conan/file_metadatum_spec.rb
@@ -3,7 +3,7 @@
require 'spec_helper'
RSpec.describe Packages::Conan::FileMetadatum, type: :model do
- let_it_be(:package_file) { create(:conan_package_file, :conan_recipe_file) }
+ let_it_be(:package_file) { build(:conan_package_file) }
describe 'relationships' do
it { is_expected.to belong_to(:package_file) }
@@ -31,7 +31,7 @@
describe '#conan_package_reference' do
context 'recipe file' do
- let(:conan_file_metadatum) { build(:conan_file_metadatum, :recipe_file, package_file: package_file) }
+ 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
@@ -47,7 +47,7 @@
end
context 'package file' do
- let(:conan_file_metadatum) { build(:conan_file_metadatum, :package_file, package_file: package_file) }
+ 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'
@@ -69,6 +69,62 @@
end
end
+ describe '#package_reference' do
+ let_it_be(:package_reference) { build(:conan_package_reference) }
+
+ context 'recipe file' do
+ let(:conan_file_metadatum) { build(:conan_file_metadatum, :recipe_file, package_file: package_file) }
+
+ it 'is valid when package_reference is absent' do
+ conan_file_metadatum.package_reference = nil
+
+ expect(conan_file_metadatum).to be_valid
+ end
+
+ it 'is invalid when package_reference is present' do
+ conan_file_metadatum.package_reference = package_reference
+
+ expect(conan_file_metadatum).to be_invalid
+ end
+ 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
+
+ expect(conan_file_metadatum).to be_invalid
+ end
+ 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
+
+ expect(existing_metadatum).to be_valid
+ end
+ end
+ end
+ end
+
describe '#conan_package_type' do
it 'validates package of type conan' do
package = build('package')
diff --git a/spec/models/packages/conan/package_reference_spec.rb b/spec/models/packages/conan/package_reference_spec.rb
index a94b30531f8305ada5e8d91b98147f8ac133e0c8..3b8bff658068162835e2880b51d4241df9826d7d 100644
--- a/spec/models/packages/conan/package_reference_spec.rb
+++ b/spec/models/packages/conan/package_reference_spec.rb
@@ -33,10 +33,48 @@
it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:reference) }
- it do
- # ignore case, same revision string with different case are converted to same hexa binary
- is_expected.to validate_uniqueness_of(:reference).scoped_to([:package_id,
- :recipe_revision_id]).case_insensitive
+ describe 'uniqueness of reference' do
+ let_it_be(:conan_package) { create(:conan_package, without_package_files: true) }
+ let_it_be(:existing_reference) { create(:conan_package_reference, package: conan_package) }
+
+ context 'when recipe_revision_id is not nil' do
+ it 'validates uniqueness scoped to package_id and recipe_revision_id', :aggregate_failures do
+ duplicate_reference = build(:conan_package_reference, package_id: existing_reference.package_id,
+ recipe_revision_id: existing_reference.recipe_revision_id, reference: existing_reference.reference)
+
+ expect(duplicate_reference).not_to be_valid
+ expect(duplicate_reference.errors[:reference]).to include('has already been taken')
+ end
+
+ it 'is valid if the reference is unique' do
+ new_reference = build(:conan_package_reference, package_id: existing_reference.package_id,
+ recipe_revision_id: existing_reference.recipe_revision_id,
+ reference: Digest::SHA1.hexdigest('new_unique_reference')) # rubocop:disable Fips/SHA1 -- The conan registry is not FIPS compliant
+
+ expect(new_reference).to be_valid
+ end
+ end
+
+ context 'when recipe_revision_id is nil' do
+ let_it_be(:existing_nil_revision_reference) do
+ create(:conan_package_reference, package: conan_package, recipe_revision_id: nil)
+ end
+
+ it 'validates uniqueness scoped to package_id when both have nil recipe_revision_id', :aggregate_failures do
+ duplicate_reference = build(:conan_package_reference, package_id: existing_nil_revision_reference.package_id,
+ recipe_revision_id: nil, reference: existing_nil_revision_reference.reference)
+
+ expect(duplicate_reference).not_to be_valid
+ expect(duplicate_reference.errors[:reference]).to include('has already been taken')
+ end
+
+ it 'is valid if existing reference has a non-nil recipe_revision_id' do
+ duplicate_reference = build(:conan_package_reference, package_id: existing_reference.package_id,
+ recipe_revision_id: nil, reference: existing_reference.reference)
+
+ expect(duplicate_reference).to be_valid
+ end
+ end
end
context 'on reference' do
@@ -109,4 +147,20 @@
end
end
end
+
+ describe '.for_package_id_and_reference' do
+ let_it_be(:package_reference) { create(:conan_package_reference) }
+
+ let(:reference) { package_reference.reference }
+
+ subject { described_class.for_package_id_and_reference(package_reference.package_id, reference) }
+
+ it { is_expected.to contain_exactly(package_reference) }
+
+ context 'when no match is found' do
+ let(:reference) { 'non_existent_reference' }
+
+ it { is_expected.to be_empty }
+ end
+ end
end
diff --git a/spec/presenters/packages/conan/package_presenter_spec.rb b/spec/presenters/packages/conan/package_presenter_spec.rb
index 0fe810ef59caf1c772d27242e8b55ab667ae3fe1..71e10e0c2540a59a4f849dd59a5b8c380f5cc990 100644
--- a/spec/presenters/packages/conan/package_presenter_spec.rb
+++ b/spec/presenters/packages/conan/package_presenter_spec.rb
@@ -4,10 +4,11 @@
RSpec.describe ::Packages::Conan::PackagePresenter, feature_category: :package_registry do
let_it_be(:user) { create(:user) }
- let_it_be(:package) { create(:conan_package) }
+ let_it_be(:conan_package_reference) { '1234567890abcdef1234567890abcdef12345678' }
+ let_it_be(:alternative_reference) { '1111111111111111111111111111111111111111' }
+ let_it_be(:package) { create(:conan_package, package_references: [conan_package_reference, alternative_reference]) }
let_it_be(:project) { package.project }
let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package) }
- let_it_be(:conan_package_reference) { '123456789' }
let(:params) { { package_scope: :instance } }
let(:presenter) { described_class.new(package, user, project, params) }
@@ -143,18 +144,6 @@
end
context 'multiple packages with different references' do
- let(:info_file) { create(:conan_package_file, :conan_package_info, package: package) }
- let(:manifest_file) { create(:conan_package_file, :conan_package_manifest, package: package) }
- let(:package_file) { create(:conan_package_file, :conan_package, package: package) }
- let(:alternative_reference) { 'abcdefghi' }
-
- before do
- [info_file, manifest_file, package_file].each do |file|
- file.conan_file_metadatum.conan_package_reference = alternative_reference
- file.save!
- end
- end
-
it { is_expected.to eq(expected_result) }
context 'requesting the alternative reference' do
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 3668ebf9ffb92c715a8290adddbc262c5e5f721c..0edaf2acc2796ed8b5a5d654840b7c0a79914699 100644
--- a/spec/services/packages/conan/create_package_file_service_spec.rb
+++ b/spec/services/packages/conan/create_package_file_service_spec.rb
@@ -4,13 +4,14 @@
RSpec.describe Packages::Conan::CreatePackageFileService, feature_category: :package_registry do
include WorkhorseHelpers
- let_it_be(:package) { create(:conan_package) }
+ let_it_be(:package) { create(:conan_package, without_package_files: true) }
let_it_be(:user) { create(:user) }
- describe '#execute' do
+ describe '#execute', :aggregate_failures do
let(:file_name) { 'foo.tgz' }
+ let(:conan_package_reference) { '1234567890abcdef1234567890abcdef12345678' }
- subject { described_class.new(package, file, params).execute }
+ subject(:response) { described_class.new(package, file, params).execute }
shared_examples 'a valid package_file' do
let(:params) do
@@ -22,13 +23,16 @@
'file.type': 'txt',
recipe_revision: '0',
package_revision: '0',
- conan_package_reference: '123456789',
+ conan_package_reference: conan_package_reference,
conan_file_type: :package_file
}.with_indifferent_access
end
it 'creates a new package file' do
- package_file = subject
+ response = subject
+
+ expect(response).to be_success
+ package_file = response[:package_file]
expect(package_file).to be_valid
expect(package_file.file_name).to eq(file_name)
@@ -39,12 +43,15 @@
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('123456789')
+ 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')
end
- it_behaves_like 'assigns build to package file'
+ it_behaves_like 'assigns build to package file' do
+ subject(:package_file) { response[:package_file] }
+ end
end
shared_examples 'a valid recipe_file' do
@@ -61,7 +68,10 @@
end
it 'creates a new recipe file' do
- package_file = subject
+ response = subject
+
+ expect(response).to be_success
+ package_file = response[:package_file]
expect(package_file).to be_valid
expect(package_file.file_name).to eq(file_name)
@@ -73,14 +83,17 @@
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')
end
- it_behaves_like 'assigns build to package file'
+ it_behaves_like 'assigns build to package file' do
+ subject(:package_file) { response[:package_file] }
+ end
end
- context 'with temp file' do
+ shared_context 'with temp file setup' do
let!(:file) do
upload_path = ::Packages::PackageFileUploader.workhorse_local_upload_path
file_path = upload_path + '/' + file_name
@@ -94,6 +107,10 @@
before do
allow_any_instance_of(Packages::PackageFileUploader).to receive(:size).and_return(128)
end
+ end
+
+ context 'with temp file' do
+ include_context 'with temp file setup'
it_behaves_like 'a valid package_file'
it_behaves_like 'a valid recipe_file'
@@ -123,16 +140,77 @@
context 'file is missing' do
let(:file) { nil }
+
+ context 'with recipe_file' do
+ let(:params) do
+ {
+ file_name: file_name,
+ recipe_revision: '0',
+ conan_file_type: :recipe_file
+ }
+ end
+
+ it 'returns an error response' do
+ response = subject
+
+ expect(response).to be_error
+ expect(response.message).to eq('Validation failed: File can\'t be blank')
+ 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 }
+ end
+ end
+
+ context 'with package_file' do
+ let(:params) do
+ {
+ file_name: file_name,
+ package_revision: '0',
+ conan_file_type: :package_file,
+ conan_package_reference: conan_package_reference
+ }
+ end
+
+ it 'returns an error response' do
+ response = subject
+
+ expect(response).to be_error
+ expect(response.message).to eq('Validation failed: File can\'t be blank')
+ 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 }
+ expect { response }.not_to change { Packages::Conan::PackageReference.count }
+ end
+ end
+ end
+
+ context 'when an invalid conan package reference is provided' do
+ include_context 'with temp file setup'
+
let(:params) do
{
file_name: file_name,
+ 'file.md5': '12345',
+ 'file.sha1': '54321',
+ 'file.size': '128',
+ 'file.type': 'txt',
recipe_revision: '0',
- conan_file_type: :recipe_file
- }
+ package_revision: '0',
+ conan_package_reference: 'invalid_reference',
+ conan_file_type: :package_file
+ }.with_indifferent_access
end
- it 'raises an error' do
- expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
+ it 'returns an error response and does not create a package file' do
+ response = subject
+
+ expect(response).to be_error
+ expect(response.message).to include('Validation failed: ' \
+ 'Conan file metadatum conan package 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 }
+ expect { response }.not_to change { Packages::Conan::PackageReference.count }
end
end
end
diff --git a/spec/services/packages/conan/upsert_package_reference_service_spec.rb b/spec/services/packages/conan/upsert_package_reference_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..68e72868b2c4ea4c25782299e36c9674c60fd702
--- /dev/null
+++ b/spec/services/packages/conan/upsert_package_reference_service_spec.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Conan::UpsertPackageReferenceService, feature_category: :package_registry do
+ let_it_be(:package) { create(:conan_package, without_package_files: true, package_references: []) }
+ let_it_be(:conan_package_reference) { '1234567890abcdef1234567890abcdef12345678' }
+
+ describe '#execute!', :aggregate_failures do
+ subject(:response) { described_class.new(package, conan_package_reference).execute! }
+
+ context 'when the package reference doesn\'t exist' do
+ it 'creates the package reference' do
+ expect { response }.to change { Packages::Conan::PackageReference.count }.by(1)
+
+ package_reference = Packages::Conan::PackageReference.last
+ expect(package_reference.reference).to eq(conan_package_reference)
+ expect(response).to be_success
+ expect(response[:package_reference_id]).to eq(package_reference.id)
+ end
+
+ context 'when the package reference is invalid' do
+ let(:conan_package_reference) { nil }
+
+ it 'raises the error' do
+ expect { response }.to raise_error(ActiveRecord::RecordInvalid)
+ end
+ end
+ end
+
+ context 'when the package reference already exists' do
+ let_it_be(:package_reference) do
+ create(:conan_package_reference, package: package, reference: conan_package_reference, recipe_revision: nil)
+ end
+
+ it 'returns existing package reference' do
+ expect { response }.not_to change { Packages::Conan::PackageReference.count }
+
+ expect(response).to be_success
+ expect(response[:package_reference_id]).to eq(package_reference.id)
+ end
+ end
+ end
+end
diff --git a/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb b/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb
index c2e6980f81bc869f39db294587043d63abadc744..4cc1588726d2c4175175e096f1901fb59fa6fd05 100644
--- a/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb
+++ b/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb
@@ -18,7 +18,10 @@
let_it_be(:job, freeze: true) { create(:ci_build, :running, user: user, project: project) }
- let_it_be_with_reload(:package) { create(:conan_package, project: project) }
+ let_it_be(:conan_package_reference) { '1234567890abcdef1234567890abcdef12345678' }
+ let_it_be_with_reload(:package) do
+ create(:conan_package, project: project, package_references: [conan_package_reference])
+ end
let(:job_token) { job.token }
let(:auth_token) { personal_access_token.token }
@@ -50,7 +53,6 @@
let(:jwt) { build_jwt(personal_access_token) }
let(:headers) { build_token_auth_header(jwt.encoded) }
- let(:conan_package_reference) { '123456789' }
end
RSpec.shared_context 'conan file download endpoints' do
diff --git a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb
index 087830eb06bf1944d6deb69d8f8ac92a59d2943b..975f37c536d02f417ddab88181baf1631eeda9c6 100644
--- a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb
+++ b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb
@@ -370,9 +370,9 @@
it 'returns the download_urls for the package files' do
expected_response = {
- 'conaninfo.txt' => "#{url_prefix}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conaninfo.txt",
- 'conanmanifest.txt' => "#{url_prefix}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conanmanifest.txt",
- 'conan_package.tgz' => "#{url_prefix}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conan_package.tgz"
+ 'conaninfo.txt' => "#{url_prefix}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{conan_package_reference}/0/conaninfo.txt",
+ 'conanmanifest.txt' => "#{url_prefix}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{conan_package_reference}/0/conanmanifest.txt",
+ 'conan_package.tgz' => "#{url_prefix}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{conan_package_reference}/0/conan_package.tgz"
}
subject