Commit e7420d28 authored by David Fernandez's avatar David Fernandez Committed by Hugo Ortiz

Add package file status attribute

This is used to differentiate package files in the pending_destruction
status from the default status.
Package files in the pending_destruction status are not returned by APIs/UI

Changelog: other
parent b4c272ca
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -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
......
# 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
......@@ -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
......@@ -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
......@@ -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) }
......
......@@ -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
......
......@@ -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)
......
......@@ -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,
......
......@@ -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
......
......@@ -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
)
......
......@@ -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)
......
......@@ -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)
......
---
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
# 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
# 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
fccb1d6c7ac4e31cecaf7bc2e23f13f6c8147a3820cbd996a545a5b01cc03865
\ No newline at end of file
deec24bae35829454a09d4e97478c0b57d5f80e3271f96b2554b1ab10dc84d7f
\ No newline at end of file
......@@ -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);
......@@ -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
......
......@@ -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)
......
......@@ -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
......
......@@ -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
......@@ -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) }
......
......@@ -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' }
......
......@@ -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' }
......
......@@ -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
......@@ -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
......@@ -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
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -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) }
......
......@@ -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
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -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
......
......@@ -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
......@@ -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 }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment