From 4cb06461ddff9626269b1cb2dbd1a766f4a5fdd2 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Thu, 17 Dec 2020 15:25:00 +0200 Subject: [PATCH] Proper support for table splitting (#55) Fixes #54 --- Directory.Build.targets | 4 + .../EFCore.NamingConventions.Test.csproj | 5 + EFCore.NamingConventions.Test/EndToEndTest.cs | 103 ++++++++++++++++++ .../NameRewritingConventionTest.cs | 48 ++++++++ .../Internal/NameRewritingConvention.cs | 102 +++++++++++------ .../Internal/NamingConventionSetPlugin.cs | 1 + 6 files changed, 228 insertions(+), 35 deletions(-) create mode 100644 EFCore.NamingConventions.Test/EndToEndTest.cs diff --git a/Directory.Build.targets b/Directory.Build.targets index c318b6f..91250dc 100644 --- a/Directory.Build.targets +++ b/Directory.Build.targets @@ -9,6 +9,10 @@ + + + + diff --git a/EFCore.NamingConventions.Test/EFCore.NamingConventions.Test.csproj b/EFCore.NamingConventions.Test/EFCore.NamingConventions.Test.csproj index 77bf493..bbb0e1a 100644 --- a/EFCore.NamingConventions.Test/EFCore.NamingConventions.Test.csproj +++ b/EFCore.NamingConventions.Test/EFCore.NamingConventions.Test.csproj @@ -9,7 +9,12 @@ + + + + + diff --git a/EFCore.NamingConventions.Test/EndToEndTest.cs b/EFCore.NamingConventions.Test/EndToEndTest.cs new file mode 100644 index 0000000..506106b --- /dev/null +++ b/EFCore.NamingConventions.Test/EndToEndTest.cs @@ -0,0 +1,103 @@ +// 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.Linq; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata; +using Xunit; + +namespace EFCore.NamingConventions.Test +{ + public class EndToEndTest : IClassFixture + { + public EndToEndTest(EndToEndTestFixture fixture) + => Fixture = fixture; + + [Fact] + public void Table_splitting() + { + using var context = CreateContext(); + + var split1EntityType = context.Model.FindEntityType(typeof(Split1)); + var split2EntityType = context.Model.FindEntityType(typeof(Split2)); + + var table = StoreObjectIdentifier.Create(split1EntityType, StoreObjectType.Table)!.Value; + Assert.Equal(table, StoreObjectIdentifier.Create(split2EntityType, StoreObjectType.Table)); + + Assert.Equal("common", split1EntityType.FindProperty("Common").GetColumnName(table)); + Assert.Equal("split2_common", split2EntityType.FindProperty("Common").GetColumnName(table)); + + var split1 = context.Set().Include(s1 => s1.Split2).Single(); + Assert.Equal(100, split1.Common); + Assert.Equal(101, split1.Split2.Common); + } + + TestContext CreateContext() => Fixture.CreateContext(); + + readonly EndToEndTestFixture Fixture; + + public class TestContext : DbContext + { + public TestContext(SqliteConnection connection) + : base(new DbContextOptionsBuilder() + .UseSqlite(connection) + .UseSnakeCaseNamingConvention() + .Options) + { + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(e => + { + e.ToTable("split"); + e.HasOne(s1 => s1.Split2).WithOne(s2 => s2.Split1).HasForeignKey(s2 => s2.Id); + e.HasData(new Split1 { Id = 1, OneProp = 1, Common = 100 }); + }); + + modelBuilder.Entity(e => + { + e.ToTable("split"); + e.HasData(new Split2 { Id = 1, TwoProp = 2, Common = 101 }); + }); + } + } + + public class Split1 + { + public int Id { get; set; } + public int OneProp { get; set; } + public int Common { get; set; } + + public Split2 Split2 { get; set; } + } + + public class Split2 + { + public int Id { get; set; } + public int TwoProp { get; set; } + public int Common { get; set; } + + public Split1 Split1 { get; set; } + } + + public class EndToEndTestFixture : IDisposable + { + private readonly SqliteConnection _connection; + + public TestContext CreateContext() => new(_connection); + + public EndToEndTestFixture() + { + _connection = new SqliteConnection("Filename=:memory:"); + _connection.Open(); + using var context = new TestContext(_connection); + context.Database.EnsureCreated(); + } + + public void Dispose() => _connection.Dispose(); + } + } +} diff --git a/EFCore.NamingConventions.Test/NameRewritingConventionTest.cs b/EFCore.NamingConventions.Test/NameRewritingConventionTest.cs index ee1052e..a579228 100644 --- a/EFCore.NamingConventions.Test/NameRewritingConventionTest.cs +++ b/EFCore.NamingConventions.Test/NameRewritingConventionTest.cs @@ -133,6 +133,54 @@ namespace EFCore.NamingConventions.Test Assert.Equal("ix_simple_blog_indexed_property", entityType.GetIndexes().Single().GetDatabaseName()); } + [Fact] + public void Table_splitting() + { + var model = BuildModel(b => + { + b.Entity("One", e => + { + e.ToTable("table"); + e.Property("Id"); + e.Property("OneProp"); + e.Property("Common"); + + e.HasOne("Two").WithOne().HasForeignKey("Two", "Id"); + }); + + b.Entity("Two", e => + { + e.ToTable("table"); + e.Property("Id"); + e.Property("TwoProp"); + e.Property("Common"); + }); + }); + + var oneEntityType = model.FindEntityType("One"); + var twoEntityType = model.FindEntityType("Two"); + + var table = StoreObjectIdentifier.Create(oneEntityType, StoreObjectType.Table)!.Value; + Assert.Equal(table, StoreObjectIdentifier.Create(twoEntityType, StoreObjectType.Table)); + + Assert.Equal("table", oneEntityType.GetTableName()); + Assert.Equal("one_prop", oneEntityType.FindProperty("OneProp").GetColumnName(table)); + + Assert.Equal("table", twoEntityType.GetTableName()); + Assert.Equal("two_prop", twoEntityType.FindProperty("TwoProp").GetColumnName(table)); + + var foreignKey = twoEntityType.GetForeignKeys().Single(); + Assert.Same(oneEntityType.FindPrimaryKey(), foreignKey.PrincipalKey); + Assert.Same(twoEntityType.FindPrimaryKey().Properties.Single(), foreignKey.Properties.Single()); + Assert.Equal(oneEntityType.FindPrimaryKey().GetName(), twoEntityType.FindPrimaryKey().GetName()); + + Assert.Equal( + foreignKey.PrincipalKey.Properties.Single().GetColumnName(table), + foreignKey.Properties.Single().GetColumnName(table)); + + Assert.Empty(oneEntityType.GetForeignKeys()); + } + #region Owned entities [Fact] diff --git a/EFCore.NamingConventions/Internal/NameRewritingConvention.cs b/EFCore.NamingConventions/Internal/NameRewritingConvention.cs index f12ae70..6104e67 100644 --- a/EFCore.NamingConventions/Internal/NameRewritingConvention.cs +++ b/EFCore.NamingConventions/Internal/NameRewritingConvention.cs @@ -12,7 +12,7 @@ namespace EFCore.NamingConventions.Internal public class NameRewritingConvention : IEntityTypeAddedConvention, IEntityTypeAnnotationChangedConvention, IPropertyAddedConvention, IForeignKeyOwnershipChangedConvention, IKeyAddedConvention, IForeignKeyAddedConvention, - IIndexAddedConvention, IEntityTypeBaseTypeChangedConvention + IIndexAddedConvention, IEntityTypeBaseTypeChangedConvention, IModelFinalizingConvention { private static readonly StoreObjectType[] _storeObjectTypes = { StoreObjectType.Table, StoreObjectType.View, StoreObjectType.Function, StoreObjectType.SqlQuery}; @@ -70,7 +70,7 @@ namespace EFCore.NamingConventions.Internal var entityType = propertyBuilder.Metadata.DeclaringEntityType; var property = propertyBuilder.Metadata; - property.SetColumnName(_namingNameRewriter.RewriteName(property.GetColumnBaseName())); + propertyBuilder.HasColumnName(_namingNameRewriter.RewriteName(property.GetColumnBaseName())); foreach (var storeObjectType in _storeObjectTypes) { @@ -80,7 +80,7 @@ namespace EFCore.NamingConventions.Internal if (property.GetColumnNameConfigurationSource(identifier.Value) == ConfigurationSource.Convention) { - property.SetColumnName( + propertyBuilder.HasColumnName( _namingNameRewriter.RewriteName(property.GetColumnName(identifier.Value)), identifier.Value); } } @@ -124,41 +124,34 @@ namespace EFCore.NamingConventions.Internal } } } - - // Finally, we need to apply the entity type prefix to all the owned properties, since - // the convention that normally does this (SharedTableConvention) runs at model finalization - // time, and will not overwrite our own rewritten column names. - foreach (var property in ownedEntityType.GetProperties() - .Except(ownedEntityType.FindPrimaryKey().Properties) - .Where(p => p.Builder.CanSetColumnName(null))) - { - var columnName = property.GetColumnBaseName(); - var prefix = ownedEntityType.ShortName() + '_'; - - if (columnName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) - { - columnName = columnName[prefix.Length..]; - } - - var rewrittenPrefix = _namingNameRewriter.RewriteName(prefix); - if (!columnName.StartsWith(rewrittenPrefix, StringComparison.OrdinalIgnoreCase)) - { - columnName = rewrittenPrefix + columnName; - } - - // TODO: We should uniquify, but we don't know about all the entity types mapped - // to this table. SharedTableConvention does its thing during model finalization, - // so it has the full list of entities and can uniquify. - property.Builder.HasColumnName(columnName); - } } } public void ProcessEntityTypeAnnotationChanged(IConventionEntityTypeBuilder entityTypeBuilder, string name, IConventionAnnotation annotation, IConventionAnnotation oldAnnotation, IConventionContext context) { - if (name == RelationalAnnotationNames.TableName && - oldAnnotation?.Value is null && + if (name != RelationalAnnotationNames.TableName) + { + return; + } + + // The table's name is changing - rewrite keys, index names + if (entityTypeBuilder.Metadata.FindPrimaryKey() is IConventionKey primaryKey) + { + primaryKey.Builder.HasName(_namingNameRewriter.RewriteName(primaryKey.GetDefaultName())); + } + + foreach (var foreignKey in entityTypeBuilder.Metadata.GetForeignKeys()) + { + foreignKey.Builder.HasConstraintName(_namingNameRewriter.RewriteName(foreignKey.GetDefaultName())); + } + + foreach (var index in entityTypeBuilder.Metadata.GetIndexes()) + { + index.Builder.HasDatabaseName(_namingNameRewriter.RewriteName(index.GetDefaultDatabaseName())); + } + + if (oldAnnotation?.Value is null && annotation?.Value is not null && entityTypeBuilder.Metadata.FindOwnership() is IConventionForeignKey ownership && (string)annotation.Value != ownership.PrincipalEntityType.GetTableName()) @@ -183,14 +176,53 @@ namespace EFCore.NamingConventions.Internal } public void ProcessForeignKeyAdded(IConventionForeignKeyBuilder relationshipBuilder, IConventionContext context) - => relationshipBuilder.HasConstraintName(_namingNameRewriter.RewriteName(relationshipBuilder.Metadata.GetConstraintName())); + => relationshipBuilder.HasConstraintName(_namingNameRewriter.RewriteName(relationshipBuilder.Metadata.GetDefaultName())); public void ProcessKeyAdded(IConventionKeyBuilder keyBuilder, IConventionContext context) - => keyBuilder.HasName(_namingNameRewriter.RewriteName(keyBuilder.Metadata.GetName())); + => keyBuilder.HasName(_namingNameRewriter.RewriteName(keyBuilder.Metadata.GetDefaultName())); public void ProcessIndexAdded( IConventionIndexBuilder indexBuilder, IConventionContext context) - => indexBuilder.HasDatabaseName(_namingNameRewriter.RewriteName(indexBuilder.Metadata.GetDatabaseName())); + => indexBuilder.HasDatabaseName(_namingNameRewriter.RewriteName(indexBuilder.Metadata.GetDefaultDatabaseName())); + + /// + /// EF Core's runs at model finalization time, and adds entity type prefixes to + /// clashing columns. These prefixes also needs to be rewritten by us, so we run after that convention to do that. + /// + public void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, IConventionContext context) + { + foreach (var entityType in modelBuilder.Metadata.GetEntityTypes()) + { + foreach (var property in entityType.GetProperties()) + { + var columnName = property.GetColumnBaseName(); + if (columnName.StartsWith(entityType.ShortName() + '_', StringComparison.Ordinal)) + { + property.Builder.HasColumnName( + _namingNameRewriter.RewriteName(entityType.ShortName()) + + columnName.Substring(entityType.ShortName().Length)); + } + + foreach (var storeObjectType in _storeObjectTypes) + { + var identifier = StoreObjectIdentifier.Create(entityType, storeObjectType); + if (identifier is null) + continue; + + if (property.GetColumnNameConfigurationSource(identifier.Value) == ConfigurationSource.Convention) + { + columnName = property.GetColumnName(identifier.Value); + if (columnName.StartsWith(entityType.ShortName() + '_', StringComparison.Ordinal)) + { + property.Builder.HasColumnName( + _namingNameRewriter.RewriteName(entityType.ShortName()) + + columnName.Substring(entityType.ShortName().Length)); + } + } + } + } + } + } } } diff --git a/EFCore.NamingConventions/Internal/NamingConventionSetPlugin.cs b/EFCore.NamingConventions/Internal/NamingConventionSetPlugin.cs index 401fcb2..2b1c990 100644 --- a/EFCore.NamingConventions/Internal/NamingConventionSetPlugin.cs +++ b/EFCore.NamingConventions/Internal/NamingConventionSetPlugin.cs @@ -39,6 +39,7 @@ namespace EFCore.NamingConventions.Internal conventionSet.ForeignKeyAddedConventions.Add(convention); conventionSet.IndexAddedConventions.Add(convention); conventionSet.EntityTypeBaseTypeChangedConventions.Add(convention); + conventionSet.ModelFinalizingConventions.Add(convention); return conventionSet; }