GODRIVER-2017 Skip empty extJSON objects correctly in Unmarshal

[benjirewis]

GODRIVER-2017

Adds a case for empty objects in extJSONValueReader#skipObject(). Adds tests for empty objects in undefined fields for UnmarshalExtJSON.

Empty objects in extended JSON were being skipped without setting ejvr.p.emptyObject back to false. So, a regular object following an empty object, as in

{
   "skip": {},
   "a": {"b", "c"}
}

was being unmarshaled to an empty object even though it wasn't empty (i.e. a unmarshaled to bson.D{}).

[iwysiu]

Do we want to return or continue? what happens if we need to skip something with a nested empty document, something like
"UndefinedField": {"empty": {}, "key": 1}

[benjirewis]

continue is more correct, but it actually doesn't change the parsing behavior 💭

In that example, the parser will realize UndefinedField is undefined in the destination struct and will call skipObject on the value of UndefinedField: {"empty": {}, "key": 1}. Since that value is not an empty object itself, the emptyObject flag will not have been set, so the logic in this block won't actually apply. That said, continueing is more accurate than returning in my opinion, since the depth variable will be set back to 0 and return anyway.

Also updated the existing test for "empty embedded document" to be the case you described above.

[vfedosieiev]

hi guys, any update when this will be merged?

[matthewdale]

Looks good, with a suggestion to extend the comment to describe a little more about the expected state(s) of the parser.

[matthewdale]

Optional: Add a description of the expected state of the parser/buffer to help people reading the code understand why we need different behavior for when ejvr.p.emptyObject is true.

E.g.

When emptyObject is true, the parser has already read both the opening and closing brackets of an empty object ("{}"), so the next valid token will be part of the parent document, not part of the nested document.

[benjirewis]

Good idea! Added a description of the state of the parser (the exact text you gave).

[A40in]

Hi!
the function does not handle the state jpsInvalidState.

func (ejvr *extJSONValueReader) skipObject() {
	// read entire object until depth returns to 0 (last ending } or ] seen)
	depth := 1
	for depth > 0 {
		ejvr.p.advanceState()    // loop if returned jpsInvalidState

Next test loops in skipObject() function

var WrongJson string = `{
"undefined_field":{
	"u": [
		"q":{
		}
	]
	}
}`

type V struct {
	Cmd bson.D
}

func TestBson1(t *testing.T) {
	var req V
	err := bson.UnmarshalExtJSON([]byte(WrongJson), false, &req)
	t.Log(req, err)
}