diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index b38128fe714..6472d3c3454 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -16,7 +16,9 @@ class Projects::TagsController < Projects::ApplicationController # rubocop: disable CodeReuse/ActiveRecord def index begin - tags_params[:sort] = tags_params[:sort].presence || sort_value_recently_updated + tags_params = params + .permit(:search, :sort, :per_page, :page_token, :page) + .with_defaults(sort: sort_value_recently_updated) @sort = tags_params[:sort] @search = tags_params[:search] @@ -101,10 +103,6 @@ class Projects::TagsController < Projects::ApplicationController private - def tags_params - params.permit(:search, :sort, :per_page, :page_token, :page) - end - # TODO: remove this with the release creation moved to it's own form https://gitlab.com/gitlab-org/gitlab/-/issues/214245 def find_evidence_pipeline evidence_pipeline_sha = @project.repository.commit(params[:ref])&.sha diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index ca0ffc19019..c76473c9438 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -63,12 +63,17 @@ class Packages::Package < ApplicationRecord validates :name, format: { with: Gitlab::Regex.package_name_regex }, unless: -> { conan? || generic? || debian? } validates :name, - uniqueness: { scope: %i[project_id version package_type] }, unless: -> { conan? || debian_package? } - validate :unique_debian_package_name, if: :debian_package? + uniqueness: { + scope: %i[project_id version package_type], + conditions: -> { not_pending_destruction} + }, + unless: -> { pending_destruction? || conan? || debian_package? } + validate :unique_debian_package_name, if: :debian_package? validate :valid_conan_package_recipe, if: :conan? validate :valid_composer_global_name, if: :composer? validate :npm_package_already_taken, if: :npm? + validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan? validates :name, format: { with: Gitlab::Regex.generic_package_name_regex }, if: :generic? validates :name, format: { with: Gitlab::Regex.helm_package_regex }, if: :helm? @@ -320,6 +325,7 @@ class Packages::Package < ApplicationRecord recipe_exists = project.packages .conan .includes(:conan_metadatum) + .not_pending_destruction .with_name(name) .with_version(version) .with_conan_channel(conan_metadatum.package_channel) @@ -334,9 +340,14 @@ class Packages::Package < ApplicationRecord # .default_scoped is required here due to a bug in rails that leaks # the scope and adds `self` to the query incorrectly # See https://github.com/rails/rails/pull/35186 - if Packages::Package.default_scoped.composer.with_name(name).where.not(project_id: project_id).exists? - errors.add(:name, 'is already taken by another project') - end + package_exists = Packages::Package.default_scoped + .composer + .not_pending_destruction + .with_name(name) + .where.not(project_id: project_id) + .exists? + + errors.add(:name, 'is already taken by another project') if package_exists end def npm_package_already_taken @@ -361,6 +372,7 @@ class Packages::Package < ApplicationRecord package_exists = debian_publication.distribution.packages .with_name(name) .with_version(version) + .not_pending_destruction .id_not_in(id) .exists? diff --git a/app/models/project.rb b/app/models/project.rb index b1d76deee36..e86d704f3b6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2641,6 +2641,7 @@ class Project < ApplicationRecord Packages::Package.with_name(package_name) .with_version(package_version) .with_package_type(package_type) + .not_pending_destruction .for_projects( root_ancestor.all_projects .id_not_in(id) diff --git a/app/services/packages/create_package_service.rb b/app/services/packages/create_package_service.rb index 7e1b6ecbe51..7565b017a64 100644 --- a/app/services/packages/create_package_service.rb +++ b/app/services/packages/create_package_service.rb @@ -11,6 +11,7 @@ module Packages project .packages .with_package_type(package_type) + .not_pending_destruction .find_or_create_by!(name: name, version: version) do |package| package.status = params[:status] if params[:status] package.creator = package_creator diff --git a/app/services/packages/debian/find_or_create_package_service.rb b/app/services/packages/debian/find_or_create_package_service.rb index 46e06c9f584..3b2be7b6874 100644 --- a/app/services/packages/debian/find_or_create_package_service.rb +++ b/app/services/packages/debian/find_or_create_package_service.rb @@ -11,6 +11,7 @@ module Packages .with_name(params[:name]) .with_version(params[:version]) .with_debian_codename(params[:distribution_name]) + .not_pending_destruction .first package ||= create_package!( diff --git a/app/services/packages/helm/process_file_service.rb b/app/services/packages/helm/process_file_service.rb index 31b357c1616..f53c63d2b01 100644 --- a/app/services/packages/helm/process_file_service.rb +++ b/app/services/packages/helm/process_file_service.rb @@ -68,6 +68,7 @@ module Packages package = project_packages.with_package_type(:helm) .with_name(metadata['name']) .with_version(metadata['version']) + .not_pending_destruction .last package || temp_package end diff --git a/app/services/packages/npm/create_package_service.rb b/app/services/packages/npm/create_package_service.rb index 76a7f3bdc72..b0a5f37cfa3 100644 --- a/app/services/packages/npm/create_package_service.rb +++ b/app/services/packages/npm/create_package_service.rb @@ -33,6 +33,7 @@ module Packages .npm .with_name(name) .with_version(version) + .not_pending_destruction .exists? end diff --git a/app/services/packages/nuget/update_package_from_metadata_service.rb b/app/services/packages/nuget/update_package_from_metadata_service.rb index d1e47ad00a1..5456ad4cad7 100644 --- a/app/services/packages/nuget/update_package_from_metadata_service.rb +++ b/app/services/packages/nuget/update_package_from_metadata_service.rb @@ -98,6 +98,7 @@ module Packages .nuget .with_name(package_name) .with_version(package_version) + .not_pending_destruction .first end end diff --git a/app/services/packages/rubygems/process_gem_service.rb b/app/services/packages/rubygems/process_gem_service.rb index 109c87a0444..c771af28f73 100644 --- a/app/services/packages/rubygems/process_gem_service.rb +++ b/app/services/packages/rubygems/process_gem_service.rb @@ -91,6 +91,7 @@ module Packages .rubygems .with_name(gemspec.name) .with_version(gemspec.version.to_s) + .not_pending_destruction .last package || temp_package end diff --git a/app/services/packages/terraform_module/create_package_service.rb b/app/services/packages/terraform_module/create_package_service.rb index d1bc79089a3..3afecc6c1ca 100644 --- a/app/services/packages/terraform_module/create_package_service.rb +++ b/app/services/packages/terraform_module/create_package_service.rb @@ -29,6 +29,7 @@ module Packages .for_projects(project.root_namespace.all_projects.id_not_in(project.id)) .with_package_type(:terraform_module) .with_name(name) + .not_pending_destruction .exists? end @@ -37,6 +38,7 @@ module Packages .with_package_type(:terraform_module) .with_name(name) .with_version(params[:module_version]) + .not_pending_destruction .exists? end diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 68723097de3..906c4b98f56 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -69,27 +69,24 @@ module QuickActions Gitlab::QuickActions::Extractor.new(self.class.command_definitions) end - # rubocop: disable CodeReuse/ActiveRecord def extract_users(params) - return [] if params.nil? + return [] if params.blank? - args = params.split(' ').uniq - users = extract_references(params, :user) + # We are using the a simple User.by_username query here rather than a ReferenceExtractor + # because the needs here are much simpler: we only deal in usernames, and + # want to also handle bare usernames. The ReferenceExtractor also has + # different behaviour, and will return all group members for groups named + # using a user-style reference, which is not in scope here. + args = params.split(/\s|,/).select(&:present?).uniq - ['and'] + usernames = (args - ['me']).map { _1.delete_prefix('@') } + found = User.by_username(usernames).to_a.select { can?(:read_user_profile, _1) } + found_names = found.map(&:username).to_set + missing = args.reject { |arg| arg == 'me' || found_names.include?(arg.delete_prefix('@')) }.map { "'#{_1}'" } - if users.empty? - users = - if params.strip == 'me' - [current_user] - else - User.where(username: args) - end - end + failed_parse(format(_("Failed to find users for %{missing}"), missing: missing.to_sentence)) if missing.present? - failed_parse(format(_("Failed to find users for '%{params}'"), params: params)) unless users.size == args.size - - users + found + [current_user].select { args.include?('me') } end - # rubocop: enable CodeReuse/ActiveRecord def find_milestones(project, params = {}) group_ids = project.group.self_and_ancestors.select(:id) if project.group @@ -194,6 +191,10 @@ module QuickActions user: current_user ) end + + def can?(ability, object) + Ability.allowed?(current_user, ability, object) + end end end diff --git a/config/initializers/gitlab_experiment.rb b/config/initializers/gitlab_experiment.rb index 3d4077c858d..fdb21d90c28 100644 --- a/config/initializers/gitlab_experiment.rb +++ b/config/initializers/gitlab_experiment.rb @@ -1,15 +1,5 @@ # frozen_string_literal: true -module Gitlab - class Experiment - class Configuration - def self.deprecated(*args, version:, stack: 0) - # do nothing here - end - end - end -end - Gitlab::Experiment.configure do |config| # The base experiment class that will be instantiated when using the # `experiment` DSL, is our ApplicationExperiment. If a custom experiment @@ -81,4 +71,31 @@ Gitlab::Experiment.configure do |config| ) )) end + + # Deprecation warnings resolution for 0.7.0 + # + # We're working through deprecation warnings one by one in: + # https://gitlab.com/gitlab-org/gitlab/-/issues/350944 + # + config.singleton_class.prepend(Module.new do + # Disable all deprecations in non dev/test environments. + # + def deprecated(*args, version:, stack: 0) + super if Gitlab.dev_or_test_env? + end + + # Maintain a list of resolved deprecations to ensure that no new uses appear. + # + # Once a resolved deprecation warning has been added here, any future use will + # raise an exception. + # + ActiveSupport::Deprecation.disallowed_warnings += [ + # 'Gitlab::Experiment 0.8 (instead use `control`)', # don't use `use` + # 'Gitlab::Experiment 0.8 (instead use `candidate`)', # don't use `try` + # 'Gitlab::Experiment 0.8 (instead use `variant(:variant_name)`)', # don't use `try(:variant_name)` + # 'Gitlab::Experiment 0.8 (instead use `assigned(:candidate)`)', # don't use variant(:variant_name) to assign + # 'Gitlab::Experiment 0.8 (instead use `assigned`)', # don't use variant.name to get the assigned variant + # 'Gitlab::Experiment 0.8, instead register variants using:', # don't use public `*_behavior` methods + ] + end) end diff --git a/db/migrate/20220203091304_fix_unique_packages_index_excluding_pending_destruction_status.rb b/db/migrate/20220203091304_fix_unique_packages_index_excluding_pending_destruction_status.rb new file mode 100644 index 00000000000..c30d8de23db --- /dev/null +++ b/db/migrate/20220203091304_fix_unique_packages_index_excluding_pending_destruction_status.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class FixUniquePackagesIndexExcludingPendingDestructionStatus < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + GO_UNIQUE_INDEX_NAME = 'index_packages_on_project_id_name_version_unique_when_golang' + GENERIC_UNIQUE_INDEX_NAME = 'index_packages_on_project_id_name_version_unique_when_generic' + HELM_UNIQUE_INDEX_NAME = 'index_packages_on_project_id_name_version_unique_when_helm' + + NEW_GO_UNIQUE_INDEX_NAME = 'idx_packages_on_project_id_name_version_unique_when_golang' + NEW_GENERIC_UNIQUE_INDEX_NAME = 'idx_packages_on_project_id_name_version_unique_when_generic' + NEW_HELM_UNIQUE_INDEX_NAME = 'idx_packages_on_project_id_name_version_unique_when_helm' + + def up + add_concurrent_index :packages_packages, [:project_id, :name, :version], unique: true, name: NEW_GO_UNIQUE_INDEX_NAME, where: 'package_type = 8 AND status != 4' + add_concurrent_index :packages_packages, [:project_id, :name, :version], unique: true, name: NEW_GENERIC_UNIQUE_INDEX_NAME, where: 'package_type = 7 AND status != 4' + add_concurrent_index :packages_packages, [:project_id, :name, :version], unique: true, name: NEW_HELM_UNIQUE_INDEX_NAME, where: 'package_type = 11 AND status != 4' + + remove_concurrent_index_by_name :packages_packages, GO_UNIQUE_INDEX_NAME + remove_concurrent_index_by_name :packages_packages, GENERIC_UNIQUE_INDEX_NAME + remove_concurrent_index_by_name :packages_packages, HELM_UNIQUE_INDEX_NAME + end + + def down + # no-op + # We can't guarantee that the old index can be recreated since it targets a set larger that the new index. + end +end diff --git a/db/schema_migrations/20220203091304 b/db/schema_migrations/20220203091304 new file mode 100644 index 00000000000..847620db3d2 --- /dev/null +++ b/db/schema_migrations/20220203091304 @@ -0,0 +1 @@ +dac90da1a8c5af69610dd103c8db9cd70a395ad5f9addafb554a853d6acd2a6e \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c1645c2169f..7ecb57145c9 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -25322,6 +25322,12 @@ CREATE INDEX idx_packages_debian_group_component_files_on_architecture_id ON pac CREATE INDEX idx_packages_debian_project_component_files_on_architecture_id ON packages_debian_project_component_files USING btree (architecture_id); +CREATE UNIQUE INDEX idx_packages_on_project_id_name_version_unique_when_generic ON packages_packages USING btree (project_id, name, version) WHERE ((package_type = 7) AND (status <> 4)); + +CREATE UNIQUE INDEX idx_packages_on_project_id_name_version_unique_when_golang ON packages_packages USING btree (project_id, name, version) WHERE ((package_type = 8) AND (status <> 4)); + +CREATE UNIQUE INDEX idx_packages_on_project_id_name_version_unique_when_helm ON packages_packages USING btree (project_id, name, version) WHERE ((package_type = 11) AND (status <> 4)); + CREATE INDEX idx_packages_packages_on_project_id_name_version_package_type ON packages_packages USING btree (project_id, name, version, package_type); CREATE INDEX idx_pkgs_debian_group_distribution_keys_on_distribution_id ON packages_debian_group_distribution_keys USING btree (distribution_id); @@ -27190,12 +27196,6 @@ CREATE INDEX index_packages_maven_metadata_on_path ON packages_maven_metadata US CREATE INDEX index_packages_nuget_dl_metadata_on_dependency_link_id ON packages_nuget_dependency_link_metadata USING btree (dependency_link_id); -CREATE UNIQUE INDEX index_packages_on_project_id_name_version_unique_when_generic ON packages_packages USING btree (project_id, name, version) WHERE (package_type = 7); - -CREATE UNIQUE INDEX index_packages_on_project_id_name_version_unique_when_golang ON packages_packages USING btree (project_id, name, version) WHERE (package_type = 8); - -CREATE UNIQUE INDEX index_packages_on_project_id_name_version_unique_when_helm ON packages_packages USING btree (project_id, name, version) WHERE (package_type = 11); - CREATE INDEX index_packages_package_file_build_infos_on_package_file_id ON packages_package_file_build_infos USING btree (package_file_id); CREATE INDEX index_packages_package_file_build_infos_on_pipeline_id ON packages_package_file_build_infos USING btree (pipeline_id); diff --git a/doc/administration/gitaly/faq.md b/doc/administration/gitaly/faq.md index f79b9555c10..b0a88e8adc9 100644 --- a/doc/administration/gitaly/faq.md +++ b/doc/administration/gitaly/faq.md @@ -23,10 +23,10 @@ Gitaly Cluster and [Geo](../geo/index.md) both provide redundancy. However the r The following table outlines the major differences between Gitaly Cluster and Geo: -| Tool | Nodes | Locations | Latency tolerance | Failover | Consistency | Provides redundancy for | -|:---------------|:---------|:----------|:-------------------|:----------------------------------------------------------------------------|:-----------------------------------------|:------------------------| -| Gitaly Cluster | Multiple | Single | Approximately 1 ms | [Automatic](praefect.md#automatic-failover-and-primary-election-strategies) | [Strong](index.md#strong-consistency) | Data storage in Git | -| Geo | Multiple | Multiple | Up to one minute | [Manual](../geo/disaster_recovery/index.md) | Eventual | Entire GitLab instance | +| Tool | Nodes | Locations | Latency tolerance | Failover | Consistency | Provides redundancy for | +|:---------------|:---------|:----------|:------------------------------------------------------------------------------------------------------|:----------------------------------------------------------------------------|:--------------------------------------|:------------------------| +| Gitaly Cluster | Multiple | Single | [Less than 1 second, ideally single-digit milliseconds](praefect.md#network-latency-and-connectivity) | [Automatic](praefect.md#automatic-failover-and-primary-election-strategies) | [Strong](index.md#strong-consistency) | Data storage in Git | +| Geo | Multiple | Multiple | Up to one minute | [Manual](../geo/disaster_recovery/index.md) | Eventual | Entire GitLab instance | For more information, see: diff --git a/doc/administration/gitaly/praefect.md b/doc/administration/gitaly/praefect.md index 9b55cbb660c..8dc9056c434 100644 --- a/doc/administration/gitaly/praefect.md +++ b/doc/administration/gitaly/praefect.md @@ -39,10 +39,26 @@ NOTE: If not set in GitLab, feature flags are read as false from the console and Praefect uses their default value. The default value depends on the GitLab version. -### Network connectivity +### Network latency and connectivity -Gitaly Cluster [components](index.md#components) need to communicate with each other over many -routes. Your firewall rules must allow the following for Gitaly Cluster to function properly: +Network latency for Gitaly Cluster should ideally be measurable in single-digit milliseconds. Latency is particularly +important for: + +- Gitaly node health checks. Nodes must be able to respond within 1 second. +- Reference transactions that enforce [strong consistency](index.md#strong-consistency). Lower latencies mean Gitaly + nodes can agree on changes faster. + +Achieving acceptable latency between Gitaly nodes: + +- On physical networks generally means high bandwidth, single location connections. +- On the cloud generally means within the same region, including allowing cross availability zone replication. These links + are designed for this type of synchronization. Latency of less than 2ms should be sufficient for Gitaly Cluster. + +If you can't provide low network latencies for replication (for example, between distant locations), consider Geo. For +more information, see [How does Gitaly Cluster compare to Geo](faq.md#how-does-gitaly-cluster-compare-to-geo). + +Gitaly Cluster [components](index.md#components) communicate with each other over many routes. Your firewall rules must +allow the following for Gitaly Cluster to function properly: | From | To | Default port | TLS port | |:-----------------------|:-----------------------|:-------------|:---------| diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb index 031c29e7472..e92547890e8 100644 --- a/lib/api/helpers/packages/conan/api_helpers.rb +++ b/lib/api/helpers/packages/conan/api_helpers.rb @@ -145,6 +145,7 @@ module API .with_conan_username(params[:package_username]) .with_conan_channel(params[:package_channel]) .order_created + .not_pending_destruction .last end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4a3eb306f1f..d35d2b86d8e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -14921,7 +14921,7 @@ msgstr "" msgid "Failed to find import label for Jira import." msgstr "" -msgid "Failed to find users for '%{params}'" +msgid "Failed to find users for %{missing}" msgstr "" msgid "Failed to generate export, please try again later." diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb index dbcdda647c0..f955f9d0248 100644 --- a/spec/controllers/projects/tags_controller_spec.rb +++ b/spec/controllers/projects/tags_controller_spec.rb @@ -17,6 +17,14 @@ RSpec.describe Projects::TagsController do expect(assigns(:tags).map(&:name)).to include('v1.1.0', 'v1.0.0') end + context 'default sort for tags' do + it 'sorts tags by recently updated' do + subject + + expect(assigns(:sort)).to eq('updated_desc') + end + end + context 'when Gitaly is unavailable' do where(:format) do [:html, :atom] diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb index 896cb1e5d55..70ee4bd3c5a 100644 --- a/spec/experiments/application_experiment_spec.rb +++ b/spec/experiments/application_experiment_spec.rb @@ -391,4 +391,29 @@ RSpec.describe ApplicationExperiment, :experiment do end end end + + context "with deprecation warnings" do + before do + Gitlab::Experiment::Configuration.instance_variable_set(:@__dep_versions, nil) # clear the internal memoization + + allow(ActiveSupport::Deprecation).to receive(:new).and_call_original + end + + it "doesn't warn on non dev/test environments" do + allow(Gitlab).to receive(:dev_or_test_env?).and_return(false) + + expect { experiment(:example) { |e| e.use { } } }.not_to raise_error + expect(ActiveSupport::Deprecation).not_to have_received(:new).with(anything, 'Gitlab::Experiment') + end + + it "warns on dev and test environments" do + allow(Gitlab).to receive(:dev_or_test_env?).and_return(true) + + # This will eventually raise an ActiveSupport::Deprecation exception, + # it's ok to change it when that happens. + expect { experiment(:example) { |e| e.use { } } }.not_to raise_error + + expect(ActiveSupport::Deprecation).to have_received(:new).with(anything, 'Gitlab::Experiment') + end + end end diff --git a/spec/features/admin/integrations/instance_integrations_spec.rb b/spec/features/admin/integrations/instance_integrations_spec.rb new file mode 100644 index 00000000000..7b326ec161c --- /dev/null +++ b/spec/features/admin/integrations/instance_integrations_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Instance integrations', :js do + include_context 'instance integration activation' + + it_behaves_like 'integration settings form' do + let(:integrations) { Integration.find_or_initialize_all_non_project_specific(Integration.for_instance) } + + def navigate_to_integration(integration) + visit_instance_integration(integration.title) + end + end +end diff --git a/spec/features/groups/integrations/group_integrations_spec.rb b/spec/features/groups/integrations/group_integrations_spec.rb new file mode 100644 index 00000000000..0d65fa5964b --- /dev/null +++ b/spec/features/groups/integrations/group_integrations_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Group integrations', :js do + include_context 'group integration activation' + + it_behaves_like 'integration settings form' do + let(:integrations) { Integration.find_or_initialize_all_non_project_specific(Integration.for_group(group)) } + + def navigate_to_integration(integration) + visit_group_integration(integration.title) + end + end +end diff --git a/spec/features/projects/integrations/project_integrations_spec.rb b/spec/features/projects/integrations/project_integrations_spec.rb new file mode 100644 index 00000000000..708a5bca8c1 --- /dev/null +++ b/spec/features/projects/integrations/project_integrations_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Project integrations', :js do + include_context 'project integration activation' + + it_behaves_like 'integration settings form' do + let(:integrations) { project.find_or_initialize_integrations } + + def navigate_to_integration(integration) + visit_project_integration(integration.title) + end + end +end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 0cb92f81da2..52ed52de193 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -475,6 +475,15 @@ RSpec.describe Packages::Package, type: :model do end end + shared_examples 'validating both if the first package is pending destruction' do + before do + package.status = :pending_destruction + end + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + context 'following the naming convention' do let(:name) { "@#{group.path}/test" } @@ -503,6 +512,7 @@ RSpec.describe Packages::Package, type: :model do it_behaves_like 'validating the first package' it_behaves_like 'not validating the second package', field_with_error: :name + it_behaves_like 'validating both if the first package is pending destruction' end end @@ -531,6 +541,7 @@ RSpec.describe Packages::Package, type: :model do it_behaves_like 'validating the first package' it_behaves_like 'not validating the second package', field_with_error: :base + it_behaves_like 'validating both if the first package is pending destruction' end end end @@ -563,6 +574,7 @@ RSpec.describe Packages::Package, type: :model do it_behaves_like 'validating the first package' it_behaves_like 'not validating the second package', field_with_error: :name + it_behaves_like 'validating both if the first package is pending destruction' end end @@ -591,6 +603,7 @@ RSpec.describe Packages::Package, type: :model do it_behaves_like 'validating the first package' it_behaves_like 'validating the second package' + it_behaves_like 'validating both if the first package is pending destruction' end end end @@ -598,19 +611,53 @@ RSpec.describe Packages::Package, type: :model do end context "recipe uniqueness for conan packages" do - let!(:package) { create('conan_package') } + let_it_be(:package) { create(:conan_package) } it "will allow a conan package with same project, name, version and package_type" do - new_package = build('conan_package', project: package.project, name: package.name, version: package.version) + new_package = build(:conan_package, project: package.project, name: package.name, version: package.version) new_package.conan_metadatum.package_channel = 'beta' expect(new_package).to be_valid end it "will not allow a conan package with same recipe (name, version, metadatum.package_channel, metadatum.package_username, and package_type)" do - new_package = build('conan_package', project: package.project, name: package.name, version: package.version) + new_package = build(:conan_package, project: package.project, name: package.name, version: package.version) expect(new_package).not_to be_valid expect(new_package.errors.to_a).to include("Package recipe already exists") end + + context 'with pending destruction package' do + let_it_be(:package) { create(:conan_package, :pending_destruction) } + + it 'will allow a conan package with same recipe (name, version, metadatum.package_channel, metadatum.package_username, and package_type)' do + new_package = build(:conan_package, project: package.project, name: package.name, version: package.version) + expect(new_package).to be_valid + end + end + end + + describe '#valid_composer_global_name' do + let_it_be(:package) { create(:composer_package) } + + context 'with different name and different project' do + let(:new_package) { build(:composer_package, name: 'different_name') } + + it { expect(new_package).to be_valid } + end + + context 'with same name and different project' do + let(:new_package) { build(:composer_package, name: package.name) } + + it 'will not validate second package' do + expect(new_package).not_to be_valid + expect(new_package.errors.to_a).to include('Name is already taken by another project') + end + + context 'with pending destruction package' do + let_it_be(:package) { create(:composer_package, :pending_destruction) } + + it { expect(new_package).to be_valid } + end + end end describe "#unique_debian_package_name" do @@ -632,6 +679,16 @@ RSpec.describe Packages::Package, type: :model do new_package = build(:debian_package, project: package.project, name: package.name, version: package.version, published_in: nil) expect(new_package).to be_valid end + + context 'with pending_destruction package' do + let!(:package) { create(:debian_package, :pending_destruction) } + + it "will allow a Debian package with same project, name, version and distribution" do + new_package = build(:debian_package, project: package.project, name: package.name, version: package.version) + new_package.debian_publication.distribution = package.debian_publication.distribution + expect(new_package).to be_valid + end + end end Packages::Package.package_types.keys.without('conan', 'debian').each do |pt| @@ -1267,4 +1324,16 @@ RSpec.describe Packages::Package, type: :model do end end end + + context 'with identical pending destruction package' do + described_class.package_types.keys.each do |package_format| + context "for package format #{package_format}" do + let_it_be(:package_pending_destruction) { create("#{package_format}_package", :pending_destruction) } + + let(:new_package) { build("#{package_format}_package", name: package_pending_destruction.name, version: package_pending_destruction.version, project: package_pending_destruction.project) } + + it { expect(new_package).to be_valid } + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1ea54b3fedb..fbbd1e3b2a2 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -7159,7 +7159,7 @@ RSpec.describe Project, factory_default: :keep do describe '#package_already_taken?' do let_it_be(:namespace) { create(:namespace, path: 'test') } let_it_be(:project) { create(:project, :public, namespace: namespace) } - let_it_be(:package) { create(:npm_package, project: project, name: "@#{namespace.path}/foo", version: '1.2.3') } + let_it_be_with_reload(:package) { create(:npm_package, project: project, name: "@#{namespace.path}/foo", version: '1.2.3') } subject { project.package_already_taken?(package_name, package_version, package_type: :npm) } @@ -7198,6 +7198,23 @@ RSpec.describe Project, factory_default: :keep do expect(result).to be false end end + + context 'with a pending_destruction package' do + before do + package.pending_destruction! + end + + where(:package_name, :package_version, :expected_result) do + '@test/bar' | '1.2.3' | false + '@test/bar' | '5.5.5' | false + '@test/foo' | '1.2.3' | false + '@test/foo' | '5.5.5' | false + end + + with_them do + it { is_expected.to eq expected_result} + end + end end end diff --git a/spec/requests/api/composer_packages_spec.rb b/spec/requests/api/composer_packages_spec.rb index 21b4634ce25..bc30fc3b230 100644 --- a/spec/requests/api/composer_packages_spec.rb +++ b/spec/requests/api/composer_packages_spec.rb @@ -327,6 +327,35 @@ RSpec.describe API::ComposerPackages do it_behaves_like 'rejects Composer access with unknown project id' end + context 'with existing package' do + include_context 'Composer api project access', 'PRIVATE', :developer, true, true + + let_it_be_with_reload(:existing_package) { create(:composer_package, name: package_name, version: '1.2.99', project: project) } + + let(:params) { { tag: 'v1.2.99' } } + + before do + project.add_maintainer(user) + end + + it 'does not create a new package' do + expect { subject } + .to change { project.packages.composer.count }.by(0) + + expect(response).to have_gitlab_http_status(:created) + end + + context 'marked as pending_destruction' do + it 'does create a new package' do + existing_package.pending_destruction! + expect { subject } + .to change { project.packages.composer.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + end + end + end + context 'with no tag or branch params' do let(:headers) { basic_auth_header(user.username, personal_access_token.token) } diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index 1836233594d..e1d8a9f0229 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -426,6 +426,33 @@ RSpec.describe API::GenericPackages do it_behaves_like 'a package tracking event', described_class.name, 'push_package' end + context 'with existing package' do + let_it_be(:package_name) { 'mypackage' } + let_it_be(:package_version) { '1.2.3' } + let_it_be_with_reload(:existing_package) { create(:generic_package, name: package_name, version: package_version, project: project) } + + let(:headers) { workhorse_headers.merge(personal_access_token_header) } + + it 'does not create a new package' do + expect { upload_file(params, headers, package_name: package_name, package_version: package_version) } + .to change { project.packages.generic.count }.by(0) + .and change { Packages::PackageFile.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + end + + context 'marked as pending_destruction' do + it 'does create a new package' do + existing_package.pending_destruction! + expect { upload_file(params, headers, package_name: package_name, package_version: package_version) } + .to change { project.packages.generic.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + end + end + end + it 'rejects request without a file from workhorse' do headers = workhorse_headers.merge(personal_access_token_header) upload_file({}, headers) diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index c17d0600aca..fcd2d56e655 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -230,6 +230,35 @@ RSpec.describe API::PypiPackages do it_behaves_like 'returning response status', :bad_request end + + context 'with existing package' do + let_it_be_with_reload(:existing_package) { create(:pypi_package, name: 'sample-project', version: '1.0.0', project: project) } + + let(:headers) { basic_auth_header(user.username, personal_access_token.token).merge(workhorse_headers) } + + before do + project.add_maintainer(user) + end + + it 'does not create a new package', :aggregate_failures do + expect { subject } + .to change { project.packages.pypi.count }.by(0) + .and change { Packages::PackageFile.count }.by(1) + .and change { Packages::Pypi::Metadatum.count }.by(0) + expect(response).to have_gitlab_http_status(:created) + end + + context 'marked as pending_destruction' do + it 'does create a new package', :aggregate_failures do + existing_package.pending_destruction! + expect { subject } + .to change { project.packages.pypi.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) + .and change { Packages::Pypi::Metadatum.count }.by(1) + expect(response).to have_gitlab_http_status(:created) + end + end + end end context 'file download endpoint' do diff --git a/spec/requests/api/terraform/modules/v1/packages_spec.rb b/spec/requests/api/terraform/modules/v1/packages_spec.rb index 1c77a0bea36..7d86244cb1b 100644 --- a/spec/requests/api/terraform/modules/v1/packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/packages_spec.rb @@ -401,6 +401,28 @@ RSpec.describe API::Terraform::Modules::V1::Packages do .and change { Packages::PackageFile.count }.by(0) expect(response).to have_gitlab_http_status(:error) end + + context 'with an existing package' do + let_it_be_with_reload(:existing_package) { create(:terraform_module_package, name: 'mymodule/mysystem', version: '1.0.0', project: project) } + + it 'does not create a new package' do + expect { subject } + .to change { project.packages.count }.by(0) + .and change { Packages::PackageFile.count }.by(0) + expect(response).to have_gitlab_http_status(:forbidden) + end + + context 'marked as pending_destruction' do + it 'does create a new package' do + existing_package.pending_destruction! + + expect { subject } + .to change { project.packages.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) + expect(response).to have_gitlab_http_status(:created) + end + end + end end end end diff --git a/spec/services/packages/composer/create_package_service_spec.rb b/spec/services/packages/composer/create_package_service_spec.rb index 593777faa55..b04a6c8382f 100644 --- a/spec/services/packages/composer/create_package_service_spec.rb +++ b/spec/services/packages/composer/create_package_service_spec.rb @@ -88,13 +88,23 @@ RSpec.describe Packages::Composer::CreatePackageService do end context 'belonging to another project' do - let(:other_project) { create(:project) } + let(:other_project) { create(:project)} let!(:other_package) { create(:composer_package, name: package_name, version: 'dev-master', project: other_project) } it 'fails with an error' do expect { subject } .to raise_error(/is already taken/) end + + context 'with pending_destruction package' do + let!(:other_package) { create(:composer_package, :pending_destruction, name: package_name, version: 'dev-master', project: other_project) } + + it 'creates the package' do + expect { subject } + .to change { Packages::Package.composer.count }.by(1) + .and change { Packages::Composer::Metadatum.count }.by(1) + end + end end context 'same name but of different type' do diff --git a/spec/services/packages/debian/find_or_create_package_service_spec.rb b/spec/services/packages/debian/find_or_create_package_service_spec.rb index f06f86b0146..84a0e1465e8 100644 --- a/spec/services/packages/debian/find_or_create_package_service_spec.rb +++ b/spec/services/packages/debian/find_or_create_package_service_spec.rb @@ -32,8 +32,6 @@ RSpec.describe Packages::Debian::FindOrCreatePackageService do end context 'run twice' do - let(:subject2) { service.execute } - let(:package2) { service.execute.payload[:package] } it 'returns the same object' do @@ -42,6 +40,16 @@ RSpec.describe Packages::Debian::FindOrCreatePackageService do expect(package2.id).to eq(package.id) end + + context 'with package marked as pending_destruction' do + it 'creates a new package' do + expect { subject }.to change { ::Packages::Package.count }.by(1) + package.pending_destruction! + expect { package2 }.to change { ::Packages::Package.count }.by(1) + + expect(package2.id).not_to eq(package.id) + end + end end context 'with non-existing distribution' do diff --git a/spec/services/packages/debian/process_changes_service_spec.rb b/spec/services/packages/debian/process_changes_service_spec.rb index 8e5e36cdbcb..52bcddb6f5e 100644 --- a/spec/services/packages/debian/process_changes_service_spec.rb +++ b/spec/services/packages/debian/process_changes_service_spec.rb @@ -25,6 +25,30 @@ RSpec.describe Packages::Debian::ProcessChangesService do expect(created_package.version).to eq '1.2.3~alpha2' expect(created_package.creator).to eq user end + + context 'with existing package' do + let_it_be_with_reload(:existing_package) { create(:debian_package, name: 'sample', version: '1.2.3~alpha2', project: distribution.project) } + + before do + existing_package.update!(debian_distribution: distribution) + end + + it 'does not create a package' do + expect { subject.execute } + .to not_change { Packages::Package.count } + .and not_change { Packages::PackageFile.count } + end + + context 'marked as pending_destruction' do + it 'creates a package' do + existing_package.pending_destruction! + + expect { subject.execute } + .to change { Packages::Package.count }.by(1) + .and not_change { Packages::PackageFile.count } + end + end + end end context 'with invalid package file' do diff --git a/spec/services/packages/generic/find_or_create_package_service_spec.rb b/spec/services/packages/generic/find_or_create_package_service_spec.rb index a045cb36418..10ec917bc99 100644 --- a/spec/services/packages/generic/find_or_create_package_service_spec.rb +++ b/spec/services/packages/generic/find_or_create_package_service_spec.rb @@ -83,6 +83,23 @@ RSpec.describe Packages::Generic::FindOrCreatePackageService do expect(package.reload.original_build_info.pipeline).to eq(pipeline) end end + + context 'when a pending_destruction package exists', :aggregate_failures do + let!(:package) { project.packages.generic.create!(params.merge(status: :pending_destruction)) } + + it 'creates a new package' do + service = described_class.new(project, user, params) + + expect { service.execute }.to change { project.packages.generic.count }.by(1) + + package = project.packages.generic.last + + expect(package.creator).to eq(user) + expect(package.name).to eq('mypackage') + expect(package.version).to eq('0.0.1') + expect(package.original_build_info).to be_nil + end + end end end end diff --git a/spec/services/packages/helm/process_file_service_spec.rb b/spec/services/packages/helm/process_file_service_spec.rb index 2e98590a4f4..d22c1de2335 100644 --- a/spec/services/packages/helm/process_file_service_spec.rb +++ b/spec/services/packages/helm/process_file_service_spec.rb @@ -53,6 +53,19 @@ RSpec.describe Packages::Helm::ProcessFileService do expect(package_file.helm_file_metadatum.channel).to eq(channel) expect(package_file.helm_file_metadatum.metadata).to eq(expected) end + + context 'marked as pending_destruction' do + before do + existing_package.pending_destruction! + end + + it 'reuses the processing package' do + expect { execute } + .to not_change { Packages::Package.count } + .and not_change { Packages::PackageFile.count } + .and change { Packages::Helm::FileMetadatum.count }.by(1) + end + end end context 'with a valid file' do diff --git a/spec/services/packages/maven/find_or_create_package_service_spec.rb b/spec/services/packages/maven/find_or_create_package_service_spec.rb index 59f5677f526..cca074e2fa6 100644 --- a/spec/services/packages/maven/find_or_create_package_service_spec.rb +++ b/spec/services/packages/maven/find_or_create_package_service_spec.rb @@ -81,6 +81,14 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do let!(:existing_package) { create(:maven_package, name: path, version: version, project: project) } it_behaves_like 'reuse existing package' + + context 'marked as pending_destruction' do + before do + existing_package.pending_destruction! + end + + it_behaves_like 'create package' + end end context 'without existing package' do diff --git a/spec/services/packages/npm/create_package_service_spec.rb b/spec/services/packages/npm/create_package_service_spec.rb index a5b36527565..5b41055397b 100644 --- a/spec/services/packages/npm/create_package_service_spec.rb +++ b/spec/services/packages/npm/create_package_service_spec.rb @@ -112,6 +112,20 @@ RSpec.describe Packages::Npm::CreatePackageService do it { expect(subject[:http_status]).to eq 403 } it { expect(subject[:message]).to be 'Package already exists.' } + + context 'marked as pending_destruction' do + before do + existing_package.pending_destruction! + end + + it 'creates a new package' do + expect { subject } + .to change { Packages::Package.count }.by(1) + .and change { Packages::Package.npm.count }.by(1) + .and change { Packages::Tag.count }.by(1) + .and change { Packages::Npm::Metadatum.count }.by(1) + end + end end describe 'max file size validation' do diff --git a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb index d682ee12ed5..6a4dbeb10dc 100644 --- a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb +++ b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb @@ -106,6 +106,20 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_ it_behaves_like 'taking the lease' it_behaves_like 'not updating the package if the lease is taken' + + context 'marked as pending_destruction' do + before do + existing_package.pending_destruction! + end + + it 'reuses the processing package', :aggregate_failures do + expect { subject } + .to not_change { ::Packages::Package.count } + .and change { Packages::Dependency.count }.by(1) + .and change { Packages::DependencyLink.count }.by(1) + .and change { ::Packages::Nuget::Metadatum.count }.by(0) + end + end end context 'with a nuspec file with metadata' do diff --git a/spec/services/packages/pypi/create_package_service_spec.rb b/spec/services/packages/pypi/create_package_service_spec.rb index a932cf73eb7..3d0c10724d4 100644 --- a/spec/services/packages/pypi/create_package_service_spec.rb +++ b/spec/services/packages/pypi/create_package_service_spec.rb @@ -72,6 +72,27 @@ RSpec.describe Packages::Pypi::CreatePackageService do .and change { Packages::PackageFile.count }.by(0) .and raise_error(/File name has already been taken/) end + + context 'with a pending_destruction package', :aggregate_failures do + before do + Packages::Package.pypi.last.pending_destruction! + end + + it 'creates a new package' do + expect { subject } + .to change { Packages::Package.pypi.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) + + expect(created_package.name).to eq 'foo' + expect(created_package.version).to eq '1.0' + + expect(created_package.pypi_metadatum.required_python).to eq '>=2.7' + expect(created_package.package_files.size).to eq 1 + expect(created_package.package_files.first.file_name).to eq 'foo.tgz' + expect(created_package.package_files.first.file_sha256).to eq 'abc' + expect(created_package.package_files.first.file_md5).to eq 'def' + end + end end context 'without an existing file' do diff --git a/spec/services/packages/rubygems/process_gem_service_spec.rb b/spec/services/packages/rubygems/process_gem_service_spec.rb index 64deb39c6d8..caff338ef53 100644 --- a/spec/services/packages/rubygems/process_gem_service_spec.rb +++ b/spec/services/packages/rubygems/process_gem_service_spec.rb @@ -85,6 +85,28 @@ RSpec.describe Packages::Rubygems::ProcessGemService do end end + context 'when the package already exists marked as pending_destruction' do + let_it_be_with_reload(:existing_package) { create(:rubygems_package, name: 'package', version: '0.0.1', project: package.project) } + + let(:sub_service) { double } + + before do + expect(Packages::Rubygems::MetadataExtractionService).to receive(:new).with(package, gemspec).and_return(sub_service) + expect(Packages::Rubygems::CreateGemspecService).to receive(:new).with(package, gemspec).and_return(sub_service) + expect(Packages::Rubygems::CreateDependenciesService).to receive(:new).with(package, gemspec).and_return(sub_service) + + expect(sub_service).to receive(:execute).exactly(3).times.and_return(true) + + existing_package.pending_destruction! + end + + it 'reuses the processing package' do + expect { subject } + .to not_change { package.project.packages.count } + .and not_change { existing_package.package_files.count } + end + end + context 'sub-service failure' do before do expect(Packages::Rubygems::MetadataExtractionService).to receive(:new).with(package, gemspec).and_raise(::Packages::Rubygems::ProcessGemService::ExtractionError.new('failure')) diff --git a/spec/services/packages/terraform_module/create_package_service_spec.rb b/spec/services/packages/terraform_module/create_package_service_spec.rb index e172aa726fd..f73b5682835 100644 --- a/spec/services/packages/terraform_module/create_package_service_spec.rb +++ b/spec/services/packages/terraform_module/create_package_service_spec.rb @@ -6,8 +6,6 @@ RSpec.describe Packages::TerraformModule::CreatePackageService do let_it_be(:project) { create(:project, namespace: namespace) } let_it_be(:user) { create(:user) } let_it_be(:sha256) { '440e5e148a25331bbd7991575f7d54933c0ebf6cc735a18ee5066ac1381bb590' } - let_it_be(:temp_file) { Tempfile.new('test') } - let_it_be(:file) { UploadedFile.new(temp_file.path, sha256: sha256) } let(:overrides) { {} } @@ -16,7 +14,7 @@ RSpec.describe Packages::TerraformModule::CreatePackageService do module_name: 'foo', module_system: 'bar', module_version: '1.0.1', - file: file, + file: UploadedFile.new(Tempfile.new('test').path, sha256: sha256), file_name: 'foo-bar-1.0.1.tgz' }.merge(overrides) end @@ -24,7 +22,7 @@ RSpec.describe Packages::TerraformModule::CreatePackageService do subject { described_class.new(project, user, params).execute } describe '#execute' do - context 'valid package' do + shared_examples 'creating a package' do it 'creates a package' do expect { subject } .to change { ::Packages::Package.count }.by(1) @@ -32,12 +30,24 @@ RSpec.describe Packages::TerraformModule::CreatePackageService do end end + context 'valid package' do + it_behaves_like 'creating a package' + end + context 'package already exists elsewhere' do let(:project2) { create(:project, namespace: namespace) } let!(:existing_package) { create(:terraform_module_package, project: project2, name: 'foo/bar', version: '1.0.0') } it { expect(subject[:http_status]).to eq 403 } it { expect(subject[:message]).to be 'Access Denied' } + + context 'marked as pending_destruction' do + before do + existing_package.pending_destruction! + end + + it_behaves_like 'creating a package' + end end context 'version already exists' do @@ -45,6 +55,14 @@ RSpec.describe Packages::TerraformModule::CreatePackageService do it { expect(subject[:http_status]).to eq 403 } it { expect(subject[:message]).to be 'Package version already exists.' } + + context 'marked as pending_destruction' do + before do + existing_version.pending_destruction! + end + + it_behaves_like 'creating a package' + end end context 'with empty version' do diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index bd0896bf0a8..ca3ae437540 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -11,13 +11,14 @@ RSpec.describe QuickActions::InterpretService do let_it_be(:developer2) { create(:user) } let_it_be(:developer3) { create(:user) } let_it_be_with_reload(:issue) { create(:issue, project: project) } - let(:milestone) { create(:milestone, project: project, title: '9.10') } - let(:commit) { create(:commit, project: project) } let_it_be(:inprogress) { create(:label, project: project, title: 'In Progress') } let_it_be(:helmchart) { create(:label, project: project, title: 'Helm Chart Registry') } let_it_be(:bug) { create(:label, project: project, title: 'Bug') } - let(:service) { described_class.new(project, developer) } + let(:milestone) { create(:milestone, project: project, title: '9.10') } + let(:commit) { create(:commit, project: project) } + + subject(:service) { described_class.new(project, developer) } before_all do public_project.add_developer(developer) @@ -970,6 +971,32 @@ RSpec.describe QuickActions::InterpretService do it_behaves_like 'assign_reviewer command' end + context 'with a private user' do + let(:ref) { create(:user, :unconfirmed).to_reference } + let(:content) { "/assign_reviewer #{ref}" } + + it_behaves_like 'failed command', 'a parse error' do + let(:match_msg) { eq "Could not apply assign_reviewer command. Failed to find users for '#{ref}'." } + end + end + + context 'with a private user, bare username' do + let(:ref) { create(:user, :unconfirmed).username } + let(:content) { "/assign_reviewer #{ref}" } + + it_behaves_like 'failed command', 'a parse error' do + let(:match_msg) { eq "Could not apply assign_reviewer command. Failed to find users for '#{ref}'." } + end + end + + context 'with @all' do + let(:content) { "/assign_reviewer @all" } + + it_behaves_like 'failed command', 'a parse error' do + let(:match_msg) { eq "Could not apply assign_reviewer command. Failed to find users for '@all'." } + end + end + context 'with an incorrect user' do let(:content) { '/assign_reviewer @abcd1234' } @@ -997,11 +1024,10 @@ RSpec.describe QuickActions::InterpretService do end context 'with extra text' do - let(:arg) { "@#{developer.username} do it!" } - let(:content) { "/assign_reviewer #{arg}" } + let(:content) { "/assign_reviewer #{developer.to_reference} do it!" } it_behaves_like 'failed command', 'a parse error' do - let(:match_msg) { eq "Could not apply assign_reviewer command. Failed to find users for '#{arg}'." } + let(:match_msg) { eq "Could not apply assign_reviewer command. Failed to find users for 'do' and 'it!'." } end end end @@ -2451,7 +2477,8 @@ RSpec.describe QuickActions::InterpretService do it 'tells us why we cannot do that' do _, explanations = service.explain(content, merge_request) - expect(explanations).to eq ["Problem with assign command: Failed to find users for '#{arg}'."] + expect(explanations) + .to contain_exactly "Problem with assign command: Failed to find users for 'to', 'this', and 'issue'." end end end diff --git a/spec/support/gitlab_experiment.rb b/spec/support/gitlab_experiment.rb index cb5a4385e6b..823aab0436e 100644 --- a/spec/support/gitlab_experiment.rb +++ b/spec/support/gitlab_experiment.rb @@ -26,7 +26,3 @@ RSpec.configure do |config| stub_snowplow end end - -# Once you've resolved a given deprecation, you can disallow it here, which -# will raise an exception if it's used anywhere. -ActiveSupport::Deprecation.disallowed_warnings << "`experiment_group?` is deprecated" diff --git a/spec/features/projects/integrations/integrations_settings_form_spec.rb b/spec/support/shared_examples/integrations/integration_settings_form.rb similarity index 72% rename from spec/features/projects/integrations/integrations_settings_form_spec.rb rename to spec/support/shared_examples/integrations/integration_settings_form.rb index 07b775a974f..d0bb40e43ee 100644 --- a/spec/features/projects/integrations/integrations_settings_form_spec.rb +++ b/spec/support/shared_examples/integrations/integration_settings_form.rb @@ -1,24 +1,14 @@ # frozen_string_literal: true -require 'spec_helper' - -RSpec.describe 'Integrations settings form', :js do +RSpec.shared_examples 'integration settings form' do include IntegrationsHelper - include_context 'project integration activation' - - # Github integration is EE, so let's remove it here. - integration_names = Integration.available_integration_names - %w[github] - integrations = integration_names.map do |name| - Integration.integration_name_to_model(name).new - end - # Note: these specs don't validate channel fields # which are present on a few integrations - integrations.each do |integration| - it "shows on #{integration.title}" do - visit_project_integration(integration.title) + it 'displays all the integrations' do + aggregate_failures do + integrations.each do |integration| + navigate_to_integration(integration) - aggregate_failures do page.within('form.integration-settings-form') do expect(page).to have_field('Active', type: 'checkbox', wait: 0), "#{integration.title} active field not present" @@ -43,6 +33,8 @@ RSpec.describe 'Integrations settings form', :js do end end + private + def normalize_title(title, integration) return 'Merge request' if integration.is_a?(Integrations::Jira) && title == 'merge_request' diff --git a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb index 71f3a0235be..b30c4186f0d 100644 --- a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb @@ -777,6 +777,32 @@ RSpec.shared_examples 'uploads a package file' do subject end + + context 'with existing package' do + let!(:existing_package) { create(:conan_package, name: 'foo', version: 'bar', project: project) } + + before do + existing_package.conan_metadatum.update!(package_username: project.full_path.tr('/', '+'), package_channel: 'baz') + end + + it 'does not create a new package' do + expect { subject } + .to not_change { project.packages.count } + .and not_change { Packages::Conan::Metadatum.count } + .and change { Packages::PackageFile.count }.by(1) + end + + context 'marked as pending_destruction' do + it 'does not create a new package' do + existing_package.pending_destruction! + + expect { subject } + .to change { project.packages.count }.by(1) + .and change { Packages::Conan::Metadatum.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) + end + end + end end end diff --git a/spec/workers/packages/go/sync_packages_worker_spec.rb b/spec/workers/packages/go/sync_packages_worker_spec.rb index ad1a85b26e4..5eeef1f7c08 100644 --- a/spec/workers/packages/go/sync_packages_worker_spec.rb +++ b/spec/workers/packages/go/sync_packages_worker_spec.rb @@ -57,6 +57,18 @@ RSpec.describe Packages::Go::SyncPackagesWorker, type: :worker do it_behaves_like 'it creates a package', '', 'v1.0.3' it_behaves_like 'it creates a package', 'mod', 'v1.0.3' it_behaves_like 'it creates a package', 'v2', 'v2.0.0' + + context 'marked as pending_destruction' do + before do + project.packages.each(&:pending_destruction!) + end + + it_behaves_like 'it creates a package', '', 'v1.0.1' + it_behaves_like 'it creates a package', '', 'v1.0.2' + it_behaves_like 'it creates a package', '', 'v1.0.3' + it_behaves_like 'it creates a package', 'mod', 'v1.0.3' + it_behaves_like 'it creates a package', 'v2', 'v2.0.0' + end end context 'with a package that exceeds project limits' do