Skip to content

Commit

Permalink
Yet more fixes to table naming in hierarchies and DbSet naming (#260)
Browse files Browse the repository at this point in the history 
Fixes #259
  • Loading branch information
@roji
roji authored Jan 10, 2024
1 parent c885031 commit da2b75c
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 28 deletions.
39 changes: 39 additions & 0 deletions EFCore.NamingConventions.Test/IntegrationTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,28 @@ public void Table_name_is_taken_from_DbSet_property_with_TPH()
Assert.Equal("ix_blogs_special_blog_property", specialBlogIndex.GetDatabaseName());
}

[Fact]
public void Table_name_is_taken_from_DbSet_property_with_TPT()
{
using var context = new TptBlogContext();

var blogEntityType = context.Model.FindEntityType(typeof(Blog))!;
var specialBlogEntityType = context.Model.FindEntityType(typeof(SpecialBlog))!;

Assert.Equal("blogs", blogEntityType.GetTableName());
Assert.Equal("special_blogs", specialBlogEntityType.GetTableName());

var blogProperty = blogEntityType.FindProperty(nameof(Blog.BlogProperty))!;
Assert.Equal("blog_property", blogProperty.GetColumnName());
var specialBlogProperty = specialBlogEntityType.FindProperty(nameof(SpecialBlog.SpecialBlogProperty))!;
Assert.Equal("special_blog_property", specialBlogProperty.GetColumnName());

var blogIndex = Assert.Single(blogEntityType.GetIndexes());
Assert.Equal("ix_blogs_blog_property", blogIndex.GetDatabaseName());
var specialBlogIndex = Assert.Single(specialBlogEntityType.GetDeclaredIndexes());
Assert.Equal("ix_special_blogs_special_blog_property", specialBlogIndex.GetDatabaseName());
}

public class Blog
{
public int Id { get; set; }
Expand Down Expand Up @@ -84,6 +106,23 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
}
}

public class TptBlogContext : DbContext
{
public DbSet<Blog> Blogs { get; set; } = null!;
public DbSet<SpecialBlog> SpecialBlogs { get; set; } = null!;

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder.UseSqlServer("foo").UseSnakeCaseNamingConvention();

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Blog>().HasIndex(b => b.BlogProperty);
modelBuilder.Entity<SpecialBlog>().HasIndex(b => b.SpecialBlogProperty);

modelBuilder.Entity<Blog>().UseTptMappingStrategy();
}
}

#endregion Table_name_is_taken_from_DbSet_property

