diff options
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | Gemfile.lock | 12 | ||||
| -rw-r--r-- | changelogs/unreleased/sh-use-grape-path-helpers.yml | 5 | ||||
| -rw-r--r-- | config/initializers/grape_route_helpers_fix.rb | 51 | ||||
| -rw-r--r-- | lib/api/helpers/related_resources_helpers.rb | 2 | ||||
| -rw-r--r-- | spec/initializers/grape_route_helpers_fix_spec.rb | 14 | ||||
| -rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 12 |
7 files changed, 25 insertions, 73 deletions
@@ -28,7 +28,7 @@ gem 'mysql2', '~> 0.4.10', group: :mysql gem 'pg', '~> 0.18.2', group: :postgres gem 'rugged', '~> 0.27' -gem 'grape-route-helpers', '~> 2.1.0' +gem 'grape-path-helpers', '~> 1.0' gem 'faraday', '~> 0.12' diff --git a/Gemfile.lock b/Gemfile.lock index 8ae183188cc..471deae7e25 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -348,7 +348,7 @@ GEM signet (~> 0.7) gpgme (2.0.13) mini_portile2 (~> 2.1) - grape (1.0.2) + grape (1.0.3) activesupport builder mustermann-grape (~> 1.0.0) @@ -358,10 +358,10 @@ GEM grape-entity (0.7.1) activesupport (>= 4.0) multi_json (>= 1.3.2) - grape-route-helpers (2.1.0) - activesupport - grape (>= 0.16.0) - rake + grape-path-helpers (1.0.1) + activesupport (~> 4) + grape (~> 1.0) + rake (~> 12) grape_logging (1.7.0) grape grpc (1.11.0) @@ -1049,7 +1049,7 @@ DEPENDENCIES gpgme grape (~> 1.0) grape-entity (~> 0.7.1) - grape-route-helpers (~> 2.1.0) + grape-path-helpers (~> 1.0) grape_logging (~> 1.7) grpc (~> 1.11.0) haml_lint (~> 0.26.0) diff --git a/changelogs/unreleased/sh-use-grape-path-helpers.yml b/changelogs/unreleased/sh-use-grape-path-helpers.yml new file mode 100644 index 00000000000..c462c7e8194 --- /dev/null +++ b/changelogs/unreleased/sh-use-grape-path-helpers.yml @@ -0,0 +1,5 @@ +--- +title: Replace grape-route-helpers with our own grape-path-helpers +merge_request: +author: +type: performance diff --git a/config/initializers/grape_route_helpers_fix.rb b/config/initializers/grape_route_helpers_fix.rb deleted file mode 100644 index 612cca3dfbd..00000000000 --- a/config/initializers/grape_route_helpers_fix.rb +++ /dev/null @@ -1,51 +0,0 @@ -if defined?(GrapeRouteHelpers) - module GrapeRouteHelpers - module AllRoutes - # Bringing in PR https://github.com/reprah/grape-route-helpers/pull/21 due to abandonment. - # - # Without the following fix, when two helper methods are the same, but have different arguments - # (for example: api_v1_cats_owners_path(id: 1) vs api_v1_cats_owners_path(id: 1, owner_id: 2)) - # if the helper method with the least number of arguments is defined first (because the route was defined first) - # then it will shadow the longer route. - # - # The fix is to sort descending by amount of arguments - def decorated_routes - @decorated_routes ||= all_routes - .map { |r| DecoratedRoute.new(r) } - .sort_by { |r| -r.dynamic_path_segments.count } - end - end - - class DecoratedRoute - # GrapeRouteHelpers gem tries to parse the versions - # from a string, not supporting Grape `version` array definition. - # - # Without the following fix, we get this on route helpers generation: - # - # => undefined method `scan' for ["v3", "v4"] - # - # 2.0.0 implementation of this method: - # - # ``` - # def route_versions - # version_pattern = /[^\[",\]\s]+/ - # if route_version - # route_version.scan(version_pattern) - # else - # [nil] - # end - # end - # ``` - def route_versions - return [nil] if route_version.nil? || route_version.empty? - - if route_version.is_a?(String) - version_pattern = /[^\[",\]\s]+/ - route_version.scan(version_pattern) - else - route_version - end - end - end - end -end diff --git a/lib/api/helpers/related_resources_helpers.rb b/lib/api/helpers/related_resources_helpers.rb index a2cbed30229..bc7333ca4b3 100644 --- a/lib/api/helpers/related_resources_helpers.rb +++ b/lib/api/helpers/related_resources_helpers.rb @@ -1,7 +1,7 @@ module API module Helpers module RelatedResourcesHelpers - include GrapeRouteHelpers::NamedRouteMatcher + include GrapePathHelpers::NamedRouteMatcher def issues_available?(project, options) available?(:issues, project, options[:current_user]) diff --git a/spec/initializers/grape_route_helpers_fix_spec.rb b/spec/initializers/grape_route_helpers_fix_spec.rb deleted file mode 100644 index 2cf5924128f..00000000000 --- a/spec/initializers/grape_route_helpers_fix_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'spec_helper' -require_relative '../../config/initializers/grape_route_helpers_fix' - -describe 'route shadowing' do - include GrapeRouteHelpers::NamedRouteMatcher - - it 'does not occur' do - path = api_v4_projects_merge_requests_path(id: 1) - expect(path).to eq('/api/v4/projects/1/merge_requests') - - path = api_v4_projects_merge_requests_path(id: 1, merge_request_iid: 3) - expect(path).to eq('/api/v4/projects/1/merge_requests/3') - end -end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 6b91e48ae6a..8b168816d6c 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -29,6 +29,18 @@ describe API::MergeRequests do project.add_reporter(user) end + describe 'route shadowing' do + include GrapePathHelpers::NamedRouteMatcher + + it 'does not occur' do + path = api_v4_projects_merge_requests_path(id: 1) + expect(path).to eq('/api/v4/projects/1/merge_requests') + + path = api_v4_projects_merge_requests_path(id: 1, merge_request_iid: 3) + expect(path).to eq('/api/v4/projects/1/merge_requests/3') + end + end + describe 'GET /merge_requests' do context 'when unauthenticated' do it 'returns an array of all merge requests' do |