Merge branch '42680-10io-nuget-add-tags-support' into 'master'

Add NuGet package tags support

See merge request gitlab-org/gitlab!30726

ee/app/models/packages/package.rb

@@ -137,6 +137,10 @@ class Packages::Package < ApplicationRecord
     build_info&.pipeline
   end
 
+  def tag_names
+    tags.pluck(:name)
+  end
+
   private
 
   def valid_conan_package_recipe

ee/app/models/packages/tag.rb

@@ -5,8 +5,10 @@ class Packages::Tag < ApplicationRecord
   validates :package, :name, presence: true
 
   TAGS_LIMIT = 200.freeze
+  NUGET_TAGS_SEPARATOR = ' ' # https://docs.microsoft.com/en-us/nuget/reference/nuspec#tags
 
   scope :preload_package, -> { preload(:package) }
+  scope :with_name, -> (name) { where(name: name) }
 
   def self.for_packages(packages, max_tags_limit = TAGS_LIMIT)
     where(package_id: packages.select(:id))

ee/app/presenters/packages/nuget/search_results_presenter.rb

@@ -16,15 +16,19 @@ module Packages
       def data
         strong_memoize(:data) do
           @search.results.group_by(&:name).map do |package_name, packages|
+            latest_version = latest_version(packages)
+            latest_package = packages.find { |pkg| pkg.version == latest_version }
+
             {
               type: 'Package',
               authors: '',
               name: package_name,
-              version: latest_version(packages),
+              version: latest_version,
               versions: build_package_versions(packages),
               summary: '',
               total_downloads: 0,
-              verified: true
+              verified: true,
+              tags: tags_for(latest_package)
             }
           end
         end

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

@@ -14,6 +14,7 @@ module Packages
 
       XPATH_DEPENDENCIES = '//xmlns:package/xmlns:metadata/xmlns:dependencies/xmlns:dependency'
       XPATH_DEPENDENCY_GROUPS = '//xmlns:package/xmlns:metadata/xmlns:dependencies/xmlns:group'
+      XPATH_TAGS = '//xmlns:package/xmlns:metadata/xmlns:tags'
 
       MAX_FILE_SIZE = 4.megabytes.freeze
 
@@ -48,6 +49,7 @@ module Packages
               .to_h
               .tap do |metadata|
                 metadata[:package_dependencies] = extract_dependencies(doc)
+                metadata[:package_tags] = extract_tags(doc)
               end
       end
 
@@ -76,6 +78,14 @@ module Packages
         }.compact
       end
 
+      def extract_tags(doc)
+        tags = doc.xpath(XPATH_TAGS).text
+
+        return [] if tags.blank?
+
+        tags.split(::Packages::Tag::NUGET_TAGS_SEPARATOR)
+      end
+
       def nuspec_file
         package_file.file.use_file do |file_path|
           Zip::File.open(file_path) do |zip_file|

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

@@ -4,6 +4,10 @@ module Packages
   module Nuget
     class UpdatePackageFromMetadataService
       include Gitlab::Utils::StrongMemoize
+      include ExclusiveLeaseGuard
+
+      # used by ExclusiveLeaseGuard
+      DEFAULT_LEASE_TIMEOUT = 1.hour.to_i.freeze
 
       InvalidMetadataError = Class.new(StandardError)
 
@@ -14,24 +18,29 @@ module Packages
       def execute
         raise InvalidMetadataError.new('package name and/or package version not found in metadata') unless valid_metadata?
 
-        @package_file.transaction do
-          if existing_package_id
-            link_to_existing_package
-          else
-            update_linked_package
-          end
+        try_obtain_lease do
+          @package_file.transaction do
+            package = existing_package ? link_to_existing_package : update_linked_package
+
+            update_package(package)
 
-          # Updating file_name updates the path where the file is stored.
-          # We must pass the file again so that CarrierWave can handle the update
-          @package_file.update!(
-            file_name: package_filename,
-            file: @package_file.file
-          )
+            # Updating file_name updates the path where the file is stored.
+            # We must pass the file again so that CarrierWave can handle the update
+            @package_file.update!(
+              file_name: package_filename,
+              file: @package_file.file
+            )
+          end
         end
       end
 
       private
 
