Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request/Response lifecycle refactoring #845

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,16 +233,31 @@ conn.put '/profile', payload
## Writing middleware

Middleware are classes that implement a `call` instance method. They hook into
the request/response cycle.
the request/response cycle with two callback methods: `on_request` and `on_response`.

```ruby
def on_request(env)
# do something with the request
# env.request_headers.merge!(...)
end

def on_complete(env)
# do something with the response
# raise RuntimeError if env.response.status != 200
end
```

Alternatively, if your middleware requires a special implementation, you can take control of the cycle
by overriding the `call` method, and calling `@app.call` where necessary to hand the request over to the next middleware.

```ruby
def call(request_env)
# do something with the request
# request_env[:request_headers].merge!(...)
# request_env.request_headers.merge!(...)

@app.call(request_env).on_complete do |response_env|
# do something with the response
# response_env[:response_headers].merge!(...)
# response_env.response_headers.merge!(...)
end
end
```
Expand Down
17 changes: 17 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,23 @@ Please note `Faraday::ClientError` was previously used for both.
* Faraday::ForbiddenError (403)
* Faraday::ProxyAuthError (407). Please note this raised a `Faraday::ConnectionFailed` before.
* Faraday::UnprocessableEntityError (422)

### Adapters
Adapters have been refactored so that they're not middlewares anymore.
This means that they do not take `@app` as an initializer parameter anymore and they don't call `@app.call` anymore.
If you're using a custom adapter, please ensure to change its initializer and `call` method.

### Faraday::Env
The `Faraday::Env` has been refactored by moving all response-related fields into the `response` property.
This means that if you need to access the response `body`, `headers`, `reason_phrase` or `status`, you'll need to pass through the `response`. (e.g. `env.response.headers`)
However, the following helper methods have been introduced in `Faraday::Env` to ensure backwards compatibility when READING these fields: `response_body`, `response_headers`, `reason_phrase` and `status`.
Moreover, since many existing middlewares still rely on the fact that the `body` is overridden after the response, the `body` getter maintains that functionality.
But now you can access the request body even after a request has been completed using the `request_body` getter.

### Middlewares
Middlewares have been refactored, there's no `Faraday::Response::Middleware` class anymore and all "response" middlewares now inherit from the same `Faraday::Middleware` class as the "request" ones.
There's also a new `on_request` callback that works very similarly to `on_complete` and can be used in "request" middlewares to avoid overriding `call`.


### Others
* Dropped support for jruby and Rubinius.
Expand Down
122 changes: 2 additions & 120 deletions lib/faraday.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
require 'cgi'
require 'set'
require 'forwardable'
require 'faraday/middleware_registry'
require 'faraday/dependency_loader'

# This is the main namespace for Faraday.
#
Expand Down Expand Up @@ -147,126 +149,6 @@ def self.default_connection_options=(options)
Timer = Timeout
end

# Adds the ability for other modules to register and lookup
# middleware classes.
module MiddlewareRegistry
# Register middleware class(es) on the current module.
#
# @param autoload_path [String] Middleware autoload path
# @param mapping [Hash{Symbol => Module, Symbol, Array<Module, Symbol, String>}] Middleware mapping from a lookup symbol to a reference to the middleware. - Classes can be expressed as:
# - a fully qualified constant
# - a Symbol
# - a Proc that will be lazily called to return the former
# - an array is given, its first element is the constant or symbol,
# and its second is a file to `require`.
# @return [void]
#
# @example Lookup by a constant
#
# module Faraday
# class Whatever
# # Middleware looked up by :foo returns Faraday::Whatever::Foo.
# register_middleware :foo => Foo
# end
# end
#
# @example Lookup by a symbol
#
# module Faraday
# class Whatever
# # Middleware looked up by :bar returns Faraday::Whatever.const_get(:Bar)
# register_middleware :bar => :Bar
# end
# end
#
# @example Lookup by a symbol and string in an array
#
# module Faraday
# class Whatever
# # Middleware looked up by :baz requires 'baz' and returns Faraday::Whatever.const_get(:Baz)
# register_middleware :baz => [:Baz, 'baz']
# end
# end
#
def register_middleware(autoload_path = nil, mapping = nil)
if mapping.nil?
mapping = autoload_path
autoload_path = nil
end
middleware_mutex do
@middleware_autoload_path = autoload_path if autoload_path
(@registered_middleware ||= {}).update(mapping)
end
end

