[CSHARP-371] In BsonUtils ToDateTimeFromMillisecondsSinceEpoch and ToMillisecondsSinceEpoch should be mirrors of each other Created: 29/Dec/11 Updated: 02/Apr/15 Resolved: 03/Jan/12 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | None |
| Affects Version/s: | 1.3.1 |
| Fix Version/s: | 1.4 |
| Type: | Bug | Priority: | Trivial - P5 |
| Reporter: | Sridhar Nanjundeswaran | Assignee: | Sridhar Nanjundeswaran |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
ToMillisecondsSinceEpoch does not special case DateTimeMax. |
| Comments |
| Comment by Sridhar Nanjundeswaran [ 03/Jan/12 ] | |||||
|
Special casing for min value not needed since it works as is. Also ensured that greater than DateTime.MaxValue or less than DateTime.MinValue throws the expected ArgumentOutOfRange exception | |||||
| Comment by Robert Stam [ 30/Dec/11 ] | |||||
|
Here's a unit test that might support the original notion that we should map MaxValue to MaxValue and MinValue to MinValue:
If these unit tests don't pass, what is the rational for them not passing? I'm not 100% sure they should pass, but it does feel difficult to explain why they wouldn't. And there should probably be similar unit tests to see what happens when you serialize a BsonDateTime created from DateTime.MaxValue and DateTime.MinValue. If you insert MaxValue/MinValue into a collection and read it back what do you get? | |||||
| Comment by Robert Stam [ 29/Dec/11 ] | |||||
|
Note: currently all callers of ToDateTimeFromMillisecondsSinceEpoch from within the driver itself would appear to always call this method with a millisecondsSinceEpoch value that is greater than DateTime.MinValue and less than DateTime.MaxValue. Therefore the mapping is always valid. An alternative to mapping Max to Max (and Min to Min which is not being done) is to throw an exception if the value being passed in cannot be mapped to a valid .NET DateTime. After seeing how ToDateTimeFromMillisecondsSinceEpoch is used I think an ArgumentOutOfRange exception is appropriate. ToMillisecondsSinceEpoch can't fail since the range of .NET DateTime is a subset of the range of BSON DateTimes, so it probably doesn't need to change at all. |