[CSHARP-1999] Support for readonly properties/fields that contain instances Created: 13/Jun/17 Updated: 08/Feb/23 Resolved: 28/Oct/20 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | Serialization |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Daniel Hegener | Assignee: | James Kovacs |
| Resolution: | Won't Fix | Votes: | 4 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Case: | (copied to CRM) | ||||||||
| Description |
|
We are dealing with lots of entities that create instances for contained properties/fields as part of their instantiation like this:
or more generic
The C# driver is able to serialize these properties nicely once we map them explicitly. However, they are considered readonly so deserialization gets skipped and our entities end up with not populated instances. In order to solve this, we would suggest to enhance the driver in a way that - similar to SetIgnoreIfNull - one can select to skip the instantiation of a type and populate an existing instance on the target object. Let me know your thoughts. I'd be happy to implement that right away if you agree. |
| Comments |
| Comment by Daniel Hegener [ 08/Dec/20 ] | ||||||||||||||||||||||||||||||||||||||
|
Thanks for eventually looking into my suggestion. I do see the gentle increase in complexity, too, of course, so I can accept your negative decision based on this. I would like to point out three things, though:
| ||||||||||||||||||||||||||||||||||||||
| Comment by James Kovacs [ 27/Oct/20 ] | ||||||||||||||||||||||||||||||||||||||
|
We apologize for letting this PR sit for 3 years without public comment. While we appreciate the effort that it took to implement the feature, it makes significant changes to how serialization works internally. At a high level, the Deserialize method is responsible for creating and populating the object itself and only itself. If there are child documents or arrays, responsibility for instantiating and deserializing them is left to the child's deserializer. This is why readonly properties and fields are ignored. Because the owning object has no way of setting these properties/fields. The provided PR changes deserialization to optionally allow a parent object to provide a child instance for the child deserializer to populate. This changes the semantics of deserialization from strictly bottom-up building to a combination of bottom-up and top-down. This increases the complexity of the deserialization code and may result in new subtle bugs. We do not feel that the increase in complexity is warranted as we can already support readonly properties and fields using a private [BsonConstuctor] as follows. (Note the constructor doesn't have to be private, but can be if you don't want it callable from your code.)
As you can see, we declare a private constructor marked with [BsonConstuctor], which takes the deserialized child instances and can either set the readonly properties/fields or use the instances to populate the existing instances. Note that Foo and Bar must be explicitly attributed with [BsonElement] as readonly fields/properties are not automatically added to class maps. If you do not want to modify your POCOs with a private [BsonConstructor] and [BsonElement] attributes, the same effect can be achieved using a BsonClassMap configured using MapProperty and MapCreator:
| ||||||||||||||||||||||||||||||||||||||
| Comment by Daniel Hegener [ 24/Nov/17 ] | ||||||||||||||||||||||||||||||||||||||