Skip to content

Commit d85d290

Browse files
authored
sort map keys in JSON Document shape encodes (#599)
1 parent 16d913d commit d85d290

File tree

4 files changed

+80
-6
lines changed

4 files changed

+80
-6
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"id": "ed10934b-f38a-4ea9-a93d-f3498ffdfd8a",
3+
"type": "feature",
4+
"description": "Sort map keys in JSON Document types.",
5+
"modules": [
6+
"."
7+
]
8+
}

document/json/encoder.go

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"fmt"
55
"math/big"
66
"reflect"
7+
"slices"
8+
"strings"
79

810
"github.com/aws/smithy-go/document"
911
"github.com/aws/smithy-go/document/internal/serde"
@@ -181,16 +183,25 @@ func (e *Encoder) encodeMap(vp valueProvider, rv reflect.Value) error {
181183
object := vp.GetValue().Object()
182184
defer object.Close()
183185

184-
for _, key := range rv.MapKeys() {
185-
keyName := fmt.Sprint(key.Interface())
186-
if keyName == "" {
186+
rawKeys := rv.MapKeys()
187+
keys := make([]*mapKey, 0, len(rawKeys))
188+
for _, raw := range rawKeys {
189+
keys = append(keys, &mapKey{
190+
Value: raw,
191+
Underlying: fmt.Sprint(raw.Interface()),
192+
})
193+
}
194+
195+
slices.SortFunc(keys, sortMapKeys)
196+
for _, key := range keys {
197+
if key.Underlying == "" {
187198
return &document.InvalidMarshalError{Message: "map key cannot be empty"}
188199
}
189200

190-
ev := rv.MapIndex(key)
201+
ev := rv.MapIndex(key.Value)
191202
err := e.encode(jsonObjectKeyProvider{
192203
Object: object,
193-
Key: keyName,
204+
Key: key.Underlying,
194205
}, ev, serde.Tag{})
195206
if err != nil {
196207
return err
@@ -326,4 +337,15 @@ func isValidJSONNumber(s string) bool {
326337

327338
// Make sure we are at the end.
328339
return s == ""
329-
}
340+
}
341+
342+
// cache struct to cache a map key's reflect.Value with its underlying value,
343+
// since repeated calls to reflect.Value.Interface() may be expensive
344+
type mapKey struct {
345+
Value reflect.Value
346+
Underlying string
347+
}
348+
349+
func sortMapKeys(i, j *mapKey) int {
350+
return strings.Compare(i.Underlying, j.Underlying)
351+
}

document/json/encoder_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,32 @@ func TestNewEncoderUnsupportedTypes(t *testing.T) {
6767
}
6868
}
6969

70+
func TestDeterministicMapOrder(t *testing.T) {
71+
value := struct {
72+
Map map[string]string
73+
}{
74+
Map: map[string]string{
75+
"foo": "bar",
76+
"a": "b",
77+
"bar": "baz",
78+
"b": "c",
79+
"baz": "qux",
80+
"c": "d",
81+
},
82+
}
83+
expect := `{"Map":{"a":"b","b":"c","bar":"baz","baz":"qux","c":"d","foo":"bar"}}`
84+
85+
encoder := json.NewEncoder()
86+
actual, err := encoder.Encode(value)
87+
if err != nil {
88+
t.Fatal(err)
89+
}
90+
91+
if expect != string(actual) {
92+
t.Errorf("encode determinstic order:\n%q !=\n%q", expect, actual)
93+
}
94+
}
95+
7096
func testEncode(t *testing.T, tt testCase) {
7197
t.Helper()
7298

document/json/shared_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ type StructA struct {
2727
FieldNestedStruct *StructA `document:"field_nested_struct"`
2828
FieldNestedStructOmitEmpty *StructA `document:"field_nested_struct_omit_empty,omitempty"`
2929

30+
FieldMap map[string]string `document:",omitempty"`
31+
3032
fieldUnexported string
3133

3234
StructB
@@ -88,6 +90,14 @@ var sharedObjectTests = map[string]testCase{
8890
},
8991
"filled json structure": {
9092
json: []byte(`{
93+
"FieldMap": {
94+
"a": "b",
95+
"bar": "baz",
96+
"baz": "qux",
97+
"c": "d",
98+
"foo": "bar",
99+
"z": "a"
100+
},
91101
"FieldName": "a",
92102
"FieldPtrName": "b",
93103
"field_rename": "c",
@@ -115,6 +125,14 @@ var sharedObjectTests = map[string]testCase{
115125
return &v
116126
}(),
117127
want: &StructA{
128+
FieldMap: map[string]string{
129+
"foo": "bar",
130+
"bar": "baz",
131+
"baz": "qux",
132+
"a": "b",
133+
"c": "d",
134+
"z": "a",
135+
},
118136
FieldName: "a",
119137
FieldPtrName: ptr.String("b"),
120138
FieldRename: "c",

0 commit comments

Comments
 (0)