Skip to content

Commit

Permalink
Reflect an object's primary ID from a path back in the response
Browse files Browse the repository at this point in the history 
When requesting a path from `stripe-mock` with an object's primary ID in
the URL:

```
GET /v1/charges/ch_idFromURL
```

Previously, we'd return the ID from the bundled fixtures. This patch
changes the behavior to try and detect when the path contains an
object's primary ID and reflect that back into the respond instead:

```
{
    "id": "ch_idFromURL",
}
```

(Previously, this would have been `ch_123` or whatever the fixtures
contained.)

We also tweak things in a few other places for consistency. If we find a
string value in our generated response with the same value as the ID we
replaced, we replace it as well. This allows an embedded resource with
an ID field that references its parent to reference it with a sane ID
(e.g., the `charge` field on a refund that's embedded in a charge's
`refunds` sublist).

We also modify URLs in embedded sublists to contain the right ID. e.g.,

```
{
    "url": "/v1/charges/ch_idFromURL"
}
```

This is necessary because those URLs are used by our client libraries to
determine where they should make calls to (e.g., in `stripe-ruby`
calling `charge.refunds.create(...)` uses that `url` values to know
where to post).

All of this is designed to get our test suite working again with the
latest OpenAPI fixtures. In particular, this will allow us to return the
right `id` for any deleted object, which will allow `stripe-ruby`'s web
mock checks to pass correctly.
  • Loading branch information
@brandur
brandur committed Apr 5, 2018
1 parent d58d54d commit b42b8bc
Show file tree
Hide file tree
Showing 5 changed files with 219 additions and 28 deletions.
91 changes: 78 additions & 13 deletions generator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ type DataGenerator struct {
}

