diff --git a/app/graphql/resolvers/package_details_resolver.rb b/app/graphql/resolvers/package_details_resolver.rb index 8d25d378eb64dd900ca4ec30ac04df7f34d08189..dc9eecbdd2dfb209547766f6685b07d6c4a0d39e 100644 --- a/app/graphql/resolvers/package_details_resolver.rb +++ b/app/graphql/resolvers/package_details_resolver.rb @@ -12,7 +12,7 @@ class PackageDetailsResolver < BaseResolver def resolve(id:) Gitlab::Graphql::Lazy.with_value(find_object(id: id)) do |package| - package if package&.default? + package if package&.detailed_info? end end diff --git a/app/graphql/types/packages/package_links_type.rb b/app/graphql/types/packages/package_links_type.rb index eb29fb655bd69139bcf575cfe2f60ad3f5f8c5c5..001391fc2560682e1ea189a8b8d4ed40be288a31 100644 --- a/app/graphql/types/packages/package_links_type.rb +++ b/app/graphql/types/packages/package_links_type.rb @@ -12,7 +12,7 @@ class PackageLinksType < BaseObject field :web_path, GraphQL::Types::String, null: true, description: 'Path to the package details page.' def web_path - return unless object.default? + return unless object.detailed_info? package_path(object) end diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index 003e96c7910518f2f8741d4fb6eb4d96e7c5a7e6..1d16dcde8eb5368d13ba4afd511879de18a51947 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -9,8 +9,9 @@ class Packages::Package < ApplicationRecord include Packages::Downloadable include EnumInheritance - DISPLAYABLE_STATUSES = [:default, :error].freeze - INSTALLABLE_STATUSES = [:default, :hidden].freeze + DISPLAYABLE_STATUSES = [:default, :error, :deprecated].freeze + INSTALLABLE_STATUSES = [:default, :hidden, :deprecated].freeze + DETAILED_INFO_STATUSES = [:default, :deprecated].freeze STATUS_MESSAGE_MAX_LENGTH = 255 enum package_type: { @@ -30,7 +31,7 @@ class Packages::Package < ApplicationRecord ml_model: 14 } - enum status: { default: 0, hidden: 1, processing: 2, error: 3, pending_destruction: 4 } + enum status: { default: 0, hidden: 1, processing: 2, error: 3, pending_destruction: 4, deprecated: 5 } belongs_to :project belongs_to :creator, class_name: 'User' @@ -271,6 +272,10 @@ def publish_creation_event ) end + def detailed_info? + DETAILED_INFO_STATUSES.include?(status.to_sym) + end + private # This method will block while another database transaction attempts to insert the same data. diff --git a/app/services/packages/npm/deprecate_package_service.rb b/app/services/packages/npm/deprecate_package_service.rb index fc4a670b5f56fb20e592a433ab99a7af5ca71a07..ad1914087555e926fc0104eb3a3bb3eaf1d015f3 100644 --- a/app/services/packages/npm/deprecate_package_service.rb +++ b/app/services/packages/npm/deprecate_package_service.rb @@ -18,7 +18,13 @@ def execute attributes = relation.preload_npm_metadatum.filter_map { |package| metadatum_attributes(package) } next if attributes.empty? - ::Packages::Npm::Metadatum.upsert_all(attributes) + package_ids = attributes.pluck(:package_id) # rubocop:disable CodeReuse/ActiveRecord, Database/AvoidUsingPluckWithoutLimit -- This is a hash, not an ActiveRecord relation. + + ApplicationRecord.transaction do + ::Packages::Npm::Metadatum.upsert_all(attributes) + ::Packages::Package.id_in(package_ids).update_all(status: package_status) + end + enqueue_metadata_cache_worker = true end @@ -85,6 +91,13 @@ def deprecation_message metadatum['deprecated'] end strong_memoize_attr :deprecation_message + + def package_status + return ::Packages::Package.statuses[:default] if deprecation_message_empty? + + ::Packages::Package.statuses[:deprecated] + end + strong_memoize_attr :package_status end end end diff --git a/db/docs/batched_background_migrations/update_status_for_deprecated_npm_packages.yml b/db/docs/batched_background_migrations/update_status_for_deprecated_npm_packages.yml new file mode 100644 index 0000000000000000000000000000000000000000..4194c8608e0ef744adfc7d1b1cdc37a0a095e8b5 --- /dev/null +++ b/db/docs/batched_background_migrations/update_status_for_deprecated_npm_packages.yml @@ -0,0 +1,8 @@ +--- +migration_job_name: UpdateStatusForDeprecatedNpmPackages +description: Update status to deprecated for deprecated npm packages +feature_category: package_registry +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/174312 +milestone: '17.7' +queued_migration_version: 20241201164238 +finalized_by: # version of the migration that finalized this BBM diff --git a/db/post_migrate/20241129124907_add_index_on_package_json_deprecate_exist_to_packages_npm_metadata.rb b/db/post_migrate/20241129124907_add_index_on_package_json_deprecate_exist_to_packages_npm_metadata.rb new file mode 100644 index 0000000000000000000000000000000000000000..9c1d0007205e882489021ff9fc2f8cb9a9cf892c --- /dev/null +++ b/db/post_migrate/20241129124907_add_index_on_package_json_deprecate_exist_to_packages_npm_metadata.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddIndexOnPackageJsonDeprecateExistToPackagesNpmMetadata < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '17.7' + + INDEX_NAME = 'index_packages_npm_metadata_on_package_json_deprecate_exist' + + def up + add_concurrent_index :packages_npm_metadata, :package_id, where: "(package_json ? 'deprecated')", name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :packages_npm_metadata, INDEX_NAME + end +end diff --git a/db/post_migrate/20241201164238_queue_update_status_for_deprecated_npm_packages.rb b/db/post_migrate/20241201164238_queue_update_status_for_deprecated_npm_packages.rb new file mode 100644 index 0000000000000000000000000000000000000000..67d59db519ef713d1a00742f2637b4eebdb7629b --- /dev/null +++ b/db/post_migrate/20241201164238_queue_update_status_for_deprecated_npm_packages.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class QueueUpdateStatusForDeprecatedNpmPackages < Gitlab::Database::Migration[2.2] + milestone '17.7' + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + MIGRATION = 'UpdateStatusForDeprecatedNpmPackages' + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 200 + SUB_BATCH_SIZE = 100 + + def up + queue_batched_background_migration( + MIGRATION, + :packages_npm_metadata, + :package_id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :packages_npm_metadata, :package_id, []) + end +end diff --git a/db/schema_migrations/20241129124907 b/db/schema_migrations/20241129124907 new file mode 100644 index 0000000000000000000000000000000000000000..ea1bae157d6e876a956597659779d0742b9b7b81 --- /dev/null +++ b/db/schema_migrations/20241129124907 @@ -0,0 +1 @@ +7ebd2deb433fdb61697742d2a122dc356d9c0ba4a4eb219582c9a139ae8dc9b2 \ No newline at end of file diff --git a/db/schema_migrations/20241201164238 b/db/schema_migrations/20241201164238 new file mode 100644 index 0000000000000000000000000000000000000000..0852e7d2fbadf387861d2816e530c8a7cd6f7951 --- /dev/null +++ b/db/schema_migrations/20241201164238 @@ -0,0 +1 @@ +9e4719cd1baf7a689ce931d864d887a30b3a02ca61d15ea88ac05f7b890f1023 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 04ea6be00054daaa9ebd0ff93eebdfad2f3f2d26..9b6e01209fbc5a48033977846b2df1ad1bdb4601 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -32009,6 +32009,8 @@ CREATE UNIQUE INDEX index_packages_npm_metadata_caches_on_object_storage_key ON CREATE INDEX index_packages_npm_metadata_caches_on_project_id_status ON packages_npm_metadata_caches USING btree (project_id, status); +CREATE INDEX index_packages_npm_metadata_on_package_json_deprecate_exist ON packages_npm_metadata USING btree (package_id) WHERE (package_json ? 'deprecated'::text); + CREATE INDEX index_packages_npm_metadata_on_project_id ON packages_npm_metadata USING btree (project_id); CREATE INDEX index_packages_nuget_dl_metadata_on_dependency_link_id ON packages_nuget_dependency_link_metadata USING btree (dependency_link_id); diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 2e3c3c0e5167cf19fe9094a91d0522133ead9347..cf4a96677d0c5a2617e757705858b434b63e72d6 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -40389,6 +40389,7 @@ Values for sorting package. | Value | Description | | ----- | ----------- | | <a id="packagestatusdefault"></a>`DEFAULT` | Packages with a default status. | +| <a id="packagestatusdeprecated"></a>`DEPRECATED` | Packages with a deprecated status. | | <a id="packagestatuserror"></a>`ERROR` | Packages with a error status. | | <a id="packagestatushidden"></a>`HIDDEN` | Packages with a hidden status. | | <a id="packagestatuspending_destruction"></a>`PENDING_DESTRUCTION` | Packages with a pending_destruction status. | diff --git a/doc/api/packages.md b/doc/api/packages.md index 944f1991d32b8537cdacedb8141a5a621d82bf4c..1363259c5c94cd6dbb40d9d4e890df3f4f3fedde 100644 --- a/doc/api/packages.md +++ b/doc/api/packages.md @@ -20,7 +20,7 @@ The API documentation of [GitLab Packages](../administration/packages/index.md). Get a list of project packages. All package types are included in results. When accessed without authentication, only packages of public projects are returned. -By default, packages with `default` and `error` status are returned. Use the `status` parameter to view other +By default, packages with `default`, `deprecated`, and `error` status are returned. Use the `status` parameter to view other packages. ```plaintext @@ -89,7 +89,7 @@ can result in malformed data or broken packages. Get a list of project packages at the group level. When accessed without authentication, only packages of public projects are returned. -By default, packages with `default` and `error` status are returned. Use the `status` parameter to view other +By default, packages with `default`, `deprecated`, and `error` status are returned. Use the `status` parameter to view other packages. ```plaintext @@ -187,7 +187,7 @@ can result in malformed data or broken packages. ## Get a project package -Get a single project package. Only packages with status `default` are returned. +Get a single project package. Only packages with status `default` or `deprecated` are returned. ```plaintext GET /projects/:id/packages/:package_id @@ -262,7 +262,7 @@ Example response: The `_links` object contains the following properties: -- `web_path`: The path which you can visit in GitLab and see the details of the package. Only available if the package has status `default`. +- `web_path`: The path which you can visit in GitLab and see the details of the package. Only available if the package has status `default` or `deprecated`. - `delete_api_path`: The API path to delete the package. Only available if the request user has permission to do so. ## List package files diff --git a/doc/user/packages/npm_registry/index.md b/doc/user/packages/npm_registry/index.md index a9103fa12620c42fa4a781887de9350bd2ce633d..57ad3856d3e06ee47faee82ccab87938a29867df 100644 --- a/doc/user/packages/npm_registry/index.md +++ b/doc/user/packages/npm_registry/index.md @@ -455,6 +455,8 @@ npm deprecate @scope/package@1.0.1 "Only version 1.0.1 is deprecated" npm deprecate @scope/package@"< 1.0.5" "All package versions less than 1.0.5 are deprecated" ``` +When a package is deprecated, its status will be updated to `deprecated`. + ### Remove deprecation warning To remove a package's deprecation warning, specify `""` (an empty string) for the message. For example: @@ -463,6 +465,8 @@ To remove a package's deprecation warning, specify `""` (an empty string) for th npm deprecate @scope/package "" ``` +When a package's deprecation warning is removed, its status will be updated to `default`. + ## Helpful hints ### Install npm packages from other organizations diff --git a/lib/api/entities/package.rb b/lib/api/entities/package.rb index a4452daa034e7fb481069d06d33fe3b35f80db65..bfde770a83177c1b42f9a446cbd6306fb4b69ef6 100644 --- a/lib/api/entities/package.rb +++ b/lib/api/entities/package.rb @@ -28,7 +28,7 @@ class Package < Grape::Entity expose :status, documentation: { type: 'string', example: 'default' } expose :_links do - expose :web_path, if: ->(package) { package.default? } do |package| + expose :web_path, if: ->(package) { package.detailed_info? } do |package| package_path(package) end diff --git a/lib/api/project_packages.rb b/lib/api/project_packages.rb index 233171b1279badefb5216123c97fe53686dbd64e..d9a8613069a56e49965e3f06cbc95de6959677f4 100644 --- a/lib/api/project_packages.rb +++ b/lib/api/project_packages.rb @@ -77,7 +77,7 @@ def package end route_setting :authentication, job_token_allowed: true get ':id/packages/:package_id' do - render_api_error!('Package not found', 404) unless package.default? + render_api_error!('Package not found', 404) unless package.detailed_info? present package, with: ::API::Entities::Package, user: current_user, namespace: user_project.namespace end @@ -103,7 +103,7 @@ def package end route_setting :authentication, job_token_allowed: true get ':id/packages/:package_id/pipelines' do - not_found!('Package not found') unless package.default? + not_found!('Package not found') unless package.detailed_info? params[:pagination] = 'keyset' # keyset is the only available pagination pipelines = paginate_with_strategies( diff --git a/lib/gitlab/background_migration/update_status_for_deprecated_npm_packages.rb b/lib/gitlab/background_migration/update_status_for_deprecated_npm_packages.rb new file mode 100644 index 0000000000000000000000000000000000000000..3d599b706257e233756506822ae033f23bbf36c6 --- /dev/null +++ b/lib/gitlab/background_migration/update_status_for_deprecated_npm_packages.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + class UpdateStatusForDeprecatedNpmPackages < BatchedMigrationJob + operation_name :update_all + scope_to ->(relation) { relation.where("package_json ? 'deprecated'") } + feature_category :package_registry + + STATUS_DEPRECATED = 5 + + module Packages + class Package < ApplicationRecord + self.table_name = 'packages_packages' + end + end + + def perform + each_sub_batch do |sub_batch| + Packages::Package.id_in(sub_batch.select(:package_id)).update_all(status: STATUS_DEPRECATED) + end + end + end + end +end diff --git a/spec/graphql/resolvers/package_details_resolver_spec.rb b/spec/graphql/resolvers/package_details_resolver_spec.rb index 73ceac39eb29fa700baf4dde32df4cc8ab046a2a..5adb056b9663c1d8a6a7e88a2d2b06fe349124ef 100644 --- a/spec/graphql/resolvers/package_details_resolver_spec.rb +++ b/spec/graphql/resolvers/package_details_resolver_spec.rb @@ -22,5 +22,13 @@ it { is_expected.to be_nil } end + + context 'when package has status deprecated' do + before do + package.deprecated! + end + + it { is_expected.to eq(package) } + end end end diff --git a/spec/graphql/types/packages/package_status_enum_spec.rb b/spec/graphql/types/packages/package_status_enum_spec.rb index 2c79c92498bbea131dfea902b9fc54c056e5b273..4eaf7fa3b154c7793a8fdb997cee5ec35730d2ec 100644 --- a/spec/graphql/types/packages/package_status_enum_spec.rb +++ b/spec/graphql/types/packages/package_status_enum_spec.rb @@ -4,6 +4,8 @@ RSpec.describe GitlabSchema.types['PackageStatus'] do it 'exposes all package statuses' do - expect(described_class.values.keys).to contain_exactly(*%w[DEFAULT HIDDEN PROCESSING ERROR PENDING_DESTRUCTION]) + expect(described_class.values.keys).to contain_exactly( + *%w[DEFAULT DEPRECATED HIDDEN PROCESSING ERROR PENDING_DESTRUCTION] + ) end end diff --git a/spec/lib/api/entities/package_spec.rb b/spec/lib/api/entities/package_spec.rb index 1b5f77bce16f20aa0351051c60543aaa2dcac09b..547f1f3be1483f27aed94bd5748cc8bf8100d412 100644 --- a/spec/lib/api/entities/package_spec.rb +++ b/spec/lib/api/entities/package_spec.rb @@ -7,6 +7,12 @@ subject { described_class.new(package).as_json(namespace: package.project.namespace) } + shared_examples 'expose correct web_path in _links' do + it 'exposes correct web_path in _links' do + expect(subject[:_links][:web_path]).to match('/packages/') + end + end + it 'exposes correct attributes' do expect(subject).to include( :id, @@ -21,9 +27,7 @@ ) end - it 'exposes correct web_path in _links' do - expect(subject[:_links][:web_path]).to match('/packages/') - end + it_behaves_like 'expose correct web_path in _links' context 'with a terraform_module' do let(:package) { create(:terraform_module_package) } @@ -33,7 +37,13 @@ end end - context 'when package has no default status' do + context 'when package has status deprecated' do + let(:package) { create(:package, :deprecated) } + + it_behaves_like 'expose correct web_path in _links' + end + + context 'when package has status error' do let(:package) { create(:package, :error) } it 'does not expose web_path in _links' do diff --git a/spec/lib/gitlab/background_migration/update_status_for_deprecated_npm_packages_spec.rb b/spec/lib/gitlab/background_migration/update_status_for_deprecated_npm_packages_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f2d7946200b09243ed486c73dc8ff4cbe9880a63 --- /dev/null +++ b/spec/lib/gitlab/background_migration/update_status_for_deprecated_npm_packages_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::UpdateStatusForDeprecatedNpmPackages, feature_category: :package_registry do + let!(:organization) { table(:organizations).create!(name: 'organization', path: 'organization') } + + let!(:namespace) do + table(:namespaces).create!(name: 'group', path: 'group', type: 'Group', organization_id: organization.id) + end + + let!(:project) do + table(:projects).create!(name: 'project', path: 'project', project_namespace_id: namespace.id, + namespace_id: namespace.id, organization_id: organization.id) + end + + let!(:package_1) do + table(:packages_packages).create!(name: 'test', version: '1.0.0', package_type: 2, project_id: project.id) + .tap do |package| + table(:packages_npm_metadata).create!(package_json: { deprecated: 'not supported' }, package_id: package.id) + end + end + + let!(:package_2) do + table(:packages_packages).create!(name: 'test', version: '2.0.0', package_type: 2, project_id: project.id) + .tap do |package| + table(:packages_npm_metadata).create!(package_json: { deprecated: 'not supported' }, package_id: package.id) + end + end + + let!(:package_3) do + table(:packages_packages).create!(name: 'test', version: '3.0.0', package_type: 2, project_id: project.id) + .tap do |package| + table(:packages_npm_metadata).create!(package_json: {}, package_id: package.id) + end + end + + let!(:starting_id) { table(:packages_npm_metadata).minimum(:package_id) } + let!(:end_id) { table(:packages_npm_metadata).maximum(:package_id) } + + let!(:migration) do + described_class.new( + start_id: starting_id, + end_id: end_id, + batch_table: :packages_npm_metadata, + batch_column: :package_id, + sub_batch_size: 2, + pause_ms: 0, + connection: ::ApplicationRecord.connection + ) + end + + it 'updates the status for deprecated npm packages' do + expect { migration.perform } + .to change { package_1.reload.status }.from(0).to(5) + .and change { package_2.reload.status }.from(0).to(5) + .and not_change { package_3.reload.status } + end +end diff --git a/spec/migrations/20241201164238_queue_update_status_for_deprecated_npm_packages_spec.rb b/spec/migrations/20241201164238_queue_update_status_for_deprecated_npm_packages_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..deedf24b63f2782e98331ae44b487024c6d0b256 --- /dev/null +++ b/spec/migrations/20241201164238_queue_update_status_for_deprecated_npm_packages_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueueUpdateStatusForDeprecatedNpmPackages, migration: :gitlab_main, feature_category: :package_registry do + let!(:batched_migration) { described_class::MIGRATION } + + it 'schedules a new batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).to have_scheduled_batched_migration( + gitlab_schema: :gitlab_main, + table_name: :packages_npm_metadata, + column_name: :package_id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + } + end + end +end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 5fde93519c052ad9a156c1cd35f90ab01dccf295..70ebfc8ceb1dbe19459e71179784d64d3e304d18 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -344,12 +344,14 @@ let_it_be(:hidden_package) { create(:maven_package, :hidden) } let_it_be(:processing_package) { create(:maven_package, :processing) } let_it_be(:error_package) { create(:maven_package, :error) } + let_it_be(:deprecated_package) { create(:maven_package, :deprecated) } describe '.displayable' do subject { described_class.displayable } it 'does not include non-displayable packages', :aggregate_failures do is_expected.to include(error_package) + is_expected.to include(deprecated_package) is_expected.not_to include(hidden_package) is_expected.not_to include(processing_package) end @@ -740,4 +742,23 @@ end end end + + describe '#detailed_info?' do + subject { package.detailed_info? } + + where(:status, :result) do + :default | true + :deprecated | true + :hidden | false + :processing | false + :error | false + :pending_destruction | false + end + + with_them do + let(:package) { build(:maven_package, status: status) } + + it { is_expected.to eq(result) } + end + end end diff --git a/spec/requests/api/graphql/packages/package_spec.rb b/spec/requests/api/graphql/packages/package_spec.rb index 99d18fd716097c97fb688cb6443a394bd84351c1..ebf9748018785cc79b1f1302f19a15b00d617a1e 100644 --- a/spec/requests/api/graphql/packages/package_spec.rb +++ b/spec/requests/api/graphql/packages/package_spec.rb @@ -302,23 +302,39 @@ def run_query(args) end context 'web_path' do - before do - subject + shared_examples 'return web_path correctly' do + it 'returns web_path correctly' do + expect(graphql_data_at(:package, :_links, :web_path)).to eq("/#{project.full_path}/-/packages/#{composer_package.id}") + end end - it 'returns web_path correctly' do - expect(graphql_data_at(:package, :_links, :web_path)).to eq("/#{project.full_path}/-/packages/#{composer_package.id}") - end + context 'with status default' do + before do + subject + end - context 'with terraform module' do - let_it_be(:terraform_package) { create(:terraform_module_package, project: project) } + it_behaves_like 'return web_path correctly' - let(:package_global_id) { global_id_of(terraform_package) } + context 'with terraform module' do + let_it_be(:terraform_package) { create(:terraform_module_package, project: project) } - it 'returns web_path correctly' do - expect(graphql_data_at(:package, :_links, :web_path)).to eq("/#{project.full_path}/-/terraform_module_registry/#{terraform_package.id}") + let(:package_global_id) { global_id_of(terraform_package) } + + it 'returns web_path correctly' do + expect(graphql_data_at(:package, :_links, :web_path)).to eq("/#{project.full_path}/-/terraform_module_registry/#{terraform_package.id}") + end end end + + context 'with status deprecated' do + before do + composer_package.deprecated! + + subject + end + + it_behaves_like 'return web_path correctly' + end end context 'public_package' do diff --git a/spec/services/packages/npm/deprecate_package_service_spec.rb b/spec/services/packages/npm/deprecate_package_service_spec.rb index 65f8cae26d9fb436be26d10cf5a3f77652db152a..97b273d7f70cc2897f581d62ea7202eb61aca4a6 100644 --- a/spec/services/packages/npm/deprecate_package_service_spec.rb +++ b/spec/services/packages/npm/deprecate_package_service_spec.rb @@ -44,15 +44,20 @@ package_json.merge('deprecated' => 'old deprecation message')) end - it 'adds or updates the deprecated field' do + it 'adds or updates the deprecated field and sets status to `deprecated`' do expect { execute } - .to change { package_1.reload.npm_metadatum.package_json['deprecated'] }.to('This version is deprecated') - .and change { package_2.reload.npm_metadatum.package_json['deprecated'] } - .from('old deprecation message').to('This version is deprecated') + .to change { + package_1.reload.npm_metadatum.package_json['deprecated'] + }.to('This version is deprecated') + .and change { + package_2.reload.npm_metadatum.package_json['deprecated'] + }.from('old deprecation message').to('This version is deprecated') + .and change { package_1.status }.from('default').to('deprecated') + .and change { package_2.status }.from('default').to('deprecated') expect(execute).to be_success end - it 'executes 5 queries' do + it 'executes 8 queries' do queries = ActiveRecord::QueryRecorder.new do execute end @@ -61,11 +66,30 @@ # 2. each_batch upper bound # 3. SELECT packages_packages.id, packages_packages.version FROM packages_packages # 4. SELECT packages_npm_metadata.* FROM packages_npm_metadata - # 5. UPDATE packages_npm_metadata SET package_json = - expect(queries.count).to eq(5) + # 5. TRANSACTION + # 6. INSERT INSERT INTO packages_npm_metadata (...) VALUES (...) DO UPDATE SET ... + # 7. UPDATE packages_packages SET status = + # 8. TRANSACTION + expect(queries.count).to eq(8) end it_behaves_like 'enqueues metadata cache worker' + + context 'when the database error has happened' do + let(:exception) { ActiveRecord::ConnectionTimeoutError } + + it 'rolls back previously successfully executed operations' do + allow(::Packages::Package).to receive(:id_in).and_raise(exception) + + expect { execute }.to raise_error(exception) + .and not_change { + package_1.reload.npm_metadatum.package_json['deprecated'] + }.from(nil) + .and not_change { + package_2.reload.npm_metadatum.package_json['deprecated'] + }.from('old deprecation message') + end + end end context 'when passing deprecated as empty string' do @@ -74,12 +98,15 @@ before do package_json = package_1.npm_metadatum.package_json package_1.npm_metadatum.update!(package_json: package_json.merge('deprecated' => 'This version is deprecated')) + package_1.update!(status: :deprecated) end - it 'removes the deprecation warning', :aggregate_failures do + it 'removes the deprecation warning and sets status to `default`', :aggregate_failures do expect { execute } - .to change { package_1.reload.npm_metadatum.package_json['deprecated'] } - .from('This version is deprecated').to(nil) + .to change { + package_1.reload.npm_metadatum.package_json['deprecated'] + }.from('This version is deprecated').to(nil) + .and change { package_1.status }.from('deprecated').to('default') expect(execute).to be_success end diff --git a/spec/support/shared_examples/packages/installable_shared_examples.rb b/spec/support/shared_examples/packages/installable_shared_examples.rb index 9a4e52ff6f76dcdb258ee7562e324c4827377aef..74f3e695a01d8c3cf241b5dcfef66d981d9727dd 100644 --- a/spec/support/shared_examples/packages/installable_shared_examples.rb +++ b/spec/support/shared_examples/packages/installable_shared_examples.rb @@ -6,6 +6,7 @@ let_it_be(:hidden_package) { create(factory_name, :hidden) } let_it_be(:processing_package) { create(factory_name, :processing) } let_it_be(:error_package) { create(factory_name, :error) } + let_it_be(:deprecated_package) { create(factory_name, :deprecated) } subject { described_class.installable } @@ -17,6 +18,7 @@ it 'includes installable packages' do is_expected.to include(default_package) is_expected.to include(hidden_package) + is_expected.to include(deprecated_package) end end end