faraday/test/middleware_stack_test.rb
Josh Cheek fc4cec558e Env stored on middleware response has reference to the response
Synopsis
--------

The `Env` that gets stored on the `Response` does not contain
a reference to that response. This env is a copy of the env that
was passed through the middleware stack. Thus if the env
passed to `app.call` is used, it has the response, but if the one
from the `Response` is used, then it does not. This happens when,
for example, you invoke the `on_complete` hook on the response,
after it has finished.

Fixes Longstanding VCR issue
----------------------------

Fixes https://github.com/vcr/vcr/issues/386

A longstanding issue in VCR's integration with v0.9.x
It was worked around in this commit: https://github.com/vcr/vcr/pull/439
And is merged into master, but is not released.
Merging this will fix the issue, without the changes to VCR needing to
be released.

The old code, which is currently released, and gets its env from the
Request can be seen
[here](cb9696c44d%5E/lib/vcr/middleware/faraday.rb#L99-105)

Fixes Faraday Issue
-------------------

Fixes https://github.com/lostisland/faraday/issues/441
which identifies this same problem.

Closes Faraday PR
-----------------

I'm going to recommend closing https://github.com/lostisland/faraday/pull/360/files
as the issue was probably introduced
[here](04a8514cba (diff-fc9726b31994223d11a212b5169100fdL68))
when `Response#finish` changed from storing the `env` directly,
to storing a copy:

```
- @env = env
+ @env = Env.from(env)
```

Presumably this behaviour is still desired, but the mentioned pull
essentially undoes this by returning the same `env` from `Env.from`

The commit that introduced the issue is probably good, but is called by
the app in the middleware stack that creates the response, before it
sets the rsponse onto the env. This means that the copy does not have
the link to the response.

81f16593a0/lib/faraday/rack_builder.rb (L152-155)

Not sure about the test name/location
-------------------------------------

I had difficulty figuring out where to put this test, and what to name
it. I eventually went with the test on the `middleware_stack_test.rb`,
because it deals with the `RackBuilder`, but if there's a better place,
or a better name, let me know and I'll update it.

One other weird thing
---------------------

Depending on when the `Request#on_complete` callback is invoked,
you're either getting the original env, or the copy:

81f16593a0/lib/faraday/response.rb (L64-65)
2015-05-05 00:52:04 -06:00

183 lines
4.8 KiB
Ruby

require File.expand_path('../helper', __FILE__)
class MiddlewareStackTest < Faraday::TestCase
# mock handler classes
class Handler < Struct.new(:app)
def call(env)
(env[:request_headers]['X-Middleware'] ||= '') << ":#{self.class.name.split('::').last}"
app.call(env)
end
end
class Apple < Handler; end
class Orange < Handler; end
class Banana < Handler; end
class Broken < Faraday::Middleware
dependency 'zomg/i_dont/exist'
end
def setup
@conn = Faraday::Connection.new
@builder = @conn.builder
end
def test_sets_default_adapter_if_none_set
default_middleware = Faraday::Request.lookup_middleware :url_encoded
default_adapter_klass = Faraday::Adapter.lookup_middleware Faraday.default_adapter
assert @builder[0] == default_middleware
assert @builder[1] == default_adapter_klass
end
def test_allows_rebuilding
build_stack Apple
build_stack Orange
assert_handlers %w[Orange]
end
def test_allows_extending
build_stack Apple
@conn.use Orange
assert_handlers %w[Apple Orange]
end
def test_builder_is_passed_to_new_faraday_connection
new_conn = Faraday::Connection.new :builder => @builder
assert_equal @builder, new_conn.builder
end
def test_insert_before
build_stack Apple, Orange
@builder.insert_before Apple, Banana
assert_handlers %w[Banana Apple Orange]
end
def test_insert_after
build_stack Apple, Orange
@builder.insert_after Apple, Banana
assert_handlers %w[Apple Banana Orange]
end
def test_swap_handlers
build_stack Apple, Orange
@builder.swap Apple, Banana
assert_handlers %w[Banana Orange]
end
def test_delete_handler
build_stack Apple, Orange
@builder.delete Apple
assert_handlers %w[Orange]
end
def test_stack_is_locked_after_making_requests
build_stack Apple
assert !@builder.locked?
@conn.get('/')
assert @builder.locked?
assert_raises Faraday::RackBuilder::StackLocked do
@conn.use Orange
end
end
def test_duped_stack_is_unlocked
build_stack Apple
assert !@builder.locked?
@builder.lock!
assert @builder.locked?
duped_connection = @conn.dup
assert_equal @builder, duped_connection.builder
assert !duped_connection.builder.locked?
end
def test_handler_comparison
build_stack Apple
assert_equal @builder.handlers.first, Apple
assert_equal @builder.handlers[0,1], [Apple]
assert_equal @builder.handlers.first, Faraday::RackBuilder::Handler.new(Apple)
end
def test_unregistered_symbol
err = assert_raises(Faraday::Error){ build_stack :apple }
assert_equal ":apple is not registered on Faraday::Middleware", err.message
end
def test_registered_symbol
Faraday::Middleware.register_middleware :apple => Apple
begin
build_stack :apple
assert_handlers %w[Apple]
ensure
unregister_middleware Faraday::Middleware, :apple
end
end
def test_registered_symbol_with_proc
Faraday::Middleware.register_middleware :apple => lambda { Apple }
begin
build_stack :apple
assert_handlers %w[Apple]
ensure
unregister_middleware Faraday::Middleware, :apple
end
end
def test_registered_symbol_with_array
Faraday::Middleware.register_middleware File.expand_path("..", __FILE__),
:strawberry => [lambda { Strawberry }, 'strawberry']
begin
build_stack :strawberry
assert_handlers %w[Strawberry]
ensure
unregister_middleware Faraday::Middleware, :strawberry
end
end
def test_missing_dependencies
build_stack Broken
err = assert_raises RuntimeError do
@conn.get('/')
end
assert_match "missing dependency for MiddlewareStackTest::Broken: ", err.message
assert_match "zomg/i_dont/exist", err.message
end
def test_env_stored_on_middleware_response_has_reference_to_the_response
env = response = nil
build_stack Struct.new(:app) {
define_method(:call) { |e| env, response = e, app.call(e) }
}
@conn.get("/")
assert_same env.response, response.env.response
end
private
# make a stack with test adapter that reflects the order of middleware
def build_stack(*handlers)
@builder.build do |b|
handlers.each { |handler| b.use(*handler) }
yield(b) if block_given?
b.adapter :test do |stub|
stub.get '/' do |env|
# echo the "X-Middleware" request header in the body
[200, {}, env[:request_headers]['X-Middleware'].to_s]
end
end
end
end
def assert_handlers(list)
echoed_list = @conn.get('/').body.to_s.split(':')
echoed_list.shift if echoed_list.first == ''
assert_equal list, echoed_list
end
def unregister_middleware(component, key)
# TODO: unregister API?
component.instance_variable_get('@registered_middleware').delete(key)
end
end