Changes all arrays from classic Rack encoding:
``` sh
arr[]=...&arr[]=...&arr[]=...
```
To integer-indexed encoding:
``` sh
arr[0]=...&arr[1]=...&arr[2]=...
```
We think that this should be tractable now that we've fully converted
all endpoints over to the new AbstractAPIMethod infrastructure on the
backend (although we should do a little more testing to make sure that
all endpoints still work).
As part of the conversion, we also remove any places that we were "spot
encoding" to get required integer-indexed syntax. This should now all be
built in.
Remi pointed out in #666 that we basically just have to keep adding more
more onto the `Max` exception for both these rules every time we add a
new API resource.
Here I suggest that we modify the check on method length in two ways:
1. Permanently disable the cop on `Util.object_classes`. This is just
going to keep growing until we change are approach to it.
2. Choose a more reasonable maximum of 50 lines for elsewhere (IMO, the
default of 10 is just too short). Most of our methods already come in
below this, but there's a couple outliers like `#execute_request` in
`StripeClient`. If we knock over some of those, we could lower this
number again, but I suspect that we'd probably want somewhere closer
to 30 (instead of 10) event then.
I also disable the check on module length completely. I'm not convinced
this is a very good heuristic for code quality.
This changes the predicate supplied to the #colorize method to ensure
that if a logger is set, the colorizing ANSI escape codes are not applied.
This definitely appears to have been the intention behind the original
implementation, but the tests didn't reflect how .log_internal was
actually called. In reality, it is always supplied with an `out:`
argument, not nil. This caused all logger bound output to also be
colorized.
Adds the magic `frozen_string_literal: true` comment to every file and
enables a Rubocop rule to make sure that it's always going to be there
going forward as well.
See here for more background [1], but the basic idea is that unlike many
other languages, static strings in code are mutable by default. This has
since been acknowledged as not a particularly good idea, and the
intention is to rectify the mistake when Ruby 3 comes out, where all
string literals will be frozen. The `frozen_string_literal` magic
comment was introduced in Ruby 2.3 as a way of easing the transition,
and allows libraries and projects to freeze their literals in advance.
I don't think this is breaking in any way: it's possible that users
might've been pulling out one of are literals somehow and mutating it,
but that would probably not have been useful for anything and would
certainly not be recommended, so I'm quite comfortable pushing this
change through as a minor version.
As discussed in #641.
[1] https://stackoverflow.com/a/37799399
Excludes `idempotency_key` from opts to persist between API requests.
Obviously the same idempotency key is not something that we ever want to
use again.
Fixes#598.
We were previously using a bit of a hack to get a free deep copy
implementation through Ruby's marshaling framework. Lint call this out
as a security problem though, and rightfully so: when combined with
unsanitized user input, unmarshaling can result in very serious security
breaches involving arbitrary code execution.
This patch removes all uses of marshal/unmarshal in favor of
implementing a deep copy method for `StripeObject`. I also reworked some
of the constants around what keys are available for `opts`. I'm still
not completely happy with the results, but I think it's going to need a
slightly larger refactor in order to get somewhere truly good.
There is what could be a breaking change for people doing non-standard
stuff with the library: the opts that we copy with an object are now
whitelisted, so if they were being used to pass around extraneous data,
that might not work as expected anymore. But because this is a contract
that we never committed to, I don't think I'd bump the major version for
change.
I wanted to see what fixing Rubocop TODOs was like, so I tried to
eliminate all the easy ones. Most of these were pretty easy, and the
changes required are relatively minimal.
Some of the stuff left is harder. Pretty much everything under
`Metrics/*` is going to be a pretty big yak shave. A few of the others
are just going to need a little more work (e.g. `Style/ClassVars` and
`Style/GuardClause`). Going to stop here for now.
A few changes:
* Add a new `Util.log_error` method which will forward to the equivalent
of `#error` on a logger.
* Move errors produced by `StripeClient` to use `Util.log_error`.
* Change standard stdout logging behavior to log to stderr in the case
of `Util.log_error.
* Change `Stripe.log_level` values to be an enum in a similar fashion as
the standard library's built in `Logger`.
Adds support for setting `Stripe.logger` to a logger that's compatible
with `Logger` from Ruby's standard library. In set, the library will no
longer log to stdout, and instead emit straight to the logger and defer
decision on what log level to print to it.
Addresses a request in #566.
Adds logging support for stripe-ruby in a similar way that we did it for
stripe-python [1], with the idea that users you can optionally get some
additional low-cost-to-configure logging for operational visibility or
debugging.
I made a few tweaks from the Python implementation (which I'll try to
contribute back to there):
* Added an elapsed parameter to responses so you can tell how long they
lasted.
* Mixed in idempotency_key to all lines that users have a way to
aggregate logs related to a request from start to finish.
* Standardized naming between different log lines as much as possible.
* Detect a TTY and produce output that's colorized and formatted.
[1] https://github.com/stripe/stripe-python/pull/269
The Transfer object used to represent all movements of funds in Stripe. It
split in three resources:
- Transfer: this describes the movement of funds between Stripe accounts
and is specific to Stripe Connect.
- Payout: this describes the movement of funds from a Stripe account to a
bank account, debit card or any future payout method.
- RecipientTransfer: this describes the movement of funds from a Stripe
account to a Recipient's card or Bank Account. This is here for legacy
reasons and can only be accessed from an expanded BalanceTransaction.
This change is behind an API version so old API versions would still use
the Transfer object for everything while new API version would see the
split.
This applies beyond the new object as some properties/methods are removed
from Transfer and other properties are renamed on other objects.
This is a pretty pedestrian change, but here we alphabetize the list of
StripeObject class mappings so that it's easier to scan it for
accidental omissions.
* Add support for multiplan subscriptions:
Serialize indexed arrays into hashes with index keys in subscription create, subscription update, and upcoming invoice
Add a SubscriptionItem object that supports creation, deletion, update, listing, and retrieval
* Remove helpers that convert items array to indexed hash
This produces an error when we detect an "array of maps" that cannot be
encoded with `application/x-www-form-urlencoded`; that is to say, one
that does not have each hash starting with a consistent key that will
allow a Rack-compliant server to recognize boundaries.
So for example, this is fine:
```
items: [
{ :type => 'sku', :parent => 'sku_94ZYSC0wppRTbk' },
{ :type => 'discount', :amount => -10000, :currency => 'cad', :description => 'potato' }
],
```
But this is _not_ okay:
```
items: [
{ :type => 'sku', :parent => 'sku_94ZYSC0wppRTbk' },
{ :amount => -10000, :currency => 'cad', :description => 'potato', :type => 'discount' }
],
```
(`type` should be moved to the beginning of the array.)
The purpose of this change is to give users better feedback when they
run into an encoding problem like this one. Currently, they just get
something confusing from the server, and someone on support usually
needs to examine a request log to figure out what happened.
CI will fail until the changes in #453 are brought in.
Alphabetizing maps being encoded by key can cause problems because the
server side Rack relies on the fact that that a new array item will
start with a repeated key.
For example, given this encoding:
```
items: [
{ :type => 'sku', :parent => 'sku_94ZYSC0wppRTbk' },
{ :type => 'discount', :amount => -10000, :currency => 'cad', :description => 'potato' }
],
```
We need to have `type` appear first so that an array boundary is
recognized. So the encoded form should take:
```
items[][type]=sku&items[][parent]=...&items[][type]=discount&items[][amount]=...
```
But currently `type` gets sorted to the back, so we get something more
like:
```
items[][parent]=...&items[][type]=...&items[][amount]=...&items[][currency]=...&items[][description]=...&items[][type]=potato
```
Which the server will receive as this:
```
items: [
{ :type => 'sku', :parent => 'sku_94ZYSC0wppRTbk', :amount => -10000, :currency => 'cad', :description => 'potato' }
{ :type => 'discount' }
],
```
Here we remove the alphabetization to fix the problem and correct a bad
test.
I'm not sure exactly what changed here (did we change the `$VERBOSE`
setting?), but I'm not seeing a whole lot of warnings when running the
test suites locally and in CI. For example:
```
Started
........................................./home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
............../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
......../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
.../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
........./home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
...
..../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
....../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
..../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
......./home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
........./home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
........../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
................./home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
.../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
..../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
....../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
..........
........./home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
....../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
......../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
......../home/travis/build/stripe/stripe-ruby/lib/stripe/api_operations/list.rb:6: warning: instance variable @opts not initialized
............./home/travis/build/stripe/stripe-ruby/lib/stripe/stripe_object.rb:35: warning: instance variable @values not initialized
./home/travis/build/stripe/stripe-ruby/lib/stripe/stripe_object.rb:35: warning: instance variable @values not initialized
...................../home/travis/build/stripe/stripe-ruby/lib/stripe/transfer.rb:8: warning: instance variable @api_key not initialized
..............
..
Finished in 0.785621037 seconds.
```
Most of these are due to unused or uninitialized variables. This patch
fixes all warnings by fixing offending code.
