diff --git a/app/models/packages/tag.rb b/app/models/packages/tag.rb index 0df64bfba54c9c8d9a6d11423aee6273d6b7103f..95cf312c1745b6fcac6e40c294a6452ad463e13b 100644 --- a/app/models/packages/tag.rb +++ b/app/models/packages/tag.rb @@ -19,6 +19,17 @@ def self.for_package_ids(package_ids) .limit(FOR_PACKAGES_TAGS_LIMIT) end + def self.for_package_ids_with_distinct_names(package_ids) + inner_query = select('DISTINCT ON (name) *').order(:name).for_package_ids(package_ids) + + cte = Gitlab::SQL::CTE.new(:distinct_names_cte, inner_query) + cte_alias = cte.table.alias(table_name) + + with(cte.to_arel) + .from(cte_alias) + .order(updated_at: :desc) + end + def ensure_project_id self.project_id ||= package.project_id end diff --git a/app/services/packages/npm/generate_metadata_service.rb b/app/services/packages/npm/generate_metadata_service.rb index 8eaac547f7e406113f1e64d9f71f27dd0ece1e1a..240c657039fb97c232260e22597dd8437087cbc5 100644 --- a/app/services/packages/npm/generate_metadata_service.rb +++ b/app/services/packages/npm/generate_metadata_service.rb @@ -105,7 +105,7 @@ def sorted_versions end def package_tags - Packages::Tag.for_package_ids(packages) + Packages::Tag.for_package_ids_with_distinct_names(packages) .preload_package end diff --git a/spec/models/packages/tag_spec.rb b/spec/models/packages/tag_spec.rb index 6842d1946e5b61a66fe47d0b9b4ff3fa449d4d73..2d04561575604f0e449bd010df60e4a54194a858 100644 --- a/spec/models/packages/tag_spec.rb +++ b/spec/models/packages/tag_spec.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true + require 'spec_helper' RSpec.describe Packages::Tag, type: :model, feature_category: :package_registry do - let!(:project) { create(:project) } - let!(:package) { create(:npm_package, version: '1.0.2', project: project, updated_at: 3.days.ago) } + let_it_be(:project) { create(:project) } + let_it_be(:package) { create(:npm_package, version: '1.0.2', project: project, updated_at: 3.days.ago) } describe '#ensure_project_id' do it 'sets the project_id before saving' do @@ -83,4 +84,25 @@ it { is_expected.to contain_exactly(tag1, tag3) } end end + + describe '.for_package_ids_with_distinct_names' do + let_it_be(:package2) { create(:package, project: project) } + let_it_be(:package3) { create(:package, project: project) } + let_it_be(:tag1) { create(:packages_tag, name: 'latest', package: package, updated_at: 4.days.ago) } + let_it_be(:tag2) { create(:packages_tag, name: 'latest', package: package2, updated_at: 3.days.ago) } + let_it_be(:tag3) { create(:packages_tag, name: 'latest', package: package2, updated_at: 2.days.ago) } + let_it_be(:tag4) { create(:packages_tag, name: 'tag4', package: package3, updated_at: 5.days.ago) } + let_it_be(:tag5) { create(:packages_tag, name: 'tag5', package: package3, updated_at: 4.days.ago) } + let_it_be(:tag6) { create(:packages_tag, name: 'tag6', package: package3, updated_at: 6.days.ago) } + + subject { described_class.for_package_ids_with_distinct_names(project.packages) } + + before do + stub_const("#{described_class}::FOR_PACKAGES_TAGS_LIMIT", 3) + end + + # `tag3` is returned because it's the most recently updated with the name `latest`. + # `tag5` is returned before `tag4` because it was updated more recently than `tag4`. + it { is_expected.to eq([tag3, tag5, tag4]) } + end end diff --git a/spec/services/packages/npm/generate_metadata_service_spec.rb b/spec/services/packages/npm/generate_metadata_service_spec.rb index f5d7f13d22c32c467b7092ae0d802a91f3998b90..ad3d4fde665fcaa944e9c1a0b5dd2a245935204b 100644 --- a/spec/services/packages/npm/generate_metadata_service_spec.rb +++ b/spec/services/packages/npm/generate_metadata_service_spec.rb @@ -5,7 +5,8 @@ RSpec.describe ::Packages::Npm::GenerateMetadataService, feature_category: :package_registry do using RSpec::Parameterized::TableSyntax - let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, namespace: group) } let_it_be(:package_name) { "@#{project.root_namespace.path}/test" } let_it_be(:package1) { create(:npm_package, version: '2.0.4', project: project, name: package_name) } let_it_be(:package2) { create(:npm_package, version: '2.0.6', project: project, name: package_name) } @@ -156,6 +157,19 @@ end end 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') } + + 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) + end + end end end