[CSHARP-1894] InvalidCastException in FieldValueSerializerHelper on implicit type casting Created: 15/Jan/17 Updated: 08/Mar/17 Resolved: 13/Feb/17 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | Linq, Serialization |
| Affects Version/s: | 2.4.1 |
| Fix Version/s: | 2.4.3 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Vyacheslav Stroy | Assignee: | Robert Stam |
| Resolution: | Done | Votes: | 1 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Windows 10, MongoDB 3.4.0 |
||
| Issue Links: |
|
||||||||
| Description |
|
Recent driver release (v.2.4.1) has broken my project completely. It seems that the problem is related to the newly added serialization helper (FieldValueSerializerHelper). After updating from version 2.3.0 I got InvalidCastException.
We are using active-record approach and lazy-load reference wrappers that can be casted to Id using implicit operator conversion.
FieldValueSerializerHelper.CastingSerializer is trying to convert value using boxing conversion. It will always fail if destination type does not match source type (the only exclusion is the situation when source type inherits from destination type).
Boxing conversion omits user-defined type conversion therefor custom TypeConverter nor implicit operator conversions won't work. Please add support for user-defined implicit type casting to the predicate translator. I've prepared pull request that implements the requested feature. It might help with
|
| Comments |
| Comment by Githook User [ 13/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Author: {u'username': u'rstam', u'name': u'rstam', u'email': u'robert@robertstam.org'}Message: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Githook User [ 13/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Author: {u'username': u'rstam', u'name': u'rstam', u'email': u'robert@robertstam.org'}Message: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Githook User [ 13/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Author: {u'username': u'kreig', u'name': u'kreig', u'email': u'mrkreig@gmail.com'}Message: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Vyacheslav Stroy [ 08/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I have updated my code. Please review that pull request again. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Vyacheslav Stroy [ 07/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Ok, I will update my pull request, no problem at all.
Actually, TypeConverter approach is widely used in various low-level libraries. For example, I know for sure that TypeConverter technique is used by AutoMapper for unbound type casting. One of the main advantages over Convert.ChangeType() is that you can define custom type conversion for third-party types.
In some cases it's even cheaper then Convert.ChangeType() call because specific type converter are usually designed to convert to/from particular types while Convert.ChangeType() has to perform multiple type checks. Convert performs better on primitive types (int, bool, long etc) because no boxing conversion applied for such calls as ToInt32(). Furthermore, Convert.ChangeType() will always throw exception on improper conversion attempt while TypeConverter provides CanConvertTo() and CanConvertFrom() methods that allow to check suitable types before conversion. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 07/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
TypeConverters are primarily used in design-time environments to convert to and from strings. I wonder how relevant they are here in the .NET driver to convert to and from arbitrary types. Convert.ChangeType builds on top of IConvertible, which may or may not be more generally applicable. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 07/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Do you want to submit a new pull request? Your work looks good, but the existing pull request won't easily rebase on master. I don't want to make extra work for you. If you prefer I can take your ideas and manually incorporate them into the current 2.4.2 code base. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 07/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Got it. I can reproduce this easily now also. Thanks. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Vyacheslav Stroy [ 07/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
"Foo and Bar" were given just as example in the original issue description. The simplest way to reproduce:
It's the first case, described actually in Implicit_Type_Casting_Primitive_Types test. The user-specified conversion is described in Implicit_Type_Casting_With_Custom_TypeConverter test. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 07/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I had looked at the test case in the pull request and saw no mention of Foo and Bar. Which classes in the new test case correspond to Foo and Bar? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Vyacheslav Stroy [ 07/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yes, I wrote a test case. You can view it alongside with proposed solution in my pull-request.
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 07/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Can you please provide the class definitions for Foo and Bar so that I may reproduce this locally? Thanks. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Vyacheslav Stroy [ 07/Feb/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
After updating to recent 2.4.2 release:
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Juliana Gradova [ 24/Jan/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I have the same issue with LINQ. Please add the custom serialization support as was mentioned by @VyacheslavStroy or simply turn off casting. CastingSerializer can be omitted to allow "as is" value serialization. In this case we can even specify custom serializer for specific type to control its db representation. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Vyacheslav Stroy [ 15/Jan/17 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||