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

Add NPM dist tags support

See merge request gitlab-org/gitlab!20636

db/migrate/20191126134210_rename_packages_package_tags.rb

+# frozen_string_literal: true
+
+class RenamePackagesPackageTags < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    rename_table(:packages_package_tags, :packages_tags)
+  end
+end

db/migrate/20200106085831_add_timestamps_to_packages_tags.rb

+# frozen_string_literal: true
+
+class AddTimestampsToPackagesTags < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  # We disable these cops here because adding this column is safe. The table does not
+  # have any data in it.
+  # rubocop: disable Migration/AddIndex
+  def up
+    add_timestamps_with_timezone(:packages_tags, null: false)
+    add_index(:packages_tags, [:package_id, :updated_at], order: { updated_at: :desc })
+  end
+
+  # We disable these cops here because adding this column is safe. The table does not
+  # have any data in it.
+  # rubocop: disable Migration/RemoveIndex
+  def down
+    remove_index(:packages_tags, [:package_id, :updated_at])
+    remove_timestamps(:packages_tags)
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_01_06_071113) do
+ActiveRecord::Schema.define(version: 2020_01_06_085831) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -2938,12 +2938,6 @@ ActiveRecord::Schema.define(version: 2020_01_06_071113) do
     t.index ["package_id", "file_name"], name: "index_packages_package_files_on_package_id_and_file_name"
   end
 
-  create_table "packages_package_tags", force: :cascade do |t|
-    t.integer "package_id", null: false
-    t.string "name", limit: 255, null: false
-    t.index ["package_id"], name: "index_packages_package_tags_on_package_id"
-  end
-
   create_table "packages_packages", force: :cascade do |t|
     t.integer "project_id", null: false
     t.datetime_with_timezone "created_at", null: false
@@ -2956,6 +2950,15 @@ ActiveRecord::Schema.define(version: 2020_01_06_071113) do
     t.index ["project_id"], name: "index_packages_packages_on_project_id"
   end
 
+  create_table "packages_tags", force: :cascade do |t|
+    t.integer "package_id", null: false
+    t.string "name", limit: 255, null: false
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.index ["package_id", "updated_at"], name: "index_packages_tags_on_package_id_and_updated_at", order: { updated_at: :desc }
+    t.index ["package_id"], name: "index_packages_tags_on_package_id"
+  end
+
   create_table "pages_domain_acme_orders", force: :cascade do |t|
     t.integer "pages_domain_id", null: false
     t.datetime_with_timezone "expires_at", null: false
@@ -4705,8 +4708,8 @@ ActiveRecord::Schema.define(version: 2020_01_06_071113) do
   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_package_files", "packages_packages", column: "package_id", name: "fk_86f0f182f8", 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_tags", "packages_packages", column: "package_id", on_delete: :cascade
   add_foreign_key "pages_domain_acme_orders", "pages_domains", on_delete: :cascade
   add_foreign_key "pages_domains", "projects", name: "fk_ea2f6dfc6f", on_delete: :cascade
   add_foreign_key "path_locks", "projects", name: "fk_5265c98f24", on_delete: :cascade

doc/user/packages/npm_registry/index.md

@@ -242,3 +242,27 @@ Starting from GitLab 12.6, new packages published to the GitLab NPM Registry exp
   - bundleDependencies
   - peerDependencies
   - deprecated