#region Multiple_DbContexts_with_external_di_does_not_throw
Expand Down
10 changes: 7 additions & 3 deletions EFCore.NamingConventions.Test/NameRewritingConventionTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,16 +705,20 @@ private IEntityType BuildEntityType(Action<ModelBuilder> builderAction, CultureI

private IModel BuildModel(Action<ModelBuilder> builderAction, CultureInfo? culture = null)
{
var conventionSet = SqlServerTestHelpers
var services = SqlServerTestHelpers
.Instance
.CreateContextServices()
.CreateContextServices();

var conventionSet = services
.GetRequiredService<IConventionSetBuilder>()
.CreateConventionSet();

var dependencies = services.GetRequiredService<ProviderConventionSetBuilderDependencies>();

var optionsBuilder = new DbContextOptionsBuilder();
SqlServerTestHelpers.Instance.UseProviderOptions(optionsBuilder);
optionsBuilder.UseSnakeCaseNamingConvention(culture);
var plugin = new NamingConventionSetPlugin(optionsBuilder.Options);
var plugin = new NamingConventionSetPlugin(dependencies, optionsBuilder.Options);
plugin.ModifyConventions(conventionSet);

var modelBuilder = new ModelBuilder(conventionSet);
Expand Down
9 changes: 6 additions & 3 deletions EFCore.NamingConventions.Test/NamingConventionSetPluginTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@ public class NamingConventionSetPluginTest
[Fact]
public void ModifyConventions_without_extensions_not_throw_exception()
{
var conventionSet = SqlServerTestHelpers
var services = SqlServerTestHelpers
.Instance
.CreateContextServices()
.CreateContextServices();

var conventionSet = services
.GetRequiredService<IConventionSetBuilder>()
.CreateConventionSet();

var dependencies = services.GetRequiredService<ProviderConventionSetBuilderDependencies>();
var optionsBuilder = new DbContextOptionsBuilder();
SqlServerTestHelpers.Instance.UseProviderOptions(optionsBuilder);
var plugin = new NamingConventionSetPlugin(optionsBuilder.Options);
var plugin = new NamingConventionSetPlugin(dependencies, optionsBuilder.Options);

var exception = Record.Exception(() => plugin.ModifyConventions(conventionSet));

Expand Down
92 changes: 72 additions & 20 deletions EFCore.NamingConventions/Internal/NameRewritingConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;

namespace EFCore.NamingConventions.Internal;

Expand All @@ -24,10 +25,38 @@ public class NameRewritingConvention :
private static readonly StoreObjectType[] _storeObjectTypes
= { StoreObjectType.Table, StoreObjectType.View, StoreObjectType.Function, StoreObjectType.SqlQuery };

private readonly IDictionary<Type, string> _sets;
private readonly INameRewriter _namingNameRewriter;

public NameRewritingConvention(INameRewriter nameRewriter)
=> _namingNameRewriter = nameRewriter;
public NameRewritingConvention(ProviderConventionSetBuilderDependencies dependencies, INameRewriter nameRewriter)
{
_namingNameRewriter = nameRewriter;

// Copied from TableNameFromDbSetConvention
_sets = new Dictionary<Type, string>();
List<Type>? ambiguousTypes = null;
foreach (var set in dependencies.SetFinder.FindSets(dependencies.ContextType))
{
if (!_sets.ContainsKey(set.Type))
{
_sets.Add(set.Type, set.Name);
}
else
{
ambiguousTypes ??= new List<Type>();

ambiguousTypes.Add(set.Type);
}
}

if (ambiguousTypes != null)
{
foreach (var type in ambiguousTypes)
{
_sets.Remove(type);
}
}
}

public virtual void ProcessEntityTypeAdded(
IConventionEntityTypeBuilder entityTypeBuilder,
Expand Down Expand Up @@ -84,21 +113,6 @@ private void ProcessHierarchyChange(IConventionEntityTypeBuilder entityTypeBuild

foreach (var entityType in entityTypeBuilder.Metadata.GetDerivedTypesInclusive())
{
// There's no need to rewrite the name on the hierarchy root, since a mapping strategy change doesn't affect it (unlike the
// name on children). Doing this would also reset the name initially set by TableNameFromDbSetConvention.
if (entityType.GetRootType() == entityType)
{
// The one exception is for an abstract root type in TPC, where we must remove any previous table name annotation
// (e.g. transition from TPH->TPC), since having one isn't valid (the type isn't mapped to any table at all).
if (newMappingStrategy == RelationalAnnotationNames.TpcMappingStrategy && entityType.ClrType.IsAbstract)
{
entityTypeBuilder.HasNoAnnotation(RelationalAnnotationNames.TableName);
entityTypeBuilder.HasNoAnnotation(RelationalAnnotationNames.Schema);
}

continue;
}

entityTypeBuilder = entityType.Builder;

// First, reset any rewritten name we previously set (e.g. when changing from TPH to TPT), and then rewrite the default name.
Expand All @@ -109,7 +123,7 @@ private void ProcessHierarchyChange(IConventionEntityTypeBuilder entityTypeBuild

if (!(newMappingStrategy == RelationalAnnotationNames.TpcMappingStrategy && entityType.ClrType.IsAbstract))
{
if (entityType.GetTableName() is { } tableName)
if (GetDefaultTableName(entityType) is { } tableName)
{
entityTypeBuilder.ToTable(_namingNameRewriter.RewriteName(tableName), entityType.GetSchema());
}
Expand All @@ -121,6 +135,11 @@ private void ProcessHierarchyChange(IConventionEntityTypeBuilder entityTypeBuild
}
}
}

string? GetDefaultTableName(IConventionEntityType entityType)
=> !entityType.HasSharedClrType && _sets.TryGetValue(entityType.ClrType, out var setName)
? setName
: entityType.GetTableName();
}

public virtual void ProcessPropertyAdded(
Expand Down Expand Up @@ -234,6 +253,8 @@ when annotation?.Value is not null
case RelationalAnnotationNames.TableName
when StoreObjectIdentifier.Create(entityType, StoreObjectType.Table) is StoreObjectIdentifier tableIdentifier:
{
var mappingStrategy = entityType.GetMappingStrategy();

if (entityType.FindPrimaryKey() is IConventionKey primaryKey)
{
// We need to rewrite the PK name.
Expand Down Expand Up @@ -269,15 +290,25 @@ when StoreObjectIdentifier.Create(entityType, StoreObjectType.Table) is StoreObj

foreach (var foreignKey in entityType.GetDeclaredForeignKeys())
{
if (foreignKey.GetDefaultName() is { } foreignKeyName)
// See note in ProcessHierarchyChange on indexes and foreign keys in TPC hierarchies
if (mappingStrategy == RelationalAnnotationNames.TpcMappingStrategy && entityType.GetDerivedTypes().Any())
{
foreignKey.Builder.HasNoAnnotation(RelationalAnnotationNames.Name);
}
else if (foreignKey.GetDefaultName() is { } foreignKeyName)
{
foreignKey.Builder.HasConstraintName(_namingNameRewriter.RewriteName(foreignKeyName));
}
}

foreach (var index in entityType.GetDeclaredIndexes())
{
if (index.GetDefaultDatabaseName() is { } indexName)
// See note in ProcessHierarchyChange on indexes and foreign keys in TPC hierarchies
if (mappingStrategy == RelationalAnnotationNames.TpcMappingStrategy && entityType.GetDerivedTypes().Any())
{
index.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
}
else if (index.GetDefaultDatabaseName() is { } indexName)
{
index.Builder.HasDatabaseName(_namingNameRewriter.RewriteName(indexName));
}
Expand Down Expand Up @@ -359,6 +390,27 @@ public void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, IConven
{
foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
{
// Copied from TableNameFromDbSetConvention
if (entityType.GetTableName() != null
&& _sets.ContainsKey(entityType.ClrType))
{
// if (entityType.GetViewNameConfigurationSource() != null)
// {
// // Undo the convention change if the entity type is mapped to a view
// entityType.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
// }

switch (entityType.GetMappingStrategy())
{
// Undo the convention change if the entity type is mapped using TPC
case RelationalAnnotationNames.TpcMappingStrategy when entityType.IsAbstract():
// Undo the convention change if the hierarchy ultimately ends up TPH
case RelationalAnnotationNames.TphMappingStrategy when entityType.BaseType is not null:
entityType.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
break;
}
}

foreach (var property in entityType.GetProperties())
{
var columnName = property.GetColumnName();
Expand Down
12 changes: 10 additions & 2 deletions EFCore.NamingConventions/Internal/NamingConventionSetPlugin.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,16 @@ namespace EFCore.NamingConventions.Internal;

public class NamingConventionSetPlugin : IConventionSetPlugin
{
private readonly ProviderConventionSetBuilderDependencies _dependencies;
private readonly IDbContextOptions _options;
public NamingConventionSetPlugin(IDbContextOptions options) => _options = options;

public NamingConventionSetPlugin(
ProviderConventionSetBuilderDependencies dependencies,
IDbContextOptions options)
{
_dependencies = dependencies;
_options = options;
}

public ConventionSet ModifyConventions(ConventionSet conventionSet)
{
Expand All @@ -23,7 +31,7 @@ public ConventionSet ModifyConventions(ConventionSet conventionSet)
return conventionSet;
}

var convention = new NameRewritingConvention(namingStyle switch
var convention = new NameRewritingConvention(_dependencies, namingStyle switch
{
NamingConvention.SnakeCase => new SnakeCaseNameRewriter(culture ?? CultureInfo.InvariantCulture),
NamingConvention.LowerCase => new LowerCaseNameRewriter(culture ?? CultureInfo.InvariantCulture),
Expand Down

0 comments on commit da2b75c

Please sign in to comment.