Skip to content

Commit

Permalink
Add index on foreign key properties by convention
Browse files Browse the repository at this point in the history
  • Loading branch information
@smitpatel
smitpatel committed Nov 17, 2015
1 parent 3892784 commit f18e36c
Show file tree
Hide file tree
Showing 6 changed files with 334 additions and 132 deletions.
5 changes: 4 additions & 1 deletion src/EntityFramework.Core/Metadata/Internal/Index.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using JetBrains.Annotations;
using Microsoft.Data.Entity.Utilities;

Expand Down Expand Up @@ -31,7 +32,7 @@ public Index(

public virtual IReadOnlyList<Property> Properties { get; }
public virtual EntityType DeclaringEntityType { get; }
public virtual InternalIndexBuilder Builder { get; [param: CanBeNull] set; }
public virtual InternalIndexBuilder Builder { get;[param: CanBeNull] set; }

public virtual ConfigurationSource GetConfigurationSource() => _configurationSource;

Expand All @@ -49,5 +50,7 @@ public virtual ConfigurationSource UpdateConfigurationSource(ConfigurationSource

[UsedImplicitly]
private string DebuggerDisplay => Property.Format(Properties);

public virtual bool IsInUse() => DeclaringEntityType.FindForeignKeys(Properties).Any();
}
}
27 changes: 20 additions & 7 deletions src/EntityFramework.Core/Metadata/Internal/InternalEntityTypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,12 +600,6 @@ private static Dictionary<EntityType, List<RelationshipSnapshot>> GroupRelations
return null;
}

foreach (var index in Metadata.GetIndexes().Where(i => i.Properties.Contains(property)).ToList())
{
var removed = RemoveIndex(index, configurationSource);
Debug.Assert(removed.HasValue);
}

var detachedRelationships = property.FindContainingForeignKeys().ToList()
.Select(DetachRelationship).ToList();

Expand All @@ -617,6 +611,12 @@ private static Dictionary<EntityType, List<RelationshipSnapshot>> GroupRelations
Debug.Assert(removed.HasValue);
}

foreach (var index in Metadata.GetIndexes().Where(i => i.Properties.Contains(property)).ToList())
{
var removed = RemoveIndex(index, configurationSource);
Debug.Assert(removed.HasValue);
}

