Skip to content

Commit

Permalink
Fix for 21133. IndexAttribute did not set TypeMappings correctly.
Browse files Browse the repository at this point in the history 
Also create indexes before ModelFinalizing to allow fluent api overrides.
  • Loading branch information
@lajones
lajones committed Jun 9, 2020
1 parent d3ad139 commit f9ea870
Show file tree
Hide file tree
Showing 7 changed files with 461 additions and 24 deletions.
83 changes: 70 additions & 13 deletions src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
Expand All @@ -16,7 +17,8 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
/// <summary>
/// A convention that configures database indexes based on the <see cref="IndexAttribute" />.
/// </summary>
public class IndexAttributeConvention : IModelFinalizingConvention
public class IndexAttributeConvention : IEntityTypeAddedConvention,
IEntityTypeBaseTypeChangedConvention, IPropertyAddedConvention, IModelFinalizingConvention
{
/// <summary>
/// Creates a new instance of <see cref="IndexAttributeConvention" />.
Expand All @@ -33,9 +35,48 @@ public IndexAttributeConvention([NotNull] ProviderConventionSetBuilderDependenci
protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; }

/// <inheritdoc/>
public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, IConventionContext<IConventionModelBuilder> context)
public virtual void ProcessEntityTypeAdded(
IConventionEntityTypeBuilder entityTypeBuilder,
IConventionContext<IConventionEntityTypeBuilder> context)
{
foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
CheckIndexAttributesAndEnsureIndex(
new[] { entityTypeBuilder.Metadata }, true);
}

/// <inheritdoc/>
public virtual void ProcessEntityTypeBaseTypeChanged(
IConventionEntityTypeBuilder entityTypeBuilder,
IConventionEntityType newBaseType,
IConventionEntityType oldBaseType,
IConventionContext<IConventionEntityType> context)
{
CheckIndexAttributesAndEnsureIndex(
entityTypeBuilder.Metadata.GetDerivedTypesInclusive(), true);
}

/// <inheritdoc/>
public virtual void ProcessPropertyAdded(
IConventionPropertyBuilder propertyBuilder,
IConventionContext<IConventionPropertyBuilder> context)
{
CheckIndexAttributesAndEnsureIndex(
propertyBuilder.Metadata.DeclaringEntityType.GetDerivedTypesInclusive(), true);
}

/// <inheritdoc/>
public virtual void ProcessModelFinalizing(
IConventionModelBuilder modelBuilder,
IConventionContext<IConventionModelBuilder> context)
{
CheckIndexAttributesAndEnsureIndex(
modelBuilder.Metadata.GetEntityTypes(), false);
}