+      def update_package(package)
+        ::Packages::UpdateTagsService.new(package, package_tags)
+                                     .execute
+      end
+
       def valid_metadata?
         package_name.present? && package_version.present?
       end
@@ -41,10 +50,11 @@ module Packages
         # Updating package_id updates the path where the file is stored.
         # We must pass the file again so that CarrierWave can handle the update
         @package_file.update!(
-          package_id: existing_package_id,
+          package_id: existing_package.id,
           file: @package_file.file
         )
         package_to_destroy.destroy!
+        existing_package
       end
 
       def update_linked_package
@@ -55,15 +65,15 @@ module Packages
 
         ::Packages::Nuget::CreateDependencyService.new(@package_file.package, package_dependencies)
                                                   .execute
+        @package_file.package
       end
 
-      def existing_package_id
-        strong_memoize(:existing_package_id) do
+      def existing_package
+        strong_memoize(:existing_package) do
           @package_file.project.packages
                                .nuget
                                .with_name(package_name)
                                .with_version(package_version)
-                               .pluck_primary_key
                                .first
         end
       end
@@ -80,6 +90,10 @@ module Packages
         metadata.fetch(:package_dependencies, [])
       end
 
+      def package_tags
+        metadata.fetch(:package_tags, [])
+      end
+
       def metadata
         strong_memoize(:metadata) do
           ::Packages::Nuget::MetadataExtractionService.new(@package_file.id).execute
@@ -89,6 +103,17 @@ module Packages
       def package_filename
         "#{package_name.downcase}.#{package_version.downcase}.nupkg"
       end
+
+      # used by ExclusiveLeaseGuard
+      def lease_key
+        package_id = existing_package ? existing_package.id : @package_file.package.id
+        "packages:nuget:update_package_from_metadata_service:package:#{package_id}"
+      end
+
+      # used by ExclusiveLeaseGuard
+      def lease_timeout
+        DEFAULT_LEASE_TIMEOUT
+      end
     end
   end
 end

ee/app/services/packages/update_tags_service.rb

+# frozen_string_literal: true
+module Packages
+  class UpdateTagsService
+    include Gitlab::Utils::StrongMemoize
+
+    def initialize(package, tags = [])
+      @package = package
+      @tags = tags
+    end
+
+    def execute
+      return if @tags.empty?
+
+      tags_to_destroy = existing_tags - @tags
+      tags_to_create = @tags - existing_tags
+
+      @package.tags.with_name(tags_to_destroy).delete_all if tags_to_destroy.any?
+      ::Gitlab::Database.bulk_insert(Packages::Tag.table_name, rows(tags_to_create)) if tags_to_create.any?
+    end
+
+    private
+
+    def existing_tags
+      strong_memoize(:existing_tags) do
+        @package.tag_names
+      end
+    end
+
+    def rows(tags)
+      now = Time.zone.now
+      tags.map do |tag|
+        {
+          package_id: @package.id,
+          name: tag,
+          created_at: now,
+          updated_at: now
+        }
+      end
+    end
+  end
+end

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

+---
+title: Add package tags support to the GitLab NuGet Packages Repository
+merge_request: 30726
+author:
+type: added

ee/lib/api/helpers/packages/nuget/metadata_presenter_helpers.rb

@@ -49,13 +49,18 @@ module API
               package_name: package.name,
               package_version: package.version,
               archive_url: archive_url_for(package),
-              summary: BLANK_STRING
+              summary: BLANK_STRING,
+              tags: tags_for(package)
             }
           end
 
           def base_path_for(package)
             api_v4_projects_packages_nuget_path(id: package.project.id)
           end
+
+          def tags_for(package)
+            package.tag_names.join(::Packages::Tag::NUGET_TAGS_SEPARATOR)
+          end
         end
       end
     end

ee/lib/ee/api/entities/nuget/package_metadata_catalog_entry.rb

