Merge branch '348677-cleanup-feature-flag' into 'master'

Remove the `packages_installable_package_files` feature flag

See merge request gitlab-org/gitlab!79745

app/controllers/projects/packages/infrastructure_registry_controller.rb

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

app/finders/packages/package_file_finder.rb

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

app/graphql/types/packages/package_details_type.rb

@@ -12,7 +12,7 @@ module Types
       field :versions, ::Types::Packages::PackageType.connection_type, null: true,
         description: 'Other versions of the package.'
 
-      field :package_files, Types::Packages::PackageFileType.connection_type, null: true, description: 'Package files.'
+      field :package_files, Types::Packages::PackageFileType.connection_type, null: true, method: :installable_package_files, description: 'Package files.'
 
       field :dependency_links, Types::Packages::PackageDependencyLinkType.connection_type, null: true, description: 'Dependency link.'
 
@@ -36,14 +36,6 @@ module Types
         object.versions
       end
 
-      def package_files
-        if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-          object.installable_package_files
-        else
-          object.package_files
-        end
-      end
-
       def composer_config_repository_url
         composer_config_repository_name(object.project.group&.id)
       end

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

@@ -97,12 +97,8 @@ module Packages
         end
 
         def package_files
-          if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-            ::Packages::PackageFile.installable
-                                   .for_package_ids(packages.select(:id))
-          else
-            ::Packages::PackageFile.for_package_ids(packages.select(:id))
-          end
+          ::Packages::PackageFile.installable
+                                 .for_package_ids(packages.select(:id))
         end
 
         private

app/models/packages/package.rb

@@ -133,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, -> { Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) ? preload(:installable_package_files) : preload(:package_files) }
+  scope :preload_files, -> { preload(:installable_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

@@ -55,12 +55,11 @@ class Packages::PackageFile < ApplicationRecord
   end
 
   scope :for_helm_with_channel, ->(project, channel) do
-    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, default_enabled: :yaml)
-    result
+    joins(:package)
+      .merge(project.packages.helm.installable)
+      .joins(:helm_file_metadatum)
+      .where(packages_helm_file_metadata: { channel: channel })
+      .installable
   end
 
   scope :with_conan_file_type, ->(file_type) do
@@ -110,13 +109,9 @@ class Packages::PackageFile < ApplicationRecord
     cte_name = :packages_cte
     cte = Gitlab::SQL::CTE.new(cte_name, packages.select(:id))
 
-    package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-                      ::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 = ::Packages::PackageFile.installable
+                                           .limit_recent(1)
+                                           .where(arel_table[:package_id].eq(Arel.sql("#{cte_name}.id")))
 
     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

@@ -81,11 +81,7 @@ module Packages
         return unless @package
 
         strong_memoize(:package_files) do
-          if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-            @package.installable_package_files.preload_conan_file_metadata
-          else
-            @package.package_files.preload_conan_file_metadata
-          end
+          @package.installable_package_files.preload_conan_file_metadata
         end
       end
 

app/presenters/packages/detail/package_presenter.rb

@@ -39,11 +39,7 @@ module Packages
       private
 
       def package_file_views
-        package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-                          @package.installable_package_files
-                        else
-                          @package.package_files
-                        end
+        package_files = @package.installable_package_files
 
         package_files.map { |pf| build_package_file_view(pf) }
       end

app/presenters/packages/npm/package_presenter.rb

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

app/presenters/packages/nuget/presenter_helpers.rb

@@ -27,13 +27,10 @@ module Packages
       end
 
       def archive_url_for(package)
-        package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-                          package.installable_package_files
-                        else
-                          package.package_files
-                        end
-
-        package_filename = package_files.with_format(NUGET_PACKAGE_FORMAT).last&.file_name
+        package_filename = package.installable_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,

app/presenters/packages/pypi/package_presenter.rb

@@ -36,11 +36,7 @@ module Packages
         refs = []
 
         @packages.map do |package|
-          package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-                            package.installable_package_files
-                          else
-                            package.package_files
-                          end
+          package_files = package.installable_package_files
 
           package_files.each do |file|
             url = build_pypi_package_path(file)

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

@@ -93,11 +93,7 @@ module Packages
         def metadata_package_file_for(package)
           return unless package
 
-          package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-                            package.installable_package_files
-                          else
-                            package.package_files
-                          end
+          package_files = package.installable_package_files
 
           package_files.with_file_name(Metadata.filename)
                        .recent

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: true

lib/api/package_files.rb

@@ -28,13 +28,8 @@ module API
         package = ::Packages::PackageFinder
           .new(user_project, params[:package_id]).execute
 
-        package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-                          package.installable_package_files
-                        else
-                          package.package_files
-                        end
-
-        package_files = package_files.preload_pipelines
+        package_files = package.installable_package_files
+                               .preload_pipelines
 
         present paginate(package_files), with: ::API::Entities::PackageFile
       end
@@ -55,13 +50,8 @@ module API
 
         not_found! unless package
 
-        package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml)
-                          package.installable_package_files
-                        else
-                          package.package_files
-                        end
-
-        package_file = package_files.find_by_id(params[:package_file_id])
+        package_file = package.installable_package_files
+                              .find_by_id(params[:package_file_id])
 
         not_found! unless package_file
 

