Skip to content

Commit

Permalink
Rename API resource's request method
Browse files Browse the repository at this point in the history
As seen in #928, the `refresh` method doesn't work for an event class.
This is because event has a field called `request`, and it ends up
replacing the `request` method that it inherited from being an API
resource, so when `refresh` tries to make a request, it fails because it
tries to invoke it on the accessor added for the event's property.

Here we give `request` a much more unique name so that it will never
conflict with a property field again, and update all internal references
to use the new name. We use `alias` to make the old name available for
backwards compatibility reasons because its been around for so long that
people are probably calling it.

Fixes #928.
  • Loading branch information
brandur committed Aug 4, 2020
1 parent f240405 commit 6d6c71f
Show file tree
Hide file tree
Showing 17 changed files with 65 additions and 40 deletions.
8 changes: 4 additions & 4 deletions .rubocop.yml
Expand Up @@ -44,10 +44,10 @@ Metrics/ClassLength:
- "test/**/*.rb"

Metrics/MethodLength:
# There's ~2 long methods in `StripeClient`. If we want to truncate those a
# little, we could move this to be closer to ~30 (but the default of 10 is
# probably too short).
Max: 50
# There's ~2 long methods in `StripeClient` and one in `NestedResource`. If
# we want to truncate those a little, we could move this to be closer to ~30
# (but the default of 10 is probably too short).
Max: 55

