Merge branch '345755-package-files-pending-destruction' into 'master'

Add package file status attribute and introduce the installable scope

See merge request gitlab-org/gitlab!76767

app/controllers/projects/packages/infrastructure_registry_controller.rb

@@ -9,7 +9,11 @@ module Projects
 
       def show
         @package = project.packages.find(params[:id])
-        @package_files = @package.package_files.recent
+        @package_files = if Feature.enabled?(:packages_installable_package_files)
+                           @package.installable_package_files.recent
+                         else
+                           @package.package_files.recent
+                         end
       end
     end
   end

app/finders/packages/package_file_finder.rb

@@ -19,7 +19,11 @@ class Packages::PackageFileFinder
   private
 
   def package_files
-    files = package.package_files
+    files = if Feature.enabled?(:packages_installable_package_files)
+              package.installable_package_files
+            else
+              package.package_files
+            end
 
     by_file_name(files)
   end

app/graphql/types/packages/package_details_type.rb

@@ -24,6 +24,14 @@ module Types
       def versions
         object.versions
       end
+
+      def package_files
+        if Feature.enabled?(:packages_installable_package_files)
+          object.installable_package_files
+        else
+          object.package_files
+        end
+      end
     end
   end
 end

app/models/concerns/packages/debian/distribution.rb

@@ -96,6 +96,15 @@ module Packages
           architectures.pluck(:name).sort
         end
 
+        def package_files
+          if Feature.enabled?(:packages_installable_package_files)
+            ::Packages::PackageFile.installable
+                                   .for_package_ids(packages.select(:id))
+          else
+            ::Packages::PackageFile.for_package_ids(packages.select(:id))
+          end
+        end
+
         private
 
         def unique_codename_and_suite

app/models/concerns/packages/installable.rb

+# frozen_string_literal: true
+
+module Packages
+  # This module requires a status column.
+  # It also requires a constant INSTALLABLE_STATUSES. This should be
+  # an array that defines which values of the status column are
+  # considered as installable.
+  module Installable
+    extend ActiveSupport::Concern
+
+    included do
+      scope :with_status, ->(status) { where(status: status) }
+      scope :installable, -> { with_status(const_get(:INSTALLABLE_STATUSES, false)) }
+    end
+  end
+end

app/models/packages/debian/group_distribution.rb

@@ -12,8 +12,4 @@ class Packages::Debian::GroupDistribution < ApplicationRecord
       .for_projects(group.all_projects.public_only)
       .with_debian_codename(codename)
   end
-
-  def package_files
-    ::Packages::PackageFile.for_package_ids(packages.select(:id))
-  end
 end

app/models/packages/debian/project_distribution.rb

@@ -9,5 +9,4 @@ class Packages::Debian::ProjectDistribution < ApplicationRecord
 
   has_many :publications, class_name: 'Packages::Debian::Publication', inverse_of: :distribution, foreign_key: :distribution_id
   has_many :packages, class_name: 'Packages::Package', through: :publications
-  has_many :package_files, class_name: 'Packages::PackageFile', through: :packages
 end

app/models/packages/package.rb

@@ -5,6 +5,7 @@ class Packages::Package < ApplicationRecord
   include Gitlab::SQL::Pattern
   include UsageStatistics
   include Gitlab::Utils::StrongMemoize
+  include Packages::Installable
 
   DISPLAYABLE_STATUSES = [:default, :error].freeze
   INSTALLABLE_STATUSES = [:default, :hidden].freeze
@@ -31,6 +32,9 @@ class Packages::Package < ApplicationRecord
 
   # package_files must be destroyed by ruby code in order to properly remove carrierwave uploads and update project statistics
   has_many :package_files, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+  # TODO: put the installable default scope on the :package_files association once the dependent: :destroy is removed
+  # See https://gitlab.com/gitlab-org/gitlab/-/issues/349191
+  has_many :installable_package_files, -> { installable }, class_name: 'Packages::PackageFile', inverse_of: :package
   has_many :dependency_links, inverse_of: :package, class_name: 'Packages::DependencyLink'
   has_many :tags, inverse_of: :package, class_name: 'Packages::Tag'
   has_one :conan_metadatum, inverse_of: :package, class_name: 'Packages::Conan::Metadatum'
@@ -100,9 +104,7 @@ class Packages::Package < ApplicationRecord
   scope :without_version_like, -> (version) { where.not(arel_table[:version].matches(version)) }
   scope :with_package_type, ->(package_type) { where(package_type: package_type) }
   scope :without_package_type, ->(package_type) { where.not(package_type: package_type) }
-  scope :with_status, ->(status) { where(status: status) }
   scope :displayable, -> { with_status(DISPLAYABLE_STATUSES) }
-  scope :installable, -> { with_status(INSTALLABLE_STATUSES) }
   scope :including_project_route, -> { includes(project: { namespace: :route }) }
   scope :including_tags, -> { includes(:tags) }
   scope :including_dependency_links, -> { includes(dependency_links: :dependency) }
@@ -131,7 +133,7 @@ class Packages::Package < ApplicationRecord
   scope :without_nuget_temporary_name, -> { where.not(name: Packages::Nuget::TEMPORARY_PACKAGE_NAME) }
 
   scope :has_version, -> { where.not(version: nil) }
-  scope :preload_files, -> { preload(:package_files) }
+  scope :preload_files, -> { Feature.enabled?(:packages_installable_package_files) ? preload(:installable_package_files) : preload(:package_files) }
   scope :preload_pipelines, -> { preload(pipelines: :user) }
   scope :last_of_each_version, -> { where(id: all.select('MAX(id) AS id').group(:version)) }
   scope :limit_recent, ->(limit) { order_created_desc.limit(limit) }

app/models/packages/package_file.rb

@@ -2,12 +2,17 @@
 class Packages::PackageFile < ApplicationRecord
   include UpdateProjectStatistics
   include FileStoreMounter
+  include Packages::Installable
+
+  INSTALLABLE_STATUSES = [:default].freeze
 
   delegate :project, :project_id, to: :package
   delegate :conan_file_type, to: :conan_file_metadatum
   delegate :file_type, :dsc?, :component, :architecture, :fields, to: :debian_file_metadatum, prefix: :debian
   delegate :channel, :metadata, to: :helm_file_metadatum, prefix: :helm
 
+  enum status: { default: 0, pending_destruction: 1 }
+
   belongs_to :package
 
   # used to move the linked file within object storage
@@ -48,9 +53,12 @@ class Packages::PackageFile < ApplicationRecord
   end
 
   scope :for_helm_with_channel, ->(project, channel) do
-    joins(:package).merge(project.packages.helm.installable)
-                   .joins(:helm_file_metadatum)
-                   .where(packages_helm_file_metadata: { channel: channel })
+    result = joins(:package)
+               .merge(project.packages.helm.installable)
+               .joins(:helm_file_metadatum)
+               .where(packages_helm_file_metadata: { channel: channel })
+    result = result.installable if Feature.enabled?(:packages_installable_package_files)
+    result
   end
 
   scope :with_conan_file_type, ->(file_type) do
@@ -94,14 +102,19 @@ class Packages::PackageFile < ApplicationRecord
   skip_callback :commit, :after, :remove_previously_stored_file, if: :execute_move_in_object_storage?
   after_commit :move_in_object_storage, if: :execute_move_in_object_storage?
 
-  # Returns the most recent package files for *each* of the given packages.
+  # Returns the most recent installable package file for *each* of the given packages.
   # The order is not guaranteed.
   def self.most_recent_for(packages, extra_join: nil, extra_where: nil)
     cte_name = :packages_cte
     cte = Gitlab::SQL::CTE.new(cte_name, packages.select(:id))
 
-    package_files = ::Packages::PackageFile.limit_recent(1)
-                      .where(arel_table[:package_id].eq(Arel.sql("#{cte_name}.id")))
+    package_files = if Feature.enabled?(:packages_installable_package_files)
+                      ::Packages::PackageFile.installable.limit_recent(1)
+                        .where(arel_table[:package_id].eq(Arel.sql("#{cte_name}.id")))
+                    else
+                      ::Packages::PackageFile.limit_recent(1)
+                        .where(arel_table[:package_id].eq(Arel.sql("#{cte_name}.id")))
+                    end
 
     package_files = package_files.joins(extra_join) if extra_join
     package_files = package_files.where(extra_where) if extra_where

app/presenters/packages/conan/package_presenter.rb

@@ -80,7 +80,13 @@ module Packages
       def package_files
         return unless @package
 
-        @package_files ||= @package.package_files.preload_conan_file_metadata
+        strong_memoize(:package_files) do
+          if Feature.enabled?(:packages_installable_package_files)
+            @package.installable_package_files.preload_conan_file_metadata
+          else
+            @package.package_files.preload_conan_file_metadata
+          end
+        end
       end
 
       def matching_reference?(package_file)

app/presenters/packages/detail/package_presenter.rb

@@ -15,7 +15,7 @@ module Packages
           id: @package.id,
           created_at: @package.created_at,
           name: name,
-          package_files: @package.package_files.map { |pf| build_package_file_view(pf) },
+          package_files: package_file_views,
           package_type: @package.package_type,
           status: @package.status,
           project_id: @package.project_id,
@@ -38,6 +38,16 @@ module Packages
 
       private
 
