Skip to content

Spring cleaning #3

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

Open
rrrene wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Spring cleaning #3

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e08705
Add constants for default values in Engine
rrrene Apr 7, 2016
4b498f7
Improve method naming in middleware
rrrene Apr 7, 2016
778c04d
Rename `was_get_request?`
Apr 7, 2016
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
7 changes: 5 additions & 2 deletions lib/facebook_canvas/engine.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
module FacebookCanvas
class Engine < ::Rails::Engine
DEFAULT_SERVER_REGEX = /.*/
DEFAULT_CUSTOM_FILTER = proc { |_env| true }

initializer "FacebookCanvas.middleware" do |app|
server_name = FacebookCanvas.server_name || /.*/
custom_filter = FacebookCanvas.custom_filter || proc { |_env| true }
server_name = FacebookCanvas.server_name || DEFAULT_SERVER_REGEX
custom_filter = FacebookCanvas.custom_filter || DEFAULT_CUSTOM_FILTER
app.config.middleware.use FacebookCanvas::Middleware, server_name, custom_filter

ApplicationController.prepend FacebookCanvas::Helpers
Expand Down
13 changes: 10 additions & 3 deletions lib/facebook_canvas/middleware.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def initialize(app, request_host, custom_filter)

# Forces REQUEST_METHOD to GET if required.
def call(env)
if matches_server_name?(env) && was_get_request?(env) && !was_xhr_request?(env) && custom_filter?(env)
if matches_server_name?(env) && was_get_request?(env) && !xhr_request?(env) && custom_filter?(env)
Copy link
Contributor

@splattael splattael Apr 7, 2016

Choose a reason for hiding this comment

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

What about was_get_request?? Should we name it get_request??

Copy link
Contributor Author

@rrrene rrrene Apr 7, 2016

Choose a reason for hiding this comment

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

Maybe originally_was_get_request? 😉

env["REQUEST_METHOD"] = "GET"
end
@app.call env
Expand All @@ -33,11 +33,18 @@ def matches_server_name?(env)
end

def was_get_request?(env)
!submitted_via_rails_form?(env)
end

# We assume that Rails inserts a hidden parameter called `utf8` for all non
# GET requests.
def submitted_via_rails_form?(env)
form_hash = env["rack.request.form_hash"] || {}
!form_hash["utf8"]
form_hash["utf8"]
end

def was_xhr_request?(env)
# XHR requests generated by JavaScript should be left alone
def xhr_request?(env)
env['HTTP_X_REQUESTED_WITH'] == 'XMLHttpRequest'
end

Expand Down