Merge 9c381a1 into 65276d7

lib/http/client.rb

@@ -32,7 +32,7 @@ def request(verb, uri, opts = {})
       return res unless opts.follow
 
       Redirector.new(opts.follow).perform(req, res) do |request|
-        perform(wrap_request(request, opts), opts)
+        perform(request, opts)
       end
     end
 
@@ -43,16 +43,14 @@ def build_request(verb, uri, opts = {})
       headers = make_request_headers(opts)
       body    = make_request_body(opts, headers)
 
-      req = HTTP::Request.new(
+      HTTP::Request.new(
         :verb           => verb,
         :uri            => uri,
         :uri_normalizer => opts.feature(:normalize_uri)&.normalizer,
         :proxy          => opts.proxy,
         :headers        => headers,
         :body           => body
       )
-
-      wrap_request(req, opts)
     end
 
     # @!method persistent?
@@ -62,6 +60,21 @@ def build_request(verb, uri, opts = {})
 
     # Perform a single (no follow) HTTP request
     def perform(req, options)
+      app = ->(r) { perform__(r, options) }
+      options.features.values.reverse.inject(app) do |newapp, feature|
+        ->(r) { feature.call(r, &newapp) }
+      end.call(req)
+    end
+
+    def close
+      @connection&.close
+      @connection = nil
+      @state = :clean
+    end
+
+    private
+
+    def perform__(req, options)
       verify_connection!(req.uri)
 
       @state = :dirty
@@ -73,18 +86,9 @@ def perform(req, options)
           @connection.send_request(req)
           @connection.read_headers!
         end
-      rescue Error => e
-        options.features.each_value do |feature|
-          feature.on_error(req, e)
-        end
-        raise
       end
       res = build_response(req, options)
 
-      res = options.features.inject(res) do |response, (_name, feature)|
-        feature.wrap_response(response)
-      end
-
       @connection.finish_response if req.verb == :head
       @state = :clean
 
@@ -94,20 +98,6 @@ def perform(req, options)
       raise
     end
 
-    def close
-      @connection&.close
-      @connection = nil
-      @state = :clean
-    end
-
-    private
-
-    def wrap_request(req, opts)
-      opts.features.inject(req) do |request, (_name, feature)|
-        feature.wrap_request(request)
-      end
-    end
-
     def build_response(req, options)
       Response.new(
         :status        => @connection.status_code,

lib/http/feature.rb

@@ -6,6 +6,14 @@ def initialize(opts = {})
       @opts = opts
     end
 
+    def call(req, &app)
+      req = wrap_request(req)
+      wrap_response(app.call(req))
+    rescue => e
+      on_error(req, e)
+      raise
+    end
+
     def wrap_request(request)
       request
     end

spec/lib/http/client_spec.rb

@@ -13,7 +13,7 @@
 
   before do
     stubbed_client = Class.new(HTTP::Client) do
-      def perform(request, options)
+      def perform__(request, options)
         stubbed = stubs[HTTP::URI::NORMALIZER.call(request.uri).to_s]
         stubbed ? stubbed.call(request) : super(request, options)
       end
@@ -142,11 +142,19 @@ def simple_response(body, status = 200)
         ** INFO **
         > GET http://example.com/
         ** INFO **
+        < 302 Found
+        ** INFO **
         > GET http://example.com/1
         ** INFO **
+        < 302 Found
+        ** INFO **
         > GET http://example.com/2
         ** INFO **
+        < 302 Found
+        ** INFO **
         > GET http://example.com/3
+        ** INFO **
+        < 200 OK
       OUTPUT
     end
   end
@@ -300,19 +308,13 @@ def simple_response(body, status = 200)
       let(:client)  { described_class.new :headers => headers, :features => {:auto_deflate => {}}, :body => "foo" }
 
       it "deletes Content-Length header" do
-        expect(client).to receive(:perform) do |req, _|
-          expect(req["Content-Length"]).to eq nil
-        end
-
-        client.request(:get, "http://example.com/")
+        res = client.request(:get, "http://example.com/")
+        expect(res.request["Content-Length"]).to eq nil
       end
 
       it "sets Content-Encoding header" do
-        expect(client).to receive(:perform) do |req, _|
-          expect(req["Content-Encoding"]).to eq "gzip"
-        end
-
-        client.request(:get, "http://example.com/")
+        res = client.request(:get, "http://example.com/")
+        expect(res.request["Content-Encoding"]).to eq "gzip"
       end
 
       context "and there is no body" do
@@ -395,6 +397,40 @@ def on_error(request, error)
         end.to raise_error(HTTP::ConnectTimeoutError)
         expect(feature_instance.captured_error).to be_a(HTTP::ConnectTimeoutError)
       end
+
+      it "handle features use rack-like orders" do
+        cls = Class.new(HTTP::Feature) do
+          @orders = []
+
+          class << self
+            attr_reader :orders
+          end
+
+          def initialize(id:)
+            @id = id
+          end
+
+          def wrap_request(req)
+            self.class.orders << "req.#{@id}"
+            req
+          end
+
+          def wrap_response(res)
+            self.class.orders << "res.#{@id}"
+            res
+          end
+
+          HTTP::Options.register_feature(:feature_orders_a, self)
+          HTTP::Options.register_feature(:feature_orders_b, self)
+        end
+
+        sub = HTTP.use(
+          :feature_orders_a => {:id =>  1},
+          :feature_orders_b => {:id =>  2}
+        )
+        sub.get(dummy.endpoint)
+        expect(cls.orders).to eq(["req.1", "req.2", "res.2", "res.1"])
+      end
     end
   end