From da7d55c62accf50837a0dd5241257139c1bc83ee Mon Sep 17 00:00:00 2001 From: Aaron Rosenberg Date: Sat, 31 Oct 2020 16:13:34 -0500 Subject: [PATCH 1/4] Still green test update Nest existing test for coming compatibility test Test assumed behavior --- spec/iex/client_spec.rb | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/spec/iex/client_spec.rb b/spec/iex/client_spec.rb index 47da75b..005253d 100644 --- a/spec/iex/client_spec.rb +++ b/spec/iex/client_spec.rb @@ -101,20 +101,24 @@ end end end + context 'logger option' do let(:logger) { Logger.new(STDOUT) } after { IEX::Api.logger.reset! } context 'when assigning an instance' do - before { IEX::Api.logger = logger } - context '#initialize' do - it 'sets logger' do - expect(client.logger.instance).to eq(logger) - end - it 'creates a connection with a logger' do - expect(client.send(:connection).builder.handlers).to include ::Faraday::Response::Logger + context 'when directly assigning `logger`' do + before { IEX::Api.logger = logger } + + it 'sets logger' do + expect(client.logger.instance).to eq(logger) + end + + it 'creates a connection with a logger' do + expect(client.send(:connection).builder.handlers).to include ::Faraday::Response::Logger + end end end end @@ -164,6 +168,23 @@ end end end + + context 'when resetting/changing configuration' do + before do + IEX::Api.configure { |config| config.user_agent = 'custom/user-agent' } + end + + it 'does not reset the client' do + expect { IEX::Api.config.reset! }.not_to change(client, :user_agent).from('custom/user-agent') + end + + it 'effects the next client' do + pre_config_client = described_class.new + IEX::Api.configure { |config| config.user_agent = 'custom/user-agent-2' } + expect(described_class.new.user_agent).not_to eq(pre_config_client.user_agent) + end + end + context 'without a token' do let(:client) { described_class.new } it 'results in an API key error', vcr: { cassette_name: 'client/access_denied' } do From 500f1141acaec0c93e2d071961ed5e98021b7130 Mon Sep 17 00:00:00 2001 From: Aaron Rosenberg Date: Sat, 31 Oct 2020 16:51:25 -0500 Subject: [PATCH 2/4] Structural cleanup Fix Rubocop violation around `extend self` in `Module`s. `class << self` is clearest in this case because we need to set attr_accessors on the Module as well as the `reset!` method. `module_function` also pollutes the including object as it gets a private method of the same name. No need to include instance methods on Config::Client or include it in Api::Client if the ATTRIBUTE array is simply splatted into attr_accessor (and is easier to follow where it comes from) --- .rubocop_todo.yml | 9 --------- lib/iex/api/client.rb | 2 +- lib/iex/api/config/client.rb | 28 ++++++++++++++-------------- lib/iex/api/config/logger.rb | 14 +++++++------- spec/iex/client_spec.rb | 4 ++++ 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 458782e..44ff278 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,12 +11,3 @@ Style/AsciiComments: Exclude: - 'lib/iex/resources/quote.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, Autocorrect. -# SupportedStyles: module_function, extend_self -Style/ModuleFunction: - Exclude: - - 'lib/iex/api/config/client.rb' - - 'lib/iex/api/config/logger.rb' diff --git a/lib/iex/api/client.rb b/lib/iex/api/client.rb index 6218417..8ebce6d 100644 --- a/lib/iex/api/client.rb +++ b/lib/iex/api/client.rb @@ -22,7 +22,7 @@ class Client include Cloud::Connection include Cloud::Request - include Config::Client + attr_accessor(*Config::Client::ATTRIBUTES) attr_reader :logger diff --git a/lib/iex/api/config/client.rb b/lib/iex/api/config/client.rb index 5ba1c3e..08ff640 100644 --- a/lib/iex/api/config/client.rb +++ b/lib/iex/api/config/client.rb @@ -2,8 +2,6 @@ module IEX module Api module Config module Client - extend self - ATTRIBUTES = %i[ ca_file ca_path @@ -17,20 +15,22 @@ module Client user_agent ].freeze - attr_accessor(*ATTRIBUTES) + class << self + attr_accessor(*ATTRIBUTES) - def reset! - self.ca_file = defined?(OpenSSL) ? OpenSSL::X509::DEFAULT_CERT_FILE : nil - self.ca_path = defined?(OpenSSL) ? OpenSSL::X509::DEFAULT_CERT_DIR : nil - self.endpoint = 'https://cloud.iexapis.com/v1' - self.publishable_token = ENV['IEX_API_PUBLISHABLE_TOKEN'] - self.secret_token = ENV['IEX_API_SECRET_TOKEN'] - self.user_agent = "IEX Ruby Client/#{IEX::VERSION}" + def reset! + self.ca_file = defined?(OpenSSL) ? OpenSSL::X509::DEFAULT_CERT_FILE : nil + self.ca_path = defined?(OpenSSL) ? OpenSSL::X509::DEFAULT_CERT_DIR : nil + self.endpoint = 'https://cloud.iexapis.com/v1' + self.publishable_token = ENV['IEX_API_PUBLISHABLE_TOKEN'] + self.secret_token = ENV['IEX_API_SECRET_TOKEN'] + self.user_agent = "IEX Ruby Client/#{IEX::VERSION}" - self.open_timeout = nil - self.proxy = nil - self.referer = nil - self.timeout = nil + self.open_timeout = nil + self.proxy = nil + self.referer = nil + self.timeout = nil + end end end end diff --git a/lib/iex/api/config/logger.rb b/lib/iex/api/config/logger.rb index 3d6af08..83aec62 100644 --- a/lib/iex/api/config/logger.rb +++ b/lib/iex/api/config/logger.rb @@ -2,20 +2,20 @@ module IEX module Api module Config module Logger - extend self - ATTRIBUTES = %i[ instance options proc ].freeze - attr_accessor(*ATTRIBUTES) + class << self + attr_accessor(*ATTRIBUTES) - def reset! - self.instance = nil - self.options = {} - self.proc = nil + def reset! + self.instance = nil + self.options = {} + self.proc = nil + end end end end diff --git a/spec/iex/client_spec.rb b/spec/iex/client_spec.rb index 005253d..a6a1e8c 100644 --- a/spec/iex/client_spec.rb +++ b/spec/iex/client_spec.rb @@ -183,6 +183,10 @@ IEX::Api.configure { |config| config.user_agent = 'custom/user-agent-2' } expect(described_class.new.user_agent).not_to eq(pre_config_client.user_agent) end + + it 'should not allow the client to reset' do + expect { client.reset! }.to raise_error(NoMethodError) + end end context 'without a token' do From 3948bb5dad1f1c877f66bbf24f3cde82145f582f Mon Sep 17 00:00:00 2001 From: Aaron Rosenberg Date: Sat, 31 Oct 2020 16:51:32 -0500 Subject: [PATCH 3/4] Remove unnecessary setup step --- spec/iex/config/logger_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/iex/config/logger_spec.rb b/spec/iex/config/logger_spec.rb index f5e75bf..298d8ed 100644 --- a/spec/iex/config/logger_spec.rb +++ b/spec/iex/config/logger_spec.rb @@ -1,7 +1,6 @@ require 'spec_helper' describe IEX::Api::Config::Logger do - before { IEX::Api.logger.reset! } after { IEX::Api.logger.reset! } describe '#defaults' do From 46365db17830f63999a6dc474b343c6a4535fa9c Mon Sep 17 00:00:00 2001 From: Aaron Rosenberg Date: Sat, 31 Oct 2020 17:09:29 -0500 Subject: [PATCH 4/4] Ensure backward compatibility w/ previous logger config Extract Config accessory to module for clearer inclusion/extension into IEX::Api --- CHANGELOG.md | 1 + README.md | 18 +++++++++++++++--- lib/iex/api.rb | 2 +- lib/iex/api/client.rb | 4 ++++ lib/iex/api/config/client.rb | 18 ++++++++++-------- lib/iex/api/config/logger.rb | 16 ++++++++-------- spec/iex/client_spec.rb | 20 ++++++++++++++++++++ spec/iex/config/client_spec.rb | 27 +++++++++++++++++++++++++++ 8 files changed, 86 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b355072..31ceeee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * [#84](https://github.com/dblock/iex-ruby-client/pull/84): Added support for Advanced Stats API - [@FanaHOVA](https://github.com/fanahova). * [#86](https://github.com/dblock/iex-ruby-client/issues/86): Added advanced logger configuration to `config.logger`. Now a hash with keys `:instance, :options, and :proc` can be passed and used directly with Faraday - [@agrberg](https://github.com/agrberg). * [#88](https://github.com/dblock/iex-ruby-client/pull/88): Updated logger configuration to work like `Config` class and allow attribute and block based configuration - [@agrberg](https://github.com/agrberg). +* [#89](https://github.com/dblock/iex-ruby-client/pull/89): Backfill breaking `Config#logger` setting change from [#88](https://github.com/dblock/iex-ruby-client/pull/88) - [@agrberg](https://github.com/agrberg). * Your contribution here. ### 1.2.0 (2020/09/01) diff --git a/README.md b/README.md index 5fc0830..f0c004c 100644 --- a/README.md +++ b/README.md @@ -445,7 +445,7 @@ client.post('ref-data/isin', isin: ['US0378331005'], token: 'secret_token') # [{ You can configure client options globally or directly with a `IEX::Api::Client` instance. ```ruby -IEX::Api::Client.configure do |config| +IEX::Api.configure do |config| config.publishable_token = ENV['IEX_API_PUBLISHABLE_TOKEN'] config.endpoint = 'https://sandbox.iexapis.com/v1' # use sandbox environment end @@ -478,11 +478,23 @@ referer | Optional string for HTTP `Referer` header, enables token d Faraday will not log HTTP requests by default. In order to do this you can either provide a `logger` instance or configuration attributes to `IEX::Api::Client`. Configuration allows you to supply the `instance`, `options`, and `proc` to [Faraday](https://lostisland.github.io/faraday/middleware/logger#include-and-exclude-headersbodies). ```ruby -IEX::Api::Client.configure do |config| - config.logger.instance = Logger.new(STDOUT) +logger_instance = Logger.new(STDOUT) + +IEX::Api.configure do |config| + config.logger.instance = logger_instance config.logger.options = { bodies: true } config.logger.proc = proc { |logger| logger.filter(/T?[sp]k_\w+/i, '[REMOVED]') } end +# or +IEX::Api.logger do |logger| + logger.instance = logger_instance + logger.options = … + logger.proc = … +end +# or +IEX::Api.logger = logger_instance +# or +IEX::Api::Client.new(logger: logger_instance) ``` ## Sandbox Environment diff --git a/lib/iex/api.rb b/lib/iex/api.rb index 72c6829..9f8cab5 100644 --- a/lib/iex/api.rb +++ b/lib/iex/api.rb @@ -16,6 +16,6 @@ require_relative 'endpoints/ref_data' require_relative 'endpoints/stock_market' -require_relative 'api/config/client' require_relative 'api/config/logger' +require_relative 'api/config/client' require_relative 'api/client' diff --git a/lib/iex/api/client.rb b/lib/iex/api/client.rb index 8ebce6d..6181885 100644 --- a/lib/iex/api/client.rb +++ b/lib/iex/api/client.rb @@ -1,5 +1,8 @@ module IEX module Api + extend Config::Client::Accessor + extend Config::Logger::Accessor + class Client include Endpoints::AdvancedStats include Endpoints::Chart @@ -31,6 +34,7 @@ def initialize(options = {}) send("#{key}=", options[key] || IEX::Api.config.send(key)) end @logger = Config::Logger.dup + @logger.instance = options[:logger] if options.key?(:logger) end end end diff --git a/lib/iex/api/config/client.rb b/lib/iex/api/config/client.rb index 08ff640..9c520a4 100644 --- a/lib/iex/api/config/client.rb +++ b/lib/iex/api/config/client.rb @@ -16,6 +16,8 @@ module Client ].freeze class << self + include Config::Logger::Accessor + attr_accessor(*ATTRIBUTES) def reset! @@ -32,16 +34,16 @@ def reset! self.timeout = nil end end - end - end - class << self - def configure - block_given? ? yield(Config::Client) : Config::Client - end + module Accessor + def configure + block_given? ? yield(Config::Client) : Config::Client + end - def config - Config::Client + def config + Config::Client + end + end end end end diff --git a/lib/iex/api/config/logger.rb b/lib/iex/api/config/logger.rb index 83aec62..505b94d 100644 --- a/lib/iex/api/config/logger.rb +++ b/lib/iex/api/config/logger.rb @@ -17,16 +17,16 @@ def reset! self.proc = nil end end - end - end - class << self - def logger - block_given? ? yield(Config::Logger) : Config::Logger - end + module Accessor + def logger + block_given? ? yield(Config::Logger) : Config::Logger + end - def logger=(instance) - logger.instance = instance + def logger=(instance) + logger.instance = instance + end + end end end end diff --git a/spec/iex/client_spec.rb b/spec/iex/client_spec.rb index a6a1e8c..ef9453d 100644 --- a/spec/iex/client_spec.rb +++ b/spec/iex/client_spec.rb @@ -120,6 +120,26 @@ expect(client.send(:connection).builder.handlers).to include ::Faraday::Response::Logger end end + + context 'when assigning through `configure.logger`' do + it 'sets the logger' do + IEX::Api.configure.logger = logger + expect(client.logger.instance).to eq(logger) + end + end + + context 'when passing in at initialization' do + it 'sets the logger' do + client = described_class.new(logger: logger) + expect(client.logger.instance).to eq(logger) + end + + it 'can overwrite a set logger' do + IEX::Api.logger = logger + client = described_class.new(logger: nil) + expect(client.logger.instance).to be_nil + end + end end end diff --git a/spec/iex/config/client_spec.rb b/spec/iex/config/client_spec.rb index ad4974c..b358ce5 100644 --- a/spec/iex/config/client_spec.rb +++ b/spec/iex/config/client_spec.rb @@ -19,4 +19,31 @@ expect(IEX::Api.config.endpoint).to eq 'updated' end end + + context 'when configuring the logger' do + after { IEX::Api.configure.logger.reset! } + + let(:logger) { Logger.new(STDOUT) } + + describe '#logger=' do + it 'updates IEX::Api.config correctly' do + expect do + IEX::Api.configure { |config| config.logger = logger } + end.to change(IEX::Api.config.logger, :instance).from(nil).to(logger) + end + + it 'updates IEX::Api.logger correctly' do + expect do + IEX::Api.configure { |config| config.logger = logger } + end.to change(IEX::Api.logger, :instance).from(nil).to(logger) + end + end + + describe '#logger' do + it 'accesses the current logger' do + expect { IEX::Api.logger = logger } + .to change(IEX::Api.config.logger, :instance).from(nil).to(logger) + end + end + end end