Open cgountanis opened 5 years ago
To add more information, I feel this is a bug due to the way db-first scaffold names the property [ParentTableName]Navigation when it should name it [ChildTable]Navigation. If you have a composite FK from parent to child on say 2 fields why would it name the "Navigation" property prefixed with the parent table and not the object it is navigating to?
In this example Entity is the parent and the EntitySubType is the child with the composite FK, mainly for data integrity reasons, although I was not sold on this concept, came from the DBA that way.
There has to be a way it can use the child name + "Navigation", you know something that would make more sense from the API side when using the models logically. Right now it would seem every time I scaffold I have to rename a bunch of properties.
@cgountanis First, if you are using EF Core 2.0, then you should upgrade. That release has been out-of-support for some time now.
Following that, if you are still seeing issues, then please post the tables and the generated classes so we can fully investigate.
Sorry I should have been more clear. Easy to reproduce, make a parent table and a child table with composite key (2 columns PK on parent > child as FK), then do database-first scaffold.
Scaffold-DbContext "Server=localhost;Database=DBNAME;Trusted_Connection=True;" Microsoft.EntityFrameworkCore.SqlServer -OutputDir Models -Context Context -ContextDir Models -Force
The models when it creates the virtual property, the property name is backwards.
In Parent model: Named ParentTableNameNavigation when it should be ChildTableNameNavigation, to make any sense. Plus, if you ever had 2 children with comp from same parent, my guess is it would really get funky. Have not tested that yet.
Please post the CREATE TABLE statements and the generated model code (DbContext and POCO classes)
using System;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata;
namespace CaseManagerData.TestModels
{
public partial class TestContext : DbContext
{
public TestContext()
{
}
public TestContext(DbContextOptions<TestContext> options)
: base(options)
{
}
public virtual DbSet<Book> Book { get; set; }
public virtual DbSet<BookType> BookType { get; set; }
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
if (!optionsBuilder.IsConfigured)
{
#warning To protect potentially sensitive information in your connection string, you should move it out of source code. See http://go.microsoft.com/fwlink/?LinkId=723263 for guidance on storing connection strings.
optionsBuilder.UseSqlServer("Server=localhost;Database=Test;Trusted_Connection=True;");
}
}
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.HasAnnotation("ProductVersion", "2.2.6-servicing-10079");
modelBuilder.Entity<Book>(entity =>
{
entity.Property(e => e.BookId).ValueGeneratedNever();
entity.Property(e => e.Name).HasMaxLength(50);
entity.HasOne(d => d.BookNavigation)
.WithMany(p => p.Book)
.HasForeignKey(d => new { d.BookTypeId, d.BookSubTypeId })
.OnDelete(DeleteBehavior.ClientSetNull)
.HasConstraintName("FK_Book_BookType");
});
modelBuilder.Entity<BookType>(entity =>
{
entity.HasKey(e => new { e.BookTypeId, e.BookSubTypeId });
entity.Property(e => e.Name).HasMaxLength(50);
});
}
}
}
using System;
using System.Collections.Generic;
namespace CaseManagerData.TestModels
{
public partial class Book
{
public int BookId { get; set; }
public string Name { get; set; }
public int BookTypeId { get; set; }
public int BookSubTypeId { get; set; }
public virtual BookType BookNavigation { get; set; }
}
}
using System;
using System.Collections.Generic;
namespace CaseManagerData.TestModels
{
public partial class BookType
{
public BookType()
{
Book = new HashSet<Book>();
}
public int BookTypeId { get; set; }
public int BookSubTypeId { get; set; }
public string Name { get; set; }
public virtual ICollection<Book> Book { get; set; }
}
}
USE [Test]
GO
/****** Object: Table [dbo].[Book] Script Date: 7/25/2019 1:33:23 PM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[Book](
[BookId] [int] NOT NULL,
[Name] [nvarchar](50) NULL,
[BookTypeId] [int] NOT NULL,
[BookSubTypeId] [int] NOT NULL,
CONSTRAINT [PK_Book] PRIMARY KEY CLUSTERED
(
[BookId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
ALTER TABLE [dbo].[Book] WITH CHECK ADD CONSTRAINT [FK_Book_BookType] FOREIGN KEY([BookTypeId], [BookSubTypeId])
REFERENCES [dbo].[BookType] ([BookTypeId], [BookSubTypeId])
GO
ALTER TABLE [dbo].[Book] CHECK CONSTRAINT [FK_Book_BookType]
GO
USE [Test]
GO
/****** Object: Table [dbo].[BookType] Script Date: 7/25/2019 1:33:35 PM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[BookType](
[BookTypeId] [int] NOT NULL,
[BookSubTypeId] [int] NOT NULL,
[Name] [nvarchar](50) NULL,
CONSTRAINT [PK_BookType] PRIMARY KEY CLUSTERED
(
[BookTypeId] ASC,
[BookSubTypeId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
Notice the ParentTable "Book" virtual property named as BookNavigation even though it is of type BookType. The reason for this FK is to make sure the parent table (in this example Book) can not set TypeId/SubTypeId without it being in the BookType child table for obvious reasons.
I made sure I was 100% up-to-date on the latest stable version just to make sure I did my part here.
Microsoft Visual Studio Professional 2019 Version 16.2.0 .NET Core SDK 2.2.401 .NET Core Runtime 2.2.6 ASP.NET Core Runtime 2.2.6
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netcoreapp2.2</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="2.2.6" />
<PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="2.2.6" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="2.2.6">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>
</ItemGroup>
</Project>
Same results, though. Sorry I thought I was on the latest and greatest (stable).
For the case of composite key, the dependent to principal navigation name is generated using common prefix of foreign key properties. It is common practice to write the code that way. In your case it turns out to be Book. Since the class itself is named book, it added BookNavigation.
The FK is from Book > BookType, look at the virtual BookType BookNavigation
. The property should be named "what it is, no?" BookTypeNavigation. That would make more sense from the front-end auto-complete API side of things. Now what if you had two composite keys? You could not name them both BookNavigation. Sorry, respectfully, but that answer does not compute.
I believe an example would clarify.
public class Blog
{
public int Id1 { get; set; }
public int Id2 { get; set; }
}
public class Post
{
public int Id { get; set; }
public int BlogId1 { get; set; }
public int BlogId2 { get; set; }
public Blog Blog { get; set; }
}
Above is the general pattern we recognize. Of course people can write their code according to their preference. We follow heuristic to arrive at the name. You are free to change it to whatever you prefer.
To add, we have solved this by NOT using FK and just using CHECK CONTRAINTS within the DBMS so EF can stay happy. I posted this in an effort to shed some light on Composite Keys property naming conventions as it would seem from my examples you would not name the property the parent but the "TYPE" it is representing.
Triage: we will move this to the backlog to consider changing the default pattern.
/cc @ErikEJ in case he has thoughts.
I would appreciate it I think other people with composite keys will run into something like this at some point database first.
The point should not be to "change the default pattern".
The point should be to enable users to control the pattern.
I gave up on using EFCore and have been pounding the table at work for us to move to either EF6 or NHibernate. For any reasonably complicated database architecture, the pattern-based approach tightly couples you to a specific version of EFCore. It is not even guaranteed the patterns wont break from release to release, making you stuck without an upgrade path and piles of EFCore linq queries to port to fix the issue.
@jzabroski
That is a shame. We have been using EF Core for simple to complex designs and it overall has been working well. Very fast and it really simplifies the business data layer for us personally. As long as your DB design is solid, be it relational or complex relational (transaction design), it DOES work. Maybe revisit it in a few months after a couple 3.0 releases. Sounds like the 3.0 release will really help, including the SqlFrom for people that must use SPROCS, etc... Would be nice if it supported views but I would much rather use the relational tracking object model approach honestly. (KISS)
Let's be honest database-first was never a priority and I am sure it will come around. Composite keys mentioned my post are replaced with CHECK CONSTRAINTS because it was not really even needed for the parent > child connections, more of a constraint issue anyway for good data.
I have a database first solution core 2.2.4. Scaffolding does work and it creates a navigation property. The issue I'm having is how the scaffolding names that property. It causes parent table navigation when really it should call it the child table navigation. What if for some reason there was two composite keys. It wouldn't be able to use the same name.
Is there any way to customize the name does it use something from the foreign key description or anything where it would name the navigation property that it automatically creates a little more smartly?