+
+## NPM distribution tags
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/9425) in GitLab Premium 12.7.
+
+Dist Tags for newly published packages are supported, and they follow NPM's convention where they are optional, and each tag can only be assigned to 1 package at
+You can add [distribution tags](https://docs.npmjs.com/cli/dist-tag) for newly
+published packages. They follow NPM's convention where they are optional, and
+each tag can only be assigned to one package at a time. The latest tag is added
+by default when a package is published without a tag. The same goes to installing
+a package without specifying the tag or version.
+
+Examples of the supported `dist-tag` commands and using tags in general:
+
+```sh
+npm publish @scope/package --tag               # Publish new package with new tag
+npm dist-tag add @scope/package@version my-tag # Add a tag to an existing package
+npm dist-tag ls @scope/package                 # List all tags under the package
+npm dist-tag rm @scope/package@version my-tag  # Delete a tag from the package
+npm install @scope/package@my-tag              # Install a specific tag
+```
+
+CAUTION: **Warning:**
+Due to a bug in NPM 6.9.0, deleting dist tags fails. Make sure your NPM version is greater than 6.9.1.

ee/app/finders/packages/npm_packages_finder.rb

@@ -2,6 +2,8 @@
 class Packages::NpmPackagesFinder
   attr_reader :project, :package_name
 
+  delegate :find_by_version, to: :execute
+
   def initialize(project, package_name)
     @project = project
     @package_name = package_name

ee/app/finders/packages/tags_finder.rb

+# frozen_string_literal: true
+class Packages::TagsFinder
+  attr_reader :project, :package_name, :params
+
+  delegate :find_by_name, to: :execute
+
+  def initialize(project, package_name, params = {})
+    @project = project
+    @package_name = package_name
+    @params = params
+  end
+
+  def execute
+    packages = project.packages
+                      .with_name(package_name)
+    packages = packages.with_package_type(package_type) if package_type.present?
+
+    Packages::Tag.for_packages(packages)
+  end
+
+  private
+
+  def package_type
+    params[:package_type]
+  end
+end

ee/app/models/packages/package.rb

@@ -6,6 +6,7 @@ class Packages::Package < ApplicationRecord
   # 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 :dependency_links, inverse_of: :package, class_name: 'Packages::DependencyLink'
+  has_many :tags, inverse_of: :package, class_name: 'Packages::Tag'
   has_one :conan_metadatum, inverse_of: :package
   has_one :maven_metadatum, inverse_of: :package
   has_one :build_info, inverse_of: :package

ee/app/models/packages/tag.rb

+# frozen_string_literal: true
+class Packages::Tag < ApplicationRecord
+  belongs_to :package, inverse_of: :tags
+
+  validates :package, :name, presence: true
+
+  TAGS_LIMIT = 200.freeze
+
+  scope :preload_package, -> { preload(:package) }
+
+  def self.for_packages(packages, max_tags_limit = TAGS_LIMIT)
+    where(package_id: packages.select(:id))
+      .order(updated_at: :desc)
+      .limit(max_tags_limit)
+  end
+end

ee/app/presenters/npm_package_presenter.rb

@@ -27,13 +27,17 @@ class NpmPackagePresenter
   end
 
   def dist_tags
-    {
-      latest: sorted_versions.last
-    }
+    build_package_tags.tap { |t| t["latest"] ||= sorted_versions.last }
   end
 
   private
 
+  def build_package_tags
+    Hash[
+      package_tags.map { |tag| [tag.name, tag.package.version] }
+    ]
+  end
+
   def build_package_version(package, package_file)
     {
       name: package.name,
@@ -73,4 +77,9 @@ class NpmPackagePresenter
     versions = packages.map(&:version).compact
     VersionSorter.sort(versions)
   end
+
+  def package_tags
+    Packages::Tag.for_packages(packages)
+                 .preload_package
+  end
 end

ee/app/services/packages/create_npm_package_service.rb

-# frozen_string_literal: true
-module Packages
-  class CreateNpmPackageService < BaseService
-    def execute
-      name = params[:name]
-      version = params[:versions].keys.first
-      version_data = params[:versions][version]
-      build = params[:build]
-
-      existing_package = project.packages.npm.with_name(name).with_version(version)
-
-      return error('Package already exists.', 403) if existing_package.exists?
-
-      package = project.packages.create!(
-        name: name,
-        version: version,
-        package_type: 'npm'
-      )
-
-      if build.present?
-        package.create_build_info!(pipeline: build.pipeline)
-      end
-
-      package_file_name = "#{name}-#{version}.tgz"
-      attachment = params['_attachments'][package_file_name]
-
-      file_params = {
-        file:      CarrierWaveStringFile.new(Base64.decode64(attachment['data'])),
-        size:      attachment['length'],
-        file_sha1: version_data[:dist][:shasum],
-        file_name: package_file_name
-      }
-
-      package.transaction do
-        ::Packages::CreatePackageFileService.new(package, file_params).execute
-        ::Packages::CreateDependencyService.new(package, package_dependencies).execute
-      end
-
-      package
-    end
-
-    def package_dependencies
-      _version, version_data = params[:versions].first
-      version_data
-    end
-  end
-end

ee/app/services/packages/npm/create_package_service.rb

+# frozen_string_literal: true
+module Packages
+  module Npm
+    class CreatePackageService < BaseService
+      include Gitlab::Utils::StrongMemoize
+
+      def execute
+        return error('Version is empty.', 400) if version.blank?
+        return error('Package already exists.', 403) if current_package_exists?
+
+        ActiveRecord::Base.transaction { create_package! }
+      end
+
+      private
+
+      def create_package!
+        package = project.packages.create!(
+          name: name,
+          version: version,
+          package_type: 'npm'
+        )
+
+        if build.present?
+          package.create_build_info!(pipeline: build.pipeline)
+        end
+
+        ::Packages::CreatePackageFileService.new(package, file_params).execute
+        ::Packages::CreateDependencyService.new(package, package_dependencies).execute
+        ::Packages::Npm::CreateTagService.new(package, dist_tag).execute
+
+        package
+      end
+
+      def current_package_exists?
+        project.packages
+               .npm
+               .with_name(name)
+               .with_version(version)
+               .exists?
+      end
+
+      def name
+        params[:name]
+      end
+
+      def version
+        strong_memoize(:version) do
+          params[:versions].keys.first
+        end
+      end
+
+      def version_data
+        params[:versions][version]
+      end
+
+      def build
+        params[:build]
+      end
+
+      def dist_tag
+        params['dist-tags'].keys.first
+      end
+
+      def package_file_name
+        strong_memoize(:package_file_name) do
+          "#{name}-#{version}.tgz"
+        end
+      end
+
+      def attachment
+        strong_memoize(:attachment) do
+          params['_attachments'][package_file_name]
+        end
+      end
+
+      def file_params
+        {
+          file:      CarrierWaveStringFile.new(Base64.decode64(attachment['data'])),
+          size:      attachment['length'],
+          file_sha1: version_data[:dist][:shasum],
+          file_name: package_file_name
+        }
+      end
+
+      def package_dependencies
+        _version, versions_data = params[:versions].first
+        versions_data
+      end
+    end
+  end
+end

ee/app/services/packages/npm/create_tag_service.rb

+# frozen_string_literal: true
+module Packages
+  module Npm
+    class CreateTagService
+      include Gitlab::Utils::StrongMemoize
+
+      attr_reader :package, :tag_name
+
+      def initialize(package, tag_name)
+        @package = package
+        @tag_name = tag_name
+      end
+
+      def execute
+        if existing_tag.present?
+          existing_tag.update_column(:package_id, package.id)
+          existing_tag
+        else
+          package.tags.create!(name: tag_name)
+        end
+      end
+
+      private
+
+      def existing_tag
+        strong_memoize(:existing_tag) do
+          Packages::TagsFinder
+            .new(package.project, package.name, package_type: package.package_type)
+            .find_by_name(tag_name)
+        end
+      end
+    end
+  end
+end

ee/app/services/packages/remove_tag_service.rb

+# frozen_string_literal: true
+module Packages
+  class RemoveTagService < BaseService
+    attr_reader :package_tag
+
+    def initialize(package_tag)
+      raise ArgumentError, "Package tag must be set" if package_tag.blank?
+
+      @package_tag = package_tag
+    end
+
+    def execute
+      package_tag.delete
+    end
+  end
+end

ee/changelogs/unreleased/10io-add-npm-tag-support.yml

+---
+title: Add NPM dist-tag support
+merge_request: 20636
+author:
+type: added

ee/db/fixtures/development/26_packages.rb

@@ -16,7 +16,7 @@ class Gitlab::Seeder::Packages
           .gsub('1.0.1', version))
         .with_indifferent_access
 
-      ::Packages::CreateNpmPackageService.new(@project, @user, params).execute
+      ::Packages::Npm::CreatePackageService.new(@project, @user, params).execute
     end
   end
 

ee/lib/api/maven_packages.rb

@@ -169,7 +169,7 @@ module API
       end
       route_setting :authentication, job_token_allowed: true
       put ':id/packages/maven/*path/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do
-        authorize_create_package!
+        authorize_create_package!(user_project)
 
         require_gitlab_workhorse!
         Gitlab::Workhorse.verify_api_request!(headers)
@@ -195,7 +195,7 @@ module API
       end
       route_setting :authentication, job_token_allowed: true
       put ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do
-        authorize_create_package!
+        authorize_create_package!(user_project)
         require_gitlab_workhorse!
 
         file_name, format = extract_format(params[:file_name])

ee/lib/api/npm_packages.rb

@@ -17,8 +17,85 @@ module API
     helpers ::API::Helpers::PackagesHelpers
 
     helpers do
-      def find_project_by_package_name(name)
-        ::Packages::Package.npm.with_name(name).first&.project
+      def project_by_package_name
+        strong_memoize(:project_by_package_name) do
+          ::Packages::Package.npm.with_name(params[:package_name]).first&.project
+        end
+      end
+    end
+
+    desc 'Get all tags for a given an NPM package' do
+      detail 'This feature was introduced in GitLab 12.7'
+      success EE::API::Entities::NpmPackageTag
+    end
+    params do
+      requires :package_name, type: String, desc: 'Package name'
+    end
+    get 'packages/npm/-/package/*package_name/dist-tags', format: false, requirements: NPM_ENDPOINT_REQUIREMENTS do
+      package_name = params[:package_name]
+
+      bad_request!('Package Name') if package_name.blank?
+
+      authorize_read_package!(project_by_package_name)
+      authorize_packages_feature!(project_by_package_name)
+
+      packages = ::Packages::NpmPackagesFinder.new(project_by_package_name, package_name)
+                                              .execute
+
+      present NpmPackagePresenter.new(package_name, packages),
+              with: EE::API::Entities::NpmPackageTag
+    end
+
+    params do
+      requires :package_name, type: String, desc: 'Package name'
+      requires :tag, type: String, desc: "Package dist-tag"
+    end
+    namespace 'packages/npm/-/package/*package_name/dist-tags/:tag', requirements: NPM_ENDPOINT_REQUIREMENTS do
+      desc 'Create or Update the given tag for the given NPM package and version' do
+        detail 'This feature was introduced in GitLab 12.7'
+      end
+      put format: false do
+        package_name = params[:package_name]
+        version = env['api.request.body']
+        tag = params[:tag]
+
+        bad_request!('Package Name') if package_name.blank?
+        bad_request!('Version') if version.blank?
+        bad_request!('Tag') if tag.blank?
+
+        authorize_create_package!(project_by_package_name)
+
+        package = ::Packages::NpmPackagesFinder
+          .new(project_by_package_name, package_name)
+          .find_by_version(version)
+        not_found!('Package') unless package
+
+        ::Packages::Npm::CreateTagService.new(package, tag).execute
+
+        no_content!
+      end
+
+      desc 'Deletes the given tag' do
+        detail 'This feature was introduced in GitLab 12.7'
+      end
+      delete format: false do
+        package_name = params[:package_name]
+        tag = params[:tag]
+
+        bad_request!('Package Name') if package_name.blank?
+        bad_request!('Tag') if tag.blank?
+
+        authorize_destroy_package!(project_by_package_name)
+
+        package_tag = ::Packages::TagsFinder
+          .new(project_by_package_name, package_name, package_type: :npm)
+          .find_by_name(tag)
+
+        not_found!('Package tag') unless package_tag
+
+        ::Packages::RemoveTagService.new(package_tag).execute
+
+        no_content!
       end
     end
 
@@ -32,13 +109,11 @@ module API
     get 'packages/npm/*package_name', format: false, requirements: NPM_ENDPOINT_REQUIREMENTS do
       package_name = params[:package_name]
 
-      project = find_project_by_package_name(package_name)
-
-      authorize_read_package!(project)
-      authorize_packages_feature!(project)
+      authorize_read_package!(project_by_package_name)
+      authorize_packages_feature!(project_by_package_name)
 
       packages = ::Packages::NpmPackagesFinder
-        .new(project, package_name).execute
+        .new(project_by_package_name, package_name).execute
 
       present NpmPackagePresenter.new(package_name, packages),
         with: EE::API::Entities::NpmPackage
@@ -81,9 +156,9 @@ module API
       end
       route_setting :authentication, job_token_allowed: true
       put ':id/packages/npm/:package_name', requirements: NPM_ENDPOINT_REQUIREMENTS do
-        authorize_create_package!
+        authorize_create_package!(user_project)
 
-        created_package = ::Packages::CreateNpmPackageService
+        created_package = ::Packages::Npm::CreatePackageService
           .new(user_project, current_user, params.merge(build: current_authenticated_job)).execute
 
         if created_package[:status] == :error

ee/lib/api/project_packages.rb

@@ -48,7 +48,7 @@ module API
         requires :package_id, type: Integer, desc: 'The ID of a package'
       end
       delete ':id/packages/:package_id' do
-        authorize_destroy_package!
+        authorize_destroy_package!(user_project)
 
         package = ::Packages::PackageFinder
           .new(user_project, params[:package_id]).execute

ee/lib/ee/api/entities.rb

@@ -865,6 +865,10 @@ module EE
         expose :dist_tags, as: 'dist-tags'
       end
 
+      class NpmPackageTag < Grape::Entity
+        expose :dist_tags, merge: true
+      end
+
       class Package < Grape::Entity
         include ::API::Helpers::RelatedResourcesHelpers
         extend EntityHelpers

ee/spec/factories/packages.rb

@@ -205,4 +205,9 @@ FactoryBot.define do
     dependency { create(:packages_dependency) }
     dependency_type { :dependencies }
   end
+
+  factory :packages_tag, class: Packages::Tag do
+    package
+    sequence(:name) { |n| "tag-#{n}"}
+  end
 end

ee/spec/finders/packages/npm_packages_finder_spec.rb

@@ -4,18 +4,31 @@ require 'spec_helper'
 describe Packages::NpmPackagesFinder do
   let(:package) { create(:npm_package) }
   let(:project) { package.project }
+  let(:package_name) { package.name }
 
   describe '#execute!' do
-    it 'returns project packages' do
-      finder = described_class.new(project, package.name)
+    subject { described_class.new(project, package_name).execute }
 
-      expect(finder.execute).to eq([package])
+    it { is_expected.to eq([package]) }
+
+    context 'with unknown package name' do
+      let(:package_name) { 'baz' }
+
+      it { is_expected.to be_empty }
     end
+  end
+
+  describe '#find_by_version' do
+    let(:version) { package.version }
+
+    subject { described_class.new(project, package.name).find_by_version(version) }
+
+    it { is_expected.to eq(package) }
 
-    it 'returns an empty collection' do
-      finder = described_class.new(project, 'baz')
+    context 'with unknown version' do
+      let(:version) { 'foobar' }
 
-      expect(finder.execute).to be_empty
+      it { is_expected.to be_nil }
     end
   end
 end

ee/spec/finders/packages/tags_finder_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Packages::TagsFinder do
+  let(:package) { create(:npm_package) }
+  let(:project) { package.project }
+  let!(:tag1) { create(:packages_tag, package: package) }
+  let!(:tag2) { create(:packages_tag, package: package) }
+  let(:package_name) { package.name }
+  let(:params) { {} }
+
+  describe '#execute' do
+    subject { described_class.new(project, package_name, params).execute }
+
+    it { is_expected.to match_array([tag1, tag2]) }
+
+    context 'with package type' do
+      let(:package_maven) { create(:maven_package, project: project) }
+      let!(:tag_maven) { create(:packages_tag, package: package_maven) }
+      let(:package_name) { package_maven.name }
+      let(:params) { { package_type: package_maven.package_type } }
+
+      it { is_expected.to match_array([tag_maven]) }
+    end
+
+    context 'with blank package type' do
+      let(:params) { { package_type: '   ' } }
+
+      it { is_expected.to match_array([tag1, tag2]) }
+    end
+
+    context 'with nil package type' do
+      let(:params) { { package_type: nil } }
+
+      it { is_expected.to match_array([tag1, tag2]) }
+    end
+
+    context 'with unknown package name' do
+      let(:package_name) { 'foobar' }
+
+      it { is_expected.to be_empty }
+    end
+  end
+
+  describe '#find_by_name' do
+    let(:tag_name) { tag1.name }
+
+    subject { described_class.new(project, package_name, params).execute.find_by_name(tag_name) }
+
+    it { is_expected.to eq(tag1) }
+
+    context 'with package type' do
+      let(:package_maven) { create(:maven_package, project: project) }
+      let!(:tag_maven) { create(:packages_tag, package: package_maven) }
+      let(:package_name) { package_maven.name }
+      let(:params) { { package_type: package_maven.package_type } }
+      let(:tag_name) { tag_maven.name }
+
+      it { is_expected.to eq(tag_maven) }
+    end
+
+    context 'with unknown tag_name' do
+      let(:tag_name) { 'foobar' }
+
+      it { is_expected.to be_nil }
+    end
+  end
+end

ee/spec/fixtures/api/schemas/public_api/v4/packages/npm_package_tags.json

+{
+  "type": "object",
+  "properties" : {
+    "$tag": { "type": "string" },
+    "$version": { "type": "string" }
+    }
+}

ee/spec/models/packages/package_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe Packages::Package, type: :model do
     it { is_expected.to belong_to(:project) }
     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_many(:tags).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

ee/spec/models/packages/tag_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Packages::Tag, type: :model do
+  let!(:project) { create(:project) }
+  let!(:package) { create(:npm_package, version: '1.0.2', project: project, updated_at: 3.days.ago) }
+
+  describe 'relationships' do
+    it { is_expected.to belong_to(:package).inverse_of(:tags) }
+  end
+
+  describe 'validations' do
+    subject { create(:packages_tag) }
+
+    it { is_expected.to validate_presence_of(:package) }
+    it { is_expected.to validate_presence_of(:name) }
+  end
+
+  describe '.for_packages' do
+    let(:package2) { create(:package, project: project, updated_at: 2.days.ago) }
+    let(:package3) { create(:package, project: project, updated_at: 1.day.ago) }
+    let(:tags_limit) { Packages::Tag::TAGS_LIMIT }
+    let!(:tag1) { create(:packages_tag, package: package) }
+    let!(:tag2) { create(:packages_tag, package: package2) }
+    let!(:tag3) { create(:packages_tag, package: package3) }
+
+    subject { described_class.for_packages(project.packages, tags_limit) }
+
+    it { is_expected.to match_array([tag1, tag2, tag3]) }
+
+    context 'with too many tags' do
+      let(:tags_limit) { 2 }
+
+      it { is_expected.to match_array([tag2, tag3]) }
+    end
+  end
+end

ee/spec/presenters/npm_package_presenter_spec.rb

@@ -39,11 +39,27 @@ describe NpmPackagePresenter do
     end
   end
 
-  describe '#dist-tags' do
+  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) }
+    context 'for packages without tags' do
+      it { is_expected.to be_a(Hash) }
+      it { expect(subject["latest"]).to eq(latest_package.version) }
+    end
+
+    context 'for packages with tags' do
+      let!(:package_tag1) { create(:packages_tag, package: package1, name: 'release_a') }
+      let!(:package_tag2) { create(:packages_tag, package: package1, name: 'test_release') }
+      let!(:package_tag3) { create(:packages_tag, package: package2, name: 'release_b') }
+      let!(:package_tag4) { create(:packages_tag, package: latest_package, name: 'release_c') }
+      let!(:package_tag5) { create(:packages_tag, package: latest_package, name: 'latest') }
+
+      it { is_expected.to be_a(Hash) }
+      it { expect(subject[package_tag1.name]).to eq(package1.version) }
+      it { expect(subject[package_tag2.name]).to eq(package1.version) }
+      it { expect(subject[package_tag3.name]).to eq(package2.version) }
+      it { expect(subject[package_tag4.name]).to eq(latest_package.version) }
+      it { expect(subject[package_tag5.name]).to eq(latest_package.version) }
+    end
   end
 end

