From 36a7a40643ef9afff31372490489af6b8c3107d7 Mon Sep 17 00:00:00 2001 From: Joshua Flanagan Date: Wed, 24 Apr 2019 14:42:56 -0500 Subject: [PATCH 1/2] Access the HTTP::Request from the HTTP::Response This fixes https://github.com/httprb/http/issues/463 This will greatly enhance the abilities of pluggable `Features` by being able to reference the Request that initiated a Response. --- lib/http/client.rb | 3 ++- lib/http/features/auto_inflate.rb | 3 ++- lib/http/response.rb | 4 ++++ spec/lib/http/client_spec.rb | 14 ++++++++++++-- spec/lib/http/features/auto_inflate_spec.rb | 6 ++++-- spec/lib/http/features/instrumentation_spec.rb | 3 ++- spec/lib/http/features/logging_spec.rb | 3 ++- spec/lib/http/redirector_spec.rb | 3 ++- spec/lib/http/response_spec.rb | 6 ++++-- 9 files changed, 34 insertions(+), 11 deletions(-) diff --git a/lib/http/client.rb b/lib/http/client.rb index 5a5d75c3..91ac8409 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -82,7 +82,8 @@ def perform(req, options) :proxy_headers => @connection.proxy_response_headers, :connection => @connection, :encoding => options.encoding, - :uri => req.uri + :uri => req.uri, + :request => req ) res = options.features.inject(res) do |response, (_name, feature)| diff --git a/lib/http/features/auto_inflate.rb b/lib/http/features/auto_inflate.rb index e94a1324..6b659560 100644 --- a/lib/http/features/auto_inflate.rb +++ b/lib/http/features/auto_inflate.rb @@ -17,7 +17,8 @@ def wrap_response(response) :headers => response.headers, :proxy_headers => response.proxy_headers, :connection => response.connection, - :body => stream_for(response.connection) + :body => stream_for(response.connection), + :request => response.request } options[:uri] = response.uri if response.uri diff --git a/lib/http/response.rb b/lib/http/response.rb index 600e6d4e..8b86df05 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -29,6 +29,9 @@ class Response # @return [URI, nil] attr_reader :uri + # @return [Request] + attr_reader :request + # @return [Hash] attr_reader :proxy_headers @@ -48,6 +51,7 @@ def initialize(opts) @status = HTTP::Response::Status.new(opts.fetch(:status)) @headers = HTTP::Headers.coerce(opts[:headers] || {}) @proxy_headers = HTTP::Headers.coerce(opts[:proxy_headers] || {}) + @request = opts.fetch(:request) if opts.include?(:body) @body = opts.fetch(:body) diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 4ba473b3..a3a144cc 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -31,7 +31,8 @@ def redirect_response(location, status = 302) :status => status, :version => "1.1", :headers => {"Location" => location}, - :body => "" + :body => "", + :request => HTTP::Request.new(:verb => :get, :uri => "http://example.com") ) end @@ -39,7 +40,8 @@ def simple_response(body, status = 200) HTTP::Response.new( :status => status, :version => "1.1", - :body => body + :body => body, + :request => HTTP::Request.new(:verb => :get, :uri => "http://example.com") ) end @@ -316,6 +318,14 @@ def simple_response(body, status = 200) client.get(dummy.endpoint).to_s end + it "provides access to the Request from the Response" do + unique_value = "20190424" + response = client.headers("X-Value" => unique_value).get(dummy.endpoint) + + expect(response.request).to be_a(HTTP::Request) + expect(response.request.headers["X-Value"]).to eq(unique_value) + end + context "with HEAD request" do it "does not iterates through body" do expect_any_instance_of(HTTP::Connection).to_not receive(:readpartial) diff --git a/spec/lib/http/features/auto_inflate_spec.rb b/spec/lib/http/features/auto_inflate_spec.rb index c93e41d3..3350055f 100644 --- a/spec/lib/http/features/auto_inflate_spec.rb +++ b/spec/lib/http/features/auto_inflate_spec.rb @@ -11,7 +11,8 @@ :version => "1.1", :status => 200, :headers => headers, - :connection => connection + :connection => connection, + :request => HTTP::Request.new(:verb => :get, :uri => "http://example.com") ) end @@ -73,7 +74,8 @@ :status => 200, :headers => {:content_encoding => "gzip"}, :connection => connection, - :uri => "https://example.com" + :uri => "https://example.com", + :request => HTTP::Request.new(:verb => :get, :uri => "https://example.com") ) end diff --git a/spec/lib/http/features/instrumentation_spec.rb b/spec/lib/http/features/instrumentation_spec.rb index c382093b..e44db43c 100644 --- a/spec/lib/http/features/instrumentation_spec.rb +++ b/spec/lib/http/features/instrumentation_spec.rb @@ -28,7 +28,8 @@ :uri => "https://example.com", :status => 200, :headers => {:content_type => "application/json"}, - :body => '{"success": true}' + :body => '{"success": true}', + :request => HTTP::Request.new(:verb => :get, :uri => "https://example.com") ) end diff --git a/spec/lib/http/features/logging_spec.rb b/spec/lib/http/features/logging_spec.rb index 30b2fc53..b0f3cd7a 100644 --- a/spec/lib/http/features/logging_spec.rb +++ b/spec/lib/http/features/logging_spec.rb @@ -47,7 +47,8 @@ :uri => "https://example.com", :status => 200, :headers => {:content_type => "application/json"}, - :body => '{"success": true}' + :body => '{"success": true}', + :request => HTTP::Request.new(:verb => :get, :uri => "https://example.com") ) end diff --git a/spec/lib/http/redirector_spec.rb b/spec/lib/http/redirector_spec.rb index 99c9e9cb..e30b1ff9 100644 --- a/spec/lib/http/redirector_spec.rb +++ b/spec/lib/http/redirector_spec.rb @@ -6,7 +6,8 @@ def simple_response(status, body = "", headers = {}) :status => status, :version => "1.1", :headers => headers, - :body => body + :body => body, + :request => HTTP::Request.new(:verb => :get, :uri => "http://example.com") ) end diff --git a/spec/lib/http/response_spec.rb b/spec/lib/http/response_spec.rb index 8297c0cd..398f7413 100644 --- a/spec/lib/http/response_spec.rb +++ b/spec/lib/http/response_spec.rb @@ -11,7 +11,8 @@ :version => "1.1", :headers => headers, :body => body, - :uri => uri + :uri => uri, + :request => HTTP::Request.new(:verb => :get, :uri => "http://example.com") ) end @@ -152,7 +153,8 @@ HTTP::Response.new( :version => "1.1", :status => 200, - :connection => connection + :connection => connection, + :request => HTTP::Request.new(:verb => :get, :uri => "http://example.com") ) end From 57ecccc0125e1bb4b843958117f8ffd95ffc4da1 Mon Sep 17 00:00:00 2001 From: Joshua Flanagan Date: Wed, 24 Apr 2019 18:01:20 -0500 Subject: [PATCH 2/2] Response#uri can be set directly, or pulled from the Request During normal operation, the Response#uri will be pulled from the Request passed into the Response. However, we still allow explicitly setting a `:uri` option when creating a Response, for backwards compatibility. This was motivated by rubocop/code climate complaining that `Client#perform` was now 1-line too long. I'll defer to the project maintainers to decide if that is a good thing. --- lib/http/client.rb | 1 - lib/http/response.rb | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/http/client.rb b/lib/http/client.rb index 91ac8409..daca1c23 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -82,7 +82,6 @@ def perform(req, options) :proxy_headers => @connection.proxy_response_headers, :connection => @connection, :encoding => options.encoding, - :uri => req.uri, :request => req ) diff --git a/lib/http/response.rb b/lib/http/response.rb index 8b86df05..6527b2bc 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -47,11 +47,11 @@ class Response # @option opts [String] :uri def initialize(opts) @version = opts.fetch(:version) - @uri = HTTP::URI.parse(opts.fetch(:uri)) if opts.include? :uri + @request = opts.fetch(:request) + @uri = HTTP::URI.parse(opts[:uri] || @request.uri) @status = HTTP::Response::Status.new(opts.fetch(:status)) @headers = HTTP::Headers.coerce(opts[:headers] || {}) @proxy_headers = HTTP::Headers.coerce(opts[:proxy_headers] || {}) - @request = opts.fetch(:request) if opts.include?(:body) @body = opts.fetch(:body)