Metrics/ModuleLength:
Enabled: false
Expand Down
2 changes: 1 addition & 1 deletion lib/stripe/api_operations/create.rb
Expand Up @@ -4,7 +4,7 @@ module Stripe
module APIOperations
module Create
def create(params = {}, opts = {})
resp, opts = request(:post, resource_url, params, opts)
resp, opts = execute_resource_request(:post, resource_url, params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
end
Expand Down
10 changes: 7 additions & 3 deletions lib/stripe/api_operations/delete.rb
Expand Up @@ -15,15 +15,19 @@ module ClassMethods
# * +opts+ - A Hash of additional options (separate from the params /
# object values) to be added to the request. E.g. to allow for an
# idempotency_key to be passed in the request headers, or for the
# api_key to be overwritten. See {APIOperations::Request.request}.
# api_key to be overwritten. See
# {APIOperations::Request.execute_resource_request}.
def delete(id, params = {}, opts = {})
resp, opts = request(:delete, "#{resource_url}/#{id}", params, opts)
resp, opts = execute_resource_request(:delete,
"#{resource_url}/#{id}",
params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
end

def delete(params = {}, opts = {})
resp, opts = request(:delete, resource_url, params, opts)
resp, opts = execute_resource_request(:delete, resource_url,
params, opts)
initialize_from(resp.data, opts)
end

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

resp, opts = request(:get, resource_url, filters, opts)
resp, opts = execute_resource_request(:get, resource_url, filters, opts)
obj = ListObject.construct_from(resp.data, opts)

# set filters so that we can fetch the same limit, expansions, and
Expand Down
11 changes: 6 additions & 5 deletions lib/stripe/api_operations/nested_resource.rb
Expand Up @@ -31,35 +31,36 @@ def nested_resource_class_methods(resource, path: nil, operations: nil,
define_singleton_method(:"create_#{resource}") \
do |id, params = {}, opts = {}|
url = send(resource_url_method, id)
resp, opts = request(:post, url, params, opts)
resp, opts = execute_resource_request(:post, url, params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
when :retrieve
define_singleton_method(:"retrieve_#{resource}") \
do |id, nested_id, opts = {}|
url = send(resource_url_method, id, nested_id)
resp, opts = request(:get, url, {}, opts)
resp, opts = execute_resource_request(:get, url, {}, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
when :update
define_singleton_method(:"update_#{resource}") \
do |id, nested_id, params = {}, opts = {}|
url = send(resource_url_method, id, nested_id)
resp, opts = request(:post, url, params, opts)
resp, opts = execute_resource_request(:post, url, params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
when :delete
define_singleton_method(:"delete_#{resource}") \
do |id, nested_id, params = {}, opts = {}|
url = send(resource_url_method, id, nested_id)
resp, opts = request(:delete, url, params, opts)
resp, opts = execute_resource_request(:delete, url, params,
opts)
Util.convert_to_stripe_object(resp.data, opts)
end
when :list
define_singleton_method(:"list_#{resource_plural}") \
do |id, params = {}, opts = {}|
url = send(resource_url_method, id)
resp, opts = request(:get, url, params, opts)
resp, opts = execute_resource_request(:get, url, params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
else
Expand Down
22 changes: 19 additions & 3 deletions lib/stripe/api_operations/request.rb
Expand Up @@ -4,7 +4,8 @@ module Stripe
module APIOperations
module Request
module ClassMethods
def request(method, url, params = {}, opts = {})
def execute_resource_request(method, url,
params = {}, opts = {})
params ||= {}

error_on_invalid_params(params)
Expand Down Expand Up @@ -36,6 +37,17 @@ def request(method, url, params = {}, opts = {})
[resp, opts_to_persist]
end

# This method used to be called `request`, but it's such a short name
# that it eventually conflicted with the name of a field on an API
# resource (specifically, `Event#request`), so it was renamed to
# something more unique.
#
# The former name had been around for just about forever though, and
# although all internal uses have been renamed, I've left this alias in
# place for backwards compatibility. Consider removing it on the next
# major.
alias request execute_resource_request

private def error_on_non_string_user_opts(opts)
Util::OPTS_USER_SPECIFIED.each do |opt|
next unless opts.key?(opt)
Expand Down Expand Up @@ -71,10 +83,14 @@ def self.included(base)
base.extend(ClassMethods)
end

protected def request(method, url, params = {}, opts = {})
protected def execute_resource_request(method, url,
params = {}, opts = {})
opts = @opts.merge(Util.normalize_opts(opts))
self.class.request(method, url, params, opts)
self.class.execute_resource_request(method, url, params, opts)
end

# See notes on `alias` above.
alias request execute_resource_request
end
end
end
11 changes: 7 additions & 4 deletions lib/stripe/api_operations/save.rb
Expand Up @@ -15,15 +15,17 @@ module ClassMethods
# * +opts+ - A Hash of additional options (separate from the params /
# object values) to be added to the request. E.g. to allow for an
# idempotency_key to be passed in the request headers, or for the
# api_key to be overwritten. See {APIOperations::Request.request}.
# api_key to be overwritten. See
# {APIOperations::Request.execute_resource_request}.
def update(id, params = {}, opts = {})
params.each_key do |k|
if protected_fields.include?(k)
raise ArgumentError, "Cannot update protected field: #{k}"
end
end

resp, opts = request(:post, "#{resource_url}/#{id}", params, opts)
resp, opts = execute_resource_request(:post, "#{resource_url}/#{id}",
params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
end
Expand All @@ -43,7 +45,8 @@ def update(id, params = {}, opts = {})
# * +opts+ - A Hash of additional options (separate from the params /
# object values) to be added to the request. E.g. to allow for an
# idempotency_key to be passed in the request headers, or for the
# api_key to be overwritten. See {APIOperations::Request.request}.
# api_key to be overwritten. See
# {APIOperations::Request.execute_resource_request}.
def save(params = {}, opts = {})
# We started unintentionally (sort of) allowing attributes sent to
# +save+ to override values used during the update. So as not to break
Expand All @@ -59,7 +62,7 @@ def save(params = {}, opts = {})
# generated a uri for this object with an identifier baked in
values.delete(:id)

resp, opts = request(:post, save_url, values, opts)
resp, opts = execute_resource_request(:post, save_url, values, opts)
initialize_from(resp.data, opts)
end

Expand Down
7 changes: 4 additions & 3 deletions lib/stripe/api_resource.rb
Expand Up @@ -76,7 +76,7 @@ def self.custom_method(name, http_verb:, http_path: nil)
end

url = "#{resource_url}/#{CGI.escape(id)}/#{CGI.escape(http_path)}"
resp, opts = request(http_verb, url, params, opts)
resp, opts = execute_resource_request(http_verb, url, params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
end
Expand All @@ -93,7 +93,8 @@ def resource_url
end

def refresh
resp, opts = request(:get, resource_url, @retrieve_params)
resp, opts = execute_resource_request(:get, resource_url,
@retrieve_params)
initialize_from(resp.data, opts)
end

Expand All @@ -105,7 +106,7 @@ def self.retrieve(id, opts = {})
end

protected def request_stripe_object(method:, path:, params:, opts: {})
resp, opts = request(method, path, params, opts)
resp, opts = execute_resource_request(method, path, params, opts)

# If we're getting back this thing, update; otherwise, instantiate.
if Util.object_name_matches_class?(resp.data[:object], self.class)
Expand Down
4 changes: 2 additions & 2 deletions lib/stripe/list_object.rb
Expand Up @@ -92,8 +92,8 @@ def empty?

def retrieve(id, opts = {})
id, retrieve_params = Util.normalize_id(id)
resp, opts = request(:get, "#{resource_url}/#{CGI.escape(id)}",
retrieve_params, opts)
url = "#{resource_url}/#{CGI.escape(id)}"
resp, opts = execute_resource_request(:get, url, retrieve_params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end

Expand Down
6 changes: 3 additions & 3 deletions lib/stripe/oauth.rb
Expand Up @@ -5,7 +5,7 @@ module OAuth
module OAuthOperations
extend APIOperations::Request::ClassMethods

def self.request(method, url, params, opts)
def self.execute_resource_request(method, url, params, opts)
opts = Util.normalize_opts(opts)
opts[:client] ||= StripeClient.active_client
opts[:api_base] ||= Stripe.connect_base
Expand Down Expand Up @@ -44,7 +44,7 @@ def self.authorize_url(params = {}, opts = {})
def self.token(params = {}, opts = {})
opts = Util.normalize_opts(opts)
opts[:api_key] = params[:client_secret] if params[:client_secret]
resp, opts = OAuthOperations.request(
resp, opts = OAuthOperations.execute_resource_request(
:post, "/oauth/token", params, opts
)
# This is just going to return a generic StripeObject, but that's okay
Expand All @@ -54,7 +54,7 @@ def self.token(params = {}, opts = {})
def self.deauthorize(params = {}, opts = {})
opts = Util.normalize_opts(opts)
params[:client_id] = get_client_id(params)
resp, opts = OAuthOperations.request(
resp, opts = OAuthOperations.execute_resource_request(
:post, "/oauth/deauthorize", params, opts
)
# This is just going to return a generic StripeObject, but that's okay
Expand Down
2 changes: 1 addition & 1 deletion lib/stripe/resources/account.rb
Expand Up @@ -65,7 +65,7 @@ def self.retrieve(id = ARGUMENT_NOT_PROVIDED, opts = {})
end

def persons(params = {}, opts = {})
resp, opts = request(:get, resource_url + "/persons", params, opts)
resp, opts = execute_resource_request(:get, resource_url + "/persons", params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end

Expand Down
2 changes: 1 addition & 1 deletion lib/stripe/resources/bank_account.rb
Expand Up @@ -10,7 +10,7 @@ class BankAccount < APIResource
OBJECT_NAME = "bank_account"

def verify(params = {}, opts = {})
resp, opts = request(:post, resource_url + "/verify", params, opts)
resp, opts = execute_resource_request(:post, resource_url + "/verify", params, opts)
initialize_from(resp.data, opts)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/stripe/resources/credit_note.rb
Expand Up @@ -21,12 +21,12 @@ def void_credit_note(params = {}, opts = {})
end

def self.preview(params, opts = {})
resp, opts = request(:get, resource_url + "/preview", params, opts)
resp, opts = execute_resource_request(:get, resource_url + "/preview", params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end

def self.list_preview_line_items(params, opts = {})
resp, opts = request(:get, resource_url + "/preview/lines", params, opts)
resp, opts = execute_resource_request(:get, resource_url + "/preview/lines", params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/stripe/resources/customer.rb
Expand Up @@ -29,7 +29,7 @@ class << self
end

def delete_discount
resp, opts = request(:delete, resource_url + "/discount")
resp, opts = execute_resource_request(:delete, resource_url + "/discount")
initialize_from(resp.data, opts, true)
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/stripe/resources/invoice.rb
Expand Up @@ -62,12 +62,12 @@ def void_invoice(params = {}, opts = {})
end

def self.upcoming(params, opts = {})
resp, opts = request(:get, resource_url + "/upcoming", params, opts)
resp, opts = execute_resource_request(:get, resource_url + "/upcoming", params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end

def self.list_upcoming_line_items(params, opts = {})
resp, opts = request(:get, resource_url + "/upcoming/lines", params, opts)
resp, opts = execute_resource_request(:get, resource_url + "/upcoming/lines", params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/stripe/resources/source.rb
Expand Up @@ -32,13 +32,13 @@ def detach(params = {}, opts = {})

url = "#{Customer.resource_url}/#{CGI.escape(customer)}/sources" \
"/#{CGI.escape(id)}"
resp, opts = request(:delete, url, params, opts)
resp, opts = execute_resource_request(:delete, url, params, opts)
initialize_from(resp.data, opts)
end

def source_transactions(params = {}, opts = {})
resp, opts = request(:get, resource_url + "/source_transactions", params,
opts)
resp, opts = execute_resource_request(:get, resource_url + "/source_transactions", params,
opts)
Util.convert_to_stripe_object(resp.data, opts)
end
extend Gem::Deprecate
Expand Down
2 changes: 1 addition & 1 deletion lib/stripe/resources/subscription_item.rb
Expand Up @@ -17,7 +17,7 @@ class SubscriptionItem < APIResource
resource_plural: "usage_record_summaries"

def usage_record_summaries(params = {}, opts = {})
resp, opts = request(:get, resource_url + "/usage_record_summaries", params, opts)
resp, opts = execute_resource_request(:get, resource_url + "/usage_record_summaries", params, opts)
Util.convert_to_stripe_object(resp.data, opts)
end
extend Gem::Deprecate
Expand Down

0 comments on commit 6d6c71f

Please sign in to comment.