Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

FaradayMiddleware::MethodOverride to complement Rack::MethodOverride. #48

Closed
wants to merge 4 commits into from

3 participants

@pda

Rewrites poorly supported HTTP request methods to a custom X-Http-Method-Override header and sends the request as POST.

This is supported by default in Rack / Rails apps via the Rack::MethodOverride module.

A current use-case is sending HTTP PATCH requests to an app on the Heroku Cedar stack, which does not route that request method at all.

Example usage:

faraday = Faraday.new(url) do |conn|
  # Send PATCH and DELETE requests as POST.
  conn.request :method_override, :patch, :delete
end
@pda pda FaradayMiddleware::MethodOverride to complement Rack::MethodOverride.
Rewrites poorly supported HTTP request methods to a custom
X-Http-Method-Override header and sends the request as POST.

This is supported by default in Rack / Rails apps via the
Rack::MethodOverride module.
51a7198
@travisbot

This pull request fails (merged 51a7198 into 9b7c8c6).

@pda

Failing due to pre-existing cane warnings which will be resolved by #49.

@mislav
Collaborator
  1. Where does HEADER in the implementation come from?
  2. Don't use private methods in specs. Simply instantiate the middleware, call call on it with an env hash and see what comes to the other side.
@travisbot

This pull request passes (merged ce247d3 into 9b7c8c6).

@pda
  1. Woah. That must have crept in when I was dithering between using a constant or just a string literal. Interesting that the HEADER constant declared in the spec's describe block was causing the spec to pass… I'd have thought that constant would be scoped to the RSpec ExampleGroup and not be accessible by the subject under test. Anyway, fixed.

  2. I gather you're talking about the #method_and_header_for method? I agree tests should have as little indirection as possible, so I've removed that, and simplified the instantiation of the middleware under test. The #env_for_method(method) helper still seems worthwhile.

I'm happy to squash the commits together if you'd prefer a simpler and more correct history.

Here's the spec output… I think the thorough coverage justifies the meta / declarative specs, and I've included the simple use case example group to pick up any errors which the complexity of the declarative specs might hide.

FaradayMiddleware::MethodOverride
  for a simple use case
    rewrites a PATCH request
    does not rewrite a GET request
  configured for [:delete, :put, :patch]
    sends DELETE as POST with DELETE in header
    sends GET unmodified with no header
    sends PATCH as POST with PATCH in header
    sends POST unmodified with no header
    sends PUT as POST with PUT in header
  configured for []
    sends GET unmodified with no header
    sends POST unmodified with no header
    sends PUT unmodified with no header
  configured for [:post]
    sends POST as POST with POST in header
  configured for ["patch", "PUT"]
    sends GET unmodified with no header
    sends PATCH as POST with PATCH in header
    sends PUT as POST with PUT in header
  configured for [:patch, :put]
    sends GET unmodified with no header
    sends PATCH as POST with PATCH in header
    sends POST unmodified with no header
    sends PUT as POST with PUT in header
@travisbot

This pull request passes (merged 85b14bc into 9b7c8c6).

@mislav mislav referenced this pull request from a commit
@mislav mislav Improve MethodOverride middleware and specs
By default, the middleware now rewrites all requests except GET and
POST. A `:rewrite` option can be passed to whitelist only the methods
which should be overridden.

References #48
c020035
@mislav
Collaborator

I have merged your stuff and improved in c020035.

I wanted it to have a sane default. So I defined it to rewrite all requests sans GET and POST by default.

I also rewrote your specs. They were needlessly complex, you doing the whole declarative matrix and all. Specs are supposed to be written with a predictable style and it's OK to have some amount of repetition if it helps clarity of the each spec. Also, because the logic in the middleware is simple and straightforward, you don't have to test every imaginable HTTP method combination.

Nested spec output:

FaradayMiddleware::MethodOverride
  with default options
    GET
      keeps original method
      doesn't set header value
    POST
      keeps original method
      doesn't set header value
    PUT
      sets physical method to POST
      sets header to PUT
  configured to rewrite [:patch, :delete]
    PUT
      keeps original method
      doesn't set header value
    PATCH
      sets physical method to POST
      sets header to PATCH
    DELETE
      sets physical method to POST
      sets header to DELETE
  configured to rewrite ['PATCH']
    PATCH
      sets physical method to POST
      sets header to PATCH
  with invalid option
    raises KeyError
@mislav mislav closed this
@pda

Thanks - appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 29, 2012
  1. @pda

    FaradayMiddleware::MethodOverride to complement Rack::MethodOverride.

    pda authored
    Rewrites poorly supported HTTP request methods to a custom
    X-Http-Method-Override header and sends the request as POST.
    
    This is supported by default in Rack / Rails apps via the
    Rack::MethodOverride module.
Commits on Aug 30, 2012
  1. @pda
  2. @pda
  3. @pda