+      def package_file_views
+        package_files = if Feature.enabled?(:packages_installable_package_files)
+                          @package.installable_package_files
+                        else
+                          @package.package_files
+                        end
+
+        package_files.map { |pf| build_package_file_view(pf) }
+      end
+
       def build_package_file_view(package_file)
         file_view = {
           created_at: package_file.created_at,

app/presenters/packages/npm/package_presenter.rb

@@ -26,7 +26,11 @@ module Packages
                                      .preload_npm_metadatum
 
           batched_packages.each do |package|
-            package_file = package.package_files.last
+            package_file = if Feature.enabled?(:packages_installable_package_files)
+                             package.installable_package_files.last
+                           else
+                             package.package_files.last
+                           end
 
             next unless package_file
 

app/presenters/packages/nuget/presenter_helpers.rb

@@ -27,12 +27,19 @@ module Packages
       end
 
       def archive_url_for(package)
+        package_files = if Feature.enabled?(:packages_installable_package_files)
+                          package.installable_package_files
+                        else
+                          package.package_files
+                        end
+
+        package_filename = package_files.with_format(NUGET_PACKAGE_FORMAT).last&.file_name
         path = api_v4_projects_packages_nuget_download_package_name_package_version_package_filename_path(
           {
             id: package.project_id,
             package_name: package.name,
             package_version: package.version,
-            package_filename: package.package_files.with_format(NUGET_PACKAGE_FORMAT).last&.file_name
+            package_filename: package_filename
           },
           true
         )

app/presenters/packages/pypi/package_presenter.rb

@@ -36,7 +36,13 @@ module Packages
         refs = []
 
         @packages.map do |package|
-          package.package_files.each do |file|
+          package_files = if Feature.enabled?(:packages_installable_package_files)
+                            package.installable_package_files
+                          else
+                            package.package_files
+                          end
+
+          package_files.each do |file|
             url = build_pypi_package_path(file)
 
             refs << package_link(url, package.pypi_metadatum.required_python, file.file_name)

app/services/packages/maven/metadata/sync_service.rb

@@ -93,10 +93,15 @@ module Packages
         def metadata_package_file_for(package)
           return unless package
 
-          package.package_files
-                 .with_file_name(Metadata.filename)
-                 .recent
-                 .first
+          package_files = if Feature.enabled?(:packages_installable_package_files)
+                            package.installable_package_files
+                          else
+                            package.package_files
+                          end
+
+          package_files.with_file_name(Metadata.filename)
+                       .recent
+                       .first
         end
 
         def versionless_package_named(name)

config/feature_flags/development/packages_installable_package_files.yml

+---
+name: packages_installable_package_files
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76767
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348677
+milestone: '14.6'
+type: development
+group: group::package
+default_enabled: false

db/migrate/20211213154259_add_status_to_packages_package_files.rb

+# frozen_string_literal: true
+
+class AddStatusToPackagesPackageFiles < Gitlab::Database::Migration[1.0]
+  def change
+    add_column :packages_package_files, :status, :smallint, default: 0, null: false
+  end
+end

db/migrate/20211213154704_add_status_index_to_packages_package_files.rb

+# frozen_string_literal: true
+
+class AddStatusIndexToPackagesPackageFiles < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_packages_package_files_on_package_id_status_and_id'
+
+  def up
+    add_concurrent_index :packages_package_files, [:package_id, :status, :id], name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :packages_package_files, name: INDEX_NAME
+  end
+end

db/schema_migrations/20211213154259

+fccb1d6c7ac4e31cecaf7bc2e23f13f6c8147a3820cbd996a545a5b01cc03865
\ No newline at end of file

db/schema_migrations/20211213154704

+deec24bae35829454a09d4e97478c0b57d5f80e3271f96b2554b1ab10dc84d7f
\ No newline at end of file

db/structure.sql

@@ -17425,6 +17425,7 @@ CREATE TABLE packages_package_files (
     verification_checksum bytea,
     verification_state smallint DEFAULT 0 NOT NULL,
     verification_started_at timestamp with time zone,
+    status smallint DEFAULT 0 NOT NULL,
     CONSTRAINT check_4c5e6bb0b3 CHECK ((file_store IS NOT NULL))
 );
 
@@ -26921,6 +26922,8 @@ CREATE INDEX index_packages_package_files_on_package_id_and_file_name ON package
 
 CREATE INDEX index_packages_package_files_on_package_id_id ON packages_package_files USING btree (package_id, id);
 
+CREATE INDEX index_packages_package_files_on_package_id_status_and_id ON packages_package_files USING btree (package_id, status, id);
+
 CREATE INDEX index_packages_package_files_on_verification_state ON packages_package_files USING btree (verification_state);
 
 CREATE INDEX index_packages_packages_on_creator_id ON packages_packages USING btree (creator_id);

lib/api/package_files.rb

@@ -28,10 +28,15 @@ module API
         package = ::Packages::PackageFinder
           .new(user_project, params[:package_id]).execute
 
-        files = package.package_files
-                       .preload_pipelines
+        package_files = if Feature.enabled?(:packages_installable_package_files)
+                          package.installable_package_files
+                        else
+                          package.package_files
+                        end
 
-        present paginate(files), with: ::API::Entities::PackageFile
+        package_files = package_files.preload_pipelines
+
+        present paginate(package_files), with: ::API::Entities::PackageFile
       end
 
       desc 'Remove a package file' do
@@ -50,7 +55,13 @@ module API
 
         not_found! unless package
 
-        package_file = package.package_files.find_by_id(params[:package_file_id])
+        package_files = if Feature.enabled?(:packages_installable_package_files)
+                          package.installable_package_files
+                        else
+                          package.package_files
+                        end
+
+        package_file = package_files.find_by_id(params[:package_file_id])
 
         not_found! unless package_file
 

lib/api/rubygem_packages.rb

@@ -66,9 +66,12 @@ module API
         get "gems/:file_name", requirements: FILE_NAME_REQUIREMENTS do
           authorize!(:read_package, user_project)
 
-          package_file = ::Packages::PackageFile.for_rubygem_with_file_name(
-            user_project, params[:file_name]
-          ).last!
+          package_files = ::Packages::PackageFile
+                            .for_rubygem_with_file_name(user_project, params[:file_name])
+
+          package_files = package_files.installable if Feature.enabled?(:packages_installable_package_files)
+
+          package_file = package_files.last!
 
           track_package_event('pull_package', :rubygems, project: user_project, namespace: user_project.namespace)
 

lib/api/terraform/modules/v1/packages.rb

@@ -71,7 +71,11 @@ module API
 
             def package_file
               strong_memoize(:package_file) do
-                package.package_files.first
+                if Feature.enabled?(:packages_installable_package_files)
+                  package.installable_package_files.first
+                else
+                  package.package_files.first
+                end
               end
             end
           end

spec/controllers/projects/packages/infrastructure_registry_controller_spec.rb

@@ -41,5 +41,29 @@ RSpec.describe Projects::Packages::InfrastructureRegistryController do
 
       it_behaves_like 'returning response status', :not_found
     end
+
+    context 'with package file pending destruction' do
+      let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: terraform_module) }
+
+      let(:terraform_module_package_file) { terraform_module.package_files.first }
+
+      it 'does not return them' do
+        subject
+
+        expect(assigns(:package_files)).to contain_exactly(terraform_module_package_file)
+      end
+
+      context 'with packages_installable_package_files disabled' do
+        before do
+          stub_feature_flags(packages_installable_package_files: false)
+        end
+
+        it 'returns them' do
+          subject
+
+          expect(assigns(:package_files)).to contain_exactly(package_file_pending_destruction, terraform_module_package_file)
+        end
+      end
+    end
   end
 end

