diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-29 21:09:07 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-29 21:09:07 +0000 |
| commit | 777f6da99ae8dd4111bb880893cee9c8cfefa132 (patch) | |
| tree | 06f28423a1a8277c1f5ed6e2216bed391b0ecf47 | |
| parent | aa99514d5c37e08c0fa49d03212ccdc943b8d31e (diff) | |
| download | gitlab-ce-777f6da99ae8dd4111bb880893cee9c8cfefa132.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
148 files changed, 927 insertions, 423 deletions
diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 43b55245e51..86e3011e8f4 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -119,6 +119,7 @@ .db-patterns: &db-patterns - "{,ee/}{db}/**/*" + - "{,ee}/spec/{db,migrations}/**/*" .backstage-patterns: &backstage-patterns - "Dangerfile" diff --git a/.rubocop.yml b/.rubocop.yml index 1967cbfb982..89e6214fe47 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -308,6 +308,18 @@ Gitlab/Union: - '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: @@ -19,7 +19,7 @@ gem 'default_value_for', '~> 3.3.0' 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.0' @@ -81,7 +81,7 @@ gem 'gitlab_omniauth-ldap', '~> 2.1.1', require: 'omniauth-ldap' gem 'net-ldap' # API -gem 'grape', '~> 1.1.0' +gem 'grape', '~> 1.3.3' gem 'grape-entity', '~> 0.7.1' gem 'rack-cors', '~> 1.0.6', require: 'rack/cors' diff --git a/Gemfile.lock b/Gemfile.lock index be2d98cb063..ae35fa30e6e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -103,10 +103,6 @@ GEM 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) @@ -164,8 +160,6 @@ GEM 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) @@ -221,8 +215,6 @@ GEM 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) @@ -249,6 +241,28 @@ GEM 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 @@ GEM signet (~> 0.14) gpgme (2.0.20) mini_portile2 (~> 2.3) - grape (1.1.0) + grape (1.3.3) 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 @@ -642,9 +656,10 @@ GEM 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) @@ -959,6 +974,7 @@ GEM 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) @@ -1122,11 +1138,6 @@ GEM 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) @@ -1257,9 +1268,9 @@ DEPENDENCIES google-api-client (~> 0.33) google-protobuf (~> 3.8.0) gpgme (~> 2.0.19) - grape (~> 1.1.0) + grape (~> 1.3.3) 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) diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index cfeb4e71c56..068612c4c92 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -411,7 +411,6 @@ img.emoji { .append-right-15 { margin-right: 15px; } .append-right-default { margin-right: $gl-padding; } .append-right-20 { margin-right: 20px; } -.append-right-48 { margin-right: 48px; } .append-bottom-5 { margin-bottom: 5px; } .append-bottom-10 { margin-bottom: 10px; } .append-bottom-15 { margin-bottom: 15px; } diff --git a/app/controllers/projects/refs_controller.rb b/app/controllers/projects/refs_controller.rb index a2581e72257..0d1583e05e5 100644 --- a/app/controllers/projects/refs_controller.rb +++ b/app/controllers/projects/refs_controller.rb @@ -57,22 +57,11 @@ class Projects::RefsController < Projects::ApplicationController render json: logs end - - # Deprecated due to https://gitlab.com/gitlab-org/gitlab/-/issues/36863 - # Will be removed soon https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29895 - format.js do - @logs, _ = tree_summary.summarize - @more_log_url = more_url(tree_summary.next_offset) if tree_summary.more? - end end end private - def more_url(offset) - logs_file_project_ref_path(@project, @ref, @path, offset: offset) - end - def validate_ref_id return not_found! if params[:id].present? && params[:id] !~ Gitlab::PathRegex.git_reference_regex end diff --git a/app/graphql/mutations/jira_import/start.rb b/app/graphql/mutations/jira_import/start.rb index 3df26d33711..eda28059272 100644 --- a/app/graphql/mutations/jira_import/start.rb +++ b/app/graphql/mutations/jira_import/start.rb @@ -21,12 +21,17 @@ module Mutations argument :jira_project_name, GraphQL::STRING_TYPE, required: false, description: 'Project name of the importer Jira project' + argument :users_mapping, + [Types::JiraUsersMappingInputType], + required: false, + description: 'The mapping of Jira to GitLab users' - def resolve(project_path:, jira_project_key:) + def resolve(project_path:, jira_project_key:, users_mapping:) project = authorized_find!(full_path: project_path) + mapping = users_mapping.to_ary.map { |map| map.to_hash } service_response = ::JiraImport::StartImportService - .new(context[:current_user], project, jira_project_key) + .new(context[:current_user], project, jira_project_key, mapping) .execute jira_import = service_response.success? ? service_response.payload[:import_data] : nil diff --git a/app/graphql/types/jira_users_mapping_input_type.rb b/app/graphql/types/jira_users_mapping_input_type.rb new file mode 100644 index 00000000000..61cf1474493 --- /dev/null +++ b/app/graphql/types/jira_users_mapping_input_type.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Types + # rubocop: disable Graphql/AuthorizeTypes + class JiraUsersMappingInputType < BaseInputObject + graphql_name 'JiraUsersMappingInputType' + + argument :jira_account_id, + GraphQL::STRING_TYPE, + required: true, + description: 'Jira account id of the user' + argument :gitlab_id, + GraphQL::INT_TYPE, + required: false, + description: 'Id of the GitLab user' + end + # rubocop: enable Graphql/AuthorizeTypes +end diff --git a/app/services/jira_import/start_import_service.rb b/app/services/jira_import/start_import_service.rb index a06cc6df719..f85f686c61a 100644 --- a/app/services/jira_import/start_import_service.rb +++ b/app/services/jira_import/start_import_service.rb @@ -2,23 +2,39 @@ module JiraImport class StartImportService - attr_reader :user, :project, :jira_project_key + attr_reader :user, :project, :jira_project_key, :users_mapping - def initialize(user, project, jira_project_key) + def initialize(user, project, jira_project_key, users_mapping) @user = user @project = project @jira_project_key = jira_project_key + @users_mapping = users_mapping end def execute validation_response = validate return validation_response if validation_response&.error? + store_users_mapping create_and_schedule_import end private + def store_users_mapping + return if users_mapping.blank? + + mapping = users_mapping.map do |map| + next if !map[:jira_account_id] || !map[:gitlab_id] + + [map[:jira_account_id], map[:gitlab_id]] + end.compact.to_h + + return if mapping.blank? + + Gitlab::JiraImport.cache_users_mapping(project.id, mapping) + end + def create_and_schedule_import jira_import = build_jira_import project.import_type = 'jira' diff --git a/app/views/projects/refs/logs_tree.js.haml b/app/views/projects/refs/logs_tree.js.haml deleted file mode 100644 index 506bf54b3f8..00000000000 --- a/app/views/projects/refs/logs_tree.js.haml +++ /dev/null @@ -1,23 +0,0 @@ -- @logs.each do |content_data| - - file_name = content_data[:file_name] - - commit = content_data[:commit] - - next unless commit - - :plain - var row = $("table.table_#{@hex_path} tr.file_#{hexdigest(file_name)}"); - row.find("td.tree-time-ago").html('#{escape_javascript time_ago_with_tooltip(commit.committed_date)}'); - row.find("td.tree-commit").html('#{escape_javascript render("projects/tree/tree_commit_column", commit: commit)}'); - - = render_if_exists 'projects/refs/logs_tree_lock_label', lock_label: content_data[:lock_label] - -- if @more_log_url - :plain - if($('#tree-slider').length) { - // Load more commit logs for each file in tree - // if we still on the same page - var url = "#{escape_javascript(@more_log_url)}"; - gl.utils.ajaxGet(url); - } - -:plain - gl.utils.localTimeAgo($('.js-timeago', 'table.table_#{@hex_path} tbody')); diff --git a/app/views/projects/tree/_tree_commit_column.html.haml b/app/views/projects/tree/_tree_commit_column.html.haml deleted file mode 100644 index 065fef606d5..00000000000 --- a/app/views/projects/tree/_tree_commit_column.html.haml +++ /dev/null @@ -1,3 +0,0 @@ -- full_title = markdown_field(commit, :full_title) -%span.str-truncated - = link_to_html full_title, project_commit_path(@project, commit.id), title: full_title, class: 'tree-commit-link' diff --git a/changelogs/unreleased/216145-graphql-import.yml b/changelogs/unreleased/216145-graphql-import.yml new file mode 100644 index 00000000000..a107b95cd4a --- /dev/null +++ b/changelogs/unreleased/216145-graphql-import.yml @@ -0,0 +1,5 @@ +--- +title: Add Jira users mapping to start Jira import mutation +merge_request: 34609 +author: +type: added diff --git a/changelogs/unreleased/sh-update-grape-gem.yml b/changelogs/unreleased/sh-update-grape-gem.yml new file mode 100644 index 00000000000..fbed6ad665b --- /dev/null +++ b/changelogs/unreleased/sh-update-grape-gem.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade Grape v1.1.0 to v1.3.3 +merge_request: 33450 +author: +type: other diff --git a/doc/administration/logs.md b/doc/administration/logs.md index 0ec6ea7986e..140f6cd6f0a 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -26,7 +26,7 @@ GitLab, thanks to [Lograge](https://github.com/roidrage/lograge/). Note that requests from the API are logged to a separate file in `api_json.log`. Each line contains a JSON line that can be ingested by services like Elasticsearch and Splunk. -Line breaks have been added to this example for legibility: +Line breaks were added to examples for legibility: ```json { @@ -79,7 +79,7 @@ seconds: User clone and fetch activity using HTTP transport appears in this log as `action: git_upload_pack`. In addition, the log contains the originating IP address, -(`remote_ip`),the user's ID (`user_id`), and username (`username`). +(`remote_ip`), the user's ID (`user_id`), and username (`username`). Some endpoints such as `/search` may make requests to Elasticsearch if using [Advanced Global Search](../user/search/advanced_global_search.md). These will @@ -227,7 +227,7 @@ It helps you see requests made directly to the API. For example: } ``` -This entry shows an access to an internal endpoint to check whether an +This entry shows an internal endpoint accessed to check whether an associated SSH key can download the project in question via a `git fetch` or `git clone`. In this example, we see: @@ -320,7 +320,7 @@ packages or in `/home/git/gitlab/log/kubernetes.log` for installations from source. It logs information related to the Kubernetes Integration including errors -during installing cluster applications on your GitLab managed Kubernetes +during installing cluster applications on your managed Kubernetes clusters. Each line contains a JSON line that can be ingested by services like Elasticsearch and Splunk. @@ -362,7 +362,7 @@ After 12.2, this file was renamed from `githost.log` to `git_json.log` and stored in JSON format. GitLab has to interact with Git repositories, but in some rare cases -something can go wrong, and in this case you will know what exactly +something can go wrong, and in this case you may need know what exactly happened. This log file contains all failed requests from GitLab to Git repositories. In the majority of cases this file will be useful for developers only. For example: @@ -473,8 +473,8 @@ This file lives in `/var/log/gitlab/gitlab-rails/sidekiq_client.log` for Omnibus GitLab packages or in `/home/git/gitlab/log/sidekiq_client.log` for installations from source. -This file contains logging information about jobs before they are start -being processed by Sidekiq, for example before being enqueued. +This file contains logging information about jobs before Sidekiq starts +processing them, such as before being enqueued. This log file follows the same structure as [`sidekiq.log`](#sidekiqlog), so it will be structured as JSON if @@ -571,32 +571,45 @@ User clone/fetch activity using SSH transport appears in this log as `executing ## Gitaly Logs -This file lives in `/var/log/gitlab/gitaly/current` and is produced by [runit](http://smarden.org/runit/). `runit` is packaged with Omnibus and a brief explanation of its purpose is available [in the omnibus documentation](https://docs.gitlab.com/omnibus/architecture/#runit). [Log files are rotated](http://smarden.org/runit/svlogd.8.html), renamed in Unix timestamp format and `gzip`-compressed (e.g. `@1584057562.s`). +This file lives in `/var/log/gitlab/gitaly/current` and is produced by [runit](http://smarden.org/runit/). `runit` is packaged with Omnibus GitLab and a brief explanation of its purpose is available [in the Omnibus GitLab documentation](https://docs.gitlab.com/omnibus/architecture/#runit). [Log files are rotated](http://smarden.org/runit/svlogd.8.html), renamed in Unix timestamp format, and `gzip`-compressed (like `@1584057562.s`). ### `grpc.log` This file lives in `/var/log/gitlab/gitlab-rails/grpc.log` for Omnibus GitLab packages. Native [gRPC](https://grpc.io/) logging used by Gitaly. -## `puma_stderr.log` & `puma_stdout.log` +## Puma Logs -This file lives in `/var/log/gitlab/puma/puma_stderr.log` and `/var/log/gitlab/puma/puma_stdout.log` for -Omnibus GitLab packages or in `/home/git/gitlab/log/puma_stderr.log` and `/home/git/gitlab/log/puma_stdout.log` -for installations from source. +### `puma_stdout.log` + +This file lives in `/var/log/gitlab/puma/puma_stdout.log` for +Omnibus GitLab packages, and `/home/git/gitlab/log/puma_stdout.log` for +installations from source. + +### `puma_stderr.log` + +This file lives in `/var/log/gitlab/puma/puma_stderr.log` for +Omnibus GitLab packages, or in `/home/git/gitlab/log/puma_stderr.log` for +installations from source. -## `unicorn_stderr.log` & `unicorn_stdout.log` +## Unicorn Logs NOTE: **Note:** Starting with GitLab 13.0, Puma is the default web server used in GitLab all-in-one package based installations as well as GitLab Helm chart deployments. -This file lives in `/var/log/gitlab/unicorn/unicorn_stderr.log` and `/var/log/gitlab/unicorn/unicorn_stdout.log` for -Omnibus GitLab packages or in `/home/git/gitlab/log/unicorn_stderr.log` and `/home/git/gitlab/log/unicorn_stdout.log` +### `unicorn_stdout.log` + +This file lives in `/var/log/gitlab/unicorn/unicorn_stdout.log` for +Omnibus GitLab packages or in `/home/git/gitlab/log/unicorn_stdout.log` for +for installations from source. + +### `unicorn_stderr.log` + +This file lives in `/var/log/gitlab/unicorn/unicorn_stderr.log` for +Omnibus GitLab packages or in `/home/git/gitlab/log/unicorn_stderr.log` for for installations from source. -Unicorn is a high-performance forking Web server which is used for -serving the GitLab application. You can look at this log if, for -example, your application does not respond. This log contains all -information about the state of Unicorn processes at any given time. +These logs contain all information about the state of Unicorn processes at any given time. ```plaintext I, [2015-02-13T06:14:46.680381 #9047] INFO -- : Refreshing Gem list @@ -657,7 +670,7 @@ This log records: - [Protected paths](../user/admin_area/settings/protected_paths.md) abusive requests. NOTE: **Note:** -From [%12.3](https://gitlab.com/gitlab-org/gitlab/-/issues/29239), user ID and username are also +In GitLab versions [12.3](https://gitlab.com/gitlab-org/gitlab/-/issues/29239) and greater, user ID and username are also recorded on this log, if available. ## `graphql_json.log` @@ -686,7 +699,7 @@ installations from source. > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/19186) in GitLab 12.6. -This file lives in `/var/log/gitlab/mailroom/mail_room_json.log` for +This file lives in `/var/log/gitlab/mailroom/current` for Omnibus GitLab packages or in `/home/git/gitlab/log/mail_room_json.log` for installations from source. @@ -793,8 +806,8 @@ This file lives in `/var/log/gitlab/gitlab-rails/service_measurement.log` for Omnibus GitLab packages or in `/home/git/gitlab/log/service_measurement.log` for installations from source. -It contain only a single structured log with measurements for each service execution. -It will contain measurement such as: number of SQL calls, `execution_time`, `gc_stats`, `memory_usage`, etc... +It contains only a single structured log with measurements for each service execution. +It will contain measurements such as the number of SQL calls, `execution_time`, `gc_stats`, and `memory usage`. For example: @@ -870,22 +883,46 @@ For example: } ``` +## Mattermost Logs + +For Omnibus GitLab installations, Mattermost logs reside in `/var/log/gitlab/mattermost/mattermost.log`. + ## Workhorse Logs -For Omnibus installations, Workhorse logs reside in `/var/log/gitlab/gitlab-workhorse/current`. +For Omnibus GitLab installations, Workhorse logs reside in `/var/log/gitlab/gitlab-workhorse/`. ## PostgreSQL Logs -For Omnibus installations, PostgreSQL logs reside in `/var/log/gitlab/postgresql/current`. +For Omnibus GitLab installations, PostgreSQL logs reside in `/var/log/gitlab/postgresql/`. ## Prometheus Logs -For Omnibus installations, Prometheus logs reside in `/var/log/gitlab/prometheus/current`. +For Omnibus GitLab installations, Prometheus logs reside in `/var/log/gitlab/prometheus/`. ## Redis Logs -For Omnibus installations, Redis logs reside in `/var/log/gitlab/redis/current`. +For Omnibus GitLab installations, Redis logs reside in `/var/log/gitlab/redis/`. -## Mattermost Logs +## Alertmanager Logs + +For Omnibus GitLab installations, Alertmanager logs reside in `/var/log/gitlab/alertmanager/`. + +## Crond Logs + +For Omnibus GitLab installations, crond logs reside in `/var/log/gitlab/crond/`. + +## Grafana Logs + +For Omnibus GitLab installations, Grafana logs reside in `/var/log/gitlab/grafana/`. + +## LogRotate Logs + +For Omnibus GitLab installations, logrotate logs reside in `/var/log/gitlab/logrotate/`. + +## GitLab Monitor Logs + +For Omnibus GitLab installations, GitLab Monitor logs reside in `/var/log/gitlab/gitlab-monitor/`. + +## GitLab Exporter -For Omnibus installations, Mattermost logs reside in `/var/log/gitlab/mattermost/mattermost.log`. +For Omnibus GitLab installations, GitLab Exporter logs reside in `/var/log/gitlab/gitlab-exporter/`. diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index d0a03534c6b..9310b5081a4 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -6368,6 +6368,11 @@ input JiraImportStartInput { The project to import the Jira project into """ projectPath: ID! + + """ + The mapping of Jira to GitLab users + """ + usersMapping: [JiraUsersMappingInputType!] } """ @@ -6546,6 +6551,18 @@ type JiraUser { jiraEmail: String } +input JiraUsersMappingInputType { + """ + Id of the GitLab user + """ + gitlabId: Int + + """ + Jira account id of the user + """ + jiraAccountId: String! +} + type Label { """ Background color of the label diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 7e53d48594e..203cfc1c65c 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -17606,6 +17606,24 @@ "defaultValue": null }, { + "name": "usersMapping", + "description": "The mapping of Jira to GitLab users", + "type": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "INPUT_OBJECT", + "name": "JiraUsersMappingInputType", + "ofType": null + } + } + }, + "defaultValue": null + }, + { "name": "clientMutationId", "description": "A unique identifier for the client performing the mutation.", "type": { @@ -18168,6 +18186,41 @@ "possibleTypes": null }, { + "kind": "INPUT_OBJECT", + "name": "JiraUsersMappingInputType", + "description": null, + "fields": null, + "inputFields": [ + { + "name": "jiraAccountId", + "description": "Jira account id of the user", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + }, + "defaultValue": null + }, + { + "name": "gitlabId", + "description": "Id of the GitLab user", + "type": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + }, + "defaultValue": null + } + ], + "interfaces": null, + "enumValues": null, + "possibleTypes": null + }, + { "kind": "OBJECT", "name": "Label", "description": null, diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md index 6a044004926..327f919d7f4 100644 --- a/doc/development/api_styleguide.md +++ b/doc/development/api_styleguide.md @@ -98,6 +98,46 @@ For instance: 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. + +### Automatic coercion of nil inputs + +Prior to Grape v1.3.3, Array parameters with `nil` values would +automatically be coerced to an empty Array. However, due to [this pull +request in v1.3.3](https://github.com/ruby-grape/grape/pull/2040), this +is no longer the case. For example, suppose you define a PUT `/test` +request that has an optional parameter: + +```ruby +optional :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The user ids for this rule' +``` + +Normally, a request to PUT `/test?user_ids` would cause Grape to pass +`params` of `{ user_ids: nil }`. + +This may introduce errors with endpoints that expect a blank array and +do not handle `nil` inputs properly. To preserve the previous behavior, +there is a helper method `coerce_nil_params_to_array!` that is used +in the `before` block of all API calls: + +```ruby +before do + coerce_nil_params_to_array! +end +``` + +With this change, a request to PUT `/test?user_ids` will cause Grape to +pass `params` to be `{ user_ids: [] }`. + +There is [an open issue in the Grape tracker](https://github.com/ruby-grape/grape/issues/2068) +to make this easier. + ## Using HTTP status helpers 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. diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index e22e96b6f06..e2cbcd6cc22 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -512,12 +512,12 @@ do that, so we'll follow regular object-oriented practices that we define the 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 @@ -578,7 +578,7 @@ class definition to make it easy and clear: ```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! @@ -622,7 +622,7 @@ route. Something like this: ```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) @@ -691,7 +691,7 @@ least argument. We would approach this as follows: ```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[ @@ -707,7 +707,7 @@ API::MergeRequests::Parameters.prepend_if_ee('EE::API::MergeRequests::Parameters # 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 diff --git a/doc/development/permissions.md b/doc/development/permissions.md index 06a4a03de38..e0364342dc6 100644 --- a/doc/development/permissions.md +++ b/doc/development/permissions.md @@ -13,9 +13,16 @@ Groups and projects can have the following visibility levels: - internal (`10`) - an entity is visible to logged in users - private (`0`) - an entity is visible only to the approved members of the entity +By default, subgroups can **not** have higher visibility levels. +For example, if you create a new private group, it can not include a public subgroup. + The visibility level of a group can be changed only if all subgroups and -sub-projects have the same or lower visibility level. (e.g., a group can be set -to internal only if all subgroups and projects are internal or private). +sub-projects have the same or lower visibility level. For example, a group can be set +to internal only if all subgroups and projects are internal or private. + +CAUTION: **Warning:** +If you migrate an existing group to a lower visibility level, that action does not migrate subgroups +in the same way. This is a [known issue](https://gitlab.com/gitlab-org/gitlab/-/issues/22406). Visibility levels can be found in the `Gitlab::VisibilityLevel` module. diff --git a/doc/user/application_security/security_dashboard/img/pipeline_security_dashboard_v12_6.png b/doc/user/application_security/security_dashboard/img/pipeline_security_dashboard_v12_6.png Binary files differdeleted file mode 100644 index 670c90d10a3..00000000000 --- a/doc/user/application_security/security_dashboard/img/pipeline_security_dashboard_v12_6.png +++ /dev/null diff --git a/doc/user/application_security/security_dashboard/img/pipeline_security_dashboard_v13_2.png b/doc/user/application_security/security_dashboard/img/pipeline_security_dashboard_v13_2.png Binary files differnew file mode 100644 index 00000000000..d935af96212 --- /dev/null +++ b/doc/user/application_security/security_dashboard/img/pipeline_security_dashboard_v13_2.png diff --git a/doc/user/application_security/security_dashboard/index.md b/doc/user/application_security/security_dashboard/index.md index 1594ed77f74..c12ce677de3 100644 --- a/doc/user/application_security/security_dashboard/index.md +++ b/doc/user/application_security/security_dashboard/index.md @@ -45,7 +45,7 @@ At the pipeline level, the Security section displays the vulnerabilities present Visit the page for any pipeline which has run any of the [supported reports](#supported-reports). Click the **Security** tab to view the Security findings. - + NOTE: **Note:** A pipeline consists of multiple jobs, including SAST and DAST scanning. If any job fails to finish for any reason, the security dashboard will not show SAST scanner output. For example, if the SAST job finishes but the DAST job fails, the security dashboard will not show SAST results. The analyzer will output an [exit code](../../../development/integrations/secure.md#exit-code) on failure. diff --git a/doc/user/group/subgroups/index.md b/doc/user/group/subgroups/index.md index 842e2082be4..8494fb4fa6a 100644 --- a/doc/user/group/subgroups/index.md +++ b/doc/user/group/subgroups/index.md @@ -19,6 +19,9 @@ By using subgroups you can do the following: - **Make it easier to manage people and control visibility.** Give people different [permissions](../../permissions.md#group-members-permissions) depending on their group [membership](#membership). +For more information on allowed permissions in groups and projects, see +[visibility levels](../../../development/permissions.md#general-permissions). + ## Overview A group can have many subgroups inside it, and at the same time a group can have diff --git a/doc/user/project/import/gemnasium.md b/doc/user/project/import/gemnasium.md index 0d6e059f1cf..3838289aec4 100644 --- a/doc/user/project/import/gemnasium.md +++ b/doc/user/project/import/gemnasium.md @@ -105,7 +105,7 @@ back to both GitLab and GitHub when completed. 1. The result of the job will be visible directly from the pipeline view: -  +  NOTE: **Note:** If you don't commit very often to your project, you may want to use diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index ee8dc822098..5305b25538f 100644 --- 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! } diff --git a/lib/api/admin/ci/variables.rb b/lib/api/admin/ci/variables.rb index df731148bac..6b0ff5e9395 100644 --- 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! } diff --git a/lib/api/admin/sidekiq.rb b/lib/api/admin/sidekiq.rb index a700bea0fd7..f4c84f2eee8 100644 --- 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 diff --git a/lib/api/api.rb b/lib/api/api.rb index 1cdb500e6ac..e5bc49de32e 100644 --- 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") @@ -46,6 +46,8 @@ module API end before do + coerce_nil_params_to_array! + Gitlab::ApplicationContext.push( user: -> { @current_user }, project: -> { @project }, diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index c6557fce541..1e56b1f3bfc 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -153,7 +153,16 @@ module API { 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 diff --git a/lib/api/appearance.rb b/lib/api/appearance.rb index 71a35bb4493..f98004af480 100644 --- 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 diff --git a/lib/api/applications.rb b/lib/api/applications.rb index 70e6b8395d7..4e8d68c8d09 100644 --- 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 diff --git a/lib/api/avatar.rb b/lib/api/avatar.rb index 0f14d003065..9501e777fff 100644 --- 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 diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb index 8e3b3ff8ce5..0a3df3ed96e 100644 --- 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! } diff --git a/lib/api/badges.rb b/lib/api/badges.rb index d2152fad07b..f6cd3f83ff3 100644 --- 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! } diff --git a/lib/api/boards.rb b/lib/api/boards.rb index 87818903705..1f5086127a8 100644 --- 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 diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 081e8ffe4f0..bbed50e96ea 100644 --- 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) diff --git a/lib/api/broadcast_messages.rb b/lib/api/broadcast_messages.rb index 42e7dc751f0..dcf950d7a03 100644 --- 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 diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb index 3116af24ff5..b216406695b 100644 --- a/lib/api/ci/runner.rb +++ b/lib/api/ci/runner.rb @@ -2,7 +2,7 @@ module API module Ci - class Runner < Grape::API + class Runner < Grape::API::Instance helpers ::API::Helpers::Runner resource :runners do @@ -19,7 +19,7 @@ module API 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 diff --git a/lib/api/ci/runners.rb b/lib/api/ci/runners.rb index a4ef54534ba..2c156a71160 100644 --- a/lib/api/ci/runners.rb +++ b/lib/api/ci/runners.rb @@ -2,7 +2,7 @@ module API module Ci - class Runners < Grape::API + class Runners < Grape::API::Instance include PaginationParams before { authenticate! } @@ -18,7 +18,7 @@ module API 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 @@ -41,7 +41,7 @@ module API 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 @@ -76,7 +76,7 @@ module API 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, @@ -146,7 +146,7 @@ module API 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 @@ -209,7 +209,7 @@ module API 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 diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index fcd317b2daa..140351c9e5c 100644 --- 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 diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 086a1b7c402..1a0fe393753 100644 --- 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 diff --git a/lib/api/container_registry_event.rb b/lib/api/container_registry_event.rb index 6d93cc65336..0b7c35cadbd 100644 --- 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! } diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 3259b615369..ad37b7578ad 100644 --- 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! } diff --git a/lib/api/deploy_tokens.rb b/lib/api/deploy_tokens.rb index 0fbbd96cf02..96aa2445f56 100644 --- 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 @@ -56,7 +56,7 @@ module API 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 @@ module API 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.' optional :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`' diff --git a/lib/api/deployments.rb b/lib/api/deployments.rb index cb1dca11e87..87144fd31cc 100644 --- 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! } diff --git a/lib/api/discussions.rb b/lib/api/discussions.rb index 7b453ada41c..29a13e4420d 100644 --- 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 diff --git a/lib/api/environments.rb b/lib/api/environments.rb index 28019ce7796..b825904e2c5 100644 --- 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! } diff --git a/lib/api/error_tracking.rb b/lib/api/error_tracking.rb index 14888037f53..64ec6f0a57a 100644 --- 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 diff --git a/lib/api/events.rb b/lib/api/events.rb index e4c017fab42..0b79431a76d 100644 --- 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 diff --git a/lib/api/features.rb b/lib/api/features.rb index 3fb3fc92e42..9d011d658f6 100644 --- 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 diff --git a/lib/api/files.rb b/lib/api/files.rb index c525897b8fe..748bdfa894d 100644 --- 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) diff --git a/lib/api/freeze_periods.rb b/lib/api/freeze_periods.rb index 9c7e5a5832d..b8254ee9ab4 100644 --- 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! } diff --git a/lib/api/group_boards.rb b/lib/api/group_boards.rb index 88d04e70e11..7efc12121d2 100644 --- 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 diff --git a/lib/api/group_clusters.rb b/lib/api/group_clusters.rb index 2c12c6387fb..c6d10f22bb4 100644 --- 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! } diff --git a/lib/api/group_container_repositories.rb b/lib/api/group_container_repositories.rb index d34317b5271..25b3059f63b 100644 --- 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! } diff --git a/lib/api/group_export.rb b/lib/api/group_export.rb index d3010b6d147..dc14813eefc 100644 --- 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 helpers Helpers::RateLimiter before do diff --git a/lib/api/group_import.rb b/lib/api/group_import.rb index afcbc24d3ce..b82d9fc519a 100644 --- 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 helpers Helpers::FileUploadHelpers helpers do diff --git a/lib/api/group_labels.rb b/lib/api/group_labels.rb index 7585293031f..56f2b769464 100644 --- 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 diff --git a/lib/api/group_milestones.rb b/lib/api/group_milestones.rb index 9e9f5101285..82f5df79356 100644 --- a/lib/api/group_milestones.rb +++ b/lib/api/group_milestones.rb @@ -1,13 +1,11 @@ # frozen_string_literal: true module API - class GroupMilestones < Grape::API + class GroupMilestones < Grape::API::Instance include MilestoneResponses include PaginationParams - before do - authenticate! - end + before { authenticate! } params do requires :id, type: String, desc: 'The ID of a group' diff --git a/lib/api/group_variables.rb b/lib/api/group_variables.rb index 95aa587d0c7..d3ca1c79e73 100644 --- 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! } diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 6e07bb46721..4671e82ab66 100644 --- 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 @@ module API 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' diff --git a/lib/api/helpers/common_helpers.rb b/lib/api/helpers/common_helpers.rb index 32a15381f27..a44fd4b0a5b 100644 --- a/lib/api/helpers/common_helpers.rb +++ b/lib/api/helpers/common_helpers.rb @@ -12,6 +12,26 @@ module API end end end + + # Grape v1.3.3 no longer automatically coerces an Array + # type to an empty array if the value is nil. + def coerce_nil_params_to_array! + keys_to_coerce = params_with_array_types + + params.each do |key, val| + params[key] = Array(val) if val.nil? && keys_to_coerce.include?(key) + end + end + + def params_with_array_types + options[:route_options][:params].map do |key, val| + param_type = val[:type] + # Search for parameters with Array types (e.g. "[String]", "[Integer]", etc.) + if param_type =~ %r(\[\w*\]) + key + end + end.compact.to_set + end end end end diff --git a/lib/api/helpers/merge_requests_helpers.rb b/lib/api/helpers/merge_requests_helpers.rb index 9dab2a88f0b..939f29a04b3 100644 --- a/lib/api/helpers/merge_requests_helpers.rb +++ b/lib/api/helpers/merge_requests_helpers.rb @@ -24,7 +24,7 @@ module API 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 :with_merge_status_recheck, type: Boolean, desc: 'Request that stale merge statuses be rechecked asynchronously', default: false diff --git a/lib/api/helpers/projects_helpers.rb b/lib/api/helpers/projects_helpers.rb index 92030415d02..d541d3d42db 100644 --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -48,7 +48,7 @@ module API optional :only_allow_merge_if_pipeline_succeeds, type: Boolean, desc: 'Only allow to merge if builds succeed' optional :allow_merge_on_skipped_pipeline, type: Boolean, desc: 'Allow to merge if pipeline is skipped' 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' diff --git a/lib/api/import_github.rb b/lib/api/import_github.rb index 21d4928193e..986827e80be 100644 --- 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 diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index 83615c7639b..6d4a4fc9c8b 100644 --- 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 diff --git a/lib/api/internal/pages.rb b/lib/api/internal/pages.rb index 6c8da414e4d..5f8d23f15fa 100644 --- 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 diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 2374ac11f4a..93b0fbc5223 100644 --- 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 @@ -10,9 +10,9 @@ module API 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' @@ -62,12 +62,12 @@ module API 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 :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: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' + optional :add_labels, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' + optional :remove_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" diff --git a/lib/api/job_artifacts.rb b/lib/api/job_artifacts.rb index 6a82256cc96..61c279a76e9 100644 --- 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 diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index 9432b992d74..bcc00429dd6 100644 --- 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! } diff --git a/lib/api/keys.rb b/lib/api/keys.rb index b730e027063..c014641ca04 100644 --- 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 diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 2b283d82e4a..edf4a8ca14e 100644 --- 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 diff --git a/lib/api/lint.rb b/lib/api/lint.rb index a7672021db0..f7796b1e969 100644 --- 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 diff --git a/lib/api/markdown.rb b/lib/api/markdown.rb index de77bef43ce..a0822271cca 100644 --- 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" diff --git a/lib/api/members.rb b/lib/api/members.rb index 341d4837468..3de7ae13d5e 100644 --- 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 @@ module API 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 @@ module API 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 diff --git a/lib/api/merge_request_diffs.rb b/lib/api/merge_request_diffs.rb index 6ad30aa56e0..3e43fe8b257 100644 --- 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! } diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 773a451d3a8..82f103ff0ae 100644 --- 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 @@ -179,11 +179,11 @@ module API 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 :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::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' @@ -198,7 +198,7 @@ module API 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 @@ -315,7 +315,7 @@ module API end params do - requires :commits, type: Array, allow_blank: false, desc: 'List of context commits sha' + requires :commits, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, allow_blank: false, desc: 'List of context commits sha' end desc 'create context commits of merge request' do success Entities::Commit @@ -345,7 +345,7 @@ module API end params do - requires :commits, type: Array, allow_blank: false, desc: 'List of context commits sha' + requires :commits, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, allow_blank: false, desc: 'List of context commits sha' end desc 'remove context commits of merge request' delete ':id/merge_requests/:merge_request_iid/context_commits' do diff --git a/lib/api/metrics/dashboard/annotations.rb b/lib/api/metrics/dashboard/annotations.rb index c8ec4d29657..e07762ac6d3 100644 --- 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 diff --git a/lib/api/metrics/user_starred_dashboards.rb b/lib/api/metrics/user_starred_dashboards.rb index 85fc0f33ed8..263d2394276 100644 --- 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 diff --git a/lib/api/milestone_responses.rb b/lib/api/milestone_responses.rb index 62e159ab003..8ff885983bc 100644 --- a/lib/api/milestone_responses.rb +++ b/lib/api/milestone_responses.rb @@ -15,7 +15,7 @@ module API 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 diff --git a/lib/api/namespaces.rb b/lib/api/namespaces.rb index e40a5dde7ce..e1f279df045 100644 --- 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! } diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 3eafc1ead77..4fb7bffb3d5 100644 --- 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 diff --git a/lib/api/notification_settings.rb b/lib/api/notification_settings.rb index 8cb46bd3ad6..f8b621c1c38 100644 --- 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 diff --git a/lib/api/pages.rb b/lib/api/pages.rb index ee7fe669519..79a6b527581 100644 --- 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! diff --git a/lib/api/pages_domains.rb b/lib/api/pages_domains.rb index 4c3d2d131ac..7d27b575efa 100644 --- 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) diff --git a/lib/api/pagination_params.rb b/lib/api/pagination_params.rb index ae03595eb25..a232b58d3f7 100644 --- a/lib/api/pagination_params.rb +++ b/lib/api/pagination_params.rb @@ -4,7 +4,7 @@ module API # Concern for declare pagination params. # # @example - # class CustomApiResource < Grape::API + # class CustomApiResource < Grape::API::Instance # include PaginationParams # # params do diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 7486518cd4a..9af16f61967 100644 --- 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! } diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index b27ff2d24f8..1aac7b7deb4 100644 --- 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! } diff --git a/lib/api/project_clusters.rb b/lib/api/project_clusters.rb index 299301aabc4..e1dfb647fa0 100644 --- 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! } diff --git a/lib/api/project_container_repositories.rb b/lib/api/project_container_repositories.rb index 2a0099018d9..8f2a62bc5a4 100644 --- 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( diff --git a/lib/api/project_events.rb b/lib/api/project_events.rb index 734311e1142..726e693826e 100644 --- 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 diff --git a/lib/api/project_export.rb b/lib/api/project_export.rb index 4b35f245b8c..d11c47f8d78 100644 --- 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 diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 0e7576c9243..7cea44e6304 100644 --- 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! } diff --git a/lib/api/project_import.rb b/lib/api/project_import.rb index 17d08d14a20..9f43c3c7993 100644 --- 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 diff --git a/lib/api/project_milestones.rb b/lib/api/project_milestones.rb index 8643854a655..2f8dd1085dc 100644 --- a/lib/api/project_milestones.rb +++ b/lib/api/project_milestones.rb @@ -1,13 +1,11 @@ # frozen_string_literal: true module API - class ProjectMilestones < Grape::API + class ProjectMilestones < Grape::API::Instance include PaginationParams include MilestoneResponses - before do - authenticate! - end + before { authenticate! } params do requires :id, type: String, desc: 'The ID of a project' diff --git a/lib/api/project_repository_storage_moves.rb b/lib/api/project_repository_storage_moves.rb index 5de623102fb..c318907542b 100644 --- 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! } diff --git a/lib/api/project_snapshots.rb b/lib/api/project_snapshots.rb index 175fbb2ce92..360000861fc 100644 --- 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! } diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index 68f4a0dcb65..d93454f949d 100644 --- 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! } diff --git a/lib/api/project_statistics.rb b/lib/api/project_statistics.rb index 14ee0f75513..2196801096f 100644 --- 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 diff --git a/lib/api/project_templates.rb b/lib/api/project_templates.rb index cfcc7f5212d..f0fe4d85c8f 100644 --- 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 diff --git a/lib/api/projects.rb b/lib/api/projects.rb index f4582d36f4b..d24dab63bd9 100644 --- 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 @@ -546,7 +546,7 @@ module API 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 diff --git a/lib/api/protected_branches.rb b/lib/api/protected_branches.rb index 1fd86d1e720..b0a7f898eec 100644 --- 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) diff --git a/lib/api/protected_tags.rb b/lib/api/protected_tags.rb index ee13473c848..aaa31cb7cc6 100644 --- 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) diff --git a/lib/api/release/links.rb b/lib/api/release/links.rb index 07c27f39539..7e1815480a5 100644 --- 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 diff --git a/lib/api/releases.rb b/lib/api/releases.rb index a5bb1a44f1f..30c5e06053e 100644 --- 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 @@ -54,7 +54,7 @@ module API requires :url, type: String end end - optional :milestones, type: Array, desc: 'The titles of the related milestones', default: [] + optional :milestones, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The titles of the related milestones', default: [] optional :released_at, type: DateTime, desc: 'The date when the release will be/was ready. Defaults to the current time.' end route_setting :authentication, job_token_allowed: true diff --git a/lib/api/remote_mirrors.rb b/lib/api/remote_mirrors.rb index 0808541d3c7..d1def05808b 100644 --- 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 diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index bf4f08ce390..81702f8f02a 100644 --- 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 helpers ::API::Helpers::HeadersHelpers @@ -143,7 +143,7 @@ module API 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] diff --git a/lib/api/resource_label_events.rb b/lib/api/resource_label_events.rb index 1fa6898b92c..a8d3419528c 100644 --- 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 diff --git a/lib/api/resource_milestone_events.rb b/lib/api/resource_milestone_events.rb index e466e236371..a8f221f8740 100644 --- a/lib/api/resource_milestone_events.rb +++ b/lib/api/resource_milestone_events.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module API - class ResourceMilestoneEvents < Grape::API + class ResourceMilestoneEvents < Grape::API::Instance include PaginationParams helpers ::API::Helpers::NotesHelpers diff --git a/lib/api/search.rb b/lib/api/search.rb index ac00d3682a0..092da6571c8 100644 --- 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! } diff --git a/lib/api/services.rb b/lib/api/services.rb index 5fd5c6bd9b0..9ee1822339c 100644 --- 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 diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 0bf5eed26b4..3463e29041b 100644 --- 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 @@ module API 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 @@ module API 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' @@ -113,13 +114,13 @@ module API 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 :repository_storages_weighted, type: Hash, desc: 'Storage paths for new projects with a weighted value between 0 and 100' 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' diff --git a/lib/api/sidekiq_metrics.rb b/lib/api/sidekiq_metrics.rb index 693c20cb73a..de1373144e3 100644 --- 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 diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index be58b832f97..3e6ccf7c0cf 100644 --- 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! } diff --git a/lib/api/statistics.rb b/lib/api/statistics.rb index d2dce34dfa5..3869fd3ac76 100644 --- 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, diff --git a/lib/api/submodules.rb b/lib/api/submodules.rb index 72d7d994102..34d21d3d7d8 100644 --- 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 diff --git a/lib/api/subscriptions.rb b/lib/api/subscriptions.rb index dfb54446ddf..533663fb087 100644 --- 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! } diff --git a/lib/api/suggestions.rb b/lib/api/suggestions.rb index 05aaa8a6f41..38e96c080f2 100644 --- 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 @@ -25,7 +25,7 @@ module API success Entities::Suggestion end params do - requires :ids, type: Array[String], desc: "An array of suggestion ID's" + requires :ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: "An array of suggestion ID's" end put 'batch_apply' do ids = params[:ids] diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb index 51fae0e54aa..d8e0a425625 100644 --- 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 diff --git a/lib/api/tags.rb b/lib/api/tags.rb index 796b1450602..c1fbd3ca7c6 100644 --- 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) diff --git a/lib/api/templates.rb b/lib/api/templates.rb index 51f357d9477..80a97aae429 100644 --- 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 = { diff --git a/lib/api/terraform/state.rb b/lib/api/terraform/state.rb index e7c9627c753..6b9809c76a4 100644 --- a/lib/api/terraform/state.rb +++ b/lib/api/terraform/state.rb @@ -4,7 +4,7 @@ require_dependency 'api/validations/validators/limit' module API module Terraform - class State < Grape::API + class State < Grape::API::Instance include ::Gitlab::Utils::StrongMemoize default_format :json diff --git a/lib/api/todos.rb b/lib/api/todos.rb index e36ddf21277..4a73e3e0e94 100644 --- 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! } diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index 65e9118e3ef..de67a149274 100644 --- 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 diff --git a/lib/api/user_counts.rb b/lib/api/user_counts.rb index 8df4b381bbf..90127ecbc73 100644 --- 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' diff --git a/lib/api/users.rb b/lib/api/users.rb index 3f89f1c253a..3124acd511f 100644 --- 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 diff --git a/lib/api/validations/types/comma_separated_to_array.rb b/lib/api/validations/types/comma_separated_to_array.rb index b551878abd1..409eb67a3d3 100644 --- a/lib/api/validations/types/comma_separated_to_array.rb +++ b/lib/api/validations/types/comma_separated_to_array.rb @@ -10,7 +10,7 @@ module API 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 diff --git a/lib/api/validations/types/comma_separated_to_integer_array.rb b/lib/api/validations/types/comma_separated_to_integer_array.rb new file mode 100644 index 00000000000..b8ab08b3fd4 --- /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 diff --git a/lib/api/validations/types/labels_list.rb b/lib/api/validations/types/labels_list.rb deleted file mode 100644 index 60277b99106..00000000000 --- 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 diff --git a/lib/api/validations/types/safe_file.rb b/lib/api/validations/types/safe_file.rb deleted file mode 100644 index 53b5790bfa2..00000000000 --- 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 diff --git a/lib/api/validations/types/workhorse_file.rb b/lib/api/validations/types/workhorse_file.rb index 18d111f6556..e65e94fc8db 100644 --- 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 diff --git a/lib/api/variables.rb b/lib/api/variables.rb index f3def756fe8..2a051c2adae 100644 --- 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! } diff --git a/lib/api/version.rb b/lib/api/version.rb index 2d8c90260fa..6a480fc2bd9 100644 --- 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 diff --git a/lib/api/wikis.rb b/lib/api/wikis.rb index 006dc257fe1..713136e0887 100644 --- 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 ::API::Helpers::WikisHelpers helpers do @@ -112,7 +112,7 @@ module API 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 diff --git a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb index a25a5e2335b..ae20d23a15c 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb @@ -51,12 +51,12 @@ module Gitlab partition_column = find_column_definition(table_name, column_name) raise "partition column #{column_name} does not exist on #{table_name}" if partition_column.nil? - new_table_name = partitioned_table_name(table_name) + partitioned_table_name = make_partitioned_table_name(table_name) - create_range_partitioned_copy(new_table_name, table_name, partition_column, primary_key) - create_daterange_partitions(new_table_name, partition_column.name, min_date, max_date) - create_trigger_to_sync_tables(table_name, new_table_name, primary_key) - enqueue_background_migration(table_name, new_table_name, primary_key) + create_range_partitioned_copy(table_name, partitioned_table_name, partition_column, primary_key) + create_daterange_partitions(partitioned_table_name, partition_column.name, min_date, max_date) + create_trigger_to_sync_tables(table_name, partitioned_table_name, primary_key) + enqueue_background_migration(table_name, partitioned_table_name, primary_key) end # Clean up a partitioned copy of an existing table. This deletes the partitioned table and all partitions. @@ -70,15 +70,15 @@ module Gitlab assert_not_in_transaction_block(scope: ERROR_SCOPE) with_lock_retries do - trigger_name = sync_trigger_name(table_name) + trigger_name = make_sync_trigger_name(table_name) drop_trigger(table_name, trigger_name) end - function_name = sync_function_name(table_name) + function_name = make_sync_function_name(table_name) drop_function(function_name) - part_table_name = partitioned_table_name(table_name) - drop_table(part_table_name) + partitioned_table_name = make_partitioned_table_name(table_name) + drop_table(partitioned_table_name) end def create_hash_partitions(table_name, number_of_partitions) @@ -107,15 +107,15 @@ module Gitlab "for more information please contact the database team" end - def partitioned_table_name(table) + def make_partitioned_table_name(table) tmp_table_name("#{table}_part") end - def sync_function_name(table) + def make_sync_function_name(table) object_name(table, 'table_sync_function') end - def sync_trigger_name(table) + def make_sync_trigger_name(table) object_name(table, 'table_sync_trigger') end @@ -123,11 +123,11 @@ module Gitlab connection.columns(table).find { |c| c.name == column.to_s } end - def create_range_partitioned_copy(table_name, template_table_name, partition_column, primary_key) - if table_exists?(table_name) + def create_range_partitioned_copy(source_table_name, partitioned_table_name, partition_column, primary_key) + if table_exists?(partitioned_table_name) # rubocop:disable Gitlab/RailsLogger Rails.logger.warn "Partitioned table not created because it already exists" \ - " (this may be due to an aborted migration or similar): table_name: #{table_name} " + " (this may be due to an aborted migration or similar): table_name: #{partitioned_table_name} " # rubocop:enable Gitlab/RailsLogger return end @@ -135,20 +135,20 @@ module Gitlab tmp_column_name = object_name(partition_column.name, 'partition_key') transaction do execute(<<~SQL) - CREATE TABLE #{table_name} ( - LIKE #{template_table_name} INCLUDING ALL EXCLUDING INDEXES, + CREATE TABLE #{partitioned_table_name} ( + LIKE #{source_table_name} INCLUDING ALL EXCLUDING INDEXES, #{tmp_column_name} #{partition_column.sql_type} NOT NULL, PRIMARY KEY (#{[primary_key, tmp_column_name].join(", ")}) ) PARTITION BY RANGE (#{tmp_column_name}) SQL - remove_column(table_name, partition_column.name) - rename_column(table_name, tmp_column_name, partition_column.name) - change_column_default(table_name, primary_key, nil) + remove_column(partitioned_table_name, partition_column.name) + rename_column(partitioned_table_name, tmp_column_name, partition_column.name) + change_column_default(partitioned_table_name, primary_key, nil) - if column_of_type?(table_name, primary_key, :integer) + if column_of_type?(partitioned_table_name, primary_key, :integer) # Default to int8 primary keys to prevent overflow - change_column(table_name, primary_key, :bigint) + change_column(partitioned_table_name, primary_key, :bigint) end end end @@ -191,19 +191,19 @@ module Gitlab create_range_partition(partition_name, table_name, lower_bound, upper_bound) end - def create_trigger_to_sync_tables(source_table, target_table, unique_key) - function_name = sync_function_name(source_table) - trigger_name = sync_trigger_name(source_table) + def create_trigger_to_sync_tables(source_table_name, partitioned_table_name, unique_key) + function_name = make_sync_function_name(source_table_name) + trigger_name = make_sync_trigger_name(source_table_name) with_lock_retries do - create_sync_function(function_name, target_table, unique_key) - create_comment('FUNCTION', function_name, "Partitioning migration: table sync for #{source_table} table") + create_sync_function(function_name, partitioned_table_name, unique_key) + create_comment('FUNCTION', function_name, "Partitioning migration: table sync for #{source_table_name} table") - create_sync_trigger(source_table, trigger_name, function_name) + create_sync_trigger(source_table_name, trigger_name, function_name) end end - def create_sync_function(name, target_table, unique_key) + def create_sync_function(name, partitioned_table_name, unique_key) if function_exists?(name) # rubocop:disable Gitlab/RailsLogger Rails.logger.warn "Partitioning sync function not created because it already exists" \ @@ -213,20 +213,20 @@ module Gitlab end delimiter = ",\n " - column_names = connection.columns(target_table).map(&:name) + column_names = connection.columns(partitioned_table_name).map(&:name) set_statements = build_set_statements(column_names, unique_key) insert_values = column_names.map { |name| "NEW.#{name}" } create_trigger_function(name, replace: false) do <<~SQL IF (TG_OP = 'DELETE') THEN - DELETE FROM #{target_table} where #{unique_key} = OLD.#{unique_key}; + DELETE FROM #{partitioned_table_name} where #{unique_key} = OLD.#{unique_key}; ELSIF (TG_OP = 'UPDATE') THEN - UPDATE #{target_table} + UPDATE #{partitioned_table_name} SET #{set_statements.join(delimiter)} - WHERE #{target_table}.#{unique_key} = NEW.#{unique_key}; + WHERE #{partitioned_table_name}.#{unique_key} = NEW.#{unique_key}; ELSIF (TG_OP = 'INSERT') THEN - INSERT INTO #{target_table} (#{column_names.join(delimiter)}) + INSERT INTO #{partitioned_table_name} (#{column_names.join(delimiter)}) VALUES (#{insert_values.join(delimiter)}); END IF; RETURN NULL; @@ -250,15 +250,15 @@ module Gitlab create_trigger(table_name, trigger_name, function_name, fires: 'AFTER INSERT OR UPDATE OR DELETE') end - def enqueue_background_migration(source_table, partitioned_table, source_key) - model_class = define_batchable_model(source_table) + def enqueue_background_migration(source_table_name, partitioned_table_name, source_key) + source_model = define_batchable_model(source_table_name) queue_background_migration_jobs_by_range_at_intervals( - model_class, + source_model, MIGRATION_CLASS_NAME, BATCH_INTERVAL, batch_size: BATCH_SIZE, - other_job_arguments: [source_table.to_s, partitioned_table, source_key]) + other_job_arguments: [source_table_name.to_s, partitioned_table_name, source_key]) end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d9e4037ff16..e0f55c3f51f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -19141,6 +19141,9 @@ msgstr "" msgid "Reports|Failure" msgstr "" +msgid "Reports|Identifier" +msgstr "" + msgid "Reports|Metrics reports are loading" msgstr "" diff --git a/rubocop/cop/api/grape_api_instance.rb b/rubocop/cop/api/grape_api_instance.rb new file mode 100644 index 00000000000..de11b9ef3f6 --- /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 diff --git a/rubocop/cop/api/grape_array_missing_coerce.rb b/rubocop/cop/api/grape_array_missing_coerce.rb new file mode 100644 index 00000000000..3d7a6a72d81 --- /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 diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb index a6a4aff7ce9..d10351feb9e 100644 --- a/spec/controllers/projects/refs_controller_spec.rb +++ b/spec/controllers/projects/refs_controller_spec.rb @@ -41,19 +41,12 @@ RSpec.describe Projects::RefsController do expect { xhr_get }.not_to raise_error end - it 'renders 404 for non-JS requests' do + it 'renders 404 for HTML requests' do xhr_get expect(response).to be_not_found end - it 'renders JS' do - expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original - - xhr_get(:js) - expect(response).to be_successful - end - context 'when json is requested' do it 'renders JSON' do expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original diff --git a/spec/lib/api/helpers/common_helpers_spec.rb b/spec/lib/api/helpers/common_helpers_spec.rb new file mode 100644 index 00000000000..5162d2f1000 --- /dev/null +++ b/spec/lib/api/helpers/common_helpers_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Helpers::CommonHelpers do + include Rack::Test::Methods + + subject do + Class.new(Grape::API) do + helpers API::Helpers::CommonHelpers + + before do + coerce_nil_params_to_array! + end + + params do + requires :id, type: String + optional :array, type: Array, coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce + optional :array_of_strings, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce + optional :array_of_ints, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce + end + get ":id" do + params.to_json + end + end + end + + def app + subject + end + + describe '.coerce_nil_params_to_array!' do + let(:json_response) { Gitlab::Json.parse(last_response.body) } + + it 'converts all nil parameters to empty arrays' do + get '/test?array=&array_of_strings=&array_of_ints=' + + expect(json_response['array']).to eq([]) + expect(json_response['array_of_strings']).to eq([]) + expect(json_response['array_of_ints']).to eq([]) + end + + it 'leaves non-nil parameters alone' do + get '/test?array=&array_of_strings=test,me&array_of_ints=1,2' + + expect(json_response['array']).to eq([]) + expect(json_response['array_of_strings']).to eq(%w(test me)) + expect(json_response['array_of_ints']).to eq([1, 2]) + end + end +end diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb index a257ca19385..2b3d7e76ae6 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end let_it_be(:connection) { ActiveRecord::Base.connection } - let(:template_table) { :audit_events } + let(:source_table) { :audit_events } let(:partitioned_table) { '_test_migration_partitioned_table' } let(:function_name) { '_test_migration_function_name' } let(:trigger_name) { '_test_migration_trigger_name' } @@ -22,9 +22,9 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe before do allow(migration).to receive(:puts) allow(migration).to receive(:transaction_open?).and_return(false) - allow(migration).to receive(:partitioned_table_name).and_return(partitioned_table) - allow(migration).to receive(:sync_function_name).and_return(function_name) - allow(migration).to receive(:sync_trigger_name).and_return(trigger_name) + allow(migration).to receive(:make_partitioned_table_name).and_return(partitioned_table) + allow(migration).to receive(:make_sync_function_name).and_return(function_name) + allow(migration).to receive(:make_sync_trigger_name).and_return(trigger_name) allow(migration).to receive(:assert_table_is_allowed) end @@ -38,14 +38,14 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end context 'when the table is not allowed' do - let(:template_table) { :this_table_is_not_allowed } + let(:source_table) { :this_table_is_not_allowed } it 'raises an error' do - expect(migration).to receive(:assert_table_is_allowed).with(template_table).and_call_original + expect(migration).to receive(:assert_table_is_allowed).with(source_table).and_call_original expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date - end.to raise_error(/#{template_table} is not allowed for use/) + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/#{source_table} is not allowed for use/) end end @@ -54,7 +54,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe expect(migration).to receive(:transaction_open?).and_return(true) expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date end.to raise_error(/can not be run inside a transaction/) end end @@ -64,7 +64,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe it 'raises an error' do expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date end.to raise_error(/max_date #{max_date} must be greater than min_date #{min_date}/) end end @@ -74,24 +74,24 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe it 'raises an error' do expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date end.to raise_error(/max_date #{max_date} must be greater than min_date #{min_date}/) end end context 'when the given table does not have a primary key' do - let(:template_table) { :_partitioning_migration_helper_test_table } + let(:source_table) { :_partitioning_migration_helper_test_table } let(:partition_column) { :some_field } it 'raises an error' do - migration.create_table template_table, id: false do |t| + migration.create_table source_table, id: false do |t| t.integer :id t.datetime partition_column end expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date - end.to raise_error(/primary key not defined for #{template_table}/) + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/primary key not defined for #{source_table}/) end end @@ -100,14 +100,14 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe it 'raises an error' do expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date end.to raise_error(/partition column #{partition_column} does not exist/) end end describe 'constructing the partitioned table' do it 'creates a table partitioned by the proper column' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect(connection.table_exists?(partitioned_table)).to be(true) expect(connection.primary_key(partitioned_table)).to eq(new_primary_key) @@ -116,7 +116,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end it 'changes the primary key datatype to bigint' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date pk_column = connection.columns(partitioned_table).find { |c| c.name == old_primary_key } @@ -131,13 +131,13 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end end - let(:template_table) { :another_example } + let(:source_table) { :another_example } let(:old_primary_key) { 'identifier' } it 'does not change the primary key datatype' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date - original_pk_column = connection.columns(template_table).find { |c| c.name == old_primary_key } + original_pk_column = connection.columns(source_table).find { |c| c.name == old_primary_key } pk_column = connection.columns(partitioned_table).find { |c| c.name == old_primary_key } expect(pk_column).not_to be_nil @@ -146,7 +146,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end it 'removes the default from the primary key column' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date pk_column = connection.columns(partitioned_table).find { |c| c.name == old_primary_key } @@ -154,16 +154,16 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end it 'creates the partitioned table with the same non-key columns' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date copied_columns = filter_columns_by_name(connection.columns(partitioned_table), new_primary_key) - original_columns = filter_columns_by_name(connection.columns(template_table), new_primary_key) + original_columns = filter_columns_by_name(connection.columns(source_table), new_primary_key) expect(copied_columns).to match_array(original_columns) end it 'creates a partition spanning over each month in the range given' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect_range_partitions_for(partitioned_table, { '000000' => ['MINVALUE', "'2019-12-01 00:00:00'"], @@ -175,7 +175,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end context 'when min_date is not given' do - let(:template_table) { :todos } + let(:source_table) { :todos } context 'with records present already' do before do @@ -183,7 +183,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end it 'creates a partition spanning over each month from the first record' do - migration.partition_table_by_date template_table, partition_column, max_date: max_date + migration.partition_table_by_date source_table, partition_column, max_date: max_date expect_range_partitions_for(partitioned_table, { '000000' => ['MINVALUE', "'2019-11-01 00:00:00'"], @@ -198,7 +198,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe context 'without data' do it 'creates the catchall partition plus two actual partition' do - migration.partition_table_by_date template_table, partition_column, max_date: max_date + migration.partition_table_by_date source_table, partition_column, max_date: max_date expect_range_partitions_for(partitioned_table, { '000000' => ['MINVALUE', "'2020-02-01 00:00:00'"], @@ -214,7 +214,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe today = Date.new(2020, 5, 8) Timecop.freeze(today) do - migration.partition_table_by_date template_table, partition_column, min_date: min_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date expect_range_partitions_for(partitioned_table, { '000000' => ['MINVALUE', "'2019-12-01 00:00:00'"], @@ -234,7 +234,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe it 'creates partitions for the current and next month' do current_date = Date.new(2020, 05, 22) Timecop.freeze(current_date.to_time) do - migration.partition_table_by_date template_table, partition_column + migration.partition_table_by_date source_table, partition_column expect_range_partitions_for(partitioned_table, { '000000' => ['MINVALUE', "'2020-05-01 00:00:00'"], @@ -247,7 +247,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end describe 'keeping data in sync with the partitioned table' do - let(:template_table) { :todos } + let(:source_table) { :todos } let(:model) { Class.new(ActiveRecord::Base) } let(:timestamp) { Time.utc(2019, 12, 1, 12).round } @@ -258,16 +258,16 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe it 'creates a trigger function on the original table' do expect_function_not_to_exist(function_name) - expect_trigger_not_to_exist(template_table, trigger_name) + expect_trigger_not_to_exist(source_table, trigger_name) - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect_function_to_exist(function_name) - expect_valid_function_trigger(template_table, trigger_name, function_name, after: %w[delete insert update]) + expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update]) end it 'syncs inserts to the partitioned tables' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect(model.count).to eq(0) @@ -280,7 +280,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end it 'syncs updates to the partitioned tables' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date first_todo = create(:todo, :pending, commit_id: nil, created_at: timestamp, updated_at: timestamp) second_todo = create(:todo, created_at: timestamp, updated_at: timestamp) @@ -301,7 +301,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end it 'syncs deletes to the partitioned tables' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date first_todo = create(:todo, created_at: timestamp, updated_at: timestamp) second_todo = create(:todo, created_at: timestamp, updated_at: timestamp) @@ -317,7 +317,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end describe 'copying historic data to the partitioned table' do - let(:template_table) { 'todos' } + let(:source_table) { 'todos' } let(:migration_class) { '::Gitlab::Database::PartitioningMigrationHelpers::BackfillPartitionedTable' } let(:sub_batch_size) { described_class::SUB_BATCH_SIZE } let(:pause_seconds) { described_class::PAUSE_SECONDS } @@ -333,14 +333,14 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe it 'enqueues jobs to copy each batch of data' do Sidekiq::Testing.fake! do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect(BackgroundMigrationWorker.jobs.size).to eq(2) - first_job_arguments = [first_id, second_id, template_table, partitioned_table, 'id'] + first_job_arguments = [first_id, second_id, source_table, partitioned_table, 'id'] expect(BackgroundMigrationWorker.jobs[0]['args']).to eq([migration_class, first_job_arguments]) - second_job_arguments = [third_id, third_id, template_table, partitioned_table, 'id'] + second_job_arguments = [third_id, third_id, source_table, partitioned_table, 'id'] expect(BackgroundMigrationWorker.jobs[1]['args']).to eq([migration_class, second_job_arguments]) end end @@ -353,37 +353,37 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end context 'when the table is not allowed' do - let(:template_table) { :this_table_is_not_allowed } + let(:source_table) { :this_table_is_not_allowed } it 'raises an error' do - expect(migration).to receive(:assert_table_is_allowed).with(template_table).and_call_original + expect(migration).to receive(:assert_table_is_allowed).with(source_table).and_call_original expect do - migration.drop_partitioned_table_for template_table - end.to raise_error(/#{template_table} is not allowed for use/) + migration.drop_partitioned_table_for source_table + end.to raise_error(/#{source_table} is not allowed for use/) end end it 'drops the trigger syncing to the partitioned table' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect_function_to_exist(function_name) - expect_valid_function_trigger(template_table, trigger_name, function_name, after: %w[delete insert update]) + expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update]) - migration.drop_partitioned_table_for template_table + migration.drop_partitioned_table_for source_table expect_function_not_to_exist(function_name) - expect_trigger_not_to_exist(template_table, trigger_name) + expect_trigger_not_to_exist(source_table, trigger_name) end it 'drops the partitioned copy and all partitions' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expected_tables.each do |table| expect(connection.table_exists?(table)).to be(true) end - migration.drop_partitioned_table_for template_table + migration.drop_partitioned_table_for source_table expected_tables.each do |table| expect(connection.table_exists?(table)).to be(false) diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index 5b2d835f89a..63fbf6e32dd 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -74,14 +74,15 @@ RSpec.describe API::Applications, :api do expect(json_response['error']).to eq('scopes is missing') end - it 'does not allow creating an application with confidential set to nil' do + it 'defaults to creating an application with confidential' do expect do post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '', confidential: nil } - end.not_to change { Doorkeeper::Application.count } + end.to change { Doorkeeper::Application.count }.by(1) - expect(response).to have_gitlab_http_status(:bad_request) + expect(response).to have_gitlab_http_status(:created) expect(json_response).to be_a Hash - expect(json_response['message']['confidential'].first).to eq('is not included in the list') + expect(json_response['callback_url']).to eq('http://application.url') + expect(json_response['confidential']).to be true end end diff --git a/spec/requests/api/graphql/mutations/jira_import/start_spec.rb b/spec/requests/api/graphql/mutations/jira_import/start_spec.rb index 050073f8b98..e7124512ef1 100644 --- a/spec/requests/api/graphql/mutations/jira_import/start_spec.rb +++ b/spec/requests/api/graphql/mutations/jira_import/start_spec.rb @@ -14,7 +14,8 @@ RSpec.describe 'Starting a Jira Import' do let(:mutation) do variables = { jira_project_key: jira_project_key, - project_path: project_path + project_path: project_path, + users_mapping: [{ jiraAccountId: 'abc', gitlabId: 5 }] } graphql_mutation(:jira_import_start, variables) diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 1df4d7ea9f6..602aacb6ced 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -62,14 +62,14 @@ RSpec.describe API::Settings, 'Settings' do 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, @@ -94,7 +94,9 @@ RSpec.describe API::Settings, 'Settings' do issues_create_limit: 300, raw_blob_request_limit: 300, spam_check_endpoint_enabled: true, - spam_check_endpoint_url: 'https://example.com/spam_check' + spam_check_endpoint_url: 'https://example.com/spam_check', + disabled_oauth_sign_in_sources: 'unknown', + import_sources: 'github,bitbucket' } expect(response).to have_gitlab_http_status(:ok) @@ -135,6 +137,8 @@ RSpec.describe API::Settings, 'Settings' do expect(json_response['raw_blob_request_limit']).to eq(300) expect(json_response['spam_check_endpoint_enabled']).to be_truthy expect(json_response['spam_check_endpoint_url']).to eq('https://example.com/spam_check') + expect(json_response['disabled_oauth_sign_in_sources']).to eq([]) + expect(json_response['import_sources']).to match_array(%w(github bitbucket)) end end diff --git a/spec/rubocop/cop/api/grape_api_instance_spec.rb b/spec/rubocop/cop/api/grape_api_instance_spec.rb new file mode 100644 index 00000000000..74f175cb707 --- /dev/null +++ b/spec/rubocop/cop/api/grape_api_instance_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require_relative '../../../../rubocop/cop/api/grape_api_instance' + +RSpec.describe RuboCop::Cop::API::GrapeAPIInstance do + include CopHelper + + subject(:cop) { described_class.new } + + it 'adds an offense when inheriting from Grape::API' do + inspect_source(<<~CODE) + class SomeAPI < Grape::API + end + CODE + + expect(cop.offenses.size).to eq(1) + end + + it 'does not add an offense when inheriting from Grape::API::Instance' do + inspect_source(<<~CODE) + class SomeAPI < Grape::API::Instance + end + CODE + + expect(cop.offenses.size).to be_zero + end +end diff --git a/spec/rubocop/cop/api/grape_array_missing_coerce_spec.rb b/spec/rubocop/cop/api/grape_array_missing_coerce_spec.rb new file mode 100644 index 00000000000..c7bb8255398 --- /dev/null +++ b/spec/rubocop/cop/api/grape_array_missing_coerce_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require_relative '../../../../rubocop/cop/api/grape_array_missing_coerce' + +RSpec.describe RuboCop::Cop::API::GrapeArrayMissingCoerce do + include CopHelper + + subject(:cop) { described_class.new } + + it 'adds an offense with a required parameter' do + inspect_source(<<~CODE) + 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) + 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) + 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) + class SomeClass + params do + requires :values, type: Array[String] + end + end + CODE + + expect(cop.offenses.size).to be_zero + end +end diff --git a/spec/rubocop/cop/code_reuse/worker_spec.rb b/spec/rubocop/cop/code_reuse/worker_spec.rb index bd1246ceb07..1f502e554c4 100644 --- a/spec/rubocop/cop/code_reuse/worker_spec.rb +++ b/spec/rubocop/cop/code_reuse/worker_spec.rb @@ -31,7 +31,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::Worker, type: :rubocop do .and_return(true) expect_offense(<<~SOURCE) - class Foo < Grape::API + class Foo < Grape::API::Instance resource :projects do get '/' do FooWorker.perform_async diff --git a/spec/services/jira_import/start_import_service_spec.rb b/spec/services/jira_import/start_import_service_spec.rb index 0437dd19e34..a10928355ef 100644 --- a/spec/services/jira_import/start_import_service_spec.rb +++ b/spec/services/jira_import/start_import_service_spec.rb @@ -8,8 +8,15 @@ RSpec.describe JiraImport::StartImportService do let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project) } let(:key) { 'KEY' } + let(:mapping) do + [ + { jira_account_id: 'abc', gitlab_id: 12 }, + { jira_account_id: 'def', gitlab_id: nil }, + { jira_account_id: nil, gitlab_id: 1 } + ] + end - subject { described_class.new(user, project, key).execute } + subject { described_class.new(user, project, key, mapping).execute } context 'when an error is returned from the project validation' do before do @@ -37,7 +44,7 @@ RSpec.describe JiraImport::StartImportService do context 'when correct data provided' do let(:fake_key) { 'some-key' } - subject { described_class.new(user, project, fake_key).execute } + subject { described_class.new(user, project, fake_key, mapping).execute } context 'when import is already running' do let_it_be(:jira_import_state) { create(:jira_import_state, :started, project: project) } @@ -62,35 +69,68 @@ RSpec.describe JiraImport::StartImportService do end context 'when everything is ok' do - it 'returns success response' do - expect(subject).to be_a(ServiceResponse) - expect(subject).to be_success - end + context 'with complete mapping' do + before do + expect(Gitlab::JiraImport).to receive(:cache_users_mapping).with(project.id, { 'abc' => 12 }) + end - it 'schedules Jira import' do - subject + it 'returns success response' do + expect(subject).to be_a(ServiceResponse) + expect(subject).to be_success + end - expect(project.latest_jira_import).to be_scheduled - end + it 'schedules Jira import' do + subject - it 'creates Jira import data', :aggregate_failures do - jira_import = subject.payload[:import_data] + expect(project.latest_jira_import).to be_scheduled + end + + it 'creates Jira import data', :aggregate_failures do + jira_import = subject.payload[:import_data] + + expect(jira_import.jira_project_xid).to eq(0) + expect(jira_import.jira_project_name).to eq(fake_key) + expect(jira_import.jira_project_key).to eq(fake_key) + expect(jira_import.user).to eq(user) + end + + it 'creates Jira import label' do + expect { subject }.to change { Label.count }.by(1) + end + + it 'creates Jira label title with correct number' do + jira_import = subject.payload[:import_data] + label_title = "jira-import::#{jira_import.jira_project_key}-1" - expect(jira_import.jira_project_xid).to eq(0) - expect(jira_import.jira_project_name).to eq(fake_key) - expect(jira_import.jira_project_key).to eq(fake_key) - expect(jira_import.user).to eq(user) + expect(jira_import.label.title).to eq(label_title) + end end - it 'creates Jira import label' do - expect { subject }.to change { Label.count }.by(1) + context 'when mapping is nil' do + let(:mapping) { nil } + + it 'returns success response' do + expect(Gitlab::JiraImport).not_to receive(:cache_users_mapping) + + expect(subject).to be_a(ServiceResponse) + expect(subject).to be_success + end end - it 'creates Jira label title with correct number' do - jira_import = subject.payload[:import_data] - label_title = "jira-import::#{jira_import.jira_project_key}-1" + context 'when no mapping value is complete' do + let(:mapping) do + [ + { jira_account_id: 'def', gitlab_id: nil }, + { jira_account_id: nil, gitlab_id: 1 } + ] + end - expect(jira_import.label.title).to eq(label_title) + it 'returns success response' do + expect(Gitlab::JiraImport).not_to receive(:cache_users_mapping) + + expect(subject).to be_a(ServiceResponse) + expect(subject).to be_success + end end end |