@@ -10,6 +10,7 @@ module EE
           expose :dependencies, as: :dependencyGroups
           expose :package_name, as: :id
           expose :package_version, as: :version
+          expose :tags
           expose :archive_url, as: :packageContent
           expose :summary
         end

ee/lib/ee/api/entities/nuget/search_result.rb

@@ -14,6 +14,7 @@ module EE
           expose :verified
           expose :version
           expose :versions, using: EE::API::Entities::Nuget::SearchResultVersion
+          expose :tags
         end
       end
     end

ee/spec/fixtures/api/schemas/public_api/v4/packages/nuget/package_metadata.json

@@ -14,6 +14,7 @@
         "id": { "type": "string" },
         "packageContent": { "type": "string" },
         "summary": { "const": "" },
+        "tags": { "type": "string" },
         "version": { "type": "string" }
       }
     }

ee/spec/fixtures/api/schemas/public_api/v4/packages/nuget/packages_metadata.json

@@ -30,6 +30,7 @@
                     "id": { "type": "string" },
                     "packageContent": { "type": "string" },
                     "summary": { "const": "" },
+                    "tags": { "type": "string" },
                     "version": { "type": "string" }
                   }
                 }

ee/spec/fixtures/api/schemas/public_api/v4/packages/nuget/search.json

@@ -16,6 +16,7 @@
           "title": { "type": "string" },
           "totalDownloads": { "const": 0 },
           "verified": { "const": true },
