Skip to content
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

[Required]/IsRequired() on properties of owned entities is ignored #16943

Closed
salaros opened this issue Aug 4, 2019 · 15 comments
Closed

[Required]/IsRequired() on properties of owned entities is ignored #16943

salaros opened this issue Aug 4, 2019 · 15 comments

Comments

@salaros
Copy link

salaros commented Aug 4, 2019

In previous stable EF Core version (2.2.6) required properties of owned entities were marked as not null-able in the migration code. With .NET Core 3.0.0 preview 7 (6, 5 and probably previous versions too) they are marked as null-able (nullable: true).
My tests in this solution demonstrate it.

image

Steps to reproduce

Please take a look OwnedEntityRequired project in this solution and InMemoryTests.OwnedEntityRequired() xUnit test.

public class DummyModel
{
    [Required]
    [Key, Column(Order = 0)]
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public long Id { get; set; }

    [Required]
    public OwnedModel OwnedModel { get; set; }
}

[Owned]
public class OwnedModel
{
    [Column(nameof(RequiredField))]
    [Required]
    public string RequiredField { get; set; }
}

migration on EF Core 2.2.6

RequiredField = table.Column<string>(nullable: false)

migration on EF Core 3.0.0 preview 7

RequiredField = table.Column<string>(nullable: true)

Further technical details

EF Core version: 3.0.0-preview7.19362.6
Database Provider: any (InMemory cannot be used for test, because it ignores IsRequired())
Operating system: any
IDE: Visual Studio 2019 16.2

@smitpatel
Copy link
Member

#9005

@smitpatel
Copy link
Member

https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#de

@salaros
Copy link
Author

salaros commented Aug 4, 2019

#9005

@smitpatel please read both issues carefully, I'm not using table splitting.
IMO this might be a breaking change, but at the same time it's a regression (just like global filters in EF Core 3.0.0 preview 6+)

@smitpatel
Copy link
Member

The issue has been discussed in detail in #16552

@salaros
Copy link
Author

salaros commented Aug 5, 2019

@ajcvickers this is pretty frustrating experience when a bot-like user with no real code on GitHub shuts down your bug reports, you spent hours to polish, by creating demonstrable samples.

I just want to understand how can we get the same functionality we had on 2.2.6 when [Required]/IsRequired()-marked properties are CORRECTLY created as NON-nullable fields in the migration code.

If there is not way to revert new behavior, then it's a bug/regression

@salaros
Copy link
Author

salaros commented Aug 5, 2019

@ajcvickers @AndriySvyryd will I be able to restore 2.2.6 behavior with #15607 implemented and by marking owned property itself as [Required]/IsRequired()?

@AndriySvyryd
Copy link
Member

@salaros Yes, #15607 will allow you to configure the columns as non-nullable.

@aliensqueegee
Copy link

aliensqueegee commented Sep 19, 2019

I'm having the same issue with IsRequired() being ignored for owned type properties.
I'm using EF Core 3.0.0-rc1.19456.14 with SQLite database provider.
@salaros @AndriySvyryd can you provide some code example with the fix?

Cheers

@Mike-Wazowski
Copy link

@StefanSimion
Have you found out a solution?

@salaros
Copy link
Author

salaros commented Oct 8, 2019

@StefanSimion @Mike-Wazowski there is not solution, unfortunately for now EF Core team is busy breaking things right before stable releases, by committing controversial stuff
#16999

@davidhenley
Copy link

davidhenley commented Oct 14, 2019

I really need this functionality back. (or a workaround for now)

@AndriySvyryd
Copy link
Member

@davidhenley If you need the column to not be nullable you can modify the generated migration.

@AndriySvyryd
Copy link
Member

Everyone that needs this functionality please vote (:+1:) for #12100

@ftathiago
Copy link

Why this issue was closed, since still not working at efcore 5.0.8?

@ajcvickers
Copy link
Member

@ftathiago It is closed because it is a duplicate of #12100. This is fixed in 5.0. If you are seeing otherwise, then please open a new issues and attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants