Cache adapter connections (attempt 1)

lib/faraday/adapter.rb

@@ -37,6 +37,24 @@ def inherited(subclass)
       end
     end
 
+    # This module marks an Adapter as supporting a cached HTTP connection.
+    module CacheConnection
+      def connection(env)
+        recfg = reconfigure_connection?(env)
+        conn = !recfg && @connection
+        conn ||= build_connection(env)
+        @connection = conn unless recfg
+
+        return conn unless block_given?
+
+        yield conn
+      end
+
+      def cache_connection?
+        true
+      end
+    end
+
     extend Parallelism
     self.supports_parallel = false
 
@@ -60,6 +78,10 @@ def connection(env)
       yield conn
     end
 
+    def cache_connection?
+      false
+    end
+
     def call(env)
       env.clear_body if env.needs_body?
       env.response = Response.new
@@ -104,5 +126,14 @@ def request_timeout(type, options)
       open: :open_timeout,
       write: :write_timeout
     }.freeze
+
+    def reconfigure_connection?(env)
+      return unless (req = env && env[:request])
+
+      CONNECTION_OPTIONS.any? { |k| req.key?(k) }
+    end
+
+    CONNECTION_OPTIONS = %i[proxy bind timeout open_timeout read_timeout
+                            write_timeout].freeze
   end
 end

lib/faraday/adapter/excon.rb

@@ -6,6 +6,8 @@ class Adapter
     class Excon < Faraday::Adapter
       dependency 'excon'
 
+      include CacheConnection
+
       def build_connection(env)
         opts = opts_from_env(env)
         ::Excon.new(env[:url].to_s, opts.merge(@connection_options))
@@ -52,20 +54,11 @@ def read_body(env)
 
       def opts_from_env(env)
         opts = {}
-        amend_opts_with_ssl!(opts, env[:ssl]) if needs_ssl_settings?(env)
-
-        if (req = env[:request])
-          amend_opts_with_timeouts!(opts, req)
-          amend_opts_with_proxy_settings!(opts, req)
-        end
-
+        amend_opts_with_ssl!(opts, env[:ssl])
+        amend_opts_for_request!(opts, env[:request])
         opts
       end
 
-      def needs_ssl_settings?(env)
-        env[:url].scheme == 'https' && env[:ssl]
-      end
-
       OPTS_KEYS = [
         %i[client_cert client_cert],
         %i[client_key client_key],
@@ -79,6 +72,8 @@ def needs_ssl_settings?(env)
       ].freeze
 
       def amend_opts_with_ssl!(opts, ssl)
+        return unless ssl
+
         opts[:ssl_verify_peer] = !!ssl.fetch(:verify, true)
         # https://github.com/geemus/excon/issues/106
         # https://github.com/jruby/jruby-ossl/issues/19
@@ -91,6 +86,13 @@ def amend_opts_with_ssl!(opts, ssl)
         end
       end
 
+      def amend_opts_for_request!(opts, req)
+        return unless req
+
+        amend_opts_with_timeouts!(opts, req)
+        amend_opts_with_proxy_settings!(opts, req)
+      end
+
       def amend_opts_with_timeouts!(opts, req)
         if (sec = request_timeout(:read, req))
           opts[:read_timeout] = sec

lib/faraday/adapter/httpclient.rb

@@ -6,31 +6,15 @@ class Adapter
     class HTTPClient < Faraday::Adapter
       dependency 'httpclient'
 
-      def build_connection(env)
-        @client ||= ::HTTPClient.new.tap do |cli|
-          # enable compression
-          cli.transparent_gzip_decompression = true
-        end
-
-        if (req = env[:request])
-          if (proxy = req[:proxy])
-            configure_proxy @client, proxy
-          end
-
-          if (bind = req[:bind])
-            configure_socket @client, bind
-          end
-
-          configure_timeouts @client, req
-        end
-
-        if env[:url].scheme == 'https' && (ssl = env[:ssl])
-          configure_ssl @client, ssl
-        end
+      include CacheConnection
 
-        configure_client @client
-
-        @client
+      def build_connection(env)
+        conn = ::HTTPClient.new
+        conn.transparent_gzip_decompression = true
+        configure_client(conn)
+        configure_for_request(conn, env[:request])
+        configure_ssl(conn, env[:ssl])
+        conn
       end
 
       def call(env)
