Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

using System;
using MongoDB.Bson.IO;
using MongoDB.Bson.Serialization.Attributes;
using MongoDB.Bson.Serialization.Options;

namespace MongoDB.Bson.Serialization.Serializers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/

using System;
using MongoDB.Bson.IO;
using MongoDB.Bson.Serialization.Options;

namespace MongoDB.Bson.Serialization.Serializers
Expand All @@ -32,7 +33,20 @@ public sealed class TimeOnlySerializer: StructSerializerBase<TimeOnly>, IReprese
/// </summary>
public static TimeOnlySerializer Instance => __instance;

// private constants
private static class Flags
{
public const long Hour = 1;
public const long Minute = 2;
public const long Second = 4;
public const long Millisecond = 8;
public const long Microsecond = 16;
public const long Nanosecond = 32;
public const long Ticks = 64;
}

// private fields
private readonly SerializerHelper _helper;
private readonly BsonType _representation;
private readonly TimeOnlyUnits _units;

Expand All @@ -58,7 +72,7 @@ public TimeOnlySerializer(BsonType representation)
/// Initializes a new instance of the <see cref="TimeOnlySerializer"/> class.
/// </summary>
/// <param name="representation">The representation.</param>
/// <param name="units">The units.</param>
/// <param name="units">The units. Ignored if representation is BsonType.Document.</param>
public TimeOnlySerializer(BsonType representation, TimeOnlyUnits units)
{
switch (representation)
Expand All @@ -67,6 +81,7 @@ public TimeOnlySerializer(BsonType representation, TimeOnlyUnits units)
case BsonType.Int32:
case BsonType.Int64:
case BsonType.String:
case BsonType.Document:
break;

default:
Expand All @@ -75,6 +90,17 @@ public TimeOnlySerializer(BsonType representation, TimeOnlyUnits units)

_representation = representation;
_units = units;

_helper = new SerializerHelper
(
new SerializerHelper.Member("Hour", Flags.Hour, isOptional: true),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little weird that these are optional.

In understand that we ignore them during deserialization (we only use Ticks). But that doesn't necessarily mean they are optional.

We also might want to validate during deserialization that the Hour, Minutes, Second etc.. fields are consistent with the Ticks value. If WE wrote the values they should be consistent, but if the data came from outside the C# driver they might not be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider the following change:

_helper = new SerializerHelper
(
    new SerializerHelper.Member("Hour", Flags.Hour, isOptional: false),
    new SerializerHelper.Member("Minute", Flags.Minute, isOptional: false),
    new SerializerHelper.Member("Second", Flags.Second, isOptional: false),
    new SerializerHelper.Member("Millisecond", Flags.Millisecond, isOptional: false),
    new SerializerHelper.Member("Microsecond", Flags.Microsecond, isOptional: true), // not serialized by older versions of the driver
    new SerializerHelper.Member("Nanosecond", Flags.Nanosecond, isOptional: true), // not serialized by older versions of the driver  
    new SerializerHelper.Member("Ticks", Flags.Ticks, isOptional: false)                                                              
);                                                                                                                                    

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, that probably those should not be optional, I'll answer the question about validation in the other comment.

new SerializerHelper.Member("Minute", Flags.Minute, isOptional: true),
new SerializerHelper.Member("Second", Flags.Second, isOptional: true),
new SerializerHelper.Member("Millisecond", Flags.Millisecond, isOptional: true),
new SerializerHelper.Member("Microsecond", Flags.Microsecond, isOptional: true),
new SerializerHelper.Member("Nanosecond", Flags.Nanosecond, isOptional: true),
new SerializerHelper.Member("Ticks", Flags.Ticks, isOptional: false)
);
}

