Merge branch '214727-composer-publish-package-mvp' into 'master'

PHP Composer: Publish Package MVP

See merge request gitlab-org/gitlab!30448

db/migrate/20200429023324_add_composer_metadata.rb

+# frozen_string_literal: true
+
+class AddComposerMetadata < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    create_table :packages_composer_metadata, id: false do |t|
+      t.references :package, primary_key: true, index: false, default: nil, foreign_key: { to_table: :packages_packages, on_delete: :cascade }, type: :bigint
+      t.binary :target_sha, null: false
+    end
+  end
+end

db/migrate/20200528054112_add_index_to_package_name.rb

+# frozen_string_literal: true
+
+class AddIndexToPackageName < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'package_name_index'.freeze
+
+  def up
+    add_concurrent_index(:packages_packages, :name, name: INDEX_NAME)
+  end
+
+  def down
+    remove_concurrent_index(:packages_packages, :name, name: INDEX_NAME)
+  end
+end

db/structure.sql

@@ -4611,6 +4611,11 @@ CREATE SEQUENCE public.packages_build_infos_id_seq
 
 ALTER SEQUENCE public.packages_build_infos_id_seq OWNED BY public.packages_build_infos.id;
 
+CREATE TABLE public.packages_composer_metadata (
+    package_id bigint NOT NULL,
+    target_sha bytea NOT NULL
+);
+
 CREATE TABLE public.packages_conan_file_metadata (
     id bigint NOT NULL,
     package_file_id bigint NOT NULL,
@@ -8618,6 +8623,9 @@ ALTER TABLE ONLY public.operations_user_lists
 ALTER TABLE ONLY public.packages_build_infos
     ADD CONSTRAINT packages_build_infos_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.packages_composer_metadata
+    ADD CONSTRAINT packages_composer_metadata_pkey PRIMARY KEY (package_id);
+
 ALTER TABLE ONLY public.packages_conan_file_metadata
     ADD CONSTRAINT packages_conan_file_metadata_pkey PRIMARY KEY (id);
 
@@ -11074,6 +11082,8 @@ CREATE INDEX note_mentions_temp_index ON public.notes USING btree (id, noteable_
 
 CREATE UNIQUE INDEX one_canonical_wiki_page_slug_per_metadata ON public.wiki_page_slugs USING btree (wiki_page_meta_id) WHERE (canonical = true);
 
+CREATE INDEX package_name_index ON public.packages_packages USING btree (name);
+
 CREATE INDEX packages_packages_verification_checksum_partial ON public.packages_package_files USING btree (verification_checksum) WHERE (verification_checksum IS NOT NULL);
 
 CREATE INDEX packages_packages_verification_failure_partial ON public.packages_package_files USING btree (verification_failure) WHERE (verification_failure IS NOT NULL);
@@ -12449,6 +12459,9 @@ ALTER TABLE ONLY public.ci_build_trace_sections
 ALTER TABLE ONLY public.clusters
     ADD CONSTRAINT fk_rails_ac3a663d79 FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY public.packages_composer_metadata
+    ADD CONSTRAINT fk_rails_ad48c2e5bb FOREIGN KEY (package_id) REFERENCES public.packages_packages(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.analytics_cycle_analytics_group_stages
     ADD CONSTRAINT fk_rails_ae5da3409b FOREIGN KEY (group_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;
 
@@ -13698,6 +13711,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200429001827
 20200429002150
 20200429015603
+20200429023324
 20200429181335
 20200429181955
 20200429182245
@@ -13775,6 +13789,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200527151413
 20200527152116
 20200527152657
+20200528054112
 20200528123703
 20200603073101
 \.

ee/app/models/packages/composer/metadatum.rb

+# frozen_string_literal: true
+
+module Packages
+  module Composer
+    class Metadatum < ApplicationRecord
+      self.table_name = 'packages_composer_metadata'
+      self.primary_key = :package_id
+
+      belongs_to :package, -> { where(package_type: :composer) }, inverse_of: :composer_metadatum
+
+      validates :package, presence: true
+    end
+  end
+end

ee/app/models/packages/package.rb

@@ -13,6 +13,7 @@ class Packages::Package < ApplicationRecord
   has_one :pypi_metadatum, inverse_of: :package, class_name: 'Packages::Pypi::Metadatum'
   has_one :maven_metadatum, inverse_of: :package, class_name: 'Packages::Maven::Metadatum'
   has_one :nuget_metadatum, inverse_of: :package, class_name: 'Packages::Nuget::Metadatum'
+  has_one :composer_metadatum, inverse_of: :package, class_name: 'Packages::Composer::Metadatum'
   has_one :build_info, inverse_of: :package
 
   accepts_nested_attributes_for :conan_metadatum
@@ -30,6 +31,7 @@ class Packages::Package < ApplicationRecord
 
   validate :valid_conan_package_recipe, if: :conan?
   validate :valid_npm_package_name, if: :npm?
+  validate :valid_composer_global_name, if: :composer?
   validate :package_already_taken, if: :npm?
   validates :version, format: { with: Gitlab::Regex.semver_regex }, if: -> { npm? || nuget? }
   validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
@@ -158,6 +160,15 @@ class Packages::Package < ApplicationRecord
     errors.add(:base, _('Package recipe already exists')) if recipe_exists
   end
 
+  def valid_composer_global_name
+    # .default_scoped is required here due to a bug in rails that leaks
+    # the scope and adds `self` to the query incorrectly
+    # See https://github.com/rails/rails/pull/35186
+    if Packages::Package.default_scoped.composer.with_name(name).where.not(project_id: project_id).exists?
+      errors.add(:name, 'is already taken by another project')
+    end
+  end
+
   def valid_npm_package_name
     return unless project&.root_namespace
 

ee/app/services/packages/composer/composer_json_service.rb

+# frozen_string_literal: true
+
+module Packages
+  module Composer
+    class ComposerJsonService
+      def initialize(project, target)
+        @project, @target = project, target
+      end
+
+      def execute
+        composer_json
+      end
+
+      private
+
+      def composer_json
+        composer_file = @project.repository.blob_at(@target, 'composer.json')
+
+        composer_file_not_found! unless composer_file
+
+        Gitlab::Json.parse(composer_file.data)
+      rescue JSON::ParserError
+        raise 'Could not parse composer.json file. Invalid JSON.'
+      end
+
+      def composer_file_not_found!
+        raise 'The file composer.json was not found.'
+      end
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+module Packages
+  module Composer
+    class CreatePackageService < BaseService
+      include ::Gitlab::Utils::StrongMemoize
+
+      def execute
+        # fetches json outside of transaction
+        composer_json
+
+        ::Packages::Package.transaction do
+          ::Packages::Composer::Metadatum.upsert(
+            package_id: created_package.id,
+            target_sha: target
+          )
+        end
+      end
+
+      private
+
+      def created_package
+        project
+          .packages
+          .composer
+          .safe_find_or_create_by!(name: package_name, version: package_version)
+      end
+
+      def composer_json
+        strong_memoize(:composer_json) do
+          ::Packages::Composer::ComposerJsonService.new(project, target).execute
+        end
+      end
+
+      def package_name
+        composer_json['name']
+      end
+
+      def target
+        (branch || tag).target
+      end
+
+      def branch
+        params[:branch]
+      end
+
+      def tag
+        params[:tag]
+      end
+
+      def package_version
+        ::Packages::Composer::VersionParserService.new(tag_name: tag&.name, branch_name: branch&.name).execute
+      end
+    end
+  end
+end

ee/app/services/packages/composer/version_parser_service.rb

+# frozen_string_literal: true
+
+module Packages
+  module Composer
+    class VersionParserService
+      def initialize(tag_name: nil, branch_name: nil)
+        @tag_name, @branch_name = tag_name, branch_name
+      end
+
+      def execute
+        if @tag_name.present?
+          @tag_name.match(Gitlab::Regex.composer_package_version_regex).captures[0]
+        elsif @branch_name.present?
+          branch_sufix_or_prefix(@branch_name.match(Gitlab::Regex.composer_package_version_regex))
+        end
+      end
+
+      private
+
+      def branch_sufix_or_prefix(match)
+        if match
+          if match.captures[1] == '.x'
+            match.captures[0] + '-dev'
+          else
+            match.captures[0] + '.x-dev'
+          end
+        else
+          "dev-#{@branch_name}"
+        end
+      end
+    end
+  end
+end

ee/lib/api/composer_packages.rb

@@ -86,6 +86,7 @@ module API
       params do
         optional :branch, type: String, desc: 'The name of the branch'
         optional :tag, type: String, desc: 'The name of the tag'
+        exactly_one_of :tag, :branch
       end
 
       namespace ':id/packages/composer' do
@@ -93,13 +94,17 @@ module API
           authorize_create_package!(authorized_user_project)
 
           if params[:branch].present?
-            find_branch!(params[:branch])
+            params[:branch] = find_branch!(params[:branch])
           elsif params[:tag].present?
-            find_tag!(params[:tag])
+            params[:tag] = find_tag!(params[:tag])
           else
             bad_request!
           end
 
+          ::Packages::Composer::CreatePackageService
+            .new(authorized_user_project, current_user, declared_params)
+            .execute
+
           created!
         end
       end

ee/lib/ee/gitlab/regex.rb

@@ -23,6 +23,10 @@ module EE
           @conan_recipe_component_regex ||= %r{\A[a-zA-Z0-9_][a-zA-Z0-9_\+\.-]{1,49}\z}.freeze
         end
 
+        def composer_package_version_regex
+          @composer_package_version_regex ||= %r{^v?(\d+(\.(\d+|x))*(-.+)?)}.freeze
+        end
+
         def package_name_regex
           @package_name_regex ||= %r{\A\@?(([\w\-\.\+]*)\/)*([\w\-\.]+)@?(([\w\-\.\+]*)\/)*([\w\-\.]*)\z}.freeze
         end

ee/spec/requests/api/composer_packages_spec.rb

@@ -7,7 +7,7 @@ describe API::ComposerPackages do
   let_it_be(:user) { create(:user) }
   let_it_be(:group, reload: true) { create(:group, :public) }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
-  let_it_be(:project, reload: true) { create(:project, :repository, path: 'my.project') }
+  let_it_be(:project, reload: true) { create(:project, :custom_repo, files: { 'composer.json' => '{ "name": "package-name" }' } ) }
   let(:headers) { {} }
 
   describe 'GET /api/v4/group/:id/-/packages/composer/packages' do
@@ -157,6 +157,10 @@ describe API::ComposerPackages do
     let(:url) { "/projects/#{project.id}/packages/composer" }
     let(:params) { {} }
 
+    before(:all) do
+      project.repository.add_tag(user, 'v1.2.99', 'master')
+    end
+
     subject { post api(url), headers: headers, params: params }
 
     shared_examples 'composer package publish' do
@@ -169,7 +173,7 @@ describe API::ComposerPackages do
           using RSpec::Parameterized::TableSyntax
 
           where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do
-            'PUBLIC'  | :developer  | true  | true  | 'process Composer api request' | :created
+            'PUBLIC'  | :developer  | true  | true  | 'Composer package creation'    | :created
             'PUBLIC'  | :guest      | true  | true  | 'process Composer api request' | :forbidden
             'PUBLIC'  | :developer  | true  | false | 'process Composer api request' | :unauthorized
             'PUBLIC'  | :guest      | true  | false | 'process Composer api request' | :unauthorized
@@ -178,7 +182,7 @@ describe API::ComposerPackages do
             'PUBLIC'  | :developer  | false | false | 'process Composer api request' | :unauthorized
             'PUBLIC'  | :guest      | false | false | 'process Composer api request' | :unauthorized
             'PUBLIC'  | :anonymous  | false | true  | 'process Composer api request' | :unauthorized
-            'PRIVATE' | :developer  | true  | true  | 'process Composer api request' | :created
+            'PRIVATE' | :developer  | true  | true  | 'Composer package creation'    | :created
             'PRIVATE' | :guest      | true  | true  | 'process Composer api request' | :forbidden
             'PRIVATE' | :developer  | true  | false | 'process Composer api request' | :unauthorized
             'PRIVATE' | :guest      | true  | false | 'process Composer api request' | :unauthorized
@@ -214,7 +218,7 @@ describe API::ComposerPackages do
 
     context 'with a tag' do
       context 'with an existing branch' do
-        let(:params) { { tag: 'v1.0.0' } }
+        let(:params) { { tag: 'v1.2.99' } }
 
         it_behaves_like 'composer package publish'
       end
@@ -233,7 +237,7 @@ describe API::ComposerPackages do
 
     context 'with a branch' do
       context 'with an existing branch' do
-        let(:params) { { branch: 'feature' } }
+        let(:params) { { branch: 'master' } }
 
         it_behaves_like 'composer package publish'
       end

ee/spec/services/packages/composer/composer_json_service_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Packages::Composer::ComposerJsonService do
+  describe '#execute' do
+    let(:branch) { project.repository.find_branch('master') }
+    let(:target) { branch.target }
+
+    subject { described_class.new(project, target).execute }
+
+    context 'with an existing file' do
+      let(:project) { create(:project, :custom_repo, files: { 'composer.json' => json } ) }
+
+      context 'with a valid file' do
+        let(:json) { '{ "name": "package-name"}' }
+
+        it 'returns the parsed json' do
+          expect(subject).to eq({ 'name' => 'package-name' })
+        end
+      end
+
+      context 'with an invalid file' do
+        let(:json) { '{ name": "package-name"}' }
+
+        it 'raises an error' do
+          expect { subject }.to raise_error(/Invalid/)
+        end
+      end
+    end
+
+    context 'without the composer.json file' do
+      let(:project) { create(:project, :repository) }
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(/not found/)
+      end
+    end
+  end
+end

ee/spec/services/packages/composer/create_package_service_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Packages::Composer::CreatePackageService do
+  include EE::PackagesManagerApiSpecHelpers
+
+  let_it_be(:package_name) { 'composer-package-name' }
+  let_it_be(:json) { { name: package_name }.to_json }
+  let_it_be(:project) { create(:project, :custom_repo, files: { 'composer.json' => json } ) }
+  let_it_be(:user) { create(:user) }
+  let(:params) do
+    {
+      branch: branch,
+      tag: tag
+    }
+  end
+
+  describe '#execute' do
+    let(:tag) { nil }
+    let(:branch) { nil }
+
+    subject { described_class.new(project, user, params).execute }
+
+    let(:created_package) { Packages::Package.composer.last }
+
+    context 'without an existing package' do
+      context 'with a branch' do
+        let(:branch) { project.repository.find_branch('master') }
+
+        it 'creates the package' do
+          expect { subject }
+            .to change { Packages::Package.composer.count }.by(1)
+            .and change { Packages::Composer::Metadatum.count }.by(1)
+
+          expect(created_package.name).to eq package_name
+          expect(created_package.version).to eq 'dev-master'
+        end
+      end
+
+      context 'with a tag' do
+        let(:tag) { project.repository.find_tag('v1.2.3') }
+
+        before do
+          project.repository.add_tag(user, 'v1.2.3', 'master')
+        end
+
+        it 'creates the package' do
+          expect { subject }
+            .to change { Packages::Package.composer.count }.by(1)
+            .and change { Packages::Composer::Metadatum.count }.by(1)
+
+          expect(created_package.name).to eq package_name
+          expect(created_package.version).to eq '1.2.3'
+        end
+      end
+    end
+
+    context 'with an existing package' do
+      let(:branch) { project.repository.find_branch('master') }
+
+      context 'belonging to the same project' do
+        before do
+          described_class.new(project, user, params).execute
+        end
+
+        it 'does not create a new package' do
+          expect { subject }
+            .to change { Packages::Package.composer.count }.by(0)
+            .and change { Packages::Composer::Metadatum.count }.by(0)
+        end
+      end
+
+      context 'belonging to another project' do
+        let(:other_project) { create(:project) }
+        let!(:other_package) { create(:composer_package, name: package_name, version: 'dev-master', project: other_project) }
+
+        it 'fails with an error' do
+          expect { subject }
+            .to raise_error(/is already taken/)
+        end
+      end
+
+      context 'same name but of different type' do
+        let(:other_project) { create(:project) }
+        let!(:other_package) { create(:package, name: package_name, version: 'dev-master', project: other_project) }
+
+        it 'creates the package' do
+          expect { subject }
+            .to change { Packages::Package.composer.count }.by(1)
+            .and change { Packages::Composer::Metadatum.count }.by(1)
+        end
+      end
+    end
+  end
+end

ee/spec/services/packages/composer/version_parser_service_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Packages::Composer::VersionParserService do
+  let_it_be(:params) { {} }
+
+  describe '#execute' do
+    using RSpec::Parameterized::TableSyntax
+
+    subject { described_class.new(tag_name: tagname, branch_name: branchname).execute }
+
+    where(:tagname, :branchname, :expected_version) do
+      nil             | 'master'     | 'dev-master'
+      nil             | 'my-feature' | 'dev-my-feature'
+      nil             | 'v1'         | '1.x-dev'
+      nil             | 'v1.x'       | '1.x-dev'
+      nil             | 'v1.7.x'     | '1.7.x-dev'
+      nil             | 'v1.7'       | '1.7.x-dev'
+      nil             | '1.7.x'      | '1.7.x-dev'
+      'v1.0.0'        | nil          | '1.0.0'
+      'v1.0'          | nil          | '1.0'
+      '1.0'           | nil          | '1.0'
+      '1.0.2'         | nil          | '1.0.2'
+      '1.0.2-beta2'   | nil          | '1.0.2-beta2'
+    end
+
+    with_them do
+      it { is_expected.to eq expected_version }
+    end
+  end
+end

ee/spec/support/shared_examples/requests/api/composer_packages_shared_examples.rb

 # frozen_string_literal: true
 
+RSpec.shared_examples 'Composer package creation' do |user_type, status, add_member = true|
+  context "for user type #{user_type}" do
+    before do
+      group.send("add_#{user_type}", user) if add_member && user_type != :anonymous
+      project.send("add_#{user_type}", user) if add_member && user_type != :anonymous
+    end
+
+    it 'creates package files' do
+      expect { subject }
+          .to change { project.packages.composer.count }.by(1)
+
+      expect(response).to have_gitlab_http_status(status)
+    end
+  end
+end
+
 RSpec.shared_examples 'process Composer api request' do |user_type, status, add_member = true|
   context "for user type #{user_type}" do
     before do