diff --git a/app/models/packages/conan/metadatum.rb b/app/models/packages/conan/metadatum.rb index f489b1bd2581e9f24f2ae239874338c4fe447205..ce784f82d2fb5ede83f6b83f5740fe4ce240a0d9 100644 --- a/app/models/packages/conan/metadatum.rb +++ b/app/models/packages/conan/metadatum.rb @@ -12,7 +12,7 @@ class Packages::Conan::Metadatum < ApplicationRecord presence: true, format: { with: Gitlab::Regex.conan_recipe_user_channel_regex } - validate :username_channel_none_values + validate :username_required_when_channel_set def recipe "#{package.name}/#{package.version}@#{package_username}/#{package_channel}" @@ -30,20 +30,12 @@ def self.full_path_from(package_username:) package_username.tr('+', '/') end - def self.validate_username_and_channel(username, channel) - return if (username != NONE_VALUE && channel != NONE_VALUE) || - (username == NONE_VALUE && channel == NONE_VALUE) - - none_field = username == NONE_VALUE ? :username : :channel - - yield(none_field) - end - private - def username_channel_none_values - self.class.validate_username_and_channel(package_username, package_channel) do |none_field| - errors.add(:"package_#{none_field}", _("can't be solely blank")) - end + def username_required_when_channel_set + return unless package_channel.present? && package_channel != NONE_VALUE + return unless package_username == NONE_VALUE + + errors.add(:package_username, 'must be set when package_channel is specified') end end diff --git a/lib/api/conan/v2/project_packages.rb b/lib/api/conan/v2/project_packages.rb index 03b2b575be100574f96bfc8d5f02175f17d31198..0bd1a03a0257f6f446090c8cdfd69e890a57c1a0 100644 --- a/lib/api/conan/v2/project_packages.rb +++ b/lib/api/conan/v2/project_packages.rb @@ -59,6 +59,7 @@ def package_revision resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do namespace ':id/packages/conan/v2' do include ::API::Concerns::Packages::Conan::SharedEndpoints + params do requires :package_name, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package name', documentation: { example: 'my-package' } @@ -71,10 +72,6 @@ def package_revision end namespace 'conans/:package_name/:package_version/:package_username/:package_channel', requirements: PACKAGE_REQUIREMENTS do - after_validation do - check_username_channel - end - namespace 'latest' do desc 'Get the latest recipe revision' do detail 'This feature was introduced in GitLab 17.11' @@ -275,8 +272,6 @@ def package_revision allow_public_access_for_enabled_project_features: :package_registry get 'search', urgency: :low do - check_username_channel - authorize_read_package!(project) not_found!('Package') unless package not_found!('Revision') unless recipe_revision.present? diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb index 65b0cdaf85d45c7203b9196c367add9e84530622..6f65dca6596794352a8c174d8aef5201d831a66d 100644 --- a/lib/api/helpers/packages/conan/api_helpers.rb +++ b/lib/api/helpers/packages/conan/api_helpers.rb @@ -7,19 +7,14 @@ module Conan module ApiHelpers include Gitlab::Utils::StrongMemoize + # TODO: rename to check_username def check_username_channel - username = declared(params)[:package_username] - channel = declared(params)[:package_channel] - - if username == ::Packages::Conan::Metadatum::NONE_VALUE && package_scope == :instance + if declared(params)[:package_username] == ::Packages::Conan::Metadatum::NONE_VALUE && + package_scope == :instance # at the instance level, username must not be empty (naming convention) # don't try to process the empty username and eagerly return not found. not_found! end - - ::Packages::Conan::Metadatum.validate_username_and_channel(username, channel) do |none_field| - bad_request!("#{none_field} can't be solely blank") - end end def present_download_urls(entity) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6e89321bb73e20ceb628cde8ee82908dbca87760..8b5eadb03745173426ad031df759705593a2b69e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -79967,9 +79967,6 @@ msgstr "" msgid "can't be nil" msgstr "" -msgid "can't be solely blank" -msgstr "" - msgid "can't be specified if a merge request was already provided" msgstr "" diff --git a/spec/models/packages/conan/metadatum_spec.rb b/spec/models/packages/conan/metadatum_spec.rb index f0a3ef318ff2aad5285bfca3573a7c366364e7b0..ea518448f712fae278e4892efd5a4f637c281265 100644 --- a/spec/models/packages/conan/metadatum_spec.rb +++ b/spec/models/packages/conan/metadatum_spec.rb @@ -47,16 +47,14 @@ it { is_expected.not_to allow_value("my@channel").for(:package_channel) } end - describe '#username_channel_none_values' do - let_it_be(:package) { create(:conan_package) } - - let(:metadatum) { package.conan_metadatum } + describe '#username_required_with_channel_set' do + let_it_be(:metadatum) { create(:conan_metadatum) } subject { metadatum.valid? } where(:username, :channel, :valid) do 'username' | 'channel' | true - 'username' | '_' | false + 'username' | '_' | true '_' | 'channel' | false '_' | '_' | true end @@ -103,27 +101,4 @@ expect(described_class.full_path_from(package_username: username)).to eq('foo/bar/baz-buz') end end - - describe '.validate_username_and_channel' do - where(:username, :channel, :error_field) do - 'username' | 'channel' | nil - 'username' | '_' | :channel - '_' | 'channel' | :username - '_' | '_' | nil - end - - with_them do - if params[:error_field] - it 'yields the block when there is an error' do - described_class.validate_username_and_channel(username, channel) do |none_field| - expect(none_field).to eq(error_field) - end - end - else - it 'does not yield the block when there is no error' do - expect { |b| described_class.validate_username_and_channel(username, channel, &b) }.not_to yield_control - end - end - end - end end 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 d1b9dfa951498ab3c16ba0adc517ea4a952fa733..6717ece3bf285e6b95fef2124c93c99373486332 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 @@ -348,7 +348,7 @@ where(:username, :channel, :status) do 'username' | 'channel' | success_status - 'username' | '_' | :bad_request + 'username' | '_' | success_status '_' | 'channel' | :bad_request_or_not_found '_' | '_' | :success_status_or_not_found end