@@ -91,6 +75,8 @@ def configure_proxy(client, proxy)
 
       # @param ssl [Hash]
       def configure_ssl(client, ssl)
+        return unless ssl
+
         ssl_config = client.ssl_config
         ssl_config.verify_mode = ssl_verify_mode(ssl)
         ssl_config.cert_store = ssl_cert_store(ssl)
@@ -117,6 +103,20 @@ def configure_timeouts(client, req)
         client.receive_timeout = sec
       end
 
+      def configure_for_request(client, options)
+        return unless options
+
+        if (proxy = options[:proxy])
+          configure_proxy(client, proxy)
+        end
+
+        if (bind = options[:bind])
+          configure_socket(client, bind)
+        end
+
+        configure_timeouts(client, options)
+      end
+
       def configure_client(client)
         @config_block&.call(client)
       end

lib/faraday/adapter/net_http.rb

@@ -45,6 +45,12 @@ def build_connection(env)
           if http.respond_to?(:use_ssl=)
             http.use_ssl = env[:url].scheme == 'https'
           end
+
+          # Only set if Net::Http supports it, since Ruby 2.5.
+          http.max_retries = 0 if http.respond_to?(:max_retries=)
+
+          @config_block&.call(http)
+
           configure_ssl(http, env[:ssl])
           configure_request(http, env[:request])
         end
@@ -167,6 +173,8 @@ def configure_ssl(http, ssl)
       end
 
       def configure_request(http, req)
+        return unless req
+
         if (sec = request_timeout(:read, req))
           http.read_timeout = sec
         end
@@ -176,14 +184,9 @@ def configure_request(http, req)
           http.write_timeout = sec
         end
 
-        if (sec = request_timeout(:open, req))
-          http.open_timeout = sec
-        end
-
-        # Only set if Net::Http supports it, since Ruby 2.5.
-        http.max_retries = 0 if http.respond_to?(:max_retries=)
+        return unless (sec = request_timeout(:open, req))
 
-        @config_block&.call(http)
+        http.open_timeout = sec
       end
 
       def ssl_cert_store(ssl)

lib/faraday/adapter/net_http_persistent.rb

@@ -6,10 +6,12 @@ class Adapter
     class NetHttpPersistent < NetHttp
       dependency 'net/http/persistent'
 
+      include CacheConnection
+
       private
 
       def net_http_connection(env)
-        @cached_connection ||=
+        conn =
           if Net::HTTP::Persistent.instance_method(:initialize)
                                   .parameters.first == %i[key name]
             options = { name: 'Faraday' }
@@ -22,10 +24,8 @@ def net_http_connection(env)
           end
 
         proxy_uri = proxy_uri(env)
-        if @cached_connection.proxy_uri != proxy_uri
-          @cached_connection.proxy = proxy_uri
-        end
-        @cached_connection
+        conn.proxy = proxy_uri if conn.proxy_uri != proxy_uri
+        conn
       end
 
       def proxy_uri(env)

lib/faraday/adapter/patron.rb

@@ -6,18 +6,14 @@ class Adapter
     class Patron < Faraday::Adapter
       dependency 'patron'
 
+      include CacheConnection
+
       def build_connection(env)
         session = ::Patron::Session.new
         @config_block&.call(session)
-        if (env[:url].scheme == 'https') && env[:ssl]
-          configure_ssl(session, env[:ssl])
-        end
-
-        if (req = env[:request])
-          configure_timeouts(session, req)
-          configure_proxy(session, req[:proxy])
-        end
-
+        configure_ssl(session, env[:ssl]) if env[:ssl]
+        configure_for_request(session, env[:connection])
+        configure_for_request(session, env[:request])
         session
       end
 
@@ -86,9 +82,14 @@ def configure_ssl(session, ssl)
         end
       end
 
-      def configure_timeouts(session, req)
+      def configure_for_request(session, req)
         return unless req
 
+        configure_timeouts(session, req)
+        configure_proxy(session, req[:proxy])
+      end
+
+      def configure_timeouts(session, req)
         if (sec = request_timeout(:read, req))
           session.timeout = sec
         end

lib/faraday/options/connection_options.rb

