diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index f920f45b745..6a3d944df94 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -883,13 +883,26 @@ protected virtual void ValidateSharedForeignKeysCompatibility( StoreObjectIdentifier storeObject, [NotNull] IDiagnosticsLogger logger) { + if (storeObject.StoreObjectType != StoreObjectType.Table) + { + return; + } + var foreignKeyMappings = new Dictionary(); foreach (var foreignKey in mappedTypes.SelectMany(et => et.GetDeclaredForeignKeys())) { + var principalTable = foreignKey.PrincipalEntityType.GetTableName(); + var principalSchema = foreignKey.PrincipalEntityType.GetSchema(); + + if (principalTable == null) + { + continue; + } + var foreignKeyName = foreignKey.GetConstraintName( storeObject, - StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(), foreignKey.PrincipalEntityType.GetSchema())); + StoreObjectIdentifier.Table(principalTable, principalSchema)); if (!foreignKeyMappings.TryGetValue(foreignKeyName, out var duplicateForeignKey)) { foreignKeyMappings[foreignKeyName] = foreignKey; diff --git a/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs b/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs index ba79f94ef15..13bad0c05d9 100644 --- a/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs @@ -365,15 +365,17 @@ private void TryUniquifyForeignKeyNames( { foreach (var foreignKey in entityType.GetDeclaredForeignKeys()) { - if (foreignKey.DeclaringEntityType.GetTableName() == foreignKey.PrincipalEntityType.GetTableName() - && foreignKey.DeclaringEntityType.GetSchema() == foreignKey.PrincipalEntityType.GetSchema()) + var principalTable = foreignKey.PrincipalEntityType.GetTableName(); + var principalSchema = foreignKey.PrincipalEntityType.GetSchema(); + if (principalTable == null + || (foreignKey.DeclaringEntityType.GetTableName() == principalTable + && foreignKey.DeclaringEntityType.GetSchema() == principalSchema)) { continue; } var foreignKeyName = foreignKey.GetConstraintName(storeObject, - StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(), - foreignKey.PrincipalEntityType.GetSchema())); + StoreObjectIdentifier.Table(principalTable, principalSchema)); if (!foreignKeys.TryGetValue(foreignKeyName, out var otherForeignKey)) { foreignKeys[foreignKeyName] = foreignKey; diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index c7eb11c7363..2d454a0ea43 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -842,7 +842,7 @@ public virtual IEnumerable GetDependents( IUpdateEntry principalEntry, IForeignKey foreignKey) { var dependentIdentityMap = FindIdentityMap(foreignKey.DeclaringEntityType.FindPrimaryKey()); - return dependentIdentityMap != null + return dependentIdentityMap != null && foreignKey.PrincipalEntityType.IsAssignableFrom(principalEntry.EntityType) ? dependentIdentityMap.GetDependentsMap(foreignKey).GetDependents(principalEntry) : Enumerable.Empty(); } diff --git a/src/EFCore/Metadata/Internal/ClrCollectionAccessor.cs b/src/EFCore/Metadata/Internal/ClrCollectionAccessor.cs index 8ae71c560e6..c46e1236633 100644 --- a/src/EFCore/Metadata/Internal/ClrCollectionAccessor.cs +++ b/src/EFCore/Metadata/Internal/ClrCollectionAccessor.cs @@ -109,7 +109,6 @@ public virtual object GetOrCreate(object entity, bool forMaterialization) private ICollection GetOrCreateCollection(object instance, bool forMaterialization) { var collection = GetCollection(instance); - if (collection == null) { var setCollection = forMaterialization diff --git a/test/EFCore.Relational.Specification.Tests/TestUtilities/TestSqlLoggerFactory.cs b/test/EFCore.Relational.Specification.Tests/TestUtilities/TestSqlLoggerFactory.cs index b45ecec9ad1..a0d453e21be 100644 --- a/test/EFCore.Relational.Specification.Tests/TestUtilities/TestSqlLoggerFactory.cs +++ b/test/EFCore.Relational.Specification.Tests/TestUtilities/TestSqlLoggerFactory.cs @@ -35,7 +35,7 @@ public TestSqlLoggerFactory(Func shouldLogCategory) public IReadOnlyList Parameters => ((TestSqlLogger)Logger).Parameters; public string Sql => string.Join(_eol + _eol, SqlStatements); - public void AssertBaseline(string[] expected) + public void AssertBaseline(string[] expected, bool assertOrder = true) { if (_proceduralQueryGeneration) { @@ -44,12 +44,25 @@ public void AssertBaseline(string[] expected) try { - for (var i = 0; i < expected.Length; i++) + if (assertOrder) { - Assert.Equal(expected[i], SqlStatements[i], ignoreLineEndingDifferences: true); - } + for (var i = 0; i < expected.Length; i++) + { + Assert.Equal(expected[i], SqlStatements[i], ignoreLineEndingDifferences: true); + } - Assert.Empty(SqlStatements.Skip(expected.Length)); + Assert.Empty(SqlStatements.Skip(expected.Length)); + } + else + { + foreach (var expectedFragment in expected) + { + var normalizedExpectedFragment = expectedFragment.Replace("\r", string.Empty).Replace("\n", _eol); + Assert.Contains( + normalizedExpectedFragment, + SqlStatements); + } + } } catch { diff --git a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Category.cs b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Category.cs index a2e97376d18..efa79237c8d 100644 --- a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Category.cs +++ b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Category.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; + namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel { public class Category @@ -8,5 +10,6 @@ public class Category public int Id { get; set; } public int? PrincipalId { get; set; } public string Name { get; set; } + public ICollection ProductCategories { get; set; } } } diff --git a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Product.cs b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Product.cs index 553923df1c3..8e9402c9281 100644 --- a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Product.cs +++ b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Product.cs @@ -1,18 +1,19 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel { - public class Product + public class Product : ProductBase { - public Guid Id { get; set; } public int? DependentId { get; set; } public string Name { get; set; } [ConcurrencyCheck] public decimal Price { get; set; } + + public ICollection ProductCategories { get; set; } } } diff --git a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductBase.cs b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductBase.cs new file mode 100644 index 00000000000..53a4d2c1eb8 --- /dev/null +++ b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductBase.cs @@ -0,0 +1,12 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; + +namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel +{ + public abstract class ProductBase + { + public Guid Id { get; set; } + } +} diff --git a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductCategory.cs b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductCategory.cs new file mode 100644 index 00000000000..d626f979601 --- /dev/null +++ b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductCategory.cs @@ -0,0 +1,13 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; + +namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel +{ + public class ProductCategory + { + public int CategoryId { get; set; } + public Guid ProductId { get; set; } + } +} diff --git a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductWithBytes.cs b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductWithBytes.cs index c2f7398b7aa..8ca023e393b 100644 --- a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductWithBytes.cs +++ b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/ProductWithBytes.cs @@ -1,17 +1,18 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel { - public class ProductWithBytes + public class ProductWithBytes : ProductBase { - public Guid Id { get; set; } public string Name { get; set; } [ConcurrencyCheck] public byte[] Bytes { get; set; } + + public ICollection ProductCategories { get; set; } } } diff --git a/test/EFCore.Specification.Tests/UpdatesFixtureBase.cs b/test/EFCore.Specification.Tests/UpdatesFixtureBase.cs index 1442a3f31ed..b7e14e89e49 100644 --- a/test/EFCore.Specification.Tests/UpdatesFixtureBase.cs +++ b/test/EFCore.Specification.Tests/UpdatesFixtureBase.cs @@ -11,21 +11,24 @@ public abstract class UpdatesFixtureBase : SharedStoreFixtureBase().HasMany(e => e.ProductCategories).WithOne() + .HasForeignKey(e => e.ProductId); + modelBuilder.Entity().HasMany(e => e.ProductCategories).WithOne() + .HasForeignKey(e => e.ProductId); + + modelBuilder.Entity() + .HasKey(p => new { p.CategoryId, p.ProductId }); + modelBuilder.Entity().HasOne().WithMany() .HasForeignKey(e => e.DependentId) .HasPrincipalKey(e => e.PrincipalId); - modelBuilder.Entity() - .Property(e => e.Id) - .ValueGeneratedNever(); - modelBuilder.Entity() .Property(e => e.Id) .ValueGeneratedNever(); - modelBuilder.Entity() - .Property(e => e.Id) - .ValueGeneratedNever(); + modelBuilder.Entity().HasMany(e => e.ProductCategories).WithOne() + .HasForeignKey(e => e.CategoryId); modelBuilder.Entity() .Property(e => e.Id) diff --git a/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerFixture.cs b/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerFixture.cs index 136dce636b9..492c70cb39c 100644 --- a/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerFixture.cs +++ b/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerFixture.cs @@ -22,6 +22,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con { base.OnModelCreating(modelBuilder, context); + modelBuilder.Entity() + .Property(p => p.Id).HasDefaultValueSql("NEWID()"); + modelBuilder.Entity() .Property(p => p.Price).HasColumnType("decimal(18,2)"); diff --git a/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs index 1c491026abc..448c3c7314f 100644 --- a/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using System.Linq; using Microsoft.EntityFrameworkCore.TestModels.UpdatesModel; using Xunit; @@ -18,36 +19,77 @@ public UpdatesSqlServerTest(UpdatesSqlServerFixture fixture, ITestOutputHelper t Fixture.TestSqlLoggerFactory.Clear(); } - public override void Save_replaced_principal() + [ConditionalFact] + public virtual void Save_with_shared_foreign_key() { - base.Save_replaced_principal(); + ExecuteWithStrategyInTransaction( + context => + { + context.AddRange( + new ProductWithBytes + { + ProductCategories = new List + { + new ProductCategory { CategoryId = 77 } + } + }, + new Category { Id = 77, PrincipalId = 777 }); - AssertSql( - @"SELECT TOP(2) [c].[Id], [c].[Name], [c].[PrincipalId] -FROM [Categories] AS [c]", - // - @"@__category_PrincipalId_0='778' (Nullable = true) + context.SaveChanges(); + }, + context => + { + var product = context.Set() + .Include(p => ((ProductWithBytes)p).ProductCategories) + .Include(p => ((Product)p).ProductCategories) + .OfType() + .Single(); + var productCategory = product.ProductCategories.Single(); + Assert.Equal(productCategory.CategoryId, context.Set().Single().CategoryId); + Assert.Equal(productCategory.CategoryId, context.Set().Single(c => c.PrincipalId == 777).Id); + }); -SELECT [p].[Id], [p].[DependentId], [p].[Name], [p].[Price] -FROM [Products] AS [p] -WHERE [p].[DependentId] = @__category_PrincipalId_0", + AssertContainsSql( + @"@p0='77' +@p1=NULL (Size = 4000) +@p2='777' + +SET NOCOUNT ON; +INSERT INTO [Categories] ([Id], [Name], [PrincipalId]) +VALUES (@p0, @p1, @p2);", // + @"@p0='ProductWithBytes' (Nullable = false) (Size = 4000) +@p1=NULL (Size = 8000) (DbType = Binary) +@p2=NULL (Size = 4000) + +SET NOCOUNT ON; +DECLARE @inserted0 TABLE ([Id] uniqueidentifier, [_Position] [int]); +MERGE [ProductBase] USING ( +VALUES (@p0, @p1, @p2, 0)) AS i ([Discriminator], [Bytes], [ProductWithBytes_Name], _Position) ON 1=0 +WHEN NOT MATCHED THEN +INSERT ([Discriminator], [Bytes], [ProductWithBytes_Name]) +VALUES (i.[Discriminator], i.[Bytes], i.[ProductWithBytes_Name]) +OUTPUT INSERTED.[Id], i._Position +INTO @inserted0; + +SELECT [t].[Id] FROM [ProductBase] t +INNER JOIN @inserted0 i ON ([t].[Id] = [i].[Id]) +ORDER BY [i].[_Position];"); + + } + + public override void Save_replaced_principal() + { + base.Save_replaced_principal(); + + AssertContainsSql( @"@p1='78' @p0='New Category' (Size = 4000) SET NOCOUNT ON; UPDATE [Categories] SET [Name] = @p0 WHERE [Id] = @p1; -SELECT @@ROWCOUNT;", - // - @"SELECT TOP(2) [c].[Id], [c].[Name], [c].[PrincipalId] -FROM [Categories] AS [c]", - // - @"@__category_PrincipalId_0='778' (Nullable = true) - -SELECT [p].[Id], [p].[DependentId], [p].[Name], [p].[Price] -FROM [Products] AS [p] -WHERE [p].[DependentId] = @__category_PrincipalId_0"); +SELECT @@ROWCOUNT;"); } public override void Identifiers_are_generated_correctly() @@ -94,5 +136,8 @@ public override void Identifiers_are_generated_correctly() private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); + + protected void AssertContainsSql(params string[] expected) + => Fixture.TestSqlLoggerFactory.AssertBaseline(expected, assertOrder: false); } }