Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add API resource instance methods to StripeClient #954

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/stripe.rb
Expand Up @@ -31,7 +31,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"
Expand All @@ -40,10 +39,15 @@
require "stripe/singleton_api_resource"
require "stripe/webhook"
require "stripe/stripe_configuration"
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"

Expand Down
2 changes: 1 addition & 1 deletion lib/stripe/api_operations/list.rb
Expand Up @@ -11,7 +11,7 @@ def list(filters = {}, opts = {})

# set filters so that we can fetch the same limit, expansions, and
# predicates when accessing the next and previous pages
obj.filters = filters.dup
obj.filters = filters.dup unless filters.nil?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible for filters to be nil due to how we delegate methods to the API resources.

obj
end
end
Expand Down
107 changes: 107 additions & 0 deletions lib/stripe/client_api_operations.rb
@@ -0,0 +1,107 @@
# 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.
def method_missing(method, *args)
super unless @resource.respond_to?(method)

update_args_with_client!(method, args)

@resource.public_send(method, *args)
end

def respond_to_missing?(symbol, include_private = false)
super unless @resource
@resource.respond_to?(symbol) || super
end

# 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.
#
# Two noteworthy caveats:
# 1) Does not merge into methods that use `_opts` as that means
# the param is unused.
# 2) Preserves incorrect options (e.g. passing nil) so that APIResource
# can handle errors.
def update_args_with_client!(method, args)
opts_pos = @resource.method(method).parameters.index(%i[opt opts])

return unless opts_pos

opts = opts_pos >= args.length ? {} : args[opts_pos]

normalized_opts = Stripe::Util.normalize_opts(opts)
args[opts_pos] = { client: @client }.merge(normalized_opts)
end
Comment on lines +47 to +56
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary regressions that I found during the first PR all had to do with nuances when it came time to merge the options. I was able to identify the scenarios and lock them in under test: https://github.com/stripe/stripe-ruby/pull/954/files#diff-50b82fc5b424d8d4c21c2ddd6c67485e40fbc76fdf5cc1e4320949a4ad20fb62R56

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

# Update `invoiceitems` to match snake case convention
invoice_item_class = api_resources.delete("invoiceitem")
api_resources["invoice_item"] = invoice_item_class
Comment on lines +67 to +69
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brandur-stripe Here's the update to turn invoiceitem into invoice_item that you previously mentioned.


# 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.
proxy = ClientProxy.new(client: nil)
resources.each do |resource_name, resource_class|
method_name = resource_name.split(".").last
proxy.define_singleton_method(method_name) 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.to_sym) do
ClientProxy.new(client: self, resource: resources[0][1])
end
end
end
end
end
end
end
7 changes: 4 additions & 3 deletions 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
{
Comment on lines +10 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOC, why is this change necessary? list is an API object name 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ob-stripe it was done so I could get a list of API resources to dynamically define the methods here: https://github.com/stripe/stripe-ruby/pull/954/files#diff-992f231b0add61aeb46ab11fb68529936809a99a74ed9f1c1755a4b01f57fb58R63

Certainly open to other suggestions!

Account::OBJECT_NAME => Account,
AccountLink::OBJECT_NAME => AccountLink,
AlipayAccount::OBJECT_NAME => AlipayAccount,
Expand Down Expand Up @@ -98,5 +100,4 @@ def self.object_names_to_classes
end
end
end

# rubocop:enable Metrics/MethodLength
2 changes: 2 additions & 0 deletions lib/stripe/stripe_client.rb
Expand Up @@ -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 = Set.new
Expand Down
6 changes: 6 additions & 0 deletions lib/stripe/util.rb
Expand Up @@ -39,10 +39,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
Expand Down
2 changes: 1 addition & 1 deletion test/stripe/account_link_test.rb
Expand Up @@ -5,7 +5,7 @@
module Stripe
class AccountLinkTest < Test::Unit::TestCase
should "be creatable" do
link = Stripe::AccountLink.create(
link = StripeClient.new.account_link.create(
account: "acct_123",
refresh_url: "https://stripe.com/refresh",
return_url: "https://stripe.com/return",
Expand Down