Skip to content

Commit

Permalink
Refactor AutoInflate/Deflate into generic features
Browse files Browse the repository at this point in the history
This makes HTTP::Client, Request and Response unaware of the
implementation details behind inflating and deflating stream bodies.
Instead, the client iterates over all enabled features on request and
response, calling `#wrap_request` or `#wrap_response`, which is expected
to return the Request or Response as-is if nothing should be done, or
returns a new Request or Response object after having done whatever
transformation the feature requires.
  • Loading branch information
paul committed Jun 8, 2018
1 parent 6766730 commit 539409c
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 49 deletions.
14 changes: 10 additions & 4 deletions lib/http/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,17 @@ def build_request(verb, uri, opts = {}) # rubocop:disable Style/OptionHash
body = make_request_body(opts, headers)
proxy = opts.proxy

HTTP::Request.new(
req = HTTP::Request.new(
:verb => verb,
:uri => uri,
:headers => headers,
:proxy => proxy,
:body => body,
:auto_deflate => opts.feature(:auto_deflate)
:body => body
)

opts.features.inject(req) do |request, (_name, feature)|
feature.wrap_request(request)
end
end

# @!method persistent?
Expand Down Expand Up @@ -78,10 +81,13 @@ def perform(req, options)
:proxy_headers => @connection.proxy_response_headers,
:connection => @connection,
:encoding => options.encoding,
:auto_inflate => options.feature(:auto_inflate),
:uri => req.uri
)

res = options.features.inject(res) do |response, (_name, feature)|
feature.wrap_response(response)
end

@connection.finish_response if req.verb == :head
@state = :clean

Expand Down
8 changes: 8 additions & 0 deletions lib/http/feature.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,13 @@ class Feature
def initialize(opts = {}) # rubocop:disable Style/OptionHash
@opts = opts
end

def wrap_request(request)
request
end

def wrap_response(response)
response
end
end
end
18 changes: 18 additions & 0 deletions lib/http/features/auto_deflate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,19 @@ def initialize(*)
raise Error, "Only gzip and deflate methods are supported" unless %w[gzip deflate].include?(@method)
end

def wrap_request(request)
return request unless method

Request.new(
:version => request.version,
:verb => request.verb,
:uri => request.uri,
:headers => request.headers,
:proxy => request.proxy,
:body => deflated_body(request.body)
)
end

def deflated_body(body)
case method
when "gzip"
Expand All @@ -36,6 +49,11 @@ def size
@compressed.size
end

def read(*a)
compress_all! unless @compressed
@compressed.read(*a)
end

def each(&block)
return to_enum __method__ unless block

Expand Down
20 changes: 14 additions & 6 deletions lib/http/features/auto_inflate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,20 @@
module HTTP
module Features
class AutoInflate < Feature
def stream_for(connection, response)
if %w[deflate gzip x-gzip].include?(response.headers[:content_encoding])
Response::Inflater.new(connection)
else
connection
end
def wrap_response(response)
return response unless %w[deflate gzip x-gzip].include?(response.headers[:content_encoding])
Response.new(
:status => response.status,
:version => response.version,
:headers => response.headers,
:proxy_headers => response.proxy_headers,
:connection => response.connection,
:body => stream_for(response.connection)
)
end

def stream_for(connection)
Response::Body.new(Response::Inflater.new(connection))
end
end
end
Expand Down
3 changes: 0 additions & 3 deletions lib/http/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ def initialize(opts)

@proxy = opts[:proxy] || {}
@body = Request::Body.new(opts[:body])
@deflate = opts[:auto_deflate]
@version = opts[:version] || "1.1"
@headers = HTTP::Headers.coerce(opts[:headers] || {})

Expand All @@ -106,15 +105,13 @@ def redirect(uri, verb = @verb)
:headers => headers,
:proxy => proxy,
:body => body.source,
:auto_deflate => @deflate,
:version => version
)
end