ee/spec/requests/api/npm_packages_spec.rb

@@ -200,6 +200,17 @@ describe API::NpmPackages do
 
             expect(response).to have_gitlab_http_status(400)
           end
+
+          context 'with empty versions' do
+            let(:params) { upload_params(package_name).merge!(versions: {}) }
+
+            it 'throws a 400 error' do
+              expect { upload_package_with_token(package_name, params) }
+              .not_to change { project.packages.count }
+
+              expect(response).to have_gitlab_http_status(400)
+            end
+          end
         end
 
         context 'invalid package name' do
@@ -223,6 +234,7 @@ describe API::NpmPackages 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::Tag.count }.by(1)
 
           expect(response).to have_gitlab_http_status(200)
         end
@@ -320,6 +332,177 @@ describe API::NpmPackages do
     end
   end
 
+  describe 'GET /api/v4/packages/npm/-/package/*package_name/dist-tags' do
+    let(:package) { create(:npm_package, project: project) }
+    let!(:package_tag1) { create(:packages_tag, package: package) }
+    let!(:package_tag2) { create(:packages_tag, package: package) }
+    let(:package_name) { package.name }
+    let(:user) { create(:user) }
+    let(:url) { "/packages/npm/-/package/#{package_name}/dist-tags" }
+
+    subject { get api(url) }
+
+    context 'with packages feature enabled' do
+      before do
+        stub_licensed_features(packages: true)
+      end
+
+      context 'with public project' do
+        context 'with authenticated user' do
+          subject { get api(url, user) }
+
+          it_behaves_like 'returns package tags', :maintainer
+          it_behaves_like 'returns package tags', :developer
+          it_behaves_like 'returns package tags', :reporter
+          it_behaves_like 'returns package tags', :guest
+        end
+
+        context 'with unauthenticated user' do
+          it_behaves_like 'returns package tags', :no_type
+        end
+      end
+
+      context 'with private project' do
+        let(:project) { create(:project, :private) }
+
+        context 'with authenticated user' do
+          subject { get api(url, user) }
+
+          it_behaves_like 'returns package tags', :maintainer
+          it_behaves_like 'returns package tags', :developer
+          it_behaves_like 'returns package tags', :reporter
+          it_behaves_like 'rejects package tags access', :guest, :forbidden
+        end
+
+        context 'with unauthenticated user' do
+          it_behaves_like 'rejects package tags access', :no_type, :forbidden
+        end
+      end
+    end
+
+    context 'with packages feature disabled' do
+      before do
+        stub_licensed_features(packages: false)
+      end
+
+      it_behaves_like 'rejects package tags access', :no_type, :forbidden
+    end
+  end
+
+  describe 'PUT /api/v4/packages/npm/-/package/*package_name/dist-tags/:tag' do
+    let(:package) { create(:npm_package, project: project) }
+    let(:package_name) { package.name }
+    let(:user) { create(:user) }
+    let(:tag_name) { 'test' }
+    let(:version) { package.version }
+    let(:url) { "/packages/npm/-/package/#{package_name}/dist-tags/#{tag_name}" }
+
+    subject { put api(url), env: { 'api.request.body': version } }
+
+    context 'with packages feature enabled' do
+      before do
+        stub_licensed_features(packages: true)
+      end
+
+      context 'with public project' do
+        context 'with authenticated user' do
+          subject { put api(url, user), env: { 'api.request.body': version } }
+
+          it_behaves_like 'create package tag', :maintainer
+          it_behaves_like 'create package tag', :developer
+          it_behaves_like 'rejects package tags access', :reporter, :forbidden
+          it_behaves_like 'rejects package tags access', :guest, :forbidden
+        end
+
+        context 'with unauthenticated user' do
+          it_behaves_like 'rejects package tags access', :no_type, :unauthorized
+        end
+      end
+
+      context 'with private project' do
+        let(:project) { create(:project, :private) }
+
+        context 'with authenticated user' do
+          subject { put api(url, user), env: { 'api.request.body': version } }
+
+          it_behaves_like 'create package tag', :maintainer
+          it_behaves_like 'create package tag', :developer
+          it_behaves_like 'rejects package tags access', :reporter, :forbidden
+          it_behaves_like 'rejects package tags access', :guest, :forbidden
+        end
+
+        context 'with unauthenticated user' do
+          it_behaves_like 'rejects package tags access', :no_type, :unauthorized
+        end
+      end
+    end
+
+    context 'with packages feature disabled' do
+      before do
+        stub_licensed_features(packages: false)
+      end
+
+      it_behaves_like 'rejects package tags access', :no_type, :unauthorized
+    end
+  end
+
+  describe 'DELETE /api/v4/packages/npm/-/package/*package_name/dist-tags/:tag' do
+    let(:package) { create(:npm_package, project: project) }
+    let(:package_tag) { create(:packages_tag, package: package) }
+    let(:user) { create(:user) }
+    let(:package_name) { package.name }
+    let(:tag_name) { package_tag.name }
+    let(:url) { "/packages/npm/-/package/#{package_name}/dist-tags/#{tag_name}" }
+
+    subject { delete api(url) }
+
+    context 'with packages feature enabled' do
+      before do
+        stub_licensed_features(packages: true)
+      end
+
+      context 'with public project' do
+        context 'with authenticated user' do
+          subject { delete api(url, user) }
+
+          it_behaves_like 'delete package tag', :maintainer
+          it_behaves_like 'rejects package tags access', :developer, :forbidden
+          it_behaves_like 'rejects package tags access', :reporter, :forbidden
+          it_behaves_like 'rejects package tags access', :guest, :forbidden
+        end
+
+        context 'with unauthenticated user' do
+          it_behaves_like 'rejects package tags access', :no_type, :unauthorized
+        end
+      end
+
+      context 'with private project' do
+        let(:project) { create(:project, :private) }
+
+        context 'with authenticated user' do
+          subject { delete api(url, user) }
+
+          it_behaves_like 'delete package tag', :maintainer
+          it_behaves_like 'rejects package tags access', :developer, :forbidden
+          it_behaves_like 'rejects package tags access', :reporter, :forbidden
+          it_behaves_like 'rejects package tags access', :guest, :forbidden
+        end
+
+        context 'with unauthenticated user' do
+          it_behaves_like 'rejects package tags access', :no_type, :unauthorized
+        end
+      end
+    end
+
+    context 'with packages feature disabled' do
+      before do
+        stub_licensed_features(packages: false)
+      end
+
+      it_behaves_like 'rejects package tags access', :no_type, :unauthorized
+    end
+  end
+
   def expect_a_valid_package_response
     expect(response).to have_gitlab_http_status(200)
     expect(response.content_type.to_s).to eq('application/json')
@@ -329,5 +512,6 @@ describe API::NpmPackages do
     NpmPackagePresenter::NPM_VALID_DEPENDENCY_TYPES.each do |dependency_type|
       expect(json_response.dig('versions', package.version, dependency_type.to_s)).to be_any
     end
+    expect(json_response['dist-tags']).to match_schema('public_api/v4/packages/npm_package_tags', dir: 'ee')
   end
 end

ee/spec/services/packages/create_npm_package_service_spec.rb → ee/spec/services/packages/npm/create_package_service_spec.rb

 # frozen_string_literal: true
 require 'spec_helper'
 
-describe Packages::CreateNpmPackageService do
+describe Packages::Npm::CreatePackageService do
   let(:namespace) {create(:namespace)}
   let(:project) { create(:project, namespace: namespace) }
   let(:user) { create(:user) }
@@ -11,9 +11,12 @@ describe Packages::CreateNpmPackageService do
     JSON.parse(
       fixture_file('npm/payload.json', dir: 'ee')
         .gsub('@root/npm-test', package_name)
-        .gsub('1.0.1', version))
-        .with_indifferent_access
+        .gsub('1.0.1', version)
+    ).with_indifferent_access
+      .merge!(override)
   end
+  let(:override) { {} }
+  let(:package_name) { "@#{namespace.path}/my-app".freeze }
 
   subject { described_class.new(project, user, params).execute }
 
