-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-5345: Unable to deserialize C# DateOnly objects #1534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
672d248
fcc2c0e
70cd446
cd71b70
c223684
1f7c307
434baed
41ec43c
1e9f28c
0d3bc23
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| /* Copyright 2010-present MongoDB Inc. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| using System; | ||
| using MongoDB.Bson.Serialization.Options; | ||
| using MongoDB.Bson.Serialization.Serializers; | ||
|
|
||
| namespace MongoDB.Bson.Serialization.Attributes | ||
| { | ||
| #if NET6_0_OR_GREATER | ||
| /// <summary> | ||
| /// Specifies the external representation and related options for a DateOnly field or property. | ||
| /// </summary> | ||
| [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field)] | ||
| public class BsonDateOnlyOptionsAttribute : BsonSerializationOptionsAttribute | ||
| { | ||
| // private fields | ||
| private BsonType _representation; | ||
| private DateOnlyDocumentFormat _documentFormat; | ||
|
|
||
| // constructors | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the BsonDateOnlyOptionsAttribute class. | ||
| /// </summary> | ||
| /// <param name="representation">The external representation.</param> | ||
| /// <param name="documentDocumentFormat">The format to use with document representation.</param> | ||
| public BsonDateOnlyOptionsAttribute(BsonType representation, DateOnlyDocumentFormat documentDocumentFormat = DateOnlyDocumentFormat.DateTimeTicks) | ||
| { | ||
| _representation = representation; | ||
| _documentFormat = documentDocumentFormat; | ||
| } | ||
|
|
||
| // public properties | ||
| /// <summary> | ||
| /// Gets the external representation. | ||
| /// </summary> | ||
| public BsonType Representation => _representation; | ||
|
|
||
| /// <summary> | ||
| /// Gets the document format. | ||
| /// </summary> | ||
| public DateOnlyDocumentFormat DocumentFormat => _documentFormat; | ||
|
|
||
| /// <summary> | ||
| /// Reconfigures the specified serializer by applying this attribute to it. | ||
| /// </summary> | ||
| /// <param name="serializer">The serializer.</param> | ||
| /// <returns>A reconfigured serializer.</returns> | ||
| protected override IBsonSerializer Apply(IBsonSerializer serializer) | ||
| { | ||
| var reconfiguredSerializer = SerializerConfigurator.ReconfigureSerializer(serializer, (DateOnlySerializer s) => s.WithRepresentation(_representation, _documentFormat)); | ||
| return reconfiguredSerializer ?? base.Apply(serializer); | ||
| } | ||
| } | ||
| #endif | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /* Copyright 2010-present MongoDB Inc. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| namespace MongoDB.Bson.Serialization.Options | ||
| { | ||
| /// <summary> | ||
| /// Represents the format to use with a DateOnly serializer when the representation is BsonType.Document. | ||
| /// </summary> | ||
| public enum DateOnlyDocumentFormat | ||
| { | ||
| /// <summary> | ||
| /// The document will contain "DateTime" (BsonType.DateTime) and "Ticks" (BsonType.Int64). | ||
| /// </summary> | ||
| DateTimeTicks, | ||
|
|
||
| /// <summary> | ||
| /// The document will contain "Year", "Month" and "Day" (all BsonType.Int32). | ||
| /// </summary> | ||
| YearMonthDay | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| /* Copyright 2010-present MongoDB Inc. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| using System; | ||
|
|
||
| namespace MongoDB.Bson.Serialization | ||
| { | ||
| internal static class SerializerConfigurator | ||
| { | ||
| /// <summary> | ||
| /// Reconfigures a serializer using the specified <paramref name="reconfigure"/> method. | ||
| /// If the serializer implements <see cref="IChildSerializerConfigurable"/>, | ||
| /// the method traverses and applies the reconfiguration to its child serializers recursively until an appropriate leaf serializer is found. | ||
| /// </summary> | ||
| /// <param name="serializer">The input serializer to be reconfigured.</param> | ||
| /// <param name="reconfigure">A function that defines how the serializer of type <typeparamref name="TSerializer"/> should be reconfigured.</param> | ||
| /// <typeparam name="TSerializer">The input type for the reconfigure method.</typeparam> | ||
| /// <returns> | ||
| /// The reconfigured serializer, or <c>null</c> if no leaf serializer could be reconfigured. | ||
| /// </returns> | ||
| internal static IBsonSerializer ReconfigureSerializer<TSerializer>(IBsonSerializer serializer, Func<TSerializer, IBsonSerializer> reconfigure) | ||
| { | ||
| switch (serializer) | ||
| { | ||
| case IChildSerializerConfigurable childSerializerConfigurable: | ||
| var childSerializer = childSerializerConfigurable.ChildSerializer; | ||
| var reconfiguredChildSerializer = ReconfigureSerializer(childSerializer, reconfigure); | ||
| return reconfiguredChildSerializer != null? childSerializerConfigurable.WithChildSerializer(reconfiguredChildSerializer) : null; | ||
|
|
||
| case TSerializer typedSerializer: | ||
| return reconfigure(typedSerializer); | ||
|
|
||
| default: | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
|
|
||
| using System; | ||
| using MongoDB.Bson.IO; | ||
| using MongoDB.Bson.Serialization.Attributes; | ||
| using MongoDB.Bson.Serialization.Options; | ||
|
|
||
| namespace MongoDB.Bson.Serialization.Serializers | ||
|
|
@@ -26,7 +27,7 @@ namespace MongoDB.Bson.Serialization.Serializers | |
| public sealed class DateOnlySerializer : StructSerializerBase<DateOnly>, IRepresentationConfigurable<DateOnlySerializer> | ||
| { | ||
| // static | ||
| private static readonly DateOnlySerializer __instance = new DateOnlySerializer(); | ||
| private static readonly DateOnlySerializer __instance = new(); | ||
|
|
||
| /// <summary> | ||
| /// Gets the default DateOnlySerializer. | ||
|
|
@@ -38,19 +39,26 @@ private static class Flags | |
| { | ||
| public const long DateTime = 1; | ||
| public const long Ticks = 2; | ||
| public const long Year = 4; | ||
| public const long Month = 8; | ||
| public const long Day = 16; | ||
|
|
||
| public const long DateTimeTicks = DateTime | Ticks; | ||
| public const long YearMonthDay = Year | Month | Day; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for two sets of flags. We can do this: See below for how the last two are used.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes sense. Before I put it separately mostly for readability, but together with the rest this makes a lot of sense |
||
|
|
||
| // private fields | ||
| private readonly RepresentationConverter _converter; | ||
| private readonly SerializerHelper _helper; | ||
| private readonly BsonType _representation; | ||
| private readonly DateOnlyDocumentFormat _documentFormat; | ||
|
|
||
| // constructors | ||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="DateOnlySerializer"/> class. | ||
| /// </summary> | ||
| public DateOnlySerializer() | ||
| : this(BsonType.DateTime) | ||
| : this(BsonType.DateTime, DateOnlyDocumentFormat.DateTimeTicks) | ||
| { | ||
| } | ||
|
|
||
|
|
@@ -59,6 +67,16 @@ public DateOnlySerializer() | |
| /// </summary> | ||
| /// <param name="representation">The representation.</param> | ||
| public DateOnlySerializer(BsonType representation) | ||
| : this(representation, DateOnlyDocumentFormat.DateTimeTicks) | ||
| { | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="DateOnlySerializer"/> class. | ||
| /// </summary> | ||
| /// <param name="representation">The representation.</param> | ||
| /// <param name="documentFormat">The format to use with the BsonType.Document representation. It will be ignored if the representation is different.</param> | ||
| public DateOnlySerializer(BsonType representation, DateOnlyDocumentFormat documentFormat) | ||
| { | ||
| switch (representation) | ||
| { | ||
|
|
@@ -73,73 +91,84 @@ public DateOnlySerializer(BsonType representation) | |
| } | ||
|
|
||
| _representation = representation; | ||
| _documentFormat = documentFormat; | ||
| _converter = new RepresentationConverter(false, false); | ||
|
|
||
| _helper = new SerializerHelper | ||
| ( | ||
| new SerializerHelper.Member("DateTime", Flags.DateTime), | ||
| new SerializerHelper.Member("Ticks", Flags.Ticks) | ||
| new SerializerHelper.Member("DateTime", Flags.DateTime, isOptional: true), | ||
| new SerializerHelper.Member("Ticks", Flags.Ticks, isOptional: true), | ||
| new SerializerHelper.Member("Year", Flags.Year, isOptional: true), | ||
| new SerializerHelper.Member("Month", Flags.Month, isOptional: true), | ||
| new SerializerHelper.Member("Day", Flags.Day, isOptional: true) | ||
| ); | ||
| } | ||
|
|
||
| // public properties | ||
| /// <inheritdoc /> | ||
| public BsonType Representation => _representation; | ||
|
|
||
| /// <summary> | ||
| /// The format to use for the BsonType.Document representation. It will be ignored if the representation is different. | ||
| /// </summary> | ||
| public DateOnlyDocumentFormat DocumentFormat => _documentFormat; | ||
|
|
||
| //public methods | ||
| /// <inheritdoc /> | ||
| public override DateOnly Deserialize(BsonDeserializationContext context, BsonDeserializationArgs args) | ||
| { | ||
| var bsonReader = context.Reader; | ||
| DateOnly value; | ||
|
|
||
| var bsonType = bsonReader.GetCurrentBsonType(); | ||
|
|
||
| switch (bsonType) | ||
| { | ||
| case BsonType.DateTime: | ||
| value = VerifyAndMakeDateOnly(BsonUtils.ToDateTimeFromMillisecondsSinceEpoch(bsonReader.ReadDateTime())); | ||
| break; | ||
| return VerifyAndMakeDateOnly(BsonUtils.ToDateTimeFromMillisecondsSinceEpoch(bsonReader.ReadDateTime())); | ||
|
|
||
| case BsonType.Document: | ||
| value = default; | ||
| _helper.DeserializeMembers(context, (_, flag) => | ||
| var ticks = 0L; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also get rid of the Like in line 148 below. Refer to the suggested refactoring in this comment if necessary:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense! My bad, I didn't notice the returns in your suggestion before 🤦 |
||
| var year = 0; | ||
| var month = 0; | ||
| var day = 0; | ||
|
|
||
| var foundMemberFlags = _helper.DeserializeMembers(context, (_, flag) => | ||
| { | ||
| switch (flag) | ||
| { | ||
| case Flags.DateTime: bsonReader.SkipValue(); break; // ignore value (use Ticks instead) | ||
| case Flags.Ticks: | ||
| value = VerifyAndMakeDateOnly(new DateTime(Int64Serializer.Instance.Deserialize(context), DateTimeKind.Utc)); | ||
| break; | ||
| case Flags.DateTime: bsonReader.SkipValue(); break; // ignore value (use Ticks instead) | ||
| case Flags.Ticks: ticks = Int64Serializer.Instance.Deserialize(context); break; | ||
| case Flags.Year: year = Int32Serializer.Instance.Deserialize(context); break; | ||
| case Flags.Month: month = Int32Serializer.Instance.Deserialize(context); break; | ||
| case Flags.Day: day = Int32Serializer.Instance.Deserialize(context); break; | ||
| } | ||
| }); | ||
| break; | ||
|
|
||
| return foundMemberFlags switch | ||
| { | ||
| Flags.DateTimeTicks => VerifyAndMakeDateOnly(new DateTime(ticks, DateTimeKind.Utc)), | ||
| Flags.YearMonthDay => new DateOnly(year, month, day), | ||
| _ => throw new FormatException("Invalid document format.") | ||
| }; | ||
|
|
||
| case BsonType.Decimal128: | ||
| value = VerifyAndMakeDateOnly(new DateTime(_converter.ToInt64(bsonReader.ReadDecimal128()), DateTimeKind.Utc)); | ||
| break; | ||
| return VerifyAndMakeDateOnly(new DateTime(_converter.ToInt64(bsonReader.ReadDecimal128()), DateTimeKind.Utc)); | ||
|
|
||
| case BsonType.Double: | ||
| value = VerifyAndMakeDateOnly(new DateTime(_converter.ToInt64(bsonReader.ReadDouble()), DateTimeKind.Utc)); | ||
| break; | ||
| return VerifyAndMakeDateOnly(new DateTime(_converter.ToInt64(bsonReader.ReadDouble()), DateTimeKind.Utc)); | ||
|
|
||
| case BsonType.Int32: | ||
| value = VerifyAndMakeDateOnly(new DateTime(bsonReader.ReadInt32(), DateTimeKind.Utc)); | ||
| break; | ||
| return VerifyAndMakeDateOnly(new DateTime(bsonReader.ReadInt32(), DateTimeKind.Utc)); | ||
|
|
||
| case BsonType.Int64: | ||
| value = VerifyAndMakeDateOnly(new DateTime(bsonReader.ReadInt64(), DateTimeKind.Utc)); | ||
| break; | ||
| return VerifyAndMakeDateOnly(new DateTime(bsonReader.ReadInt64(), DateTimeKind.Utc)); | ||
|
|
||
| case BsonType.String: | ||
| value = DateOnly.ParseExact(bsonReader.ReadString(), "yyyy-MM-dd"); | ||
| break; | ||
| return DateOnly.ParseExact(bsonReader.ReadString(), "yyyy-MM-dd"); | ||
|
|
||
| default: | ||
| throw CreateCannotDeserializeFromBsonTypeException(bsonType); | ||
| } | ||
|
|
||
| return value; | ||
|
|
||
| DateOnly VerifyAndMakeDateOnly(DateTime dt) | ||
| { | ||
|
|
@@ -160,7 +189,8 @@ public override bool Equals(object obj) | |
| return | ||
| base.Equals(obj) && | ||
| obj is DateOnlySerializer other && | ||
| _representation.Equals(other._representation); | ||
| _representation.Equals(other._representation) && | ||
| _documentFormat.Equals(other._documentFormat); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably need to refactor |
||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
|
|
@@ -182,8 +212,17 @@ public override void Serialize(BsonSerializationContext context, BsonSerializati | |
|
|
||
| case BsonType.Document: | ||
| bsonWriter.WriteStartDocument(); | ||
| bsonWriter.WriteDateTime("DateTime", millisecondsSinceEpoch); | ||
| bsonWriter.WriteInt64("Ticks", utcDateTime.Ticks); | ||
| if (_documentFormat is DateOnlyDocumentFormat.DateTimeTicks) | ||
| { | ||
| bsonWriter.WriteDateTime("DateTime", millisecondsSinceEpoch); | ||
| bsonWriter.WriteInt64("Ticks", utcDateTime.Ticks); | ||
| } | ||
| else | ||
| { | ||
| bsonWriter.WriteInt32("Year", value.Year); | ||
| bsonWriter.WriteInt32("Month", value.Month); | ||
| bsonWriter.WriteInt32("Day", value.Day); | ||
| } | ||
| bsonWriter.WriteEndDocument(); | ||
| break; | ||
|
|
||
|
|
@@ -200,10 +239,28 @@ public override void Serialize(BsonSerializationContext context, BsonSerializati | |
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Returns a serializer that has been reconfigured with the specified representation and document format. | ||
| /// </summary> | ||
| /// <param name="representation">The representation.</param> | ||
| /// <param name="documentFormat">The document format to use with BsonType.Document representation.</param> | ||
| /// <returns> | ||
| /// The reconfigured serializer. | ||
| /// </returns> | ||
| public DateOnlySerializer WithRepresentation(BsonType representation, DateOnlyDocumentFormat documentFormat) | ||
| { | ||
| if (representation == _representation && documentFormat == _documentFormat) | ||
| { | ||
| return this; | ||
| } | ||
|
|
||
| return new DateOnlySerializer(representation, documentFormat); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably should include
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely |
||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public DateOnlySerializer WithRepresentation(BsonType representation) | ||
| { | ||
| return representation == _representation ? this : new DateOnlySerializer(representation); | ||
| return representation == _representation ? this : new DateOnlySerializer(representation, _documentFormat); | ||
| } | ||
|
|
||
| // explicit interface implementations | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
internalthings don't need doc comments.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They don't need it, but I think it's not bad to have it. Maybe it helps at understanding what this method does without trying to read too much the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure you can leave these comments, but we're not going to start writing doc comments for internal classes in general. In general code should be written with proper naming that makes code self-describing.
Occasionally some small comments here and there may be helpful, but full-blown doc comments are not necessary.