Import global configuration for options not available on StripeClient options (#1512)

* Fix client options not importing global config

* update tests and fix bugs

* lint

* catch NotImplementedError and map for older Ruby

* rubocop todo

Author: helenye-stripe

lib/stripe.rb

@@ -74,6 +74,28 @@ module Stripe
   DEFAULT_UPLOAD_BASE = "https://files.stripe.com"
   DEFAULT_METER_EVENTS_BASE = "https://meter-events.stripe.com"
 
+  # Options that can be configured globally by users
+  USER_CONFIGURABLE_GLOBAL_OPTIONS = Set.new(%i[
+    api_key
+    api_version
+    stripe_account
+    api_base
+    uploads_base
+    connect_base
+    meter_events_base
+    open_timeout
+    read_timeout
+    write_timeout
+    proxy
+    verify_ssl_certs
+    ca_bundle_path
+    log_level
+    logger
+    max_network_retries
+    enable_telemetry
+    client_id
+  ])
+
   @app_info = nil
 
   @config = Stripe::StripeConfiguration.setup

lib/stripe/stripe_client.rb

@@ -10,6 +10,10 @@ class StripeClient
 
     # attr_readers: The end of the section generated from our OpenAPI spec
 
+    # For internal use only. Does not provide a stable API and may be broken
+    # with future non-major changes.
+    CLIENT_OPTIONS = Set.new(%i[api_key stripe_account stripe_context api_version api_base uploads_base connect_base meter_events_base client_id])
+
     # Initializes a new StripeClient
     def initialize(api_key, # rubocop:todo Metrics/ParameterLists
                    stripe_account: nil,
@@ -40,7 +44,8 @@ def initialize(api_key, # rubocop:todo Metrics/ParameterLists
         client_id: client_id,
       }.reject { |_k, v| v.nil? }
 
-      @requestor = APIRequestor.new(config_opts)
+      config = StripeConfiguration.client_init(config_opts)
+      @requestor = APIRequestor.new(config)
 
       # top-level services: The beginning of the section generated from our OpenAPI spec
       @v1 = Stripe::V1Services.new(@requestor)

lib/stripe/stripe_configuration.rb

@@ -37,6 +37,25 @@ def self.setup
       end
     end
 
+    # Set options to the StripeClient configured options, if valid as a client option and provided
+    # Otherwise, for user configurable global options, set them to the global configuration
+    # For all other options, set them to the StripeConfiguration default value
+    def self.client_init(config_opts)
+      global_config = Stripe.config
+      imported_options = USER_CONFIGURABLE_GLOBAL_OPTIONS - StripeClient::CLIENT_OPTIONS
+      client_config = StripeConfiguration.setup do |instance|
+        imported_options.each do |key|
+          begin
+            instance.public_send("#{key}=", global_config.public_send(key)) if global_config.respond_to?(key)
+          rescue NotImplementedError => e
+            # In Ruby <= 2.5, we can't set write_timeout on Net::HTTP, log an error and continue
+            Util.log_error("Failed to set #{key} on client configuration: #{e}")
+          end
+        end
+      end
+      client_config.reverse_duplicate_merge(config_opts)
+    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)
@@ -68,7 +87,8 @@ def initialize
       @connect_base = DEFAULT_CONNECT_BASE
       @uploads_base = DEFAULT_UPLOAD_BASE
       @meter_events_base = DEFAULT_METER_EVENTS_BASE
-      @base_addresses = { api: @api_base, connect: @connect_base, files: @uploads_base, events: @meter_events_base }
+      @base_addresses = { api: @api_base, connect: @connect_base, files: @uploads_base,
+                          meter_events: @meter_events_base, }
     end
 
     def log_level=(val)

test/stripe/stripe_client_test.rb

@@ -20,6 +20,7 @@ class StripeClientTest < Test::Unit::TestCase
         @orig_api_key = Stripe.api_key
         @orig_stripe_account = Stripe.stripe_account
         @orig_open_timeout = Stripe.open_timeout
+        @orig_api_version = Stripe.api_version
 
         Stripe.api_key = "DONT_USE_THIS_KEY"
         Stripe.stripe_account = "DONT_USE_THIS_ACCOUNT"
@@ -30,6 +31,7 @@ class StripeClientTest < Test::Unit::TestCase
         Stripe.api_key = @orig_api_key
         Stripe.stripe_account = @orig_stripe_account
         Stripe.open_timeout = @orig_open_timeout
+        Stripe.api_version = @orig_api_version
       end
 
       should "use default config options" do
@@ -45,7 +47,6 @@ class StripeClientTest < Test::Unit::TestCase
         client = StripeClient.new("test_123")
         assert_equal "test_123", client.instance_variable_get(:@requestor).config.api_key
         assert_nil client.instance_variable_get(:@requestor).config.stripe_account
-        assert_equal 30, client.instance_variable_get(:@requestor).config.open_timeout
       end
 
       should "use client config options" do
@@ -67,6 +68,30 @@ class StripeClientTest < Test::Unit::TestCase
         assert_equal "2022-11-15", req.headers["Stripe-Version"]
       end
 
+      should "use global config options for options unavailable in client" do
+        Stripe.api_key = "NOT_THIS_KEY"
+        Stripe.stripe_account = "NOT_THIS_ACCOUNT"
+        Stripe.api_version = "2022-11-15"
+        client = StripeClient.new("test_123", stripe_account: "acct_123")
+        # Imported from global options
+        assert_equal 30_000, client.instance_variable_get(:@requestor).config.open_timeout
+        # Not set in client options, not imported from global
+        assert_equal client.instance_variable_get(:@requestor).config.api_base, Stripe::DEFAULT_API_BASE
+        assert_equal client.instance_variable_get(:@requestor).config.api_version, Stripe::ApiVersion::CURRENT
+
+        req = nil
+        stub_request(:get, "#{Stripe::DEFAULT_API_BASE}/v1/customers/cus_123")
+          .with { |request| req = request }
+          .to_return(body: JSON.generate(object: "customer"))
+
+        client.v1.customers.retrieve("cus_123")
+
+        # Set in client options
+        assert_equal "Bearer test_123", req.headers["Authorization"]
+        assert_equal "acct_123", req.headers["Stripe-Account"]
+        assert_requested(:get, "#{Stripe::DEFAULT_API_BASE}/v1/customers/cus_123")
+      end
+
       should "request options overrides client config options" do
         client = StripeClient.new("test_123", stripe_version: "2022-11-15", stripe_account: "acct_123")
 

test/stripe/stripe_configuration_test.rb

@@ -97,6 +97,50 @@ class StripeConfigurationTest < Test::Unit::TestCase
       end
     end
 
+    context "client_init" do
+      setup do
+        @client_opts = Hash[StripeClient::CLIENT_OPTIONS.map { |k| [k, nil] }] # rubocop:todo Style/HashConversion - necessary for Ruby <= 2.5
+        @old_api_key = Stripe.api_key
+        @old_stripe_account = Stripe.stripe_account
+        @old_enable_telemetry = Stripe.instance_variable_get(:@enable_telemetry)
+        @old_open_timeout = Stripe.open_timeout
+        @old_uploads_base = Stripe.uploads_base
+      end
+
+      teardown do
+        Stripe.api_key = @old_api_key
+        Stripe.stripe_account = @old_stripe_account
+        Stripe.enable_telemetry = @old_enable_telemetry
+        Stripe.open_timeout = @old_open_timeout
+        Stripe.uploads_base = @old_uploads_base
+      end
+
+      should "correctly set options for the client" do
+        # mix of default, global, client options
+        Stripe.api_key = "global_test_123"
+        Stripe.stripe_account = "global_acct_123"
+        Stripe.enable_telemetry = false
+        Stripe.open_timeout = 30_000
+        Stripe.uploads_base = "global_uploads_base.stripe.com"
+
+        @client_opts[:api_key] = "client_test_123"
+        @client_opts[:stripe_account] = "client_acct_123"
+        @client_opts[:uploads_base] = "client_uploads_base.stripe.com"
+        @client_opts.reject! { |_k, v| v.nil? }
+
+        client_config = Stripe::StripeConfiguration.client_init(@client_opts)
+
+        assert_equal("client_test_123", client_config.api_key) # client api key
+        assert_equal("client_acct_123", client_config.stripe_account) # client stripe account
+        assert_equal(false, client_config.enable_telemetry) # global telemetry
+        assert_equal(30_000, client_config.open_timeout) # global timeout
+        assert_equal("client_uploads_base.stripe.com", client_config.base_addresses[:files]) # client uploads base
+        assert_equal(Stripe::DEFAULT_API_BASE, client_config.base_addresses[:api]) # default api base
+        assert_equal(ApiVersion::CURRENT, client_config.api_version) # default api version
+        assert_equal(Stripe::DEFAULT_CA_BUNDLE_PATH, client_config.ca_bundle_path) # default ca bundle path
+      end
+    end
+
     context "#max_network_retries=" do
       should "coerce the option into an integer" do
         config = Stripe::StripeConfiguration.setup