From b3a4056897e9b751eb815aa4d218584d3369259b Mon Sep 17 00:00:00 2001 From: Pirate Praveen Date: Sat, 30 May 2020 17:02:49 +0530 Subject: [PATCH] Update patch to update grape dependency (from upstream repo) --- debian/patches/0760-update-grape.patch | 367 +++++++++++++++++-------- 1 file changed, 251 insertions(+), 116 deletions(-) diff --git a/debian/patches/0760-update-grape.patch b/debian/patches/0760-update-grape.patch index 576119d0d2..ce5c0a0187 100644 --- a/debian/patches/0760-update-grape.patch +++ b/debian/patches/0760-update-grape.patch @@ -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 -Date: Tue, 21 Apr 2020 23:31:35 -0700 -Subject: [PATCH 1/2] Upgrade Grape v1.1.0 to v1.3.2 +Date: Fri, 29 May 2020 16:12:45 -0700 +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 -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. -3. Extract Rack::Response from API error -4. Grape v1.2.3 pulled in a fix used in SafeFile: +3. Extract `Rack::Response` from API error +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 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. -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 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 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.lock | 55 +++++++++++-------- + Gemfile.lock | 55 +++++++----- changelogs/unreleased/sh-update-grape-gem.yml | 5 ++ - doc/development/api_styleguide.md | 8 +++ - doc/development/ee_features.md | 12 ++-- + doc/development/api_styleguide.md | 8 ++ + doc/development/ee_features.md | 12 +-- ee/lib/api/analytics/code_review_analytics.rb | 2 +- .../api/analytics/group_activity_analytics.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 +- ee/lib/api/epic_issues.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_flags.rb | 2 +- ee/lib/api/feature_flags_user_lists.rb | 2 +- ee/lib/api/geo.rb | 2 +- ee/lib/api/geo_nodes.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_packages.rb | 2 +- + .../helpers/project_approval_rules_helpers.rb | 14 ++-- ee/lib/api/issue_links.rb | 2 +- ee/lib/api/ldap.rb | 2 +- ee/lib/api/ldap_group_links.rb | 2 +- ee/lib/api/license.rb | 2 +- ee/lib/api/managed_licenses.rb | 2 +- ee/lib/api/maven_packages.rb | 2 +- - ee/lib/api/merge_request_approval_rules.rb | 2 +- - ee/lib/api/merge_request_approvals.rb | 2 +- + ee/lib/api/merge_request_approval_rules.rb | 12 ++- + ee/lib/api/merge_request_approvals.rb | 10 +-- ee/lib/api/merge_trains.rb | 2 +- ee/lib/api/npm_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_approval_rules.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_packages.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/vulnerabilities.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/ee/api/boards.rb | 2 +- ee/lib/ee/api/group_boards.rb | 2 +- ee/lib/ee/api/helpers/settings_helpers.rb | 6 +- 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/admin/ci/variables.rb | 2 +- lib/api/admin/sidekiq.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/applications.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/features.rb | 2 +- lib/api/files.rb | 2 +- + lib/api/freeze_periods.rb | 2 +- lib/api/group_boards.rb | 2 +- lib/api/group_clusters.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/internal/base.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/jobs.rb | 2 +- lib/api/keys.rb | 2 +- lib/api/labels.rb | 2 +- lib/api/lint.rb | 2 +- - lib/api/lsif_data.rb | 2 +- lib/api/markdown.rb | 2 +- lib/api/members.rb | 6 +- 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/user_starred_dashboards.rb | 2 +- lib/api/milestone_responses.rb | 2 +- lib/api/namespaces.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_import.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_snippets.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/repositories.rb | 4 +- lib/api/resource_label_events.rb | 2 +- + lib/api/resource_milestone_events.rb | 2 +- lib/api/runner.rb | 4 +- - lib/api/runners.rb | 12 ++-- + lib/api/runners.rb | 12 +-- lib/api/search.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/snippets.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/users.rb | 2 +- .../types/comma_separated_to_array.rb | 2 +- - .../types/comma_separated_to_integer_array.rb | 15 +++++ - lib/api/validations/types/labels_list.rb | 24 -------- - lib/api/validations/types/safe_file.rb | 15 ----- - lib/api/validations/types/workhorse_file.rb | 13 ++--- + .../types/comma_separated_to_integer_array.rb | 15 ++++ + lib/api/validations/types/labels_list.rb | 24 ------ + lib/api/validations/types/safe_file.rb | 15 ---- + lib/api/validations/types/workhorse_file.rb | 13 ++- lib/api/variables.rb | 2 +- lib/api/version.rb | 2 +- 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 +- - 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 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/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 +++ b/Gemfile @@ -19,7 +19,7 @@ @@ -205,10 +263,12 @@ coerced to arrays of integers. Before this was done within Virtus. gem 'faraday', '~> 0.12' gem 'marginalia', '~> 1.8' -@@ -80,6 +80,7 @@ +@@ -79,7 +79,8 @@ + gem 'net-ldap' # API - gem 'grape', '~> 1.1' +-gem 'grape', '~> 1.1' ++gem 'grape', '~> 1.3', '>= 1.3.3' +gem 'rack-timeout' gem 'grape-entity', '~> 0.7.1' 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) mini_portile2 (~> 2.3) - grape (1.1.0) -+ grape (1.3.2) ++ grape (1.3.3) activesupport builder + dry-types (>= 1.1) @@ -336,7 +396,7 @@ coerced to arrays of integers. Before this was done within Virtus. google-protobuf (~> 3.8.0) gpgme (~> 2.0.19) - grape (~> 1.1.0) -+ grape (~> 1.3.2) ++ grape (~> 1.3.3) grape-entity (~> 0.7.1) - grape-path-helpers (~> 1.2) + 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 @@ -0,0 +1,5 @@ +--- -+title: Upgrade Grape v1.1.0 to v1.3.2 -+merge_request: 27276 ++title: Upgrade Grape v1.1.0 to v1.3.3 ++merge_request: 33450 +author: +type: other --- 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. --- a/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 @@ ```ruby @@ -417,6 +492,17 @@ coerced to arrays of integers. Before this was done within Virtus. include PaginationParams 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 +++ b/lib/api/admin/sidekiq.rb @@ -2,7 +2,7 @@ @@ -607,6 +693,15 @@ coerced to arrays of integers. Before this was done within Virtus. params do 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], 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), 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.' @@ -688,6 +783,17 @@ coerced to arrays of integers. Before this was done within Virtus. include APIGuard 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 +++ b/lib/api/group_boards.rb @@ -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 :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: ::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 :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" @@ -942,17 +1048,6 @@ coerced to arrays of integers. Before this was done within Virtus. namespace :ci do desc 'Validation of .gitlab-ci.yml content' 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 +++ b/lib/api/markdown.rb @@ -1,7 +1,7 @@ @@ -1015,7 +1110,7 @@ coerced to arrays of integers. Before this was done within Virtus. include PaginationParams CONTEXT_COMMITS_POST_LIMIT = 20 -@@ -182,9 +182,9 @@ +@@ -182,11 +182,11 @@ params :optional_params do 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' @@ -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 :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 :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 :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 :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::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' 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 @@ end 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 success Entities::Metrics::Dashboard::Annotation 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 +++ b/lib/api/milestone_responses.rb @@ -15,7 +15,7 @@ @@ -1223,6 +1333,17 @@ coerced to arrays of integers. Before this was done within Virtus. include PaginationParams 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 +++ b/lib/api/project_snapshots.rb @@ -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' 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' -@@ -112,12 +113,12 @@ +@@ -112,7 +113,7 @@ requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha' 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." @@ -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' 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' - 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 +++ b/lib/api/sidekiq_metrics.rb @@ -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' end 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 +++ b/rubocop/cop/api/grape_api_instance.rb @@ -0,0 +1,42 @@ @@ -1991,6 +2055,25 @@ coerced to arrays of integers. Before this was done within Virtus. + 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 +++ b/spec/rubocop/cop/api/grape_api_instance_spec.rb @@ -0,0 +1,31 @@ @@ -2008,7 +2091,7 @@ coerced to arrays of integers. Before this was done within Virtus. + subject(:cop) { described_class.new } + + it 'adds an offense when inheriting from Grape::API' do -+ inspect_source(<<~CODE.strip_indent) ++ inspect_source(<<~CODE) + class SomeAPI < Grape::API + end + CODE @@ -2016,8 +2099,8 @@ coerced to arrays of integers. Before this was done within Virtus. + expect(cop.offenses.size).to eq(1) + end + -+ it 'adds an offense when inheriting from Grape::API::Instance' do -+ inspect_source(<<~CODE.strip_indent) ++ it 'does not add an offense when inheriting from Grape::API::Instance' do ++ inspect_source(<<~CODE) + class SomeAPI < Grape::API::Instance + end + CODE @@ -2042,7 +2125,7 @@ coerced to arrays of integers. Before this was done within Virtus. + subject(:cop) { described_class.new } + + it 'adds an offense with a required parameter' do -+ inspect_source(<<~CODE.strip_indent) ++ inspect_source(<<~CODE) + class SomeAPI < Grape::API::Instance + params do + requires :values, type: Array[String] @@ -2054,7 +2137,7 @@ coerced to arrays of integers. Before this was done within Virtus. + end + + it 'adds an offense with an optional parameter' do -+ inspect_source(<<~CODE.strip_indent) ++ inspect_source(<<~CODE) + class SomeAPI < Grape::API::Instance + params do + optional :values, type: Array[String] @@ -2066,7 +2149,7 @@ coerced to arrays of integers. Before this was done within Virtus. + end + + it 'does not add an offense' do -+ inspect_source(<<~CODE.strip_indent) ++ inspect_source(<<~CODE) + class SomeAPI < Grape::API::Instance + params do + 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 + + it 'does not add an offense for unrelated classes' do -+ inspect_source(<<~CODE.strip_indent) ++ inspect_source(<<~CODE) + class SomeClass + params do + 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 + 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