fix(#2350): Update recognize_paths taking into account the `route_p…

…aram` type (#2379)

* fix(#2350): Use musterman-grape 1.1.0 for recognize_paths taking into account the route_param type

* fix(#2350): Updating wording and passing rubocop

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