+          "tags": { "type": "string" },
           "versions": {
             "type": "array",
             "items": {

ee/spec/fixtures/nuget/with_metadata.nuspec

+<?xml version="1.0" encoding="utf-8"?>
+<package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
+  <metadata>
+    <id>DummyProject.WithMetadata</id>
+    <version>1.2.3</version>
+    <title>nuspec with metadata</title>
+    <authors>Author Test</authors>
+    <owners>Author Test</owners>
+    <developmentDependency>true</developmentDependency>
+    <requireLicenseAcceptance>true</requireLicenseAcceptance>
+    <licenseUrl>https://opensource.org/licenses/MIT</licenseUrl>
+    <projectUrl>https://gitlab.com/gitlab-org/gitlab</projectUrl>
+    <iconUrl>https://opensource.org/files/osi_keyhole_300X300_90ppi_0.png</iconUrl>
+    <description>Description Test</description>
+    <releaseNotes>Release Notes Test</releaseNotes>
+    <copyright>Copyright Test</copyright>
+    <tags>foo bar test tag1 tag2 tag3 tag4 tag5</tags>
+  </metadata>
+</package>

ee/spec/lib/ee/api/entities/nuget/package_metadata_catalog_entry_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::API::Entities::Nuget::PackageMetadataCatalogEntry do
+  let(:entry) do
+    {
+      json_url: 'http://sandbox.com/json/package',
+      authors: 'Authors',
+      dependencies: [],
+      package_name: 'PackageTest',
+      package_version: '1.2.3',
+      tags: 'tag1 tag2 tag3',
+      archive_url: 'http://sandbox.com/archive/package',
+      summary: 'Summary'
+    }
+  end
+
+  let(:expected) do
+    {
+      '@id': 'http://sandbox.com/json/package',
+      'id': 'PackageTest',
+      'version': '1.2.3',
+      'authors': 'Authors',
+      'dependencyGroups': [],
+      'tags': 'tag1 tag2 tag3',
+      'packageContent': 'http://sandbox.com/archive/package',
+      'summary': 'Summary'
+    }
+  end
+
+  subject { described_class.new(entry).as_json }
+
+  it { is_expected.to eq(expected) }
+end

ee/spec/lib/ee/api/entities/nuget/search_result_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::API::Entities::Nuget::SearchResult do
+  let(:search_result) do
+    {
+      type: 'Package',
+      authors: 'Author',
+      name: 'PackageTest',
+      version: '1.2.3',
+      versions: [
+        {
+          json_url: 'http://sandbox.com/json/package',
+          downloads: 100,
+          version: '1.2.3'
+        }
+      ],
+      summary: 'Summary',
+      total_downloads: 100,
+      verified: true,
+      tags: 'tag1 tag2 tag3'
+    }
+  end
+
+  let(:expected) do
+    {
+      '@type': 'Package',
+      'authors': 'Author',
+      'id': 'PackageTest',
+      'title': 'PackageTest',
+      'summary': 'Summary',
+      'totalDownloads': 100,
+      'verified': true,
+      'version': '1.2.3',
+      'tags': 'tag1 tag2 tag3',
+      'versions': [
+        {
+          '@id': 'http://sandbox.com/json/package',
+          'downloads': 100,
+          'version': '1.2.3'
+        }
+      ]
+    }
+  end
+
+  subject { described_class.new(search_result).as_json }
+
+  it { is_expected.to eq(expected) }
+end

ee/spec/models/packages/package_spec.rb

@@ -421,4 +421,22 @@ RSpec.describe Packages::Package, type: :model do
       end
     end
   end
+
+  describe '#tag_names' do
+    let_it_be(:package) { create(:nuget_package) }
+
+    subject { package.tag_names }
+
+    it { is_expected.to eq([]) }
+
+    context 'with tags' do
+      let(:tags) { %w(tag1 tag2 tag3) }
+
+      before do
+        tags.each { |t| create(:packages_tag, name: t, package: package) }
+      end
+
+      it { is_expected.to contain_exactly(*tags) }
+    end
+  end
 end

ee/spec/models/packages/tag_spec.rb

@@ -34,4 +34,28 @@ RSpec.describe Packages::Tag, type: :model do
       it { is_expected.to match_array([tag2, tag3]) }
     end
   end
+
+  describe '.with_name' do
+    let_it_be(:package) { create(:package) }
+    let_it_be(:tag1) { create(:packages_tag, package: package, name: 'tag1') }
+    let_it_be(:tag2) { create(:packages_tag, package: package, name: 'tag2') }
+    let_it_be(:tag3) { create(:packages_tag, package: package, name: 'tag3') }
+    let(:name) { 'tag1' }
+
+    subject { described_class.with_name(name) }
+
+    it { is_expected.to contain_exactly(tag1) }
+
+    context 'with nil name' do
+      let(:name) { nil }
+
+      it { is_expected.to eq([]) }
+    end
+
+    context 'with multiple names' do
+      let(:name) { %w(tag1 tag3) }
+
+      it { is_expected.to contain_exactly(tag1, tag3) }
+    end
+  end
 end

ee/spec/presenters/packages/nuget/package_metadata_presenter_spec.rb

@@ -4,6 +4,8 @@ require 'spec_helper'
 
 describe Packages::Nuget::PackageMetadataPresenter do
   let_it_be(:package) { create(:nuget_package) }
+  let_it_be(:tag1) { create(:packages_tag, name: 'tag1', package: package) }
+  let_it_be(:tag2) { create(:packages_tag, name: 'tag2', package: package) }
   let_it_be(:presenter) { described_class.new(package) }
 
   describe '#json_url' do
@@ -34,6 +36,7 @@ describe Packages::Nuget::PackageMetadataPresenter do
       expect(entry[:dependencies]).to eq []
       expect(entry[:package_name]).to eq package.name
       expect(entry[:package_version]).to eq package.version
+      expect(entry[:tags].split(::Packages::Tag::NUGET_TAGS_SEPARATOR)).to contain_exactly('tag1', 'tag2')
     end
   end
 end

ee/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb

@@ -13,8 +13,16 @@ describe Packages::Nuget::PackagesMetadataPresenter do
   end
 
   describe '#items' do
+    let(:tag_names) { %w(tag1 tag2) }
+
     subject { presenter.items }
 
+    before do
+      packages.each do |pkg|
+        tag_names.each { |tag| create(:packages_tag, package: pkg, name: tag) }
+      end
+    end
+
     it 'returns an array' do
       items = subject
 
@@ -42,6 +50,7 @@ describe Packages::Nuget::PackagesMetadataPresenter do
         %i[json_url archive_url package_name package_version].each { |field| expect(catalog_entry[field]).not_to be_blank }
         %i[authors summary].each { |field| expect(catalog_entry[field]).to be_blank }
         expect(catalog_entry[:dependencies]).to eq []
+        expect(catalog_entry[:tags].split(::Packages::Tag::NUGET_TAGS_SEPARATOR)).to contain_exactly('tag1', 'tag2')
       end
     end
   end

ee/spec/presenters/packages/nuget/search_results_presenter_spec.rb

@@ -5,6 +5,8 @@ require 'spec_helper'
 describe Packages::Nuget::SearchResultsPresenter do
   let_it_be(:project) { create(:project) }
   let_it_be(:package_a) { create(:nuget_package, project: project, name: 'DummyPackageA') }
+  let_it_be(:tag1) { create(:packages_tag, package: package_a, name: 'tag1') }
+  let_it_be(:tag2) { create(:packages_tag, package: package_a, name: 'tag2') }
   let_it_be(:packages_b) { create_list(:nuget_package, 5, project: project, name: 'DummyPackageB') }
   let_it_be(:packages_c) { create_list(:nuget_package, 5, project: project, name: 'DummyPackageC') }
   let_it_be(:search_results) { OpenStruct.new(total_count: 3, results: [package_a, packages_b, packages_c].flatten) }
@@ -22,12 +24,13 @@ describe Packages::Nuget::SearchResultsPresenter do
     it 'returns the proper data structure' do
       expect(data.size).to eq 3
       pkg_a, pkg_b, pkg_c = data
-      expect_package_result(pkg_a, package_a.name, [package_a.version])
+      expect_package_result(pkg_a, package_a.name, [package_a.version], %w(tag1 tag2))
       expect_package_result(pkg_b, packages_b.first.name, packages_b.map(&:version))
       expect_package_result(pkg_c, packages_c.first.name, packages_c.map(&:version))
     end
 
-    def expect_package_result(package_json, name, versions)
+    # rubocop:disable Metrics/AbcSize
+    def expect_package_result(package_json, name, versions, tags = [])
       expect(package_json[:type]).to eq 'Package'
       expect(package_json[:authors]).to be_blank
       expect(package_json[:name]).to eq(name)
@@ -40,6 +43,13 @@ describe Packages::Nuget::SearchResultsPresenter do
         expect(version_json[:downloads]).to eq 0
         expect(version_json[:version]).to eq version
       end
+
+      if tags.any?
+        expect(package_json[:tags].split(::Packages::Tag::NUGET_TAGS_SEPARATOR)).to contain_exactly(*tags)
+      else
+        expect(package_json[:tags]).to be_blank
+      end
     end
+    # rubocop:enable Metrics/AbcSize
   end
 end

ee/spec/requests/api/nuget_packages_spec.rb

@@ -197,6 +197,7 @@ describe API::NugetPackages do
   describe 'GET /api/v4/projects/:id/packages/nuget/metadata/*package_name/index' do
     let_it_be(:package_name) { 'Dummy.Package' }
     let_it_be(:packages) { create_list(:nuget_package, 5, name: package_name, project: project) }
+    let_it_be(:tags) { packages.each { |pkg| create(:packages_tag, package: pkg, name: 'test') } }
     let(:url) { "/projects/#{project.id}/packages/nuget/metadata/#{package_name}/index.json" }
 
     subject { get api(url) }
@@ -255,6 +256,7 @@ describe API::NugetPackages do
   describe 'GET /api/v4/projects/:id/packages/nuget/metadata/*package_name/*package_version' do
     let_it_be(:package_name) { 'Dummy.Package' }
     let_it_be(:package) { create(:nuget_package, name: 'Dummy.Package', project: project) }
+    let_it_be(:tag) { create(:packages_tag, package: package, name: 'test') }
     let(:url) { "/projects/#{project.id}/packages/nuget/metadata/#{package_name}/#{package.version}.json" }
 
     subject { get api(url) }
@@ -431,6 +433,7 @@ describe API::NugetPackages do
 
   describe 'GET /api/v4/projects/:id/packages/nuget/query' do
     let_it_be(:package_a) { create(:nuget_package, name: 'Dummy.PackageA', project: project) }
+    let_it_be(:tag) { create(:packages_tag, package: package_a, name: 'test') }
     let_it_be(:packages_b) { create_list(:nuget_package, 5, name: 'Dummy.PackageB', project: project) }
     let_it_be(:packages_c) { create_list(:nuget_package, 5, name: 'Dummy.PackageC', project: project) }
     let_it_be(:package_d) { create(:nuget_package, name: 'Dummy.PackageD', version: '5.0.5-alpha', project: project) }

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

@@ -19,28 +19,37 @@ describe Packages::Nuget::MetadataExtractionService do
             target_framework: '.NETCoreApp3.0',
             version: '12.0.3'
           }
-        ]
+        ],
+        package_tags: []
       }
 
       it { is_expected.to eq(expected_metadata) }
     end
 
