Skip to content
This repository has been archived by the owner on Jan 16, 2024. It is now read-only.

Unidirectional Many-to-One Association missing Required (EF Core) #145

Closed
majoreffort opened this issue Feb 9, 2020 · 3 comments
Closed
Labels
bug Confirmed bug released Issue is resolved in a current release
Milestone

Comments

@majoreffort
Copy link

An unidirectional association from Entity1 to Entity2 where End 1 is many and End 2 is one should have the required attribute set / configured for the navigation property on Entity1 to Entity2. See the following simple diagram:
diagram

With the current extension version 1.3.0.12 and EF Core this is missing, neither the Required attribute on the property nor the fluent IsRequired for the foreign key gets generated. Interestingly, "Required" is inserted into the summary comment of the navigation property.

By the way: Awesome tool! Really makes working with EF (Core in my instance) so much more pleasant.

@msawczyn msawczyn added bug Confirmed bug pending release Issue is resolved in the current codebase, will be published with the next release and removed pending release Issue is resolved in the current codebase, will be published with the next release labels Feb 17, 2020
@msawczyn
Copy link
Owner

Thanks for the nice words. Glad that this fun little project is helping!

Let me see if I understand what you're saying by rewording a bit, because I believe I do see the problem.

The context currently builds the model as

modelBuilder.Entity<global::Sandbox.Entity1>().ToTable("Entity1").HasKey(t => t.Id);
modelBuilder.Entity<global::Sandbox.Entity1>().Property(t => t.Id).IsRequired().ValueGeneratedOnAdd();
modelBuilder.Entity<global::Sandbox.Entity1>().HasOne(x => x.Entity2).WithMany().HasForeignKey("Entity2_Id");

modelBuilder.Entity<global::Sandbox.Entity2>().ToTable("Entity2").HasKey(t => t.Id);
modelBuilder.Entity<global::Sandbox.Entity2>().Property(t => t.Id).IsRequired().ValueGeneratedOnAdd();

but this isn't quite right ... that defines a 0..1-* relationship. It should rather be

modelBuilder.Entity<global::Sandbox.Entity1>().ToTable("Entity1").HasKey(t => t.Id);
modelBuilder.Entity<global::Sandbox.Entity1>().Property(t => t.Id).IsRequired().ValueGeneratedOnAdd();
modelBuilder.Entity<global::Sandbox.Entity1>().HasOne(x => x.Entity2).WithMany().HasForeignKey("Entity2_Id").IsRequired();

modelBuilder.Entity<global::Sandbox.Entity2>().ToTable("Entity2").HasKey(t => t.Id);
modelBuilder.Entity<global::Sandbox.Entity2>().Property(t => t.Id).IsRequired().ValueGeneratedOnAdd();

There really aren't any foreign keys in this mix since we're letting EF create shadow properties for those (as it should be! :-) ), but the cardinality of the navigation properties is clearly messed up. This has additional repercussions as, since the T4 doesn't see the navigation property as required, it's not creating proper constructors and we get two default constructors generated for Entity1. I can't imagine how this hasn't been caught previously, so I'm guessing this must be a recent regression. We'll need to get a patch in asap.

Thanks a million!

@msawczyn msawczyn added this to the 2.0.0.0 milestone Feb 17, 2020
@majoreffort
Copy link
Author

Exactly, that's what I meant.

Regarding the constructors, the generated code seems to be correct as far as I can tell: there is a protected default one for EF and the public one requiring an instance of Entity2, so it might not be messed up completely after all.

@msawczyn msawczyn added the pending release Issue is resolved in the current codebase, will be published with the next release label Feb 19, 2020
@msawczyn
Copy link
Owner

Fixed in 2.0RC2.

@msawczyn msawczyn modified the milestones: 2.0.0.0, 2.0.0 Mar 27, 2020
@msawczyn msawczyn added released Issue is resolved in a current release and removed pending release Issue is resolved in the current codebase, will be published with the next release labels Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Confirmed bug released Issue is resolved in a current release
Projects
None yet
Development

No branches or pull requests

2 participants