dnx ef dbcontext scaffold generates improper WithMany definitions on related model?

[NullVoxPopuli]

I generated my context and models from this command:

dnx ef dbcontext scaffold "Connection String" EntityFramework.MicrosoftSqlServer --outputDir DatabaseModels

Which generated these onModelCreating definitions for my Book Model.

entity.HasOne(d => d.UserId_CreatedByNavigation).WithMany(p => p.Book).HasForeignKey(d => d.UserId_CreatedBy).OnDelete(DeleteBehavior.Restrict);
entity.HasOne(d => d.UserId_ModifiedByNavigation).WithMany(p => p.BookNavigation).HasForeignKey(d => d.UserId_ModifiedBy).OnDelete(DeleteBehavior.Restrict);
entity.HasOne(d => d.UserId_OwnerNavigation).WithMany(p => p.Book1).HasForeignKey(d => d.UserId_Owner).OnDelete(DeleteBehavior.Restrict);

So, in the database, all 3 of those are configured the same,
on the Book table, the foreign keys, UserId_CreatedBy, UserId_ModifiedBy and UserId_Owner all point to the User table's UserId column.

I assume that the UserId_CreatedByNavigation, UserId_ModifiedByNavigation and UserId_OwnerNavigation are things used by entity framework?

So,

where does Book1, and BookNavigation come from? Why aren't all the WithMany lambdas the same?

[rowanmiller]

Why aren't all the WithMany lambdas the same?

Because they are three separate relationships, so they can't share a navigation property. They all point to the same primary key, but they must be represented by three different foreign key columns in the database.

I wonder if we could do something better with collection navigation property names by using the foreign key name - like we do for the reference navigation property (something like CreatedByBooks, ModifiedByBooks,OwnerBooks).

Out of interest, could you share the database schema for User and Book.

[NullVoxPopuli]

ah that makes sense, thanks!

but yeah, It would be super cool if the navigation property could be named after the foreign key somehow.

Here are the relevant portions of the schema:

   modelBuilder.Entity<User>(entity =>
   {
       entity.ToTable("User");
       ...
       // column definition stuff, no relationships
   });


   modelBuilder.Entity<Book>(entity =>
   {
       entity.ToTable("Book");
       ...
       // column definition stuff, other relationships omitted

       entity.HasOne(d => d.UserId_CreatedByNavigation).WithMany(p => p.Book1).HasForeignKey(d => d.UserId_CreatedBy).OnDelete(DeleteBehavior.Restrict);

       entity.HasOne(d => d.UserId_ModifiedByNavigation).WithMany(p => p.BookNavigation).HasForeignKey(d => d.UserId_ModifiedBy).OnDelete(DeleteBehavior.Restrict);

       entity.HasOne(d => d.UserId_OwnerNavigation).WithMany(p => p.Book).HasForeignKey(d => d.UserId_Owner).OnDelete(DeleteBehavior.Restrict);

   });

[rowanmiller]

@NullVoxPopuli any chance you could share the SQL definition for the tables. I can fairly confidently infer what it is based on the code that was generated... but it's always great to get actual real-world schemas to test against.

[rowanmiller]

Notes for triage: Given the following schema:

CREATE TABLE [dbo].[Book](
    [BookId] [int] IDENTITY(1,1) NOT NULL PRIMARY KEY,
    [Name] [nvarchar](max) NULL,
    [CreatedByUserId] [int] NULL,
    [ModifiedByUserId] [int] NULL,
    [OwnerUserId] [int] NULL)

GO

CREATE TABLE [dbo].[User](
    [UserId] [int] IDENTITY(1,1) NOT NULL PRIMARY KEY,
    [Name] [nvarchar](max) NULL)

GO

ALTER TABLE [dbo].[Book]  WITH CHECK ADD  CONSTRAINT [FK_Book_User_CreatedByUserId] FOREIGN KEY([CreatedByUserId])
REFERENCES [dbo].[User] ([UserId])
GO

ALTER TABLE [dbo].[Book]  WITH CHECK ADD  CONSTRAINT [FK_Book_User_ModifiedByUserId] FOREIGN KEY([ModifiedByUserId])
REFERENCES [dbo].[User] ([UserId])
GO

ALTER TABLE [dbo].[Book]  WITH CHECK ADD  CONSTRAINT [FK_Book_User_OwnerUserId] FOREIGN KEY([OwnerUserId])
REFERENCES [dbo].[User] ([UserId])
GO

The properties on the dependent class look good (I reordered the generated properties for the sake of readability):

public int? CreatedByUserId { get; set; }
public virtual User CreatedByUser { get; set; }

public int? ModifiedByUserId { get; set; }
public virtual User ModifiedByUser { get; set; }

public int? OwnerUserId { get; set; }
public virtual User OwnerUser { get; set; }

But properties on the principal are pretty ugly (no way to tell what lines up with what unless you look at the model configuration):

        public virtual ICollection<Book> Book { get; set; }
        public virtual ICollection<Book> BookNavigation { get; set; }
        public virtual ICollection<Book> Book1 { get; set; }

My proposal is that if there are multiple navigation to the same entity, then we use the FK column name in the nav prop name too (same as we do for the dependent end).

A simple rule that should give something fairly clear in most cases would be <dependent_type_name><dependent_nav_prop_name>. We already have some logic to tidy up the name of the dependent property (removing Id etc.). Obviously these probably aren't the names you are going to keep around (you'll want to pluralize etc.)... but it at least makes it clear how the nav props line up so that renaming them is easier.

        public virtual ICollection<Book> BookModifiedByUser { get; set; }
        public virtual ICollection<Book> BookCreatedByUser { get; set; }
        public virtual ICollection<Book> BookOwnerUser { get; set; }

[rowanmiller]

@lajones the work item is just to implement the change mentioned in the comment above

[lajones]

Note: Given the more usual set-up of only 1 FK between the 2 tables - Book and User where the FK column on Book is just called UserId - the above rules would result in a principal end property of the form:

public virtual ICollection<Book> BookUser { get; set; }

which is a bit weird. So we decided to only implement the above rules if there is more than 1 FK between the 2 tables.

[lajones]

Checked in with commit d6abe4a.

[NullVoxPopuli]

Is there a way I can test this out before waiting for a new release?

like, in a Gemfile, you'd specify:

gem 'fictional-entity-framework', github: 'aspnet/EntityFramework', branch: 'dev'

to use the latest commit on that branch

[rowanmiller]

@NullVoxPopuli we have "nightly" builds (http://myget.org/F/aspnetvnext)... but they are a mess at the moment due to everyone moving over to .NET CLI. So I would hold off for a while until trying to use them.