Merge branch '42680-10io-nuget-dependencies-support' into 'master'

Add nuget dependencies support to the metadata extraction

See merge request gitlab-org/gitlab!30618

db/migrate/20200424135319_create_nuget_dependency_link_metadata.rb

+# frozen_string_literal: true
+
+class CreateNugetDependencyLinkMetadata < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  CONSTRAINT_NAME = 'packages_nuget_dependency_link_metadata_target_framework_constraint'
+
+  def up
+    unless table_exists?(:packages_nuget_dependency_link_metadata)
+      create_table :packages_nuget_dependency_link_metadata, id: false do |t|
+        t.references :dependency_link, primary_key: true, default: nil, foreign_key: { to_table: :packages_dependency_links, on_delete: :cascade }, index: { name: 'index_packages_nuget_dl_metadata_on_dependency_link_id' }, type: :bigint
+        t.text :target_framework, null: false
+      end
+    end
+
+    add_text_limit :packages_nuget_dependency_link_metadata, :target_framework, 255, constraint_name: CONSTRAINT_NAME
+  end
+
+  def down
+    drop_table :packages_nuget_dependency_link_metadata
+  end
+end

db/structure.sql

@@ -4617,6 +4617,12 @@ CREATE SEQUENCE public.packages_maven_metadata_id_seq
 
 ALTER SEQUENCE public.packages_maven_metadata_id_seq OWNED BY public.packages_maven_metadata.id;
 
