Skip to content

Commit

Permalink
Fix NullReferenceException when trying to seed a keyless entity
Browse files Browse the repository at this point in the history
Fixes #23030
  • Loading branch information
umitkavala committed Jan 11, 2021
1 parent 5c74e0d commit f97cf05
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 15 deletions.
4 changes: 4 additions & 0 deletions src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
8 changes: 8 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,9 @@
<data name="SeedDatumSignedNumericValue" xml:space="preserve">
<value>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.</value>
</data>
<data name="SeedKeylessEntity" xml:space="preserve">
<value>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.</value>
</data>
<data name="SelfReferencingNavigationWithInverseProperty" xml:space="preserve">
<value>The inverse for the navigation '{entityType}.{property}' cannot be the same navigation. Change the value in the [InverseProperty] attribute to a different navigation.</value>
</data>
Expand Down Expand Up @@ -1476,4 +1479,4 @@
<data name="WrongStateManager" xml:space="preserve">
<value>Cannot start tracking the entry for entity type '{entityType}' because it was created by a different StateManager instance.</value>
</data>
</root>
</root>
4 changes: 4 additions & 0 deletions test/EFCore.InMemory.FunctionalTests/SeedingInMemoryTest.cs
Original file line number Diff line number Diff line change
@@ -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);

Expand Down
46 changes: 46 additions & 0 deletions test/EFCore.Specification.Tests/SeedingTestBase.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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();
Expand All @@ -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<InvalidOperationException>(
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; }
Expand All @@ -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<KeylessSeed>()
.HasNoKey()
.HasData(
new KeylessSeed { Species = "Apple" },
new KeylessSeed { Species = "Orange" }
);
}

public class KeylessSeed
{
public string Species { get; set; }
}
}
}
10 changes: 3 additions & 7 deletions test/EFCore.SqlServer.FunctionalTests/SeedingSqlServerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
12 changes: 5 additions & 7 deletions test/EFCore.Sqlite.FunctionalTests/SeedingSqliteTest.cs
Original file line number Diff line number Diff line change
@@ -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
{
Expand Down
18 changes: 18 additions & 0 deletions test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<KeylessSeed>(
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
{
Expand Down
5 changes: 5 additions & 0 deletions test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ protected class Product
public virtual ICollection<Order> 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);

Expand Down

0 comments on commit f97cf05

Please sign in to comment.