Skip to content

Add complex collection support to PropertyValues #36366

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

Merged
merged 2 commits into from
Jul 18, 2025

Conversation

AndriySvyryd
Copy link
Member

Part of #31237

This pull request extends PropertyValues and derived types to handle complex collection properties and adds an error message in the Cosmos provider strings. It also fixes some bugs in complex collection change detection.

Enhancements to PropertyValues:

  • PropertyValues.cs: Extended the class to support complex collection properties, including methods for cloning, setting values, and converting complex collections to objects. Introduced new properties like ComplexCollectionProperties for managing complex type collections.

Validation for complex type collections:

@AndriySvyryd AndriySvyryd requested a review from Copilot July 12, 2025 04:43
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner July 12, 2025 04:43
Copy link

@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 enhances the PropertyValues infrastructure in EF Core to fully support complex collection properties. It adds new APIs and internal logic to:

  • Store, clone, and materialize complex collections in PropertyValues, EntryPropertyValues, and ArrayPropertyValues.
  • Detect, reorder, and track state changes in complex collection entries via InternalEntryBase and ChangeDetector.
  • Provide empty and populated materializers for complex types in EntityMaterializerSource.
  • Surface a clear error when complex type collections are used with the Cosmos provider.

Reviewed Changes

Copilot reviewed 36 out of 38 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/EFCore/ChangeTracking/PropertyValues.cs Added IComplexProperty indexer, ComplexCollectionProperties, and abstracted SetValues to support complex collections.
src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs Extended original/current value logic to reorder and detect complex collection entries on state changes.
src/EFCore/ChangeTracking/Internal/ChangeDetector.cs Enhanced DetectComplexCollectionChanges to handle additions, removals, moves, and null elements in complex collections.
src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs Added storage for nested ArrayPropertyValues lists and logic to populate complex objects in ToObject().
src/EFCore/Query/Internal/EntityMaterializerSource.cs Initialize complex collections to null during materialization and added public methods to get materializers for complex types.
src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs Throw an exception for unsupported complex type collections in the Cosmos provider.
Files not reviewed (2)
  • src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs: Language not supported
  • src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)

src/EFCore/Query/IEntityMaterializerSource.cs:83

  • [nitpick] The XML documentation for GetMaterializer and GetEmptyMaterializer still refers to "entity type" and mentions caching, but these methods now target complex types and always compile a new delegate. Update the <summary> and <param> tags to correctly describe complex type materialization and clarify that the returned delegate is not necessarily cached.
    ///     <para>

@AndriySvyryd AndriySvyryd force-pushed the Issue31237_PropertyValues branch from 827202c to 351ae49 Compare July 14, 2025 21:27
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

🐑 🇮🇹 on some parts of change detection, but overall understandable and LGTM.

@AndriySvyryd AndriySvyryd force-pushed the Issue31237_PropertyValues branch 4 times, most recently from bfa6a86 to af9e1dc Compare July 18, 2025 02:56
@AndriySvyryd AndriySvyryd force-pushed the Issue31237_PropertyValues branch from af9e1dc to 1b05bed Compare July 18, 2025 03:41
@AndriySvyryd AndriySvyryd enabled auto-merge (rebase) July 18, 2025 04:47
@AndriySvyryd AndriySvyryd merged commit badb1e8 into main Jul 18, 2025
7 checks passed
@AndriySvyryd AndriySvyryd deleted the Issue31237_PropertyValues branch July 18, 2025 05:32
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.

2 participants