+CREATE TABLE public.packages_nuget_dependency_link_metadata (
+    dependency_link_id bigint NOT NULL,
+    target_framework text NOT NULL,
+    CONSTRAINT packages_nuget_dependency_link_metadata_target_framework_constr CHECK ((char_length(target_framework) <= 255))
+);
+
 CREATE TABLE public.packages_package_files (
     id bigint NOT NULL,
     package_id bigint NOT NULL,
@@ -8476,6 +8482,9 @@ ALTER TABLE ONLY public.packages_dependency_links
 ALTER TABLE ONLY public.packages_maven_metadata
     ADD CONSTRAINT packages_maven_metadata_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.packages_nuget_dependency_link_metadata
+    ADD CONSTRAINT packages_nuget_dependency_link_metadata_pkey PRIMARY KEY (dependency_link_id);
+
 ALTER TABLE ONLY public.packages_package_files
     ADD CONSTRAINT packages_package_files_pkey PRIMARY KEY (id);
 
@@ -10104,6 +10113,8 @@ CREATE INDEX index_packages_dependency_links_on_dependency_id ON public.packages
 
 CREATE INDEX index_packages_maven_metadata_on_package_id_and_path ON public.packages_maven_metadata USING btree (package_id, path);
 
+CREATE INDEX index_packages_nuget_dl_metadata_on_dependency_link_id ON public.packages_nuget_dependency_link_metadata USING btree (dependency_link_id);
+
 CREATE INDEX index_packages_package_files_on_package_id_and_file_name ON public.packages_package_files USING btree (package_id, file_name);
 
 CREATE INDEX index_packages_packages_on_name_trigram ON public.packages_packages USING gin (name public.gin_trgm_ops);
@@ -12302,6 +12313,9 @@ ALTER TABLE ONLY public.user_canonical_emails
 ALTER TABLE ONLY public.project_repositories
     ADD CONSTRAINT fk_rails_c3258dc63b FOREIGN KEY (shard_id) REFERENCES public.shards(id) ON DELETE RESTRICT;
 
+ALTER TABLE ONLY public.packages_nuget_dependency_link_metadata
+    ADD CONSTRAINT fk_rails_c3313ee2e4 FOREIGN KEY (dependency_link_id) REFERENCES public.packages_dependency_links(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.merge_request_user_mentions
     ADD CONSTRAINT fk_rails_c440b9ea31 FOREIGN KEY (note_id) REFERENCES public.notes(id) ON DELETE CASCADE;
 
@@ -13702,6 +13716,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200424043515
 20200424050250
 20200424101920
+20200424135319
 20200427064130
 20200429015603
 20200429181335

ee/app/models/packages/dependency_link.rb

@@ -2,6 +2,7 @@
 class Packages::DependencyLink < ApplicationRecord
   belongs_to :package, inverse_of: :dependency_links
   belongs_to :dependency, inverse_of: :dependency_links, class_name: 'Packages::Dependency'
+  has_one :nuget_metadatum, inverse_of: :dependency_link, class_name: 'Packages::NugetDependencyLinkMetadatum'
 
   validates :package, :dependency, presence: true
 
@@ -12,4 +13,6 @@ class Packages::DependencyLink < ApplicationRecord
 
   scope :with_dependency_type, ->(dependency_type) { where(dependency_type: dependency_type) }
   scope :includes_dependency, -> { includes(:dependency) }
+  scope :for_package, ->(package) { where(package_id: package.id) }
+  scope :preload_dependency, -> { preload(:dependency) }
 end

ee/app/models/packages/nuget_dependency_link_metadatum.rb

+# frozen_string_literal: true
+
+class Packages::NugetDependencyLinkMetadatum < ApplicationRecord
+  self.primary_key = :dependency_link_id
+
+  belongs_to :dependency_link, inverse_of: :nuget_metadatum
+
+  validates :dependency_link, :target_framework, presence: true
+
+  validate :ensure_nuget_package_type
+
+  private
+
+  def ensure_nuget_package_type
+    return if dependency_link&.package&.nuget?
+
+    errors.add(:base, _('Package type must be NuGet'))
+  end
+end

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

+# frozen_string_literal: true
+module Packages
+  module Nuget
+    class CreateDependencyService < BaseService
+      def initialize(package, dependencies = [])
+        @package = package
+        @dependencies = dependencies
+      end
+
+      def execute
+        return if @dependencies.empty?
+
+        @package.transaction do
+          create_dependency_links
+          create_dependency_link_metadata
+        end
+      end
+
+      private
+
+      def create_dependency_links
+        ::Packages::CreateDependencyService
+          .new(@package, dependencies_for_create_dependency_service)
+          .execute
+      end
+
+      def create_dependency_link_metadata
+        inserted_links = ::Packages::DependencyLink.preload_dependency
+                                                   .for_package(@package)
+
+        return if inserted_links.empty?
+
+        rows = inserted_links.map do |dependency_link|
+          raw_dependency = raw_dependency_for(dependency_link.dependency)
+
+          next if raw_dependency[:target_framework].blank?
+
+          {
+            dependency_link_id: dependency_link.id,
+            target_framework: raw_dependency[:target_framework]
+          }
+        end
+
+        ::Gitlab::Database.bulk_insert(::Packages::NugetDependencyLinkMetadatum.table_name, rows.compact)
+      end
+
+      def raw_dependency_for(dependency)
+        name = dependency.name
+        version = dependency.version_pattern.presence
+
+        @dependencies.find do |raw_dependency|
+          raw_dependency[:name] == name && raw_dependency[:version] == version
+        end
+      end
+
+      def dependencies_for_create_dependency_service
+        names_and_versions = @dependencies.map do |dependency|
+          [dependency[:name], version_or_empty_string(dependency[:version])]
+        end.to_h
+
+        { 'dependencies' => names_and_versions }
+      end
+
+      def version_or_empty_string(version)
+        return '' if version.blank?
+
+        version
+      end
+    end
+  end
+end

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

@@ -12,6 +12,9 @@ module Packages
         package_version: '//xmlns:package/xmlns:metadata/xmlns:version'
       }.freeze
 
+      XPATH_DEPENDENCIES = '//xmlns:package/xmlns:metadata/xmlns:dependencies/xmlns:dependency'
+      XPATH_DEPENDENCY_GROUPS = '//xmlns:package/xmlns:metadata/xmlns:dependencies/xmlns:group'
+
       MAX_FILE_SIZE = 4.megabytes.freeze
 
       def initialize(package_file_id)
@@ -41,9 +44,36 @@ module Packages
       def extract_metadata(file)
         doc = Nokogiri::XML(file)
 
-        XPATHS.map do |key, query|
-          [key, doc.xpath(query).text]
-        end.to_h
+        XPATHS.map { |key, query| [key, doc.xpath(query).text] }
+              .to_h
+              .tap do |metadata|
+                metadata[:package_dependencies] = extract_dependencies(doc)
+              end
+      end
+
+      def extract_dependencies(doc)
+        dependencies = []
+
+        doc.xpath(XPATH_DEPENDENCIES).each do |node|
+          dependencies << extract_dependency(node)
+        end
+
+        doc.xpath(XPATH_DEPENDENCY_GROUPS).each do |group_node|
+          target_framework = group_node.attr("targetFramework")
+
+          group_node.xpath("xmlns:dependency").each do |node|
+            dependencies << extract_dependency(node).merge(target_framework: target_framework)
+          end
+        end
+
+        dependencies
+      end
+
+      def extract_dependency(node)
+        {
+          name: node.attr('id'),
+          version: node.attr('version')
+        }.compact
       end
 
       def nuspec_file

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

@@ -48,12 +48,13 @@ module Packages
       end
 
       def update_linked_package
-        return unless package_name && package_version
-
         @package_file.package.update!(
           name: package_name,
           version: package_version
         )
+
+        ::Packages::Nuget::CreateDependencyService.new(@package_file.package, package_dependencies)
+                                                  .execute
       end
 
       def existing_package_id
@@ -75,6 +76,10 @@ module Packages
         metadata[:package_version]
       end
 
+      def package_dependencies
+        metadata.fetch(:package_dependencies, [])
+      end
+
       def metadata
         strong_memoize(:metadata) do
           ::Packages::Nuget::MetadataExtractionService.new(@package_file.id).execute

ee/changelogs/unreleased/42680-10io-nuget-dependencies-support.yml

+---
+title: Add NuGet dependencies extraction to the GitLab Packages Repository
+merge_request: 30618
+author:
+type: added

ee/spec/factories/packages.rb

@@ -266,6 +266,16 @@ FactoryBot.define do
     package
     dependency { create(:packages_dependency) }
     dependency_type { :dependencies }
+
+    trait(:with_nuget_metadatum) do
+      after :build do |link|
+        link.nuget_metadatum = build(:nuget_dependency_link_metadatum)
+      end
+    end
+  end
+
+  factory :nuget_dependency_link_metadatum, class: 'Packages::NugetDependencyLinkMetadatum' do
+    target_framework { '.NETStandard2.0' }
   end
 
   factory :packages_tag, class: 'Packages::Tag' do

ee/spec/fixtures/nuget/with_dependencies.nuspec

+<?xml version="1.0" encoding="utf-8"?>
+<package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
+  <metadata>
+    <id>Test.Package</id>
+    <version>3.5.2</version>
+    <authors>Test Author</authors>
+    <owners>Test Owner</owners>
+    <requireLicenseAcceptance>false</requireLicenseAcceptance>
+    <description>Package Description</description>
+    <dependencies>
+      <dependency id="Moqi" version="2.5.6" include="Runtime,Compile" />
+      <group targetFramework=".NETStandard2.0">
+        <dependency id="Test.Dependency" version="2.3.7" exclude="Build,Analyzers" include="Runtime,Compile" />
+        <dependency id="Newtonsoft.Json" version="12.0.3" exclude="Build,Analyzers" />
+      </group>
+      <dependency id="Castle.Core" />
+    </dependencies>
+  </metadata>
+</package>

ee/spec/models/packages/dependency_link_spec.rb

@@ -5,6 +5,7 @@ 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) }
+    it { is_expected.to have_one(:nuget_metadatum).inverse_of(:dependency_link) }
   end
 
   describe 'validations' do
@@ -29,15 +30,27 @@ RSpec.describe Packages::DependencyLink, type: :model do
     end
   end
 
-  describe '.with_dependency_type' do
+  context 'with multiple links' 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])
+    describe '.with_dependency_type' do
+      it 'returns links of the given type' do
+        expect(subject.with_dependency_type(:bundleDependencies)).to eq([link3])
+      end
+    end
+
+    describe '.for_package' 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) }
+
+      it 'returns the link for the given package' do
+        expect(subject.for_package(link1.package)).to eq([link1])
+      end
     end
   end
 end

ee/spec/models/packages/nuget_dependency_link_metadatum_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::NugetDependencyLinkMetadatum, type: :model do
+  describe 'relationships' do
+    it { is_expected.to belong_to(:dependency_link) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:dependency_link) }
+    it { is_expected.to validate_presence_of(:target_framework) }
+
+    describe '#ensure_nuget_package_type' do
+      it 'validates package of type nuget' do
+        package = build('conan_package')
+        dependency_link = build('packages_dependency_link', package: package)
+        nuget_metadatum = build('nuget_dependency_link_metadatum', dependency_link: dependency_link)
+
+        expect(nuget_metadatum).not_to be_valid
+        expect(nuget_metadatum.errors.to_a).to contain_exactly('Package type must be NuGet')
+      end
+
+      it 'validates package of type nuget with nil dependency_link' do
+        nuget_metadatum = build('nuget_dependency_link_metadatum', dependency_link: nil)
+
+        expect(nuget_metadatum).not_to be_valid
+        expect(nuget_metadatum.errors.to_a).to contain_exactly("Dependency link can't be blank", 'Package type must be NuGet')
+      end
+    end
+  end
+end

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

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Packages::Nuget::CreateDependencyService do
+  let_it_be(:package, reload: true) { create(:nuget_package) }
+
+  describe '#execute' do
+    RSpec.shared_examples 'creating dependencies, links and nuget metadata for' do |expected_dependency_names, dependency_count, dependency_link_count|
+      let(:dependencies_with_metadata) { dependencies.select { |dep| dep[:target_framework].present? } }
+
+      it 'creates dependencies, links and nuget metadata' do
+        expect { subject }
+          .to change { Packages::Dependency.count }.by(dependency_count)
+          .and change { Packages::DependencyLink.count }.by(dependency_link_count)
+          .and change { Packages::NugetDependencyLinkMetadatum.count }.by(dependencies_with_metadata.size)
+        expect(expected_dependency_names).to contain_exactly(*dependency_names)
+        expect(package.dependency_links.map(&:dependency_type).uniq).to contain_exactly('dependencies')
+
+        dependencies_with_metadata.each do |dependency|
+          name = dependency[:name]
+          version_pattern = service.send(:version_or_empty_string, dependency[:version])
+          metadatum = package.dependency_links.joins(:dependency)
+                                              .find_by(packages_dependencies: { name: name, version_pattern: version_pattern })
+                                              .nuget_metadatum
+          expect(metadatum.target_framework).to eq dependency[:target_framework]
+        end
+      end
+    end
+
+    let_it_be(:dependencies) do
+      [
+        { name: 'Moqi', version: '2.5.6' },
+        { name: 'Castle.Core' },
+        { name: 'Test.Dependency', version: '2.3.7', target_framework: '.NETStandard2.0' },
+        { name: 'Newtonsoft.Json', version: '12.0.3', target_framework: '.NETStandard2.0' }
+      ]
+    end
+
+    let(:dependency_names) { package.dependency_links.flat_map(&:dependency).map(&:name) }
+    let(:service) { described_class.new(package, dependencies) }
+
+    subject { service.execute }
+
+    it_behaves_like 'creating dependencies, links and nuget metadata for', %w(Castle.Core Moqi Newtonsoft.Json Test.Dependency), 4, 4
+
+    context 'with existing dependencies' do
+      let_it_be(:exisiting_dependency) { create(:packages_dependency, name: 'Moqi', version_pattern: '2.5.6') }
+
+      it_behaves_like 'creating dependencies, links and nuget metadata for', %w(Castle.Core Moqi Newtonsoft.Json Test.Dependency), 3, 4
+    end
+
+    context 'with dependencies with no target framework' do
+      let_it_be(:dependencies) do
+        [
+          { name: 'Moqi', version: '2.5.6' },
+          { name: 'Castle.Core' },
+          { name: 'Test.Dependency', version: '2.3.7' },
+          { name: 'Newtonsoft.Json', version: '12.0.3' }
+        ]
+      end
+
+      it_behaves_like 'creating dependencies, links and nuget metadata for', %w(Castle.Core Moqi Newtonsoft.Json Test.Dependency), 4, 4
+    end
+
+    context 'with empty dependencies' do
+      let_it_be(:dependencies) { [] }
+
+      it 'is a no op' do
+        expect(service).not_to receive(:create_dependency_links)
+        expect(service).not_to receive(:create_dependency_link_metadata)
+
+        subject
+      end
+    end
+  end
+end

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

