NullReferenceException when trying to seed a keyless entity #23030

[umitkavala]

Fixes #23030
I'm not sure about tests. Is it enough or correct place? I've changed base class so implemented classes also should change.
Maybe we can create new test class?

[umitkavala]

SqlServer & SqlLite tests passed but InMemory couldn't because its depend on CreateFactory has key.
I assume this fix should apply for all DB types right?

private Func<IInMemoryTable> CreateTable([NotNull] IEntityType entityType, IInMemoryTable baseTable)
=> (Func<IInMemoryTable>)typeof(InMemoryTableFactory).GetTypeInfo()
	.GetDeclaredMethod(nameof(CreateFactory))
	.MakeGenericMethod(entityType.FindPrimaryKey().GetKeyType())
	.Invoke(null, new object[] { entityType, baseTable, _sensitiveLoggingEnabled, _nullabilityCheckEnabled });

[UsedImplicitly]
private static Func<IInMemoryTable> CreateFactory<TKey>(
IEntityType entityType,
IInMemoryTable baseTable,
bool sensitiveLoggingEnabled,
bool nullabilityCheckEnabled)
=> () => new InMemoryTable<TKey>(entityType, baseTable, sensitiveLoggingEnabled, nullabilityCheckEnabled);

[ajcvickers]

@umitkavala This does not look like the correct fix. The comment from Smit in #23030 says:

Keyless entities don't have keys so they cannot be seeded using state manager based seeding implementation.
Should throw a better validation or need different implementation for seeding.

This means the correct resolution for this bug is to throw an InvalidOperationException with a message indicating that seeding keyless entity types are not supported.

[umitkavala]

@ajcvickers Sure. I'll implement that way. I couldn't make sure that's why I've created as draft. And this rule apply for all right? SqlServer, InMemory, SqlLite. So in that case test I can add test case to the SeedingTestBase.cs

[ajcvickers]

@umitkavala Sounds good.

[umitkavala]

I did required changes.

[umitkavala]

Can I convert this draft to PR for final review?

[umitkavala]

Hi @AndriySvyryd, I think this PR is ready.

[AndriySvyryd]

Suggested change

	protected abstract KeylessSeedingContext CreateKeylessContextWithEmptyDatabase(string testId);
	protected virtual KeylessSeedingContext CreateKeylessContextWithEmptyDatabase()
	=> new KeylessSeedingContext(TestStore.AddProviderOptions(new DbContextOptionsBuilder()).Options)

[umitkavala]

KeylessSeedingContext won't be abstract anymore and we won't need KeylessSeedingContext implementations on other classes (KeylessSeedingSqlServerContext, KeylessSeedingSqliteContext, KeylessSeedingInMemoryContext)

public class KeylessSeedingContext : DbContext
        {
            public KeylessSeedingContext(DbContextOptions options)
                : base(options)
            {
            }

[AndriySvyryd]

Thanks for your contribution

[umitkavala]

Thanks.