Skip to content

Commit

Permalink
Fix for 21089. Allow multiple indexes on the same properties. (#21115)
Browse files Browse the repository at this point in the history 
Fix for 21089. Allow multiple indexes on the same properties.
  • Loading branch information
@lajones
lajones committed Jun 4, 2020
1 parent 30279c9 commit 546547d
Show file tree
Hide file tree
Showing 55 changed files with 1,067 additions and 618 deletions.
31 changes: 19 additions & 12 deletions src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -750,21 +750,26 @@ protected virtual void GenerateIndex(
stringBuilder
.AppendLine()
.Append(builderName)
.Append(".HasIndex(")
.Append(string.Join(", ", index.Properties.Select(p => Code.Literal(p.Name))))
.Append(")");
.Append(".HasIndex(");

using (stringBuilder.Indent())
if (index.Name == null)
{
if (index.Name != null)
{
stringBuilder
.AppendLine()
.Append(".HasName(")
.Append(Code.Literal(index.Name))
.Append(")");
}
stringBuilder
.Append(string.Join(", ", index.Properties.Select(p => Code.Literal(p.Name))));
}
else
{
stringBuilder
.Append("new[] { ")
.Append(string.Join(", ", index.Properties.Select(p => Code.Literal(p.Name))))
.Append(" }, ")
.Append(Code.Literal(index.Name));
}

stringBuilder.Append(")");

using (stringBuilder.Indent())
{
if (index.IsUnique)
{
stringBuilder
Expand Down Expand Up @@ -792,6 +797,8 @@ protected virtual void GenerateIndexAnnotations(
annotations,
CSharpModelGenerator.IgnoredIndexAnnotations);

GenerateFluentApiForAnnotation(
ref annotations, RelationalAnnotationNames.Name, nameof(RelationalIndexBuilderExtensions.HasDatabaseName), stringBuilder);
GenerateFluentApiForAnnotation(
ref annotations, RelationalAnnotationNames.Filter, nameof(RelationalIndexBuilderExtensions.HasFilter), stringBuilder);

Expand Down
15 changes: 6 additions & 9 deletions src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,22 +586,19 @@ private void GenerateTableName(IEntityType entityType)

private void GenerateIndex(IIndex index)
{
var lines = new List<string> { $".{nameof(EntityTypeBuilder.HasIndex)}({_code.Lambda(index.Properties)})" };

var annotations = index.GetAnnotations().ToList();

var lines = new List<string> {
$".{nameof(EntityTypeBuilder.HasIndex)}" +
$"({_code.Lambda(index.Properties)}, " +
$"{_code.Literal(index.GetDatabaseName())})" };
RemoveAnnotation(ref annotations, RelationalAnnotationNames.Name);

foreach (var annotation in CSharpModelGenerator.IgnoredIndexAnnotations)
{
RemoveAnnotation(ref annotations, annotation);
}

if (index.Name != null)
{
lines.Add(
$".{nameof(IndexBuilder.HasName)}" +
$"({_code.Literal(index.GetDatabaseName())})");
}

if (index.IsUnique)
{
lines.Add($".{nameof(IndexBuilder.IsUnique)}()");
Expand Down
31 changes: 12 additions & 19 deletions src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,14 +617,7 @@ protected virtual IndexBuilder VisitUniqueConstraint(
}

var propertyNames = uniqueConstraint.Columns.Select(GetPropertyName).ToArray();
var indexBuilder = builder.HasIndex(propertyNames).IsUnique();

if (!string.IsNullOrEmpty(uniqueConstraint.Name)
&& uniqueConstraint.Name != indexBuilder.Metadata.GetDefaultDatabaseName())
{
indexBuilder.HasName(uniqueConstraint.Name);
}

var indexBuilder = builder.HasIndex(propertyNames, uniqueConstraint.Name).IsUnique();
indexBuilder.Metadata.AddAnnotations(uniqueConstraint.GetAnnotations());

return indexBuilder;
Expand Down Expand Up @@ -674,20 +667,17 @@ protected virtual IndexBuilder VisitIndex([NotNull] EntityTypeBuilder builder, [
}

var propertyNames = index.Columns.Select(GetPropertyName).ToArray();
var indexBuilder = builder.HasIndex(propertyNames)
var indexBuilder =
index.Name == null
? builder.HasIndex(propertyNames)
: builder.HasIndex(propertyNames, index.Name)
.IsUnique(index.IsUnique);

if (index.Filter != null)
{
indexBuilder.HasFilter(index.Filter);
}

if (!string.IsNullOrEmpty(index.Name)
&& index.Name != indexBuilder.Metadata.GetDefaultDatabaseName())
{
indexBuilder.HasName(index.Name);
}

indexBuilder.Metadata.AddAnnotations(index.GetAnnotations());

return indexBuilder;
Expand Down Expand Up @@ -803,8 +793,10 @@ protected virtual IMutableForeignKey VisitForeignKey([NotNull] ModelBuilder mode
var principalKey = principalEntityType.FindKey(principalProperties);
if (principalKey == null)
{
var index = principalEntityType.FindIndex(principalProperties.AsReadOnly());
if (index?.IsUnique == true)
var index = principalEntityType.GetIndexes()
.Where(i => i.Properties.SequenceEqual(principalProperties) && i.IsUnique)
.FirstOrDefault();
if (index != null)
{
// ensure all principal properties are non-nullable even if the columns
// are nullable on the database. EF's concept of a key requires this.
Expand Down Expand Up @@ -844,9 +836,10 @@ protected virtual IMutableForeignKey VisitForeignKey([NotNull] ModelBuilder mode
dependentProperties, principalKey, principalEntityType);

var dependentKey = dependentEntityType.FindKey(dependentProperties);
var dependentIndex = dependentEntityType.FindIndex(dependentProperties);
var dependentIndexes = dependentEntityType.GetIndexes()
.Where(i => i.Properties.SequenceEqual(dependentProperties));
newForeignKey.IsUnique = dependentKey != null
|| dependentIndex?.IsUnique == true;
|| dependentIndexes.Any(i => i.IsUnique);

if (!string.IsNullOrEmpty(foreignKey.Name)
&& foreignKey.Name != newForeignKey.GetDefaultName())
Expand Down
74 changes: 67 additions & 7 deletions src/EFCore.Relational/Extensions/RelationalIndexBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,36 @@ public static class RelationalIndexBuilderExtensions
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <returns> A builder to further configure the index. </returns>
[Obsolete("Use IndexBuilder.HasName() instead.")]
public static IndexBuilder HasDatabaseName([NotNull] this IndexBuilder indexBuilder, [CanBeNull] string name)
{
indexBuilder.Metadata.SetDatabaseName(name);

return indexBuilder;
}

/// <summary>
/// Configures the name of the index in the database when targeting a relational database.
/// </summary>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <returns> A builder to further configure the index. </returns>
[Obsolete("Use HasDatabaseName() instead.")]
public static IndexBuilder HasName([NotNull] this IndexBuilder indexBuilder, [CanBeNull] string name)
=> indexBuilder.HasName(name);
=> HasDatabaseName(indexBuilder, name);

/// <summary>
/// Configures the name of the index in the database when targeting a relational database.
/// </summary>
/// <typeparam name="TEntity"> The entity type being configured. </typeparam>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <returns> A builder to further configure the index. </returns>
public static IndexBuilder<TEntity> HasDatabaseName<TEntity>([NotNull] this IndexBuilder<TEntity> indexBuilder, [CanBeNull] string name)
{
indexBuilder.Metadata.SetDatabaseName(name);

return indexBuilder;
}

/// <summary>
/// Configures the name of the index in the database when targeting a relational database.
Expand All @@ -32,7 +59,7 @@ public static IndexBuilder HasName([NotNull] this IndexBuilder indexBuilder, [Ca
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <returns> A builder to further configure the index. </returns>
[Obsolete("Use IndexBuilder<TEntity>.HasName() instead.")]
[Obsolete("Use HasDatabaseName() instead.")]
public static IndexBuilder<TEntity> HasName<TEntity>([NotNull] this IndexBuilder<TEntity> indexBuilder, [CanBeNull] string name)
=> indexBuilder.HasName(name);

Expand All @@ -46,10 +73,43 @@ public static IndexBuilder<TEntity> HasName<TEntity>([NotNull] this IndexBuilder
/// The same builder instance if the configuration was applied,
/// <see langword="null" /> otherwise.
/// </returns>
[Obsolete("Use IConventionIndexBuilder.HasName() instead.")]
public static IConventionIndexBuilder HasDatabaseName(
[NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false)
{
if (indexBuilder.CanSetDatabaseName(name, fromDataAnnotation))
{
indexBuilder.Metadata.SetDatabaseName(name, fromDataAnnotation);
return indexBuilder;
}

return null;
}

/// <summary>
/// Configures the name of the index in the database when targeting a relational database.
/// </summary>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns>
/// The same builder instance if the configuration was applied,
/// <see langword="null" /> otherwise.
/// </returns>
[Obsolete("Use HasDatabaseName() instead.")]
public static IConventionIndexBuilder HasName(
[NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false)
=> indexBuilder.HasName(name, fromDataAnnotation);
=> indexBuilder.HasDatabaseName(name, fromDataAnnotation);

/// <summary>
/// Returns a value indicating whether the given name can be set for the index.
/// </summary>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> <see langword="true" /> if the given name can be set for the index. </returns>
public static bool CanSetDatabaseName(
[NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false)
=> indexBuilder.CanSetAnnotation(RelationalAnnotationNames.Name, name, fromDataAnnotation);

/// <summary>
/// Returns a value indicating whether the given name can be set for the index.
Expand All @@ -58,10 +118,10 @@ public static IConventionIndexBuilder HasName(
/// <param name="name"> The name of the index. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> <see langword="true" /> if the given name can be set for the index. </returns>
[Obsolete("Use IConventionIndexBuilder.CanSetName() instead.")]
[Obsolete("Use CanSetDatabaseName() instead.")]
public static bool CanSetName(
[NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false)
=> indexBuilder.CanSetName(name, fromDataAnnotation);
=> CanSetDatabaseName(indexBuilder, name, fromDataAnnotation);

/// <summary>
/// Configures the filter expression for the index.
Expand Down
77 changes: 59 additions & 18 deletions src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,38 @@ namespace Microsoft.EntityFrameworkCore
public static class RelationalIndexExtensions
{
/// <summary>
/// Returns the name for this index.
/// Returns the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <returns> The name for this index. </returns>
/// <returns> The name of the index on the database. </returns>
public static string GetDatabaseName([NotNull] this IIndex index)
=> index.Name ?? index.GetDefaultDatabaseName();
=> (string)index[RelationalAnnotationNames.Name]
?? index.Name
?? index.GetDefaultDatabaseName();

/// <summary>
/// Returns the name for this index.
/// Returns the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <returns> The name for this index. </returns>
/// <returns> The name of the index on the database. </returns>
[Obsolete("Use GetDatabaseName() instead")]
public static string GetName([NotNull] this IIndex index)
=> GetDatabaseName(index);

/// <summary>
/// Returns the name for this index.
/// Returns the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="tableName"> The table name. </param>
/// <param name="schema"> The schema. </param>
/// <returns> The name for this index. </returns>
/// <returns> The name of the index on the database. </returns>
public static string GetDatabaseName(
[NotNull] this IIndex index,
[NotNull] string tableName,
[CanBeNull] string schema)
=> index.Name ?? index.GetDefaultDatabaseName(tableName, schema);
=> (string)index[RelationalAnnotationNames.Name]
?? index.Name
?? index.GetDefaultDatabaseName(tableName, schema);

/// <summary>
/// Returns the default name that would be used for this index.
Expand Down Expand Up @@ -123,33 +127,70 @@ public static string GetDefaultDatabaseName(
}

/// <summary>
/// Sets the index name.
/// Sets the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="name"> The value to set. </param>
[Obsolete("Use IMutableIndex.Name instead.")]
public static void SetDatabaseName([NotNull] this IMutableIndex index, [CanBeNull] string name)
{
index.SetOrRemoveAnnotation(
RelationalAnnotationNames.Name,
Check.NullButNotEmpty(name, nameof(name)));
}

/// <summary>
/// Sets the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="name"> The value to set. </param>
[Obsolete("Use SetDatabaseName() instead.")]
public static void SetName([NotNull] this IMutableIndex index, [CanBeNull] string name)
=> index.Name = Check.NullButNotEmpty(name, nameof(name));
=> SetDatabaseName(index, name);

/// <summary>
/// Sets the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="name"> The value to set. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> The configured value. </returns>
public static string SetDatabaseName([NotNull] this IConventionIndex index, [CanBeNull] string name, bool fromDataAnnotation = false)
{
index.SetOrRemoveAnnotation(
RelationalAnnotationNames.Name,
Check.NullButNotEmpty(name, nameof(name)),
fromDataAnnotation);

return name;
}

/// <summary>
/// Sets the index name.
/// Sets the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="name"> The value to set. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> The configured value. </returns>
[Obsolete("Use IConventionIndex.SetName() instead.")]
[Obsolete("Use SetDatabaseName() instead.")]
public static string SetName([NotNull] this IConventionIndex index, [CanBeNull] string name, bool fromDataAnnotation = false)
=> index.SetName(Check.NullButNotEmpty(name, nameof(name)), fromDataAnnotation);
=> SetDatabaseName(index, name, fromDataAnnotation);

/// <summary>
/// Gets the <see cref="ConfigurationSource" /> for the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <returns> The <see cref="ConfigurationSource" /> for the name of the index on the database. </returns>
public static ConfigurationSource? GetDatabaseNameConfigurationSource([NotNull] this IConventionIndex index)
=> index.FindAnnotation(RelationalAnnotationNames.Name)?.GetConfigurationSource();

/// <summary>
/// Gets the <see cref="ConfigurationSource" /> for the index name.
/// Gets the <see cref="ConfigurationSource" /> for the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <returns> The <see cref="ConfigurationSource" /> for the index name. </returns>
[Obsolete("Use IConventionIndex.GetNameConfigurationSource() instead.")]
/// <returns> The <see cref="ConfigurationSource" /> for the name of the index on the database. </returns>
[Obsolete("Use GetDatabaseNameConfigurationSource() instead.")]
public static ConfigurationSource? GetNameConfigurationSource([NotNull] this IConventionIndex index)
=> index.GetNameConfigurationSource();
=> GetDatabaseNameConfigurationSource(index);

/// <summary>
/// Returns the index filter expression.
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,10 @@ protected virtual bool AreCompatible(
private static string TryUniquify<T>(
IConventionIndex index, string indexName, Dictionary<string, T> indexes, int maxLength)
{
if (index.Builder.CanSetName(null))
if (index.Builder.CanSetDatabaseName(null))
{
indexName = Uniquifier.Uniquify(indexName, indexes, maxLength);
index.Builder.HasName(indexName);
index.Builder.HasDatabaseName(indexName);
return indexName;
}

Expand Down
Loading

0 comments on commit 546547d

Please sign in to comment.