# Stream the request to a socket
def stream(socket)
include_proxy_headers if using_proxy? && !@uri.https?
body = @deflate ? @deflate.deflated_body(self.body) : self.body
Request::Writer.new(socket, body, headers, headline).stream
end

Expand Down
22 changes: 7 additions & 15 deletions lib/http/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ class Response
# @return [Status]
attr_reader :status

# @return [String]
attr_reader :version

# @return [Body]
attr_reader :body

Expand All @@ -46,14 +49,13 @@ def initialize(opts)
@headers = HTTP::Headers.coerce(opts[:headers] || {})
@proxy_headers = HTTP::Headers.coerce(opts[:proxy_headers] || {})

if opts.include?(:connection)
if opts.include?(:body)
@body = opts.fetch(:body)
else
connection = opts.fetch(:connection)
encoding = opts[:encoding] || charset || Encoding::BINARY
stream = body_stream_for(connection, opts)

@body = Response::Body.new(stream, :encoding => encoding)
else
@body = opts.fetch(:body)
@body = Response::Body.new(connection, :encoding => encoding)
end
end

Expand Down Expand Up @@ -160,15 +162,5 @@ def parse(as = nil)
def inspect
"#<#{self.class}/#{@version} #{code} #{reason} #{headers.to_h.inspect}>"
end

private

def body_stream_for(connection, opts)
if opts[:auto_inflate]
opts[:auto_inflate].stream_for(connection, self)
else
connection
end
end
end
end
38 changes: 17 additions & 21 deletions spec/lib/http/features/auto_inflate_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

RSpec.describe HTTP::Features::AutoInflate do
subject { HTTP::Features::AutoInflate.new }
subject(:feature) { HTTP::Features::AutoInflate.new }
let(:connection) { double }
let(:headers) { {} }
let(:response) do
Expand All @@ -13,56 +13,52 @@
)
end

describe "stream_for" do
describe "wrap_response" do
subject(:result) { feature.wrap_response(response) }

context "when there is no Content-Encoding header" do
it "returns connection" do
stream = subject.stream_for(connection, response)
expect(stream).to eq(connection)
it "returns original request" do
expect(result).to be response
end
end

context "for identity Content-Encoding header" do
let(:headers) { {:content_encoding => "not-supported"} }
let(:headers) { {:content_encoding => "identity"} }

it "returns connection" do
stream = subject.stream_for(connection, response)
expect(stream).to eq(connection)
it "returns original request" do
expect(result).to be response
end
end

context "for unknown Content-Encoding header" do
let(:headers) { {:content_encoding => "not-supported"} }

it "returns connection" do
stream = subject.stream_for(connection, response)
expect(stream).to eq(connection)
it "returns original request" do
expect(result).to be response
end
end

context "for deflate Content-Encoding header" do
let(:headers) { {:content_encoding => "deflate"} }

it "returns HTTP::Response::Inflater instance - connection wrapper" do
stream = subject.stream_for(connection, response)
expect(stream).to be_instance_of HTTP::Response::Inflater
it "returns a HTTP::Response wrapping the inflated response body" do
expect(result.body).to be_instance_of HTTP::Response::Body
end
end

context "for gzip Content-Encoding header" do
let(:headers) { {:content_encoding => "gzip"} }

it "returns HTTP::Response::Inflater instance - connection wrapper" do
stream = subject.stream_for(connection, response)
expect(stream).to be_instance_of HTTP::Response::Inflater
it "returns a HTTP::Response wrapping the inflated response body" do
expect(result.body).to be_instance_of HTTP::Response::Body
end
end

context "for x-gzip Content-Encoding header" do
let(:headers) { {:content_encoding => "x-gzip"} }

it "returns HTTP::Response::Inflater instance - connection wrapper" do
stream = subject.stream_for(connection, response)
expect(stream).to be_instance_of HTTP::Response::Inflater
it "returns a HTTP::Response wrapping the inflated response body" do
expect(result.body).to be_instance_of HTTP::Response::Body
end
end
end
Expand Down

0 comments on commit 539409c

Please sign in to comment.