spec/factories/packages/package_files.rb

@@ -6,6 +6,8 @@ FactoryBot.define do
 
     file_name { 'somefile.txt' }
 
+    status { :default }
+
     transient do
       file_fixture { 'spec/fixtures/packages/conan/recipe_files/conanfile.py' }
     end
@@ -14,6 +16,10 @@ FactoryBot.define do
       package_file.file = fixture_file_upload(evaluator.file_fixture)
     end
 
+    trait :pending_destruction do
+      status { :pending_destruction }
+    end
+
     factory :conan_package_file do
       package { association(:conan_package, without_package_files: true) }
 

spec/finders/packages/conan/package_file_finder_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe ::Packages::Conan::PackageFileFinder do
   let(:package_file_name) { package_file.file_name }
   let(:params) { {} }
 
-  RSpec.shared_examples 'package file finder examples' do
+  shared_examples 'package file finder examples' do
     it { is_expected.to eq(package_file) }
 
     context 'with conan_file_type' do
@@ -39,11 +39,37 @@ RSpec.describe ::Packages::Conan::PackageFileFinder do
     end
   end
 
+  shared_examples 'not returning pending_destruction package files' do
+    let_it_be(:recent_package_file_pending_destruction) do
+      create(:package_file, :pending_destruction, package: package, file_name: package_file.file_name)
+    end
+
+    it 'returns the correct package file' do
+      expect(package.package_files.last).to eq(recent_package_file_pending_destruction)
+
+      expect(subject).to eq(package_file)
+    end
+
+    context 'with packages_installable_package_files disabled' do
+      before do
+        stub_feature_flags(packages_installable_package_files: false)
+      end
+
+      it 'returns the correct package file' do
+        expect(package.package_files.last).to eq(recent_package_file_pending_destruction)
+
+        expect(subject).to eq(recent_package_file_pending_destruction)
+      end
+    end
+  end
+
   describe '#execute' do
     subject { described_class.new(package, package_file_name, params).execute }
 
     it_behaves_like 'package file finder examples'
 
+    it_behaves_like 'not returning pending_destruction package files'
+
     context 'with unknown file_name' do
       let(:package_file_name) { 'unknown.jpg' }
 
@@ -56,6 +82,8 @@ RSpec.describe ::Packages::Conan::PackageFileFinder do
 
     it_behaves_like 'package file finder examples'
 
+    it_behaves_like 'not returning pending_destruction package files'
+
     context 'with unknown file_name' do
       let(:package_file_name) { 'unknown.jpg' }
 

spec/finders/packages/package_file_finder_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Packages::PackageFileFinder do
   let(:package_file_name) { package_file.file_name }
   let(:params) { {} }
 
-  RSpec.shared_examples 'package file finder examples' do
+  shared_examples 'package file finder examples' do
     it { is_expected.to eq(package_file) }
 
     context 'with file_name_like' do
@@ -19,11 +19,35 @@ RSpec.describe Packages::PackageFileFinder do
     end
   end
 
+  shared_examples 'not returning pending_destruction package files' do
+    let_it_be(:recent_package_file_pending_destruction) do
+      create(:package_file, :pending_destruction, package: package, file_name: package_file.file_name)
+    end
+
+    it 'returns the correct package file' do
+      expect(package.package_files.last).to eq(recent_package_file_pending_destruction)
+
+      expect(subject).to eq(package_file)
+    end
+
+    context 'with packages_installable_package_files disabled' do
+      before do
+        stub_feature_flags(packages_installable_package_files: false)
+      end
+
+      it 'returns them' do
+        expect(subject).to eq(recent_package_file_pending_destruction)
+      end
+    end
+  end
+
   describe '#execute' do
     subject { described_class.new(package, package_file_name, params).execute }
 
     it_behaves_like 'package file finder examples'
 