private void CheckIndexAttributesAndEnsureIndex(
IEnumerable<IConventionEntityType> entityTypes,
bool shouldEnsureIndexOrFailSilently)
{
foreach (var entityType in entityTypes)
{
if (entityType.ClrType != null)
{
Expand All @@ -48,6 +89,11 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
{
if (ignoredMembers.Contains(propertyName))
{
if (shouldEnsureIndexOrFailSilently)
{
return;
}

if (indexAttribute.Name == null)
{
throw new InvalidOperationException(
Expand All @@ -70,6 +116,11 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
var property = entityType.FindProperty(propertyName);
if (property == null)
{
if (shouldEnsureIndexOrFailSilently)
{
return;
}

if (indexAttribute.Name == null)
{
throw new InvalidOperationException(
Expand All @@ -89,20 +140,26 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
}
}

indexProperties.Add(property);
if (shouldEnsureIndexOrFailSilently)
{
indexProperties.Add(property);
}
}

var indexBuilder = indexAttribute.Name == null
? entityType.Builder.HasIndex(
indexProperties, fromDataAnnotation: true)
: entityType.Builder.HasIndex(
indexProperties, indexAttribute.Name, fromDataAnnotation: true);

if (indexBuilder != null)
if (shouldEnsureIndexOrFailSilently)
{
if (indexAttribute.GetIsUnique().HasValue)
var indexBuilder = indexAttribute.Name == null
? entityType.Builder.HasIndex(
indexProperties, fromDataAnnotation: true)
: entityType.Builder.HasIndex(
indexProperties, indexAttribute.Name, fromDataAnnotation: true);

if (indexBuilder != null)
{
indexBuilder.IsUnique(indexAttribute.GetIsUnique().Value, fromDataAnnotation: true);
if (indexAttribute.GetIsUnique().HasValue)
{
indexBuilder.IsUnique(indexAttribute.GetIsUnique().Value, fromDataAnnotation: true);
}
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public virtual ConventionSet CreateConventionSet()
var inversePropertyAttributeConvention = new InversePropertyAttributeConvention(Dependencies);
var relationshipDiscoveryConvention = new RelationshipDiscoveryConvention(Dependencies);
var servicePropertyDiscoveryConvention = new ServicePropertyDiscoveryConvention(Dependencies);
var indexAttributeConvention = new IndexAttributeConvention(Dependencies);

conventionSet.EntityTypeAddedConventions.Add(new NotMappedEntityTypeAttributeConvention(Dependencies));
conventionSet.EntityTypeAddedConventions.Add(new OwnedEntityTypeAttributeConvention(Dependencies));
Expand All @@ -70,6 +71,7 @@ public virtual ConventionSet CreateConventionSet()
conventionSet.EntityTypeAddedConventions.Add(propertyDiscoveryConvention);
conventionSet.EntityTypeAddedConventions.Add(servicePropertyDiscoveryConvention);
conventionSet.EntityTypeAddedConventions.Add(keyDiscoveryConvention);
conventionSet.EntityTypeAddedConventions.Add(indexAttributeConvention);
conventionSet.EntityTypeAddedConventions.Add(inversePropertyAttributeConvention);
conventionSet.EntityTypeAddedConventions.Add(relationshipDiscoveryConvention);
conventionSet.EntityTypeAddedConventions.Add(new DerivedTypeDiscoveryConvention(Dependencies));
Expand All @@ -87,6 +89,7 @@ public virtual ConventionSet CreateConventionSet()
conventionSet.EntityTypeBaseTypeChangedConventions.Add(propertyDiscoveryConvention);
conventionSet.EntityTypeBaseTypeChangedConventions.Add(servicePropertyDiscoveryConvention);
conventionSet.EntityTypeBaseTypeChangedConventions.Add(keyDiscoveryConvention);
conventionSet.EntityTypeBaseTypeChangedConventions.Add(indexAttributeConvention);
conventionSet.EntityTypeBaseTypeChangedConventions.Add(inversePropertyAttributeConvention);
conventionSet.EntityTypeBaseTypeChangedConventions.Add(relationshipDiscoveryConvention);
conventionSet.EntityTypeBaseTypeChangedConventions.Add(foreignKeyIndexConvention);
Expand Down Expand Up @@ -123,6 +126,7 @@ public virtual ConventionSet CreateConventionSet()
conventionSet.PropertyAddedConventions.Add(backingFieldAttributeConvention);
conventionSet.PropertyAddedConventions.Add(keyAttributeConvention);
conventionSet.PropertyAddedConventions.Add(keyDiscoveryConvention);
conventionSet.PropertyAddedConventions.Add(indexAttributeConvention);
conventionSet.PropertyAddedConventions.Add(foreignKeyPropertyDiscoveryConvention);

conventionSet.EntityTypePrimaryKeyChangedConventions.Add(foreignKeyPropertyDiscoveryConvention);
Expand Down Expand Up @@ -170,6 +174,7 @@ public virtual ConventionSet CreateConventionSet()

conventionSet.ModelFinalizingConventions.Add(new ModelCleanupConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(keyAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(indexAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(foreignKeyAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(new ChangeTrackingStrategyConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(new ConstructorBindingConvention(Dependencies));
Expand All @@ -182,7 +187,6 @@ public virtual ConventionSet CreateConventionSet()
conventionSet.ModelFinalizingConventions.Add(new QueryFilterDefiningQueryRewritingConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(inversePropertyAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(backingFieldConvention);
conventionSet.ModelFinalizingConventions.Add(new IndexAttributeConvention(Dependencies));

conventionSet.ModelFinalizedConventions.Add(new ValidatingConvention(Dependencies));

Expand Down
166 changes: 165 additions & 1 deletion test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,30 @@ private class EntityWithGenericProperty<TProperty>
public TProperty Property { get; set; }
}

[Index(nameof(FirstName), nameof(LastName))]
private class EntityWithIndexAttribute
{
public int Id { get; set; }
public string FirstName { get; set; }
public string LastName { get; set; }
}

[Index(nameof(FirstName), nameof(LastName), Name = "NamedIndex")]
private class EntityWithNamedIndexAttribute
{
public int Id { get; set; }
public string FirstName { get; set; }
public string LastName { get; set; }
}

[Index(nameof(FirstName), nameof(LastName), IsUnique = true)]
private class EntityWithUniqueIndexAttribute
{
public int Id { get; set; }
public string FirstName { get; set; }
public string LastName { get; set; }
}

public class TestOwner
{
public int Id { get; set; }
Expand Down Expand Up @@ -2363,7 +2387,7 @@ public virtual void Key_multiple_annotations_are_stored_in_snapshot()

#endregion

#region HasIndex
#region Index

[ConditionalFact]
public virtual void Index_annotations_are_stored_in_snapshot()
Expand Down Expand Up @@ -2581,6 +2605,146 @@ public virtual void Index_with_default_constraint_name_exceeding_max()
model => Assert.Equal(128, model.GetEntityTypes().First().GetIndexes().First().GetDatabaseName().Length));
}

[ConditionalFact]
public virtual void IndexAttribute_causes_column_to_have_key_or_index_column_length()
{
Test(
builder => builder.Entity<EntityWithIndexAttribute>(),
AddBoilerPlate(
GetHeading()
+ @"
modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithIndexAttribute"", b =>
{
b.Property<int>(""Id"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.HasAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.IdentityColumn);
b.Property<string>(""FirstName"")
.HasColumnType(""nvarchar(450)"");
b.Property<string>(""LastName"")
.HasColumnType(""nvarchar(450)"");
b.HasKey(""Id"");
b.HasIndex(""FirstName"", ""LastName"");
b.ToTable(""EntityWithIndexAttribute"");
});"),
model =>
Assert.Collection(
model.GetEntityTypes().First().GetIndexes().First().Properties,
p0 =>
{
Assert.Equal("FirstName", p0.Name);
Assert.Equal("nvarchar(450)", p0.GetColumnType());
},
p1 =>
{
Assert.Equal("LastName", p1.Name);
Assert.Equal("nvarchar(450)", p1.GetColumnType());
}
));
}

[ConditionalFact]
public virtual void IndexAttribute_name_is_stored_in_snapshot()
{
Test(
builder => builder.Entity<EntityWithNamedIndexAttribute>(),
AddBoilerPlate(
GetHeading()
+ @"
modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithNamedIndexAttribute"", b =>
{
b.Property<int>(""Id"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.HasAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.IdentityColumn);
b.Property<string>(""FirstName"")
.HasColumnType(""nvarchar(450)"");
b.Property<string>(""LastName"")
.HasColumnType(""nvarchar(450)"");
b.HasKey(""Id"");
b.HasIndex(new[] { ""FirstName"", ""LastName"" }, ""NamedIndex"");
b.ToTable(""EntityWithNamedIndexAttribute"");
});"),
model =>
{
var index = model.GetEntityTypes().First().GetIndexes().First();
Assert.Equal("NamedIndex", index.Name);
Assert.Collection(
index.Properties,
p0 =>
{
Assert.Equal("FirstName", p0.Name);
Assert.Equal("nvarchar(450)", p0.GetColumnType());
},
p1 =>
{
Assert.Equal("LastName", p1.Name);
Assert.Equal("nvarchar(450)", p1.GetColumnType());
}
);
});
}


[ConditionalFact]
public virtual void IndexAttribute_IsUnique_is_stored_in_snapshot()
{
Test(
builder => builder.Entity<EntityWithUniqueIndexAttribute>(),
AddBoilerPlate(
GetHeading()
+ @"
modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithUniqueIndexAttribute"", b =>
{
b.Property<int>(""Id"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.HasAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.IdentityColumn);
b.Property<string>(""FirstName"")
.HasColumnType(""nvarchar(450)"");
b.Property<string>(""LastName"")
.HasColumnType(""nvarchar(450)"");
b.HasKey(""Id"");
b.HasIndex(""FirstName"", ""LastName"")
.IsUnique()
.HasFilter(""[FirstName] IS NOT NULL AND [LastName] IS NOT NULL"");
b.ToTable(""EntityWithUniqueIndexAttribute"");
});"),
model =>
{
var index = model.GetEntityTypes().First().GetIndexes().First();
Assert.True(index.IsUnique);
Assert.Collection(
index.Properties,
p0 =>
{
Assert.Equal("FirstName", p0.Name);
Assert.Equal("nvarchar(450)", p0.GetColumnType());
},
p1 =>
{
Assert.Equal("LastName", p1.Name);
Assert.Equal("nvarchar(450)", p1.GetColumnType());
}
);
});
}

#endregion

#region ForeignKey
Expand Down
Loading

0 comments on commit f9ea870

Please sign in to comment.