@@ -10,7 +10,38 @@ describe Packages::Nuget::MetadataExtractionService 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') }
+      expected_metadata = {
+        package_name: 'DummyProject.DummyPackage',
+        package_version: '1.0.0',
+        package_dependencies: [
+          {
+            name: 'Newtonsoft.Json',
+            target_framework: '.NETCoreApp3.0',
+            version: '12.0.3'
+          }
+        ]
+      }
+
+      it { is_expected.to eq(expected_metadata) }
+    end
+
+    context 'with nuspec file with dependencies' do
+      let(:nuspec_filepath) { 'nuget/with_dependencies.nuspec' }
+
+      before do
+        allow(service).to receive(:nuspec_file).and_return(fixture_file(nuspec_filepath, dir: 'ee'))
+      end
+
+      it { is_expected.to have_key(:package_dependencies) }
+
+      it 'extracts dependencies' do
+        dependencies = subject[:package_dependencies]
+
+        expect(dependencies).to include(name: 'Moqi', version: '2.5.6')
+        expect(dependencies).to include(name: 'Castle.Core')
+        expect(dependencies).to include(name: 'Test.Dependency', version: '2.3.7', target_framework: '.NETStandard2.0')
+        expect(dependencies).to include(name: 'Newtonsoft.Json', version: '12.0.3', target_framework: '.NETStandard2.0')
+      end
     end
 
     context 'with invalid package file id' do

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

