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

Incorrect simple join table determination in reverse engineering leads to incorrect code generation #28905

Closed
akarboush opened this issue Aug 26, 2022 · 5 comments · Fixed by #32627
Assignees
Labels
area-scaffolding closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@akarboush
Copy link

The current simple join table determination in version 6 and 7 does not take into account the join table with referencing forgein key(s), resulting in incorrectly generated code.

6.*

internal static bool IsManyToManyJoinEntityType(IEntityType entityType)

7.*

public static bool IsSimpleManyToManyJoinEntityType(this IEntityType entityType)

Microsoft sql db script

CREATE TABLE Blogs (Id int IDENTITY CONSTRAINT [PK_Blogs] PRIMARY KEY,) 
go 

CREATE TABLE Posts (Id int IDENTITY CONSTRAINT [PK_Posts] PRIMARY KEY,) 
go 

CREATE TABLE BlogPosts (
  BlogId int NOT NULL CONSTRAINT [FK_BlogPosts_Blogs] REFERENCES Blogs ON DELETE CASCADE,
  PostId int NOT NULL CONSTRAINT [FK_BlogPosts_Posts] REFERENCES Posts ON DELETE CASCADE,
  CONSTRAINT [PK_BlogPosts ] PRIMARY KEY (BlogId, PostId)
)
go 

CREATE TABLE LinkToBlogPosts (
  LinkId1 int NOT NULL,
  LinkId2 int NOT NULL,
  CONSTRAINT [PK_LinkToBlogPosts] PRIMARY KEY (LinkId1, LinkId2),
  CONSTRAINT [FK_LinkToBlogPosts_BlogPosts] FOREIGN KEY (LinkId1, LinkId2) REFERENCES BlogPosts
) go

code generated

public partial class Blog
{
/* ctor */
    public int Id { get; set; }

    public virtual ICollection<Post> Posts { get; set; }
}

 public partial class Post
{
/* ctor */
    public int Id { get; set; }

    public virtual ICollection<Blog> Blogs { get; set; }
}


 public partial class BlogPost
{
    public int BlogId { get; set; }
    public int PostId { get; set; }

    public virtual LinkToBlogPost LinkToBlogPost { get; set; } = null!;
}

 public partial class LinkToBlogPost
  {
      public int LinkId1 { get; set; }
      public int LinkId2 { get; set; }
  
      public virtual BlogPost Link { get; set; } = null!;
  }



protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Blog>(entity =>
            {
                entity.HasMany(d => d.Posts)
                    .WithMany(p => p.Blogs)
                    .UsingEntity<Dictionary<string, object>>(
                        "BlogPost",
                        l => l.HasOne<Post>().WithMany().HasForeignKey("PostId").HasConstraintName("FK_BlogPosts_Posts"),
                        r => r.HasOne<Blog>().WithMany().HasForeignKey("BlogId").HasConstraintName("FK_BlogPosts_Blogs"),
                        j =>
                        {
                            j.HasKey("BlogId", "PostId").HasName("PK_BlogPosts ");

                            j.ToTable("BlogPosts");
                        });
            });

            modelBuilder.Entity<BlogPost>(entity =>
            {
                entity.HasKey(e => new { e.BlogId, e.PostId })
                    .HasName("PK_BlogPosts ");

// error here
                entity.HasOne() 
                    .WithMany()
                    .HasForeignKey(d => d.BlogId)
                    .HasConstraintName("FK_BlogPosts_Blogs");

// error here
                entity.HasOne() 
                    .WithMany()
                    .HasForeignKey(d => d.PostId)
                    .HasConstraintName("FK_BlogPosts_Posts");
            });

            modelBuilder.Entity<LinkToBlogPost>(entity =>
            {
                entity.HasKey(e => new { e.LinkId1, e.LinkId2 });

                entity.HasOne(d => d.Link)
                    .WithOne(p => p.LinkToBlogPost)
                    .HasForeignKey<LinkToBlogPost>(d => new { d.LinkId1, d.LinkId2 })
                    .OnDelete(DeleteBehavior.ClientSetNull)
                    .HasConstraintName("FK_LinkToBlogPosts_BlogPosts");
            });

            OnModelCreatingPartial(modelBuilder);
        }

As you can see

  • A skip navigation for Blog and Post is created despite the BlogPost entity.
  • The Blog and Post navigation properties are missing from the BlogPost entity and therefore the navigation expression in the Builder module for the BlogPost is also missing.

I would like to submit a PR fixing the problem as I have already experimented with it locally and fixed it adding a referencing foreign keys check.
Is there anything I should consider before submitting??

@smitpatel
Copy link
Member

Referencing FK check shouldn't required ideally. In the presence of FK targeting potential join table, it should create navigations in model which fails navigation check and shouldn't mark entity as join entity. Hence it may not be right fix. Need to investigate why navigations are not assigned for the FK.

@ajcvickers ajcvickers added this to the Backlog milestone Aug 31, 2022
@ajcvickers
Copy link
Member

Note from triage: it should be possible to work around this issue in EF7 by editing the T4 template used to generate the entity types.

@ajcvickers ajcvickers self-assigned this Dec 15, 2023
@ajcvickers ajcvickers modified the milestones: Backlog, 9.0.0 Dec 15, 2023
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 15, 2023
@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 16, 2023

Patch? At least EF Core 8?

@ajcvickers
Copy link
Member

@ErikEJ It's been this way since 6, right? If so, then it doesn't meet the bar to patch.

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 16, 2023

Fair enough. And Power Tools let's you work around it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-scaffolding closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants