Implement "advanced" path parameter response reflection

[brandur-stripe]

Implement "advanced" path parameter response reflection

Builds on reflecting URL parameters as introduced in #59 to also support
reflecting parameters beyond just the object's primary ID. More
concretely, if we have a path like this:

/application_fees/fee_123/refunds/re_123

Before we'd just reflect the last part in (re_123), but here we now
reflect the parent object in as well (fee_123).

The one major caveat to this is that currently our query parameters are
not very introspectable in that we don't really know what kind of object
fee_123 is supposed to be. To that end, we use the name of the
parameter (in this case fee) and compare that it against two places
to see if it's likely a referencing this object:

* A fixture's `object` value.
* The name of the key containing an ID value of subobject.

This will work for some objects, but I'm probably going to go through
and either (1) do a pass in the backend to correctly name parameters, or
(2) implement a lookup table in stripe-mock to get this working 100%.

Does most of the work to address #61.

[brandur-stripe]

Here's the new code working with a replaced fee ID in an application fee refund response object:

$ curl -H "Authorization: Bearer sk_test_123" http://localhost:12112/v1/application_fees/fee_123/refunds/re_123
{
  "amount": 100,
  "created": 1234567890,
  "currency": "usd",
  "fee": "fee_123",
  "id": "re_123",
  "metadata": {},
  "object": "fee_refund"
}

And here's it working with an expanded fee object (it recurses through and tries to replace every fee ID found in either the direct child or any of its descendants):

$ curl -H "Authorization: Bearer sk_test_123" http://localhost:12112/v1/application_fees/fee_123/refunds/re_123 -d "expand[]=fee"
{
  "amount": 100,
  "created": 1234567890,
  "currency": "usd",
  "fee": {
    "account": "",
    "amount": 0,
    "amount_refunded": 0,
    "application": "",
    "balance_transaction": "",
    "charge": "",
    "created": 0,
    "currency": "",
    "id": "fee_123",
    "livemode": true,
    "object": "application_fee",
    "refunded": true,
    "refunds": {
      "data": [
        {
          "amount": 100,
          "created": 1234567890,
          "currency": "usd",
          "fee": "fee_123",
          "id": "re_123",
          "metadata": {},
          "object": "fee_refund"
        }
      ],
      "has_more": false,
      "object": "list",
      "url": ""
    }
  },
  "id": "re_123",
  "metadata": {},
  "object": "fee_refund"
}

[brandur-stripe]

@tmaxwell-stripe Unfortunately I had to write quite a lot of code to get this working property in order to account for all the sharp edges. Would you mind taking a quick pass to see if you can spot anything in here?

If it helps at all, although it's far from perfect, I'm fairly confident that the implementation is now quite a bit better than before along pretty much every dimension we care about: (1) ability to reason about code (it's still not easy to reason about per se, but it's gotten better), (2) how exhaustive the testing is, (3) documentation, and (4) handling of edge cases.

[tmaxwell-stripe]

This comment is no longer correct. Can you replace it with a comment saying that this is actually not passed down recursively?

[brandur-stripe]

Ah shoot. Yep, fixing.

[tmaxwell-stripe]

This file is getting big. We should consider moving the ID-replacement logic into a separate file.

[brandur-stripe]

Yeah, I think this is a good idea. Let me get it on a different pass though. (Aside from a different file, it should probably be in its own package to enforce some modularity too.)

[tmaxwell-stripe]

This two-step structure is counterintuitive; I didn't realize that replaceIDs was modifying PathParams. A couple ideas: Could we give replaceIDs a more descriptive name? Could we make it return a separate object instead of modifying PathParams, to make the data flow more obvious? Could these comments be clearer?

[brandur-stripe]

Yes, that's a great point. I ended up doing three things:

1. Renaming: replaceIDs is now called recordAndReplaceIDs to hint that it's also storing the IDs it replaces.
2. Changing the signatures. params.PathParams is still mutated in place, but the same struct is now returned as well just to make it clearer that information is flowing out of the first function and being used by the second.
3. Tightening up the comments.

Here's what the code looks like now:

	if params.PathParams != nil {
		// Passses through the generated data and replaces IDs that existed in
		// the fixtures with IDs that were extracted from the request path, if
		// and where appropriate.
		//
		// Note that the path params are mutated by the function, but we return
		// them anyway to make the control flow here more clear.
		pathParams := recordAndReplaceIDs(params.PathParams, data)

		// Passes through the generated data again to replace the values of any old
		// IDs that we replaced. This is a separate step because IDs could have
		// been found and replace at any point in the generation process.
		distributeReplacedIDs(pathParams, data)
	}

I think it'll help clarity a lot. Lemme know if you think of anything else I could improve, and I'll make those changes as well.

[tmaxwell-stripe]

r? @brandur-stripe

Looks generally good, although the implementation is getting unwieldly. I left some suggestions for how to make it easier to understand.

[brandur-stripe]

Thanks for the review Tim!

Going to pull this in for now, but if any other ideas come up, let me know.