Check name for null before attempting to rewrite

And annotate the plugin for nullability.

Fixes #179
This commit is contained in:
Shay Rojansky
2023-01-13 13:30:20 +01:00
parent c0bcefe529
commit 81d512004d
8 changed files with 134 additions and 81 deletions

View File

@@ -468,6 +468,24 @@ public class NameRewritingConventionTest
Assert.Null(entityType.GetTableName());
}
[Fact]
public void Foreign_key_without_name_because_over_view()
{
var model = BuildModel(b =>
{
b.Entity<Blog>();
b.Entity<Post>().ToView("posts_view");
b.Entity<Post>().HasOne(x => x.Blog).WithMany().HasForeignKey(x => x.BlogId);
});
var postEntityType = model.FindEntityType(typeof(Post))!;
Assert.Null(postEntityType.GetTableName());
Assert.Collection(
postEntityType.GetForeignKeys().Select(fk => fk.GetConstraintName()),
Assert.Null,
Assert.Null);
}
private IEntityType BuildEntityType(Action<ModelBuilder> builderAction, CultureInfo culture = null)
=> BuildModel(builderAction, culture).GetEntityTypes().Single();

View File

@@ -12,7 +12,7 @@ namespace Microsoft.EntityFrameworkCore;
internal static class Check
{
[ContractAnnotation("value:null => halt")]
public static T NotNull<T>([NoEnumeration] T value, [InvokerParameterName] [NotNull] string parameterName)
public static T NotNull<T>([NoEnumeration] T value, [InvokerParameterName] string parameterName)
{
if (ReferenceEquals(value, null))
{
@@ -27,8 +27,8 @@ internal static class Check
[ContractAnnotation("value:null => halt")]
public static T NotNull<T>(
[NoEnumeration] T value,
[InvokerParameterName] [NotNull] string parameterName,
[NotNull] string propertyName)
[InvokerParameterName] string parameterName,
string propertyName)
{
if (ReferenceEquals(value, null))
{
@@ -42,7 +42,7 @@ internal static class Check
}
[ContractAnnotation("value:null => halt")]
public static IReadOnlyList<T> NotEmpty<T>(IReadOnlyList<T> value, [InvokerParameterName] [NotNull] string parameterName)
public static IReadOnlyList<T> NotEmpty<T>(IReadOnlyList<T> value, [InvokerParameterName] string parameterName)
{
NotNull(value, parameterName);
@@ -57,32 +57,16 @@ internal static class Check
}
[ContractAnnotation("value:null => halt")]
public static string NotEmpty(string value, [InvokerParameterName] [NotNull] string parameterName)
public static string NotEmpty(string? value, [InvokerParameterName] string parameterName)
{
Exception e = null;
if (ReferenceEquals(value, null))
{
e = new ArgumentNullException(parameterName);
}
else if (value.Trim().Length == 0)
{
e = new ArgumentException(AbstractionsStrings.ArgumentIsEmpty(parameterName));
}
if (e != null)
if (value is null)
{
NotEmpty(parameterName, nameof(parameterName));
throw e;
throw new ArgumentNullException(parameterName);
}
return value;
}
public static string NullButNotEmpty([CanBeNull] string value, [InvokerParameterName] [NotNull] string parameterName)
{
if (!ReferenceEquals(value, null)
&& (value.Length == 0))
if (value.Trim().Length == 0)
{
NotEmpty(parameterName, nameof(parameterName));
@@ -92,7 +76,19 @@ internal static class Check
return value;
}
public static IReadOnlyList<T> HasNoNulls<T>(IReadOnlyList<T> value, [InvokerParameterName] [NotNull] string parameterName)
public static string? NullButNotEmpty(string? value, [InvokerParameterName] string parameterName)
{
if (value is not null && value.Length == 0)
{
NotEmpty(parameterName, nameof(parameterName));
throw new ArgumentException(AbstractionsStrings.ArgumentIsEmpty(parameterName));
}
return value;
}
public static IReadOnlyList<T> HasNoNulls<T>(IReadOnlyList<T> value, [InvokerParameterName] string parameterName)
where T : class
{
NotNull(value, parameterName);

View File

@@ -3,6 +3,7 @@
<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<VersionPrefix>7.0.2</VersionPrefix>
<Nullable>enable</Nullable>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<SignAssembly>true</SignAssembly>
<AssemblyOriginatorKeyFile>../EFCore.NamingConventions.snk</AssemblyOriginatorKeyFile>

View File

@@ -36,19 +36,23 @@ public class NameRewritingConvention :
// (see ProcessEntityTypeBaseTypeChanged). But we still have this check for safety.
if (entityType.BaseType is null)
{
entityTypeBuilder.ToTable(_namingNameRewriter.RewriteName(entityType.GetTableName()), entityType.GetSchema());
if (entityType.GetViewNameConfigurationSource() == ConfigurationSource.Convention)
if (entityType.GetTableName() is { } tableName)
{
entityTypeBuilder.ToView(_namingNameRewriter.RewriteName(entityType.GetViewName()), entityType.GetViewSchema());
entityTypeBuilder.ToTable(_namingNameRewriter.RewriteName(tableName), entityType.GetSchema());
}
if (entityType.GetViewNameConfigurationSource() == ConfigurationSource.Convention
&& entityType.GetViewName() is { } viewName)
{
entityTypeBuilder.ToView(_namingNameRewriter.RewriteName(viewName), entityType.GetViewSchema());
}
}
}
public void ProcessEntityTypeBaseTypeChanged(
IConventionEntityTypeBuilder entityTypeBuilder,
IConventionEntityType newBaseType,
IConventionEntityType oldBaseType,
IConventionEntityType? newBaseType,
IConventionEntityType? oldBaseType,
IConventionContext<IConventionEntityType> context)
{
var entityType = entityTypeBuilder.Metadata;
@@ -56,7 +60,10 @@ public class NameRewritingConvention :
if (newBaseType is null)
{
// The entity is getting removed from a hierarchy. Set the (rewritten) TableName.
entityTypeBuilder.ToTable(_namingNameRewriter.RewriteName(entityType.GetTableName()), entityType.GetSchema());
if (entityType.GetTableName() is { } tableName)
{
entityTypeBuilder.ToTable(_namingNameRewriter.RewriteName(tableName), entityType.GetSchema());
}
}
else
{
@@ -83,7 +90,7 @@ public class NameRewritingConvention :
// Unless it's a collection navigation, or the owned entity table name was explicitly set by the user, this triggers table
// splitting, which means we need to undo rewriting which we've done previously.
if (foreignKey.IsOwnership
&& !foreignKey.GetNavigation(false).IsCollection
&& !foreignKey.GetNavigation(false)!.IsCollection
&& ownedEntityType.GetTableNameConfigurationSource() != ConfigurationSource.Explicit)
{
// Reset the table name which we've set when the entity type was added.
@@ -106,8 +113,8 @@ public class NameRewritingConvention :
public void ProcessEntityTypeAnnotationChanged(
IConventionEntityTypeBuilder entityTypeBuilder,
string name,
IConventionAnnotation annotation,
IConventionAnnotation oldAnnotation,
IConventionAnnotation? annotation,
IConventionAnnotation? oldAnnotation,
IConventionContext<IConventionAnnotation> context)
{
var entityType = entityTypeBuilder.Metadata;
@@ -116,7 +123,7 @@ public class NameRewritingConvention :
// we're the one who set the table name back when the entity type was originally added. We now undo this as the entity type
// should only be mapped to the View/SqlQuery/Function.
if (name is RelationalAnnotationNames.ViewName or RelationalAnnotationNames.SqlQuery or RelationalAnnotationNames.FunctionName
&& annotation.Value is not null
&& annotation?.Value is not null
&& entityType.GetTableNameConfigurationSource() == ConfigurationSource.Convention)
{
entityType.SetTableName(null);
@@ -142,7 +149,10 @@ public class NameRewritingConvention :
if (entityType.FindRowInternalForeignKeys(tableIdentifier).FirstOrDefault() is null && !isTPT)
{
primaryKey.Builder.HasName(_namingNameRewriter.RewriteName(primaryKey.GetDefaultName()));
if (primaryKey.GetDefaultName() is { } primaryKeyName)
{
primaryKey.Builder.HasName(_namingNameRewriter.RewriteName(primaryKeyName));
}
}
else
{
@@ -162,12 +172,18 @@ public class NameRewritingConvention :
foreach (var foreignKey in entityType.GetForeignKeys())
{
foreignKey.Builder.HasConstraintName(_namingNameRewriter.RewriteName(foreignKey.GetDefaultName()));
if (foreignKey.GetDefaultName() is { } foreignKeyName)
{
foreignKey.Builder.HasConstraintName(_namingNameRewriter.RewriteName(foreignKeyName));
}
}
foreach (var index in entityType.GetIndexes())
{
index.Builder.HasDatabaseName(_namingNameRewriter.RewriteName(index.GetDefaultDatabaseName()));
if (index.GetDefaultDatabaseName() is { } indexName)
{
index.Builder.HasDatabaseName(_namingNameRewriter.RewriteName(indexName));
}
}
if (annotation?.Value is not null
@@ -178,7 +194,7 @@ public class NameRewritingConvention :
// When the entity became owned, we prefixed all of its properties - we must now undo that.
foreach (var property in entityType.GetProperties()
.Except(entityType.FindPrimaryKey().Properties)
.Except(entityType.FindPrimaryKey()?.Properties ?? Array.Empty<IConventionProperty>())
.Where(p => p.Builder.CanSetColumnName(null)))
{
RewriteColumnName(property.Builder);
@@ -186,9 +202,10 @@ public class NameRewritingConvention :
// We previously rewrote the owned entity's primary key name, when the owned entity was still in table splitting.
// Now that its getting its own table, rewrite the primary key constraint name again.
if (entityType.FindPrimaryKey() is IConventionKey key)
if (entityType.FindPrimaryKey() is IConventionKey key
&& key.GetDefaultName() is { } keyName)
{
key.Builder.HasName(_namingNameRewriter.RewriteName(key.GetDefaultName()));
key.Builder.HasName(_namingNameRewriter.RewriteName(keyName));
}
}
}
@@ -196,15 +213,30 @@ public class NameRewritingConvention :
public void ProcessForeignKeyAdded(
IConventionForeignKeyBuilder relationshipBuilder,
IConventionContext<IConventionForeignKeyBuilder> context)
=> relationshipBuilder.HasConstraintName(_namingNameRewriter.RewriteName(relationshipBuilder.Metadata.GetDefaultName()));
{
if (relationshipBuilder.Metadata.GetDefaultName() is { } constraintName)
{
relationshipBuilder.HasConstraintName(_namingNameRewriter.RewriteName(constraintName));
}
}
public void ProcessKeyAdded(IConventionKeyBuilder keyBuilder, IConventionContext<IConventionKeyBuilder> context)
=> keyBuilder.HasName(_namingNameRewriter.RewriteName(keyBuilder.Metadata.GetName()));
{
if (keyBuilder.Metadata.GetName() is { } keyName)
{
keyBuilder.HasName(_namingNameRewriter.RewriteName(keyName));
}
}
public void ProcessIndexAdded(
IConventionIndexBuilder indexBuilder,
IConventionContext<IConventionIndexBuilder> context)
=> indexBuilder.HasDatabaseName(_namingNameRewriter.RewriteName(indexBuilder.Metadata.GetDefaultDatabaseName()));
{
if (indexBuilder.Metadata.GetDefaultDatabaseName() is { } indexName)
{
indexBuilder.HasDatabaseName(_namingNameRewriter.RewriteName(indexName));
}
}
/// <summary>
/// EF Core's <see cref="SharedTableConvention" /> runs at model finalization time, and adds entity type prefixes to
@@ -269,7 +301,10 @@ public class NameRewritingConvention :
var baseColumnName = StoreObjectIdentifier.Create(property.DeclaringEntityType, StoreObjectType.Table) is { } tableIdentifier
? property.GetDefaultColumnName(tableIdentifier)
: property.GetDefaultColumnName();
propertyBuilder.HasColumnName(_namingNameRewriter.RewriteName(baseColumnName));
if (baseColumnName is not null)
{
propertyBuilder.HasColumnName(_namingNameRewriter.RewriteName(baseColumnName));
}
foreach (var storeObjectType in _storeObjectTypes)
{
@@ -279,10 +314,10 @@ public class NameRewritingConvention :
continue;
}
if (property.GetColumnNameConfigurationSource(identifier.Value) == ConfigurationSource.Convention)
if (property.GetColumnNameConfigurationSource(identifier.Value) == ConfigurationSource.Convention
&& property.GetColumnName(identifier.Value) is { } columnName)
{
propertyBuilder.HasColumnName(
_namingNameRewriter.RewriteName(property.GetColumnName(identifier.Value)), identifier.Value);
propertyBuilder.HasColumnName(_namingNameRewriter.RewriteName(columnName), identifier.Value);
}
}
}

View File

@@ -10,11 +10,11 @@ namespace EFCore.NamingConventions.Internal;
public class NamingConventionSetPlugin : IConventionSetPlugin
{
private readonly IDbContextOptions _options;
public NamingConventionSetPlugin([NotNull] IDbContextOptions options) => _options = options;
public NamingConventionSetPlugin(IDbContextOptions options) => _options = options;
public ConventionSet ModifyConventions(ConventionSet conventionSet)
{
var extension = _options.FindExtension<NamingConventionsOptionsExtension>();
var extension = _options.FindExtension<NamingConventionsOptionsExtension>()!;
var namingStyle = extension.NamingConvention;
var culture = extension.Culture;
if (namingStyle == NamingConvention.None)

View File

@@ -4,18 +4,17 @@ using System.Globalization;
using System.Text;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.Extensions.DependencyInjection;
using JetBrains.Annotations;
namespace EFCore.NamingConventions.Internal;
public class NamingConventionsOptionsExtension : IDbContextOptionsExtension
{
private DbContextOptionsExtensionInfo _info;
private DbContextOptionsExtensionInfo? _info;
private NamingConvention _namingConvention;
private CultureInfo _culture;
private CultureInfo? _culture;
public NamingConventionsOptionsExtension() {}
protected NamingConventionsOptionsExtension([NotNull] NamingConventionsOptionsExtension copyFrom)
protected NamingConventionsOptionsExtension(NamingConventionsOptionsExtension copyFrom)
{
_namingConvention = copyFrom._namingConvention;
_culture = copyFrom._culture;
@@ -26,7 +25,7 @@ public class NamingConventionsOptionsExtension : IDbContextOptionsExtension
protected virtual NamingConventionsOptionsExtension Clone() => new(this);
internal virtual NamingConvention NamingConvention => _namingConvention;
internal virtual CultureInfo Culture => _culture;
internal virtual CultureInfo? Culture => _culture;
public virtual NamingConventionsOptionsExtension WithoutNaming()
{
@@ -35,7 +34,7 @@ public class NamingConventionsOptionsExtension : IDbContextOptionsExtension
return clone;
}
public virtual NamingConventionsOptionsExtension WithSnakeCaseNamingConvention(CultureInfo culture = null)
public virtual NamingConventionsOptionsExtension WithSnakeCaseNamingConvention(CultureInfo? culture = null)
{
var clone = Clone();
clone._namingConvention = NamingConvention.SnakeCase;
@@ -43,7 +42,7 @@ public class NamingConventionsOptionsExtension : IDbContextOptionsExtension
return clone;
}
public virtual NamingConventionsOptionsExtension WithLowerCaseNamingConvention(CultureInfo culture = null)
public virtual NamingConventionsOptionsExtension WithLowerCaseNamingConvention(CultureInfo? culture = null)
{
var clone = Clone();
clone._namingConvention = NamingConvention.LowerCase;
@@ -51,7 +50,7 @@ public class NamingConventionsOptionsExtension : IDbContextOptionsExtension
return clone;
}
public virtual NamingConventionsOptionsExtension WithUpperCaseNamingConvention(CultureInfo culture = null)
public virtual NamingConventionsOptionsExtension WithUpperCaseNamingConvention(CultureInfo? culture = null)
{
var clone = Clone();
clone._namingConvention = NamingConvention.UpperCase;
@@ -59,7 +58,7 @@ public class NamingConventionsOptionsExtension : IDbContextOptionsExtension
return clone;
}
public virtual NamingConventionsOptionsExtension WithUpperSnakeCaseNamingConvention(CultureInfo culture = null)
public virtual NamingConventionsOptionsExtension WithUpperSnakeCaseNamingConvention(CultureInfo? culture = null)
{
var clone = Clone();
clone._namingConvention = NamingConvention.UpperSnakeCase;
@@ -67,7 +66,7 @@ public class NamingConventionsOptionsExtension : IDbContextOptionsExtension
return clone;
}
public virtual NamingConventionsOptionsExtension WithCamelCaseNamingConvention(CultureInfo culture = null)
public virtual NamingConventionsOptionsExtension WithCamelCaseNamingConvention(CultureInfo? culture = null)
{
var clone = Clone();
clone._namingConvention = NamingConvention.CamelCase;
@@ -82,7 +81,7 @@ public class NamingConventionsOptionsExtension : IDbContextOptionsExtension
private sealed class ExtensionInfo : DbContextOptionsExtensionInfo
{
private string _logFragment;
private string? _logFragment;
public ExtensionInfo(IDbContextOptionsExtension extension) : base(extension) {}

View File

@@ -13,11 +13,6 @@ public class SnakeCaseNameRewriter : INameRewriter
public virtual string RewriteName(string name)
{
if (string.IsNullOrEmpty(name))
{
return name;
}
var builder = new StringBuilder(name.Length + Math.Min(2, name.Length / 5));
var previousCategory = default(UnicodeCategory?);

View File

@@ -9,7 +9,8 @@ namespace Microsoft.EntityFrameworkCore;
public static class NamingConventionsExtensions
{
public static DbContextOptionsBuilder UseSnakeCaseNamingConvention(
[NotNull] this DbContextOptionsBuilder optionsBuilder , CultureInfo culture = null)
this DbContextOptionsBuilder optionsBuilder,
CultureInfo? culture = null)
{
Check.NotNull(optionsBuilder, nameof(optionsBuilder));
@@ -23,12 +24,13 @@ public static class NamingConventionsExtensions
}
public static DbContextOptionsBuilder<TContext> UseSnakeCaseNamingConvention<TContext>(
[NotNull] this DbContextOptionsBuilder<TContext> optionsBuilder , CultureInfo culture = null)
this DbContextOptionsBuilder<TContext> optionsBuilder , CultureInfo? culture = null)
where TContext : DbContext
=> (DbContextOptionsBuilder<TContext>)UseSnakeCaseNamingConvention((DbContextOptionsBuilder)optionsBuilder, culture);
public static DbContextOptionsBuilder UseLowerCaseNamingConvention(
[NotNull] this DbContextOptionsBuilder optionsBuilder, CultureInfo culture = null)
this DbContextOptionsBuilder optionsBuilder,
CultureInfo? culture = null)
{
Check.NotNull(optionsBuilder, nameof(optionsBuilder));
@@ -42,12 +44,14 @@ public static class NamingConventionsExtensions
}
public static DbContextOptionsBuilder<TContext> UseLowerCaseNamingConvention<TContext>(
[NotNull] this DbContextOptionsBuilder<TContext> optionsBuilder, CultureInfo culture = null)
this DbContextOptionsBuilder<TContext> optionsBuilder,
CultureInfo? culture = null)
where TContext : DbContext
=> (DbContextOptionsBuilder<TContext>)UseLowerCaseNamingConvention((DbContextOptionsBuilder)optionsBuilder ,culture);
public static DbContextOptionsBuilder UseUpperCaseNamingConvention(
[NotNull] this DbContextOptionsBuilder optionsBuilder, CultureInfo culture = null)
this DbContextOptionsBuilder optionsBuilder,
CultureInfo? culture = null)
{
Check.NotNull(optionsBuilder, nameof(optionsBuilder));
@@ -61,12 +65,14 @@ public static class NamingConventionsExtensions
}
public static DbContextOptionsBuilder<TContext> UseUpperCaseNamingConvention<TContext>(
[NotNull] this DbContextOptionsBuilder<TContext> optionsBuilder, CultureInfo culture = null)
this DbContextOptionsBuilder<TContext> optionsBuilder,
CultureInfo? culture = null)
where TContext : DbContext
=> (DbContextOptionsBuilder<TContext>)UseUpperCaseNamingConvention((DbContextOptionsBuilder)optionsBuilder, culture);
public static DbContextOptionsBuilder UseUpperSnakeCaseNamingConvention(
[NotNull] this DbContextOptionsBuilder optionsBuilder, CultureInfo culture = null)
this DbContextOptionsBuilder optionsBuilder,
CultureInfo? culture = null)
{
Check.NotNull(optionsBuilder, nameof(optionsBuilder));
@@ -80,12 +86,14 @@ public static class NamingConventionsExtensions
}
public static DbContextOptionsBuilder<TContext> UseUpperSnakeCaseNamingConvention<TContext>(
[NotNull] this DbContextOptionsBuilder<TContext> optionsBuilder, CultureInfo culture = null)
this DbContextOptionsBuilder<TContext> optionsBuilder,
CultureInfo? culture = null)
where TContext : DbContext
=> (DbContextOptionsBuilder<TContext>)UseUpperSnakeCaseNamingConvention((DbContextOptionsBuilder)optionsBuilder, culture);
public static DbContextOptionsBuilder UseCamelCaseNamingConvention(
[NotNull] this DbContextOptionsBuilder optionsBuilder, CultureInfo culture = null)
this DbContextOptionsBuilder optionsBuilder,
CultureInfo? culture = null)
{
Check.NotNull(optionsBuilder, nameof(optionsBuilder));
@@ -99,7 +107,8 @@ public static class NamingConventionsExtensions
}
public static DbContextOptionsBuilder<TContext> UseCamelCaseNamingConvention<TContext>(
[NotNull] this DbContextOptionsBuilder<TContext> optionsBuilder, CultureInfo culture = null)
this DbContextOptionsBuilder<TContext> optionsBuilder,
CultureInfo? culture = null)
where TContext : DbContext
=> (DbContextOptionsBuilder<TContext>)UseCamelCaseNamingConvention((DbContextOptionsBuilder)optionsBuilder, culture);
}