Skip to content
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
17 changes: 6 additions & 11 deletions internal/api/handler_ri_exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -1926,12 +1926,9 @@ func (h *Handler) getExchangeQuote(ctx context.Context, req *events.LambdaFuncti
return nil, mapAWSExchangeError("exchange quote failed", err)
}

// Echo the region the SDK chain resolved back to the caller (issue
// #238): the frontend modal has no other way to learn it, and
// execute rejects a missing region outright since exchanges are
// financially irreversible. Carrying the same value the quote used
// keeps quote and execute pinned to one region instead of letting
// execute re-resolve (and potentially land on a different one).
// Return the region the quote was priced in. Execute requires a region,
// and callers must send this same one so the exchange runs where it was
// priced.
return ExchangeQuoteResponse{ExchangeQuoteSummary: quote, Region: region}, nil
}

Expand Down Expand Up @@ -2196,11 +2193,9 @@ type ExchangeExecuteResponse struct {
ExchangeID string `json:"exchange_id"`
}

// ExchangeQuoteResponse wraps the library's quote summary with the
// region the SDK chain resolved (issue #238). The embedded pointer
// promotes the summary's own fields to the JSON top level, matching
// the shape callers got before this type existed; Region is the only
// addition.
// ExchangeQuoteResponse is the library's quote summary plus the region
// the quote was priced in, which callers must send back on execute. The embedded
// pointer promotes the summary's fields to the JSON top level.
type ExchangeQuoteResponse struct {
*exchange.ExchangeQuoteSummary
Region string `json:"Region"`
Expand Down
16 changes: 15 additions & 1 deletion internal/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2321,6 +2321,14 @@ paths:
application/json:
schema:
type: object
required: [Region]
properties:
Region:
type: string
description: >
AWS region the quote was priced in. Always set. Send
this same value as `region` on
`/api/ri-exchange/execute`.
'400':
$ref: '#/components/responses/BadRequest'
'401':
Expand Down Expand Up @@ -2366,7 +2374,7 @@ paths:
application/json:
schema:
type: object
required: [ri_ids, targets, max_payment_due_usd]
required: [ri_ids, targets, max_payment_due_usd, region]
properties:
ri_ids:
type: array
Expand All @@ -2379,6 +2387,12 @@ paths:
max_payment_due_usd:
type: string
description: Spend-cap guardrail (decimal string, e.g. "1000.00")
region:
type: string
description: >
AWS region to execute in. Required: a missing region is
rejected with 400 rather than defaulted, since an exchange
cannot be undone. Send the `Region` the quote returned.
responses:
'200':
description: Exchange submitted
Expand Down
105 changes: 105 additions & 0 deletions internal/api/openapi_ri_exchange_region_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package api

import (
"encoding/json"
"testing"

"github.com/LeanerCloud/cloud-commitments-go/pkg/exchange"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v3"
)

// riExchangeSpecSchema is the slice of an inline openapi.yaml schema these
// checks read.
type riExchangeSpecSchema struct {
Type string `yaml:"type"`
Required []string `yaml:"required"`
Properties map[string]struct {
Type string `yaml:"type"`
Description string `yaml:"description"`
} `yaml:"properties"`
}

type riExchangeSpecMedia struct {
Content map[string]struct {
Schema riExchangeSpecSchema `yaml:"schema"`
} `yaml:"content"`
}

type riExchangeSpecOp struct {
RequestBody riExchangeSpecMedia `yaml:"requestBody"`
Responses map[string]riExchangeSpecMedia `yaml:"responses"`
}

func loadRIExchangeSpecOp(t *testing.T, path string) riExchangeSpecOp {
t.Helper()
var spec struct {
Paths map[string]map[string]yaml.Node `yaml:"paths"`
}
require.NoError(t, yaml.Unmarshal(openapiSpec, &spec))
node, ok := spec.Paths[path]["post"]
require.True(t, ok, "post %s missing from openapi.yaml", path)
var op riExchangeSpecOp
require.NoError(t, node.Decode(&op), "post %s", path)
return op
}

// jsonKeys returns the top-level keys v marshals to, so the spec checks
// follow the Go json tags rather than a hand-copied field name.
func jsonKeys(t *testing.T, v any) map[string]bool {
t.Helper()
data, err := json.Marshal(v)
require.NoError(t, err)
var m map[string]json.RawMessage
require.NoError(t, json.Unmarshal(data, &m))
keys := make(map[string]bool, len(m))
for k := range m {
keys[k] = true
}
return keys
}

// TestOpenAPIRIExchangeRegionRequired keeps openapi.yaml in step with the
// handler on the exchange region (issue #363): execute refuses a request
// without a region (validateExecuteExchangeBody), and the quote response
// always carries the region the quote was priced in, which execute must send
// back.
func TestOpenAPIRIExchangeRegionRequired(t *testing.T) {
// The handler side of the contract: a body that is otherwise valid but
// names no region is a 400.
err := validateExecuteExchangeBody(ExchangeExecuteRequestBody{
RIIDs: []string{"ri-1"},
Targets: []ExchangeTargetBody{{OfferingID: "4b2293b4-5fbc-4017-9c75-d5a9d3aa8c91", Count: 1}},
MaxPaymentDueUSD: "100.00",
})
var ce *clientError
require.ErrorAs(t, err, &ce)
assert.Equal(t, 400, ce.code)
assert.Contains(t, ce.message, "region is required")

execKeys := jsonKeys(t, ExchangeExecuteRequestBody{Region: "us-west-2"})
require.True(t, execKeys["region"], "execute body's region json key changed; update this test and openapi.yaml")

execSchema, ok := loadRIExchangeSpecOp(t, "/api/ri-exchange/execute").RequestBody.Content["application/json"]
require.True(t, ok, "execute must document an application/json request body")
assert.Contains(t, execSchema.Schema.Required, "region", "execute must list region as required")
require.Contains(t, execSchema.Schema.Properties, "region", "execute must document the region property")
assert.Equal(t, "string", execSchema.Schema.Properties["region"].Type)
assert.NotEmpty(t, execSchema.Schema.Properties["region"].Description)

quoteKeys := jsonKeys(t, ExchangeQuoteResponse{
ExchangeQuoteSummary: &exchange.ExchangeQuoteSummary{},
Region: "us-west-2",
})
require.True(t, quoteKeys["Region"], "quote response's Region json key changed; update this test and openapi.yaml")

quoteSchema, ok := loadRIExchangeSpecOp(t, "/api/ri-exchange/quote").Responses["200"].Content["application/json"]
require.True(t, ok, "quote must document an application/json 200 response")
assert.Equal(t, "object", quoteSchema.Schema.Type)
assert.Contains(t, quoteSchema.Schema.Required, "Region", "quote response must list Region as always present")
require.Contains(t, quoteSchema.Schema.Properties, "Region", "quote response must document Region")
assert.Equal(t, "string", quoteSchema.Schema.Properties["Region"].Type)
assert.Contains(t, quoteSchema.Schema.Properties["Region"].Description, "execute",
"quote Region must tell callers to send it back on execute")
}
Loading