summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--.rubocop.yml4
-rw-r--r--app/assets/javascripts/blob/blob_file_dropzone.js4
-rw-r--r--app/controllers/groups/group_members_controller.rb2
-rw-r--r--app/controllers/projects/issues_controller.rb2
-rw-r--r--app/finders/group_members_finder.rb9
-rw-r--r--app/serializers/diffs_entity.rb34
-rw-r--r--app/services/concerns/deploy_token_methods.rb6
-rw-r--r--app/services/groups/deploy_tokens/destroy_service.rb13
-rw-r--r--app/services/projects/deploy_tokens/destroy_service.rb13
-rw-r--r--app/services/resource_events/synthetic_milestone_notes_builder_service.rb6
-rw-r--r--app/workers/gitlab/github_import/advance_stage_worker.rb4
-rw-r--r--app/workers/gitlab/import/advance_stage.rb2
-rw-r--r--app/workers/gitlab/jira_import/advance_stage_worker.rb4
-rw-r--r--changelogs/unreleased/212523-delete-deploy-tokens.yml5
-rw-r--r--changelogs/unreleased/212985-remove-ff-check-for-synthetic-milestone-notes-creation.yml5
-rw-r--r--changelogs/unreleased/42639-remove-custom-metrics-license-check.yml5
-rw-r--r--changelogs/unreleased/merge-request-typo.yml5
-rw-r--r--changelogs/unreleased/ph-212642-userUploadsFile.yml5
-rw-r--r--changelogs/unreleased/refactor-ability-spec.yml5
-rw-r--r--changelogs/unreleased/refactor-build-spec.yml5
-rw-r--r--changelogs/unreleased/separate-validator-files.yml5
-rw-r--r--config/initializers/grape_validators.rb9
-rw-r--r--doc/development/migration_style_guide.md1
-rw-r--r--lib/api/deploy_tokens.rb18
-rw-r--r--lib/api/entities/commit_with_link.rb8
-rw-r--r--lib/api/environments.rb1
-rw-r--r--lib/api/helpers.rb6
-rw-r--r--lib/api/helpers/custom_validators.rb97
-rw-r--r--lib/api/helpers/merge_requests_helpers.rb1
-rw-r--r--lib/api/issues.rb2
-rw-r--r--lib/api/validations/check_assignees_count.rb34
-rw-r--r--lib/api/validations/validators/absence.rb15
-rw-r--r--lib/api/validations/validators/array_none_any.rb19
-rw-r--r--lib/api/validations/validators/check_assignees_count.rb36
-rw-r--r--lib/api/validations/validators/file_path.rb18
-rw-r--r--lib/api/validations/validators/git_ref.rb36
-rw-r--r--lib/api/validations/validators/git_sha.rb18
-rw-r--r--lib/api/validations/validators/integer_none_any.rb19
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb20
-rw-r--r--spec/features/projects/files/user_uploads_files_spec.rb25
-rw-r--r--spec/finders/group_members_finder_spec.rb10
-rw-r--r--spec/frontend/diffs/components/commit_item_spec.js25
-rw-r--r--spec/lib/api/helpers/custom_validators_spec.rb193
-rw-r--r--spec/lib/api/validations/validators/absence_spec.rb23
-rw-r--r--spec/lib/api/validations/validators/array_none_any_spec.rb28
-rw-r--r--spec/lib/api/validations/validators/file_path_spec.rb36
-rw-r--r--spec/lib/api/validations/validators/git_ref_spec.rb46
-rw-r--r--spec/lib/api/validations/validators/git_sha_spec.rb37
-rw-r--r--spec/lib/api/validations/validators/integer_none_any_spec.rb28
-rw-r--r--spec/lib/banzai/filter/autolink_filter_spec.rb5
-rw-r--r--spec/lib/gitlab/git/blob_spec.rb42
-rw-r--r--spec/models/ability_spec.rb6
-rw-r--r--spec/models/ci/build_spec.rb2
-rw-r--r--spec/requests/api/deploy_tokens_spec.rb30
-rw-r--r--spec/serializers/diffs_entity_spec.rb42
-rw-r--r--spec/services/groups/deploy_tokens/destroy_service_spec.rb11
-rw-r--r--spec/services/projects/deploy_tokens/destroy_service_spec.rb11
-rw-r--r--spec/services/resource_events/synthetic_milestone_notes_builder_service_spec.rb25
-rw-r--r--spec/support/helpers/api_validators_helpers.rb23
-rw-r--r--spec/support/services/deploy_token_shared_examples.rb22
60 files changed, 754 insertions, 417 deletions
diff --git a/.rubocop.yml b/.rubocop.yml
index 96ff65b3c49..4a6e90f5263 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -401,13 +401,9 @@ RSpec/RepeatedExample:
- 'spec/frontend/fixtures/merge_requests.rb'
- 'spec/graphql/gitlab_schema_spec.rb'
- 'spec/helpers/users_helper_spec.rb'
- - 'spec/lib/banzai/filter/autolink_filter_spec.rb'
- 'spec/lib/gitlab/closing_issue_extractor_spec.rb'
- 'spec/lib/gitlab/danger/changelog_spec.rb'
- - 'spec/lib/gitlab/git/blob_spec.rb'
- 'spec/lib/gitlab/import_export/project/relation_factory_spec.rb'
- - 'spec/models/ability_spec.rb'
- - 'spec/models/ci/build_spec.rb'
- 'spec/models/concerns/issuable_spec.rb'
- 'spec/models/project_services/chat_message/pipeline_message_spec.rb'
- 'spec/routing/admin_routing_spec.rb'
diff --git a/app/assets/javascripts/blob/blob_file_dropzone.js b/app/assets/javascripts/blob/blob_file_dropzone.js
index 8acf0827c44..e8772b7240a 100644
--- a/app/assets/javascripts/blob/blob_file_dropzone.js
+++ b/app/assets/javascripts/blob/blob_file_dropzone.js
@@ -43,6 +43,10 @@ export default class BlobFileDropzone {
previewsContainer: '.dropzone-previews',
headers: csrf.headers,
init() {
+ this.on('processing', function() {
+ this.options.url = form.attr('action');
+ });
+
this.on('addedfile', () => {
toggleLoading(submitButton, submitButtonLoadingIcon, false);
dropzoneMessage.addClass(HIDDEN_CLASS);
diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index d1eed85fde6..664c58e8b7a 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -57,7 +57,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
def find_members
filter_params = params.slice(:two_factor, :search).merge(sort: @sort)
- GroupMembersFinder.new(@group, current_user).execute(include_relations: requested_relations, params: filter_params)
+ GroupMembersFinder.new(@group, current_user, params: filter_params).execute(include_relations: requested_relations)
end
def can_manage_members
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index a50d516f75e..f552c471eb2 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -186,7 +186,7 @@ class Projects::IssuesController < Projects::ApplicationController
if result[:status] == :success
render json: MergeRequestCreateSerializer.new.represent(result[:merge_request])
else
- render json: result[:messsage], status: :unprocessable_entity
+ render json: result[:message], status: :unprocessable_entity
end
end
diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb
index bee18017cf3..a56d4ebb368 100644
--- a/app/finders/group_members_finder.rb
+++ b/app/finders/group_members_finder.rb
@@ -10,13 +10,16 @@ class GroupMembersFinder < UnionFinder
# created_after: datetime
# created_before: datetime
- def initialize(group, user = nil)
+ attr_reader :params
+
+ def initialize(group, user = nil, params: {})
@group = group
@user = user
+ @params = params
end
# rubocop: disable CodeReuse/ActiveRecord
- def execute(include_relations: [:inherited, :direct], params: {})
+ def execute(include_relations: [:inherited, :direct])
group_members = group.members
relations = []
@params = params
@@ -50,7 +53,7 @@ class GroupMembersFinder < UnionFinder
private
- attr_reader :user, :group, :params
+ attr_reader :user, :group
def filter_members(members)
members = members.search(params[:search]) if params[:search].present?
diff --git a/app/serializers/diffs_entity.rb b/app/serializers/diffs_entity.rb
index b709fc7228b..1d0b4183f96 100644
--- a/app/serializers/diffs_entity.rb
+++ b/app/serializers/diffs_entity.rb
@@ -16,12 +16,7 @@ class DiffsEntity < Grape::Entity
end
expose :commit do |diffs, options|
- CommitEntity.represent options[:commit], options.merge(
- type: :full,
- commit_url_params: { merge_request_iid: merge_request&.iid },
- pipeline_ref: merge_request&.source_branch,
- pipeline_project: merge_request&.source_project
- )
+ CommitEntity.represent(options[:commit], commit_options(options))
end
expose :context_commits, using: API::Entities::Commit, if: -> (diffs, options) { merge_request&.project&.context_commits_enabled? } do |diffs|
@@ -88,4 +83,31 @@ class DiffsEntity < Grape::Entity
def merge_request
options[:merge_request]
end
+
+ private
+
+ def commit_ids
+ @commit_ids ||= merge_request.recent_commits.map(&:id)
+ end
+
+ def commit_neighbors(commit_id)
+ index = commit_ids.index(commit_id)
+
+ return [] unless index
+
+ [(index > 0 ? commit_ids[index - 1] : nil), commit_ids[index + 1]]
+ end
+
+ def commit_options(options)
+ prev_commit_id, next_commit_id = *commit_neighbors(options[:commit]&.id)
+
+ options.merge(
+ type: :full,
+ commit_url_params: { merge_request_iid: merge_request&.iid },
+ pipeline_ref: merge_request&.source_branch,
+ pipeline_project: merge_request&.source_project,
+ prev_commit_id: prev_commit_id,
+ next_commit_id: next_commit_id
+ )
+ end
end
diff --git a/app/services/concerns/deploy_token_methods.rb b/app/services/concerns/deploy_token_methods.rb
index c0208b16623..c875342a07c 100644
--- a/app/services/concerns/deploy_token_methods.rb
+++ b/app/services/concerns/deploy_token_methods.rb
@@ -8,4 +8,10 @@ module DeployTokenMethods
deploy_token.username = params[:username].presence
end
end
+
+ def destroy_deploy_token(entity, params)
+ deploy_token = entity.deploy_tokens.find_by_id!(params[:token_id])
+
+ deploy_token.destroy
+ end
end
diff --git a/app/services/groups/deploy_tokens/destroy_service.rb b/app/services/groups/deploy_tokens/destroy_service.rb
new file mode 100644
index 00000000000..6dae22f29d2
--- /dev/null
+++ b/app/services/groups/deploy_tokens/destroy_service.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module Groups
+ module DeployTokens
+ class DestroyService < BaseService
+ include DeployTokenMethods
+
+ def execute
+ destroy_deploy_token(@group, params)
+ end
+ end
+ end
+end
diff --git a/app/services/projects/deploy_tokens/destroy_service.rb b/app/services/projects/deploy_tokens/destroy_service.rb
new file mode 100644
index 00000000000..e063f86a65c
--- /dev/null
+++ b/app/services/projects/deploy_tokens/destroy_service.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module Projects
+ module DeployTokens
+ class DestroyService < BaseService
+ include DeployTokenMethods
+
+ def execute
+ destroy_deploy_token(@project, params)
+ end
+ end
+ end
+end
diff --git a/app/services/resource_events/synthetic_milestone_notes_builder_service.rb b/app/services/resource_events/synthetic_milestone_notes_builder_service.rb
index ad58417834e..cc6383d7083 100644
--- a/app/services/resource_events/synthetic_milestone_notes_builder_service.rb
+++ b/app/services/resource_events/synthetic_milestone_notes_builder_service.rb
@@ -10,8 +10,6 @@ module ResourceEvents
private
def synthetic_notes
- return [] unless tracking_enabled?
-
milestone_change_events.map do |event|
MilestoneNote.from_event(event, resource: resource, resource_parent: resource_parent)
end
@@ -23,9 +21,5 @@ module ResourceEvents
events = resource.resource_milestone_events.includes(user: :status) # rubocop: disable CodeReuse/ActiveRecord
since_fetch_at(events)
end
-
- def tracking_enabled?
- ::Feature.enabled?(:track_resource_milestone_change_events, resource.project)
- end
end
end
diff --git a/app/workers/gitlab/github_import/advance_stage_worker.rb b/app/workers/gitlab/github_import/advance_stage_worker.rb
index 8bbfb10ed6e..21e478f935b 100644
--- a/app/workers/gitlab/github_import/advance_stage_worker.rb
+++ b/app/workers/gitlab/github_import/advance_stage_worker.rb
@@ -21,6 +21,10 @@ module Gitlab
finish: Stage::FinishImportWorker
}.freeze
+ def find_import_state(project_id)
+ ProjectImportState.jid_by(project_id: project_id, status: :started)
+ end
+
private
def next_stage_worker(next_stage)
diff --git a/app/workers/gitlab/import/advance_stage.rb b/app/workers/gitlab/import/advance_stage.rb
index 5c836413ae3..3f34437294e 100644
--- a/app/workers/gitlab/import/advance_stage.rb
+++ b/app/workers/gitlab/import/advance_stage.rb
@@ -48,7 +48,7 @@ module Gitlab
end
def find_import_state(project_id)
- ProjectImportState.jid_by(project_id: project_id, status: :started)
+ raise NotImplementedError
end
private
diff --git a/app/workers/gitlab/jira_import/advance_stage_worker.rb b/app/workers/gitlab/jira_import/advance_stage_worker.rb
index 1b6fc54151e..c83a57bcd83 100644
--- a/app/workers/gitlab/jira_import/advance_stage_worker.rb
+++ b/app/workers/gitlab/jira_import/advance_stage_worker.rb
@@ -16,6 +16,10 @@ module Gitlab
finish: Gitlab::JiraImport::Stage::FinishImportWorker
}.freeze
+ def find_import_state(project_id)
+ ProjectImportState.jid_by(project_id: project_id, status: :started)
+ end
+
private
def next_stage_worker(next_stage)
diff --git a/changelogs/unreleased/212523-delete-deploy-tokens.yml b/changelogs/unreleased/212523-delete-deploy-tokens.yml
new file mode 100644
index 00000000000..f16703bb594
--- /dev/null
+++ b/changelogs/unreleased/212523-delete-deploy-tokens.yml
@@ -0,0 +1,5 @@
+---
+title: Fix deploy token API to properly delete all associated deploy token records
+merge_request: 28156
+author:
+type: fixed
diff --git a/changelogs/unreleased/212985-remove-ff-check-for-synthetic-milestone-notes-creation.yml b/changelogs/unreleased/212985-remove-ff-check-for-synthetic-milestone-notes-creation.yml
new file mode 100644
index 00000000000..5395723e125
--- /dev/null
+++ b/changelogs/unreleased/212985-remove-ff-check-for-synthetic-milestone-notes-creation.yml
@@ -0,0 +1,5 @@
+---
+title: Fix missing synthetic milestone change notes for disabled milestone change event tracking feature flag
+merge_request: 28440
+author:
+type: fixed
diff --git a/changelogs/unreleased/42639-remove-custom-metrics-license-check.yml b/changelogs/unreleased/42639-remove-custom-metrics-license-check.yml
new file mode 100644
index 00000000000..018245a7384
--- /dev/null
+++ b/changelogs/unreleased/42639-remove-custom-metrics-license-check.yml
@@ -0,0 +1,5 @@
+---
+title: Move 'Additional Metrics' feature to GitLab Core
+merge_request: 28527
+author:
+type: changed
diff --git a/changelogs/unreleased/merge-request-typo.yml b/changelogs/unreleased/merge-request-typo.yml
new file mode 100644
index 00000000000..5815f5942bf
--- /dev/null
+++ b/changelogs/unreleased/merge-request-typo.yml
@@ -0,0 +1,5 @@
+---
+title: Return error message for create_merge_request
+merge_request: 28482
+author:
+type: fixed
diff --git a/changelogs/unreleased/ph-212642-userUploadsFile.yml b/changelogs/unreleased/ph-212642-userUploadsFile.yml
new file mode 100644
index 00000000000..39053504d1a
--- /dev/null
+++ b/changelogs/unreleased/ph-212642-userUploadsFile.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed upload file creating a file in the wrong directory
+merge_request:
+author:
+type: fixed
diff --git a/changelogs/unreleased/refactor-ability-spec.yml b/changelogs/unreleased/refactor-ability-spec.yml
new file mode 100644
index 00000000000..0578a6cd674
--- /dev/null
+++ b/changelogs/unreleased/refactor-ability-spec.yml
@@ -0,0 +1,5 @@
+---
+title: Remove duplicate specs in ability model
+merge_request: 28644
+author: Rajendra Kadam
+type: added
diff --git a/changelogs/unreleased/refactor-build-spec.yml b/changelogs/unreleased/refactor-build-spec.yml
new file mode 100644
index 00000000000..7b087933ed1
--- /dev/null
+++ b/changelogs/unreleased/refactor-build-spec.yml
@@ -0,0 +1,5 @@
+---
+title: Fix build duplicate spec
+merge_request: 28633
+author: Rajendra Kadam
+type: added
diff --git a/changelogs/unreleased/separate-validator-files.yml b/changelogs/unreleased/separate-validator-files.yml
new file mode 100644
index 00000000000..0003064c7cb
--- /dev/null
+++ b/changelogs/unreleased/separate-validator-files.yml
@@ -0,0 +1,5 @@
+---
+title: Separate validators into own class files
+merge_request: 28266
+author: Rajendra Kadam
+type: added
diff --git a/config/initializers/grape_validators.rb b/config/initializers/grape_validators.rb
new file mode 100644
index 00000000000..9d2b6dc9bd1
--- /dev/null
+++ b/config/initializers/grape_validators.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+Grape::Validations.register_validator(:absence, ::API::Validations::Validators::Absence)
+Grape::Validations.register_validator(:file_path, ::API::Validations::Validators::FilePath)
+Grape::Validations.register_validator(:git_ref, ::API::Validations::Validators::GitRef)
+Grape::Validations.register_validator(:git_sha, ::API::Validations::Validators::GitSha)
+Grape::Validations.register_validator(:integer_none_any, ::API::Validations::Validators::IntegerNoneAny)
+Grape::Validations.register_validator(:array_none_any, ::API::Validations::Validators::ArrayNoneAny)
+Grape::Validations.register_validator(:check_assignees_count, ::API::Validations::Validators::CheckAssigneesCount)
diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md
index 776ac252b76..46ea91fcdf3 100644
--- a/doc/development/migration_style_guide.md
+++ b/doc/development/migration_style_guide.md
@@ -246,6 +246,7 @@ Example changes:
- `add_foreign_key` / `remove_foreign_key`
- `add_column` / `remove_column`
- `change_column_default`
+- `create_table` / `drop_table`
**Note:** `with_lock_retries` method **cannot** be used with `disable_ddl_transaction!`.
diff --git a/lib/api/deploy_tokens.rb b/lib/api/deploy_tokens.rb
index fb4c4265aef..d36b75f5bfd 100644
--- a/lib/api/deploy_tokens.rb
+++ b/lib/api/deploy_tokens.rb
@@ -85,12 +85,10 @@ module API
delete ':id/deploy_tokens/:token_id' do
authorize!(:destroy_deploy_token, user_project)
- deploy_token = user_project.project_deploy_tokens
- .find_by_deploy_token_id(params[:token_id])
-
- not_found!('Deploy Token') unless deploy_token
+ ::Projects::DeployTokens::DestroyService.new(
+ user_project, current_user, token_id: params[:token_id]
+ ).execute
- deploy_token.destroy
no_content!
end
end
@@ -144,13 +142,17 @@ module API
desc 'Delete a group deploy token' do
detail 'This feature was introduced in GitLab 12.9'
end
+ params do
+ requires :token_id, type: Integer, desc: 'The deploy token ID'
+ end
delete ':id/deploy_tokens/:token_id' do
authorize!(:destroy_deploy_token, user_group)
- deploy_token = user_group.group_deploy_tokens
- .find_by_deploy_token_id!(params[:token_id])
+ ::Groups::DeployTokens::DestroyService.new(
+ user_group, current_user, token_id: params[:token_id]
+ ).execute
- destroy_conditionally!(deploy_token)
+ no_content!
end
end
end
diff --git a/lib/api/entities/commit_with_link.rb b/lib/api/entities/commit_with_link.rb
index 31a9efed9bc..a135cc19480 100644
--- a/lib/api/entities/commit_with_link.rb
+++ b/lib/api/entities/commit_with_link.rb
@@ -32,6 +32,14 @@ module API
render('projects/commit/_signature', signature: commit.signature) if commit.has_signature?
end
+ expose :prev_commit_id, if: { type: :full } do |commit|
+ options[:prev_commit_id]
+ end
+
+ expose :next_commit_id, if: { type: :full } do |commit|
+ options[:next_commit_id]
+ end
+
expose :pipeline_status_path, if: { type: :full } do |commit, options|
pipeline_ref = options[:pipeline_ref]
pipeline_project = options[:pipeline_project] || commit.project
diff --git a/lib/api/environments.rb b/lib/api/environments.rb
index e5db9cdedc8..28019ce7796 100644
--- a/lib/api/environments.rb
+++ b/lib/api/environments.rb
@@ -3,7 +3,6 @@
module API
# Environments RESTfull API endpoints
class Environments < Grape::API
- include ::API::Helpers::CustomValidators
include PaginationParams
before { authenticate! }
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index be41788ac77..42b82aac1c4 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -179,8 +179,10 @@ module API
end
# rubocop: disable CodeReuse/ActiveRecord
- def find_project_issue(iid)
- IssuesFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
+ def find_project_issue(iid, project_id = nil)
+ project = project_id ? find_project!(project_id) : user_project
+
+ ::IssuesFinder.new(current_user, project_id: project.id).find_by!(iid: iid)
end
# rubocop: enable CodeReuse/ActiveRecord
diff --git a/lib/api/helpers/custom_validators.rb b/lib/api/helpers/custom_validators.rb
deleted file mode 100644
index 76f5fe555b4..00000000000
--- a/lib/api/helpers/custom_validators.rb
+++ /dev/null
@@ -1,97 +0,0 @@
-# frozen_string_literal: true
-
-module API
- module Helpers
- module CustomValidators
- class FilePath < Grape::Validations::Base
- def validate_param!(attr_name, params)
- path = params[attr_name]
-
- Gitlab::Utils.check_path_traversal!(path)
- rescue StandardError
- raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
- message: "should be a valid file path"
- end
- end
-
- class GitSha < Grape::Validations::Base
- def validate_param!(attr_name, params)
- sha = params[attr_name]
-
- return if Commit::EXACT_COMMIT_SHA_PATTERN.match?(sha)
-
- raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
- message: "should be a valid sha"
- end
- end
-
- class Absence < Grape::Validations::Base
- def validate_param!(attr_name, params)
- return if params.respond_to?(:key?) && !params.key?(attr_name)
-
- raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:absence)
- end
- end
-
- class IntegerNoneAny < Grape::Validations::Base
- def validate_param!(attr_name, params)
- value = params[attr_name]
-
- return if value.is_a?(Integer) ||
- [IssuableFinder::Params::FILTER_NONE, IssuableFinder::Params::FILTER_ANY].include?(value.to_s.downcase)
-
- raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
- message: "should be an integer, 'None' or 'Any'"
- end
- end
-
- class ArrayNoneAny < Grape::Validations::Base
- def validate_param!(attr_name, params)
- value = params[attr_name]
-
- return if value.is_a?(Array) ||
- [IssuableFinder::Params::FILTER_NONE, IssuableFinder::Params::FILTER_ANY].include?(value.to_s.downcase)
-
- raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
- message: "should be an array, 'None' or 'Any'"
- end
- end
-
- class GitRef < Grape::Validations::Base
- # There are few checks that a Git reference should pass through to be valid reference.
- # The link contains some rules that have been added to this validator.
- # https://mirrors.edge.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
- # We have skipped some checks that are optional and can be skipped for exception.
- # We also check for control characters, More info on ctrl chars - https://ruby-doc.org/core-2.7.0/Regexp.html#class-Regexp-label-Character+Classes
- INVALID_CHARS = Regexp.union('..', '\\', '@', '@{', ' ', '~', '^', ':', '*', '?', '[', /[[:cntrl:]]/).freeze
- GIT_REF_LENGTH = (1..1024).freeze
-
- def validate_param!(attr_name, params)
- revision = params[attr_name]
-
- return unless invalid_character?(revision)
-
- raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
- message: 'should be a valid reference path'
- end
-
- private
-
- def invalid_character?(revision)
- revision.nil? ||
- revision.start_with?('-') ||
- revision.end_with?('.') ||
- GIT_REF_LENGTH.exclude?(revision.length) ||
- INVALID_CHARS.match?(revision)
- end
- end
- end
- end
-end
-
-Grape::Validations.register_validator(:file_path, ::API::Helpers::CustomValidators::FilePath)
-Grape::Validations.register_validator(:git_sha, ::API::Helpers::CustomValidators::GitSha)
-Grape::Validations.register_validator(:absence, ::API::Helpers::CustomValidators::Absence)
-Grape::Validations.register_validator(:integer_none_any, ::API::Helpers::CustomValidators::IntegerNoneAny)
-Grape::Validations.register_validator(:array_none_any, ::API::Helpers::CustomValidators::ArrayNoneAny)
-Grape::Validations.register_validator(:git_ref, ::API::Helpers::CustomValidators::GitRef)
diff --git a/lib/api/helpers/merge_requests_helpers.rb b/lib/api/helpers/merge_requests_helpers.rb
index e0753254002..73711a7e0ba 100644
--- a/lib/api/helpers/merge_requests_helpers.rb
+++ b/lib/api/helpers/merge_requests_helpers.rb
@@ -4,7 +4,6 @@ module API
module Helpers
module MergeRequestsHelpers
extend Grape::API::Helpers
- include ::API::Helpers::CustomValidators
params :merge_requests_base_params do
optional :state,
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index d34c205281a..a78202877fb 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -23,7 +23,7 @@ module API
optional :assignee_id, types: [Integer, String], integer_none_any: true,
desc: 'Return issues which are assigned to the user with the given ID'
optional :assignee_username, type: Array[String], check_assignees_count: true,
- coerce_with: Validations::CheckAssigneesCount.coerce,
+ coerce_with: Validations::Validators::CheckAssigneesCount.coerce,
desc: 'Return issues which are assigned to the user with the given username'
mutually_exclusive :assignee_id, :assignee_username
end
diff --git a/lib/api/validations/check_assignees_count.rb b/lib/api/validations/check_assignees_count.rb
deleted file mode 100644
index 451b14c623c..00000000000
--- a/lib/api/validations/check_assignees_count.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-# frozen_string_literal: true
-
-module API
- module Validations
- class CheckAssigneesCount < Grape::Validations::Base
- def self.coerce
- lambda do |value|
- case value
- when String, Array
- Array.wrap(value)
- else
- []
- end
- end
- end
-
- def validate_param!(attr_name, params)
- return if param_allowed?(attr_name, params)
-
- raise Grape::Exceptions::Validation,
- params: [@scope.full_name(attr_name)],
- message: "allows one value, but found #{params[attr_name].size}: #{params[attr_name].join(", ")}"
- end
-
- private
-
- def param_allowed?(attr_name, params)
- params[attr_name].size <= 1
- end
- end
- end
-end
-
-API::Validations::CheckAssigneesCount.prepend_if_ee('EE::API::Validations::CheckAssigneesCount')
diff --git a/lib/api/validations/validators/absence.rb b/lib/api/validations/validators/absence.rb
new file mode 100644
index 00000000000..1f43f3ab126
--- /dev/null
+++ b/lib/api/validations/validators/absence.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module API
+ module Validations
+ module Validators
+ class Absence < Grape::Validations::Base
+ def validate_param!(attr_name, params)
+ return if params.respond_to?(:key?) && !params.key?(attr_name)
+
+ raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:absence)
+ end
+ end
+ end
+ end
+end
diff --git a/lib/api/validations/validators/array_none_any.rb b/lib/api/validations/validators/array_none_any.rb
new file mode 100644
index 00000000000..7efb8e6ccee
--- /dev/null
+++ b/lib/api/validations/validators/array_none_any.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module API
+ module Validations
+ module Validators
+ class ArrayNoneAny < Grape::Validations::Base
+ def validate_param!(attr_name, params)
+ value = params[attr_name]
+
+ return if value.is_a?(Array) ||
+ [IssuableFinder::Params::FILTER_NONE, IssuableFinder::Params::FILTER_ANY].include?(value.to_s.downcase)
+
+ raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
+ message: "should be an array, 'None' or 'Any'"
+ end
+ end
+ end
+ end
+end
diff --git a/lib/api/validations/validators/check_assignees_count.rb b/lib/api/validations/validators/check_assignees_count.rb
new file mode 100644
index 00000000000..b614058e325
--- /dev/null
+++ b/lib/api/validations/validators/check_assignees_count.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module API
+ module Validations
+ module Validators
+ class CheckAssigneesCount < Grape::Validations::Base
+ def self.coerce
+ lambda do |value|
+ case value
+ when String, Array
+ Array.wrap(value)
+ else
+ []
+ end
+ end
+ end
+
+ def validate_param!(attr_name, params)
+ return if param_allowed?(attr_name, params)
+
+ raise Grape::Exceptions::Validation,
+ params: [@scope.full_name(attr_name)],
+ message: "allows one value, but found #{params[attr_name].size}: #{params[attr_name].join(", ")}"
+ end
+
+ private
+
+ def param_allowed?(attr_name, params)
+ params[attr_name].size <= 1
+ end
+ end
+ end
+ end
+end
+
+API::Validations::Validators::CheckAssigneesCount.prepend_if_ee('EE::API::Validations::Validators::CheckAssigneesCount')
diff --git a/lib/api/validations/validators/file_path.rb b/lib/api/validations/validators/file_path.rb
new file mode 100644
index 00000000000..93a20e5bf7d
--- /dev/null
+++ b/lib/api/validations/validators/file_path.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module API
+ module Validations
+ module Validators
+ class FilePath < Grape::Validations::Base
+ def validate_param!(attr_name, params)
+ path = params[attr_name]
+
+ Gitlab::Utils.check_path_traversal!(path)
+ rescue StandardError
+ raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
+ message: "should be a valid file path"
+ end
+ end
+ end
+ end
+end
diff --git a/lib/api/validations/validators/git_ref.rb b/lib/api/validations/validators/git_ref.rb
new file mode 100644
index 00000000000..1dda9d758a7
--- /dev/null
+++ b/lib/api/validations/validators/git_ref.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module API
+ module Validations
+ module Validators
+ class GitRef < Grape::Validations::Base
+ # There are few checks that a Git reference should pass through to be valid reference.
+ # The link contains some rules that have been added to this validator.
+ # https://mirrors.edge.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
+ # We have skipped some checks that are optional and can be skipped for exception.
+ # We also check for control characters, More info on ctrl chars - https://ruby-doc.org/core-2.7.0/Regexp.html#class-Regexp-label-Character+Classes
+ INVALID_CHARS = Regexp.union('..', '\\', '@', '@{', ' ', '~', '^', ':', '*', '?', '[', /[[:cntrl:]]/).freeze
+ GIT_REF_LENGTH = (1..1024).freeze
+
+ def validate_param!(attr_name, params)
+ revision = params[attr_name]
+
+ return unless invalid_character?(revision)
+
+ raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
+ message: 'should be a valid reference path'
+ end
+
+ private
+
+ def invalid_character?(revision)
+ revision.nil? ||
+ revision.start_with?('-') ||
+ revision.end_with?('.') ||
+ GIT_REF_LENGTH.exclude?(revision.length) ||
+ INVALID_CHARS.match?(revision)
+ end
+ end
+ end
+ end
+end
diff --git a/lib/api/validations/validators/git_sha.rb b/lib/api/validations/validators/git_sha.rb
new file mode 100644
index 00000000000..657307db1df
--- /dev/null
+++ b/lib/api/validations/validators/git_sha.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module API
+ module Validations
+ module Validators
+ class GitSha < Grape::Validations::Base
+ def validate_param!(attr_name, params)
+ sha = params[attr_name]
+
+ return if Commit::EXACT_COMMIT_SHA_PATTERN.match?(sha)
+
+ raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
+ message: "should be a valid sha"
+ end
+ end
+ end
+ end
+end
diff --git a/lib/api/validations/validators/integer_none_any.rb b/lib/api/validations/validators/integer_none_any.rb
new file mode 100644
index 00000000000..aa8c137a6ab
--- /dev/null
+++ b/lib/api/validations/validators/integer_none_any.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module API
+ module Validations
+ module Validators
+ class IntegerNoneAny < Grape::Validations::Base
+ def validate_param!(attr_name, params)
+ value = params[attr_name]
+
+ return if value.is_a?(Integer) ||
+ [IssuableFinder::Params::FILTER_NONE, IssuableFinder::Params::FILTER_ANY].include?(value.to_s.downcase)
+
+ raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
+ message: "should be an integer, 'None' or 'Any'"
+ end
+ end
+ end
+ end
+end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 1fda9d63bbe..30228248baf 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1249,6 +1249,26 @@ describe Projects::IssuesController do
expect(response).to have_gitlab_http_status(:not_found)
end
+ context 'invalid branch name' do
+ it 'is unprocessable' do
+ post(
+ :create_merge_request,
+ params: {
+ target_project_id: nil,
+ branch_name: 'master',
+ ref: 'master',
+ namespace_id: project.namespace.to_param,
+ project_id: project.to_param,
+ id: issue.to_param
+ },
+ format: :json
+ )
+
+ expect(response.body).to eq('Branch already exists')
+ expect(response).to have_gitlab_http_status(:unprocessable_entity)
+ end
+ end
+
context 'target_project_id is set' do
let(:target_project) { fork_project(project, user, repository: true) }
let(:target_project_id) { target_project.id }
diff --git a/spec/features/projects/files/user_uploads_files_spec.rb b/spec/features/projects/files/user_uploads_files_spec.rb
index 8a20c1387a3..ecf40969541 100644
--- a/spec/features/projects/files/user_uploads_files_spec.rb
+++ b/spec/features/projects/files/user_uploads_files_spec.rb
@@ -22,6 +22,31 @@ describe 'Projects > Files > User uploads files' do
include_examples 'it uploads and commit a new text file'
include_examples 'it uploads and commit a new image file'
+
+ it 'uploads a file to a sub-directory', :js do
+ click_link 'files'
+
+ page.within('.repo-breadcrumb') do
+ expect(page).to have_content('files')
+ end
+
+ find('.add-to-tree').click
+ click_link('Upload file')
+ drop_in_dropzone(File.join(Rails.root, 'spec', 'fixtures', 'doc_sample.txt'))
+
+ page.within('#modal-upload-blob') do
+ fill_in(:commit_message, with: 'New commit message')
+ end
+
+ click_button('Upload file')
+
+ expect(page).to have_content('New commit message')
+
+ page.within('.repo-breadcrumb') do
+ expect(page).to have_content('files')
+ expect(page).to have_content('doc_sample.txt')
+ end
+ end
end
context 'when a user does not have write access' do
diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb
index 34649097f70..d1d97f6f9f0 100644
--- a/spec/finders/group_members_finder_spec.rb
+++ b/spec/finders/group_members_finder_spec.rb
@@ -82,7 +82,7 @@ describe GroupMembersFinder, '#execute' do
group.add_developer(user3)
member = group.add_maintainer(user1)
- result = described_class.new(group).execute(params: { search: user1.name })
+ result = described_class.new(group, params: { search: user1.name }).execute
expect(result.to_a).to match_array([member])
end
@@ -92,7 +92,7 @@ describe GroupMembersFinder, '#execute' do
group.add_developer(user3)
group.add_maintainer(user1)
- result = described_class.new(group).execute(include_relations: [:inherited], params: { search: user1.name })
+ result = described_class.new(group, params: { search: user1.name }).execute(include_relations: [:inherited])
expect(result.to_a).to match_array([])
end
@@ -103,7 +103,7 @@ describe GroupMembersFinder, '#execute' do
nested_group.add_maintainer(create(:user, name: user1.name))
member = group.add_maintainer(user1)
- result = described_class.new(nested_group).execute(include_relations: [:inherited], params: { search: member.user.name })
+ result = described_class.new(nested_group, params: { search: member.user.name }).execute(include_relations: [:inherited])
expect(result.to_a).to contain_exactly(member)
end
@@ -113,7 +113,7 @@ describe GroupMembersFinder, '#execute' do
group.add_maintainer(user1)
member = group.add_maintainer(user5)
- result = described_class.new(group, user2).execute(params: { two_factor: 'enabled' })
+ result = described_class.new(group, user2, params: { two_factor: 'enabled' }).execute
expect(result.to_a).to contain_exactly(member)
end
@@ -123,7 +123,7 @@ describe GroupMembersFinder, '#execute' do
member2 = group.add_maintainer(user1)
member_with_2fa = group.add_maintainer(user5)
- result = described_class.new(group, user2).execute(params: { two_factor: 'disabled' })
+ result = described_class.new(group, user2, params: { two_factor: 'disabled' }).execute
expect(result.to_a).not_to include(member_with_2fa)
expect(result.to_a).to match_array([member1, member2])
diff --git a/spec/frontend/diffs/components/commit_item_spec.js b/spec/frontend/diffs/components/commit_item_spec.js
index 61bab77964e..6bb3a0dcf21 100644
--- a/spec/frontend/diffs/components/commit_item_spec.js
+++ b/spec/frontend/diffs/components/commit_item_spec.js
@@ -30,12 +30,12 @@ describe('diffs/components/commit_item', () => {
const getCommitActionsElement = () => wrapper.find('.commit-actions');
const getCommitPipelineStatus = () => wrapper.find(CommitPipelineStatus);
- const defaultProps = {
- commit: getDiffWithCommit().commit,
- };
- const mountComponent = (propsData = defaultProps) => {
+ const mountComponent = propsData => {
wrapper = mount(Component, {
- propsData,
+ propsData: {
+ commit,
+ ...propsData,
+ },
stubs: {
CommitPipelineStatus: true,
},
@@ -59,9 +59,7 @@ describe('diffs/components/commit_item', () => {
expect(titleElement.text()).toBe(commit.title_html);
});
- // https://gitlab.com/gitlab-org/gitlab/-/issues/209776
- // eslint-disable-next-line jest/no-disabled-tests
- it.skip('renders commit description', () => {
+ it('renders commit description', () => {
const descElement = getDescElement();
const descExpandElement = getDescExpandElement();
@@ -107,7 +105,7 @@ describe('diffs/components/commit_item', () => {
describe('without commit description', () => {
beforeEach(() => {
- mountComponent({ defaultProps, commit: { ...defaultProps.commit, description_html: '' } });
+ mountComponent({ commit: { ...commit, description_html: '' } });
});
it('hides description', () => {
@@ -122,9 +120,8 @@ describe('diffs/components/commit_item', () => {
describe('with no matching user', () => {
beforeEach(() => {
mountComponent({
- defaultProps,
commit: {
- ...defaultProps.commit,
+ ...commit,
author: null,
author_email: TEST_AUTHOR_EMAIL,
author_name: TEST_AUTHOR_NAME,
@@ -154,8 +151,7 @@ describe('diffs/components/commit_item', () => {
describe('with signature', () => {
beforeEach(() => {
mountComponent({
- defaultProps,
- commit: { ...defaultProps.commit, signature_html: TEST_SIGNATURE_HTML },
+ commit: { ...commit, signature_html: TEST_SIGNATURE_HTML },
});
});
@@ -169,8 +165,7 @@ describe('diffs/components/commit_item', () => {
describe('with pipeline status', () => {
beforeEach(() => {
mountComponent({
- defaultProps,
- commit: { ...defaultProps.commit, pipeline_status_path: TEST_PIPELINE_STATUS_PATH },
+ commit: { ...commit, pipeline_status_path: TEST_PIPELINE_STATUS_PATH },
});
});
diff --git a/spec/lib/api/helpers/custom_validators_spec.rb b/spec/lib/api/helpers/custom_validators_spec.rb
deleted file mode 100644
index a4f2cd3452c..00000000000
--- a/spec/lib/api/helpers/custom_validators_spec.rb
+++ /dev/null
@@ -1,193 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe API::Helpers::CustomValidators do
- let(:scope) do
- Struct.new(:opts) do
- def full_name(attr_name)
- attr_name
- end
- end
- end
-
- describe API::Helpers::CustomValidators::Absence do
- subject do
- described_class.new(['test'], {}, false, scope.new)
- end
-
- context 'empty param' do
- it 'does not raise a validation error' do
- expect_no_validation_error({})
- end
- end
-
- context 'invalid parameters' do
- it 'raises a validation error' do
- expect_validation_error('test' => 'some_value')
- end
- end
- end
-
- describe API::Helpers::CustomValidators::GitSha do
- let(:sha) { RepoHelpers.sample_commit.id }
- let(:short_sha) { sha[0, Gitlab::Git::Commit::MIN_SHA_LENGTH] }
- let(:too_short_sha) { sha[0, Gitlab::Git::Commit::MIN_SHA_LENGTH - 1] }
-
- subject do
- described_class.new(['test'], {}, false, scope.new)
- end
-
- context 'valid sha' do
- it 'does not raise a validation error' do
- expect_no_validation_error('test' => sha)
- expect_no_validation_error('test' => short_sha)
- end
- end
-
- context 'empty params' do
- it 'raises a validation error' do
- expect_validation_error('test' => nil)
- expect_validation_error('test' => '')
- end
- end
-
- context 'invalid sha' do
- it 'raises a validation error' do
- expect_validation_error('test' => "#{sha}2") # Sha length > 40
- expect_validation_error('test' => 'somestring')
- expect_validation_error('test' => too_short_sha) # sha length < MIN_SHA_LENGTH (7)
- end
- end
- end
-
- describe API::Helpers::CustomValidators::GitRef do
- subject do
- described_class.new(['test'], {}, false, scope.new)
- end
-
- context 'valid revision param' do
- it 'does not raise a validation error' do
- expect_no_validation_error('test' => '4e963fe')
- expect_no_validation_error('test' => 'foo/bar/baz')
- expect_no_validation_error('test' => "heads/fu\303\237")
- expect_no_validation_error('test' => 'a' * 1024)
- end
- end
-
- context "revision param contains invalid chars" do
- it 'raises a validation error' do
- expect_validation_error('test' => '-4e963fe')
- expect_validation_error('test' => '4e963fe..ed4ef')
- expect_validation_error('test' => '4e96\3fe')
- expect_validation_error('test' => '4e96@3fe')
- expect_validation_error('test' => '4e9@{63fe')
- expect_validation_error('test' => '4e963 fe')
- expect_validation_error('test' => '4e96~3fe')
- expect_validation_error('test' => '^4e963fe')
- expect_validation_error('test' => '4:e963fe')
- expect_validation_error('test' => '4e963fe.')
- expect_validation_error('test' => 'heads/foo..bar')
- expect_validation_error('test' => 'foo/bar/.')
- expect_validation_error('test' => 'heads/v@{ation')
- expect_validation_error('test' => 'refs/heads/foo.')
- expect_validation_error('test' => 'heads/foo\bar')
- expect_validation_error('test' => 'heads/f[/bar')
- expect_validation_error('test' => "heads/foo\t")
- expect_validation_error('test' => "heads/foo\177")
- expect_validation_error('test' => "#{'a' * 1025}")
- expect_validation_error('test' => nil)
- expect_validation_error('test' => '')
- end
- end
- end
-
- describe API::Helpers::CustomValidators::FilePath do
- subject do
- described_class.new(['test'], {}, false, scope.new)
- end
-
- context 'valid file path' do
- it 'does not raise a validation error' do
- expect_no_validation_error('test' => './foo')
- expect_no_validation_error('test' => './bar.rb')
- expect_no_validation_error('test' => 'foo%2Fbar%2Fnew%2Ffile.rb')
- expect_no_validation_error('test' => 'foo%2Fbar%2Fnew')
- expect_no_validation_error('test' => 'foo%252Fbar%252Fnew%252Ffile.rb')
- end
- end
-
- context 'invalid file path' do
- it 'raise a validation error' do
- expect_validation_error('test' => '../foo')
- expect_validation_error('test' => '../')
- expect_validation_error('test' => 'foo/../../bar')
- expect_validation_error('test' => 'foo/../')
- expect_validation_error('test' => 'foo/..')
- expect_validation_error('test' => '../')
- expect_validation_error('test' => '..\\')
- expect_validation_error('test' => '..\/')
- expect_validation_error('test' => '%2e%2e%2f')
- expect_validation_error('test' => '/etc/passwd')
- end
- end
- end
-
- describe API::Helpers::CustomValidators::IntegerNoneAny do
- subject do
- described_class.new(['test'], {}, false, scope.new)
- end
-
- context 'valid parameters' do
- it 'does not raise a validation error' do
- expect_no_validation_error('test' => 2)
- expect_no_validation_error('test' => 100)
- expect_no_validation_error('test' => 'None')
- expect_no_validation_error('test' => 'Any')
- expect_no_validation_error('test' => 'none')
- expect_no_validation_error('test' => 'any')
- end
- end
-
- context 'invalid parameters' do
- it 'raises a validation error' do
- expect_validation_error({ 'test' => 'some_other_string' })
- end
- end
- end
-
- describe API::Helpers::CustomValidators::ArrayNoneAny do
- subject do
- described_class.new(['test'], {}, false, scope.new)
- end
-
- context 'valid parameters' do
- it 'does not raise a validation error' do
- expect_no_validation_error('test' => [])
- expect_no_validation_error('test' => [1, 2, 3])
- expect_no_validation_error('test' => 'None')
- expect_no_validation_error('test' => 'Any')
- expect_no_validation_error('test' => 'none')
- expect_no_validation_error('test' => 'any')
- end
- end
-
- context 'invalid parameters' do
- it 'raises a validation error' do
- expect_validation_error('test' => 'some_other_string')
- end
- end
- end
-
- def expect_no_validation_error(params)
- expect { validate_test_param!(params) }.not_to raise_error
- end
-
- def expect_validation_error(params)
- expect { validate_test_param!(params) }.to raise_error(Grape::Exceptions::Validation)
- end
-
- def validate_test_param!(params)
- subject.validate_param!('test', params)
- end
-end
diff --git a/spec/lib/api/validations/validators/absence_spec.rb b/spec/lib/api/validations/validators/absence_spec.rb
new file mode 100644
index 00000000000..31120979d4f
--- /dev/null
+++ b/spec/lib/api/validations/validators/absence_spec.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::Validations::Validators::Absence do
+ include ApiValidatorsHelpers
+
+ subject do
+ described_class.new(['test'], {}, false, scope.new)
+ end
+
+ context 'empty param' do
+ it 'does not raise a validation error' do
+ expect_no_validation_error({})
+ end
+ end
+
+ context 'invalid parameters' do
+ it 'raises a validation error' do
+ expect_validation_error('test' => 'some_value')
+ end
+ end
+end
diff --git a/spec/lib/api/validations/validators/array_none_any_spec.rb b/spec/lib/api/validations/validators/array_none_any_spec.rb
new file mode 100644
index 00000000000..03f1c63b117
--- /dev/null
+++ b/spec/lib/api/validations/validators/array_none_any_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::Validations::Validators::ArrayNoneAny do
+ include ApiValidatorsHelpers
+
+ subject do
+ described_class.new(['test'], {}, false, scope.new)
+ end
+
+ context 'valid parameters' do
+ it 'does not raise a validation error' do
+ expect_no_validation_error('test' => [])
+ expect_no_validation_error('test' => [1, 2, 3])
+ expect_no_validation_error('test' => 'None')
+ expect_no_validation_error('test' => 'Any')
+ expect_no_validation_error('test' => 'none')
+ expect_no_validation_error('test' => 'any')
+ end
+ end
+
+ context 'invalid parameters' do
+ it 'raises a validation error' do
+ expect_validation_error('test' => 'some_other_string')
+ end
+ end
+end
diff --git a/spec/lib/api/validations/validators/file_path_spec.rb b/spec/lib/api/validations/validators/file_path_spec.rb
new file mode 100644
index 00000000000..8679f102d23
--- /dev/null
+++ b/spec/lib/api/validations/validators/file_path_spec.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::Validations::Validators::FilePath do
+ include ApiValidatorsHelpers
+
+ subject do
+ described_class.new(['test'], {}, false, scope.new)
+ end
+
+ context 'valid file path' do
+ it 'does not raise a validation error' do
+ expect_no_validation_error('test' => './foo')
+ expect_no_validation_error('test' => './bar.rb')
+ expect_no_validation_error('test' => 'foo%2Fbar%2Fnew%2Ffile.rb')
+ expect_no_validation_error('test' => 'foo%2Fbar%2Fnew')
+ expect_no_validation_error('test' => 'foo%252Fbar%252Fnew%252Ffile.rb')
+ end
+ end
+
+ context 'invalid file path' do
+ it 'raise a validation error' do
+ expect_validation_error('test' => '../foo')
+ expect_validation_error('test' => '../')
+ expect_validation_error('test' => 'foo/../../bar')
+ expect_validation_error('test' => 'foo/../')
+ expect_validation_error('test' => 'foo/..')
+ expect_validation_error('test' => '../')
+ expect_validation_error('test' => '..\\')
+ expect_validation_error('test' => '..\/')
+ expect_validation_error('test' => '%2e%2e%2f')
+ expect_validation_error('test' => '/etc/passwd')
+ end
+ end
+end
diff --git a/spec/lib/api/validations/validators/git_ref_spec.rb b/spec/lib/api/validations/validators/git_ref_spec.rb
new file mode 100644
index 00000000000..84de6272fe1
--- /dev/null
+++ b/spec/lib/api/validations/validators/git_ref_spec.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::Validations::Validators::GitRef do
+ include ApiValidatorsHelpers
+
+ subject do
+ described_class.new(['test'], {}, false, scope.new)
+ end
+
+ context 'valid revision param' do
+ it 'does not raise a validation error' do
+ expect_no_validation_error('test' => '4e963fe')
+ expect_no_validation_error('test' => 'foo/bar/baz')
+ expect_no_validation_error('test' => "heads/fu\303\237")
+ expect_no_validation_error('test' => 'a' * 1024)
+ end
+ end
+
+ context "revision param contains invalid chars" do
+ it 'raises a validation error' do
+ expect_validation_error('test' => '-4e963fe')
+ expect_validation_error('test' => '4e963fe..ed4ef')
+ expect_validation_error('test' => '4e96\3fe')
+ expect_validation_error('test' => '4e96@3fe')
+ expect_validation_error('test' => '4e9@{63fe')
+ expect_validation_error('test' => '4e963 fe')
+ expect_validation_error('test' => '4e96~3fe')
+ expect_validation_error('test' => '^4e963fe')
+ expect_validation_error('test' => '4:e963fe')
+ expect_validation_error('test' => '4e963fe.')
+ expect_validation_error('test' => 'heads/foo..bar')
+ expect_validation_error('test' => 'foo/bar/.')
+ expect_validation_error('test' => 'heads/v@{ation')
+ expect_validation_error('test' => 'refs/heads/foo.')
+ expect_validation_error('test' => 'heads/foo\bar')
+ expect_validation_error('test' => 'heads/f[/bar')
+ expect_validation_error('test' => "heads/foo\t")
+ expect_validation_error('test' => "heads/foo\177")
+ expect_validation_error('test' => "#{'a' * 1025}")
+ expect_validation_error('test' => nil)
+ expect_validation_error('test' => '')
+ end
+ end
+end
diff --git a/spec/lib/api/validations/validators/git_sha_spec.rb b/spec/lib/api/validations/validators/git_sha_spec.rb
new file mode 100644
index 00000000000..39c2fe1dcf9
--- /dev/null
+++ b/spec/lib/api/validations/validators/git_sha_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::Validations::Validators::GitSha do
+ include ApiValidatorsHelpers
+
+ let(:sha) { RepoHelpers.sample_commit.id }
+ let(:short_sha) { sha[0, Gitlab::Git::Commit::MIN_SHA_LENGTH] }
+ let(:too_short_sha) { sha[0, Gitlab::Git::Commit::MIN_SHA_LENGTH - 1] }
+
+ subject do
+ described_class.new(['test'], {}, false, scope.new)
+ end
+
+ context 'valid sha' do
+ it 'does not raise a validation error' do
+ expect_no_validation_error('test' => sha)
+ expect_no_validation_error('test' => short_sha)
+ end
+ end
+
+ context 'empty params' do
+ it 'raises a validation error' do
+ expect_validation_error('test' => nil)
+ expect_validation_error('test' => '')
+ end
+ end
+
+ context 'invalid sha' do
+ it 'raises a validation error' do
+ expect_validation_error('test' => "#{sha}2") # Sha length > 40
+ expect_validation_error('test' => 'somestring')
+ expect_validation_error('test' => too_short_sha) # sha length < MIN_SHA_LENGTH (7)
+ end
+ end
+end
diff --git a/spec/lib/api/validations/validators/integer_none_any_spec.rb b/spec/lib/api/validations/validators/integer_none_any_spec.rb
new file mode 100644
index 00000000000..a42f69fd96e
--- /dev/null
+++ b/spec/lib/api/validations/validators/integer_none_any_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::Validations::Validators::IntegerNoneAny do
+ include ApiValidatorsHelpers
+
+ subject do
+ described_class.new(['test'], {}, false, scope.new)
+ end
+
+ context 'valid parameters' do
+ it 'does not raise a validation error' do
+ expect_no_validation_error('test' => 2)
+ expect_no_validation_error('test' => 100)
+ expect_no_validation_error('test' => 'None')
+ expect_no_validation_error('test' => 'Any')
+ expect_no_validation_error('test' => 'none')
+ expect_no_validation_error('test' => 'any')
+ end
+ end
+
+ context 'invalid parameters' do
+ it 'raises a validation error' do
+ expect_validation_error({ 'test' => 'some_other_string' })
+ end
+ end
+end
diff --git a/spec/lib/banzai/filter/autolink_filter_spec.rb b/spec/lib/banzai/filter/autolink_filter_spec.rb
index 8fba72a23f6..be6192f9ead 100644
--- a/spec/lib/banzai/filter/autolink_filter_spec.rb
+++ b/spec/lib/banzai/filter/autolink_filter_spec.rb
@@ -192,11 +192,6 @@ describe Banzai::Filter::AutolinkFilter do
expect(doc.text).to eq "See <<<#{link}>>>"
end
- it 'accepts link_attr options' do
- doc = filter("See #{link}", link_attr: { class: 'custom' })
- expect(doc.at_css('a')['class']).to eq 'custom'
- end
-
it 'escapes RTLO and other characters' do
# rendered text looks like "http://example.com/evilexe.mp3"
evil_link = "#{link}evil\u202E3pm.exe"
diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb
index 9c2f0e910b1..f25383ef416 100644
--- a/spec/lib/gitlab/git/blob_spec.rb
+++ b/spec/lib/gitlab/git/blob_spec.rb
@@ -409,37 +409,43 @@ describe Gitlab::Git::Blob, :seed_helper do
end
end
- describe 'encoding' do
+ describe 'encoding', :aggregate_failures do
context 'file with russian text' do
let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "encoding/russian.rb") }
- it { expect(blob.name).to eq("russian.rb") }
- it { expect(blob.data.lines.first).to eq("Хороший файл") }
- it { expect(blob.size).to eq(23) }
- it { expect(blob.truncated?).to be_falsey }
- # Run it twice since data is encoded after the first run
- it { expect(blob.truncated?).to be_falsey }
- it { expect(blob.mode).to eq("100755") }
+ it 'has the correct blob attributes' do
+ expect(blob.name).to eq("russian.rb")
+ expect(blob.data.lines.first).to eq("Хороший файл")
+ expect(blob.size).to eq(23)
+ expect(blob.truncated?).to be_falsey
+ # Run it twice since data is encoded after the first run
+ expect(blob.truncated?).to be_falsey
+ expect(blob.mode).to eq("100755")
+ end
end
context 'file with Japanese text' do
let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "encoding/テスト.txt") }
- it { expect(blob.name).to eq("テスト.txt") }
- it { expect(blob.data).to include("これはテスト") }
- it { expect(blob.size).to eq(340) }
- it { expect(blob.mode).to eq("100755") }
- it { expect(blob.truncated?).to be_falsey }
+ it 'has the correct blob attributes' do
+ expect(blob.name).to eq("テスト.txt")
+ expect(blob.data).to include("これはテスト")
+ expect(blob.size).to eq(340)
+ expect(blob.mode).to eq("100755")
+ expect(blob.truncated?).to be_falsey
+ end
end
context 'file with ISO-8859 text' do
let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::LastCommit::ID, "encoding/iso8859.txt") }
- it { expect(blob.name).to eq("iso8859.txt") }
- it { expect(blob.loaded_size).to eq(4) }
- it { expect(blob.size).to eq(4) }
- it { expect(blob.mode).to eq("100644") }
- it { expect(blob.truncated?).to be_falsey }
+ it 'has the correct blob attributes' do
+ expect(blob.name).to eq("iso8859.txt")
+ expect(blob.loaded_size).to eq(4)
+ expect(blob.size).to eq(4)
+ expect(blob.mode).to eq("100644")
+ expect(blob.truncated?).to be_falsey
+ end
end
end
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index d9d60e02a97..2bf971f553f 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -137,12 +137,6 @@ describe Ability do
expect(users_for_snippet(snippet)).to match_array([author])
end
- it 'internal snippet is readable by all registered users' do
- snippet = create(:personal_snippet, :public, author: author)
-
- expect(users_for_snippet(snippet)).to match_array(users)
- end
-
it 'public snippet is readable by all users' do
snippet = create(:personal_snippet, :public, author: author)
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 88c26c92417..673b9e5f076 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -3337,7 +3337,7 @@ describe Ci::Build do
end
it "doesn't save timeout" do
- expect { run_job_without_exception }.not_to change { job.reload.ensure_metadata.timeout_source }
+ expect { run_job_without_exception }.not_to change { job.reload.ensure_metadata.timeout }
end
it "doesn't save timeout_source" do
diff --git a/spec/requests/api/deploy_tokens_spec.rb b/spec/requests/api/deploy_tokens_spec.rb
index a885e80fd55..5948c3d719f 100644
--- a/spec/requests/api/deploy_tokens_spec.rb
+++ b/spec/requests/api/deploy_tokens_spec.rb
@@ -175,8 +175,12 @@ describe API::DeployTokens do
it { is_expected.to have_gitlab_http_status(:no_content) }
- it 'deletes the deploy token' do
- expect { subject }.to change { project.deploy_tokens.count }.by(-1)
+ it 'calls the deploy token destroy service' do
+ expect(::Projects::DeployTokens::DestroyService).to receive(:new)
+ .with(project, user, token_id: deploy_token.id)
+ .and_return(true)
+
+ subject
end
context 'invalid request' do
@@ -187,9 +191,13 @@ describe API::DeployTokens do
end
it 'returns bad_request with invalid token id' do
- delete api("/projects/#{project.id}/deploy_tokens/123abc", user)
+ expect(::Projects::DeployTokens::DestroyService).to receive(:new)
+ .with(project, user, token_id: 999)
+ .and_raise(ActiveRecord::RecordNotFound)
+
+ delete api("/projects/#{project.id}/deploy_tokens/999", user)
- expect(response).to have_gitlab_http_status(:bad_request)
+ expect(response).to have_gitlab_http_status(:not_found)
end
end
end
@@ -307,10 +315,12 @@ describe API::DeployTokens do
group.add_maintainer(user)
end
- it 'deletes the deploy token' do
- expect { subject }.to change { group.deploy_tokens.count }.by(-1)
+ it 'calls the deploy token destroy service' do
+ expect(::Groups::DeployTokens::DestroyService).to receive(:new)
+ .with(group, user, token_id: group_deploy_token.id)
+ .and_return(true)
- expect(group.deploy_tokens).to be_empty
+ subject
end
context 'invalid request' do
@@ -321,7 +331,11 @@ describe API::DeployTokens do
end
it 'returns not found with invalid deploy token id' do
- delete api("/groups/#{group.id}/deploy_tokens/bad_id", user)
+ expect(::Groups::DeployTokens::DestroyService).to receive(:new)
+ .with(group, user, token_id: 999)
+ .and_raise(ActiveRecord::RecordNotFound)
+
+ delete api("/groups/#{group.id}/deploy_tokens/999", user)
expect(response).to have_gitlab_http_status(:not_found)
end
diff --git a/spec/serializers/diffs_entity_spec.rb b/spec/serializers/diffs_entity_spec.rb
index bb4ac5f9608..b42240037df 100644
--- a/spec/serializers/diffs_entity_spec.rb
+++ b/spec/serializers/diffs_entity_spec.rb
@@ -26,5 +26,47 @@ describe DiffsEntity do
:merge_request_diffs, :definition_path_prefix
)
end
+
+ context "when a commit_id is passed" do
+ let(:commits) { merge_request.commits }
+ let(:entity) do
+ described_class.new(
+ merge_request_diffs.first.diffs,
+ request: request,
+ merge_request: merge_request,
+ merge_request_diffs: merge_request_diffs,
+ commit: commit
+ )
+ end
+
+ subject { entity.as_json }
+
+ context "when the passed commit is not the first or last in the group" do
+ let(:commit) { commits.third }
+
+ it 'includes commit references for previous and next' do
+ expect(subject[:commit][:prev_commit_id]).to eq(commits.second.id)
+ expect(subject[:commit][:next_commit_id]).to eq(commits.fourth.id)
+ end
+ end
+
+ context "when the passed commit is the first in the group" do
+ let(:commit) { commits.first }
+
+ it 'includes commit references for nil and next' do
+ expect(subject[:commit][:prev_commit_id]).to be_nil
+ expect(subject[:commit][:next_commit_id]).to eq(commits.second.id)
+ end
+ end
+
+ context "when the passed commit is the last in the group" do
+ let(:commit) { commits.last }
+
+ it 'includes commit references for previous and nil' do
+ expect(subject[:commit][:prev_commit_id]).to eq(commits[-2].id)
+ expect(subject[:commit][:next_commit_id]).to be_nil
+ end
+ end
+ end
end
end
diff --git a/spec/services/groups/deploy_tokens/destroy_service_spec.rb b/spec/services/groups/deploy_tokens/destroy_service_spec.rb
new file mode 100644
index 00000000000..d4ef5963558
--- /dev/null
+++ b/spec/services/groups/deploy_tokens/destroy_service_spec.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::DeployTokens::DestroyService do
+ it_behaves_like 'a deploy token deletion service' do
+ let_it_be(:entity) { create(:group) }
+ let_it_be(:deploy_token_class) { GroupDeployToken }
+ let_it_be(:deploy_token) { create(:deploy_token, :group, groups: [entity]) }
+ end
+end
diff --git a/spec/services/projects/deploy_tokens/destroy_service_spec.rb b/spec/services/projects/deploy_tokens/destroy_service_spec.rb
new file mode 100644
index 00000000000..24407f46615
--- /dev/null
+++ b/spec/services/projects/deploy_tokens/destroy_service_spec.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::DeployTokens::DestroyService do
+ it_behaves_like 'a deploy token deletion service' do
+ let_it_be(:entity) { create(:project) }
+ let_it_be(:deploy_token_class) { ProjectDeployToken }
+ let_it_be(:deploy_token) { create(:deploy_token, projects: [entity]) }
+ end
+end
diff --git a/spec/services/resource_events/synthetic_milestone_notes_builder_service_spec.rb b/spec/services/resource_events/synthetic_milestone_notes_builder_service_spec.rb
new file mode 100644
index 00000000000..e98b8bd00dc
--- /dev/null
+++ b/spec/services/resource_events/synthetic_milestone_notes_builder_service_spec.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ResourceEvents::SyntheticMilestoneNotesBuilderService do
+ describe '#execute' do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:issue) { create(:issue, author: user) }
+
+ before do
+ create_list(:resource_milestone_event, 3, issue: issue)
+
+ stub_feature_flags(track_resource_milestone_change_events: false)
+ end
+
+ context 'when resource milestone events are disabled' do
+ # https://gitlab.com/gitlab-org/gitlab/-/issues/212985
+ it 'still builds notes for existing resource milestone events' do
+ notes = described_class.new(issue, user).execute
+
+ expect(notes.size).to eq(3)
+ end
+ end
+ end
+end
diff --git a/spec/support/helpers/api_validators_helpers.rb b/spec/support/helpers/api_validators_helpers.rb
new file mode 100644
index 00000000000..dc05e5afd57
--- /dev/null
+++ b/spec/support/helpers/api_validators_helpers.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module ApiValidatorsHelpers
+ def scope
+ Struct.new(:opts) do
+ def full_name(attr_name)
+ attr_name
+ end
+ end
+ end
+
+ def expect_no_validation_error(params)
+ expect { validate_test_param!(params) }.not_to raise_error
+ end
+
+ def expect_validation_error(params)
+ expect { validate_test_param!(params) }.to raise_error(Grape::Exceptions::Validation)
+ end
+
+ def validate_test_param!(params)
+ subject.validate_param!('test', params)
+ end
+end
diff --git a/spec/support/services/deploy_token_shared_examples.rb b/spec/support/services/deploy_token_shared_examples.rb
index 70efd1fcd0c..9d681970739 100644
--- a/spec/support/services/deploy_token_shared_examples.rb
+++ b/spec/support/services/deploy_token_shared_examples.rb
@@ -58,3 +58,25 @@ RSpec.shared_examples 'a deploy token creation service' do
end
end
end
+
+RSpec.shared_examples 'a deploy token deletion service' do
+ let(:user) { create(:user) }
+ let(:deploy_token_params) { { token_id: deploy_token.id } }
+
+ describe '#execute' do
+ subject { described_class.new(entity, user, deploy_token_params).execute }
+
+ it "destroys a token record and it's associated DeployToken" do
+ expect { subject }.to change { deploy_token_class.count }.by(-1)
+ .and change { DeployToken.count }.by(-1)
+ end
+
+ context 'invalid token id' do
+ let(:deploy_token_params) { { token_id: 9999 } }
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+ end
+ end
+end