Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Makes sure Grape::API behaves as a Rack::App #1893

Merged
merged 2 commits into from
Jul 5, 2019
Merged

Makes sure Grape::API behaves as a Rack::App #1893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d29eb79
Makes sure Grape::API behaves as a Rack::App by calling: 'call' on th…
myxoh Jul 5, 2019
0efc8c7
Addresses code style issues
myxoh Jul 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#### Fixes

* Your contribution here.
* [#1893](https://github.com/ruby-grape/grape/pull/1893): Allows `Grape::API` to behave like a Rack::app in some instances where it was misbehaving - [@myxoh](https://github.com/myxoh).

### 1.2.4 (2019/06/13)

Expand Down
15 changes: 14 additions & 1 deletion lib/grape/api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ def override_all_methods!
# (http://www.rubydoc.info/github/rack/rack/master/file/SPEC) for more.
# NOTE: This will only be called on an API directly mounted on RACK
def call(*args, &block)
base_instance.call(*args, &block)
instance_for_rack = if never_mounted?
base_instance
else
mounted_instances.first
end
instance_for_rack.call(*args, &block)
end

# Allows an API to itself be inheritable:
Expand Down Expand Up @@ -147,6 +152,14 @@ def evaluate_arguments(configuration, *args)
end
end
end

def never_mounted?
mounted_instances.empty?
end

def mounted_instances
instances - [base_instance]
end
end
end
end
22 changes: 22 additions & 0 deletions spec/grape/integration/rack_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,26 @@
input.unlink
end
end

context 'when the app is mounted' do
def app
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pollutess the global namespace, put it into a namespace itself?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what way @dblock ? I am under the impression that methods defined in a context are contained to that context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, this is news to me. Since when does it work as one would expect?!

require 'rspec'

describe 'Context' do
  def m1
    'm1'
  end

  it 'is m1' do
    expect(m1).to eq 'm1'
  end

  context 'c1' do
  	def m2
	  'c1:m2'
  	end

    it 'is m1' do
      expect(m1).to eq 'm1'
    end

    it 'is m2' do
      expect(m2).to eq 'c1:m2'
    end
  end

  context 'c2' do
	it 'is m2' do
	  expect(m2).to eq 'c1:m2'
    end
  end
end
  1) Context c2 is m2
     Failure/Error: expect(m2).to eq 'c1:m2'
     
     NameError:
       undefined local variable or method `m2' for #<RSpec::ExampleGroups::Context::C2 "is m2" (./spec.rb:27)>
     # ./spec.rb:28:in `block (3 levels) in <top (required)>'

My bad. Ignore me.

@main_app ||= Class.new(Grape::API) do
get 'ping'
end
end

let!(:base) do
app_to_mount = app
Class.new(Grape::API) do
namespace 'namespace' do
mount app_to_mount
end
end
end

it 'finds the app on the namespace' do
get '/namespace/ping'
expect(last_response.status).to eq 200
end
end
end