From a9ffc8f1bc91ab7d837dfdfd50801a6a35f04717 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 This brings in Ruby 2.7 suport and a number of fixes: https://github.com/ruby-grape/grape/blob/master/CHANGELOG.md 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: 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 with Grape and dry-types. 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 The change from Virtus to dry-types now requires all strings to be coerced to arrays of integers. Before this was done within Virtus. --- Gemfile | 4 +- Gemfile.lock | 55 +++++++++++-------- changelogs/unreleased/sh-update-grape-gem.yml | 5 ++ 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 +- ee/lib/api/composer_packages.rb | 2 +- ee/lib/api/conan_packages.rb | 2 +- ee/lib/api/dependencies.rb | 3 +- ee/lib/api/dependency_proxy.rb | 2 +- .../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/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/group_hooks.rb | 2 +- ee/lib/api/group_packages.rb | 2 +- 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_trains.rb | 2 +- ee/lib/api/npm_packages.rb | 2 +- ee/lib/api/nuget_packages.rb | 2 +- ee/lib/api/package_files.rb | 2 +- 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_mirror.rb | 2 +- ee/lib/api/project_packages.rb | 2 +- ee/lib/api/project_push_rule.rb | 2 +- ee/lib/api/protected_environments.rb | 2 +- ee/lib/api/pypi_packages.rb | 2 +- ee/lib/api/scim.rb | 2 +- ee/lib/api/unleash.rb | 2 +- ee/lib/api/v3/github.rb | 2 +- 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_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 +- lib/api/access_requests.rb | 2 +- lib/api/admin/sidekiq.rb | 2 +- lib/api/api.rb | 2 +- lib/api/api_guard.rb | 11 +++- lib/api/appearance.rb | 2 +- lib/api/applications.rb | 2 +- lib/api/avatar.rb | 2 +- lib/api/award_emoji.rb | 2 +- lib/api/badges.rb | 2 +- lib/api/boards.rb | 2 +- lib/api/branches.rb | 2 +- lib/api/broadcast_messages.rb | 2 +- lib/api/commit_statuses.rb | 2 +- lib/api/commits.rb | 2 +- lib/api/container_registry_event.rb | 2 +- lib/api/deploy_keys.rb | 2 +- lib/api/deploy_tokens.rb | 6 +- lib/api/deployments.rb | 2 +- lib/api/discussions.rb | 2 +- lib/api/environments.rb | 2 +- lib/api/error_tracking.rb | 2 +- lib/api/events.rb | 2 +- lib/api/features.rb | 2 +- lib/api/files.rb | 2 +- lib/api/group_boards.rb | 2 +- lib/api/group_clusters.rb | 2 +- lib/api/group_container_repositories.rb | 2 +- lib/api/group_export.rb | 2 +- lib/api/group_import.rb | 2 +- lib/api/group_labels.rb | 2 +- lib/api/group_milestones.rb | 2 +- lib/api/group_variables.rb | 2 +- lib/api/groups.rb | 4 +- lib/api/helpers/merge_requests_helpers.rb | 2 +- lib/api/helpers/projects_helpers.rb | 2 +- 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/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/metrics/dashboard/annotations.rb | 2 +- lib/api/milestone_responses.rb | 2 +- lib/api/namespaces.rb | 2 +- lib/api/notes.rb | 2 +- lib/api/notification_settings.rb | 2 +- lib/api/pages.rb | 2 +- lib/api/pages_domains.rb | 2 +- lib/api/pagination_params.rb | 2 +- lib/api/pipeline_schedules.rb | 2 +- lib/api/pipelines.rb | 2 +- lib/api/project_clusters.rb | 2 +- lib/api/project_container_repositories.rb | 2 +- lib/api/project_events.rb | 2 +- lib/api/project_export.rb | 2 +- lib/api/project_hooks.rb | 2 +- lib/api/project_import.rb | 2 +- lib/api/project_milestones.rb | 2 +- lib/api/project_snapshots.rb | 2 +- lib/api/project_snippets.rb | 2 +- lib/api/project_statistics.rb | 2 +- lib/api/project_templates.rb | 2 +- lib/api/projects.rb | 4 +- lib/api/protected_branches.rb | 2 +- lib/api/protected_tags.rb | 2 +- lib/api/release/links.rb | 2 +- lib/api/releases.rb | 2 +- lib/api/remote_mirrors.rb | 2 +- lib/api/repositories.rb | 4 +- lib/api/resource_label_events.rb | 2 +- lib/api/runner.rb | 4 +- lib/api/runners.rb | 12 ++-- lib/api/search.rb | 2 +- lib/api/services.rb | 2 +- lib/api/settings.rb | 11 ++-- lib/api/sidekiq_metrics.rb | 2 +- lib/api/snippets.rb | 2 +- lib/api/statistics.rb | 2 +- lib/api/submodules.rb | 2 +- lib/api/subscriptions.rb | 2 +- lib/api/suggestions.rb | 2 +- lib/api/system_hooks.rb | 2 +- lib/api/tags.rb | 2 +- lib/api/templates.rb | 2 +- lib/api/terraform/state.rb | 2 +- lib/api/todos.rb | 2 +- lib/api/triggers.rb | 2 +- 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 ++--- lib/api/variables.rb | 2 +- lib/api/version.rb | 2 +- lib/api/wikis.rb | 4 +- spec/requests/api/settings_spec.rb | 10 +++- spec/rubocop/cop/code_reuse/worker_spec.rb | 2 +- 164 files changed, 282 insertions(+), 264 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 --- a/Gemfile +++ b/Gemfile @@ -19,7 +19,7 @@ gem 'pg', '~> 1.1' gem 'rugged', '~> 0.28' -gem 'grape-path-helpers', '~> 1.2' +gem 'grape-path-helpers', '~> 1.3' gem 'faraday', '~> 0.12' gem 'marginalia', '~> 1.8' --- a/Gemfile.lock +++ b/Gemfile.lock @@ -103,10 +103,6 @@ aws-sdk-core (= 2.11.374) aws-sigv4 (1.1.0) aws-eventstream (~> 1.0, >= 1.0.2) - axiom-types (0.1.1) - descendants_tracker (~> 0.0.4) - ice_nine (~> 0.11.0) - thread_safe (~> 0.3, >= 0.3.1) babosa (1.0.2) base32 (0.3.2) batch-loader (1.4.0) @@ -165,8 +161,6 @@ nap open4 (~> 1.3) coderay (1.1.2) - coercible (1.0.0) - descendants_tracker (~> 0.0.1) colored2 (3.1.2) commonmarker (0.20.1) ruby-enum (~> 0.5) @@ -222,8 +216,6 @@ ruby-statistics (>= 2.1) thor (>= 0.19, < 2) unicode_plot (>= 0.0.4, < 1.0.0) - descendants_tracker (0.0.4) - thread_safe (~> 0.3, >= 0.3.1) device_detector (1.0.0) devise (4.7.1) bcrypt (~> 3.0) @@ -250,6 +242,28 @@ doorkeeper-openid_connect (1.6.3) doorkeeper (>= 5.0, < 5.2) json-jwt (~> 1.6) + dry-configurable (0.11.5) + concurrent-ruby (~> 1.0) + dry-core (~> 0.4, >= 0.4.7) + dry-equalizer (~> 0.2) + dry-container (0.7.2) + concurrent-ruby (~> 1.0) + dry-configurable (~> 0.1, >= 0.1.3) + dry-core (0.4.9) + concurrent-ruby (~> 1.0) + dry-equalizer (0.3.0) + dry-inflector (0.2.0) + dry-logic (1.0.6) + concurrent-ruby (~> 1.0) + dry-core (~> 0.2) + dry-equalizer (~> 0.2) + dry-types (1.4.0) + concurrent-ruby (~> 1.0) + dry-container (~> 0.3) + dry-core (~> 0.4, >= 0.4.4) + dry-equalizer (~> 0.3) + dry-inflector (~> 0.1, >= 0.1.2) + dry-logic (~> 1.0, >= 1.0.2) ed25519 (1.2.4) elasticsearch (6.8.0) elasticsearch-api (= 6.8.0) @@ -439,19 +453,19 @@ signet (~> 0.7) gpgme (2.0.20) mini_portile2 (~> 2.3) - grape (1.1.0) + grape (1.3.2) activesupport builder + dry-types (>= 1.1) mustermann-grape (~> 1.0.0) rack (>= 1.3.0) rack-accept - virtus (>= 1.0.0) grape-entity (0.7.1) activesupport (>= 4.0) multi_json (>= 1.3.2) - grape-path-helpers (1.2.0) + grape-path-helpers (1.3.0) activesupport - grape (~> 1.0) + grape (~> 1.3) rake (~> 12) grape_logging (1.8.3) grape @@ -645,9 +659,10 @@ multi_xml (0.6.0) multipart-post (2.1.1) murmurhash3 (0.1.6) - mustermann (1.0.3) - mustermann-grape (1.0.0) - mustermann (~> 1.0.0) + mustermann (1.1.1) + ruby2_keywords (~> 0.0.1) + mustermann-grape (1.0.1) + mustermann (>= 1.0.0) nakayoshi_fork (0.0.4) nap (1.1.0) nenv (0.3.0) @@ -960,6 +975,7 @@ ruby-saml (1.7.2) nokogiri (>= 1.5.10) ruby-statistics (2.1.2) + ruby2_keywords (0.0.2) ruby_dep (1.5.0) ruby_parser (3.13.1) sexp_processor (~> 4.9) @@ -1118,11 +1134,6 @@ activerecord (>= 3.0) activesupport (>= 3.0) version_sorter (2.2.4) - virtus (1.0.5) - axiom-types (~> 0.1) - coercible (~> 1.0) - descendants_tracker (~> 0.0, >= 0.0.3) - equalizer (~> 0.0, >= 0.0.9) vmstat (2.3.0) warden (1.2.8) rack (>= 2.0.6) @@ -1253,9 +1264,9 @@ google-api-client (~> 0.23) google-protobuf (~> 3.8.0) gpgme (~> 2.0.19) - grape (~> 1.1.0) + grape (~> 1.3.2) grape-entity (~> 0.7.1) - grape-path-helpers (~> 1.2) + grape-path-helpers (~> 1.3) grape_logging (~> 1.7) graphiql-rails (~> 1.4.10) graphql (~> 1.10.5) --- /dev/null +++ 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 +author: +type: other --- a/doc/development/api_styleguide.md +++ b/doc/development/api_styleguide.md @@ -98,6 +98,14 @@ Model.create(foo: params[:foo]) ``` +## Array types + +With Grape v1.3+, Array types must be defined with a `coerce_with` +block, or parameters will fail to validate when passed a string from an +API request. See the [Grape upgrading +documentation](https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#ensure-that-array-types-have-explicit-coercions) +for more details. + ## Using HTTP status helpers For non-200 HTTP responses, use the provided helpers in `lib/api/helpers.rb` to ensure correct behaviour (`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 @@ -513,12 +513,12 @@ interface first here. For example, suppose we have a few more optional parameters for EE. We can move the -paramters 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 @@ -579,7 +579,7 @@ ```ruby module API - class JobArtifacts < Grape::API + class JobArtifacts < Grape::API::Instance # EE::API::JobArtifacts would override the following helpers helpers do def authorize_download_artifacts! @@ -623,7 +623,7 @@ ```ruby module API - class MergeRequests < Grape::API + class MergeRequests < Grape::API::Instance helpers do # EE::API::MergeRequests would override the following helpers def update_merge_request_ee(merge_request) @@ -692,7 +692,7 @@ ```ruby # api/merge_requests/parameters.rb module API - class MergeRequests < Grape::API + class MergeRequests < Grape::API::Instance module Parameters def self.update_params_at_least_one_of %i[ @@ -708,7 +708,7 @@ # api/merge_requests.rb module API - class MergeRequests < Grape::API + class MergeRequests < Grape::API::Instance params do at_least_one_of(*Parameters.update_params_at_least_one_of) end --- a/lib/api/access_requests.rb +++ b/lib/api/access_requests.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class AccessRequests < Grape::API + class AccessRequests < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/admin/sidekiq.rb +++ b/lib/api/admin/sidekiq.rb @@ -2,7 +2,7 @@ module API module Admin - class Sidekiq < Grape::API + class Sidekiq < Grape::API::Instance before { authenticated_as_admin! } namespace 'admin' do --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class API < Grape::API + class API < Grape::API::Instance include APIGuard LOG_FILENAME = Rails.root.join("log", "api_json.log") --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -148,7 +148,16 @@ { scope: e.scopes }) end - response.finish + finished_response = nil + response.finish do |rack_response| + # Grape expects a Rack::Response + # (https://github.com/ruby-grape/grape/commit/c117bff7d22971675f4b34367d3a98bc31c8fc02), + # and we need to retrieve it here: + # https://github.com/nov/rack-oauth2/blob/40c9a99fd80486ccb8de0e4869ae384547c0d703/lib/rack/oauth2/server/abstract/error.rb#L28 + finished_response = rack_response + end + + finished_response end end end --- a/lib/api/appearance.rb +++ b/lib/api/appearance.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Appearance < Grape::API + class Appearance < Grape::API::Instance before { authenticated_as_admin! } helpers do --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -2,7 +2,7 @@ module API # External applications API - class Applications < Grape::API + class Applications < Grape::API::Instance before { authenticated_as_admin! } resource :applications do --- a/lib/api/avatar.rb +++ b/lib/api/avatar.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Avatar < Grape::API + class Avatar < Grape::API::Instance resource :avatar do desc 'Return avatar url for a user' do success Entities::Avatar --- a/lib/api/award_emoji.rb +++ b/lib/api/award_emoji.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class AwardEmoji < Grape::API + class AwardEmoji < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/badges.rb +++ b/lib/api/badges.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Badges < Grape::API + class Badges < Grape::API::Instance include PaginationParams before { authenticate_non_get! } --- a/lib/api/boards.rb +++ b/lib/api/boards.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Boards < Grape::API + class Boards < Grape::API::Instance include BoardsResponses include PaginationParams --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -3,7 +3,7 @@ require 'mime/types' module API - class Branches < Grape::API + class Branches < Grape::API::Instance include PaginationParams BRANCH_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(branch: API::NO_SLASH_URL_PART_REGEX) --- a/lib/api/broadcast_messages.rb +++ b/lib/api/broadcast_messages.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class BroadcastMessages < Grape::API + class BroadcastMessages < Grape::API::Instance include PaginationParams resource :broadcast_messages do --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -3,7 +3,7 @@ require 'mime/types' module API - class CommitStatuses < Grape::API + class CommitStatuses < Grape::API::Instance params do requires :id, type: String, desc: 'The ID of a project' end --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -3,7 +3,7 @@ require 'mime/types' module API - class Commits < Grape::API + class Commits < Grape::API::Instance include PaginationParams before do --- a/lib/api/container_registry_event.rb +++ b/lib/api/container_registry_event.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ContainerRegistryEvent < Grape::API + class ContainerRegistryEvent < Grape::API::Instance DOCKER_DISTRIBUTION_EVENTS_V1_JSON = 'application/vnd.docker.distribution.events.v1+json' before { authenticate_registry_notification! } --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class DeployKeys < Grape::API + class DeployKeys < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/deploy_tokens.rb +++ b/lib/api/deploy_tokens.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class DeployTokens < Grape::API + class DeployTokens < Grape::API::Instance include PaginationParams helpers do @@ -54,7 +54,7 @@ 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", or "write_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}`' @@ -117,7 +117,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", or "write_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}`' --- a/lib/api/deployments.rb +++ b/lib/api/deployments.rb @@ -2,7 +2,7 @@ module API # Deployments RESTful API endpoints - class Deployments < Grape::API + class Deployments < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/discussions.rb +++ b/lib/api/discussions.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Discussions < Grape::API + class Discussions < Grape::API::Instance include PaginationParams helpers ::API::Helpers::NotesHelpers helpers ::RendersNotes --- a/lib/api/environments.rb +++ b/lib/api/environments.rb @@ -2,7 +2,7 @@ module API # Environments RESTfull API endpoints - class Environments < Grape::API + class Environments < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/error_tracking.rb +++ b/lib/api/error_tracking.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ErrorTracking < Grape::API + class ErrorTracking < Grape::API::Instance before { authenticate! } params do --- a/lib/api/events.rb +++ b/lib/api/events.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Events < Grape::API + class Events < Grape::API::Instance include PaginationParams include APIGuard helpers ::API::Helpers::EventsHelpers --- a/lib/api/features.rb +++ b/lib/api/features.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Features < Grape::API + class Features < Grape::API::Instance before { authenticated_as_admin! } helpers do --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Files < Grape::API + class Files < Grape::API::Instance include APIGuard FILE_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(file_path: API::NO_SLASH_URL_PART_REGEX) --- a/lib/api/group_boards.rb +++ b/lib/api/group_boards.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class GroupBoards < Grape::API + class GroupBoards < Grape::API::Instance include BoardsResponses include PaginationParams --- a/lib/api/group_clusters.rb +++ b/lib/api/group_clusters.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class GroupClusters < Grape::API + class GroupClusters < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/group_container_repositories.rb +++ b/lib/api/group_container_repositories.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class GroupContainerRepositories < Grape::API + class GroupContainerRepositories < Grape::API::Instance include PaginationParams before { authorize_read_group_container_images! } --- a/lib/api/group_export.rb +++ b/lib/api/group_export.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class GroupExport < Grape::API + class GroupExport < Grape::API::Instance before do not_found! unless Feature.enabled?(:group_import_export, user_group, default_enabled: true) --- a/lib/api/group_import.rb +++ b/lib/api/group_import.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class GroupImport < Grape::API + class GroupImport < Grape::API::Instance MAXIMUM_FILE_SIZE = 50.megabytes.freeze helpers do --- a/lib/api/group_labels.rb +++ b/lib/api/group_labels.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class GroupLabels < Grape::API + class GroupLabels < Grape::API::Instance include PaginationParams helpers ::API::Helpers::LabelHelpers --- a/lib/api/group_milestones.rb +++ b/lib/api/group_milestones.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class GroupMilestones < Grape::API + class GroupMilestones < Grape::API::Instance include MilestoneResponses include PaginationParams --- a/lib/api/group_variables.rb +++ b/lib/api/group_variables.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class GroupVariables < Grape::API + class GroupVariables < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Groups < Grape::API + class Groups < Grape::API::Instance include PaginationParams include Helpers::CustomAttributes @@ -16,7 +16,7 @@ params :group_list_params do use :statistics_params - optional :skip_groups, type: Array[Integer], desc: 'Array of group ids to exclude from list' + optional :skip_groups, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Array of group ids to exclude from list' optional :all_available, type: Boolean, desc: 'Show all group that you have access to' optional :search, type: String, desc: 'Search for a specific group' optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user' --- a/lib/api/helpers/merge_requests_helpers.rb +++ b/lib/api/helpers/merge_requests_helpers.rb @@ -24,7 +24,7 @@ optional :milestone, type: String, desc: 'Return merge requests for a specific milestone' optional :labels, type: Array[String], - coerce_with: Validations::Types::LabelsList.coerce, + coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' optional :with_labels_details, type: Boolean, desc: 'Return titles of labels and other details', default: false optional :created_after, type: DateTime, desc: 'Return merge requests created after the specified time' --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -44,7 +44,7 @@ optional :request_access_enabled, type: Boolean, desc: 'Allow users to request member access' optional :only_allow_merge_if_pipeline_succeeds, type: Boolean, desc: 'Only allow to merge if builds succeed' optional :only_allow_merge_if_all_discussions_are_resolved, type: Boolean, desc: 'Only allow to merge if all discussions are resolved' - optional :tag_list, type: Array[String], desc: 'The list of tags for a project' + optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The list of tags for a project' # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 optional :avatar, type: File, desc: 'Avatar image for project' # rubocop:disable Scalability/FileUploads optional :printing_merge_request_link_enabled, type: Boolean, desc: 'Show link to create/view merge request when pushing from the command line' --- a/lib/api/import_github.rb +++ b/lib/api/import_github.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ImportGithub < Grape::API + class ImportGithub < Grape::API::Instance rescue_from Octokit::Unauthorized, with: :provider_unauthorized helpers do --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -3,7 +3,7 @@ module API # Internal access API module Internal - class Base < Grape::API + class Base < Grape::API::Instance before { authenticate_by_gitlab_shell_token! } before do --- a/lib/api/internal/pages.rb +++ b/lib/api/internal/pages.rb @@ -3,7 +3,7 @@ module API # Pages Internal API module Internal - class Pages < Grape::API + class Pages < Grape::API::Instance before do authenticate_gitlab_pages_request! end --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Issues < Grape::API + class Issues < Grape::API::Instance include PaginationParams helpers Helpers::IssuesHelpers helpers Helpers::RateLimiter @@ -11,9 +11,9 @@ helpers do params :negatable_issue_filter_params do - 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 :milestone, type: String, desc: 'Milestone title' - optional :iids, type: Array[Integer], desc: 'The IID array of issues' + optional :iids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IID array of issues' optional :search, type: String, desc: 'Search issues for text present in the title, description, or any combination of these' optional :in, type: String, desc: '`title`, `description`, or a string joining them with comma' @@ -63,10 +63,10 @@ params :issue_params do optional :description, type: String, desc: 'The description of an issue' - optional :assignee_ids, type: Array[Integer], 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 :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 :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" --- a/lib/api/job_artifacts.rb +++ b/lib/api/job_artifacts.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class JobArtifacts < Grape::API + class JobArtifacts < Grape::API::Instance before { authenticate_non_get! } # EE::API::JobArtifacts would override the following helpers --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Jobs < Grape::API + class Jobs < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/keys.rb +++ b/lib/api/keys.rb @@ -2,7 +2,7 @@ module API # Keys API - class Keys < Grape::API + class Keys < Grape::API::Instance before { authenticate! } resource :keys do --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Labels < Grape::API + class Labels < Grape::API::Instance include PaginationParams helpers ::API::Helpers::LabelHelpers --- a/lib/api/lint.rb +++ b/lib/api/lint.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Lint < Grape::API + class Lint < Grape::API::Instance 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 @@ # frozen_string_literal: true module API - class Markdown < Grape::API + class Markdown < Grape::API::Instance params do requires :text, type: String, desc: "The markdown text to render" optional :gfm, type: Boolean, desc: "Render text using GitLab Flavored Markdown" --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Members < Grape::API + class Members < Grape::API::Instance include PaginationParams before { authenticate! } @@ -18,7 +18,7 @@ end params do optional :query, type: String, desc: 'A query string to search for members' - optional :user_ids, type: Array[Integer], desc: 'Array of user ids to look up for membership' + optional :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Array of user ids to look up for membership' optional :show_seat_info, type: Boolean, desc: 'Show seat information for members' use :optional_filter_params_ee use :pagination @@ -37,7 +37,7 @@ end params do optional :query, type: String, desc: 'A query string to search for members' - optional :user_ids, type: Array[Integer], desc: 'Array of user ids to look up for membership' + optional :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Array of user ids to look up for membership' optional :show_seat_info, type: Boolean, desc: 'Show seat information for members' use :pagination end --- a/lib/api/merge_request_diffs.rb +++ b/lib/api/merge_request_diffs.rb @@ -2,7 +2,7 @@ module API # MergeRequestDiff API - class MergeRequestDiffs < Grape::API + class MergeRequestDiffs < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class MergeRequests < Grape::API + class MergeRequests < Grape::API::Instance include PaginationParams CONTEXT_COMMITS_POST_LIMIT = 20 @@ -177,9 +177,9 @@ 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' - optional :assignee_ids, type: Array[Integer], 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 :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 :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' @@ -194,7 +194,7 @@ end params do use :merge_requests_params - optional :iids, type: Array[Integer], desc: 'The IID array of merge requests' + optional :iids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IID array of merge requests' end get ":id/merge_requests" do authorize! :read_merge_request, user_project --- a/lib/api/metrics/dashboard/annotations.rb +++ b/lib/api/metrics/dashboard/annotations.rb @@ -3,7 +3,7 @@ module API module Metrics module Dashboard - class Annotations < Grape::API + class Annotations < Grape::API::Instance desc 'Create a new monitoring dashboard annotation' do success Entities::Metrics::Dashboard::Annotation end --- a/lib/api/milestone_responses.rb +++ b/lib/api/milestone_responses.rb @@ -15,7 +15,7 @@ params :list_params do optional :state, type: String, values: %w[active closed all], default: 'all', desc: 'Return "active", "closed", or "all" milestones' - optional :iids, type: Array[Integer], desc: 'The IIDs of the milestones' + optional :iids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IIDs of the milestones' optional :title, type: String, desc: 'The title of the milestones' optional :search, type: String, desc: 'The search criteria for the title or description of the milestone' use :pagination --- a/lib/api/namespaces.rb +++ b/lib/api/namespaces.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Namespaces < Grape::API + class Namespaces < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Notes < Grape::API + class Notes < Grape::API::Instance include PaginationParams helpers ::API::Helpers::NotesHelpers --- a/lib/api/notification_settings.rb +++ b/lib/api/notification_settings.rb @@ -2,7 +2,7 @@ module API # notification_settings API - class NotificationSettings < Grape::API + class NotificationSettings < Grape::API::Instance before { authenticate! } helpers ::API::Helpers::MembersHelpers --- a/lib/api/pages.rb +++ b/lib/api/pages.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Pages < Grape::API + class Pages < Grape::API::Instance before do require_pages_config_enabled! authenticated_with_can_read_all_resources! --- a/lib/api/pages_domains.rb +++ b/lib/api/pages_domains.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class PagesDomains < Grape::API + class PagesDomains < Grape::API::Instance include PaginationParams PAGES_DOMAINS_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(domain: API::NO_SLASH_URL_PART_REGEX) --- a/lib/api/pagination_params.rb +++ b/lib/api/pagination_params.rb @@ -4,7 +4,7 @@ # Concern for declare pagination params. # # @example - # class CustomApiResource < Grape::API + # class CustomApiResource < Grape::API::Instance # include PaginationParams # # params do --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class PipelineSchedules < Grape::API + class PipelineSchedules < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Pipelines < Grape::API + class Pipelines < Grape::API::Instance include PaginationParams before { authenticate_non_get! } --- a/lib/api/project_clusters.rb +++ b/lib/api/project_clusters.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectClusters < Grape::API + class ProjectClusters < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/project_container_repositories.rb +++ b/lib/api/project_container_repositories.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectContainerRepositories < Grape::API + class ProjectContainerRepositories < Grape::API::Instance include PaginationParams REPOSITORY_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge( --- a/lib/api/project_events.rb +++ b/lib/api/project_events.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectEvents < Grape::API + class ProjectEvents < Grape::API::Instance include PaginationParams include APIGuard helpers ::API::Helpers::EventsHelpers --- a/lib/api/project_export.rb +++ b/lib/api/project_export.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectExport < Grape::API + class ProjectExport < Grape::API::Instance helpers Helpers::RateLimiter before do --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectHooks < Grape::API + class ProjectHooks < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/project_import.rb +++ b/lib/api/project_import.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectImport < Grape::API + class ProjectImport < Grape::API::Instance include PaginationParams MAXIMUM_FILE_SIZE = 50.megabytes --- a/lib/api/project_milestones.rb +++ b/lib/api/project_milestones.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectMilestones < Grape::API + class ProjectMilestones < Grape::API::Instance include PaginationParams include MilestoneResponses --- a/lib/api/project_snapshots.rb +++ b/lib/api/project_snapshots.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectSnapshots < Grape::API + class ProjectSnapshots < Grape::API::Instance helpers ::API::Helpers::ProjectSnapshotsHelpers before { authorize_read_git_snapshot! } --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectSnippets < Grape::API + class ProjectSnippets < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/project_statistics.rb +++ b/lib/api/project_statistics.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectStatistics < Grape::API + class ProjectStatistics < Grape::API::Instance before do authenticate! authorize! :daily_statistics, user_project --- a/lib/api/project_templates.rb +++ b/lib/api/project_templates.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProjectTemplates < Grape::API + class ProjectTemplates < Grape::API::Instance include PaginationParams TEMPLATE_TYPES = %w[dockerfiles gitignores gitlab_ci_ymls licenses].freeze --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -3,7 +3,7 @@ require_dependency 'declarative_policy' module API - class Projects < Grape::API + class Projects < Grape::API::Instance include PaginationParams include Helpers::CustomAttributes @@ -520,7 +520,7 @@ end params do optional :search, type: String, desc: 'Return list of users matching the search criteria' - optional :skip_users, type: Array[Integer], desc: 'Filter out users with the specified IDs' + optional :skip_users, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Filter out users with the specified IDs' use :pagination end get ':id/users' do --- a/lib/api/protected_branches.rb +++ b/lib/api/protected_branches.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProtectedBranches < Grape::API + class ProtectedBranches < Grape::API::Instance include PaginationParams BRANCH_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(name: API::NO_SLASH_URL_PART_REGEX) --- a/lib/api/protected_tags.rb +++ b/lib/api/protected_tags.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ProtectedTags < Grape::API + class ProtectedTags < Grape::API::Instance include PaginationParams TAG_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(name: API::NO_SLASH_URL_PART_REGEX) --- a/lib/api/release/links.rb +++ b/lib/api/release/links.rb @@ -2,7 +2,7 @@ module API module Release - class Links < Grape::API + class Links < Grape::API::Instance include PaginationParams RELEASE_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS --- a/lib/api/releases.rb +++ b/lib/api/releases.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Releases < Grape::API + class Releases < Grape::API::Instance include PaginationParams RELEASE_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS --- a/lib/api/remote_mirrors.rb +++ b/lib/api/remote_mirrors.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class RemoteMirrors < Grape::API + class RemoteMirrors < Grape::API::Instance include PaginationParams before do --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -3,7 +3,7 @@ require 'mime/types' module API - class Repositories < Grape::API + class Repositories < Grape::API::Instance include PaginationParams before { authorize! :download_code, user_project } @@ -139,7 +139,7 @@ success Entities::Commit end params do - requires :refs, type: Array[String] + requires :refs, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce end get ':id/repository/merge_base' do refs = params[:refs] --- a/lib/api/resource_label_events.rb +++ b/lib/api/resource_label_events.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ResourceLabelEvents < Grape::API + class ResourceLabelEvents < Grape::API::Instance include PaginationParams helpers ::API::Helpers::NotesHelpers --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Runner < Grape::API + class Runner < Grape::API::Instance helpers ::API::Helpers::Runner resource :runners do @@ -18,7 +18,7 @@ optional :access_level, type: String, values: Ci::Runner.access_levels.keys, desc: 'The access_level of the runner' optional :run_untagged, type: Boolean, desc: 'Should Runner handle untagged jobs' - optional :tag_list, type: Array[String], desc: %q(List of Runner's tags) + optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: %q(List of Runner's tags) optional :maximum_timeout, type: Integer, desc: 'Maximum timeout set when this Runner will handle the job' end post '/' do --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Runners < Grape::API + class Runners < Grape::API::Instance include PaginationParams before { authenticate! } @@ -17,7 +17,7 @@ desc: 'The type of the runners to show' optional :status, type: String, values: Ci::Runner::AVAILABLE_STATUSES, desc: 'The status of the runners to show' - optional :tag_list, type: Array[String], desc: 'The tags of the runners to show' + optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The tags of the runners to show' use :pagination end get do @@ -40,7 +40,7 @@ desc: 'The type of the runners to show' optional :status, type: String, values: Ci::Runner::AVAILABLE_STATUSES, desc: 'The status of the runners to show' - optional :tag_list, type: Array[String], desc: 'The tags of the runners to show' + optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The tags of the runners to show' use :pagination end get 'all' do @@ -75,7 +75,7 @@ requires :id, type: Integer, desc: 'The ID of the runner' optional :description, type: String, desc: 'The description of the runner' optional :active, type: Boolean, desc: 'The state of a runner' - optional :tag_list, type: Array[String], desc: 'The list of tags for a runner' + optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The list of tags for a runner' optional :run_untagged, type: Boolean, desc: 'Flag indicating the runner can execute untagged jobs' optional :locked, type: Boolean, desc: 'Flag indicating the runner is locked' optional :access_level, type: String, values: Ci::Runner.access_levels.keys, @@ -145,7 +145,7 @@ desc: 'The type of the runners to show' optional :status, type: String, values: Ci::Runner::AVAILABLE_STATUSES, desc: 'The status of the runners to show' - optional :tag_list, type: Array[String], desc: 'The tags of the runners to show' + optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The tags of the runners to show' use :pagination end get ':id/runners' do @@ -208,7 +208,7 @@ desc: 'The type of the runners to show' optional :status, type: String, values: Ci::Runner::AVAILABLE_STATUSES, desc: 'The status of the runners to show' - optional :tag_list, type: Array[String], desc: 'The tags of the runners to show' + optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The tags of the runners to show' use :pagination end get ':id/runners' do --- a/lib/api/search.rb +++ b/lib/api/search.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Search < Grape::API + class Search < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true module API - class Services < Grape::API + class Services < Grape::API::Instance services = Helpers::ServicesHelpers.services service_classes = Helpers::ServicesHelpers.service_classes --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Settings < Grape::API + class Settings < Grape::API::Instance before { authenticated_as_admin! } helpers Helpers::SettingsHelpers @@ -49,7 +49,7 @@ optional :default_project_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default project visibility' optional :default_projects_limit, type: Integer, desc: 'The maximum number of personal projects' optional :default_snippet_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default snippet visibility' - optional :disabled_oauth_sign_in_sources, type: Array[String], desc: 'Disable certain OAuth sign-in sources' + optional :disabled_oauth_sign_in_sources, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Disable certain OAuth sign-in sources' optional :domain_blacklist_enabled, type: Boolean, desc: 'Enable domain blacklist for sign ups' optional :domain_blacklist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' optional :domain_whitelist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' @@ -79,7 +79,8 @@ requires :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run." end optional :html_emails_enabled, type: Boolean, desc: 'By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format.' - optional :import_sources, type: Array[String], values: %w[github bitbucket bitbucket_server gitlab google_code fogbugz git gitlab_project gitea manifest phabricator], + optional :import_sources, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, + values: %w[github bitbucket bitbucket_server gitlab google_code fogbugz git gitlab_project gitea manifest phabricator], 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' @@ -121,12 +122,12 @@ 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." - optional :repository_storages, type: Array[String], desc: 'Storage paths for new projects' + optional :repository_storages, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Storage paths for new projects' 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 @@ require 'sidekiq/api' module API - class SidekiqMetrics < Grape::API + class SidekiqMetrics < Grape::API::Instance before { authenticated_as_admin! } helpers do --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -2,7 +2,7 @@ module API # Snippets API - class Snippets < Grape::API + class Snippets < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/statistics.rb +++ b/lib/api/statistics.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Statistics < Grape::API + class Statistics < Grape::API::Instance before { authenticated_as_admin! } COUNTED_ITEMS = [Project, User, Group, ForkNetworkMember, ForkNetwork, Issue, --- a/lib/api/submodules.rb +++ b/lib/api/submodules.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Submodules < Grape::API + class Submodules < Grape::API::Instance before { authenticate! } helpers do --- a/lib/api/subscriptions.rb +++ b/lib/api/subscriptions.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Subscriptions < Grape::API + class Subscriptions < Grape::API::Instance helpers ::API::Helpers::LabelHelpers before { authenticate! } --- a/lib/api/suggestions.rb +++ b/lib/api/suggestions.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Suggestions < Grape::API + class Suggestions < Grape::API::Instance before { authenticate! } resource :suggestions do --- a/lib/api/system_hooks.rb +++ b/lib/api/system_hooks.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class SystemHooks < Grape::API + class SystemHooks < Grape::API::Instance include PaginationParams before do --- a/lib/api/tags.rb +++ b/lib/api/tags.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Tags < Grape::API + class Tags < Grape::API::Instance include PaginationParams TAG_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(tag_name: API::NO_SLASH_URL_PART_REGEX) --- a/lib/api/templates.rb +++ b/lib/api/templates.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Templates < Grape::API + class Templates < Grape::API::Instance include PaginationParams GLOBAL_TEMPLATE_TYPES = { --- a/lib/api/terraform/state.rb +++ b/lib/api/terraform/state.rb @@ -4,7 +4,7 @@ module API module Terraform - class State < Grape::API + class State < Grape::API::Instance include ::Gitlab::Utils::StrongMemoize default_format :json --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Todos < Grape::API + class Todos < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Triggers < Grape::API + class Triggers < Grape::API::Instance include PaginationParams HTTP_GITLAB_EVENT_HEADER = "HTTP_#{WebHookService::GITLAB_EVENT_HEADER}".underscore.upcase --- a/lib/api/user_counts.rb +++ b/lib/api/user_counts.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class UserCounts < Grape::API + class UserCounts < Grape::API::Instance resource :user_counts do desc 'Return the user specific counts' do detail 'Open MR Count' --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Users < Grape::API + class Users < Grape::API::Instance include PaginationParams include APIGuard include Helpers::CustomAttributes --- a/lib/api/validations/types/comma_separated_to_array.rb +++ b/lib/api/validations/types/comma_separated_to_array.rb @@ -10,7 +10,7 @@ when String value.split(',').map(&:strip) when Array - value.map { |v| v.to_s.split(',').map(&:strip) }.flatten + value.flat_map { |v| v.to_s.split(',').map(&:strip) } else [] end --- /dev/null +++ b/lib/api/validations/types/comma_separated_to_integer_array.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module API + module Validations + module Types + class CommaSeparatedToIntegerArray < CommaSeparatedToArray + def self.coerce + lambda do |value| + super.call(value).map(&:to_i) + end + end + end + end + end +end --- a/lib/api/validations/types/labels_list.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -module API - module Validations - module Types - class LabelsList - def self.coerce - lambda do |value| - case value - when String - value.split(',').map(&:strip) - when Array - value.flat_map { |v| v.to_s.split(',').map(&:strip) } - when LabelsList - value - else - [] - end - end - end - end - end - end -end --- a/lib/api/validations/types/safe_file.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -# This module overrides the Grape type validator defined in -# https://github.com/ruby-grape/grape/blob/master/lib/grape/validations/types/file.rb -module API - module Validations - module Types - class SafeFile < ::Grape::Validations::Types::File - def value_coerced?(value) - super && value[:tempfile].is_a?(Tempfile) - end - end - end - end -end --- a/lib/api/validations/types/workhorse_file.rb +++ b/lib/api/validations/types/workhorse_file.rb @@ -3,15 +3,14 @@ module API module Validations module Types - class WorkhorseFile < Virtus::Attribute - def coerce(input) - # Processing of multipart file objects - # is already taken care of by Gitlab::Middleware::Multipart. - # Nothing to do here. - input + class WorkhorseFile + def self.parse(value) + raise "#{value.class} is not an UploadedFile type" unless parsed?(value) + + value end - def value_coerced?(value) + def self.parsed?(value) value.is_a?(::UploadedFile) end end --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Variables < Grape::API + class Variables < Grape::API::Instance include PaginationParams before { authenticate! } --- a/lib/api/version.rb +++ b/lib/api/version.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Version < Grape::API + class Version < Grape::API::Instance helpers ::API::Helpers::GraphqlHelpers include APIGuard --- a/lib/api/wikis.rb +++ b/lib/api/wikis.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class Wikis < Grape::API + class Wikis < Grape::API::Instance helpers do def commit_params(attrs) # In order to avoid service disruption this can work with an old workhorse without the acceleration @@ -117,7 +117,7 @@ success Entities::WikiAttachment end params do - requires :file, types: [::API::Validations::Types::SafeFile, ::API::Validations::Types::WorkhorseFile], desc: 'The attachment file to be uploaded' + requires :file, types: [Rack::Multipart::UploadedFile, ::API::Validations::Types::WorkhorseFile], desc: 'The attachment file to be uploaded' 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 @@ -274,6 +274,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 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module API + class GrapeAPIInstance < RuboCop::Cop::Cop + # This cop checks that APIs subclass Grape::API::Instance with Grape v1.3+. + # + # @example + # + # # bad + # module API + # class Projects < Grape::API + # end + # end + # + # # good + # module API + # class Projects < Grape::API::Instance + # end + # end + MSG = 'Inherit from Grape::API::Instance instead of Grape::API. ' \ + 'For more details check the https://gitlab.com/gitlab-org/gitlab/-/issues/215230.' + + def_node_matcher :grape_api_definition, <<~PATTERN + (class + (const _ _) + (const + (const nil? :Grape) :API) + ... + ) + PATTERN + + def on_class(node) + grape_api_definition(node) do + add_offense(node.children[1]) + end + end + end + end + end +end --- /dev/null +++ b/rubocop/cop/api/grape_array_missing_coerce.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module API + class GrapeArrayMissingCoerce < RuboCop::Cop::Cop + # This cop checks that Grape API parameters using an Array type + # implement a coerce_with method: + # + # https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#ensure-that-array-types-have-explicit-coercions + # + # @example + # + # # bad + # requires :values, type: Array[String] + # + # # good + # requires :values, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce + # + # end + MSG = 'This Grape parameter defines an Array but is missing a coerce_with definition. ' \ + 'For more details, see https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#ensure-that-array-types-have-explicit-coercions' + + def_node_matcher :grape_api_instance?, <<~PATTERN + (class + (const _ _) + (const + (const + (const nil? :Grape) :API) :Instance) + ... + ) + PATTERN + + def_node_matcher :grape_api_param_block?, <<~PATTERN + (send _ {:requires :optional} + (sym _) + $_) + PATTERN + + def_node_matcher :grape_type_def?, <<~PATTERN + (sym :type) + PATTERN + + def_node_matcher :grape_array_type?, <<~PATTERN + (send + (const nil? :Array) :[] + (const nil? _)) + PATTERN + + def_node_matcher :grape_coerce_with?, <<~PATTERN + (sym :coerce_with) + PATTERN + + def on_class(node) + @grape_api ||= grape_api_instance?(node) + end + + def on_send(node) + return unless @grape_api + + match = grape_api_param_block?(node) + + return unless match.is_a?(RuboCop::AST::HashNode) + + is_array_type = false + has_coerce_method = false + + match.each_pair do |first, second| + has_coerce_method ||= grape_coerce_with?(first) + + if grape_type_def?(first) && grape_array_type?(second) + is_array_type = true + end + end + + if is_array_type && !has_coerce_method + add_offense(node) + end + end + end + end + end +end --- /dev/null +++ b/spec/rubocop/cop/api/grape_api_instance_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require_relative '../../../support/helpers/expect_offense' +require_relative '../../../../rubocop/cop/api/grape_api_instance' + +describe RuboCop::Cop::API::GrapeAPIInstance do + include CopHelper + include ExpectOffense + + subject(:cop) { described_class.new } + + it 'adds an offense when inheriting from Grape::API' do + inspect_source(<<~CODE.strip_indent) + class SomeAPI < Grape::API + end + CODE + + expect(cop.offenses.size).to eq(1) + end + + it 'adds an offense when inheriting from Grape::API::Instance' do + inspect_source(<<~CODE.strip_indent) + class SomeAPI < Grape::API::Instance + end + CODE + + expect(cop.offenses.size).to be_zero + end +end --- /dev/null +++ b/spec/rubocop/cop/api/grape_array_missing_coerce_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require_relative '../../../support/helpers/expect_offense' +require_relative '../../../../rubocop/cop/api/grape_array_missing_coerce' + +describe RuboCop::Cop::API::GrapeArrayMissingCoerce do + include CopHelper + include ExpectOffense + + subject(:cop) { described_class.new } + + it 'adds an offense with a required parameter' do + inspect_source(<<~CODE.strip_indent) + class SomeAPI < Grape::API::Instance + params do + requires :values, type: Array[String] + end + end + CODE + + expect(cop.offenses.size).to eq(1) + end + + it 'adds an offense with an optional parameter' do + inspect_source(<<~CODE.strip_indent) + class SomeAPI < Grape::API::Instance + params do + optional :values, type: Array[String] + end + end + CODE + + expect(cop.offenses.size).to eq(1) + end + + it 'does not add an offense' do + inspect_source(<<~CODE.strip_indent) + class SomeAPI < Grape::API::Instance + params do + requires :values, type: Array[String], coerce_with: ->(val) { val.split(',').map(&:strip) } + requires :milestone, type: String, desc: 'Milestone title' + optional :assignee_id, types: [Integer, String], integer_none_any: true, + desc: 'Return issues which are assigned to the user with the given ID' + end + end + CODE + + expect(cop.offenses.size).to be_zero + end + + it 'does not add an offense for unrelated classes' do + inspect_source(<<~CODE.strip_indent) + class SomeClass + params do + requires :values, type: Array[String] + end + end + CODE + + expect(cop.offenses.size).to be_zero + end +end