Clean up payment instruments code

[ob-stripe]

The current implementation of payment instruments (i.e. cards, bank accounts, sources) has a lot of inconsistencies and is hard to maintain.

Current state

Cards

• Entity: StripeCard

• API methods:

  • Creation:

    • On a customer: StripeCardService.Create() with StripeCardCreateOptions
    • On a recipient: StripeCardService.Create() with StripeCreditCardOptions
    • On an account: not possible, but you can pass a card when creating an account via StripeAccountCardOptions

  • Retrieval:

    • On a customer: StripeCardService.Get() with isRecipient=false
    • On a recipient: StripeCardService.Get() with isRecipient=true
    • On an account: not possible, but you can access the ExternalAccounts attribute which is a StripeList<Source> where Source is a wrapper over StripeCard / StripeBankAccount / StripeDeleted

  • Update:

    • On a customer: StripeCardService.Update() with StripeCardUpdateOptions and isRecipient=false
    • On a recipient: StripeCardService.Update() with StripeCardUpdateOptions and isRecipient=true
    • On an account: not possible

  • Delete:

    • On a customer: StripeCardService.Delete() with isRecipient=false
    • On a recipient: StripeCardService.Delete() with isRecipient=true
    • On an account: not possible

  • List:

    • On a customer: StripeCardService.List() with StripeListOptions and isRecipient=false
      • NB: currently this will hit /v1/customers/{CUSTOMER_ID}/sources which will return all payment sources, and non-cards will be incorrectly deserialized. Cf. StripeCardService.List() deserializing all types of sources as Card objects #989
    • On a recipient: StripeCardService.List() with StripeListOptions and isRecipient=true
    • On an account: not possible, but you can access the ExternalAccounts attribute (cf. "Retrieval" above)

Bank accounts

• Entities:

  • CustomerBankAccount when the bank account is a payment source on a customer
  • StripeBankAccount when the bank account is an external account on a custom account

• API methods:

  • Creation:

    • On a customer: BankAccountService.Create() with BankAccountCreateOptions
    • On an account: not possible, but you can pass a bank account when creating an account via StripeAccountBankAccountOptions

  • Retrieval:

    • On a customer: BankAccountService.Get()
    • On an account: not possible, but you can access the ExternalAccounts attribute which is a StripeList<Source> where Source is a wrapper over StripeCard / StripeBankAccount / StripeDeleted

  • Update:

    • On a customer: BankAccountService.Update() with BankAccountUpdateOptions
    • On an account: not possible

  • Delete:

    • On a customer: BankAccountService.Delete()
    • On an account: not possible

  • List:

    • On a customer: BankAccountService.List() with StripeListOptions
      • This will hit /v1/customers/{CUSTOMER_ID}/bank_accounts. While this returns the correct results, it is deprecated and hitting /v1/customers/{CUSTOMER_ID}/sources?object=bank_account would be preferable

  • Verify:

    • On a customer: BankAccountService.Verify() with BankAccountVerifyOptions
    • On an account: N/A (external accounts don't need to be verified)

Sources

• Entity: StripeSource

• API methods:

  • Creation: StripeSourceService.Create() with StripeSourceCreateOptions

  • Retrieval: StripeSourceService.Get()

  • Update: StripeSourceService.Update() with StripeSourceUpdateOptions

  • Delete: N/A (source objects are not deletable)

  • Attach to a customer: StripeSourceService.Attach() with StripeSourceAttachOptions

  • Detach from a customer: StripeSourceService.Detach()

  • Listing all sources on a customer: StripeSourceService.List() with StripeSourceListOptions (once Add support for listing sources on customers #1064 is merged)

  • Verify: not possible

    • Other libraries support this so we should implement this, but there are no source types that use this yet so this is very lo-pri.

TODO

• Decide whether methods for nested resources should live on the service for the top-level owning resource (e.g. StripeCustomerService) or the service for the owned resource (e.g. StripeCardService)
  Both sides have pros and cons. In the current state, most methods live on the service for the owned resource so that's probably what we should use in order to minimize breaking changes. This is slightly problematic for source objects which are a top-level API resource that can also be nested under customer objects, so we might need some special cases there.

• Get rid of CustomerBankAccount and use StripeBankAccount for all bank accounts, whether they're a customer's payment source or an account's external account.

• Add support for managing external accounts (cf. Support ExternalAccount Create, Edit, and List methods #980). If we do decide that methods should live on the service for the owned resource, then we might want to get rid of the isRecipient flag and either use different method names (CreateOnCustomer, CreateOnRecipient and CreateOnAccount) or an enum parameter to decide which URL to hit.

• Refactor the Source object, and ideally rename it as it is used as a "destination" and not a "source" in many places (external accounts, payout destinations)

• Get rid of StripeCreditCardOptions (or at least give it a better name)

• Add Stripe prefix to BankAccount* service + option objects

• Add StripeSourceService.Verify() method

I probably forgot a lot of things, feel free to comment/edit as needed!

[anelder-stripe]

re: Divergence between Card + Recipient Behavior

I really think we should just fully decouple recipient behavior into it's own methods, so we can add a deprecation notices everywhere.

re: Nested Ownership vs. Top-Level Ownership

I'm pro maintaining the status-quo here and keeping everything on the nested resource level. It diverges a bit from what we do in other bindings, but it does manage to keep these bindings quite clean. It also means fewer breaking changes (like you said) so ++.

re: CustomerBankAccount + StripeBankAccount

If it's actually possible to merge these without the JSON deserializer falling apart, I'm very much pro this idea!

re: Everything else listed

YES! ++