// public properties
Expand Down Expand Up @@ -102,6 +128,7 @@ public override TimeOnly Deserialize(BsonDeserializationContext context, BsonDes
BsonType.Int64 => FromInt64(bsonReader.ReadInt64(), _units),
BsonType.Int32 => FromInt32(bsonReader.ReadInt32(), _units),
BsonType.Double => FromDouble(bsonReader.ReadDouble(), _units),
BsonType.Document => FromDocument(context),
_ => throw CreateCannotDeserializeFromBsonTypeException(bsonType)
};
}
Expand Down Expand Up @@ -145,6 +172,20 @@ public override void Serialize(BsonSerializationContext context, BsonSerializati
bsonWriter.WriteString(value.ToString("o"));
break;

case BsonType.Document:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put case BsonType.Document first in the switch statement (alphabetical order).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider the following refactoring so that we consistently write the Microsecond and Nanosecond properties even on older versions of .NET:

                case BsonType.Document:
#if NET7_0_OR_GREATER
                    var microsecond = value.Microsecond;
                    var nanosecond = value.Nanosecond;
#else
                    var microsecond = ComputeMicrosecondFromTicks(value.Ticks);
                    var nanosecond = ComputeNanosecondFromTicks(value.Ticks);
#endif
                    bsonWriter.WriteStartDocument();
                    bsonWriter.WriteInt32("Hour", value.Hour);
                    bsonWriter.WriteInt32("Minute", value.Minute);
                    bsonWriter.WriteInt32("Second", value.Second);
                    bsonWriter.WriteInt32("Millisecond", value.Millisecond);
                    bsonWriter.WriteInt32("Microsecond", microsecond);
                    bsonWriter.WriteInt32("Nanosecond", nanosecond);
                    bsonWriter.WriteInt64("Ticks", value.Ticks);
                    bsonWriter.WriteEndDocument();
                    break;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ComputeMicrosecondFromTicks and ComputeNanosecondFromTicks helper methods can be something like:

private int ComputeMicrosecondFromTicks(long ticks)
{
    return (int)(ticks / (TimeSpan.TicksPerMillisecond * 1_000)) % 1000;
}
 
private int ComputeNanosecondFromTicks(long ticks)
{
    return (int)(ticks / (TimeSpan.TicksPerMillisecond * 1_000_000)) % 1000;
}                                                                           

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the use of the preprocessor directives, that was my first implementation but I thought that given that the implementation is the same, we would not need to necessarily make a differentiation.

bsonWriter.WriteStartDocument();
bsonWriter.WriteInt32("Hour", value.Hour);
bsonWriter.WriteInt32("Minute", value.Minute);
bsonWriter.WriteInt32("Second", value.Second);
bsonWriter.WriteInt32("Millisecond", value.Millisecond);
#if NET7_0_OR_GREATER
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Microsecond and Nanosecond were introduced in .NET 7.0

bsonWriter.WriteInt32("Microsecond", value.Microsecond);
bsonWriter.WriteInt32("Nanosecond", value.Nanosecond);
#endif
bsonWriter.WriteInt64("Ticks", value.Ticks);
bsonWriter.WriteEndDocument();
break;

default:
throw new BsonSerializationException($"'{_representation}' is not a valid TimeOnly representation.");
}
Expand Down Expand Up @@ -196,6 +237,29 @@ private TimeOnly FromInt64(long value, TimeOnlyUnits units)
: new TimeOnly(value * TicksPerUnit(units));
}

private TimeOnly FromDocument(BsonDeserializationContext context)
{
var bsonReader = context.Reader;
var ticks = 0L;

_helper.DeserializeMembers(context, (_, flag) =>
{
switch (flag)
{
case Flags.Hour:
case Flags.Minute:
case Flags.Second:
case Flags.Millisecond:
case Flags.Microsecond:
case Flags.Nanosecond:
bsonReader.SkipValue(); break; // ignore value (use Ticks instead)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to ignore these values?

Should we check that they are in agreement with the Ticks value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is similar to what we do with DateTimeSerializer and DateOnlySerializer, where we ignore the DateTime value when the representation is BsonType.Document and use only the ticks.
The difference is that here we have many more fields that we ignoring. I was trying to be the most permissive here, but I understand if want to be more strict.

case Flags.Ticks: ticks = Int64Serializer.Instance.Deserialize(context); break;
}
});

return FromInt64(ticks, TimeOnlyUnits.Ticks);
}

