Commit 99a20916 authored by Steve Abrams's avatar Steve Abrams Committed by Nick Thomas

Update naming validation for Conan recipes

Conan recipes do not share the same naming validation
as other packages. A new validation is added to
check the conan_metadatum for recipe uniqueness
parent c274e589
---
title: Conan packages are validated based on full recipe instead of name/version alone
merge_request: 23467
author:
type: changed
# frozen_string_literal: true
class ReplaceConanMetadataIndex < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
OLD_INDEX = 'index_packages_conan_metadata_on_package_id'
NEW_INDEX = 'index_packages_conan_metadata_on_package_id_username_channel'
disable_ddl_transaction!
def up
add_concurrent_index :packages_conan_metadata,
[:package_id, :package_username, :package_channel],
unique: true, name: NEW_INDEX
remove_concurrent_index_by_name :packages_conan_metadata, OLD_INDEX
end
def down
add_concurrent_index :packages_conan_metadata, :package_id, name: OLD_INDEX
remove_concurrent_index_by_name :packages_conan_metadata, NEW_INDEX
end
end
...@@ -2927,7 +2927,7 @@ ActiveRecord::Schema.define(version: 2020_02_03_025821) do ...@@ -2927,7 +2927,7 @@ ActiveRecord::Schema.define(version: 2020_02_03_025821) do
t.datetime_with_timezone "updated_at", null: false t.datetime_with_timezone "updated_at", null: false
t.string "package_username", limit: 255, null: false t.string "package_username", limit: 255, null: false
t.string "package_channel", limit: 255, null: false t.string "package_channel", limit: 255, null: false
t.index ["package_id"], name: "index_packages_conan_metadata_on_package_id", unique: true t.index ["package_id", "package_username", "package_channel"], name: "index_packages_conan_metadata_on_package_id_username_channel", unique: true
end end
create_table "packages_dependencies", force: :cascade do |t| create_table "packages_dependencies", force: :cascade do |t|
......
...@@ -23,8 +23,9 @@ class Packages::Package < ApplicationRecord ...@@ -23,8 +23,9 @@ class Packages::Package < ApplicationRecord
format: { with: Gitlab::Regex.package_name_regex } format: { with: Gitlab::Regex.package_name_regex }
validates :name, validates :name,
uniqueness: { scope: %i[project_id version package_type] } uniqueness: { scope: %i[project_id version package_type] }, unless: :conan?
validate :valid_conan_package_recipe, if: :conan?
validate :valid_npm_package_name, if: :npm? validate :valid_npm_package_name, if: :npm?
validate :package_already_taken, if: :npm? validate :package_already_taken, if: :npm?
...@@ -39,6 +40,10 @@ class Packages::Package < ApplicationRecord ...@@ -39,6 +40,10 @@ class Packages::Package < ApplicationRecord
joins(:conan_metadatum).where(packages_conan_metadata: { package_channel: package_channel }) joins(:conan_metadatum).where(packages_conan_metadata: { package_channel: package_channel })
end end
scope :with_conan_username, ->(package_username) do
joins(:conan_metadatum).where(packages_conan_metadata: { package_username: package_username })
end
scope :has_version, -> { where.not(version: nil) } scope :has_version, -> { where.not(version: nil) }
scope :processed, -> do scope :processed, -> do
where.not(package_type: :nuget).or( where.not(package_type: :nuget).or(
...@@ -100,6 +105,20 @@ class Packages::Package < ApplicationRecord ...@@ -100,6 +105,20 @@ class Packages::Package < ApplicationRecord
private private
def valid_conan_package_recipe
recipe_exists = project.packages
.conan
.includes(:conan_metadatum)
.with_name(name)
.with_version(version)
.with_conan_channel(conan_metadatum.package_channel)
.with_conan_username(conan_metadatum.package_username)
.id_not_in(id)
.exists?
errors.add(:base, 'Package recipe already exists') if recipe_exists
end
def valid_npm_package_name def valid_npm_package_name
return unless project&.root_namespace return unless project&.root_namespace
......
...@@ -47,7 +47,23 @@ RSpec.describe Packages::Package, type: :model do ...@@ -47,7 +47,23 @@ RSpec.describe Packages::Package, type: :model do
end end
end end
Packages::Package.package_types.keys.each do |pt| context "recipe uniqueness for conan packages" do
let!(:package) { create('conan_package') }
it "will allow a conan package with same project, name, version and package_type" do
new_package = build('conan_package', project: package.project, name: package.name, version: package.version)
new_package.conan_metadatum.package_channel = 'beta'
expect(new_package).to be_valid
end
it "will not allow a conan package with same recipe (name, version, metadatum.package_channel, metadatum.package_username, and package_type)" do
new_package = build('conan_package', project: package.project, name: package.name, version: package.version)
expect(new_package).not_to be_valid
expect(new_package.errors.to_a).to include("Package recipe already exists")
end
end
Packages::Package.package_types.keys.without('conan').each do |pt|
context "project id, name, version and package type uniqueness for package type #{pt}" do context "project id, name, version and package type uniqueness for package type #{pt}" do
let(:package) { create("#{pt}_package") } let(:package) { create("#{pt}_package") }
...@@ -119,24 +135,28 @@ RSpec.describe Packages::Package, type: :model do ...@@ -119,24 +135,28 @@ RSpec.describe Packages::Package, type: :model do
end end
end end
describe '.with_conan_channel' do context 'conan scopes' do
let!(:package) { create(:conan_package) } let!(:package) { create(:conan_package) }
describe '.with_conan_channel' do
subject { described_class.with_conan_channel('stable') } subject { described_class.with_conan_channel('stable') }
it 'includes only packages with specified version' do it 'includes only packages with specified version' do
is_expected.to match_array([package]) is_expected.to match_array([package])
end end
end end
end
describe '.with_conan_channel' do describe '.with_conan_username' do
let!(:package) { create(:conan_package) } subject do
described_class.with_conan_username(
subject { described_class.with_conan_channel('stable') } Packages::ConanMetadatum.package_username_from(full_path: package.project.full_path)
)
end
it 'includes only packages with specified version' do it 'includes only packages with specified version' do
is_expected.to eq([package]) is_expected.to match_array([package])
end
end
end end
end end
......
...@@ -21,7 +21,7 @@ describe Packages::Nuget::MetadataExtractionService do ...@@ -21,7 +21,7 @@ describe Packages::Nuget::MetadataExtractionService do
context 'linked to a non nuget package' do context 'linked to a non nuget package' do
before do before do
package_file.package.conan! package_file.package.maven!
end end
it { expect { subject }.to raise_error(::Packages::Nuget::MetadataExtractionService::ExtractionError, 'invalid package file') } it { expect { subject }.to raise_error(::Packages::Nuget::MetadataExtractionService::ExtractionError, 'invalid package file') }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment