diff --git a/app/graphql/types/packages/package_dependency_link_type.rb b/app/graphql/types/packages/package_dependency_link_type.rb index d49c467a53da54b576cf2cc84320b89cad0cdefd..d5194c8c373dfeccd5ecb8c42757da86a47a3040 100644 --- a/app/graphql/types/packages/package_dependency_link_type.rb +++ b/app/graphql/types/packages/package_dependency_link_type.rb @@ -34,7 +34,15 @@ def metadata end def dependency - ::Gitlab::Graphql::Loaders::BatchModelLoader.new(::Packages::Dependency, object.dependency_id).find + dep = ::Gitlab::Graphql::Loaders::BatchModelLoader.new(::Packages::Dependency, object.dependency_id).find + + ::Gitlab::Graphql::Lazy.with_value(dep) do |dependency| + if dependency&.name&.start_with?(::Packages::Nuget::ExtractMetadataContentService::EMPTY_DEPENDENCY_PREFIX) + next + end + + dependency + end end end end diff --git a/app/presenters/packages/nuget/presenter_helpers.rb b/app/presenters/packages/nuget/presenter_helpers.rb index 16c32a9d0d0d238267a6ec4b260a09b3c2bc6c53..b5f0e40ff05f5019f99b0fb916d12718141e8bf3 100644 --- a/app/presenters/packages/nuget/presenter_helpers.rb +++ b/app/presenters/packages/nuget/presenter_helpers.rb @@ -78,15 +78,17 @@ def dependency_links_grouped_by_target_framework(package) def dependencies_for(nuget_id, dependency_links) return [] if dependency_links.empty? - dependency_links.map do |dependency_link| + dependency_links.filter_map do |dependency_link| dependency = dependency_link.dependency + next if dependency.name.start_with?(Packages::Nuget::ExtractMetadataContentService::EMPTY_DEPENDENCY_PREFIX) + { id: "#{nuget_id}/#{dependency.name.downcase}", type: PACKAGE_DEPENDENCY, name: dependency.name, range: dependency.version_pattern } - end + end.presence end def target_framework_nuget_id(base_nuget_id, target_framework) diff --git a/app/services/packages/nuget/create_dependency_service.rb b/app/services/packages/nuget/create_dependency_service.rb index 85f295ac7b7163a1f1a05b519ac5cb5cdb36f1e4..e8ad2cb048d244797819978aa2d372ceb31cc856 100644 --- a/app/services/packages/nuget/create_dependency_service.rb +++ b/app/services/packages/nuget/create_dependency_service.rb @@ -54,7 +54,15 @@ def raw_dependency_for(dependency) end def dependencies_for_create_dependency_service - names_and_versions = @dependencies.to_h do |dependency| + names_and_versions = @dependencies + + if ::Feature.disabled?(:persist_nuget_packages_empty_target_frameworks, @package.project) + names_and_versions.reject! do |dependency| + dependency[:name].start_with?(::Packages::Nuget::ExtractMetadataContentService::EMPTY_DEPENDENCY_PREFIX) + end + end + + names_and_versions = names_and_versions.to_h do |dependency| [dependency[:name], version_or_empty_string(dependency[:version])] end diff --git a/app/services/packages/nuget/extract_metadata_content_service.rb b/app/services/packages/nuget/extract_metadata_content_service.rb index 6df902a5acad4bf225a097d06ca4940e72e4e06b..d1929bf25834f09db9a8d503718376ae43838685 100644 --- a/app/services/packages/nuget/extract_metadata_content_service.rb +++ b/app/services/packages/nuget/extract_metadata_content_service.rb @@ -20,6 +20,8 @@ class ExtractMetadataContentService XPATH_TAGS = "#{ROOT_XPATH}:tags".freeze XPATH_PACKAGE_TYPES = "#{ROOT_XPATH}:packageTypes/xmlns:packageType".freeze + EMPTY_DEPENDENCY_PREFIX = 'EmptyDependencyForNuGetPackages-' + def initialize(nuspec_file_content) @doc = Nokogiri::XML(nuspec_file_content) end @@ -51,9 +53,16 @@ def extract_dependencies 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) + group_dependencies = group_node.xpath('xmlns:dependency') + + if group_dependencies.any? + group_dependencies.each do |node| + dependencies << extract_dependency(node).merge(target_framework: target_framework) + end + else + # Add an entry for target frameworks with no dependencies + name = "#{EMPTY_DEPENDENCY_PREFIX}#{target_framework}" + dependencies << { name:, target_framework: } end end diff --git a/config/feature_flags/gitlab_com_derisk/persist_nuget_packages_empty_target_frameworks.yml b/config/feature_flags/gitlab_com_derisk/persist_nuget_packages_empty_target_frameworks.yml new file mode 100644 index 0000000000000000000000000000000000000000..96246fb5111080e3c4b86fc64735b2d894ac1181 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/persist_nuget_packages_empty_target_frameworks.yml @@ -0,0 +1,10 @@ +--- +name: persist_nuget_packages_empty_target_frameworks +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/440403 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/211482 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/579439 +milestone: '18.6' +group: group::package registry +type: gitlab_com_derisk +default_enabled: false diff --git a/lib/api/entities/nuget/dependency_group.rb b/lib/api/entities/nuget/dependency_group.rb index f304b62d02142adea63d3e63011cc140771f384f..02c917f5acd19909d6520006458cbeecd614f792 100644 --- a/lib/api/entities/nuget/dependency_group.rb +++ b/lib/api/entities/nuget/dependency_group.rb @@ -8,7 +8,7 @@ class DependencyGroup < Grape::Entity expose :type, as: :@type, documentation: { type: 'string', example: 'PackageDependencyGroup' } expose :target_framework, as: :targetFramework, expose_nil: false, documentation: { type: 'string', example: 'fwk test' } - expose :dependencies, using: ::API::Entities::Nuget::Dependency, + expose :dependencies, using: ::API::Entities::Nuget::Dependency, expose_nil: false, documentation: { is_array: true, type: 'API::Entities::Nuget::Dependency' } end end diff --git a/spec/fixtures/api/schemas/graphql/packages/package_details.json b/spec/fixtures/api/schemas/graphql/packages/package_details.json index 348e29529f95a1326f58ffe77e0fda723e437939..c597067c054a8689d6fe7528c898ee4adfbcbf6e 100644 --- a/spec/fixtures/api/schemas/graphql/packages/package_details.json +++ b/spec/fixtures/api/schemas/graphql/packages/package_details.json @@ -202,18 +202,25 @@ "type": "string" }, "dependency": { - "type": "object", - "properties": { - "id": { - "type": "string" - }, - "name": { - "type": "string" + "anyOf": [ + { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "name": { + "type": "string" + }, + "versionPattern": { + "type": "string" + } + } }, - "versionPattern": { - "type": "string" + { + "type": "null" } - } + ] }, "metadata": { "anyOf": [ diff --git a/spec/fixtures/api/schemas/public_api/v4/packages/nuget/dependency_group.json b/spec/fixtures/api/schemas/public_api/v4/packages/nuget/dependency_group.json index 87dc2794b61bcdbc91c7ab804b896ce56d648db2..77d229ef1ba75e126b60f2b25c4d9985184b4d3e 100644 --- a/spec/fixtures/api/schemas/public_api/v4/packages/nuget/dependency_group.json +++ b/spec/fixtures/api/schemas/public_api/v4/packages/nuget/dependency_group.json @@ -1,20 +1,42 @@ { "type": "object", - "required": ["@id", "@type", "dependencies"], + "required": [ + "@id", + "@type" + ], "properties": { - "@id": { "type": "string" }, - "@type": { "const": "PackageDependencyGroup" }, - "targetFramework": { "type": "string" }, + "@id": { + "type": "string" + }, + "@type": { + "const": "PackageDependencyGroup" + }, + "targetFramework": { + "type": "string" + }, "dependencies": { "type": "array", "items": { "type": "object", - "required": ["@id", "@type", "id", "range"], + "required": [ + "@id", + "@type", + "id", + "range" + ], "properties": { - "@id": { "type": "string" }, - "@type": { "const": "PackageDependency" }, - "id": { "type": "string" }, - "range": { "type": "string" } + "@id": { + "type": "string" + }, + "@type": { + "const": "PackageDependency" + }, + "id": { + "type": "string" + }, + "range": { + "type": "string" + } } } } diff --git a/spec/fixtures/packages/nuget/with_empty_groups.nuspec b/spec/fixtures/packages/nuget/with_empty_groups.nuspec new file mode 100644 index 0000000000000000000000000000000000000000..6cae8e1180d61982da8fee708faaa9eac307dec5 --- /dev/null +++ b/spec/fixtures/packages/nuget/with_empty_groups.nuspec @@ -0,0 +1,17 @@ + + + + TestPackage + 1.0.0 + Test Author + Test package with empty dependency groups + + + + + + + + + + diff --git a/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb b/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb index 249b0a972723c36b8f481f978a38d01a20836cf3..a4da4a3505803629526b86211eee1bba118a319f 100644 --- a/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb +++ b/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb @@ -2,11 +2,13 @@ require 'spec_helper' -RSpec.describe Packages::Nuget::PackagesMetadataPresenter, feature_category: :package_registry do +RSpec.describe Packages::Nuget::PackagesMetadataPresenter, :aggregate_failures, feature_category: :package_registry do include_context 'with expected presenters dependency groups' let_it_be(:project) { create(:project) } - let_it_be(:packages) { create_list(:nuget_package, 5, :with_metadatum, name: 'Dummy.Package', project: project) } + let_it_be(:packages) do + create_list(:nuget_package, 5, :with_metadatum, without_package_files: true, name: 'Dummy.Package', project: project) + end let(:presenter) { described_class.new(::Packages::Nuget::Package.for_projects(project)) } @@ -17,11 +19,11 @@ end describe '#items' do - let(:tag_names) { %w[tag1 tag2] } + let_it_be(:tag_names) { %w[tag1 tag2] } subject { presenter.items } - before do + before_all do packages.each do |pkg| tag_names.each { |tag| create(:packages_tag, package: pkg, name: tag) } diff --git a/spec/requests/api/graphql/packages/nuget_spec.rb b/spec/requests/api/graphql/packages/nuget_spec.rb index 1c3af46909e6b7e9e64d714ea63c2adb480c448c..c4bffe0ca502fec65ed90b22bf533135d0341274 100644 --- a/spec/requests/api/graphql/packages/nuget_spec.rb +++ b/spec/requests/api/graphql/packages/nuget_spec.rb @@ -7,10 +7,16 @@ let_it_be(:package) { create(:nuget_package, :last_downloaded_at, :with_metadatum, project: project) } let_it_be(:dependency_link) { create(:packages_dependency_link, :with_nuget_metadatum, package: package) } + let_it_be(:empty_dependency) do + create(:packages_dependency, project: project, + name: Packages::Nuget::ExtractMetadataContentService::EMPTY_DEPENDENCY_PREFIX).tap do |dependency| + create(:packages_dependency_link, :with_nuget_metadatum, package:, dependency:) + end + end let(:metadata) { query_graphql_fragment('NugetMetadata') } - let(:dependency_link_response) { graphql_data_at(:package, :dependency_links, :nodes, 0) } - let(:dependency_response) { graphql_data_at(:package, :dependency_links, :nodes, 0, :dependency) } + let(:dependency_link_response) { graphql_data_at(:package, :dependency_links, :nodes) } + let(:dependency_response) { graphql_dig_at(dependency_link_response, :dependency) } subject { post_graphql(query, current_user: user) } @@ -27,14 +33,16 @@ ) end - it 'has dependency links' do - expect(dependency_link_response).to match a_graphql_entity_for( - dependency_link, - 'dependencyType' => dependency_link.dependency_type.upcase + it 'has dependency links', :aggregate_failures do + expect(dependency_link_response).to contain_exactly( + a_graphql_entity_for(dependency_link, 'dependencyType' => dependency_link.dependency_type.upcase), + a_graphql_entity_for(empty_dependency.dependency_links.take, 'dependency' => nil, + 'dependencyType' => dependency_link.dependency_type.upcase) ) - expect(dependency_response).to match a_graphql_entity_for( - dependency_link.dependency, :name, :version_pattern + expect(dependency_response).to contain_exactly( + a_graphql_entity_for(dependency_link.dependency, :name, :version_pattern), + nil ) end diff --git a/spec/services/packages/nuget/create_dependency_service_spec.rb b/spec/services/packages/nuget/create_dependency_service_spec.rb index aa469114b6f3f747d8b7c75267277130514a8802..71629bb4bddd07723e9c63585006fc4464bb9432 100644 --- a/spec/services/packages/nuget/create_dependency_service_spec.rb +++ b/spec/services/packages/nuget/create_dependency_service_spec.rb @@ -80,5 +80,22 @@ subject end end + + context 'when persist_nuget_packages_empty_target_frameworks feature flag is disabled' do + let(:dependencies) do + [ + { name: 'Moqi', version: '2.5.6' }, + { name: Packages::Nuget::ExtractMetadataContentService::EMPTY_DEPENDENCY_PREFIX, target_framework: '.NETStandard2.0' } + ] + end + + before do + stub_feature_flags(persist_nuget_packages_empty_target_frameworks: false) + end + + it_behaves_like 'creating dependencies, links and nuget metadata for', %w[Moqi], 1, 1 do + let(:dependencies_with_metadata) { [] } + end + end end end diff --git a/spec/services/packages/nuget/extract_metadata_content_service_spec.rb b/spec/services/packages/nuget/extract_metadata_content_service_spec.rb index 1a87c88fc077a4a3df3be55601177b53422a927b..c56531fa28f675329536971f21f76c6339bf7b86 100644 --- a/spec/services/packages/nuget/extract_metadata_content_service_spec.rb +++ b/spec/services/packages/nuget/extract_metadata_content_service_spec.rb @@ -58,5 +58,18 @@ expect(subject.slice(*expected_metadata.keys)).to eq(expected_metadata) end end + + context 'with empty dependency groups' do + let(:nuspec_filepath) { 'packages/nuget/with_empty_groups.nuspec' } + + it 'extracts target frameworks with no dependencies' do + expect(subject[:package_dependencies]) + .to include(name: "#{described_class::EMPTY_DEPENDENCY_PREFIX}.NETFramework4.5", + target_framework: '.NETFramework4.5') + .and include(name: "#{described_class::EMPTY_DEPENDENCY_PREFIX}.NETCore4.5", target_framework: '.NETCore4.5') + .and include(name: 'System.Net.Http', version: '4.3.1', target_framework: '.NETStandard2.0') + .and include(name: 'Moqi', version: '2.5.6') + end + end end end diff --git a/spec/support/shared_contexts/presenters/nuget_shared_context.rb b/spec/support/shared_contexts/presenters/nuget_shared_context.rb index 280967d86f72e080ec0900eed37c6fd7793931ea..ef5f138b8754e7afa056798a7a4ff5fead984c3f 100644 --- a/spec/support/shared_contexts/presenters/nuget_shared_context.rb +++ b/spec/support/shared_contexts/presenters/nuget_shared_context.rb @@ -27,6 +27,11 @@ def expected_dependency_groups(project_id, package_name, package_version) type: 'PackageDependency' } ] + }, + { + id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/.netcore4.5", + type: 'PackageDependencyGroup', + target_framework: '.NETCore4.5' } ] end @@ -37,7 +42,14 @@ def create_dependencies_for(package) dependency2 = Packages::Dependency.find_by(name: 'Castle.Core', version_pattern: '4.4.1') || create(:packages_dependency, name: 'Castle.Core', version_pattern: '4.4.1') + empty_dep_name = "#{Packages::Nuget::ExtractMetadataContentService::EMPTY_DEPENDENCY_PREFIX}.NETCore4.5}" + dependency3 = Packages::Dependency.find_by(name: empty_dep_name) || + create(:packages_dependency, name: empty_dep_name) + create(:packages_dependency_link, :with_nuget_metadatum, package: package, dependency: dependency1) create(:packages_dependency_link, package: package, dependency: dependency2) + create(:packages_dependency_link, package: package, dependency: dependency3).tap do |link| + create(:nuget_dependency_link_metadatum, dependency_link: link, target_framework: '.NETCore4.5') + end end end