Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Handle file objects like file_upload #689

Merged
merged 3 commits into from
Sep 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ sudo: false
env:
global:
# If changing this number, please also change it in `test/test_helper.rb`.
- STRIPE_MOCK_VERSION=0.30.0
- STRIPE_MOCK_VERSION=0.32.0

cache:
directories:
Expand Down
4 changes: 2 additions & 2 deletions lib/stripe.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@
require "stripe/ephemeral_key"
require "stripe/event"
require "stripe/exchange_rate"
require "stripe/file"
require "stripe/file_link"
require "stripe/file_upload"
require "stripe/invoice"
require "stripe/invoice_item"
require "stripe/invoice_line_item"
Expand Down Expand Up @@ -103,7 +103,7 @@ module Stripe

@api_base = "https://api.stripe.com"
@connect_base = "https://connect.stripe.com"
@uploads_base = "https://uploads.stripe.com"
@uploads_base = "https://files.stripe.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

We may eventually want to rename this to files_base for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought of that but was afraid it should count as a breaking change. wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, true (although hopefully no one's overriding this). It does seem less churnful to just leave it for now though, so NM. If we ever feel strongly about it later, we can always change it then.


@log_level = nil
@logger = nil
Expand Down
20 changes: 11 additions & 9 deletions lib/stripe/file_upload.rb → lib/stripe/file.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
# frozen_string_literal: true

module Stripe
class FileUpload < APIResource
class File < APIResource
extend Stripe::APIOperations::Create
extend Stripe::APIOperations::List

OBJECT_NAME = "file_upload".freeze
# 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 = "file".freeze
OBJECT_NAME_ALT = "file_upload".freeze

def self.resource_url
"/v1/files"
end

def self.request(method, url, params = {}, opts = {})
opts = {
api_base: Stripe.uploads_base,
}.merge(Util.normalize_opts(opts))
super
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
Expand All @@ -27,9 +25,13 @@ def self.create(params = {}, opts = {})
end

opts = {
api_base: Stripe.uploads_base,
content_type: "multipart/form-data",
}.merge(Util.normalize_opts(opts))
super
end
end

# For backwards compatibility, the `File` class is aliased to `FileUpload`.
FileUpload = File
end
3 changes: 2 additions & 1 deletion lib/stripe/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ def self.object_classes # rubocop:disable Metrics/MethodLength
EphemeralKey::OBJECT_NAME => EphemeralKey,
Event::OBJECT_NAME => Event,
ExchangeRate::OBJECT_NAME => ExchangeRate,
File::OBJECT_NAME => File,
File::OBJECT_NAME_ALT => File,
FileLink::OBJECT_NAME => FileLink,
FileUpload::OBJECT_NAME => FileUpload,
Invoice::OBJECT_NAME => Invoice,
InvoiceItem::OBJECT_NAME => InvoiceItem,
InvoiceLineItem::OBJECT_NAME => InvoiceLineItem,
Expand Down
73 changes: 73 additions & 0 deletions test/stripe/file_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# frozen_string_literal: true

require ::File.expand_path("../../test_helper", __FILE__)

module Stripe
class FileTest < Test::Unit::TestCase
should "be listable" do
files = Stripe::File.list
assert_requested :get, "#{Stripe.api_base}/v1/files"
assert files.data.is_a?(Array)
assert files.data[0].is_a?(Stripe::File)
end

should "be retrievable" do
file = Stripe::File.retrieve("file_123")
assert_requested :get, "#{Stripe.api_base}/v1/files/file_123"
assert file.is_a?(Stripe::File)
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.
Stripe.api_base = nil
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit awkward, but it's necessary because if api_base and uploads_base have the same value, then assert_requested is not actually asserting that the request is for the correct host.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. It may be worth adding a comment that notes that api_base will be reset between every test case so this won't mutate the global state of test runs that come after it.

end

should "be creatable with a File" do
file = Stripe::File.create(
purpose: "dispute_evidence",
file: ::File.new(__FILE__)
)
assert_requested :post, "#{Stripe.uploads_base}/v1/files"
assert file.is_a?(Stripe::File)
end

should "be creatable with a Tempfile" do
tempfile = Tempfile.new("foo")
tempfile.write("Hello world")
tempfile.rewind

file = Stripe::File.create(
purpose: "dispute_evidence",
file: tempfile
)
assert_requested :post, "#{Stripe.uploads_base}/v1/files"
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)
)
assert_requested :post, "#{Stripe.uploads_base}/v1/files"
assert file.is_a?(Stripe::File)
end
end

should "be deserializable when `object=file`" do
file = Stripe::Util.convert_to_stripe_object({ object: "file" }, {})
assert file.is_a?(Stripe::File)
end

should "be deserializable when `object=file_upload`" do
file = Stripe::Util.convert_to_stripe_object({ object: "file_upload" }, {})
assert file.is_a?(Stripe::File)
end
end
end
70 changes: 50 additions & 20 deletions test/stripe/file_upload_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,73 @@
require ::File.expand_path("../../test_helper", __FILE__)

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

should "be creatable with a File" do
file = Stripe::FileUpload.create(
purpose: "dispute_evidence",
file: ::File.new(__FILE__)
)
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__)
)
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
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
)
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)
)
assert_requested :post, "#{Stripe.uploads_base}/v1/files"
assert file.is_a?(Stripe::FileUpload)
end
end

file = Stripe::FileUpload.create(
purpose: "dispute_evidence",
file: tempfile
)
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 creatable with Faraday::UploadIO" do
file = Stripe::FileUpload.create(
purpose: "dispute_evidence",
file: Faraday::UploadIO.new(::File.new(__FILE__), nil)
)
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
Expand Down
2 changes: 1 addition & 1 deletion test/stripe/issuing/dispute_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class DisputeTest < Test::Unit::TestCase
should "be creatable" do
dispute = Stripe::Issuing::Dispute.create(
reason: "fraudulent",
transaction: "ipi_123"
disputed_transaction: "ipi_123"
)
assert_requested :post, "#{Stripe.api_base}/v1/issuing/disputes"
assert dispute.is_a?(Stripe::Issuing::Dispute)
Expand Down
6 changes: 1 addition & 5 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
require ::File.expand_path("../test_data", __FILE__)

# If changing this number, please also change it in `.travis.yml`.
MOCK_MINIMUM_VERSION = "0.30.0".freeze
MOCK_MINIMUM_VERSION = "0.32.0".freeze
MOCK_PORT = ENV["STRIPE_MOCK_PORT"] || 12_111

# Disable all real network connections except those that are outgoing to
Expand Down Expand Up @@ -50,10 +50,6 @@ class TestCase
Stripe.api_key = "sk_test_123"
Stripe.api_base = "http://localhost:#{MOCK_PORT}"

# 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

stub_connect
end

Expand Down