From 174935e44bd4d49dbbc13b049b9119361ea9e909 Mon Sep 17 00:00:00 2001 From: Joel Taylor Date: Sun, 9 Aug 2020 14:32:12 -0700 Subject: [PATCH] Extract configurations into separate object Adds a `Stripe::StripeConfiguration` object to manage internal and user supplied configuration options. This is primarily motivated by #921 in order to provide a way to set options on for an instance of `StripeClient`. --- lib/stripe.rb | 198 ++++------------------- lib/stripe/stripe_configuration.rb | 167 +++++++++++++++++++ test/stripe/stripe_configuration_test.rb | 128 +++++++++++++++ test/stripe_test.rb | 107 +++++++++--- 4 files changed, 415 insertions(+), 185 deletions(-) create mode 100644 lib/stripe/stripe_configuration.rb create mode 100644 test/stripe/stripe_configuration_test.rb diff --git a/lib/stripe.rb b/lib/stripe.rb index 8a64ba59e..13ea28835 100644 --- a/lib/stripe.rb +++ b/lib/stripe.rb @@ -12,6 +12,7 @@ require "set" require "socket" require "uri" +require "forwardable" # Version require "stripe/version" @@ -38,6 +39,7 @@ require "stripe/api_resource" require "stripe/singleton_api_resource" require "stripe/webhook" +require "stripe/stripe_configuration" # Named API resources require "stripe/resources" @@ -48,47 +50,41 @@ module Stripe DEFAULT_CA_BUNDLE_PATH = __dir__ + "/data/ca-certificates.crt" - @app_info = nil - - @api_base = "https://api.stripe.com" - @connect_base = "https://connect.stripe.com" - @uploads_base = "https://files.stripe.com" - - @log_level = nil - @logger = nil - - @proxy = nil - - @max_network_retries = 0 - @max_network_retry_delay = 2 - @initial_network_retry_delay = 0.5 - - @ca_bundle_path = DEFAULT_CA_BUNDLE_PATH - @ca_store = nil - @verify_ssl_certs = true + # map to the same values as the standard library's logger + LEVEL_DEBUG = Logger::DEBUG + LEVEL_ERROR = Logger::ERROR + LEVEL_INFO = Logger::INFO - @open_timeout = 30 - @read_timeout = 80 + @app_info = nil - @enable_telemetry = true + @configuration = Stripe::StripeConfiguration.setup class << self - attr_accessor :api_key - attr_accessor :api_version + extend Forwardable + + # User configurable options + def_delegators :@configuration, :api_key, :api_key= + def_delegators :@configuration, :api_version, :api_version= + def_delegators :@configuration, :stripe_account, :stripe_account= + def_delegators :@configuration, :api_base, :api_base= + def_delegators :@configuration, :uploads_base, :uploads_base= + def_delegators :@configuration, :connect_base, :connect_base= + def_delegators :@configuration, :open_timeout, :open_timeout= + def_delegators :@configuration, :read_timeout, :read_timeout= + def_delegators :@configuration, :proxy, :proxy= + def_delegators :@configuration, :verify_ssl_certs, :verify_ssl_certs= + def_delegators :@configuration, :ca_bundle_path, :ca_bundle_path= + def_delegators :@configuration, :log_level, :log_level= + def_delegators :@configuration, :logger, :logger= + def_delegators :@configuration, :max_network_retries, :max_network_retries= + def_delegators :@configuration, :enable_telemetry=, :enable_telemetry? + + # Internal configurations + def_delegators :@configuration, :max_network_retry_delay + def_delegators :@configuration, :initial_network_retry_delay + def_delegators :@configuration, :ca_store + attr_accessor :client_id - attr_accessor :stripe_account - - # These all get manual attribute writers so that we can reset connections - # if they change. - attr_reader :api_base - attr_reader :connect_base - attr_reader :open_timeout - attr_reader :proxy - attr_reader :read_timeout - attr_reader :uploads_base - attr_reader :verify_ssl_certs - - attr_reader :max_network_retry_delay, :initial_network_retry_delay end # Gets the application for a plugin that's identified some. See @@ -101,126 +97,6 @@ def self.app_info=(info) @app_info = info end - def self.api_base=(api_base) - @api_base = api_base - StripeClient.clear_all_connection_managers - end - - # The location of a file containing a bundle of CA certificates. By default - # the library will use an included bundle that can successfully validate - # Stripe certificates. - def self.ca_bundle_path - @ca_bundle_path - end - - def self.ca_bundle_path=(path) - @ca_bundle_path = path - - # empty this field so a new store is initialized - @ca_store = nil - - StripeClient.clear_all_connection_managers - end - - # A certificate store initialized from the the bundle in #ca_bundle_path and - # which is used to validate TLS on every request. - # - # This was added to the give the gem "pseudo thread safety" in that it seems - # when initiating many parallel requests marshaling the certificate store is - # the most likely point of failure (see issue #382). Any program attempting - # to leverage this pseudo safety should make a call to this method (i.e. - # `Stripe.ca_store`) in their initialization code because it marshals lazily - # and is itself not thread safe. - def self.ca_store - @ca_store ||= begin - store = OpenSSL::X509::Store.new - store.add_file(ca_bundle_path) - store - end - end - - def self.connect_base=(connect_base) - @connect_base = connect_base - StripeClient.clear_all_connection_managers - end - - def self.enable_telemetry? - @enable_telemetry - end - - def self.enable_telemetry=(val) - @enable_telemetry = val - end - - # map to the same values as the standard library's logger - LEVEL_DEBUG = Logger::DEBUG - LEVEL_ERROR = Logger::ERROR - LEVEL_INFO = Logger::INFO - - # When set prompts the library to log some extra information to $stdout and - # $stderr about what it's doing. For example, it'll produce information about - # requests, responses, and errors that are received. Valid log levels are - # `debug` and `info`, with `debug` being a little more verbose in places. - # - # Use of this configuration is only useful when `.logger` is _not_ set. When - # it is, the decision what levels to print is entirely deferred to the logger. - def self.log_level - @log_level - end - - def self.log_level=(val) - # Backwards compatibility for values that we briefly allowed - if val == "debug" - val = LEVEL_DEBUG - elsif val == "info" - val = LEVEL_INFO - end - - if !val.nil? && ![LEVEL_DEBUG, LEVEL_ERROR, LEVEL_INFO].include?(val) - raise ArgumentError, - "log_level should only be set to `nil`, `debug` or `info`" - end - @log_level = val - end - - # Sets a logger to which logging output will be sent. The logger should - # support the same interface as the `Logger` class that's part of Ruby's - # standard library (hint, anything in `Rails.logger` will likely be - # suitable). - # - # If `.logger` is set, the value of `.log_level` is ignored. The decision on - # what levels to print is entirely deferred to the logger. - def self.logger - @logger - end - - def self.logger=(val) - @logger = val - end - - def self.max_network_retries - @max_network_retries - end - - def self.max_network_retries=(val) - @max_network_retries = val.to_i - end - - def self.open_timeout=(open_timeout) - @open_timeout = open_timeout - StripeClient.clear_all_connection_managers - end - - def self.proxy=(proxy) - @proxy = proxy - StripeClient.clear_all_connection_managers - end - - def self.read_timeout=(read_timeout) - @read_timeout = read_timeout - StripeClient.clear_all_connection_managers - end - # Sets some basic information about the running application that's sent along # with API requests. Useful for plugin authors to identify their plugin when # communicating with Stripe. @@ -234,16 +110,6 @@ def self.set_app_info(name, partner_id: nil, url: nil, version: nil) version: version, } end - - def self.uploads_base=(uploads_base) - @uploads_base = uploads_base - StripeClient.clear_all_connection_managers - end - - def self.verify_ssl_certs=(verify_ssl_certs) - @verify_ssl_certs = verify_ssl_certs - StripeClient.clear_all_connection_managers - end end Stripe.log_level = ENV["STRIPE_LOG"] unless ENV["STRIPE_LOG"].nil? diff --git a/lib/stripe/stripe_configuration.rb b/lib/stripe/stripe_configuration.rb new file mode 100644 index 000000000..71f01f2a4 --- /dev/null +++ b/lib/stripe/stripe_configuration.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +module Stripe + # Configurable options: + # + # =ca_bundle_path= + # The location of a file containing a bundle of CA certificates. By default + # the library will use an included bundle that can successfully validate + # Stripe certificates. + # + # =log_level= + # When set prompts the library to log some extra information to $stdout and + # $stderr about what it's doing. For example, it'll produce information about + # requests, responses, and errors that are received. Valid log levels are + # `debug` and `info`, with `debug` being a little more verbose in places. + # + # Use of this configuration is only useful when `.logger` is _not_ set. When + # it is, the decision what levels to print is entirely deferred to the logger. + # + # =logger= + # The logger should support the same interface as the `Logger` class that's + # part of Ruby's standard library (hint, anything in `Rails.logger` will + # likely be suitable). + # + # If `.logger` is set, the value of `.log_level` is ignored. The decision on + # what levels to print is entirely deferred to the logger. + class StripeConfiguration + attr_accessor :api_key + attr_accessor :api_version + attr_accessor :client_id + attr_accessor :enable_telemetry + attr_accessor :logger + attr_accessor :stripe_account + + attr_reader :api_base + attr_reader :uploads_base + attr_reader :connect_base + attr_reader :ca_bundle_path + attr_reader :log_level + attr_reader :initial_network_retry_delay + attr_reader :max_network_retries + attr_reader :max_network_retry_delay + attr_reader :open_timeout + attr_reader :read_timeout + attr_reader :proxy + attr_reader :verify_ssl_certs + + def self.setup + new.tap do |instance| + yield(instance) if block_given? + end + end + + # Create a new config based off an existing one. This is useful when the + # caller wants to override the global configuration + def reverse_duplicate_merge(hash) + dup.tap do |instance| + hash.each do |option, value| + instance.public_send("#{option}=", value) + end + end + end + + def initialize + @ca_bundle_path = Stripe::DEFAULT_CA_BUNDLE_PATH + @enable_telemetry = true + @verify_ssl_certs = true + + @max_network_retries = 0 + @initial_network_retry_delay = 0.5 + @max_network_retry_delay = 2 + + @open_timeout = 30 + @read_timeout = 80 + + @api_base = "https://api.stripe.com" + @connect_base = "https://connect.stripe.com" + @uploads_base = "https://files.stripe.com" + end + + def log_level=(val) + # Backwards compatibility for values that we briefly allowed + if val == "debug" + val = Stripe::LEVEL_DEBUG + elsif val == "info" + val = Stripe::LEVEL_INFO + end + + levels = [Stripe::LEVEL_INFO, Stripe::LEVEL_DEBUG, Stripe::LEVEL_ERROR] + + if !val.nil? && !levels.include?(val) + raise ArgumentError, + "log_level should only be set to `nil`, `debug` or `info`" + end + @log_level = val + end + + def max_network_retries=(val) + @max_network_retries = val.to_i + end + + def open_timeout=(open_timeout) + @open_timeout = open_timeout + StripeClient.clear_all_connection_managers + end + + def read_timeout=(read_timeout) + @read_timeout = read_timeout + StripeClient.clear_all_connection_managers + end + + def proxy=(proxy) + @proxy = proxy + StripeClient.clear_all_connection_managers + end + + def verify_ssl_certs=(verify_ssl_certs) + @verify_ssl_certs = verify_ssl_certs + StripeClient.clear_all_connection_managers + end + + def uploads_base=(uploads_base) + @uploads_base = uploads_base + StripeClient.clear_all_connection_managers + end + + def connect_base=(connect_base) + @connect_base = connect_base + StripeClient.clear_all_connection_managers + end + + def api_base=(api_base) + @api_base = api_base + StripeClient.clear_all_connection_managers + end + + def ca_bundle_path=(path) + @ca_bundle_path = path + + # empty this field so a new store is initialized + @ca_store = nil + + StripeClient.clear_all_connection_managers + end + + # A certificate store initialized from the the bundle in #ca_bundle_path and + # which is used to validate TLS on every request. + # + # This was added to the give the gem "pseudo thread safety" in that it seems + # when initiating many parallel requests marshaling the certificate store is + # the most likely point of failure (see issue #382). Any program attempting + # to leverage this pseudo safety should make a call to this method (i.e. + # `Stripe.ca_store`) in their initialization code because it marshals lazily + # and is itself not thread safe. + def ca_store + @ca_store ||= begin + store = OpenSSL::X509::Store.new + store.add_file(ca_bundle_path) + store + end + end + + def enable_telemetry? + enable_telemetry + end + end +end diff --git a/test/stripe/stripe_configuration_test.rb b/test/stripe/stripe_configuration_test.rb new file mode 100644 index 000000000..774d5ea3f --- /dev/null +++ b/test/stripe/stripe_configuration_test.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require ::File.expand_path("../test_helper", __dir__) + +module Stripe + class StripeConfigurationTest < Test::Unit::TestCase + context ".setup" do + should "initialize a new configuration with defaults" do + config = Stripe::StripeConfiguration.setup + + assert_equal Stripe::DEFAULT_CA_BUNDLE_PATH, config.ca_bundle_path + assert_equal true, config.enable_telemetry + assert_equal true, config.verify_ssl_certs + assert_equal 2, config.max_network_retry_delay + assert_equal 0.5, config.initial_network_retry_delay + assert_equal 0, config.max_network_retries + assert_equal 30, config.open_timeout + assert_equal 80, config.read_timeout + assert_equal "https://api.stripe.com", config.api_base + assert_equal "https://connect.stripe.com", config.connect_base + assert_equal "https://files.stripe.com", config.uploads_base + end + + should "allow for overrides when a block is passed" do + config = Stripe::StripeConfiguration.setup do |c| + c.open_timeout = 100 + c.read_timeout = 100 + end + + assert_equal 100, config.open_timeout + assert_equal 100, config.read_timeout + end + end + + context "#reverse_duplicate_merge" do + should "return a duplicate object with overrides" do + config = Stripe::StripeConfiguration.setup do |c| + c.open_timeout = 100 + end + + duped_config = config.reverse_duplicate_merge(read_timeout: 500) + + assert_equal config.open_timeout, duped_config.open_timeout + assert_equal 500, duped_config.read_timeout + end + end + + context "#max_network_retries=" do + should "coerce the option into an integer" do + config = Stripe::StripeConfiguration.setup + + config.max_network_retries = "10" + assert_equal 10, config.max_network_retries + end + end + + context "#log_level=" do + should "be backwards compatible with old values" do + config = Stripe::StripeConfiguration.setup + + config.log_level = "debug" + assert_equal Stripe::LEVEL_DEBUG, config.log_level + + config.log_level = "info" + assert_equal Stripe::LEVEL_INFO, config.log_level + end + + should "raise an error if the value isn't valid" do + config = Stripe::StripeConfiguration.setup + + assert_raises ArgumentError do + config.log_level = "Foo" + end + end + end + + context "options that require all connection managers to be cleared" do + should "clear when setting allow ca_bundle_path" do + config = Stripe::StripeConfiguration.setup + + StripeClient.expects(:clear_all_connection_managers) + config.ca_bundle_path = "/path/to/ca/bundle" + end + + should "clear when setting open timeout" do + config = Stripe::StripeConfiguration.setup + + StripeClient.expects(:clear_all_connection_managers) + config.open_timeout = 10 + end + + should "clear when setting read timeout" do + config = Stripe::StripeConfiguration.setup + + StripeClient.expects(:clear_all_connection_managers) + config.read_timeout = 10 + end + + should "clear when setting uploads_base" do + config = Stripe::StripeConfiguration.setup + + StripeClient.expects(:clear_all_connection_managers) + config.uploads_base = "https://other.stripe.com" + end + + should "clearn when setting api_base to be configured" do + config = Stripe::StripeConfiguration.setup + + StripeClient.expects(:clear_all_connection_managers) + config.api_base = "https://other.stripe.com" + end + + should "clear when setting connect_base" do + config = Stripe::StripeConfiguration.setup + + StripeClient.expects(:clear_all_connection_managers) + config.connect_base = "https://other.stripe.com" + end + + should "clear when setting verify_ssl_certs" do + config = Stripe::StripeConfiguration.setup + + StripeClient.expects(:clear_all_connection_managers) + config.verify_ssl_certs = false + end + end + end +end diff --git a/test/stripe_test.rb b/test/stripe_test.rb index bdc283c27..11f100b85 100644 --- a/test/stripe_test.rb +++ b/test/stripe_test.rb @@ -23,28 +23,97 @@ class StripeTest < Test::Unit::TestCase end end - should "allow ca_bundle_path to be configured" do - begin - old = Stripe.ca_bundle_path - Stripe.ca_bundle_path = "path/to/ca/bundle" - assert_equal "path/to/ca/bundle", Stripe.ca_bundle_path - ensure - Stripe.ca_bundle_path = old + context "forwardable configurations" do + context "internal configurations" do + should "return the certificate store" do + assert Stripe.ca_store.is_a?(OpenSSL::X509::Store) + end + + should "return the max_network_retry_delay" do + assert_equal 2, Stripe.max_network_retry_delay + end + + should "return the initial_network_retry_delay" do + assert_equal 0.5, Stripe.initial_network_retry_delay + end end - end - should "allow max_network_retries to be configured" do - begin - old = Stripe.max_network_retries - Stripe.max_network_retries = 99 - assert_equal 99, Stripe.max_network_retries - ensure - Stripe.max_network_retries = old + should "allow ca_bundle_path to be configured" do + Stripe::StripeClient.expects(:clear_all_connection_managers) + Stripe.ca_bundle_path = "/path/to/ca/bundle" + assert_equal "/path/to/ca/bundle", Stripe.ca_bundle_path end - end - should "have default open and read timeouts" do - assert_equal Stripe.open_timeout, 30 - assert_equal Stripe.read_timeout, 80 + should "allow open timeout to be configured" do + Stripe.open_timeout = 10 + assert_equal 10, Stripe.open_timeout + end + + should "allow read timeout to be configured" do + Stripe.read_timeout = 10 + assert_equal 10, Stripe.read_timeout + end + + should "allow api_key to be configured" do + Stripe.api_key = "sk_local_test" + assert_equal "sk_local_test", Stripe.api_key + end + + should "allow stripe_account to be configured" do + Stripe.stripe_account = "acct_1234" + assert_equal "acct_1234", Stripe.stripe_account + end + + should "allow enable_telemetry to be configured" do + begin + old = Stripe.enable_telemetry? + + Stripe.enable_telemetry = false + assert_equal false, Stripe.enable_telemetry? + ensure + Stripe.enable_telemetry = old + end + end + + should "allow log_level to be configured" do + Stripe.log_level = "debug" + assert_equal ::Logger::DEBUG, Stripe.log_level + end + + should "allow logger to be configured" do + logger = Object.new + Stripe.logger = logger + assert_equal logger, Stripe.logger + end + + should "allow proxy to be configured" do + Stripe.proxy = "http://proxy" + assert_equal "http://proxy", Stripe.proxy + end + + should "allow uploads_base to be configured" do + Stripe.uploads_base = "https://other.stripe.com" + assert_equal "https://other.stripe.com", Stripe.uploads_base + end + + should "allow api_base to be configured" do + Stripe.api_base = "https://other.stripe.com" + assert_equal "https://other.stripe.com", Stripe.api_base + end + + should "allow connect_base to be configured" do + Stripe.connect_base = "https://other.stripe.com" + assert_equal "https://other.stripe.com", Stripe.connect_base + end + + should "allow verify_ssl_certs to be configured" do + Stripe.verify_ssl_certs = false + assert_equal false, Stripe.verify_ssl_certs + end + + should "allow client_id to be configured" do + Stripe.client_id = "client" + assert_equal "client", Stripe.client_id + end end end