Skip to content

Conversation

AndriySvyryd
Copy link
Member

Fixes #36402

@AndriySvyryd AndriySvyryd requested review from roji and Copilot July 21, 2025 17:38
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner July 21, 2025 17: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 fixes the null validation logic for required nested complex properties in Entity Framework Core. The issue was that EF Core was incorrectly throwing validation errors for required properties within complex types when the parent complex type itself was null, which should be allowed.

  • Removes workaround configuration that marked required nested complex properties as optional
  • Updates null validation logic to properly handle nested complex properties within optional parent complex types
  • Ensures validation only occurs when the parent complex type is non-null

Reviewed Changes

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

File Description
ComplexJsonRelationalFixtureBase.cs Removes TODO comment and workaround configuration, allowing RequiredNested to use its natural required configuration
InternalEntryBase.cs Updates null validation logic for both collections and complex properties to check parent complex type nullability

@AndriySvyryd AndriySvyryd merged commit 10ca1e9 into main Jul 21, 2025
7 checks passed
@AndriySvyryd AndriySvyryd deleted the Issue36402 branch July 21, 2025 19:02
// TODO: Currently everything within an optional complex type must be configured as optional - seems wrong.
// #36402
orb.ComplexProperty(r => r.RequiredNested).IsRequired(false);
orb.ComplexProperty(r => r.RequiredNested);
Copy link
Member

Choose a reason for hiding this comment

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

Should be removable altogether no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not until I change the navigation discovery convention logic

Copy link
Member

Choose a reason for hiding this comment

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

I might be cnofused, but looking at the configuration just above - for RequiredRelated - the configuration for RequiredNested is omitted there. And yet I can see it mapped as a complex JSON property (e.g. see test Where_nested_related_property for SQL Server)...

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.

Figure out the story around required complex properties within optional complex types
3 participants