if (Metadata.GetProperties().Contains(property))
{
var removedProperty = Metadata.RemoveProperty(property.Name);
Expand Down Expand Up @@ -670,7 +670,15 @@ private RelationshipBuilderSnapshot DetachRelationship([NotNull] ForeignKey fore
var removedForeignKey = Metadata.RemoveForeignKey(foreignKey);
Debug.Assert(removedForeignKey == foreignKey);

RemoveShadowPropertiesIfUnused(foreignKey.Properties);
var index = Metadata.FindIndex(foreignKey.Properties);
if (index != null
&& !index.IsInUse())
{
// Remove index if created by convention
index.DeclaringEntityType.Builder.RemoveIndex(index, ConfigurationSource.DataAnnotation);
}

RemoveShadowPropertiesIfUnused(foreignKey.Properties.Where(p => p.DeclaringEntityType.FindDeclaredProperty(p.Name) != null).ToList());
foreignKey.PrincipalKey.DeclaringEntityType.Builder?.RemoveKeyIfUnused(foreignKey.PrincipalKey);

if (runConventions)
Expand Down Expand Up @@ -980,6 +988,11 @@ private InternalRelationshipBuilder CreateRelationshipBuilder(
value = ModelBuilder.ConventionDispatcher.OnForeignKeyAdded(value);
}

if (value != null)
{
HasIndex(value.Metadata.Properties, ConfigurationSource.Convention);
}

return value;
}

Expand Down
12 changes: 12 additions & 0 deletions test/EntityFramework.Commands.FunctionalTests/Migrations/ModelSnapshotTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,8 @@ public void Foreign_keys_are_stored_in_snapshot()
b.Property<int>(""AlternateId"");
b.HasKey(""Id"");
b.HasIndex(""AlternateId"");
});
builder.Entity(""Microsoft.Data.Entity.Commands.Migrations.ModelSnapshotTest+EntityWithTwoProperties"", b =>
Expand Down Expand Up @@ -367,6 +369,8 @@ public void Relationship_principal_key_is_stored_in_snapshot()
b.Property<int>(""Id"");
b.HasKey(""Id"");
b.HasIndex(""Id"");
});
builder.Entity(""Microsoft.Data.Entity.Commands.Migrations.ModelSnapshotTest+EntityWithTwoProperties"", b =>
Expand Down Expand Up @@ -597,6 +601,8 @@ public void ForeignKey_annotations_are_stored_in_snapshot()
b.Property<int>(""AlternateId"");
b.HasKey(""Id"");
b.HasIndex(""AlternateId"");
});
builder.Entity(""Microsoft.Data.Entity.Commands.Migrations.ModelSnapshotTest+EntityWithTwoProperties"", b =>
Expand Down Expand Up @@ -639,6 +645,8 @@ public void ForeignKey_isRequired_is_stored_in_snapshot()
.IsRequired();
b.HasKey(""Id"");
b.HasIndex(""Name"");
});
builder.Entity(""Microsoft.Data.Entity.Commands.Migrations.ModelSnapshotTest+EntityWithStringProperty"", b =>
Expand Down Expand Up @@ -678,6 +686,8 @@ public void ForeignKey_isUnique_is_stored_in_snapshot()
b.Property<string>(""Name"");
b.HasKey(""Id"");
b.HasIndex(""Name"");
});
builder.Entity(""Microsoft.Data.Entity.Commands.Migrations.ModelSnapshotTest+EntityWithStringProperty"", b =>
Expand Down Expand Up @@ -707,6 +717,8 @@ public void ForeignKey_deleteBehavior_is_stored_in_snapshot()
b.Property<int>(""Id"");
b.HasKey(""Id"");
b.HasIndex(""Id"");
});
builder.Entity(""Microsoft.Data.Entity.Commands.Migrations.ModelSnapshotTest+EntityWithTwoProperties"", b =>
Expand Down
70 changes: 70 additions & 0 deletions test/EntityFramework.Core.Tests/Metadata/Internal/InternalEntityTypeBuilderTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,30 @@ public void Replaces_inherited_relationship_of_lower_or_equal_source()
Assert.Same(relationshipBuilder.Metadata, dependentEntityBuilder.Metadata.GetForeignKeys().Single());
}

[Fact]
public void Adds_index_on_foreign_key_properties_by_convention()
{
var modelBuilder = CreateModelBuilder();
var principalEntityBuilder = modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit);
var dependentEntityBuilder = modelBuilder.Entity(typeof(Order), ConfigurationSource.Explicit);

var relationshipBuilder = dependentEntityBuilder.HasForeignKey(
principalEntityBuilder,
new[]
{
dependentEntityBuilder.Property(Order.CustomerIdProperty, ConfigurationSource.Convention).Metadata
},
ConfigurationSource.Convention);
Assert.NotNull(relationshipBuilder);

var index = dependentEntityBuilder.Metadata.GetIndexes().FirstOrDefault();
Assert.NotNull(index);
Assert.Equal(Order.CustomerIdProperty.Name, index.Properties.First().Name);

Assert.Equal(ConfigurationSource.Convention, dependentEntityBuilder.RemoveIndex(index, ConfigurationSource.DataAnnotation));
Assert.Empty(dependentEntityBuilder.Metadata.GetIndexes());
}

[Fact]
public void Can_only_remove_lower_or_equal_source_relationship()
{
Expand Down Expand Up @@ -261,6 +285,52 @@ public void Removing_relationship_removes_unused_contained_shadow_properties()
Assert.Empty(dependentEntityBuilder.Metadata.GetForeignKeys());
}

[Fact]
public void Removing_relationship_removes_unused_conventional_index()
{
var modelBuilder = CreateModelBuilder();
var principalEntityBuilder = modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit);
var dependentEntityBuilder = modelBuilder.Entity(typeof(Order), ConfigurationSource.Explicit);

var relationshipBuilder = dependentEntityBuilder.HasForeignKey(
principalEntityBuilder,
new[]
{
dependentEntityBuilder.Property(Order.CustomerIdProperty, ConfigurationSource.Convention).Metadata,
},
ConfigurationSource.Convention);
Assert.NotNull(relationshipBuilder);

Assert.Equal(ConfigurationSource.Convention, dependentEntityBuilder.RemoveForeignKey(relationshipBuilder.Metadata, ConfigurationSource.DataAnnotation));

Assert.Empty(dependentEntityBuilder.Metadata.GetIndexes());
Assert.Empty(dependentEntityBuilder.Metadata.GetForeignKeys());
}

[Fact] // TODO: Add test if the index is being used by another FK when support for multiple FK on same set of properties is added
public void Removing_relationship_does_not_remove_conventional_index_if_in_use()
{
var modelBuilder = CreateModelBuilder();
var principalEntityBuilder = modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit);
var dependentEntityBuilder = modelBuilder.Entity(typeof(Order), ConfigurationSource.Explicit);

var relationshipBuilder = dependentEntityBuilder.HasForeignKey(
principalEntityBuilder,
new[]
{
dependentEntityBuilder.Property(Order.CustomerIdProperty, ConfigurationSource.Convention).Metadata,
},
ConfigurationSource.Convention);
Assert.NotNull(relationshipBuilder);
dependentEntityBuilder.HasIndex(new[] { Order.CustomerIdProperty }, ConfigurationSource.Explicit);

Assert.Equal(ConfigurationSource.Convention, dependentEntityBuilder.RemoveForeignKey(relationshipBuilder.Metadata, ConfigurationSource.DataAnnotation));

Assert.Equal(1, dependentEntityBuilder.Metadata.GetIndexes().Count());
Assert.Equal(Order.CustomerIdProperty.Name, dependentEntityBuilder.Metadata.GetIndexes().First().Properties.First().Name);
Assert.Empty(dependentEntityBuilder.Metadata.GetForeignKeys());
}

[Fact]
public void Removing_relationship_does_not_remove_contained_shadow_properties_if_referenced_elsewhere()
{
Expand Down
31 changes: 21 additions & 10 deletions test/EntityFramework.MicrosoftSqlServer.Tests/Migrations/SqlServerModelDifferTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,12 +455,17 @@ public void Add_foreign_key_overridden()
}),
operations =>
{
Assert.Equal(1, operations.Count);
Assert.Equal(2, operations.Count);
var operation = Assert.IsType<AddForeignKeyOperation>(operations[0]);
Assert.Equal("dbo", operation.Schema);
Assert.Equal("Amoeba", operation.Table);
Assert.Equal("FK_Amoeba_Parent", operation.Name);
var createIndexOperation = Assert.IsType<CreateIndexOperation>(operations[0]);
Assert.Equal("dbo", createIndexOperation.Schema);
Assert.Equal("Amoeba", createIndexOperation.Table);
Assert.Equal("IX_Amoeba_ParentId", createIndexOperation.Name);
var addFkOperation = Assert.IsType<AddForeignKeyOperation>(operations[1]);
Assert.Equal("dbo", addFkOperation.Schema);
Assert.Equal("Amoeba", addFkOperation.Table);
Assert.Equal("FK_Amoeba_Parent", addFkOperation.Name);
});
}

Expand Down Expand Up @@ -489,12 +494,18 @@ public void Drop_foreign_key_overridden()
}),
operations =>
{
Assert.Equal(1, operations.Count);
Assert.Equal(2, operations.Count);
var dropFkOperation = Assert.IsType<DropForeignKeyOperation>(operations[0]);
Assert.Equal("dbo", dropFkOperation.Schema);
Assert.Equal("Anemone", dropFkOperation.Table);
Assert.Equal("FK_Anemone_Parent", dropFkOperation.Name);
var dropIndexOperation = Assert.IsType<DropIndexOperation>(operations[1]);
Assert.Equal("dbo", dropIndexOperation.Schema);
Assert.Equal("Anemone", dropIndexOperation.Table);
Assert.Equal("IX_Anemone_ParentId", dropIndexOperation.Name);
var operation = Assert.IsType<DropForeignKeyOperation>(operations[0]);
Assert.Equal("dbo", operation.Schema);
Assert.Equal("Anemone", operation.Table);
Assert.Equal("FK_Anemone_Parent", operation.Name);
});
}

Expand Down
Loading

0 comments on commit f18e36c

Please sign in to comment.