Browse files

Fix FakeWeb library hook for when a Net::HTTP request is re-used.

Previously I assumed a single Net::HTTP request object would only be used for a single
request. It turns out it can trivially be re-used. I've refactored the implementation to
no longer assume that. The request object was being used to hold a VCR request type
object to handle internal recursion performed by Net::HTTP; now I have a special
recursive request handler for this situation.

Closes #178.
  • Loading branch information...
1 parent 498994a commit 22915c41dd74b312ca0225e25c195c2291359a57 @myronmarston committed Jul 3, 2012
Showing with 52 additions and 33 deletions.
  1. +37 −21 lib/vcr/library_hooks/fakeweb.rb
  2. +15 −12 spec/vcr/library_hooks/fakeweb_spec.rb
View
58 lib/vcr/library_hooks/fakeweb.rb
@@ -18,12 +18,6 @@ def initialize(net_http, request, request_body = nil, &response_block)
@net_http, @request, @request_body, @response_block =
net_http, request, request_body, response_block
@vcr_response, @recursing = nil, false
-
- if ([:@__vcr_request_type, "@__vcr_request_type"] & request.instance_variables).any?
- @request_type = request.instance_variable_get(:@__vcr_request_type)
- else
- @request_type = nil
- end
end
def handle
@@ -34,7 +28,7 @@ def handle
class << self
def already_seen_requests
- @already_seen_requests ||= Set.new
+ @@already_seen_requests ||= Set.new
end
end
@@ -44,15 +38,6 @@ def externally_stubbed?
::FakeWeb.registered_uri?(request_method, uri)
end
- def request_type(*args)
- @request_type || super
- end
-
- def set_typed_request_for_after_hook(request_type)
- @request.instance_variable_set(:@__vcr_request_type, request_type)
- super
- end
-
def invoke_before_request_hook
unless self.class.already_seen_requests.include?(request.object_id)
super
@@ -95,6 +80,7 @@ def perform_request(started, record_interaction = false)
# that is the final time through #request.
unless started
@recursing = true
+ request.instance_variable_set(:@__vcr_request_handler, recursive_request_handler)
return net_http.request_without_vcr(request, request_body, &response_block)
end
@@ -152,6 +138,34 @@ def vcr_response_from(response)
response.body,
response.http_version
end
+
+ def recursive_request_handler
+ @recursive_request_handler ||= RecursiveRequestHandler.new(
+ @after_hook_typed_request.type, @stubbed_response, @vcr_request,
+ @net_http, @request, @request_body, &@response_block
+ )
+ end
+ end
+
+ class RecursiveRequestHandler < RequestHandler
+ attr_reader :stubbed_response
+
+ def initialize(request_type, stubbed_response, vcr_request, *args, &response_block)
+ @request_type, @stubbed_response, @vcr_request =
+ request_type, stubbed_response, vcr_request
+ super(*args)
+ end
+
+ def handle
+ set_typed_request_for_after_hook(@request_type)
+ send "on_#{@request_type}_request"
+ ensure
+ invoke_after_request_hook(@vcr_response)
+ end
+
+ def request_type(*args)
+ @request_type
+ end
end
end
end
@@ -162,13 +176,15 @@ module Net
# @private
class HTTP
unless method_defined?(:request_with_vcr)
- def request_with_vcr(*args, &block)
+ def request_with_vcr(request, *args, &block)
if VCR.turned_on?
- VCR::LibraryHooks::FakeWeb::RequestHandler.new(
- self, *args, &block
- ).handle
+ handler = request.instance_eval do
+ remove_instance_variable(:@__vcr_request_handler) if defined?(@__vcr_request_handler)
+ end || VCR::LibraryHooks::FakeWeb::RequestHandler.new(self, request, *args, &block)
+
+ handler.handle
else
- request_without_vcr(*args, &block)
+ request_without_vcr(request, *args, &block)
end
end
View
27 spec/vcr/library_hooks/fakeweb_spec.rb
@@ -95,19 +95,22 @@ def make_post_request
ignored_body.should_not eq(recorded_body)
ignored_body.should match(/Response \d+/)
end
-
- it "Make request twice against cassette using the same http request object" do
- uri = URI.parse("http://localhost:#{VCR::SinatraApp.port}/foo")
- http = Net::HTTP.new(uri.host, uri.port)
- VCR.use_cassette("new_cassette", :record => :once) do
- request = Net::HTTP::Get.new(uri.request_uri)
- http.request(request)
- end
- VCR.use_cassette("new_cassette", :record => :once) do
- request = Net::HTTP::Get.new(uri.request_uri)
- http.request(request)
- http.request(request)
+ context 'when the same Net::HTTP request object is used twice' do
+ let(:uri) { URI("http://localhost:#{VCR::SinatraApp.port}/foo") }
+ let(:http) { Net::HTTP.new(uri.host, uri.port) }
+
+ it 'raises an UnhandledHTTPRequestError when using a cassette that only recorded one request' do
+ VCR.use_cassette("new_cassette", :record => :once) do
+ request = Net::HTTP::Get.new(uri.request_uri)
+ http.request(request)
+ end
+
+ VCR.use_cassette("new_cassette", :record => :once) do
+ request = Net::HTTP::Get.new(uri.request_uri)
+ http.request(request)
+ http.request(request)
+ end
end
end
end

0 comments on commit 22915c4

Please sign in to comment.