This page is out of date. Refresh to see the latest.
View
1  README.md
@@ -28,6 +28,7 @@ require 'faraday_middleware'
connection = Faraday.new 'http://example.com/api' do |conn|
conn.request :oauth2, 'TOKEN'
conn.request :json
+ conn.request :method_override, :patch, :put, :delete
conn.response :xml, :content_type => /\bxml$/
conn.response :json, :content_type => /\bjson$/
View
4 lib/faraday_middleware.rb
@@ -4,6 +4,7 @@ module FaradayMiddleware
autoload :OAuth, 'faraday_middleware/request/oauth'
autoload :OAuth2, 'faraday_middleware/request/oauth2'
autoload :EncodeJson, 'faraday_middleware/request/encode_json'
+ autoload :MethodOverride, 'faraday_middleware/request/method_override'
autoload :Mashify, 'faraday_middleware/response/mashify'
autoload :Rashify, 'faraday_middleware/response/rashify'
autoload :ParseJson, 'faraday_middleware/response/parse_json'
@@ -21,7 +22,8 @@ module FaradayMiddleware
Faraday.register_middleware :request,
:oauth => lambda { OAuth },
:oauth2 => lambda { OAuth2 },
- :json => lambda { EncodeJson }
+ :json => lambda { EncodeJson },
+ :method_override => lambda { MethodOverride }
Faraday.register_middleware :response,
:mashify => lambda { Mashify },
View
42 lib/faraday_middleware/request/method_override.rb
@@ -0,0 +1,42 @@
+require 'faraday'
+
+module FaradayMiddleware
+
+ # Rewrites poorly supported HTTP request methods to a custom
+ # X-Http-Method-Override header and sends the request as POST.
+ #
+ # This is supported by default in Rack / Rails apps via the
+ # Rack::MethodOverride module.
+ #
+ # See: http://rack.rubyforge.org/doc/classes/Rack/MethodOverride.html
+ class MethodOverride < Faraday::Middleware
+
+ HEADER = "X-Http-Method-Override"
+
+ def initialize(app, *methods)
+ super(app)
+ @methods = methods.map { |m| normalize(m) }
+ end
+
+ def call(env)
+ method = normalize(env[:method])
+ rewrite_env(env, method) if @methods.include?(method)
+ @app.call(env)
+ end
+
+ private
+
+ # Move the real request method to a header, send the request as HTTP POST.
+ def rewrite_env(env, method)
+ env[:request_headers][HEADER] = method
+ env[:method] = :post
+ end
+
+ # Normalize an HTTP method (String or Symbol) to an upper-case string.
+ def normalize(method)
+ method.to_s.upcase
+ end
+
+ end
+
+end
View
100 spec/method_override_spec.rb
@@ -0,0 +1,100 @@
+require 'helper'
+require 'faraday_middleware/request/method_override'
+
+describe FaradayMiddleware::MethodOverride do
+
+ subject do
+ FaradayMiddleware::MethodOverride.new(
+ lambda { |env| env },
+ *options
+ )
+ end
+
+ # A minimal Faraday request env for the given HTTP method.
+ def env_for_method(method)
+ { :method => method, :request_headers => Faraday::Utils::Headers.new }
+ end
+
+ # A simple test, before getting into more complicated declarative tests.
+ describe "for a simple use case" do
+ let(:options) { [:patch] }
+
+ it "rewrites a PATCH request" do
+ env = subject.call(env_for_method(:patch))
+ env[:method].should == :post
+ env[:request_headers]["X-Http-Method-Override"].should == "PATCH"
+ end
+
+ it "does not rewrite a GET request" do
+ env = subject.call(env_for_method(:get))
+ env[:method].should == :get
+ env[:request_headers].should_not have_key("X-Http-Method-Override")
+ end
+ end
+
+ # Declarative mapping of whether a request method should be rewritten under a
+ # given configuration.
+ {
+ # A normal use case.
+ [:delete, :put, :patch] => {
+ :delete => true,
+ :get => false,
+ :patch => true,
+ :post => false,
+ :put => true,
+ },
+ # Without any methods specified.
+ [] => {
+ :get => false,
+ :post => false,
+ :put => false,
+ },
+ # Of little value, but valid.
+ [:post] => {
+ :post => true,
+ },
+ # Configured with strings instead of symbols.
+ ["patch", "PUT"] => {
+ :get => false,
+ :patch => true,
+ :put => true,
+ },
+ # With methods as strings in faraday env.
+ [:patch, :put] => {
+ "GET" => false,
+ "PATCH" => true,
+ "post" => false,
+ "put" => true,
+ },
+ }.each do |options, expectations|
+
+ context "configured for #{options.inspect}" do
+
+ let(:options) { options }
+
+ expectations.each do |method, should_override|
+ method_up = method.to_s.upcase
+ if should_override
+
+ it "sends #{method_up} as POST with #{method_up} in header" do
+ env = subject.call(env_for_method(method))
+ env[:method].should == :post
+ env[:request_headers]["X-Http-Method-Override"].should == method_up
+ end
+
+ else
+
+ it "sends #{method_up} unmodified with no header" do
+ env = subject.call(env_for_method(method))
+ env[:method].should == method
+ env[:request_headers].should_not have_key("X-Http-Method-Override")
+ end
+
+ end
+ end
+
+ end
+
+ end
+
+end
Something went wrong with that request. Please try again.