Merge branch...

Merge branch '36502-nuget-package-metadata-extraction-job-db-changes-and-dependencies' into 'master'

Nuget - metadata extraction

See merge request gitlab-org/gitlab!21561

config/sidekiq_queues.yml

@@ -152,6 +152,8 @@
   - 1
 - - object_storage
   - 1
+- - package_repositories
+  - 1
 - - pages
   - 1
 - - pages_domain_ssl_renewal

ee/app/controllers/projects/packages/packages_controller.rb

@@ -11,6 +11,7 @@ module Projects
       def index
         @packages = project.packages
                       .has_version
+                      .processed
                       .sort_by_attribute(@sort = params[:sort] || 'created_desc')
                       .page(params[:page])
       end

ee/app/models/packages/package.rb

@@ -40,6 +40,11 @@ class Packages::Package < ApplicationRecord
   end
 
   scope :has_version, -> { where.not(version: nil) }
+  scope :processed, -> do
+    where.not(package_type: :nuget).or(
+      where.not(name: Packages::Nuget::CreatePackageService::TEMPORARY_PACKAGE_NAME)
+    )
+  end
   scope :preload_files, -> { preload(:package_files) }
   scope :last_of_each_version, -> { where(id: all.select('MAX(id) AS id').group(:version)) }
 

ee/app/services/packages/nuget/metadata_extraction_service.rb

+# frozen_string_literal: true
+
+module Packages
+  module Nuget
+    class MetadataExtractionService
+      include Gitlab::Utils::StrongMemoize
+
+      ExtractionError = Class.new(StandardError)
+
+      attr_reader :package_file_id
+
+      XPATHS = {
+        package_name: '//xmlns:package/xmlns:metadata/xmlns:id',
+        package_version: '//xmlns:package/xmlns:metadata/xmlns:version'
+      }.freeze
+
+      MAX_FILE_SIZE = 4.megabytes.freeze
+
+      def initialize(package_file_id)
+        @package_file_id = package_file_id
+      end
+
+      def execute
+        raise ExtractionError.new('invalid package file') unless valid_package_file?
+
+        extract_metadata(nuspec_file)
+      end
+
+      private
+
+      def package_file
+        strong_memoize(:package_file) do
+          ::Packages::PackageFile.find_by_id(package_file_id)
+        end
+      end
+
+      def valid_package_file?
+        package_file &&
+          package_file.package&.nuget? &&
+          package_file.file.size.positive?
+      end
+
+      def extract_metadata(file)
+        doc = Nokogiri::XML(file)
+
+        XPATHS.map do |key, query|
+          [key, doc.xpath(query).text]
+        end.to_h
+      end
+
+      def nuspec_file
+        package_file.file.use_file do |file_path|
+          Zip::File.open(file_path) do |zip_file|
+            entry = zip_file.glob('*.nuspec').first
+
+            raise ExtractionError.new('nuspec file not found') unless entry
+            raise ExtractionError.new('nuspec file too big') if entry.size > MAX_FILE_SIZE
+
+            entry.get_input_stream.read
+          end
+        end
+      end
+    end
+  end
+end

ee/app/services/packages/nuget/update_package_from_metadata_service.rb

