Update patch to update grape dependency (from upstream repo)

This commit is contained in:
Pirate Praveen 2020-05-30 17:02:49 +05:30
parent e28ade6223
commit b3a4056897

View file

@ -1,34 +1,52 @@
From a9ffc8f1bc91ab7d837dfdfd50801a6a35f04717 Mon Sep 17 00:00:00 2001 From 7c07471b98d105724cb6d6b4cd6853bd2ee9350f Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com> From: Stan Hu <stanhu@gmail.com>
Date: Tue, 21 Apr 2020 23:31:35 -0700 Date: Fri, 29 May 2020 16:12:45 -0700
Subject: [PATCH 1/2] Upgrade Grape v1.1.0 to v1.3.2 Subject: [PATCH 1/4] Upgrade to Grape v1.3.3
This brings in Ruby 2.7 suport and a number of fixes: This brings back many of the changes in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/27276. This was
reverted due to some failures in the QA tests with nil parameters.
This brings in Ruby 2.7 support and a number of fixes:
https://github.com/ruby-grape/grape/blob/master/CHANGELOG.md https://github.com/ruby-grape/grape/blob/master/CHANGELOG.md
1. Move all inherited Grape::API -> Grape::API::Instance 1. Move all inherited `Grape::API` -> `Grape::API::Instance`
2. Remove use of Virtus since this has been removed from Grape. 2. Remove use of Virtus since this has been removed from Grape.
3. Extract Rack::Response from API error 3. Extract `Rack::Response` from API error
4. Grape v1.2.3 pulled in a fix used in SafeFile: 4. Grape v1.2.3 pulled in a fix used in `SafeFile`:
https://github.com/ruby-grape/grape/pull/1844, so we no longer need https://github.com/ruby-grape/grape/pull/1844, so we no longer need
to maintain our custom type. to maintain our custom type.
5. Adapt WorkhorseFile with the latest changes to make custom types work 5. Adapt `WorkhorseFile` with the latest changes to make custom types work
with Grape and dry-types. with Grape and dry-types.
6. Ensure Array[String] is coerced properly. 6. Ensure `Array[String]` is coerced properly.
The change from Virtus to dry-types now requires all strings to be The change from Virtus to dry-types now requires all strings to be
coerced to arrays. Before this was done within Virtus. coerced to arrays. Before this was done within Virtus.
7. Coerce Array[Integer] types to arrays of integers 7. Coerce `Array[Integer]` types to arrays of integers
The change from Virtus to dry-types now requires all strings to be The change from Virtus to dry-types now requires all strings to be
coerced to arrays of integers. Before this was done within Virtus. coerced to arrays of integers. Before this was done within Virtus.
This merge request also introduces two Rubocop rules for Grape v1.3:
1. `Grape::API::Instance` instead of `Grape::API` is required, unless we
solve https://gitlab.com/gitlab-org/gitlab/-/issues/215230.
2. Grape parameters defined with `Array` types (e.g. `Array[String]`,
`Array[Integer]`) must have a `coerce_with` block or they will fail to
parse properly. See
https://github.com/ruby-grape/grape/blob/master/UPGRADING.md for more
details.
Closes https://gitlab.com/gitlab-org/gitlab/-/issues/195960
--- ---
.rubocop.yml | 12 +++
Gemfile | 4 +- Gemfile | 4 +-
Gemfile.lock | 55 +++++++++++-------- Gemfile.lock | 55 +++++++-----
changelogs/unreleased/sh-update-grape-gem.yml | 5 ++ changelogs/unreleased/sh-update-grape-gem.yml | 5 ++
doc/development/api_styleguide.md | 8 +++ doc/development/api_styleguide.md | 8 ++
doc/development/ee_features.md | 12 ++-- doc/development/ee_features.md | 12 +--
ee/lib/api/analytics/code_review_analytics.rb | 2 +- ee/lib/api/analytics/code_review_analytics.rb | 2 +-
.../api/analytics/group_activity_analytics.rb | 2 +- .../api/analytics/group_activity_analytics.rb | 2 +-
ee/lib/api/audit_events.rb | 2 +- ee/lib/api/audit_events.rb | 2 +-
@ -39,23 +57,25 @@ coerced to arrays of integers. Before this was done within Virtus.
.../api/elasticsearch_indexed_namespaces.rb | 2 +- .../api/elasticsearch_indexed_namespaces.rb | 2 +-
ee/lib/api/epic_issues.rb | 2 +- ee/lib/api/epic_issues.rb | 2 +-
ee/lib/api/epic_links.rb | 2 +- ee/lib/api/epic_links.rb | 2 +-
ee/lib/api/epics.rb | 8 +-- ee/lib/api/epics.rb | 8 +-
ee/lib/api/feature_flag_scopes.rb | 2 +- ee/lib/api/feature_flag_scopes.rb | 2 +-
ee/lib/api/feature_flags.rb | 2 +- ee/lib/api/feature_flags.rb | 2 +-
ee/lib/api/feature_flags_user_lists.rb | 2 +- ee/lib/api/feature_flags_user_lists.rb | 2 +-
ee/lib/api/geo.rb | 2 +- ee/lib/api/geo.rb | 2 +-
ee/lib/api/geo_nodes.rb | 2 +- ee/lib/api/geo_nodes.rb | 2 +-
ee/lib/api/geo_replication.rb | 2 +- ee/lib/api/geo_replication.rb | 2 +-
ee/lib/api/go_proxy.rb | 2 +-
ee/lib/api/group_hooks.rb | 2 +- ee/lib/api/group_hooks.rb | 2 +-
ee/lib/api/group_packages.rb | 2 +- ee/lib/api/group_packages.rb | 2 +-
.../helpers/project_approval_rules_helpers.rb | 14 ++--
ee/lib/api/issue_links.rb | 2 +- ee/lib/api/issue_links.rb | 2 +-
ee/lib/api/ldap.rb | 2 +- ee/lib/api/ldap.rb | 2 +-
ee/lib/api/ldap_group_links.rb | 2 +- ee/lib/api/ldap_group_links.rb | 2 +-
ee/lib/api/license.rb | 2 +- ee/lib/api/license.rb | 2 +-
ee/lib/api/managed_licenses.rb | 2 +- ee/lib/api/managed_licenses.rb | 2 +-
ee/lib/api/maven_packages.rb | 2 +- ee/lib/api/maven_packages.rb | 2 +-
ee/lib/api/merge_request_approval_rules.rb | 2 +- ee/lib/api/merge_request_approval_rules.rb | 12 ++-
ee/lib/api/merge_request_approvals.rb | 2 +- ee/lib/api/merge_request_approvals.rb | 10 +--
ee/lib/api/merge_trains.rb | 2 +- ee/lib/api/merge_trains.rb | 2 +-
ee/lib/api/npm_packages.rb | 2 +- ee/lib/api/npm_packages.rb | 2 +-
ee/lib/api/nuget_packages.rb | 2 +- ee/lib/api/nuget_packages.rb | 2 +-
@ -63,7 +83,7 @@ coerced to arrays of integers. Before this was done within Virtus.
ee/lib/api/project_aliases.rb | 2 +- ee/lib/api/project_aliases.rb | 2 +-
ee/lib/api/project_approval_rules.rb | 2 +- ee/lib/api/project_approval_rules.rb | 2 +-
ee/lib/api/project_approval_settings.rb | 2 +- ee/lib/api/project_approval_settings.rb | 2 +-
ee/lib/api/project_approvals.rb | 2 +- ee/lib/api/project_approvals.rb | 8 +-
ee/lib/api/project_mirror.rb | 2 +- ee/lib/api/project_mirror.rb | 2 +-
ee/lib/api/project_packages.rb | 2 +- ee/lib/api/project_packages.rb | 2 +-
ee/lib/api/project_push_rule.rb | 2 +- ee/lib/api/project_push_rule.rb | 2 +-
@ -75,16 +95,22 @@ coerced to arrays of integers. Before this was done within Virtus.
ee/lib/api/visual_review_discussions.rb | 2 +- ee/lib/api/visual_review_discussions.rb | 2 +-
ee/lib/api/vulnerabilities.rb | 2 +- ee/lib/api/vulnerabilities.rb | 2 +-
ee/lib/api/vulnerability_exports.rb | 2 +- ee/lib/api/vulnerability_exports.rb | 2 +-
ee/lib/api/vulnerability_findings.rb | 8 ++- ee/lib/api/vulnerability_findings.rb | 8 +-
ee/lib/api/vulnerability_issue_links.rb | 2 +- ee/lib/api/vulnerability_issue_links.rb | 2 +-
ee/lib/ee/api/boards.rb | 2 +- ee/lib/ee/api/boards.rb | 2 +-
ee/lib/ee/api/group_boards.rb | 2 +- ee/lib/ee/api/group_boards.rb | 2 +-
ee/lib/ee/api/helpers/settings_helpers.rb | 6 +- ee/lib/ee/api/helpers/settings_helpers.rb | 6 +-
ee/spec/lib/ee/api/helpers_spec.rb | 2 +- ee/spec/lib/ee/api/helpers_spec.rb | 2 +-
.../api/merge_request_approval_rules_spec.rb | 34 +++++---
.../api/merge_request_approvals_spec.rb | 10 +--
.../api/project_approval_rules_spec.rb | 1 +
.../api/project_approval_settings_spec.rb | 1 +
...ject_approval_rules_api_shared_examples.rb | 7 +-
lib/api/access_requests.rb | 2 +- lib/api/access_requests.rb | 2 +-
lib/api/admin/ci/variables.rb | 2 +-
lib/api/admin/sidekiq.rb | 2 +- lib/api/admin/sidekiq.rb | 2 +-
lib/api/api.rb | 2 +- lib/api/api.rb | 2 +-
lib/api/api_guard.rb | 11 +++- lib/api/api_guard.rb | 11 ++-
lib/api/appearance.rb | 2 +- lib/api/appearance.rb | 2 +-
lib/api/applications.rb | 2 +- lib/api/applications.rb | 2 +-
lib/api/avatar.rb | 2 +- lib/api/avatar.rb | 2 +-
@ -105,6 +131,7 @@ coerced to arrays of integers. Before this was done within Virtus.
lib/api/events.rb | 2 +- lib/api/events.rb | 2 +-
lib/api/features.rb | 2 +- lib/api/features.rb | 2 +-
lib/api/files.rb | 2 +- lib/api/files.rb | 2 +-
lib/api/freeze_periods.rb | 2 +-
lib/api/group_boards.rb | 2 +- lib/api/group_boards.rb | 2 +-
lib/api/group_clusters.rb | 2 +- lib/api/group_clusters.rb | 2 +-
lib/api/group_container_repositories.rb | 2 +- lib/api/group_container_repositories.rb | 2 +-
@ -119,18 +146,18 @@ coerced to arrays of integers. Before this was done within Virtus.
lib/api/import_github.rb | 2 +- lib/api/import_github.rb | 2 +-
lib/api/internal/base.rb | 2 +- lib/api/internal/base.rb | 2 +-
lib/api/internal/pages.rb | 2 +- lib/api/internal/pages.rb | 2 +-
lib/api/issues.rb | 10 ++-- lib/api/issues.rb | 14 ++--
lib/api/job_artifacts.rb | 2 +- lib/api/job_artifacts.rb | 2 +-
lib/api/jobs.rb | 2 +- lib/api/jobs.rb | 2 +-
lib/api/keys.rb | 2 +- lib/api/keys.rb | 2 +-
lib/api/labels.rb | 2 +- lib/api/labels.rb | 2 +-
lib/api/lint.rb | 2 +- lib/api/lint.rb | 2 +-
lib/api/lsif_data.rb | 2 +-
lib/api/markdown.rb | 2 +- lib/api/markdown.rb | 2 +-
lib/api/members.rb | 6 +- lib/api/members.rb | 6 +-
lib/api/merge_request_diffs.rb | 2 +- lib/api/merge_request_diffs.rb | 2 +-
lib/api/merge_requests.rb | 8 +-- lib/api/merge_requests.rb | 12 +--
lib/api/metrics/dashboard/annotations.rb | 2 +- lib/api/metrics/dashboard/annotations.rb | 2 +-
lib/api/metrics/user_starred_dashboards.rb | 2 +-
lib/api/milestone_responses.rb | 2 +- lib/api/milestone_responses.rb | 2 +-
lib/api/namespaces.rb | 2 +- lib/api/namespaces.rb | 2 +-
lib/api/notes.rb | 2 +- lib/api/notes.rb | 2 +-
@ -147,6 +174,7 @@ coerced to arrays of integers. Before this was done within Virtus.
lib/api/project_hooks.rb | 2 +- lib/api/project_hooks.rb | 2 +-
lib/api/project_import.rb | 2 +- lib/api/project_import.rb | 2 +-
lib/api/project_milestones.rb | 2 +- lib/api/project_milestones.rb | 2 +-
lib/api/project_repository_storage_moves.rb | 2 +-
lib/api/project_snapshots.rb | 2 +- lib/api/project_snapshots.rb | 2 +-
lib/api/project_snippets.rb | 2 +- lib/api/project_snippets.rb | 2 +-
lib/api/project_statistics.rb | 2 +- lib/api/project_statistics.rb | 2 +-
@ -159,11 +187,12 @@ coerced to arrays of integers. Before this was done within Virtus.
lib/api/remote_mirrors.rb | 2 +- lib/api/remote_mirrors.rb | 2 +-
lib/api/repositories.rb | 4 +- lib/api/repositories.rb | 4 +-
lib/api/resource_label_events.rb | 2 +- lib/api/resource_label_events.rb | 2 +-
lib/api/resource_milestone_events.rb | 2 +-
lib/api/runner.rb | 4 +- lib/api/runner.rb | 4 +-
lib/api/runners.rb | 12 ++-- lib/api/runners.rb | 12 +--
lib/api/search.rb | 2 +- lib/api/search.rb | 2 +-
lib/api/services.rb | 2 +- lib/api/services.rb | 2 +-
lib/api/settings.rb | 11 ++-- lib/api/settings.rb | 11 +--
lib/api/sidekiq_metrics.rb | 2 +- lib/api/sidekiq_metrics.rb | 2 +-
lib/api/snippets.rb | 2 +- lib/api/snippets.rb | 2 +-
lib/api/statistics.rb | 2 +- lib/api/statistics.rb | 2 +-
@ -179,21 +208,50 @@ coerced to arrays of integers. Before this was done within Virtus.
lib/api/user_counts.rb | 2 +- lib/api/user_counts.rb | 2 +-
lib/api/users.rb | 2 +- lib/api/users.rb | 2 +-
.../types/comma_separated_to_array.rb | 2 +- .../types/comma_separated_to_array.rb | 2 +-
.../types/comma_separated_to_integer_array.rb | 15 +++++ .../types/comma_separated_to_integer_array.rb | 15 ++++
lib/api/validations/types/labels_list.rb | 24 -------- lib/api/validations/types/labels_list.rb | 24 ------
lib/api/validations/types/safe_file.rb | 15 ----- lib/api/validations/types/safe_file.rb | 15 ----
lib/api/validations/types/workhorse_file.rb | 13 ++--- lib/api/validations/types/workhorse_file.rb | 13 ++-
lib/api/variables.rb | 2 +- lib/api/variables.rb | 2 +-
lib/api/version.rb | 2 +- lib/api/version.rb | 2 +-
lib/api/wikis.rb | 4 +- lib/api/wikis.rb | 4 +-
spec/requests/api/settings_spec.rb | 10 +++- rubocop/cop/api/grape_api_instance.rb | 42 ++++++++++
rubocop/cop/api/grape_array_missing_coerce.rb | 83 +++++++++++++++++++
spec/requests/api/settings_spec.rb | 10 ++-
.../cop/api/grape_api_instance_spec.rb | 31 +++++++
.../api/grape_array_missing_coerce_spec.rb | 64 ++++++++++++++
spec/rubocop/cop/code_reuse/worker_spec.rb | 2 +- spec/rubocop/cop/code_reuse/worker_spec.rb | 2 +-
164 files changed, 282 insertions(+), 264 deletions(-) 180 files changed, 571 insertions(+), 316 deletions(-)
create mode 100644 changelogs/unreleased/sh-update-grape-gem.yml create mode 100644 changelogs/unreleased/sh-update-grape-gem.yml
create mode 100644 lib/api/validations/types/comma_separated_to_integer_array.rb create mode 100644 lib/api/validations/types/comma_separated_to_integer_array.rb
delete mode 100644 lib/api/validations/types/labels_list.rb delete mode 100644 lib/api/validations/types/labels_list.rb
delete mode 100644 lib/api/validations/types/safe_file.rb delete mode 100644 lib/api/validations/types/safe_file.rb
create mode 100644 rubocop/cop/api/grape_api_instance.rb
create mode 100644 rubocop/cop/api/grape_array_missing_coerce.rb
create mode 100644 spec/rubocop/cop/api/grape_api_instance_spec.rb
create mode 100644 spec/rubocop/cop/api/grape_array_missing_coerce_spec.rb
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -292,6 +292,18 @@
- 'spec/**/*'
- 'ee/spec/**/*'
+API/GrapeAPIInstance:
+ Enabled: true
+ Include:
+ - 'lib/**/api/**/*.rb'
+ - 'ee/**/api/**/*.rb'
+
+API/GrapeArrayMissingCoerce:
+ Enabled: true
+ Include:
+ - 'lib/**/api/**/*.rb'
+ - 'ee/**/api/**/*.rb'
+
Cop/SidekiqOptionsQueue:
Enabled: true
Exclude:
--- a/Gemfile --- a/Gemfile
+++ b/Gemfile +++ b/Gemfile
@@ -19,7 +19,7 @@ @@ -19,7 +19,7 @@
@ -205,10 +263,12 @@ coerced to arrays of integers. Before this was done within Virtus.
gem 'faraday', '~> 0.12' gem 'faraday', '~> 0.12'
gem 'marginalia', '~> 1.8' gem 'marginalia', '~> 1.8'
@@ -80,6 +80,7 @@ @@ -79,7 +79,8 @@
gem 'net-ldap'
# API # API
gem 'grape', '~> 1.1' -gem 'grape', '~> 1.1'
+gem 'grape', '~> 1.3', '>= 1.3.3'
+gem 'rack-timeout' +gem 'rack-timeout'
gem 'grape-entity', '~> 0.7.1' gem 'grape-entity', '~> 0.7.1'
gem 'rack-cors', '~> 1.0', '>= 1.0.6', require: 'rack/cors' gem 'rack-cors', '~> 1.0', '>= 1.0.6', require: 'rack/cors'
@ -278,7 +338,7 @@ coerced to arrays of integers. Before this was done within Virtus.
gpgme (2.0.20) gpgme (2.0.20)
mini_portile2 (~> 2.3) mini_portile2 (~> 2.3)
- grape (1.1.0) - grape (1.1.0)
+ grape (1.3.2) + grape (1.3.3)
activesupport activesupport
builder builder
+ dry-types (>= 1.1) + dry-types (>= 1.1)
@ -336,7 +396,7 @@ coerced to arrays of integers. Before this was done within Virtus.
google-protobuf (~> 3.8.0) google-protobuf (~> 3.8.0)
gpgme (~> 2.0.19) gpgme (~> 2.0.19)
- grape (~> 1.1.0) - grape (~> 1.1.0)
+ grape (~> 1.3.2) + grape (~> 1.3.3)
grape-entity (~> 0.7.1) grape-entity (~> 0.7.1)
- grape-path-helpers (~> 1.2) - grape-path-helpers (~> 1.2)
+ grape-path-helpers (~> 1.3) + grape-path-helpers (~> 1.3)
@ -347,8 +407,8 @@ coerced to arrays of integers. Before this was done within Virtus.
+++ b/changelogs/unreleased/sh-update-grape-gem.yml +++ b/changelogs/unreleased/sh-update-grape-gem.yml
@@ -0,0 +1,5 @@ @@ -0,0 +1,5 @@
+--- +---
+title: Upgrade Grape v1.1.0 to v1.3.2 +title: Upgrade Grape v1.1.0 to v1.3.3
+merge_request: 27276 +merge_request: 33450
+author: +author:
+type: other +type: other
--- a/doc/development/api_styleguide.md --- a/doc/development/api_styleguide.md
@ -370,6 +430,21 @@ coerced to arrays of integers. Before this was done within Virtus.
For non-200 HTTP responses, use the provided helpers in `lib/api/helpers.rb` to ensure correct behavior (`not_found!`, `no_content!` etc.). These will `throw` inside Grape and abort the execution of your endpoint. For non-200 HTTP responses, use the provided helpers in `lib/api/helpers.rb` to ensure correct behavior (`not_found!`, `no_content!` etc.). These will `throw` inside Grape and abort the execution of your endpoint.
--- a/doc/development/ee_features.md --- a/doc/development/ee_features.md
+++ b/doc/development/ee_features.md +++ b/doc/development/ee_features.md
@@ -514,12 +514,12 @@
interface first here.
For example, suppose we have a few more optional parameters for EE. We can move the
-parameters out of the `Grape::API` class to a helper module, so we can inject it
+parameters out of the `Grape::API::Instance` class to a helper module, so we can inject it
before it would be used in the class.
```ruby
module API
- class Projects < Grape::API
+ class Projects < Grape::API::Instance
helpers Helpers::ProjectsHelpers
end
end
@@ -580,7 +580,7 @@ @@ -580,7 +580,7 @@
```ruby ```ruby
@ -417,6 +492,17 @@ coerced to arrays of integers. Before this was done within Virtus.
include PaginationParams include PaginationParams
before { authenticate! } before { authenticate! }
--- a/lib/api/admin/ci/variables.rb
+++ b/lib/api/admin/ci/variables.rb
@@ -3,7 +3,7 @@
module API
module Admin
module Ci
- class Variables < Grape::API
+ class Variables < Grape::API::Instance
include PaginationParams
before { authenticated_as_admin! }
--- a/lib/api/admin/sidekiq.rb --- a/lib/api/admin/sidekiq.rb
+++ b/lib/api/admin/sidekiq.rb +++ b/lib/api/admin/sidekiq.rb
@@ -2,7 +2,7 @@ @@ -2,7 +2,7 @@
@ -607,6 +693,15 @@ coerced to arrays of integers. Before this was done within Virtus.
params do params do
requires :name, type: String, desc: "New deploy token's name" requires :name, type: String, desc: "New deploy token's name"
- requires :scopes, type: Array[String], values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s), - requires :scopes, type: Array[String], values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s),
+ requires :scopes, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s),
desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository", "read_registry", "write_registry", "read_package_registry", or "write_package_registry".'
optional :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.'
optional :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`'
@@ -119,7 +119,7 @@
params do
requires :name, type: String, desc: 'The name of the deploy token'
- requires :scopes, type: Array[String], values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s),
+ requires :scopes, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s), + requires :scopes, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s),
desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository", "read_registry", "write_registry", "read_package_registry", or "write_package_registry".' desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository", "read_registry", "write_registry", "read_package_registry", or "write_package_registry".'
optional :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.' optional :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.'
@ -688,6 +783,17 @@ coerced to arrays of integers. Before this was done within Virtus.
include APIGuard include APIGuard
FILE_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(file_path: API::NO_SLASH_URL_PART_REGEX) FILE_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(file_path: API::NO_SLASH_URL_PART_REGEX)
--- a/lib/api/freeze_periods.rb
+++ b/lib/api/freeze_periods.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
module API
- class FreezePeriods < Grape::API
+ class FreezePeriods < Grape::API::Instance
include PaginationParams
before { authenticate! }
--- a/lib/api/group_boards.rb --- a/lib/api/group_boards.rb
+++ b/lib/api/group_boards.rb +++ b/lib/api/group_boards.rb
@@ -1,7 +1,7 @@ @@ -1,7 +1,7 @@
@ -883,7 +989,7 @@ coerced to arrays of integers. Before this was done within Virtus.
optional :assignee_id, type: Integer, desc: '[Deprecated] The ID of a user to assign issue' optional :assignee_id, type: Integer, desc: '[Deprecated] The ID of a user to assign issue'
optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue' optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue'
- optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names' - optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
+ optional :labels, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' + optional :labels, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Comma-separated list of label names'
optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY' optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY'
optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential' optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential'
optional :discussion_locked, type: Boolean, desc: " Boolean parameter indicating if the issue's discussion is locked" optional :discussion_locked, type: Boolean, desc: " Boolean parameter indicating if the issue's discussion is locked"
@ -942,17 +1048,6 @@ coerced to arrays of integers. Before this was done within Virtus.
namespace :ci do namespace :ci do
desc 'Validation of .gitlab-ci.yml content' desc 'Validation of .gitlab-ci.yml content'
params do params do
--- a/lib/api/lsif_data.rb
+++ b/lib/api/lsif_data.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
module API
- class LsifData < Grape::API
+ class LsifData < Grape::API::Instance
MAX_FILE_SIZE = 10.megabytes
before do
--- a/lib/api/markdown.rb --- a/lib/api/markdown.rb
+++ b/lib/api/markdown.rb +++ b/lib/api/markdown.rb
@@ -1,7 +1,7 @@ @@ -1,7 +1,7 @@
@ -1015,7 +1110,7 @@ coerced to arrays of integers. Before this was done within Virtus.
include PaginationParams include PaginationParams
CONTEXT_COMMITS_POST_LIMIT = 20 CONTEXT_COMMITS_POST_LIMIT = 20
@@ -182,9 +182,9 @@ @@ -182,11 +182,11 @@
params :optional_params do params :optional_params do
optional :description, type: String, desc: 'The description of the merge request' optional :description, type: String, desc: 'The description of the merge request'
optional :assignee_id, type: Integer, desc: 'The ID of a user to assign the merge request' optional :assignee_id, type: Integer, desc: 'The ID of a user to assign the merge request'
@ -1023,10 +1118,14 @@ coerced to arrays of integers. Before this was done within Virtus.
+ optional :assignee_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The array of user IDs to assign issue' + optional :assignee_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The array of user IDs to assign issue'
optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request' optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request'
- optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names' - optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
- optional :add_labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
- optional :remove_labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
+ optional :labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' + optional :labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names'
optional :add_labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names' + optional :add_labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names'
optional :remove_labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names' + optional :remove_labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names'
optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging' optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging'
optional :allow_collaboration, type: Boolean, desc: 'Allow commits from members who can merge to the target branch'
optional :allow_maintainer_to_push, type: Boolean, as: :allow_collaboration, desc: '[deprecated] See allow_collaboration'
@@ -201,7 +201,7 @@ @@ -201,7 +201,7 @@
end end
params do params do
@ -1047,6 +1146,17 @@ coerced to arrays of integers. Before this was done within Virtus.
desc 'Create a new monitoring dashboard annotation' do desc 'Create a new monitoring dashboard annotation' do
success Entities::Metrics::Dashboard::Annotation success Entities::Metrics::Dashboard::Annotation
end end
--- a/lib/api/metrics/user_starred_dashboards.rb
+++ b/lib/api/metrics/user_starred_dashboards.rb
@@ -2,7 +2,7 @@
module API
module Metrics
- class UserStarredDashboards < Grape::API
+ class UserStarredDashboards < Grape::API::Instance
resource :projects do
desc 'Marks selected metrics dashboard as starred' do
success Entities::Metrics::UserStarredDashboard
--- a/lib/api/milestone_responses.rb --- a/lib/api/milestone_responses.rb
+++ b/lib/api/milestone_responses.rb +++ b/lib/api/milestone_responses.rb
@@ -15,7 +15,7 @@ @@ -15,7 +15,7 @@
@ -1223,6 +1333,17 @@ coerced to arrays of integers. Before this was done within Virtus.
include PaginationParams include PaginationParams
include MilestoneResponses include MilestoneResponses
--- a/lib/api/project_repository_storage_moves.rb
+++ b/lib/api/project_repository_storage_moves.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
module API
- class ProjectRepositoryStorageMoves < Grape::API
+ class ProjectRepositoryStorageMoves < Grape::API::Instance
include PaginationParams
before { authenticated_as_admin! }
--- a/lib/api/project_snapshots.rb --- a/lib/api/project_snapshots.rb
+++ b/lib/api/project_snapshots.rb +++ b/lib/api/project_snapshots.rb
@@ -1,7 +1,7 @@ @@ -1,7 +1,7 @@
@ -1500,7 +1621,7 @@ coerced to arrays of integers. Before this was done within Virtus.
desc: 'Enabled sources for code import during project creation. OmniAuth must be configured for GitHub, Bitbucket, and GitLab.com' desc: 'Enabled sources for code import during project creation. OmniAuth must be configured for GitHub, Bitbucket, and GitLab.com'
optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size for each job's artifacts" optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size for each job's artifacts"
optional :max_attachment_size, type: Integer, desc: 'Maximum attachment size in MB' optional :max_attachment_size, type: Integer, desc: 'Maximum attachment size in MB'
@@ -112,12 +113,12 @@ @@ -112,7 +113,7 @@
requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha' requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha'
end end
optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues." optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues."
@ -1509,12 +1630,6 @@ coerced to arrays of integers. Before this was done within Virtus.
optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to set up Two-factor authentication' optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to set up Two-factor authentication'
given require_two_factor_authentication: ->(val) { val } do given require_two_factor_authentication: ->(val) { val } do
requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication'
end
- optional :restricted_visibility_levels, type: Array[String], desc: 'Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.'
+ optional :restricted_visibility_levels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.'
optional :send_user_confirmation_email, type: Boolean, desc: 'Send confirmation email on sign-up'
optional :session_expire_delay, type: Integer, desc: 'Session duration in minutes. GitLab restart is required to apply changes.'
optional :shared_runners_enabled, type: Boolean, desc: 'Enable shared runners for new projects'
--- a/lib/api/sidekiq_metrics.rb --- a/lib/api/sidekiq_metrics.rb
+++ b/lib/api/sidekiq_metrics.rb +++ b/lib/api/sidekiq_metrics.rb
@@ -3,7 +3,7 @@ @@ -3,7 +3,7 @@
@ -1809,57 +1924,6 @@ coerced to arrays of integers. Before this was done within Virtus.
optional :branch, type: String, desc: 'The name of the branch' optional :branch, type: String, desc: 'The name of the branch'
end end
post ":id/wikis/attachments" do post ":id/wikis/attachments" do
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -60,14 +60,14 @@
default_projects_limit: 3,
default_project_creation: 2,
password_authentication_enabled_for_web: false,
- repository_storages: ['custom'],
+ repository_storages: 'custom',
plantuml_enabled: true,
plantuml_url: 'http://plantuml.example.com',
sourcegraph_enabled: true,
sourcegraph_url: 'https://sourcegraph.com',
sourcegraph_public_only: false,
default_snippet_visibility: 'internal',
- restricted_visibility_levels: ['public'],
+ restricted_visibility_levels: 'public',
default_artifacts_expire_in: '2 days',
help_page_text: 'custom help text',
help_page_hide_commercial_content: true,
--- a/spec/rubocop/cop/code_reuse/worker_spec.rb
+++ b/spec/rubocop/cop/code_reuse/worker_spec.rb
@@ -31,7 +31,7 @@
.and_return(true)
expect_offense(<<~SOURCE)
- class Foo < Grape::API
+ class Foo < Grape::API::Instance
resource :projects do
get '/' do
FooWorker.perform_async
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -292,6 +292,18 @@
- 'spec/**/*'
- 'ee/spec/**/*'
+API/GrapeAPIInstance:
+ Enabled: true
+ Include:
+ - 'lib/**/api/**/*.rb'
+ - 'ee/**/api/**/*.rb'
+
+API/GrapeArrayMissingCoerce:
+ Enabled: true
+ Include:
+ - 'lib/**/api/**/*.rb'
+ - 'ee/**/api/**/*.rb'
+
Cop/SidekiqOptionsQueue:
Enabled: true
Exclude:
--- /dev/null --- /dev/null
+++ b/rubocop/cop/api/grape_api_instance.rb +++ b/rubocop/cop/api/grape_api_instance.rb
@@ -0,0 +1,42 @@ @@ -0,0 +1,42 @@
@ -1991,6 +2055,25 @@ coerced to arrays of integers. Before this was done within Virtus.
+ end + end
+ end + end
+end +end
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -60,14 +60,14 @@
default_projects_limit: 3,
default_project_creation: 2,
password_authentication_enabled_for_web: false,
- repository_storages: ['custom'],
+ repository_storages: 'custom',
plantuml_enabled: true,
plantuml_url: 'http://plantuml.example.com',
sourcegraph_enabled: true,
sourcegraph_url: 'https://sourcegraph.com',
sourcegraph_public_only: false,
default_snippet_visibility: 'internal',
- restricted_visibility_levels: ['public'],
+ restricted_visibility_levels: 'public',
default_artifacts_expire_in: '2 days',
help_page_text: 'custom help text',
help_page_hide_commercial_content: true,
--- /dev/null --- /dev/null
+++ b/spec/rubocop/cop/api/grape_api_instance_spec.rb +++ b/spec/rubocop/cop/api/grape_api_instance_spec.rb
@@ -0,0 +1,31 @@ @@ -0,0 +1,31 @@
@ -2008,7 +2091,7 @@ coerced to arrays of integers. Before this was done within Virtus.
+ subject(:cop) { described_class.new } + subject(:cop) { described_class.new }
+ +
+ it 'adds an offense when inheriting from Grape::API' do + it 'adds an offense when inheriting from Grape::API' do
+ inspect_source(<<~CODE.strip_indent) + inspect_source(<<~CODE)
+ class SomeAPI < Grape::API + class SomeAPI < Grape::API
+ end + end
+ CODE + CODE
@ -2016,8 +2099,8 @@ coerced to arrays of integers. Before this was done within Virtus.
+ expect(cop.offenses.size).to eq(1) + expect(cop.offenses.size).to eq(1)
+ end + end
+ +
+ it 'adds an offense when inheriting from Grape::API::Instance' do + it 'does not add an offense when inheriting from Grape::API::Instance' do
+ inspect_source(<<~CODE.strip_indent) + inspect_source(<<~CODE)
+ class SomeAPI < Grape::API::Instance + class SomeAPI < Grape::API::Instance
+ end + end
+ CODE + CODE
@ -2042,7 +2125,7 @@ coerced to arrays of integers. Before this was done within Virtus.
+ subject(:cop) { described_class.new } + subject(:cop) { described_class.new }
+ +
+ it 'adds an offense with a required parameter' do + it 'adds an offense with a required parameter' do
+ inspect_source(<<~CODE.strip_indent) + inspect_source(<<~CODE)
+ class SomeAPI < Grape::API::Instance + class SomeAPI < Grape::API::Instance
+ params do + params do
+ requires :values, type: Array[String] + requires :values, type: Array[String]
@ -2054,7 +2137,7 @@ coerced to arrays of integers. Before this was done within Virtus.
+ end + end
+ +
+ it 'adds an offense with an optional parameter' do + it 'adds an offense with an optional parameter' do
+ inspect_source(<<~CODE.strip_indent) + inspect_source(<<~CODE)
+ class SomeAPI < Grape::API::Instance + class SomeAPI < Grape::API::Instance
+ params do + params do
+ optional :values, type: Array[String] + optional :values, type: Array[String]
@ -2066,7 +2149,7 @@ coerced to arrays of integers. Before this was done within Virtus.
+ end + end
+ +
+ it 'does not add an offense' do + it 'does not add an offense' do
+ inspect_source(<<~CODE.strip_indent) + inspect_source(<<~CODE)
+ class SomeAPI < Grape::API::Instance + class SomeAPI < Grape::API::Instance
+ params do + params do
+ requires :values, type: Array[String], coerce_with: ->(val) { val.split(',').map(&:strip) } + requires :values, type: Array[String], coerce_with: ->(val) { val.split(',').map(&:strip) }
@ -2081,7 +2164,7 @@ coerced to arrays of integers. Before this was done within Virtus.
+ end + end
+ +
+ it 'does not add an offense for unrelated classes' do + it 'does not add an offense for unrelated classes' do
+ inspect_source(<<~CODE.strip_indent) + inspect_source(<<~CODE)
+ class SomeClass + class SomeClass
+ params do + params do
+ requires :values, type: Array[String] + requires :values, type: Array[String]
@ -2092,3 +2175,55 @@ coerced to arrays of integers. Before this was done within Virtus.
+ expect(cop.offenses.size).to be_zero + expect(cop.offenses.size).to be_zero
+ end + end
+end +end
--- a/spec/rubocop/cop/code_reuse/worker_spec.rb
+++ b/spec/rubocop/cop/code_reuse/worker_spec.rb
@@ -31,7 +31,7 @@
.and_return(true)
expect_offense(<<~SOURCE)
- class Foo < Grape::API
+ class Foo < Grape::API::Instance
resource :projects do
get '/' do
FooWorker.perform_async
--- /dev/null
+++ b/spec/support/shared_examples/requests/api/issuable_update_shared_examples.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'issuable update endpoint' do
+ let(:area) { entity.class.name.underscore.pluralize }
+
+ describe 'PUT /projects/:id/issues/:issue_id' do
+ let(:url) { "/projects/#{project.id}/#{area}/#{entity.iid}" }
+
+ it 'clears labels when labels param is nil' do
+ put api(url, user), params: { labels: 'label1' }
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response['labels']).to contain_exactly('label1')
+
+ put api(url, user), params: { labels: nil }
+
+ expect(response).to have_gitlab_http_status(:ok)
+ json_response = Gitlab::Json.parse(response.body)
+ expect(json_response['labels']).to be_empty
+ end
+
+ it 'updates the issuable with labels param as array' do
+ stub_const("Gitlab::QueryLimiting::Transaction::THRESHOLD", 110)
+
+ params = { labels: ['label1', 'label2', 'foo, bar', '&,?'] }
+
+ put api(url, user), params: params
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response['labels']).to include 'label1'
+ expect(json_response['labels']).to include 'label2'
+ expect(json_response['labels']).to include 'foo'
+ expect(json_response['labels']).to include 'bar'
+ expect(json_response['labels']).to include '&'
+ expect(json_response['labels']).to include '?'
+ end
+ end
+end