Upgrade Grape v1.1.0 to v1.3.2

This brings in Ruby 2.7 suport and a number of fixes: https://github.com/ruby-grape/grape/blob/master/CHANGELOG.md 1. Move all inherited Grape::API -> Grape::API::Instance 2. Remove use of Virtus since this has been removed from Grape. 3. Extract Rack::Response from API error 4. Grape v1.2.3 pulled in a fix used in SafeFile: ruby-grape/grape#1844, so we no longer need to maintain our custom type. 5. Adapt WorkhorseFile with the latest changes to make custom types work with Grape and dry-types. 6. Ensure Array[String] is coerced properly. The change from Virtus to dry-types now requires all strings to be coerced to arrays. Before this was done within Virtus. 7. Coerce Array[Integer] types to arrays of integers The change from Virtus to dry-types now requires all strings to be coerced to arrays of integers. Before this was done within Virtus.
terrchen · Apr 27, 2020 · f31bac1 · f31bac1
1 parent afc9724
commit f31bac1
Show file tree

Hide file tree

Showing 164 changed files with 282 additions and 264 deletions.
diff --git a/Gemfile b/Gemfile
@@ -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'
@@ -82,7 +82,7 @@ gem 'gitlab_omniauth-ldap', '~> 2.1.1', require: 'omniauth-ldap'
 gem 'net-ldap'
 
 # API
-gem 'grape', '~> 1.1.0'
+gem 'grape', '~> 1.3.2'
 gem 'grape-entity', '~> 0.7.1'
 gem 'rack-cors', '~> 1.0.6', require: 'rack/cors'
 

diff --git 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)
@@ -165,8 +161,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)
@@ -222,8 +216,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)
@@ -250,6 +242,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.7)
     gpgme (2.0.20)
       mini_portile2 (~> 2.3)
-    grape (1.1.0)
+    grape (1.3.2)
       activesupport
       builder
+      dry-types (>= 1.1)
       mustermann-grape (~> 1.0.0)
       rack (>= 1.3.0)
       rack-accept
-      virtus (>= 1.0.0)
     grape-entity (0.7.1)
       activesupport (>= 4.0)
       multi_json (>= 1.3.2)
-    grape-path-helpers (1.2.0)
+    grape-path-helpers (1.3.0)
       activesupport
-      grape (~> 1.0)
+      grape (~> 1.3)
       rake (~> 12)
     grape_logging (1.8.3)
       grape
@@ -645,9 +659,10 @@ 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)
@@ -961,6 +976,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)
@@ -1119,11 +1135,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)
@@ -1254,9 +1265,9 @@ DEPENDENCIES
   google-api-client (~> 0.23)
   google-protobuf (~> 3.8.0)
   gpgme (~> 2.0.19)
-  grape (~> 1.1.0)
+  grape (~> 1.3.2)
   grape-entity (~> 0.7.1)
-  grape-path-helpers (~> 1.2)
+  grape-path-helpers (~> 1.3)
   grape_logging (~> 1.7)
   graphiql-rails (~> 1.4.10)
   graphql (~> 1.10.5)

diff --git a/changelogs/unreleased/sh-update-grape-gem.yml b/changelogs/unreleased/sh-update-grape-gem.yml
@@ -0,0 +1,5 @@
+---
+title: Upgrade Grape v1.1.0 to v1.3.2
+merge_request: 27276
+author:
+type: other
diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md
@@ -98,6 +98,14 @@ 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.
+
 ## 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