# Unregister a previously registered middleware class.
#
# @param key [Symbol] key for the registered middleware.
def unregister_middleware(key)
@registered_middleware.delete(key)
end

# Lookup middleware class with a registered Symbol shortcut.
#
# @param key [Symbol] key for the registered middleware.
# @return [Class] a middleware Class.
# @raise [Faraday::Error] if given key is not registered
#
# @example
#
# module Faraday
# class Whatever
# register_middleware :foo => Foo
# end
# end
#
# Faraday::Whatever.lookup_middleware(:foo)
# # => Faraday::Whatever::Foo
#
def lookup_middleware(key)
load_middleware(key) ||
raise(Faraday::Error.new("#{key.inspect} is not registered on #{self}"))
end

def middleware_mutex(&block)
@middleware_mutex ||= begin
require 'monitor'
Monitor.new
end
@middleware_mutex.synchronize(&block)
end

def fetch_middleware(key)
defined?(@registered_middleware) && @registered_middleware[key]
end

def load_middleware(key)
value = fetch_middleware(key)
case value
when Module
value
when Symbol, String
middleware_mutex do
@registered_middleware[key] = const_get(value)
end
when Proc
middleware_mutex do
@registered_middleware[key] = value.call
end
when Array
middleware_mutex do
const, path = value
if root = @middleware_autoload_path
path = "#{root}/#{path}"
end
require(path)
@registered_middleware[key] = const
end
load_middleware(key)
end
end
end

require_libs "utils", "options", "connection", "rack_builder", "parameters",
"middleware", "adapter", "request", "response", "upload_io", "error"

Expand Down
47 changes: 25 additions & 22 deletions lib/faraday/adapter.rb
Original file line number Diff line number Diff line change
@@ -1,55 +1,58 @@
module Faraday
# Base class for all Faraday adapters. Adapters are
# responsible for fulfilling a Faraday request.
class Adapter < Middleware
class Adapter
extend MiddlewareRegistry
extend DependencyLoader

CONTENT_LENGTH = 'Content-Length'.freeze

register_middleware File.expand_path('../adapter', __FILE__),
:test => [:Test, 'test'],
:net_http => [:NetHttp, 'net_http'],
:net_http_persistent => [:NetHttpPersistent, 'net_http_persistent'],
:typhoeus => [:Typhoeus, 'typhoeus'],
:patron => [:Patron, 'patron'],
:em_synchrony => [:EMSynchrony, 'em_synchrony'],
:em_http => [:EMHttp, 'em_http'],
:excon => [:Excon, 'excon'],
:rack => [:Rack, 'rack'],
:httpclient => [:HTTPClient, 'httpclient']
:test => [:Test, 'test'],
:net_http => [:NetHttp, 'net_http'],
:net_http_persistent => [:NetHttpPersistent, 'net_http_persistent'],
:typhoeus => [:Typhoeus, 'typhoeus'],
:patron => [:Patron, 'patron'],
:em_synchrony => [:EMSynchrony, 'em_synchrony'],
:em_http => [:EMHttp, 'em_http'],
:excon => [:Excon, 'excon'],
:rack => [:Rack, 'rack'],
:httpclient => [:HTTPClient, 'httpclient']

# This module marks an Adapter as supporting parallel requests.
module Parallelism
attr_writer :supports_parallel
def supports_parallel?() @supports_parallel end

def supports_parallel?()
@supports_parallel
end

def inherited(subclass)
super
subclass.supports_parallel = self.supports_parallel?
end
end

extend Parallelism
self.supports_parallel = false

def initialize(app = nil, opts = {}, &block)
super(app)
def initialize(opts = {}, &block)
@app = lambda { |env| env.response }
@connection_options = opts
@config_block = block
end

def call(env)
env.clear_body if env.needs_body?
env.response ||= Response.new
end

private

