Skip to content

Commit

Permalink
Support decimal number type keys in maps (#140)
Browse files Browse the repository at this point in the history
* socket: only send client metadata once per socket (#105)

Periodic cluster synchronisation calls isMaster() which currently resends the
"client" metadata every call - the spec specifies:

	isMaster commands issued after the initial connection handshake MUST NOT
	contain handshake arguments

	https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.rst#connection-handshake

This hotfix prevents subsequent isMaster calls from sending the client metadata
again - fixes #101 and fixes #103.

Thanks to @changwoo-nam @qhenkart @canthefason @jyoon17 for spotting the initial
issue, opening tickets, and having the problem debugged with a PoC fix before I
even woke up.

* Merge Development (#111)

* Brings in a patch on having flusher not suppress errors. (#81)

go-mgo#360

* Fallback to JSON tags when BSON tag isn't present (#91)

* Fallback to JSON tags when BSON tag isn't present


Cleanup.

* Add test to demonstrate tagging fallback.

- Test coverage for tagging test.

* socket: only send client metadata once per socket

Periodic cluster synchronisation calls isMaster() which currently resends the
"client" metadata every call - the spec specifies:

	isMaster commands issued after the initial connection handshake MUST NOT
	contain handshake arguments

	https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.rst#connection-handshake

This hotfix prevents subsequent isMaster calls from sending the client metadata
again - fixes #101 and fixes #103.

Thanks to @changwoo-nam @qhenkart @canthefason @jyoon17 for spotting the initial
issue, opening tickets, and having the problem debugged with a PoC fix before I
even woke up.

* Cluster abended test 254 (#100)

* Add a test that mongo Server gets their abended reset as necessary.

See https://github.com/go-mgo/mgo/issues/254 and
https://github.com/go-mgo/mgo/pull/255/files

* Include the patch from Issue 255.

This brings in a test which fails without the patch, and passes with the
patch. Still to be tested, manual tcpkill of a socket.

* changeStream support (#97)

Add $changeStream support

* readme: credit @peterdeka and @steve-gray (#110)

* Hotfix #120  (#136)

* cluster: fix deadlock in cluster synchronisation (#120)

For a impressively thorough breakdown of the problem, see:
	#120 (comment)

Huge thanks to @dvic and @KJTsanaktsidis for the report and fix.

* readme: credit @dvic and @KJTsanaktsidis

* added support for marshalling/unmarshalling maps with non-string keys

* refactor method receiver
  • Loading branch information
aksentyev authored and domodwyer committed Apr 12, 2018
1 parent e6a7e81 commit 32b18f7
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 17 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ A [sub-package](https://godoc.org/github.com/globalsign/mgo/bson) that implement
* Gracefully recover from a temporarily unreachable server ([details](https://github.com/globalsign/mgo/pull/69))
* Use JSON tags when no explicit BSON are tags set ([details](https://github.com/globalsign/mgo/pull/91))
* Support [$changeStream](https://docs.mongodb.com/manual/changeStreams/) tailing on 3.6+ ([details](https://github.com/globalsign/mgo/pull/97))
* Fix deadlock in cluster synchronisation ([details](https://github.com/globalsign/mgo/issues/120))

---

Expand All @@ -55,11 +56,13 @@ A [sub-package](https://godoc.org/github.com/globalsign/mgo/bson) that implement
* @carter2000
* @cezarsa
* @drichelson
* @dvic
* @eaglerayp
* @feliixx
* @fmpwizard
* @idy
* @jameinel
* @KJTsanaktsidis
* @gazoon
* @mapete94
* @peterdeka
Expand Down
34 changes: 22 additions & 12 deletions bson/bson_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,8 @@ func (s *S) TestMarshalOneWayItems(c *C) {
// --------------------------------------------------------------------------
// One-way unmarshaling tests.

type intAlias int

var unmarshalItems = []testItemType{
// Field is private. Should not attempt to unmarshal it.
{&struct{ priv byte }{},
Expand Down Expand Up @@ -661,6 +663,14 @@ var unmarshalItems = []testItemType{
// Decode a doc within a doc in to a slice within a doc; shouldn't error
{&struct{ Foo []string }{},
"\x03\x66\x6f\x6f\x00\x05\x00\x00\x00\x00"},

// int key maps
{map[int]string{10: "s"},
"\x0210\x00\x02\x00\x00\x00s\x00"},

//// event if type is alias to int
{map[intAlias]string{10: "s"},
"\x0210\x00\x02\x00\x00\x00s\x00"},
}

func (s *S) TestUnmarshalOneWayItems(c *C) {
Expand Down Expand Up @@ -738,11 +748,6 @@ var unmarshalErrorItems = []unmarshalErrorType{
"\x10name\x00\x08\x00\x00\x00",
"Duplicated key 'name' in struct bson_test.structWithDupKeys"},

// Non-string map key.
{map[int]interface{}{},
"\x10name\x00\x08\x00\x00\x00",
"BSON map must have string keys. Got: map\\[int\\]interface \\{\\}"},

{nil,
"\xEEname\x00",
"Unknown element kind \\(0xEE\\)"},
Expand All @@ -758,6 +763,11 @@ var unmarshalErrorItems = []unmarshalErrorType{
{nil,
"\x08\x62\x00\x02",
"encoded boolean must be 1 or 0, found 2"},

// Non-string and not numeric map key.
{map[bool]interface{}{true: 1},
"\x10true\x00\x01\x00\x00\x00",
"BSON map must have string or decimal keys. Got: map\\[bool\\]interface \\{\\}"},
}

func (s *S) TestUnmarshalErrorItems(c *C) {
Expand Down Expand Up @@ -1161,8 +1171,8 @@ type inlineBadKeyMap struct {
M map[int]int `bson:",inline"`
}
type inlineUnexported struct {
M map[string]interface{} `bson:",inline"`
unexported `bson:",inline"`
M map[string]interface{} `bson:",inline"`
unexported `bson:",inline"`
}
type unexported struct {
A int
Expand Down Expand Up @@ -1219,11 +1229,11 @@ func (s ifaceSlice) GetBSON() (interface{}, error) {

type (
MyString string
MyBytes []byte
MyBool bool
MyD []bson.DocElem
MyRawD []bson.RawDocElem
MyM map[string]interface{}
MyBytes []byte
MyBool bool
MyD []bson.DocElem
MyRawD []bson.RawDocElem
MyM map[string]interface{}
)

var (
Expand Down
50 changes: 46 additions & 4 deletions bson/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,6 @@ func (d *decoder) readDocTo(out reflect.Value) {
switch outk {
case reflect.Map:
keyType = outt.Key()
if keyType.Kind() != reflect.String {
panic("BSON map must have string keys. Got: " + outt.String())
}
if keyType != typeString {
convertKey = true
}
Expand Down Expand Up @@ -240,7 +237,42 @@ func (d *decoder) readDocTo(out reflect.Value) {
if d.readElemTo(e, kind) {
k := reflect.ValueOf(name)
if convertKey {
k = k.Convert(keyType)
mapKeyType := out.Type().Key()
mapKeyKind := mapKeyType.Kind()

switch mapKeyKind {
case reflect.Int:
fallthrough
case reflect.Int8:
fallthrough
case reflect.Int16:
fallthrough
case reflect.Int32:
fallthrough
case reflect.Int64:
fallthrough
case reflect.Uint:
fallthrough
case reflect.Uint8:
fallthrough
case reflect.Uint16:
fallthrough
case reflect.Uint32:
fallthrough
case reflect.Uint64:
fallthrough
case reflect.Float32:
fallthrough
case reflect.Float64:
parsed := d.parseMapKeyAsFloat(k, mapKeyKind)
k = reflect.ValueOf(parsed)
case reflect.String:
mapKeyType = keyType
default:
panic("BSON map must have string or decimal keys. Got: " + outt.String())
}

k = k.Convert(mapKeyType)
}
out.SetMapIndex(k, e)
}
Expand Down Expand Up @@ -276,6 +308,16 @@ func (d *decoder) readDocTo(out reflect.Value) {
d.docType = docType
}

func (decoder) parseMapKeyAsFloat(k reflect.Value, mapKeyKind reflect.Kind) float64 {
parsed, err := strconv.ParseFloat(k.String(), 64)
if err != nil {
panic("Map key is defined to be a decimal type (" + mapKeyKind.String() + ") but got error " +
err.Error())
}

return parsed
}

func (d *decoder) readArrayDocTo(out reflect.Value) {
end := int(d.readInt32())
end += d.i - 4
Expand Down
2 changes: 1 addition & 1 deletion bson/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (e *encoder) addDoc(v reflect.Value) {

func (e *encoder) addMap(v reflect.Value) {
for _, k := range v.MapKeys() {
e.addElem(k.String(), v.MapIndex(k), false)
e.addElem(fmt.Sprint(k), v.MapIndex(k), false)
}
}

Expand Down

0 comments on commit 32b18f7

Please sign in to comment.