CSHARP-5964: Wrap Mql.Hash result serializer in BsonValueCSharpNullSerializer#1968
CSHARP-5964: Wrap Mql.Hash result serializer in BsonValueCSharpNullSerializer#1968kyra-rk wants to merge 8 commits intomongodb:mainfrom
Conversation
kyra-rk
added
the
improvement
| return classMap; | ||
| } | ||
|
|
||
| private static IBsonSerializer CoerceSourceSerializerToMemberSerializer(BsonMemberMap memberMap, IBsonSerializer sourceSerializer) |
There was a problem hiding this comment.
CoerceSourceSerializerToMemberSerializer is only called from CreateNewExpressionSerializer, which seems to be called by the serializer finder visitor when it comes across an unrecognized type, eg. when a LINQ projection results build a new type during runtime eg. .Select(x => new { NewField = Mql.SomeCommand(x.OriginalField) }). Does that sound right?
|
General context for reviewer: the default class-map registration wraps My fix is to wrap |
kyra-rk
changed the title
There was a problem hiding this comment.
Pull request overview
Updates LINQ3 MQL Hash serializer deduction so Mql.Hash(...) results can safely round-trip to C# null (e.g., when the $hash operator yields BSON null due to missing/null input), and adjusts/extends tests to cover the behavior.
Changes:
- Wrap the
Mql.Hashresult serializer inBsonValueCSharpNullSerializer<BsonBinaryData>during serializer deduction. - Update LINQ3 translator/serializer-finder unit tests to expect the wrapped serializer type.
- Extend integration coverage to include a document with missing
Dataand validate projections (including named-type projections) returnnullhashes.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Translators/ExpressionToAggregationExpressionTranslators/MethodTranslators/HashMethodToAggregationExpressionTranslatorTests.cs | Updates expected serializer type for Mql.Hash translations to the C#-null-capable wrapper. |
| tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/SerializerFinders/MqlTests.cs | Updates serializer-finder expectation for Mql.Hash to the wrapper serializer type. |
| tests/MongoDB.Driver.Tests/Linq/Integration/MqlHashTests.cs | Adds missing-Data fixture doc and validates projected hash values include null (including named-type projection). |
| src/MongoDB.Driver/Linq/Linq3Implementation/SerializerFinders/SerializerFinderVisitMethodCall.cs | Wraps BsonBinaryDataSerializer with BsonValueCSharpNullSerializer for Mql.Hash deduction. |
| src/MongoDB.Driver/Linq/Linq3Implementation/SerializerFinders/SerializerFinderNewExpressionSerializerCreator.cs | Adds new using directives (currently unused). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| new() { Id = 1, Data = new BsonBinaryData([0x01, 0x02]) }, | ||
| new() { Id = 1, Data = new BsonBinaryData([0x01, 0x02])}, | ||
| new() { Id = 2, Data = new BsonBinaryData(Guid.Parse("E4A10FB8-7A83-494C-9710-29BBFFB1C262"), GuidRepresentation.Standard) }, | ||
| new() { Id = 4 }, |
There was a problem hiding this comment.
Minor: It could be 3, so no one will ask "is there any reasons to skip values here"
There was a problem hiding this comment.
Oooh, yes agreed. I'll change it!
| } | ||
|
|
||
| [Fact] | ||
| public async Task MqlHash_in_projection_builder_named_type() |
There was a problem hiding this comment.
Why do we need this new test?
There was a problem hiding this comment.
While investigating the ticket I added this test in because I was curious if giving the projection a named type would affect the behaviour, eg. new ContainerHash { Hash = Mql.Hash(...) }. It doesn't, so I'm happy to just remove it.
There was a problem hiding this comment.
Ok I think it's actually more confusing than it is helpful, so will remove it!
No description provided.