From f97cf05ef198d596a721ee828dd418cd70c9cc7e Mon Sep 17 00:00:00 2001 From: Umit Kavala Date: Mon, 11 Jan 2021 22:52:04 +0100 Subject: [PATCH] Fix NullReferenceException when trying to seed a keyless entity Fixes #23030 --- src/EFCore/Infrastructure/ModelValidator.cs | 4 ++ src/EFCore/Properties/CoreStrings.Designer.cs | 8 ++++ src/EFCore/Properties/CoreStrings.resx | 5 +- .../SeedingInMemoryTest.cs | 4 ++ .../SeedingTestBase.cs | 46 +++++++++++++++++++ .../SeedingSqlServerTest.cs | 10 ++-- .../SeedingSqliteTest.cs | 12 ++--- .../Infrastructure/ModelValidatorTest.cs | 18 ++++++++ .../Infrastructure/ModelValidatorTestBase.cs | 5 ++ 9 files changed, 97 insertions(+), 15 deletions(-) diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs index 14f3d3d0a7f..63a5992b596 100644 --- a/src/EFCore/Infrastructure/ModelValidator.cs +++ b/src/EFCore/Infrastructure/ModelValidator.cs @@ -988,6 +988,10 @@ protected virtual void ValidateData([NotNull] IModel model, [NotNull] IDiagnosti var key = entityType.FindPrimaryKey(); if (key == null) { + if (entityType.GetSeedData().Any()) + { + throw new InvalidOperationException(CoreStrings.SeedKeylessEntity(entityType.DisplayName())); + } continue; } diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 332181285db..bcfdcf56ccf 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -2381,6 +2381,14 @@ public static string SeedDatumSignedNumericValue([CanBeNull] object? entityType, GetString("SeedDatumSignedNumericValue", nameof(entityType), nameof(property)), entityType, property); + /// + /// The seed entity for entity type '{entityType}' cannot be added because keyless entity types are not supported. Consider providing a key to seed data or use this entity for query only. + /// + public static string SeedKeylessEntity([CanBeNull] object? entityType) + => string.Format( + GetString("SeedKeylessEntity", nameof(entityType)), + entityType); + /// /// The inverse for the navigation '{entityType}.{property}' cannot be the same navigation. Change the value in the [InverseProperty] attribute to a different navigation. /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index a6eaf0a1b04..9c284ee7808 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1329,6 +1329,9 @@ The seed entity for entity type '{entityType}' cannot be added because a non-zero value is required for property '{property}'. Consider providing a negative value to avoid collisions with non-seed data. + + The seed entity for entity type '{entityType}' cannot be added because keyless entity types are not supported. Consider providing a key or removing the seed data. + The inverse for the navigation '{entityType}.{property}' cannot be the same navigation. Change the value in the [InverseProperty] attribute to a different navigation. @@ -1476,4 +1479,4 @@ Cannot start tracking the entry for entity type '{entityType}' because it was created by a different StateManager instance. - \ No newline at end of file + diff --git a/test/EFCore.InMemory.FunctionalTests/SeedingInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/SeedingInMemoryTest.cs index 8752ad7d0a7..f579f0a25a9 100644 --- a/test/EFCore.InMemory.FunctionalTests/SeedingInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/SeedingInMemoryTest.cs @@ -1,10 +1,14 @@ // 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 Microsoft.EntityFrameworkCore.TestUtilities; + namespace Microsoft.EntityFrameworkCore { public class SeedingInMemoryTest : SeedingTestBase { + protected override TestStore TestStore => InMemoryTestStore.Create("SeedingTest"); + protected override SeedingContext CreateContextWithEmptyDatabase(string testId) => new SeedingInMemoryContext(testId); diff --git a/test/EFCore.Specification.Tests/SeedingTestBase.cs b/test/EFCore.Specification.Tests/SeedingTestBase.cs index 4cdd898f353..ef9c7d05070 100644 --- a/test/EFCore.Specification.Tests/SeedingTestBase.cs +++ b/test/EFCore.Specification.Tests/SeedingTestBase.cs @@ -1,9 +1,11 @@ // 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.ComponentModel.DataAnnotations.Schema; using System.Linq; using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.TestUtilities; using Xunit; @@ -17,6 +19,7 @@ public abstract class SeedingTestBase public virtual async Task Seeding_does_not_leave_context_contaminated(bool async) { using var context = CreateContextWithEmptyDatabase(async ? "1A" : "1S"); + TestStore.Clean(context); var _ = async ? await context.Database.EnsureCreatedResilientlyAsync() : context.Database.EnsureCreatedResiliently(); @@ -31,8 +34,30 @@ public virtual async Task Seeding_does_not_leave_context_contaminated(bool async Assert.Equal("Orange", seeds[1].Species); } + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public virtual async Task Seeding_keyless_entity_throws_exception(bool async) + { + var exception = await Assert.ThrowsAsync( + async () => + { + using var context = CreateKeylessContextWithEmptyDatabase(); + TestStore.Clean(context); + var _ = async + ? await context.Database.EnsureCreatedResilientlyAsync() + : context.Database.EnsureCreatedResiliently(); + }); + Assert.Equal(CoreStrings.SeedKeylessEntity(nameof(KeylessSeed)), exception.Message); + } + + protected abstract TestStore TestStore { get; } + protected abstract SeedingContext CreateContextWithEmptyDatabase(string testId); + protected virtual KeylessSeedingContext CreateKeylessContextWithEmptyDatabase() + => new KeylessSeedingContext(TestStore.AddProviderOptions(new DbContextOptionsBuilder()).Options); + protected abstract class SeedingContext : DbContext { public string TestId { get; } @@ -54,5 +79,26 @@ protected class Seed public string Species { get; set; } } + + public class KeylessSeedingContext : DbContext + { + public KeylessSeedingContext(DbContextOptions options) + : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + => modelBuilder.Entity() + .HasNoKey() + .HasData( + new KeylessSeed { Species = "Apple" }, + new KeylessSeed { Species = "Orange" } + ); + } + + public class KeylessSeed + { + public string Species { get; set; } + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/SeedingSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/SeedingSqlServerTest.cs index cf0f2e3bf4a..10ed0b4a7c4 100644 --- a/test/EFCore.SqlServer.FunctionalTests/SeedingSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/SeedingSqlServerTest.cs @@ -7,14 +7,10 @@ namespace Microsoft.EntityFrameworkCore { public class SeedingSqlServerTest : SeedingTestBase { - protected override SeedingContext CreateContextWithEmptyDatabase(string testId) - { - var context = new SeedingSqlServerContext(testId); - - context.Database.EnsureClean(); + protected override TestStore TestStore => SqlServerTestStore.Create("SeedingTest"); - return context; - } + protected override SeedingContext CreateContextWithEmptyDatabase(string testId) + => new SeedingSqlServerContext(testId); protected class SeedingSqlServerContext : SeedingContext { diff --git a/test/EFCore.Sqlite.FunctionalTests/SeedingSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/SeedingSqliteTest.cs index 340fba102d5..5cbfa375618 100644 --- a/test/EFCore.Sqlite.FunctionalTests/SeedingSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/SeedingSqliteTest.cs @@ -1,18 +1,16 @@ // 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 Microsoft.EntityFrameworkCore.TestUtilities; + namespace Microsoft.EntityFrameworkCore { public class SeedingSqliteTest : SeedingTestBase { - protected override SeedingContext CreateContextWithEmptyDatabase(string testId) - { - var context = new SeedingSqliteContext(testId); + protected override TestStore TestStore => SqliteTestStore.Create("SeedingTest"); - context.Database.EnsureClean(); - - return context; - } + protected override SeedingContext CreateContextWithEmptyDatabase(string testId) + => new SeedingSqliteContext(testId); protected class SeedingSqliteContext : SeedingContext { diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs index 12d922e842a..acaf6e71aae 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs @@ -1322,6 +1322,24 @@ public virtual void Shared_type_inheritance_throws() VerifyError(CoreStrings.SharedTypeDerivedType("Shared2 (C)"), modelBuilder.Model); } + [ConditionalFact] + public virtual void Seeding_keyless_entity_throws() + { + var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity( + e => + { + e.HasNoKey(); + e.HasData( + new KeylessSeed + { + Species = "Apple" + }); + }); + + VerifyError(CoreStrings.SeedKeylessEntity(nameof(KeylessSeed)), modelBuilder.Model); + } + // INotify interfaces not really implemented; just marking the classes to test metadata construction private class FullNotificationEntity : INotifyPropertyChanging, INotifyPropertyChanged { diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs index 8b914cbe9b8..910572c2767 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs @@ -247,6 +247,11 @@ protected class Product public virtual ICollection Orders { get; set; } } + protected class KeylessSeed + { + public string Species { get; set; } + } + protected ModelValidatorTestBase() => LoggerFactory = new ListLoggerFactory(l => l == DbLoggerCategory.Model.Validation.Name || l == DbLoggerCategory.Model.Name);