-    context 'with nuspec file with dependencies' do
-      let(:nuspec_filepath) { 'nuget/with_dependencies.nuspec' }
-
+    context 'with nuspec file' do
       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) }
+      context 'with dependencies' do
+        let(:nuspec_filepath) { 'nuget/with_dependencies.nuspec' }
+
+        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
 
-      it 'extracts dependencies' do
-        dependencies = subject[:package_dependencies]
+      context 'with a nuspec file with metadata' do
+        let(:nuspec_filepath) { 'nuget/with_metadata.nuspec' }
 
-        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')
+        it { expect(subject[:package_tags].sort).to eq(%w(foo bar test tag1 tag2 tag3 tag4 tag5).sort) }
       end
     end
 

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

@@ -2,7 +2,9 @@
 
 require 'spec_helper'
 
-describe Packages::Nuget::UpdatePackageFromMetadataService do
+describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_redis_shared_state do
+  include ExclusiveLeaseHelpers
+
   let(:package) { create(:nuget_package) }
   let(:package_file) { package.package_files.first }
   let(:service) { described_class.new(package_file) }
@@ -17,22 +19,74 @@ describe Packages::Nuget::UpdatePackageFromMetadataService do
       stub_package_file_object_storage(enabled: true, direct_upload: true)
     end
 
