[CSHARP-635] Postpone creation of serializers until they are first needed Created: 26/Nov/12 Updated: 20/Mar/14 Resolved: 01/Jan/13 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | None |
| Affects Version/s: | 1.7 |
| Fix Version/s: | 1.8 |
| Type: | Bug | Priority: | Minor - P4 |
| Reporter: | Wesley McClure | Assignee: | Robert Stam |
| Resolution: | Done | Votes: | 0 |
| Labels: | c#, driver | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Windows 8 |
||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Major Change | ||||||||
| Description |
|
In March 2012, https://github.com/g0t4/mongo-csharp-driver/commit/1ad28750b052a8c6f789d39c6d522798c0aafc46, the driver was updated to clone & freeze default options in BsonBaseSerializer's constructor. As a result, any calls to several static constructors in the API will trigger this and freeze the default options at that point in time for DictionarySerializationOptions and DateTimeSerializationOptions. It's very easy to accidentally trigger and in some cases hard to avoid when re-using modules that depend on mongodb that have their own initialization that might trigger it as well. I have a patch request coming from GitHub. If clone & freeze is left, this should be reflected with exceptions in the defaults API (DateTimeSerializationOptions.Defaults), but even doing that would cause issues when modules trigger the Clone & Freeze, so I would suggest Freezing be a feature the consumer can request if desired and not forced on them via the driver and Clone should be removed. IMO, the best fix would be to refactor the configuration of serialization (primarily BsonSerializer) to be contextual and not static, this way different modules can operate in the same process and not stomp on each other's configuration. This could be passed into a new MongoDatabase and forwarded to MongoCollection instances. [Edited by Robert Stam]: See the comment added 11/29 describing the actual change made to address setting default options for serializers. The summary of the issue has also been edited to reflect that. To summarize the changes:
|
| Comments |
| Comment by auto [ 01/Jan/13 ] |
|
Author: {u'date': u'2012-12-31T23:59:11Z', u'email': u'robert@10gen.com', u'name': u'rstam'}Message: |
| Comment by Robert Stam [ 31/Dec/12 ] |
|
Changed the description of the JIRA issue to match what we are actually changing to address this issue. |
| Comment by Robert Stam [ 31/Dec/12 ] |
|
Linked to a new JIRA proposing possible support for multiple serialization contexts. |
| Comment by Robert Stam [ 30/Nov/12 ] |
|
I don't think removing Clone and Freeze is the proper solution. It is important to freeze the default serialization options when the serializer is created because we want serialization to be stable. We don't want you to be able to modify the behavior of a serializer after it has been created and registered. I think the real problem is that the static BsonDateTimeSerializer Instance is being created too early (and therefore picking up the value of DateTimeSerializationOptions.Defaults before you have had a chance to set it). I think the proper solution is to get rid of the static Instance property in BsonDateTimeSerializer (the one that is being created too early) and to change the __serializers dictionary in BsonDefaultSerializationProvider to be a mapping from data types to serializer types (instead of to serializer instances) and to postpone creating the serializer instance until it is first needed, giving you ample chance to set the DateTimeSerializationOptions Defaults first. It would probably also be a good idea to deprecate the Defaults property of DateTimeSerializationOptions, and instead provide a new constructor of DateTimeSerializer that lets you pass in the desired default options. Then instead of setting DateTimeSerializationOptions.Defaults you would create and register an instance of DateTimeSerializer with the desired defaults. |
| Comment by Robert Stam [ 30/Nov/12 ] |
|
Thanks for the sample code. It's very helpful to have an actual use case to look at. |
| Comment by Wesley McClure [ 30/Nov/12 ] |
|
Yes and no, I don't think it's necessary at any point, but: This code works: However if I simply flip the order around, it will no longer accept my defaults because the static call to BsonSerializer reaches all the way to clone & freeze in the datetime serializer: This is not easy to figure out without looking at driver code and is breaking several of our apps that were initially built on the 1.3 drivers. With that said, I'd prefer to have serialization contextual, I think that's the better approach to clone & freeze, I'd be willing to help make those changes happen but the bigger problem is the fact that it might not be possible without breaking changes to the API surface. |
| Comment by Robert Stam [ 29/Nov/12 ] |
|
I'm not entirely sure what your point is. Are you saying that you would like to be able to change the value of DateTimeSerializationOptions.Defaults at any time during the execution of your program and have the BsonDateTimeSerializer start using the new value from then on? We have talked about (and prototyped) having multiple serialization configs (instead of a single global one like we do know), but it's a pretty big change. |
| Comment by Wesley McClure [ 26/Nov/12 ] |