From 7767f3e1cab4c250b13315d50921c4c49d05f147 Mon Sep 17 00:00:00 2001 From: Brice Lambson Date: Wed, 4 Jan 2023 10:31:00 -0800 Subject: [PATCH] Scaffolding: Fix missing HasForeignKey when principal key is an alternate key (#29731) Also adds coverage for more scenarios involving alternate principal keys Fixes #29418 --- .../Extensions/ScaffoldingModelExtensions.cs | 5 + .../Internal/CSharpEntityTypeGeneratorTest.cs | 335 +++++++++++++++++- .../RelationalScaffoldingModelFactoryTest.cs | 80 +++++ 3 files changed, 419 insertions(+), 1 deletion(-) diff --git a/src/EFCore.Design/Extensions/ScaffoldingModelExtensions.cs b/src/EFCore.Design/Extensions/ScaffoldingModelExtensions.cs index 7d234d99369..bcf837a7b3f 100644 --- a/src/EFCore.Design/Extensions/ScaffoldingModelExtensions.cs +++ b/src/EFCore.Design/Extensions/ScaffoldingModelExtensions.cs @@ -720,6 +720,11 @@ public static IEnumerable GetDataAnnotations( var hasForeignKey = new FluentApiCodeFragment(nameof(ReferenceReferenceBuilder.HasForeignKey)) { IsHandledByDataAnnotations = true }; + if (!foreignKey.PrincipalKey.IsPrimaryKey()) + { + hasForeignKey.IsHandledByDataAnnotations = false; + } + if (foreignKey.IsUnique) { hasForeignKey.TypeArguments.Add(foreignKey.DeclaringEntityType.Name); diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs index e8be03c4d58..443a21368d7 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs @@ -1636,7 +1636,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) { entity.Property(e => e.Id).UseIdentityColumn(); - entity.HasOne(d => d.BlogNavigation).WithMany(p => p.Posts).HasPrincipalKey(p => new { p.Id1, p.Id2 }); + entity.HasOne(d => d.BlogNavigation).WithMany(p => p.Posts) + .HasPrincipalKey(p => new { p.Id1, p.Id2 }) + .HasForeignKey(d => new { d.BlogId1, d.BlogId2 }); }); OnModelCreatingPartial(modelBuilder); @@ -1656,6 +1658,137 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) Assert.Equal(new[] { "Id1", "Id2" }, blogNavigation.ForeignKey.PrincipalKey.Properties.Select(p => p.Name)); }); + [ConditionalFact] + public Task ForeignKeyAttribute_is_generated_for_fk_referencing_ak() + => TestAsync( + modelBuilder => modelBuilder + .Entity( + "Color", + x => + { + x.Property("Id"); + x.Property("ColorCode"); + }) + .Entity( + "Car", + x => + { + x.Property("Id"); + + x.HasOne("Color", "Color").WithMany("Cars") + .HasPrincipalKey("ColorCode") + .HasForeignKey("ColorCode"); + }), + new ModelCodeGenerationOptions + { + UseDataAnnotations = true, + UseNullableReferenceTypes = true + }, + code => + { + AssertFileContents( + @"using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace; + +public partial class Color +{ + [Key] + public int Id { get; set; } + + public string ColorCode { get; set; } = null!; + + public virtual ICollection Cars { get; } = new List(); +} +", + code.AdditionalFiles.Single(f => f.Path == "Color.cs")); + + AssertFileContents( + @"using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace; + +public partial class Car +{ + [Key] + public int Id { get; set; } + + public string? ColorCode { get; set; } + + public virtual Color? Color { get; set; } +} +", + code.AdditionalFiles.Single(f => f.Path == "Car.cs")); + + AssertFileContents( + @"using System; +using System.Collections.Generic; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace; + +public partial class TestDbContext : DbContext +{ + public TestDbContext() + { + } + + public TestDbContext(DbContextOptions options) + : base(options) + { + } + + public virtual DbSet Car { get; set; } + + public virtual DbSet Color { get; set; } + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) +#warning " + + DesignStrings.SensitiveInformationWarning + + @" + => optionsBuilder.UseSqlServer(""Initial Catalog=TestDatabase""); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.Property(e => e.Id).UseIdentityColumn(); + + entity.HasOne(d => d.Color).WithMany(p => p.Cars) + .HasPrincipalKey(p => p.ColorCode) + .HasForeignKey(d => d.ColorCode); + }); + + modelBuilder.Entity(entity => + { + entity.Property(e => e.Id).UseIdentityColumn(); + }); + + OnModelCreatingPartial(modelBuilder); + } + + partial void OnModelCreatingPartial(ModelBuilder modelBuilder); +} +", + code.ContextFile); + }, + model => + { + var carType = model.FindEntityType("TestNamespace.Car"); + var colorNavigation = carType.FindNavigation("Color"); + Assert.Equal("TestNamespace.Color", colorNavigation.ForeignKey.PrincipalEntityType.Name); + Assert.Equal(new[] { "ColorCode" }, colorNavigation.ForeignKey.Properties.Select(p => p.Name)); + Assert.Equal(new[] { "ColorCode" }, colorNavigation.ForeignKey.PrincipalKey.Properties.Select(p => p.Name)); + }); + [ConditionalFact] public Task InverseProperty_when_navigation_property_with_same_type_and_navigation_name() => TestAsync( @@ -1832,6 +1965,54 @@ public partial class Post Assert.Equal("OriginalPosts", originalInverseNavigation.Name); }); + [ConditionalFact] + public Task InverseProperty_when_navigation_property_and_keyless() + => TestAsync( + modelBuilder => modelBuilder + .Entity( + "Blog", + x => x.Property("Id")) + .Entity( + "Post", + x => + { + x.HasNoKey(); + x.HasOne("Blog", "Blog").WithMany(); + }), + new ModelCodeGenerationOptions { UseDataAnnotations = true }, + code => + { + AssertFileContents( + @"using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace; + +[Keyless] +public partial class Post +{ + public int? BlogId { get; set; } + + [ForeignKey(""BlogId"")] + public virtual Blog Blog { get; set; } +} +", + code.AdditionalFiles.Single(f => f.Path == "Post.cs")); + }, + model => + { + var postType = model.FindEntityType("TestNamespace.Post"); + var blogNavigation = postType.FindNavigation("Blog"); + + var foreignKeyProperty = Assert.Single(blogNavigation.ForeignKey.Properties); + Assert.Equal("BlogId", foreignKeyProperty.Name); + + Assert.Null(blogNavigation.Inverse); + }); + [ConditionalFact] public Task Entity_with_custom_annotation() => TestAsync( @@ -2386,6 +2567,158 @@ public partial class Post Assert.Equal(2, joinEntityType.GetForeignKeys().Count()); }); + [ConditionalFact] + public Task Scaffold_skip_navigations_alternate_key_data_annotations() + => TestAsync( + modelBuilder => modelBuilder + .Entity( + "Blog", + x => + { + x.Property("Id"); + x.Property("Key"); + }) + .Entity( + "Post", + x => x.Property("Id")) + .Entity("Blog").HasMany("Post", "Posts").WithMany("Blogs") + .UsingEntity( + "BlogPost", + r => r.HasOne("Post").WithMany(), + l => l.HasOne("Blog").WithMany().HasPrincipalKey("Key")), + new ModelCodeGenerationOptions { UseDataAnnotations = true }, + code => + { + AssertFileContents( + @"using System; +using System.Collections.Generic; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace; + +public partial class TestDbContext : DbContext +{ + public TestDbContext() + { + } + + public TestDbContext(DbContextOptions options) + : base(options) + { + } + + public virtual DbSet Blog { get; set; } + + public virtual DbSet Post { get; set; } + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) +#warning " + + DesignStrings.SensitiveInformationWarning + + @" + => optionsBuilder.UseSqlServer(""Initial Catalog=TestDatabase""); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.Property(e => e.Id).UseIdentityColumn(); + + entity.HasMany(d => d.Posts).WithMany(p => p.Blogs) + .UsingEntity>( + ""BlogPost"", + r => r.HasOne().WithMany().HasForeignKey(""PostsId""), + l => l.HasOne().WithMany() + .HasPrincipalKey(""Key"") + .HasForeignKey(""BlogsKey""), + j => + { + j.HasKey(""BlogsKey"", ""PostsId""); + j.HasIndex(new[] { ""PostsId"" }, ""IX_BlogPost_PostsId""); + }); + }); + + modelBuilder.Entity(entity => + { + entity.Property(e => e.Id).UseIdentityColumn(); + }); + + OnModelCreatingPartial(modelBuilder); + } + + partial void OnModelCreatingPartial(ModelBuilder modelBuilder); +} +", + code.ContextFile); + + AssertFileContents( + @"using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace; + +public partial class Blog +{ + [Key] + public int Id { get; set; } + + public int Key { get; set; } + + public virtual ICollection Posts { get; } = new List(); +} +", + code.AdditionalFiles.Single(e => e.Path == "Blog.cs")); + + AssertFileContents( + @"using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace; + +public partial class Post +{ + [Key] + public int Id { get; set; } + + [ForeignKey(""PostsId"")] + [InverseProperty(""Posts"")] + public virtual ICollection Blogs { get; } = new List(); +} +", + code.AdditionalFiles.Single(e => e.Path == "Post.cs")); + + Assert.Equal(2, code.AdditionalFiles.Count); + }, + model => + { + var blogType = model.FindEntityType("TestNamespace.Blog"); + Assert.Empty(blogType.GetNavigations()); + var postsNavigation = Assert.Single(blogType.GetSkipNavigations()); + Assert.Equal("Posts", postsNavigation.Name); + + var postType = model.FindEntityType("TestNamespace.Post"); + Assert.Empty(postType.GetNavigations()); + var blogsNavigation = Assert.Single(postType.GetSkipNavigations()); + Assert.Equal("Blogs", blogsNavigation.Name); + + Assert.Equal(postsNavigation, blogsNavigation.Inverse); + Assert.Equal(blogsNavigation, postsNavigation.Inverse); + + var joinEntityType = blogsNavigation.ForeignKey.DeclaringEntityType; + Assert.Equal("BlogPost", joinEntityType.Name); + Assert.Equal(typeof(Dictionary), joinEntityType.ClrType); + Assert.Single(joinEntityType.GetIndexes()); + Assert.Equal(2, joinEntityType.GetForeignKeys().Count()); + + var fk = Assert.Single(joinEntityType.FindDeclaredForeignKeys(new[] { joinEntityType.GetProperty("BlogsKey") })); + Assert.False(fk.PrincipalKey.IsPrimaryKey()); + }); + protected override void AddModelServices(IServiceCollection services) => services.Replace(ServiceDescriptor.Singleton()); diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs index abbd3bea032..b93a9b28625 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs @@ -2359,6 +2359,86 @@ public void Scaffold_skip_navigation_for_many_to_many_join_table_basic() }); } + [ConditionalFact] + public void Scaffold_skip_navigation_for_many_to_many_join_table_unique_constraint() + { + var database = new DatabaseModel + { + Tables = + { + new DatabaseTable + { + Name = "Blogs", + Columns = + { + new DatabaseColumn { Name = "Id", StoreType = "int" }, + new DatabaseColumn { Name = "Key", StoreType = "int" } + }, + PrimaryKey = new DatabasePrimaryKey { Columns = { new DatabaseColumnRef("Id") } }, + UniqueConstraints = { new DatabaseUniqueConstraint { Columns = { new DatabaseColumnRef("Key") } } } + }, + new DatabaseTable + { + Name = "Posts", + Columns = { new DatabaseColumn { Name = "Id", StoreType = "int" } }, + PrimaryKey = new DatabasePrimaryKey { Columns = { new DatabaseColumnRef("Id") } } + }, + new DatabaseTable + { + Name = "BlogPosts", + Columns = + { + new DatabaseColumn { Name = "BlogKey", StoreType = "int" }, + new DatabaseColumn { Name = "PostId", StoreType = "int" } + }, + PrimaryKey = + new DatabasePrimaryKey { Columns = { new DatabaseColumnRef("BlogKey"), new DatabaseColumnRef("PostId") } }, + ForeignKeys = + { + new DatabaseForeignKey + { + Columns = { new DatabaseColumnRef("BlogKey") }, + PrincipalColumns = { new DatabaseColumnRef("Key") }, + PrincipalTable = new DatabaseTableRef("Blogs"), + }, + new DatabaseForeignKey + { + Columns = { new DatabaseColumnRef("PostId") }, + PrincipalColumns = { new DatabaseColumnRef("Id") }, + PrincipalTable = new DatabaseTableRef("Posts"), + } + } + } + } + }; + + var model = _factory.Create(database, new ModelReverseEngineerOptions()); + + Assert.Collection( + model.GetEntityTypes().OrderBy(e => e.Name), + t1 => + { + Assert.Empty(t1.GetNavigations()); + var skipNavigation = Assert.Single(t1.GetSkipNavigations()); + Assert.Equal("Posts", skipNavigation.Name); + Assert.Equal("BlogKeys", skipNavigation.Inverse.Name); + }, + t2 => + { + Assert.Empty(t2.GetNavigations()); + Assert.Equal(2, t2.GetForeignKeys().Count()); + var fk = Assert.Single(t2.FindDeclaredForeignKeys(new[] { t2.GetProperty("BlogKey") })); + Assert.False(fk.PrincipalKey.IsPrimaryKey()); + }, + t3 => + { + Assert.Empty(t3.GetNavigations()); + var skipNavigation = Assert.Single(t3.GetSkipNavigations()); + Assert.Equal("BlogKeys", skipNavigation.Name); + Assert.Equal("Posts", skipNavigation.Inverse.Name); + }); + } + [ConditionalFact] public void Scaffold_skip_navigation_for_many_to_many_join_table_self_ref() {