Skip to content

Commit

Permalink
fix(ruby-grape#2350): Use musterman-grape 1.1.0 for recognize_paths t…
Browse files Browse the repository at this point in the history
…aking into account the route_param type
  • Loading branch information
jcagarcia committed Nov 24, 2023
1 parent 103928a commit c8adb79
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!`.
Expand Down
42 changes: 42 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,48 @@ class TwitterAPI < Grape::API
end
```

#### Recognizing Path

Due to the changes done in [#2379](https://github.com/ruby-grape/grape/pull/2379), Grape now takes in mind 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
```

This was the behavior before the changes:
```ruby
API.recognize_path '/books/1' # => /books/:id
API.recognize_path '/books/share' # => /books/:id
API.recognize_path '/books/other' # => /books/:id
```

And this is the behavior now:
```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`.

### Upgrading to >= 2.0.0

#### Headers
Expand Down
2 changes: 1 addition & 1 deletion grape.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down
2 changes: 2 additions & 0 deletions lib/grape/router/pattern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
67 changes: 67 additions & 0 deletions spec/grape/api/recognize_path_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,72 @@
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

context 'when the parameter does not match with the specified type' do
it 'recognizes the static route' do
actual = subject.recognize_path('/books/share').routes[0].origin
expect(actual).to eq('/books/share')
end

context 'when there is not other endpoint that matches with the requested path' do
it 'does not recognize any endpoint' do
actual = subject.recognize_path('/books/other')
expect(actual).to be_nil
end
end
end

context 'when the parameter matches with the specified type' do
it 'recognizes the parametrized route' do
actual = subject.recognize_path('/books/1').routes[0].origin
expect(actual).to eq('/books/:id')
end
end

context 'when requesting nested paths' do
context 'when the parameter does not match with the specified type' do
it 'recognizes the static route' do
actual = subject.recognize_path('/books/1/loans/print').routes[0].origin
expect(actual).to eq('/books/:id/loans/print')
end
end

context 'when the parameter matches with the specified type' do
it 'recognizes the parametrized route' 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
end
end
23 changes: 20 additions & 3 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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! }
Expand All @@ -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
Expand Down Expand Up @@ -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

0 comments on commit c8adb79

Please sign in to comment.