Skip to content

Commit

Permalink
Add basic parameter validation support.
Browse files Browse the repository at this point in the history
Basic parameter validation checks for missing required parameters only.
Parameter validation can be disabled with
`aws.Config.DisableParamValidation = true`.

Fixes aws#1
  • Loading branch information
lsegal committed Mar 14, 2015
1 parent a0112dc commit 813fdd8
Show file tree
Hide file tree
Showing 52 changed files with 4,019 additions and 3,804 deletions.
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ generate-test: generate-protocol-test generate-integration-test
generate:
go generate ./aws
go generate ./service

test: generate-test
go test ./... -tags=integration
40 changes: 24 additions & 16 deletions aws/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,27 @@ import (
const DEFAULT_RETRIES = -1

var DefaultConfig = &Config{
Credentials: DefaultCreds(),
Endpoint: "",
Region: os.Getenv("AWS_REGION"),
DisableSSL: false,
ManualSend: false,
HTTPClient: http.DefaultClient,
LogLevel: 0,
MaxRetries: DEFAULT_RETRIES,
Credentials: DefaultCreds(),
Endpoint: "",
Region: os.Getenv("AWS_REGION"),
DisableSSL: false,
ManualSend: false,
HTTPClient: http.DefaultClient,
LogLevel: 0,
MaxRetries: DEFAULT_RETRIES,
DisableParamValidation: false,
}

type Config struct {
Credentials CredentialsProvider
Endpoint string
Region string
DisableSSL bool
ManualSend bool
HTTPClient *http.Client
LogLevel uint
MaxRetries int
Credentials CredentialsProvider
Endpoint string
Region string
DisableSSL bool
ManualSend bool
HTTPClient *http.Client
LogLevel uint
MaxRetries int
DisableParamValidation bool
}

func (c Config) Merge(newcfg *Config) *Config {
Expand Down Expand Up @@ -80,5 +82,11 @@ func (c Config) Merge(newcfg *Config) *Config {
cfg.MaxRetries = c.MaxRetries
}

if newcfg != nil && newcfg.DisableParamValidation {
cfg.DisableParamValidation = newcfg.DisableParamValidation
} else {
cfg.DisableParamValidation = c.DisableParamValidation
}

return &cfg
}
3 changes: 3 additions & 0 deletions aws/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package aws
import "container/list"

type Handlers struct {
Validate HandlerList
Build HandlerList
Sign HandlerList
Send HandlerList
Expand All @@ -16,6 +17,7 @@ type Handlers struct {

func (h *Handlers) copy() Handlers {
return Handlers{
Validate: h.Validate.copy(),
Build: h.Build.copy(),
Sign: h.Sign.copy(),
Send: h.Send.copy(),
Expand All @@ -30,6 +32,7 @@ func (h *Handlers) copy() Handlers {

// Clear removes callback functions for all handlers
func (h *Handlers) Clear() {
h.Validate.Init()
h.Build.Init()
h.Send.Init()
h.Sign.Init()
Expand Down
80 changes: 80 additions & 0 deletions aws/param_validator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package aws

import (
"fmt"
"reflect"
"strings"
)

func ValidateParameters(r *Request) {
if r.ParamsFilled() {
v := validator{errors: []string{}}
v.validateAny(reflect.ValueOf(r.Params), "")

if count := len(v.errors); count > 0 {
format := "%d validation errors:\n- %s"
msg := fmt.Sprintf(format, count, strings.Join(v.errors, "\n- "))
r.Error = APIError{Code: "InvalidParameter", Message: msg}
}
}
}

type validator struct {
errors []string
}

func (v *validator) validateAny(value reflect.Value, path string) {
value = reflect.Indirect(value)
if !value.IsValid() {
return
}

switch value.Kind() {
case reflect.Struct:
v.validateStruct(value, path)
case reflect.Slice:
for i := 0; i < value.Len(); i++ {
v.validateAny(value.Index(i), path+fmt.Sprintf("[%d]", i))
}
case reflect.Map:
for _, n := range value.MapKeys() {
v.validateAny(value.MapIndex(n), path+fmt.Sprintf("[%q]", n.String()))
}
}
}

func (v *validator) validateStruct(value reflect.Value, path string) {
prefix := "."
if path == "" {
prefix = ""
}

for i := 0; i < value.Type().NumField(); i++ {
f := value.Type().Field(i)
if strings.ToLower(f.Name[0:1]) == f.Name[0:1] {
continue
}
fvalue := value.FieldByName(f.Name)

notset := false
if f.Tag.Get("required") != "" {
switch fvalue.Kind() {
case reflect.Ptr, reflect.Slice:
if fvalue.IsNil() {
notset = true
}
default:
if !fvalue.IsValid() {
notset = true
}
}
}

if notset {
msg := "missing required parameter: " + path + prefix + f.Name
v.errors = append(v.errors, msg)
} else {
v.validateAny(fvalue, path+prefix+f.Name)
}
}
}
85 changes: 85 additions & 0 deletions aws/param_validator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package aws_test

import (
"testing"

"github.com/awslabs/aws-sdk-go/aws"
"github.com/stretchr/testify/assert"
)

var service = func() *aws.Service {
s := &aws.Service{
Config: &aws.Config{},
ServiceName: "mock-service",
APIVersion: "2015-01-01",
}
return s
}()

type StructShape struct {
RequiredList []*ConditionalStructShape `required:"true"`
RequiredMap *map[string]*ConditionalStructShape `required:"true"`
RequiredBool *bool `required:"true"`
OptionalStruct *ConditionalStructShape

hiddenParameter *string

metadataStructureShape
}

type metadataStructureShape struct {
SDKShapeTraits bool
}

type ConditionalStructShape struct {
Name *string `required:"true"`
SDKShapeTraits bool
}

func TestNoErrors(t *testing.T) {
input := &StructShape{
RequiredList: []*ConditionalStructShape{},
RequiredMap: &map[string]*ConditionalStructShape{
"key1": &ConditionalStructShape{Name: aws.String("Name")},
"key2": &ConditionalStructShape{Name: aws.String("Name")},
},
RequiredBool: aws.Boolean(true),
OptionalStruct: &ConditionalStructShape{Name: aws.String("Name")},
}

req := aws.NewRequest(service, &aws.Operation{}, input, nil)
aws.ValidateParameters(req)
assert.NoError(t, req.Error)
}

func TestMissingRequiredParameters(t *testing.T) {
input := &StructShape{}
req := aws.NewRequest(service, &aws.Operation{}, input, nil)
aws.ValidateParameters(req)
err := aws.Error(req.Error)

assert.Error(t, err)
assert.Equal(t, "InvalidParameter", err.Code)
assert.Equal(t, "3 validation errors:\n- missing required parameter: RequiredList\n- missing required parameter: RequiredMap\n- missing required parameter: RequiredBool", err.Message)
}

func TestNestedMissingRequiredParameters(t *testing.T) {
input := &StructShape{
RequiredList: []*ConditionalStructShape{&ConditionalStructShape{}},
RequiredMap: &map[string]*ConditionalStructShape{
"key1": &ConditionalStructShape{Name: aws.String("Name")},
"key2": &ConditionalStructShape{},
},
RequiredBool: aws.Boolean(true),
OptionalStruct: &ConditionalStructShape{},
}

req := aws.NewRequest(service, &aws.Operation{}, input, nil)
aws.ValidateParameters(req)
err := aws.Error(req.Error)

assert.Error(t, err)
assert.Equal(t, "InvalidParameter", err.Code)
assert.Equal(t, "3 validation errors:\n- missing required parameter: RequiredList[0].Name\n- missing required parameter: RequiredMap[\"key2\"].Name\n- missing required parameter: OptionalStruct.Name", err.Message)

}
4 changes: 4 additions & 0 deletions aws/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ func (r *Request) Presign(expireTime time.Duration) (string, error) {
func (r *Request) Build() error {
if !r.built {
r.Error = nil
r.Handlers.Validate.Run(r)
if r.Error != nil {
return r.Error
}
r.Handlers.Build.Run(r)
r.built = true
}
Expand Down
4 changes: 4 additions & 0 deletions aws/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ func (s *Service) Initialize() {
s.Handlers.ValidateResponse.PushBack(ValidateResponseHandler)
s.AddDebugHandlers()
s.buildEndpoint()

if !s.Config.DisableParamValidation {
s.Handlers.Validate.PushBack(ValidateParameters)
}
}

func (s *Service) buildEndpoint() {
Expand Down
5 changes: 5 additions & 0 deletions internal/model/api/passes.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ func (a *API) renameExportable() {
}

s.Payload = a.ExportableName(s.Payload)

// fix required trait names
for i, n := range s.Required {
s.Required[i] = a.ExportableName(n)
}
}
}

Expand Down
24 changes: 17 additions & 7 deletions internal/model/api/shape.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (ref *ShapeRef) GoType() string {
return ref.Shape.GoType()
}

func (ref *ShapeRef) GoTags(toplevel bool) string {
func (ref *ShapeRef) GoTags(toplevel bool, isRequired bool) string {
code := "`"
if ref.Location != "" {
code += `location:"` + ref.Location + `" `
Expand Down Expand Up @@ -174,13 +174,14 @@ func (ref *ShapeRef) GoTags(toplevel bool) string {
code += `xmlAttribute:"true" `
}

if isRequired {
code += `required:"true"`
}

if toplevel {
if ref.Shape.Payload != "" {
code += `payload:"` + ref.Shape.Payload + `" `
}
if len(ref.Shape.Required) > 0 {
code += `required:"` + strings.Join(ref.Shape.Required, ",") + `" `
}
if ref.XMLNamespace.Prefix != "" {
code += `xmlPrefix:"` + ref.XMLNamespace.Prefix + `" `
} else if ref.Shape.XMLNamespace.Prefix != "" {
Expand Down Expand Up @@ -212,21 +213,30 @@ func (s *Shape) GoCode() string {
m := s.MemberRefs[n]
if (m.Streaming || s.Streaming) && s.Payload == n {
s.API.imports["io"] = true
code += n + " io.ReadSeeker " + m.GoTags(false) + "\n"
code += n + " io.ReadSeeker " + m.GoTags(false, s.IsRequired(n)) + "\n"
} else {
code += n + " " + m.GoType() + " " + m.GoTags(false) + "\n"
code += n + " " + m.GoType() + " " + m.GoTags(false, s.IsRequired(n)) + "\n"
}
}
metaStruct := "metadata" + s.ShapeName
ref := &ShapeRef{ShapeName: s.ShapeName, API: s.API, Shape: s}
code += "\n" + metaStruct + " `json:\"-\", xml:\"-\"`\n"
code += "}\n\n"
code += "type " + metaStruct + " struct {\n"
code += "SDKShapeTraits bool " + ref.GoTags(true)
code += "SDKShapeTraits bool " + ref.GoTags(true, false)
code += "}"
default:
panic("Cannot generate toplevel shape for " + s.Type)
}

return util.GoFmt(code)
}

func (s *Shape) IsRequired(member string) bool {
for _, n := range s.Required {
if n == member {
return true
}
}
return false
}
12 changes: 6 additions & 6 deletions internal/protocol/jsonrpc/unmarshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,20 @@ func NewOutputService1ProtocolTest(config *OutputService1ProtocolTestConfig) *Ou
}

// OutputService1TestCaseOperation1Request generates a request for the OutputService1TestCaseOperation1 operation.
func (c *OutputService1ProtocolTest) OutputService1TestCaseOperation1Request(input *OutputService1TestShapeOutputService1TestCaseOperation1Input) (req *aws.Request, output *OutputService1TestShapeOutputService1TestCaseOperation1Output) {
func (c *OutputService1ProtocolTest) OutputService1TestCaseOperation1Request(input *OutputService1TestShapeOutputService1TestCaseOperation1Input) (req *aws.Request, output *OutputService1TestShapeOutputService1TestShapeOutputService1TestCaseOperation1Output) {
if opOutputService1TestCaseOperation1 == nil {
opOutputService1TestCaseOperation1 = &aws.Operation{
Name: "OperationName",
}
}

req = aws.NewRequest(c.Service, opOutputService1TestCaseOperation1, input, output)
output = &OutputService1TestShapeOutputService1TestCaseOperation1Output{}
output = &OutputService1TestShapeOutputService1TestShapeOutputService1TestCaseOperation1Output{}
req.Data = output
return
}

func (c *OutputService1ProtocolTest) OutputService1TestCaseOperation1(input *OutputService1TestShapeOutputService1TestCaseOperation1Input) (output *OutputService1TestShapeOutputService1TestCaseOperation1Output, err error) {
func (c *OutputService1ProtocolTest) OutputService1TestCaseOperation1(input *OutputService1TestShapeOutputService1TestCaseOperation1Input) (output *OutputService1TestShapeOutputService1TestShapeOutputService1TestCaseOperation1Output, err error) {
req, out := c.OutputService1TestCaseOperation1Request(input)
output = out
err = req.Send()
Expand All @@ -91,7 +91,7 @@ type metadataOutputService1TestShapeOutputService1TestCaseOperation1Input struct
SDKShapeTraits bool `type:"structure" json:",omitempty"`
}

type OutputService1TestShapeOutputService1TestCaseOperation1Output struct {
type OutputService1TestShapeOutputService1TestShapeOutputService1TestCaseOperation1Output struct {
Char *string `type:"character" json:",omitempty"`
Double *float64 `type:"double" json:",omitempty"`
FalseBool *bool `type:"boolean" json:",omitempty"`
Expand All @@ -101,10 +101,10 @@ type OutputService1TestShapeOutputService1TestCaseOperation1Output struct {
Str *string `type:"string" json:",omitempty"`
TrueBool *bool `type:"boolean" json:",omitempty"`

metadataOutputService1TestShapeOutputService1TestCaseOperation1Output `json:"-", xml:"-"`
metadataOutputService1TestShapeOutputService1TestShapeOutputService1TestCaseOperation1Output `json:"-", xml:"-"`
}

type metadataOutputService1TestShapeOutputService1TestCaseOperation1Output struct {
type metadataOutputService1TestShapeOutputService1TestShapeOutputService1TestCaseOperation1Output struct {
SDKShapeTraits bool `type:"structure" json:",omitempty"`
}

Expand Down
Loading

0 comments on commit 813fdd8

Please sign in to comment.