Merge branch 'grape-router-helpers-bugfix' into 'master'

Fix grape-route-helper route shadowing

See merge request gitlab-org/gitlab-ce!16781

2 files changed, 30 insertions, 0 deletions

diff --git a/config/initializers/grape_route_helpers_fix.rb b/config/initializers/grape_route_helpers_fix.rb
index d3cf9e453d0..612cca3dfbd 100644
--- a/config/initializers/grape_route_helpers_fix.rb
+++ b/config/initializers/grape_route_helpers_fix.rb
@@ -1,5 +1,21 @@
 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.
diff --git a/spec/initializers/grape_route_helpers_fix_spec.rb b/spec/initializers/grape_route_helpers_fix_spec.rb
new file mode 100644
index 00000000000..2cf5924128f
--- /dev/null
+++ b/spec/initializers/grape_route_helpers_fix_spec.rb
@@ -0,0 +1,14 @@
+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