CSHARP-5656: Support Aggregation Operator to generate random object ids#1931
Open
sanych-sun wants to merge 5 commits intomongodb:mainfrom
Open
CSHARP-5656: Support Aggregation Operator to generate random object ids#1931sanych-sun wants to merge 5 commits intomongodb:mainfrom
sanych-sun wants to merge 5 commits intomongodb:mainfrom
Conversation
sanych-sun
requested review from
BorisDog,
adelinowona and
kyra-rk
and removed request for
kyra-rk
Contributor
There was a problem hiding this comment.
Pull request overview
Adds LINQ3 translation support for ObjectId.GenerateNewId() (parameterless overload) by translating it to the aggregation expression $createObjectId, rather than partially evaluating it client-side to a constant.
Changes:
- Introduces a new AST expression node for
$createObjectIdand wires it into the AST visitor/rendering surface. - Adds a dedicated method translator + serializer deduction support for
ObjectId.GenerateNewId()and registers it in the method-call translation pipeline. - Prevents partial evaluation of
ObjectId.GenerateNewId()and adds unit + integration test coverage guarded by a new server feature flag.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Translators/ExpressionToAggregationExpressionTranslators/MethodTranslators/GenerateNewIdMethodToAggregationExpressionTranslatorTests.cs | Unit tests validating $createObjectId AST output and unsupported overload behavior. |
| tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/SerializerFinders/ObjectIdCreateNewIdTests.cs | SerializerFinder test ensuring ObjectId.GenerateNewId() resolves to ObjectIdSerializer. |
| tests/MongoDB.Driver.Tests/Linq/Integration/ObjectIdGenerateNewIdTests.cs | Integration coverage for LINQ/queryable + builders + pipeline translation/execution with $createObjectId. |
| src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToAggregationExpressionTranslators/MethodTranslators/GenerateNewIdMethodToAggregationExpressionTranslator.cs | New translator mapping ObjectId.GenerateNewId() to AstExpression.CreateObjectId(). |
| src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToAggregationExpressionTranslators/MethodCallExpressionToAggregationExpressionTranslator.cs | Registers GenerateNewId method name dispatch to the new translator. |
| src/MongoDB.Driver/Linq/Linq3Implementation/SerializerFinders/SerializerFinderVisitMethodCall.cs | Adds serializer deduction for ObjectId.GenerateNewId() calls. |
| src/MongoDB.Driver/Linq/Linq3Implementation/Reflection/ReflectionInfo.cs | Adds parameterless Method<TResult>(Expression<Func<TResult>>) helper for reflection extraction. |
| src/MongoDB.Driver/Linq/Linq3Implementation/Reflection/ObjectIdMethod.cs | Centralizes reflection info for the parameterless ObjectId.GenerateNewId() overload. |
| src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs | Prevents partial evaluation of ObjectId.GenerateNewId() to avoid constant folding. |
| src/MongoDB.Driver/Linq/Linq3Implementation/Ast/Visitors/AstNodeVisitor.cs | Adds a visitor hook for the new $createObjectId AST node. |
| src/MongoDB.Driver/Linq/Linq3Implementation/Ast/Expressions/AstExpression.cs | Adds AstExpression.CreateObjectId() factory method. |
| src/MongoDB.Driver/Linq/Linq3Implementation/Ast/Expressions/AstCreateObjectIdExpression.cs | New AST node rendering { $createObjectId: {} }. |
| src/MongoDB.Driver/Linq/Linq3Implementation/Ast/AstNodeType.cs | Adds CreateObjectIdExpression node type. |
| src/MongoDB.Driver/Core/Misc/Feature.cs | Adds Feature.CreateObjectIdExpression for server capability gating. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+24
to
+27
| public class ObjectIdCreateNewIdTests | ||
| { | ||
| [Fact] | ||
| public void SerializerFinder_should_resolve_objectId_createNewId() |
There was a problem hiding this comment.
The test file/class/method names use CreateNewId/createNewId, but the expression under test is ObjectId.GenerateNewId() and it translates to $createObjectId. Renaming the test class (and test method name) to reference GenerateNewId (or $createObjectId) would reduce confusion and improve discoverability.
Suggested change
| public class ObjectIdCreateNewIdTests | |
| { | |
| [Fact] | |
| public void SerializerFinder_should_resolve_objectId_createNewId() | |
| public class ObjectIdGenerateNewIdTests | |
| { | |
| [Fact] | |
| public void SerializerFinder_should_resolve_objectId_generateNewId() |
BorisDog
requested changes
Apr 8, 2026
|
|
||
| namespace MongoDB.Driver.Linq.Linq3Implementation.Ast.Expressions | ||
| { | ||
| internal sealed class AstCreateObjectIdExpression : AstExpression |
Contributor
There was a problem hiding this comment.
minor, for your consideration: file scoped namespaces, and expression body methods.
I think we should write the new code with new style and not worry about mixed styles.
adelinowona
requested changes
Apr 10, 2026
Comment on lines
+25
to
+36
| public override AstNode Accept(AstNodeVisitor visitor) | ||
| { | ||
| return visitor.VisitCreateObjectIdExpression(this); | ||
| } | ||
|
|
||
| public override BsonValue Render() | ||
| { | ||
| return new BsonDocument | ||
| { | ||
| { "$createObjectId", new BsonDocument() } | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
nit: Could use expression bodies instead of block bodies
| using MongoDB.Driver.Linq.Linq3Implementation.Misc; | ||
| using MongoDB.Driver.Linq.Linq3Implementation.Reflection; | ||
|
|
||
| namespace MongoDB.Driver.Linq.Linq3Implementation.Translators.ExpressionToAggregationExpressionTranslators.MethodTranslators |
Contributor
There was a problem hiding this comment.
use file scoped namespace?
BorisDog
requested changes
Apr 10, 2026
| } | ||
|
|
||
| [Fact] | ||
| public void MqlCreateObjectId_in_where() |
Contributor
There was a problem hiding this comment.
Should we continue adding the "expected behavior" part in tests names here as well?
| return ExtractConstructorInfoFromLambda(lambda); | ||
| } | ||
|
|
||
| public static MethodInfo Method<TResult>(Expression<Func<TResult>> lambda) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.