Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

private/protocol/restjson: Define JSONValue marshaling for body and querystring #1640

Merged
merged 6 commits into from
Nov 17, 2017
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
3 changes: 2 additions & 1 deletion models/protocol_tests/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ func GenerateAssertions(out interface{}, shape *api.Shape, prefix string) string
code += GenerateAssertions(v, s, prefix+"[\""+k+"\"]")
}
} else if shape.Type == "jsonvalue" {
code += fmt.Sprintf("reflect.DeepEqual(%s, map[string]interface{}%s)", prefix, walkMap(out.(map[string]interface{})))
code += fmt.Sprintf("reflect.DeepEqual(%s, map[string]interface{}%s)\n", prefix, walkMap(out.(map[string]interface{})))
} else {
for _, k := range keys {
v := t[k]
Expand Down Expand Up @@ -517,6 +517,7 @@ func getType(t string) uint {
}

func main() {
fmt.Println("Generating test suite", os.Args[1:])
out := generateTestSuite(os.Args[1])
if len(os.Args) == 3 {
f, err := os.Create(os.Args[2])
Expand Down
68 changes: 61 additions & 7 deletions models/protocol_tests/input/rest-json.json
Original file line number Diff line number Diff line change
Expand Up @@ -1310,17 +1310,46 @@
"shapes": {
"InputShape": {
"type": "structure",
"payload": "Body",
"members": {
"Attr": {
"HeaderField": {
"shape": "StringType",
"jsonvalue": true,
"location": "header",
"locationName": "X-Amz-Foo"
"jsonvalue": true,
"location": "header",
"locationName": "X-Amz-Foo"
},
"QueryField": {
"shape": "StringType",
"jsonvalue": true,
"location": "querystring",
"locationName": "Bar"
},
"Body": {
"shape": "BodyStructure"
}
}
},
"StringType": {
"type": "string"
},
"ListType": {
"type": "list",
"member": {
"shape": "StringType",
"jsonvalue": true
}
},
"BodyStructure": {
"type": "structure",
"members": {
"BodyField": {
"shape": "StringType",
"jsonvalue": true
},
"BodyListField": {
"shape": "ListType"
}
}
}
},
"cases": [
Expand All @@ -1335,12 +1364,16 @@
"name": "OperationName"
},
"params": {
"Attr": {"Foo":"Bar"}
"HeaderField": {"Foo":"Bar"},
"QueryField": {"Foo":"Bar"},
"Body": {
"BodyField": {"Foo":"Bar"}
}
},
"serialized": {
"uri": "/",
"uri": "/?Bar=%7B%22Foo%22%3A%22Bar%22%7D",
"headers": {"X-Amz-Foo": "eyJGb28iOiJCYXIifQ=="},
"body": ""
"body": "{\"BodyField\":\"{\\\"Foo\\\":\\\"Bar\\\"}\"}"
}
},
{
Expand All @@ -1353,6 +1386,27 @@
},
"name": "OperationName"
},
"params": {
"Body": {
"BodyListField": [{"Foo":"Bar"}]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is testing for empty output, may want to have nothing in the params.

}
},
"serialized": {
"uri": "/",
"headers": {},
"body": "{\"BodyListField\":[\"{\\\"Foo\\\":\\\"Bar\\\"}\"]}"
}
},
{
"given": {
"input": {
"shape": "InputShape"
},
"http": {
"method": "POST"
},
"name": "OperationName"
},
"params": {
},
"serialized": {
Expand Down
56 changes: 51 additions & 5 deletions models/protocol_tests/output/rest-json.json
Original file line number Diff line number Diff line change
Expand Up @@ -614,16 +614,30 @@
"OutputShape": {
"type": "structure",
"members": {
"Attr": {
"HeaderField": {
"shape": "StringType",
"jsonvalue": true,
"location": "header",
"locationName": "X-Amz-Foo"
"jsonvalue": true,
"location": "header",
"locationName": "X-Amz-Foo"
},
"BodyField":{
"shape": "StringType",
"jsonvalue": true
},
"BodyListField": {
"shape": "ListType"
}
}
},
"StringType": {
"type": "string"
},
"ListType": {
"type": "list",
"member": {
"shape": "StringType",
"jsonvalue": true
}
}
},
"cases": [
Expand All @@ -635,11 +649,43 @@
"name": "OperationName"
},
"result": {
"Attr": {"Foo":"Bar"}
"HeaderField": {"Foo":"Bar"},
"BodyField": {"Foo":"Bar"}
},
"response": {
"status_code": 200,
"headers": {"X-Amz-Foo": "eyJGb28iOiJCYXIifQ=="},
"body": "{\"BodyField\":\"{\\\"Foo\\\":\\\"Bar\\\"}\"}"
}
},
{
"given": {
"output": {
"shape": "OutputShape"
},
"name": "OperationName"
},
"result": {
"BodyListField": [{"Foo":"Bar"}]
},
"response": {
"status_code": 200,
"headers": {},
"body": "{\"BodyListField\":[\"{\\\"Foo\\\":\\\"Bar\\\"}\"]}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

}
},
{
"given": {
"output": {
"shape": "OutputShape"
},
"name": "OperationName"
},
"result": {
},
"response": {
"status_code": 200,
"headers": {},
"body": ""
}
}
Expand Down
14 changes: 14 additions & 0 deletions private/model/api/param_filler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package api

import (
"fmt"
"encoding/json"
"reflect"
"strings"

Expand Down Expand Up @@ -79,6 +80,19 @@ func (f paramFiller) paramsStructAny(value interface{}, shape *Shape) string {
if v.IsValid() {
return fmt.Sprintf("aws.Time(time.Unix(%d, 0))", int(v.Float()))
}
case "jsonvalue":
v, err := json.Marshal(value)
if err != nil {
panic("failed to marshal JSONValue, "+err.Error())
}
const tmpl = `func() aws.JSONValue {
var m aws.JSONValue
if err := json.Unmarshal([]byte(%q), &m); err != nil {
panic("failed to unmarshal JSONValue, "+err.Error())
}
return m
}()`
return fmt.Sprintf(tmpl, string(v))
default:
panic("Unhandled type " + shape.Type)
}
Expand Down
21 changes: 14 additions & 7 deletions private/protocol/json/jsonutil/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strconv"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/private/protocol"
)

Expand Down Expand Up @@ -49,7 +50,10 @@ func buildAny(value reflect.Value, buf *bytes.Buffer, tag reflect.StructTag) err
t = "list"
}
case reflect.Map:
t = "map"
// cannot be a JSONValue map
if _, ok := value.Interface().(aws.JSONValue); !ok {
t = "map"
}
}
}

