[CSHARP-2215] Update.Set can generate invalid $set when type of value does not match type of POCO Created: 14/Mar/18 Updated: 28/Oct/23 Resolved: 14/Jan/19 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | API |
| Affects Version/s: | 2.5 |
| Fix Version/s: | 2.8.0 |
| Type: | Bug | Priority: | Critical - P2 |
| Reporter: | Jason Ford | Assignee: | Robert Stam |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||
| Case: | (copied to CRM) | ||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||
| Description |
|
When the value provided to Update.Set does not match the type of the underlying property we attempt to convert the value to an instance of the type of the underlying property. This process sometimes results in an invalid $set update operator. Here's one repro, where the underlying property is of type List<string> and the value provided is of type BsonArray:
The resulting update specification is:
which is incorrect (note that the value of L is a string, not an array of strings). In the original description below this invalid update then subsequently resulted in a deserialization error. Original description below: I am encountering an odd deserialization error while using the newest version of the c# driver. Updates to a List<string> with a BsonArray seem to causes the write to be performed incorrectly. I wrote a sample program to demonstrate this. I should have a field with an array of strings. Instead of ending up with a document that looks like this: { "_id" : "4a82b074-c9d2-4507-b762-4b32ebbf88be", "Strings" : [ "seven", "eight", "nine" ] }I get one that looks like this: { "_id" : "4a82b074-c9d2-4507-b762-4b32ebbf88be", "Strings" : "[seven, eight, nine]" }Here's my sample application that will reproduce the error:
|
| Comments |
| Comment by Githook User [ 14/Jan/19 ] | |||||||||||||
|
Author: {'username': 'rstam', 'email': 'robert@robertstam.org', 'name': 'rstam'}Message: | |||||||||||||
| Comment by Githook User [ 14/Jan/19 ] | |||||||||||||
|
Author: {'username': 'rstam', 'email': 'robert@robertstam.org', 'name': 'rstam'}Message: | |||||||||||||
| Comment by Robert Stam [ 14/Jan/19 ] | |||||||||||||
|
The fix implemented for this ticket is somewhat narrowly targeted, namely to only serialize a scalar value using an array field's item serializer when we know that is approriate. | |||||||||||||
| Comment by Robert Stam [ 03/Apr/18 ] | |||||||||||||
|
To illustrate that this is not an issue specific to BsonArray, replace:
with:
which results in the following update operator:
Converting an arbitrary value to string results in the ToString method being called, and the default implementation of ToString is to return the type name. | |||||||||||||
| Comment by Robert Stam [ 03/Apr/18 ] | |||||||||||||
|
Diagnosis: the example in the description is resulting in the invalid $set update operation due to a combination of two features: 1. When searching for a serializer for TValue, if the underlying field serializer is for IEnumerable<TField> we try to use the itemSerializer for TField if possible In the example in the description, TValue is BsonArray, and TField is string. And it turns out that BsonArray can be converted to string (hence the string value for $set). In fact, ANY type can be converted to string, so this issue is not limited to BsonArray. The reasons why we do 1. and 2. above are useful in general: 1. Is in order to support queries against array fields with scalar values The problem this issue is exposing is that while useful in general it turns out that sometimes they break down: 1. Treating arrays and scalars this way is useful for queries but not for updates The lines of code where we extract the itemSerializer from an array-like field serializer are: For the example in the description fieldSerializer is of type IBsonSerializer<List<string>> and itemSerializer is of type IBsonSerializer<string>. And the lines of code where we convert from TValue to TField are: For the example in the description fromType is BsonArray and toType is string. | |||||||||||||
| Comment by Robert Stam [ 03/Apr/18 ] | |||||||||||||
|
The mismatch between the type of the value provided and the type of the underlying property can be caught at compile time by using a more type safe way of specifying the field to update:
| |||||||||||||
| Comment by Robert Stam [ 03/Apr/18 ] | |||||||||||||
|
Using Builders<BsonDocument>.Update instead of Builders<TestEntity> is a good workaround. It does require that the call to database.GetCollection<TestEntity> also be changed to database.GetCollection<BsonDocument>. | |||||||||||||
| Comment by James Kovacs [ 02/Apr/18 ] | |||||||||||||
|
Upon further investigation, the root cause of the problem is a type mismatch between Update.Set and the POCO class. Using the 2.5.0 driver, we can resolve the issue in either of the following ways... 1. Perform the Update.Set with the correct type, List<string>:
2. Perform the Update.Set using BsonDocument rather than TestEntity:
Although the sample code can be corrected to work properly, we should either produce a correct update operation or throw an exception rather than producing an update operation that cannot be re-read. | |||||||||||||
| Comment by James Kovacs [ 02/Apr/18 ] | |||||||||||||
|
Note that this is a regression as updates with BsonArrays worked in MongoDB .NET Driver 2.4.0 and 2.4.2. The key piece of code is:
2.4.0 / 2.4.2: Correct update document sent to `mongod`:
2.4.1: Update fails
2.4.3 / 2.4.4: Update fails
2.5.0 Deserialization error when we attempt to re-read the document because BsonArray $set generated a bad update document:
Note that rather than a BsonArray of strings, driver 2.5.0 sends a string containing an array. |