Skip to content

Conversation

AndriySvyryd
Copy link
Member

@AndriySvyryd AndriySvyryd commented Jul 30, 2025

  • Change the convention to automatically configure nested properties inside complex types as complex properties instead of navigations
  • Throw for shadow properties on value complex types
  • Throw for complex collections not mapped to JSON
  • Warn when mapping a collection as non-collection complex property
  • Fix typos in string resources

Part of #31237

cc @artl93

@AndriySvyryd AndriySvyryd requested a review from a team as a code owner July 30, 2025 22:38
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 implements several improvements to complex property handling in Entity Framework Core, focusing on validation, conventions, and error handling for null complex properties with nested required properties. The changes include enhanced validation for complex collections, better conventions for discovering nested complex properties, and fixes for various string resources throughout the codebase.

Key Changes

  • Modified complex property discovery convention to automatically configure nested properties as complex properties instead of navigations
  • Added comprehensive validation for complex collections requiring JSON mapping and detecting accidental collection properties
  • Enhanced shadow property validation for value type complex types with proper error handling
  • Fixed multiple typos and grammatical errors in string resources and error messages

Reviewed Changes

Copilot reviewed 20 out of 24 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs Added StructTag struct for testing complex property scenarios
test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs Added tests for collection type warnings and shadow property validation on value types
test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.ComplexType.cs Simplified complex property configuration by removing explicit nested configurations
test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.ComplexCollections.cs Added ConfigureComplexCollection helper method and updated all test configurations
test/EFCore.Specification.Tests/ComplexTypesTrackingTestBase.cs Removed explicit nested complex property configurations, relying on convention
test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.PropertyMapping.cs Added tests for complex collection JSON mapping validation
test/EFCore.Relational.Specification.Tests/ModelBuilding/RelationalModelBuilderTest.cs Updated relational model builder tests with JSON configuration requirements
src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs Fixed typo in error message from "conversation" to "conversion"
src/EFCore/Properties/CoreStrings.resx Fixed multiple typos and grammatical errors in resource strings
src/EFCore/Metadata/Internal/MemberClassifier.cs Updated complex property detection to use IList<> instead of sequence types
src/EFCore/Metadata/Conventions/ComplexPropertyDiscoveryConvention.cs Enhanced convention to work with complex types and updated documentation
src/EFCore/Infrastructure/ModelValidator.cs Added comprehensive validation for complex properties including collection detection and shadow property validation
src/EFCore/Diagnostics/LoggingDefinitions.cs Added new logging definition for accidental complex property collections
src/EFCore/Diagnostics/CoreLoggerExtensions.cs Added logging method for accidental complex property collection warnings
src/EFCore/Diagnostics/CoreEventId.cs Added new event ID for accidental complex property collection warnings
src/EFCore.Relational/Update/ModificationCommand.cs Removed unused using statements for cleaner code
src/EFCore.Relational/Update/ColumnModification.cs Enhanced null handling for nested complex properties with recursive validation
src/EFCore.Relational/Properties/RelationalStrings.resx Fixed typos and added new error message for complex collections not mapped to JSON
src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs Added validation requiring complex collections to be mapped to JSON
src/EFCore.InMemory/Storage/Internal/InMemoryTypeMappingSource.cs Simplified type checking using IsScalarType() extension method
Files not reviewed (2)
  • src/EFCore.Relational/Properties/RelationalStrings.Designer.cs: Language not supported
  • src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported

@AndriySvyryd AndriySvyryd force-pushed the Issue31237_ModelBuilding2 branch 3 times, most recently from fb3deee to 18f5a98 Compare July 31, 2025 00:52
Copy link
Contributor

@cincuranet cincuranet left a comment

Choose a reason for hiding this comment

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

I manually verified my previously failing case and now the data is stored properly. The test failure (BigModel_with_JSON_columns) is unrelated to my case.

…rties

Change the convention to automatically configure nested properties inside complex types as complex properties instead of navigations
Throw for shadow properties on value complex types
Throw for complex collections not mapped to JSON
Warn when mapping a collection as non-collection complex property
Fix typos in string resources

Part of #31237
@AndriySvyryd AndriySvyryd force-pushed the Issue31237_ModelBuilding2 branch from 18f5a98 to b001aab Compare July 31, 2025 19:39
@AndriySvyryd AndriySvyryd enabled auto-merge (squash) July 31, 2025 19:59
@AndriySvyryd AndriySvyryd disabled auto-merge July 31, 2025 23:29
@AndriySvyryd AndriySvyryd merged commit d48457e into main Jul 31, 2025
5 of 7 checks passed
@AndriySvyryd AndriySvyryd deleted the Issue31237_ModelBuilding2 branch July 31, 2025 23:29
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