diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index 34cf2c044188d732c0976e22d2f8fa046ccfd80d..9a80a607b4aa9897401e6f6da30eb0064a8d766e 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -190,6 +190,7 @@ class Packages::Package < ApplicationRecord scope :preload_files, -> { preload(:installable_package_files) } scope :preload_nuget_files, -> { preload(:installable_nuget_package_files) } scope :preload_pipelines, -> { preload(pipelines: :user) } + scope :preload_tags, -> { preload(:tags) } scope :limit_recent, ->(limit) { order_created_desc.limit(limit) } scope :select_distinct_name, -> { select(:name).distinct } diff --git a/app/services/packages/npm/generate_metadata_service.rb b/app/services/packages/npm/generate_metadata_service.rb index 240c657039fb97c232260e22597dd8437087cbc5..a9ee439cdd9d2da115fb06fd52dbdb2327cb57c7 100644 --- a/app/services/packages/npm/generate_metadata_service.rb +++ b/app/services/packages/npm/generate_metadata_service.rb @@ -16,17 +16,29 @@ def initialize(name, packages) @packages = packages @dependencies = {} @dependency_ids = Hash.new { |h, key| h[key] = {} } + @tags = {} + @tags_updated_at = {} + @versions_hash = {} + @latest_version = nil end def execute(only_dist_tags: false) - ServiceResponse.success(payload: metadata(only_dist_tags)) + payload = if Feature.enabled?(:package_registry_npm_fetch_all_tags, Feature.current_request, + type: :gitlab_com_derisk) + metadata(only_dist_tags) + else + legacy_metadata(only_dist_tags) + end + + ServiceResponse.success(payload: payload) end private - attr_reader :name, :packages, :dependencies, :dependency_ids + attr_reader :name, :packages, :dependencies, :dependency_ids, :tags, :tags_updated_at, :versions_hash + attr_accessor :latest_version - def metadata(only_dist_tags) + def legacy_metadata(only_dist_tags) result = { dist_tags: dist_tags } unless only_dist_tags @@ -37,6 +49,26 @@ def metadata(only_dist_tags) result end + def metadata(only_dist_tags) + packages.each_batch do |batch| + relation = preload_needed_relations(batch, only_dist_tags) + + relation.each do |package| + build_tags(package) + store_latest_version(package.version) + next if only_dist_tags + + build_versions(package) + end + end + + { + name: only_dist_tags ? nil : name, + versions: versions_hash, + dist_tags: tags.tap { |t| t['latest'] ||= latest_version } + }.compact_blank + end + def versions package_versions = {} @@ -138,6 +170,40 @@ def load_dependency_ids(packages) end end end + + def preload_needed_relations(batch, only_dist_tags) + relation = batch.preload_tags + + unless only_dist_tags + load_dependencies(relation) + load_dependency_ids(relation) + + relation = relation.preload_files.preload_npm_metadatum + end + + relation + end + + def build_tags(package) + package.tags.each do |tag| + next if tags.key?(tag.name) && tags_updated_at[tag.name] > tag.updated_at + + tags[tag.name] = package.version + tags_updated_at[tag.name] = tag.updated_at + end + end + + def store_latest_version(version) + self.latest_version = version if latest_version.blank? || VersionSorter.compare(version, latest_version) == 1 + end + + def build_versions(package) + package_file = package.installable_package_files.last + + return unless package_file + + versions_hash[package.version] = build_package_version(package, package_file) + end end end end diff --git a/config/feature_flags/gitlab_com_derisk/package_registry_npm_fetch_all_tags.yml b/config/feature_flags/gitlab_com_derisk/package_registry_npm_fetch_all_tags.yml new file mode 100644 index 0000000000000000000000000000000000000000..18ac4960907b7466272a16b9f1317e3563c74293 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/package_registry_npm_fetch_all_tags.yml @@ -0,0 +1,9 @@ +--- +name: package_registry_npm_fetch_all_tags +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/436099 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142573 +rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/17455 +milestone: '16.9' +group: group::package registry +type: gitlab_com_derisk +default_enabled: false diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 148d4b4dab60532171be636967cbe0e00d060ca8..f8a2b2ff6354a86c41be9c8570e61e7d68fcc469 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1127,6 +1127,17 @@ end end + describe '.preload_tags' do + let_it_be(:package) { create(:npm_package) } + let_it_be(:tags) { create_list(:packages_tag, 2, package: package) } + + subject { described_class.preload_tags } + + it 'preloads tags' do + expect(subject.first.association(:tags)).to be_loaded + end + end + describe '#versions' do let_it_be(:project) { create(:project) } let_it_be(:package) { create(:maven_package, project: project) } diff --git a/spec/services/packages/npm/generate_metadata_service_spec.rb b/spec/services/packages/npm/generate_metadata_service_spec.rb index ad3d4fde665fcaa944e9c1a0b5dd2a245935204b..fd29e79a1b9ef1a1b4656bc681ee71cda943fcc5 100644 --- a/spec/services/packages/npm/generate_metadata_service_spec.rb +++ b/spec/services/packages/npm/generate_metadata_service_spec.rb @@ -159,15 +159,54 @@ end context 'with duplicate tags' do - let_it_be(:project2) { create(:project, namespace: group) } - let_it_be(:package2) { create(:npm_package, version: '3.0.0', project: project2, name: package_name) } - let_it_be(:package_tag1) { create(:packages_tag, package: package1, name: 'latest') } - let_it_be(:package_tag2) { create(:packages_tag, package: package2, name: 'latest') } + context 'when in different projects' do + let_it_be(:project2) { create(:project, namespace: group) } + let_it_be(:package2) { create(:npm_package, version: '3.0.0', project: project2, name: package_name) } + let_it_be(:package_tag1) { create(:packages_tag, package: package1, name: 'latest') } + let_it_be(:package_tag2) { create(:packages_tag, package: package2, name: 'latest') } - let(:packages) { ::Packages::Package.for_projects([project.id, project2.id]).with_name(package_name) } + let(:packages) { ::Packages::Package.for_projects([project.id, project2.id]).with_name(package_name) } - it "returns the tag of the latest package's version" do - expect(subject['latest']).to eq(package2.version) + it "returns the tag of the latest package's version" do + expect(subject['latest']).to eq(package2.version) + end + end + + context 'when in the same project' do + let_it_be(:package_a) { create(:npm_package, version: '1.2.3', project: project, name: package_name) } + let_it_be(:package_b) { create(:npm_package, version: '1.1.1', project: project, name: package_name) } + let_it_be(:package_tag_a) { create(:packages_tag, package: package_a, name: 'tag', updated_at: 1.week.ago) } + let_it_be(:package_tag_b) { create(:packages_tag, package: package_b, name: 'tag') } + + it "returns the most recent tagged package's version" do + expect(subject['tag']).to eq(package_b.version) + end + end + end + + context 'when fetching all package tags' do + let_it_be(:tags_limit) { 3 } + + before do + stub_const('Packages::Tag::FOR_PACKAGES_TAGS_LIMIT', tags_limit) + end + + it 'returns all tags' do + expect(::Packages::Package).to receive(:preload_tags).and_call_original + + expect(subject.size).to eq(Packages::Tag.count) + end + + context 'with disabled package_registry_npm_fetch_all_tags feature flag' do + before do + stub_feature_flags(package_registry_npm_fetch_all_tags: false) + end + + it 'adheres to the tags limit' do + expect(::Packages::Package).not_to receive(:preload_tags) + + expect(subject.size).to eq(tags_limit) + end end end end