From 68d202b2be3daf5be6a63b33cd9b92f63594b054 Mon Sep 17 00:00:00 2001 From: Brandur Date: Fri, 5 Jul 2019 10:44:31 -0700 Subject: [PATCH 01/18] Convert library to use built-in `Net::HTTP` Moves the library off of Faraday and over onto the standard library's built-in `Net::HTTP` module. The upside of the transition is that we break away from a few dependencies that have caused us a fair bit of trouble in the past, the downside is that we need more of our own code to do things (although surprisingly, not that much more). The biggest new pieces are: * `ConnectionManager`: A per-thread class that manages a connection to each Stripe infrastructure URL (like `api.stripe.com`, `connect.stripe.com`, etc.) so that we can reuse them between requests. It's also responsible for setting up and configuring new `Net::HTTP` connections, which is a little more heavyweight code-wise compared to other libraries. All of this could have lived in `StripeClient`, but I extracted it because that class has gotten so big. * `MultipartEncoder`: A class that does multipart form encoding for file uploads. Unfortunately, Ruby doesn't bundle anything like this. I built this by referencing the Go implementation because the original RFC is not very detailed or well-written. I also made sure that it was behaving similarly to our other custom implementations like stripe-node, and that it can really upload a file outside the test suite. There's some risk here in that it's easy to miss something across one of these big transitions. I've tried to test out various error cases through tests, but also by leaving scripts running as I terminate my network connection and bring it back. That said, we'd certainly release on a major version bump because some of the interface (like setting `Stripe.default_client`) changes. --- .rubocop.yml | 6 + .rubocop_todo.yml | 17 +- README.md | 11 +- Rakefile | 15 +- lib/stripe.rb | 4 +- lib/stripe/connection_manager.rb | 122 ++++++++++ lib/stripe/multipart_encoder.rb | 131 +++++++++++ lib/stripe/resources/file.rb | 7 +- lib/stripe/stripe_client.rb | 295 ++++++++++++------------- lib/stripe/stripe_response.rb | 74 +++++-- stripe.gemspec | 3 - test/stripe/connection_manager_test.rb | 121 ++++++++++ test/stripe/file_test.rb | 10 - test/stripe/file_upload_test.rb | 10 - test/stripe/multipart_encoder_test.rb | 130 +++++++++++ test/stripe/stripe_client_test.rb | 73 ++++-- test/stripe/stripe_response_test.rb | 94 ++++++-- test/test_helper.rb | 13 +- 18 files changed, 865 insertions(+), 271 deletions(-) create mode 100644 lib/stripe/connection_manager.rb create mode 100644 lib/stripe/multipart_encoder.rb create mode 100644 test/stripe/connection_manager_test.rb create mode 100644 test/stripe/multipart_encoder_test.rb diff --git a/.rubocop.yml b/.rubocop.yml index 9ed7e7fc0..e7130b785 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -10,6 +10,12 @@ Layout/CaseIndentation: Layout/IndentArray: EnforcedStyle: consistent +# This can be re-enabled once we're 2.3+ only and can use the squiggly heredoc +# operator. Prior to that, Rubocop recommended bringing in a library like +# ActiveSupport to get heredoc indentation, which is just terrible. +Layout/IndentHeredoc: + Enabled: false + Layout/IndentHash: EnforcedStyle: consistent diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 955bc632c..73cee4a73 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -6,19 +6,28 @@ # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. +Layout/ClosingHeredocIndentation: + # As far as I can tell, this lint rule just doesn't work when it comes to + # heredoc in `it` blocks. + Exclude: + - 'test/**/*' + # Offense count: 20 Metrics/AbcSize: Max: 53 -# Offense count: 31 -# Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: - Max: 498 + Max: 39 + + # Test `context` is a block and generates all kinds of false positives + # without exclusion. + Exclude: + - 'test/**/*' # Offense count: 11 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 673 + Max: 700 # Offense count: 12 Metrics/CyclomaticComplexity: diff --git a/README.md b/README.md index 3dd19af0f..2e4a79cb6 100644 --- a/README.md +++ b/README.md @@ -112,15 +112,13 @@ Stripe::Charge.retrieve( ) ``` -### Configuring a Client +### Accessing a response object -While a default HTTP client is used by default, it's also possible to have the -library use any client supported by [Faraday][faraday] by initializing a -`Stripe::StripeClient` object and giving it a connection: +Get access to response objects by initializing a client and using its `request` +method: ```ruby -conn = Faraday.new -client = Stripe::StripeClient.new(conn) +client = Stripe::StripeClient.new charge, resp = client.request do Stripe::Charge.retrieve( "ch_18atAXCdGbJFKhCuBAa4532Z", @@ -275,7 +273,6 @@ Update the bundled [stripe-mock] by editing the version number found in [api-keys]: https://dashboard.stripe.com/account/apikeys [connect]: https://stripe.com/connect [curl]: http://curl.haxx.se/docs/caextract.html -[faraday]: https://github.com/lostisland/faraday [idempotency-keys]: https://stripe.com/docs/api/ruby#idempotent_requests [stripe-mock]: https://github.com/stripe/stripe-mock [versioning]: https://stripe.com/docs/api/ruby#versioning diff --git a/Rakefile b/Rakefile index 182a56399..4b3c61cfa 100644 --- a/Rakefile +++ b/Rakefile @@ -13,7 +13,8 @@ RuboCop::RakeTask.new desc "Update bundled certs" task :update_certs do - require "faraday" + require "net/http" + require "uri" fetch_file "https://curl.haxx.se/ca/cacert.pem", ::File.expand_path("../lib/data/ca-certificates.crt", __FILE__) @@ -23,14 +24,14 @@ end # helpers # -def fetch_file(url, dest) +def fetch_file(uri, dest) ::File.open(dest, "w") do |file| - resp = Faraday.get(url) - unless resp.status == 200 - abort("bad response when fetching: #{url}\n" \ - "Status #{resp.status}: #{resp.body}") + resp = Net::HTTP.get_response(URI.parse(uri)) + unless resp.code.to_i == 200 + abort("bad response when fetching: #{uri}\n" \ + "Status #{resp.code}: #{resp.body}") end file.write(resp.body) - puts "Successfully fetched: #{url}" + puts "Successfully fetched: #{uri}" end end diff --git a/lib/stripe.rb b/lib/stripe.rb index 251cb781c..17b67b9ae 100644 --- a/lib/stripe.rb +++ b/lib/stripe.rb @@ -3,9 +3,9 @@ # Stripe Ruby bindings # API spec at https://stripe.com/docs/api require "cgi" -require "faraday" require "json" require "logger" +require "net/http" require "openssl" require "rbconfig" require "securerandom" @@ -28,6 +28,8 @@ require "stripe/errors" require "stripe/object_types" require "stripe/util" +require "stripe/connection_manager" +require "stripe/multipart_encoder" require "stripe/stripe_client" require "stripe/stripe_object" require "stripe/stripe_response" diff --git a/lib/stripe/connection_manager.rb b/lib/stripe/connection_manager.rb new file mode 100644 index 000000000..30bea4ab5 --- /dev/null +++ b/lib/stripe/connection_manager.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +module Stripe + # Manages connections across multiple hosts which is useful because the + # library may connect to multiple hosts during a typical session (main API, + # Connect, Uploads). Ruby doesn't provide an easy way to make this happen + # easily, so this class is designed to track what we're connected to and + # manage the lifecycle of those connections. + # + # Note that this class in itself is *not* thread safe. We expect it to be + # instantiated once per thread. + # + # Note also that this class doesn't currently clean up after itself because + # it expects to only ever have a few connections. It'd be possible to tank + # memory by constantly changing the value of `Stripe.api_base` or the like. A + # possible improvement might be to detect and prune old connections whenever + # a request is executed. + class ConnectionManager + def initialize + @active_connections = {} + end + + # Gets a connection for a given URI. This is for internal use only as it's + # subject to change (we've moved between HTTP client schemes in the past + # and may do it again). + # + # `uri` is expected to be a string. + def connection_for(uri) + u = URI.parse(uri) + connection = @active_connections[[u.host, u.port]] + + if connection.nil? + connection = create_connection(u) + + # TODO: what happens after TTL? + connection.start + + @active_connections[[u.host, u.port]] = connection + end + + connection + end + + # Executes an HTTP request to the given URI with the given method. Also + # allows a request body, headers, and query string to be specified. + def execute_request(method, uri, body: nil, headers: nil, query: nil) + # Perform some basic argument validation because it's easy to get + # confused between strings and hashes for things like body and query + # parameters. + raise ArgumentError, "method should be a symbol" \ + unless method.is_a?(Symbol) + raise ArgumentError, "uri should be a string" \ + unless uri.is_a?(String) + raise ArgumentError, "body should be a string" \ + if body && !body.is_a?(String) + raise ArgumentError, "headers should be a hash" \ + if headers && !headers.is_a?(Hash) + raise ArgumentError, "query should be a string" \ + if query && !query.is_a?(String) + + connection = connection_for(uri) + + u = URI.parse(uri) + path = if query + u.path + "?" + query + else + u.path + end + + connection.send_request(method.to_s.upcase, path, body, headers) + end + + # + # private + # + + # `uri` should be a parsed `URI` object. + private def create_connection(uri) + # These all come back as `nil` if no proxy is configured. + proxy_host, proxy_port, proxy_user, proxy_pass = proxy_parts + + connection = Net::HTTP.new(uri.host, uri.port, + proxy_host, proxy_port, + proxy_user, proxy_pass) + + connection.open_timeout = Stripe.open_timeout + connection.read_timeout = Stripe.read_timeout + + connection.use_ssl = uri.scheme == "https" + + if Stripe.verify_ssl_certs + connection.verify_mode = OpenSSL::SSL::VERIFY_PEER + connection.cert_store = Stripe.ca_store + else + connection.verify_mode = OpenSSL::SSL::VERIFY_NONE + + unless @verify_ssl_warned + @verify_ssl_warned = true + warn("WARNING: Running without SSL cert verification. " \ + "You should never do this in production. " \ + "Execute `Stripe.verify_ssl_certs = true` to enable " \ + "verification.") + end + end + + connection + end + + # `Net::HTTP` somewhat awkwardly requires each component of a proxy URI + # (host, port, etc.) rather than the URI itself. This method simply parses + # out those pieces to make passing them into a new connection a little less + # ugly. + private def proxy_parts + if Stripe.proxy.nil? + [nil, nil, nil, nil] + else + u = URI.parse(Stripe.proxy) + [u.host, u.port, u.user, u.password] + end + end + end +end diff --git a/lib/stripe/multipart_encoder.rb b/lib/stripe/multipart_encoder.rb new file mode 100644 index 000000000..fbfde51dc --- /dev/null +++ b/lib/stripe/multipart_encoder.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +require "securerandom" +require "tempfile" + +module Stripe + # Encodes parameters into a `multipart/form-data` payload as described by RFC + # 2388: + # + # https://tools.ietf.org/html/rfc2388 + # + # This is most useful for transferring file-like objects. + # + # Parameters should be added with `#encode`. When ready, use `#body` to get + # the encoded result and `#content_type` to get the value that should be + # placed in the `Content-Type` header of a subsequent request (which includes + # a boundary value). + class MultipartEncoder + MULTIPART_FORM_DATA = "multipart/form-data".freeze + + # A shortcut for encoding a single set of parameters and finalizing a + # result. + # + # Returns an encoded body and the value that should be set in the content + # type header of a subsequent request. + def self.encode(params) + encoder = MultipartEncoder.new + encoder.encode(params) + encoder.close + [encoder.body, encoder.content_type] + end + + # Gets the object's randomly generated boundary string. + attr_reader :boundary + + # Initializes a new multipart encoder. + def initialize + # We seed this with an empty string so that it encoding defaults to UTF-8 + # instead of ASCII. The empty string is UTF-8 and new string inherits the + # encoding of the string it's seeded with. + @body = String.new("") + + # Chose the same number of random bytes that Go uses in its standard + # library implementation. Easily enough entropy to ensure that it won't + # be present in a file we're sending. + @boundary = SecureRandom.hex(30) + + @closed = false + @first_field = true + end + + # Gets the encoded body. `#close` must be called first. + def body + raise "object must be closed before getting body" unless @closed + @body + end + + # Finalizes the object by writing the final boundary. + def close + raise "object already closed" if @closed + + @body << "\r\n" + @body << "--#{@boundary}--" + + @closed = true + + nil + end + + # Gets the value including boundary that should be put into a multipart + # request's `Content-Type`. + def content_type + "#{MULTIPART_FORM_DATA}; boundary=#{@boundary}" + end + + # Encodes a set of parameters to the body. + # + # Note that parameters are expected to be a hash, but a "flat" hash such + # that complex substructures like hashes and arrays have already been + # appropriately Stripe-encoded. Pass a complex structure through + # `Util.flatten_params` first before handing it off to this method. + def encode(params) + raise "no more parameters can be written to closed object" if @closed + + params.each do |name, val| + if val.is_a?(::File) || val.is_a?(::Tempfile) + write_field(name, val.read, filename: ::File.basename(val.path)) + elsif val.respond_to?(:read) + write_field(name, val.read, filename: "blob") + else + write_field(name, val, filename: nil) + end + end + + nil + end + + # + # private + # + + # Escapes double quotes so that the given value can be used in a + # double-quoted string and replaces any linebreak characters with spaces. + private def escape(str) + str.gsub('"', "%22").tr("\n", " ").tr("\r", " ") + end + + private def write_field(name, data, filename:) + if !@first_field + @body << "\r\n" + else + @first_field = false + end + + @body << "--#{@boundary}\r\n" + + if filename + @body << %(Content-Disposition: form-data) + + %(; name="#{escape(name.to_s)}") + + %(; filename="#{escape(filename)}"\r\n) + @body << %(Content-Type: application/octet-stream\r\n) + else + @body << %(Content-Disposition: form-data) + + %(; name="#{escape(name.to_s)}"\r\n) + end + + @body << "\r\n" + @body << data.to_s + end + end +end diff --git a/lib/stripe/resources/file.rb b/lib/stripe/resources/file.rb index 5880da928..ff9cff6e3 100644 --- a/lib/stripe/resources/file.rb +++ b/lib/stripe/resources/file.rb @@ -18,20 +18,15 @@ def self.resource_url end def self.create(params = {}, opts = {}) - # rest-client would accept a vanilla `File` for upload, but Faraday does - # not. Support the old API by wrapping a `File`-like object with an - # `UploadIO` object if we're given one. if params[:file] && !params[:file].is_a?(String) unless params[:file].respond_to?(:read) raise ArgumentError, "file must respond to `#read`" end - - params[:file] = Faraday::UploadIO.new(params[:file], nil) end opts = { api_base: Stripe.uploads_base, - content_type: "multipart/form-data", + content_type: MultipartEncoder::MULTIPART_FORM_DATA, }.merge(Util.normalize_opts(opts)) super end diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 6162ff479..126c5d96e 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -5,66 +5,35 @@ module Stripe # recover both a resource a call returns as well as a response object that # contains information on the HTTP call. class StripeClient - attr_accessor :conn + attr_accessor :connection_manager - # Initializes a new StripeClient. Expects a Faraday connection object, and - # uses a default connection unless one is passed. - def initialize(conn = nil) - self.conn = conn || self.class.default_conn + # Initializes a new `StripeClient`. Expects a `ConnectionManager` object, + # and uses a default connection manager unless one is passed. + def initialize(connection_manager = nil) + self.connection_manager = connection_manager || + self.class.default_connection_manager @system_profiler = SystemProfiler.new @last_request_metrics = nil end + # For internal use: sets a value on the current thread's store 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 + # the original value (hopefully `nil`) after the block ends. def self.active_client Thread.current[:stripe_client] || default_client end + # A default client for the current thread. def self.default_client Thread.current[:stripe_client_default_client] ||= - StripeClient.new(default_conn) + StripeClient.new(default_connection_manager) end - # A default Faraday connection to be used when one isn't configured. This - # object should never be mutated, and instead instantiating your own - # connection and wrapping it in a StripeClient object should be preferred. - def self.default_conn - # We're going to keep connections around so that we can take advantage - # of connection re-use, so make sure that we have a separate connection - # object per thread. - Thread.current[:stripe_client_default_conn] ||= begin - conn = Faraday.new do |builder| - builder.use Faraday::Request::Multipart - builder.use Faraday::Request::UrlEncoded - builder.use Faraday::Response::RaiseError - - # Net::HTTP::Persistent doesn't seem to do well on Windows or JRuby, - # so fall back to default there. - if Gem.win_platform? || RUBY_PLATFORM == "java" - builder.adapter :net_http - else - builder.adapter :net_http_persistent - end - end - - conn.proxy = Stripe.proxy if Stripe.proxy - - if Stripe.verify_ssl_certs - conn.ssl.verify = true - conn.ssl.cert_store = Stripe.ca_store - else - conn.ssl.verify = false - - unless @verify_ssl_warned - @verify_ssl_warned = true - warn("WARNING: Running without SSL cert verification. " \ - "You should never do this in production. " \ - "Execute `Stripe.verify_ssl_certs = true` to enable " \ - "verification.") - end - end - - conn - end + # A default connection manager for the current thread. + def self.default_connection_manager + Thread.current[:stripe_client_default_connection_manager] ||= + ConnectionManager.new end # Checks if an error is a problem that we should retry on. This includes @@ -74,16 +43,18 @@ def self.should_retry?(error, num_retries) return false if num_retries >= Stripe.max_network_retries # Retry on timeout-related problems (either on open or read). - return true if error.is_a?(Faraday::TimeoutError) + return true if error.is_a?(Net::OpenTimeout) + return true if error.is_a?(Net::ReadTimeout) # Destination refused the connection, the connection was reset, or a # variety of other connection failures. This could occur from a single # saturated server, so retry in case it's intermittent. - return true if error.is_a?(Faraday::ConnectionFailed) + return true if error.is_a?(Errno::ECONNREFUSED) + return true if error.is_a?(SocketError) - if error.is_a?(Faraday::ClientError) && error.response + if error.is_a?(Stripe::StripeError) # 409 conflict - return true if error.response[:status] == 409 + return true if error.http_status == 409 end false @@ -134,19 +105,18 @@ def execute_request(method, path, check_api_key!(api_key) - body = nil + body_params = nil query_params = nil case method.to_s.downcase.to_sym when :get, :head, :delete query_params = params else - body = params + body_params = params end # This works around an edge case where we end up with both query # parameters in `query_params` and query parameters that are appended - # onto the end of the given path. In this case, Faraday will silently - # discard the URL's parameters which may break a request. + # onto the end of the given path. # # Here we decode any parameters that were added onto the end of a path # and add them to `query_params` so that all parameters end up in one @@ -162,38 +132,44 @@ def execute_request(method, path, headers = request_headers(api_key, method) .update(Util.normalize_headers(headers)) - params_encoder = FaradayStripeEncoder.new url = api_url(path, api_base) + query = query_params ? Util.encode_parameters(query_params) : nil + + # Encoding body parameters is a little more complex because we may have + # to send a multipart-encoded body. `body_log` is produced separately as + # a variant of the encoded form that's output-friendly. e.g. Doesn't show + # as multipart. File objects are displayed as such instead of as their + # file contents. + body, body_log = if body_params + encode_body(body_params, headers) + else + [nil, nil] + end + # stores information on the request we're about to make so that we don't # have to pass as many parameters around for logging. context = RequestLogContext.new context.account = headers["Stripe-Account"] context.api_key = api_key context.api_version = headers["Stripe-Version"] - context.body = body ? params_encoder.encode(body) : nil + context.body = body_log context.idempotency_key = headers["Idempotency-Key"] context.method = method context.path = path - context.query_params = if query_params - params_encoder.encode(query_params) - end + context.query_params = query - # note that both request body and query params will be passed through - # `FaradayStripeEncoder` http_resp = execute_request_with_rescues(api_base, context) do - conn.run_request(method, url, body, headers) do |req| - req.options.open_timeout = Stripe.open_timeout - req.options.params_encoder = params_encoder - req.options.timeout = Stripe.read_timeout - req.params = query_params unless query_params.nil? - end + connection_manager.execute_request(method, url, + body: body, + headers: headers, + query: query) end begin - resp = StripeResponse.from_faraday_response(http_resp) + resp = StripeResponse.from_net_http(http_resp) rescue JSON::ParserError - raise general_api_error(http_resp.status, http_resp.body) + raise general_api_error(http_resp.code.to_i, http_resp.body) end # Allows StripeClient#request to return a response object to a caller. @@ -201,41 +177,52 @@ def execute_request(method, path, [resp, api_key] end - # Used to workaround buggy behavior in Faraday: the library will try to - # reshape anything that we pass to `req.params` with one of its default - # encoders. I don't think this process is supposed to be lossy, but it is - # -- in particular when we send our integer-indexed maps (i.e. arrays), - # Faraday ends up stripping out the integer indexes. # - # We work around the problem by implementing our own simplified encoder and - # telling Faraday to use that. + # private # - # The class also performs simple caching so that we don't have to encode - # parameters twice for every request (once to build the request and once - # for logging). - # - # When initialized with `multipart: true`, the encoder just inspects the - # hash instead to get a decent representation for logging. In the case of a - # multipart request, Faraday won't use the result of this encoder. - class FaradayStripeEncoder - def initialize - @cache = {} - end - # This is quite subtle, but for a `multipart/form-data` request Faraday - # will throw away the result of this encoder and build its body. - def encode(hash) - @cache.fetch(hash) do |k| - @cache[k] = Util.encode_parameters(hash) - end - end - - # We should never need to do this so it's not implemented. - def decode(_str) - raise NotImplementedError, - "#{self.class.name} does not implement #decode" - end - end + ERROR_MESSAGE_CONNECTION = + "Unexpected error communicating when trying to connect to " \ + "Stripe (%s). You may be seeing this message because your DNS is not " \ + "working or you don't have an internet connection. To check, try " \ + "running `host stripe.com` from the command line.".freeze + ERROR_MESSAGE_SSL = + "Could not establish a secure connection to Stripe (%s), you " \ + "may need to upgrade your OpenSSL version. To check, try running " \ + "`openssl s_client -connect api.stripe.com:443` from the command " \ + "line.".freeze + + # Common error suffix sared by both connect and read timeout messages. + ERROR_MESSAGE_TIMEOUT_SUFFIX = + "Please check your internet connection and try again. " \ + "If this problem persists, you should check Stripe's service " \ + "status at https://status.stripe.com, or let us know at " \ + "support@stripe.com.".freeze + + ERROR_MESSAGE_TIMEOUT_CONNECT = ( + "Timed out connecting to Stripe (%s). " + + ERROR_MESSAGE_TIMEOUT_SUFFIX + ).freeze + + ERROR_MESSAGE_TIMEOUT_READ = ( + "Timed out communicating with Stripe (%s). " + + ERROR_MESSAGE_TIMEOUT_SUFFIX + ).freeze + + # Maps types of exceptions that we're likely to see during a network + # request to more user-friendly messages that we put in front of people. + # The original error message is also appended onto the final exception for + # full transparency. + NETWORK_ERROR_MESSAGES_MAP = { + Errno::ECONNREFUSED => ERROR_MESSAGE_CONNECTION, + SocketError => ERROR_MESSAGE_CONNECTION, + + Net::OpenTimeout => ERROR_MESSAGE_TIMEOUT_CONNECT, + Net::ReadTimeout => ERROR_MESSAGE_TIMEOUT_READ, + + OpenSSL::SSL::SSLError => ERROR_MESSAGE_SSL, + }.freeze + private_constant :NETWORK_ERROR_MESSAGES_MAP private def api_url(url = "", api_base = nil) (api_base || Stripe.api_base) + url @@ -258,14 +245,48 @@ def decode(_str) "email support@stripe.com if you have any questions.)" end + # Encodes a set of body parameters using multipart if `Content-Type` is set + # for that, or standard form-encoding otherwise. Returns the encoded body + # and a version of the encoded body that's safe to be logged. + private def encode_body(body_params, headers) + body = nil + flattened_params = Util.flatten_params(body_params) + + if headers["Content-Type"] == MultipartEncoder::MULTIPART_FORM_DATA + body, content_type = MultipartEncoder.encode(flattened_params) + + # Set a new content type that also includes the multipart boundary. + # See `MultipartEncoder` for details. + headers["Content-Type"] = content_type + + # `#to_s` any complex objects like files and the like to build output + # that's more condusive to logging. + flattened_params = + flattened_params.map { |k, v| [k, v.is_a?(String) ? v : v.to_s] }.to_h + else + body = Util.encode_parameters(body_params) + end + + # We don't use `Util.encode_parameters` partly as an optimization (to not + # redo work we've already done), and partly because the encoded forms of + # certain characters introduce a lot of visual noise and it's nice to + # have a clearer format for logs. + body_log = flattened_params.map { |k, v| "#{k}=#{v}" }.join("&") + + [body, body_log] + end + private def execute_request_with_rescues(api_base, context) num_retries = 0 begin request_start = Time.now log_request(context, num_retries) resp = yield - context = context.dup_from_response(resp) - log_response(context, request_start, resp.status, resp.body) + context = context.dup_from_response_headers(resp) + + handle_error_response(resp, context) if resp.code.to_i >= 400 + + log_response(context, request_start, resp.code.to_i, resp.body) if Stripe.enable_telemetry? && context.request_id request_duration_ms = ((Time.now - request_start) * 1000).to_int @@ -281,10 +302,10 @@ def decode(_str) # taint the original on a retry. error_context = context - if e.respond_to?(:response) && e.response - error_context = context.dup_from_response(e.response) + if e.is_a?(Stripe::StripeError) + error_context = context.dup_from_response_headers(e.http_headers) log_response(error_context, request_start, - e.response[:status], e.response[:body]) + e.http_status, e.http_body) else log_response_error(error_context, request_start, e) end @@ -296,12 +317,10 @@ def decode(_str) end case e - when Faraday::ClientError - if e.response - handle_error_response(e.response, error_context) - else - handle_network_error(e, error_context, num_retries, api_base) - end + when Stripe::StripeError + raise + when *NETWORK_ERROR_MESSAGES_MAP.keys + handle_network_error(e, error_context, num_retries, api_base) # Only handle errors when we know we can do so, and re-raise otherwise. # This should be pretty infrequent. @@ -332,12 +351,12 @@ def decode(_str) private def handle_error_response(http_resp, context) begin - resp = StripeResponse.from_faraday_hash(http_resp) + resp = StripeResponse.from_net_http(http_resp) error_data = resp.data[:error] raise StripeError, "Indeterminate error" unless error_data rescue JSON::ParserError, StripeError - raise general_api_error(http_resp[:status], http_resp[:body]) + raise general_api_error(http_resp.code.to_i, http_resp.body) end error = if error_data.is_a?(String) @@ -444,33 +463,18 @@ def decode(_str) idempotency_key: context.idempotency_key, request_id: context.request_id) - case error - when Faraday::ConnectionFailed - message = "Unexpected error communicating when trying to connect to " \ - "Stripe. You may be seeing this message because your DNS is not " \ - "working. To check, try running `host stripe.com` from the " \ - "command line." - - when Faraday::SSLError - message = "Could not establish a secure connection to Stripe, you " \ - "may need to upgrade your OpenSSL version. To check, try running " \ - "`openssl s_client -connect api.stripe.com:443` from the command " \ - "line." - - when Faraday::TimeoutError - api_base ||= Stripe.api_base - message = "Could not connect to Stripe (#{api_base}). " \ - "Please check your internet connection and try again. " \ - "If this problem persists, you should check Stripe's service " \ - "status at https://status.stripe.com, or let us know at " \ - "support@stripe.com." - - else - message = "Unexpected error communicating with Stripe. " \ - "If this problem persists, let us know at support@stripe.com." + errors, message = NETWORK_ERROR_MESSAGES_MAP.detect do |(e, _)| + error.is_a?(e) + end + if errors.nil? + message = "Unexpected error #{error.class.name} communicating " \ + "with Stripe. Please let us know at support@stripe.com." end + api_base ||= Stripe.api_base + message = message % api_base + message += " Request was retried #{num_retries} times." if num_retries > 0 raise APIConnectionError, @@ -586,18 +590,7 @@ class RequestLogContext # with for a request. For example, we should trust whatever came back in # a `Stripe-Version` header beyond what configuration information that we # might have had available. - def dup_from_response(resp) - return self if resp.nil? - - # Faraday's API is a little unusual. Normally it'll produce a response - # object with a `headers` method, but on error what it puts into - # `e.response` is an untyped `Hash`. - headers = if resp.is_a?(Faraday::Response) - resp.headers - else - resp[:headers] - end - + def dup_from_response_headers(headers) context = dup context.account = headers["Stripe-Account"] context.api_version = headers["Stripe-Version"] diff --git a/lib/stripe/stripe_response.rb b/lib/stripe/stripe_response.rb index 2028d1cc7..8c4749f1e 100644 --- a/lib/stripe/stripe_response.rb +++ b/lib/stripe/stripe_response.rb @@ -4,6 +4,53 @@ module Stripe # StripeResponse encapsulates some vitals of a response that came back from # the Stripe API. class StripeResponse + # Headers provides an access wrapper to an API response's header data. It + # mainly exists so that we don't need to expose the entire + # `Net::HTTPResponse` object while still getting some of its benefits like + # case-insensitive access to header names and flattening of header values. + class Headers + # Initializes a Headers object from a Net::HTTP::HTTPResponse object. + def self.from_net_http(resp) + new(resp.to_hash) + end + + # `hash` is expected to be a hash mapping header names to arrays of + # header values. This is the default format generated by calling + # `#to_hash` on a `Net::HTTPResponse` object because headers can be + # repeated multiple times. Using `#[]` will collapse values down to just + # the first. + def initialize(hash) + if !hash.is_a?(Hash) || + !hash.keys.all? { |n| n.is_a?(String) } || + !hash.values.all? { |a| a.is_a?(Array) } || + !hash.values.all? { |a| a.all? { |v| v.is_a?(String) } } + raise ArgumentError, + "expect hash to be a map of string header names to arrays of " \ + "header values" + end + + @hash = {} + + # This shouldn't be strictly necessary because `Net::HTTPResponse` will + # produce a hash with all headers downcased, but do it anyway just in + # case an object of this class was constructed manually. + # + # Also has the effect of duplicating the hash, which is desirable for a + # little extra object safety. + hash.each do |k, v| + @hash[k.downcase] = v + end + end + + def [](name) + values = @hash[name.downcase] + if values && values.count > 1 + warn("Duplicate header values for `#{name}`; returning only first") + end + values ? values.first : nil + end + end + # The data contained by the HTTP body of the response deserialized from # JSON. attr_accessor :data @@ -20,30 +67,15 @@ class StripeResponse # The Stripe request ID of the response. attr_accessor :request_id - # Initializes a StripeResponse object from a Hash like the kind returned as - # part of a Faraday exception. - # - # This may throw JSON::ParserError if the response body is not valid JSON. - def self.from_faraday_hash(http_resp) - resp = StripeResponse.new - resp.data = JSON.parse(http_resp[:body], symbolize_names: true) - resp.http_body = http_resp[:body] - resp.http_headers = http_resp[:headers] - resp.http_status = http_resp[:status] - resp.request_id = http_resp[:headers]["Request-Id"] - resp - end - - # Initializes a StripeResponse object from a Faraday HTTP response object. - # - # This may throw JSON::ParserError if the response body is not valid JSON. - def self.from_faraday_response(http_resp) + # Initializes a StripeResponse object from a Net::HTTP::HTTPResponse + # object. + def self.from_net_http(http_resp) resp = StripeResponse.new resp.data = JSON.parse(http_resp.body, symbolize_names: true) resp.http_body = http_resp.body - resp.http_headers = http_resp.headers - resp.http_status = http_resp.status - resp.request_id = http_resp.headers["Request-Id"] + resp.http_headers = Headers.from_net_http(http_resp) + resp.http_status = http_resp.code.to_i + resp.request_id = http_resp["request-id"] resp end end diff --git a/stripe.gemspec b/stripe.gemspec index 07e88ae1f..126ca5732 100644 --- a/stripe.gemspec +++ b/stripe.gemspec @@ -26,9 +26,6 @@ Gem::Specification.new do |s| "source_code_uri" => "https://github.com/stripe/stripe-ruby", } - s.add_dependency("faraday", "~> 0.13") - s.add_dependency("net-http-persistent", "~> 3.0") - s.files = `git ls-files`.split("\n") s.test_files = `git ls-files -- test/*`.split("\n") s.executables = `git ls-files -- bin/*`.split("\n") diff --git a/test/stripe/connection_manager_test.rb b/test/stripe/connection_manager_test.rb new file mode 100644 index 000000000..74f4f0ae4 --- /dev/null +++ b/test/stripe/connection_manager_test.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require ::File.expand_path("../test_helper", __dir__) + +module Stripe + class ConnectionManagerTest < Test::Unit::TestCase + setup do + @manager = Stripe::ConnectionManager.new + end + + context "#connection_for" do + should "correctly initialize a connection" do + old_proxy = Stripe.proxy + + old_open_timeout = Stripe.open_timeout + old_read_timeout = Stripe.read_timeout + + begin + # Make sure any global initialization here is undone in the `ensure` + # block below. + Stripe.proxy = "http://user:pass@localhost:8080" + + Stripe.open_timeout = 123 + Stripe.read_timeout = 456 + + conn = @manager.connection_for("https://stripe.com") + + # Host/port + assert_equal "stripe.com", conn.address + assert_equal 443, conn.port + + # Proxy + assert_equal "localhost", conn.proxy_address + assert_equal 8080, conn.proxy_port + assert_equal "user", conn.proxy_user + assert_equal "pass", conn.proxy_pass + + # Timeouts + assert_equal 123, conn.open_timeout + assert_equal 456, conn.read_timeout + + assert_equal true, conn.use_ssl? + assert_equal OpenSSL::SSL::VERIFY_PEER, conn.verify_mode + assert_equal Stripe.ca_store, conn.cert_store + ensure + Stripe.proxy = old_proxy + + Stripe.open_timeout = old_open_timeout + Stripe.read_timeout = old_read_timeout + end + end + + should "produce the same connection multiple times" do + conn1 = @manager.connection_for("https://stripe.com") + conn2 = @manager.connection_for("https://stripe.com") + + assert_equal conn1, conn2 + end + + should "produce different connections for different hosts" do + conn1 = @manager.connection_for("https://example.com") + conn2 = @manager.connection_for("https://stripe.com") + + refute_equal conn1, conn2 + end + + should "produce different connections for different ports" do + conn1 = @manager.connection_for("https://stripe.com:80") + conn2 = @manager.connection_for("https://stripe.com:443") + + refute_equal conn1, conn2 + end + end + + context "#execute_request" do + should "make a request" do + stub_request(:post, "#{Stripe.api_base}/path?query=bar") + .with( + body: "body=foo", + headers: { "Stripe-Account" => "bar" } + ) + .to_return(body: JSON.generate(object: "account")) + + @manager.execute_request(:post, "#{Stripe.api_base}/path", + body: "body=foo", + headers: { "Stripe-Account" => "bar" }, + query: "query=bar") + end + + should "perform basic argument validation" do + e = assert_raises ArgumentError do + @manager.execute_request("POST", "#{Stripe.api_base}/path") + end + assert_equal e.message, "method should be a symbol" + + e = assert_raises ArgumentError do + @manager.execute_request(:post, :uri) + end + assert_equal e.message, "uri should be a string" + + e = assert_raises ArgumentError do + @manager.execute_request(:post, "#{Stripe.api_base}/path", + body: {}) + end + assert_equal e.message, "body should be a string" + + e = assert_raises ArgumentError do + @manager.execute_request(:post, "#{Stripe.api_base}/path", + headers: "foo") + end + assert_equal e.message, "headers should be a hash" + + e = assert_raises ArgumentError do + @manager.execute_request(:post, "#{Stripe.api_base}/path", + query: {}) + end + assert_equal e.message, "query should be a string" + end + end + end +end diff --git a/test/stripe/file_test.rb b/test/stripe/file_test.rb index 61a2f14ec..1b16f5bbe 100644 --- a/test/stripe/file_test.rb +++ b/test/stripe/file_test.rb @@ -52,16 +52,6 @@ class FileTest < Test::Unit::TestCase assert file.is_a?(Stripe::File) end - should "be creatable with Faraday::UploadIO" do - file = Stripe::File.create( - purpose: "dispute_evidence", - file: Faraday::UploadIO.new(::File.new(__FILE__), nil), - file_link_data: { create: true } - ) - assert_requested :post, "#{Stripe.uploads_base}/v1/files" - assert file.is_a?(Stripe::File) - end - should "be creatable with a string" do file = Stripe::File.create( purpose: "dispute_evidence", diff --git a/test/stripe/file_upload_test.rb b/test/stripe/file_upload_test.rb index 47d34b86c..bd4010f87 100644 --- a/test/stripe/file_upload_test.rb +++ b/test/stripe/file_upload_test.rb @@ -54,16 +54,6 @@ class FileUploadTest < Test::Unit::TestCase assert_requested :post, "#{Stripe.uploads_base}/v1/files" assert file.is_a?(Stripe::FileUpload) end - - should "be creatable with Faraday::UploadIO" do - file = Stripe::FileUpload.create( - purpose: "dispute_evidence", - file: Faraday::UploadIO.new(::File.new(__FILE__), nil), - file_link_data: { create: true } - ) - assert_requested :post, "#{Stripe.uploads_base}/v1/files" - assert file.is_a?(Stripe::FileUpload) - end end should "be deserializable when `object=file`" do diff --git a/test/stripe/multipart_encoder_test.rb b/test/stripe/multipart_encoder_test.rb new file mode 100644 index 000000000..bbb0bc7d5 --- /dev/null +++ b/test/stripe/multipart_encoder_test.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +require ::File.expand_path("../test_helper", __dir__) + +module Stripe + class MultipartEncoderTest < Test::Unit::TestCase + should "multipart encode parameters" do + Tempfile.create("image.jpg") do |f| + f.write "file-content" + f.flush + f.rewind + + encoder = MultipartEncoder.new + encoder.encode( + file: f, + other_param: "other-param-content" + ) + encoder.close + body = encoder.body + + assert_equal <<-BODY.rstrip, body +--#{encoder.boundary}\r +Content-Disposition: form-data; name="file"; filename="#{::File.basename(f.path)}"\r +Content-Type: application/octet-stream\r +\r +file-content\r +--#{encoder.boundary}\r +Content-Disposition: form-data; name="other_param"\r +\r +other-param-content\r +--#{encoder.boundary}-- + BODY + end + end + + should "encode file-like objects" do + klass = Class.new do + def read + "klass-read-content" + end + end + + encoder = MultipartEncoder.new + encoder.encode( + file_like: klass.new + ) + encoder.close + body = encoder.body + + assert_equal <<-BODY.rstrip, body +--#{encoder.boundary}\r +Content-Disposition: form-data; name="file_like"; filename="blob"\r +Content-Type: application/octet-stream\r +\r +klass-read-content\r +--#{encoder.boundary}-- + BODY + end + + should "escape quotes and line break characters in parameter names" do + encoder = MultipartEncoder.new + encoder.encode( + %("quoted\n\r") => "content" + ) + encoder.close + body = encoder.body + + assert_equal <<-BODY.rstrip, body +--#{encoder.boundary}\r +Content-Disposition: form-data; name="%22quoted %22"\r +\r +content\r +--#{encoder.boundary}-- + BODY + end + + context ".encode" do + should "provide an easy encoding shortcut" do + body, content_type = MultipartEncoder.encode( + param: "content" + ) + assert_include body, %(Content-Disposition: form-data; name="param") + assert_include content_type, "#{MultipartEncoder::MULTIPART_FORM_DATA}; boundary=" + end + end + + context "#body" do + should "error if not yet closed" do + encoder = MultipartEncoder.new + + e = assert_raises RuntimeError do + encoder.body + end + assert_equal "object must be closed before getting body", e.message + end + end + + context "#close" do + should "error if closed twice" do + encoder = MultipartEncoder.new + encoder.close + + e = assert_raises RuntimeError do + encoder.close + end + assert_equal "object already closed", e.message + end + end + + context "#content_type" do + should "produce a content type containing boundary" do + encoder = MultipartEncoder.new + assert_equal "#{MultipartEncoder::MULTIPART_FORM_DATA}; boundary=#{encoder.boundary}", + encoder.content_type + end + end + + context "#encode" do + should "error if already closed" do + encoder = MultipartEncoder.new + encoder.close + + e = assert_raises RuntimeError do + encoder.encode(param: "content") + end + assert_equal "no more parameters can be written to closed object", e.message + end + end + end +end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 35f8cd1e3..6fd167f7b 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -32,18 +32,19 @@ class StripeClientTest < Test::Unit::TestCase end end - context ".default_conn" do - should "be a Faraday::Connection" do - assert_kind_of Faraday::Connection, StripeClient.default_conn + context ".default_connection_manager" do + should "be a ConnectionManager" do + assert_kind_of ConnectionManager, + StripeClient.default_connection_manager end should "be a different connection on each thread" do - other_thread_conn = nil + other_thread_manager = nil thread = Thread.new do - other_thread_conn = StripeClient.default_conn + other_thread_manager = StripeClient.default_connection_manager end thread.join - refute_equal StripeClient.default_conn, other_thread_conn + refute_equal StripeClient.default_connection_manager, other_thread_manager end end @@ -52,18 +53,24 @@ class StripeClientTest < Test::Unit::TestCase Stripe.stubs(:max_network_retries).returns(2) end - should "retry on timeout" do - assert StripeClient.should_retry?(Faraday::TimeoutError.new(""), 0) + should "retry on Errno::ECONNREFUSED" do + assert StripeClient.should_retry?(Errno::ECONNREFUSED.new, 0) end - should "retry on a failed connection" do - assert StripeClient.should_retry?(Faraday::ConnectionFailed.new(""), 0) + should "retry on Net::OpenTimeout" do + assert StripeClient.should_retry?(Net::OpenTimeout.new, 0) + end + + should "retry on Net::ReadTimeout" do + assert StripeClient.should_retry?(Net::ReadTimeout.new, 0) + end + + should "retry on SocketError" do + assert StripeClient.should_retry?(SocketError.new, 0) end should "retry on a conflict" do - error = make_rate_limit_error - e = Faraday::ClientError.new(error[:error][:message], status: 409) - assert StripeClient.should_retry?(e, 0) + assert StripeClient.should_retry?(Stripe::StripeError.new(http_status: 409), 0) end should "not retry at maximum count" do @@ -71,7 +78,7 @@ class StripeClientTest < Test::Unit::TestCase end should "not retry on a certificate validation error" do - refute StripeClient.should_retry?(Faraday::SSLError.new(""), 0) + refute StripeClient.should_retry?(OpenSSL::SSL::SSLError.new, 0) end end @@ -115,15 +122,16 @@ class StripeClientTest < Test::Unit::TestCase end context "#initialize" do - should "set Stripe.default_conn" do + should "set Stripe.default_connection_manager" do client = StripeClient.new - assert_equal StripeClient.default_conn, client.conn + assert_equal StripeClient.default_connection_manager, + client.connection_manager end should "set a different connection if one was specified" do - conn = Faraday.new - client = StripeClient.new(conn) - assert_equal conn, client.conn + connection_manager = ConnectionManager.new + client = StripeClient.new(connection_manager) + assert_equal connection_manager, client.connection_manager end end @@ -403,6 +411,20 @@ class StripeClientTest < Test::Unit::TestCase assert_equal 'Invalid response object from API: "" (HTTP response code was 200)', e.message end + should "handle low level error" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_raise(Errno::ECONNREFUSED.new) + + client = StripeClient.new + e = assert_raises Stripe::APIConnectionError do + client.execute_request(:post, "/v1/charges") + end + + assert_equal StripeClient::ERROR_MESSAGE_CONNECTION % Stripe.api_base + + "\n\n(Network error: Connection refused)", + e.message + end + should "handle error response with unknown value" do stub_request(:post, "#{Stripe.api_base}/v1/charges") .to_return(body: JSON.generate(bar: "foo"), status: 500) @@ -753,18 +775,23 @@ class StripeClientTest < Test::Unit::TestCase context "#proxy" do should "run the request through the proxy" do begin - Thread.current[:stripe_client_default_conn] = nil + Thread.current[:stripe_client_default_connection_manager] = nil - Stripe.proxy = "http://localhost:8080" + Stripe.proxy = "http://user:pass@localhost:8080" client = StripeClient.new client.request {} - assert_equal "http://localhost:8080", Stripe::StripeClient.default_conn.proxy.uri.to_s + connection = Stripe::StripeClient.default_connection_manager.connection_for(Stripe.api_base) + + assert_equal "localhost", connection.proxy_address + assert_equal 8080, connection.proxy_port + assert_equal "user", connection.proxy_user + assert_equal "pass", connection.proxy_pass ensure Stripe.proxy = nil - Thread.current[:stripe_client_default_conn] = nil + Thread.current[:stripe_client_default_connection_manager] = nil end end end diff --git a/test/stripe/stripe_response_test.rb b/test/stripe/stripe_response_test.rb index 0e89b6fc9..8cf7a8320 100644 --- a/test/stripe/stripe_response_test.rb +++ b/test/stripe/stripe_response_test.rb @@ -4,46 +4,92 @@ module Stripe class StripeResponseTest < Test::Unit::TestCase - context ".from_faraday_hash" do - should "converts to StripeResponse" do - body = '{"foo": "bar"}' + context "Headers" do + should "allow case-insensitive header access" do headers = { "Request-Id" => "request-id" } + http_resp = create_net_http_resp(200, "", headers) - http_resp = { - body: body, - headers: headers, - status: 200, - } + headers = StripeResponse::Headers.from_net_http(http_resp) - resp = StripeResponse.from_faraday_hash(http_resp) + assert_equal "request-id", headers["request-id"] + assert_equal "request-id", headers["Request-Id"] + assert_equal "request-id", headers["Request-ID"] + end - assert_equal JSON.parse(body, symbolize_names: true), resp.data - assert_equal body, resp.http_body - assert_equal headers, resp.http_headers - assert_equal 200, resp.http_status - assert_equal "request-id", resp.request_id + should "initialize without error" do + StripeResponse::Headers.new({}) + StripeResponse::Headers.new("Request-Id" => []) + StripeResponse::Headers.new("Request-Id" => ["request-id"]) + end + + should "initialize with error on a malformed hash" do + assert_raises(ArgumentError) do + StripeResponse::Headers.new(nil) + end + + assert_raises(ArgumentError) do + StripeResponse::Headers.new(1 => []) + end + + assert_raises(ArgumentError) do + StripeResponse::Headers.new("Request-Id" => 1) + end + + assert_raises(ArgumentError) do + StripeResponse::Headers.new("Request-Id" => [1]) + end + end + + should "warn on duplicate header values" do + old_stderr = $stderr + $stderr = StringIO.new + begin + headers = StripeResponse::Headers.new("Duplicated" => %w[a b]) + assert_equal "a", headers["Duplicated"] + assert_equal "Duplicate header values for `Duplicated`; returning only first", + $stderr.string.rstrip + ensure + $stderr = old_stderr + end end end - context ".from_faraday_response" do + context ".from_net_http" do should "converts to StripeResponse" do + code = 200 body = '{"foo": "bar"}' headers = { "Request-Id" => "request-id" } + http_resp = create_net_http_resp(code, body, headers) - env = Faraday::Env.from( - status: 200, body: body, - response_headers: headers - ) - http_resp = Faraday::Response.new(env) - - resp = StripeResponse.from_faraday_response(http_resp) + resp = StripeResponse.from_net_http(http_resp) assert_equal JSON.parse(body, symbolize_names: true), resp.data assert_equal body, resp.http_body - assert_equal headers, resp.http_headers - assert_equal 200, resp.http_status + assert_equal "request-id", resp.http_headers["Request-ID"] + assert_equal code, resp.http_status assert_equal "request-id", resp.request_id end end + + # Synthesizes a `Net::HTTPResponse` object for testing purposes. + private def create_net_http_resp(code, body, headers) + # The "1.1" is HTTP version. + http_resp = Net::HTTPResponse.new("1.1", code.to_s, nil) + http_resp.body = body + + # This is obviously super sketchy, but the Ruby team has done everything + # in their power to make these objects as difficult to test with as + # possible. Even if you specify a body, accessing `#body` the first time + # will attempt to read from a non-existent socket which will subsequently + # blow up. Setting this internal variable skips that read and allows the + # object to use the body that we specified above. + http_resp.instance_variable_set(:@read, true) + + headers.each do |name, value| + http_resp[name] = value + end + + http_resp + end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index acde53db0..d14b98ddd 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -28,16 +28,21 @@ # we can print one error and fail fast so that it's more clear to the user how # they should fix the problem. begin - conn = Faraday::Connection.new("http://localhost:#{MOCK_PORT}") - resp = conn.get("/") - version = resp.headers["Stripe-Mock-Version"] + resp = Net::HTTP.get_response(URI("http://localhost:#{MOCK_PORT}/")) + version = resp["Stripe-Mock-Version"] + + if version.nil? + abort("Couldn't find `Stripe-Mock-Version` in response from " \ + "`localhost:#{MOCK_PORT}`. Is the service running there stripe-mock?") + end + if version != "master" && Gem::Version.new(version) < Gem::Version.new(MOCK_MINIMUM_VERSION) abort("Your version of stripe-mock (#{version}) is too old. The minimum " \ "version to run this test suite is #{MOCK_MINIMUM_VERSION}. Please " \ "see its repository for upgrade instructions.") end -rescue Faraday::ConnectionFailed +rescue Errno::ECONNREFUSED abort("Couldn't reach stripe-mock at `localhost:#{MOCK_PORT}`. Is " \ "it running? Please see README for setup instructions.") end From 15538b0e87ceabb5894ba56579c5ccb905b5a494 Mon Sep 17 00:00:00 2001 From: Brandur Date: Mon, 29 Jul 2019 14:18:53 -0700 Subject: [PATCH 02/18] Drop support for old versions of Ruby Drops support for Ruby 2.1 (EOL March 31, 2017) and 2.2 (EOL March 31, 2018). They're removed from `.travis.yml` and the gemspec and RuboCop configuration have also been updated to the new lower bound. Most of the diff here are minor updates to styling as required by RuboCop: * String literals are frozen by default, so the `.freeze` we had everywhere is now considered redundant. * We can now use Ruby 1.9 style hash syntax with string keys like `{ "foo": "bar" }`. * Converted a few heredocs over to use squiggly (leading whitespace removed) syntax. As discussed in Slack, I didn't drop support for Ruby 2.3 (EOL March 31, 2019) as we still have quite a few users on it. As far as I know dropping it doesn't get us access to any major syntax improvements or anything, so it's probably not a big deal. --- .rubocop.yml | 5 +- .rubocop_todo.yml | 6 --- .travis.yml | 4 +- lib/stripe/list_object.rb | 2 +- lib/stripe/multipart_encoder.rb | 9 ++-- lib/stripe/resources/account.rb | 2 +- lib/stripe/resources/account_link.rb | 2 +- lib/stripe/resources/alipay_account.rb | 2 +- lib/stripe/resources/apple_pay_domain.rb | 2 +- lib/stripe/resources/application_fee.rb | 2 +- .../resources/application_fee_refund.rb | 2 +- lib/stripe/resources/balance.rb | 2 +- lib/stripe/resources/balance_transaction.rb | 2 +- lib/stripe/resources/bank_account.rb | 2 +- lib/stripe/resources/bitcoin_receiver.rb | 2 +- lib/stripe/resources/bitcoin_transaction.rb | 2 +- lib/stripe/resources/capability.rb | 2 +- lib/stripe/resources/card.rb | 2 +- lib/stripe/resources/charge.rb | 2 +- lib/stripe/resources/checkout/session.rb | 2 +- lib/stripe/resources/country_spec.rb | 2 +- lib/stripe/resources/coupon.rb | 2 +- lib/stripe/resources/credit_note.rb | 2 +- lib/stripe/resources/customer.rb | 2 +- .../resources/customer_balance_transaction.rb | 2 +- lib/stripe/resources/discount.rb | 2 +- lib/stripe/resources/dispute.rb | 2 +- lib/stripe/resources/ephemeral_key.rb | 2 +- lib/stripe/resources/event.rb | 2 +- lib/stripe/resources/exchange_rate.rb | 2 +- lib/stripe/resources/file.rb | 4 +- lib/stripe/resources/file_link.rb | 2 +- lib/stripe/resources/invoice.rb | 2 +- lib/stripe/resources/invoice_item.rb | 2 +- lib/stripe/resources/invoice_line_item.rb | 2 +- lib/stripe/resources/issuer_fraud_record.rb | 2 +- lib/stripe/resources/issuing/authorization.rb | 2 +- lib/stripe/resources/issuing/card.rb | 2 +- lib/stripe/resources/issuing/card_details.rb | 2 +- lib/stripe/resources/issuing/cardholder.rb | 2 +- lib/stripe/resources/issuing/dispute.rb | 2 +- lib/stripe/resources/issuing/transaction.rb | 2 +- lib/stripe/resources/login_link.rb | 2 +- lib/stripe/resources/order.rb | 2 +- lib/stripe/resources/order_return.rb | 2 +- lib/stripe/resources/payment_intent.rb | 2 +- lib/stripe/resources/payment_method.rb | 2 +- lib/stripe/resources/payout.rb | 2 +- lib/stripe/resources/person.rb | 2 +- lib/stripe/resources/plan.rb | 2 +- lib/stripe/resources/product.rb | 2 +- .../resources/radar/early_fraud_warning.rb | 2 +- lib/stripe/resources/radar/value_list.rb | 2 +- lib/stripe/resources/radar/value_list_item.rb | 2 +- lib/stripe/resources/recipient.rb | 2 +- lib/stripe/resources/recipient_transfer.rb | 2 +- lib/stripe/resources/refund.rb | 2 +- lib/stripe/resources/reporting/report_run.rb | 2 +- lib/stripe/resources/reporting/report_type.rb | 2 +- lib/stripe/resources/reversal.rb | 2 +- lib/stripe/resources/review.rb | 2 +- lib/stripe/resources/setup_intent.rb | 2 +- .../resources/sigma/scheduled_query_run.rb | 2 +- lib/stripe/resources/sku.rb | 2 +- lib/stripe/resources/source.rb | 2 +- lib/stripe/resources/source_transaction.rb | 2 +- lib/stripe/resources/subscription.rb | 2 +- lib/stripe/resources/subscription_item.rb | 2 +- lib/stripe/resources/subscription_schedule.rb | 2 +- lib/stripe/resources/tax_id.rb | 2 +- lib/stripe/resources/tax_rate.rb | 2 +- .../resources/terminal/connection_token.rb | 2 +- lib/stripe/resources/terminal/location.rb | 2 +- lib/stripe/resources/terminal/reader.rb | 2 +- lib/stripe/resources/three_d_secure.rb | 2 +- lib/stripe/resources/token.rb | 2 +- lib/stripe/resources/topup.rb | 2 +- lib/stripe/resources/transfer.rb | 2 +- lib/stripe/resources/usage_record.rb | 2 +- lib/stripe/resources/usage_record_summary.rb | 2 +- lib/stripe/resources/webhook_endpoint.rb | 2 +- lib/stripe/stripe_client.rb | 6 +-- lib/stripe/stripe_object.rb | 3 +- lib/stripe/util.rb | 5 +- lib/stripe/version.rb | 2 +- lib/stripe/webhook.rb | 2 +- stripe.gemspec | 2 +- test/stripe/api_operations_test.rb | 4 +- test/stripe/api_resource_test.rb | 4 +- test/stripe/multipart_encoder_test.rb | 48 +++++++++---------- test/stripe/payment_intent_test.rb | 2 +- test/stripe/setup_intent_test.rb | 2 +- test/stripe/stripe_object_test.rb | 6 +-- test/stripe/webhook_test.rb | 4 +- test/stripe_mock.rb | 4 +- test/test_helper.rb | 2 +- 96 files changed, 134 insertions(+), 142 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index e7130b785..18682165d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,7 +2,7 @@ inherit_from: .rubocop_todo.yml AllCops: DisplayCopNames: true - TargetRubyVersion: 2.1 + TargetRubyVersion: 2.3 Layout/CaseIndentation: EnforcedStyle: end @@ -39,6 +39,9 @@ Style/AccessModifierDeclarations: Style/FrozenStringLiteralComment: EnforcedStyle: always +Style/NumericPredicate: + Enabled: false + Style/StringLiterals: EnforcedStyle: double_quotes diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 73cee4a73..8d96346ef 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -6,12 +6,6 @@ # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -Layout/ClosingHeredocIndentation: - # As far as I can tell, this lint rule just doesn't work when it comes to - # heredoc in `it` blocks. - Exclude: - - 'test/**/*' - # Offense count: 20 Metrics/AbcSize: Max: 53 diff --git a/.travis.yml b/.travis.yml index e58c9c856..241cc08c2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,13 +1,11 @@ language: ruby rvm: - - 2.1 - - 2.2 - 2.3 - 2.4 - 2.5 - 2.6 - - jruby-9.0.5.0 + - jruby-9.2.7.0 notifications: email: diff --git a/lib/stripe/list_object.rb b/lib/stripe/list_object.rb index 30c6e8580..8322ba1f5 100644 --- a/lib/stripe/list_object.rb +++ b/lib/stripe/list_object.rb @@ -7,7 +7,7 @@ class ListObject < StripeObject include Stripe::APIOperations::Request include Stripe::APIOperations::Create - OBJECT_NAME = "list".freeze + OBJECT_NAME = "list" # This accessor allows a `ListObject` to inherit various filters that were # given to a predecessor. This allows for things like consistent limits, diff --git a/lib/stripe/multipart_encoder.rb b/lib/stripe/multipart_encoder.rb index fbfde51dc..6f498f68c 100644 --- a/lib/stripe/multipart_encoder.rb +++ b/lib/stripe/multipart_encoder.rb @@ -16,7 +16,7 @@ module Stripe # placed in the `Content-Type` header of a subsequent request (which includes # a boundary value). class MultipartEncoder - MULTIPART_FORM_DATA = "multipart/form-data".freeze + MULTIPART_FORM_DATA = "multipart/form-data" # A shortcut for encoding a single set of parameters and finalizing a # result. @@ -35,10 +35,9 @@ def self.encode(params) # Initializes a new multipart encoder. def initialize - # We seed this with an empty string so that it encoding defaults to UTF-8 - # instead of ASCII. The empty string is UTF-8 and new string inherits the - # encoding of the string it's seeded with. - @body = String.new("") + # Kind of weird, but required by Rubocop because the unary plus operator + # is considered faster than `Stripe.new`. + @body = +"" # Chose the same number of random bytes that Go uses in its standard # library implementation. Easily enough entropy to ensure that it won't diff --git a/lib/stripe/resources/account.rb b/lib/stripe/resources/account.rb index 36db22a5b..48c0e83e6 100644 --- a/lib/stripe/resources/account.rb +++ b/lib/stripe/resources/account.rb @@ -9,7 +9,7 @@ class Account < APIResource include Stripe::APIOperations::Save extend Stripe::APIOperations::NestedResource - OBJECT_NAME = "account".freeze + OBJECT_NAME = "account" custom_method :reject, http_verb: :post diff --git a/lib/stripe/resources/account_link.rb b/lib/stripe/resources/account_link.rb index af6f064aa..110b17079 100644 --- a/lib/stripe/resources/account_link.rb +++ b/lib/stripe/resources/account_link.rb @@ -4,6 +4,6 @@ module Stripe class AccountLink < APIResource extend Stripe::APIOperations::Create - OBJECT_NAME = "account_link".freeze + OBJECT_NAME = "account_link" end end diff --git a/lib/stripe/resources/alipay_account.rb b/lib/stripe/resources/alipay_account.rb index 56dc5cd10..785c184cf 100644 --- a/lib/stripe/resources/alipay_account.rb +++ b/lib/stripe/resources/alipay_account.rb @@ -5,7 +5,7 @@ class AlipayAccount < APIResource include Stripe::APIOperations::Save include Stripe::APIOperations::Delete - OBJECT_NAME = "alipay_account".freeze + OBJECT_NAME = "alipay_account" def resource_url if !respond_to?(:customer) || customer.nil? diff --git a/lib/stripe/resources/apple_pay_domain.rb b/lib/stripe/resources/apple_pay_domain.rb index 740f1f10d..8bac3c1c4 100644 --- a/lib/stripe/resources/apple_pay_domain.rb +++ b/lib/stripe/resources/apple_pay_domain.rb @@ -7,7 +7,7 @@ class ApplePayDomain < APIResource include Stripe::APIOperations::Delete extend Stripe::APIOperations::List - OBJECT_NAME = "apple_pay_domain".freeze + OBJECT_NAME = "apple_pay_domain" def self.resource_url "/v1/apple_pay/domains" diff --git a/lib/stripe/resources/application_fee.rb b/lib/stripe/resources/application_fee.rb index af411aaf5..367c41d95 100644 --- a/lib/stripe/resources/application_fee.rb +++ b/lib/stripe/resources/application_fee.rb @@ -5,7 +5,7 @@ class ApplicationFee < APIResource extend Stripe::APIOperations::List extend Stripe::APIOperations::NestedResource - OBJECT_NAME = "application_fee".freeze + OBJECT_NAME = "application_fee" nested_resource_class_methods :refund, operations: %i[create retrieve update list] diff --git a/lib/stripe/resources/application_fee_refund.rb b/lib/stripe/resources/application_fee_refund.rb index a7e02566a..43465da96 100644 --- a/lib/stripe/resources/application_fee_refund.rb +++ b/lib/stripe/resources/application_fee_refund.rb @@ -5,7 +5,7 @@ class ApplicationFeeRefund < APIResource include Stripe::APIOperations::Save extend Stripe::APIOperations::List - OBJECT_NAME = "fee_refund".freeze + OBJECT_NAME = "fee_refund" def resource_url "#{ApplicationFee.resource_url}/#{CGI.escape(fee)}/refunds" \ diff --git a/lib/stripe/resources/balance.rb b/lib/stripe/resources/balance.rb index 34b9d64e2..24ea840b1 100644 --- a/lib/stripe/resources/balance.rb +++ b/lib/stripe/resources/balance.rb @@ -2,6 +2,6 @@ module Stripe class Balance < SingletonAPIResource - OBJECT_NAME = "balance".freeze + OBJECT_NAME = "balance" end end diff --git a/lib/stripe/resources/balance_transaction.rb b/lib/stripe/resources/balance_transaction.rb index 94b67f422..d16407259 100644 --- a/lib/stripe/resources/balance_transaction.rb +++ b/lib/stripe/resources/balance_transaction.rb @@ -4,6 +4,6 @@ module Stripe class BalanceTransaction < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "balance_transaction".freeze + OBJECT_NAME = "balance_transaction" end end diff --git a/lib/stripe/resources/bank_account.rb b/lib/stripe/resources/bank_account.rb index 833f13838..668a10824 100644 --- a/lib/stripe/resources/bank_account.rb +++ b/lib/stripe/resources/bank_account.rb @@ -6,7 +6,7 @@ class BankAccount < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "bank_account".freeze + OBJECT_NAME = "bank_account" def verify(params = {}, opts = {}) resp, opts = request(:post, resource_url + "/verify", params, opts) diff --git a/lib/stripe/resources/bitcoin_receiver.rb b/lib/stripe/resources/bitcoin_receiver.rb index 9a5877bb8..474aeb4b9 100644 --- a/lib/stripe/resources/bitcoin_receiver.rb +++ b/lib/stripe/resources/bitcoin_receiver.rb @@ -6,7 +6,7 @@ module Stripe class BitcoinReceiver < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "bitcoin_receiver".freeze + OBJECT_NAME = "bitcoin_receiver" def self.resource_url "/v1/bitcoin/receivers" diff --git a/lib/stripe/resources/bitcoin_transaction.rb b/lib/stripe/resources/bitcoin_transaction.rb index 966937cb0..ba9dd6b22 100644 --- a/lib/stripe/resources/bitcoin_transaction.rb +++ b/lib/stripe/resources/bitcoin_transaction.rb @@ -6,7 +6,7 @@ class BitcoinTransaction < APIResource # Sources API instead: https://stripe.com/docs/sources/bitcoin extend Stripe::APIOperations::List - OBJECT_NAME = "bitcoin_transaction".freeze + OBJECT_NAME = "bitcoin_transaction" def self.resource_url "/v1/bitcoin/transactions" diff --git a/lib/stripe/resources/capability.rb b/lib/stripe/resources/capability.rb index e692efb5a..054ec040a 100644 --- a/lib/stripe/resources/capability.rb +++ b/lib/stripe/resources/capability.rb @@ -5,7 +5,7 @@ class Capability < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "capability".freeze + OBJECT_NAME = "capability" def resource_url if !respond_to?(:account) || account.nil? diff --git a/lib/stripe/resources/card.rb b/lib/stripe/resources/card.rb index 194ad2cc7..eeffd6f75 100644 --- a/lib/stripe/resources/card.rb +++ b/lib/stripe/resources/card.rb @@ -6,7 +6,7 @@ class Card < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "card".freeze + OBJECT_NAME = "card" def resource_url if respond_to?(:recipient) && !recipient.nil? && !recipient.empty? diff --git a/lib/stripe/resources/charge.rb b/lib/stripe/resources/charge.rb index f9ec48d43..d6d1c7a92 100644 --- a/lib/stripe/resources/charge.rb +++ b/lib/stripe/resources/charge.rb @@ -6,7 +6,7 @@ class Charge < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "charge".freeze + OBJECT_NAME = "charge" custom_method :capture, http_verb: :post diff --git a/lib/stripe/resources/checkout/session.rb b/lib/stripe/resources/checkout/session.rb index c821913bb..b07561609 100644 --- a/lib/stripe/resources/checkout/session.rb +++ b/lib/stripe/resources/checkout/session.rb @@ -5,7 +5,7 @@ module Checkout class Session < APIResource extend Stripe::APIOperations::Create - OBJECT_NAME = "checkout.session".freeze + OBJECT_NAME = "checkout.session" end end end diff --git a/lib/stripe/resources/country_spec.rb b/lib/stripe/resources/country_spec.rb index cddd9b8f5..54032bf85 100644 --- a/lib/stripe/resources/country_spec.rb +++ b/lib/stripe/resources/country_spec.rb @@ -4,6 +4,6 @@ module Stripe class CountrySpec < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "country_spec".freeze + OBJECT_NAME = "country_spec" end end diff --git a/lib/stripe/resources/coupon.rb b/lib/stripe/resources/coupon.rb index e795e66f3..2025422c8 100644 --- a/lib/stripe/resources/coupon.rb +++ b/lib/stripe/resources/coupon.rb @@ -7,6 +7,6 @@ class Coupon < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "coupon".freeze + OBJECT_NAME = "coupon" end end diff --git a/lib/stripe/resources/credit_note.rb b/lib/stripe/resources/credit_note.rb index 3538d81ad..91c3b1f70 100644 --- a/lib/stripe/resources/credit_note.rb +++ b/lib/stripe/resources/credit_note.rb @@ -6,7 +6,7 @@ class CreditNote < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "credit_note".freeze + OBJECT_NAME = "credit_note" custom_method :void_credit_note, http_verb: :post, http_path: "void" diff --git a/lib/stripe/resources/customer.rb b/lib/stripe/resources/customer.rb index 02baef6d9..ba968cf39 100644 --- a/lib/stripe/resources/customer.rb +++ b/lib/stripe/resources/customer.rb @@ -8,7 +8,7 @@ class Customer < APIResource include Stripe::APIOperations::Save extend Stripe::APIOperations::NestedResource - OBJECT_NAME = "customer".freeze + OBJECT_NAME = "customer" nested_resource_class_methods :balance_transaction, operations: %i[create retrieve update list] diff --git a/lib/stripe/resources/customer_balance_transaction.rb b/lib/stripe/resources/customer_balance_transaction.rb index 25519bcc1..9d3e44fe1 100644 --- a/lib/stripe/resources/customer_balance_transaction.rb +++ b/lib/stripe/resources/customer_balance_transaction.rb @@ -5,7 +5,7 @@ class CustomerBalanceTransaction < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "customer_balance_transaction".freeze + OBJECT_NAME = "customer_balance_transaction" def resource_url if !respond_to?(:customer) || customer.nil? diff --git a/lib/stripe/resources/discount.rb b/lib/stripe/resources/discount.rb index a16cbc925..7fc0275b5 100644 --- a/lib/stripe/resources/discount.rb +++ b/lib/stripe/resources/discount.rb @@ -2,6 +2,6 @@ module Stripe class Discount < StripeObject - OBJECT_NAME = "discount".freeze + OBJECT_NAME = "discount" end end diff --git a/lib/stripe/resources/dispute.rb b/lib/stripe/resources/dispute.rb index 882651a90..59d9a92e4 100644 --- a/lib/stripe/resources/dispute.rb +++ b/lib/stripe/resources/dispute.rb @@ -5,7 +5,7 @@ class Dispute < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "dispute".freeze + OBJECT_NAME = "dispute" custom_method :close, http_verb: :post diff --git a/lib/stripe/resources/ephemeral_key.rb b/lib/stripe/resources/ephemeral_key.rb index 1ad6977e3..80d460257 100644 --- a/lib/stripe/resources/ephemeral_key.rb +++ b/lib/stripe/resources/ephemeral_key.rb @@ -5,7 +5,7 @@ class EphemeralKey < APIResource extend Stripe::APIOperations::Create include Stripe::APIOperations::Delete - OBJECT_NAME = "ephemeral_key".freeze + OBJECT_NAME = "ephemeral_key" def self.create(params = {}, opts = {}) opts = Util.normalize_opts(opts) diff --git a/lib/stripe/resources/event.rb b/lib/stripe/resources/event.rb index 5a627a523..f016018b7 100644 --- a/lib/stripe/resources/event.rb +++ b/lib/stripe/resources/event.rb @@ -4,6 +4,6 @@ module Stripe class Event < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "event".freeze + OBJECT_NAME = "event" end end diff --git a/lib/stripe/resources/exchange_rate.rb b/lib/stripe/resources/exchange_rate.rb index 427d5e0aa..67f1f2c7a 100644 --- a/lib/stripe/resources/exchange_rate.rb +++ b/lib/stripe/resources/exchange_rate.rb @@ -4,6 +4,6 @@ module Stripe class ExchangeRate < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "exchange_rate".freeze + OBJECT_NAME = "exchange_rate" end end diff --git a/lib/stripe/resources/file.rb b/lib/stripe/resources/file.rb index ff9cff6e3..41979485c 100644 --- a/lib/stripe/resources/file.rb +++ b/lib/stripe/resources/file.rb @@ -5,13 +5,13 @@ class File < APIResource extend Stripe::APIOperations::Create extend Stripe::APIOperations::List - OBJECT_NAME = "file".freeze + OBJECT_NAME = "file" # This resource can have two different object names. In latter API # versions, only `file` is used, but since stripe-ruby may be used with # any API version, we need to support deserializing the older # `file_upload` object into the same class. - OBJECT_NAME_ALT = "file_upload".freeze + OBJECT_NAME_ALT = "file_upload" def self.resource_url "/v1/files" diff --git a/lib/stripe/resources/file_link.rb b/lib/stripe/resources/file_link.rb index ea6cc64e9..e517af952 100644 --- a/lib/stripe/resources/file_link.rb +++ b/lib/stripe/resources/file_link.rb @@ -6,6 +6,6 @@ class FileLink < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "file_link".freeze + OBJECT_NAME = "file_link" end end diff --git a/lib/stripe/resources/invoice.rb b/lib/stripe/resources/invoice.rb index 24ffa74e0..840cc062e 100644 --- a/lib/stripe/resources/invoice.rb +++ b/lib/stripe/resources/invoice.rb @@ -7,7 +7,7 @@ class Invoice < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "invoice".freeze + OBJECT_NAME = "invoice" custom_method :finalize_invoice, http_verb: :post, http_path: "finalize" custom_method :mark_uncollectible, http_verb: :post diff --git a/lib/stripe/resources/invoice_item.rb b/lib/stripe/resources/invoice_item.rb index 350258b0b..78e1cc2cf 100644 --- a/lib/stripe/resources/invoice_item.rb +++ b/lib/stripe/resources/invoice_item.rb @@ -7,6 +7,6 @@ class InvoiceItem < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "invoiceitem".freeze + OBJECT_NAME = "invoiceitem" end end diff --git a/lib/stripe/resources/invoice_line_item.rb b/lib/stripe/resources/invoice_line_item.rb index a2056bbc4..499ad6707 100644 --- a/lib/stripe/resources/invoice_line_item.rb +++ b/lib/stripe/resources/invoice_line_item.rb @@ -2,6 +2,6 @@ module Stripe class InvoiceLineItem < StripeObject - OBJECT_NAME = "line_item".freeze + OBJECT_NAME = "line_item" end end diff --git a/lib/stripe/resources/issuer_fraud_record.rb b/lib/stripe/resources/issuer_fraud_record.rb index 76b12488b..43eb11334 100644 --- a/lib/stripe/resources/issuer_fraud_record.rb +++ b/lib/stripe/resources/issuer_fraud_record.rb @@ -4,6 +4,6 @@ module Stripe class IssuerFraudRecord < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "issuer_fraud_record".freeze + OBJECT_NAME = "issuer_fraud_record" end end diff --git a/lib/stripe/resources/issuing/authorization.rb b/lib/stripe/resources/issuing/authorization.rb index 05ac7ef36..632edf2b1 100644 --- a/lib/stripe/resources/issuing/authorization.rb +++ b/lib/stripe/resources/issuing/authorization.rb @@ -6,7 +6,7 @@ class Authorization < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "issuing.authorization".freeze + OBJECT_NAME = "issuing.authorization" custom_method :approve, http_verb: :post custom_method :decline, http_verb: :post diff --git a/lib/stripe/resources/issuing/card.rb b/lib/stripe/resources/issuing/card.rb index 32efcd6d4..d2345d1c6 100644 --- a/lib/stripe/resources/issuing/card.rb +++ b/lib/stripe/resources/issuing/card.rb @@ -7,7 +7,7 @@ class Card < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "issuing.card".freeze + OBJECT_NAME = "issuing.card" custom_method :details, http_verb: :get diff --git a/lib/stripe/resources/issuing/card_details.rb b/lib/stripe/resources/issuing/card_details.rb index 4d483cd57..c215abe1e 100644 --- a/lib/stripe/resources/issuing/card_details.rb +++ b/lib/stripe/resources/issuing/card_details.rb @@ -3,7 +3,7 @@ module Stripe module Issuing class CardDetails < Stripe::StripeObject - OBJECT_NAME = "issuing.card_details".freeze + OBJECT_NAME = "issuing.card_details" end end end diff --git a/lib/stripe/resources/issuing/cardholder.rb b/lib/stripe/resources/issuing/cardholder.rb index f555798c1..9163a2180 100644 --- a/lib/stripe/resources/issuing/cardholder.rb +++ b/lib/stripe/resources/issuing/cardholder.rb @@ -7,7 +7,7 @@ class Cardholder < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "issuing.cardholder".freeze + OBJECT_NAME = "issuing.cardholder" end end end diff --git a/lib/stripe/resources/issuing/dispute.rb b/lib/stripe/resources/issuing/dispute.rb index 9cfe732b9..a7ca28197 100644 --- a/lib/stripe/resources/issuing/dispute.rb +++ b/lib/stripe/resources/issuing/dispute.rb @@ -7,7 +7,7 @@ class Dispute < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "issuing.dispute".freeze + OBJECT_NAME = "issuing.dispute" end end end diff --git a/lib/stripe/resources/issuing/transaction.rb b/lib/stripe/resources/issuing/transaction.rb index dbd29825c..e9ed28686 100644 --- a/lib/stripe/resources/issuing/transaction.rb +++ b/lib/stripe/resources/issuing/transaction.rb @@ -6,7 +6,7 @@ class Transaction < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "issuing.transaction".freeze + OBJECT_NAME = "issuing.transaction" end end end diff --git a/lib/stripe/resources/login_link.rb b/lib/stripe/resources/login_link.rb index 950e01607..2a9dee414 100644 --- a/lib/stripe/resources/login_link.rb +++ b/lib/stripe/resources/login_link.rb @@ -2,7 +2,7 @@ module Stripe class LoginLink < APIResource - OBJECT_NAME = "login_link".freeze + OBJECT_NAME = "login_link" def self.retrieve(_id, _opts = nil) raise NotImplementedError, diff --git a/lib/stripe/resources/order.rb b/lib/stripe/resources/order.rb index 8c9220861..504e4638c 100644 --- a/lib/stripe/resources/order.rb +++ b/lib/stripe/resources/order.rb @@ -6,7 +6,7 @@ class Order < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "order".freeze + OBJECT_NAME = "order" custom_method :pay, http_verb: :post custom_method :return_order, http_verb: :post, http_path: "returns" diff --git a/lib/stripe/resources/order_return.rb b/lib/stripe/resources/order_return.rb index d33df79f1..66ffcaaf4 100644 --- a/lib/stripe/resources/order_return.rb +++ b/lib/stripe/resources/order_return.rb @@ -4,6 +4,6 @@ module Stripe class OrderReturn < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "order_return".freeze + OBJECT_NAME = "order_return" end end diff --git a/lib/stripe/resources/payment_intent.rb b/lib/stripe/resources/payment_intent.rb index b9e46bd12..a4abeb73b 100644 --- a/lib/stripe/resources/payment_intent.rb +++ b/lib/stripe/resources/payment_intent.rb @@ -6,7 +6,7 @@ class PaymentIntent < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "payment_intent".freeze + OBJECT_NAME = "payment_intent" custom_method :cancel, http_verb: :post custom_method :capture, http_verb: :post diff --git a/lib/stripe/resources/payment_method.rb b/lib/stripe/resources/payment_method.rb index d366a6f34..f51d005ed 100644 --- a/lib/stripe/resources/payment_method.rb +++ b/lib/stripe/resources/payment_method.rb @@ -6,7 +6,7 @@ class PaymentMethod < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "payment_method".freeze + OBJECT_NAME = "payment_method" custom_method :attach, http_verb: :post custom_method :detach, http_verb: :post diff --git a/lib/stripe/resources/payout.rb b/lib/stripe/resources/payout.rb index 79ec44d66..e4cae3e24 100644 --- a/lib/stripe/resources/payout.rb +++ b/lib/stripe/resources/payout.rb @@ -6,7 +6,7 @@ class Payout < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "payout".freeze + OBJECT_NAME = "payout" custom_method :cancel, http_verb: :post diff --git a/lib/stripe/resources/person.rb b/lib/stripe/resources/person.rb index 185b8d336..d7bd5fba9 100644 --- a/lib/stripe/resources/person.rb +++ b/lib/stripe/resources/person.rb @@ -5,7 +5,7 @@ class Person < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "person".freeze + OBJECT_NAME = "person" def resource_url if !respond_to?(:account) || account.nil? diff --git a/lib/stripe/resources/plan.rb b/lib/stripe/resources/plan.rb index 192bf301b..fc5da942e 100644 --- a/lib/stripe/resources/plan.rb +++ b/lib/stripe/resources/plan.rb @@ -7,6 +7,6 @@ class Plan < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "plan".freeze + OBJECT_NAME = "plan" end end diff --git a/lib/stripe/resources/product.rb b/lib/stripe/resources/product.rb index 9b318c49f..8fd952fa1 100644 --- a/lib/stripe/resources/product.rb +++ b/lib/stripe/resources/product.rb @@ -7,6 +7,6 @@ class Product < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "product".freeze + OBJECT_NAME = "product" end end diff --git a/lib/stripe/resources/radar/early_fraud_warning.rb b/lib/stripe/resources/radar/early_fraud_warning.rb index 6f53787ad..62610f8e0 100644 --- a/lib/stripe/resources/radar/early_fraud_warning.rb +++ b/lib/stripe/resources/radar/early_fraud_warning.rb @@ -5,7 +5,7 @@ module Radar class EarlyFraudWarning < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "radar.early_fraud_warning".freeze + OBJECT_NAME = "radar.early_fraud_warning" end end end diff --git a/lib/stripe/resources/radar/value_list.rb b/lib/stripe/resources/radar/value_list.rb index 2f68f09c9..5be70700f 100644 --- a/lib/stripe/resources/radar/value_list.rb +++ b/lib/stripe/resources/radar/value_list.rb @@ -8,7 +8,7 @@ class ValueList < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "radar.value_list".freeze + OBJECT_NAME = "radar.value_list" end end end diff --git a/lib/stripe/resources/radar/value_list_item.rb b/lib/stripe/resources/radar/value_list_item.rb index 6000ddd84..d1f4560e3 100644 --- a/lib/stripe/resources/radar/value_list_item.rb +++ b/lib/stripe/resources/radar/value_list_item.rb @@ -7,7 +7,7 @@ class ValueListItem < APIResource include Stripe::APIOperations::Delete extend Stripe::APIOperations::List - OBJECT_NAME = "radar.value_list_item".freeze + OBJECT_NAME = "radar.value_list_item" end end end diff --git a/lib/stripe/resources/recipient.rb b/lib/stripe/resources/recipient.rb index 9efd40714..5e95bb87c 100644 --- a/lib/stripe/resources/recipient.rb +++ b/lib/stripe/resources/recipient.rb @@ -8,7 +8,7 @@ class Recipient < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "recipient".freeze + OBJECT_NAME = "recipient" def transfers Transfer.all({ recipient: id }, @api_key) diff --git a/lib/stripe/resources/recipient_transfer.rb b/lib/stripe/resources/recipient_transfer.rb index 30faaaee3..c5ef96449 100644 --- a/lib/stripe/resources/recipient_transfer.rb +++ b/lib/stripe/resources/recipient_transfer.rb @@ -2,6 +2,6 @@ module Stripe class RecipientTransfer < StripeObject - OBJECT_NAME = "recipient_transfer".freeze + OBJECT_NAME = "recipient_transfer" end end diff --git a/lib/stripe/resources/refund.rb b/lib/stripe/resources/refund.rb index b832b0f07..518c17de5 100644 --- a/lib/stripe/resources/refund.rb +++ b/lib/stripe/resources/refund.rb @@ -6,6 +6,6 @@ class Refund < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "refund".freeze + OBJECT_NAME = "refund" end end diff --git a/lib/stripe/resources/reporting/report_run.rb b/lib/stripe/resources/reporting/report_run.rb index 25029052e..58de595ce 100644 --- a/lib/stripe/resources/reporting/report_run.rb +++ b/lib/stripe/resources/reporting/report_run.rb @@ -6,7 +6,7 @@ class ReportRun < APIResource extend Stripe::APIOperations::Create extend Stripe::APIOperations::List - OBJECT_NAME = "reporting.report_run".freeze + OBJECT_NAME = "reporting.report_run" end end end diff --git a/lib/stripe/resources/reporting/report_type.rb b/lib/stripe/resources/reporting/report_type.rb index e13501fb3..ac6deaa3d 100644 --- a/lib/stripe/resources/reporting/report_type.rb +++ b/lib/stripe/resources/reporting/report_type.rb @@ -6,7 +6,7 @@ class ReportType < APIResource extend Stripe::APIOperations::Create extend Stripe::APIOperations::List - OBJECT_NAME = "reporting.report_type".freeze + OBJECT_NAME = "reporting.report_type" end end end diff --git a/lib/stripe/resources/reversal.rb b/lib/stripe/resources/reversal.rb index c88809698..3af982608 100644 --- a/lib/stripe/resources/reversal.rb +++ b/lib/stripe/resources/reversal.rb @@ -5,7 +5,7 @@ class Reversal < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "transfer_reversal".freeze + OBJECT_NAME = "transfer_reversal" def resource_url "#{Transfer.resource_url}/#{CGI.escape(transfer)}/reversals" \ diff --git a/lib/stripe/resources/review.rb b/lib/stripe/resources/review.rb index 701ef2d0e..4161b6ce6 100644 --- a/lib/stripe/resources/review.rb +++ b/lib/stripe/resources/review.rb @@ -4,7 +4,7 @@ module Stripe class Review < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "review".freeze + OBJECT_NAME = "review" custom_method :approve, http_verb: :post diff --git a/lib/stripe/resources/setup_intent.rb b/lib/stripe/resources/setup_intent.rb index 296bad55c..085d4879a 100644 --- a/lib/stripe/resources/setup_intent.rb +++ b/lib/stripe/resources/setup_intent.rb @@ -6,7 +6,7 @@ class SetupIntent < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "setup_intent".freeze + OBJECT_NAME = "setup_intent" custom_method :cancel, http_verb: :post custom_method :confirm, http_verb: :post diff --git a/lib/stripe/resources/sigma/scheduled_query_run.rb b/lib/stripe/resources/sigma/scheduled_query_run.rb index 1ccfe1455..343a36e9a 100644 --- a/lib/stripe/resources/sigma/scheduled_query_run.rb +++ b/lib/stripe/resources/sigma/scheduled_query_run.rb @@ -5,7 +5,7 @@ module Sigma class ScheduledQueryRun < APIResource extend Stripe::APIOperations::List - OBJECT_NAME = "scheduled_query_run".freeze + OBJECT_NAME = "scheduled_query_run" def self.resource_url "/v1/sigma/scheduled_query_runs" diff --git a/lib/stripe/resources/sku.rb b/lib/stripe/resources/sku.rb index d9d20a1ff..009a751ad 100644 --- a/lib/stripe/resources/sku.rb +++ b/lib/stripe/resources/sku.rb @@ -7,6 +7,6 @@ class SKU < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "sku".freeze + OBJECT_NAME = "sku" end end diff --git a/lib/stripe/resources/source.rb b/lib/stripe/resources/source.rb index dc0a61995..5d41c4b41 100644 --- a/lib/stripe/resources/source.rb +++ b/lib/stripe/resources/source.rb @@ -5,7 +5,7 @@ class Source < APIResource extend Stripe::APIOperations::Create include Stripe::APIOperations::Save - OBJECT_NAME = "source".freeze + OBJECT_NAME = "source" custom_method :verify, http_verb: :post diff --git a/lib/stripe/resources/source_transaction.rb b/lib/stripe/resources/source_transaction.rb index 545162a03..1cfa3c5dd 100644 --- a/lib/stripe/resources/source_transaction.rb +++ b/lib/stripe/resources/source_transaction.rb @@ -2,6 +2,6 @@ module Stripe class SourceTransaction < StripeObject - OBJECT_NAME = "source_transaction".freeze + OBJECT_NAME = "source_transaction" end end diff --git a/lib/stripe/resources/subscription.rb b/lib/stripe/resources/subscription.rb index 0d077d44f..bbb85b887 100644 --- a/lib/stripe/resources/subscription.rb +++ b/lib/stripe/resources/subscription.rb @@ -7,7 +7,7 @@ class Subscription < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "subscription".freeze + OBJECT_NAME = "subscription" custom_method :delete_discount, http_verb: :delete, http_path: "discount" diff --git a/lib/stripe/resources/subscription_item.rb b/lib/stripe/resources/subscription_item.rb index 8c30987fe..fd3a40c43 100644 --- a/lib/stripe/resources/subscription_item.rb +++ b/lib/stripe/resources/subscription_item.rb @@ -8,7 +8,7 @@ class SubscriptionItem < APIResource include Stripe::APIOperations::Save extend Stripe::APIOperations::NestedResource - OBJECT_NAME = "subscription_item".freeze + OBJECT_NAME = "subscription_item" nested_resource_class_methods :usage_record, operations: %i[create] diff --git a/lib/stripe/resources/subscription_schedule.rb b/lib/stripe/resources/subscription_schedule.rb index 27385046a..7f42439a2 100644 --- a/lib/stripe/resources/subscription_schedule.rb +++ b/lib/stripe/resources/subscription_schedule.rb @@ -6,7 +6,7 @@ class SubscriptionSchedule < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "subscription_schedule".freeze + OBJECT_NAME = "subscription_schedule" custom_method :cancel, http_verb: :post custom_method :release, http_verb: :post diff --git a/lib/stripe/resources/tax_id.rb b/lib/stripe/resources/tax_id.rb index 7b1edc58c..2e11fc12a 100644 --- a/lib/stripe/resources/tax_id.rb +++ b/lib/stripe/resources/tax_id.rb @@ -5,7 +5,7 @@ class TaxId < APIResource include Stripe::APIOperations::Delete extend Stripe::APIOperations::List - OBJECT_NAME = "tax_id".freeze + OBJECT_NAME = "tax_id" def resource_url if !respond_to?(:customer) || customer.nil? diff --git a/lib/stripe/resources/tax_rate.rb b/lib/stripe/resources/tax_rate.rb index 9789d9b8d..4e13a140b 100644 --- a/lib/stripe/resources/tax_rate.rb +++ b/lib/stripe/resources/tax_rate.rb @@ -6,6 +6,6 @@ class TaxRate < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "tax_rate".freeze + OBJECT_NAME = "tax_rate" end end diff --git a/lib/stripe/resources/terminal/connection_token.rb b/lib/stripe/resources/terminal/connection_token.rb index 247243a0a..06aede895 100644 --- a/lib/stripe/resources/terminal/connection_token.rb +++ b/lib/stripe/resources/terminal/connection_token.rb @@ -5,7 +5,7 @@ module Terminal class ConnectionToken < APIResource extend Stripe::APIOperations::Create - OBJECT_NAME = "terminal.connection_token".freeze + OBJECT_NAME = "terminal.connection_token" end end end diff --git a/lib/stripe/resources/terminal/location.rb b/lib/stripe/resources/terminal/location.rb index 3c1beb7d4..01fb8dc91 100644 --- a/lib/stripe/resources/terminal/location.rb +++ b/lib/stripe/resources/terminal/location.rb @@ -8,7 +8,7 @@ class Location < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "terminal.location".freeze + OBJECT_NAME = "terminal.location" end end end diff --git a/lib/stripe/resources/terminal/reader.rb b/lib/stripe/resources/terminal/reader.rb index 760c74e8e..48e33bc55 100644 --- a/lib/stripe/resources/terminal/reader.rb +++ b/lib/stripe/resources/terminal/reader.rb @@ -8,7 +8,7 @@ class Reader < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "terminal.reader".freeze + OBJECT_NAME = "terminal.reader" end end end diff --git a/lib/stripe/resources/three_d_secure.rb b/lib/stripe/resources/three_d_secure.rb index e713eb0a9..ba7b260f0 100644 --- a/lib/stripe/resources/three_d_secure.rb +++ b/lib/stripe/resources/three_d_secure.rb @@ -4,7 +4,7 @@ module Stripe class ThreeDSecure < APIResource extend Stripe::APIOperations::Create - OBJECT_NAME = "three_d_secure".freeze + OBJECT_NAME = "three_d_secure" def self.resource_url "/v1/3d_secure" diff --git a/lib/stripe/resources/token.rb b/lib/stripe/resources/token.rb index acf35275c..dbd8a9e19 100644 --- a/lib/stripe/resources/token.rb +++ b/lib/stripe/resources/token.rb @@ -4,6 +4,6 @@ module Stripe class Token < APIResource extend Stripe::APIOperations::Create - OBJECT_NAME = "token".freeze + OBJECT_NAME = "token" end end diff --git a/lib/stripe/resources/topup.rb b/lib/stripe/resources/topup.rb index de59727f5..8b3c4d0f5 100644 --- a/lib/stripe/resources/topup.rb +++ b/lib/stripe/resources/topup.rb @@ -6,7 +6,7 @@ class Topup < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "topup".freeze + OBJECT_NAME = "topup" custom_method :cancel, http_verb: :post diff --git a/lib/stripe/resources/transfer.rb b/lib/stripe/resources/transfer.rb index bff728231..127965b9f 100644 --- a/lib/stripe/resources/transfer.rb +++ b/lib/stripe/resources/transfer.rb @@ -7,7 +7,7 @@ class Transfer < APIResource include Stripe::APIOperations::Save extend Stripe::APIOperations::NestedResource - OBJECT_NAME = "transfer".freeze + OBJECT_NAME = "transfer" custom_method :cancel, http_verb: :post diff --git a/lib/stripe/resources/usage_record.rb b/lib/stripe/resources/usage_record.rb index 9b7bffe22..1dd332a2e 100644 --- a/lib/stripe/resources/usage_record.rb +++ b/lib/stripe/resources/usage_record.rb @@ -2,7 +2,7 @@ module Stripe class UsageRecord < APIResource - OBJECT_NAME = "usage_record".freeze + OBJECT_NAME = "usage_record" def self.create(params = {}, opts = {}) unless params.key?(:subscription_item) diff --git a/lib/stripe/resources/usage_record_summary.rb b/lib/stripe/resources/usage_record_summary.rb index 9b9e515cb..621b9d2a9 100644 --- a/lib/stripe/resources/usage_record_summary.rb +++ b/lib/stripe/resources/usage_record_summary.rb @@ -2,6 +2,6 @@ module Stripe class UsageRecordSummary < StripeObject - OBJECT_NAME = "usage_record_summary".freeze + OBJECT_NAME = "usage_record_summary" end end diff --git a/lib/stripe/resources/webhook_endpoint.rb b/lib/stripe/resources/webhook_endpoint.rb index df752ec9a..cca531015 100644 --- a/lib/stripe/resources/webhook_endpoint.rb +++ b/lib/stripe/resources/webhook_endpoint.rb @@ -7,6 +7,6 @@ class WebhookEndpoint < APIResource extend Stripe::APIOperations::List include Stripe::APIOperations::Save - OBJECT_NAME = "webhook_endpoint".freeze + OBJECT_NAME = "webhook_endpoint" end end diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 126c5d96e..fea84fb1b 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -185,19 +185,19 @@ def execute_request(method, path, "Unexpected error communicating when trying to connect to " \ "Stripe (%s). You may be seeing this message because your DNS is not " \ "working or you don't have an internet connection. To check, try " \ - "running `host stripe.com` from the command line.".freeze + "running `host stripe.com` from the command line." ERROR_MESSAGE_SSL = "Could not establish a secure connection to Stripe (%s), you " \ "may need to upgrade your OpenSSL version. To check, try running " \ "`openssl s_client -connect api.stripe.com:443` from the command " \ - "line.".freeze + "line." # Common error suffix sared by both connect and read timeout messages. ERROR_MESSAGE_TIMEOUT_SUFFIX = "Please check your internet connection and try again. " \ "If this problem persists, you should check Stripe's service " \ "status at https://status.stripe.com, or let us know at " \ - "support@stripe.com.".freeze + "support@stripe.com." ERROR_MESSAGE_TIMEOUT_CONNECT = ( "Timed out connecting to Stripe (%s). " + diff --git a/lib/stripe/stripe_object.rb b/lib/stripe/stripe_object.rb index ea48076c5..e37b0257d 100644 --- a/lib/stripe/stripe_object.rb +++ b/lib/stripe/stripe_object.rb @@ -192,7 +192,8 @@ def as_json(*opts) def to_hash maybe_to_hash = lambda do |value| - value && value.respond_to?(:to_hash) ? value.to_hash : value + return nil if value.nil? + value.respond_to?(:to_hash) ? value.to_hash : value end @values.each_with_object({}) do |(key, value), acc| diff --git a/lib/stripe/util.rb b/lib/stripe/util.rb index 6201a8913..9da5bfaaf 100644 --- a/lib/stripe/util.rb +++ b/lib/stripe/util.rb @@ -281,10 +281,7 @@ def self.level_name(level) end private_class_method :level_name - # TODO: Make these named required arguments when we drop support for Ruby - # 2.0. - def self.log_internal(message, data = {}, color: nil, level: nil, - logger: nil, out: nil) + def self.log_internal(message, data = {}, color:, level:, logger:, out:) data_str = data.reject { |_k, v| v.nil? } .map do |(k, v)| format("%s=%s", diff --git a/lib/stripe/version.rb b/lib/stripe/version.rb index 2535c6cfe..f0701a9fa 100644 --- a/lib/stripe/version.rb +++ b/lib/stripe/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Stripe - VERSION = "4.24.0".freeze + VERSION = "4.24.0" end diff --git a/lib/stripe/webhook.rb b/lib/stripe/webhook.rb index 5aea9490a..ef06ccafa 100644 --- a/lib/stripe/webhook.rb +++ b/lib/stripe/webhook.rb @@ -22,7 +22,7 @@ def self.construct_event(payload, sig_header, secret, end module Signature - EXPECTED_SCHEME = "v1".freeze + EXPECTED_SCHEME = "v1" def self.compute_signature(payload, secret) OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload) diff --git a/stripe.gemspec b/stripe.gemspec index 126ca5732..63172b493 100644 --- a/stripe.gemspec +++ b/stripe.gemspec @@ -7,7 +7,7 @@ require "stripe/version" Gem::Specification.new do |s| s.name = "stripe" s.version = Stripe::VERSION - s.required_ruby_version = ">= 2.1.0" + s.required_ruby_version = ">= 2.3.0" s.summary = "Ruby bindings for the Stripe API" s.description = "Stripe is the easiest way to accept payments online. " \ "See https://stripe.com for details." diff --git a/test/stripe/api_operations_test.rb b/test/stripe/api_operations_test.rb index fa6b8d9bd..5d2954ae8 100644 --- a/test/stripe/api_operations_test.rb +++ b/test/stripe/api_operations_test.rb @@ -7,7 +7,7 @@ class ApiOperationsTest < Test::Unit::TestCase class UpdateableResource < APIResource include Stripe::APIOperations::Save - OBJECT_NAME = "updateableresource".freeze + OBJECT_NAME = "updateableresource" def self.protected_fields [:protected] @@ -34,7 +34,7 @@ def self.protected_fields context ".nested_resource_class_methods" do class MainResource < APIResource extend Stripe::APIOperations::NestedResource - OBJECT_NAME = "mainresource".freeze + OBJECT_NAME = "mainresource" nested_resource_class_methods :nested, operations: %i[create retrieve update delete list] end diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index 13c0e06ba..8c3a40a91 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -5,7 +5,7 @@ module Stripe class ApiResourceTest < Test::Unit::TestCase class CustomMethodAPIResource < APIResource - OBJECT_NAME = "custom_method".freeze + OBJECT_NAME = "custom_method" custom_method :my_method, http_verb: :post end @@ -513,7 +513,7 @@ class NestedTestAPIResource < APIResource context "#request_stripe_object" do class HelloTestAPIResource < APIResource - OBJECT_NAME = "hello".freeze + OBJECT_NAME = "hello" def say_hello(params = {}, opts = {}) request_stripe_object( method: :post, diff --git a/test/stripe/multipart_encoder_test.rb b/test/stripe/multipart_encoder_test.rb index bbb0bc7d5..10eb4df32 100644 --- a/test/stripe/multipart_encoder_test.rb +++ b/test/stripe/multipart_encoder_test.rb @@ -18,17 +18,17 @@ class MultipartEncoderTest < Test::Unit::TestCase encoder.close body = encoder.body - assert_equal <<-BODY.rstrip, body ---#{encoder.boundary}\r -Content-Disposition: form-data; name="file"; filename="#{::File.basename(f.path)}"\r -Content-Type: application/octet-stream\r -\r -file-content\r ---#{encoder.boundary}\r -Content-Disposition: form-data; name="other_param"\r -\r -other-param-content\r ---#{encoder.boundary}-- + assert_equal <<~BODY.rstrip, body + --#{encoder.boundary}\r + Content-Disposition: form-data; name="file"; filename="#{::File.basename(f.path)}"\r + Content-Type: application/octet-stream\r + \r + file-content\r + --#{encoder.boundary}\r + Content-Disposition: form-data; name="other_param"\r + \r + other-param-content\r + --#{encoder.boundary}-- BODY end end @@ -47,13 +47,13 @@ def read encoder.close body = encoder.body - assert_equal <<-BODY.rstrip, body ---#{encoder.boundary}\r -Content-Disposition: form-data; name="file_like"; filename="blob"\r -Content-Type: application/octet-stream\r -\r -klass-read-content\r ---#{encoder.boundary}-- + assert_equal <<~BODY.rstrip, body + --#{encoder.boundary}\r + Content-Disposition: form-data; name="file_like"; filename="blob"\r + Content-Type: application/octet-stream\r + \r + klass-read-content\r + --#{encoder.boundary}-- BODY end @@ -65,12 +65,12 @@ def read encoder.close body = encoder.body - assert_equal <<-BODY.rstrip, body ---#{encoder.boundary}\r -Content-Disposition: form-data; name="%22quoted %22"\r -\r -content\r ---#{encoder.boundary}-- + assert_equal <<~BODY.rstrip, body + --#{encoder.boundary}\r + Content-Disposition: form-data; name="%22quoted %22"\r + \r + content\r + --#{encoder.boundary}-- BODY end diff --git a/test/stripe/payment_intent_test.rb b/test/stripe/payment_intent_test.rb index ef17b7857..d5832334a 100644 --- a/test/stripe/payment_intent_test.rb +++ b/test/stripe/payment_intent_test.rb @@ -4,7 +4,7 @@ module Stripe class PaymentIntentTest < Test::Unit::TestCase - TEST_RESOURCE_ID = "pi_123".freeze + TEST_RESOURCE_ID = "pi_123" should "be listable" do payment_intents = Stripe::PaymentIntent.list diff --git a/test/stripe/setup_intent_test.rb b/test/stripe/setup_intent_test.rb index 84044157f..4ce3eafcb 100644 --- a/test/stripe/setup_intent_test.rb +++ b/test/stripe/setup_intent_test.rb @@ -4,7 +4,7 @@ module Stripe class SetupIntentTest < Test::Unit::TestCase - TEST_RESOURCE_ID = "seti_123".freeze + TEST_RESOURCE_ID = "seti_123" should "be listable" do setup_intents = Stripe::SetupIntent.list diff --git a/test/stripe/stripe_object_test.rb b/test/stripe/stripe_object_test.rb index 57d38050a..3bb44e800 100644 --- a/test/stripe/stripe_object_test.rb +++ b/test/stripe/stripe_object_test.rb @@ -52,9 +52,9 @@ class StripeObjectTest < Test::Unit::TestCase 2, ], map: { - :"0" => StripeObject.construct_from({ id: "index0" }, opts), - :"1" => "index1", - :"2" => 2, + "0": StripeObject.construct_from({ id: "index0" }, opts), + "1": "index1", + "2": 2, }, } diff --git a/test/stripe/webhook_test.rb b/test/stripe/webhook_test.rb index 3c8431939..42ed1258e 100644 --- a/test/stripe/webhook_test.rb +++ b/test/stripe/webhook_test.rb @@ -4,13 +4,13 @@ module Stripe class WebhookTest < Test::Unit::TestCase - EVENT_PAYLOAD = <<-PAYLOAD.freeze + EVENT_PAYLOAD = <<~PAYLOAD { "id": "evt_test_webhook", "object": "event" } PAYLOAD - SECRET = "whsec_test_secret".freeze + SECRET = "whsec_test_secret" def generate_header(opts = {}) opts[:timestamp] ||= Time.now.to_i diff --git a/test/stripe_mock.rb b/test/stripe_mock.rb index 49b742e4a..9118f119c 100644 --- a/test/stripe_mock.rb +++ b/test/stripe_mock.rb @@ -4,8 +4,8 @@ module Stripe class StripeMock include Singleton - PATH_SPEC = "#{::File.dirname(__FILE__)}/openapi/spec3.json".freeze - PATH_FIXTURES = "#{::File.dirname(__FILE__)}/openapi/fixtures3.json".freeze + PATH_SPEC = "#{::File.dirname(__FILE__)}/openapi/spec3.json" + PATH_FIXTURES = "#{::File.dirname(__FILE__)}/openapi/fixtures3.json" @pid = nil @port = -1 diff --git a/test/test_helper.rb b/test/test_helper.rb index d14b98ddd..2b41274cb 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -17,7 +17,7 @@ require ::File.expand_path("stripe_mock", __dir__) # If changing this number, please also change it in `.travis.yml`. -MOCK_MINIMUM_VERSION = "0.63.0".freeze +MOCK_MINIMUM_VERSION = "0.63.0" MOCK_PORT = Stripe::StripeMock.start # Disable all real network connections except those that are outgoing to From 445555617cd4d6adc118b70d05a060f159a3bc50 Mon Sep 17 00:00:00 2001 From: Brandur Date: Mon, 29 Jul 2019 18:51:55 -0700 Subject: [PATCH 03/18] Make `CardError`'s `code` parameter named instead of positional (#816) Makes the `code` parameter on `CardError` named instead of positional. This makes it more consistent with the rest of the constructor's parameters and makes instantiating `CardError` from `StripeClient` cleaner. This is a minor breaking change so we're aiming to release it for the next major version of stripe-ruby. --- lib/stripe/errors.rb | 3 +-- lib/stripe/stripe_client.rb | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/stripe/errors.rb b/lib/stripe/errors.rb index 9eb4f6edf..24254e126 100644 --- a/lib/stripe/errors.rb +++ b/lib/stripe/errors.rb @@ -59,8 +59,7 @@ class APIError < StripeError class CardError < StripeError attr_reader :param - # TODO: make code a keyword arg in next major release - def initialize(message, param, code, http_status: nil, http_body: nil, + def initialize(message, param, code: nil, http_status: nil, http_body: nil, json_body: nil, http_headers: nil) super(message, http_status: http_status, http_body: http_body, json_body: json_body, http_headers: http_headers, diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index fea84fb1b..bbf4efe4c 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -403,11 +403,8 @@ def execute_request(method, path, when 401 AuthenticationError.new(error_data[:message], opts) when 402 - # TODO: modify CardError constructor to make code a keyword argument - # so we don't have to delete it from opts - opts.delete(:code) CardError.new( - error_data[:message], error_data[:param], error_data[:code], + error_data[:message], error_data[:param], opts ) when 403 From f4cd69b72a65e864673aee641510e89caef1d005 Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Tue, 30 Jul 2019 10:12:15 +0800 Subject: [PATCH 04/18] Bump Rubocop to latest version (#818) --- .rubocop.yml | 8 ++++---- .rubocop_todo.yml | 24 +++++++++++------------- Gemfile | 4 +--- lib/stripe/list_object.rb | 1 + lib/stripe/multipart_encoder.rb | 1 + lib/stripe/resources/usage_record.rb | 1 + lib/stripe/stripe_client.rb | 8 ++++---- lib/stripe/stripe_object.rb | 2 ++ lib/stripe/util.rb | 16 +++++++++------- stripe.gemspec | 10 +++++----- test/stripe/login_link_test.rb | 2 +- test/stripe/stripe_object_test.rb | 8 ++++---- test/stripe_mock.rb | 3 ++- 13 files changed, 46 insertions(+), 42 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 18682165d..3c7513949 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,7 +7,10 @@ AllCops: Layout/CaseIndentation: EnforcedStyle: end -Layout/IndentArray: +Layout/IndentFirstArrayElement: + EnforcedStyle: consistent + +Layout/IndentFirstHashElement: EnforcedStyle: consistent # This can be re-enabled once we're 2.3+ only and can use the squiggly heredoc @@ -16,9 +19,6 @@ Layout/IndentArray: Layout/IndentHeredoc: Enabled: false -Layout/IndentHash: - EnforcedStyle: consistent - Metrics/LineLength: Exclude: - "test/**/*.rb" diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8d96346ef..cd8c6f5c5 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,27 +1,25 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2019-05-24 10:18:48 -0700 using RuboCop version 0.57.2. +# on 2019-07-30 09:56:31 +0800 using RuboCop version 0.73.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 20 +# Offense count: 23 Metrics/AbcSize: - Max: 53 + Max: 48 +# Offense count: 33 +# Configuration parameters: CountComments, ExcludedMethods. +# ExcludedMethods: refine Metrics/BlockLength: - Max: 39 + Max: 509 - # Test `context` is a block and generates all kinds of false positives - # without exclusion. - Exclude: - - 'test/**/*' - -# Offense count: 11 +# Offense count: 12 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 700 + Max: 694 # Offense count: 12 Metrics/CyclomaticComplexity: @@ -32,10 +30,10 @@ Metrics/CyclomaticComplexity: Metrics/ParameterLists: Max: 7 -# Offense count: 7 +# Offense count: 8 Metrics/PerceivedComplexity: Max: 17 -# Offense count: 84 +# Offense count: 86 Style/Documentation: Enabled: false diff --git a/Gemfile b/Gemfile index 3dd44ad77..342b74b8e 100644 --- a/Gemfile +++ b/Gemfile @@ -18,9 +18,7 @@ group :development do # `Gemfile.lock` checked in, so to prevent good builds from suddenly going # bad, pin to a specific version number here. Try to keep this relatively # up-to-date, but it's not the end of the world if it's not. - # Note that 0.57.2 is the most recent version we can use until we drop - # support for Ruby 2.1. - gem "rubocop", "0.57.2" + gem "rubocop", "0.73" # Rack 2.0+ requires Ruby >= 2.2.2 which is problematic for the test suite on # older Ruby versions. Check Ruby the version here and put a maximum diff --git a/lib/stripe/list_object.rb b/lib/stripe/list_object.rb index 8322ba1f5..2394e506e 100644 --- a/lib/stripe/list_object.rb +++ b/lib/stripe/list_object.rb @@ -83,6 +83,7 @@ def retrieve(id, opts = {}) # was given, the default limit will be fetched again. def next_page(params = {}, opts = {}) return self.class.empty_list(opts) unless has_more + last_id = data.last.id params = filters.merge(starting_after: last_id).merge(params) diff --git a/lib/stripe/multipart_encoder.rb b/lib/stripe/multipart_encoder.rb index 6f498f68c..0df93460d 100644 --- a/lib/stripe/multipart_encoder.rb +++ b/lib/stripe/multipart_encoder.rb @@ -51,6 +51,7 @@ def initialize # Gets the encoded body. `#close` must be called first. def body raise "object must be closed before getting body" unless @closed + @body end diff --git a/lib/stripe/resources/usage_record.rb b/lib/stripe/resources/usage_record.rb index 1dd332a2e..f34987940 100644 --- a/lib/stripe/resources/usage_record.rb +++ b/lib/stripe/resources/usage_record.rb @@ -8,6 +8,7 @@ def self.create(params = {}, opts = {}) unless params.key?(:subscription_item) raise ArgumentError, "Params must have a subscription_item key" end + req_params = params.clone.delete_if do |key, _value| key == :subscription_item end diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index bbf4efe4c..52c42bdb6 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -214,11 +214,11 @@ def execute_request(method, path, # The original error message is also appended onto the final exception for # full transparency. NETWORK_ERROR_MESSAGES_MAP = { - Errno::ECONNREFUSED => ERROR_MESSAGE_CONNECTION, - SocketError => ERROR_MESSAGE_CONNECTION, + Errno::ECONNREFUSED => ERROR_MESSAGE_CONNECTION, + SocketError => ERROR_MESSAGE_CONNECTION, - Net::OpenTimeout => ERROR_MESSAGE_TIMEOUT_CONNECT, - Net::ReadTimeout => ERROR_MESSAGE_TIMEOUT_READ, + Net::OpenTimeout => ERROR_MESSAGE_TIMEOUT_CONNECT, + Net::ReadTimeout => ERROR_MESSAGE_TIMEOUT_READ, OpenSSL::SSL::SSLError => ERROR_MESSAGE_SSL, }.freeze diff --git a/lib/stripe/stripe_object.rb b/lib/stripe/stripe_object.rb index e37b0257d..f0f16b579 100644 --- a/lib/stripe/stripe_object.rb +++ b/lib/stripe/stripe_object.rb @@ -193,6 +193,7 @@ def as_json(*opts) def to_hash maybe_to_hash = lambda do |value| return nil if value.nil? + value.respond_to?(:to_hash) ? value.to_hash : value end @@ -257,6 +258,7 @@ def serialize_params(options = {}) # unsaved = @unsaved_values.include?(k) next unless options[:force] || unsaved || v.is_a?(StripeObject) + update_hash[k.to_sym] = serialize_params_value( @values[k], @original_values[k], unsaved, options[:force], key: k ) diff --git a/lib/stripe/util.rb b/lib/stripe/util.rb index 9da5bfaaf..f2d4fa4d3 100644 --- a/lib/stripe/util.rb +++ b/lib/stripe/util.rb @@ -198,11 +198,13 @@ def self.normalize_opts(opts) def self.check_string_argument!(key) raise TypeError, "argument must be a string" unless key.is_a?(String) + key end def self.check_api_key!(key) raise TypeError, "api_key must be a string" unless key.is_a?(String) + key end @@ -245,14 +247,14 @@ def self.secure_compare(str_a, str_b) # COLOR_CODES = { - black: 0, light_black: 60, - red: 1, light_red: 61, - green: 2, light_green: 62, - yellow: 3, light_yellow: 63, - blue: 4, light_blue: 64, + black: 0, light_black: 60, + red: 1, light_red: 61, + green: 2, light_green: 62, + yellow: 3, light_yellow: 63, + blue: 4, light_blue: 64, magenta: 5, light_magenta: 65, - cyan: 6, light_cyan: 66, - white: 7, light_white: 67, + cyan: 6, light_cyan: 66, + white: 7, light_white: 67, default: 9, }.freeze private_constant :COLOR_CODES diff --git a/stripe.gemspec b/stripe.gemspec index 63172b493..e8eab921f 100644 --- a/stripe.gemspec +++ b/stripe.gemspec @@ -17,13 +17,13 @@ Gem::Specification.new do |s| s.license = "MIT" s.metadata = { - "bug_tracker_uri" => "https://github.com/stripe/stripe-ruby/issues", - "changelog_uri" => + "bug_tracker_uri" => "https://github.com/stripe/stripe-ruby/issues", + "changelog_uri" => "https://github.com/stripe/stripe-ruby/blob/master/CHANGELOG.md", "documentation_uri" => "https://stripe.com/docs/api/ruby", - "github_repo" => "ssh://github.com/stripe/stripe-ruby", - "homepage_uri" => "https://stripe.com/docs/api/ruby", - "source_code_uri" => "https://github.com/stripe/stripe-ruby", + "github_repo" => "ssh://github.com/stripe/stripe-ruby", + "homepage_uri" => "https://stripe.com/docs/api/ruby", + "source_code_uri" => "https://github.com/stripe/stripe-ruby", } s.files = `git ls-files`.split("\n") diff --git a/test/stripe/login_link_test.rb b/test/stripe/login_link_test.rb index 053052217..8682e2616 100644 --- a/test/stripe/login_link_test.rb +++ b/test/stripe/login_link_test.rb @@ -12,7 +12,7 @@ class LoginLinkTest < Test::Unit::TestCase "data" => [], "has_more" => false, "object" => "list", - "url" => "/v1/accounts/acct_123/login_links", + "url" => "/v1/accounts/acct_123/login_links", }, } @account = Stripe::Account.construct_from(account_fixture) diff --git a/test/stripe/stripe_object_test.rb b/test/stripe/stripe_object_test.rb index 3bb44e800..d22b50468 100644 --- a/test/stripe/stripe_object_test.rb +++ b/test/stripe/stripe_object_test.rb @@ -302,14 +302,14 @@ def to_hash end should "#serialize_params on an array that shortens" do - obj = Stripe::StripeObject.construct_from(foo: ["0-index", "1-index", "2-index"]) + obj = Stripe::StripeObject.construct_from(foo: %w[0-index 1-index 2-index]) obj.foo = ["new-value"] assert_equal({ foo: ["new-value"] }, obj.serialize_params) end should "#serialize_params on an array that lengthens" do - obj = Stripe::StripeObject.construct_from(foo: ["0-index", "1-index", "2-index"]) + obj = Stripe::StripeObject.construct_from(foo: %w[0-index 1-index 2-index]) obj.foo = ["new-value"] * 4 assert_equal({ foo: ["new-value"] * 4 }, obj.serialize_params) @@ -331,8 +331,8 @@ def to_hash end should "#serialize_params on an array that is unchanged" do - obj = Stripe::StripeObject.construct_from(foo: ["0-index", "1-index", "2-index"]) - obj.foo = ["0-index", "1-index", "2-index"] + obj = Stripe::StripeObject.construct_from(foo: %w[0-index 1-index 2-index]) + obj.foo = %w[0-index 1-index 2-index] assert_equal({}, obj.serialize_params) end diff --git a/test/stripe_mock.rb b/test/stripe_mock.rb index 9118f119c..edb3835e4 100644 --- a/test/stripe_mock.rb +++ b/test/stripe_mock.rb @@ -29,7 +29,7 @@ def self.start @stderr, @child_stderr = ::IO.pipe @pid = ::Process.spawn( - ["stripe-mock", "stripe-mock"], + %w[stripe-mock stripe-mock], "-http-port", "0", # have stripe-mock select a port "-spec", @@ -66,6 +66,7 @@ def self.start # Stops stripe-mock, if necessary. def self.stop return if @pid.nil? + puts("Stopping stripe-mock...") ::Process.kill(:SIGTERM, @pid) ::Process.waitpid2(@pid) From cf45ea70a84344f7ccccbbd5f0df9cd95d13f154 Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Tue, 30 Jul 2019 10:13:44 +0800 Subject: [PATCH 05/18] Ruby minimum version increase followup (#819) --- .travis.yml | 2 -- Gemfile | 10 +--------- README.md | 2 +- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/.travis.yml b/.travis.yml index 241cc08c2..92bfb904a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,8 +23,6 @@ cache: - stripe-mock before_install: - # Install bundler 1.x, because we need to support Ruby 2.1 for now - - gem install bundler -v "~> 1.0" # Unpack and start stripe-mock so that the test suite can talk to it - | if [ ! -d "stripe-mock/stripe-mock_${STRIPE_MOCK_VERSION}" ]; then diff --git a/Gemfile b/Gemfile index 342b74b8e..0d4b59e9a 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,7 @@ gemspec group :development do gem "coveralls", require: false gem "mocha", "~> 0.13.2" + gem "rack", ">= 2.0.6" gem "rake" gem "shoulda-context" gem "test-unit" @@ -20,15 +21,6 @@ group :development do # up-to-date, but it's not the end of the world if it's not. gem "rubocop", "0.73" - # Rack 2.0+ requires Ruby >= 2.2.2 which is problematic for the test suite on - # older Ruby versions. Check Ruby the version here and put a maximum - # constraint on Rack if necessary. - if RUBY_VERSION >= "2.2.2" - gem "rack", ">= 2.0.6" - else - gem "rack", ">= 1.6.11", "< 2.0" # rubocop:disable Bundler/DuplicatedGem - end - platforms :mri do gem "byebug" gem "pry" diff --git a/README.md b/README.md index 2e4a79cb6..d17983e04 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ gem build stripe.gemspec ### Requirements -- Ruby 2.1+. +- Ruby 2.3+. ### Bundler From 53c96e602de3399e21fcaf6de1e279f3b7fe90da Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Wed, 7 Aug 2019 10:53:07 -0700 Subject: [PATCH 06/18] Remove old deprecated methods (#820) --- lib/stripe/resources/account.rb | 4 -- lib/stripe/resources/application_fee.rb | 11 ---- lib/stripe/resources/charge.rb | 68 +----------------------- lib/stripe/resources/customer.rb | 64 +---------------------- lib/stripe/resources/dispute.rb | 6 --- lib/stripe/resources/file.rb | 5 -- lib/stripe/resources/order.rb | 8 --- lib/stripe/resources/payout.rb | 6 --- lib/stripe/resources/recipient.rb | 4 -- lib/stripe/resources/source.rb | 6 --- lib/stripe/resources/subscription.rb | 8 +-- lib/stripe/resources/transfer.rb | 5 -- test/stripe/account_test.rb | 16 ------ test/stripe/api_resource_test.rb | 8 --- test/stripe/charge_test.rb | 16 ------ test/stripe/customer_test.rb | 45 +--------------- test/stripe/file_upload_test.rb | 69 ------------------------- test/stripe/source_test.rb | 18 ------- 18 files changed, 6 insertions(+), 361 deletions(-) delete mode 100644 test/stripe/file_upload_test.rb diff --git a/lib/stripe/resources/account.rb b/lib/stripe/resources/account.rb index 48c0e83e6..05c1d7897 100644 --- a/lib/stripe/resources/account.rb +++ b/lib/stripe/resources/account.rb @@ -35,10 +35,6 @@ def reject(params = {}, opts = {}) nested_resource_class_methods :login_link, operations: %i[create] - # This method is deprecated. Please use `#external_account=` instead. - save_nested_resource :bank_account - deprecate :bank_account=, "#external_account=", 2017, 8 - def resource_url if self["id"] super diff --git a/lib/stripe/resources/application_fee.rb b/lib/stripe/resources/application_fee.rb index 367c41d95..808d6bb42 100644 --- a/lib/stripe/resources/application_fee.rb +++ b/lib/stripe/resources/application_fee.rb @@ -9,16 +9,5 @@ class ApplicationFee < APIResource nested_resource_class_methods :refund, operations: %i[create retrieve update list] - - # If you don't need access to an updated fee object after the refund, it's - # more performant to just call `fee.refunds.create` directly. - def refund(params = {}, opts = {}) - refunds.create(params, opts) - - # now that a refund has been created, we expect the state of this object - # to change as well (i.e. `refunded` will now be `true`) so refresh it - # from the server - refresh - end end end diff --git a/lib/stripe/resources/charge.rb b/lib/stripe/resources/charge.rb index d6d1c7a92..64f76b20b 100644 --- a/lib/stripe/resources/charge.rb +++ b/lib/stripe/resources/charge.rb @@ -10,75 +10,9 @@ class Charge < APIResource custom_method :capture, http_verb: :post - def refund(params = {}, opts = {}) - # Old versions of charge objects included a `refunds` field that was just - # a vanilla array instead of a Stripe list object. - # - # Where possible, we'd still like to use the new refund endpoint (thus - # `self.refunds.create`), but detect the old API version by looking for - # an `Array` and fall back to the old refund URL if necessary so as to - # maintain internal compatibility. - if refunds.is_a?(Array) - resp, opts = request(:post, refund_url, params, opts) - initialize_from(resp.data, opts) - else - refunds.create(params, opts) - - # now that a refund has been created, we expect the state of this object - # to change as well (i.e. `refunded` will now be `true`) so refresh it - # from the server - refresh - end - end - def capture(params = {}, opts = {}) - resp, opts = request(:post, capture_url, params, opts) - initialize_from(resp.data, opts) - end - - def update_dispute(params = {}, opts = {}) - resp, opts = request(:post, dispute_url, params, opts) - initialize_from({ dispute: resp.data }, opts, true) - dispute - end - - def close_dispute(params = {}, opts = {}) - resp, opts = request(:post, close_dispute_url, params, opts) - initialize_from(resp.data, opts) - end - - def mark_as_fraudulent - params = { - fraud_details: { user_report: "fraudulent" }, - } - resp, opts = request(:post, resource_url, params) + resp, opts = request(:post, resource_url + "/capture", params, opts) initialize_from(resp.data, opts) end - - def mark_as_safe - params = { - fraud_details: { user_report: "safe" }, - } - resp, opts = request(:post, resource_url, params) - initialize_from(resp.data, opts) - end - - private def capture_url - resource_url + "/capture" - end - - private def dispute_url - resource_url + "/dispute" - end - - private def close_dispute_url - resource_url + "/dispute/close" - end - - # Note that this is actually the *old* refund URL and its use is no longer - # preferred. - private def refund_url - resource_url + "/refund" - end end end diff --git a/lib/stripe/resources/customer.rb b/lib/stripe/resources/customer.rb index ba968cf39..39a844a43 100644 --- a/lib/stripe/resources/customer.rb +++ b/lib/stripe/resources/customer.rb @@ -27,69 +27,9 @@ class << self alias detach_source delete_source end - def add_invoice_item(params, opts = {}) - opts = @opts.merge(Util.normalize_opts(opts)) - InvoiceItem.create(params.merge(customer: id), opts) - end - - def invoices(params = {}, opts = {}) - opts = @opts.merge(Util.normalize_opts(opts)) - Invoice.all(params.merge(customer: id), opts) - end - - def invoice_items(params = {}, opts = {}) - opts = @opts.merge(Util.normalize_opts(opts)) - InvoiceItem.all(params.merge(customer: id), opts) - end - - def upcoming_invoice(params = {}, opts = {}) - opts = @opts.merge(Util.normalize_opts(opts)) - Invoice.upcoming(params.merge(customer: id), opts) - end - - def charges(params = {}, opts = {}) - opts = @opts.merge(Util.normalize_opts(opts)) - Charge.all(params.merge(customer: id), opts) - end - - def create_upcoming_invoice(params = {}, opts = {}) - opts = @opts.merge(Util.normalize_opts(opts)) - Invoice.create(params.merge(customer: id), opts) - end - - def cancel_subscription(params = {}, opts = {}) - resp, opts = request(:delete, subscription_url, params, opts) - initialize_from({ subscription: resp.data }, opts, true) - subscription - end - - def update_subscription(params = {}, opts = {}) - resp, opts = request(:post, subscription_url, params, opts) - initialize_from({ subscription: resp.data }, opts, true) - subscription - end - - def create_subscription(params = {}, opts = {}) - resp, opts = request(:post, subscriptions_url, params, opts) - initialize_from({ subscription: resp.data }, opts, true) - subscription - end - def delete_discount - _, opts = request(:delete, discount_url) - initialize_from({ discount: nil }, opts, true) - end - - private def discount_url - resource_url + "/discount" - end - - private def subscription_url - resource_url + "/subscription" - end - - private def subscriptions_url - resource_url + "/subscriptions" + resp, opts = request(:delete, resource_url + "/discount") + initialize_from(resp.data, opts, true) end end end diff --git a/lib/stripe/resources/dispute.rb b/lib/stripe/resources/dispute.rb index 59d9a92e4..0495828c1 100644 --- a/lib/stripe/resources/dispute.rb +++ b/lib/stripe/resources/dispute.rb @@ -17,11 +17,5 @@ def close(params = {}, opts = {}) opts: opts ) end - - def close_url - resource_url + "/close" - end - extend Gem::Deprecate - deprecate :close_url, :none, 2019, 11 end end diff --git a/lib/stripe/resources/file.rb b/lib/stripe/resources/file.rb index 41979485c..1d7e3ac06 100644 --- a/lib/stripe/resources/file.rb +++ b/lib/stripe/resources/file.rb @@ -32,8 +32,3 @@ def self.create(params = {}, opts = {}) end end end - -module Stripe - # For backwards compatibility, the `File` class is aliased to `FileUpload`. - FileUpload = File -end diff --git a/lib/stripe/resources/order.rb b/lib/stripe/resources/order.rb index 504e4638c..cbd6c339b 100644 --- a/lib/stripe/resources/order.rb +++ b/lib/stripe/resources/order.rb @@ -28,13 +28,5 @@ def return_order(params = {}, opts = {}) opts: opts ) end - - private def pay_url - resource_url + "/pay" - end - - private def returns_url - resource_url + "/returns" - end end end diff --git a/lib/stripe/resources/payout.rb b/lib/stripe/resources/payout.rb index e4cae3e24..4540f96ad 100644 --- a/lib/stripe/resources/payout.rb +++ b/lib/stripe/resources/payout.rb @@ -18,11 +18,5 @@ def cancel(params = {}, opts = {}) opts: opts ) end - - def cancel_url - resource_url + "/cancel" - end - extend Gem::Deprecate - deprecate :cancel_url, :none, 2019, 11 end end diff --git a/lib/stripe/resources/recipient.rb b/lib/stripe/resources/recipient.rb index 5e95bb87c..aeb768e76 100644 --- a/lib/stripe/resources/recipient.rb +++ b/lib/stripe/resources/recipient.rb @@ -9,9 +9,5 @@ class Recipient < APIResource include Stripe::APIOperations::Save OBJECT_NAME = "recipient" - - def transfers - Transfer.all({ recipient: id }, @api_key) - end end end diff --git a/lib/stripe/resources/source.rb b/lib/stripe/resources/source.rb index 5d41c4b41..6e4adf238 100644 --- a/lib/stripe/resources/source.rb +++ b/lib/stripe/resources/source.rb @@ -31,12 +31,6 @@ def detach(params = {}, opts = {}) initialize_from(resp.data, opts) end - def delete(params = {}, opts = {}) - detach(params, opts) - end - extend Gem::Deprecate - deprecate :delete, "#detach", 2017, 10 - def source_transactions(params = {}, opts = {}) resp, opts = request(:get, resource_url + "/source_transactions", params, opts) diff --git a/lib/stripe/resources/subscription.rb b/lib/stripe/resources/subscription.rb index bbb85b887..98f00b90b 100644 --- a/lib/stripe/resources/subscription.rb +++ b/lib/stripe/resources/subscription.rb @@ -14,12 +14,8 @@ class Subscription < APIResource save_nested_resource :source def delete_discount - _, opts = request(:delete, discount_url) - initialize_from({ discount: nil }, opts, true) - end - - private def discount_url - resource_url + "/discount" + resp, opts = request(:delete, resource_url + "/discount") + initialize_from(resp.data, opts, true) end end end diff --git a/lib/stripe/resources/transfer.rb b/lib/stripe/resources/transfer.rb index 127965b9f..a01e4c2e0 100644 --- a/lib/stripe/resources/transfer.rb +++ b/lib/stripe/resources/transfer.rb @@ -22,10 +22,5 @@ def cancel(params = {}, opts = {}) opts: opts ) end - - def cancel_url - resource_url + "/cancel" - end - deprecate :cancel_url, :none, 2019, 11 end end diff --git a/test/stripe/account_test.rb b/test/stripe/account_test.rb index 02aa0723f..e4a135e31 100644 --- a/test/stripe/account_test.rb +++ b/test/stripe/account_test.rb @@ -82,22 +82,6 @@ class AccountTest < Test::Unit::TestCase assert persons.data[0].is_a?(Stripe::Person) end - context "#bank_account=" do - should "warn that #bank_account= is deprecated" do - old_stderr = $stderr - $stderr = StringIO.new - begin - account = Stripe::Account.retrieve("acct_123") - account.bank_account = "tok_123" - message = "NOTE: Stripe::Account#bank_account= is " \ - "deprecated; use #external_account= instead" - assert_match Regexp.new(message), $stderr.string - ensure - $stderr = old_stderr - end - end - end - context "#deauthorize" do should "deauthorize an account" do account = Stripe::Account.retrieve("acct_123") diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index 8c3a40a91..82b4a3bca 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -271,14 +271,6 @@ class NestedTestAPIResource < APIResource assert_equal c.created, 12_345 end - should "accessing a property other than id or parent on an unfetched object should fetch it" do - stub_request(:get, "#{Stripe.api_base}/v1/charges") - .with(query: { customer: "cus_123" }) - .to_return(body: JSON.generate(customer_fixture)) - c = Stripe::Customer.new("cus_123") - c.charges - end - should "updating an object should issue a POST request with only the changed properties" do stub_request(:post, "#{Stripe.api_base}/v1/customers/cus_123") .with(body: { "description" => "another_mn" }) diff --git a/test/stripe/charge_test.rb b/test/stripe/charge_test.rb index 2d7bde167..9fe327a36 100644 --- a/test/stripe/charge_test.rb +++ b/test/stripe/charge_test.rb @@ -60,21 +60,5 @@ class ChargeTest < Test::Unit::TestCase assert charge.is_a?(Stripe::Charge) end end - - context "#mark_as_fraudulent" do - should "charges should be able to be marked as fraudulent" do - charge = Stripe::Charge.retrieve("ch_123") - charge = charge.mark_as_fraudulent - assert charge.is_a?(Stripe::Charge) - end - end - - context "#mark_as_safe" do - should "charges should be able to be marked as safe" do - charge = Stripe::Charge.retrieve("ch_123") - charge = charge.mark_as_safe - assert charge.is_a?(Stripe::Charge) - end - end end end diff --git a/test/stripe/customer_test.rb b/test/stripe/customer_test.rb index 1eefcfa74..370240e48 100644 --- a/test/stripe/customer_test.rb +++ b/test/stripe/customer_test.rb @@ -53,50 +53,6 @@ class CustomerTest < Test::Unit::TestCase end end - context "#create_subscription" do - should "create a new subscription" do - customer = Stripe::Customer.retrieve("cus_123") - subscription = customer.create_subscription(items: [{ plan: "silver" }]) - assert_requested :post, "#{Stripe.api_base}/v1/customers/#{customer.id}/subscriptions" - assert subscription.is_a?(Stripe::Subscription) - end - end - - context "#create_upcoming_invoice" do - should "create a new invoice" do - customer = Stripe::Customer.retrieve("cus_123") - invoice = customer.create_upcoming_invoice - assert_requested :post, "#{Stripe.api_base}/v1/invoices" - assert invoice.is_a?(Stripe::Invoice) - end - end - - context "#update_subscription" do - should "update a subscription" do - customer = Stripe::Customer.retrieve("cus_123") - - # deprecated API and not in schema - stub_request(:post, "#{Stripe.api_base}/v1/customers/#{customer.id}/subscription") - .with(body: { plan: "silver" }) - .to_return(body: JSON.generate(object: "subscription")) - subscription = customer.update_subscription(plan: "silver") - assert subscription.is_a?(Stripe::Subscription) - end - end - - context "#cancel_subscription" do - should "cancel a subscription" do - customer = Stripe::Customer.retrieve("cus_123") - - # deprecated API and not in schema - stub_request(:delete, "#{Stripe.api_base}/v1/customers/#{customer.id}/subscription") - .with(query: { at_period_end: "true" }) - .to_return(body: JSON.generate(object: "subscription")) - subscription = customer.cancel_subscription(at_period_end: "true") - assert subscription.is_a?(Stripe::Subscription) - end - end - context "#delete_discount" do should "delete a discount" do customer = Stripe::Customer.retrieve("cus_123") @@ -113,6 +69,7 @@ class CustomerTest < Test::Unit::TestCase assert discount.is_a?(Stripe::Discount) end end + context "#create_source" do should "create a source" do Stripe::Customer.create_source( diff --git a/test/stripe/file_upload_test.rb b/test/stripe/file_upload_test.rb deleted file mode 100644 index bd4010f87..000000000 --- a/test/stripe/file_upload_test.rb +++ /dev/null @@ -1,69 +0,0 @@ -# frozen_string_literal: true - -require ::File.expand_path("../test_helper", __dir__) - -module Stripe - # This is a strict copy of `FileTest`, except that it uses - # `Stripe::FileUpload` instead of `Stripe::File`. - class FileUploadTest < Test::Unit::TestCase - should "be listable" do - files = Stripe::FileUpload.list - assert_requested :get, "#{Stripe.api_base}/v1/files" - assert files.data.is_a?(Array) - assert files.data[0].is_a?(Stripe::FileUpload) - end - - should "be retrievable" do - file = Stripe::FileUpload.retrieve("file_123") - assert_requested :get, "#{Stripe.api_base}/v1/files/file_123" - assert file.is_a?(Stripe::FileUpload) - end - - context ".create" do - setup do - # We don't point to the same host for the API and uploads in - # production, but `stripe-mock` supports both APIs. - Stripe.uploads_base = Stripe.api_base - - # Set `api_base` to `nil` to ensure that these requests are _not_ sent - # to the default API hostname. `api_base` is reset when each test - # starts so this won't affect the global state. - Stripe.api_base = nil - end - - should "be creatable with a File" do - file = Stripe::FileUpload.create( - purpose: "dispute_evidence", - file: ::File.new(__FILE__), - file_link_data: { create: true } - ) - assert_requested :post, "#{Stripe.uploads_base}/v1/files" - assert file.is_a?(Stripe::FileUpload) - end - - should "be creatable with a Tempfile" do - tempfile = Tempfile.new("foo") - tempfile.write("Hello world") - tempfile.rewind - - file = Stripe::FileUpload.create( - purpose: "dispute_evidence", - file: tempfile, - file_link_data: { create: true } - ) - assert_requested :post, "#{Stripe.uploads_base}/v1/files" - assert file.is_a?(Stripe::FileUpload) - end - end - - should "be deserializable when `object=file`" do - file = Stripe::Util.convert_to_stripe_object({ object: "file" }, {}) - assert file.is_a?(Stripe::FileUpload) - end - - should "be deserializable when `object=file_upload`" do - file = Stripe::Util.convert_to_stripe_object({ object: "file_upload" }, {}) - assert file.is_a?(Stripe::FileUpload) - end - end -end diff --git a/test/stripe/source_test.rb b/test/stripe/source_test.rb index 02e78907d..5c2e55a1b 100644 --- a/test/stripe/source_test.rb +++ b/test/stripe/source_test.rb @@ -51,24 +51,6 @@ class SourceTest < Test::Unit::TestCase end end - context "#delete" do - should "warn that #delete is deprecated" do - old_stderr = $stderr - $stderr = StringIO.new - begin - source = Stripe::Source.construct_from(customer: "cus_123", - id: "src_123", - object: "source") - source.delete - message = "NOTE: Stripe::Source#delete is " \ - "deprecated; use #detach instead" - assert_match Regexp.new(message), $stderr.string - ensure - $stderr = old_stderr - end - end - end - should "not be listable" do assert_raises NoMethodError do Stripe::Source.list From fd004e5160003c031de809a8cfa40dfd04369d91 Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Wed, 7 Aug 2019 11:15:14 -0700 Subject: [PATCH 07/18] Remove all alias for list methods (#823) --- lib/stripe/api_operations/list.rb | 6 ------ test/stripe/list_object_test.rb | 16 ---------------- 2 files changed, 22 deletions(-) diff --git a/lib/stripe/api_operations/list.rb b/lib/stripe/api_operations/list.rb index 65c46adc0..5e118ce8c 100644 --- a/lib/stripe/api_operations/list.rb +++ b/lib/stripe/api_operations/list.rb @@ -19,12 +19,6 @@ def list(filters = {}, opts = {}) obj end - - # The original version of #list was given the somewhat unfortunate name of - # #all, and this alias allows us to maintain backward compatibility (the - # choice was somewhat misleading in the way that it only returned a single - # page rather than all objects). - alias all list end end end diff --git a/test/stripe/list_object_test.rb b/test/stripe/list_object_test.rb index 6774a5d7a..2970e1ec3 100644 --- a/test/stripe/list_object_test.rb +++ b/test/stripe/list_object_test.rb @@ -132,22 +132,6 @@ class ListObjectTest < Test::Unit::TestCase next_list = list.previous_page assert_equal({ expand: ["data.source"], limit: 3 }, next_list.filters) end - - # - # backward compatibility - # - - # note that the name #all is deprecated, as is using it fetch the next page - # in a list - should "be able to retrieve full lists given a listobject" do - c = Stripe::Charge.all - assert c.is_a?(Stripe::ListObject) - assert_equal("/v1/charges", c.resource_url) - all = c.all - assert all.is_a?(Stripe::ListObject) - assert_equal("/v1/charges", all.resource_url) - assert all.data.is_a?(Array) - end end end From b87c5a512f2a94e7a113687fbe8c29afb2ea454d Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Tue, 13 Aug 2019 13:04:23 -0700 Subject: [PATCH 08/18] Remove UsageRecord.create method (#826) --- lib/stripe/resources/usage_record.rb | 17 ----------------- test/stripe/usage_record_test.rb | 28 ---------------------------- 2 files changed, 45 deletions(-) delete mode 100644 test/stripe/usage_record_test.rb diff --git a/lib/stripe/resources/usage_record.rb b/lib/stripe/resources/usage_record.rb index f34987940..f6c08ae46 100644 --- a/lib/stripe/resources/usage_record.rb +++ b/lib/stripe/resources/usage_record.rb @@ -3,22 +3,5 @@ module Stripe class UsageRecord < APIResource OBJECT_NAME = "usage_record" - - def self.create(params = {}, opts = {}) - unless params.key?(:subscription_item) - raise ArgumentError, "Params must have a subscription_item key" - end - - req_params = params.clone.delete_if do |key, _value| - key == :subscription_item - end - resp, opts = request( - :post, - "/v1/subscription_items/#{params[:subscription_item]}/usage_records", - req_params, - opts - ) - Util.convert_to_stripe_object(resp.data, opts) - end end end diff --git a/test/stripe/usage_record_test.rb b/test/stripe/usage_record_test.rb deleted file mode 100644 index f6f7bd5ea..000000000 --- a/test/stripe/usage_record_test.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -require ::File.expand_path("../test_helper", __dir__) - -module Stripe - class UsageRecordTest < Test::Unit::TestCase - should "be creatable" do - usage_record = Stripe::UsageRecord.create( - quantity: 5000, - subscription_item: "si_abc", - timestamp: Time.now.to_i, - action: "increment" - ) - assert_requested :post, "#{Stripe.api_base}/v1/subscription_items/si_abc/usage_records" - assert usage_record.is_a?(Stripe::UsageRecord) - end - - should "raise when subscription_item is missing" do - assert_raise ArgumentError do - Stripe::UsageRecord.create( - quantity: 5000, - timestamp: Time.now.to_i, - action: "increment" - ) - end - end - end -end From 56ea5f4292b3613cbbec9ae6e025b017559513fa Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Thu, 15 Aug 2019 14:39:29 -0700 Subject: [PATCH 09/18] Remove IssuerFraudRecord (#827) --- lib/stripe/object_types.rb | 1 - lib/stripe/resources.rb | 1 - lib/stripe/resources/issuer_fraud_record.rb | 9 --------- test/stripe/issuer_fraud_record_test.rb | 20 -------------------- 4 files changed, 31 deletions(-) delete mode 100644 lib/stripe/resources/issuer_fraud_record.rb delete mode 100644 test/stripe/issuer_fraud_record_test.rb diff --git a/lib/stripe/object_types.rb b/lib/stripe/object_types.rb index 284790293..c1510f8d0 100644 --- a/lib/stripe/object_types.rb +++ b/lib/stripe/object_types.rb @@ -41,7 +41,6 @@ def self.object_names_to_classes Invoice::OBJECT_NAME => Invoice, InvoiceItem::OBJECT_NAME => InvoiceItem, InvoiceLineItem::OBJECT_NAME => InvoiceLineItem, - IssuerFraudRecord::OBJECT_NAME => IssuerFraudRecord, Issuing::Authorization::OBJECT_NAME => Issuing::Authorization, Issuing::Card::OBJECT_NAME => Issuing::Card, Issuing::CardDetails::OBJECT_NAME => Issuing::CardDetails, diff --git a/lib/stripe/resources.rb b/lib/stripe/resources.rb index 81362a5c3..0064d5a93 100644 --- a/lib/stripe/resources.rb +++ b/lib/stripe/resources.rb @@ -30,7 +30,6 @@ require "stripe/resources/invoice" require "stripe/resources/invoice_item" require "stripe/resources/invoice_line_item" -require "stripe/resources/issuer_fraud_record" require "stripe/resources/issuing/authorization" require "stripe/resources/issuing/card" require "stripe/resources/issuing/card_details" diff --git a/lib/stripe/resources/issuer_fraud_record.rb b/lib/stripe/resources/issuer_fraud_record.rb deleted file mode 100644 index 43eb11334..000000000 --- a/lib/stripe/resources/issuer_fraud_record.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -module Stripe - class IssuerFraudRecord < APIResource - extend Stripe::APIOperations::List - - OBJECT_NAME = "issuer_fraud_record" - end -end diff --git a/test/stripe/issuer_fraud_record_test.rb b/test/stripe/issuer_fraud_record_test.rb deleted file mode 100644 index 82f11d420..000000000 --- a/test/stripe/issuer_fraud_record_test.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -require ::File.expand_path("../test_helper", __dir__) - -module Stripe - class IssuerFraudRecordTest < Test::Unit::TestCase - should "be listable" do - issfrs = Stripe::IssuerFraudRecord.list - assert_requested :get, "#{Stripe.api_base}/v1/issuer_fraud_records" - assert issfrs.data.is_a?(Array) - assert issfrs.data[0].is_a?(Stripe::IssuerFraudRecord) - end - - should "be retrievable" do - issfr = Stripe::IssuerFraudRecord.retrieve("issfr_123") - assert_requested :get, "#{Stripe.api_base}/v1/issuer_fraud_records/issfr_123" - assert issfr.is_a?(Stripe::IssuerFraudRecord) - end - end -end From e840ce89bc9641ebc64b440e5c11be42714ffd46 Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Fri, 16 Aug 2019 10:18:48 -0700 Subject: [PATCH 10/18] Add ErrorObject to StripeError exceptions (#811) --- lib/stripe.rb | 1 + lib/stripe/error_object.rb | 94 ++++++++++++++++++++++++++++++++++++++ lib/stripe/errors.rb | 14 ++++++ test/stripe/errors_test.rb | 37 +++++++++++---- 4 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 lib/stripe/error_object.rb diff --git a/lib/stripe.rb b/lib/stripe.rb index 17b67b9ae..2cdc9f913 100644 --- a/lib/stripe.rb +++ b/lib/stripe.rb @@ -34,6 +34,7 @@ require "stripe/stripe_object" require "stripe/stripe_response" require "stripe/list_object" +require "stripe/error_object" require "stripe/api_resource" require "stripe/singleton_api_resource" require "stripe/webhook" diff --git a/lib/stripe/error_object.rb b/lib/stripe/error_object.rb new file mode 100644 index 000000000..ea6be9e89 --- /dev/null +++ b/lib/stripe/error_object.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module Stripe + # Represents an error object as returned by the API. + # + # @see https://stripe.com/docs/api/errors + class ErrorObject < StripeObject + # Unlike other objects, we explicitly declare getter methods here. This + # is because the API doesn't return `null` values for fields on this + # object, rather the fields are omitted entirely. Not declaring the getter + # methods would cause users to run into `NoMethodError` exceptions and + # get in the way of generic error handling. + + # For card errors, the ID of the failed charge. + def charge + @values[:charge] + end + + # For some errors that could be handled programmatically, a short string + # indicating the error code reported. + def code + @values[:code] + end + + # For card errors resulting from a card issuer decline, a short string + # indicating the card issuer's reason for the decline if they provide one. + def decline_code + @values[:decline_code] + end + + # A URL to more information about the error code reported. + def doc_url + @values[:doc_url] + end + + # A human-readable message providing more details about the error. For card + # errors, these messages can be shown to your users. + def message + @values[:message] + end + + # If the error is parameter-specific, the parameter related to the error. + # For example, you can use this to display a message near the correct form + # field. + def param + @values[:param] + end + + # The PaymentIntent object for errors returned on a request involving a + # PaymentIntent. + def payment_intent + @values[:payment_intent] + end + + # The PaymentMethod object for errors returned on a request involving a + # PaymentMethod. + def payment_method + @values[:payment_method] + end + + # The SetupIntent object for errors returned on a request involving a + # SetupIntent. + def setup_intent + @values[:setup_intent] + end + + # The source object for errors returned on a request involving a source. + def source + @values[:source] + end + + # The type of error returned. One of `api_connection_error`, `api_error`, + # `authentication_error`, `card_error`, `idempotency_error`, + # `invalid_request_error`, or `rate_limit_error`. + def type + @values[:type] + end + end + + # Represents on OAuth error returned by the OAuth API. + # + # @see https://stripe.com/docs/connect/oauth-reference#post-token-errors + class OAuthErrorObject < StripeObject + # A unique error code per error type. + def error + @values[:error] + end + + # A human readable description of the error. + def error_description + @values[:error_description] + end + end +end diff --git a/lib/stripe/errors.rb b/lib/stripe/errors.rb index 24254e126..3a0af708e 100644 --- a/lib/stripe/errors.rb +++ b/lib/stripe/errors.rb @@ -11,6 +11,7 @@ class StripeError < StandardError attr_accessor :response attr_reader :code + attr_reader :error attr_reader :http_body attr_reader :http_headers attr_reader :http_status @@ -27,6 +28,13 @@ def initialize(message = nil, http_status: nil, http_body: nil, @json_body = json_body @code = code @request_id = @http_headers[:request_id] + @error = construct_error_object + end + + def construct_error_object + return nil if @json_body.nil? || !@json_body.key?(:error)\ + + ErrorObject.construct_from(@json_body[:error]) end def to_s @@ -118,6 +126,12 @@ def initialize(code, description, http_status: nil, http_body: nil, json_body: json_body, http_headers: http_headers, code: code) end + + def construct_error_object + return nil if @json_body.nil? + + OAuthErrorObject.construct_from(@json_body) + end end # InvalidClientError is raised when the client doesn't belong to you, or diff --git a/test/stripe/errors_test.rb b/test/stripe/errors_test.rb index adb836372..817ea13ef 100644 --- a/test/stripe/errors_test.rb +++ b/test/stripe/errors_test.rb @@ -4,16 +4,37 @@ module Stripe class StripeErrorTest < Test::Unit::TestCase - context "#to_s" do - should "convert to string" do - e = StripeError.new("message") - assert_equal "message", e.to_s + context "StripeError" do + context "#initialize" do + should "initialize error if json_body is set" do + e = StripeError.new("message", json_body: { error: { code: "some_error" } }) + assert_not_nil e.error + assert_equal "some_error", e.error.code + assert_nil e.error.charge + end + end + + context "#to_s" do + should "convert to string" do + e = StripeError.new("message") + assert_equal "message", e.to_s + + e = StripeError.new("message", http_status: 200) + assert_equal "(Status 200) message", e.to_s - e = StripeError.new("message", http_status: 200) - assert_equal "(Status 200) message", e.to_s + e = StripeError.new("message", http_status: nil, http_body: nil, json_body: nil, http_headers: { request_id: "request-id" }) + assert_equal "(Request request-id) message", e.to_s + end + end + end - e = StripeError.new("message", http_status: nil, http_body: nil, json_body: nil, http_headers: { request_id: "request-id" }) - assert_equal "(Request request-id) message", e.to_s + context "OAuth::OAuthError" do + context "#initialize" do + should "initialize error if json_body is set" do + e = OAuth::OAuthError.new("message", "description", json_body: { error: "some_oauth_error" }) + assert_not_nil e.error + assert_equal "some_oauth_error", e.error.error + end end end end From 084276b4b8c85974fc9c177d18a105ff6bc72c25 Mon Sep 17 00:00:00 2001 From: Brandur Date: Fri, 16 Aug 2019 16:45:21 -0700 Subject: [PATCH 11/18] Tweak retry logic to be a little more like stripe-node (#828) Tweaks the retry logic to be a little more like stripe-node's. In particular, we also retry under these conditions: * If we receive a 500 on a non-`POST` request. * If we receive a 503. I made it slightly different from stripe-node which checks for a 500 with `>= 500`. I don't really like that -- if we want to retry specific status codes we should be explicit about it. We're actively re-examining ways on how to make it easier for clients to figure out when to retry right now, but I figure V5 is a good time to tweak this because the modifications change the method signature of `should_retry?` slightly, and it's technically a public method. --- .rubocop.yml | 8 ++++++-- .rubocop_todo.yml | 2 +- lib/stripe/stripe_client.rb | 25 ++++++++++++++++++++----- test/stripe/stripe_client_test.rb | 28 ++++++++++++++++++++-------- 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 3c7513949..1c7113778 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -19,16 +19,20 @@ Layout/IndentFirstHashElement: Layout/IndentHeredoc: Enabled: false -Metrics/LineLength: +Metrics/ClassLength: Exclude: - "test/**/*.rb" + +Metrics/LineLength: + Exclude: - "lib/stripe/resources/**/*.rb" + - "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 + Max: 55 Metrics/ModuleLength: Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index cd8c6f5c5..5d1ea8c17 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -8,7 +8,7 @@ # Offense count: 23 Metrics/AbcSize: - Max: 48 + Max: 51 # Offense count: 33 # Configuration parameters: CountComments, ExcludedMethods. diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 52c42bdb6..5e09aa633 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -39,7 +39,7 @@ def self.default_connection_manager # Checks if an error is a problem that we should retry on. This includes # both socket errors that may represent an intermittent problem and some # special HTTP statuses. - def self.should_retry?(error, num_retries) + def self.should_retry?(error, method:, num_retries:) return false if num_retries >= Stripe.max_network_retries # Retry on timeout-related problems (either on open or read). @@ -53,8 +53,18 @@ def self.should_retry?(error, num_retries) return true if error.is_a?(SocketError) if error.is_a?(Stripe::StripeError) - # 409 conflict + # 409 Conflict return true if error.http_status == 409 + + # 500 Internal Server Error + # + # We only bother retrying these for non-POST requests. POSTs end up + # being cached by the idempotency layer so there's no purpose in + # retrying them. + return true if error.http_status == 500 && method == :post + + # 503 Service Unavailable + return true if error.http_status == 503 end false @@ -99,6 +109,11 @@ def request def execute_request(method, path, api_base: nil, api_key: nil, headers: {}, params: {}) + raise ArgumentError, "method should be a symbol" \ + unless method.is_a?(Symbol) + raise ArgumentError, "path should be a string" \ + unless path.is_a?(String) + api_base ||= Stripe.api_base api_key ||= Stripe.api_key params = Util.objects_to_ids(params) @@ -159,7 +174,7 @@ def execute_request(method, path, context.path = path context.query_params = query - http_resp = execute_request_with_rescues(api_base, context) do + http_resp = execute_request_with_rescues(method, api_base, context) do connection_manager.execute_request(method, url, body: body, headers: headers, @@ -276,7 +291,7 @@ def execute_request(method, path, [body, body_log] end - private def execute_request_with_rescues(api_base, context) + private def execute_request_with_rescues(method, api_base, context) num_retries = 0 begin request_start = Time.now @@ -310,7 +325,7 @@ def execute_request(method, path, log_response_error(error_context, request_start, e) end - if self.class.should_retry?(e, num_retries) + if self.class.should_retry?(e, method: method, num_retries: num_retries) num_retries += 1 sleep self.class.sleep_time(num_retries) retry diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 6fd167f7b..74e5b9f6c 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -54,31 +54,43 @@ class StripeClientTest < Test::Unit::TestCase end should "retry on Errno::ECONNREFUSED" do - assert StripeClient.should_retry?(Errno::ECONNREFUSED.new, 0) + assert StripeClient.should_retry?(Errno::ECONNREFUSED.new, + method: :post, num_retries: 0) end should "retry on Net::OpenTimeout" do - assert StripeClient.should_retry?(Net::OpenTimeout.new, 0) + assert StripeClient.should_retry?(Net::OpenTimeout.new, + method: :post, num_retries: 0) end should "retry on Net::ReadTimeout" do - assert StripeClient.should_retry?(Net::ReadTimeout.new, 0) + assert StripeClient.should_retry?(Net::ReadTimeout.new, + method: :post, num_retries: 0) end should "retry on SocketError" do - assert StripeClient.should_retry?(SocketError.new, 0) + assert StripeClient.should_retry?(SocketError.new, + method: :post, num_retries: 0) end - should "retry on a conflict" do - assert StripeClient.should_retry?(Stripe::StripeError.new(http_status: 409), 0) + should "retry on a 409 Conflict" do + assert StripeClient.should_retry?(Stripe::StripeError.new(http_status: 409), + method: :post, num_retries: 0) + end + + should "retry on a 503 Service Unavailable" do + assert StripeClient.should_retry?(Stripe::StripeError.new(http_status: 503), + method: :post, num_retries: 0) end should "not retry at maximum count" do - refute StripeClient.should_retry?(RuntimeError.new, Stripe.max_network_retries) + refute StripeClient.should_retry?(RuntimeError.new, + method: :post, num_retries: Stripe.max_network_retries) end should "not retry on a certificate validation error" do - refute StripeClient.should_retry?(OpenSSL::SSL::SSLError.new, 0) + refute StripeClient.should_retry?(OpenSSL::SSL::SSLError.new, + method: :post, num_retries: 0) end end From 4d52345a50c80859579dd669209b81bb00cdaa41 Mon Sep 17 00:00:00 2001 From: Brandur Date: Fri, 16 Aug 2019 17:31:57 -0700 Subject: [PATCH 12/18] Fix inverted sign for 500 retries (#830) I messed up in #828 by (1) accidentally flipping the comparison against `:post` when checking whether to retry on 500, and (2) forgetting to write new tests for the condition, which is how (1) got through. This patch fixes both those problems. --- lib/stripe/stripe_client.rb | 2 +- test/stripe/stripe_client_test.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 5e09aa633..26025e3d6 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -61,7 +61,7 @@ def self.should_retry?(error, method:, num_retries:) # We only bother retrying these for non-POST requests. POSTs end up # being cached by the idempotency layer so there's no purpose in # retrying them. - return true if error.http_status == 500 && method == :post + return true if error.http_status == 500 && method != :post # 503 Service Unavailable return true if error.http_status == 503 diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 74e5b9f6c..89b752890 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -78,6 +78,11 @@ class StripeClientTest < Test::Unit::TestCase method: :post, num_retries: 0) end + should "retry on a 500 Internal Server Error when non-POST" do + assert StripeClient.should_retry?(Stripe::StripeError.new(http_status: 500), + method: :get, num_retries: 0) + end + should "retry on a 503 Service Unavailable" do assert StripeClient.should_retry?(Stripe::StripeError.new(http_status: 503), method: :post, num_retries: 0) @@ -92,6 +97,11 @@ class StripeClientTest < Test::Unit::TestCase refute StripeClient.should_retry?(OpenSSL::SSL::SSLError.new, method: :post, num_retries: 0) end + + should "not retry on a 500 Internal Server Error when POST" do + refute StripeClient.should_retry?(Stripe::StripeError.new(http_status: 500), + method: :post, num_retries: 0) + end end context ".sleep_time" do From 0b58fb8f7d6b001ef9b8940a27afaa0e88db70c1 Mon Sep 17 00:00:00 2001 From: Brandur Date: Fri, 16 Aug 2019 17:40:29 -0700 Subject: [PATCH 13/18] Remove a few more very old deprecated methods (#831) I noticed that we had a couple of other deprecated methods on `Stripe` and `StripeObject` that have been around for a long time. May as well get rid of them too -- luckily they were using `Gem::Deprecate` so they've been producing annoying deprecated warnings for quite a while now. --- lib/stripe.rb | 10 ---------- lib/stripe/stripe_object.rb | 22 ---------------------- test/stripe/stripe_object_test.rb | 28 ---------------------------- test/stripe_test.rb | 13 ------------- 4 files changed, 73 deletions(-) diff --git a/lib/stripe.rb b/lib/stripe.rb index 2cdc9f913..bf8aecd1e 100644 --- a/lib/stripe.rb +++ b/lib/stripe.rb @@ -196,16 +196,6 @@ def self.set_app_info(name, partner_id: nil, url: nil, version: nil) version: version, } end - - # DEPRECATED. Use `Util#encode_parameters` instead. - def self.uri_encode(params) - Util.encode_parameters(params) - end - private_class_method :uri_encode - class << self - extend Gem::Deprecate - deprecate :uri_encode, "Stripe::Util#encode_parameters", 2016, 1 - end end Stripe.log_level = ENV["STRIPE_LOG"] unless ENV["STRIPE_LOG"].nil? diff --git a/lib/stripe/stripe_object.rb b/lib/stripe/stripe_object.rb index f0f16b579..aec74b1cb 100644 --- a/lib/stripe/stripe_object.rb +++ b/lib/stripe/stripe_object.rb @@ -127,18 +127,6 @@ def inspect JSON.pretty_generate(@values) end - # Re-initializes the object based on a hash of values (usually one that's - # come back from an API call). Adds or removes value accessors as necessary - # and updates the state of internal data. - # - # Please don't use this method. If you're trying to do mass assignment, try - # #initialize_from instead. - def refresh_from(values, opts, partial = false) - initialize_from(values, opts, partial) - end - extend Gem::Deprecate - deprecate :refresh_from, "#update_attributes", 2016, 1 - # Mass assigns attributes on the model. # # This is a version of +update_attributes+ that takes some extra options @@ -271,16 +259,6 @@ def serialize_params(options = {}) update_hash end - class << self - # This class method has been deprecated in favor of the instance method - # of the same name. - def serialize_params(obj, options = {}) - obj.serialize_params(options) - end - extend Gem::Deprecate - deprecate :serialize_params, "#serialize_params", 2016, 9 - end - # A protected field is one that doesn't get an accessor assigned to it # (i.e. `obj.public = ...`) and one which is not allowed to be updated via # the class level `Model.update(id, { ... })`. diff --git a/test/stripe/stripe_object_test.rb b/test/stripe/stripe_object_test.rb index d22b50468..44b5e136e 100644 --- a/test/stripe/stripe_object_test.rb +++ b/test/stripe/stripe_object_test.rb @@ -235,20 +235,6 @@ def to_hash assert_equal true, obj.send(:metaclass).method_defined?(:foo) end - should "warn that #refresh_from is deprecated" do - old_stderr = $stderr - $stderr = StringIO.new - begin - obj = Stripe::StripeObject.construct_from({}) - obj.refresh_from({}, {}) - message = "NOTE: Stripe::StripeObject#refresh_from is " \ - "deprecated; use #update_attributes instead" - assert_match Regexp.new(message), $stderr.string - ensure - $stderr = old_stderr - end - end - should "pass opts down to children when initializing" do opts = { custom: "opts" } @@ -475,20 +461,6 @@ class WithAdditiveObjectParam < Stripe::StripeObject assert_equal(expected, obj.to_s) end - should "warn that .serialize_params is deprecated" do - old_stderr = $stderr - $stderr = StringIO.new - begin - obj = Stripe::StripeObject.construct_from({}) - Stripe::StripeObject.serialize_params(obj) - message = "NOTE: Stripe::StripeObject.serialize_params is " \ - "deprecated; use #serialize_params instead" - assert_match Regexp.new(message), $stderr.string - ensure - $stderr = old_stderr - end - end - should "error on setting a property to an empty string" do obj = Stripe::StripeObject.construct_from(foo: "bar") e = assert_raises ArgumentError do diff --git a/test/stripe_test.rb b/test/stripe_test.rb index 30842bb10..bdc283c27 100644 --- a/test/stripe_test.rb +++ b/test/stripe_test.rb @@ -3,19 +3,6 @@ require ::File.expand_path("test_helper", __dir__) class StripeTest < Test::Unit::TestCase - should "warn that #refresh_from is deprecated" do - old_stderr = $stderr - $stderr = StringIO.new - begin - Stripe.uri_encode({}) - message = "NOTE: Stripe.uri_encode is deprecated; use " \ - "Stripe::Util#encode_parameters instead" - assert_match Regexp.new(message), $stderr.string - ensure - $stderr = old_stderr - end - end - should "allow app_info to be configured" do begin old = Stripe.app_info From a1249af10a00c8ec944f1a5706afada1ce1ca023 Mon Sep 17 00:00:00 2001 From: Brandur Date: Mon, 19 Aug 2019 11:47:01 -0700 Subject: [PATCH 14/18] Remove extraneous slash at the end of the line --- lib/stripe/errors.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stripe/errors.rb b/lib/stripe/errors.rb index 3a0af708e..d4865655f 100644 --- a/lib/stripe/errors.rb +++ b/lib/stripe/errors.rb @@ -32,7 +32,7 @@ def initialize(message = nil, http_status: nil, http_body: nil, end def construct_error_object - return nil if @json_body.nil? || !@json_body.key?(:error)\ + return nil if @json_body.nil? || !@json_body.key?(:error) ErrorObject.construct_from(@json_body[:error]) end From a9a7af0f1a8e1b49e4a16d18f0b7e91b192f3182 Mon Sep 17 00:00:00 2001 From: Brandur Date: Mon, 19 Aug 2019 13:11:54 -0700 Subject: [PATCH 15/18] Reset connections when connection-changing configuration changes (#829) Adds a few basic features around connection and connection manager management: * `clear` on connection manager, which calls `finish` on each active connection and then disposes of it. * A centralized cross-thread tracking system for connection managers in `StripeClient` and `clear_all_connection_managers` which clears all known connection managers across all threads in a thread-safe way. The addition of these allow us to modify the implementation of some of our configuration on `Stripe` so that it can reset all currently open connections when its value changes. This fixes a currently problem with the library whereby certain configuration must be set before the first request or it remains fixed on any open connections. For example, if `Stripe.proxy` is set after a request is made from the library, it has no effect because the proxy must have been set when the connection was originally being initialized. The impetus for getting this out is that I noticed that we will need this internally in a few places when we're upgrading to stripe-ruby V5. Those spots used to be able to hack around the unavailability of this feature by just accessing the Faraday connection directly and resetting state on it, but in V5 `StripeClient#conn` is gone, and that's no longer possible. --- lib/stripe.rb | 62 +++++++++++++++++++++++--- lib/stripe/connection_manager.rb | 9 ++++ lib/stripe/stripe_client.rb | 30 ++++++++++++- test/stripe/connection_manager_test.rb | 17 +++++++ test/stripe/stripe_client_test.rb | 44 ++++++++++++++++++ 5 files changed, 153 insertions(+), 9 deletions(-) diff --git a/lib/stripe.rb b/lib/stripe.rb index bf8aecd1e..4f501077e 100644 --- a/lib/stripe.rb +++ b/lib/stripe.rb @@ -73,9 +73,20 @@ module Stripe @enable_telemetry = true class << self - attr_accessor :stripe_account, :api_key, :api_base, :verify_ssl_certs, - :api_version, :client_id, :connect_base, :uploads_base, - :open_timeout, :read_timeout, :proxy + attr_accessor :api_key + attr_accessor :api_version + 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 @@ -90,6 +101,11 @@ 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. @@ -102,6 +118,8 @@ def self.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 @@ -121,6 +139,19 @@ def self.ca_store end end + def self.connection_base=(connection_base) + @connection_base = connection_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 @@ -175,12 +206,19 @@ def self.max_network_retries=(val) @max_network_retries = val.to_i end - def self.enable_telemetry? - @enable_telemetry + def self.open_timeout=(open_timeout) + @open_timeout = open_timeout + StripeClient.clear_all_connection_managers end - def self.enable_telemetry=(val) - @enable_telemetry = val + 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 @@ -196,6 +234,16 @@ 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/connection_manager.rb b/lib/stripe/connection_manager.rb index 30bea4ab5..4188b92d3 100644 --- a/lib/stripe/connection_manager.rb +++ b/lib/stripe/connection_manager.rb @@ -20,6 +20,15 @@ def initialize @active_connections = {} end + # Finishes any active connections by closing their TCP connection and + # clears them from internal tracking. + def clear + @active_connections.each do |_, connection| + connection.finish + end + @active_connections = {} + end + # Gets a connection for a given URI. This is for internal use only as it's # subject to change (we've moved between HTTP client schemes in the past # and may do it again). diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 26025e3d6..2f2a65d1f 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -5,6 +5,11 @@ module Stripe # recover both a resource a call returns as well as a response object that # contains information on the HTTP call. class StripeClient + # A set of all known connection managers across all threads and a mutex to + # synchronize global access to them. + @all_connection_managers = [] + @all_connection_managers_mutex = Mutex.new + attr_accessor :connection_manager # Initializes a new `StripeClient`. Expects a `ConnectionManager` object, @@ -24,6 +29,20 @@ def self.active_client Thread.current[:stripe_client] || default_client end + # Finishes any active connections by closing their TCP connection and + # clears them from internal tracking in all connection managers across all + # threads. + def self.clear_all_connection_managers + # Just a quick path for when configuration is being set for the first + # time before any connections have been opened. There is technically some + # potential for thread raciness here, but not in a practical sense. + return if @all_connection_managers.empty? + + @all_connection_managers_mutex.synchronize do + @all_connection_managers.each(&:clear) + end + end + # A default client for the current thread. def self.default_client Thread.current[:stripe_client_default_client] ||= @@ -32,8 +51,15 @@ def self.default_client # A default connection manager for the current thread. def self.default_connection_manager - Thread.current[:stripe_client_default_connection_manager] ||= - ConnectionManager.new + Thread.current[:stripe_client_default_connection_manager] ||= begin + connection_manager = ConnectionManager.new + + @all_connection_managers_mutex.synchronize do + @all_connection_managers << connection_manager + end + + connection_manager + end end # Checks if an error is a problem that we should retry on. This includes diff --git a/test/stripe/connection_manager_test.rb b/test/stripe/connection_manager_test.rb index 74f4f0ae4..c74af59e3 100644 --- a/test/stripe/connection_manager_test.rb +++ b/test/stripe/connection_manager_test.rb @@ -8,6 +8,23 @@ class ConnectionManagerTest < Test::Unit::TestCase @manager = Stripe::ConnectionManager.new end + context "#clear" do + should "clear any active connections" do + stub_request(:post, "#{Stripe.api_base}/path") + .to_return(body: JSON.generate(object: "account")) + + # Making a request lets us know that at least one connection is open. + @manager.execute_request(:post, "#{Stripe.api_base}/path") + + # Now clear the manager. + @manager.clear + + # This check isn't great, but it's otherwise difficult to tell that + # anything happened with just the public-facing API. + assert_equal({}, @manager.instance_variable_get(:@active_connections)) + end + end + context "#connection_for" do should "correctly initialize a connection" do old_proxy = Stripe.proxy diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 89b752890..647892010 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -17,6 +17,50 @@ class StripeClientTest < Test::Unit::TestCase end end + context ".clear_all_connection_managers" do + should "clear connection managers across all threads" do + stub_request(:post, "#{Stripe.api_base}/path") + .to_return(body: JSON.generate(object: "account")) + + num_threads = 3 + + # Poorly named class -- note this is actually a concurrent queue. + recv_queue = Queue.new + send_queue = Queue.new + + threads = num_threads.times.map do |_| + Thread.start do + # Causes a connection manager to be created on this thread and a + # connection within that manager to be created for API access. + manager = StripeClient.default_connection_manager + manager.execute_request(:post, "#{Stripe.api_base}/path") + + # Signal to the main thread we're ready. + recv_queue << true + + # Wait for the main thread to signal continue. + send_queue.pop + + # This check isn't great, but it's otherwise difficult to tell that + # anything happened with just the public-facing API. + assert_equal({}, manager.instance_variable_get(:@active_connections)) + end + end + + # Wait for threads to start up. + threads.each { recv_queue.pop } + + # Do the clear (the method we're actually trying to test). + StripeClient.clear_all_connection_managers + + # Tell threads to run their check. + threads.each { send_queue << true } + + # And finally, give all threads time to perform their check. + threads.each(&:join) + end + end + context ".default_client" do should "be a StripeClient" do assert_kind_of StripeClient, StripeClient.default_client From 0abfc55e26f4de55ec2fca2ba733848b553147d3 Mon Sep 17 00:00:00 2001 From: Brandur Date: Mon, 19 Aug 2019 15:57:23 -0700 Subject: [PATCH 16/18] Minor cleanup in `StripeClient` (#832) I ended up having to relax the maximum method line length in a few previous PRs, so I wanted to try one more cleanup pass in `execute_request` to see if I could get it back at all. The answer was "not by much" (without reducing clarity), but I found a few places that could be tweaked. Unfortunately, ~50 lines is probably the "right" length for this method in that you _could_ extract it further, but you'd end up passing huge amounts of state all over the place in method parameters, and it really wouldn't look that good. --- .rubocop.yml | 2 +- lib/stripe/stripe_client.rb | 59 +++++++++++++++++-------------- test/stripe/stripe_client_test.rb | 2 +- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 1c7113778..6e531c80f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -32,7 +32,7 @@ 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: 55 + Max: 50 Metrics/ModuleLength: Enabled: false diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 2f2a65d1f..9714482dd 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -148,45 +148,28 @@ def execute_request(method, path, body_params = nil query_params = nil - case method.to_s.downcase.to_sym + case method when :get, :head, :delete query_params = params else body_params = params end - # This works around an edge case where we end up with both query - # parameters in `query_params` and query parameters that are appended - # onto the end of the given path. - # - # Here we decode any parameters that were added onto the end of a path - # and add them to `query_params` so that all parameters end up in one - # place and all of them are correctly included in the final request. - u = URI.parse(path) - unless u.query.nil? - query_params ||= {} - query_params = Hash[URI.decode_www_form(u.query)].merge(query_params) - - # Reset the path minus any query parameters that were specified. - path = u.path - end + query_params, path = merge_query_params(query_params, path) headers = request_headers(api_key, method) .update(Util.normalize_headers(headers)) url = api_url(path, api_base) + # Merge given query parameters with any already encoded in the path. query = query_params ? Util.encode_parameters(query_params) : nil # Encoding body parameters is a little more complex because we may have # to send a multipart-encoded body. `body_log` is produced separately as - # a variant of the encoded form that's output-friendly. e.g. Doesn't show - # as multipart. File objects are displayed as such instead of as their - # file contents. - body, body_log = if body_params - encode_body(body_params, headers) - else - [nil, nil] - end + # a log-friendly variant of the encoded form. File objects are displayed + # as such instead of as their file contents. + body, body_log = + body_params ? encode_body(body_params, headers) : [nil, nil] # stores information on the request we're about to make so that we don't # have to pass as many parameters around for logging. @@ -198,7 +181,7 @@ def execute_request(method, path, context.idempotency_key = headers["Idempotency-Key"] context.method = method context.path = path - context.query_params = query + context.query = query http_resp = execute_request_with_rescues(method, api_base, context) do connection_manager.execute_request(method, url, @@ -410,6 +393,28 @@ def execute_request(method, path, raise(error) end + # Works around an edge case where we end up with both query parameters from + # parameteers and query parameters that were appended onto the end of the + # given path. + # + # Decode any parameters that were added onto the end of a path and add them + # to a unified query parameter hash so that all parameters end up in one + # place and all of them are correctly included in the final request. + private def merge_query_params(query_params, path) + u = URI.parse(path) + + # Return original results if there was nothing to be found. + return query_params, path if u.query.nil? + + query_params ||= {} + query_params = Hash[URI.decode_www_form(u.query)].merge(query_params) + + # Reset the path minus any query parameters that were specified. + path = u.path + + [query_params, path] + end + private def specific_api_error(resp, error_data, context) Util.log_error("Stripe API error", status: resp.http_status, @@ -572,7 +577,7 @@ def execute_request(method, path, Util.log_debug("Request details", body: context.body, idempotency_key: context.idempotency_key, - query_params: context.query_params) + query: context.query) end private def log_response(context, request_start, status, body) @@ -619,7 +624,7 @@ class RequestLogContext attr_accessor :idempotency_key attr_accessor :method attr_accessor :path - attr_accessor :query_params + attr_accessor :query attr_accessor :request_id # The idea with this method is that we might want to update some of diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 647892010..f24e85f34 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -252,7 +252,7 @@ class StripeClientTest < Test::Unit::TestCase Util.expects(:log_debug).with("Request details", body: "", idempotency_key: "abc", - query_params: nil) + query: nil) Util.expects(:log_info).with("Response from Stripe API", account: "acct_123", From 95ac3de0a218d77788664cebbd06dc95f48279f2 Mon Sep 17 00:00:00 2001 From: Brandur Date: Tue, 20 Aug 2019 10:49:16 -0700 Subject: [PATCH 17/18] Do better bookkeeping when tracking state in `Thread.current` (#833) This is largely just another cleanup patch, but does a couple main things: * Hoists the `last_response` value into thread state. This is a very minor nicety, but effectively makes `StripeClient` fully thread-safe, which seems like a minor nicety. Two calls to `#request` to the same `StripeObject` can now be executed on two different threads and their results won't interfere with each other. * Moves state off one-off `Thread.current` keys and into a single one for the whole client which stores a new simple type of record called `ThreadContext`. Again, this doesn't change much, but adds some minor type safety and lets us document each field we expect to have in a thread's context. --- lib/stripe/stripe_client.rb | 84 ++++++++++++++++++++---- test/stripe/stripe_client_test.rb | 102 ++++++++++++++++++++++++++++-- 2 files changed, 170 insertions(+), 16 deletions(-) diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 9714482dd..f981089ab 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -21,17 +21,23 @@ def initialize(connection_manager = nil) @last_request_metrics = nil end - # For internal use: sets a value on the current thread's store when + # 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 # the original value (hopefully `nil`) after the block ends. + # + # For internal use only. Does not provide a stable API and may be broken + # with future non-major changes. def self.active_client - Thread.current[:stripe_client] || default_client + current_thread_context.active_client || default_client end # Finishes any active connections by closing their TCP connection and # clears them from internal tracking in all connection managers across all # threads. + # + # For internal use only. Does not provide a stable API and may be broken + # with future non-major changes. def self.clear_all_connection_managers # Just a quick path for when configuration is being set for the first # time before any connections have been opened. There is technically some @@ -45,13 +51,13 @@ def self.clear_all_connection_managers # A default client for the current thread. def self.default_client - Thread.current[:stripe_client_default_client] ||= + current_thread_context.default_client ||= StripeClient.new(default_connection_manager) end # A default connection manager for the current thread. def self.default_connection_manager - Thread.current[:stripe_client_default_connection_manager] ||= begin + current_thread_context.default_connection_manager ||= begin connection_manager = ConnectionManager.new @all_connection_managers_mutex.synchronize do @@ -121,15 +127,22 @@ def self.sleep_time(num_retries) # charge, resp = client.request { Charge.create } # def request - @last_response = nil - old_stripe_client = Thread.current[:stripe_client] - Thread.current[:stripe_client] = self + old_stripe_client = self.class.current_thread_context.active_client + self.class.current_thread_context.active_client = self + + if self.class.current_thread_context.last_responses&.key?(object_id) + raise "calls to StripeClient#request cannot be nested within a thread" + end + + self.class.current_thread_context.last_responses ||= {} + self.class.current_thread_context.last_responses[object_id] = nil begin res = yield - [res, @last_response] + [res, self.class.current_thread_context.last_responses[object_id]] ensure - Thread.current[:stripe_client] = old_stripe_client + self.class.current_thread_context.active_client = old_stripe_client + self.class.current_thread_context.last_responses.delete(object_id) end end @@ -196,8 +209,13 @@ def execute_request(method, path, raise general_api_error(http_resp.code.to_i, http_resp.body) end - # Allows StripeClient#request to return a response object to a caller. - @last_response = resp + # If being called from `StripeClient#request`, put the last response in + # thread-local memory so that it can be returned to the user. Don't store + # anything otherwise so that we don't leak memory. + if self.class.current_thread_context.last_responses&.key?(object_id) + self.class.current_thread_context.last_responses[object_id] = resp + end + [resp, api_key] end @@ -248,6 +266,50 @@ def execute_request(method, path, }.freeze private_constant :NETWORK_ERROR_MESSAGES_MAP + # A record representing any data that `StripeClient` puts into + # `Thread.current`. Making it a class likes this gives us a little extra + # type safety and lets us document what each field does. + # + # For internal use only. Does not provide a stable API and may be broken + # with future non-major changes. + class ThreadContext + # A `StripeClient` that's been flagged as currently active within a + # thread by `StripeClient#request`. A client stays active until the + # completion of the request block. + attr_accessor :active_client + + # A default `StripeClient` object for the thread. Used in all cases where + # the user hasn't specified their own. + attr_accessor :default_client + + # A default `ConnectionManager` for the thread. Normally shared between + # all `StripeClient` objects on a particular thread, and created so as to + # minimize the number of open connections that an application needs. + attr_accessor :default_connection_manager + + # A temporary map of object IDs to responses from last executed API + # calls. Used to return a responses from calls to `StripeClient#request`. + # + # Stored in the thread data to make the use of a single `StripeClient` + # object safe across multiple threads. Stored as a map so that multiple + # `StripeClient` objects can run concurrently on the same thread. + # + # Responses are only left in as long as they're needed, which means + # they're removed as soon as a call leaves `StripeClient#request`, and + # because that's wrapped in an `ensure` block, they should never leave + # garbage in `Thread.current`. + attr_accessor :last_responses + end + + # Access data stored for `StripeClient` within the thread's current + # context. Returns `ThreadContext`. + # + # For internal use only. Does not provide a stable API and may be broken + # with future non-major changes. + def self.current_thread_context + Thread.current[:stripe_client__internal_use_only] ||= ThreadContext.new + end + private def api_url(url = "", api_base = nil) (api_base || Stripe.api_base) + url end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index f24e85f34..2d8fba808 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -826,14 +826,106 @@ class StripeClientTest < Test::Unit::TestCase should "reset local thread state after a call" do begin - Thread.current[:stripe_client] = :stripe_client + StripeClient.current_thread_context.active_client = :stripe_client client = StripeClient.new client.request {} - assert_equal :stripe_client, Thread.current[:stripe_client] + assert_equal :stripe_client, + StripeClient.current_thread_context.active_client ensure - Thread.current[:stripe_client] = nil + StripeClient.current_thread_context.active_client = nil + end + end + + should "correctly return last responses despite multiple clients" do + charge_resp = { object: "charge" } + coupon_resp = { object: "coupon" } + + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(charge_resp)) + stub_request(:post, "#{Stripe.api_base}/v1/coupons") + .to_return(body: JSON.generate(coupon_resp)) + + client1 = StripeClient.new + client2 = StripeClient.new + + client2_resp = nil + _charge, client1_resp = client1.request do + Charge.create + + # This is contrived, but we run one client nested in the `request` + # block of another one just to ensure that the parent is still + # unwinding when this goes through. If the parent's last response + # were to be overridden by this client (through a bug), then it would + # happen here. + _coupon, client2_resp = client2.request do + Coupon.create + end + end + + assert_equal charge_resp, client1_resp.data + assert_equal coupon_resp, client2_resp.data + end + + should "correctly return last responses despite multiple threads" do + charge_resp = { object: "charge" } + coupon_resp = { object: "coupon" } + + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(charge_resp)) + stub_request(:post, "#{Stripe.api_base}/v1/coupons") + .to_return(body: JSON.generate(coupon_resp)) + + client = StripeClient.new + + # Poorly named class -- note this is actually a concurrent queue. + recv_queue = Queue.new + send_queue = Queue.new + + # Start a thread, make an API request, but then idle in the `request` + # block until the main thread has been able to make its own API request + # and signal that it's done. If this thread's last response were to be + # overridden by the main thread (through a bug), then this routine + # should suss it out. + resp1 = nil + thread = Thread.start do + _charge, resp1 = client.request do + Charge.create + + # Idle in `request` block until main thread signals. + send_queue.pop + end + + # Signal main thread that we're done and it can run its checks. + recv_queue << true + end + + # Make an API request. + _coupon, resp2 = client.request do + Coupon.create + end + + # Tell background thread to finish `request`, then wait for it to + # signal back to us that it's ready. + send_queue << true + recv_queue.pop + + assert_equal charge_resp, resp1.data + assert_equal coupon_resp, resp2.data + + # And for maximum hygiene, make sure that our thread rejoins. + thread.join + end + + should "error if calls to #request are nested on the same thread" do + client = StripeClient.new + client.request do + e = assert_raises(RuntimeError) do + client.request {} + end + assert_equal "calls to StripeClient#request cannot be nested within a thread", + e.message end end end @@ -841,7 +933,7 @@ class StripeClientTest < Test::Unit::TestCase context "#proxy" do should "run the request through the proxy" do begin - Thread.current[:stripe_client_default_connection_manager] = nil + StripeClient.current_thread_context.default_connection_manager = nil Stripe.proxy = "http://user:pass@localhost:8080" @@ -857,7 +949,7 @@ class StripeClientTest < Test::Unit::TestCase ensure Stripe.proxy = nil - Thread.current[:stripe_client_default_connection_manager] = nil + StripeClient.current_thread_context.default_connection_manager = nil end end end From ff63ef4737f9c4a2d1f243417430d250e925328b Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Tue, 20 Aug 2019 11:24:08 -0700 Subject: [PATCH 18/18] Add Invoice.list_upcoming_line_items method (#834) --- lib/stripe/resources/invoice.rb | 5 +++++ test/stripe/invoice_test.rb | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/stripe/resources/invoice.rb b/lib/stripe/resources/invoice.rb index 840cc062e..c8e83efb1 100644 --- a/lib/stripe/resources/invoice.rb +++ b/lib/stripe/resources/invoice.rb @@ -64,5 +64,10 @@ def self.upcoming(params, opts = {}) resp, opts = 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) + Util.convert_to_stripe_object(resp.data, opts) + end end end diff --git a/test/stripe/invoice_test.rb b/test/stripe/invoice_test.rb index b6464fbaf..468a2cabb 100644 --- a/test/stripe/invoice_test.rb +++ b/test/stripe/invoice_test.rb @@ -142,7 +142,7 @@ class InvoiceTest < Test::Unit::TestCase end end - context "#upcoming" do + context ".upcoming" do should "retrieve upcoming invoices" do invoice = Stripe::Invoice.upcoming( customer: "cus_123", @@ -192,6 +192,22 @@ class InvoiceTest < Test::Unit::TestCase end end + context ".list_upcoming_line_items" do + should "retrieve upcoming invoices" do + line_items = Stripe::Invoice.list_upcoming_line_items( + customer: "cus_123", + subscription: "sub_123" + ) + assert_requested :get, "#{Stripe.api_base}/v1/invoices/upcoming/lines", + query: { + customer: "cus_123", + subscription: "sub_123", + } + assert line_items.data.is_a?(Array) + assert line_items.data[0].is_a?(Stripe::InvoiceLineItem) + end + end + context "#void_invoice" do should "void invoice" do invoice = Stripe::Invoice.retrieve("in_123")