From c9feaeb9b792d0283fb85174df4eef30f15a4a3e Mon Sep 17 00:00:00 2001 From: Benjamin Rewis <32186188+benjirewis@users.noreply.github.com> Date: Fri, 16 Apr 2021 11:49:23 -0400 Subject: [PATCH] GODRIVER-1930 bsoncore Array functions should return Array (#626) --- x/bsonx/bsoncore/array_test.go | 2 +- x/bsonx/bsoncore/bsoncore.go | 2 +- x/bsonx/bsoncore/bsoncore_test.go | 6 ++--- x/bsonx/bsoncore/document.go | 3 ++- x/bsonx/bsoncore/value.go | 43 +++--------------------------- x/bsonx/bsoncore/value_test.go | 8 +++--- x/mongo/driver/errors.go | 12 ++++----- x/mongo/driver/operation/count.go | 4 +-- x/mongo/driver/operation_legacy.go | 2 +- 9 files changed, 24 insertions(+), 58 deletions(-) diff --git a/x/bsonx/bsoncore/array_test.go b/x/bsonx/bsoncore/array_test.go index e6f6493d99..8249d82808 100644 --- a/x/bsonx/bsoncore/array_test.go +++ b/x/bsonx/bsoncore/array_test.go @@ -299,7 +299,7 @@ func TestArray(t *testing.T) { '\x00', '\x00', }, `[[null,null]]`, - `Array(19)[[null ,null ]]`, + `Array(19)[Array(11)[null,null]]`, }, { "malformed--length too small", diff --git a/x/bsonx/bsoncore/bsoncore.go b/x/bsonx/bsoncore/bsoncore.go index 97ef1b85d5..b8db838a5f 100644 --- a/x/bsonx/bsoncore/bsoncore.go +++ b/x/bsonx/bsoncore/bsoncore.go @@ -308,7 +308,7 @@ func BuildArrayElement(dst []byte, key string, values ...Value) []byte { // ReadArray will read an array from src. If there are not enough bytes it // will return false. -func ReadArray(src []byte) (arr Document, rem []byte, ok bool) { return readLengthBytes(src) } +func ReadArray(src []byte) (arr Array, rem []byte, ok bool) { return readLengthBytes(src) } // AppendBinary will append subtype and b to dst and return the extended buffer. func AppendBinary(dst []byte, subtype byte, b []byte) []byte { diff --git a/x/bsonx/bsoncore/bsoncore_test.go b/x/bsonx/bsoncore/bsoncore_test.go index 84b889955d..f4a1e781f0 100644 --- a/x/bsonx/bsoncore/bsoncore_test.go +++ b/x/bsonx/bsoncore/bsoncore_test.go @@ -548,19 +548,19 @@ func TestRead(t *testing.T) { "ReadArray/not enough bytes (length)", ReadArray, []byte{}, - []interface{}{Document(nil), []byte{}, false}, + []interface{}{Array(nil), []byte{}, false}, }, { "ReadArray/not enough bytes (value)", ReadArray, []byte{0x0F, 0x00, 0x00, 0x00}, - []interface{}{Document(nil), []byte{0x0F, 0x00, 0x00, 0x00}, false}, + []interface{}{Array(nil), []byte{0x0F, 0x00, 0x00, 0x00}, false}, }, { "ReadArray/success", ReadArray, []byte{0x08, 0x00, 0x00, 0x00, 0x0A, '0', 0x00, 0x00}, - []interface{}{Document{0x08, 0x00, 0x00, 0x00, 0x0A, '0', 0x00, 0x00}, []byte{}, true}, + []interface{}{Array{0x08, 0x00, 0x00, 0x00, 0x0A, '0', 0x00, 0x00}, []byte{}, true}, }, { "ReadBinary/not enough bytes (length)", diff --git a/x/bsonx/bsoncore/document.go b/x/bsonx/bsoncore/document.go index b77c593ede..b687b5a806 100644 --- a/x/bsonx/bsoncore/document.go +++ b/x/bsonx/bsoncore/document.go @@ -200,7 +200,8 @@ func (d Document) LookupErr(key ...string) (Value, error) { } return val, nil case bsontype.Array: - val, err := elem.Value().Array().LookupErr(key[1:]...) + // Convert to Document to continue Lookup recursion. + val, err := Document(elem.Value().Array()).LookupErr(key[1:]...) if err != nil { return Value{}, err } diff --git a/x/bsonx/bsoncore/value.go b/x/bsonx/bsoncore/value.go index ca29815495..ed95233d96 100644 --- a/x/bsonx/bsoncore/value.go +++ b/x/bsonx/bsoncore/value.go @@ -250,7 +250,7 @@ func (v Value) String() string { if !ok { return "" } - return docAsArray(arr, false) + return arr.String() case bsontype.Binary: subtype, data, ok := v.BinaryOK() if !ok { @@ -366,7 +366,7 @@ func (v Value) DebugString() string { if !ok { return "" } - return docAsArray(arr, true) + return arr.DebugString() case bsontype.CodeWithScope: code, scope, ok := v.CodeWithScopeOK() if !ok { @@ -464,7 +464,7 @@ func (v Value) DocumentOK() (Document, bool) { // Array returns the BSON array the Value represents as an Array. It panics if the // value is a BSON type other than array. -func (v Value) Array() Document { +func (v Value) Array() Array { if v.Type != bsontype.Array { panic(ElementTypeError{"bsoncore.Value.Array", v.Type}) } @@ -477,7 +477,7 @@ func (v Value) Array() Document { // ArrayOK is the same as Array, except it returns a boolean instead // of panicking. -func (v Value) ArrayOK() (Document, bool) { +func (v Value) ArrayOK() (Array, bool) { if v.Type != bsontype.Array { return nil, false } @@ -978,38 +978,3 @@ func sortStringAlphebeticAscending(s string) string { sort.Sort(ss) return string([]rune(ss)) } - -func docAsArray(d Document, debug bool) string { - if len(d) < 5 { - return "" - } - var buf bytes.Buffer - buf.WriteByte('[') - - length, rem, _ := ReadLength(d) // We know we have enough bytes to read the length - - length -= 4 - - var elem Element - var ok bool - first := true - for length > 1 { - if !first { - buf.WriteByte(',') - } - elem, rem, ok = ReadElement(rem) - length -= int32(len(elem)) - if !ok { - return "" - } - if debug { - fmt.Fprintf(&buf, "%s ", elem.Value().DebugString()) - } else { - fmt.Fprintf(&buf, "%s", elem.Value()) - } - first = false - } - buf.WriteByte(']') - - return buf.String() -} diff --git a/x/bsonx/bsoncore/value_test.go b/x/bsonx/bsoncore/value_test.go index d7016b83e3..9d35929dea 100644 --- a/x/bsonx/bsoncore/value_test.go +++ b/x/bsonx/bsoncore/value_test.go @@ -182,22 +182,22 @@ func TestValue(t *testing.T) { { "Array/Success", Value.Array, Value{Type: bsontype.Array, Data: []byte{0x05, 0x00, 0x00, 0x00, 0x00}}, nil, - []interface{}{Document{0x05, 0x00, 0x00, 0x00, 0x00}}, + []interface{}{Array{0x05, 0x00, 0x00, 0x00, 0x00}}, }, { "ArrayOK/Not Array", Value.ArrayOK, Value{Type: bsontype.String}, nil, - []interface{}{Document(nil), false}, + []interface{}{Array(nil), false}, }, { "ArrayOK/Insufficient Bytes", Value.ArrayOK, Value{Type: bsontype.Array, Data: []byte{0x01, 0x02, 0x03, 0x04}}, nil, - []interface{}{Document(nil), false}, + []interface{}{Array(nil), false}, }, { "ArrayOK/Success", Value.ArrayOK, Value{Type: bsontype.Array, Data: []byte{0x05, 0x00, 0x00, 0x00, 0x00}}, nil, - []interface{}{Document{0x05, 0x00, 0x00, 0x00, 0x00}, true}, + []interface{}{Array{0x05, 0x00, 0x00, 0x00, 0x00}, true}, }, { "Binary/Not Binary", Value.Binary, Value{Type: bsontype.String}, diff --git a/x/mongo/driver/errors.go b/x/mongo/driver/errors.go index 4b92221ea4..a70745f0c1 100644 --- a/x/mongo/driver/errors.go +++ b/x/mongo/driver/errors.go @@ -376,12 +376,12 @@ func ExtractErrorFromServerResponse(doc bsoncore.Document) error { } case "errorLabels": if arr, okay := elem.Value().ArrayOK(); okay { - elems, err := arr.Elements() + vals, err := arr.Values() if err != nil { continue } - for _, elem := range elems { - if str, ok := elem.Value().StringValueOK(); ok { + for _, val := range vals { + if str, ok := val.StringValueOK(); ok { labels = append(labels, str) } } @@ -433,12 +433,12 @@ func ExtractErrorFromServerResponse(doc bsoncore.Document) error { copy(wcError.WriteConcernError.Details, info) } if errLabels, exists := doc.Lookup("errorLabels").ArrayOK(); exists { - elems, err := errLabels.Elements() + vals, err := errLabels.Values() if err != nil { continue } - for _, elem := range elems { - if str, ok := elem.Value().StringValueOK(); ok { + for _, val := range vals { + if str, ok := val.StringValueOK(); ok { labels = append(labels, str) } } diff --git a/x/mongo/driver/operation/count.go b/x/mongo/driver/operation/count.go index deded6dcfa..f38d6aa3dc 100644 --- a/x/mongo/driver/operation/count.go +++ b/x/mongo/driver/operation/count.go @@ -70,8 +70,8 @@ elementLoop: } // get count value from first batch - element = firstBatch.Array().Index(0) - count, err := element.Value().Document().LookupErr("n") + val := firstBatch.Array().Index(0) + count, err := val.Document().LookupErr("n") if err != nil { break elementLoop } diff --git a/x/mongo/driver/operation_legacy.go b/x/mongo/driver/operation_legacy.go index ded541bc90..4278c2ee5d 100644 --- a/x/mongo/driver/operation_legacy.go +++ b/x/mongo/driver/operation_legacy.go @@ -353,7 +353,7 @@ func (op Operation) createLegacyKillCursorsWiremessage(dst []byte, desc descript } var collName string - var cursors bsoncore.Document + var cursors bsoncore.Array for _, elem := range cmdElems { switch elem.Key() { case "killCursors":