+    it_behaves_like 'not returning pending_destruction package files'
+
     context 'with unknown file_name' do
       let(:package_file_name) { 'unknown.jpg' }
 
@@ -36,6 +60,8 @@ RSpec.describe Packages::PackageFileFinder do
 
     it_behaves_like 'package file finder examples'
 
+    it_behaves_like 'not returning pending_destruction package files'
+
     context 'with unknown file_name' do
       let(:package_file_name) { 'unknown.jpg' }
 

spec/models/packages/package_file_spec.rb

@@ -10,6 +10,8 @@ RSpec.describe Packages::PackageFile, type: :model do
   let_it_be(:package_file3) { create(:package_file, :xml, file_name: 'formatted.zip') }
   let_it_be(:debian_package) { create(:debian_package, project: project) }
 
+  it_behaves_like 'having unique enum values'
+
   describe 'relationships' do
     it { is_expected.to belong_to(:package) }
     it { is_expected.to have_one(:conan_file_metadatum) }
@@ -138,6 +140,24 @@ RSpec.describe Packages::PackageFile, type: :model do
     it 'returns the matching file only for Helm packages' do
       expect(described_class.for_helm_with_channel(project, channel)).to contain_exactly(helm_file2)
     end
+
+    context 'with package files pending destruction' do
+      let_it_be(:package_file_pending_destruction) { create(:helm_package_file, :pending_destruction, package: helm_package2, channel: channel) }
+
+      it 'does not return them' do
+        expect(described_class.for_helm_with_channel(project, channel)).to contain_exactly(helm_file2)
+      end
+
+      context 'with packages_installable_package_files disabled' do
+        before do
+          stub_feature_flags(packages_installable_package_files: false)
+        end
+
+        it 'returns them' do
+          expect(described_class.for_helm_with_channel(project, channel)).to contain_exactly(helm_file2, package_file_pending_destruction)
+        end
+      end
+    end
   end
 
   describe '.most_recent!' do
@@ -154,15 +174,17 @@ RSpec.describe Packages::PackageFile, type: :model do
 
     let_it_be(:package_file3_2) { create(:package_file, :npm, package: package3) }
     let_it_be(:package_file3_3) { create(:package_file, :npm, package: package3) }
+    let_it_be(:package_file3_4) { create(:package_file, :npm, :pending_destruction, package: package3) }
 
     let_it_be(:package_file4_2) { create(:package_file, :npm, package: package2) }
     let_it_be(:package_file4_3) { create(:package_file, :npm, package: package2) }
     let_it_be(:package_file4_4) { create(:package_file, :npm, package: package2) }
+    let_it_be(:package_file4_4) { create(:package_file, :npm, :pending_destruction, package: package2) }
 
-    let(:most_recent_package_file1) { package1.package_files.recent.first }
-    let(:most_recent_package_file2) { package2.package_files.recent.first }
-    let(:most_recent_package_file3) { package3.package_files.recent.first }
-    let(:most_recent_package_file4) { package4.package_files.recent.first }
+    let(:most_recent_package_file1) { package1.installable_package_files.recent.first }
+    let(:most_recent_package_file2) { package2.installable_package_files.recent.first }
+    let(:most_recent_package_file3) { package3.installable_package_files.recent.first }
+    let(:most_recent_package_file4) { package4.installable_package_files.recent.first }
 
     subject { described_class.most_recent_for(packages) }
 
@@ -202,6 +224,24 @@ RSpec.describe Packages::PackageFile, type: :model do
       it 'returns the most recent package for the selected channel' do
         expect(subject).to contain_exactly(helm_package_file2)
       end
+
+      context 'with package files pending destruction' do
+        let_it_be(:package_file_pending_destruction) { create(:helm_package_file, :pending_destruction, package: helm_package, channel: 'alpha') }
+
+        it 'does not return them' do
+          expect(subject).to contain_exactly(helm_package_file2)
+        end
+
+        context 'with packages_installable_package_files disabled' do
+          before do
+            stub_feature_flags(packages_installable_package_files: false)
+          end
+
+          it 'returns them' do
+            expect(subject).to contain_exactly(package_file_pending_destruction)
+          end
+        end
+      end
     end
   end
 
@@ -314,4 +354,25 @@ RSpec.describe Packages::PackageFile, type: :model do
       end
     end
   end
+
+  context 'status scopes' do
+    let_it_be(:package) { create(:package) }
+    let_it_be(:default_package_file) { create(:package_file, package: package) }
+    let_it_be(:pending_destruction_package_file) { create(:package_file, :pending_destruction, package: package) }
+
+    describe '.installable' do
+      subject { package.installable_package_files }
+
+      it 'does not include non-displayable packages', :aggregate_failures do
+        is_expected.to include(default_package_file)
+        is_expected.not_to include(pending_destruction_package_file)
+      end
+    end
+
+    describe '.with_status' do
+      subject { described_class.with_status(:pending_destruction) }
+
+      it { is_expected.to contain_exactly(pending_destruction_package_file) }
+    end
+  end
 end

spec/presenters/packages/conan/package_presenter_spec.rb

@@ -9,6 +9,7 @@ RSpec.describe ::Packages::Conan::PackagePresenter do
   let_it_be(:conan_package_reference) { '123456789'}
 
   let(:params) { { package_scope: :instance } }
