Commit 6e8304d9 authored by Nick Thomas's avatar Nick Thomas

Revert "Merge branch '37335-conan-name-validation' into 'master'"

This reverts merge request !23467
parent 4d216218
---
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
...@@ -2945,7 +2945,7 @@ ActiveRecord::Schema.define(version: 2020_02_06_111847) do ...@@ -2945,7 +2945,7 @@ ActiveRecord::Schema.define(version: 2020_02_06_111847) 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", "package_username", "package_channel"], name: "index_packages_conan_metadata_on_package_id_username_channel", unique: true t.index ["package_id"], name: "index_packages_conan_metadata_on_package_id", unique: true
end end
create_table "packages_dependencies", force: :cascade do |t| create_table "packages_dependencies", force: :cascade do |t|
......
...@@ -23,9 +23,8 @@ class Packages::Package < ApplicationRecord ...@@ -23,9 +23,8 @@ 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] }, unless: :conan? uniqueness: { scope: %i[project_id version package_type] }
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?
...@@ -40,10 +39,6 @@ class Packages::Package < ApplicationRecord ...@@ -40,10 +39,6 @@ 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(
...@@ -105,20 +100,6 @@ class Packages::Package < ApplicationRecord ...@@ -105,20 +100,6 @@ 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,23 +47,7 @@ RSpec.describe Packages::Package, type: :model do ...@@ -47,23 +47,7 @@ RSpec.describe Packages::Package, type: :model do
end end
end end
context "recipe uniqueness for conan packages" do Packages::Package.package_types.keys.each do |pt|
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") }
...@@ -135,28 +119,24 @@ RSpec.describe Packages::Package, type: :model do ...@@ -135,28 +119,24 @@ RSpec.describe Packages::Package, type: :model do
end end
end end
context 'conan scopes' do describe '.with_conan_channel' 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
describe '.with_conan_username' do
subject do
described_class.with_conan_username(
Packages::ConanMetadatum.package_username_from(full_path: package.project.full_path)
)
end end
describe '.with_conan_channel' do
let!(:package) { create(:conan_package) }
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 eq([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.maven! package_file.package.conan!
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