@@ -18,7 +18,9 @@ describe Packages::Nuget::UpdatePackageFromMetadataService do
     end
 
     it 'updates package and package file' do
-      subject
+      expect { subject }
+        .to change { Packages::Dependency.count }.by(1)
+        .and change { Packages::DependencyLink.count }.by(1)
 
       expect(package.reload.name).to eq(package_name)
       expect(package.version).to eq(package_version)
@@ -31,12 +33,43 @@ describe Packages::Nuget::UpdatePackageFromMetadataService 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 { subject }
+          .to change { ::Packages::Package.count }.by(-1)
+          .and change { Packages::Dependency.count }.by(0)
+          .and change { Packages::DependencyLink.count }.by(0)
+          .and change { Packages::NugetDependencyLinkMetadatum.count }.by(0)
         expect(package_file.reload.file_name).to eq(package_file_name)
         expect(package_file.package).to eq(existing_package)
       end
     end
 
+    context 'with nuspec file with dependencies' do
+      let(:nuspec_filepath) { 'nuget/with_dependencies.nuspec' }
+      let(:package_name) { 'Test.Package' }
+      let(:package_version) { '3.5.2' }
+      let(:package_file_name) { 'test.package.3.5.2.nupkg' }
+
+      before do
+        allow_any_instance_of(Packages::Nuget::MetadataExtractionService)
+          .to receive(:nuspec_file)
+          .and_return(fixture_file(nuspec_filepath, dir: 'ee'))
+      end
+
+      it 'updates package and package file' do
+        expect { subject }
+          .to change { ::Packages::Package.count }.by(1)
+          .and change { Packages::Dependency.count }.by(4)
+          .and change { Packages::DependencyLink.count }.by(4)
+          .and change { Packages::NugetDependencyLinkMetadatum.count }.by(2)
+
+        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)
+        # hard reset needed to properly reload package_file.file
+        expect(Packages::PackageFile.find(package_file.id).file.size).not_to eq 0
+      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([])

locale/gitlab.pot

@@ -14611,6 +14611,9 @@ msgstr ""
 msgid "Package type must be Maven"
 msgstr ""
 
+msgid "Package type must be NuGet"
+msgstr ""
+
 msgid "Package was removed"
 msgstr ""