@@ -5,7 +5,8 @@ module Faraday
   # connection object.
   class ConnectionOptions < Options.new(:request, :proxy, :ssl, :builder, :url,
                                         :parallel_manager, :params, :headers,
-                                        :builder_class)
+                                        :builder_class, :bind, :read_timeout,
+                                        :timeout, :open_timeout, :write_timeout)
 
     options request: RequestOptions, ssl: SSLOptions
 

lib/faraday/options/env.rb

@@ -46,10 +46,14 @@ module Faraday
   #
   # @!attribute reason_phrase
   #   @return [String]
+  #
+  # @!attribute connection
+  #   @return [Faraday::ConnectionOptions] The connection options for the
+  #   current adapter.
   class Env < Options.new(:method, :request_body, :url, :request,
                           :request_headers, :ssl, :parallel_manager, :params,
                           :response, :response_headers, :status,
-                          :reason_phrase, :response_body)
+                          :reason_phrase, :response_body, :connection)
 
     # rubocop:disable Naming/ConstantName
     ContentLength = 'Content-Length'
@@ -61,7 +65,7 @@ class Env < Options.new(:method, :request_body, :url, :request,
     # these requests, the Content-Length header is set to 0.
     MethodsWithBodies = Set.new(Faraday::METHODS_WITH_BODY.map(&:to_sym))
 
-    options request: RequestOptions,
+    options request: RequestOptions, connection: ConnectionOptions,
             request_headers: Utils::Headers, response_headers: Utils::Headers
 
     extend Forwardable

lib/faraday/rack_builder.rb

@@ -202,14 +202,7 @@ def dup
     #     :password   - Proxy server password
     # :ssl - Hash of options for configuring SSL requests.
     def build_env(connection, request)
-      exclusive_url = connection.build_exclusive_url(
-        request.path, request.params,
-        request.options.params_encoder
-      )
-
-      Env.new(request.method, request.body, exclusive_url,
-              request.options, request.headers, connection.ssl,
-              connection.parallel_manager)
+      request.to_env(connection)
     end
 
     private

lib/faraday/request.rb

@@ -139,8 +139,12 @@ def marshal_load(serialised)
 
     # @return [Env] the Env for this Request
     def to_env(connection)
-      Env.new(method, body, connection.build_exclusive_url(path, params),
-              options, headers, connection.ssl, connection.parallel_manager)
+      exc_url = connection.build_exclusive_url(path, params,
+                                               options.params_encoder)
+      env = Env.new(method, body, exc_url, options, headers, connection.ssl,
+                    connection.parallel_manager)
+      env.connection = connection
+      env
     end
   end
 end

spec/faraday/adapter/excon_spec.rb

@@ -16,10 +16,42 @@
   end
 
   context 'config' do
-    let(:adapter) { Faraday::Adapter::Excon.new }
+    let(:adapter) { described_class.new }
     let(:request) { Faraday::RequestOptions.new }
     let(:uri) { URI.parse('https://example.com') }
-    let(:env) { { request: request, url: uri } }
+    let(:env) do
+      Faraday::Env.from(
+        request: request,
+        ssl: Faraday::SSLOptions.new,
+        url: uri
+      )
+    end
+
+    it 'caches connection' do
+      # before client is created
+      env.ssl.client_cert = 'client-cert'
+      request.boundary = 'doesnt-matter'
+
+      client = adapter.connection(env)
+      expect(client.data[:ssl_verify_peer]).to eq(true)
+      expect(client.data[:client_cert]).to eq('client-cert')
+      expect(client.data[:connect_timeout]).to eq(60)
+
+      # client2 is cached because no important request options are set
+      client2 = adapter.connection(env)
+      expect(client2.data[:ssl_verify_peer]).to eq(true)
+      expect(client2.data[:client_cert]).to eq('client-cert')
+      expect(client2.data[:connect_timeout]).to eq(60)
+      expect(client2.object_id).to eq(client.object_id)
+
+      # important request setting, so client3 is new
+      env.request.timeout = 5
+      client3 = adapter.connection(env)
+      expect(client3.data[:ssl_verify_peer]).to eq(true)
+      expect(client3.data[:client_cert]).to eq('client-cert')
+      expect(client3.data[:connect_timeout]).to eq(5)
+      expect(client3.object_id).not_to eq(client.object_id)
+    end
 
     it 'sets timeout' do
       request.timeout = 5