Skip to content

Conversation

ajcvickers
Copy link
Collaborator

  • EF-76: Flow EF metadata down through CreateGetValueExpression

That is, the IProperty or INavigation associated with the access. Types, requiredness, type-mappings, etc. are then obtained from the property.

An alias is passed in if the name in the BSON document is different from the mapped element name for the property.

  • Updates based on co-pilot review.

…b#240)

* EF-76: Flow EF metadata down through CreateGetValueExpression

That is, the IProperty or INavigation associated with the access. Types, requiredness, type-mappings, etc. are then obtained from the property.

An alias is passed in if the name in the BSON document is different from the mapped element name for the property.

* Updates based on co-pilot review.
@ajcvickers ajcvickers requested a review from a team as a code owner October 8, 2025 09:07
@ajcvickers ajcvickers requested review from BorisDog and Copilot October 8, 2025 09:07
@ajcvickers ajcvickers closed this Oct 8, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the BSON binding system to flow Entity Framework metadata through the CreateGetValueExpression method, replacing primitive parameters with rich metadata objects. The core change passes an IProperty or INavigation object that contains type information, nullability, and type mappings instead of relying on separate parameters.

Key changes:

  • Modified CreateGetValueExpression to accept IPropertyBase instead of separate name, type, and requirement parameters
  • Updated GetPropertyValue and GetPropertySerializationInfo methods to accept an alias parameter for field name mapping
  • Removed the root entity type parameter from MongoProjectionBindingRemovingExpressionVisitor constructor

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
BsonBinding.cs Core refactoring to use IPropertyBase metadata instead of primitive parameters
MongoProjectionBindingRemovingExpressionVisitor.cs Updated to work with new metadata-driven approach and removed root entity type dependency
BsonSerializerFactory.cs Added alias parameter support for property serialization
EntitySerializer.cs Updated method call to include null alias parameter
MongoUpdate.cs Updated method call to include null alias parameter
BsonBindingTests.cs Updated test method calls to include null alias parameter
NorthwindMiscellaneousQueryMongoTest.cs Updated test assertions to reflect improved error handling
Comments suppressed due to low confidence (1)

src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoProjectionBindingRemovingExpressionVisitor.cs:1

  • Consider using a more specific exception type or converting this to a runtime check with an appropriate exception. Debug assertions are removed in release builds, potentially allowing invalid states to proceed.
/* Copyright 2023-present MongoDB Inc.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +423 to +425
// TODO: handle more nesting; not currently used.
//RootReferenceExpression => CreateGetValueExpression(DocParameter, null, required, typeof(BsonDocument), propertyBase: propertyBase, declaredType: propertyBase!.DeclaringType),
//ObjectAccessExpression docAccessExpression => CreateGetValueExpression(docAccessExpression.AccessExpression, docAccessExpression.Name, required, typeof(BsonDocument), propertyBase: propertyBase, declaredType: propertyBase!.DeclaringType),
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented-out code that is not currently used. If this functionality is needed for future development, it should be tracked in an issue rather than left as commented code.

Suggested change
// TODO: handle more nesting; not currently used.
//RootReferenceExpression => CreateGetValueExpression(DocParameter, null, required, typeof(BsonDocument), propertyBase: propertyBase, declaredType: propertyBase!.DeclaringType),
//ObjectAccessExpression docAccessExpression => CreateGetValueExpression(docAccessExpression.AccessExpression, docAccessExpression.Name, required, typeof(BsonDocument), propertyBase: propertyBase, declaredType: propertyBase!.DeclaringType),
// TODO: handle more nesting in future.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant