From 3abe003dbcfe84db5653dd56bbaab70739098b8b Mon Sep 17 00:00:00 2001 From: Joel Taylor Date: Sat, 23 May 2020 09:09:18 -0700 Subject: [PATCH] Add API resource instance methods to StripeClient This change introduces a proof-of-concept to add convenience methods to access API resources through a StripeClient for per-client configuration. This first iteration only allows for the `api_key` to be configured but can be extended to allow other options such as `stripe_version`, which should solve #872. The primary workhorse for this feature is a new module called `Stripe::ClientAPIOperations` that defines instance methods on `StripeClient` when it is included. A `ClientProxy` is used to send any method calls to an API resource with the instantiated client injected. There are a few noteworthy aspects of this approach: - Many resources are namespaced, which introduces a unique challenge when it comes to method chaining calls (e.g. client.issuing.authorizations). In order to handle those cases, we create a `ClientProxy` object for the root namespace (e.g., "issuing") and define all resource methods (e.g. "authorizations") at once to avoid re-defining the proxy object when there are multiple resources per namespace. - Sigma deviates from other namespaced API resources and does not have an `OBJECT_NAME` separated by a period. We account for that nuance directly. - `method_missing` is substantially slower than direct calls. Therefore, methods are defined where possible but `method_missing` is still used at the last step when delegating resource methods to the actual resource. --- lib/stripe.rb | 6 +- lib/stripe/client_api_operations.rb | 89 +++++++++++++++++ lib/stripe/object_types.rb | 7 +- lib/stripe/stripe_client.rb | 13 ++- lib/stripe/util.rb | 14 +++ test/stripe/stripe_client_test.rb | 142 ++++++++++++++++++++++++++++ 6 files changed, 263 insertions(+), 8 deletions(-) create mode 100644 lib/stripe/client_api_operations.rb diff --git a/lib/stripe.rb b/lib/stripe.rb index 8a64ba59e..89d4bbe54 100644 --- a/lib/stripe.rb +++ b/lib/stripe.rb @@ -30,7 +30,6 @@ require "stripe/util" require "stripe/connection_manager" require "stripe/multipart_encoder" -require "stripe/stripe_client" require "stripe/stripe_object" require "stripe/stripe_response" require "stripe/list_object" @@ -38,10 +37,15 @@ require "stripe/api_resource" require "stripe/singleton_api_resource" require "stripe/webhook" +require "stripe/client_api_operations" # Named API resources require "stripe/resources" +# StripeClient requires API Resources to be loaded +# due to dynamic methods defined by ClientAPIOperations +require "stripe/stripe_client" + # OAuth require "stripe/oauth" diff --git a/lib/stripe/client_api_operations.rb b/lib/stripe/client_api_operations.rb new file mode 100644 index 000000000..d14de8e72 --- /dev/null +++ b/lib/stripe/client_api_operations.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +module Stripe + # Define instance methods on the including class (i.e. StripeClient) + # to access API resources. + module ClientAPIOperations + # Proxy object to inject the client into API resources. When included, + # all resources are defined as singleton methods on the client in the + # plural form (e.g. Stripe::Account => client.accounts). + class ClientProxy + def initialize(client:, resource: nil) + @client = client + @resource = resource + end + + attr_reader :client + + def with_client(client) + @client = client + self + end + + # Used to either send a method to the API resource or the nested + # ClientProxy when a resource is namespaced. Since the method signature + # differs when operating on a collection versus a singular resource, it's + # required to perform introspection on the parameters to respect any + # passed in options or overrides. + def method_missing(method, *args) + super unless @resource + opts_pos = @resource.method(method).parameters.index(%i[opt opts]) + args[opts_pos] = { client: @client }.merge(args[opts_pos] || {}) + + @resource.public_send(method, *args) || super + end + + def respond_to_missing?(symbol, include_private = false) + super unless @resource + @resource.respond_to?(symbol) || super + end + end + + def self.included(base) + base.class_eval do + # Sigma, unlike other namespaced API objects, is not separated by a + # period so we modify the object name to follow the expected convention. + api_resources = Stripe::Util.api_object_classes + sigma_class = api_resources.delete("scheduled_query_run") + api_resources["sigma.scheduled_query_run"] = sigma_class + + # Group namespaces that have mutiple resourses + grouped_resources = api_resources.group_by do |key, _| + key.include?(".") ? key.split(".").first : key + end + + grouped_resources.each do |resource_namespace, resources| + # Namespace resource names are separated with a period by convention. + if resources[0][0].include?(".") + + # Defines the methods required for chaining calls for resources that + # are namespaced. A proxy object is created so that all resource + # methods can be defined at once. + # + # NOTE: At some point, a smarter pluralization scheme may be + # necessary for resource names with complex pluralization rules. + proxy = ClientProxy.new(client: nil) + resources.each do |resource_name, resource_class| + method_name = resource_name.split(".").last + proxy.define_singleton_method("#{method_name}s") do + ClientProxy.new(client: proxy.client, resource: resource_class) + end + end + + # Defines the first method for resources that are namespaced. By + # convention these methods are singular. A proxy object is returned + # so that the client can be injected along the method chain. + define_method(resource_namespace) do + proxy.with_client(self) + end + else + # Defines plural methods for non-namespaced resources + define_method("#{resource_namespace}s".to_sym) do + ClientProxy.new(client: self, resource: resources[0][1]) + end + end + end + end + end + end +end diff --git a/lib/stripe/object_types.rb b/lib/stripe/object_types.rb index 2e141488e..b01d2a946 100644 --- a/lib/stripe/object_types.rb +++ b/lib/stripe/object_types.rb @@ -1,15 +1,17 @@ # frozen_string_literal: true # rubocop:disable Metrics/MethodLength - module Stripe module ObjectTypes def self.object_names_to_classes { # data structures ListObject::OBJECT_NAME => ListObject, + }.merge(api_object_names_to_classes) + end - # business objects + def self.api_object_names_to_classes + { Account::OBJECT_NAME => Account, AccountLink::OBJECT_NAME => AccountLink, AlipayAccount::OBJECT_NAME => AlipayAccount, @@ -96,5 +98,4 @@ def self.object_names_to_classes end end end - # rubocop:enable Metrics/MethodLength diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 48ee67fcd..95233447a 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -7,6 +7,8 @@ module Stripe # recover both a resource a call returns as well as a response object that # contains information on the HTTP call. class StripeClient + include Stripe::ClientAPIOperations + # A set of all known thread contexts across all threads and a mutex to # synchronize global access to them. @thread_contexts_with_connection_managers = [] @@ -17,11 +19,14 @@ class StripeClient # # Takes a connection manager object for backwards compatibility only, and # that use is DEPRECATED. - def initialize(_connection_manager = nil) + def initialize(user_options = {}) @system_profiler = SystemProfiler.new @last_request_metrics = nil + @options = Util.my_options(user_options) end + attr_reader :options + # Gets a currently active `StripeClient`. Set for the current thread when # `StripeClient#request` is being run so that API operations being executed # inside of that block can find the currently active client. It's reset to @@ -188,7 +193,7 @@ def execute_request(method, path, unless path.is_a?(String) api_base ||= Stripe.api_base - api_key ||= Stripe.api_key + api_key ||= options.api_key params = Util.objects_to_ids(params) check_api_key!(api_key) @@ -747,8 +752,8 @@ def self.maybe_gc_connection_managers headers["Idempotency-Key"] ||= SecureRandom.uuid end - headers["Stripe-Version"] = Stripe.api_version if Stripe.api_version - headers["Stripe-Account"] = Stripe.stripe_account if Stripe.stripe_account + headers["Stripe-Version"] = options.api_version if options.api_version + headers["Stripe-Account"] = options.stripe_account if options.stripe_account user_agent = @system_profiler.user_agent begin diff --git a/lib/stripe/util.rb b/lib/stripe/util.rb index dfb4146ac..497d10274 100644 --- a/lib/stripe/util.rb +++ b/lib/stripe/util.rb @@ -24,6 +24,14 @@ module Util OPTS_USER_SPECIFIED + Set[:client] - Set[:idempotency_key] ).freeze + def self.my_options(c) + OpenStruct.new( + api_key: c[:api_key] || Stripe.api_key, + api_version: c[:api_version] || Stripe.api_version, + stripe_account: c[:stripe_account] || Stripe.stripe_account + ) + end + def self.objects_to_ids(obj) case obj when APIResource @@ -39,10 +47,16 @@ def self.objects_to_ids(obj) end end + # Returns a hash of all Stripe object classes. def self.object_classes @object_classes ||= Stripe::ObjectTypes.object_names_to_classes end + # Returns a hash containling only Stripe API object classes. + def self.api_object_classes + @api_object_classes ||= ::Stripe::ObjectTypes.api_object_names_to_classes + end + def self.object_name_matches_class?(object_name, klass) Util.object_classes[object_name] == klass end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index c00ed510d..55d379be4 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -334,6 +334,148 @@ class StripeClientTest < Test::Unit::TestCase end end + context "API resource instance methods" do + should "define methods for all api resources" do + client = StripeClient.new + + # Update Sigma name to account for nuance + api_resources = Stripe::Util.api_object_classes + sigma_class = api_resources.delete("scheduled_query_run") + api_resources["sigma.scheduled_query_run"] = sigma_class + + api_resources.each do |string, _| + if string.include?(".") + resource_module, resource_name = string.split(".") + + assert client.respond_to?(resource_module), "#{resource_module} not found" + assert client.send(resource_module).respond_to?("#{resource_name}s"), "#{resource_name} not found" + else + assert client.respond_to?("#{string}s"), "#{string} not found" + end + end + end + + should "stripe_version" do + client = StripeClient.new(api_version: "2011-01-01") + account = client.accounts.retrieve("acct_1234") + assert_requested(:get, + "#{Stripe.api_base}/v1/accounts/acct_1234", + headers: { "Stripe-Version" => "2011-01-01" }) + end + should "stripe_account" do + client = StripeClient.new(stripe_account: "foo") + account = client.accounts.retrieve("acct_1234") + assert_requested(:get, + "#{Stripe.api_base}/v1/accounts/acct_1234", + headers: { "Stripe-Account" => "foo" }) + end + + should "make expected request on a singular API resource" do + client = StripeClient.new(api_key: "sk_test_local") + account = client.accounts.retrieve("acct_1234") + assert_requested(:get, + "#{Stripe.api_base}/v1/accounts/acct_1234", + headers: { "Authorization" => "Bearer sk_test_local" }) + assert account.is_a?(Stripe::Account) + end + + should "make expected request on a namespaces API resource" do + client = StripeClient.new(api_key: "sk_test_local") + list = client.radar.value_lists.retrieve("rsl_123") + assert_requested(:get, + "#{Stripe.api_base}/v1/radar/value_lists/rsl_123", + headers: { "Authorization" => "Bearer sk_test_local" }) + assert list.is_a?(Stripe::Radar::ValueList) + end + + should "allow for listing a resource" do + client = StripeClient.new(api_key: "sk_test_local") + accounts = client.accounts.list + assert_requested(:get, + "#{Stripe.api_base}/v1/accounts", + headers: { "Authorization" => "Bearer sk_test_local" }) + assert accounts.data.is_a?(Array) + assert accounts.data[0].is_a?(Stripe::Account) + end + + should "allow for listing a resource with filters" do + client = StripeClient.new(api_key: "sk_test_local") + accounts = client.accounts.list({ limit: 10 }) + assert_requested(:get, + "#{Stripe.api_base}/v1/accounts?limit=10", + headers: { "Authorization" => "Bearer sk_test_local" }) + assert accounts.data.is_a?(Array) + assert accounts.data[0].is_a?(Stripe::Account) + end + + should "allow for deleting a resource" do + client = StripeClient.new(api_key: "sk_test_local") + account = client.accounts.retrieve("acct_123") + account = account.delete + assert_requested :delete, "#{Stripe.api_base}/v1/accounts/#{account.id}" + assert account.is_a?(Stripe::Account) + end + + should "allow for creating a resource" do + client = StripeClient.new(api_key: "sk_test_local") + charge = client.charges.create( + amount: 100, + currency: "USD", + source: "src_123" + ) + assert_requested :post, "#{Stripe.api_base}/v1/charges" + assert charge.is_a?(Stripe::Charge) + end + + should "allow for updating a resource" do + client = StripeClient.new(api_key: "sk_test_local") + account = client.accounts.update("acct_123", metadata: { foo: "bar" }) + assert_requested(:post, + "#{Stripe.api_base}/v1/accounts/acct_123", + headers: { "Authorization" => "Bearer sk_test_local" }, + body: { metadata: { foo: "bar" } }) + assert account.is_a?(Stripe::Account) + end + + should "allow for overrides when operating on a collection" do + client = StripeClient.new(api_key: "sk_test_local") + accounts = client.accounts.list({}, { api_key: "sk_test_other" }) + assert_requested(:get, + "#{Stripe.api_base}/v1/accounts", + headers: { "Authorization" => "Bearer sk_test_other" }) + assert accounts.data.is_a?(Array) + assert accounts.data[0].is_a?(Stripe::Account) + end + + should "allow for overrides when operating on a resource" do + client = StripeClient.new(api_key: "sk_test_local") + account = client.accounts.update("acct_123", + {}, + { api_key: "sk_test_other" }) + assert_requested(:post, + "#{Stripe.api_base}/v1/accounts/acct_123", + headers: { "Authorization" => "Bearer sk_test_other" }) + assert account.is_a?(Stripe::Account) + end + + should "allow for overrides when retrieving a resource" do + client = StripeClient.new(api_key: "sk_test_local") + account = client.accounts.retrieve("acct_123", { api_key: "sk_test_other" }) + assert_requested(:get, "#{Stripe.api_base}/v1/accounts/acct_123", + headers: { "Authorization" => "Bearer sk_test_other" }) + assert account.is_a?(Stripe::Account) + end + + should "allow for retrieving a resource with options" do + client = Stripe::StripeClient.new(api_key: "sk_test_local") + account = client.charges.retrieve(id: "acct_123", expand: ["customer"]) + assert_requested(:get, "#{Stripe.api_base}/v1/charges/acct_123", + headers: { "Authorization" => "Bearer sk_test_local" }, + query: { "expand[]" => "customer" }) + assert account.is_a?(Stripe::Charge) + end + end + context "logging" do setup do # Freeze time for the purposes of the `elapsed` parameter that we