-    it 'updates package and package file' do
-      expect { subject }
-        .to change { Packages::Dependency.count }.by(1)
-        .and change { Packages::DependencyLink.count }.by(1)
+    RSpec.shared_examples 'taking the lease' do
+      before do
+        allow(service).to receive(:lease_release?).and_return(false)
+      end
+
+      it 'takes the lease' do
+        expect(service).to receive(:try_obtain_lease).and_call_original
+
+        subject
+
+        expect(service.exclusive_lease.exists?).to be_truthy
+      end
+    end
+
+    RSpec.shared_examples 'not updating the package if the lease is taken' do
+      context 'without obtaining the exclusive lease' do
+        let(:lease_key) { "packages:nuget:update_package_from_metadata_service:package:#{package_id}" }
+        let(:metadata) { { package_name: package_name, package_version: package_version } }
+        let(:package_from_package_file) { package_file.package }
+
+        before do
+          stub_exclusive_lease_taken(lease_key, timeout: 1.hour)
+          # to allow the above stub, we need to stub the metadata function as the
+          # original implementation will try to get an exclusive lease on the
+          # file in object storage
+          allow(service).to receive(:metadata).and_return(metadata)
+        end
+
+        it 'does not update the package' do
+          expect(service).to receive(:try_obtain_lease).and_call_original
+
+          expect { subject }
+            .to change { ::Packages::Package.count }.by(0)
+            .and change { Packages::DependencyLink.count }.by(0)
+          expect(package_file.reload.file_name).not_to eq(package_file_name)
+          expect(package_file.package.reload.name).not_to eq(package_name)
+          expect(package_file.package.version).not_to eq(package_version)
+        end
+      end
+    end
+
+    context 'with no existing package' do
+      let(:package_id) { package.id }
+
+      it 'updates package and package file' do
+        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)
+        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
+
+      it_behaves_like 'taking the lease'
 
-      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
+      it_behaves_like 'not updating the package if the lease is taken'
     end
 
