-
Notifications
You must be signed in to change notification settings - Fork 882
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.
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
GODRIVER-1808 Fix BSON unmarshaling into an interface containing a concrete value. #1584
base: master
Are you sure you want to change the base?
Changes from 1 commit
1db6753
82142f6
05bab35
8e5efd5
2e56642
76b535b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -347,11 +347,13 @@ var _ typeDecoder = decodeAdapter{} | |
// t and calls decoder.DecodeValue on it. | ||
func decodeTypeOrValue(decoder ValueDecoder, dc DecodeContext, vr bsonrw.ValueReader, t reflect.Type) (reflect.Value, error) { | ||
td, _ := decoder.(typeDecoder) | ||
return decodeTypeOrValueWithInfo(decoder, td, dc, vr, t, true) | ||
val := reflect.New(t).Elem() | ||
return decodeTypeOrValueWithInfo(decoder, td, dc, vr, val, true) | ||
} | ||
|
||
func decodeTypeOrValueWithInfo(vd ValueDecoder, td typeDecoder, dc DecodeContext, vr bsonrw.ValueReader, t reflect.Type, convert bool) (reflect.Value, error) { | ||
func decodeTypeOrValueWithInfo(vd ValueDecoder, td typeDecoder, dc DecodeContext, vr bsonrw.ValueReader, val reflect.Value, convert bool) (reflect.Value, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should change the |
||
if td != nil { | ||
t := val.Type() | ||
val, err := td.decodeType(dc, vr, t) | ||
if err == nil && convert && val.Type() != t { | ||
// This conversion step is necessary for slices and maps. If a user declares variables like: | ||
|
@@ -366,7 +368,6 @@ func decodeTypeOrValueWithInfo(vd ValueDecoder, td typeDecoder, dc DecodeContext | |
return val, err | ||
} | ||
|
||
val := reflect.New(t).Elem() | ||
err := vd.DecodeValue(dc, vr, val) | ||
return val, err | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -185,7 +185,7 @@ func (dvd DefaultValueDecoders) DDecodeValue(dc DecodeContext, vr bsonrw.ValueRe | |||||
} | ||||||
|
||||||
// Pass false for convert because we don't need to call reflect.Value.Convert for tEmpty. | ||||||
elem, err := decodeTypeOrValueWithInfo(decoder, tEmptyTypeDecoder, dc, elemVr, tEmpty, false) | ||||||
elem, err := decodeTypeOrValueWithInfo(decoder, tEmptyTypeDecoder, dc, elemVr, reflect.New(tEmpty).Elem(), false) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should pass the type here instead of allocating a new
Suggested change
|
||||||
if err != nil { | ||||||
return err | ||||||
} | ||||||
|
@@ -1666,11 +1666,15 @@ func (dvd DefaultValueDecoders) decodeDefault(dc DecodeContext, vr bsonrw.ValueR | |||||
|
||||||
eType := val.Type().Elem() | ||||||
|
||||||
decoder, err := dc.LookupDecoder(eType) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
var vDecoder ValueDecoder | ||||||
var tDecoder typeDecoder | ||||||
if !(eType.Kind() == reflect.Interface && val.Len() > 0) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Delay the decoder retrieval for interface slice/array so each element will be decoded based on its type. |
||||||
vDecoder, err = dc.LookupDecoder(eType) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
tDecoder, _ = vDecoder.(typeDecoder) | ||||||
} | ||||||
eTypeDecoder, _ := decoder.(typeDecoder) | ||||||
|
||||||
idx := 0 | ||||||
for { | ||||||
|
@@ -1682,10 +1686,34 @@ func (dvd DefaultValueDecoders) decodeDefault(dc DecodeContext, vr bsonrw.ValueR | |||||
return nil, err | ||||||
} | ||||||
|
||||||
elem, err := decodeTypeOrValueWithInfo(decoder, eTypeDecoder, dc, vr, eType, true) | ||||||
if err != nil { | ||||||
return nil, newDecodeError(strconv.Itoa(idx), err) | ||||||
var elem reflect.Value | ||||||
if vDecoder == nil { | ||||||
e := val.Index(idx).Elem() | ||||||
valueDecoder, err := dc.LookupDecoder(e.Type()) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
typeDecoder, _ := valueDecoder.(typeDecoder) | ||||||
if e.Kind() == reflect.Ptr { | ||||||
v := reflect.New(e.Type()).Elem() | ||||||
v.Set(e) | ||||||
val.Index(idx).Set(v) | ||||||
e = v | ||||||
} | ||||||
elem, err = decodeTypeOrValueWithInfo(valueDecoder, typeDecoder, dc, vr, e, true) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should call
Suggested change
|
||||||
if err != nil { | ||||||
return nil, newDecodeError(strconv.Itoa(idx), err) | ||||||
} | ||||||
if e.Kind() == reflect.Ptr && e.IsZero() { | ||||||
elem = reflect.Zero(val.Index(idx).Type()) | ||||||
} | ||||||
} else { | ||||||
elem, err = decodeTypeOrValueWithInfo(vDecoder, tDecoder, dc, vr, reflect.New(eType).Elem(), true) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should pass the type here instead of allocating a new
Suggested change
|
||||||
if err != nil { | ||||||
return nil, newDecodeError(strconv.Itoa(idx), err) | ||||||
} | ||||||
} | ||||||
|
||||||
elems = append(elems, elem) | ||||||
idx++ | ||||||
} | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -213,7 +213,7 @@ func (mc *MapCodec) DecodeValue(dc DecodeContext, vr bsonrw.ValueReader, val ref | |||||
return err | ||||||
} | ||||||
|
||||||
elem, err := decodeTypeOrValueWithInfo(decoder, eTypeDecoder, dc, vr, eType, true) | ||||||
elem, err := decodeTypeOrValueWithInfo(decoder, eTypeDecoder, dc, vr, reflect.New(eType).Elem(), true) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should pass the type here instead of allocating a new
Suggested change
|
||||||
if err != nil { | ||||||
return newDecodeError(key, err) | ||||||
} | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add a test or a case to an existing test that asserts that this behavior works when calling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should pass the type here instead of allocating a new
reflect.Value
.