Fix optional complex property default values tracking #37387
+155
−5
Conversation
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
Copilot
AI
changed the title
[WIP] Fix tracking of default values in optional complex properties
Fix: Track default values when optional complex properties transition from null
Dec 16, 2025
test/EFCore.Specification.Tests/ComplexTypesTrackingTestBase.cs
Outdated
Show resolved
Hide resolved
AndriySvyryd
force-pushed
the
copilot/fix-optional-complex-properties
branch
from
69cf1bd to
1b84b81
Compare
AndriySvyryd
changed the title
AndriySvyryd
force-pushed
the
copilot/fix-optional-complex-properties
branch
from
1b84b81 to
f45d77d
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #37386 where default values in optional complex properties were not being tracked or saved to the database when the complex property transitions from null to a non-null value with default-valued properties.
Key Changes
- Enhanced change detection logic to detect when nullable complex properties transition between null and non-null states
- When a complex property changes from null to non-null, all inner properties are now marked as modified to ensure they are persisted (even with default values)
- Added comprehensive test coverage for the scenario with multiple properties having default values
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/EFCore/ChangeTracking/Internal/ChangeDetector.cs |
Added DetectComplexPropertyChange method to detect null/non-null transitions and mark inner properties as modified; integrated this detection into both PropertyChanged and LocalDetectChanges methods |
test/EFCore.Specification.Tests/ComplexTypesTrackingTestBase.cs |
Added test Can_save_default_values_in_optional_complex_property_with_multiple_properties with entity and complex type definitions; updated ExecuteWithStrategyInTransactionAsync signature to support a third nested operation |
test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs |
Skipped the new test for proxies fixture since complex types with notification change tracking are not yet supported (Issue #36175) |
test/EFCore.Specification.Tests/ComplexTypesTrackingTestBase.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
AndriySvyryd
reviewed
Jan 7, 2026
…ory tests Updated the method signature to include nestedTestOperation3 parameter to match the base class, which was updated to support 4 nested test operations. Co-authored-by: AndriySvyryd <[email protected]>
Updated the method signature to include nestedTestOperation3 parameter to match the base class, which was updated to support 4 nested test operations. Co-authored-by: AndriySvyryd <[email protected]>
AndriySvyryd
reviewed
Jan 8, 2026
… PropertySaveBehavior - Made DetectComplexPropertyChange virtual to satisfy API consistency requirements - Updated logic to only mark properties with AfterSaveBehavior == Save to avoid modifying read-only properties like discriminators - Fixes ApiConsistencyTest.Public_inheritable_apis_should_be_virtual - Fixes AdHocComplexTypeQuerySqliteTest.Optional_complex_type_with_discriminator - Sqlite tests for multi-property complex types now pass Co-authored-by: AndriySvyryd <[email protected]>
…le_properties in InMemory tests Override the test method to return Task.CompletedTask, effectively skipping it for InMemory provider due to query compilation issues with complex types. References issue #31464. Co-authored-by: AndriySvyryd <[email protected]>
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.
Fixes #37386
When optional complex properties transition from null to non-null with default-valued properties (e.g.,
new LockInfo(default(DateTimeOffset))), those default values were not being tracked or saved to the database.Changes
ChangeDetector.PropertyChangedto detect when non-collection complex properties transition from null to non-nullExecuteWithStrategyInTransactionAsyncto support 4 nested test operations in base class and all derived test classes (InMemory and Cosmos)Testing
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.