@@ -22,6 +25,16 @@ describe Packages::CreateNpmPackageService do
       expect { subject }
         .to change { Packages::Package.count }.by(1)
         .and change { Packages::Package.npm.count }.by(1)
+        .and change { Packages::Tag.count }.by(1)
+    end
+
+    it { is_expected.to be_valid }
+
+    it 'creates a package with name and version' do
+      package = subject
+
+      expect(package.name).to eq(package_name)
+      expect(package.version).to eq(version)
     end
 
     it { is_expected.to be_valid }
@@ -31,9 +44,6 @@ describe Packages::CreateNpmPackageService do
 
   describe '#execute' do
     context 'scoped package' do
-      let(:package_name) { "@#{namespace.path}/my-app".freeze }
-      let(:package) { subject }
-
       it_behaves_like 'valid package'
 
       it_behaves_like 'assigns build to package'
@@ -60,5 +70,12 @@ describe Packages::CreateNpmPackageService do
         expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
       end
     end
+
+    context 'with empty versions' do
+      let(:override) { { versions: {} } }
+
+      it { expect(subject[:http_status]).to eq 400 }
+      it { expect(subject[:message]).to eq 'Version is empty.' }
+    end
   end
 end

ee/spec/services/packages/npm/create_tag_service_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Packages::Npm::CreateTagService do
+  let(:package) { create(:npm_package) }
+  let(:tag_name) { 'test-tag' }
+
+  describe '#execute' do
+    subject { described_class.new(package, tag_name).execute }
+
+    shared_examples 'it creates the tag' do
+      it { expect { subject }.to change { Packages::Tag.count }.by(1) }
+      it { expect(subject.name).to eq(tag_name) }
+      it 'adds tag to the package' do
+        tag = subject
+        expect(package.reload.tags).to match_array([tag])
+      end
+    end
+
+    context 'with no existing tag name' do
+      it_behaves_like 'it creates the tag'
+    end
+
+    context 'with exisiting tag name' do
+      let!(:package_tag2) { create(:packages_tag, package: package2, name: tag_name) }
+
+      context 'on package with different name' do
+        let!(:package2) { create(:npm_package, project: package.project) }
+
+        it_behaves_like 'it creates the tag'
+      end
+
+      context 'on different package type' do
+        let!(:package2) { create(:conan_package, project: package.project, name: package.name, version: package.version) }
+
+        it_behaves_like 'it creates the tag'
+      end
+
+      context 'on same package with different version' do
+        let!(:package2) { create(:npm_package, project: package.project, name: package.name, version: '5.0.0testing') }
+
+        it { expect { subject }.to not_change { Packages::Tag.count } }
+        it { expect(subject.name).to eq(tag_name) }
+
+        it 'adds tag to the package' do
+          tag = subject
+          expect(package.reload.tags).to match_array([tag])
+          expect(package2.reload.tags).to be_empty
+        end
+      end
+    end
+  end
+end

