CSHARP-5737: Add legacy representation for TimeOnly

[papafe]

No description provided.

[papafe]

Microsecond and Nanosecond were introduced in .NET 7.0

[rstam]

Overall looks good. Let me know what you think of the comments.

[rstam]

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.

[rstam]

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)                                                              
);                                                                                                                                    

[papafe]

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

[rstam]

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

[rstam]

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;

[rstam]

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;
}                                                                           

[papafe]

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.

[rstam]

Do we really want to ignore these values?

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

[papafe]

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.