def save_response(env, status, body, headers = nil, reason_phrase = nil)
env.status = status
env.body = body
env.reason_phrase = reason_phrase && reason_phrase.to_s.strip
env.response_headers = Utils::Headers.new.tap do |response_headers|
response_headers.update headers unless headers.nil?
yield(response_headers) if block_given?
end
params = { status: status, body: body, headers: headers, reason_phrase: reason_phrase&.to_s&.strip }
env.parallel? ? env.response.apply_params(params) : env.response.finish(params)
yield(env.response.headers) if block_given?
env.response
end
end
end
32 changes: 16 additions & 16 deletions lib/faraday/adapter/em_http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def connection_config(env)
def request_config(env)
options = {
:body => read_body(env),
:head => env[:request_headers],
:head => env.request_headers,
Copy link
Contributor

@mislav mislav Feb 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If env[:request_headers] still works, unnecessary changes like these make it more difficult to see the actual changes in this PR. It's best to keep the old env[:foo] approach for now in code (but document the method-based approach in usage examples) and update internal usage in its own PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, will isolate in separate PR

# :keepalive => true,
# :file => 'path/to/file', # stream data off disk
}
Expand All @@ -27,7 +27,7 @@ def request_config(env)
end

def read_body(env)
body = env[:body]
body = env.body
body.respond_to?(:read) ? body.read : body
end

Expand All @@ -54,10 +54,10 @@ def configure_socket(options, env)

# Reads out SSL certificate settings from env into options
def configure_ssl(options, env)
if env[:url].scheme == 'https' && env[:ssl]
if env.url.scheme == 'https' && env.ssl
options[:ssl] = {
:cert_chain_file => env[:ssl][:ca_file],
:verify_peer => env[:ssl].fetch(:verify, true)
:cert_chain_file => env.ssl[:ca_file],
:verify_peer => env.ssl.fetch(:verify, true)
}
end
end
Expand All @@ -71,13 +71,13 @@ def configure_timeout(options, env)

# Reads out compression header settings from env into options
def configure_compression(options, env)
if env[:method] == :get and not options[:head].key? 'accept-encoding'
if env.method == :get and not options[:head].key? 'accept-encoding'
options[:head]['accept-encoding'] = 'gzip, compressed'
end
end

def request_options(env)
env[:request]
env.request
end
end

Expand All @@ -100,10 +100,10 @@ def call(env)

def perform_request(env)
if parallel?(env)
manager = env[:parallel_manager]
manager = env.parallel_manager
manager.add {
perform_single_request(env).
callback { env[:response].finish(env) }
callback { env.response.finish(env) }
}
else
unless EventMachine.reactor_running?
Expand All @@ -120,9 +120,9 @@ def perform_request(env)
raise_error(error) if error
else
# EM is running: instruct upstream that this is an async request
env[:parallel_manager] = true
env.parallel_manager = true
perform_single_request(env).
callback { env[:response].finish(env) }.
callback { env.response.finish(env) }.
errback {
# TODO: no way to communicate the error in async mode
raise NotImplementedError
Expand All @@ -146,10 +146,10 @@ def perform_request(env)
# TODO: reuse the connection to support pipelining
def perform_single_request(env)
req = create_request(env)
req.setup_request(env[:method], request_config(env)).callback { |client|
if env[:request].stream_response?
req.setup_request(env.method, request_config(env)).callback { |client|
if env.request.stream_response?
warn "Streaming downloads for #{self.class.name} are not yet implemented."
env[:request].on_data.call(client.response, client.response.bytesize)
env.request.on_data.call(client.response, client.response.bytesize)
end
status = client.response_header.status
reason = client.response_header.http_reason
Expand All @@ -162,7 +162,7 @@ def perform_single_request(env)
end

def create_request(env)
EventMachine::HttpRequest.new(env[:url], connection_config(env).merge(@connection_options))
EventMachine::HttpRequest.new(env.url, connection_config(env).merge(@connection_options))
end

def error_message(client)
Expand All @@ -185,7 +185,7 @@ def raise_error(msg)

# @return [Boolean]
def parallel?(env)
!!env[:parallel_manager]
!!env.parallel_manager
end

# This parallel manager is designed to start an EventMachine loop
Expand Down
Loading