+  let(:presenter) { described_class.new(package, user, project, params) }
 
   shared_examples 'no existing package' do
     context 'when package does not exist' do
@@ -21,7 +22,7 @@ RSpec.describe ::Packages::Conan::PackagePresenter do
   shared_examples 'conan_file_metadatum is not found' do
     context 'when no conan_file_metadatum exists' do
       before do
-        package.package_files.each do |file|
+        package.installable_package_files.each do |file|
           file.conan_file_metadatum.delete
           file.reload
         end
@@ -32,7 +33,7 @@ RSpec.describe ::Packages::Conan::PackagePresenter do
   end
 
   describe '#recipe_urls' do
-    subject { described_class.new(package, user, project, params).recipe_urls }
+    subject { presenter.recipe_urls }
 
     it_behaves_like 'no existing package'
     it_behaves_like 'conan_file_metadatum is not found'
@@ -71,7 +72,9 @@ RSpec.describe ::Packages::Conan::PackagePresenter do
   end
 
   describe '#recipe_snapshot' do
-    subject { described_class.new(package, user, project).recipe_snapshot }
+    let(:params) { {} }
+
+    subject { presenter.recipe_snapshot }
 
     it_behaves_like 'no existing package'
     it_behaves_like 'conan_file_metadatum is not found'
@@ -180,12 +183,9 @@ RSpec.describe ::Packages::Conan::PackagePresenter do
 
   describe '#package_snapshot' do
     let(:reference) { conan_package_reference }
+    let(:params) { { conan_package_reference: reference } }
 
-    subject do
-      described_class.new(
-        package, user, project, conan_package_reference: reference
-      ).package_snapshot
-    end
+    subject { presenter.package_snapshot }
 
     it_behaves_like 'no existing package'
     it_behaves_like 'conan_file_metadatum is not found'
@@ -208,4 +208,22 @@ RSpec.describe ::Packages::Conan::PackagePresenter do
       end
     end
   end
+
+  # TODO when cleaning up packages_installable_package_files, consider removing this context and
+  # add a dummy package file pending destruction on L8
+  context 'with package files pending destruction' do
+    let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package) }
+
+    subject { presenter.send(:package_files).to_a }
+
+    it { is_expected.not_to include(package_file_pending_destruction) }
+
+    context 'with packages_installable_package_files disabled' do
+      before do
+        stub_feature_flags(packages_installable_package_files: false)
+      end
+
+      it { is_expected.to include(package_file_pending_destruction) }
+    end
+  end
 end

spec/presenters/packages/detail/package_presenter_spec.rb

@@ -6,12 +6,12 @@ RSpec.describe ::Packages::Detail::PackagePresenter do
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, creator: user) }
   let_it_be(:package) { create(:npm_package, :with_build, project: project) }
-  let(:presenter) { described_class.new(package) }
-
   let_it_be(:user_info) { { name: user.name, avatar_url: user.avatar_url } }
 
+  let(:presenter) { described_class.new(package) }
+
   let!(:expected_package_files) do
-    package.package_files.map do |file|
+    package.installable_package_files.map do |file|
       {
         created_at: file.created_at,
         download_path: file.download_path,
@@ -154,5 +154,21 @@ RSpec.describe ::Packages::Detail::PackagePresenter do
         expect(presenter.detail_view).to eq expected_package_details
       end
     end
+
+    context 'with package files pending destruction' do
+      let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package) }
+
+      subject { presenter.detail_view[:package_files].map { |e| e[:id] } }
+
+      it { is_expected.not_to include(package_file_pending_destruction.id) }
+
+      context 'with packages_installable_package_files disabled' do
+        before do
+          stub_feature_flags(packages_installable_package_files: false)
+        end
+
+        it { is_expected.to include(package_file_pending_destruction.id) }
+      end
+    end
   end
 end

spec/presenters/packages/npm/package_presenter_spec.rb

@@ -95,6 +95,26 @@ RSpec.describe ::Packages::Npm::PackagePresenter do
         end
       end
     end
+
+    context 'with package files pending destruction' do
+      let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package2, file_sha1: 'pending_destruction_sha1') }
+
+      let(:shasums) { subject.values.map { |v| v.dig(:dist, :shasum) } }
+
+      it 'does not return them' do
+        expect(shasums).not_to include(package_file_pending_destruction.file_sha1)
+      end
+
+      context 'with packages_installable_package_files disabled' do
+        before do
+          stub_feature_flags(packages_installable_package_files: false)
+        end
+
+        it 'returns them' do
+          expect(shasums).to include(package_file_pending_destruction.file_sha1)
+        end
+      end
+    end
   end
 
   describe '#dist_tags' do

spec/presenters/packages/nuget/package_metadata_presenter_spec.rb

@@ -24,6 +24,20 @@ RSpec.describe Packages::Nuget::PackageMetadataPresenter do
     subject { presenter.archive_url }
 
     it { is_expected.to end_with(expected_suffix) }
+
+    context 'with package files pending destruction' do
+      let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package, file_name: 'pending_destruction.nupkg') }
+
+      it { is_expected.not_to include('pending_destruction.nupkg') }
+
+      context 'with packages_installable_package_files disabled' do
+        before do
+          stub_feature_flags(packages_installable_package_files: false)
+        end
+
+        it { is_expected.to include('pending_destruction.nupkg') }
+      end
+    end
   end
 
   describe '#catalog_entry' do