private long TicksPerUnit(TimeOnlyUnits units)
{
return units switch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,21 @@ public void Attribute_should_set_correct_units()
Microseconds = timeOnly,
Ticks = timeOnly,
Nanoseconds = timeOnly,
Document = timeOnly
};

var json = testObj.ToJson();

var expected = "{ \"Hours\" : 13, "
+ "\"Minutes\" : 804, "
+ "\"Seconds\" : 48293, "
+ "\"Milliseconds\" : 48293000, "
+ "\"Microseconds\" : 48293000000, "
+ "\"Ticks\" : 482930000000, "
+ "\"Nanoseconds\" : 48293000000000 }";
var baseString = """
{ "Hours" : 13, "Minutes" : 804, "Seconds" : 48293, "Milliseconds" : 48293000, "Microseconds" : 48293000000, "Ticks" : 482930000000, "Nanoseconds" : 48293000000000
""";

var documentString = """
{ "Hour" : 13, "Minute" : 24, "Second" : 53, "Millisecond" : 0, "Ticks" : 482930000000 }
""";


var expected = baseString + """, "Document" : """ + documentString + " }";
Assert.Equal(expected, json);
}

Expand All @@ -69,7 +73,7 @@ public void Constructor_with_no_arguments_should_return_expected_result()
[Theory]
[ParameterAttributeData]
public void Constructor_with_representation_should_return_expected_result(
[Values(BsonType.String, BsonType.Int64, BsonType.Int32, BsonType.Double)]
[Values(BsonType.String, BsonType.Int64, BsonType.Int32, BsonType.Double, BsonType.Document)]
BsonType representation,
[Values(TimeOnlyUnits.Ticks, TimeOnlyUnits.Hours, TimeOnlyUnits.Minutes, TimeOnlyUnits.Seconds,
TimeOnlyUnits.Milliseconds, TimeOnlyUnits.Microseconds, TimeOnlyUnits.Nanoseconds)]
Expand All @@ -81,6 +85,53 @@ public void Constructor_with_representation_should_return_expected_result(
subject.Units.Should().Be(units);
}

[Theory]
[InlineData("""{ "x" : { Ticks: { "$numberLong" : "307255946583" } } }""","08:32:05.5946583" )]
[InlineData("""{ "x" : { Ticks: { "$numberLong" : "0" } } }""","00:00:00.0000000" )]
[InlineData("""{ "x" : { Ticks: { "$numberLong" : "863999999999" } } }""","23:59:59.9999999" )]
public void Deserialize_with_document_should_have_expected_result(string json, string expectedResult)
{
var subject = new TimeOnlySerializer();
TestDeserialize(subject, json, expectedResult);
}

[Theory]
[InlineData("""{ "x" : { Ticks: { "$numberDouble" : "307255946583" } } }""","08:32:05.5946583" )]
[InlineData("""{ "x" : { Ticks: { "$numberDecimal" : "307255946583" } } }""","08:32:05.5946583" )]
public void Deserialize_with_document_should_be_forgiving_of_actual_numeric_type(string json, string expectedResult)
{
var subject = new TimeOnlySerializer();
TestDeserialize(subject, json, expectedResult);
}

[Theory]
[InlineData("""
{ "x" : { Hour: { "$numberInt": 0 }, Minute: { "$numberInt": 0 }, Second: { "$numberInt": 0 },
Millisecond: { "$numberInt": 0 }, Microsecond: { "$numberInt": 0 }, Nanosecond: { "$numberInt": 0 },
Ticks: { "$numberDouble" : "307255946583" } } }
""","08:32:05.5946583" )]
public void Deserialize_with_document_should_ignore_other_time_components(string json, string expectedResult)
{
var subject = new TimeOnlySerializer();
TestDeserialize(subject, json, expectedResult);
}

[Theory]
[InlineData("""{ "x" : { "Unknown": "test", Ticks: { "$numberDouble" : "307255946583" } } }""" )]
public void Deserialize_with_document_should_throw_when_field_is_unknown(string json)
{
var subject = new TimeOnlySerializer();

using var reader = new JsonReader(json);
reader.ReadStartDocument();
reader.ReadName("x");
var context = BsonDeserializationContext.CreateRoot(reader);

var exception = Record.Exception(() => subject.Deserialize(context));
exception.Should().BeOfType<BsonSerializationException>();
exception.Message.Should().Be("Invalid element: 'Unknown'.");
}

[Theory]
[InlineData("""{ "x" : "08:32:05.5946583" }""","08:32:05.5946583" )]
[InlineData("""{ "x" : "00:00:00.0000000" }""","00:00:00.0000000")]
Expand Down Expand Up @@ -273,6 +324,17 @@ public void GetHashCode_should_return_zero()
result.Should().Be(0);
}