+# frozen_string_literal: true
+
+module Packages
+  module Nuget
+    class UpdatePackageFromMetadataService
+      include Gitlab::Utils::StrongMemoize
+
+      InvalidMetadataError = Class.new(StandardError)
+
+      attr_reader :package_file
+
+      def initialize(package_file)
+        @package_file = package_file
+      end
+
+      def execute
+        raise InvalidMetadataError.new('package name and/or package version not found in metadata') unless valid_metadata?
+
+        package_file.transaction do
+          package_file.update!(file_name: package_filename) if package_filename
+
+          if existing_package_id
+            link_to_existing_package
+          else
+            update_linked_package
+          end
+        end
+      end
+
+      private
+
+      def valid_metadata?
+        package_name.present? && package_version.present?
+      end
+
+      def link_to_existing_package
+        package_to_destroy = package_file.package
+        package_file.update!(package_id: existing_package_id)
+        package_to_destroy.destroy!
+      end
+
+      def update_linked_package
+        return unless package_name && package_version
+
+        package_file.package.update!(
+          name: package_name,
+          version: package_version
+        )
+      end
+
+      def existing_package_id
+        strong_memoize(:existing_package_id) do
+          package_file.project.packages
+                              .nuget
+                              .with_name(package_name)
+                              .with_version(package_version)
+                              .pluck_primary_key
+                              .first
+        end
+      end
+
+      def package_name
+        metadata[:package_name]
+      end
+
+      def package_version
+        metadata[:package_version]
+      end
+
+      def metadata
+        strong_memoize(:metadata) do
+          ::Packages::Nuget::MetadataExtractionService.new(package_file.id).execute
+        end
+      end
+
+      def package_filename
+        return unless package_name && package_version
+
+        "#{package_name.downcase}.#{package_version.downcase}.nupkg"
+      end
+    end
+  end
+end

ee/app/workers/all_queues.yml

@@ -321,6 +321,12 @@
   :latency_sensitive: 
   :resource_boundary: :unknown
   :weight: 1
+- :name: package_repositories:packages_nuget_extraction
+  :feature_category: :package_registry
+  :has_external_dependencies: 
+  :latency_sensitive: 
+  :resource_boundary: :unknown
+  :weight: 1
 - :name: personal_access_tokens:personal_access_tokens_policy
   :feature_category: :authentication_and_authorization
   :has_external_dependencies: 

ee/app/workers/packages/nuget/extraction_worker.rb

+# frozen_string_literal: true
+
+module Packages
+  module Nuget
+    class ExtractionWorker
+      include ApplicationWorker
+
+      queue_namespace :package_repositories
+      feature_category :package_registry
+
+      def perform(package_file_id)
+        package_file = ::Packages::PackageFile.find_by_id(package_file_id)
+
+        return unless package_file
+
+        ::Packages::Nuget::UpdatePackageFromMetadataService.new(package_file).execute
+
+      rescue ::Packages::Nuget::MetadataExtractionService::ExtractionError,
+             ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError => e
+        Gitlab::ErrorTracking.log_exception(e, project_id: package_file.project_id)
+        package_file.package.destroy!
+      end
+    end
+  end
+end

ee/lib/api/nuget_packages.rb

@@ -93,17 +93,21 @@ module API
         put do
           authorize_upload!(authorized_user_project)
 
-          package = ::Packages::Nuget::CreatePackageService.new(authorized_user_project, current_user).execute
-
           file_params = params.merge(
             file: uploaded_package_file(:package),
             file_name: PACKAGE_FILENAME,
             file_type: PACKAGE_FILETYPE
           )
 
+          package = ::Packages::Nuget::CreatePackageService.new(authorized_user_project, current_user)
+                                                           .execute
+
+          package_file = ::Packages::CreatePackageFileService.new(package, file_params)
+                                                             .execute
+
           track_event('push_package')
 
-          ::Packages::CreatePackageFileService.new(package, file_params).execute
+          ::Packages::Nuget::ExtractionWorker.perform_async(package_file.id)
 
           created!
         rescue ObjectStorage::RemoteStoreError => e

ee/spec/factories/packages.rb

@@ -40,6 +40,10 @@ FactoryBot.define do
       sequence(:name) { |n| "NugetPackage#{n}"}
       version { '1.0.0' }
       package_type { :nuget }
+
+      after :create do |package|
+        create :package_file, :nuget, package: package
+      end
     end
 
     factory :conan_package do
@@ -170,6 +174,15 @@ FactoryBot.define do
       size { 400.kilobytes }
     end
 