@@ -513,12 +513,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
-paramters out of the `Grape::API` class to a helper module, so we can inject it
+parameters out of the `Grape::API::Instance` class to a helper module, so we can inject it
 before it would be used in the class.
 
 ```ruby
 module API
-  class Projects < Grape::API
+  class Projects < Grape::API::Instance
     helpers Helpers::ProjectsHelpers
   end
 end
@@ -579,7 +579,7 @@ 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!
@@ -623,7 +623,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)
@@ -692,7 +692,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[
@@ -708,7 +708,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/ee/lib/api/analytics/code_review_analytics.rb b/ee/lib/api/analytics/code_review_analytics.rb
@@ -2,7 +2,7 @@
 
 module API
   module Analytics
-    class CodeReviewAnalytics < Grape::API
+    class CodeReviewAnalytics < Grape::API::Instance
       include PaginationParams
 
       helpers ::Gitlab::IssuableMetadata

diff --git a/ee/lib/api/analytics/group_activity_analytics.rb b/ee/lib/api/analytics/group_activity_analytics.rb
@@ -2,7 +2,7 @@
 
 module API
   module Analytics
-    class GroupActivityAnalytics < Grape::API
+    class GroupActivityAnalytics < Grape::API::Instance
       DESCRIPTION_DETAIL =
         'This feature is gated by the `:group_activity_analytics`'\
         ' feature flag, introduced in GitLab 12.9.'

diff --git a/ee/lib/api/audit_events.rb b/ee/lib/api/audit_events.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class AuditEvents < ::Grape::API
+  class AuditEvents < ::Grape::API::Instance
     include ::API::PaginationParams
 
     before do

diff --git a/ee/lib/api/composer_packages.rb b/ee/lib/api/composer_packages.rb
@@ -2,7 +2,7 @@
 
 # PHP composer support (https://getcomposer.org/)
 module API
-  class ComposerPackages < Grape::API
+  class ComposerPackages < Grape::API::Instance
     helpers ::API::Helpers::PackagesManagerClientsHelpers
     helpers ::API::Helpers::RelatedResourcesHelpers
     helpers ::API::Helpers::Packages::BasicAuthHelpers

diff --git a/ee/lib/api/conan_packages.rb b/ee/lib/api/conan_packages.rb
@@ -9,7 +9,7 @@
 #
 # Technical debt: https://gitlab.com/gitlab-org/gitlab/issues/35798
 module API
-  class ConanPackages < Grape::API
+  class ConanPackages < Grape::API::Instance
     helpers ::API::Helpers::PackagesManagerClientsHelpers
 
     PACKAGE_REQUIREMENTS = {

diff --git a/ee/lib/api/dependencies.rb b/ee/lib/api/dependencies.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class Dependencies < Grape::API
+  class Dependencies < Grape::API::Instance
     helpers do
       def dependencies_by(params)
         pipeline = ::Security::ReportFetchService.new(user_project, ::Ci::JobArtifact.dependency_list_reports).pipeline
@@ -28,6 +28,7 @@ def dependencies_by(params)
       params do
         optional :package_manager,
                  type: Array[String],
+                 coerce_with: Validations::Types::CommaSeparatedToArray.coerce,
                  desc: "Returns dependencies belonging to specified package managers: #{::Security::DependencyListService::FILTER_PACKAGE_MANAGERS_VALUES.join(', ')}.",
                  values: ::Security::DependencyListService::FILTER_PACKAGE_MANAGERS_VALUES
       end

diff --git a/ee/lib/api/dependency_proxy.rb b/ee/lib/api/dependency_proxy.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class DependencyProxy < Grape::API
+  class DependencyProxy < Grape::API::Instance
     helpers ::API::Helpers::PackagesHelpers
 
     helpers do

diff --git a/ee/lib/api/elasticsearch_indexed_namespaces.rb b/ee/lib/api/elasticsearch_indexed_namespaces.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class ElasticsearchIndexedNamespaces < Grape::API
+  class ElasticsearchIndexedNamespaces < Grape::API::Instance
     before { authenticated_as_admin! }
 
     resource :elasticsearch_indexed_namespaces do

diff --git a/ee/lib/api/epic_issues.rb b/ee/lib/api/epic_issues.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class EpicIssues < Grape::API
+  class EpicIssues < Grape::API::Instance
     before do
       authenticate!
       authorize_epics_feature!

diff --git a/ee/lib/api/epic_links.rb b/ee/lib/api/epic_links.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class EpicLinks < Grape::API
+  class EpicLinks < Grape::API::Instance
     include ::Gitlab::Utils::StrongMemoize
 
     before do

diff --git a/ee/lib/api/epics.rb b/ee/lib/api/epics.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class Epics < Grape::API
+  class Epics < Grape::API::Instance
     include PaginationParams
 
     before do
@@ -29,7 +29,7 @@ class Epics < Grape::API
         optional :state, type: String, values: %w[opened closed all], default: 'all',
                          desc: 'Return opened, closed, or all epics'
         optional :author_id, type: Integer, desc: 'Return epics which are authored by the user with the given ID'
-        optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
+        optional :labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names'
         optional :with_labels_details, type: Boolean, desc: 'Return titles of labels and other details', default: false
         optional :created_after, type: DateTime, desc: 'Return epics created after the specified time'
         optional :created_before, type: DateTime, desc: 'Return epics created before the specified time'
@@ -70,7 +70,7 @@ class Epics < Grape::API
         optional :start_date_is_fixed, type: Boolean, desc: 'Indicates start date should be sourced from start_date_fixed field not the issue milestones'
         optional :end_date, as: :due_date_fixed, type: String, desc: 'The due date of an epic'
         optional :due_date_is_fixed, type: Boolean, desc: 'Indicates due date should be sourced from due_date_fixed field not the issue milestones'
-        optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
+        optional :labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names'
         optional :parent_id, type: Integer, desc: 'The id of a parent epic'
       end
       post ':id/(-/)epics' do
@@ -96,7 +96,7 @@ class Epics < Grape::API
         optional :start_date_is_fixed, type: Boolean, desc: 'Indicates start date should be sourced from start_date_fixed field not the issue milestones'
         optional :end_date, as: :due_date_fixed, type: String, desc: 'The due date of an epic'
         optional :due_date_is_fixed, type: Boolean, desc: 'Indicates due date should be sourced from due_date_fixed field not the issue milestones'
-        optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
+        optional :labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names'
         optional :state_event, type: String, values: %w[reopen close], desc: 'State event for an epic'
         at_least_one_of :title, :description, :start_date_fixed, :start_date_is_fixed, :due_date_fixed, :due_date_is_fixed, :labels, :state_event, :confidential
       end

diff --git a/ee/lib/api/feature_flag_scopes.rb b/ee/lib/api/feature_flag_scopes.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class FeatureFlagScopes < Grape::API
+  class FeatureFlagScopes < Grape::API::Instance
     include PaginationParams
 
     ENVIRONMENT_SCOPE_ENDPOINT_REQUIREMENTS = FeatureFlags::FEATURE_FLAG_ENDPOINT_REQUIREMENTS

diff --git a/ee/lib/api/feature_flags.rb b/ee/lib/api/feature_flags.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class FeatureFlags < Grape::API
+  class FeatureFlags < Grape::API::Instance
     include PaginationParams
 
     FEATURE_FLAG_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS

diff --git a/ee/lib/api/feature_flags_user_lists.rb b/ee/lib/api/feature_flags_user_lists.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class FeatureFlagsUserLists < Grape::API
+  class FeatureFlagsUserLists < Grape::API::Instance
     include PaginationParams
 
     error_formatter :json, -> (message, _backtrace, _options, _env, _original_exception) {

diff --git a/ee/lib/api/geo.rb b/ee/lib/api/geo.rb
@@ -3,7 +3,7 @@
 require 'base64'
 
 module API
-  class Geo < Grape::API
+  class Geo < Grape::API::Instance
     resource :geo do
       helpers do
         def sanitized_node_status_params

diff --git a/ee/lib/api/geo_nodes.rb b/ee/lib/api/geo_nodes.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module API
-  class GeoNodes < Grape::API
+  class GeoNodes < Grape::API::Instance
     include PaginationParams
     include APIGuard
     include ::Gitlab::Utils::StrongMemoize