diff --git a/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs b/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs index 5b5b81f1f98..3079e004b6a 100644 --- a/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs @@ -228,6 +228,37 @@ IConventionServicePropertyBuilder ServiceProperty( /// if the entity type can be marked as keyless. bool CanRemoveKey(bool fromDataAnnotation = false); + /// + /// Configures an index on the specified property names. + /// If there is an existing index on the given list of property names, + /// then the existing index will be returned for configuration. + /// + /// The names of the properties that make up the index. + /// Indicates whether the configuration was specified using a data annotation. + /// + /// An object that can be used to configure the index if it exists on the entity type, + /// otherwise. + /// + IConventionIndexBuilder HasIndex( + [NotNull] IReadOnlyList propertyNames, bool fromDataAnnotation = false); + + /// + /// 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. + /// + /// The names of the properties that make up the index. + /// The name of the index. + /// Indicates whether the configuration was specified using a data annotation. + /// + /// An object that can be used to configure the index if it exists on the entity type, + /// otherwise. + /// + IConventionIndexBuilder HasIndex( + [NotNull] IReadOnlyList propertyNames, + [NotNull] string name, + bool fromDataAnnotation = false); + /// /// Configures an index on the specified properties. /// If there is an existing index on the given list of properties, diff --git a/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs b/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs index bbddf9209b7..0d6f3f7c72d 100644 --- a/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs @@ -3,9 +3,9 @@ using System; using System.Collections.Generic; -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; @@ -16,7 +16,8 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions /// /// A convention that configures database indexes based on the . /// - public class IndexAttributeConvention : IModelFinalizingConvention + public class IndexAttributeConvention : IEntityTypeAddedConvention, + IEntityTypeBaseTypeChangedConvention, IModelFinalizingConvention { /// /// Creates a new instance of . @@ -33,78 +34,164 @@ public IndexAttributeConvention([NotNull] ProviderConventionSetBuilderDependenci protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; } /// - public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, IConventionContext context) + public virtual void ProcessEntityTypeAdded( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionContext context) + { + CheckIndexAttributesAndEnsureIndex(entityTypeBuilder.Metadata, false); + } + + /// + public virtual void ProcessEntityTypeBaseTypeChanged( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionEntityType newBaseType, + IConventionEntityType oldBaseType, + IConventionContext context) + { + if (oldBaseType == null) + { + return; + } + + CheckIndexAttributesAndEnsureIndex(entityTypeBuilder.Metadata, false); + } + + /// + public virtual void ProcessModelFinalizing( + IConventionModelBuilder modelBuilder, + IConventionContext context) { foreach (var entityType in modelBuilder.Metadata.GetEntityTypes()) { - if (entityType.ClrType != null) + CheckIndexAttributesAndEnsureIndex(entityType, true); + } + } + + private void CheckIndexAttributesAndEnsureIndex( + IConventionEntityType entityType, + bool shouldThrow) + { + if (entityType.ClrType == null) + { + return; + } + + foreach (var indexAttribute in + entityType.ClrType.GetCustomAttributes(true)) + { + IConventionIndexBuilder indexBuilder = null; + if (!shouldThrow) { - var ignoredMembers = entityType.GetIgnoredMembers(); - foreach (var indexAttribute in - entityType.ClrType.GetCustomAttributes(true)) + var indexProperties = new List(); + foreach (var propertyName in indexAttribute.PropertyNames) { - var indexProperties = new List(); - foreach (var propertyName in indexAttribute.PropertyNames) + var property = entityType.FindProperty(propertyName); + if (property == null) { - if (ignoredMembers.Contains(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)); - } - } - - var property = entityType.FindProperty(propertyName); - if (property == null) - { - 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)); - } - } - - indexProperties.Add(property); + return; } - var indexBuilder = indexAttribute.Name == null + indexProperties.Add(property); + } + + 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 + { + // 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); } } } diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index e9fa5b444cb..8e608207fc5 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -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)); @@ -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)); @@ -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); @@ -170,6 +173,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)); @@ -182,7 +186,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)); diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 049e92329c4..d7b64ad8ce3 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -4455,6 +4455,33 @@ IConventionEntityTypeBuilder IConventionEntityTypeBuilder.HasNoKey(IConventionKe bool IConventionEntityTypeBuilder.CanRemoveKey(bool fromDataAnnotation) => CanRemoveKey(fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// + /// 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. + /// + [DebuggerStepThrough] + IConventionIndexBuilder IConventionEntityTypeBuilder.HasIndex( + IReadOnlyList propertyNames, bool fromDataAnnotation) + => HasIndex( + propertyNames, + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + + /// + /// 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. + /// + [DebuggerStepThrough] + IConventionIndexBuilder IConventionEntityTypeBuilder.HasIndex( + IReadOnlyList propertyNames, string name, bool fromDataAnnotation) + => HasIndex( + propertyNames, + name, + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// /// 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 diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index ead78b3fde7..17a4dc0031a 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -164,6 +164,30 @@ private class EntityWithGenericProperty 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; } @@ -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() @@ -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(), + AddBoilerPlate( + GetHeading() + + @" + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithIndexAttribute"", b => + { + b.Property(""Id"") + .ValueGeneratedOnAdd() + .HasColumnType(""int"") + .HasAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.IdentityColumn); + + b.Property(""FirstName"") + .HasColumnType(""nvarchar(450)""); + + b.Property(""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(), + AddBoilerPlate( + GetHeading() + + @" + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithNamedIndexAttribute"", b => + { + b.Property(""Id"") + .ValueGeneratedOnAdd() + .HasColumnType(""int"") + .HasAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.IdentityColumn); + + b.Property(""FirstName"") + .HasColumnType(""nvarchar(450)""); + + b.Property(""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(), + AddBoilerPlate( + GetHeading() + + @" + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithUniqueIndexAttribute"", b => + { + b.Property(""Id"") + .ValueGeneratedOnAdd() + .HasColumnType(""int"") + .HasAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.IdentityColumn); + + b.Property(""FirstName"") + .HasColumnType(""nvarchar(450)""); + + b.Property(""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 diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalIndexTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalIndexTest.cs new file mode 100644 index 00000000000..5adf646b1f2 --- /dev/null +++ b/test/EFCore.Relational.Tests/Metadata/RelationalIndexTest.cs @@ -0,0 +1,67 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using Microsoft.EntityFrameworkCore.Metadata.Internal; +using Microsoft.EntityFrameworkCore.TestUtilities; +using Xunit; + +// ReSharper disable InconsistentNaming + +namespace Microsoft.EntityFrameworkCore.Metadata +{ + public class RelationalIndexExtensionsTest + { + [ConditionalFact] + public void IndexAttribute_database_name_can_be_overriden_using_fluent_api() + { + var modelBuilder = CreateConventionModelBuilder(); + var entityBuilder = modelBuilder.Entity(); + + foreach (var entityType in modelBuilder.Model.GetEntityTypes()) + { + foreach (var index in entityType.GetDeclaredIndexes()) + { + index.SetDatabaseName("My" + index.Name); + } + } + + modelBuilder.Model.FinalizeModel(); + + var index0 = (Internal.Index)entityBuilder.Metadata.GetIndexes().First(); + Assert.Equal(ConfigurationSource.DataAnnotation, index0.GetConfigurationSource()); + Assert.Equal("IndexOnAAndB", index0.Name); + Assert.Equal("MyIndexOnAAndB", index0.GetDatabaseName()); + Assert.Equal(ConfigurationSource.Explicit, index0.GetDatabaseNameConfigurationSource()); + Assert.True(index0.IsUnique); + Assert.Equal(ConfigurationSource.DataAnnotation, index0.GetIsUniqueConfigurationSource()); + Assert.Collection(index0.Properties, + prop0 => Assert.Equal("A", prop0.Name), + prop1 => Assert.Equal("B", prop1.Name)); + + var index1 = (Internal.Index)entityBuilder.Metadata.GetIndexes().Skip(1).First(); + Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetConfigurationSource()); + Assert.Equal("IndexOnBAndC", index1.Name); + Assert.Equal("MyIndexOnBAndC", index1.GetDatabaseName()); + Assert.Equal(ConfigurationSource.Explicit, index1.GetDatabaseNameConfigurationSource()); + Assert.False(index1.IsUnique); + Assert.Null(index1.GetIsUniqueConfigurationSource()); + Assert.Collection(index1.Properties, + prop0 => Assert.Equal("B", prop0.Name), + prop1 => Assert.Equal("C", prop1.Name)); + } + + protected virtual ModelBuilder CreateConventionModelBuilder() => RelationalTestHelpers.Instance.CreateConventionBuilder(); + + [Index(nameof(A), nameof(B), Name = "IndexOnAAndB", IsUnique = true)] + [Index(nameof(B), nameof(C), Name = "IndexOnBAndC")] + private class EntityWithIndexes + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + public int C { get; set; } + } + } +} diff --git a/test/EFCore.Relational.Tests/Storage/RelationalTypeMapperTestBase.cs b/test/EFCore.Relational.Tests/Storage/RelationalTypeMapperTestBase.cs index b0021aea033..d8f8813fa7e 100644 --- a/test/EFCore.Relational.Tests/Storage/RelationalTypeMapperTestBase.cs +++ b/test/EFCore.Relational.Tests/Storage/RelationalTypeMapperTestBase.cs @@ -24,6 +24,7 @@ protected IMutableModel CreateModel() builder.Entity().Property(e => e.Relationship2Id).IsUnicode(); builder.Entity().Property(e => e.PrecisionOnly).HasPrecision(16); builder.Entity().Property(e => e.PrecisionAndScale).HasPrecision(18, 7); + builder.Entity(); return builder.Model; } @@ -85,5 +86,12 @@ protected class MyRelatedType4 public string Relationship2Id { get; set; } public MyRelatedType3 Relationship2 { get; set; } } + + [Index(nameof(Name))] + protected class MyTypeWithIndexAttribute + { + public int Id { get; set; } + public string Name { get; set; } + } } } diff --git a/test/EFCore.SqlServer.Tests/SqlServerTypeMapperTest.cs b/test/EFCore.SqlServer.Tests/SqlServerTypeMapperTest.cs index 44bf0c09103..849b825bd0f 100644 --- a/test/EFCore.SqlServer.Tests/SqlServerTypeMapperTest.cs +++ b/test/EFCore.SqlServer.Tests/SqlServerTypeMapperTest.cs @@ -384,6 +384,30 @@ public void Does_indexed_column_SQL_Server_string_mapping(bool? unicode, bool? f Assert.Equal(450, typeMapping.CreateParameter(new TestCommand(), "Name", "Value").Size); } + [ConditionalTheory] + [InlineData(true, false)] + [InlineData(null, false)] + [InlineData(true, null)] + [InlineData(null, null)] + public void Does_IndexAttribute_column_SQL_Server_string_mapping(bool? unicode, bool? fixedLength) + { + var model = CreateModel(); + var entityType = model.FindEntityType(typeof(MyTypeWithIndexAttribute)); + var property = entityType.FindProperty("Name"); + property.SetIsUnicode(unicode); + property.SetIsFixedLength(fixedLength); + model.FinalizeModel(); + + var typeMapping = CreateTypeMapper().GetMapping(property); + + Assert.Null(typeMapping.DbType); + Assert.Equal("nvarchar(450)", typeMapping.StoreType); + Assert.Equal(450, typeMapping.Size); + Assert.True(typeMapping.IsUnicode); + Assert.False(typeMapping.IsFixedLength); + Assert.Equal(450, typeMapping.CreateParameter(new TestCommand(), "Name", "Value").Size); + } + [ConditionalTheory] [InlineData(false)] [InlineData(null)] @@ -611,6 +635,28 @@ public void Does_indexed_column_SQL_Server_string_mapping_ansi(bool? fixedLength Assert.Equal(900, typeMapping.CreateParameter(new TestCommand(), "Name", "Value").Size); } + [ConditionalTheory] + [InlineData(false)] + [InlineData(null)] + public void Does_IndexAttribute_column_SQL_Server_string_mapping_ansi(bool? fixedLength) + { + var model = CreateModel(); + var entityType = model.FindEntityType(typeof(MyTypeWithIndexAttribute)); + var property = entityType.FindProperty("Name"); + property.SetIsUnicode(false); + property.SetIsFixedLength(fixedLength); + model.FinalizeModel(); + + var typeMapping = CreateTypeMapper().GetMapping(property); + + Assert.Equal(DbType.AnsiString, typeMapping.DbType); + Assert.Equal("varchar(900)", typeMapping.StoreType); + Assert.Equal(900, typeMapping.Size); + Assert.False(typeMapping.IsUnicode); + Assert.False(typeMapping.IsFixedLength); + Assert.Equal(900, typeMapping.CreateParameter(new TestCommand(), "Name", "Value").Size); + } + [ConditionalTheory] [InlineData(false)] [InlineData(null)] diff --git a/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs index 18031628ed6..f812da95a6d 100644 --- a/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs @@ -29,14 +29,15 @@ public void IndexAttribute_overrides_configuration_from_convention() var entityBuilder = modelBuilder.Entity(typeof(EntityWithIndex), ConfigurationSource.Convention); entityBuilder.Property("Id", ConfigurationSource.Convention); - var propA = entityBuilder.Property("A", ConfigurationSource.Convention); - var propB = entityBuilder.Property("B", ConfigurationSource.Convention); + var propABuilder = entityBuilder.Property("A", ConfigurationSource.Convention); + var propBBuilder = entityBuilder.Property("B", ConfigurationSource.Convention); entityBuilder.PrimaryKey(new List { "Id" }, ConfigurationSource.Convention); - var indexProperties = new List { propA.Metadata.Name, propB.Metadata.Name }; + var indexProperties = new List { propABuilder.Metadata.Name, propBBuilder.Metadata.Name }; var indexBuilder = entityBuilder.HasIndex(indexProperties, "IndexOnAAndB", ConfigurationSource.Convention); indexBuilder.IsUnique(false, ConfigurationSource.Convention); + RunConvention(entityBuilder); RunConvention(modelBuilder); var index = entityBuilder.Metadata.GetIndexes().Single(); @@ -74,12 +75,11 @@ public void IndexAttribute_can_be_overriden_using_explicit_configuration() public void IndexAttribute_with_no_property_names_throws() { var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); - modelBuilder.Entity(); Assert.Equal( AbstractionsStrings.CollectionArgumentIsEmpty("propertyNames"), Assert.Throws( - () => modelBuilder.Model.FinalizeModel()).Message); + () => modelBuilder.Entity()).Message); } [InlineData(typeof(EntityWithInvalidNullIndexProperty))] @@ -89,12 +89,11 @@ public void IndexAttribute_with_no_property_names_throws() public void IndexAttribute_properties_cannot_include_whitespace(Type entityTypeWithInvalidIndex) { var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); - modelBuilder.Entity(entityTypeWithInvalidIndex); Assert.Equal( AbstractionsStrings.CollectionArgumentHasEmptyElements("propertyNames"), Assert.Throws( - () => modelBuilder.Model.FinalizeModel()).Message); + () => modelBuilder.Entity(entityTypeWithInvalidIndex)).Message); } [ConditionalFact] @@ -167,7 +166,8 @@ public virtual void IndexAttribute_without_name_and_an_ignored_property_causes_e public virtual void IndexAttribute_with_name_and_an_ignored_property_causes_error() { var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); - modelBuilder.Entity(); + var entityBuilder = modelBuilder.Entity(); + Assert.Equal( CoreStrings.NamedIndexDefinedOnIgnoredProperty( @@ -210,16 +210,109 @@ public virtual void IndexAttribute_with_name_and_non_existent_property_causes_er () => modelBuilder.Model.FinalizeModel()).Message); } + [ConditionalFact] + public void IndexAttribute_index_replicated_to_derived_type_when_base_type_changes() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + var grandparentEntityBuilder = modelBuilder.Entity(typeof(GrandparentEntityWithIndex)); + var parentEntityBuilder = modelBuilder.Entity(); + var childEntityBuilder = modelBuilder.Entity(); + + // Index is created on Grandparent type but not on Parent type. + // Child has its own index. + Assert.NotNull(parentEntityBuilder.Metadata.BaseType); + Assert.NotNull(childEntityBuilder.Metadata.BaseType); + Assert.Single(grandparentEntityBuilder.Metadata.GetDeclaredIndexes()); + Assert.Empty(parentEntityBuilder.Metadata.GetDeclaredIndexes()); + Assert.Single(childEntityBuilder.Metadata.GetDeclaredIndexes()); + + parentEntityBuilder.HasBaseType((string)null); + + Assert.Null(parentEntityBuilder.Metadata.BaseType); + Assert.NotNull(childEntityBuilder.Metadata.BaseType); + + // The Index is replicated on the Parent type, but not on the Child. + var index = (Metadata.Internal.Index) + Assert.Single(parentEntityBuilder.Metadata.GetDeclaredIndexes()); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetConfigurationSource()); + Assert.Equal("IndexOnGrandparentGetsReplicatedToParent", index.Name); + Assert.False(index.IsUnique); + Assert.Null(index.GetIsUniqueConfigurationSource()); + var indexProperty = Assert.Single(index.Properties); + Assert.Equal("B", indexProperty.Name); + + // The Child still has its own index even though + // the property is defined on the Grandparent type. + var childIndex = (Metadata.Internal.Index) + Assert.Single(childEntityBuilder.Metadata.GetDeclaredIndexes()); + Assert.Equal(ConfigurationSource.DataAnnotation, childIndex.GetConfigurationSource()); + Assert.Equal("IndexOnChildUnaffectedWhenParentBaseTypeRemoved", childIndex.Name); + Assert.True(childIndex.IsUnique); + Assert.Equal(ConfigurationSource.DataAnnotation, childIndex.GetIsUniqueConfigurationSource()); + var childIndexProperty = Assert.Single(childIndex.Properties); + Assert.Equal("A", childIndexProperty.Name); + + // Check there are no errors. + modelBuilder.Model.FinalizeModel(); + } + + [ConditionalFact] + public void IndexAttribute_index_is_created_when_missing_property_added() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + var entityBuilder = modelBuilder.Entity(typeof(EntityWithIndexOnShadowProperty)); + + Assert.Empty(entityBuilder.Metadata.GetDeclaredIndexes()); + + entityBuilder.Property("Y"); + modelBuilder.Model.FinalizeModel(); + + var index = (Metadata.Internal.Index) + Assert.Single(entityBuilder.Metadata.GetDeclaredIndexes()); + + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetConfigurationSource()); + Assert.Equal("IndexOnShadowProperty", index.Name); + Assert.Collection(index.Properties, + prop0 => Assert.Equal("X", prop0.Name), + prop1 => Assert.Equal("Y", prop1.Name)); + } + + [ConditionalFact] + public void IndexAttribute_index_is_created_when_index_on_private_property() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + var entityBuilder = modelBuilder.Entity(typeof(EntityWithIndexOnPrivateProperty)); + modelBuilder.Model.FinalizeModel(); + + var index = (Metadata.Internal.Index) + Assert.Single(entityBuilder.Metadata.GetDeclaredIndexes()); + + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetConfigurationSource()); + Assert.Equal("IndexOnPrivateProperty", index.Name); + Assert.Collection(index.Properties, + prop0 => Assert.Equal("X", prop0.Name), + prop1 => Assert.Equal("Y", prop1.Name)); + } + #endregion + private void RunConvention(InternalEntityTypeBuilder entityTypeBuilder) + { + var context = new ConventionContext( + entityTypeBuilder.Metadata.Model.ConventionDispatcher); + + CreateIndexAttributeConvention().ProcessEntityTypeAdded(entityTypeBuilder, context); + } + private void RunConvention(InternalModelBuilder modelBuilder) { var context = new ConventionContext(modelBuilder.Metadata.ConventionDispatcher); - new IndexAttributeConvention(CreateDependencies()) - .ProcessModelFinalizing(modelBuilder, context); + CreateIndexAttributeConvention().ProcessModelFinalizing(modelBuilder, context); } + private IndexAttributeConvention CreateIndexAttributeConvention() => new IndexAttributeConvention(CreateDependencies()); + private ProviderConventionSetBuilderDependencies CreateDependencies() => InMemoryTestHelpers.Instance.CreateContextServices().GetRequiredService(); @@ -321,5 +414,41 @@ private class EntityIndexWithNonExistentProperty public int A { get; set; } public int B { get; set; } } + + [Index(nameof(B), Name = "IndexOnGrandparentGetsReplicatedToParent")] + private class GrandparentEntityWithIndex + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + private class ParentEntity : GrandparentEntityWithIndex + { + public int C { get; set; } + public int D { get; set; } + } + + [Index(nameof(A), Name = "IndexOnChildUnaffectedWhenParentBaseTypeRemoved", IsUnique = true)] + private class ChildEntity : ParentEntity + { + public int E { get; set; } + public int F { get; set; } + } + + [Index(nameof(X), "Y", Name = "IndexOnShadowProperty")] + private class EntityWithIndexOnShadowProperty + { + public int Id { get; set; } + public int X { get; set; } + } + + [Index(nameof(X), nameof(Y), Name = "IndexOnPrivateProperty")] + private class EntityWithIndexOnPrivateProperty + { + public int Id { get; set; } + public int X { get; set; } + private int Y { get; set; } + } } }