-    context 'with exisiting package' do
+    context 'with existing package' do
       let!(:existing_package) { create(:nuget_package, project: package.project, name: package_name, version: package_version) }
+      let(:package_id) { existing_package.id }
 
       it 'link existing package and updates package file' do
+        expect(service).to receive(:try_obtain_lease).and_call_original
+
         expect { subject }
           .to change { ::Packages::Package.count }.by(-1)
           .and change { Packages::Dependency.count }.by(0)
@@ -41,6 +95,40 @@ describe Packages::Nuget::UpdatePackageFromMetadataService do
         expect(package_file.reload.file_name).to eq(package_file_name)
         expect(package_file.package).to eq(existing_package)
       end
+
+      it_behaves_like 'taking the lease'
+
+      it_behaves_like 'not updating the package if the lease is taken'
+    end
+
+    context 'with a nuspec file with metadata' do
+      let(:nuspec_filepath) { 'nuget/with_metadata.nuspec' }
+      let(:expected_tags) { %w(foo bar test tag1 tag2 tag3 tag4 tag5) }
+
+      before do
+        allow_any_instance_of(Packages::Nuget::MetadataExtractionService)
+          .to receive(:nuspec_file)
+          .and_return(fixture_file(nuspec_filepath, dir: 'ee'))
+      end
+
+      it 'creates tags' do
+        expect(service).to receive(:try_obtain_lease).and_call_original
+        expect { subject }.to change { ::Packages::Tag.count }.by(8)
+        expect(package.reload.tags.map(&:name)).to contain_exactly(*expected_tags)
+      end
+
+      context 'with existing package and tags' do
+        let!(:existing_package) { create(:nuget_package, project: package.project, name: 'DummyProject.WithMetadata', version: '1.2.3') }
+        let!(:tag1) { create(:packages_tag, package: existing_package, name: 'tag1') }
+        let!(:tag2) { create(:packages_tag, package: existing_package, name: 'tag2') }
+        let!(:tag3) { create(:packages_tag, package: existing_package, name: 'tag_not_in_metadata') }
+
+        it 'creates tags and deletes those not in metadata' do
+          expect(service).to receive(:try_obtain_lease).and_call_original
+          expect { subject }.to change { ::Packages::Tag.count }.by(5)
+          expect(existing_package.tags.map(&:name)).to contain_exactly(*expected_tags)
+        end
+      end
     end
 
     context 'with nuspec file with dependencies' do

ee/spec/services/packages/update_tags_service_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Packages::UpdateTagsService do
+  let_it_be(:package, reload: true) { create(:nuget_package) }
+
+  let(:tags) { %w(test-tag tag1 tag2 tag3) }
+  let(:service) { described_class.new(package, tags) }
+
+  describe '#execute' do
+    subject { service.execute }
+
+    RSpec.shared_examples 'updating tags' do |tags_count|
+      it 'updates a tag' do
+        expect { subject }.to change { Packages::Tag.count }.by(tags_count)
+        expect(package.reload.tags.map(&:name)).to contain_exactly(*tags)
+      end
+    end
+
+    it_behaves_like 'updating tags', 4
+
+    context 'with an existing tag' do
+      before do
+        create(:packages_tag, package: package2, name: 'test-tag')
+      end
+
+      context 'on the same package' do
+        let_it_be(:package2) { package }
+
+        it_behaves_like 'updating tags', 3
+
+        context 'with different name' do
+          before do
+            create(:packages_tag, package: package2, name: 'to_be_destroyed')
+          end
+
+          it_behaves_like 'updating tags', 2
+        end
+      end
+
+      context 'on a different package' do
+        let_it_be(:package2) { create(:nuget_package) }
+
+        it_behaves_like 'updating tags', 4
+      end
+    end
+
+    context 'with empty tags' do
+      let(:tags) { [] }
+
+      it 'is a no op' do
+        expect(package).not_to receive(:tags)
+        expect(::Gitlab::Database).not_to receive(:bulk_insert)
+
+        subject
+      end
+    end
+  end
+end