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.

Sign up for GitHub

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

Jump to bottom

[issues-391] Fix document typecheck in encoding for nested types #394

Merged
merged 3 commits into from Oct 21, 2022
Merged

[issues-391] Fix document typecheck in encoding for nested types #394

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f2d9132
[issues-391] Fix document typecheck in encoding for nested types
isaiahvita Oct 21, 2022
5934cd5
check if value can interface, similar to decode
isaiahvita Oct 21, 2022
2e7b04d
added changelog file
isaiahvita Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 8 additions & 5 deletions document/json/encoder.go
View file
Open in desktop
Expand Up @@ -20,10 +20,6 @@ type Encoder struct {

// Encode returns the JSON encoding of v.
func (e *Encoder) Encode(v interface{}) ([]byte, error) {
if document.IsNoSerde(v) {
return nil, fmt.Errorf("unsupported type: %v", v)
}

Comment on lines -23 to -26
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if this check should stay? Otherwise I think lines 71-78 in the encode function could still serialize the zero value of a generated Smithy type as an empty object which would be weird?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in my previous implementation, the document.IsNoSerde func was still being called, i had just wrapped it in another function isUnsupported. however, in my latest commit, I replaced that to more closely mimic decode which actually directly calls document.IsNoSerde when decoding a struct

encoder := smithyjson.NewEncoder()

if err := e.encode(jsonValueProvider(encoder.Value), reflect.ValueOf(v), serde.Tag{}); err != nil {
Expand Down Expand Up @@ -137,6 +133,13 @@ func (e *Encoder) encodeZeroValue(vp valueProvider, rv reflect.Value) error {
}

func (e *Encoder) encodeStruct(vp valueProvider, rv reflect.Value) error {
if rv.CanInterface() && document.IsNoSerde(rv.Interface()) {
return &document.UnmarshalTypeError{
Value: fmt.Sprintf("unsupported type"),
Type: rv.Type(),
}
}

switch {
case rv.Type().ConvertibleTo(serde.ReflectTypeOf.Time):
return &document.InvalidMarshalError{
Expand Down Expand Up @@ -323,4 +326,4 @@ func isValidJSONNumber(s string) bool {

// Make sure we are at the end.
return s == ""
}
}
11 changes: 11 additions & 0 deletions document/json/encoder_test.go
View file
Open in desktop
@@ -1,6 +1,7 @@
package json_test

import (
"github.com/aws/smithy-go/document"
"github.com/aws/smithy-go/document/internal/serde"
"github.com/aws/smithy-go/document/json"
"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -41,10 +42,20 @@ func TestEncoder_Encode(t *testing.T) {

func TestNewEncoderUnsupportedTypes(t *testing.T) {
type customTime time.Time
type noSerde = document.NoSerde
type NestedThing struct {
SomeThing string
noSerde
}
type Thing struct {
OtherThing string
NestedThing NestedThing
}

cases := []interface{}{
time.Now().UTC(),
customTime(time.Now().UTC()),
Thing{OtherThing: "foo", NestedThing: NestedThing{SomeThing: "bar"}},
}

encoder := json.NewEncoder()
Expand Down