Expand Down Expand Up @@ -210,14 +214,11 @@ func buildScalar(v reflect.Value, buf *bytes.Buffer, tag reflect.StructTag) erro
}
buf.Write(strconv.AppendFloat(scratch[:0], f, 'f', -1, 64))
default:
switch value.Type() {
case timeType:
converted := v.Interface().(*time.Time)

switch converted := value.Interface().(type) {
case time.Time:
buf.Write(strconv.AppendInt(scratch[:0], converted.UTC().Unix(), 10))
case byteSliceType:
case []byte:
if !value.IsNil() {
converted := value.Interface().([]byte)
buf.WriteByte('"')
if len(converted) < 1024 {
// for small buffers, using Encode directly is much faster.
Expand All @@ -233,6 +234,12 @@ func buildScalar(v reflect.Value, buf *bytes.Buffer, tag reflect.StructTag) erro
}
buf.WriteByte('"')
}
case aws.JSONValue:
str, err := protocol.EncodeJSONValue(converted, protocol.QuotedEscape)
if err != nil {
return fmt.Errorf("unable to encode JSONValue, %v", err)
}
buf.WriteString(str)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized we squash all these error. Is that something we want to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely want to return errors from these functions. If the protocol marshaler is squashing them then those marshalers should be updated not to squash the errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you mean errors on write, sorry missed that. Those should be captured... I don't think the SDK does that for any writes currently like this since the type is a bytes.buffer.

default:
return fmt.Errorf("unsupported JSON value %v (%s)", value.Interface(), value.Type())
}
Expand Down
15 changes: 14 additions & 1 deletion private/protocol/json/jsonutil/unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
"io/ioutil"
"reflect"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/private/protocol"
)

// UnmarshalJSON reads a stream and unmarshals the results in object v.
Expand Down Expand Up @@ -50,7 +53,10 @@ func unmarshalAny(value reflect.Value, data interface{}, tag reflect.StructTag)
t = "list"
}
case reflect.Map:
t = "map"
// cannot be a JSONValue map
if _, ok := value.Interface().(aws.JSONValue); !ok {
t = "map"
}
}
}

Expand Down Expand Up @@ -183,6 +189,13 @@ func unmarshalScalar(value reflect.Value, data interface{}, tag reflect.StructTa
return err
}
value.Set(reflect.ValueOf(b))
case aws.JSONValue:
// No need to use escaping as the value is a non-quoted string.
v, err := protocol.DecodeJSONValue(d, protocol.NoEscape)
if err != nil {
return err
}
value.Set(reflect.ValueOf(v))
default:
return errf()
}
Expand Down
76 changes: 76 additions & 0 deletions private/protocol/jsonvalue.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package protocol
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to add tests for these new functions.


import (
"encoding/base64"
"encoding/json"
"fmt"
"strconv"

"github.com/aws/aws-sdk-go/aws"
)

// EscapeMode is the mode that should be use for escaping a value
type EscapeMode uint

// The modes for escaping a value before it is marshaled, and unmarshaled.
const (
NoEscape EscapeMode = iota
Base64Escape
QuotedEscape
)

// EncodeJSONValue marshals the value into a JSON string, and optionally base64
// encodes the string before returning it.
//
// Will panic if the escape mode is unknown.
func EncodeJSONValue(v aws.JSONValue, escape EscapeMode) (string, error) {
b, err := json.Marshal(v)
if err != nil {
return "", err
}

switch escape {
case NoEscape:
return string(b), nil
case Base64Escape:
return base64.StdEncoding.EncodeToString(b), nil
case QuotedEscape:
return strconv.Quote(string(b)), nil
}

panic(fmt.Sprintf("EncodeJSONValue called with unknown EscapeMode, %v", escape))
}

// DecodeJSONValue will attempt to decode the string input as a JSONValue.
// Optionally decoding base64 the value first before JSON unmarshaling.
//
// Will panic if the escape mode is unknown.
func DecodeJSONValue(v string, escape EscapeMode) (aws.JSONValue, error) {
var b []byte
var err error

switch escape {
case NoEscape:
b = []byte(v)
case Base64Escape:
b, err = base64.StdEncoding.DecodeString(v)
case QuotedEscape:
var u string
u, err = strconv.Unquote(v)
b = []byte(u)
default:
panic(fmt.Sprintf("DecodeJSONValue called with unknown EscapeMode, %v", escape))
}

if err != nil {
return nil, err
}

m := aws.JSONValue{}
err = json.Unmarshal(b, &m)
if err != nil {
return nil, err
}

return m, nil
}
Loading