Skip to content

Commit

Permalink
Avoid redundantly calling em-http-request middleware
Browse files Browse the repository at this point in the history
This appears to fix #899. em-http-request is such a ball of spaghetti
that I have my doubts, but the tests are green.

The patch is based on the observation that the em-http-request call
sequence goes:

1. `HttpConnection#activate_connection`
2. `HttpClient#connection_completed`
3. `HttpClient#send_request`

The `#connection_completed` method is normally responsible for calling
the request middleware before passing the adjusted headers/body to
`#send_request` (cf.
https://github.com/igrigorik/em-http-request/blob/34919d760b940dfd27159bbebce5c4dac9845eb6/lib/em-http/client.rb#L53-L62).
But webmock needs the request signature all the way back in
`#activate_connection`, which we want to compute using the
middleware-adjusted headers/body.

So instead, we let `#activate_connection` handle building the request
signature, whose result will be memoized. Then when we monkey patch
`#connection_completed` to look up the headers/body from that signature
without recomputing the values.

This required some tweaking to `#build_request_signature`, where it was
actually failing to follow the same algorithm as em-http-request.
Basically it just means calling em-http-request's internal
`#build_request` method + making sure not to clobber the `headers` local
variable (which was probably not intentional in the first place).
  • Loading branch information
ajvondrak committed Mar 25, 2021
1 parent 6ff8c0a commit 443a4c7
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 3 deletions.
9 changes: 6 additions & 3 deletions lib/webmock/http_lib_adapters/em_http_request_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ def setup(response, uri, error = nil)
end
end

def connection_completed
@state = :response_header
send_request(request_signature.headers, request_signature.body)
end

def send_request(head, body)
WebMock::RequestRegistry.instance.requested_signatures.put(request_signature)

Expand Down Expand Up @@ -164,7 +169,7 @@ def build_webmock_response
end

def build_request_signature
headers, body = @req.headers, @req.body
headers, body = build_request, @req.body

@conn.middleware.select {|m| m.respond_to?(:request) }.each do |m|
headers, body = m.request(self, headers, body)
Expand All @@ -178,8 +183,6 @@ def build_request_signature

body = form_encode_body(body) if body.is_a?(Hash)

headers = @req.headers

if headers['authorization'] && headers['authorization'].is_a?(Array)
headers['Authorization'] = WebMock::Util::Headers.basic_auth_header(headers.delete('authorization'))
end
Expand Down
56 changes: 56 additions & 0 deletions spec/acceptance/em_http_request/em_http_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,35 @@ def request(client, head, body)
end
end

it "only calls request middleware once" do
stub_request(:get, "www.example.com")

middleware = Class.new do
def self.called!
@called = called + 1
end

def self.called
@called || 0
end

def request(client, head, body)
self.class.called!
[head, body]
end
end

EM.run do
conn = EventMachine::HttpRequest.new('http://www.example.com/')
conn.use middleware
http = conn.get
http.callback do
expect(middleware.called).to eq(1)
EM.stop
end
end
end

let(:response_middleware) do
Class.new do
def response(resp)
Expand Down Expand Up @@ -119,6 +148,33 @@ def response(resp)
context 'making a real request', net_connect: true do
before { WebMock.allow_net_connect! }
include_examples "em-http-request middleware/after_request hook integration"

it "only calls request middleware once" do
middleware = Class.new do
def self.called!
@called = called + 1
end

def self.called
@called || 0
end

def request(client, head, body)
self.class.called!
[head, body]
end
end

EM.run do
conn = EventMachine::HttpRequest.new(webmock_server_url)
conn.use middleware
http = conn.get
http.callback do
expect(middleware.called).to eq(1)
EM.stop
end
end
end
end

context 'when the request is stubbed' do
Expand Down

0 comments on commit 443a4c7

Please sign in to comment.