[Theory]
[InlineData("08:32:05.5946583", """{ "x" : { "Hour" : { "$numberInt" : "8" }, "Minute" : { "$numberInt" : "32" }, "Second" : { "$numberInt" : "5" }, "Millisecond" : { "$numberInt" : "594" }, "Ticks" : { "$numberLong" : "307255946583" } } }""")]
[InlineData("00:00:00.0000000", """{ "x" : { "Hour" : { "$numberInt" : "0" }, "Minute" : { "$numberInt" : "0" }, "Second" : { "$numberInt" : "0" }, "Millisecond" : { "$numberInt" : "0" }, "Ticks" : { "$numberLong" : "0" } } }""")]
[InlineData("23:59:59.9999999", """{ "x" : { "Hour" : { "$numberInt" : "23" }, "Minute" : { "$numberInt" : "59" }, "Second" : { "$numberInt" : "59" }, "Millisecond" : { "$numberInt" : "999" }, "Ticks" : { "$numberLong" : "863999999999" } } }""")]
public void Serialize_with_document_representation_should_have_expected_result(string valueString, string expectedResult)
{
var subject = new TimeOnlySerializer(BsonType.Document);

TestSerialize(subject, valueString, expectedResult);
}

[Theory]
[InlineData(BsonType.String, "08:32:05.5946583", """{ "x" : "08:32:05.5946583" }""")]
[InlineData(BsonType.String, "00:00:00.0000000", """{ "x" : "00:00:00.0000000" }""")]
Expand Down Expand Up @@ -407,8 +469,8 @@ public void Serializer_should_be_registered()
[Theory]
[ParameterAttributeData]
public void WithRepresentation_should_return_expected_result(
[Values(BsonType.String, BsonType.Int64, BsonType.Int32, BsonType.Double)] BsonType oldRepresentation,
[Values(BsonType.String, BsonType.Int64, BsonType.Int32, BsonType.Double)] BsonType newRepresentation)
[Values(BsonType.String, BsonType.Int64, BsonType.Int32, BsonType.Double, BsonType.Document)] BsonType oldRepresentation,
[Values(BsonType.String, BsonType.Int64, BsonType.Int32, BsonType.Double, BsonType.Document)] BsonType newRepresentation)
{
var subject = new TimeOnlySerializer(oldRepresentation);

Expand Down Expand Up @@ -473,6 +535,9 @@ private class TestClass

[BsonTimeOnlyOptions(BsonType.Int64, TimeOnlyUnits.Nanoseconds )]
public TimeOnly Nanoseconds { get; set; }

[BsonTimeOnlyOptions(BsonType.Document)]
public TimeOnly Document { get; set; }
}
}
#endif
Expand Down