ee/spec/services/packages/remove_tag_service_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Packages::RemoveTagService do
+  let!(:package_tag) { create(:packages_tag) }
+
+  describe '#execute' do
+    subject { described_class.new(package_tag).execute }
+
+    context 'with existing tag' do
+      it { expect { subject }.to change { Packages::Tag.count }.by(-1) }
+    end
+
+    context 'with nil' do
+      subject { described_class.new(nil) }
+
+      it { expect { subject }.to raise_error(ArgumentError) }
+    end
+  end
+end

ee/spec/support/shared_examples/packages_shared_examples.rb

@@ -6,6 +6,8 @@ shared_examples 'assigns build to package' do
     let(:params) { super().merge(build: job) }
 
     it 'assigns the pipeline to the package' do
+      package = subject
+
       expect(package.build_info).to be_present
       expect(package.build_info.pipeline).to eq job.pipeline
     end
@@ -72,11 +74,7 @@ shared_examples 'rejects packages access' do |container_type, user_type, status|
       send(container_type)&.send("add_#{user_type}", user) unless user_type == :no_type
     end
 
-    it "returns #{status}" do
-      subject
-
-      expect(response).to have_gitlab_http_status(status)
-    end
+    it_behaves_like 'returning response status', status
   end
 end
 

ee/spec/support/shared_examples/packages_tags_shared_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'rejects package tags access' do |user_type, status|
+  context "for user type #{user_type}" do
+    before do
+      project.send("add_#{user_type}", user) unless user_type == :no_type
+    end
+
+    it_behaves_like 'returning response status', status
+  end
+end
+
+shared_examples 'returns package tags' do |user_type|
+  using RSpec::Parameterized::TableSyntax
+
+  before do
+    project.send("add_#{user_type}", user) unless user_type == :no_type
+  end
+
+  it_behaves_like 'returning response status', :success
+
+  it 'returns a valid json response' do
+    subject
+
+    expect(response.content_type.to_s).to eq('application/json')
+    expect(json_response).to be_a(Hash)
+  end
+
+  it 'returns two package tags' do
+    subject
+
+    expect(json_response).to match_schema('public_api/v4/packages/npm_package_tags', dir: 'ee')
+    expect(json_response.length).to eq(3) # two tags + latest (auto added)
+    expect(json_response[package_tag1.name]).to eq(package.version)
+    expect(json_response[package_tag2.name]).to eq(package.version)
+    expect(json_response['latest']).to eq(package.version)
+  end
+
+  context 'with invalid package name' do
+    where(:package_name, :status) do
+      '%20' | :bad_request
+      nil   | :forbidden
+    end
+
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
+    end
+  end
+end
+
+shared_examples 'create package tag' do |user_type|
+  using RSpec::Parameterized::TableSyntax
+
+  before do
+    project.send("add_#{user_type}", user) unless user_type == :no_type
+  end
+
+  it_behaves_like 'returning response status', :no_content
+
+  it 'creates the package tag' do
+    expect { subject }.to change { Packages::Tag.count }.by(1)
+
+    last_tag = Packages::Tag.last
+    expect(last_tag.name).to eq(tag_name)
+    expect(last_tag.package).to eq(package)
+  end
+
+  it 'returns a valid response' do
+    subject
+
+    expect(response.body).to be_empty
+  end
+
+  context 'with already existing tag' do
+    let(:package2) { create(:npm_package, project: project, name: package.name, version: '5.5.55') }
+    let!(:tag) { create(:packages_tag, package: package2, name: tag_name) }
+
+    it_behaves_like 'returning response status', :no_content
+
+    it 'reuses existing tag' do
+      expect(package.tags).to be_empty
+      expect(package2.tags).to eq([tag])
+      expect { subject }.to not_change { Packages::Tag.count }
+      expect(package.reload.tags).to eq([tag])
+      expect(package2.reload.tags).to be_empty
+    end
+
+    it 'returns a valid response' do
+      subject
+
+      expect(response.body).to be_empty
+    end
+  end
+
+  context 'with invalid package name' do
+    where(:package_name, :status) do
+      'unknown' | :forbidden
+      ''        | :not_found
+      '%20'     | :bad_request
+    end
+
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
+    end
+  end
+
+  context 'with invalid tag name' do
+    where(:tag_name, :status) do
+      ''    | :not_found
+      '%20' | :bad_request
+    end
+
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
+    end
+  end
+
+  context 'with invalid version' do
+    where(:version, :status) do
+      ' '   | :bad_request
+      ''    | :bad_request
+      nil   | :bad_request
+    end
+
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
+    end
+  end
+end
+
+shared_examples 'delete package tag' do |user_type|
+  using RSpec::Parameterized::TableSyntax
+
+  before do
+    project.send("add_#{user_type}", user) unless user_type == :no_type
+  end
+
+  context "for #{user_type} user" do
+    it_behaves_like 'returning response status', :no_content
+
+    it 'returns a valid response' do
+      subject
+
+      expect(response.body).to be_empty
+    end
+
+    it 'destroy the package tag' do
+      expect(package.tags).to eq([package_tag])
+      expect { subject }.to change { Packages::Tag.count }.by(-1)
+      expect(package.reload.tags).to be_empty
+    end
+
+    context 'with tag from other package' do
+      let(:package2) { create(:npm_package, project: project) }
+      let(:package_tag) { create(:packages_tag, package: package2) }
+
+      it_behaves_like 'returning response status', :not_found
+    end
+
+    context 'with invalid package name' do
+      where(:package_name, :status) do
+        'unknown' | :forbidden
+        ''        | :not_found
+        '%20'     | :bad_request
+      end
+
+      with_them do
+        it_behaves_like 'returning response status', params[:status]
+      end
+    end
+
+    context 'with invalid tag name' do
+      where(:tag_name, :status) do
+        'unknown' | :not_found
+        ''        | :not_found
+        '%20'     | :bad_request
+      end
+
+      with_them do
+        it_behaves_like 'returning response status', params[:status]
+      end
+    end
+  end
+end