Commit 6a94d30a authored by Stan Hu's avatar Stan Hu

Merge branch '10io-add-npm-dependencies-support' into 'master'

Add npm dependencies support

See merge request gitlab-org/gitlab!20549
parents 25a97e59 7ce78e6c
# frozen_string_literal: true
class CreatePackagesDependencies < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
create_table :packages_dependencies do |t|
t.string :name, null: false, limit: 255
t.string :version_pattern, null: false, limit: 255
end
add_index :packages_dependencies, [:name, :version_pattern], unique: true
end
end
# frozen_string_literal: true
class CreatePackagesDependencyLinks < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
create_table :packages_dependency_links do |t|
t.references :package, index: false, null: false, foreign_key: { to_table: :packages_packages, on_delete: :cascade }, type: :bigint
t.references :dependency, null: false, foreign_key: { to_table: :packages_dependencies, on_delete: :cascade }, type: :bigint
t.integer :dependency_type, limit: 2, null: false
end
add_index :packages_dependency_links, [:package_id, :dependency_id, :dependency_type], unique: true, name: 'idx_pkgs_dep_links_on_pkg_id_dependency_id_dependency_type'
end
end
# frozen_string_literal: true
class AddProjectIdNameVersionPackageTypeIndexToPackagesPackages < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
INDEX_NAME = 'idx_packages_packages_on_project_id_name_version_package_type'.freeze
disable_ddl_transaction!
def up
add_concurrent_index :packages_packages,
[:project_id, :name, :version, :package_type],
name: INDEX_NAME
end
def down
remove_concurrent_index :packages_packages,
[:project_id, :name, :version, :package_type],
name: INDEX_NAME
end
end
# frozen_string_literal: true
class DropPackagesPackageMetadataTable < ActiveRecord::Migration[5.2]
DOWNTIME = false
def up
drop_table :packages_package_metadata
end
def down
create_table :packages_package_metadata do |t|
t.references :package, index: { unique: true }, null: false, foreign_key: { to_table: :packages_packages, on_delete: :cascade }, type: :integer
t.binary :metadata, null: false
end
end
end
...@@ -2824,6 +2824,20 @@ ActiveRecord::Schema.define(version: 2019_12_04_093410) do ...@@ -2824,6 +2824,20 @@ ActiveRecord::Schema.define(version: 2019_12_04_093410) do
t.index ["package_id"], name: "index_packages_conan_metadata_on_package_id", 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|
t.string "name", limit: 255, null: false
t.string "version_pattern", limit: 255, null: false
t.index ["name", "version_pattern"], name: "index_packages_dependencies_on_name_and_version_pattern", unique: true
end
create_table "packages_dependency_links", force: :cascade do |t|
t.bigint "package_id", null: false
t.bigint "dependency_id", null: false
t.integer "dependency_type", limit: 2, null: false
t.index ["dependency_id"], name: "index_packages_dependency_links_on_dependency_id"
t.index ["package_id", "dependency_id", "dependency_type"], name: "idx_pkgs_dep_links_on_pkg_id_dependency_id_dependency_type", unique: true
end
create_table "packages_maven_metadata", force: :cascade do |t| create_table "packages_maven_metadata", force: :cascade do |t|
t.bigint "package_id", null: false t.bigint "package_id", null: false
t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "created_at", null: false
...@@ -2849,12 +2863,6 @@ ActiveRecord::Schema.define(version: 2019_12_04_093410) do ...@@ -2849,12 +2863,6 @@ ActiveRecord::Schema.define(version: 2019_12_04_093410) do
t.index ["package_id", "file_name"], name: "index_packages_package_files_on_package_id_and_file_name" t.index ["package_id", "file_name"], name: "index_packages_package_files_on_package_id_and_file_name"
end end
create_table "packages_package_metadata", force: :cascade do |t|
t.integer "package_id", null: false
t.binary "metadata", null: false
t.index ["package_id"], name: "index_packages_package_metadata_on_package_id", unique: true
end
create_table "packages_package_tags", force: :cascade do |t| create_table "packages_package_tags", force: :cascade do |t|
t.integer "package_id", null: false t.integer "package_id", null: false
t.string "name", limit: 255, null: false t.string "name", limit: 255, null: false
...@@ -2869,6 +2877,7 @@ ActiveRecord::Schema.define(version: 2019_12_04_093410) do ...@@ -2869,6 +2877,7 @@ ActiveRecord::Schema.define(version: 2019_12_04_093410) do
t.string "version" t.string "version"
t.integer "package_type", limit: 2, null: false t.integer "package_type", limit: 2, null: false
t.index ["name"], name: "index_packages_packages_on_name_trigram", opclass: :gin_trgm_ops, using: :gin t.index ["name"], name: "index_packages_packages_on_name_trigram", opclass: :gin_trgm_ops, using: :gin
t.index ["project_id", "name", "version", "package_type"], name: "idx_packages_packages_on_project_id_name_version_package_type"
t.index ["project_id"], name: "index_packages_packages_on_project_id" t.index ["project_id"], name: "index_packages_packages_on_project_id"
end end
...@@ -4568,9 +4577,10 @@ ActiveRecord::Schema.define(version: 2019_12_04_093410) do ...@@ -4568,9 +4577,10 @@ ActiveRecord::Schema.define(version: 2019_12_04_093410) do
add_foreign_key "operations_feature_flags_clients", "projects", on_delete: :cascade add_foreign_key "operations_feature_flags_clients", "projects", on_delete: :cascade
add_foreign_key "packages_conan_file_metadata", "packages_package_files", column: "package_file_id", on_delete: :cascade add_foreign_key "packages_conan_file_metadata", "packages_package_files", column: "package_file_id", on_delete: :cascade
add_foreign_key "packages_conan_metadata", "packages_packages", column: "package_id", on_delete: :cascade add_foreign_key "packages_conan_metadata", "packages_packages", column: "package_id", on_delete: :cascade
add_foreign_key "packages_dependency_links", "packages_dependencies", column: "dependency_id", on_delete: :cascade
add_foreign_key "packages_dependency_links", "packages_packages", column: "package_id", on_delete: :cascade
add_foreign_key "packages_maven_metadata", "packages_packages", column: "package_id", name: "fk_be88aed360", on_delete: :cascade add_foreign_key "packages_maven_metadata", "packages_packages", column: "package_id", name: "fk_be88aed360", on_delete: :cascade
add_foreign_key "packages_package_files", "packages_packages", column: "package_id", name: "fk_86f0f182f8", on_delete: :cascade add_foreign_key "packages_package_files", "packages_packages", column: "package_id", name: "fk_86f0f182f8", on_delete: :cascade
add_foreign_key "packages_package_metadata", "packages_packages", column: "package_id", on_delete: :cascade
add_foreign_key "packages_package_tags", "packages_packages", column: "package_id", on_delete: :cascade add_foreign_key "packages_package_tags", "packages_packages", column: "package_id", on_delete: :cascade
add_foreign_key "packages_packages", "projects", on_delete: :cascade add_foreign_key "packages_packages", "projects", on_delete: :cascade
add_foreign_key "pages_domain_acme_orders", "pages_domains", on_delete: :cascade add_foreign_key "pages_domain_acme_orders", "pages_domains", on_delete: :cascade
......
...@@ -122,7 +122,7 @@ Then, you could run `npm publish` either locally or via GitLab CI/CD: ...@@ -122,7 +122,7 @@ Then, you could run `npm publish` either locally or via GitLab CI/CD:
- **GitLab CI/CD:** Set an `NPM_TOKEN` [variable](../../../ci/variables/README.md) - **GitLab CI/CD:** Set an `NPM_TOKEN` [variable](../../../ci/variables/README.md)
under your project's **Settings > CI/CD > Variables**. under your project's **Settings > CI/CD > Variables**.
### Authenticating with a CI job token ### Authenticating with a CI job token
> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/9104) in GitLab Premium 12.5. > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/9104) in GitLab Premium 12.5.
...@@ -130,7 +130,7 @@ Then, you could run `npm publish` either locally or via GitLab CI/CD: ...@@ -130,7 +130,7 @@ Then, you could run `npm publish` either locally or via GitLab CI/CD:
If you’re using NPM with GitLab CI/CD, a CI job token can be used instead of a personal access token. If you’re using NPM with GitLab CI/CD, a CI job token can be used instead of a personal access token.
The token will inherit the permissions of the user that generates the pipeline. The token will inherit the permissions of the user that generates the pipeline.
Add a corresponding section to your `.npmrc` file: Add a corresponding section to your `.npmrc` file:
```ini ```ini
@foo:registry=https://gitlab.com/api/v4/packages/npm/ @foo:registry=https://gitlab.com/api/v4/packages/npm/
...@@ -226,3 +226,19 @@ And the `.npmrc` file should look like: ...@@ -226,3 +226,19 @@ And the `.npmrc` file should look like:
//gitlab.com/api/v4/packages/npm/:_authToken=<your_oauth_token> //gitlab.com/api/v4/packages/npm/:_authToken=<your_oauth_token>
@foo:registry=https://gitlab.com/api/v4/packages/npm/ @foo:registry=https://gitlab.com/api/v4/packages/npm/
``` ```
## NPM dependencies metadata
> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/11867) in GitLab Premium 12.6.
Starting from GitLab 12.6, new packages published to the GitLab NPM Registry expose the following attributes to the NPM client:
- name
- version
- dist-tags
- dependencies
- dependencies
- devDependencies
- bundleDependencies
- peerDependencies
- deprecated
# frozen_string_literal: true
class Packages::Dependency < ApplicationRecord
has_many :dependency_links, class_name: 'Packages::DependencyLink'
validates :name, :version_pattern, presence: true
validates :name, uniqueness: { scope: :version_pattern }
NAME_VERSION_PATTERN_TUPLE_MATCHING = '(name, version_pattern) = (?, ?)'.freeze
MAX_STRING_LENGTH = 255.freeze
MAX_CHUNKED_QUERIES_COUNT = 10.freeze
def self.ids_for_package_names_and_version_patterns(names_and_version_patterns = {}, chunk_size = 50, max_rows_limit = 200)
names_and_version_patterns.reject! { |key, value| key.size > MAX_STRING_LENGTH || value.size > MAX_STRING_LENGTH }
raise ArgumentError, 'Too many names_and_version_patterns' if names_and_version_patterns.size > MAX_CHUNKED_QUERIES_COUNT * chunk_size
matched_ids = []
names_and_version_patterns.each_slice(chunk_size) do |tuples|
where_statement = Array.new(tuples.size, NAME_VERSION_PATTERN_TUPLE_MATCHING)
.join(' OR ')
ids = where(where_statement, *tuples.flatten)
.limit(max_rows_limit + 1)
.pluck(:id)
matched_ids.concat(ids)
raise ArgumentError, 'Too many Dependencies selected' if matched_ids.size > max_rows_limit
end
matched_ids
end
def self.for_package_names_and_version_patterns(names_and_version_patterns = {}, chunk_size = 50, max_rows_limit = 200)
ids = ids_for_package_names_and_version_patterns(names_and_version_patterns, chunk_size, max_rows_limit)
return none if ids.empty?
id_in(ids)
end
def self.pluck_ids_and_names
pluck(:id, :name)
end
def orphaned?
self.dependency_links.empty?
end
end
# frozen_string_literal: true
class Packages::DependencyLink < ApplicationRecord
belongs_to :package, inverse_of: :dependency_links
belongs_to :dependency, inverse_of: :dependency_links, class_name: 'Packages::Dependency'
validates :package, :dependency, presence: true
validates :dependency_type,
uniqueness: { scope: %i[package_id dependency_id] }
enum dependency_type: { dependencies: 1, devDependencies: 2, bundleDependencies: 3, peerDependencies: 4, deprecated: 5 }
scope :with_dependency_type, ->(dependency_type) { where(dependency_type: dependency_type) }
scope :includes_dependency, -> { includes(:dependency) }
end
...@@ -5,6 +5,7 @@ class Packages::Package < ApplicationRecord ...@@ -5,6 +5,7 @@ class Packages::Package < ApplicationRecord
belongs_to :project belongs_to :project
# package_files must be destroyed by ruby code in order to properly remove carrierwave uploads and update project statistics # package_files must be destroyed by ruby code in order to properly remove carrierwave uploads and update project statistics
has_many :package_files, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :package_files, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :dependency_links, inverse_of: :package, class_name: 'Packages::DependencyLink'
has_one :conan_metadatum, inverse_of: :package has_one :conan_metadatum, inverse_of: :package
has_one :maven_metadatum, inverse_of: :package has_one :maven_metadatum, inverse_of: :package
...@@ -19,6 +20,9 @@ class Packages::Package < ApplicationRecord ...@@ -19,6 +20,9 @@ class Packages::Package < ApplicationRecord
presence: true, presence: true,
format: { with: Gitlab::Regex.package_name_regex } format: { with: Gitlab::Regex.package_name_regex }
validates :name,
uniqueness: { scope: %i[project_id version package_type] }
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?
......
...@@ -3,10 +3,11 @@ ...@@ -3,10 +3,11 @@
class NpmPackagePresenter class NpmPackagePresenter
include API::Helpers::RelatedResourcesHelpers include API::Helpers::RelatedResourcesHelpers
attr_reader :project, :name, :packages attr_reader :name, :packages
def initialize(project, name, packages) NPM_VALID_DEPENDENCY_TYPES = %i[dependencies devDependencies bundleDependencies peerDependencies deprecated].freeze
@project = project
def initialize(name, packages)
@name = name @name = name
@packages = packages @packages = packages
end end
...@@ -41,7 +42,9 @@ class NpmPackagePresenter ...@@ -41,7 +42,9 @@ class NpmPackagePresenter
shasum: package_file.file_sha1, shasum: package_file.file_sha1,
tarball: tarball_url(package, package_file) tarball: tarball_url(package, package_file)
} }
} }.tap do |package_version|
package_version.merge!(build_package_dependencies(package))
end
end end
def tarball_url(package, package_file) def tarball_url(package, package_file)
...@@ -50,6 +53,22 @@ class NpmPackagePresenter ...@@ -50,6 +53,22 @@ class NpmPackagePresenter
"/-/#{package_file.file_name}" "/-/#{package_file.file_name}"
end end
def build_package_dependencies(package)
return {} if package.dependency_links.empty?
dependencies = Hash.new { |h, key| h[key] = {} }
dependency_links = package.dependency_links
.with_dependency_type(NPM_VALID_DEPENDENCY_TYPES)
.includes_dependency
dependency_links.find_each do |dependency_link|
dependency = dependency_link.dependency
dependencies[dependency_link.dependency_type][dependency.name] = dependency.version_pattern
end
dependencies
end
def sorted_versions def sorted_versions
versions = packages.map(&:version).compact versions = packages.map(&:version).compact
VersionSorter.sort(versions) VersionSorter.sort(versions)
......
# frozen_string_literal: true
module Packages
class CreateDependencyService < BaseService
attr_reader :package, :dependencies
def initialize(package, dependencies)
@package = package
@dependencies = dependencies
end
def execute
Packages::DependencyLink.dependency_types.each_key do |type|
create_dependency(type)
end
end
private
def create_dependency(type)
return unless dependencies.key?(type)
names_and_version_patterns = dependencies[type]
existing_ids, existing_names = find_existing_ids_and_names(names_and_version_patterns)
dependencies_to_insert = names_and_version_patterns
if existing_names.any?
dependencies_to_insert = names_and_version_patterns.reject { |k, _| k.in?(existing_names) }
end
ActiveRecord::Base.transaction do
inserted_ids = bulk_insert_package_dependencies(dependencies_to_insert)
bulk_insert_package_dependency_links(type, (existing_ids + inserted_ids))
end
end
def find_existing_ids_and_names(names_and_version_patterns)
ids_and_names = Packages::Dependency.for_package_names_and_version_patterns(names_and_version_patterns)
.pluck_ids_and_names
ids = ids_and_names.map(&:first) || []
names = ids_and_names.map(&:second) || []
[ids, names]
end
def bulk_insert_package_dependencies(names_and_version_patterns)
return [] if names_and_version_patterns.empty?
rows = names_and_version_patterns.map do |name, version_pattern|
{
name: name,
version_pattern: version_pattern
}
end
ids = database.bulk_insert(Packages::Dependency.table_name, rows, return_ids: true, on_conflict: :do_nothing)
return ids if ids.size == names_and_version_patterns.size
Packages::Dependency.uncached do
# The bulk_insert statement above do not dirty the query cache. To make
# sure that the results are fresh from the database and not from a stalled
# and potentially wrong cache, this query has to be done with the query
# chache disabled.
Packages::Dependency.ids_for_package_names_and_version_patterns(names_and_version_patterns)
end
end
def bulk_insert_package_dependency_links(type, dependency_ids)
rows = dependency_ids.map do |dependency_id|
{
package_id: package.id,
dependency_id: dependency_id,
dependency_type: Packages::DependencyLink.dependency_types[type.to_s]
}
end
database.bulk_insert(Packages::DependencyLink.table_name, rows)
end
def database
::Gitlab::Database
end
end
end
...@@ -26,9 +26,17 @@ module Packages ...@@ -26,9 +26,17 @@ module Packages
file_name: package_file_name file_name: package_file_name
} }
::Packages::CreatePackageFileService.new(package, file_params).execute package.transaction do
::Packages::CreatePackageFileService.new(package, file_params).execute
::Packages::CreateDependencyService.new(package, package_dependencies).execute
end
package package
end end
def package_dependencies
_version, version_data = params[:versions].first
version_data
end
end end
end end
---
title: Fix dependency metadata on the NPM registry responses
merge_request: 20549
author:
type: fixed
...@@ -40,7 +40,7 @@ module API ...@@ -40,7 +40,7 @@ module API
packages = ::Packages::NpmPackagesFinder packages = ::Packages::NpmPackagesFinder
.new(project, package_name).execute .new(project, package_name).execute
present NpmPackagePresenter.new(project, package_name, packages), present NpmPackagePresenter.new(package_name, packages),
with: EE::API::Entities::NpmPackage with: EE::API::Entities::NpmPackage
end end
......
...@@ -3,7 +3,7 @@ FactoryBot.define do ...@@ -3,7 +3,7 @@ FactoryBot.define do
factory :package, class: Packages::Package do factory :package, class: Packages::Package do
project project
name { 'my/company/app/my-app' } name { 'my/company/app/my-app' }
version { '1.0-SNAPSHOT' } sequence(:version) { |n| "1.#{n}-SNAPSHOT" }
package_type { 'maven' } package_type { 'maven' }
factory :maven_package do factory :maven_package do
...@@ -188,4 +188,15 @@ FactoryBot.define do ...@@ -188,4 +188,15 @@ FactoryBot.define do
conan_package_reference { '123456789' } conan_package_reference { '123456789' }
end end
end end
factory :packages_dependency, class: Packages::Dependency do
sequence(:name) { |n| "@test/package-#{n}"}
sequence(:version_pattern) { |n| "~6.2.#{n}" }
end
factory :packages_dependency_link, class: Packages::DependencyLink do
package
dependency { create(:packages_dependency) }
dependency_type { :dependencies }
end
end end
...@@ -4,13 +4,43 @@ ...@@ -4,13 +4,43 @@
"properties" : { "properties" : {
"name": { "type": "string" }, "name": { "type": "string" },
"version": { "type": "string" }, "version": { "type": "string" },
"dist": { "dist": {
"type": "object", "type": "object",
"required": ["shasum", "tarball"], "required": ["shasum", "tarball"],
"properties" : { "properties" : {
"shasum": { "type": "string" }, "shasum": { "type": "string" },
"tarball": { "type": "string" } "tarball": { "type": "string" }
} }
},
"dependencies": {
"type": "object",
"patternProperties": {
".{1,}": { "type": "string" }
}
},
"devDependencies": {
"type": "object",
"patternProperties": {
".{1,}": { "type": "string" }
}
},
"bundleDependencies": {
"type": "object",
"patternProperties": {
".{1,}": { "type": "string" }
}
},
"peerDependencies": {
"type": "object",
"patternProperties": {
".{1,}": { "type": "string" }
}
},
"deprecated": {
"type": "object",
"patternProperties": {
".{1,}": { "type": "string" }
}
} }
} }
} }
{
"_id":"@root/npm-test",
"name":"@root/npm-test",
"description":"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.",
"dist-tags":{
"latest":"1.0.1"
},
"versions":{
"1.0.1":{
"name":"@root/npm-test",
"version":"1.0.1",
"main":"app.js",
"dependencies":{
"express":"^4.16.4",
"dagre-d3": "~0.3.2"
},
"devDependencies": {
"dagre-d3": "~0.3.2",
"d3": "~3.4.13"
},
"bundleDependencies": {
"d3": "~3.4.13"
},
"peerDependencies": {
"d3": "~3.3.0"
},
"deprecated": {
"express":"^4.16.4"
},
"dist":{
"shasum":"f572d396fae9206628714fb2ce00f72e94f2258f"
}
}
},
"_attachments":{
"@root/npm-test-1.0.1.tgz":{
"content_type":"application/octet-stream",
"data":"aGVsbG8K",
"length":8
}
},
"id":"10",
"package_name":"@root/npm-test"
}
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Packages::DependencyLink, type: :model do
describe 'relationships' do
it { is_expected.to belong_to(:package).inverse_of(:dependency_links) }
it { is_expected.to belong_to(:dependency).inverse_of(:dependency_links) }
end
describe 'validations' do
subject { create(:packages_dependency_link) }
it { is_expected.to validate_presence_of(:package) }
it { is_expected.to validate_presence_of(:dependency) }
context 'package_id and package_dependency_id uniqueness for dependency_type' do
it 'is not valid' do
exisiting_link = subject
link = build(
:packages_dependency_link,
package: exisiting_link.package,
dependency: exisiting_link.dependency,
dependency_type: exisiting_link.dependency_type
)
expect(link).not_to be_valid
expect(link.errors.to_a).to include("Dependency type has already been taken")
end
end
end
describe '.with_dependency_type' do
let_it_be(:link1) { create(:packages_dependency_link) }
let_it_be(:link2) { create(:packages_dependency_link, dependency: link1.dependency, dependency_type: :devDependencies) }
let_it_be(:link3) { create(:packages_dependency_link, dependency: link1.dependency, dependency_type: :bundleDependencies) }
subject { described_class }
it 'returns links of the given type' do
expect(subject.with_dependency_type(:bundleDependencies)).to eq([link3])
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Packages::Dependency, type: :model do
describe 'relationships' do
it { is_expected.to have_many(:dependency_links) }
end
describe 'validations' do
subject { create(:packages_dependency) }
it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_presence_of(:version_pattern) }
it { is_expected.to validate_uniqueness_of(:name).scoped_to(:version_pattern) }
end
describe '.ids_for_package_names_and_version_patterns' do
let_it_be(:package_dependency1) { create(:packages_dependency, name: 'foo', version_pattern: '~1.0.0') }
let_it_be(:package_dependency2) { create(:packages_dependency, name: 'bar', version_pattern: '~2.5.0') }
let_it_be(:expected_ids) { [package_dependency1.id, package_dependency2.id] }
let(:names_and_version_patterns) { build_names_and_version_patterns(package_dependency1, package_dependency2) }
let(:chunk_size) { 50 }
let(:rows_limit) { 50 }
subject { Packages::Dependency.ids_for_package_names_and_version_patterns(names_and_version_patterns, chunk_size, rows_limit) }
it { is_expected.to match_array(expected_ids) }
context 'with unknown names' do
let(:names_and_version_patterns) { { unknown: '~1.0.0' } }
it { is_expected.to be_empty }
end
context 'with unknown version patterns' do
let(:names_and_version_patterns) { { 'foo' => '~1.0.0beta' } }
it { is_expected.to be_empty }
end
context 'with a name bigger than column size' do
let_it_be(:big_name) { 'a' * (Packages::Dependency::MAX_STRING_LENGTH + 1) }
let(:names_and_version_patterns) { build_names_and_version_patterns(package_dependency1, package_dependency2).merge(big_name => '~1.0.0') }
it { is_expected.to match_array(expected_ids) }
end
context 'with a version pattern bigger than column size' do
let_it_be(:big_version_pattern) { 'a' * (Packages::Dependency::MAX_STRING_LENGTH + 1) }
let(:names_and_version_patterns) { build_names_and_version_patterns(package_dependency1, package_dependency2).merge('test' => big_version_pattern) }
it { is_expected.to match_array(expected_ids) }
end
context 'with too big parameter' do
let(:size) { (Packages::Dependency::MAX_CHUNKED_QUERIES_COUNT * chunk_size) + 1 }
let(:names_and_version_patterns) { Hash[(1..size).map { |v| [v, v] }] }
it { expect { subject }.to raise_error(ArgumentError, 'Too many names_and_version_patterns') }
end
context 'with parameters size' do
let_it_be(:package_dependency3) { create(:packages_dependency, name: 'foo3', version_pattern: '~1.5.3') }
let_it_be(:package_dependency4) { create(:packages_dependency, name: 'foo4', version_pattern: '~1.5.4') }
let_it_be(:package_dependency5) { create(:packages_dependency, name: 'foo5', version_pattern: '~1.5.5') }
let_it_be(:package_dependency6) { create(:packages_dependency, name: 'foo6', version_pattern: '~1.5.6') }
let_it_be(:package_dependency7) { create(:packages_dependency, name: 'foo7', version_pattern: '~1.5.7') }
let(:expected_ids) { [package_dependency1.id, package_dependency2.id, package_dependency3.id, package_dependency4.id, package_dependency5.id, package_dependency6.id, package_dependency7.id] }
let(:names_and_version_patterns) { build_names_and_version_patterns(package_dependency1, package_dependency2, package_dependency3, package_dependency4, package_dependency5, package_dependency6, package_dependency7) }
context 'above the chunk size' do
let(:chunk_size) { 2 }
it { is_expected.to match_array(expected_ids) }
end
context 'selecting too many rows' do
let(:rows_limit) { 2 }
it { expect { subject }.to raise_error(ArgumentError, 'Too many Dependencies selected') }
end
end
end
describe '.for_package_names_and_version_patterns' do
let_it_be(:package_dependency1) { create(:packages_dependency, name: 'foo', version_pattern: '~1.0.0') }
let_it_be(:package_dependency2) { create(:packages_dependency, name: 'bar', version_pattern: '~2.5.0') }
let_it_be(:expected_array) { [package_dependency1, package_dependency2] }
let(:names_and_version_patterns) { build_names_and_version_patterns(package_dependency1, package_dependency2) }
subject { Packages::Dependency.for_package_names_and_version_patterns(names_and_version_patterns) }
it { is_expected.to match_array(expected_array) }
context 'with unknown names' do
let(:names_and_version_patterns) { { unknown: '~1.0.0' } }
it { is_expected.to be_empty }
end
context 'with unknown version patterns' do
let(:names_and_version_patterns) { { 'foo' => '~1.0.0beta' } }
it { is_expected.to be_empty }
end
end
def build_names_and_version_patterns(*package_dependencies)
result = Hash.new { |h, dependency| h[dependency.name] = dependency.version_pattern }
package_dependencies.each { |dependency| result[dependency] }
result
end
end
...@@ -7,10 +7,16 @@ RSpec.describe Packages::Package, type: :model do ...@@ -7,10 +7,16 @@ RSpec.describe Packages::Package, type: :model do
describe 'relationships' do describe 'relationships' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project) }
it { is_expected.to have_many(:package_files).dependent(:destroy) } it { is_expected.to have_many(:package_files).dependent(:destroy) }
it { is_expected.to have_many(:dependency_links).inverse_of(:package) }
it { is_expected.to have_one(:conan_metadatum).inverse_of(:package) }
it { is_expected.to have_one(:maven_metadatum).inverse_of(:package) }
end end
describe 'validations' do describe 'validations' do
subject { create(:package) }
it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id, :version, :package_type) }
describe '#name' do describe '#name' do
it { is_expected.to allow_value("my/domain/com/my-app").for(:name) } it { is_expected.to allow_value("my/domain/com/my-app").for(:name) }
...@@ -39,6 +45,18 @@ RSpec.describe Packages::Package, type: :model do ...@@ -39,6 +45,18 @@ RSpec.describe Packages::Package, type: :model do
end end
end end
end end
Packages::Package.package_types.keys.each do |pt|
context "project id, name, version and package type uniqueness for package type #{pt}" do
let(:package) { create("#{pt}_package") }
it "will not allow a #{pt} package with same project, name, version and package_type" do
new_package = build("#{pt}_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("Name has already been taken")
end
end
end
end end
describe '#destroy' do describe '#destroy' do
......
...@@ -3,18 +3,47 @@ ...@@ -3,18 +3,47 @@
require 'spec_helper' require 'spec_helper'
describe NpmPackagePresenter do describe NpmPackagePresenter do
set(:project) { create(:project) } let_it_be(:project) { create(:project) }
set(:package) { create(:npm_package, version: '1.0.4', project: project) } let_it_be(:package_name) { "@#{project.root_namespace.path}/test" }
set(:latest_package) { create(:npm_package, version: '1.0.11', project: project) } let!(:package1) { create(:npm_package, version: '1.0.4', project: project, name: package_name) }
let(:presenter) { described_class.new(project, package.name, project.packages.all) } let!(:package2) { create(:npm_package, version: '1.0.6', project: project, name: package_name) }
let!(:latest_package) { create(:npm_package, version: '1.0.11', project: project, name: package_name) }
describe '#dist_tags' do let(:packages) { project.packages.npm.with_name(package_name).last_of_each_version }
it { expect(presenter.dist_tags).to be_a(Hash) } let(:presenter) { described_class.new(package_name, packages) }
it { expect(presenter.dist_tags[:latest]).to eq(latest_package.version) }
end
describe '#versions' do describe '#versions' do
it { expect(presenter.versions).to be_a(Hash) } subject { presenter.versions }
it { expect(presenter.versions[package.version]).to match_schema('public_api/v4/packages/npm_package_version', dir: 'ee') }
context 'for packages without dependencies' do
it { is_expected.to be_a(Hash) }
it { expect(subject[package1.version]).to match_schema('public_api/v4/packages/npm_package_version', dir: 'ee') }
it { expect(subject[package2.version]).to match_schema('public_api/v4/packages/npm_package_version', dir: 'ee') }
NpmPackagePresenter::NPM_VALID_DEPENDENCY_TYPES.each do |dependency_type|
it { expect(subject.dig(package1.version, dependency_type)).to be nil }
it { expect(subject.dig(package2.version, dependency_type)).to be nil }
end
end
context 'for packages with dependencies' do
NpmPackagePresenter::NPM_VALID_DEPENDENCY_TYPES.each do |dependency_type|
let!("package_dependency_link_for_#{dependency_type}") { create(:packages_dependency_link, package: package1, dependency_type: dependency_type) }
end
it { is_expected.to be_a(Hash) }
it { expect(subject[package1.version]).to match_schema('public_api/v4/packages/npm_package_version', dir: 'ee') }
it { expect(subject[package2.version]).to match_schema('public_api/v4/packages/npm_package_version', dir: 'ee') }
NpmPackagePresenter::NPM_VALID_DEPENDENCY_TYPES.each do |dependency_type|
it { expect(subject.dig(package1.version, dependency_type.to_s)).to be_any }
end
end
end
describe '#dist-tags' do
subject { presenter.dist_tags }
it { is_expected.to be_a(Hash) }
it { expect(subject.size).to eq(1) }
it { expect(subject[:latest]).to eq(latest_package.version) }
end end
end end
...@@ -36,6 +36,11 @@ describe API::NpmPackages do ...@@ -36,6 +36,11 @@ describe API::NpmPackages do
describe 'GET /api/v4/packages/npm/*package_name' do describe 'GET /api/v4/packages/npm/*package_name' do
let(:package) { create(:npm_package, project: project) } let(:package) { create(:npm_package, project: project) }
let!(:package_dependency_link1) { create(:packages_dependency_link, package: package, dependency_type: :dependencies) }
let!(:package_dependency_link2) { create(:packages_dependency_link, package: package, dependency_type: :devDependencies) }
let!(:package_dependency_link3) { create(:packages_dependency_link, package: package, dependency_type: :bundleDependencies) }
let!(:package_dependency_link4) { create(:packages_dependency_link, package: package, dependency_type: :peerDependencies) }
let!(:package_dependency_link5) { create(:packages_dependency_link, package: package, dependency_type: :deprecated) }
context 'a public project' do context 'a public project' do
it 'returns the package info without oauth token' do it 'returns the package info without oauth token' do
...@@ -243,6 +248,36 @@ describe API::NpmPackages do ...@@ -243,6 +248,36 @@ describe API::NpmPackages do
expect(response).to have_gitlab_http_status(403) expect(response).to have_gitlab_http_status(403)
end end
end end
context 'with dependencies' do
let(:package_name) { "@#{group.path}/my_package_name" }
let(:params) { upload_params(package_name, 'npm/payload_with_duplicated_packages.json') }
it 'creates npm package with file and dependencies' do
expect { upload_package_with_token(package_name, params) }
.to change { project.packages.count }.by(1)
.and change { Packages::PackageFile.count }.by(1)
.and change { Packages::Dependency.count}.by(4)
.and change { Packages::DependencyLink.count}.by(7)
expect(response).to have_gitlab_http_status(200)
end
context 'with existing dependencies' do
before do
name = "@#{group.path}/existing_package"
upload_package_with_token(name, upload_params(name, 'npm/payload_with_duplicated_packages.json'))
end
it 'reuses them' do
expect { upload_package_with_token(package_name, params) }
.to change { project.packages.count }.by(1)
.and change { Packages::PackageFile.count }.by(1)
.and not_change { Packages::Dependency.count}
.and change { Packages::DependencyLink.count}.by(7)
end
end
end
end end
def upload_package(package_name, params = {}) def upload_package(package_name, params = {})
...@@ -257,9 +292,9 @@ describe API::NpmPackages do ...@@ -257,9 +292,9 @@ describe API::NpmPackages do
upload_package(package_name, params.merge(job_token: job.token)) upload_package(package_name, params.merge(job_token: job.token))
end end
def upload_params(package_name) def upload_params(package_name, file = 'npm/payload.json')
JSON.parse( JSON.parse(
fixture_file('npm/payload.json', dir: 'ee') fixture_file(file, dir: 'ee')
.gsub('@root/npm-test', package_name)) .gsub('@root/npm-test', package_name))
end end
end end
...@@ -270,5 +305,8 @@ describe API::NpmPackages do ...@@ -270,5 +305,8 @@ describe API::NpmPackages do
expect(response).to match_response_schema('public_api/v4/packages/npm_package', dir: 'ee') expect(response).to match_response_schema('public_api/v4/packages/npm_package', dir: 'ee')
expect(json_response['name']).to eq(package.name) expect(json_response['name']).to eq(package.name)
expect(json_response['versions'][package.version]).to match_schema('public_api/v4/packages/npm_package_version', dir: 'ee') expect(json_response['versions'][package.version]).to match_schema('public_api/v4/packages/npm_package_version', dir: 'ee')
NpmPackagePresenter::NPM_VALID_DEPENDENCY_TYPES.each do |dependency_type|
expect(json_response.dig('versions', package.version, dependency_type.to_s)).to be_any
end
end end
end end
# frozen_string_literal: true
require 'spec_helper'
describe Packages::CreateDependencyService do
describe '#execute' do
let_it_be(:namespace) {create(:namespace)}
let_it_be(:version) { '1.0.1'.freeze }
let_it_be(:package_name) { "@#{namespace.path}/my-app".freeze }
context 'when packages are published' do
let(:json_file) { 'npm/payload.json' }
let(:params) do
JSON.parse(fixture_file(json_file, dir: 'ee')
.gsub('@root/npm-test', package_name)
.gsub('1.0.1', version))
.with_indifferent_access
end
let(:package_version) { params[:versions].keys.first }
let(:dependencies) { params[:versions][package_version] }
let(:package) { create(:npm_package) }
let(:dependency_names) { package.dependency_links.flat_map(&:dependency).map(&:name).sort }
let(:dependency_link_types) { package.dependency_links.map(&:dependency_type).sort }
subject { described_class.new(package, dependencies).execute }
it 'creates dependencies and links' do
expect(Packages::Dependency)
.to receive(:ids_for_package_names_and_version_patterns)
.once
.and_call_original
expect { subject }
.to change { Packages::Dependency.count }.by(1)
.and change { Packages::DependencyLink.count }.by(1)
expect(dependency_names).to match_array(%w(express))
expect(dependency_link_types).to match_array(%w(dependencies))
end
context 'with repeated packages' do
let(:json_file) { 'npm/payload_with_duplicated_packages.json' }
it 'creates dependencies and links' do
expect(Packages::Dependency)
.to receive(:ids_for_package_names_and_version_patterns)
.exactly(5).times
.and_call_original
expect { subject }
.to change { Packages::Dependency.count }.by(4)
.and change { Packages::DependencyLink.count }.by(7)
expect(dependency_names).to match_array(%w(d3 d3 d3 dagre-d3 dagre-d3 express express))
expect(dependency_link_types).to match_array(%w(bundleDependencies dependencies dependencies deprecated devDependencies devDependencies peerDependencies))
end
end
context 'with dependencies bulk insert conflicts' do
let_it_be(:rows) { [{ name: 'express', version_pattern: '^4.16.4' }] }
it 'creates dependences and links' do
original_bulk_insert = ::Gitlab::Database.method(:bulk_insert)
expect(::Gitlab::Database)
.to receive(:bulk_insert) do |table, rows, return_ids: false, disable_quote: [], on_conflict: nil|
call_count = table == Packages::Dependency.table_name ? 2 : 1
call_count.times { original_bulk_insert.call(table, rows, return_ids: return_ids, disable_quote: disable_quote, on_conflict: on_conflict) }
end.twice
expect(Packages::Dependency)
.to receive(:ids_for_package_names_and_version_patterns)
.twice
.and_call_original
expect { subject }
.to change { Packages::Dependency.count }.by(1)
.and change { Packages::DependencyLink.count }.by(1)
expect(dependency_names).to match_array(%w(express))
expect(dependency_link_types).to match_array(%w(dependencies))
end
end
context 'with existing dependencies' do
let(:other_package) { create(:npm_package) }
before do
described_class.new(other_package, dependencies).execute
end
it 'reuses them' do
expect { subject }
.to not_change { Packages::Dependency.count }
.and change { Packages::DependencyLink.count }.by(1)
end
end
end
end
end
...@@ -12,17 +12,21 @@ describe Packages::CreateNpmPackageService do ...@@ -12,17 +12,21 @@ describe Packages::CreateNpmPackageService do
fixture_file('npm/payload.json', dir: 'ee') fixture_file('npm/payload.json', dir: 'ee')
.gsub('@root/npm-test', package_name) .gsub('@root/npm-test', package_name)
.gsub('1.0.1', version)) .gsub('1.0.1', version))
.with_indifferent_access .with_indifferent_access
end end
shared_examples 'valid package' do subject { described_class.new(project, user, params).execute }
it 'creates a valid package' do
package = described_class.new(project, user, params).execute
expect(package).to be_valid shared_examples 'valid package' do
expect(package.name).to eq(package_name) it 'creates a package' do
expect(package.version).to eq(version) expect { subject }
.to change { Packages::Package.count }.by(1)
.and change { Packages::Package.npm.count }.by(1)
end end
it { is_expected.to be_valid }
it { expect(subject.name).to eq(package_name) }
it { expect(subject.version).to eq(version) }
end end
describe '#execute' do describe '#execute' do
...@@ -35,32 +39,22 @@ describe Packages::CreateNpmPackageService do ...@@ -35,32 +39,22 @@ describe Packages::CreateNpmPackageService do
context 'invalid package name' do context 'invalid package name' do
let(:package_name) { "@#{namespace.path}/my-group/my-app".freeze } let(:package_name) { "@#{namespace.path}/my-group/my-app".freeze }
it 'raises a RecordInvalid error' do it { expect { subject }.to raise_error(ActiveRecord::RecordInvalid) }
service = described_class.new(project, user, params)
expect { service.execute }.to raise_error(ActiveRecord::RecordInvalid)
end
end end
context 'package already exists' do context 'package already exists' do
let(:package_name) { "@#{namespace.path}/my_package" } let(:package_name) { "@#{namespace.path}/my_package" }
let!(:existing_package) { create(:npm_package, project: project, name: package_name, version: '1.0.1') }
it 'returns a 403 error' do it { expect(subject[:http_status]).to eq 403 }
create(:npm_package, project: project, name: package_name, version: '1.0.1') it { expect(subject[:message]).to be 'Package already exists.' }
response = described_class.new(project, user, params).execute
expect(response[:http_status]).to eq 403
expect(response[:message]).to be 'Package already exists.'
end
end end
context 'with incorrect namespace' do context 'with incorrect namespace' do
let(:package_name) { '@my_other_namespace/my-app' } let(:package_name) { '@my_other_namespace/my-app' }
it 'raises a RecordInvalid error' do it 'raises a RecordInvalid error' do
service = described_class.new(project, user, params) expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
expect { service.execute }.to raise_error(ActiveRecord::RecordInvalid)
end end
end end
end end
......
...@@ -95,6 +95,10 @@ module Gitlab ...@@ -95,6 +95,10 @@ module Gitlab
version.to_f >= 9.6 version.to_f >= 9.6
end end
def self.upsert_supported?
version.to_f >= 9.5
end
# map some of the function names that changed between PostgreSQL 9 and 10 # map some of the function names that changed between PostgreSQL 9 and 10
# https://wiki.postgresql.org/wiki/New_in_postgres_10 # https://wiki.postgresql.org/wiki/New_in_postgres_10
def self.pg_wal_lsn_diff def self.pg_wal_lsn_diff
...@@ -158,7 +162,9 @@ module Gitlab ...@@ -158,7 +162,9 @@ module Gitlab
# disable_quote - A key or an Array of keys to exclude from quoting (You # disable_quote - A key or an Array of keys to exclude from quoting (You
# become responsible for protection from SQL injection for # become responsible for protection from SQL injection for
# these keys!) # these keys!)
def self.bulk_insert(table, rows, return_ids: false, disable_quote: []) # on_conflict - Defines an upsert. Values can be: :disabled (default) or
# :do_nothing
def self.bulk_insert(table, rows, return_ids: false, disable_quote: [], on_conflict: nil)
return if rows.empty? return if rows.empty?
keys = rows.first.keys keys = rows.first.keys
...@@ -176,10 +182,12 @@ module Gitlab ...@@ -176,10 +182,12 @@ module Gitlab
VALUES #{tuples.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')} VALUES #{tuples.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')}
EOF EOF
if return_ids if upsert_supported? && on_conflict == :do_nothing
sql = "#{sql}RETURNING id" sql = "#{sql} ON CONFLICT DO NOTHING"
end end
sql = "#{sql} RETURNING id" if return_ids
result = connection.execute(sql) result = connection.execute(sql)
if return_ids if return_ids
......
...@@ -228,6 +228,7 @@ describe Gitlab::Database do ...@@ -228,6 +228,7 @@ describe Gitlab::Database do
describe '.bulk_insert' do describe '.bulk_insert' do
before do before do
allow(described_class).to receive(:connection).and_return(connection) allow(described_class).to receive(:connection).and_return(connection)
allow(described_class).to receive(:version).and_return(version)
allow(connection).to receive(:quote_column_name, &:itself) allow(connection).to receive(:quote_column_name, &:itself)
allow(connection).to receive(:quote, &:itself) allow(connection).to receive(:quote, &:itself)
allow(connection).to receive(:execute) allow(connection).to receive(:execute)
...@@ -242,6 +243,8 @@ describe Gitlab::Database do ...@@ -242,6 +243,8 @@ describe Gitlab::Database do
] ]
end end
let_it_be(:version) { 9.6 }
it 'does nothing with empty rows' do it 'does nothing with empty rows' do
expect(connection).not_to receive(:execute) expect(connection).not_to receive(:execute)
...@@ -307,6 +310,29 @@ describe Gitlab::Database do ...@@ -307,6 +310,29 @@ describe Gitlab::Database do
expect(ids).to eq([10]) expect(ids).to eq([10])
end end
context 'with version >= 9.5' do
it 'allows setting the upsert to do nothing' do
expect(connection)
.to receive(:execute)
.with(/ON CONFLICT DO NOTHING/)
described_class
.bulk_insert('test', [{ number: 10 }], on_conflict: :do_nothing)
end
end
context 'with version < 9.5' do
let(:version) { 9.4 }
it 'refuses setting the upsert' do
expect(connection)
.not_to receive(:execute)
.with(/ON CONFLICT/)
described_class
.bulk_insert('test', [{ number: 10 }], on_conflict: :do_nothing)
end
end
end end
end end
......
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