// Generate generates a fixture response.
func (g *DataGenerator) Generate(schema *spec.Schema, requestPath string, expansions *ExpansionLevel) (interface{}, error) {
return g.generateInternal(schema, requestPath, expansions, nil, fmt.Sprintf("Responding to %s:\n", requestPath))
func (g *DataGenerator) Generate(schema *spec.Schema, requestPath string, id *string, expansions *ExpansionLevel) (interface{}, error) {
return g.generateInternal(schema, requestPath, id, nil, true, expansions, nil, fmt.Sprintf("Responding to %s:\n", requestPath))
}

// The purpose of this simple wrapper is so that we can write "example = nil"
Expand All @@ -30,7 +30,7 @@ type Example struct {
value interface{}
}

func (g *DataGenerator) generateInternal(schema *spec.Schema, requestPath string, expansions *ExpansionLevel, example *Example, context string) (interface{}, error) {
func (g *DataGenerator) generateInternal(schema *spec.Schema, requestPath string, id *string, replacedID *string, doReplaceID bool, expansions *ExpansionLevel, example *Example, context string) (interface{}, error) {
// This is a bit of a mess. We don't have an elegant fully-general approach to
// generating examples, just a bunch of specific cases that we know how to
// handle. If we find ourselves in a situation that doesn't match any of the
Expand Down Expand Up @@ -70,14 +70,16 @@ func (g *DataGenerator) generateInternal(schema *spec.Schema, requestPath string
if expansions != nil {
// We're expanding this specific object
result, err := g.generateInternal(
schema.XExpansionResources.OneOf[0], requestPath, expansions, nil,
schema.XExpansionResources.OneOf[0],
requestPath, id, replacedID, false, expansions, nil,
fmt.Sprintf("%sExpanding optional expandable field:\n", context))
return result, err
} else {
// We're not expanding this specific object. Our example should be of the
// unexpanded form, which is the first branch of the AnyOf
result, err := g.generateInternal(
schema.AnyOf[0], requestPath, expansions, example,
schema.AnyOf[0],
requestPath, id, replacedID, false, expansions, example,
fmt.Sprintf("%sNot expanding optional expandable field:\n", context))
return result, err
}
Expand All @@ -91,7 +93,8 @@ func (g *DataGenerator) generateInternal(schema *spec.Schema, requestPath string
} else {
// Since there's only one subschema, we can confidently recurse into it
result, err := g.generateInternal(
schema.AnyOf[0], requestPath, expansions, example,
schema.AnyOf[0],
requestPath, id, replacedID, doReplaceID, expansions, example,
fmt.Sprintf("%sChoosing only branch of anyOf:\n", context))
return result, err
}
Expand All @@ -102,7 +105,8 @@ func (g *DataGenerator) generateInternal(schema *spec.Schema, requestPath string
// in any example, even if we have an example available, because we don't
// know which branch of the AnyOf the example corresponds to.
result, err := g.generateInternal(
schema.AnyOf[0], requestPath, expansions, nil,
schema.AnyOf[0],
requestPath, id, replacedID, doReplaceID, expansions, nil,
fmt.Sprintf("%sChoosing first branch of anyOf:\n", context))
return result, err
}
Expand All @@ -111,7 +115,7 @@ func (g *DataGenerator) generateInternal(schema *spec.Schema, requestPath string
// We special-case list resources and always fill in the list with at least
// one item of data, regardless of what was present in the example
listData, err := g.generateListResource(
schema, requestPath, expansions, example, context)
schema, requestPath, id, replacedID, expansions, example, context)
return listData, err
}

Expand All @@ -129,6 +133,20 @@ func (g *DataGenerator) generateInternal(schema *spec.Schema, requestPath string
return nil, nil
}

// If we replaced a primary object ID, then also replace any of values that
// we happen to find which had the same value as it. These will usually be
// IDs in child objects that reference the parent.
//
// For example, a charge may contain a sublist of refunds. If we replaced
// the charge's ID, we also want to replace that charge ID in every one of
// the child refunds.
if replacedID != nil && schema.Type == "string" {
valStr, ok := example.value.(string)
if ok && valStr == *replacedID {
example = &Example{value: *id}
}
}

if schema.Type == "boolean" || schema.Type == "integer" ||
schema.Type == "number" || schema.Type == "string" {
return example.value, nil
Expand Down Expand Up @@ -156,7 +174,38 @@ func (g *DataGenerator) generateInternal(schema *spec.Schema, requestPath string

resultMap := make(map[string]interface{})

// We might have obtained an ID for the object from an extracted path
// parameter. If we did, fill it in. Note that this only occurs at the
// top level of recursion because any ID fields we find at other levels
// are likely for other objects.
//
// If we do replace an ID, extract the old one so that we can inject it
// into list URLs from our fixtures.
//
// This replacement must occur before iterating through the loop below
// because we might also use the new ID to replace other values in the
// object as well.
if doReplaceID && id != nil {
_, ok := schema.Properties["id"]
if ok {
idValue, ok := exampleMap["id"]
if ok {
idValueStr := idValue.(string)
replacedID = &idValueStr
resultMap["id"] = *id

if verbose {
fmt.Printf("Found ID to replace; previous: '%s' new: '%s'\n", *replacedID, *id)
}
}
}
}

for key, subSchema := range schema.Properties {
// If these conditions are met this was handled above. Skip it.
if doReplaceID && key == "id" && replacedID != nil {
continue
}

var subExpansions *ExpansionLevel
if expansions != nil {
Expand Down Expand Up @@ -185,7 +234,8 @@ func (g *DataGenerator) generateInternal(schema *spec.Schema, requestPath string
}

subValue, err := g.generateInternal(
subSchema, requestPath, subExpansions, subExample,
subSchema,
requestPath, id, replacedID, false, subExpansions, subExample,
fmt.Sprintf("%sIn property '%s' of object:\n", context, key))
if err != nil {
return nil, err
Expand Down Expand Up @@ -216,7 +266,7 @@ func (g *DataGenerator) maybeDereference(schema *spec.Schema, context string) (*
return schema, context, nil
}

func (g *DataGenerator) generateListResource(schema *spec.Schema, requestPath string, expansions *ExpansionLevel, example *Example, context string) (interface{}, error) {
func (g *DataGenerator) generateListResource(schema *spec.Schema, requestPath string, id *string, replacedID *string, expansions *ExpansionLevel, example *Example, context string) (interface{}, error) {

var itemExpansions *ExpansionLevel
if expansions != nil {
Expand All @@ -226,6 +276,9 @@ func (g *DataGenerator) generateListResource(schema *spec.Schema, requestPath st
itemData, err := g.generateInternal(
schema.Properties["data"].Items,
requestPath,
id,
replacedID,
false,
itemExpansions,
nil,
fmt.Sprintf("%sPopulating list resource:\n", context))
Expand All @@ -249,16 +302,28 @@ func (g *DataGenerator) generateListResource(schema *spec.Schema, requestPath st
case "total_count":
val = 1
case "url":
var url string
if strings.HasPrefix(subSchema.Pattern, "^") {
// Many list resources have a URL pattern of the form "^/v1/whatevers";
// we cut off the "^" to leave the URL.
val = subSchema.Pattern[1:]
url = subSchema.Pattern[1:]
} else if example != nil {
// If an example was provided, we can assume it has the correct format
example := example.value.(map[string]interface{})
val = example["url"]
url = example["url"].(string)
} else {
url = requestPath
}

// Potentially replace a primary ID in the URL of a list so that
// requests against it may be consistent. For example, if
// `/v1/charges/ch_123` was requested, we'd want the refunds list
// within the returned object to have a URL like
// `/v1/charges/ch_123/refunds`.
if replacedID != nil {
val = strings.Replace(url, *replacedID, *id, 1)
} else {
val = requestPath
val = url
}
default:
val = nil
Expand Down
39 changes: 34 additions & 5 deletions generator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestConcurrentAccess(t *testing.T) {
go func() {
defer wg.Done()
_, err := generator.Generate(
&spec.Schema{Ref: "#/components/schemas/subscription"}, "", nil)
&spec.Schema{Ref: "#/components/schemas/subscription"}, "", nil, nil)
assert.NoError(t, err)
}()
}
Expand All @@ -68,7 +68,7 @@ func TestGenerateResponseData(t *testing.T) {
// basic reference
generator = DataGenerator{testSpec.Components.Schemas, &testFixtures}
data, err = generator.Generate(
&spec.Schema{Ref: "#/components/schemas/charge"}, "", nil)
&spec.Schema{Ref: "#/components/schemas/charge"}, "", nil, nil)
assert.Nil(t, err)
assert.Equal(t,
testFixtures.Resources["charge"].(map[string]interface{})["id"],
Expand All @@ -84,6 +84,7 @@ func TestGenerateResponseData(t *testing.T) {
data, err = generator.Generate(
&spec.Schema{Ref: "#/components/schemas/charge"},
"",
nil,
&ExpansionLevel{expansions: map[string]*ExpansionLevel{"customer": {expansions: map[string]*ExpansionLevel{}}}})
assert.Nil(t, err)
assert.Equal(t,
Expand All @@ -95,6 +96,7 @@ func TestGenerateResponseData(t *testing.T) {
data, err = generator.Generate(
&spec.Schema{Ref: "#/components/schemas/charge"},
"",
nil,
&ExpansionLevel{expansions: map[string]*ExpansionLevel{"id": {expansions: map[string]*ExpansionLevel{}}}})
assert.Equal(t, err, errExpansionNotSupported)

Expand All @@ -103,6 +105,7 @@ func TestGenerateResponseData(t *testing.T) {
data, err = generator.Generate(
&spec.Schema{Ref: "#/components/schemas/charge"},
"",
nil,
&ExpansionLevel{expansions: map[string]*ExpansionLevel{"customer.id": {expansions: map[string]*ExpansionLevel{}}}})
assert.Equal(t, err, errExpansionNotSupported)

Expand All @@ -111,6 +114,7 @@ func TestGenerateResponseData(t *testing.T) {
data, err = generator.Generate(
&spec.Schema{Ref: "#/components/schemas/charge"},
"",
nil,
&ExpansionLevel{wildcard: true})
assert.Nil(t, err)
assert.Equal(t,
Expand All @@ -119,7 +123,7 @@ func TestGenerateResponseData(t *testing.T) {

// list
generator = DataGenerator{testSpec.Components.Schemas, &testFixtures}
data, err = generator.Generate(listSchema, "/v1/charges", nil)
data, err = generator.Generate(listSchema, "/v1/charges", nil, nil)
assert.Nil(t, err)
assert.Equal(t, "list", data.(map[string]interface{})["object"])
assert.Equal(t, "/v1/charges", data.(map[string]interface{})["url"])
Expand Down Expand Up @@ -148,14 +152,39 @@ func TestGenerateResponseData(t *testing.T) {
"charges_list": listSchema,
},
XResourceID: "with_charges_list",
}, "", nil)
}, "", nil, nil)
assert.Nil(t, err)
chargesList := data.(map[string]interface{})["charges_list"]
assert.Equal(t, "list", chargesList.(map[string]interface{})["object"])
assert.Equal(t, "/v1/charges", chargesList.(map[string]interface{})["url"])
assert.Equal(t,
testFixtures.Resources["charge"].(map[string]interface{})["id"],
chargesList.(map[string]interface{})["data"].([]interface{})[0].(map[string]interface{})["id"])

// injected ID
generator = DataGenerator{testSpec.Components.Schemas, &spec.Fixtures{
Resources: map[spec.ResourceID]interface{}{
spec.ResourceID("charge"): map[string]interface{}{
// This is contrived, but we inject the value we expect to be
// replaced into `customer` as well so that we can verify the
// secondary behavior that replaces all values that look like a
// replaced ID (as well as the ID).
"customer": "ch_123",

"id": "ch_123",
},
},
}}
id := "ch_123_InjectedFromURL"
data, err = generator.Generate(
&spec.Schema{Ref: "#/components/schemas/charge"}, "", &id, nil)
assert.Nil(t, err)
assert.Equal(t,
id,
data.(map[string]interface{})["id"])
assert.Equal(t,
id,
data.(map[string]interface{})["customer"])
}

func TestValidFixtures(t *testing.T) {
Expand Down Expand Up @@ -196,7 +225,7 @@ func testCanGenerate(t *testing.T, path spec.Path, schema *spec.Schema, expand b
var example interface{}
var err error
assert.NotPanics(t, func() {
example, err = generator.Generate(schema, string(path), expansions)
example, err = generator.Generate(schema, string(path), nil, expansions)
})
assert.NoError(t, err)

Expand Down
21 changes: 21 additions & 0 deletions main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ import (
"github.com/stripe/stripe-mock/spec"
)

var applicationFeeRefundCreateMethod *spec.Operation
var applicationFeeRefundGetMethod *spec.Operation
var chargeAllMethod *spec.Operation
var chargeCreateMethod *spec.Operation
var chargeDeleteMethod *spec.Operation
var chargeGetMethod *spec.Operation
var invoicePayMethod *spec.Operation

// Try to avoid using the real spec as much as possible because it's more
// complicated and slower. A test spec is provided below. If you do use it,
Expand Down Expand Up @@ -54,6 +57,11 @@ func initRealSpec() {
}

func initTestSpec() {
// These are basically here to give us a URL to test against that has
// multiple parameters in it.
applicationFeeRefundCreateMethod = &spec.Operation{}
applicationFeeRefundGetMethod = &spec.Operation{}

chargeAllMethod = &spec.Operation{}
chargeCreateMethod = &spec.Operation{
RequestBody: &spec.RequestBody{
Expand Down Expand Up @@ -85,6 +93,10 @@ func initTestSpec() {
chargeDeleteMethod = &spec.Operation{}
chargeGetMethod = &spec.Operation{}

// Here so we can test the relatively rare "action" operations (e.g.,
// `POST` to `/pay` on an invoice).
invoicePayMethod = &spec.Operation{}

testFixtures =
spec.Fixtures{
Resources: map[spec.ResourceID]interface{}{
Expand Down Expand Up @@ -127,6 +139,12 @@ func initTestSpec() {
},
},
Paths: map[spec.Path]map[spec.HTTPVerb]*spec.Operation{
spec.Path("/v1/application_fees/{fee}/refunds"): {
"get": applicationFeeRefundCreateMethod,
},
spec.Path("/v1/application_fees/{fee}/refunds/{id}"): {
"get": applicationFeeRefundGetMethod,
},
spec.Path("/v1/charges"): {
"get": chargeAllMethod,
"post": chargeCreateMethod,
Expand All @@ -135,6 +153,9 @@ func initTestSpec() {
"get": chargeGetMethod,
"delete": chargeDeleteMethod,
},
spec.Path("/v1/invoices/{id}/pay"): {
"post": invoicePayMethod,
},
},
}
}
Loading

0 comments on commit b42b8bc

Please sign in to comment.