From b0d2d775c4ee2429b1b26e724ab717a113b83302 Mon Sep 17 00:00:00 2001 From: Paul Sadauskas Date: Thu, 16 Aug 2018 15:38:47 -0600 Subject: [PATCH 1/5] Add a mechanism to register new features --- lib/http/features/auto_deflate.rb | 2 ++ lib/http/features/auto_inflate.rb | 2 ++ lib/http/options.rb | 21 ++++++++++++--------- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/http/features/auto_deflate.rb b/lib/http/features/auto_deflate.rb index 4db92c85..108aca46 100644 --- a/lib/http/features/auto_deflate.rb +++ b/lib/http/features/auto_deflate.rb @@ -40,6 +40,8 @@ def deflated_body(body) end end + HTTP::Options.register_feature(:auto_deflate, self) + class CompressedBody < HTTP::Request::Body def initialize(uncompressed_body) @body = uncompressed_body diff --git a/lib/http/features/auto_inflate.rb b/lib/http/features/auto_inflate.rb index 1f6ce9f8..012b611f 100644 --- a/lib/http/features/auto_inflate.rb +++ b/lib/http/features/auto_inflate.rb @@ -18,6 +18,8 @@ def wrap_response(response) def stream_for(connection) Response::Body.new(Response::Inflater.new(connection)) end + + HTTP::Options.register_feature(:auto_inflate, self) end end end diff --git a/lib/http/options.rb b/lib/http/options.rb index b858691d..e7d6a3f2 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -1,30 +1,24 @@ # frozen_string_literal: true -# rubocop:disable Metrics/ClassLength, Style/RedundantSelf +# rubocop:disable Metrics/ClassLength require "http/headers" require "openssl" require "socket" require "http/uri" -require "http/feature" -require "http/features/auto_inflate" -require "http/features/auto_deflate" module HTTP class Options @default_socket_class = TCPSocket @default_ssl_socket_class = OpenSSL::SSL::SSLSocket @default_timeout_class = HTTP::Timeout::Null - @available_features = { - :auto_inflate => Features::AutoInflate, - :auto_deflate => Features::AutoDeflate - } + @available_features = {} class << self attr_accessor :default_socket_class, :default_ssl_socket_class, :default_timeout_class attr_reader :available_features - def new(options = {}) # rubocop:disable Style/OptionHash + def new(options = {}) return options if options.is_a?(self) super end @@ -33,6 +27,10 @@ def defined_options @defined_options ||= [] end + def register_feature(name, impl) + @available_features[name] = impl + end + protected def def_option(name, reader_only: false, &interpreter) @@ -197,3 +195,8 @@ def argument_error!(message) end end end + +require "http/feature" +require "http/features/auto_inflate" +require "http/features/auto_deflate" +require "http/features/logging" From eec137417931de1ac30abee6d9ec847aad7727d8 Mon Sep 17 00:00:00 2001 From: Paul Sadauskas Date: Thu, 16 Aug 2018 15:59:54 -0600 Subject: [PATCH 2/5] Add a logging feature --- lib/http/features/logging.rb | 55 +++++++++++++++++++ spec/lib/http/features/logging_spec.rb | 74 ++++++++++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 lib/http/features/logging.rb create mode 100644 spec/lib/http/features/logging_spec.rb diff --git a/lib/http/features/logging.rb b/lib/http/features/logging.rb new file mode 100644 index 00000000..cef2f8dc --- /dev/null +++ b/lib/http/features/logging.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module HTTP + module Features + # Log requests and responses. Request verb and uri, and Response status are + # logged at `info`, and the headers and bodies of both are logged at + # `debug`. Be sure to specify the logger when enabling the feature: + # + # HTTP.use(logging: {logger: Logger.new(STDOUT)}).get("https://example.com/") + # + class Logging < Feature + attr_reader :logger + + def initialize(logger: NullLogger.new) + @logger = logger + end + + def wrap_request(request) + logger.info { "> #{request.verb.to_s.upcase} #{request.uri}" } + logger.debug do + headers = request.headers.map { |name, value| "#{name}: #{value}" }.join("\n") + body = request.body.source + + headers + "\n\n" + body.to_s + end + request + end + + def wrap_response(response) + logger.info { "< #{response.status}" } + logger.debug do + headers = response.headers.map { |name, value| "#{name}: #{value}" }.join("\n") + body = response.body.to_s + + headers + "\n\n" + body + end + response + end + + class NullLogger + %w[fatal error warn info debug].each do |level| + define_method(level.to_sym) do |*_args| + nil + end + + define_method(:"#{level}?") do + true + end + end + end + + HTTP::Options.register_feature(:logging, self) + end + end +end diff --git a/spec/lib/http/features/logging_spec.rb b/spec/lib/http/features/logging_spec.rb new file mode 100644 index 00000000..fb4bf330 --- /dev/null +++ b/spec/lib/http/features/logging_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +RSpec.describe HTTP::Features::Logging do + subject(:feature) { HTTP::Features::Logging.new(:logger => logger) } + let(:logger) { TestLogger.new } + + describe "logging the request" do + let(:request) do + HTTP::Request.new( + :verb => :post, + :uri => "https://example.com/", + :headers => {:accept => "application/json"}, + :body => '{"hello": "world!"}' + ) + end + + it "should log the request" do + feature.wrap_request(request) + + expect(logger.output).to eq( + [ + "> POST https://example.com/", + <<~REQ.strip + Accept: application/json + Host: example.com + User-Agent: http.rb/4.0.0.dev + + {"hello": "world!"} + REQ + ] + ) + end + end + + describe "logging the response" do + let(:response) do + HTTP::Response.new( + :version => "1.1", + :uri => "https://example.com", + :status => 200, + :headers => {:content_type => "application/json"}, + :body => '{"success": true}' + ) + end + + it "should log the response" do + feature.wrap_response(response) + + expect(logger.output).to eq( + [ + "< 200 OK", + <<~REQ.strip + Content-Type: application/json + + {"success": true} + REQ + ] + ) + end + end + + class TestLogger + attr_reader :output + def initialize + @output = [] + end + + %w[fatal error warn info debug].each do |level| + define_method(level.to_sym) do |*args, &block| + @output << (block ? block.call : args) + end + end + end +end From a57354afe5a3cb406c3607fbab2dd9b25c9d1d89 Mon Sep 17 00:00:00 2001 From: Paul Sadauskas Date: Thu, 16 Aug 2018 16:55:41 -0600 Subject: [PATCH 3/5] Add AS::Notifications-compatible instrument feature --- lib/http/features/instrumentation.rb | 56 +++++++++++++++++++ lib/http/options.rb | 1 + .../lib/http/features/instrumentation_spec.rb | 56 +++++++++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 lib/http/features/instrumentation.rb create mode 100644 spec/lib/http/features/instrumentation_spec.rb diff --git a/lib/http/features/instrumentation.rb b/lib/http/features/instrumentation.rb new file mode 100644 index 00000000..489f2638 --- /dev/null +++ b/lib/http/features/instrumentation.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module HTTP + module Features + # Instrument requests and responses. Expects an + # ActiveSupport::Notifications-compatible instrumenter. Defaults to use a + # namespace of 'http' which may be overridden with a `:namespace` param. + # Emits a single event like `"request.{namespace}"`, eg `"request.http"`. + # Be sure to specify the instrumenter when enabling the feature: + # + # HTTP + # .use(instrumentation: {instrumenter: ActiveSupport::Notifications}) + # .get("https://example.com/") + # + class Instrumentation + attr_reader :instrumenter, :name + + def initialize(instrumenter: NullInstrumenter.new, namespace: "http") + @instrumenter = instrumenter + @name = "request.#{namespace}" + end + + def wrap_request(request) + instrumenter.instrument("start_#{name}", :request => request) + instrumenter.start(name, :request => request) + request + end + + def wrap_response(response) + instrumenter.finish(name, :response => response) + response + end + + HTTP::Options.register_feature(:instrumentation, self) + + class NullInstrumenter + def instrument(name, payload = {}) + start(name, payload) + begin + yield payload if block_given? + ensure + finish name, payload + end + end + + def start(_name, _payload) + true + end + + def finish(_name, _payload) + true + end + end + end + end +end diff --git a/lib/http/options.rb b/lib/http/options.rb index e7d6a3f2..98d3648a 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -200,3 +200,4 @@ def argument_error!(message) require "http/features/auto_inflate" require "http/features/auto_deflate" require "http/features/logging" +require "http/features/instrumentation" diff --git a/spec/lib/http/features/instrumentation_spec.rb b/spec/lib/http/features/instrumentation_spec.rb new file mode 100644 index 00000000..a15d2e2c --- /dev/null +++ b/spec/lib/http/features/instrumentation_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +RSpec.describe HTTP::Features::Instrumentation do + subject(:feature) { HTTP::Features::Instrumentation.new(:instrumenter => instrumenter) } + let(:instrumenter) { TestInstrumenter.new } + + describe "logging the request" do + let(:request) do + HTTP::Request.new( + :verb => :post, + :uri => "https://example.com/", + :headers => {:accept => "application/json"}, + :body => '{"hello": "world!"}' + ) + end + + it "should log the request" do + feature.wrap_request(request) + + expect(instrumenter.output[:start]).to eq(:request => request) + end + end + + describe "logging the response" do + let(:response) do + HTTP::Response.new( + :version => "1.1", + :uri => "https://example.com", + :status => 200, + :headers => {:content_type => "application/json"}, + :body => '{"success": true}' + ) + end + + it "should log the response" do + feature.wrap_response(response) + + expect(instrumenter.output[:finish]).to eq(:response => response) + end + end + + class TestInstrumenter < HTTP::Features::Instrumentation::NullInstrumenter + attr_reader :output + def initialize + @output = {} + end + + def start(_name, payload) + output[:start] = payload + end + + def finish(_name, payload) + output[:finish] = payload + end + end +end From 0e09e4fba1eba0740e82b4c5670b5076e1c2215d Mon Sep 17 00:00:00 2001 From: Paul Sadauskas Date: Fri, 17 Aug 2018 10:35:11 -0600 Subject: [PATCH 4/5] Make feature load-order less janky --- lib/http.rb | 1 + lib/http/client.rb | 1 + lib/http/feature.rb | 5 +++++ lib/http/options.rb | 5 ----- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/http.rb b/lib/http.rb index a1338598..9b8ec5b4 100644 --- a/lib/http.rb +++ b/lib/http.rb @@ -10,6 +10,7 @@ require "http/client" require "http/connection" require "http/options" +require "http/feature" require "http/request" require "http/request/writer" require "http/response" diff --git a/lib/http/client.rb b/lib/http/client.rb index c7ef28ea..0ca9cd0e 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -4,6 +4,7 @@ require "http/form_data" require "http/options" +require "http/feature" require "http/headers" require "http/connection" require "http/redirector" diff --git a/lib/http/feature.rb b/lib/http/feature.rb index a7e9e757..3c279e97 100644 --- a/lib/http/feature.rb +++ b/lib/http/feature.rb @@ -15,3 +15,8 @@ def wrap_response(response) end end end + +require "http/features/auto_inflate" +require "http/features/auto_deflate" +require "http/features/logging" +require "http/features/instrumentation" diff --git a/lib/http/options.rb b/lib/http/options.rb index 98d3648a..73b66165 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -196,8 +196,3 @@ def argument_error!(message) end end -require "http/feature" -require "http/features/auto_inflate" -require "http/features/auto_deflate" -require "http/features/logging" -require "http/features/instrumentation" From d13977354efecb92056326431aeafd7d6d3b3785 Mon Sep 17 00:00:00 2001 From: Paul Sadauskas Date: Fri, 17 Aug 2018 11:09:51 -0600 Subject: [PATCH 5/5] Fix rubocop issues because a line moved --- lib/http/options.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/http/options.rb b/lib/http/options.rb index 73b66165..4e4837e2 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -18,7 +18,7 @@ class << self attr_accessor :default_socket_class, :default_ssl_socket_class, :default_timeout_class attr_reader :available_features - def new(options = {}) + def new(options = {}) # rubocop:disable Style/OptionHash return options if options.is_a?(self) super end @@ -72,12 +72,12 @@ def initialize(options = {}) # rubocop:disable Style/OptionHash opts_w_defaults.each { |(k, v)| self[k] = v } end - def_option :headers do |headers| - self.headers.merge(headers) + def_option :headers do |new_headers| + headers.merge(new_headers) end - def_option :cookies do |cookies| - cookies.each_with_object self.cookies.dup do |(k, v), jar| + def_option :cookies do |new_cookies| + new_cookies.each_with_object cookies.dup do |(k, v), jar| cookie = k.is_a?(Cookie) ? k : Cookie.new(k.to_s, v.to_s) jar[cookie.name] = cookie.cookie_value end @@ -87,7 +87,7 @@ def initialize(options = {}) # rubocop:disable Style/OptionHash self.encoding = Encoding.find(encoding) end - def_option :features, :reader_only => true do |features| + def_option :features, :reader_only => true do |new_features| # Normalize features from: # # [{feature_one: {opt: 'val'}}, :feature_two] @@ -95,7 +95,7 @@ def initialize(options = {}) # rubocop:disable Style/OptionHash # into: # # {feature_one: {opt: 'val'}, feature_two: {}} - features = features.each_with_object({}) do |feature, h| + normalized_features = new_features.each_with_object({}) do |feature, h| if feature.is_a?(Hash) h.merge!(feature) else @@ -103,7 +103,7 @@ def initialize(options = {}) # rubocop:disable Style/OptionHash end end - self.features.merge(features) + features.merge(normalized_features) end def features=(features) @@ -195,4 +195,3 @@ def argument_error!(message) end end end -