Skip to content

Commit

Permalink
Updated to allow for creation of a property via HasIndex(propertyName…
Browse files Browse the repository at this point in the history 
…s) and to work around IsIgnored issue.
  • Loading branch information
@lajones
lajones committed Jun 11, 2020
1 parent b3e72a8 commit 6c9a0c2
Show file tree
Hide file tree
Showing 5 changed files with 240 additions and 110 deletions.
31 changes: 31 additions & 0 deletions src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,37 @@ IConventionServicePropertyBuilder ServiceProperty(
/// <returns> <see langword="true" /> if the entity type can be marked as keyless. </returns>
bool CanRemoveKey(bool fromDataAnnotation = false);

/// <summary>
/// Configures an index on the specified property names.
/// If there is an existing index on the given list of properyt names,
/// then the existing index will be returned for configuration.
/// </summary>
/// <param name="propertyNames"> The names of the properties that make up the index. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns>
/// An object that can be used to configure the index if it exists on the entity type,
/// <see langword="null" /> otherwise.
/// </returns>
IConventionIndexBuilder HasIndex(
[NotNull] IReadOnlyList<string> propertyNames, bool fromDataAnnotation = false);

/// <summary>
/// Configures an index on the specified property names.
/// If there is an existing index on the given list of properyt names,
/// then the existing index will be returned for configuration.
/// </summary>
/// <param name="propertyNames"> The names of the properties that make up the index. </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>
/// An object that can be used to configure the index if it exists on the entity type,
/// <see langword="null" /> otherwise.
/// </returns>
IConventionIndexBuilder HasIndex(
[NotNull] IReadOnlyList<string> propertyNames,
[NotNull] string name,
bool fromDataAnnotation = false);

/// <summary>
/// Configures an index on the specified properties.
/// If there is an existing index on the given list of properties,
Expand Down
201 changes: 118 additions & 83 deletions src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore;
Expand All @@ -18,7 +17,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
/// A convention that configures database indexes based on the <see cref="IndexAttribute" />.
/// </summary>
public class IndexAttributeConvention : IEntityTypeAddedConvention,
IEntityTypeBaseTypeChangedConvention, IPropertyAddedConvention, IModelFinalizingConvention
IEntityTypeBaseTypeChangedConvention, IModelFinalizingConvention
{
/// <summary>
/// Creates a new instance of <see cref="IndexAttributeConvention" />.
Expand All @@ -39,8 +38,7 @@ public virtual void ProcessEntityTypeAdded(
IConventionEntityTypeBuilder entityTypeBuilder,
IConventionContext<IConventionEntityTypeBuilder> context)
{
CheckIndexAttributesAndEnsureIndex(
new[] { entityTypeBuilder.Metadata }, true);
CheckIndexAttributesAndEnsureIndex(entityTypeBuilder.Metadata, false);
}

/// <inheritdoc/>
Expand All @@ -50,119 +48,156 @@ public virtual void ProcessEntityTypeBaseTypeChanged(
IConventionEntityType oldBaseType,
IConventionContext<IConventionEntityType> context)
{
CheckIndexAttributesAndEnsureIndex(
entityTypeBuilder.Metadata.GetDerivedTypesInclusive(), true);
}
if (oldBaseType == null)
{
return;
}

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

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

private void CheckIndexAttributesAndEnsureIndex(
IEnumerable<IConventionEntityType> entityTypes,
bool shouldEnsureIndexOrFailSilently)
IConventionEntityType entityType,
bool shouldThrow)
{
foreach (var entityType in entityTypes)
if (entityType.ClrType != null)
{
if (entityType.ClrType != null)
foreach (var indexAttribute in
entityType.ClrType.GetCustomAttributes<IndexAttribute>(true))
{
var ignoredMembers = entityType.GetIgnoredMembers();
foreach (var indexAttribute in
entityType.ClrType.GetCustomAttributes<IndexAttribute>(true))
IConventionIndexBuilder indexBuilder = null;
if (!shouldThrow)
{
var indexProperties = new List<IConventionProperty>();
foreach (var propertyName in indexAttribute.PropertyNames)
{
if (ignoredMembers.Contains(propertyName))
// TODO Change this to the IsIgnored() which takes
// fromDataAnnotation, when bug 21220 is fixed.
if (entityType.IsIgnored(propertyName))
{
if (shouldEnsureIndexOrFailSilently)
{
return;
}

if (indexAttribute.Name == null)
{
throw new InvalidOperationException(
CoreStrings.UnnamedIndexDefinedOnIgnoredProperty(
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName));
}
else
{
throw new InvalidOperationException(
CoreStrings.NamedIndexDefinedOnIgnoredProperty(
indexAttribute.Name,
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName));
}
return;
}

var property = entityType.FindProperty(propertyName);
if (property == null)
{
if (shouldEnsureIndexOrFailSilently)
{
return;
}

if (indexAttribute.Name == null)
{
throw new InvalidOperationException(
CoreStrings.UnnamedIndexDefinedOnNonExistentProperty(
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName));
}
else
{
throw new InvalidOperationException(
CoreStrings.NamedIndexDefinedOnNonExistentProperty(
indexAttribute.Name,
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName));
}
return;
}

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

if (shouldEnsureIndexOrFailSilently)
indexBuilder = indexAttribute.Name == null
? entityType.Builder.HasIndex(
indexProperties, fromDataAnnotation: true)
: entityType.Builder.HasIndex(
indexProperties, indexAttribute.Name, fromDataAnnotation: true);
}
else
{
// TODO See bug 21220 - we have to do this _before_ calling
// HasIndex() because during the call to HasIndex()
// IsIgnored (wrongly) returns false and we would end up
// creating a property where we shouldn't.
CheckIgnoredProperties(indexAttribute, entityType);

try
{
var indexBuilder = indexAttribute.Name == null
// Using the HasIndex(propertyNames) overload gives us
// a chance to create a missing property
// e.g. if the CLR property existed but was non-public.
indexBuilder = indexAttribute.Name == null
? entityType.Builder.HasIndex(
indexProperties, fromDataAnnotation: true)
indexAttribute.PropertyNames, fromDataAnnotation: true)
: entityType.Builder.HasIndex(
indexProperties, indexAttribute.Name, fromDataAnnotation: true);
indexAttribute.PropertyNames, indexAttribute.Name, fromDataAnnotation: true);
}
catch(InvalidOperationException exception)
{
CheckMissingProperties(indexAttribute, entityType, exception);

if (indexBuilder != null)
{
if (indexAttribute.GetIsUnique().HasValue)
{
indexBuilder.IsUnique(indexAttribute.GetIsUnique().Value, fromDataAnnotation: true);
}
}
throw;
}
}

if (indexBuilder != null
&& indexAttribute.GetIsUnique().HasValue)
{
indexBuilder.IsUnique(indexAttribute.GetIsUnique().Value, fromDataAnnotation: true);
}
}
}
}