+    trait(:nuget) do
+      package
+      file { fixture_file_upload('ee/spec/fixtures/nuget/package.nupkg') }
+      file_name { 'package.nupkg' }
+      file_sha1 { '5fe852b2a6abd96c22c11fa1ff2fb19d9ce58b57' }
+      file_type { 0 }
+      size { 300.kilobytes }
+    end
+
     trait :object_storage do
       file_store { Packages::PackageFileUploader::Store::REMOTE }
     end

ee/spec/models/packages/package_spec.rb

@@ -139,4 +139,20 @@ RSpec.describe Packages::Package, type: :model do
       is_expected.to eq([package])
     end
   end
+
+  describe '.processed' do
+    let!(:package1) { create(:nuget_package) }
+    let!(:package2) { create(:npm_package) }
+    let!(:package3) { create(:nuget_package) }
+
+    subject { described_class.processed }
+
+    it { is_expected.to eq([package1, package2, package3]) }
+
+    context 'with temporary packages' do
+      let!(:package1) { create(:nuget_package, name: Packages::Nuget::CreatePackageService::TEMPORARY_PACKAGE_NAME) }
+
+      it { is_expected.to eq([package2, package3]) }
+    end
+  end
 end

ee/spec/services/packages/nuget/metadata_extraction_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Packages::Nuget::MetadataExtractionService do
+  let(:package_file) { create(:nuget_package).package_files.first }
+  let(:service) { described_class.new(package_file.id) }
+
+  describe '#execute' do
+    subject { service.execute }
+
+    context 'with valid package file id' do
+      it { is_expected.to eq(package_name: 'DummyProject.DummyPackage', package_version: '1.0.0') }
+    end
+
+    context 'with invalid package file id' do
+      let(:package_file) { OpenStruct.new(id: 555) }
+
+      it { expect { subject }.to raise_error(::Packages::Nuget::MetadataExtractionService::ExtractionError, 'invalid package file') }
+    end
+
+    context 'linked to a non nuget package' do
+      before do
+        package_file.package.conan!
+      end
+
+      it { expect { subject }.to raise_error(::Packages::Nuget::MetadataExtractionService::ExtractionError, 'invalid package file') }
+    end
+
+    context 'with a 0 byte package file id' do
+      before do
+        allow_any_instance_of(Packages::PackageFileUploader).to receive(:size).and_return(0)
+      end
+
+      it { expect { subject }.to raise_error(::Packages::Nuget::MetadataExtractionService::ExtractionError, 'invalid package file') }
+    end
+
+    context 'without the nuspec file' do
+      before do
+        allow_any_instance_of(Zip::File).to receive(:glob).and_return([])
+      end
+
+      it { expect { subject }.to raise_error(::Packages::Nuget::MetadataExtractionService::ExtractionError, 'nuspec file not found') }
+    end
+
+    context 'with a too big nuspec file' do
+      before do
+        allow_any_instance_of(Zip::File).to receive(:glob).and_return([OpenStruct.new(size: 6.megabytes)])
+      end
+
+      it { expect { subject }.to raise_error(::Packages::Nuget::MetadataExtractionService::ExtractionError, 'nuspec file too big') }
+    end
+  end
+end

ee/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Packages::Nuget::UpdatePackageFromMetadataService do
+  let!(:package) { create(:nuget_package) }
+  let(:package_file) { package.package_files.first }
+  let(:service) { described_class.new(package_file) }
+  let(:package_name) { 'DummyProject.DummyPackage' }
+  let(:package_version) { '1.0.0' }
+  let(:package_file_name) { 'dummyproject.dummypackage.1.0.0.nupkg' }
+
+  describe '#execute' do
+    subject { service.execute }
+
+    it 'updates package and package file' do
+      subject
+
+      expect(package.reload.name).to eq(package_name)
+      expect(package.version).to eq(package_version)
+      expect(package_file.reload.file_name).to eq(package_file_name)
+    end
+
+    context 'with exisiting package' do
+      let!(:existing_package) { create(:nuget_package, project: package.project, name: package_name, version: package_version) }
+
+      it 'link existing package and updates package file' do
+        expect { subject }.to change { ::Packages::Package.count }.by(-1)
+        expect(package_file.reload.file_name).to eq(package_file_name)
+        expect(package_file.package).to eq(existing_package)
+      end
+    end
+
+    context 'with package file not containing a nuspec file' do
+      before do
+        allow_any_instance_of(Zip::File).to receive(:glob).and_return([])
+      end
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(::Packages::Nuget::MetadataExtractionService::ExtractionError)
+      end
+    end
+
+    context 'with package file with a blank package name' do
+      before do
+        allow(service).to receive(:package_name).and_return('')
+      end
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError)
+      end
+    end
+
+    context 'with package file with a blank package version' do
+      before do
+        allow(service).to receive(:package_version).and_return('')
+      end
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError)
+      end
+    end
+  end
+end

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

