fix(#2350): Update recognize_paths taking into account the route_param type

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * [#2371](https://github.com/ruby-grape/grape/pull/2371): Use a param value as the `default` value of other param - [@jcagarcia](https://github.com/jcagarcia).
 * [#2377](https://github.com/ruby-grape/grape/pull/2377): Allow to use instance variables values inside `rescue_from` - [@jcagarcia](https://github.com/jcagarcia).
+* [#2379](https://github.com/ruby-grape/grape/pull/2379): `recognize_path` now takes into account the `route_param` type - [@jcagarcia](https://github.com/jcagarcia).
 * Your contribution here.
 
 #### Fixes

README.md

@@ -2408,6 +2408,33 @@ end
 API.recognize_path '/statuses'
 ```
 
+Since version `2.1.0`, the `recognize_path` method takes into account the parameters type to determine which endpoint should match with given path.
+
+```ruby
+class Books < Grape::API
+ resource :books do
+ route_param :id, type: Integer do
+ # GET /books/:id
+ get do
+ #...
+ end
+ end
+
+ resource :share do
+ # POST /books/share
+ post do
+ # ....
+ end
+ end
+ end
+end
+
+API.recognize_path '/books/1' # => /books/:id
+API.recognize_path '/books/share' # => /books/share
+API.recognize_path '/books/other' # => nil
+```
+
+
 ## Allowed Methods
 
 When you add a `GET` route for a resource, a route for the `HEAD` method will also be added automatically. You can disable this behavior with `do_not_route_head!`.

UPGRADING.md

@@ -51,6 +51,50 @@ class TwitterAPI < Grape::API
 end
 ```
 
+#### Recognizing Path
+
+Grape now considers the types of the configured `route_params` in order to determine the endpoint that matches with the performed request.
+
+So taking into account this `Grape::API` class
+
+```ruby
+class Books < Grape::API
+ resource :books do
+ route_param :id, type: Integer do
+ # GET /books/:id
+ get do
+ #...
+ end
+ end
+
+ resource :share do
+ # POST /books/share
+ post do
+ # ....
+ end
+ end
+ end
+end
+```
+
+Before:
+```ruby
+API.recognize_path '/books/1' # => /books/:id
+API.recognize_path '/books/share' # => /books/:id
+API.recognize_path '/books/other' # => /books/:id
+```
+
+After:
+```ruby
+API.recognize_path '/books/1' # => /books/:id
+API.recognize_path '/books/share' # => /books/share
+API.recognize_path '/books/other' # => nil
+```
+
+This implies that before this changes, when you performed `/books/other` and it matched with the `/books/:id` endpoint, you get a `400 Bad Request` response because the type of the provided `:id` param was not an `Integer`. However, after upgrading to version `2.1.0` you will get a `404 Not Found` response, because there is not a defined endpoint that matches with `/books/other`.
+
+See [#2379](https://github.com/ruby-grape/grape/pull/2379) for more information.
+
 ### Upgrading to >= 2.0.0
 
 #### Headers

grape.gemspec

@@ -23,7 +23,7 @@ Gem::Specification.new do |s|
  s.add_runtime_dependency 'activesupport', '>= 5'
  s.add_runtime_dependency 'builder'
  s.add_runtime_dependency 'dry-types', '>= 1.1'
- s.add_runtime_dependency 'mustermann-grape', '~> 1.0.0'
+ s.add_runtime_dependency 'mustermann-grape', '~> 1.1.0'
  s.add_runtime_dependency 'rack', '>= 1.3.0'
  s.add_runtime_dependency 'rack-accept'
 

lib/grape/router/pattern.rb

@@ -28,8 +28,10 @@ def initialize(pattern, **options)
 
  def pattern_options(options)
  capture = extract_capture(**options)
+ params = options[:params]
  options = DEFAULT_PATTERN_OPTIONS.dup
  options[:capture] = capture if capture.present?
+ options[:params] = params if params.present?
  options
  end
 

spec/grape/api/recognize_path_spec.rb

@@ -17,5 +17,60 @@
  subject.get {}
  expect(subject.recognize_path('/bar/1234')).to be_nil
  end
+
+ context 'when parametrized route with type specified together with a static route' do
+ subject do
+ Class.new(described_class) do
+ resource :books do
+ route_param :id, type: Integer do
+ get do
+ end
+
+ resource :loans do
+ route_param :loan_id, type: Integer do
+ get do
+ end
+ end
+
+ resource :print do
+ post do
+ end
+ end
+ end
+ end
+
+ resource :share do
+ post do
+ end
+ end
+ end
+ end
+ end
+
+ it 'recognizes the static route when the parameter does not match with the specified type' do
+ actual = subject.recognize_path('/books/share').routes[0].origin
+ expect(actual).to eq('/books/share')
+ end
+
+ it 'does not recognize any endpoint when there is not other endpoint that matches with the requested path' do
+ actual = subject.recognize_path('/books/other')
+ expect(actual).to be_nil
+ end
+
+ it 'recognizes the parametrized route when the parameter matches with the specified type' do
+ actual = subject.recognize_path('/books/1').routes[0].origin
+ expect(actual).to eq('/books/:id')
+ end
+
+ it 'recognizes the static nested route when the parameter does not match with the specified type' do
+ actual = subject.recognize_path('/books/1/loans/print').routes[0].origin
+ expect(actual).to eq('/books/:id/loans/print')
+ end
+
+ it 'recognizes the nested parametrized route when the parameter matches with the specified type' do
+ actual = subject.recognize_path('/books/1/loans/33').routes[0].origin
+ expect(actual).to eq('/books/:id/loans/:loan_id')
+ end
+ end
  end
 end

spec/grape/api_spec.rb

@@ -1132,7 +1132,7 @@ class DummyFormatClass
  d = double('after mock')
 
  subject.params do
- requires :id, type: Integer
+ requires :id, type: Integer, values: [1, 2, 3]
  end
  subject.resource ':id' do
  before { a.do_something! }
@@ -1151,9 +1151,9 @@ class DummyFormatClass
  expect(c).to receive(:do_something!).exactly(0).times
  expect(d).to receive(:do_something!).exactly(0).times
 
- get '/abc'
+ get '/4'
  expect(last_response.status).to be 400
- expect(last_response.body).to eql 'id is invalid'
+ expect(last_response.body).to eql 'id does not have a valid value'
  end
 
  it 'calls filters in the correct order' do
@@ -4408,5 +4408,22 @@ def uniqe_id_route
  expect(last_response.body).to eq({ my_var: nil }.to_json)
  end
  end
+
+ context 'when set type to a route_param' do
+ context 'and the param does not match' do
+ it 'returns a 404 response' do
+ subject.namespace :books do
+ route_param :id, type: Integer do
+ get do
+ params[:id]
+ end
+ end
+ end
+
+ get '/books/other'
+ expect(last_response.status).to be 404
+ end
+ end
+ end
  end
 end