private void CheckIgnoredProperties(
IndexAttribute indexAttribute,
IConventionEntityType entityType)
{
foreach (var propertyName in indexAttribute.PropertyNames)
{
if (entityType.IsIgnored(propertyName))
{
if (indexAttribute.Name == null)
{
throw new InvalidOperationException(
CoreStrings.UnnamedIndexDefinedOnIgnoredProperty(
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName));
}
else
{
throw new InvalidOperationException(
CoreStrings.NamedIndexDefinedOnIgnoredProperty(
indexAttribute.Name,
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName));
}
}
}
}

private void CheckMissingProperties(
IndexAttribute indexAttribute,
IConventionEntityType entityType,
InvalidOperationException innerException)
{
foreach (var propertyName in indexAttribute.PropertyNames)
{
var property = entityType.FindProperty(propertyName);
if (property == null)
{
if (indexAttribute.Name == null)
{
throw new InvalidOperationException(
CoreStrings.UnnamedIndexDefinedOnNonExistentProperty(
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName),
innerException);
}
else
{
throw new InvalidOperationException(
CoreStrings.NamedIndexDefinedOnNonExistentProperty(
indexAttribute.Name,
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName),
innerException);
}
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ 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
27 changes: 27 additions & 0 deletions src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4455,6 +4455,33 @@ IConventionEntityTypeBuilder IConventionEntityTypeBuilder.HasNoKey(IConventionKe
bool IConventionEntityTypeBuilder.CanRemoveKey(bool fromDataAnnotation)
=> CanRemoveKey(fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[DebuggerStepThrough]
IConventionIndexBuilder IConventionEntityTypeBuilder.HasIndex(
IReadOnlyList<string> propertyNames, bool fromDataAnnotation)
=> HasIndex(
propertyNames,
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[DebuggerStepThrough]
IConventionIndexBuilder IConventionEntityTypeBuilder.HasIndex(
IReadOnlyList<string> propertyNames, string name, bool fromDataAnnotation)
=> HasIndex(
propertyNames,
name,
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
Loading

0 comments on commit 6c9a0c2

Please sign in to comment.