lib/api/rubygem_packages.rb

@@ -69,9 +69,7 @@ module API
           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, default_enabled: :yaml)
-
-          package_file = package_files.last!
+          package_file = package_files.installable.last!
 
           track_package_event('pull_package', :rubygems, project: user_project, namespace: user_project.namespace)
 

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

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

spec/controllers/projects/packages/infrastructure_registry_controller_spec.rb

@@ -52,18 +52,6 @@ RSpec.describe Projects::Packages::InfrastructureRegistryController do
 
         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/finders/packages/conan/package_file_finder_spec.rb

@@ -49,18 +49,6 @@ RSpec.describe ::Packages::Conan::PackageFileFinder do
 
       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

spec/finders/packages/package_file_finder_spec.rb

@@ -29,16 +29,6 @@ RSpec.describe Packages::PackageFileFinder do
 
       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

spec/models/packages/package_file_spec.rb

@@ -148,16 +148,6 @@ RSpec.describe Packages::PackageFile, type: :model do
       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
 
@@ -232,16 +222,6 @@ RSpec.describe Packages::PackageFile, type: :model do
         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

spec/presenters/packages/conan/package_presenter_spec.rb

@@ -6,6 +6,7 @@ RSpec.describe ::Packages::Conan::PackagePresenter do
   let_it_be(:user) { create(:user) }
   let_it_be(:package) { create(:conan_package) }
   let_it_be(:project) { package.project }
+  let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package) }
   let_it_be(:conan_package_reference) { '123456789'}
 
   let(:params) { { package_scope: :instance } }
@@ -208,22 +209,4 @@ 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

@@ -161,14 +161,6 @@ RSpec.describe ::Packages::Detail::PackagePresenter do
       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

@@ -104,17 +104,6 @@ RSpec.describe ::Packages::Npm::PackagePresenter do
       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)
-          package2.package_files.id_not_in(package_file_pending_destruction.id).delete_all
-        end
-
-        it 'returns them' do
-          expect(shasums).to include(package_file_pending_destruction.file_sha1)
-        end
-      end
     end
   end
 

spec/presenters/packages/nuget/package_metadata_presenter_spec.rb

@@ -29,14 +29,6 @@ RSpec.describe Packages::Nuget::PackageMetadataPresenter 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
 

spec/presenters/packages/pypi/package_presenter_spec.rb

@@ -59,14 +59,6 @@ RSpec.describe ::Packages::Pypi::PackagePresenter do
       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

@@ -102,18 +102,6 @@ RSpec.describe 'package details' do
 
         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

spec/requests/api/package_files_spec.rb

@@ -87,18 +87,6 @@ RSpec.describe API::PackageFiles do
 
           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
@@ -186,18 +174,6 @@ RSpec.describe API::PackageFiles do
 
           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 }.not_to change { package.package_files.pending_destruction.count }
-
-            expect(response).to have_gitlab_http_status(:no_content)
-          end
-        end
       end
     end
   end

spec/requests/api/rubygem_packages_spec.rb

@@ -187,19 +187,6 @@ RSpec.describe API::RubygemPackages do
         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
 

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

@@ -232,20 +232,6 @@ RSpec.describe API::Terraform::Modules::V1::Packages do
         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
 

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

@@ -9,6 +9,7 @@ RSpec.describe ::Packages::Maven::Metadata::SyncService do
   let_it_be(:user) { create(:user) }
   let_it_be_with_reload(:versionless_package_for_versions) { create(:maven_package, name: 'test', version: nil, project: project) }
   let_it_be_with_reload(:metadata_file_for_versions) { create(:package_file, :xml, package: versionless_package_for_versions) }
+  let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: versionless_package_for_versions, file_name: Packages::Maven::Metadata.filename) }
 
   let(:service) { described_class.new(container: project, current_user: user, params: { package_name: versionless_package_for_versions.name }) }
 
@@ -265,22 +266,4 @@ 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

@@ -235,18 +235,6 @@ RSpec.shared_examples 'Debian Distribution' do |factory, container, can_freeze|
           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

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

@@ -49,17 +49,5 @@ RSpec.shared_examples 'a package with files' do
 
       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