@@ -80,6 +80,7 @@ end
 RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member = true|
   shared_examples 'creates nuget package files' do
     it 'creates package files' do
+      expect(::Packages::Nuget::ExtractionWorker).to receive(:perform_async).once
       expect { subject }
           .to change { project.packages.count }.by(1)
           .and change { Packages::PackageFile.count }.by(1)

ee/spec/workers/packages/nuget/extraction_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Nuget::ExtractionWorker, type: :worker do
+  describe '#perform' do
+    let!(:package) { create(:nuget_package) }
+    let(:package_file) { package.package_files.first }
+    let(:package_file_id) { package_file.id }
+
+    let_it_be(:package_name) { 'DummyProject.DummyPackage' }
+    let_it_be(:package_version) { '1.0.0' }
+
+    subject { described_class.new.perform(package_file_id) }
+
+    context 'with valid package file' do
+      it 'updates package and package file' do
+        expect { subject }
+          .to not_change { Packages::Package.count }
+          .and not_change { Packages::PackageFile.count }
+      end
+
+      context 'with exisiting package' do
+        let!(:existing_package) { create(:nuget_package, project: package.project, name: package_name, version: package_version) }
+
+        it 'reuses existing package and updates package file' do
+          expect { subject }
+            .to change { Packages::Package.count }.by(-1)
+            .and change { existing_package.reload.package_files.count }.by(1)
+            .and not_change { Packages::PackageFile.count }
+        end
+      end
+    end
+
+    context 'with invalid package file id' do
+      let(:package_file_id) { 5555 }
+
+      it "doesn't update package and package file" do
+        expect { subject }
+          .to not_change { package.reload.name }
+          .and not_change { package.version }
+          .and not_change { package_file.reload.file_name }
+      end
+    end
+
+    context 'with package file not containing a nuspec file' do
+      before do
+        allow_any_instance_of(Zip::File).to receive(:glob).and_return([])
+      end
+
+      it 'removes the package and the package file' do
+        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+          instance_of(::Packages::Nuget::MetadataExtractionService::ExtractionError),
+          project_id: package.project_id
+        )
+        expect { subject }
+          .to change { Packages::Package.count }.by(-1)
+          .and change { Packages::PackageFile.count }.by(-1)
+      end
+    end
+
+    context 'with package file with a blank package name' do
+      before do
+        allow_any_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService).to receive(:package_name).and_return('')
+      end
+
+      it 'removes the package and the package file' do
+        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+          instance_of(::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError),
+          project_id: package.project_id
+        )
+        expect { subject }
+          .to change { Packages::Package.count }.by(-1)
+          .and change { Packages::PackageFile.count }.by(-1)
+      end
+    end
+
+    context 'with package file with a blank package version' do
+      before do
+        allow_any_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService).to receive(:package_version).and_return('')
+      end
+
+      it 'removes the package and the package file' do
+        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+          instance_of(::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError),
+          project_id: package.project_id
+        )
+        expect { subject }
+          .to change { Packages::Package.count }.by(-1)
+          .and change { Packages::PackageFile.count }.by(-1)
+      end
+    end
+  end
+end