Use `#use_open_file` for NuGet metadata extraction

instead of `#use_file` which will use an exclusive lock

Changelog: fixed

app/services/packages/nuget/metadata_extraction_service.rb

@@ -28,7 +28,7 @@ module Packages
       def execute
         raise ExtractionError, 'invalid package file' unless valid_package_file?
 
-        extract_metadata(nuspec_file)
+        extract_metadata(nuspec_file_content)
       end
 
       private
@@ -39,6 +39,10 @@ module Packages
         end
       end
 
+      def project
+        package_file.package.project
+      end
+
       def valid_package_file?
         package_file &&
           package_file.package&.nuget? &&
@@ -89,9 +93,8 @@ module Packages
         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|
+      def nuspec_file_content
+        with_zip_file do |zip_file|
           entry = zip_file.glob('*.nuspec').first
 
           raise ExtractionError, 'nuspec file not found' unless entry
@@ -100,6 +103,28 @@ module Packages
           entry.get_input_stream.read
         end
       end
+
+      def with_zip_file(&block)
+        if ::Feature.enabled?(:packages_nuget_archive_new_file_reader, project, default_enabled: :yaml)
+          with_new_file_reader(&block)
+        else
+          with_legacy_file_reader(&block)
+        end
+      end
+
+      def with_legacy_file_reader
+        package_file.file.use_file do |file_path|
+          Zip::File.open(file_path) do |zip_file|
+            yield(zip_file)
+          end
+        end
+      end
+
+      def with_new_file_reader
+        package_file.file.use_open_file do |open_file|
+          zip_file = Zip::File.new(open_file, false, true)
+          yield(zip_file)
+        end
       end
     end
   end

config/feature_flags/development/packages_nuget_archive_new_file_reader.yml

+---
+name: packages_nuget_archive_new_file_reader
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62471
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/331799
+milestone: '13.13'
+type: development
+group: group::package
+default_enabled: false

spec/services/packages/nuget/metadata_extraction_service_spec.rb

@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Packages::Nuget::MetadataExtractionService do
-  let(:package_file) { create(:nuget_package).package_files.first }
+  let_it_be(:package_file) { create(:nuget_package).package_files.first }
+
   let(:service) { described_class.new(package_file.id) }
 
   describe '#execute' do
@@ -23,12 +24,27 @@ RSpec.describe Packages::Nuget::MetadataExtractionService do
         package_tags: []
       }
 
+      context 'with packages_nuget_archive_new_file_reader enabled' do
+        before do
+          expect(service).to receive(:with_new_file_reader).and_call_original
+        end
+
         it { is_expected.to eq(expected_metadata) }
       end
 
+      context 'with packages_nuget_archive_new_file_reader disabled' do
+        before do
+          stub_feature_flags(packages_nuget_archive_new_file_reader: false)
+          expect(service).to receive(:with_legacy_file_reader).and_call_original
+        end
+
+        it { is_expected.to eq(expected_metadata) }
+      end
+    end
+
     context 'with nuspec file' do
       before do
-        allow(service).to receive(:nuspec_file).and_return(fixture_file(nuspec_filepath))
+        allow(service).to receive(:nuspec_file_content).and_return(fixture_file(nuspec_filepath))
       end
 
       context 'with dependencies' do
@@ -57,7 +73,7 @@ RSpec.describe Packages::Nuget::MetadataExtractionService do
       let_it_be(:nuspec_filepath) { 'packages/nuget/with_metadata.nuspec' }
 
       before do
-        allow(service).to receive(:nuspec_file).and_return(fixture_file(nuspec_filepath))
+        allow(service).to receive(:nuspec_file_content).and_return(fixture_file(nuspec_filepath))
       end
 
       it { expect(subject[:license_url]).to eq('https://opensource.org/licenses/MIT') }

spec/services/packages/nuget/update_package_from_metadata_service_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
   let(:package_version) { '1.0.0' }
   let(:package_file_name) { 'dummyproject.dummypackage.1.0.0.nupkg' }
 
-  RSpec.shared_examples 'raising an' do |error_class|
+  shared_examples 'raising an' do |error_class|
     it "raises an #{error_class}" do
       expect { subject }.to raise_error(error_class)
     end
@@ -21,11 +21,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
   describe '#execute' do
     subject { service.execute }
 
-    before do
-      stub_package_file_object_storage(enabled: true, direct_upload: true)
-    end
-
-    RSpec.shared_examples 'taking the lease' do
+    shared_examples 'taking the lease' do
       before do
         allow(service).to receive(:lease_release?).and_return(false)
       end
@@ -39,7 +35,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
       end
     end
 
-    RSpec.shared_examples 'not updating the package if the lease is taken' do
+    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 } }
@@ -67,6 +63,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
       end
     end
 
+    shared_examples 'handling all conditions' do
       context 'with no existing package' do
         let(:package_id) { package.id }
 
@@ -117,9 +114,10 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
         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))
+          allow_next_instance_of(Packages::Nuget::MetadataExtractionService) do |service|
+            allow(service)
+              .to receive(:nuspec_file_content).and_return(fixture_file(nuspec_filepath))
+          end
         end
 
         it 'creates tags' do
@@ -172,9 +170,10 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
         let(:package_file_name) { 'test.package.3.5.2.nupkg' }
 
         before do
-        allow_any_instance_of(Packages::Nuget::MetadataExtractionService)
-          .to receive(:nuspec_file)
-          .and_return(fixture_file(nuspec_filepath))
+          allow_next_instance_of(Packages::Nuget::MetadataExtractionService) do |service|
+            allow(service)
+              .to receive(:nuspec_file_content).and_return(fixture_file(nuspec_filepath))
+          end
         end
 
         it 'updates package and package file' do
@@ -195,7 +194,9 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
 
       context 'with package file not containing a nuspec file' do
         before do
-        allow_any_instance_of(Zip::File).to receive(:glob).and_return([])
+          allow_next_instance_of(Zip::File) do |file|
+            allow(file).to receive(:glob).and_return([])
+          end
         end
 
         it_behaves_like 'raising an', ::Packages::Nuget::MetadataExtractionService::ExtractionError
@@ -237,4 +238,16 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
         end
       end
     end
+
+    it_behaves_like 'handling all conditions'
+
+    context 'with packages_nuget_archive_new_file_reader disabled' do
+      before do
+        stub_feature_flags(packages_nuget_archive_new_file_reader: false)
+        stub_package_file_object_storage(enabled: true, direct_upload: true)
+      end
+
+      it_behaves_like 'handling all conditions'
+    end
+  end
 end