From 7b38eeaf4eeb17bb68642aac51be3e58ee912f7d Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Wed, 16 Jun 2021 17:02:00 -0400 Subject: [PATCH 1/3] Add support for making a request and receiving the response as a stream. --- lib/stripe/api_operations/request.rb | 37 +- lib/stripe/api_resource.rb | 8 + lib/stripe/connection_manager.rb | 19 +- lib/stripe/stripe_client.rb | 176 ++-- lib/stripe/stripe_response.rb | 51 ++ test/stripe/api_resource_test.rb | 53 ++ test/stripe/connection_manager_test.rb | 21 + test/stripe/stripe_client_test.rb | 1082 ++++++++++++------------ 8 files changed, 862 insertions(+), 585 deletions(-) diff --git a/lib/stripe/api_operations/request.rb b/lib/stripe/api_operations/request.rb index ec204f365..f4dcf3d42 100644 --- a/lib/stripe/api_operations/request.rb +++ b/lib/stripe/api_operations/request.rb @@ -6,6 +6,28 @@ module Request module ClassMethods def execute_resource_request(method, url, params = {}, opts = {}) + execute_resource_request_internal( + :execute_request, method, url, params, opts + ) + end + + def execute_resource_request_stream(method, url, + params = {}, opts = {}, + &read_body_chunk_block) + execute_resource_request_internal( + :execute_request_stream, + method, + url, + params, + opts, + &read_body_chunk_block + ) + end + + private def execute_resource_request_internal(client_request_method_sym, + method, url, + params, opts, + &read_body_chunk_block) params ||= {} error_on_invalid_params(params) @@ -22,10 +44,12 @@ def execute_resource_request(method, url, client = headers.delete(:client) # Assume all remaining opts must be headers - resp, opts[:api_key] = client.execute_request( + resp, opts[:api_key] = client.send( + client_request_method_sym, method, url, api_base: api_base, api_key: api_key, - headers: headers, params: params + headers: headers, params: params, + &read_body_chunk_block ) # Hash#select returns an array before 1.9 @@ -89,6 +113,15 @@ def self.included(base) self.class.execute_resource_request(method, url, params, opts) end + protected def execute_resource_request_stream(method, url, + params = {}, opts = {}, + &read_body_chunk_block) + opts = @opts.merge(Util.normalize_opts(opts)) + self.class.execute_resource_request_stream( + method, url, params, opts, &read_body_chunk_block + ) + end + # See notes on `alias` above. alias request execute_resource_request end diff --git a/lib/stripe/api_resource.rb b/lib/stripe/api_resource.rb index 56d3b0f28..ac70af441 100644 --- a/lib/stripe/api_resource.rb +++ b/lib/stripe/api_resource.rb @@ -115,5 +115,13 @@ def self.retrieve(id, opts = {}) Util.convert_to_stripe_object(resp.data, opts) end end + + protected def request_stream(method:, path:, params:, opts: {}, + &read_body_chunk_block) + resp, = execute_resource_request_stream( + method, path, params, opts, &read_body_chunk_block + ) + resp + end end end diff --git a/lib/stripe/connection_manager.rb b/lib/stripe/connection_manager.rb index c120083bd..46fd0334a 100644 --- a/lib/stripe/connection_manager.rb +++ b/lib/stripe/connection_manager.rb @@ -66,7 +66,8 @@ def connection_for(uri) # 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) + def execute_request(method, uri, body: nil, headers: nil, query: nil, + &block) # Perform some basic argument validation because it's easy to get # confused between strings and hashes for things like body and query # parameters. @@ -92,8 +93,22 @@ def execute_request(method, uri, body: nil, headers: nil, query: nil) u.path end + method_name = method.to_s.upcase + has_response_body = method_name != "HEAD" + request = Net::HTTPGenericRequest.new( + method_name, + (body ? true : false), + has_response_body, + path, + headers + ) + @mutex.synchronize do - connection.send_request(method.to_s.upcase, path, body, headers) + # The block parameter is special here. If a block is provided, the block + # is invoked with the Net::HTTPResponse. However, the body will not have + # been read yet in the block, and can be streamed by calling + # HTTPResponse#read_body. + connection.request(request, body, &block) end end diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 3a54a8bd3..605649ab6 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -213,62 +213,9 @@ 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 ||= config.api_base - api_key ||= config.api_key - params = Util.objects_to_ids(params) - - check_api_key!(api_key) - - body_params = nil - query_params = nil - case method - when :get, :head, :delete - query_params = params - else - body_params = params - 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 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. - context = RequestLogContext.new - context.account = headers["Stripe-Account"] - context.api_key = api_key - context.api_version = headers["Stripe-Version"] - context.body = body_log - context.idempotency_key = headers["Idempotency-Key"] - context.method = method - context.path = path - context.query = query - - http_resp = execute_request_with_rescues(method, api_base, context) do - self.class - .default_connection_manager(config) - .execute_request(method, url, - body: body, - headers: headers, - query: query) - end + http_resp, api_key = execute_request_internal( + method, path, api_base, api_key, headers, params + ) begin resp = StripeResponse.from_net_http(http_resp) @@ -284,6 +231,37 @@ def execute_request(method, path, [resp, api_key] end + # Executes a request and returns the body as a stream instead of converting + # it to a StripeObject. This should be used for any request where we expect + # an arbitrary binary response. + # + # A `read_body_chunk` block can be passed, which will be called repeatedly + # with the body chunks read from the socket. + # + # If a block is passed, a StripeHeadersOnlyResponse is returned as the + # block is expected to do all the necessary body processing. If no block is + # passed, then a StripeStreamResponse is returned containing an IO stream + # with the response body. + def execute_request_stream(method, path, + api_base: nil, api_key: nil, + headers: {}, params: {}, + &read_body_chunk_block) + http_resp, api_key = execute_request_internal( + method, path, api_base, api_key, headers, params, &read_body_chunk_block + ) + + # When the read_body_chunk_block is given, we no longer have access to the + # response body at this point and so return a response object containing + # only the headers. This is because the body was consumed by the block. + resp = if block_given? + StripeHeadersOnlyResponse.from_net_http(http_resp) + else + StripeStreamResponse.from_net_http(http_resp) + end + + [resp, api_key] + end + def store_last_response(object_id, resp) return unless last_response_has_key?(object_id) @@ -451,6 +429,83 @@ def self.maybe_gc_connection_managers pruned_contexts.count end + private def execute_request_internal(method, path, + api_base, api_key, headers, params, + &read_body_chunk_block) + 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 ||= config.api_base + api_key ||= config.api_key + params = Util.objects_to_ids(params) + + check_api_key!(api_key) + + body_params = nil + query_params = nil + case method + when :get, :head, :delete + query_params = params + else + body_params = params + 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 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. + context = RequestLogContext.new + context.account = headers["Stripe-Account"] + context.api_key = api_key + context.api_version = headers["Stripe-Version"] + context.body = body_log + context.idempotency_key = headers["Idempotency-Key"] + context.method = method + context.path = path + context.query = query + + # A block can be passed in to read the content directly from the response. + # We want to execute this block only when the response was actually + # successful. When it wasn't, we defer to the standard error handling as + # we have to read the body and parse the error JSON. + response_block = + if block_given? + lambda do |response| + unless should_handle_as_error(response.code.to_i) + response.read_body(&read_body_chunk_block) + end + end + end + + http_resp = execute_request_with_rescues(method, api_base, context) do + self.class + .default_connection_manager(config) + .execute_request(method, url, + body: body, + headers: headers, + query: query, + &response_block) + end + + [http_resp, api_key] + end + private def api_url(url = "", api_base = nil) (api_base || config.api_base) + url end @@ -490,6 +545,7 @@ def self.maybe_gc_connection_managers # 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 @@ -503,6 +559,10 @@ def self.maybe_gc_connection_managers [body, body_log] end + private def should_handle_as_error(http_status) + http_status >= 400 + end + private def execute_request_with_rescues(method, api_base, context) num_retries = 0 @@ -520,7 +580,9 @@ def self.maybe_gc_connection_managers http_status = resp.code.to_i context = context.dup_from_response_headers(resp) - handle_error_response(resp, context) if http_status >= 400 + if should_handle_as_error(http_status) + handle_error_response(resp, context) + end log_response(context, request_start, http_status, resp.body) notify_request_end(context, request_duration, http_status, diff --git a/lib/stripe/stripe_response.rb b/lib/stripe/stripe_response.rb index 8c4749f1e..a783b3d37 100644 --- a/lib/stripe/stripe_response.rb +++ b/lib/stripe/stripe_response.rb @@ -79,4 +79,55 @@ def self.from_net_http(http_resp) resp end end + + # StripeStreamResponse includes header-related vitals of the + # response as well as an IO stream of the body content. + class StripeStreamResponse + attr_accessor :io + + # A Hash of the HTTP headers of the response. + attr_accessor :http_headers + + # The integer HTTP status code of the response. + attr_accessor :http_status + + # The Stripe request ID of the response. + attr_accessor :request_id + + # Initializes a StripeStreamResponse object from a Net::HTTP::HTTPResponse + # object. + def self.from_net_http(http_resp) + resp = StripeStreamResponse.new + resp.io = StringIO.new http_resp.body + resp.http_headers = StripeResponse::Headers.from_net_http(http_resp) + resp.http_status = http_resp.code.to_i + resp.request_id = http_resp["request-id"] + resp + end + end + + # StripeHeadersOnlyResponse includes only header-related vitals of the + # response. This is used for streaming requests where the response was read + # directly in a block and we explicitly don't want to store the body of the + # response in memory. + class StripeHeadersOnlyResponse + # A Hash of the HTTP headers of the response. + attr_accessor :http_headers + + # The integer HTTP status code of the response. + attr_accessor :http_status + + # The Stripe request ID of the response. + attr_accessor :request_id + + # Initializes a StripeHeadersOnlyResponse object from a + # Net::HTTP::HTTPResponse object. + def self.from_net_http(http_resp) + resp = StripeHeadersOnlyResponse.new + resp.http_headers = StripeResponse::Headers.from_net_http(http_resp) + resp.http_status = http_resp.code.to_i + resp.request_id = http_resp["request-id"] + resp + end + end end diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index 02e391d2e..e7e8532ac 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -613,6 +613,59 @@ def say_hello(params = {}, opts = {}) end end + context "#request_stream" do + class StreamTestAPIResource < APIResource + OBJECT_NAME = "stream" + def read_stream(params = {}, opts = {}, &read_body_chunk_block) + request_stream( + method: :get, + path: resource_url + "/read", + params: params, + opts: opts, + &read_body_chunk_block + ) + end + end + + setup do + Util.instance_variable_set( + :@object_classes, + Stripe::ObjectTypes.object_names_to_classes.merge( + "stream" => StreamTestAPIResource + ) + ) + end + teardown do + Util.class.instance_variable_set(:@object_classes, Stripe::ObjectTypes.object_names_to_classes) + end + + should "supports requesting with a block" do + stub_request(:get, "#{Stripe.api_base}/v1/streams/hi_123/read") + .with(query: { foo: "bar" }, headers: { "Stripe-Account" => "acct_hi" }) + .to_return(body: "response body") + + accumulated_body = +"" + + resp = StreamTestAPIResource.new(id: "hi_123").read_stream({ foo: "bar" }, stripe_account: "acct_hi") do |body_chunk| + accumulated_body << body_chunk + end + + assert_instance_of Stripe::StripeHeadersOnlyResponse, resp + assert_equal "response body", accumulated_body + end + + should "supports requesting without a block" do + stub_request(:get, "#{Stripe.api_base}/v1/streams/hi_123/read") + .with(query: { foo: "bar" }, headers: { "Stripe-Account" => "acct_hi" }) + .to_return(body: "response body") + + resp = StreamTestAPIResource.new(id: "hi_123").read_stream({ foo: "bar" }, stripe_account: "acct_hi") + + assert_instance_of Stripe::StripeStreamResponse, resp + assert_equal "response body", resp.io.read + end + end + @@fixtures = {} # rubocop:disable Style/ClassVars setup do if @@fixtures.empty? diff --git a/test/stripe/connection_manager_test.rb b/test/stripe/connection_manager_test.rb index c5efcb3b0..e70468c22 100644 --- a/test/stripe/connection_manager_test.rb +++ b/test/stripe/connection_manager_test.rb @@ -159,6 +159,27 @@ class ConnectionManagerTest < Test::Unit::TestCase query: "query=bar") end + should "make a request with a block" do + stub_request(:post, "#{Stripe.api_base}/path?query=bar") + .with( + body: "body=foo", + headers: { "Stripe-Account" => "bar" } + ) + .to_return(body: "HTTP response body") + + accumulated_body = +"" + + @manager.execute_request(:post, "#{Stripe.api_base}/path", + body: "body=foo", + headers: { "Stripe-Account" => "bar" }, + query: "query=bar") do |res| + res.read_body do |body_chunk| + accumulated_body << body_chunk + end + end + assert_equal "HTTP response body", accumulated_body + end + should "perform basic argument validation" do e = assert_raises ArgumentError do @manager.execute_request("POST", "#{Stripe.api_base}/path") diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index b34eee261..26c55f79c 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -446,660 +446,694 @@ class StripeClientTest < Test::Unit::TestCase end end - context "#execute_request" do - context "headers" do - should "support literal headers" do - stub_request(:post, "#{Stripe.api_base}/v1/account") - .with(headers: { "Stripe-Account" => "bar" }) - .to_return(body: JSON.generate(object: "account")) - - client = StripeClient.new - client.execute_request(:post, "/v1/account", - headers: { "Stripe-Account" => "bar" }) - end - - should "support RestClient-style header keys" do - stub_request(:post, "#{Stripe.api_base}/v1/account") - .with(headers: { "Stripe-Account" => "bar" }) - .to_return(body: JSON.generate(object: "account")) + %w[execute_request execute_request_stream].each do |request_method| + context "request processing for #{request_method}" do + context "headers" do + should "support literal headers" do + stub_request(:post, "#{Stripe.api_base}/v1/account") + .with(headers: { "Stripe-Account" => "bar" }) + .to_return(body: JSON.generate(object: "account")) - client = StripeClient.new - client.execute_request(:post, "/v1/account", - headers: { stripe_account: "bar" }) - end - end + client = StripeClient.new + client.send(request_method, :post, "/v1/account", + headers: { "Stripe-Account" => "bar" }) + end - context "logging" do - setup do - # Freeze time for the purposes of the `elapsed` parameter that we - # emit for responses. Mocha's `anything` parameter can't match inside - # of a hash and is therefore not useful for this purpose. If we - # switch over to rspec-mocks at some point, we can probably remove - # this. - Util.stubs(:monotonic_time).returns(0.0) + should "support RestClient-style header keys" do + stub_request(:post, "#{Stripe.api_base}/v1/account") + .with(headers: { "Stripe-Account" => "bar" }) + .to_return(body: JSON.generate(object: "account")) - # Stub the Stripe.config so that mocha matchers will succeed. Currently, - # mocha does not support using param matchers within hashes. - StripeClient.any_instance.stubs(:config).returns(Stripe.config) + client = StripeClient.new + client.send(request_method, :post, "/v1/account", + headers: { stripe_account: "bar" }) + end end - should "produce appropriate logging" do - body = JSON.generate(object: "account") - - Util.expects(:log_info).with("Request to Stripe API", - account: "acct_123", - api_version: "2010-11-12", - idempotency_key: "abc", - method: :post, - num_retries: 0, - path: "/v1/account", - config: Stripe.config) - Util.expects(:log_debug).with("Request details", - body: "", - idempotency_key: "abc", - query: nil, - config: Stripe.config) - - Util.expects(:log_info).with("Response from Stripe API", - account: "acct_123", - api_version: "2010-11-12", - elapsed: 0.0, - idempotency_key: "abc", - method: :post, - path: "/v1/account", - request_id: "req_123", - status: 200, - config: Stripe.config) - Util.expects(:log_debug).with("Response details", - body: body, - idempotency_key: "abc", - request_id: "req_123", - config: Stripe.config) - Util.expects(:log_debug).with("Dashboard link for request", - idempotency_key: "abc", - request_id: "req_123", - url: Util.request_id_dashboard_url("req_123", Stripe.api_key), - config: Stripe.config) - - stub_request(:post, "#{Stripe.api_base}/v1/account") - .to_return( - body: body, - headers: { - "Idempotency-Key" => "abc", - "Request-Id" => "req_123", - "Stripe-Account" => "acct_123", - "Stripe-Version" => "2010-11-12", - } - ) + context "logging" do + setup do + # Freeze time for the purposes of the `elapsed` parameter that we + # emit for responses. Mocha's `anything` parameter can't match inside + # of a hash and is therefore not useful for this purpose. If we + # switch over to rspec-mocks at some point, we can probably remove + # this. + Util.stubs(:monotonic_time).returns(0.0) + + # Stub the Stripe.config so that mocha matchers will succeed. Currently, + # mocha does not support using param matchers within hashes. + StripeClient.any_instance.stubs(:config).returns(Stripe.config) + end - client = StripeClient.new - client.execute_request(:post, "/v1/account", - headers: { - "Idempotency-Key" => "abc", - "Stripe-Account" => "acct_123", - "Stripe-Version" => "2010-11-12", - }) - end + should "produce appropriate logging" do + body = JSON.generate(object: "account") + + Util.expects(:log_info).with("Request to Stripe API", + account: "acct_123", + api_version: "2010-11-12", + idempotency_key: "abc", + method: :post, + num_retries: 0, + path: "/v1/account", + config: Stripe.config) + Util.expects(:log_debug).with("Request details", + body: "", + idempotency_key: "abc", + query: nil, + config: Stripe.config) + + Util.expects(:log_info).with("Response from Stripe API", + account: "acct_123", + api_version: "2010-11-12", + elapsed: 0.0, + idempotency_key: "abc", + method: :post, + path: "/v1/account", + request_id: "req_123", + status: 200, + config: Stripe.config) + Util.expects(:log_debug).with("Response details", + body: body, + idempotency_key: "abc", + request_id: "req_123", + config: Stripe.config) + Util.expects(:log_debug).with("Dashboard link for request", + idempotency_key: "abc", + request_id: "req_123", + url: Util.request_id_dashboard_url("req_123", Stripe.api_key), + config: Stripe.config) - should "produce logging on API error" do - Util.expects(:log_info).with("Request to Stripe API", - account: nil, - api_version: nil, - idempotency_key: nil, - method: :post, - num_retries: 0, - path: "/v1/account", - config: Stripe.config) - Util.expects(:log_info).with("Response from Stripe API", - account: nil, - api_version: nil, - elapsed: 0.0, - idempotency_key: nil, - method: :post, - path: "/v1/account", - request_id: nil, - status: 500, - config: Stripe.config) - - error = { - code: "code", - message: "message", - param: "param", - type: "type", - } - Util.expects(:log_error).with("Stripe API error", - status: 500, - error_code: error[:code], - error_message: error[:message], - error_param: error[:param], - error_type: error[:type], - idempotency_key: nil, - request_id: nil, - config: Stripe.config) - - stub_request(:post, "#{Stripe.api_base}/v1/account") - .to_return( - body: JSON.generate(error: error), - status: 500 - ) + stub_request(:post, "#{Stripe.api_base}/v1/account") + .to_return( + body: body, + headers: { + "Idempotency-Key" => "abc", + "Request-Id" => "req_123", + "Stripe-Account" => "acct_123", + "Stripe-Version" => "2010-11-12", + } + ) - client = StripeClient.new - assert_raises Stripe::APIError do - client.execute_request(:post, "/v1/account") + client = StripeClient.new + client.send(request_method, :post, "/v1/account", + headers: { + "Idempotency-Key" => "abc", + "Stripe-Account" => "acct_123", + "Stripe-Version" => "2010-11-12", + }) end - end - should "produce logging on OAuth error" do - Util.expects(:log_info).with("Request to Stripe API", - account: nil, - api_version: nil, - idempotency_key: nil, - method: :post, - num_retries: 0, - path: "/oauth/token", - config: Stripe.config) - Util.expects(:log_info).with("Response from Stripe API", - account: nil, - api_version: nil, - elapsed: 0.0, - idempotency_key: nil, - method: :post, - path: "/oauth/token", - request_id: nil, - status: 400, - config: Stripe.config) - - Util.expects(:log_error).with("Stripe OAuth error", - status: 400, - error_code: "invalid_request", - error_description: "No grant type specified", - idempotency_key: nil, - request_id: nil, - config: Stripe.config) - - stub_request(:post, "#{Stripe.connect_base}/oauth/token") - .to_return(body: JSON.generate(error: "invalid_request", - error_description: "No grant type specified"), status: 400) + should "produce logging on API error" do + Util.expects(:log_info).with("Request to Stripe API", + account: nil, + api_version: nil, + idempotency_key: nil, + method: :post, + num_retries: 0, + path: "/v1/account", + config: Stripe.config) + Util.expects(:log_info).with("Response from Stripe API", + account: nil, + api_version: nil, + elapsed: 0.0, + idempotency_key: nil, + method: :post, + path: "/v1/account", + request_id: nil, + status: 500, + config: Stripe.config) + + error = { + code: "code", + message: "message", + param: "param", + type: "type", + } + Util.expects(:log_error).with("Stripe API error", + status: 500, + error_code: error[:code], + error_message: error[:message], + error_param: error[:param], + error_type: error[:type], + idempotency_key: nil, + request_id: nil, + config: Stripe.config) - client = StripeClient.new - opts = { api_base: Stripe.connect_base } - assert_raises Stripe::OAuth::InvalidRequestError do - client.execute_request(:post, "/oauth/token", **opts) - end - end - end + stub_request(:post, "#{Stripe.api_base}/v1/account") + .to_return( + body: JSON.generate(error: error), + status: 500 + ) - context "Stripe-Account header" do - should "use a globally set header" do - begin - old = Stripe.stripe_account - Stripe.stripe_account = "acct_1234" + client = StripeClient.new + assert_raises Stripe::APIError do + client.send(request_method, :post, "/v1/account") + end + end - stub_request(:post, "#{Stripe.api_base}/v1/account") - .with(headers: { "Stripe-Account" => Stripe.stripe_account }) - .to_return(body: JSON.generate(object: "account")) + should "produce logging on OAuth error" do + Util.expects(:log_info).with("Request to Stripe API", + account: nil, + api_version: nil, + idempotency_key: nil, + method: :post, + num_retries: 0, + path: "/oauth/token", + config: Stripe.config) + Util.expects(:log_info).with("Response from Stripe API", + account: nil, + api_version: nil, + elapsed: 0.0, + idempotency_key: nil, + method: :post, + path: "/oauth/token", + request_id: nil, + status: 400, + config: Stripe.config) + + Util.expects(:log_error).with("Stripe OAuth error", + status: 400, + error_code: "invalid_request", + error_description: "No grant type specified", + idempotency_key: nil, + request_id: nil, + config: Stripe.config) + + stub_request(:post, "#{Stripe.connect_base}/oauth/token") + .to_return(body: JSON.generate(error: "invalid_request", + error_description: "No grant type specified"), status: 400) client = StripeClient.new - client.execute_request(:post, "/v1/account") - ensure - Stripe.stripe_account = old + opts = { api_base: Stripe.connect_base } + assert_raises Stripe::OAuth::InvalidRequestError do + client.send(request_method, :post, "/oauth/token", **opts) + end end end - should "use a locally set header" do - stripe_account = "acct_0000" - stub_request(:post, "#{Stripe.api_base}/v1/account") - .with(headers: { "Stripe-Account" => stripe_account }) - .to_return(body: JSON.generate(object: "account")) + context "Stripe-Account header" do + should "use a globally set header" do + begin + old = Stripe.stripe_account + Stripe.stripe_account = "acct_1234" - client = StripeClient.new - client.execute_request(:post, "/v1/account", - headers: { stripe_account: stripe_account }) - end + stub_request(:post, "#{Stripe.api_base}/v1/account") + .with(headers: { "Stripe-Account" => Stripe.stripe_account }) + .to_return(body: JSON.generate(object: "account")) - should "not send it otherwise" do - stub_request(:post, "#{Stripe.api_base}/v1/account") - .with do |req| - req.headers["Stripe-Account"].nil? - end.to_return(body: JSON.generate(object: "account")) + client = StripeClient.new + client.send(request_method, :post, "/v1/account") + ensure + Stripe.stripe_account = old + end + end - client = StripeClient.new - client.execute_request(:post, "/v1/account") - end - end + should "use a locally set header" do + stripe_account = "acct_0000" + stub_request(:post, "#{Stripe.api_base}/v1/account") + .with(headers: { "Stripe-Account" => stripe_account }) + .to_return(body: JSON.generate(object: "account")) - context "app_info" do - should "send app_info if set" do - begin - old = Stripe.app_info - Stripe.set_app_info( - "MyAwesomePlugin", - partner_id: "partner_1234", - url: "https://myawesomeplugin.info", - version: "1.2.34" - ) + client = StripeClient.new + client.send(request_method, :post, "/v1/account", + headers: { stripe_account: stripe_account }) + end + should "not send it otherwise" do stub_request(:post, "#{Stripe.api_base}/v1/account") .with do |req| - assert_equal \ - "Stripe/v1 RubyBindings/#{Stripe::VERSION} " \ - "MyAwesomePlugin/1.2.34 (https://myawesomeplugin.info)", - req.headers["User-Agent"] - - data = JSON.parse(req.headers["X-Stripe-Client-User-Agent"], - symbolize_names: true) - - assert_equal({ - name: "MyAwesomePlugin", - partner_id: "partner_1234", - url: "https://myawesomeplugin.info", - version: "1.2.34", - }, data[:application]) - - true + req.headers["Stripe-Account"].nil? end.to_return(body: JSON.generate(object: "account")) client = StripeClient.new - client.execute_request(:post, "/v1/account") - ensure - Stripe.app_info = old + client.send(request_method, :post, "/v1/account") end end - end - - context "error handling" do - should "handle error response with empty body" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: "", status: 500) - - client = StripeClient.new - e = assert_raises Stripe::APIError do - client.execute_request(:post, "/v1/charges") + context "app_info" do + should "send app_info if set" do + begin + old = Stripe.app_info + Stripe.set_app_info( + "MyAwesomePlugin", + partner_id: "partner_1234", + url: "https://myawesomeplugin.info", + version: "1.2.34" + ) + + stub_request(:post, "#{Stripe.api_base}/v1/account") + .with do |req| + assert_equal \ + "Stripe/v1 RubyBindings/#{Stripe::VERSION} " \ + "MyAwesomePlugin/1.2.34 (https://myawesomeplugin.info)", + req.headers["User-Agent"] + + data = JSON.parse(req.headers["X-Stripe-Client-User-Agent"], + symbolize_names: true) + + assert_equal({ + name: "MyAwesomePlugin", + partner_id: "partner_1234", + url: "https://myawesomeplugin.info", + version: "1.2.34", + }, data[:application]) + + true + end.to_return(body: JSON.generate(object: "account")) + + client = StripeClient.new + client.send(request_method, :post, "/v1/account") + ensure + Stripe.app_info = old + end end - assert_equal 'Invalid response object from API: "" (HTTP response code was 500)', e.message end - should "handle success response with empty body" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: "", status: 200) + context "error handling" do + should "handle error response with empty body" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: "", status: 500) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::APIError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::APIError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal 'Invalid response object from API: "" (HTTP response code was 500)', e.message end - assert_equal 'Invalid response object from API: "" (HTTP response code was 200)', e.message - end - should "feed a request ID through to the error object" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(make_missing_id_error), - headers: { "Request-ID": "req_123" }, - status: 400) + should "feed a request ID through to the error object" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(make_missing_id_error), + headers: { "Request-ID": "req_123" }, + status: 400) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::InvalidRequestError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::InvalidRequestError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal("req_123", e.request_id) end - assert_equal("req_123", e.request_id) - end - should "handle low level error" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_raise(Errno::ECONNREFUSED.new) + should "handle low level error" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_raise(Errno::ECONNREFUSED.new) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::APIConnectionError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::APIConnectionError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal StripeClient::ERROR_MESSAGE_CONNECTION % Stripe.api_base + + "\n\n(Network error: Connection refused)", + e.message 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) + 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) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::APIError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::APIError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal 'Invalid response object from API: "{\"bar\":\"foo\"}" (HTTP response code was 500)', e.message end - assert_equal 'Invalid response object from API: "{\"bar\":\"foo\"}" (HTTP response code was 500)', e.message - end - should "raise IdempotencyError on 400 of type idempotency_error" do - data = make_missing_id_error - data[:error][:type] = "idempotency_error" + should "raise IdempotencyError on 400 of type idempotency_error" do + data = make_missing_id_error + data[:error][:type] = "idempotency_error" - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(data), status: 400) + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(data), status: 400) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::IdempotencyError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::IdempotencyError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal(400, e.http_status) + assert_equal(true, e.json_body.is_a?(Hash)) end - assert_equal(400, e.http_status) - assert_equal(true, e.json_body.is_a?(Hash)) - end - should "raise InvalidRequestError on other 400s" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(make_missing_id_error), status: 400) + should "raise InvalidRequestError on other 400s" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(make_missing_id_error), status: 400) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::InvalidRequestError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::InvalidRequestError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal(400, e.http_status) + assert_equal(true, e.json_body.is_a?(Hash)) end - assert_equal(400, e.http_status) - assert_equal(true, e.json_body.is_a?(Hash)) - end - should "raise AuthenticationError on 401" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(make_missing_id_error), status: 401) + should "raise AuthenticationError on 401" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(make_missing_id_error), status: 401) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::AuthenticationError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::AuthenticationError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal(401, e.http_status) + assert_equal(true, e.json_body.is_a?(Hash)) end - assert_equal(401, e.http_status) - assert_equal(true, e.json_body.is_a?(Hash)) - end - should "raise CardError on 402" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(make_invalid_exp_year_error), status: 402) + should "raise CardError on 402" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(make_invalid_exp_year_error), status: 402) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::CardError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::CardError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal(402, e.http_status) + assert_equal(true, e.json_body.is_a?(Hash)) + assert_equal("invalid_expiry_year", e.code) + assert_equal("exp_year", e.param) end - assert_equal(402, e.http_status) - assert_equal(true, e.json_body.is_a?(Hash)) - assert_equal("invalid_expiry_year", e.code) - assert_equal("exp_year", e.param) - end - should "raise PermissionError on 403" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(make_missing_id_error), status: 403) + should "raise PermissionError on 403" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(make_missing_id_error), status: 403) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::PermissionError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::PermissionError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal(403, e.http_status) + assert_equal(true, e.json_body.is_a?(Hash)) end - assert_equal(403, e.http_status) - assert_equal(true, e.json_body.is_a?(Hash)) - end - should "raise InvalidRequestError on 404" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(make_missing_id_error), status: 404) + should "raise InvalidRequestError on 404" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(make_missing_id_error), status: 404) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::InvalidRequestError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::InvalidRequestError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal(404, e.http_status) + assert_equal(true, e.json_body.is_a?(Hash)) end - assert_equal(404, e.http_status) - assert_equal(true, e.json_body.is_a?(Hash)) - end - should "raise RateLimitError on 429" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(make_rate_limit_error), status: 429) + should "raise RateLimitError on 429" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: JSON.generate(make_rate_limit_error), status: 429) - client = StripeClient.new + client = StripeClient.new - e = assert_raises Stripe::RateLimitError do - client.execute_request(:post, "/v1/charges") + e = assert_raises Stripe::RateLimitError do + client.send(request_method, :post, "/v1/charges") + end + assert_equal(429, e.http_status) + assert_equal(true, e.json_body.is_a?(Hash)) end - assert_equal(429, e.http_status) - assert_equal(true, e.json_body.is_a?(Hash)) - end - should "raise OAuth::InvalidRequestError when error is a string with value 'invalid_request'" do - stub_request(:post, "#{Stripe.connect_base}/oauth/token") - .to_return(body: JSON.generate(error: "invalid_request", - error_description: "No grant type specified"), status: 400) + should "raise OAuth::InvalidRequestError when error is a string with value 'invalid_request'" do + stub_request(:post, "#{Stripe.connect_base}/oauth/token") + .to_return(body: JSON.generate(error: "invalid_request", + error_description: "No grant type specified"), status: 400) - client = StripeClient.new + client = StripeClient.new + + opts = { api_base: Stripe.connect_base } + e = assert_raises Stripe::OAuth::InvalidRequestError do + client.send(request_method, :post, "/oauth/token", **opts) + end - opts = { api_base: Stripe.connect_base } - e = assert_raises Stripe::OAuth::InvalidRequestError do - client.execute_request(:post, "/oauth/token", **opts) + assert_equal(400, e.http_status) + assert_equal("No grant type specified", e.message) end - assert_equal(400, e.http_status) - assert_equal("No grant type specified", e.message) - end + should "raise OAuth::InvalidGrantError when error is a string with value 'invalid_grant'" do + stub_request(:post, "#{Stripe.connect_base}/oauth/token") + .to_return(body: JSON.generate(error: "invalid_grant", + error_description: "This authorization code has already been used. All tokens issued with this code have been revoked."), status: 400) - should "raise OAuth::InvalidGrantError when error is a string with value 'invalid_grant'" do - stub_request(:post, "#{Stripe.connect_base}/oauth/token") - .to_return(body: JSON.generate(error: "invalid_grant", - error_description: "This authorization code has already been used. All tokens issued with this code have been revoked."), status: 400) + client = StripeClient.new - client = StripeClient.new + opts = { api_base: Stripe.connect_base } + e = assert_raises Stripe::OAuth::InvalidGrantError do + client.send(request_method, :post, "/oauth/token", **opts) + end - opts = { api_base: Stripe.connect_base } - e = assert_raises Stripe::OAuth::InvalidGrantError do - client.execute_request(:post, "/oauth/token", **opts) + assert_equal(400, e.http_status) + assert_equal("invalid_grant", e.code) + assert_equal("This authorization code has already been used. All tokens issued with this code have been revoked.", e.message) end - assert_equal(400, e.http_status) - assert_equal("invalid_grant", e.code) - assert_equal("This authorization code has already been used. All tokens issued with this code have been revoked.", e.message) - end + should "raise OAuth::InvalidClientError when error is a string with value 'invalid_client'" do + stub_request(:post, "#{Stripe.connect_base}/oauth/deauthorize") + .to_return(body: JSON.generate(error: "invalid_client", + error_description: "This application is not connected to stripe account acct_19tLK7DSlTMT26Mk, or that account does not exist."), status: 401) - should "raise OAuth::InvalidClientError when error is a string with value 'invalid_client'" do - stub_request(:post, "#{Stripe.connect_base}/oauth/deauthorize") - .to_return(body: JSON.generate(error: "invalid_client", - error_description: "This application is not connected to stripe account acct_19tLK7DSlTMT26Mk, or that account does not exist."), status: 401) + client = StripeClient.new - client = StripeClient.new + opts = { api_base: Stripe.connect_base } + e = assert_raises Stripe::OAuth::InvalidClientError do + client.send(request_method, :post, "/oauth/deauthorize", **opts) + end - opts = { api_base: Stripe.connect_base } - e = assert_raises Stripe::OAuth::InvalidClientError do - client.execute_request(:post, "/oauth/deauthorize", **opts) + assert_equal(401, e.http_status) + assert_equal("invalid_client", e.code) + assert_equal("This application is not connected to stripe account acct_19tLK7DSlTMT26Mk, or that account does not exist.", e.message) end - assert_equal(401, e.http_status) - assert_equal("invalid_client", e.code) - assert_equal("This application is not connected to stripe account acct_19tLK7DSlTMT26Mk, or that account does not exist.", e.message) - end + should "raise Stripe::OAuthError on indeterminate OAuth error" do + stub_request(:post, "#{Stripe.connect_base}/oauth/deauthorize") + .to_return(body: JSON.generate(error: "new_code_not_recognized", + error_description: "Something."), status: 401) - should "raise Stripe::OAuthError on indeterminate OAuth error" do - stub_request(:post, "#{Stripe.connect_base}/oauth/deauthorize") - .to_return(body: JSON.generate(error: "new_code_not_recognized", - error_description: "Something."), status: 401) + client = StripeClient.new - client = StripeClient.new + opts = { api_base: Stripe.connect_base } + e = assert_raises Stripe::OAuth::OAuthError do + client.send(request_method, :post, "/oauth/deauthorize", **opts) + end - opts = { api_base: Stripe.connect_base } - e = assert_raises Stripe::OAuth::OAuthError do - client.execute_request(:post, "/oauth/deauthorize", **opts) + assert_equal(401, e.http_status) + assert_equal("new_code_not_recognized", e.code) + assert_equal("Something.", e.message) end - - assert_equal(401, e.http_status) - assert_equal("new_code_not_recognized", e.code) - assert_equal("Something.", e.message) end - end - context "idempotency keys" do - setup do - Stripe::StripeConfiguration.any_instance.stubs(:max_network_retries).returns(2) - end + context "idempotency keys" do + setup do + Stripe::StripeConfiguration.any_instance.stubs(:max_network_retries).returns(2) + end - should "not add an idempotency key to GET requests" do - SecureRandom.expects(:uuid).times(0) - stub_request(:get, "#{Stripe.api_base}/v1/charges/ch_123") - .with do |req| - req.headers["Idempotency-Key"].nil? - end.to_return(body: JSON.generate(object: "charge")) - client = StripeClient.new - client.execute_request(:get, "/v1/charges/ch_123") - end + should "not add an idempotency key to GET requests" do + SecureRandom.expects(:uuid).times(0) + stub_request(:get, "#{Stripe.api_base}/v1/charges/ch_123") + .with do |req| + req.headers["Idempotency-Key"].nil? + end.to_return(body: JSON.generate(object: "charge")) + client = StripeClient.new + client.send(request_method, :get, "/v1/charges/ch_123") + end - should "ensure there is always an idempotency_key on POST requests" do - SecureRandom.expects(:uuid).at_least_once.returns("random_key") - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .with(headers: { "Idempotency-Key" => "random_key" }) - .to_return(body: JSON.generate(object: "charge")) - client = StripeClient.new - client.execute_request(:post, "/v1/charges") - end + should "ensure there is always an idempotency_key on POST requests" do + SecureRandom.expects(:uuid).at_least_once.returns("random_key") + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .with(headers: { "Idempotency-Key" => "random_key" }) + .to_return(body: JSON.generate(object: "charge")) + client = StripeClient.new + client.send(request_method, :post, "/v1/charges") + end - should "ensure there is always an idempotency_key on DELETE requests" do - SecureRandom.expects(:uuid).at_least_once.returns("random_key") - stub_request(:delete, "#{Stripe.api_base}/v1/charges/ch_123") - .with(headers: { "Idempotency-Key" => "random_key" }) - .to_return(body: JSON.generate(object: "charge")) - client = StripeClient.new - client.execute_request(:delete, "/v1/charges/ch_123") - end + should "ensure there is always an idempotency_key on DELETE requests" do + SecureRandom.expects(:uuid).at_least_once.returns("random_key") + stub_request(:delete, "#{Stripe.api_base}/v1/charges/ch_123") + .with(headers: { "Idempotency-Key" => "random_key" }) + .to_return(body: JSON.generate(object: "charge")) + client = StripeClient.new + client.send(request_method, :delete, "/v1/charges/ch_123") + end - should "not override a provided idempotency_key" do - # Note that this expectation looks like `:idempotency_key` instead of - # the header `Idempotency-Key` because it's user provided as seen - # below. The ones injected by the library itself look like headers - # (`Idempotency-Key`), but rest-client does allow this symbol - # formatting and will properly override the system generated one as - # expected. - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .with(headers: { "Idempotency-Key" => "provided_key" }) - .to_return(body: JSON.generate(object: "charge")) + should "not override a provided idempotency_key" do + # Note that this expectation looks like `:idempotency_key` instead of + # the header `Idempotency-Key` because it's user provided as seen + # below. The ones injected by the library itself look like headers + # (`Idempotency-Key`), but rest-client does allow this symbol + # formatting and will properly override the system generated one as + # expected. + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .with(headers: { "Idempotency-Key" => "provided_key" }) + .to_return(body: JSON.generate(object: "charge")) - client = StripeClient.new - client.execute_request(:post, "/v1/charges", - headers: { idempotency_key: "provided_key" }) + client = StripeClient.new + client.send(request_method, :post, "/v1/charges", + headers: { idempotency_key: "provided_key" }) + end end - end - context "retry logic" do - setup do - Stripe::StripeConfiguration.any_instance.stubs(:max_network_retries).returns(2) - end + context "retry logic" do + setup do + Stripe::StripeConfiguration.any_instance.stubs(:max_network_retries).returns(2) + end - should "retry failed requests and raise if error persists" do - StripeClient.expects(:sleep_time).at_least_once.returns(0) - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_raise(Errno::ECONNREFUSED.new) + should "retry failed requests and raise if error persists" do + StripeClient.expects(:sleep_time).at_least_once.returns(0) + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_raise(Errno::ECONNREFUSED.new) - client = StripeClient.new - err = assert_raises Stripe::APIConnectionError do - client.execute_request(:post, "/v1/charges") + client = StripeClient.new + err = assert_raises Stripe::APIConnectionError do + client.send(request_method, :post, "/v1/charges") + end + assert_match(/Request was retried 2 times/, err.message) end - assert_match(/Request was retried 2 times/, err.message) - end - should "retry failed requests and return successful response" do - StripeClient.expects(:sleep_time).at_least_once.returns(0) - - i = 0 - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return do |_| - if i < 2 - i += 1 - raise Errno::ECONNREFUSED - else - { body: JSON.generate("id" => "myid") } + should "retry failed requests and return successful response" do + StripeClient.expects(:sleep_time).at_least_once.returns(0) + + i = 0 + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return do |_| + if i < 2 + i += 1 + raise Errno::ECONNREFUSED + else + { body: JSON.generate("id" => "myid") } + end end - end - client = StripeClient.new - client.execute_request(:post, "/v1/charges") - end + client = StripeClient.new + client.send(request_method, :post, "/v1/charges") + end - should "pass the client configuration when retrying" do - StripeClient.expects(:sleep_time) - .with(any_of(1, 2), - has_entry(:config, kind_of(Stripe::StripeConfiguration))) - .at_least_once.returns(0) + should "pass the client configuration when retrying" do + StripeClient.expects(:sleep_time) + .with(any_of(1, 2), + has_entry(:config, kind_of(Stripe::StripeConfiguration))) + .at_least_once.returns(0) - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_raise(Errno::ECONNREFUSED.new) + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_raise(Errno::ECONNREFUSED.new) - client = StripeClient.new - assert_raises Stripe::APIConnectionError do - client.execute_request(:post, "/v1/charges") + client = StripeClient.new + assert_raises Stripe::APIConnectionError do + client.send(request_method, :post, "/v1/charges") + end end end - end - context "params serialization" do - should "allows empty strings in params" do - client = StripeClient.new - client.execute_request(:get, "/v1/invoices/upcoming", params: { - customer: "cus_123", - coupon: "", - }) - assert_requested( - :get, - "#{Stripe.api_base}/v1/invoices/upcoming?", - query: { + context "params serialization" do + should "allows empty strings in params" do + client = StripeClient.new + client.send(request_method, :get, "/v1/invoices/upcoming", params: { customer: "cus_123", coupon: "", - } - ) - end + }) + assert_requested( + :get, + "#{Stripe.api_base}/v1/invoices/upcoming?", + query: { + customer: "cus_123", + coupon: "", + } + ) + end - should "filter nils in params" do - client = StripeClient.new - client.execute_request(:get, "/v1/invoices/upcoming", params: { - customer: "cus_123", - coupon: nil, - }) - assert_requested( - :get, - "#{Stripe.api_base}/v1/invoices/upcoming?", - query: { + should "filter nils in params" do + client = StripeClient.new + client.send(request_method, :get, "/v1/invoices/upcoming", params: { customer: "cus_123", - } - ) - end + coupon: nil, + }) + assert_requested( + :get, + "#{Stripe.api_base}/v1/invoices/upcoming?", + query: { + customer: "cus_123", + } + ) + end - should "merge query parameters in URL and params" do - client = StripeClient.new - client.execute_request(:get, "/v1/invoices/upcoming?coupon=25OFF", params: { - customer: "cus_123", - }) - assert_requested( - :get, - "#{Stripe.api_base}/v1/invoices/upcoming?", - query: { - coupon: "25OFF", + should "merge query parameters in URL and params" do + client = StripeClient.new + client.send(request_method, :get, "/v1/invoices/upcoming?coupon=25OFF", params: { customer: "cus_123", - } - ) - end + }) + assert_requested( + :get, + "#{Stripe.api_base}/v1/invoices/upcoming?", + query: { + coupon: "25OFF", + customer: "cus_123", + } + ) + end - should "prefer query parameters in params when specified in URL as well" do - client = StripeClient.new - client.execute_request(:get, "/v1/invoices/upcoming?customer=cus_query", params: { - customer: "cus_param", - }) - assert_requested( - :get, - "#{Stripe.api_base}/v1/invoices/upcoming?", - query: { + should "prefer query parameters in params when specified in URL as well" do + client = StripeClient.new + client.send(request_method, :get, "/v1/invoices/upcoming?customer=cus_query", params: { customer: "cus_param", - } - ) + }) + assert_requested( + :get, + "#{Stripe.api_base}/v1/invoices/upcoming?", + query: { + customer: "cus_param", + } + ) + end end end end + context "#execute_request" do + should "handle success response with empty body" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: "", status: 200) + + client = StripeClient.new + + e = assert_raises Stripe::APIError do + client.execute_request(:post, "/v1/charges") + end + assert_equal 'Invalid response object from API: "" (HTTP response code was 200)', e.message + end + end + + context "#execute_request_stream" do + should "returns a StripeStreamResponse with the body on success when no block is passed" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: "response body", status: 200) + + client = StripeClient.new + + resp, = client.execute_request_stream(:post, "/v1/charges") + + assert_instance_of Stripe::StripeStreamResponse, resp + assert_equal "response body", resp.io.read + end + + should "executes the read_body_chunk_block when passed" do + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_return(body: "response body", status: 200) + + client = StripeClient.new + + accumulated_body = +"" + + resp, = client.execute_request_stream(:post, "/v1/charges") do |body_chunk| + accumulated_body << body_chunk + end + + assert_instance_of Stripe::StripeHeadersOnlyResponse, resp + assert_equal "response body", accumulated_body + end + end + context "#connection_manager" do should "warn that #connection_manager is deprecated" do old_stderr = $stderr From a4abf31c7222019f36de1ae6af4ea13ea7861937 Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Fri, 18 Jun 2021 13:07:29 -0400 Subject: [PATCH 2/3] Rework response to have a common mixin. --- lib/stripe/stripe_response.rb | 151 ++++++++++++++-------------- test/stripe/api_resource_test.rb | 2 +- test/stripe/stripe_client_test.rb | 2 +- test/stripe/stripe_response_test.rb | 85 ++++++++++++---- 4 files changed, 138 insertions(+), 102 deletions(-) diff --git a/lib/stripe/stripe_response.rb b/lib/stripe/stripe_response.rb index a783b3d37..b0baed417 100644 --- a/lib/stripe/stripe_response.rb +++ b/lib/stripe/stripe_response.rb @@ -1,63 +1,54 @@ # frozen_string_literal: true 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 + # 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 StripeResponseHeaders + # 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 + # `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 - 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 + @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 - # The data contained by the HTTP body of the response deserialized from - # JSON. - attr_accessor :data - - # The raw HTTP body of the response. - attr_accessor :http_body + 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 + module StripeResponseBase # A Hash of the HTTP headers of the response. attr_accessor :http_headers @@ -67,41 +58,54 @@ def [](name) # The Stripe request ID of the response. attr_accessor :request_id + def self.populate_for_net_http(resp, http_resp) + resp.http_headers = StripeResponseHeaders.from_net_http(http_resp) + resp.http_status = http_resp.code.to_i + resp.request_id = http_resp["request-id"] + end + end + + # StripeResponse encapsulates some vitals of a response that came back from + # the Stripe API. + class StripeResponse + include StripeResponseBase + # The data contained by the HTTP body of the response deserialized from + # JSON. + attr_accessor :data + + # The raw HTTP body of the response. + attr_accessor :http_body + # 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 = Headers.from_net_http(http_resp) - resp.http_status = http_resp.code.to_i - resp.request_id = http_resp["request-id"] + StripeResponseBase.populate_for_net_http(resp, http_resp) resp end end + # We have to alias StripeResponseHeaders to StripeResponse::Headers, as this + # class used to be embedded within StripeResponse and we want to be backwards + # compatible. + StripeResponse::Headers = StripeResponseHeaders + # StripeStreamResponse includes header-related vitals of the - # response as well as an IO stream of the body content. + # response as well as as the raw HTTP body content. class StripeStreamResponse - attr_accessor :io - - # A Hash of the HTTP headers of the response. - attr_accessor :http_headers - - # The integer HTTP status code of the response. - attr_accessor :http_status + include StripeResponseBase - # The Stripe request ID of the response. - attr_accessor :request_id + # The raw HTTP body of the response. + attr_accessor :http_body # Initializes a StripeStreamResponse object from a Net::HTTP::HTTPResponse # object. def self.from_net_http(http_resp) resp = StripeStreamResponse.new - resp.io = StringIO.new http_resp.body - resp.http_headers = StripeResponse::Headers.from_net_http(http_resp) - resp.http_status = http_resp.code.to_i - resp.request_id = http_resp["request-id"] + resp.http_body = http_resp.body + StripeResponseBase.populate_for_net_http(resp, http_resp) resp end end @@ -111,22 +115,13 @@ def self.from_net_http(http_resp) # directly in a block and we explicitly don't want to store the body of the # response in memory. class StripeHeadersOnlyResponse - # A Hash of the HTTP headers of the response. - attr_accessor :http_headers - - # The integer HTTP status code of the response. - attr_accessor :http_status - - # The Stripe request ID of the response. - attr_accessor :request_id + include StripeResponseBase # Initializes a StripeHeadersOnlyResponse object from a # Net::HTTP::HTTPResponse object. def self.from_net_http(http_resp) resp = StripeHeadersOnlyResponse.new - resp.http_headers = StripeResponse::Headers.from_net_http(http_resp) - resp.http_status = http_resp.code.to_i - resp.request_id = http_resp["request-id"] + StripeResponseBase.populate_for_net_http(resp, http_resp) resp end end diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index e7e8532ac..2b7d2b330 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -662,7 +662,7 @@ def read_stream(params = {}, opts = {}, &read_body_chunk_block) resp = StreamTestAPIResource.new(id: "hi_123").read_stream({ foo: "bar" }, stripe_account: "acct_hi") assert_instance_of Stripe::StripeStreamResponse, resp - assert_equal "response body", resp.io.read + assert_equal "response body", resp.http_body end end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 26c55f79c..2dd2e8e34 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -1114,7 +1114,7 @@ class StripeClientTest < Test::Unit::TestCase resp, = client.execute_request_stream(:post, "/v1/charges") assert_instance_of Stripe::StripeStreamResponse, resp - assert_equal "response body", resp.io.read + assert_equal "response body", resp.http_body end should "executes the read_body_chunk_block when passed" do diff --git a/test/stripe/stripe_response_test.rb b/test/stripe/stripe_response_test.rb index 8cf7a8320..77d58d491 100644 --- a/test/stripe/stripe_response_test.rb +++ b/test/stripe/stripe_response_test.rb @@ -4,12 +4,12 @@ module Stripe class StripeResponseTest < Test::Unit::TestCase - context "Headers" do + context "StripeResponseHeaders" do should "allow case-insensitive header access" do headers = { "Request-Id" => "request-id" } http_resp = create_net_http_resp(200, "", headers) - headers = StripeResponse::Headers.from_net_http(http_resp) + headers = StripeResponseHeaders.from_net_http(http_resp) assert_equal "request-id", headers["request-id"] assert_equal "request-id", headers["Request-Id"] @@ -17,26 +17,26 @@ class StripeResponseTest < Test::Unit::TestCase end should "initialize without error" do - StripeResponse::Headers.new({}) - StripeResponse::Headers.new("Request-Id" => []) - StripeResponse::Headers.new("Request-Id" => ["request-id"]) + StripeResponseHeaders.new({}) + StripeResponseHeaders.new("Request-Id" => []) + StripeResponseHeaders.new("Request-Id" => ["request-id"]) end should "initialize with error on a malformed hash" do assert_raises(ArgumentError) do - StripeResponse::Headers.new(nil) + StripeResponseHeaders.new(nil) end assert_raises(ArgumentError) do - StripeResponse::Headers.new(1 => []) + StripeResponseHeaders.new(1 => []) end assert_raises(ArgumentError) do - StripeResponse::Headers.new("Request-Id" => 1) + StripeResponseHeaders.new("Request-Id" => 1) end assert_raises(ArgumentError) do - StripeResponse::Headers.new("Request-Id" => [1]) + StripeResponseHeaders.new("Request-Id" => [1]) end end @@ -44,7 +44,7 @@ class StripeResponseTest < Test::Unit::TestCase old_stderr = $stderr $stderr = StringIO.new begin - headers = StripeResponse::Headers.new("Duplicated" => %w[a b]) + headers = StripeResponseHeaders.new("Duplicated" => %w[a b]) assert_equal "a", headers["Duplicated"] assert_equal "Duplicate header values for `Duplicated`; returning only first", $stderr.string.rstrip @@ -54,20 +54,61 @@ class StripeResponseTest < Test::Unit::TestCase end end - 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) + [StripeResponse, StripeStreamResponse, StripeHeadersOnlyResponse].each do |response_class| + context "StripeResponseBase mixin for #{response_class}" do + context ".from_net_http" do + should "populate the base fields" do + code = 200 + body = '{"foo": "bar"}' + headers = { "Request-Id" => "request-id" } + http_resp = create_net_http_resp(code, body, headers) + + resp = response_class.from_net_http(http_resp) + + assert_equal "request-id", resp.http_headers["Request-ID"] + assert_equal code, resp.http_status + assert_equal "request-id", resp.request_id + end + end + end + end + + context "#StripeResponse" do + context ".from_net_http" do + should "converts to StripeResponse" do + code = 200 + body = '{"foo": "bar"}' + http_resp = create_net_http_resp(code, body, {}) + + resp = StripeResponse.from_net_http(http_resp) + + assert_instance_of StripeResponse, resp + assert_equal JSON.parse(body, symbolize_names: true), resp.data + assert_equal body, resp.http_body + end + end + + context "Headers backwards compatibility" do + should "alias StripeResponseHeaders" do + headers = StripeResponse::Headers.new("Request-Id" => ["request-id"]) + + assert_instance_of StripeResponseHeaders, headers + end + end + end - resp = StripeResponse.from_net_http(http_resp) + context "#StripeStreamResponse" do + context ".from_net_http" do + should "converts to StripeStreamResponse" do + code = 200 + body = '{"foo": "bar"}' + http_resp = create_net_http_resp(code, body, {}) - assert_equal JSON.parse(body, symbolize_names: true), resp.data - assert_equal body, resp.http_body - assert_equal "request-id", resp.http_headers["Request-ID"] - assert_equal code, resp.http_status - assert_equal "request-id", resp.request_id + resp = StripeStreamResponse.from_net_http(http_resp) + + assert_instance_of StripeStreamResponse, resp + assert_equal body, resp.http_body + end end end From 157d477b90386f3ae80f292cd96e064a66ff042e Mon Sep 17 00:00:00 2001 From: Dominic Charley-Roy Date: Tue, 22 Jun 2021 13:38:24 -0400 Subject: [PATCH 3/3] Update StripeClient to only allow streaming requests with a block. --- lib/stripe/stripe_client.rb | 11 +- lib/stripe/stripe_response.rb | 18 ---- test/stripe/api_resource_test.rb | 9 +- test/stripe/stripe_client_test.rb | 156 +++++++++++++++++----------- test/stripe/stripe_response_test.rb | 17 +-- 5 files changed, 109 insertions(+), 102 deletions(-) diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 605649ab6..c1b0abac8 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -246,6 +246,11 @@ def execute_request_stream(method, path, api_base: nil, api_key: nil, headers: {}, params: {}, &read_body_chunk_block) + unless block_given? + raise ArgumentError, + "execute_request_stream requires a read_body_chunk_block" + end + http_resp, api_key = execute_request_internal( method, path, api_base, api_key, headers, params, &read_body_chunk_block ) @@ -253,11 +258,7 @@ def execute_request_stream(method, path, # When the read_body_chunk_block is given, we no longer have access to the # response body at this point and so return a response object containing # only the headers. This is because the body was consumed by the block. - resp = if block_given? - StripeHeadersOnlyResponse.from_net_http(http_resp) - else - StripeStreamResponse.from_net_http(http_resp) - end + resp = StripeHeadersOnlyResponse.from_net_http(http_resp) [resp, api_key] end diff --git a/lib/stripe/stripe_response.rb b/lib/stripe/stripe_response.rb index b0baed417..8c8413105 100644 --- a/lib/stripe/stripe_response.rb +++ b/lib/stripe/stripe_response.rb @@ -92,24 +92,6 @@ def self.from_net_http(http_resp) # compatible. StripeResponse::Headers = StripeResponseHeaders - # StripeStreamResponse includes header-related vitals of the - # response as well as as the raw HTTP body content. - class StripeStreamResponse - include StripeResponseBase - - # The raw HTTP body of the response. - attr_accessor :http_body - - # Initializes a StripeStreamResponse object from a Net::HTTP::HTTPResponse - # object. - def self.from_net_http(http_resp) - resp = StripeStreamResponse.new - resp.http_body = http_resp.body - StripeResponseBase.populate_for_net_http(resp, http_resp) - resp - end - end - # StripeHeadersOnlyResponse includes only header-related vitals of the # response. This is used for streaming requests where the response was read # directly in a block and we explicitly don't want to store the body of the diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index 2b7d2b330..03717b358 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -654,15 +654,14 @@ def read_stream(params = {}, opts = {}, &read_body_chunk_block) assert_equal "response body", accumulated_body end - should "supports requesting without a block" do + should "fail when requesting without a block" do stub_request(:get, "#{Stripe.api_base}/v1/streams/hi_123/read") .with(query: { foo: "bar" }, headers: { "Stripe-Account" => "acct_hi" }) .to_return(body: "response body") - resp = StreamTestAPIResource.new(id: "hi_123").read_stream({ foo: "bar" }, stripe_account: "acct_hi") - - assert_instance_of Stripe::StripeStreamResponse, resp - assert_equal "response body", resp.http_body + assert_raises ArgumentError do + StreamTestAPIResource.new(id: "hi_123").read_stream({ foo: "bar" }, stripe_account: "acct_hi") + end end end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 2dd2e8e34..6774f67b1 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -448,6 +448,12 @@ class StripeClientTest < Test::Unit::TestCase %w[execute_request execute_request_stream].each do |request_method| context "request processing for #{request_method}" do + setup do + @read_body_chunk_block = if request_method == "execute_request_stream" + proc { |body_chunk| body_chunk } + end + end + context "headers" do should "support literal headers" do stub_request(:post, "#{Stripe.api_base}/v1/account") @@ -456,7 +462,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new client.send(request_method, :post, "/v1/account", - headers: { "Stripe-Account" => "bar" }) + headers: { "Stripe-Account" => "bar" }, + &@read_body_chunk_block) end should "support RestClient-style header keys" do @@ -466,7 +473,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new client.send(request_method, :post, "/v1/account", - headers: { stripe_account: "bar" }) + headers: { stripe_account: "bar" }, + &@read_body_chunk_block) end end @@ -511,11 +519,19 @@ class StripeClientTest < Test::Unit::TestCase request_id: "req_123", status: 200, config: Stripe.config) - Util.expects(:log_debug).with("Response details", - body: body, - idempotency_key: "abc", - request_id: "req_123", - config: Stripe.config) + Util.expects(:log_debug).with do |message, data| + if message == "Response details" && + data[:idempotency_key] == "abc" && + data[:request_id] == "req_123" && + data[:config] == Stripe.config + # Streaming requests have a different body. + if request_method == "execute_request_stream" + data[:body].is_a? Net::ReadAdapter + else + data[:body] == body + end + end + end Util.expects(:log_debug).with("Dashboard link for request", idempotency_key: "abc", request_id: "req_123", @@ -539,7 +555,8 @@ class StripeClientTest < Test::Unit::TestCase "Idempotency-Key" => "abc", "Stripe-Account" => "acct_123", "Stripe-Version" => "2010-11-12", - }) + }, + &@read_body_chunk_block) end should "produce logging on API error" do @@ -586,7 +603,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new assert_raises Stripe::APIError do - client.send(request_method, :post, "/v1/account") + client.send(request_method, :post, "/v1/account", + &@read_body_chunk_block) end end @@ -625,7 +643,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new opts = { api_base: Stripe.connect_base } assert_raises Stripe::OAuth::InvalidRequestError do - client.send(request_method, :post, "/oauth/token", **opts) + client.send(request_method, :post, "/oauth/token", **opts, + &@read_body_chunk_block) end end end @@ -641,7 +660,8 @@ class StripeClientTest < Test::Unit::TestCase .to_return(body: JSON.generate(object: "account")) client = StripeClient.new - client.send(request_method, :post, "/v1/account") + client.send(request_method, :post, "/v1/account", + &@read_body_chunk_block) ensure Stripe.stripe_account = old end @@ -655,7 +675,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new client.send(request_method, :post, "/v1/account", - headers: { stripe_account: stripe_account }) + headers: { stripe_account: stripe_account }, + &@read_body_chunk_block) end should "not send it otherwise" do @@ -665,7 +686,8 @@ class StripeClientTest < Test::Unit::TestCase end.to_return(body: JSON.generate(object: "account")) client = StripeClient.new - client.send(request_method, :post, "/v1/account") + client.send(request_method, :post, "/v1/account", + &@read_body_chunk_block) end end @@ -701,7 +723,8 @@ class StripeClientTest < Test::Unit::TestCase end.to_return(body: JSON.generate(object: "account")) client = StripeClient.new - client.send(request_method, :post, "/v1/account") + client.send(request_method, :post, "/v1/account", + &@read_body_chunk_block) ensure Stripe.app_info = old end @@ -716,7 +739,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::APIError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal 'Invalid response object from API: "" (HTTP response code was 500)', e.message end @@ -730,7 +754,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::InvalidRequestError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal("req_123", e.request_id) end @@ -742,7 +767,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::APIConnectionError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal StripeClient::ERROR_MESSAGE_CONNECTION % Stripe.api_base + "\n\n(Network error: Connection refused)", @@ -756,7 +782,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::APIError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal 'Invalid response object from API: "{\"bar\":\"foo\"}" (HTTP response code was 500)', e.message end @@ -771,7 +798,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::IdempotencyError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal(400, e.http_status) assert_equal(true, e.json_body.is_a?(Hash)) @@ -784,7 +812,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::InvalidRequestError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal(400, e.http_status) assert_equal(true, e.json_body.is_a?(Hash)) @@ -797,7 +826,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::AuthenticationError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal(401, e.http_status) assert_equal(true, e.json_body.is_a?(Hash)) @@ -810,7 +840,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::CardError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal(402, e.http_status) assert_equal(true, e.json_body.is_a?(Hash)) @@ -825,7 +856,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::PermissionError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal(403, e.http_status) assert_equal(true, e.json_body.is_a?(Hash)) @@ -838,7 +870,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::InvalidRequestError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal(404, e.http_status) assert_equal(true, e.json_body.is_a?(Hash)) @@ -851,7 +884,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new e = assert_raises Stripe::RateLimitError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_equal(429, e.http_status) assert_equal(true, e.json_body.is_a?(Hash)) @@ -866,7 +900,8 @@ class StripeClientTest < Test::Unit::TestCase opts = { api_base: Stripe.connect_base } e = assert_raises Stripe::OAuth::InvalidRequestError do - client.send(request_method, :post, "/oauth/token", **opts) + client.send(request_method, :post, "/oauth/token", **opts, + &@read_body_chunk_block) end assert_equal(400, e.http_status) @@ -882,7 +917,8 @@ class StripeClientTest < Test::Unit::TestCase opts = { api_base: Stripe.connect_base } e = assert_raises Stripe::OAuth::InvalidGrantError do - client.send(request_method, :post, "/oauth/token", **opts) + client.send(request_method, :post, "/oauth/token", **opts, + &@read_body_chunk_block) end assert_equal(400, e.http_status) @@ -899,7 +935,8 @@ class StripeClientTest < Test::Unit::TestCase opts = { api_base: Stripe.connect_base } e = assert_raises Stripe::OAuth::InvalidClientError do - client.send(request_method, :post, "/oauth/deauthorize", **opts) + client.send(request_method, :post, "/oauth/deauthorize", **opts, + &@read_body_chunk_block) end assert_equal(401, e.http_status) @@ -916,7 +953,8 @@ class StripeClientTest < Test::Unit::TestCase opts = { api_base: Stripe.connect_base } e = assert_raises Stripe::OAuth::OAuthError do - client.send(request_method, :post, "/oauth/deauthorize", **opts) + client.send(request_method, :post, "/oauth/deauthorize", **opts, + &@read_body_chunk_block) end assert_equal(401, e.http_status) @@ -937,7 +975,8 @@ class StripeClientTest < Test::Unit::TestCase req.headers["Idempotency-Key"].nil? end.to_return(body: JSON.generate(object: "charge")) client = StripeClient.new - client.send(request_method, :get, "/v1/charges/ch_123") + client.send(request_method, :get, "/v1/charges/ch_123", + &@read_body_chunk_block) end should "ensure there is always an idempotency_key on POST requests" do @@ -946,7 +985,8 @@ class StripeClientTest < Test::Unit::TestCase .with(headers: { "Idempotency-Key" => "random_key" }) .to_return(body: JSON.generate(object: "charge")) client = StripeClient.new - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end should "ensure there is always an idempotency_key on DELETE requests" do @@ -955,7 +995,8 @@ class StripeClientTest < Test::Unit::TestCase .with(headers: { "Idempotency-Key" => "random_key" }) .to_return(body: JSON.generate(object: "charge")) client = StripeClient.new - client.send(request_method, :delete, "/v1/charges/ch_123") + client.send(request_method, :delete, "/v1/charges/ch_123", + &@read_body_chunk_block) end should "not override a provided idempotency_key" do @@ -971,7 +1012,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new client.send(request_method, :post, "/v1/charges", - headers: { idempotency_key: "provided_key" }) + headers: { idempotency_key: "provided_key" }, + &@read_body_chunk_block) end end @@ -987,7 +1029,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new err = assert_raises Stripe::APIConnectionError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end assert_match(/Request was retried 2 times/, err.message) end @@ -1007,7 +1050,8 @@ class StripeClientTest < Test::Unit::TestCase end client = StripeClient.new - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end should "pass the client configuration when retrying" do @@ -1021,7 +1065,8 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new assert_raises Stripe::APIConnectionError do - client.send(request_method, :post, "/v1/charges") + client.send(request_method, :post, "/v1/charges", + &@read_body_chunk_block) end end end @@ -1029,10 +1074,9 @@ class StripeClientTest < Test::Unit::TestCase context "params serialization" do should "allows empty strings in params" do client = StripeClient.new - client.send(request_method, :get, "/v1/invoices/upcoming", params: { - customer: "cus_123", - coupon: "", - }) + client.send(request_method, :get, "/v1/invoices/upcoming", + params: { customer: "cus_123", coupon: "" }, + &@read_body_chunk_block) assert_requested( :get, "#{Stripe.api_base}/v1/invoices/upcoming?", @@ -1045,10 +1089,9 @@ class StripeClientTest < Test::Unit::TestCase should "filter nils in params" do client = StripeClient.new - client.send(request_method, :get, "/v1/invoices/upcoming", params: { - customer: "cus_123", - coupon: nil, - }) + client.send(request_method, :get, "/v1/invoices/upcoming", + params: { customer: "cus_123", coupon: nil }, + &@read_body_chunk_block) assert_requested( :get, "#{Stripe.api_base}/v1/invoices/upcoming?", @@ -1060,9 +1103,9 @@ class StripeClientTest < Test::Unit::TestCase should "merge query parameters in URL and params" do client = StripeClient.new - client.send(request_method, :get, "/v1/invoices/upcoming?coupon=25OFF", params: { - customer: "cus_123", - }) + client.send(request_method, :get, "/v1/invoices/upcoming?coupon=25OFF", + params: { customer: "cus_123" }, + &@read_body_chunk_block) assert_requested( :get, "#{Stripe.api_base}/v1/invoices/upcoming?", @@ -1075,9 +1118,9 @@ class StripeClientTest < Test::Unit::TestCase should "prefer query parameters in params when specified in URL as well" do client = StripeClient.new - client.send(request_method, :get, "/v1/invoices/upcoming?customer=cus_query", params: { - customer: "cus_param", - }) + client.send(request_method, :get, "/v1/invoices/upcoming?customer=cus_query", + params: { customer: "cus_param" }, + &@read_body_chunk_block) assert_requested( :get, "#{Stripe.api_base}/v1/invoices/upcoming?", @@ -1105,16 +1148,13 @@ class StripeClientTest < Test::Unit::TestCase end context "#execute_request_stream" do - should "returns a StripeStreamResponse with the body on success when no block is passed" do - stub_request(:post, "#{Stripe.api_base}/v1/charges") - .to_return(body: "response body", status: 200) - + should "requires a block" do client = StripeClient.new - resp, = client.execute_request_stream(:post, "/v1/charges") - - assert_instance_of Stripe::StripeStreamResponse, resp - assert_equal "response body", resp.http_body + e = assert_raises ArgumentError do + client.execute_request_stream(:post, "/v1/charges") + end + assert_equal "execute_request_stream requires a read_body_chunk_block", e.message end should "executes the read_body_chunk_block when passed" do diff --git a/test/stripe/stripe_response_test.rb b/test/stripe/stripe_response_test.rb index 77d58d491..9fa81aa1e 100644 --- a/test/stripe/stripe_response_test.rb +++ b/test/stripe/stripe_response_test.rb @@ -54,7 +54,7 @@ class StripeResponseTest < Test::Unit::TestCase end end - [StripeResponse, StripeStreamResponse, StripeHeadersOnlyResponse].each do |response_class| + [StripeResponse, StripeHeadersOnlyResponse].each do |response_class| context "StripeResponseBase mixin for #{response_class}" do context ".from_net_http" do should "populate the base fields" do @@ -97,21 +97,6 @@ class StripeResponseTest < Test::Unit::TestCase end end - context "#StripeStreamResponse" do - context ".from_net_http" do - should "converts to StripeStreamResponse" do - code = 200 - body = '{"foo": "bar"}' - http_resp = create_net_http_resp(code, body, {}) - - resp = StripeStreamResponse.from_net_http(http_resp) - - assert_instance_of StripeStreamResponse, resp - assert_equal body, resp.http_body - end - 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.