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.

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

Allow blocks when specifying Rack middleware to use #98

Merged
merged 2 commits into from Mar 16, 2015

Conversation

Erol
Copy link
Contributor

@Erol Erol commented Mar 15, 2015

I am using Lotus::Controller outside of a Lotus application and ran into a problem when trying to use Rack::Auth::Basic. I was trying to configure it just like below:

Lotus::Controller.configure do
  prepare do
    use Rack::Auth::Basic, "Protected Area" do |username, password|
      username == 'admin' && password == 'password'
    end
  end
end

But I noticed that Lotus::Action::Rack::ClassMethods#use does not accept blocks when specifying middleware. I checked https://github.com/rack/rack/blob/master/lib/rack/builder.rb#L81 and Rack::Builder#use does accept blocks.

Is this intended behavior? Am I missing something?

@Erol
Copy link
Contributor Author

Erol commented Mar 15, 2015

Also, can't find the test for Lotus::Action::Rack::ClassMethods#use. Should I make one? 🐹

@jodosha
Copy link
Member

jodosha commented Mar 15, 2015

@Erol You're right, it should accept a block.

Action.use

Action.use is intended for fine grained middleware configuration. For instance, if only a few resources need to use a Rack component, but not the entire app. When used, the action has a small perf penalty, but this is a good compromise because incoming requests won't go through the middleware when not needed.

Eg.

class RuntimeAction
  include Lotus::Action
  use Rack::Runtime

  def call(params)
    # ...
  end
end

class DashboardAction
  include Lotus::Action

  def call(params)
    # ...
  end
end

# GET /runtime # small perf penalty because of .use
# GET /dashboard # perf is OK, because it doesn't go through Rack::Runtime

Configuration#prepare

On the other hand, Configuration#prepare is intended for general configurations (not fine grained). Configuring .use for all the action, will add that small perf penalty to all the actions.

Solution

If you need to filter all the requests for all the actions (no fine grained approach), I suggest to mount that middleware in config.ru

@jodosha
Copy link
Member

jodosha commented Mar 15, 2015

@Erol Please add tests here test/integration/use_test.rb, and I'll be happy to merge this. Thank you 👍

@jodosha jodosha self-assigned this Mar 15, 2015
@Erol
Copy link
Contributor Author

Erol commented Mar 15, 2015

@jodosha Done. And thanks for the config.ru tip!

@jodosha jodosha added this to the v0.4.0 milestone Mar 16, 2015
@jodosha jodosha merged commit 0aace82 into hanami:master Mar 16, 2015
@jodosha
Copy link
Member

jodosha commented Mar 16, 2015

@Erol Thanks for the quick implementation. Merged 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants