BsonType configuration #97
Conversation
… model builder API.
| @@ -53,65 +54,127 @@ public static class MongoPropertyBuilderExtensions | |||
| => (PropertyBuilder<TProperty>)HasElementName((PropertyBuilder)propertyBuilder, name); | |||
|
|
|||
| /// <summary> | |||
| /// Configures the <see cref="DateTimeKind"/> for the property. | |||
| /// Configures the document element that the property is mapped to when targeting MongoDB. | |||
| /// If an empty string is supplied then the property will not be persisted. | |||
There was a problem hiding this comment.
Although not recommended, an empty string is a valid field name in MongoDB. null however is not a valid field name. Is an empty string not mapping a property an EF convention or something that we are introducing?
There was a problem hiding this comment.
This was just improving the documentation. EF uses this convention in other providers. If we want to support writing to an element with an empty name we may be able to that but we'd need to create a new ticket and figure out an alternative marker for unmapped properties.
| propertyBuilder.Metadata.SetDateTimeKind(dateTimeKind); | ||
| if (!CanSetElementName(propertyBuilder, name, fromDataAnnotation)) | ||
| { | ||
| return null; |
There was a problem hiding this comment.
Should we not be throwing an InvalidOperationException in this case? If we return null, then we will throw a NullReferenceException when we chain the fluent API.
There was a problem hiding this comment.
This return null pattern is used across all EF providers. You can see it in the Cosmos, SqlServer and relational base providers.
| { | ||
| propertyBuilder.Metadata.SetDateTimeKind(dateTimeKind); | ||
| ArgumentNullException.ThrowIfNull(bsonType); |
There was a problem hiding this comment.
BsonType is an enum and cannot be null.
There was a problem hiding this comment.
Do we want to make this BsonType? so that null is valid and can be used to unset a previous HasBsonType configuration?
|
|
||
| // Mongo structs | ||
| _ when type == typeof(ObjectId) => ObjectIdSerializer.Instance, | ||
| _ when type == typeof(Decimal128) => new Decimal128Serializer(), |
There was a problem hiding this comment.
We should configure serializers for ObjectId and Decimal128, but this seems unrelated to the PR at hand. Move to a separate PR?
There was a problem hiding this comment.
This doesn't add/remove any support, it just groups them together with comments as the list was getting long. I can revert.
There was a problem hiding this comment.
Removed this clean-up from the PR.
There was a problem hiding this comment.
This seems related to adding support for ObjectIdSerializer, not HasBsonType support.
There was a problem hiding this comment.
This is a new class for testing round-tripping data back and forth to MongoDB with an extra check via the C# driver. It came out of writing all the new BsonType tests as they were getting repetitive. It's not related to any other work.
| { | ||
| var bsonDateTime = new BsonDateTime(dateTime); | ||
| return dateTime.Kind == DateTimeKind.Utc ? bsonDateTime.ToUniversalTime() : bsonDateTime.ToLocalTime(); | ||
| } |
There was a problem hiding this comment.
Unrelated to HasBsonType support. Move to separate PR.
There was a problem hiding this comment.
This is test-only code and relates to testing DateTime's which I need for the new BsonType tests that touch BsonDateTime.
I had a ToExpectedPrecision method I was previously using for some other BsonDateTime related tests but it didn't handle UTC. The new method is used for both the new tests and the old.
Provides a
HasBsonTypefluent API that allows the model builder to configure specific properties to be stored as specific BSON types.Fixes EF-127
A subsequent PR will further expose this feature out so that
BsonRepresentationattributes can be recognized (EF-44)