spec/presenters/packages/pypi/package_presenter_spec.rb

@@ -52,5 +52,21 @@ RSpec.describe ::Packages::Pypi::PackagePresenter do
 
       it_behaves_like 'pypi package presenter'
     end
+
+    context 'with package files pending destruction' do
+      let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package1, file_name: "package_file_pending_destruction") }
+
+      let(:project_or_group) { project }
+
+      it { is_expected.not_to include(package_file_pending_destruction.file_name)}
+
+      context 'with packages_installable_package_files disabled' do
+        before do
+          stub_feature_flags(packages_installable_package_files: false)
+        end
+
+        it { is_expected.to include(package_file_pending_destruction.file_name)}
+      end
+    end
   end
 end

spec/requests/api/graphql/packages/package_spec.rb

@@ -73,6 +73,31 @@ RSpec.describe 'package details' do
     end
   end
 
+  context 'with package files pending destruction' do
+    let_it_be(:package_file) { create(:package_file, package: composer_package) }
+    let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: composer_package) }
+
+    let(:package_file_ids) { graphql_data_at(:package, :package_files, :nodes).map { |node| node["id"] } }
+
+    it 'does not return them' do
+      subject
+
+      expect(package_file_ids).to contain_exactly(package_file.to_global_id.to_s)
+    end
+
+    context 'with packages_installable_package_files disabled' do
+      before do
+        stub_feature_flags(packages_installable_package_files: false)
+      end
+
+      it 'returns them' do
+        subject
+
+        expect(package_file_ids).to contain_exactly(package_file_pending_destruction.to_global_id.to_s, package_file.to_global_id.to_s)
+      end
+    end
+  end
+
   context 'with a batched query' do
     let_it_be(:conan_package) { create(:conan_package, project: project) }
 

spec/requests/api/package_files_spec.rb

@@ -76,6 +76,30 @@ RSpec.describe API::PackageFiles do
           end
         end
       end
+
+      context 'with package files pending destruction' do
+        let!(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package) }
+
+        let(:package_file_ids) { json_response.map { |e| e['id'] } }
+
+        it 'does not return them' do
+          get api(url, user)
+
+          expect(package_file_ids).not_to include(package_file_pending_destruction.id)
+        end
+
+        context 'with packages_installable_package_files disabled' do
+          before do
+            stub_feature_flags(packages_installable_package_files: false)
+          end
+
+          it 'returns them' do
+            get api(url, user)
+
+            expect(package_file_ids).to include(package_file_pending_destruction.id)
+          end
+        end
+      end
     end
   end
 
@@ -149,6 +173,32 @@ RSpec.describe API::PackageFiles do
           expect(response).to have_gitlab_http_status(:not_found)
         end
       end
+
+      context 'with package file pending destruction' do
+        let!(:package_file_id) { create(:package_file, :pending_destruction, package: package).id }
+
+        before do
+          project.add_maintainer(user)
+        end
+
+        it 'can not be accessed', :aggregate_failures do
+          expect { api_request }.not_to change { package.package_files.count }
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+
+        context 'with packages_installable_package_files disabled' do
+          before do
+            stub_feature_flags(packages_installable_package_files: false)
+          end
+
+          it 'can be accessed', :aggregate_failures do
+            expect { api_request }.to change { package.package_files.count }.by(-1)
+
+            expect(response).to have_gitlab_http_status(:no_content)
+          end
+        end
+      end
     end
   end
 end

spec/requests/api/rubygem_packages_spec.rb

@@ -173,6 +173,34 @@ RSpec.describe API::RubygemPackages do
         it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member]
       end
     end
+
+    context 'with package files pending destruction' do
+      let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, :xml, package: package, file_name: file_name) }
+
+      before do
+        project.update_column(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
+      end
+
+      it 'does not return them' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.body).not_to eq(package_file_pending_destruction.file.file.read)
+      end
+
+      context 'with packages_installable_package_files disabled' do
+        before do
+          stub_feature_flags(packages_installable_package_files: false)
+        end
+
+        it 'returns them' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response.body).to eq(package_file_pending_destruction.file.file.read)
+        end
+      end
+    end
   end
 
   describe 'POST /api/v4/projects/:project_id/packages/rubygems/api/v1/gems/authorize' do

spec/requests/api/terraform/modules/v1/packages_spec.rb

@@ -154,6 +154,7 @@ RSpec.describe API::Terraform::Modules::V1::Packages do
   end
 
   describe 'GET /api/v4/packages/terraform/modules/v1/:module_namespace/:module_name/:module_system/:module_version/file' do
+    let(:url) { api("/packages/terraform/modules/v1/#{group.path}/#{package.name}/#{package.version}/file?token=#{token}") }
     let(:tokens) do
       {
         personal_access_token: ::Gitlab::JWTToken.new.tap { |jwt| jwt['token'] = personal_access_token.id }.encoded,
@@ -202,7 +203,6 @@ RSpec.describe API::Terraform::Modules::V1::Packages do
 
       with_them do
         let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' }
-        let(:url) { api("/packages/terraform/modules/v1/#{group.path}/#{package.name}/#{package.version}/file?token=#{token}") }
         let(:snowplow_gitlab_standard_context) { { project: project, user: user, namespace: project.namespace } }
 
         before do
@@ -212,6 +212,41 @@ RSpec.describe API::Terraform::Modules::V1::Packages do
         it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member]
       end
     end
