From 82f29407249ddcc69f32685bbeba92c676cd14ed Mon Sep 17 00:00:00 2001 From: moaz-khalifa Date: Tue, 4 Nov 2025 19:35:06 +0100 Subject: [PATCH 1/3] Persist NuGet package dependencies in metadata table - Introduced a new `dependencies` field in the `packages_nuget_metadata` table. - Implemented validation for dependencies against a JSON schema. - Updated services and presenters to handle metadata dependencies in NuGet packages. - Added tests to ensure correct behavior. - Introduced a feature flag for controlling the new dependencies functionality. Changelog: fixed --- app/models/packages/nuget/metadatum.rb | 3 + .../packages/nuget/presenter_helpers.rb | 25 +++++++ .../nuget/create_dependency_service.rb | 21 +++++- .../nuget/extract_metadata_content_service.rb | 12 +++- .../nuget_package_metadatum_dependencies.json | 30 +++++++++ .../new_nuget_registry_dependencies.yml | 10 +++ ...dependencies_to_packages_nuget_metadata.rb | 9 +++ db/schema_migrations/20251031111549 | 1 + db/structure.sql | 1 + lib/api/entities/nuget/dependency_group.rb | 2 +- .../packages/nuget/with_empty_groups.nuspec | 17 +++++ spec/models/packages/nuget/metadatum_spec.rb | 52 +++++++++++++++ .../nuget/packages_metadata_presenter_spec.rb | 46 ++++++++++++- .../nuget/create_dependency_service_spec.rb | 65 +++++++++++++++++-- .../extract_metadata_content_service_spec.rb | 11 ++++ ...date_package_from_metadata_service_spec.rb | 28 +++++++- .../presenters/nuget_shared_context.rb | 35 ++++++++++ 17 files changed, 356 insertions(+), 12 deletions(-) create mode 100644 app/validators/json_schemas/nuget_package_metadatum_dependencies.json create mode 100644 config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml create mode 100644 db/migrate/20251031111549_add_dependencies_to_packages_nuget_metadata.rb create mode 100644 db/schema_migrations/20251031111549 create mode 100644 spec/fixtures/packages/nuget/with_empty_groups.nuspec diff --git a/app/models/packages/nuget/metadatum.rb b/app/models/packages/nuget/metadatum.rb index 79c4b1a003550e..58fdda9f19d718 100644 --- a/app/models/packages/nuget/metadatum.rb +++ b/app/models/packages/nuget/metadatum.rb @@ -27,6 +27,9 @@ class Packages::Nuget::Metadatum < ApplicationRecord validates :authors, presence: true, length: { maximum: MAX_AUTHORS_LENGTH } validates :description, presence: true, length: { maximum: MAX_DESCRIPTION_LENGTH } validates :normalized_version, presence: true + validates :dependencies, + json_schema: { filename: 'nuget_package_metadatum_dependencies', size_limit: 64.kilobytes, detail_errors: true }, + if: :dependencies delegate :version, to: :package, prefix: true diff --git a/app/presenters/packages/nuget/presenter_helpers.rb b/app/presenters/packages/nuget/presenter_helpers.rb index 16c32a9d0d0d23..5d9fef838659a8 100644 --- a/app/presenters/packages/nuget/presenter_helpers.rb +++ b/app/presenters/packages/nuget/presenter_helpers.rb @@ -58,6 +58,11 @@ def catalog_entry_for(package) def dependency_groups_for(package) base_nuget_id = "#{json_url_for(package)}#dependencyGroup" + if Feature.enabled?(:new_nuget_registry_dependencies, Project.actor_from_id(package.project_id)) && + package.nuget_metadatum.dependencies.present? + return metadatum_dependencies(package.nuget_metadatum.dependencies, base_nuget_id) + end + dependency_links_grouped_by_target_framework(package).map do |target_framework, dependency_links| nuget_id = target_framework_nuget_id(base_nuget_id, target_framework) { @@ -69,6 +74,26 @@ def dependency_groups_for(package) end end + def metadatum_dependencies(dependencies, base_nuget_id) + dependencies.map do |target_framework, deps| + nuget_id = target_framework_nuget_id(base_nuget_id, target_framework) + + { + id: nuget_id, + type: PACKAGE_DEPENDENCY_GROUP, + target_framework: target_framework, + dependencies: deps.map do |dep| + { + id: "#{nuget_id}/#{dep['name'].downcase}", + type: PACKAGE_DEPENDENCY, + name: dep['name'], + range: dep['version'] + } + end + }.compact_blank + end + end + def dependency_links_grouped_by_target_framework(package) package .dependency_links diff --git a/app/services/packages/nuget/create_dependency_service.rb b/app/services/packages/nuget/create_dependency_service.rb index 85f295ac7b7163..e53ac39fad6ab5 100644 --- a/app/services/packages/nuget/create_dependency_service.rb +++ b/app/services/packages/nuget/create_dependency_service.rb @@ -14,6 +14,8 @@ def execute create_dependency_links create_dependency_link_metadata end + + create_metadatum_dependencies end private @@ -54,7 +56,7 @@ def raw_dependency_for(dependency) end def dependencies_for_create_dependency_service - names_and_versions = @dependencies.to_h do |dependency| + names_and_versions = @dependencies.reject { |dep| !dep[:name] }.to_h do |dependency| [dependency[:name], version_or_empty_string(dependency[:version])] end @@ -66,6 +68,23 @@ def version_or_empty_string(version) version end + + def create_metadatum_dependencies + return if Feature.disabled?(:new_nuget_registry_dependencies, @package.project) + + deps = @dependencies + .group_by { |dep| dep[:target_framework] } + .transform_values do |deps| + deps.map do |dep| + dep.except(:target_framework).tap do |d| + d[:version] = version_or_empty_string(d[:version]) if d[:name] + end + end.reject(&:empty?) + end + + metadatum = @package.nuget_metadatum || @package.build_nuget_metadatum + metadatum.persisted? ? metadatum.update!(dependencies: deps) : metadatum.assign_attributes(dependencies: deps) + end end end end diff --git a/app/services/packages/nuget/extract_metadata_content_service.rb b/app/services/packages/nuget/extract_metadata_content_service.rb index 6df902a5acad4b..206f4042ed7c82 100644 --- a/app/services/packages/nuget/extract_metadata_content_service.rb +++ b/app/services/packages/nuget/extract_metadata_content_service.rb @@ -51,9 +51,15 @@ 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 + dependencies << { target_framework: target_framework } end end diff --git a/app/validators/json_schemas/nuget_package_metadatum_dependencies.json b/app/validators/json_schemas/nuget_package_metadatum_dependencies.json new file mode 100644 index 00000000000000..7b94d13d0a359c --- /dev/null +++ b/app/validators/json_schemas/nuget_package_metadatum_dependencies.json @@ -0,0 +1,30 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "NuGet package metadatum dependencies", + "type": "object", + "minProperties": 1, + "patternProperties": { + "^.*$": { + "type": "array", + "items": { + "type": "object", + "properties": { + "name": { + "type": "string", + "minLength": 1 + }, + "version": { + "type": "string" + } + }, + "required": [ + "name", + "version" + ], + "additionalProperties": false + }, + "uniqueItems": true + } + }, + "additionalProperties": false +} diff --git a/config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml b/config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml new file mode 100644 index 00000000000000..6bc9fbf8b64565 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml @@ -0,0 +1,10 @@ +--- +name: new_nuget_registry_dependencies +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/440403 +introduced_by_url: +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/db/migrate/20251031111549_add_dependencies_to_packages_nuget_metadata.rb b/db/migrate/20251031111549_add_dependencies_to_packages_nuget_metadata.rb new file mode 100644 index 00000000000000..8230848695414a --- /dev/null +++ b/db/migrate/20251031111549_add_dependencies_to_packages_nuget_metadata.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddDependenciesToPackagesNugetMetadata < Gitlab::Database::Migration[2.3] + milestone '18.6' + + def change + add_column :packages_nuget_metadata, :dependencies, :jsonb + end +end diff --git a/db/schema_migrations/20251031111549 b/db/schema_migrations/20251031111549 new file mode 100644 index 00000000000000..840238c41cc859 --- /dev/null +++ b/db/schema_migrations/20251031111549 @@ -0,0 +1 @@ +0c81c450fb346b9c65559caf0a5261b0d620f4f4486d7152f2bfc183f22abecc \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index a777a5a801bd28..88fc8506521300 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -22441,6 +22441,7 @@ CREATE TABLE packages_nuget_metadata ( description text, normalized_version text, project_id bigint, + dependencies jsonb, CONSTRAINT check_6b272cad10 CHECK ((project_id IS NOT NULL)), CONSTRAINT check_9973c0cc33 CHECK ((char_length(normalized_version) <= 255)), CONSTRAINT check_d39a5fe9ee CHECK ((char_length(description) <= 4000)), diff --git a/lib/api/entities/nuget/dependency_group.rb b/lib/api/entities/nuget/dependency_group.rb index f304b62d02142a..02c917f5acd199 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/packages/nuget/with_empty_groups.nuspec b/spec/fixtures/packages/nuget/with_empty_groups.nuspec new file mode 100644 index 00000000000000..6cae8e1180d619 --- /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/models/packages/nuget/metadatum_spec.rb b/spec/models/packages/nuget/metadatum_spec.rb index b6fd7e29f8337d..05b31a835629b5 100644 --- a/spec/models/packages/nuget/metadatum_spec.rb +++ b/spec/models/packages/nuget/metadatum_spec.rb @@ -52,6 +52,58 @@ end end end + + describe '#dependencies' do + it 'validates #dependencies against the nuget_package_metadatum_dependencies schema', :aggregate_failures do + is_expected.to allow_values( + { + '.NETFramework4.7.2' => [ + { name: 'Newtonsoft.Json', version: '12.0.3' }, + { name: 'Castle.Core', version: '4.4.1' } + ], + '.NETStandard2.0' => [ + { name: 'Newtonsoft.Json', version: '12.0.3' } + ], + '' => [ + { name: 'Moqi', version: '2.5.6' }, + { name: 'Test.Dependency', version: '' } + ], + '.NETCoreApp3.1' => [] + }, + nil + ).for(:dependencies) + + is_expected.not_to allow_values( + { + '.NETFramework4.7.2' => [ + { name: 'Newtonsoft.Json' } + ] + }, + { + '.NETFramework4.7.2' => [ + { version: '12.0.3' } + ] + }, + { + '.NETFramework4.7.2' => [ + { name: 'Newtonsoft.Json', version: '12.0.3', extra_field: 'not allowed' } + ] + }, + { + '.NETFramework4.7.2' => [ + { name: '', version: '12.0.3' } + ] + }, + { '.NETStandard2.0' => [ + { name: 'Newtonsoft.Json', version: '12.0.3' }, + { name: 'Newtonsoft.Json', version: '12.0.3' } + ] }, + { '.NETFramework4.7.2' => 'should be an array, not a string' }, + 'invalid', + {} + ).for(:dependencies) + end + end end describe 'delegations' do diff --git a/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb b/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb index 249b0a972723c3..79a2d7eef3cc20 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)) } @@ -82,5 +84,45 @@ expect(item[:lower_version]).to eq sorted_versions.first expect(item[:upper_version]).to eq sorted_versions.last end + + context 'when dependencies are persisted in metadatum' do + before do + packages.each do |pkg| + pkg.nuget_metadatum.update!(dependencies: { + '.NETStandard2.0' => [{ 'name' => 'Newtonsoft.Json', 'version' => '12.0.3' }], + '.NETCoreApp3.1' => [], + nil => [{ 'name' => 'Moqi', 'version' => '2.5.6' }] + }) + end + end + + it 'uses metadatum dependencies' do + item = subject.first + + item[:packages].each do |pkg| + catalog_entry = pkg[:catalog_entry] + expect(catalog_entry[:dependency_groups]).to match_array( + expected_dependency_groups_from_metadatum(project.id, catalog_entry[:package_name], catalog_entry[:package_version]) + ) + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(new_nuget_registry_dependencies: false) + end + + it 'uses dependency links instead of metadatum dependencies' do + item = subject.first + + item[:packages].each do |pkg| + catalog_entry = pkg[:catalog_entry] + expect(catalog_entry[:dependency_groups]).to match_array( + expected_dependency_groups(project.id, catalog_entry[:package_name], catalog_entry[:package_version]) + ) + end + end + end + end end end diff --git a/spec/services/packages/nuget/create_dependency_service_spec.rb b/spec/services/packages/nuget/create_dependency_service_spec.rb index aa469114b6f3f7..88e4230d763e7b 100644 --- a/spec/services/packages/nuget/create_dependency_service_spec.rb +++ b/spec/services/packages/nuget/create_dependency_service_spec.rb @@ -1,19 +1,20 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Packages::Nuget::CreateDependencyService, feature_category: :package_registry do - let_it_be(:package, reload: true) { create(:nuget_package) } +RSpec.describe Packages::Nuget::CreateDependencyService, :aggregate_failures, feature_category: :package_registry do + let_it_be(:package, reload: true) { create(:nuget_package, :with_metadatum, without_package_files: true) } 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 + it 'creates dependencies, links, nuget metadata and metadatum dependencies' do expect { subject } .to change { Packages::Dependency.count }.by(dependency_count) .and change { Packages::DependencyLink.count }.by(dependency_link_count) .and change { Packages::Nuget::DependencyLinkMetadatum.count }.by(dependencies_with_metadata.size) - expect(expected_dependency_names).to contain_exactly(*dependency_names) + .and change { package.nuget_metadatum.dependencies }.from(nil).to(metadatum_dependencies) + expect(expected_dependency_names).to match_array(dependency_names) expect(package.dependency_links.map(&:dependency_type).uniq).to contain_exactly('dependencies') dependencies_with_metadata.each do |dependency| @@ -25,6 +26,16 @@ expect(metadatum.target_framework).to eq dependency[:target_framework] end end + + context 'when new_nuget_registry_dependencies feature flag is disabled' do + before do + stub_feature_flags(new_nuget_registry_dependencies: false) + end + + it 'does not update metadatum dependencies' do + expect { subject }.not_to change { package.nuget_metadatum.dependencies } + end + end end let_it_be(:dependencies) do @@ -36,6 +47,19 @@ ] end + let(:metadatum_dependencies) do + { + '' => [ + { 'name' => 'Moqi', 'version' => '2.5.6' }, + { 'name' => 'Castle.Core', 'version' => '' } + ], + '.NETStandard2.0' => [ + { 'name' => 'Test.Dependency', 'version' => '2.3.7' }, + { 'name' => 'Newtonsoft.Json', 'version' => '12.0.3' } + ] + } + end + let(:dependency_names) { package.dependency_links.flat_map(&:dependency).map(&:name) } let(:service) { described_class.new(package, dependencies) } @@ -67,9 +91,42 @@ ] end + let(:metadatum_dependencies) do + { + '' => [ + { 'name' => 'Moqi', 'version' => '2.5.6' }, + { 'name' => 'Castle.Core', 'version' => '' }, + { '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 target frameworks but no dependencies' do + let(:dependencies) do + [ + { target_framework: '.NETFramework4.7.2' }, + { target_framework: '.NETStandard2.0' } + ] + end + + let(:metadatum_dependencies) do + { + '.NETFramework4.7.2' => [], + '.NETStandard2.0' => [] + } + end + + it 'updates metadatum dependencies but not dependency links' do + expect { subject }.to change { package.nuget_metadatum.dependencies }.from(nil).to(metadatum_dependencies) + .and not_change { Packages::Dependency.count } + .and not_change { Packages::DependencyLink.count } + end + end + context 'with empty dependencies' do let_it_be(:dependencies) { [] } 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 1a87c88fc077a4..1b717d59ebc6ac 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,16 @@ 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(target_framework: '.NETFramework4.5') + .and include(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/services/packages/nuget/update_package_from_metadata_service_spec.rb b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb index 8c87f5b52f25af..4b603b6ab29883 100644 --- a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb +++ b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_redis_shared_state, feature_category: :package_registry do include ExclusiveLeaseHelpers - let!(:package) { create(:nuget_package, :processing, :with_symbol_package, :with_build) } + let!(:package) { create(:nuget_package, :processing, :with_symbol_package, :with_build, :with_metadatum) } # Reload factory to reset associations cache for package files let(:package_file) { package.reload.package_files.first } let(:package_zip_file) { Zip::File.new(package_file.file) } @@ -232,6 +232,32 @@ # hard reset needed to properly reload package_file.file expect(Packages::PackageFile.find(package_file.id).file.size).not_to eq 0 end + + context 'with empty target frameworks' do + let(:nuspec_filepath) { 'packages/nuget/with_empty_groups.nuspec' } + + it 'updates package metadatum dependencies', :aggregate_failures do + expect { subject } + .to change { package.reload.nuget_metadatum.dependencies }.from(nil).to( + { + '' => [{ 'name' => 'Moqi', 'version' => '2.5.6' }], + '.NETFramework4.5' => [], + '.NETCore4.5' => [], + '.NETStandard2.0' => [{ 'name' => 'System.Net.Http', 'version' => '4.3.1' }] + } + ) + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(new_nuget_registry_dependencies: false) + end + + it 'does not update metadatum dependencies' do + expect { subject }.not_to change { package.nuget_metadatum.dependencies } + end + end + end end context 'with package file not containing a nuspec file' do diff --git a/spec/support/shared_contexts/presenters/nuget_shared_context.rb b/spec/support/shared_contexts/presenters/nuget_shared_context.rb index 280967d86f72e0..0a866b610c5b9f 100644 --- a/spec/support/shared_contexts/presenters/nuget_shared_context.rb +++ b/spec/support/shared_contexts/presenters/nuget_shared_context.rb @@ -31,6 +31,41 @@ def expected_dependency_groups(project_id, package_name, package_version) ] end + def expected_dependency_groups_from_metadatum(project_id, package_name, package_version) + [ + { + id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/.netstandard2.0", + target_framework: '.NETStandard2.0', + type: 'PackageDependencyGroup', + dependencies: [ + { + id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/.netstandard2.0/newtonsoft.json", + range: '12.0.3', + name: 'Newtonsoft.Json', + type: 'PackageDependency' + } + ] + }, + { + id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/.netcoreapp3.1", + target_framework: '.NETCoreApp3.1', + type: 'PackageDependencyGroup' + }, + { + id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup", + type: 'PackageDependencyGroup', + dependencies: [ + { + id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/moqi", + range: '2.5.6', + name: 'Moqi', + type: 'PackageDependency' + } + ] + } + ] + end + def create_dependencies_for(package) dependency1 = Packages::Dependency.find_by(name: 'Newtonsoft.Json', version_pattern: '12.0.3') || create(:packages_dependency, name: 'Newtonsoft.Json', version_pattern: '12.0.3') -- GitLab From e989baaa8df56198ef6808612842ec7e28109fad Mon Sep 17 00:00:00 2001 From: Moaz Khalifa Date: Tue, 4 Nov 2025 20:47:09 +0200 Subject: [PATCH 2/3] Fix failing specs --- app/presenters/packages/nuget/presenter_helpers.rb | 4 ++-- .../gitlab_com_derisk/new_nuget_registry_dependencies.yml | 2 +- .../nuget/update_package_from_metadata_service_spec.rb | 8 ++++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/presenters/packages/nuget/presenter_helpers.rb b/app/presenters/packages/nuget/presenter_helpers.rb index 5d9fef838659a8..8adc94a98c53ed 100644 --- a/app/presenters/packages/nuget/presenter_helpers.rb +++ b/app/presenters/packages/nuget/presenter_helpers.rb @@ -59,7 +59,7 @@ def dependency_groups_for(package) base_nuget_id = "#{json_url_for(package)}#dependencyGroup" if Feature.enabled?(:new_nuget_registry_dependencies, Project.actor_from_id(package.project_id)) && - package.nuget_metadatum.dependencies.present? + package&.nuget_metadatum&.dependencies.present? return metadatum_dependencies(package.nuget_metadatum.dependencies, base_nuget_id) end @@ -84,7 +84,7 @@ def metadatum_dependencies(dependencies, base_nuget_id) target_framework: target_framework, dependencies: deps.map do |dep| { - id: "#{nuget_id}/#{dep['name'].downcase}", + id: "#{nuget_id}/#{dep['name']&.downcase}", type: PACKAGE_DEPENDENCY, name: dep['name'], range: dep['version'] diff --git a/config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml b/config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml index 6bc9fbf8b64565..e19ddc52ce0b24 100644 --- a/config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml +++ b/config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml @@ -2,7 +2,7 @@ name: new_nuget_registry_dependencies description: feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/440403 -introduced_by_url: +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 diff --git a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb index 4b603b6ab29883..bc4de5d54e35e1 100644 --- a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb +++ b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_redis_shared_state, feature_category: :package_registry do include ExclusiveLeaseHelpers - let!(:package) { create(:nuget_package, :processing, :with_symbol_package, :with_build, :with_metadatum) } + let!(:package) { create(:nuget_package, :processing, :with_symbol_package, :with_build) } # Reload factory to reset associations cache for package files let(:package_file) { package.reload.package_files.first } let(:package_zip_file) { Zip::File.new(package_file.file) } @@ -236,9 +236,13 @@ context 'with empty target frameworks' do let(:nuspec_filepath) { 'packages/nuget/with_empty_groups.nuspec' } + before do + create(:nuget_metadatum, package:) + end + it 'updates package metadatum dependencies', :aggregate_failures do expect { subject } - .to change { package.reload.nuget_metadatum.dependencies }.from(nil).to( + .to change { package.nuget_metadatum.dependencies }.from(nil).to( { '' => [{ 'name' => 'Moqi', 'version' => '2.5.6' }], '.NETFramework4.5' => [], -- GitLab From fcf64555d9a10432c88b04386d32dc6307f291e6 Mon Sep 17 00:00:00 2001 From: moaz-khalifa Date: Tue, 11 Nov 2025 16:35:32 +0100 Subject: [PATCH 3/3] Apply maintainer feedback --- .../packages/package_dependency_link_type.rb | 10 ++- app/models/packages/nuget/metadatum.rb | 3 - .../packages/nuget/presenter_helpers.rb | 31 +------ .../nuget/create_dependency_service.rb | 29 ++----- .../nuget/extract_metadata_content_service.rb | 5 +- .../nuget_package_metadatum_dependencies.json | 30 ------- ...uget_packages_empty_target_frameworks.yml} | 2 +- ...dependencies_to_packages_nuget_metadata.rb | 9 -- db/schema_migrations/20251031111549 | 1 - db/structure.sql | 1 - .../graphql/packages/package_details.json | 27 +++--- .../v4/packages/nuget/dependency_group.json | 40 +++++++-- spec/models/packages/nuget/metadatum_spec.rb | 52 ------------ .../nuget/packages_metadata_presenter_spec.rb | 44 +--------- .../api/graphql/packages/nuget_spec.rb | 24 ++++-- .../nuget/create_dependency_service_spec.rb | 82 +++++-------------- .../extract_metadata_content_service_spec.rb | 6 +- ...date_package_from_metadata_service_spec.rb | 30 ------- .../presenters/nuget_shared_context.rb | 41 ++-------- 19 files changed, 127 insertions(+), 340 deletions(-) delete mode 100644 app/validators/json_schemas/nuget_package_metadatum_dependencies.json rename config/feature_flags/gitlab_com_derisk/{new_nuget_registry_dependencies.yml => persist_nuget_packages_empty_target_frameworks.yml} (86%) delete mode 100644 db/migrate/20251031111549_add_dependencies_to_packages_nuget_metadata.rb delete mode 100644 db/schema_migrations/20251031111549 diff --git a/app/graphql/types/packages/package_dependency_link_type.rb b/app/graphql/types/packages/package_dependency_link_type.rb index d49c467a53da54..d5194c8c373dfe 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/models/packages/nuget/metadatum.rb b/app/models/packages/nuget/metadatum.rb index 58fdda9f19d718..79c4b1a003550e 100644 --- a/app/models/packages/nuget/metadatum.rb +++ b/app/models/packages/nuget/metadatum.rb @@ -27,9 +27,6 @@ class Packages::Nuget::Metadatum < ApplicationRecord validates :authors, presence: true, length: { maximum: MAX_AUTHORS_LENGTH } validates :description, presence: true, length: { maximum: MAX_DESCRIPTION_LENGTH } validates :normalized_version, presence: true - validates :dependencies, - json_schema: { filename: 'nuget_package_metadatum_dependencies', size_limit: 64.kilobytes, detail_errors: true }, - if: :dependencies delegate :version, to: :package, prefix: true diff --git a/app/presenters/packages/nuget/presenter_helpers.rb b/app/presenters/packages/nuget/presenter_helpers.rb index 8adc94a98c53ed..b5f0e40ff05f50 100644 --- a/app/presenters/packages/nuget/presenter_helpers.rb +++ b/app/presenters/packages/nuget/presenter_helpers.rb @@ -58,11 +58,6 @@ def catalog_entry_for(package) def dependency_groups_for(package) base_nuget_id = "#{json_url_for(package)}#dependencyGroup" - if Feature.enabled?(:new_nuget_registry_dependencies, Project.actor_from_id(package.project_id)) && - package&.nuget_metadatum&.dependencies.present? - return metadatum_dependencies(package.nuget_metadatum.dependencies, base_nuget_id) - end - dependency_links_grouped_by_target_framework(package).map do |target_framework, dependency_links| nuget_id = target_framework_nuget_id(base_nuget_id, target_framework) { @@ -74,26 +69,6 @@ def dependency_groups_for(package) end end - def metadatum_dependencies(dependencies, base_nuget_id) - dependencies.map do |target_framework, deps| - nuget_id = target_framework_nuget_id(base_nuget_id, target_framework) - - { - id: nuget_id, - type: PACKAGE_DEPENDENCY_GROUP, - target_framework: target_framework, - dependencies: deps.map do |dep| - { - id: "#{nuget_id}/#{dep['name']&.downcase}", - type: PACKAGE_DEPENDENCY, - name: dep['name'], - range: dep['version'] - } - end - }.compact_blank - end - end - def dependency_links_grouped_by_target_framework(package) package .dependency_links @@ -103,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 e53ac39fad6ab5..e8ad2cb048d244 100644 --- a/app/services/packages/nuget/create_dependency_service.rb +++ b/app/services/packages/nuget/create_dependency_service.rb @@ -14,8 +14,6 @@ def execute create_dependency_links create_dependency_link_metadata end - - create_metadatum_dependencies end private @@ -56,7 +54,15 @@ def raw_dependency_for(dependency) end def dependencies_for_create_dependency_service - names_and_versions = @dependencies.reject { |dep| !dep[:name] }.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 @@ -68,23 +74,6 @@ def version_or_empty_string(version) version end - - def create_metadatum_dependencies - return if Feature.disabled?(:new_nuget_registry_dependencies, @package.project) - - deps = @dependencies - .group_by { |dep| dep[:target_framework] } - .transform_values do |deps| - deps.map do |dep| - dep.except(:target_framework).tap do |d| - d[:version] = version_or_empty_string(d[:version]) if d[:name] - end - end.reject(&:empty?) - end - - metadatum = @package.nuget_metadatum || @package.build_nuget_metadatum - metadatum.persisted? ? metadatum.update!(dependencies: deps) : metadatum.assign_attributes(dependencies: deps) - end end end end diff --git a/app/services/packages/nuget/extract_metadata_content_service.rb b/app/services/packages/nuget/extract_metadata_content_service.rb index 206f4042ed7c82..d1929bf25834f0 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 @@ -59,7 +61,8 @@ def extract_dependencies end else # Add an entry for target frameworks with no dependencies - dependencies << { target_framework: target_framework } + name = "#{EMPTY_DEPENDENCY_PREFIX}#{target_framework}" + dependencies << { name:, target_framework: } end end diff --git a/app/validators/json_schemas/nuget_package_metadatum_dependencies.json b/app/validators/json_schemas/nuget_package_metadatum_dependencies.json deleted file mode 100644 index 7b94d13d0a359c..00000000000000 --- a/app/validators/json_schemas/nuget_package_metadatum_dependencies.json +++ /dev/null @@ -1,30 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "description": "NuGet package metadatum dependencies", - "type": "object", - "minProperties": 1, - "patternProperties": { - "^.*$": { - "type": "array", - "items": { - "type": "object", - "properties": { - "name": { - "type": "string", - "minLength": 1 - }, - "version": { - "type": "string" - } - }, - "required": [ - "name", - "version" - ], - "additionalProperties": false - }, - "uniqueItems": true - } - }, - "additionalProperties": false -} diff --git a/config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml b/config/feature_flags/gitlab_com_derisk/persist_nuget_packages_empty_target_frameworks.yml similarity index 86% rename from config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml rename to config/feature_flags/gitlab_com_derisk/persist_nuget_packages_empty_target_frameworks.yml index e19ddc52ce0b24..96246fb5111080 100644 --- a/config/feature_flags/gitlab_com_derisk/new_nuget_registry_dependencies.yml +++ b/config/feature_flags/gitlab_com_derisk/persist_nuget_packages_empty_target_frameworks.yml @@ -1,5 +1,5 @@ --- -name: new_nuget_registry_dependencies +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 diff --git a/db/migrate/20251031111549_add_dependencies_to_packages_nuget_metadata.rb b/db/migrate/20251031111549_add_dependencies_to_packages_nuget_metadata.rb deleted file mode 100644 index 8230848695414a..00000000000000 --- a/db/migrate/20251031111549_add_dependencies_to_packages_nuget_metadata.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class AddDependenciesToPackagesNugetMetadata < Gitlab::Database::Migration[2.3] - milestone '18.6' - - def change - add_column :packages_nuget_metadata, :dependencies, :jsonb - end -end diff --git a/db/schema_migrations/20251031111549 b/db/schema_migrations/20251031111549 deleted file mode 100644 index 840238c41cc859..00000000000000 --- a/db/schema_migrations/20251031111549 +++ /dev/null @@ -1 +0,0 @@ -0c81c450fb346b9c65559caf0a5261b0d620f4f4486d7152f2bfc183f22abecc \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 88fc8506521300..a777a5a801bd28 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -22441,7 +22441,6 @@ CREATE TABLE packages_nuget_metadata ( description text, normalized_version text, project_id bigint, - dependencies jsonb, CONSTRAINT check_6b272cad10 CHECK ((project_id IS NOT NULL)), CONSTRAINT check_9973c0cc33 CHECK ((char_length(normalized_version) <= 255)), CONSTRAINT check_d39a5fe9ee CHECK ((char_length(description) <= 4000)), diff --git a/spec/fixtures/api/schemas/graphql/packages/package_details.json b/spec/fixtures/api/schemas/graphql/packages/package_details.json index 348e29529f95a1..c597067c054a86 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 87dc2794b61bcd..77d229ef1ba75e 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/models/packages/nuget/metadatum_spec.rb b/spec/models/packages/nuget/metadatum_spec.rb index 05b31a835629b5..b6fd7e29f8337d 100644 --- a/spec/models/packages/nuget/metadatum_spec.rb +++ b/spec/models/packages/nuget/metadatum_spec.rb @@ -52,58 +52,6 @@ end end end - - describe '#dependencies' do - it 'validates #dependencies against the nuget_package_metadatum_dependencies schema', :aggregate_failures do - is_expected.to allow_values( - { - '.NETFramework4.7.2' => [ - { name: 'Newtonsoft.Json', version: '12.0.3' }, - { name: 'Castle.Core', version: '4.4.1' } - ], - '.NETStandard2.0' => [ - { name: 'Newtonsoft.Json', version: '12.0.3' } - ], - '' => [ - { name: 'Moqi', version: '2.5.6' }, - { name: 'Test.Dependency', version: '' } - ], - '.NETCoreApp3.1' => [] - }, - nil - ).for(:dependencies) - - is_expected.not_to allow_values( - { - '.NETFramework4.7.2' => [ - { name: 'Newtonsoft.Json' } - ] - }, - { - '.NETFramework4.7.2' => [ - { version: '12.0.3' } - ] - }, - { - '.NETFramework4.7.2' => [ - { name: 'Newtonsoft.Json', version: '12.0.3', extra_field: 'not allowed' } - ] - }, - { - '.NETFramework4.7.2' => [ - { name: '', version: '12.0.3' } - ] - }, - { '.NETStandard2.0' => [ - { name: 'Newtonsoft.Json', version: '12.0.3' }, - { name: 'Newtonsoft.Json', version: '12.0.3' } - ] }, - { '.NETFramework4.7.2' => 'should be an array, not a string' }, - 'invalid', - {} - ).for(:dependencies) - end - end end describe 'delegations' do diff --git a/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb b/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb index 79a2d7eef3cc20..a4da4a35058036 100644 --- a/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb +++ b/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb @@ -19,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) } @@ -84,45 +84,5 @@ expect(item[:lower_version]).to eq sorted_versions.first expect(item[:upper_version]).to eq sorted_versions.last end - - context 'when dependencies are persisted in metadatum' do - before do - packages.each do |pkg| - pkg.nuget_metadatum.update!(dependencies: { - '.NETStandard2.0' => [{ 'name' => 'Newtonsoft.Json', 'version' => '12.0.3' }], - '.NETCoreApp3.1' => [], - nil => [{ 'name' => 'Moqi', 'version' => '2.5.6' }] - }) - end - end - - it 'uses metadatum dependencies' do - item = subject.first - - item[:packages].each do |pkg| - catalog_entry = pkg[:catalog_entry] - expect(catalog_entry[:dependency_groups]).to match_array( - expected_dependency_groups_from_metadatum(project.id, catalog_entry[:package_name], catalog_entry[:package_version]) - ) - end - end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(new_nuget_registry_dependencies: false) - end - - it 'uses dependency links instead of metadatum dependencies' do - item = subject.first - - item[:packages].each do |pkg| - catalog_entry = pkg[:catalog_entry] - expect(catalog_entry[:dependency_groups]).to match_array( - expected_dependency_groups(project.id, catalog_entry[:package_name], catalog_entry[:package_version]) - ) - end - end - end - end end end diff --git a/spec/requests/api/graphql/packages/nuget_spec.rb b/spec/requests/api/graphql/packages/nuget_spec.rb index 1c3af46909e6b7..c4bffe0ca502fe 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 88e4230d763e7b..71629bb4bddd07 100644 --- a/spec/services/packages/nuget/create_dependency_service_spec.rb +++ b/spec/services/packages/nuget/create_dependency_service_spec.rb @@ -1,20 +1,19 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Packages::Nuget::CreateDependencyService, :aggregate_failures, feature_category: :package_registry do - let_it_be(:package, reload: true) { create(:nuget_package, :with_metadatum, without_package_files: true) } +RSpec.describe Packages::Nuget::CreateDependencyService, feature_category: :package_registry 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, nuget metadata and metadatum dependencies' do + 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::Nuget::DependencyLinkMetadatum.count }.by(dependencies_with_metadata.size) - .and change { package.nuget_metadatum.dependencies }.from(nil).to(metadatum_dependencies) - expect(expected_dependency_names).to match_array(dependency_names) + 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| @@ -26,16 +25,6 @@ expect(metadatum.target_framework).to eq dependency[:target_framework] end end - - context 'when new_nuget_registry_dependencies feature flag is disabled' do - before do - stub_feature_flags(new_nuget_registry_dependencies: false) - end - - it 'does not update metadatum dependencies' do - expect { subject }.not_to change { package.nuget_metadatum.dependencies } - end - end end let_it_be(:dependencies) do @@ -47,19 +36,6 @@ ] end - let(:metadatum_dependencies) do - { - '' => [ - { 'name' => 'Moqi', 'version' => '2.5.6' }, - { 'name' => 'Castle.Core', 'version' => '' } - ], - '.NETStandard2.0' => [ - { 'name' => 'Test.Dependency', 'version' => '2.3.7' }, - { 'name' => 'Newtonsoft.Json', 'version' => '12.0.3' } - ] - } - end - let(:dependency_names) { package.dependency_links.flat_map(&:dependency).map(&:name) } let(:service) { described_class.new(package, dependencies) } @@ -91,42 +67,9 @@ ] end - let(:metadatum_dependencies) do - { - '' => [ - { 'name' => 'Moqi', 'version' => '2.5.6' }, - { 'name' => 'Castle.Core', 'version' => '' }, - { '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 target frameworks but no dependencies' do - let(:dependencies) do - [ - { target_framework: '.NETFramework4.7.2' }, - { target_framework: '.NETStandard2.0' } - ] - end - - let(:metadatum_dependencies) do - { - '.NETFramework4.7.2' => [], - '.NETStandard2.0' => [] - } - end - - it 'updates metadatum dependencies but not dependency links' do - expect { subject }.to change { package.nuget_metadatum.dependencies }.from(nil).to(metadatum_dependencies) - .and not_change { Packages::Dependency.count } - .and not_change { Packages::DependencyLink.count } - end - end - context 'with empty dependencies' do let_it_be(:dependencies) { [] } @@ -137,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 1b717d59ebc6ac..c56531fa28f675 100644 --- a/spec/services/packages/nuget/extract_metadata_content_service_spec.rb +++ b/spec/services/packages/nuget/extract_metadata_content_service_spec.rb @@ -63,8 +63,10 @@ let(:nuspec_filepath) { 'packages/nuget/with_empty_groups.nuspec' } it 'extracts target frameworks with no dependencies' do - expect(subject[:package_dependencies]).to include(target_framework: '.NETFramework4.5') - .and include(target_framework: '.NETCore4.5') + 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 diff --git a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb index bc4de5d54e35e1..8c87f5b52f25af 100644 --- a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb +++ b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb @@ -232,36 +232,6 @@ # hard reset needed to properly reload package_file.file expect(Packages::PackageFile.find(package_file.id).file.size).not_to eq 0 end - - context 'with empty target frameworks' do - let(:nuspec_filepath) { 'packages/nuget/with_empty_groups.nuspec' } - - before do - create(:nuget_metadatum, package:) - end - - it 'updates package metadatum dependencies', :aggregate_failures do - expect { subject } - .to change { package.nuget_metadatum.dependencies }.from(nil).to( - { - '' => [{ 'name' => 'Moqi', 'version' => '2.5.6' }], - '.NETFramework4.5' => [], - '.NETCore4.5' => [], - '.NETStandard2.0' => [{ 'name' => 'System.Net.Http', 'version' => '4.3.1' }] - } - ) - end - - context 'when the feature flag is disabled' do - before do - stub_feature_flags(new_nuget_registry_dependencies: false) - end - - it 'does not update metadatum dependencies' do - expect { subject }.not_to change { package.nuget_metadatum.dependencies } - end - end - end end context 'with package file not containing a nuspec file' do diff --git a/spec/support/shared_contexts/presenters/nuget_shared_context.rb b/spec/support/shared_contexts/presenters/nuget_shared_context.rb index 0a866b610c5b9f..ef5f138b8754e7 100644 --- a/spec/support/shared_contexts/presenters/nuget_shared_context.rb +++ b/spec/support/shared_contexts/presenters/nuget_shared_context.rb @@ -27,41 +27,11 @@ def expected_dependency_groups(project_id, package_name, package_version) type: 'PackageDependency' } ] - } - ] - end - - def expected_dependency_groups_from_metadatum(project_id, package_name, package_version) - [ - { - id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/.netstandard2.0", - target_framework: '.NETStandard2.0', - type: 'PackageDependencyGroup', - dependencies: [ - { - id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/.netstandard2.0/newtonsoft.json", - range: '12.0.3', - name: 'Newtonsoft.Json', - type: 'PackageDependency' - } - ] }, { - id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/.netcoreapp3.1", - target_framework: '.NETCoreApp3.1', - type: 'PackageDependencyGroup' - }, - { - id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup", + id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/.netcore4.5", type: 'PackageDependencyGroup', - dependencies: [ - { - id: "http://localhost/api/v4/projects/#{project_id}/packages/nuget/metadata/#{package_name}/#{package_version}.json#dependencyGroup/moqi", - range: '2.5.6', - name: 'Moqi', - type: 'PackageDependency' - } - ] + target_framework: '.NETCore4.5' } ] end @@ -72,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 -- GitLab