+
+    context 'with package file pending destruction' do
+      let_it_be(:package) { create(:package, package_type: :terraform_module, project: project, name: "module-555/pending-destruction", version: '1.0.0') }
+      let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, :xml, package: package) }
+      let_it_be(:package_file) { create(:package_file, :terraform_module, package: package) }
+
+      let(:token) { tokens[:personal_access_token] }
+      let(:headers) { { 'Authorization' => "Bearer #{token}" } }
+
+      before do
+        project.add_maintainer(user)
+      end
+
+      it 'does not return them' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.body).not_to eq(package_file_pending_destruction.file.file.read)
+        expect(response.body).to eq(package_file.file.file.read)
+      end
+
+      context 'with packages_installable_package_files disabled' do
+        before do
+          stub_feature_flags(packages_installable_package_files: false)
+        end
+
+        it 'returns them' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response.body).to eq(package_file_pending_destruction.file.file.read)
+          expect(response.body).not_to eq(package_file.file.file.read)
+        end
+      end
+    end
   end
 
   describe 'PUT /api/v4/projects/:project_id/packages/terraform/modules/:module_name/:module_system/:module_version/file/authorize' do

spec/services/packages/maven/metadata/sync_service_spec.rb

@@ -265,4 +265,22 @@ RSpec.describe ::Packages::Maven::Metadata::SyncService do
       end
     end
   end
+
+  # TODO When cleaning up packages_installable_package_files, consider adding a
+  # dummy package file pending for destruction on L10/11 and remove this context
+  context 'with package files pending destruction' do
+    let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: versionless_package_for_versions, file_name: Packages::Maven::Metadata.filename) }
+
+    subject { service.send(:metadata_package_file_for, versionless_package_for_versions) }
+
+    it { is_expected.not_to eq(package_file_pending_destruction) }
+
+    context 'with packages_installable_package_files disabled' do
+      before do
+        stub_feature_flags(packages_installable_package_files: false)
+      end
+
+      it { is_expected.to eq(package_file_pending_destruction) }
+    end
+  end
 end

spec/support/shared_examples/models/packages/debian/distribution_shared_examples.rb

@@ -198,7 +198,6 @@ RSpec.shared_examples 'Debian Distribution' do |factory, container, can_freeze|
       describe 'relationships' do
         it { is_expected.to have_many(:publications).class_name('Packages::Debian::Publication').inverse_of(:distribution).with_foreign_key(:distribution_id) }
         it { is_expected.to have_many(:packages).class_name('Packages::Package').through(:publications) }
-        it { is_expected.to have_many(:package_files).class_name('Packages::PackageFile').through(:packages) }
       end
     end
   else
@@ -229,6 +228,26 @@ RSpec.shared_examples 'Debian Distribution' do |factory, container, can_freeze|
         it 'returns only files from public packages with same codename' do
           expect(subject.to_a).to contain_exactly(*public_package_with_same_codename.package_files)
         end
+
+        context 'with pending destruction package files' do
+          let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: public_package_with_same_codename) }
+
+          it 'does not return them' do
+            expect(subject.to_a).not_to include(package_file_pending_destruction)
+          end
+
+          context 'with packages_installable_package_files disabled' do
+            before do
+              stub_feature_flags(packages_installable_package_files: false)
+            end
+
+            it 'returns them' do
+              subject
+
+              expect(subject.to_a).to include(package_file_pending_destruction)
+            end
+          end
+        end
       end
     end
   end

spec/support/shared_examples/requests/api/graphql/packages/package_details_shared_examples.rb

@@ -38,4 +38,28 @@ RSpec.shared_examples 'a package with files' do
       'fileSha256' => first_file.file_sha256
     )
   end
+
+  context 'with package files pending destruction' do
+    let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package) }
+
+    let(:response_package_file_ids) { package_files_response.map { |pf| pf['id'] } }
+
+    it 'does not return them' do
+      expect(package.reload.package_files).to include(package_file_pending_destruction)
+
+      expect(response_package_file_ids).not_to include(package_file_pending_destruction.to_global_id.to_s)
+    end
+
+    context 'with packages_installable_package_files disabled' do
+      before(:context) do
+        stub_feature_flags(packages_installable_package_files: false)
+      end
+
+      it 'returns them' do
+        expect(package.reload.package_files).to include(package_file_pending_destruction)
+
+        expect(response_package_file_ids).to include(package_file_pending_destruction.to_global_id.to_s)
+      end
+    end
+  end
 end

spec/support/shared_examples/requests/api/nuget_endpoints_shared_examples.rb

@@ -221,6 +221,7 @@ RSpec.shared_examples 'handling nuget search requests' do |anonymous_requests_ex
   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) }
   let_it_be(:package_e) { create(:nuget_package, name: 'Foo.BarE', project: project) }
+
   let(:search_term) { 'uMmy' }
   let(:take) { 26 }
   let(:skip) { 0 }