From 0a54b2642c1b371da5e446180bf986cacde10693 Mon Sep 17 00:00:00 2001 From: lajones Date: Wed, 20 May 2020 17:05:40 -0700 Subject: [PATCH 1/6] Fix for 4050. IndexAttribute. --- src/EFCore.Abstractions/IndexAttribute.cs | 42 ++++ .../Design/CSharpSnapshotGenerator.cs | 21 +- .../Internal/CSharpDbContextGenerator.cs | 15 +- .../Internal/CSharpEntityTypeGenerator.cs | 34 +++ .../RelationalIndexBuilderExtensions.cs | 28 +-- .../Extensions/RelationalIndexExtensions.cs | 22 +- .../RelationalModelValidator.cs | 69 ++++++ .../Properties/RelationalStrings.Designer.cs | 24 +++ .../Properties/RelationalStrings.resx | 9 + .../Builders/IConventionIndexBuilder.cs | 25 ++- src/EFCore/Metadata/Builders/IndexBuilder.cs | 15 ++ src/EFCore/Metadata/Builders/IndexBuilder`.cs | 11 + .../Metadata/Conventions/ConventionSet.cs | 6 + .../IIndexNameChangedConvention.cs | 23 ++ .../IndexEntityTypeAttributeConvention.cs | 77 +++++++ .../ProviderConventionSetBuilder.cs | 1 + .../ConventionDispatcher.ConventionScope.cs | 1 + ...entionDispatcher.DelayedConventionScope.cs | 19 ++ ...tionDispatcher.ImmediateConventionScope.cs | 28 +++ .../Internal/ConventionDispatcher.cs | 9 + src/EFCore/Metadata/IConventionIndex.cs | 16 ++ src/EFCore/Metadata/IIndex.cs | 5 + src/EFCore/Metadata/IMutableIndex.cs | 7 + src/EFCore/Metadata/Internal/Index.cs | 62 ++++++ .../Metadata/Internal/InternalIndexBuilder.cs | 50 +++++ src/EFCore/Properties/CoreStrings.Designer.cs | 24 +++ src/EFCore/Properties/CoreStrings.resx | 9 + .../Migrations/ModelSnapshotSqlServerTest.cs | 6 +- .../Internal/CSharpDbContextGeneratorTest.cs | 170 +++++++++++++++ .../Internal/CSharpEntityTypeGeneratorTest.cs | 108 ++++++++++ .../RelationalModelValidatorTest.cs | 32 +++ ...RelationalMetadataBuilderExtensionsTest.cs | 2 + .../RelationalMetadataExtensionsTest.cs | 2 + .../Conventions/ConventionDispatcherTest.cs | 96 +++++++++ .../EntityTypeAttributeConventionTest.cs | 202 ++++++++++++++++++ .../Internal/InternalIndexBuilderTest.cs | 34 +++ .../ModelBuilding/ModelBuilderGenericTest.cs | 3 + .../ModelBuilderNonGenericTest.cs | 3 + .../ModelBuilding/ModelBuilderTestBase.cs | 1 + .../ModelBuilding/NonRelationshipTestBase.cs | 3 +- 40 files changed, 1268 insertions(+), 46 deletions(-) create mode 100644 src/EFCore.Abstractions/IndexAttribute.cs create mode 100644 src/EFCore/Metadata/Conventions/IIndexNameChangedConvention.cs create mode 100644 src/EFCore/Metadata/Conventions/IndexEntityTypeAttributeConvention.cs diff --git a/src/EFCore.Abstractions/IndexAttribute.cs b/src/EFCore.Abstractions/IndexAttribute.cs new file mode 100644 index 00000000000..2dc2b219c98 --- /dev/null +++ b/src/EFCore.Abstractions/IndexAttribute.cs @@ -0,0 +1,42 @@ +// 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 JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Utilities; + +namespace Microsoft.EntityFrameworkCore +{ + /// + /// Specifies an index to be generated in the database. + /// + [AttributeUsage(AttributeTargets.Class, AllowMultiple = true)] + public class IndexAttribute : Attribute + { + /// + /// Initializes a new instance of the class. + /// + /// The members which constitute the index, in order (there must be at least one). + public IndexAttribute(params string[] memberNames) + { + Check.NotEmpty(memberNames, nameof(memberNames)); + MemberNames = memberNames; + } + + /// + /// The members which constitute the index, in order. + /// + public virtual string[] MemberNames { get; } + + /// + /// The name of the index in the database. + /// + public virtual string Name { get; [param: NotNull] set; } + + + /// + /// Whether the index is unique. + /// + public virtual bool IsUnique { get; set; } + } +} diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index cb63fb5723d..4fd60ac776b 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs @@ -746,17 +746,32 @@ protected virtual void GenerateIndex( stringBuilder .AppendLine() .Append(builderName) - .Append(".HasIndex(") + .Append(".") + .Append(nameof(EntityTypeBuilder.HasIndex)) + .Append("(") .Append(string.Join(", ", index.Properties.Select(p => Code.Literal(p.Name)))) .Append(")"); using (stringBuilder.Indent()) { + if (index.Name != null) + { + stringBuilder + .AppendLine() + .Append(".") + .Append(nameof(IndexBuilder.HasName)) + .Append("(") + .Append(Code.Literal(index.Name)) + .Append(")"); + } + if (index.IsUnique) { stringBuilder .AppendLine() - .Append(".IsUnique()"); + .Append(".") + .Append(nameof(IndexBuilder.IsUnique)) + .Append("()"); } GenerateIndexAnnotations(index, stringBuilder); @@ -779,8 +794,6 @@ protected virtual void GenerateIndexAnnotations( annotations, RelationalAnnotationNames.TableIndexMappings); - GenerateFluentApiForAnnotation( - ref annotations, RelationalAnnotationNames.Name, nameof(RelationalIndexBuilderExtensions.HasName), stringBuilder); GenerateFluentApiForAnnotation( ref annotations, RelationalAnnotationNames.Filter, nameof(RelationalIndexBuilderExtensions.HasFilter), stringBuilder); diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs index c370eacf0ae..22d8944d2c2 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs @@ -414,7 +414,15 @@ private void GenerateEntityType(IEntityType entityType, bool useDataAnnotations) foreach (var index in entityType.GetIndexes()) { - GenerateIndex(index); + // If there are annotations that cannot be represented + // using an IndexAttribute then use fluent API even + // if useDataAnnotations is true. + if (!useDataAnnotations + || index.GetAnnotations().Any( + a => !RelationalAnnotationNames.TableIndexMappings.Equals(a.Name, StringComparison.Ordinal))) + { + GenerateIndex(index); + } } foreach (var property in entityType.GetProperties()) @@ -584,12 +592,11 @@ private void GenerateIndex(IIndex index) RemoveAnnotation(ref annotations, RelationalAnnotationNames.TableIndexMappings); - if (!string.IsNullOrEmpty((string)index[RelationalAnnotationNames.Name])) + if (index.Name != null) { lines.Add( - $".{nameof(RelationalIndexBuilderExtensions.HasName)}" + + $".{nameof(IndexBuilder.HasName)}" + $"({_code.Literal(index.GetName())})"); - RemoveAnnotation(ref annotations, RelationalAnnotationNames.Name); } if (index.IsUnique) diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs index 1f2a1f89a46..6ebbe9c15c1 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs @@ -136,6 +136,7 @@ protected virtual void GenerateEntityTypeDataAnnotations( GenerateKeylessAttribute(entityType); GenerateTableAttribute(entityType); + GenerateIndexAttributes(entityType); } private void GenerateKeylessAttribute(IEntityType entityType) @@ -170,6 +171,39 @@ private void GenerateTableAttribute(IEntityType entityType) } } + private void GenerateIndexAttributes(IEntityType entityType) + { + // Do not generate IndexAttributes for indexes which + // would be generated anyway by convention. + foreach (var index in entityType.GetIndexes().Where(i => + ConfigurationSource.Convention != ((IConventionIndex)i).GetConfigurationSource())) + { + // If there are annotations that cannot be represented + // using an IndexAttribute then use fluent API instead. + if (!index.GetAnnotations().Any( + a => !RelationalAnnotationNames.TableIndexMappings.Equals(a.Name, StringComparison.Ordinal))) + { + var indexAttribute = new AttributeWriter(nameof(IndexAttribute)); + foreach (var property in index.Properties) + { + indexAttribute.AddParameter($"nameof({property.Name})"); + } + + if (index.Name != null) + { + indexAttribute.AddParameter($"{nameof(IndexAttribute.Name)} = {_code.Literal(index.Name)}"); + } + + if (index.IsUnique) + { + indexAttribute.AddParameter($"{nameof(IndexAttribute.IsUnique)} = {_code.Literal(index.IsUnique)}"); + } + + _sb.AppendLine(indexAttribute.ToString()); + } + } + } + /// /// 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/src/EFCore.Relational/Extensions/RelationalIndexBuilderExtensions.cs b/src/EFCore.Relational/Extensions/RelationalIndexBuilderExtensions.cs index 84ac9207a3f..c9efc627405 100644 --- a/src/EFCore.Relational/Extensions/RelationalIndexBuilderExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalIndexBuilderExtensions.cs @@ -1,6 +1,7 @@ // 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 JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Builders; @@ -20,15 +21,9 @@ public static class RelationalIndexBuilderExtensions /// The builder for the index being configured. /// The name of the index. /// A builder to further configure the index. + [Obsolete("Use IndexBuilder.HasName() instead.")] public static IndexBuilder HasName([NotNull] this IndexBuilder indexBuilder, [CanBeNull] string name) - { - Check.NotNull(indexBuilder, nameof(indexBuilder)); - Check.NullButNotEmpty(name, nameof(name)); - - indexBuilder.Metadata.SetName(name); - - return indexBuilder; - } + => indexBuilder.HasName(name); /// /// Configures the name of the index in the database when targeting a relational database. @@ -37,8 +32,9 @@ public static IndexBuilder HasName([NotNull] this IndexBuilder indexBuilder, [Ca /// The builder for the index being configured. /// The name of the index. /// A builder to further configure the index. + [Obsolete("Use IndexBuilder.HasName() instead.")] public static IndexBuilder HasName([NotNull] this IndexBuilder indexBuilder, [CanBeNull] string name) - => (IndexBuilder)HasName((IndexBuilder)indexBuilder, name); + => indexBuilder.HasName(name); /// /// Configures the name of the index in the database when targeting a relational database. @@ -50,17 +46,10 @@ public static IndexBuilder HasName([NotNull] this IndexBuilder /// The same builder instance if the configuration was applied, /// otherwise. /// + [Obsolete("Use IConventionIndexBuilder.HasName() instead.")] public static IConventionIndexBuilder HasName( [NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false) - { - if (indexBuilder.CanSetName(name, fromDataAnnotation)) - { - indexBuilder.Metadata.SetName(name, fromDataAnnotation); - return indexBuilder; - } - - return null; - } + => indexBuilder.HasName(name, fromDataAnnotation); /// /// Returns a value indicating whether the given name can be set for the index. @@ -69,9 +58,10 @@ public static IConventionIndexBuilder HasName( /// The name of the index. /// Indicates whether the configuration was specified using a data annotation. /// if the given name can be set for the index. + [Obsolete("Use IConventionIndexBuilder.CanSetName() instead.")] public static bool CanSetName( [NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false) - => indexBuilder.CanSetAnnotation(RelationalAnnotationNames.Name, name, fromDataAnnotation); + => indexBuilder.CanSetName(name, fromDataAnnotation); /// /// Configures the filter expression for the index. diff --git a/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs b/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs index 1717a837cde..6a155615b8f 100644 --- a/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs @@ -1,6 +1,7 @@ // 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.Collections.Generic; using System.Linq; using System.Text; @@ -24,8 +25,7 @@ public static class RelationalIndexExtensions /// The index. /// The name for this index. public static string GetName([NotNull] this IIndex index) - => (string)index[RelationalAnnotationNames.Name] - ?? index.GetDefaultName(); + => index.Name ?? index.GetDefaultName(); /// /// Returns the name for this index. @@ -111,10 +111,9 @@ public static string GetDefaultName( /// /// The index. /// The value to set. + [Obsolete("Use IMutableIndex.Name instead.")] public static void SetName([NotNull] this IMutableIndex index, [CanBeNull] string name) - => index.SetOrRemoveAnnotation( - RelationalAnnotationNames.Name, - Check.NullButNotEmpty(name, nameof(name))); + => index.Name = Check.NullButNotEmpty(name, nameof(name)); /// /// Sets the index name. @@ -123,23 +122,18 @@ public static void SetName([NotNull] this IMutableIndex index, [CanBeNull] strin /// The value to set. /// Indicates whether the configuration was specified using a data annotation. /// The configured value. + [Obsolete("Use IConventionIndex.SetName() instead.")] public static string SetName([NotNull] this IConventionIndex index, [CanBeNull] string name, bool fromDataAnnotation = false) - { - index.SetOrRemoveAnnotation( - RelationalAnnotationNames.Name, - Check.NullButNotEmpty(name, nameof(name)), - fromDataAnnotation); - - return name; - } + => index.SetName(Check.NullButNotEmpty(name, nameof(name)), fromDataAnnotation); /// /// Gets the for the index name. /// /// The index. /// The for the index name. + [Obsolete("Use IConventionIndex.GetNameConfigurationSource() instead.")] public static ConfigurationSource? GetNameConfigurationSource([NotNull] this IConventionIndex index) - => index.FindAnnotation(RelationalAnnotationNames.Name)?.GetConfigurationSource(); + => index.GetNameConfigurationSource(); /// /// Returns the index filter expression. diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index e7722ab0e77..025cb04eae2 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -61,6 +61,7 @@ public override void Validate(IModel model, IDiagnosticsLogger @@ -911,5 +912,73 @@ protected virtual void ValidatePropertyOverrides( private static string Format(string tableName, string schema) => schema == null ? tableName : schema + "." + tableName; + + /// + /// Validates that the members of any one index are all mapped to columns on the same table. + /// + /// The model to validate. + /// The logger to use. + protected virtual void ValidateIndexMembersOnSameTable( + [NotNull] IModel model, [NotNull] IDiagnosticsLogger logger) + { + Check.NotNull(model, nameof(model)); + + foreach (var entityType in model.GetEntityTypes()) + { + foreach (var index in entityType.GetIndexes()) + { + (string MemberName, string Table, string Schema) existingTableMapping = (null, null, null); + foreach (var member in index.Properties) + { + //TODO - update to use table-schema override of GetColumn() below when PR #20938 is checked in + var tablesMappedToMember = member.DeclaringEntityType.GetDerivedTypesInclusive() + .Select(t => (t.GetTableName(), t.GetSchema())).Distinct() + .Where(n => member.GetColumnName(/* n.Item1, n.Item2 */) != null); + var tablesMappedToMemberCount = tablesMappedToMember.Count(); + if (tablesMappedToMemberCount == 0) + { + throw new InvalidOperationException( + RelationalStrings.IndexMemberNotMappedToAnyTable( + entityType.DisplayName(), + Format(index.Properties.Select(p => p.Name)), + member.Name)); + } + else if (tablesMappedToMemberCount > 1) + { + throw new InvalidOperationException( + RelationalStrings.IndexMemberMappedToMultipleTables( + entityType.DisplayName(), + Format(index.Properties.Select(p => p.Name)), + member.Name, + Format(tablesMappedToMember.Select(t => FormatTable(t.Item1, t.Item2))))); + } + + var table = tablesMappedToMember.Single(); + if (existingTableMapping.MemberName == null) + { + existingTableMapping = (member.Name, table.Item1, table.Item2); + } + else if (!string.Equals(existingTableMapping.Table, table.Item1, StringComparison.Ordinal) + || !string.Equals(existingTableMapping.Schema, table.Item2, StringComparison.Ordinal)) + { + throw new InvalidOperationException( + RelationalStrings.IndexMembersOnDifferentTables( + entityType.DisplayName(), + Format(index.Properties.Select(p => p.Name)), + existingTableMapping.MemberName, + FormatTable(existingTableMapping.Table, existingTableMapping.Schema), + member.Name, + FormatTable(table.Item1, table.Item2))); + } + } + } + } + } + + private static string Format(IEnumerable names) + => "{" + string.Join(", ", names.Select(s => "'" + s + "'")) + "}"; + + private static string FormatTable(string table, string schema) + => schema == null ? table : schema + "." + table; } } diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 73663d16ec4..69c201531dd 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -865,6 +865,30 @@ public static string PropertyNotMappedToTable([CanBeNull] object property, [CanB GetString("PropertyNotMappedToTable", nameof(property), nameof(entityType), nameof(table)), property, entityType, table); + /// + /// An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field with name '{memberName}' is not mapped to a column in any table. Please use only members mapped to a column. + /// + public static string IndexMemberNotMappedToAnyTable([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName) + => string.Format( + GetString("IndexMemberNotMappedToAnyTable", nameof(entityType), nameof(indexMemberList), nameof(memberName)), + entityType, indexMemberList, memberName); + + /// + /// An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field with name '{memberName}' is mapped to columns in multiple tables {tableList}. Please use only members mapped to a single table. + /// + public static string IndexMemberMappedToMultipleTables([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName, [CanBeNull] object tableList) + => string.Format( + GetString("IndexMemberMappedToMultipleTables", nameof(entityType), nameof(indexMemberList), nameof(memberName), nameof(tableList)), + entityType, indexMemberList, memberName, tableList); + + /// + /// An index on the entity type '{entityType}' specifies members {indexMemberList}. The member '{memberName1}' is mapped to a column on table '{table1}', whereas the member '{memberName2}' is mapped to a column on table '{table2}'. All index members must map to the same table. + /// + public static string IndexMembersOnDifferentTables([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName1, [CanBeNull] object table1, [CanBeNull] object memberName2, [CanBeNull] object table2) + => string.Format( + GetString("IndexMembersOnDifferentTables", nameof(entityType), nameof(indexMemberList), nameof(memberName1), nameof(table1), nameof(memberName2), nameof(table2)), + entityType, indexMemberList, memberName1, table1, memberName2, table2); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 794ce2f3bfa..7ee12afaa05 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -606,4 +606,13 @@ The property '{property}' on entity type '{entityType}' is not mapped to the table '{table}'. + + An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field with name '{memberName}' is not mapped to a column in any table. Please use only members mapped to a column. + + + An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field with name '{memberName}' is mapped to columns in multiple tables {tableList}. Please use only members mapped to a single table. + + + An index on the entity type '{entityType}' specifies members {indexMemberList}. The member '{memberName1}' is mapped to a column on table '{table1}', whereas the member '{memberName2}' is mapped to a column on table '{table2}'. All index members must map to the same table. + \ No newline at end of file diff --git a/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs b/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs index c3ef86de595..898a9478f28 100644 --- a/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs +++ b/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs @@ -1,6 +1,8 @@ // 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 JetBrains.Annotations; + namespace Microsoft.EntityFrameworkCore.Metadata.Builders { /// @@ -32,11 +34,32 @@ public interface IConventionIndexBuilder : IConventionAnnotatableBuilder /// /// Returns a value indicating whether this index uniqueness can be configured - /// from the current configuration source + /// from the current configuration source. /// /// A value indicating whether the index is unique. /// Indicates whether the configuration was specified using a data annotation. /// if the index uniqueness can be configured. bool CanSetIsUnique(bool? unique, bool fromDataAnnotation = false); + + /// + /// Configures the name of this index. + /// + /// The name of the index (can be + /// to indicate that a unique name should be generated). + /// Indicates whether the configuration was specified using a data annotation. + /// + /// The same builder instance if the name is unchanged, + /// otherwise. + /// + IConventionIndexBuilder HasName([CanBeNull] string name, bool fromDataAnnotation = false); + + /// + /// Returns a value indicating whether the name can be configured + /// from the current configuration source. + /// + /// The name of the index. + /// Indicates whether the configuration was specified using a data annotation. + /// if the name can be configured. + bool CanSetName([CanBeNull] string name, bool fromDataAnnotation = false); } } diff --git a/src/EFCore/Metadata/Builders/IndexBuilder.cs b/src/EFCore/Metadata/Builders/IndexBuilder.cs index ab92e068f62..15bdf45a082 100644 --- a/src/EFCore/Metadata/Builders/IndexBuilder.cs +++ b/src/EFCore/Metadata/Builders/IndexBuilder.cs @@ -74,6 +74,21 @@ public virtual IndexBuilder IsUnique(bool unique = true) return this; } + /// + /// Configures the name of this index. + /// + /// + /// The name of this index (can be + /// to indicate that a unique name should be generated). + /// + /// The same builder instance so that multiple configuration calls can be chained. + public virtual IndexBuilder HasName([CanBeNull] string name) + { + Builder.HasName(name, ConfigurationSource.Explicit); + + return this; + } + #region Hidden System.Object members /// diff --git a/src/EFCore/Metadata/Builders/IndexBuilder`.cs b/src/EFCore/Metadata/Builders/IndexBuilder`.cs index d964c742a0c..4217ddcd12f 100644 --- a/src/EFCore/Metadata/Builders/IndexBuilder`.cs +++ b/src/EFCore/Metadata/Builders/IndexBuilder`.cs @@ -48,5 +48,16 @@ public IndexBuilder([NotNull] IMutableIndex index) /// The same builder instance so that multiple configuration calls can be chained. public new virtual IndexBuilder IsUnique(bool unique = true) => (IndexBuilder)base.IsUnique(unique); + + /// + /// Configures the name of this index. + /// + /// + /// The name of this index (can be + /// to indicate that a unique name should be generated). + /// + /// The same builder instance so that multiple configuration calls can be chained. + public new virtual IndexBuilder HasName([CanBeNull] string name) + => (IndexBuilder)base.HasName(name); } } diff --git a/src/EFCore/Metadata/Conventions/ConventionSet.cs b/src/EFCore/Metadata/Conventions/ConventionSet.cs index 5969000e213..4ef12321184 100644 --- a/src/EFCore/Metadata/Conventions/ConventionSet.cs +++ b/src/EFCore/Metadata/Conventions/ConventionSet.cs @@ -199,6 +199,12 @@ public class ConventionSet public virtual IList IndexUniquenessChangedConventions { get; } = new List(); + /// + /// Conventions to run when the name of an index is changed. + /// + public virtual IList IndexNameChangedConventions { get; } + = new List(); + /// /// Conventions to run when an annotation is changed on an index. /// diff --git a/src/EFCore/Metadata/Conventions/IIndexNameChangedConvention.cs b/src/EFCore/Metadata/Conventions/IIndexNameChangedConvention.cs new file mode 100644 index 00000000000..3a90e68ffd3 --- /dev/null +++ b/src/EFCore/Metadata/Conventions/IIndexNameChangedConvention.cs @@ -0,0 +1,23 @@ +// 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 JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Metadata.Builders; + +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions +{ + /// + /// Represents an operation that should be performed when the name of an index is changed. + /// + public interface IIndexNameChangedConvention : IConvention + { + /// + /// Called after the name of an index is changed. + /// + /// The builder for the index. + /// Additional information associated with convention execution. + void ProcessIndexNameChanged( + [NotNull] IConventionIndexBuilder indexBuilder, + [NotNull] IConventionContext context); + } +} diff --git a/src/EFCore/Metadata/Conventions/IndexEntityTypeAttributeConvention.cs b/src/EFCore/Metadata/Conventions/IndexEntityTypeAttributeConvention.cs new file mode 100644 index 00000000000..94ecdbf2a90 --- /dev/null +++ b/src/EFCore/Metadata/Conventions/IndexEntityTypeAttributeConvention.cs @@ -0,0 +1,77 @@ +// 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.Collections.Generic; +using System.Linq; +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Metadata.Builders; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; + +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions +{ + /// + /// A convention that configures database indexes based on the . + /// + public class IndexEntityTypeAttributeConvention : EntityTypeAttributeConventionBase + { + /// + /// Creates a new instance of . + /// + /// Parameter object containing dependencies for this convention. + public IndexEntityTypeAttributeConvention([NotNull] ProviderConventionSetBuilderDependencies dependencies) + : base(dependencies) + { + } + + /// + protected override void ProcessEntityTypeAdded( + IConventionEntityTypeBuilder entityTypeBuilder, + IndexAttribute attribute, + IConventionContext context) + { + var entityType = entityTypeBuilder.Metadata; + var ignoredMembers = entityType.GetIgnoredMembers(); + var indexProperties = new List(); + foreach (var memberName in attribute.MemberNames) + { + if (string.IsNullOrWhiteSpace(memberName)) + { + throw new InvalidOperationException( + CoreStrings.IndexMemberNameEmpty( + entityType.DisplayName(), + Format(attribute.MemberNames))); + } + + if (ignoredMembers.Contains(memberName)) + { + throw new InvalidOperationException( + CoreStrings.IndexMemberIsIgnored( + entityType.DisplayName(), + Format(attribute.MemberNames), + memberName)); + } + + var member = entityType.FindProperty(memberName); + if (member == null) + { + throw new InvalidOperationException( + CoreStrings.IndexMemberHasNoMatchingMember( + entityType.DisplayName(), + Format(attribute.MemberNames), + memberName)); + } + + indexProperties.Add(member); + } + + var indexBuilder = entityTypeBuilder.HasIndex(indexProperties, fromDataAnnotation: true); + indexBuilder.HasName(attribute.Name, fromDataAnnotation: true); + indexBuilder.IsUnique(attribute.IsUnique, fromDataAnnotation: true); + } + + private static string Format(string[] memberNames) + => "{" + string.Join(", ", memberNames.Select(s => "'" + s + "'")) + "}"; + } +} diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index b069f775d6e..342ea1a80c6 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -73,6 +73,7 @@ public virtual ConventionSet CreateConventionSet() conventionSet.EntityTypeAddedConventions.Add(inversePropertyAttributeConvention); conventionSet.EntityTypeAddedConventions.Add(relationshipDiscoveryConvention); conventionSet.EntityTypeAddedConventions.Add(new DerivedTypeDiscoveryConvention(Dependencies)); + conventionSet.EntityTypeAddedConventions.Add(new IndexEntityTypeAttributeConvention(Dependencies)); conventionSet.EntityTypeIgnoredConventions.Add(inversePropertyAttributeConvention); conventionSet.EntityTypeIgnoredConventions.Add(relationshipDiscoveryConvention); diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ConventionScope.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ConventionScope.cs index f3f53c9bfff..e8d36daab53 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ConventionScope.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ConventionScope.cs @@ -116,6 +116,7 @@ public abstract IConventionIndex OnIndexRemoved( [NotNull] IConventionEntityTypeBuilder entityTypeBuilder, [NotNull] IConventionIndex index); public abstract bool? OnIndexUniquenessChanged([NotNull] IConventionIndexBuilder indexBuilder); + public abstract string OnIndexNameChanged([NotNull] IConventionIndexBuilder indexBuilder); public abstract IConventionKeyBuilder OnKeyAdded([NotNull] IConventionKeyBuilder keyBuilder); public abstract IConventionAnnotation OnKeyAnnotationChanged( diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.DelayedConventionScope.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.DelayedConventionScope.cs index 2ccc197bae9..6e0523d0b50 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.DelayedConventionScope.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.DelayedConventionScope.cs @@ -182,6 +182,12 @@ public override IConventionIndex OnIndexRemoved( return indexBuilder.Metadata.IsUnique; } + public override string OnIndexNameChanged(IConventionIndexBuilder indexBuilder) + { + Add(new OnIndexNameChangedNode(indexBuilder)); + return indexBuilder.Metadata.Name; + } + public override IConventionAnnotation OnIndexAnnotationChanged( IConventionIndexBuilder indexBuilder, string name, @@ -861,6 +867,19 @@ public override void Run(ConventionDispatcher dispatcher) => dispatcher._immediateConventionScope.OnIndexUniquenessChanged(IndexBuilder); } + private sealed class OnIndexNameChangedNode : ConventionNode + { + public OnIndexNameChangedNode(IConventionIndexBuilder indexBuilder) + { + IndexBuilder = indexBuilder; + } + + public IConventionIndexBuilder IndexBuilder { get; } + + public override void Run(ConventionDispatcher dispatcher) + => dispatcher._immediateConventionScope.OnIndexNameChanged(IndexBuilder); + } + private sealed class OnIndexAnnotationChangedNode : ConventionNode { public OnIndexAnnotationChangedNode( diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs index be8a37b18c8..bfd892d3ef4 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs @@ -1042,6 +1042,34 @@ public override IConventionIndex OnIndexRemoved(IConventionEntityTypeBuilder ent return _boolConventionContext.Result; } + public override string OnIndexNameChanged(IConventionIndexBuilder indexBuilder) + { + using (_dispatcher.DelayConventions()) + { + _stringConventionContext.ResetState(indexBuilder.Metadata.Name); + foreach (var indexConvention in _conventionSet.IndexNameChangedConventions) + { + if (indexBuilder.Metadata.Builder == null) + { + return null; + } + + indexConvention.ProcessIndexNameChanged(indexBuilder, _stringConventionContext); + if (_stringConventionContext.ShouldStopProcessing()) + { + return _stringConventionContext.Result; + } + } + } + + if (indexBuilder.Metadata.Builder == null) + { + return null; + } + + return _stringConventionContext.Result; + } + public override IConventionAnnotation OnIndexAnnotationChanged( IConventionIndexBuilder indexBuilder, string name, diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs index d0a198fb4d7..85f00a2a098 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs @@ -473,6 +473,15 @@ public virtual IConventionIndex OnIndexRemoved( public virtual bool? OnIndexUniquenessChanged([NotNull] IConventionIndexBuilder indexBuilder) => _scope.OnIndexUniquenessChanged(indexBuilder); + /// + /// 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. + /// + public virtual string OnIndexNameChanged([NotNull] IConventionIndexBuilder indexBuilder) + => _scope.OnIndexNameChanged(indexBuilder); + /// /// 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/src/EFCore/Metadata/IConventionIndex.cs b/src/EFCore/Metadata/IConventionIndex.cs index fae466eb227..2d29e824de5 100644 --- a/src/EFCore/Metadata/IConventionIndex.cs +++ b/src/EFCore/Metadata/IConventionIndex.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Metadata.Builders; namespace Microsoft.EntityFrameworkCore.Metadata @@ -53,5 +54,20 @@ public interface IConventionIndex : IIndex, IConventionAnnotatable /// /// The configuration source for . ConfigurationSource? GetIsUniqueConfigurationSource(); + + /// + /// Sets the name of the index (can be + /// to indicate that a unique name should be generated). + /// + /// The name of the index. + /// Indicates whether the configuration was specified using a data annotation. + /// The configured name. + string SetName([CanBeNull] string name, bool fromDataAnnotation = false); + + /// + /// Returns the configuration source for . + /// + /// The configuration source for . + ConfigurationSource? GetNameConfigurationSource(); } } diff --git a/src/EFCore/Metadata/IIndex.cs b/src/EFCore/Metadata/IIndex.cs index fd411aa59de..cf2bce5f05f 100644 --- a/src/EFCore/Metadata/IIndex.cs +++ b/src/EFCore/Metadata/IIndex.cs @@ -16,6 +16,11 @@ public interface IIndex : IAnnotatable /// IReadOnlyList Properties { get; } + /// + /// Gets the name of this index. + /// + string Name { get; } + /// /// Gets a value indicating whether the values assigned to the indexed properties are unique. /// diff --git a/src/EFCore/Metadata/IMutableIndex.cs b/src/EFCore/Metadata/IMutableIndex.cs index 9bf805b39a0..eab197ca895 100644 --- a/src/EFCore/Metadata/IMutableIndex.cs +++ b/src/EFCore/Metadata/IMutableIndex.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using JetBrains.Annotations; namespace Microsoft.EntityFrameworkCore.Metadata { @@ -21,6 +22,12 @@ public interface IMutableIndex : IIndex, IMutableAnnotatable /// new bool IsUnique { get; set; } + /// + /// Gets or sets the name of the index (can be + /// to indicate that a unique name should be generated). + /// + new string Name { get; [param: CanBeNull] set; } + /// /// Gets the properties that this index is defined on. /// diff --git a/src/EFCore/Metadata/Internal/Index.cs b/src/EFCore/Metadata/Internal/Index.cs index cb15b1fea98..a6e0cb4f459 100644 --- a/src/EFCore/Metadata/Internal/Index.cs +++ b/src/EFCore/Metadata/Internal/Index.cs @@ -1,6 +1,7 @@ // 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.Collections.Generic; using System.Diagnostics; using JetBrains.Annotations; @@ -22,9 +23,11 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Internal public class Index : ConventionAnnotatable, IMutableIndex, IConventionIndex { private bool? _isUnique; + private string _name; private ConfigurationSource _configurationSource; private ConfigurationSource? _isUniqueConfigurationSource; + private ConfigurationSource? _nameConfigurationSource; // Warning: Never access these fields directly as access needs to be thread-safe private object _nullableValueFactory; @@ -138,6 +141,44 @@ public virtual bool IsUnique private static bool DefaultIsUnique => false; + /// + /// 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. + /// + public virtual string Name + { + get => _name; + set => SetName(value, ConfigurationSource.Explicit); + } + + /// + /// 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. + /// + public virtual string SetName([CanBeNull] string name, ConfigurationSource configurationSource) + { + var oldName = Name; + var isChanging = !string.Equals(oldName, name, StringComparison.Ordinal); + _name = name; + + if (name == null) + { + _nameConfigurationSource = null; + } + else + { + UpdateNameConfigurationSource(configurationSource); + } + + return isChanging + ? DeclaringEntityType.Model.ConventionDispatcher.OnIndexNameChanged(Builder) + : oldName; + } + /// /// 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 @@ -149,6 +190,17 @@ public virtual bool IsUnique private void UpdateIsUniqueConfigurationSource(ConfigurationSource configurationSource) => _isUniqueConfigurationSource = configurationSource.Max(_isUniqueConfigurationSource); + /// + /// 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. + /// + public virtual ConfigurationSource? GetNameConfigurationSource() => _nameConfigurationSource; + + private void UpdateNameConfigurationSource(ConfigurationSource configurationSource) + => _nameConfigurationSource = configurationSource.Max(_nameConfigurationSource); + /// /// Runs the conventions when an annotation was set or removed. /// @@ -286,5 +338,15 @@ IConventionEntityType IConventionIndex.DeclaringEntityType [DebuggerStepThrough] bool? IConventionIndex.SetIsUnique(bool? unique, bool fromDataAnnotation) => SetIsUnique(unique, 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] + string IConventionIndex.SetName(string name, bool fromDataAnnotation) + => SetName(name, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); } } diff --git a/src/EFCore/Metadata/Internal/InternalIndexBuilder.cs b/src/EFCore/Metadata/Internal/InternalIndexBuilder.cs index 310741cf321..06226f844ba 100644 --- a/src/EFCore/Metadata/Internal/InternalIndexBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalIndexBuilder.cs @@ -1,6 +1,7 @@ // 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 JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata.Builders; @@ -53,6 +54,33 @@ public virtual bool CanSetIsUnique(bool? unique, ConfigurationSource? configurat => Metadata.IsUnique == unique || configurationSource.Overrides(Metadata.GetIsUniqueConfigurationSource()); + /// + /// 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. + /// + public virtual InternalIndexBuilder HasName([CanBeNull] string name, ConfigurationSource configurationSource) + { + if (!CanSetName(name, configurationSource)) + { + return null; + } + + Metadata.SetName(name, configurationSource); + return this; + } + + /// + /// 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. + /// + public virtual bool CanSetName([CanBeNull] string name, ConfigurationSource? configurationSource) + => string.Equals(Metadata.Name, name, StringComparison.Ordinal) + || configurationSource.Overrides(Metadata.GetNameConfigurationSource()); + /// /// 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 @@ -108,5 +136,27 @@ bool IConventionIndexBuilder.CanSetIsUnique(bool? unique, bool fromDataAnnotatio => CanSetIsUnique( unique, 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. + /// + IConventionIndexBuilder IConventionIndexBuilder.HasName(string name, bool fromDataAnnotation) + => HasName( + 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 + /// 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. + /// + bool IConventionIndexBuilder.CanSetName(string name, bool fromDataAnnotation) + => CanSetName( + name, + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); } } diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index a45d1ed215b..a0a05fe7364 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -2658,6 +2658,30 @@ public static string MissingInverseManyToManyNavigation([CanBeNull] object princ public static string QueryContextAlreadyInitializedStateManager => GetString("QueryContextAlreadyInitializedStateManager"); + /// + /// An index on the entity type '{entityType}' specifies members {indexMemberList}. Member names must not be null or empty or consist only of white-space characters. + /// + public static string IndexMemberNameEmpty([CanBeNull] object entityType, [CanBeNull] object indexMemberList) + => string.Format( + GetString("IndexMemberNameEmpty", nameof(entityType), nameof(indexMemberList)), + entityType, indexMemberList); + + /// + /// An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field '{memberName}' has been marked NotMapped or Ignore(). An index cannot use such members. + /// + public static string IndexMemberIsIgnored([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName) + => string.Format( + GetString("IndexMemberIsIgnored", nameof(entityType), nameof(indexMemberList), nameof(memberName)), + entityType, indexMemberList, memberName); + + /// + /// An index on the entity type '{entityType}' specifies members {indexMemberList}. But no property or field with name '{memberName}' exists on that entity type or any of its base types. + /// + public static string IndexMemberHasNoMatchingMember([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName) + => string.Format( + GetString("IndexMemberHasNoMatchingMember", nameof(entityType), nameof(indexMemberList), nameof(memberName)), + entityType, indexMemberList, memberName); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 9610f9b190d..a7074bdfe2b 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1403,4 +1403,13 @@ InitializeStateManager method has been called multiple times on current query context. This method is intended to be called only once before query enumeration starts. + + An index on the entity type '{entityType}' specifies members {indexMemberList}. Member names must not be null or empty or consist only of white-space characters. + + + An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field '{memberName}' has been marked NotMapped or Ignore(). An index cannot use such members. + + + An index on the entity type '{entityType}' specifies members {indexMemberList}. But no property or field with name '{memberName}' exists on that entity type or any of its base types. + \ No newline at end of file diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index 193a67755b4..c0a7b972c92 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -2414,7 +2414,7 @@ public virtual void Index_name_annotation_is_stored_in_snapshot_as_fluent_api() b.ToTable(""EntityWithTwoProperties""); });"), - o => Assert.Equal("IndexName", o.GetEntityTypes().First().GetIndexes().First()["Relational:Name"])); + o => Assert.Equal("IndexName", o.GetEntityTypes().First().GetIndexes().First().Name)); } [ConditionalFact] @@ -2486,9 +2486,9 @@ public virtual void Index_multiple_annotations_are_stored_in_snapshot() o => { var index = o.GetEntityTypes().First().GetIndexes().First(); - Assert.Equal(3, index.GetAnnotations().Count()); + Assert.Equal(2, index.GetAnnotations().Count()); Assert.Equal("AnnotationValue", index["AnnotationName"]); - Assert.Equal("IndexName", index["Relational:Name"]); + Assert.Equal("IndexName", index.Name); }); } diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs index a842c2b0981..2c843ba4c6e 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs @@ -2,6 +2,7 @@ // 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.Design; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; @@ -347,6 +348,175 @@ public void Collation_works() }); } + + [ConditionalFact] + public void Entity_with_indexes_and_use_data_annotations_false_always_generates_fluent_API() + { + Test( + modelBuilder => modelBuilder + .Entity( + "EntityWithIndexes", + x => + { + x.Property("Id"); + x.Property("A"); + x.Property("B"); + x.Property("C"); + x.HasKey("Id"); + x.HasIndex("A", "B") + .HasName("IndexOnAAndB") + .IsUnique(); + x.HasIndex("B", "C") + .HasName("IndexOnBAndC") + .HasFilter("Filter SQL") + .HasAnnotation("AnnotationName", "AnnotationValue"); + }), + new ModelCodeGenerationOptions { UseDataAnnotations = false }, + code => + { + Assert.Equal( + @"using System; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata; + +namespace TestNamespace +{ + public partial class TestDbContext : DbContext + { + public TestDbContext() + { + } + + public TestDbContext(DbContextOptions options) + : base(options) + { + } + + public virtual DbSet EntityWithIndexes { get; set; } + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + { + if (!optionsBuilder.IsConfigured) + { +#warning " + + DesignStrings.SensitiveInformationWarning + + @" + optionsBuilder.UseSqlServer(""Initial Catalog=TestDatabase""); + } + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.HasIndex(x => new { x.A, x.B }) + .HasName(""IndexOnAAndB"") + .IsUnique(); + + entity.HasIndex(x => new { x.B, x.C }) + .HasName(""IndexOnBAndC"") + .HasFilter(""Filter SQL"") + .HasAnnotation(""AnnotationName"", ""AnnotationValue""); + + entity.Property(e => e.Id).HasAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.IdentityColumn); + }); + + OnModelCreatingPartial(modelBuilder); + } + + partial void OnModelCreatingPartial(ModelBuilder modelBuilder); + } +} +", + code.ContextFile.Code, + ignoreLineEndingDifferences: true); + }, + model => + Assert.Equal(2, model.FindEntityType("TestNamespace.EntityWithIndexes").GetIndexes().Count())); + } + + [ConditionalFact] + public void Entity_with_indexes_and_use_data_annotations_true_generates_fluent_API_only_for_indexes_with_annotations() + { + Test( + modelBuilder => modelBuilder + .Entity( + "EntityWithIndexes", + x => + { + x.Property("Id"); + x.Property("A"); + x.Property("B"); + x.Property("C"); + x.HasKey("Id"); + x.HasIndex("A", "B") + .HasName("IndexOnAAndB") + .IsUnique(); + x.HasIndex("B", "C") + .HasName("IndexOnBAndC") + .HasFilter("Filter SQL") + .HasAnnotation("AnnotationName", "AnnotationValue"); + }), + new ModelCodeGenerationOptions { UseDataAnnotations = true }, + code => + { + Assert.Equal( + @"using System; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata; + +namespace TestNamespace +{ + public partial class TestDbContext : DbContext + { + public TestDbContext() + { + } + + public TestDbContext(DbContextOptions options) + : base(options) + { + } + + public virtual DbSet EntityWithIndexes { get; set; } + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + { + if (!optionsBuilder.IsConfigured) + { +#warning " + + DesignStrings.SensitiveInformationWarning + + @" + optionsBuilder.UseSqlServer(""Initial Catalog=TestDatabase""); + } + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.HasIndex(x => new { x.B, x.C }) + .HasName(""IndexOnBAndC"") + .HasFilter(""Filter SQL"") + .HasAnnotation(""AnnotationName"", ""AnnotationValue""); + + entity.Property(e => e.Id).HasAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.IdentityColumn); + }); + + OnModelCreatingPartial(modelBuilder); + } + + partial void OnModelCreatingPartial(ModelBuilder modelBuilder); + } +} +", + code.ContextFile.Code, + ignoreLineEndingDifferences: true); + }, + model => + Assert.Equal(2, model.FindEntityType("TestNamespace.EntityWithIndexes").GetIndexes().Count())); + } + private class TestCodeGeneratorPlugin : ProviderCodeGeneratorPlugin { public override MethodCallCodeFragment GenerateProviderOptions() diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs index 4ca21b197b3..6d3d9432aad 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs @@ -409,5 +409,113 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) Assert.Null(entityType.FindPrimaryKey()); }); } + + [ConditionalFact] + public void Entity_with_multiple_indexes_generates_multiple_IndexAttributes() + { + Test( + modelBuilder => modelBuilder + .Entity( + "EntityWithIndexes", + x => + { + x.Property("Id"); + x.Property("A"); + x.Property("B"); + x.Property("C"); + x.HasKey("Id"); + x.HasIndex("A", "B") + .HasName("IndexOnAAndB") + .IsUnique(); + x.HasIndex("B", "C") + .HasName("IndexOnBAndC"); + }), + new ModelCodeGenerationOptions { UseDataAnnotations = true }, + code => + { + var entityFile = code.AdditionalFiles.First(f => f.Path == "EntityWithIndexes.cs"); + Assert.Equal( + @"using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace +{ + [Index(nameof(A), nameof(B), Name = ""IndexOnAAndB"", IsUnique = true)] + [Index(nameof(B), nameof(C), Name = ""IndexOnBAndC"")] + public partial class EntityWithIndexes + { + [Key] + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + public int C { get; set; } + } +} +", + entityFile.Code, ignoreLineEndingDifferences: true); + }, + model => + { + var entityType = model.FindEntityType("TestNamespace.EntityWithIndexes"); + var indexes = entityType.GetIndexes(); + Assert.Equal(2, indexes.Count()); + Assert.Equal("IndexOnAAndB", indexes.First().Name); + Assert.Equal("IndexOnBAndC", indexes.Skip(1).First().Name); + }); + } + + [ConditionalFact] + public void Entity_with_indexes_generates_IndexAttribute_only_for_indexes_without_annotations() + { + Test( + modelBuilder => modelBuilder + .Entity( + "EntityWithIndexes", + x => + { + x.Property("Id"); + x.Property("A"); + x.Property("B"); + x.Property("C"); + x.HasKey("Id"); + x.HasIndex("A", "B") + .HasName("IndexOnAAndB") + .IsUnique(); + x.HasIndex("B", "C") + .HasName("IndexOnBAndC") + .HasFilter("Filter SQL"); + }), + new ModelCodeGenerationOptions { UseDataAnnotations = true }, + code => + { + var entityFile = code.AdditionalFiles.First(f => f.Path == "EntityWithIndexes.cs"); + Assert.Equal( + @"using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace +{ + [Index(nameof(A), nameof(B), Name = ""IndexOnAAndB"", IsUnique = true)] + public partial class EntityWithIndexes + { + [Key] + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + public int C { get; set; } + } +} +", + entityFile.Code, ignoreLineEndingDifferences: true); + }, + model => + Assert.Equal(2, model.FindEntityType("TestNamespace.EntityWithIndexes").GetIndexes().Count())); + } } } diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 66a30a5cc3b..c62f7a14e0b 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -1248,6 +1248,38 @@ var methodInfo modelBuilder.Model); } + [ConditionalFact(Skip = "Needs TPT to run")] //TODO - awaiting PR 20938 + public void Detects_index_member_not_mapped_to_any_table() + { + // detect RelationalStrings.IndexMemberNotMappedToAnyTable exception + } + + [ConditionalFact(Skip = "Needs TPT to run")] //TODO - awaiting PR 20938 + public void Detects_index_member_mapped_to_multiple_tables() + { + // detect RelationalStrings.IndexMemberMappedToMultipleTables exception + } + + [ConditionalFact(Skip = "Needs TPT to run")] //TODO - awaiting PR 20938 + public void Detects_index_members_mapped_to_different_tables() + { + var modelBuilder = CreateConventionalModelBuilder(); + + modelBuilder.Entity().ToTable("Animals"); + modelBuilder.Entity().ToTable("Cats"); + modelBuilder.Entity().HasIndex(nameof(Animal.Id), nameof(Cat.Identity)); + + VerifyError( + RelationalStrings.IndexMembersOnDifferentTables( + nameof(Cat), + "{'Id', 'Identity'}", + nameof(Animal.Id), + "Animals", + nameof(Cat.Identity), + "Cats"), + modelBuilder.Model); + } + private static void GenerateMapping(IMutableProperty property) => property[CoreAnnotationNames.TypeMapping] = new TestRelationalTypeMappingSource( diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalMetadataBuilderExtensionsTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalMetadataBuilderExtensionsTest.cs index b63cd45ce98..541da0212a8 100644 --- a/test/EFCore.Relational.Tests/Metadata/RelationalMetadataBuilderExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/Metadata/RelationalMetadataBuilderExtensionsTest.cs @@ -147,6 +147,7 @@ public void Can_access_index() entityTypeBuilder.Property(typeof(int), "Id", ConfigurationSource.Convention); var indexBuilder = entityTypeBuilder.HasIndex(new[] { "Id" }, ConfigurationSource.Convention); +#pragma warning disable CS0618 // Type or member is obsolete Assert.NotNull(indexBuilder.HasName("Splew")); Assert.Equal("Splew", indexBuilder.Metadata.GetName()); @@ -161,6 +162,7 @@ public void Can_access_index() Assert.NotNull(indexBuilder.HasName("Splod")); Assert.Equal("Splod", indexBuilder.Metadata.GetName()); +#pragma warning restore CS0618 // Type or member is obsolete Assert.NotNull(indexBuilder.HasFilter("Splew")); Assert.Equal("Splew", indexBuilder.Metadata.GetFilter()); diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalMetadataExtensionsTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalMetadataExtensionsTest.cs index 7ef93e4acb4..8564c595066 100644 --- a/test/EFCore.Relational.Tests/Metadata/RelationalMetadataExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/Metadata/RelationalMetadataExtensionsTest.cs @@ -348,11 +348,13 @@ public void Can_get_and_set_index_name() Assert.Equal("IX_Customer_Id", index.GetName()); +#pragma warning disable CS0618 // Type or member is obsolete index.SetName("MyIndex"); Assert.Equal("MyIndex", index.GetName()); index.SetName(null); +#pragma warning restore CS0618 // Type or member is obsolete Assert.Equal("IX_Customer_Id", index.GetName()); } diff --git a/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs b/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs index 11110e12509..441dd6f6a71 100644 --- a/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs @@ -2812,6 +2812,102 @@ public void ProcessIndexUniquenessChanged( } } + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + [ConditionalTheory] + public void OnIndexNameChanged_calls_conventions_in_order(bool useBuilder, bool useScope) + { + var conventions = new ConventionSet(); + + var convention1 = new IndexNameChangedConvention(terminate: false); + var convention2 = new IndexNameChangedConvention(terminate: true); + var convention3 = new IndexNameChangedConvention(terminate: false); + conventions.IndexNameChangedConventions.Add(convention1); + conventions.IndexNameChangedConventions.Add(convention2); + conventions.IndexNameChangedConventions.Add(convention3); + + var builder = new InternalModelBuilder(new Model(conventions)); + var entityBuilder = builder.Entity(typeof(Order), ConfigurationSource.Convention); + var index = entityBuilder.HasIndex( + new List { "OrderId" }, ConfigurationSource.Convention).Metadata; + + var scope = useScope ? builder.Metadata.ConventionDispatcher.DelayConventions() : null; + + if (useBuilder) + { + index.Builder.HasName("OriginalIndexName", ConfigurationSource.Convention); + } + else + { + index.Name = "OriginalIndexName"; + } + + if (useScope) + { + Assert.Empty(convention1.Calls); + Assert.Empty(convention2.Calls); + scope.Dispose(); + } + + Assert.Equal(new[] { "OriginalIndexName" }, convention1.Calls); + Assert.Equal(new[] { "OriginalIndexName" }, convention2.Calls); + Assert.Empty(convention3.Calls); + + if (useBuilder) + { + index.Builder.HasName("OriginalIndexName", ConfigurationSource.Convention); + } + else + { + index.Name = "OriginalIndexName"; + } + + Assert.Equal(new[] { "OriginalIndexName" }, convention1.Calls); + Assert.Equal(new[] { "OriginalIndexName" }, convention2.Calls); + Assert.Empty(convention3.Calls); + + if (useBuilder) + { + index.Builder.HasName("UpdatedIndexName", ConfigurationSource.Convention); + } + else + { + index.Name = "UpdatedIndexName"; + } + + Assert.Equal(new[] { "OriginalIndexName", "UpdatedIndexName" }, convention1.Calls); + Assert.Equal(new[] { "OriginalIndexName", "UpdatedIndexName" }, convention2.Calls); + Assert.Empty(convention3.Calls); + + Assert.Same(index, entityBuilder.Metadata.RemoveIndex(index.Properties)); + } + + private class IndexNameChangedConvention : IIndexNameChangedConvention + { + private readonly bool _terminate; + public readonly List Calls = new List(); + + public IndexNameChangedConvention(bool terminate) + { + _terminate = terminate; + } + + public void ProcessIndexNameChanged( + IConventionIndexBuilder indexBuilder, IConventionContext context) + { + Assert.NotNull(indexBuilder.Metadata.Builder); + + Calls.Add(indexBuilder.Metadata.Name); + + if (_terminate) + { + context.StopProcessing(); + } + } + } + [InlineData(false, false)] [InlineData(true, false)] [InlineData(false, true)] diff --git a/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs index ea925e829fe..31c263319a3 100644 --- a/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs @@ -132,6 +132,146 @@ public void KeyAttribute_does_not_override_keyless_attribute() #endregion + #region IndexAttribute + + [ConditionalFact] + public void IndexAttribute_overrides_configuration_from_convention() + { + var modelBuilder = new InternalModelBuilder(new Model()); + + 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); + entityBuilder.PrimaryKey(new List { "Id" }, ConfigurationSource.Convention); + + var indexProperties = new List { propA.Metadata.Name, propB.Metadata.Name }; + var indexBuilder = entityBuilder.HasIndex(indexProperties, ConfigurationSource.Convention); + indexBuilder.HasName("ConventionalIndexName", ConfigurationSource.Convention); + indexBuilder.IsUnique(false, ConfigurationSource.Convention); + + RunConvention(entityBuilder); + + var index = entityBuilder.Metadata.GetIndexes().Single(); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetConfigurationSource()); + Assert.Equal("IndexOnAAndB", index.Name); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetNameConfigurationSource()); + Assert.True(index.IsUnique); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetIsUniqueConfigurationSource()); + Assert.Collection(index.Properties, + prop0 => Assert.Equal("A", prop0.Name), + prop1 => Assert.Equal("B", prop1.Name)); + } + + [ConditionalFact] + public void IndexAttribute_can_be_overriden_using_explicit_configuration() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + var entityBuilder = modelBuilder.Entity(); + + var index = (Metadata.Internal.Index)entityBuilder.Metadata.GetIndexes().Single(); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetConfigurationSource()); + Assert.Equal("IndexOnAAndB", index.Name); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetNameConfigurationSource()); + Assert.True(index.IsUnique); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetIsUniqueConfigurationSource()); + Assert.Collection(index.Properties, + prop0 => Assert.Equal("A", prop0.Name), + prop1 => Assert.Equal("B", prop1.Name)); + + entityBuilder.HasIndex(e => new { e.A, e.B }) + .HasName("OverridenIndexName") + .IsUnique(false); + + index = (Metadata.Internal.Index)entityBuilder.Metadata.GetIndexes().Single(); + Assert.Equal(ConfigurationSource.Explicit, index.GetConfigurationSource()); + Assert.Equal("OverridenIndexName", index.Name); + Assert.Equal(ConfigurationSource.Explicit, index.GetNameConfigurationSource()); + Assert.False(index.IsUnique); + Assert.Equal(ConfigurationSource.Explicit, index.GetIsUniqueConfigurationSource()); + Assert.Collection(index.Properties, + prop0 => Assert.Equal("A", prop0.Name), + prop1 => Assert.Equal("B", prop1.Name)); + } + + [InlineData(typeof(EntityWithInvalidNullIndexMember), "{'A', ''}")] + [InlineData(typeof(EntityWithInvalidEmptyIndexMember), "{'A', ''}")] + [InlineData(typeof(EntityWithInvalidWhiteSpaceIndexMember), "{'A', ' \r\n\t'}")] + [ConditionalTheory] + public void IndexAttribute_members_cannot_include_whitespace(Type entityTypeWithInvalidIndex, string indexMembersString) + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + Assert.Equal( + CoreStrings.IndexMemberNameEmpty( + entityTypeWithInvalidIndex.Name, + indexMembersString), + Assert.Throws( + () => modelBuilder.Entity(entityTypeWithInvalidIndex)).Message); + } + + [ConditionalFact] + public void IndexAttribute_with_an_ignored_member_throws() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + Assert.Equal( + CoreStrings.IndexMemberIsIgnored( + nameof(EntityWithIgnoredMember), + "{'A', 'B'}", + "B"), + Assert.Throws( + () => modelBuilder.Entity()).Message); + } + + [ConditionalFact] + public void IndexAttribute_with_a_non_existent_member_throws() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + Assert.Equal( + CoreStrings.IndexMemberHasNoMatchingMember( + nameof(EntityWithNonExistentMember), + "{'A', 'DoesNotExist'}", + "DoesNotExist"), + Assert.Throws( + () => modelBuilder.Entity()).Message); + } + + [ConditionalFact] + public void IndexAttribute_can_be_applied_more_than_once_per_entity_type() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + var entityBuilder = modelBuilder.Entity(); + + var indexes = entityBuilder.Metadata.GetIndexes(); + Assert.Equal(2, indexes.Count()); + + var index0 = (Metadata.Internal.Index)indexes.First(); + Assert.Equal(ConfigurationSource.DataAnnotation, index0.GetConfigurationSource()); + Assert.Equal("IndexOnAAndB", index0.Name); + Assert.Equal(ConfigurationSource.DataAnnotation, index0.GetNameConfigurationSource()); + 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 = (Metadata.Internal.Index)indexes.Skip(1).First(); + Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetConfigurationSource()); + Assert.Equal("IndexOnBAndC", index1.Name); + Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetNameConfigurationSource()); + Assert.False(index1.IsUnique); + Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetIsUniqueConfigurationSource()); + Assert.Collection(index1.Properties, + prop0 => Assert.Equal("B", prop0.Name), + prop1 => Assert.Equal("C", prop1.Name)); + } + + #endregion + private void RunConvention(InternalEntityTypeBuilder entityTypeBuilder) { var context = new ConventionContext(entityTypeBuilder.Metadata.Model.ConventionDispatcher); @@ -144,6 +284,9 @@ private void RunConvention(InternalEntityTypeBuilder entityTypeBuilder) new KeylessEntityTypeAttributeConvention(CreateDependencies()) .ProcessEntityTypeAdded(entityTypeBuilder, context); + + new IndexEntityTypeAttributeConvention(CreateDependencies()) + .ProcessEntityTypeAdded(entityTypeBuilder, context); } private ProviderConventionSetBuilderDependencies CreateDependencies() @@ -189,5 +332,64 @@ private class KeyClash [Key] public int MyId { get; set; } } + + [Index(nameof(A), nameof(B), Name = "IndexOnAAndB", IsUnique = true)] + private class EntityWithIndex + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + [Index(nameof(A), nameof(B), Name = "IndexOnAAndB", IsUnique = true)] + [Index(nameof(B), nameof(C), Name = "IndexOnBAndC", IsUnique = false)] + private class EntityWithTwoIndexes + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + public int C { get; set; } + } + + [Index(nameof(A), null, Name = "IndexOnAAndNull")] + private class EntityWithInvalidNullIndexMember + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + [Index(nameof(A), "", Name = "IndexOnAAndEmpty")] + private class EntityWithInvalidEmptyIndexMember + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + [Index(nameof(A), " \r\n\t", Name = "IndexOnAAndWhiteSpace")] + private class EntityWithInvalidWhiteSpaceIndexMember + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + [Index(nameof(A), nameof(B), Name = "IndexOnAAndUnmappedMember")] + private class EntityWithIgnoredMember + { + public int Id { get; set; } + public int A { get; set; } + [NotMapped] + public int B { get; set; } + } + + [Index(nameof(A), "DoesNotExist", Name = "IndexOnAAndNonExistentMember")] + private class EntityWithNonExistentMember + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } } } diff --git a/test/EFCore.Tests/Metadata/Internal/InternalIndexBuilderTest.cs b/test/EFCore.Tests/Metadata/Internal/InternalIndexBuilderTest.cs index b1336c7f255..c9c0f83c7f0 100644 --- a/test/EFCore.Tests/Metadata/Internal/InternalIndexBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/InternalIndexBuilderTest.cs @@ -43,6 +43,40 @@ public void Can_only_override_existing_IsUnique_value_explicitly() Assert.False(metadata.IsUnique); } + [ConditionalFact] + public void Can_only_override_lower_source_Name() + { + var builder = CreateInternalIndexBuilder(); + var metadata = builder.Metadata; + + Assert.NotNull(builder.HasName("ConventionIndexName", ConfigurationSource.Convention)); + Assert.NotNull(builder.HasName("AnnotationIndexName", ConfigurationSource.DataAnnotation)); + + Assert.Equal("AnnotationIndexName", metadata.Name); + + Assert.Null(builder.HasName("UpdatedConventionIndexName", ConfigurationSource.Convention)); + + Assert.Equal("AnnotationIndexName", metadata.Name); + } + + [ConditionalFact] + public void Can_only_override_existing_Name_value_explicitly() + { + var builder = CreateInternalIndexBuilder(); + var metadata = builder.Metadata; + metadata.Name = "TestIndexName"; + + Assert.Equal(ConfigurationSource.Explicit, metadata.GetConfigurationSource()); + Assert.NotNull(builder.HasName("TestIndexName", ConfigurationSource.DataAnnotation)); + Assert.Null(builder.HasName("NewIndexName", ConfigurationSource.DataAnnotation)); + + Assert.Equal("TestIndexName", metadata.Name); + + Assert.NotNull(builder.HasName("ExplicitIndexName", ConfigurationSource.Explicit)); + + Assert.Equal("ExplicitIndexName", metadata.Name); + } + private InternalIndexBuilder CreateInternalIndexBuilder() { var modelBuilder = new InternalModelBuilder(new Model()); diff --git a/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs b/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs index 303670b1bb6..1f5ff552a2e 100644 --- a/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs +++ b/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs @@ -474,6 +474,9 @@ public override TestIndexBuilder HasAnnotation(string annotation, objec public override TestIndexBuilder IsUnique(bool isUnique = true) => new GenericTestIndexBuilder(IndexBuilder.IsUnique(isUnique)); + public override TestIndexBuilder HasName(string name) + => new GenericTestIndexBuilder(IndexBuilder.HasName(name)); + IndexBuilder IInfrastructure>.Instance => IndexBuilder; } diff --git a/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericTest.cs b/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericTest.cs index 83e521a7d42..ed2f8eca405 100644 --- a/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericTest.cs +++ b/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericTest.cs @@ -459,6 +459,9 @@ public override TestIndexBuilder HasAnnotation(string annotation, objec public override TestIndexBuilder IsUnique(bool isUnique = true) => new NonGenericTestIndexBuilder(IndexBuilder.IsUnique(isUnique)); + public override TestIndexBuilder HasName(string name) + => new NonGenericTestIndexBuilder(IndexBuilder.HasName(name)); + IndexBuilder IInfrastructure.Instance => IndexBuilder; } diff --git a/test/EFCore.Tests/ModelBuilding/ModelBuilderTestBase.cs b/test/EFCore.Tests/ModelBuilding/ModelBuilderTestBase.cs index 7fa1ced5ed8..f7c75be5b46 100644 --- a/test/EFCore.Tests/ModelBuilding/ModelBuilderTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/ModelBuilderTestBase.cs @@ -306,6 +306,7 @@ public abstract class TestIndexBuilder public abstract TestIndexBuilder HasAnnotation(string annotation, object value); public abstract TestIndexBuilder IsUnique(bool isUnique = true); + public abstract TestIndexBuilder HasName(string name); } public abstract class TestPropertyBuilder diff --git a/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs b/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs index 454716c3898..6505d6d0d88 100644 --- a/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs @@ -1195,13 +1195,14 @@ public virtual void Can_add_multiple_indexes() var entityBuilder = modelBuilder.Entity(); var firstIndexBuilder = entityBuilder.HasIndex(ix => ix.Id).IsUnique(); - var secondIndexBuilder = entityBuilder.HasIndex(ix => ix.Name).HasAnnotation("A1", "V1"); + var secondIndexBuilder = entityBuilder.HasIndex(ix => ix.Name).HasName("MyIndex").HasAnnotation("A1", "V1"); var entityType = (IEntityType)model.FindEntityType(typeof(Customer)); Assert.Equal(2, entityType.GetIndexes().Count()); Assert.True(firstIndexBuilder.Metadata.IsUnique); Assert.False(((IIndex)secondIndexBuilder.Metadata).IsUnique); + Assert.Equal("MyIndex", secondIndexBuilder.Metadata.Name); Assert.Equal("V1", secondIndexBuilder.Metadata["A1"]); } From 2c9d3d2de0c45ef0acff09d4976c3dd01a226b87 Mon Sep 17 00:00:00 2001 From: lajones Date: Fri, 22 May 2020 20:29:00 -0700 Subject: [PATCH 2/6] Updated per review comments. --- src/EFCore.Abstractions/IndexAttribute.cs | 36 ++- .../AbstractionsStrings.Designer.cs | 8 + .../Properties/AbstractionsStrings.resx | 3 + .../Design/CSharpSnapshotGenerator.cs | 17 +- .../Internal/CSharpDbContextGenerator.cs | 9 +- .../Internal/CSharpEntityTypeGenerator.cs | 2 +- .../Internal/CSharpModelGenerator.cs | 7 + .../RelationalScaffoldingModelFactory.cs | 2 +- .../IndexInvalidPropertiesEventData.cs | 86 +++++++ .../Diagnostics/RelationalEventId.cs | 28 +++ .../Diagnostics/RelationalLoggerExtensions.cs | 122 +++++++++ .../RelationalLoggingDefinitions.cs | 18 ++ .../Extensions/RelationalIndexExtensions.cs | 9 + .../RelationalModelValidator.cs | 99 ++++---- .../Conventions/SharedTableConvention.cs | 2 +- .../Internal/RelationalIndexExtensions.cs | 4 +- .../Metadata/Internal/RelationalModel.cs | 2 +- .../Properties/RelationalStrings.Designer.cs | 61 +++-- .../Properties/RelationalStrings.resx | 10 +- .../Internal/SqlServerIndexExtensions.cs | 2 +- src/EFCore/Diagnostics/CoreEventId.cs | 28 +++ .../Diagnostics/CoreLoggerExtensions.cs | 97 +++++++ .../IndexInvalidPropertyEventData.cs | 62 +++++ src/EFCore/Diagnostics/LoggingDefinitions.cs | 18 ++ src/EFCore/Extensions/PropertyExtensions.cs | 13 + src/EFCore/Extensions/TupleExtensions.cs | 36 +++ .../Builders/IConventionIndexBuilder.cs | 4 +- src/EFCore/Metadata/Builders/IndexBuilder.cs | 4 +- src/EFCore/Metadata/Builders/IndexBuilder`.cs | 4 +- .../Conventions/IndexAttributeConvention.cs | 96 +++++++ .../IndexEntityTypeAttributeConvention.cs | 77 ------ .../ProviderConventionSetBuilder.cs | 2 +- src/EFCore/Metadata/IConventionIndex.cs | 4 +- src/EFCore/Metadata/IMutableIndex.cs | 4 +- src/EFCore/Properties/CoreStrings.Designer.cs | 72 ++++-- src/EFCore/Properties/CoreStrings.resx | 13 +- src/Shared/Check.cs | 15 ++ .../Migrations/ModelSnapshotSqlServerTest.cs | 4 +- .../RelationalScaffoldingModelFactoryTest.cs | 4 +- .../RelationalModelValidatorTest.cs | 84 +++++-- .../RelationalBuilderExtensionsTest.cs | 6 +- .../RelationalMetadataExtensionsTest.cs | 4 +- .../RelationalEventIdTest.cs | 3 + .../DataAnnotationTestBase.cs | 55 +++- .../UpdatesSqlServerTest.cs | 4 +- .../SqlServerModelValidatorTest.cs | 4 +- .../UpdatesSqliteTest.cs | 2 +- .../Infrastructure/CoreEventIdTest.cs | 3 +- .../Infrastructure/EventIdTestBase.cs | 2 +- .../EntityTypeAttributeConventionTest.cs | 202 --------------- .../IndexAttributeConventionTest.cs | 236 ++++++++++++++++++ 51 files changed, 1233 insertions(+), 456 deletions(-) create mode 100644 src/EFCore.Relational/Diagnostics/IndexInvalidPropertiesEventData.cs create mode 100644 src/EFCore/Diagnostics/IndexInvalidPropertyEventData.cs create mode 100644 src/EFCore/Extensions/TupleExtensions.cs create mode 100644 src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs delete mode 100644 src/EFCore/Metadata/Conventions/IndexEntityTypeAttributeConvention.cs create mode 100644 test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs diff --git a/src/EFCore.Abstractions/IndexAttribute.cs b/src/EFCore.Abstractions/IndexAttribute.cs index 2dc2b219c98..c0e0a083940 100644 --- a/src/EFCore.Abstractions/IndexAttribute.cs +++ b/src/EFCore.Abstractions/IndexAttribute.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using System.Linq; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Utilities; @@ -11,32 +13,46 @@ namespace Microsoft.EntityFrameworkCore /// Specifies an index to be generated in the database. /// [AttributeUsage(AttributeTargets.Class, AllowMultiple = true)] - public class IndexAttribute : Attribute + public sealed class IndexAttribute : Attribute { + private static readonly bool DefaultIsUnique = false; + private bool? _isUnique; + /// /// Initializes a new instance of the class. /// - /// The members which constitute the index, in order (there must be at least one). - public IndexAttribute(params string[] memberNames) + /// The properties which constitute the index, in order (there must be at least one). + public IndexAttribute(params string[] propertyNames) { - Check.NotEmpty(memberNames, nameof(memberNames)); - MemberNames = memberNames; + Check.NotEmpty(propertyNames, nameof(propertyNames)); + Check.HasNoEmptyElements(propertyNames, nameof(propertyNames)); + PropertyNames = propertyNames.ToList(); } /// - /// The members which constitute the index, in order. + /// The properties which constitute the index, in order. /// - public virtual string[] MemberNames { get; } + public List PropertyNames { get; } /// - /// The name of the index in the database. + /// The name of the index. /// - public virtual string Name { get; [param: NotNull] set; } + public string Name { get; [param: NotNull] set; } /// /// Whether the index is unique. /// - public virtual bool IsUnique { get; set; } + public bool IsUnique + { + get => _isUnique ?? DefaultIsUnique; + set => _isUnique = value; + } + + /// + /// Use this method if you want to know the uniqueness of + /// the index or if it was not specified. + /// + public bool? GetIsUnique() => _isUnique; } } diff --git a/src/EFCore.Abstractions/Properties/AbstractionsStrings.Designer.cs b/src/EFCore.Abstractions/Properties/AbstractionsStrings.Designer.cs index 7b47addd73d..f770c85bce8 100644 --- a/src/EFCore.Abstractions/Properties/AbstractionsStrings.Designer.cs +++ b/src/EFCore.Abstractions/Properties/AbstractionsStrings.Designer.cs @@ -37,6 +37,14 @@ public static string CollectionArgumentIsEmpty([CanBeNull] object argumentName) GetString("CollectionArgumentIsEmpty", nameof(argumentName)), argumentName); + /// + /// The collection argument '{argumentName}' must not contain any empty elements. + /// + public static string CollectionArgumentHasEmptyElements([CanBeNull] object argumentName) + => string.Format( + GetString("CollectionArgumentHasEmptyElements", nameof(argumentName)), + argumentName); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Abstractions/Properties/AbstractionsStrings.resx b/src/EFCore.Abstractions/Properties/AbstractionsStrings.resx index f696e70e8e5..7d822b1b27f 100644 --- a/src/EFCore.Abstractions/Properties/AbstractionsStrings.resx +++ b/src/EFCore.Abstractions/Properties/AbstractionsStrings.resx @@ -123,4 +123,7 @@ The collection argument '{argumentName}' must contain at least one element. + + The collection argument '{argumentName}' must not contain any empty elements. + \ No newline at end of file diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index 4fd60ac776b..44fc7a2147f 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs @@ -11,6 +11,7 @@ using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Internal; +using Microsoft.EntityFrameworkCore.Scaffolding.Internal; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Storage.ValueConversion; using Microsoft.EntityFrameworkCore.Utilities; @@ -743,12 +744,12 @@ protected virtual void GenerateIndex( Check.NotNull(index, nameof(index)); Check.NotNull(stringBuilder, nameof(stringBuilder)); + // Note - method names below are meant to be hard-coded + // because old snapshot files will fail if they are changed stringBuilder .AppendLine() .Append(builderName) - .Append(".") - .Append(nameof(EntityTypeBuilder.HasIndex)) - .Append("(") + .Append(".HasIndex(") .Append(string.Join(", ", index.Properties.Select(p => Code.Literal(p.Name)))) .Append(")"); @@ -758,9 +759,7 @@ protected virtual void GenerateIndex( { stringBuilder .AppendLine() - .Append(".") - .Append(nameof(IndexBuilder.HasName)) - .Append("(") + .Append(".HasName(") .Append(Code.Literal(index.Name)) .Append(")"); } @@ -769,9 +768,7 @@ protected virtual void GenerateIndex( { stringBuilder .AppendLine() - .Append(".") - .Append(nameof(IndexBuilder.IsUnique)) - .Append("()"); + .Append(".IsUnique()"); } GenerateIndexAnnotations(index, stringBuilder); @@ -792,7 +789,7 @@ protected virtual void GenerateIndexAnnotations( IgnoreAnnotations( annotations, - RelationalAnnotationNames.TableIndexMappings); + CSharpModelGenerator.IgnoredIndexAnnotations.ToArray()); GenerateFluentApiForAnnotation( ref annotations, RelationalAnnotationNames.Filter, nameof(RelationalIndexBuilderExtensions.HasFilter), stringBuilder); diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs index 22d8944d2c2..941684b3aef 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs @@ -419,7 +419,7 @@ private void GenerateEntityType(IEntityType entityType, bool useDataAnnotations) // if useDataAnnotations is true. if (!useDataAnnotations || index.GetAnnotations().Any( - a => !RelationalAnnotationNames.TableIndexMappings.Equals(a.Name, StringComparison.Ordinal))) + a => !CSharpModelGenerator.IgnoredIndexAnnotations.Contains(a.Name))) { GenerateIndex(index); } @@ -590,13 +590,16 @@ private void GenerateIndex(IIndex index) var annotations = index.GetAnnotations().ToList(); - RemoveAnnotation(ref annotations, RelationalAnnotationNames.TableIndexMappings); + foreach (var annotation in CSharpModelGenerator.IgnoredIndexAnnotations) + { + RemoveAnnotation(ref annotations, annotation); + } if (index.Name != null) { lines.Add( $".{nameof(IndexBuilder.HasName)}" + - $"({_code.Literal(index.GetName())})"); + $"({_code.Literal(index.GetDatabaseName())})"); } if (index.IsUnique) diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs index 6ebbe9c15c1..d24f22b9099 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs @@ -181,7 +181,7 @@ private void GenerateIndexAttributes(IEntityType entityType) // If there are annotations that cannot be represented // using an IndexAttribute then use fluent API instead. if (!index.GetAnnotations().Any( - a => !RelationalAnnotationNames.TableIndexMappings.Equals(a.Name, StringComparison.Ordinal))) + a => !CSharpModelGenerator.IgnoredIndexAnnotations.Contains(a.Name))) { var indexAttribute = new AttributeWriter(nameof(IndexAttribute)); foreach (var property in index.Properties) diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpModelGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpModelGenerator.cs index d1e253a3fca..c0346f7bce4 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpModelGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpModelGenerator.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.IO; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Diagnostics; @@ -128,5 +129,11 @@ public override ScaffoldedModel GenerateModel( return resultingFiles; } + + /// + /// The set of annotations ignored for the purposes of code generation for indexes. + /// + public static IEnumerable IgnoredIndexAnnotations + => new List { RelationalAnnotationNames.TableIndexMappings }; } } diff --git a/src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs b/src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs index 9c0c6363e51..bca1dbcc0ba 100644 --- a/src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs +++ b/src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs @@ -815,7 +815,7 @@ protected virtual IMutableForeignKey VisitForeignKey([NotNull] ModelBuilder mode _reporter.WriteWarning( DesignStrings.ForeignKeyPrincipalEndContainsNullableColumns( foreignKey.DisplayName(), - index.GetName(), + index.GetDatabaseName(), nullablePrincipalProperties.Select(tuple => tuple.column.DisplayName()).ToList() .Aggregate((a, b) => a + "," + b))); diff --git a/src/EFCore.Relational/Diagnostics/IndexInvalidPropertiesEventData.cs b/src/EFCore.Relational/Diagnostics/IndexInvalidPropertiesEventData.cs new file mode 100644 index 00000000000..e8699cc08cb --- /dev/null +++ b/src/EFCore.Relational/Diagnostics/IndexInvalidPropertiesEventData.cs @@ -0,0 +1,86 @@ +// 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.Collections.Generic; +using System.Diagnostics; +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Metadata; + +namespace Microsoft.EntityFrameworkCore.Diagnostics +{ + /// + /// A event payload class for the + /// event. + /// + public class IndexInvalidPropertiesEventData : EventData + { + /// + /// Constructs the event payload for the event. + /// + /// The event definition. + /// A delegate that generates a log message for this event. + /// The entity type on which the index is defined. + /// The name of the index. + /// The names of the properties which define the index. + /// The name of the first property name which causes this event. + /// The tables mapped to the first property. + /// The name of the second property name which causes this event. + /// The tables mapped to the second property. + public IndexInvalidPropertiesEventData( + [NotNull] EventDefinitionBase eventDefinition, + [NotNull] Func messageGenerator, + [NotNull] IEntityType entityType, + [CanBeNull] string indexName, + [NotNull] List indexPropertyNames, + [NotNull] string property1Name, + [NotNull] List<(string Table, string Schema)> tablesMappedToProperty1, + [NotNull] string property2Name, + [NotNull] List<(string Table, string Schema)> tablesMappedToProperty2) + : base(eventDefinition, messageGenerator) + { + EntityType = entityType; + Name = indexName; + PropertyNames = indexPropertyNames; + Property1Name = property1Name; + TablesMappedToProperty1 = tablesMappedToProperty1; + Property2Name = property2Name; + TablesMappedToProperty2 = tablesMappedToProperty2; + } + + /// + /// The entity type on which the index is defined. + /// + public virtual IEntityType EntityType { get; } + + /// + /// The name of the index. + /// + public virtual string Name { get; } + + /// + /// The list of properties which define the index. + /// + public virtual List PropertyNames { get; } + + /// + /// The name of the first property. + /// + public virtual string Property1Name { get; } + + /// + /// The tables mapped to the first property. + /// + public virtual List<(string Table, string Schema)> TablesMappedToProperty1 { get; } + + /// + /// The name of the second property. + /// + public virtual string Property2Name { get; } + + /// + /// The tables mapped to the second property. + /// + public virtual List<(string Table, string Schema)> TablesMappedToProperty2 { get; } + } +} diff --git a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs index 1e788ac7043..43fdd18198b 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs @@ -75,6 +75,8 @@ private enum Id // Model validation events ModelValidationKeyDefaultValueWarning = CoreEventId.RelationalBaseId + 600, BoolWithDefaultWarning, + IndexPropertyNotMappedToAnyTable, + IndexPropertiesMappedToNonOverlappingTables, // Update events BatchReadyForExecution = CoreEventId.RelationalBaseId + 700, @@ -553,6 +555,32 @@ private enum Id /// public static readonly EventId BoolWithDefaultWarning = MakeValidationId(Id.BoolWithDefaultWarning); + /// + /// + /// An index specifies a property which is not mapped to a column in any table. + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId IndexPropertyNotMappedToAnyTable = MakeValidationId(Id.IndexPropertyNotMappedToAnyTable); + + /// + /// + /// An index specifies properties which map to columns on non-overlapping tables. + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId IndexPropertiesMappedToNonOverlappingTables = MakeValidationId(Id.IndexPropertiesMappedToNonOverlappingTables); + private static readonly string _updatePrefix = DbLoggerCategory.Update.Name + "."; private static EventId MakeUpdateId(Id id) => new EventId((int)id, _updatePrefix + id); diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs index 0cc06180ec0..c14f5d26c7e 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs @@ -7,6 +7,7 @@ using System.Data.Common; using System.Diagnostics; using System.Globalization; +using System.Linq; using System.Linq.Expressions; using System.Reflection; using System.Threading; @@ -3600,5 +3601,126 @@ private static string BatchSmallerThanMinBatchSize(EventDefinitionBase definitio var p = (MinBatchSizeEventData)payload; return d.GenerateMessage(p.CommandCount, p.MinBatchSize); } + + /// + /// Logs the event. + /// + /// The diagnostics logger to use. + /// The entity type on which the index is defined. + /// The index on the entity type. + /// The name of the property which is not mapped. + public static void IndexPropertyNotMappedToAnyTable( + [NotNull] this IDiagnosticsLogger diagnostics, + [NotNull] IEntityType entityType, + [NotNull] IIndex index, + [NotNull] string unmappedPropertyName) + { + var definition = RelationalResources.LogIndexPropertyNotMappedToAnyTable(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + index.Name, + entityType.DisplayName(), + index.Properties.Format(), + unmappedPropertyName); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexInvalidPropertyEventData( + definition, + IndexPropertyNotMappedToAnyTable, + entityType, + index.Name, + index.Properties.Select(p => p.Name).ToList(), + unmappedPropertyName); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string IndexPropertyNotMappedToAnyTable(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (IndexInvalidPropertyEventData)payload; + return d.GenerateMessage( + p.Name, + p.EntityType.DisplayName(), + p.PropertyNames.Format(), + p.InvalidPropertyName); + } + + /// + /// Logs the event. + /// + /// The diagnostics logger to use. + /// The entity type on which the index is defined. + /// The index on the entity type. + /// The first property name which is invalid. + /// The tables mapped to the first property. + /// The second property name which is invalid. + /// The tables mapped to the second property. + public static void IndexPropertiesMappedToNonOverlappingTables( + [NotNull] this IDiagnosticsLogger diagnostics, + [NotNull] IEntityType entityType, + [NotNull] IIndex index, + [NotNull] string property1Name, + [NotNull] List<(string Table, string Schema)> tablesMappedToProperty1, + [NotNull] string property2Name, + [NotNull] List<(string Table, string Schema)> tablesMappedToProperty2) + { + var definition = RelationalResources.LogIndexPropertiesMappedToNonOverlappingTables(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + l => l.Log( + definition.Level, + definition.EventId, + definition.MessageFormat, + index.Name, + entityType.DisplayName(), + index.Properties.Format(), + property1Name, + tablesMappedToProperty1.FormatTables(), + property2Name, + tablesMappedToProperty2.FormatTables())); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexInvalidPropertiesEventData( + definition, + IndexPropertiesMappedToNonOverlappingTables, + entityType, + index.Name, + index.Properties.Select(p => p.Name).ToList(), + property1Name, + tablesMappedToProperty1, + property2Name, + tablesMappedToProperty2); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string IndexPropertiesMappedToNonOverlappingTables(EventDefinitionBase definition, EventData payload) + { + var d = (FallbackEventDefinition)definition; + var p = (IndexInvalidPropertiesEventData)payload; + return d.GenerateMessage( + l => l.Log( + d.Level, + d.EventId, + d.MessageFormat, + p.Name, + p.EntityType.DisplayName(), + p.PropertyNames.Format(), + p.Property1Name, + p.TablesMappedToProperty1.FormatTables(), + p.Property2Name, + p.TablesMappedToProperty2.FormatTables())); + } } } diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs index e561e55352a..b51ec167613 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs @@ -366,5 +366,23 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions /// [EntityFrameworkInternal] public EventDefinitionBase LogValueConversionSqlLiteralWarning; + + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase LogIndexPropertyNotMappedToAnyTable; + + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase LogIndexPropertiesMappedToNonOverlappingTables; } } diff --git a/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs b/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs index 6a155615b8f..a272dd9ef03 100644 --- a/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs @@ -24,6 +24,15 @@ public static class RelationalIndexExtensions /// /// The index. /// The name for this index. + public static string GetDatabaseName([NotNull] this IIndex index) + => index.Name ?? index.GetDefaultName(); + + /// + /// Returns the name for this index. + /// + /// The index. + /// The name for this index. + [Obsolete("Use GetDatabaseName() instead")] public static string GetName([NotNull] this IIndex index) => index.Name ?? index.GetDefaultName(); diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 025cb04eae2..5070f396775 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore; @@ -61,7 +62,7 @@ public override void Validate(IModel model, IDiagnosticsLogger @@ -692,7 +693,7 @@ protected virtual void ValidateSharedIndexesCompatibility( foreach (var index in mappedTypes.SelectMany(et => et.GetDeclaredIndexes())) { - var indexName = index.GetName(tableName, schema); + var indexName = index.GetDatabaseName(); if (!indexMappings.TryGetValue(indexName, out var duplicateIndex)) { indexMappings[indexName] = index; @@ -914,71 +915,85 @@ private static string Format(string tableName, string schema) => schema == null ? tableName : schema + "." + tableName; /// - /// Validates that the members of any one index are all mapped to columns on the same table. + /// Validates that the properties of any one index are + /// all mapped to columns on at least one common table. /// /// The model to validate. /// The logger to use. - protected virtual void ValidateIndexMembersOnSameTable( + protected virtual void ValidateIndexProperties( [NotNull] IModel model, [NotNull] IDiagnosticsLogger logger) { Check.NotNull(model, nameof(model)); foreach (var entityType in model.GetEntityTypes()) { - foreach (var index in entityType.GetIndexes()) + foreach (var index in entityType.GetIndexes() + .Where(i => ConfigurationSource.Convention != ((IConventionIndex)i).GetConfigurationSource())) { - (string MemberName, string Table, string Schema) existingTableMapping = (null, null, null); - foreach (var member in index.Properties) + Tuple> firstPropertyTables = null; + Tuple> lastPropertyTables = null; + HashSet<(string Table, string Schema)> overlappingTables = null; + foreach (var property in index.Properties) { //TODO - update to use table-schema override of GetColumn() below when PR #20938 is checked in - var tablesMappedToMember = member.DeclaringEntityType.GetDerivedTypesInclusive() + var tablesMappedToProperty = property.DeclaringEntityType.GetDerivedTypesInclusive() .Select(t => (t.GetTableName(), t.GetSchema())).Distinct() - .Where(n => member.GetColumnName(/* n.Item1, n.Item2 */) != null); - var tablesMappedToMemberCount = tablesMappedToMember.Count(); - if (tablesMappedToMemberCount == 0) + .Where(n => n.Item1 != null && property.GetColumnName(/* n.Item1, n.Item2 */) != null) + .ToList<(string Table, string Schema)>(); + if (tablesMappedToProperty.Count == 0) { - throw new InvalidOperationException( - RelationalStrings.IndexMemberNotMappedToAnyTable( - entityType.DisplayName(), - Format(index.Properties.Select(p => p.Name)), - member.Name)); + logger.IndexPropertyNotMappedToAnyTable( + entityType, + index, + property.Name); + + overlappingTables = null; + break; + } + + if (firstPropertyTables == null) + { + // store off which tables the first member maps to + firstPropertyTables = + new Tuple>(property.Name, tablesMappedToProperty); } - else if (tablesMappedToMemberCount > 1) + else { - throw new InvalidOperationException( - RelationalStrings.IndexMemberMappedToMultipleTables( - entityType.DisplayName(), - Format(index.Properties.Select(p => p.Name)), - member.Name, - Format(tablesMappedToMember.Select(t => FormatTable(t.Item1, t.Item2))))); + // store off which tables the last member we encountered maps to + lastPropertyTables = + new Tuple>(property.Name, tablesMappedToProperty); } - var table = tablesMappedToMember.Single(); - if (existingTableMapping.MemberName == null) + if (overlappingTables == null) { - existingTableMapping = (member.Name, table.Item1, table.Item2); + overlappingTables = new HashSet<(string Table, string Schema)>(tablesMappedToProperty); } - else if (!string.Equals(existingTableMapping.Table, table.Item1, StringComparison.Ordinal) - || !string.Equals(existingTableMapping.Schema, table.Item2, StringComparison.Ordinal)) + else { - throw new InvalidOperationException( - RelationalStrings.IndexMembersOnDifferentTables( - entityType.DisplayName(), - Format(index.Properties.Select(p => p.Name)), - existingTableMapping.MemberName, - FormatTable(existingTableMapping.Table, existingTableMapping.Schema), - member.Name, - FormatTable(table.Item1, table.Item2))); + overlappingTables.IntersectWith(tablesMappedToProperty); + if (overlappingTables.Count == 0) + { + break; + } } } + + if (overlappingTables != null + && overlappingTables.Count == 0) + { + Debug.Assert(firstPropertyTables != null, nameof(firstPropertyTables)); + Debug.Assert(lastPropertyTables != null, nameof(lastPropertyTables)); + + logger.IndexPropertiesMappedToNonOverlappingTables( + entityType, + index, + firstPropertyTables.Item1, + firstPropertyTables.Item2, + lastPropertyTables.Item1, + lastPropertyTables.Item2); + } } } } - - private static string Format(IEnumerable names) - => "{" + string.Join(", ", names.Select(s => "'" + s + "'")) + "}"; - - private static string FormatTable(string table, string schema) - => schema == null ? table : schema + "." + table; } } diff --git a/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs b/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs index effa880329f..8ea5f81f439 100644 --- a/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs @@ -303,7 +303,7 @@ private void TryUniquifyIndexNames( { foreach (var index in entityType.GetDeclaredIndexes()) { - var indexName = index.GetName(tableName, schema); + var indexName = index.GetDatabaseName(); if (!indexes.TryGetValue(indexName, out var otherIndex)) { indexes[indexName] = index; diff --git a/src/EFCore.Relational/Metadata/Internal/RelationalIndexExtensions.cs b/src/EFCore.Relational/Metadata/Internal/RelationalIndexExtensions.cs index 5ca8c309b64..e86c4d0ced1 100644 --- a/src/EFCore.Relational/Metadata/Internal/RelationalIndexExtensions.cs +++ b/src/EFCore.Relational/Metadata/Internal/RelationalIndexExtensions.cs @@ -42,7 +42,7 @@ public static bool AreCompatible( duplicateIndex.Properties.Format(), duplicateIndex.DeclaringEntityType.DisplayName(), index.DeclaringEntityType.GetSchemaQualifiedTableName(), - index.GetName(tableName, schema), + index.GetDatabaseName(), index.Properties.FormatColumns(tableName, schema), duplicateIndex.Properties.FormatColumns(tableName, schema))); } @@ -61,7 +61,7 @@ public static bool AreCompatible( duplicateIndex.Properties.Format(), duplicateIndex.DeclaringEntityType.DisplayName(), index.DeclaringEntityType.GetSchemaQualifiedTableName(), - index.GetName(tableName, schema))); + index.GetDatabaseName())); } return false; diff --git a/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs b/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs index 17288196e23..4f2416ce101 100644 --- a/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs +++ b/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs @@ -475,7 +475,7 @@ private static void PopulateConstraints(Table table) foreach (var index in entityType.GetIndexes()) { - var name = index.GetName(table.Name, table.Schema); + var name = index.GetDatabaseName(); if (!table.Indexes.TryGetValue(name, out var tableIndex)) { var columns = new Column[index.Properties.Count]; diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 69c201531dd..76879fea376 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -873,22 +873,6 @@ public static string IndexMemberNotMappedToAnyTable([CanBeNull] object entityTyp GetString("IndexMemberNotMappedToAnyTable", nameof(entityType), nameof(indexMemberList), nameof(memberName)), entityType, indexMemberList, memberName); - /// - /// An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field with name '{memberName}' is mapped to columns in multiple tables {tableList}. Please use only members mapped to a single table. - /// - public static string IndexMemberMappedToMultipleTables([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName, [CanBeNull] object tableList) - => string.Format( - GetString("IndexMemberMappedToMultipleTables", nameof(entityType), nameof(indexMemberList), nameof(memberName), nameof(tableList)), - entityType, indexMemberList, memberName, tableList); - - /// - /// An index on the entity type '{entityType}' specifies members {indexMemberList}. The member '{memberName1}' is mapped to a column on table '{table1}', whereas the member '{memberName2}' is mapped to a column on table '{table2}'. All index members must map to the same table. - /// - public static string IndexMembersOnDifferentTables([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName1, [CanBeNull] object table1, [CanBeNull] object memberName2, [CanBeNull] object table2) - => string.Format( - GetString("IndexMembersOnDifferentTables", nameof(entityType), nameof(indexMemberList), nameof(memberName1), nameof(table1), nameof(memberName2), nameof(table2)), - entityType, indexMemberList, memberName1, table1, memberName2, table2); - private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); @@ -1826,5 +1810,50 @@ public static EventDefinition LogMigrationAttributeMissingWarning([NotNu return (EventDefinition)definition; } + + /// + /// The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. But the property '{propertyName}' is not mapped to a column in any table. Please use only properties mapped to a column. + /// + public static EventDefinition LogIndexPropertyNotMappedToAnyTable([NotNull] IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertyNotMappedToAnyTable; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertyNotMappedToAnyTable, + () => new EventDefinition( + logger.Options, + RelationalEventId.IndexPropertyNotMappedToAnyTable, + LogLevel.Error, + "RelationalEventId.IndexPropertyNotMappedToAnyTable", + level => LoggerMessage.Define( + level, + RelationalEventId.IndexPropertyNotMappedToAnyTable, + _resourceManager.GetString("LogIndexPropertyNotMappedToAnyTable")))); + } + + return (EventDefinition)definition; + } + + /// + /// The index named '{indexName}' on the entity type '{entityType}' specifies members {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. + /// + public static FallbackEventDefinition LogIndexPropertiesMappedToNonOverlappingTables([NotNull] IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertiesMappedToNonOverlappingTables; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertiesMappedToNonOverlappingTables, + () => new FallbackEventDefinition( + logger.Options, + RelationalEventId.IndexPropertiesMappedToNonOverlappingTables, + LogLevel.Error, + "RelationalEventId.IndexPropertiesMappedToNonOverlappingTables", + _resourceManager.GetString("LogIndexPropertiesMappedToNonOverlappingTables"))); + } + + return (FallbackEventDefinition)definition; + } } } diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 7ee12afaa05..147219f72e5 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -609,10 +609,12 @@ An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field with name '{memberName}' is not mapped to a column in any table. Please use only members mapped to a column. - - An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field with name '{memberName}' is mapped to columns in multiple tables {tableList}. Please use only members mapped to a single table. + + The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. But the property '{propertyName}' is not mapped to a column in any table. Please use only properties mapped to a column. + Error RelationalEventId.IndexPropertyNotMappedToAnyTable string string string string - - An index on the entity type '{entityType}' specifies members {indexMemberList}. The member '{memberName1}' is mapped to a column on table '{table1}', whereas the member '{memberName2}' is mapped to a column on table '{table2}'. All index members must map to the same table. + + The index named '{indexName}' on the entity type '{entityType}' specifies members {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. + Error RelationalEventId.IndexPropertiesMappedToNonOverlappingTables string string string string string string string \ No newline at end of file diff --git a/src/EFCore.SqlServer/Metadata/Internal/SqlServerIndexExtensions.cs b/src/EFCore.SqlServer/Metadata/Internal/SqlServerIndexExtensions.cs index 2640419f7af..c021bffe362 100644 --- a/src/EFCore.SqlServer/Metadata/Internal/SqlServerIndexExtensions.cs +++ b/src/EFCore.SqlServer/Metadata/Internal/SqlServerIndexExtensions.cs @@ -51,7 +51,7 @@ public static bool AreCompatibleForSqlServer( FormatInclude(duplicateIndex, tableName, schema))); } - return false; + return false; } } diff --git a/src/EFCore/Diagnostics/CoreEventId.cs b/src/EFCore/Diagnostics/CoreEventId.cs index 11bdd2cd170..9af974dc6ee 100644 --- a/src/EFCore/Diagnostics/CoreEventId.cs +++ b/src/EFCore/Diagnostics/CoreEventId.cs @@ -105,6 +105,8 @@ private enum Id CollectionWithoutComparer, ConflictingKeylessAndKeyAttributesWarning, PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning, + IndexDefinedOnIgnoredProperty, + IndexDefinedOnNonExistentProperty, // ChangeTracking events DetectChangesStarting = CoreBaseId + 800, @@ -663,6 +665,32 @@ public static readonly EventId InvalidIncludePathError public static readonly EventId PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning = MakeModelValidationId(Id.PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning); + /// + /// + /// An index has specified a property which has been marked [NotMapped] or Ignore(). + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId IndexDefinedOnIgnoredProperty = MakeModelId(Id.IndexDefinedOnIgnoredProperty); + + /// + /// + /// An index has specified a property which does not exist on the entity type or any of its base types. + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId IndexDefinedOnNonExistentProperty = MakeModelId(Id.IndexDefinedOnNonExistentProperty); + private static readonly string _changeTrackingPrefix = DbLoggerCategory.ChangeTracking.Name + "."; private static EventId MakeChangeTrackingId(Id id) => new EventId((int)id, _changeTrackingPrefix + id); diff --git a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs index ed75b4623a8..3339ddfb9e4 100644 --- a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs +++ b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs @@ -3035,5 +3035,102 @@ private static string PossibleIncorrectRequiredNavigationWithQueryFilterInteract p.ForeignKey.PrincipalEntityType.DisplayName(), p.ForeignKey.DeclaringEntityType.DisplayName()); } + + /// + /// Logs the event. + /// + /// The diagnostics logger to use. + /// The entity type on which the index is defined. + /// The on the entity type. + /// The name of the property which is ignored. + public static void IndexDefinedOnIgnoredProperty( + [NotNull] this IDiagnosticsLogger diagnostics, + [NotNull] IEntityType entityType, + [NotNull] IndexAttribute indexAttribute, + [NotNull] string ignoredPropertyName) + { + var definition = CoreResources.LogIndexDefinedOnIgnoredProperty(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + indexAttribute.Name, + entityType.DisplayName(), + indexAttribute.PropertyNames.Format(), + ignoredPropertyName); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexInvalidPropertyEventData( + definition, + IndexDefinedOnIgnoredProperty, + entityType, + indexAttribute.Name, + indexAttribute.PropertyNames, + ignoredPropertyName); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + private static string IndexDefinedOnIgnoredProperty(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (IndexInvalidPropertyEventData)payload; + return d.GenerateMessage( + p.Name, + p.EntityType.DisplayName(), + p.PropertyNames.Format(), + p.InvalidPropertyName); + } + + /// + /// Logs the event. + /// + /// The diagnostics logger to use. + /// The entity type on which the index is defined. + /// The on the entity type. + /// The name of the property which does not exist. + public static void IndexDefinedOnNonExistentProperty( + [NotNull] this IDiagnosticsLogger diagnostics, + [NotNull] IEntityType entityType, + [NotNull] IndexAttribute indexAttribute, + [NotNull] string nonExistentPropertyName) + { + var definition = CoreResources.LogIndexDefinedOnNonExistentProperty(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + indexAttribute.Name, + entityType.DisplayName(), + indexAttribute.PropertyNames.Format(), + nonExistentPropertyName); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexInvalidPropertyEventData( + definition, + IndexDefinedOnNonExistentProperty, + entityType, + indexAttribute.Name, + indexAttribute.PropertyNames, + nonExistentPropertyName); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string IndexDefinedOnNonExistentProperty(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (IndexInvalidPropertyEventData)payload; + return d.GenerateMessage( + p.Name, + p.EntityType.DisplayName(), + p.PropertyNames.Format(), + p.InvalidPropertyName); + } } } diff --git a/src/EFCore/Diagnostics/IndexInvalidPropertyEventData.cs b/src/EFCore/Diagnostics/IndexInvalidPropertyEventData.cs new file mode 100644 index 00000000000..f8dad5769d6 --- /dev/null +++ b/src/EFCore/Diagnostics/IndexInvalidPropertyEventData.cs @@ -0,0 +1,62 @@ +// 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.Collections.Generic; +using System.Diagnostics; +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Metadata; + +namespace Microsoft.EntityFrameworkCore.Diagnostics +{ + /// + /// A event payload class for + /// the events involving an invalid property name on an index. + /// + public class IndexInvalidPropertyEventData : EventData + { + /// + /// Constructs the event payload for the event. + /// + /// The event definition. + /// A delegate that generates a log message for this event. + /// The entity type on which the index is defined. + /// The name of the index. + /// The names of the properties which define the index. + /// The property name which is invalid. + public IndexInvalidPropertyEventData( + [NotNull] EventDefinitionBase eventDefinition, + [NotNull] Func messageGenerator, + [NotNull] IEntityType entityType, + [CanBeNull] string indexName, + [NotNull] List indexPropertyNames, + [NotNull] string invalidPropertyName) + : base(eventDefinition, messageGenerator) + { + EntityType = entityType; + Name = indexName; + PropertyNames = indexPropertyNames; + InvalidPropertyName = invalidPropertyName; + } + + /// + /// The entity type on which the index is defined. + /// + public virtual IEntityType EntityType { get; } + + /// + /// The name of the index. + /// + public virtual string Name { get; } + + /// + /// The list of properties which define the index. + /// + public virtual List PropertyNames { get; } + + /// + /// The name of the invalid property. + /// + public virtual string InvalidPropertyName { get; } + } +} diff --git a/src/EFCore/Diagnostics/LoggingDefinitions.cs b/src/EFCore/Diagnostics/LoggingDefinitions.cs index 149a7606de6..5960d64db0a 100644 --- a/src/EFCore/Diagnostics/LoggingDefinitions.cs +++ b/src/EFCore/Diagnostics/LoggingDefinitions.cs @@ -663,5 +663,23 @@ public abstract class LoggingDefinitions /// [EntityFrameworkInternal] public EventDefinitionBase LogPossibleIncorrectRequiredNavigationWithQueryFilterInteraction; + + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase LogIndexDefinedOnIgnoredProperty; + + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase LogIndexDefinedOnNonExistentProperty; } } diff --git a/src/EFCore/Extensions/PropertyExtensions.cs b/src/EFCore/Extensions/PropertyExtensions.cs index f96879def4b..a3232053cc4 100644 --- a/src/EFCore/Extensions/PropertyExtensions.cs +++ b/src/EFCore/Extensions/PropertyExtensions.cs @@ -400,6 +400,19 @@ public static string Format([NotNull] this IEnumerable properties p => "'" + p.Name + "'" + (includeTypes ? " : " + p.ClrType.DisplayName(fullName: false) : ""))) + "}"; + /// + /// Creates a formatted string representation of the given + /// property names such as is useful when throwing exceptions. + /// + /// The names of the properties to format. + /// The string representation. + public static string Format([NotNull] this IEnumerable propertyNames) + => "{" + + string.Join( + ", ", + propertyNames.Select(name => "'" + name + "'")) + + "}"; + /// /// /// Creates a human-readable representation of the given metadata. diff --git a/src/EFCore/Extensions/TupleExtensions.cs b/src/EFCore/Extensions/TupleExtensions.cs new file mode 100644 index 00000000000..843f1c56709 --- /dev/null +++ b/src/EFCore/Extensions/TupleExtensions.cs @@ -0,0 +1,36 @@ +// 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.Collections.Generic; +using System.Linq; +using JetBrains.Annotations; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore +{ + /// + /// Extension methods for tuples. + /// + public static class TupleExtensions + { + /// + /// Creates a formatted string representation of the given tables such as is useful + /// when throwing exceptions. + /// + /// The (Table, Schema) tuples to format. + /// The string representation. + public static string FormatTables([NotNull] this IEnumerable<(string Table, string Schema)> tables) + => "{" + + string.Join(", ", tables.Select(FormatTable)) + + "}"; + + /// + /// Creates a formatted string representation of the given table such as is useful + /// when throwing exceptions. + /// + /// The (Table, Schema) tuple to format. + /// The string representation. + public static string FormatTable(this (string Table, string Schema) table) + => table.Schema == null ? table.Table : table.Schema + "." + table.Table; + } +} diff --git a/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs b/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs index 898a9478f28..9dce6b218d6 100644 --- a/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs +++ b/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs @@ -44,8 +44,8 @@ public interface IConventionIndexBuilder : IConventionAnnotatableBuilder /// /// Configures the name of this index. /// - /// The name of the index (can be - /// to indicate that a unique name should be generated). + /// The name of the index which can be + /// to indicate that a unique name should be generated. /// Indicates whether the configuration was specified using a data annotation. /// /// The same builder instance if the name is unchanged, diff --git a/src/EFCore/Metadata/Builders/IndexBuilder.cs b/src/EFCore/Metadata/Builders/IndexBuilder.cs index 15bdf45a082..6b0eca95631 100644 --- a/src/EFCore/Metadata/Builders/IndexBuilder.cs +++ b/src/EFCore/Metadata/Builders/IndexBuilder.cs @@ -78,8 +78,8 @@ public virtual IndexBuilder IsUnique(bool unique = true) /// Configures the name of this index. /// /// - /// The name of this index (can be - /// to indicate that a unique name should be generated). + /// The name of this index which can be + /// to indicate that a unique name should be generated. /// /// The same builder instance so that multiple configuration calls can be chained. public virtual IndexBuilder HasName([CanBeNull] string name) diff --git a/src/EFCore/Metadata/Builders/IndexBuilder`.cs b/src/EFCore/Metadata/Builders/IndexBuilder`.cs index 4217ddcd12f..cc5b1fe290c 100644 --- a/src/EFCore/Metadata/Builders/IndexBuilder`.cs +++ b/src/EFCore/Metadata/Builders/IndexBuilder`.cs @@ -53,8 +53,8 @@ public IndexBuilder([NotNull] IMutableIndex index) /// Configures the name of this index. /// /// - /// The name of this index (can be - /// to indicate that a unique name should be generated). + /// The name of this index which can be + /// to indicate that a unique name should be generated. /// /// The same builder instance so that multiple configuration calls can be chained. public new virtual IndexBuilder HasName([CanBeNull] string name) diff --git a/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs b/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs new file mode 100644 index 00000000000..eea98f3df90 --- /dev/null +++ b/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs @@ -0,0 +1,96 @@ +// 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.Collections.Generic; +using System.Linq; +using System.Reflection; +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Metadata.Builders; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; + +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions +{ + /// + /// A convention that configures database indexes based on the . + /// + public class IndexAttributeConvention : IModelFinalizingConvention + { + /// + /// Creates a new instance of . + /// + /// Parameter object containing dependencies for this convention. + public IndexAttributeConvention([NotNull] ProviderConventionSetBuilderDependencies dependencies) + { + Dependencies = dependencies; + } + + /// + /// Parameter object containing service dependencies. + /// + protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; } + + /// + public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, IConventionContext context) + { + foreach (var entityType in modelBuilder.Metadata.GetEntityTypes()) + { + if (entityType.ClrType != null) + { + var ignoredMembers = entityType.GetIgnoredMembers(); + foreach (var indexAttribute in + entityType.ClrType.GetCustomAttributes(true)) + { + var shouldGenerate = true; + var indexProperties = new List(); + foreach (var propertyName in indexAttribute.PropertyNames) + { + if (ignoredMembers.Contains(propertyName)) + { + Dependencies.Logger + .IndexDefinedOnIgnoredProperty( + entityType, + indexAttribute, + propertyName); + shouldGenerate = false; + break; + } + + var property = entityType.FindProperty(propertyName); + if (property == null) + { + Dependencies.Logger + .IndexDefinedOnNonExistentProperty( + entityType, + indexAttribute, + propertyName); + shouldGenerate = false; + break; + } + + indexProperties.Add(property); + } + + if (shouldGenerate) + { + var indexBuilder = entityType.Builder.HasIndex(indexProperties, fromDataAnnotation: true); + if (indexBuilder != null) + { + if (indexAttribute.Name != null) + { + indexBuilder.HasName(indexAttribute.Name, fromDataAnnotation: true); + } + + if (indexAttribute.GetIsUnique().HasValue) + { + indexBuilder.IsUnique(indexAttribute.GetIsUnique().Value, fromDataAnnotation: true); + } + } + } + } + } + } + } + } +} diff --git a/src/EFCore/Metadata/Conventions/IndexEntityTypeAttributeConvention.cs b/src/EFCore/Metadata/Conventions/IndexEntityTypeAttributeConvention.cs deleted file mode 100644 index 94ecdbf2a90..00000000000 --- a/src/EFCore/Metadata/Conventions/IndexEntityTypeAttributeConvention.cs +++ /dev/null @@ -1,77 +0,0 @@ -// 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.Collections.Generic; -using System.Linq; -using JetBrains.Annotations; -using Microsoft.EntityFrameworkCore.Diagnostics; -using Microsoft.EntityFrameworkCore.Metadata.Builders; -using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; - -namespace Microsoft.EntityFrameworkCore.Metadata.Conventions -{ - /// - /// A convention that configures database indexes based on the . - /// - public class IndexEntityTypeAttributeConvention : EntityTypeAttributeConventionBase - { - /// - /// Creates a new instance of . - /// - /// Parameter object containing dependencies for this convention. - public IndexEntityTypeAttributeConvention([NotNull] ProviderConventionSetBuilderDependencies dependencies) - : base(dependencies) - { - } - - /// - protected override void ProcessEntityTypeAdded( - IConventionEntityTypeBuilder entityTypeBuilder, - IndexAttribute attribute, - IConventionContext context) - { - var entityType = entityTypeBuilder.Metadata; - var ignoredMembers = entityType.GetIgnoredMembers(); - var indexProperties = new List(); - foreach (var memberName in attribute.MemberNames) - { - if (string.IsNullOrWhiteSpace(memberName)) - { - throw new InvalidOperationException( - CoreStrings.IndexMemberNameEmpty( - entityType.DisplayName(), - Format(attribute.MemberNames))); - } - - if (ignoredMembers.Contains(memberName)) - { - throw new InvalidOperationException( - CoreStrings.IndexMemberIsIgnored( - entityType.DisplayName(), - Format(attribute.MemberNames), - memberName)); - } - - var member = entityType.FindProperty(memberName); - if (member == null) - { - throw new InvalidOperationException( - CoreStrings.IndexMemberHasNoMatchingMember( - entityType.DisplayName(), - Format(attribute.MemberNames), - memberName)); - } - - indexProperties.Add(member); - } - - var indexBuilder = entityTypeBuilder.HasIndex(indexProperties, fromDataAnnotation: true); - indexBuilder.HasName(attribute.Name, fromDataAnnotation: true); - indexBuilder.IsUnique(attribute.IsUnique, fromDataAnnotation: true); - } - - private static string Format(string[] memberNames) - => "{" + string.Join(", ", memberNames.Select(s => "'" + s + "'")) + "}"; - } -} diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index 342ea1a80c6..f1353edf8fd 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -73,7 +73,6 @@ public virtual ConventionSet CreateConventionSet() conventionSet.EntityTypeAddedConventions.Add(inversePropertyAttributeConvention); conventionSet.EntityTypeAddedConventions.Add(relationshipDiscoveryConvention); conventionSet.EntityTypeAddedConventions.Add(new DerivedTypeDiscoveryConvention(Dependencies)); - conventionSet.EntityTypeAddedConventions.Add(new IndexEntityTypeAttributeConvention(Dependencies)); conventionSet.EntityTypeIgnoredConventions.Add(inversePropertyAttributeConvention); conventionSet.EntityTypeIgnoredConventions.Add(relationshipDiscoveryConvention); @@ -182,6 +181,7 @@ 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/IConventionIndex.cs b/src/EFCore/Metadata/IConventionIndex.cs index 2d29e824de5..3b50dedbb85 100644 --- a/src/EFCore/Metadata/IConventionIndex.cs +++ b/src/EFCore/Metadata/IConventionIndex.cs @@ -56,8 +56,8 @@ public interface IConventionIndex : IIndex, IConventionAnnotatable ConfigurationSource? GetIsUniqueConfigurationSource(); /// - /// Sets the name of the index (can be - /// to indicate that a unique name should be generated). + /// Sets the name of the index which can be + /// to indicate that a unique name should be generated. /// /// The name of the index. /// Indicates whether the configuration was specified using a data annotation. diff --git a/src/EFCore/Metadata/IMutableIndex.cs b/src/EFCore/Metadata/IMutableIndex.cs index eab197ca895..cbd341409e9 100644 --- a/src/EFCore/Metadata/IMutableIndex.cs +++ b/src/EFCore/Metadata/IMutableIndex.cs @@ -23,8 +23,8 @@ public interface IMutableIndex : IIndex, IMutableAnnotatable new bool IsUnique { get; set; } /// - /// Gets or sets the name of the index (can be - /// to indicate that a unique name should be generated). + /// Gets or sets the name of the index which can be + /// to indicate that a unique name should be generated. /// new string Name { get; [param: CanBeNull] set; } diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index a0a05fe7364..56f68b45084 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -2658,30 +2658,6 @@ public static string MissingInverseManyToManyNavigation([CanBeNull] object princ public static string QueryContextAlreadyInitializedStateManager => GetString("QueryContextAlreadyInitializedStateManager"); - /// - /// An index on the entity type '{entityType}' specifies members {indexMemberList}. Member names must not be null or empty or consist only of white-space characters. - /// - public static string IndexMemberNameEmpty([CanBeNull] object entityType, [CanBeNull] object indexMemberList) - => string.Format( - GetString("IndexMemberNameEmpty", nameof(entityType), nameof(indexMemberList)), - entityType, indexMemberList); - - /// - /// An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field '{memberName}' has been marked NotMapped or Ignore(). An index cannot use such members. - /// - public static string IndexMemberIsIgnored([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName) - => string.Format( - GetString("IndexMemberIsIgnored", nameof(entityType), nameof(indexMemberList), nameof(memberName)), - entityType, indexMemberList, memberName); - - /// - /// An index on the entity type '{entityType}' specifies members {indexMemberList}. But no property or field with name '{memberName}' exists on that entity type or any of its base types. - /// - public static string IndexMemberHasNoMatchingMember([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName) - => string.Format( - GetString("IndexMemberHasNoMatchingMember", nameof(entityType), nameof(indexMemberList), nameof(memberName)), - entityType, indexMemberList, memberName); - private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); @@ -4291,5 +4267,53 @@ public static EventDefinition LogPossibleIncorrectRequiredNaviga return (EventDefinition)definition; } + + /// + /// The index named '{indexName}' on the entity type '{entityType}' with properties {indexPropertyList} is invalid. The property '{propertyName}' has been marked NotMapped or Ignore(). An index cannot use such properties. + /// + public static EventDefinition LogIndexDefinedOnIgnoredProperty([NotNull] IDiagnosticsLogger logger) + { + var definition = ((LoggingDefinitions)logger.Definitions).LogIndexDefinedOnIgnoredProperty; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((LoggingDefinitions)logger.Definitions).LogIndexDefinedOnIgnoredProperty, + () => new EventDefinition( + logger.Options, + CoreEventId.IndexDefinedOnIgnoredProperty, + LogLevel.Error, + "CoreEventId.IndexDefinedOnIgnoredProperty", + level => LoggerMessage.Define( + level, + CoreEventId.IndexDefinedOnIgnoredProperty, + _resourceManager.GetString("LogIndexDefinedOnIgnoredProperty")))); + } + + return (EventDefinition)definition; + } + + /// + /// An index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertyList}. But no property with name '{propertyName}' exists on that entity type or any of its base types. + /// + public static EventDefinition LogIndexDefinedOnNonExistentProperty([NotNull] IDiagnosticsLogger logger) + { + var definition = ((LoggingDefinitions)logger.Definitions).LogIndexDefinedOnNonExistentProperty; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((LoggingDefinitions)logger.Definitions).LogIndexDefinedOnNonExistentProperty, + () => new EventDefinition( + logger.Options, + CoreEventId.IndexDefinedOnNonExistentProperty, + LogLevel.Error, + "CoreEventId.IndexDefinedOnNonExistentProperty", + level => LoggerMessage.Define( + level, + CoreEventId.IndexDefinedOnNonExistentProperty, + _resourceManager.GetString("LogIndexDefinedOnNonExistentProperty")))); + } + + return (EventDefinition)definition; + } } } diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index a7074bdfe2b..e86dd9d6dbd 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1403,13 +1403,12 @@ InitializeStateManager method has been called multiple times on current query context. This method is intended to be called only once before query enumeration starts. - - An index on the entity type '{entityType}' specifies members {indexMemberList}. Member names must not be null or empty or consist only of white-space characters. + + The index named '{indexName}' on the entity type '{entityType}' with properties {indexPropertyList} is invalid. The property '{propertyName}' has been marked NotMapped or Ignore(). An index cannot use such properties. + Error CoreEventId.IndexDefinedOnIgnoredProperty string string string string - - An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field '{memberName}' has been marked NotMapped or Ignore(). An index cannot use such members. - - - An index on the entity type '{entityType}' specifies members {indexMemberList}. But no property or field with name '{memberName}' exists on that entity type or any of its base types. + + An index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertyList}. But no property with name '{propertyName}' exists on that entity type or any of its base types. + Error CoreEventId.IndexDefinedOnNonExistentProperty string string string string \ No newline at end of file diff --git a/src/Shared/Check.cs b/src/Shared/Check.cs index c974f3e3915..962ebca2d74 100644 --- a/src/Shared/Check.cs +++ b/src/Shared/Check.cs @@ -94,6 +94,21 @@ public static IReadOnlyList HasNoNulls(IReadOnlyList value, [InvokerPar return value; } + + public static IReadOnlyList HasNoEmptyElements(IReadOnlyList value, [InvokerParameterName][NotNull] string parameterName) + { + NotNull(value, parameterName); + + if (value.Any(s => string.IsNullOrWhiteSpace(s))) + { + NotEmpty(parameterName, nameof(parameterName)); + + throw new ArgumentException(AbstractionsStrings.CollectionArgumentHasEmptyElements(parameterName)); + } + + return value; + } + [Conditional("DEBUG")] public static void DebugAssert([System.Diagnostics.CodeAnalysis.DoesNotReturnIf(false)] bool condition, string message) { diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index c0a7b972c92..c2a815b404f 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -2526,7 +2526,7 @@ public virtual void Index_with_default_constraint_name_exceeding_max() b.ToTable(""EntityWithStringProperty""); });"), - model => Assert.Equal(128, model.GetEntityTypes().First().GetIndexes().First().GetName().Length)); + model => Assert.Equal(128, model.GetEntityTypes().First().GetIndexes().First().GetDatabaseName().Length)); } #endregion @@ -2876,7 +2876,7 @@ public virtual void ForeignKey_name_preserved_when_generic() var originalIndex = originalChild.FindIndex(originalChild.FindProperty("Property")); var index = child.FindIndex(child.FindProperty("Property")); - Assert.Equal(originalIndex.GetName(), index.GetName()); + Assert.Equal(originalIndex.GetDatabaseName(), index.GetDatabaseName()); }); } diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs index 710be4a42a3..88699a5055a 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs @@ -464,7 +464,7 @@ public void Unique_constraint() var index = entityType.GetIndexes().Single(); Assert.True(index.IsUnique); - Assert.Equal("MyUniqueConstraint", index.GetName()); + Assert.Equal("MyUniqueConstraint", index.GetDatabaseName()); Assert.Same(entityType.FindProperty("MyColumn"), index.Properties.Single()); } @@ -530,7 +530,7 @@ public void Indexes_and_alternate_keys() indexColumn1 => { Assert.False(indexColumn1.IsUnique); - Assert.Equal("IDX_C1", indexColumn1.GetName()); + Assert.Equal("IDX_C1", indexColumn1.GetDatabaseName()); Assert.Same(entityType.FindProperty("C1"), indexColumn1.Properties.Single()); }, uniqueColumn2 => diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index c62f7a14e0b..46f8d894f8f 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -692,7 +692,7 @@ public virtual void Detects_duplicate_foreignKey_names_within_hierarchy_with_dif var index1 = fk1.DeclaringEntityType.GetDeclaredIndexes().Single(); var index2 = fk2.DeclaringEntityType.GetDeclaredIndexes().Single(); Assert.NotSame(index1, index2); - Assert.NotEqual(index1.GetName(), index2.GetName()); + Assert.NotEqual(index1.GetDatabaseName(), index2.GetDatabaseName()); } [ConditionalFact] @@ -732,7 +732,7 @@ public virtual void Passes_for_incompatible_foreignKeys_within_hierarchy() var index1 = fk1.DeclaringEntityType.GetDeclaredIndexes().Single(); var index2 = fk2.DeclaringEntityType.GetDeclaredIndexes().Single(); Assert.NotSame(index1, index2); - Assert.Equal(index1.GetName(), index2.GetName()); + Assert.Equal(index1.GetDatabaseName(), index2.GetDatabaseName()); } [ConditionalFact] @@ -753,7 +753,7 @@ public virtual void Passes_for_incompatible_foreignKeys_within_hierarchy_when_on var index1 = fk1.DeclaringEntityType.GetDeclaredIndexes().Single(); var index2 = fk2.DeclaringEntityType.GetDeclaredIndexes().Single(); Assert.NotSame(index1, index2); - Assert.Equal(index1.GetName(), index2.GetName()); + Assert.Equal(index1.GetDatabaseName(), index2.GetDatabaseName()); } [ConditionalFact] @@ -799,7 +799,7 @@ public virtual void Passes_for_compatible_duplicate_foreignKey_names_within_hier var index1 = fk1.DeclaringEntityType.GetDeclaredIndexes().Single(); var index2 = fk2.DeclaringEntityType.GetDeclaredIndexes().Single(); Assert.NotSame(index1, index2); - Assert.Equal(index1.GetName(), index2.GetName()); + Assert.Equal(index1.GetDatabaseName(), index2.GetDatabaseName()); } [ConditionalFact] @@ -847,7 +847,7 @@ public virtual void Passes_for_compatible_duplicate_foreignKey_names_within_hier var index1 = fk1.DeclaringEntityType.GetDeclaredIndexes().Single(); var index2 = fk2.DeclaringEntityType.GetDeclaredIndexes().Single(); Assert.NotSame(index1, index2); - Assert.Equal(index1.GetName(), index2.GetName()); + Assert.Equal(index1.GetDatabaseName(), index2.GetDatabaseName()); } [ConditionalFact] @@ -946,8 +946,8 @@ public virtual void Passes_for_incompatible_indexes_within_hierarchy_when_one_na Validate(modelBuilder.Model); - Assert.Equal("IX_Animal_Name", index1.GetName()); - Assert.Equal("IX_Animal_Name1", index2.GetName()); + Assert.Equal("IX_Animal_Name", index1.GetDatabaseName()); + Assert.Equal("IX_Animal_Name1", index2.GetDatabaseName()); } [ConditionalFact] @@ -973,7 +973,7 @@ public virtual void Passes_for_compatible_duplicate_index_names_within_hierarchy Validate(modelBuilder.Model); Assert.NotSame(index1, index2); - Assert.Equal(index1.GetName(), index2.GetName()); + Assert.Equal(index1.GetDatabaseName(), index2.GetDatabaseName()); } [ConditionalFact] @@ -1248,36 +1248,68 @@ var methodInfo modelBuilder.Model); } - [ConditionalFact(Skip = "Needs TPT to run")] //TODO - awaiting PR 20938 - public void Detects_index_member_not_mapped_to_any_table() + [ConditionalFact] + public void Detects_index_property_not_mapped_to_any_table() { - // detect RelationalStrings.IndexMemberNotMappedToAnyTable exception + var modelBuilder = CreateConventionalModelBuilder(); + + modelBuilder.Entity().ToTable(null); + modelBuilder.Entity().HasIndex(nameof(Animal.Name)); + + var definition = RelationalResources + .LogIndexPropertyNotMappedToAnyTable( + new TestLogger()); + VerifyWarning( + definition.GenerateMessage( + "(null)", + nameof(Animal), + "{'Name'}", + "Name"), + modelBuilder.Model, + LogLevel.Error); } - [ConditionalFact(Skip = "Needs TPT to run")] //TODO - awaiting PR 20938 - public void Detects_index_member_mapped_to_multiple_tables() + [ConditionalFact(Skip = "Needs TPT (PR 20938) to run")] //TODO - awaiting PR 20938 + public void Passes_for_index_properties_mapped_to_same_table_in_TPT_hierarchy() { - // detect RelationalStrings.IndexMemberMappedToMultipleTables exception + var modelBuilder = CreateConventionalModelBuilder(); + + modelBuilder.Entity().ToTable("Animals"); + modelBuilder.Entity().ToTable("Cats"); + modelBuilder.Entity().HasIndex(nameof(Animal.Id), nameof(Cat.Identity)); + + Validate(modelBuilder.Model); + + Assert.Empty(LoggerFactory.Log); } - [ConditionalFact(Skip = "Needs TPT to run")] //TODO - awaiting PR 20938 - public void Detects_index_members_mapped_to_different_tables() + [ConditionalFact(Skip = "Needs TPT (PR 20938) to run")] //TODO - awaiting PR 20938 + public void Detects_index_properties_mapped_to_different_tables_in_TPT_hierarchy() { var modelBuilder = CreateConventionalModelBuilder(); modelBuilder.Entity().ToTable("Animals"); modelBuilder.Entity().ToTable("Cats"); - modelBuilder.Entity().HasIndex(nameof(Animal.Id), nameof(Cat.Identity)); + modelBuilder.Entity().HasIndex(nameof(Animal.Name), nameof(Cat.Identity)); - VerifyError( - RelationalStrings.IndexMembersOnDifferentTables( - nameof(Cat), - "{'Id', 'Identity'}", - nameof(Animal.Id), - "Animals", - nameof(Cat.Identity), - "Cats"), - modelBuilder.Model); + var definition = RelationalResources + .LogIndexPropertiesMappedToNonOverlappingTables( + new TestLogger()); + VerifyWarning( + definition.GenerateMessage( + l => l.Log( + definition.Level, + definition.EventId, + definition.MessageFormat, + "(null)", + nameof(Cat), + "{'Name', 'Identity'}", + nameof(Animal.Name), + "{'Animals'}", + nameof(Cat.Identity), + "{'Cats'}")), + modelBuilder.Model, + LogLevel.Error); } private static void GenerateMapping(IMutableProperty property) diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalBuilderExtensionsTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalBuilderExtensionsTest.cs index b642aec703c..de5bd1d3ad8 100644 --- a/test/EFCore.Relational.Tests/Metadata/RelationalBuilderExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/Metadata/RelationalBuilderExtensionsTest.cs @@ -405,14 +405,14 @@ public void Default_index_name_is_based_on_index_column_names() var index = modelBuilder.Model.FindEntityType(typeof(Customer)).GetIndexes().Single(); - Assert.Equal("IX_Customer_Id", index.GetName()); + Assert.Equal("IX_Customer_Id", index.GetDatabaseName()); modelBuilder .Entity() .Property(e => e.Id) .HasColumnName("Eendax"); - Assert.Equal("IX_Customer_Eendax", index.GetName()); + Assert.Equal("IX_Customer_Eendax", index.GetDatabaseName()); } [ConditionalFact] @@ -427,7 +427,7 @@ public void Can_set_index_name() var index = modelBuilder.Model.FindEntityType(typeof(Customer)).GetIndexes().Single(); - Assert.Equal("Eeeendeeex", index.GetName()); + Assert.Equal("Eeeendeeex", index.GetDatabaseName()); } [ConditionalFact] diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalMetadataExtensionsTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalMetadataExtensionsTest.cs index 8564c595066..76d4fe3a172 100644 --- a/test/EFCore.Relational.Tests/Metadata/RelationalMetadataExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/Metadata/RelationalMetadataExtensionsTest.cs @@ -346,17 +346,17 @@ public void Can_get_and_set_index_name() .HasIndex(e => e.Id) .Metadata; +#pragma warning disable CS0618 // Type or member is obsolete Assert.Equal("IX_Customer_Id", index.GetName()); -#pragma warning disable CS0618 // Type or member is obsolete index.SetName("MyIndex"); Assert.Equal("MyIndex", index.GetName()); index.SetName(null); -#pragma warning restore CS0618 // Type or member is obsolete Assert.Equal("IX_Customer_Id", index.GetName()); +#pragma warning restore CS0618 // Type or member is obsolete } [ConditionalFact] diff --git a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs index b690f3c57af..40cc715b1f7 100644 --- a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs +++ b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs @@ -40,6 +40,7 @@ public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() var property = new Property( "A", typeof(int), null, null, entityType, ConfigurationSource.Convention, ConfigurationSource.Convention); var contextServices = RelationalTestHelpers.Instance.CreateContextServices(model.FinalizeModel()); + var index = new Metadata.Internal.Index(new List { property }, entityType, ConfigurationSource.Convention); var fakeFactories = new Dictionary> { @@ -65,7 +66,9 @@ public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() { typeof(IMigrationsAssembly), () => new FakeMigrationsAssembly() }, { typeof(MethodCallExpression), () => Expression.Call(constantExpression, typeof(object).GetMethod("ToString")) }, { typeof(Expression), () => constantExpression }, + { typeof(IEntityType), () => entityType }, { typeof(IProperty), () => property }, + { typeof(IIndex), () => index }, { typeof(TypeInfo), () => typeof(object).GetTypeInfo() }, { typeof(Type), () => typeof(object) }, { typeof(ValueConverter), () => new BoolToZeroOneConverter() }, diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs index 74cbdcf95ff..72e06c796d2 100644 --- a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs +++ b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs @@ -808,6 +808,40 @@ public virtual void Key_fluent_api_and_keyless_attribute_do_not_cause_warning() Assert.Empty(Fixture.ListLoggerFactory.Log); } + [ConditionalFact] + public virtual void IndexAttribute_with_an_ignored_property_causes_error() + { + var modelBuilder = CreateModelBuilder(); + var entity = modelBuilder.Entity(); + modelBuilder.Model.FinalizeModel(); + + var logEntry = Fixture.ListLoggerFactory.Log.Single(); + Assert.Equal(LogLevel.Error, logEntry.Level); + Assert.Equal( + CoreResources.LogIndexDefinedOnIgnoredProperty(new TestLogger()) + .GenerateMessage("(null)", nameof(EntityWithIgnoredProperty), "{'A', 'B'}", "B"), + logEntry.Message); + } + + [ConditionalFact] + public virtual void IndexAttribute_with_a_non_existent_property_causes_error() + { + var modelBuilder = CreateModelBuilder(); + var entity = modelBuilder.Entity(); + modelBuilder.Model.FinalizeModel(); + + var logEntry = Fixture.ListLoggerFactory.Log.Single(); + Assert.Equal(LogLevel.Error, logEntry.Level); + Assert.Equal( + CoreResources.LogIndexDefinedOnNonExistentProperty(new TestLogger()) + .GenerateMessage( + "IndexOnAAndNonExistentProperty", + nameof(EntityWithNonExistentProperty), + "{'A', 'DoesNotExist'}", + "DoesNotExist"), + logEntry.Message); + } + private class CompositeKeyAttribute { [Key] @@ -2373,7 +2407,9 @@ public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder build .Log(CoreEventId.ConflictingForeignKeyAttributesOnNavigationAndPropertyWarning) .Log(CoreEventId.ForeignKeyAttributesOnBothNavigationsWarning) .Log(CoreEventId.ForeignKeyAttributesOnBothPropertiesWarning) - .Log(CoreEventId.ConflictingKeylessAndKeyAttributesWarning)); + .Log(CoreEventId.ConflictingKeylessAndKeyAttributesWarning) + .Log(CoreEventId.IndexDefinedOnIgnoredProperty) + .Log(CoreEventId.IndexDefinedOnNonExistentProperty)); protected override bool ShouldLogCategory(string logCategory) => logCategory == DbLoggerCategory.Model.Name; @@ -2470,5 +2506,22 @@ protected class KeyFluentApiAndKeylessAttribute { public int MyKey { get; set; } } + + [Index(nameof(A), nameof(B))] + private class EntityWithIgnoredProperty + { + public int Id { get; set; } + public int A { get; set; } + [NotMapped] + public int B { get; set; } + } + + [Index(nameof(A), "DoesNotExist", Name = "IndexOnAAndNonExistentProperty")] + private class EntityWithNonExistentProperty + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs index 7232e64683b..1c491026abc 100644 --- a/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs @@ -68,7 +68,7 @@ public override void Identifiers_are_generated_correctly() entityType.GetForeignKeys().Single().GetConstraintName()); Assert.Equal( "IX_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWork~", - entityType.GetIndexes().Single().GetName()); + entityType.GetIndexes().Single().GetDatabaseName()); var entityType2 = context.Model.FindEntityType( typeof( @@ -89,7 +89,7 @@ public override void Identifiers_are_generated_correctly() entityType2.GetProperties().ElementAt(2).GetColumnName()); Assert.Equal( "IX_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWor~1", - entityType2.GetIndexes().Single().GetName()); + entityType2.GetIndexes().Single().GetDatabaseName()); } private void AssertSql(params string[] expected) diff --git a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs index a06df8f44ea..c68e77fbcf8 100644 --- a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs +++ b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs @@ -231,8 +231,8 @@ public virtual void Passes_for_compatible_duplicate_convention_indexes_for_forei var model = Validate(modelBuilder.Model); - Assert.Equal("IX_Animal_Name", model.FindEntityType(typeof(Cat)).GetDeclaredIndexes().Single().GetName()); - Assert.Equal("IX_Animal_Name", model.FindEntityType(typeof(Dog)).GetDeclaredIndexes().Single().GetName()); + Assert.Equal("IX_Animal_Name", model.FindEntityType(typeof(Cat)).GetDeclaredIndexes().Single().GetDatabaseName()); + Assert.Equal("IX_Animal_Name", model.FindEntityType(typeof(Dog)).GetDeclaredIndexes().Single().GetDatabaseName()); } [ConditionalFact] diff --git a/test/EFCore.Sqlite.FunctionalTests/UpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/UpdatesSqliteTest.cs index f277e129cbf..f6dc0554e9f 100644 --- a/test/EFCore.Sqlite.FunctionalTests/UpdatesSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/UpdatesSqliteTest.cs @@ -32,7 +32,7 @@ public override void Identifiers_are_generated_correctly() entityType.GetForeignKeys().Single().GetConstraintName()); Assert.Equal( "IX_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWorkingCorrectly_ProfileId_ProfileId1_ProfileId3_ProfileId4_ProfileId5_ProfileId6_ProfileId7_ProfileId8_ProfileId9_ProfileId10_ProfileId11_ProfileId12_ProfileId13_ProfileId14_ExtraProperty", - entityType.GetIndexes().Single().GetName()); + entityType.GetIndexes().Single().GetDatabaseName()); } } } diff --git a/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs b/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs index 23dd6850ce5..415b7ae167b 100644 --- a/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs +++ b/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs @@ -66,7 +66,8 @@ public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() typeof(IList>), () => new List> { new Dictionary { { "A", "B" } } } }, - { typeof(IDictionary), () => new Dictionary() } + { typeof(IDictionary), () => new Dictionary() }, + { typeof(IndexAttribute), () => new IndexAttribute("FakeProperty")} }; TestEventLogging( diff --git a/test/EFCore.Tests/Infrastructure/EventIdTestBase.cs b/test/EFCore.Tests/Infrastructure/EventIdTestBase.cs index 67d19bf267f..ab2fa5578d6 100644 --- a/test/EFCore.Tests/Infrastructure/EventIdTestBase.cs +++ b/test/EFCore.Tests/Infrastructure/EventIdTestBase.cs @@ -91,7 +91,7 @@ public void TestEventLogging( } catch (Exception) { - Assert.True(false, "Need to add factory for type " + type.DisplayName()); + Assert.True(false, "Need to add fake test factory for type " + type.DisplayName() + " in class " + eventIdType.Name + "Test"); } } } diff --git a/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs index 31c263319a3..ea925e829fe 100644 --- a/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs @@ -132,146 +132,6 @@ public void KeyAttribute_does_not_override_keyless_attribute() #endregion - #region IndexAttribute - - [ConditionalFact] - public void IndexAttribute_overrides_configuration_from_convention() - { - var modelBuilder = new InternalModelBuilder(new Model()); - - 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); - entityBuilder.PrimaryKey(new List { "Id" }, ConfigurationSource.Convention); - - var indexProperties = new List { propA.Metadata.Name, propB.Metadata.Name }; - var indexBuilder = entityBuilder.HasIndex(indexProperties, ConfigurationSource.Convention); - indexBuilder.HasName("ConventionalIndexName", ConfigurationSource.Convention); - indexBuilder.IsUnique(false, ConfigurationSource.Convention); - - RunConvention(entityBuilder); - - var index = entityBuilder.Metadata.GetIndexes().Single(); - Assert.Equal(ConfigurationSource.DataAnnotation, index.GetConfigurationSource()); - Assert.Equal("IndexOnAAndB", index.Name); - Assert.Equal(ConfigurationSource.DataAnnotation, index.GetNameConfigurationSource()); - Assert.True(index.IsUnique); - Assert.Equal(ConfigurationSource.DataAnnotation, index.GetIsUniqueConfigurationSource()); - Assert.Collection(index.Properties, - prop0 => Assert.Equal("A", prop0.Name), - prop1 => Assert.Equal("B", prop1.Name)); - } - - [ConditionalFact] - public void IndexAttribute_can_be_overriden_using_explicit_configuration() - { - var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); - - var entityBuilder = modelBuilder.Entity(); - - var index = (Metadata.Internal.Index)entityBuilder.Metadata.GetIndexes().Single(); - Assert.Equal(ConfigurationSource.DataAnnotation, index.GetConfigurationSource()); - Assert.Equal("IndexOnAAndB", index.Name); - Assert.Equal(ConfigurationSource.DataAnnotation, index.GetNameConfigurationSource()); - Assert.True(index.IsUnique); - Assert.Equal(ConfigurationSource.DataAnnotation, index.GetIsUniqueConfigurationSource()); - Assert.Collection(index.Properties, - prop0 => Assert.Equal("A", prop0.Name), - prop1 => Assert.Equal("B", prop1.Name)); - - entityBuilder.HasIndex(e => new { e.A, e.B }) - .HasName("OverridenIndexName") - .IsUnique(false); - - index = (Metadata.Internal.Index)entityBuilder.Metadata.GetIndexes().Single(); - Assert.Equal(ConfigurationSource.Explicit, index.GetConfigurationSource()); - Assert.Equal("OverridenIndexName", index.Name); - Assert.Equal(ConfigurationSource.Explicit, index.GetNameConfigurationSource()); - Assert.False(index.IsUnique); - Assert.Equal(ConfigurationSource.Explicit, index.GetIsUniqueConfigurationSource()); - Assert.Collection(index.Properties, - prop0 => Assert.Equal("A", prop0.Name), - prop1 => Assert.Equal("B", prop1.Name)); - } - - [InlineData(typeof(EntityWithInvalidNullIndexMember), "{'A', ''}")] - [InlineData(typeof(EntityWithInvalidEmptyIndexMember), "{'A', ''}")] - [InlineData(typeof(EntityWithInvalidWhiteSpaceIndexMember), "{'A', ' \r\n\t'}")] - [ConditionalTheory] - public void IndexAttribute_members_cannot_include_whitespace(Type entityTypeWithInvalidIndex, string indexMembersString) - { - var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); - - Assert.Equal( - CoreStrings.IndexMemberNameEmpty( - entityTypeWithInvalidIndex.Name, - indexMembersString), - Assert.Throws( - () => modelBuilder.Entity(entityTypeWithInvalidIndex)).Message); - } - - [ConditionalFact] - public void IndexAttribute_with_an_ignored_member_throws() - { - var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); - - Assert.Equal( - CoreStrings.IndexMemberIsIgnored( - nameof(EntityWithIgnoredMember), - "{'A', 'B'}", - "B"), - Assert.Throws( - () => modelBuilder.Entity()).Message); - } - - [ConditionalFact] - public void IndexAttribute_with_a_non_existent_member_throws() - { - var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); - - Assert.Equal( - CoreStrings.IndexMemberHasNoMatchingMember( - nameof(EntityWithNonExistentMember), - "{'A', 'DoesNotExist'}", - "DoesNotExist"), - Assert.Throws( - () => modelBuilder.Entity()).Message); - } - - [ConditionalFact] - public void IndexAttribute_can_be_applied_more_than_once_per_entity_type() - { - var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); - - var entityBuilder = modelBuilder.Entity(); - - var indexes = entityBuilder.Metadata.GetIndexes(); - Assert.Equal(2, indexes.Count()); - - var index0 = (Metadata.Internal.Index)indexes.First(); - Assert.Equal(ConfigurationSource.DataAnnotation, index0.GetConfigurationSource()); - Assert.Equal("IndexOnAAndB", index0.Name); - Assert.Equal(ConfigurationSource.DataAnnotation, index0.GetNameConfigurationSource()); - 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 = (Metadata.Internal.Index)indexes.Skip(1).First(); - Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetConfigurationSource()); - Assert.Equal("IndexOnBAndC", index1.Name); - Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetNameConfigurationSource()); - Assert.False(index1.IsUnique); - Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetIsUniqueConfigurationSource()); - Assert.Collection(index1.Properties, - prop0 => Assert.Equal("B", prop0.Name), - prop1 => Assert.Equal("C", prop1.Name)); - } - - #endregion - private void RunConvention(InternalEntityTypeBuilder entityTypeBuilder) { var context = new ConventionContext(entityTypeBuilder.Metadata.Model.ConventionDispatcher); @@ -284,9 +144,6 @@ private void RunConvention(InternalEntityTypeBuilder entityTypeBuilder) new KeylessEntityTypeAttributeConvention(CreateDependencies()) .ProcessEntityTypeAdded(entityTypeBuilder, context); - - new IndexEntityTypeAttributeConvention(CreateDependencies()) - .ProcessEntityTypeAdded(entityTypeBuilder, context); } private ProviderConventionSetBuilderDependencies CreateDependencies() @@ -332,64 +189,5 @@ private class KeyClash [Key] public int MyId { get; set; } } - - [Index(nameof(A), nameof(B), Name = "IndexOnAAndB", IsUnique = true)] - private class EntityWithIndex - { - public int Id { get; set; } - public int A { get; set; } - public int B { get; set; } - } - - [Index(nameof(A), nameof(B), Name = "IndexOnAAndB", IsUnique = true)] - [Index(nameof(B), nameof(C), Name = "IndexOnBAndC", IsUnique = false)] - private class EntityWithTwoIndexes - { - public int Id { get; set; } - public int A { get; set; } - public int B { get; set; } - public int C { get; set; } - } - - [Index(nameof(A), null, Name = "IndexOnAAndNull")] - private class EntityWithInvalidNullIndexMember - { - public int Id { get; set; } - public int A { get; set; } - public int B { get; set; } - } - - [Index(nameof(A), "", Name = "IndexOnAAndEmpty")] - private class EntityWithInvalidEmptyIndexMember - { - public int Id { get; set; } - public int A { get; set; } - public int B { get; set; } - } - - [Index(nameof(A), " \r\n\t", Name = "IndexOnAAndWhiteSpace")] - private class EntityWithInvalidWhiteSpaceIndexMember - { - public int Id { get; set; } - public int A { get; set; } - public int B { get; set; } - } - - [Index(nameof(A), nameof(B), Name = "IndexOnAAndUnmappedMember")] - private class EntityWithIgnoredMember - { - public int Id { get; set; } - public int A { get; set; } - [NotMapped] - public int B { get; set; } - } - - [Index(nameof(A), "DoesNotExist", Name = "IndexOnAAndNonExistentMember")] - private class EntityWithNonExistentMember - { - public int Id { get; set; } - public int A { get; set; } - public int B { get; set; } - } } } diff --git a/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs new file mode 100644 index 00000000000..4675ecd9762 --- /dev/null +++ b/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs @@ -0,0 +1,236 @@ +// 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.Collections.Generic; +using System.ComponentModel.DataAnnotations.Schema; +using System.Linq; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Metadata.Builders; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal; +using Microsoft.EntityFrameworkCore.Metadata.Internal; +using Microsoft.EntityFrameworkCore.TestUtilities; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +// ReSharper disable UnusedMember.Local +// ReSharper disable InconsistentNaming +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions +{ + public class IndexAttributeConventionTest + { + #region IndexAttribute + + [ConditionalFact] + public void IndexAttribute_overrides_configuration_from_convention() + { + var modelBuilder = new InternalModelBuilder(new Model()); + + 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); + entityBuilder.PrimaryKey(new List { "Id" }, ConfigurationSource.Convention); + + var indexProperties = new List { propA.Metadata.Name, propB.Metadata.Name }; + var indexBuilder = entityBuilder.HasIndex(indexProperties, ConfigurationSource.Convention); + indexBuilder.HasName("ConventionalIndexName", ConfigurationSource.Convention); + indexBuilder.IsUnique(false, ConfigurationSource.Convention); + + RunConvention(modelBuilder); + + var index = entityBuilder.Metadata.GetIndexes().Single(); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetConfigurationSource()); + Assert.Equal("IndexOnAAndB", index.Name); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetNameConfigurationSource()); + Assert.True(index.IsUnique); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetIsUniqueConfigurationSource()); + Assert.Collection(index.Properties, + prop0 => Assert.Equal("A", prop0.Name), + prop1 => Assert.Equal("B", prop1.Name)); + } + + [ConditionalFact] + public void IndexAttribute_can_be_overriden_using_explicit_configuration() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + var entityBuilder = modelBuilder.Entity(); + + entityBuilder.HasIndex("A", "B") + .HasName("OverridenIndexName") + .IsUnique(false); + + modelBuilder.Model.FinalizeModel(); + + var index = (Metadata.Internal.Index)entityBuilder.Metadata.GetIndexes().Single(); + Assert.Equal(ConfigurationSource.Explicit, index.GetConfigurationSource()); + Assert.Equal("OverridenIndexName", index.Name); + Assert.Equal(ConfigurationSource.Explicit, index.GetNameConfigurationSource()); + Assert.False(index.IsUnique); + Assert.Equal(ConfigurationSource.Explicit, index.GetIsUniqueConfigurationSource()); + Assert.Collection(index.Properties, + prop0 => Assert.Equal("A", prop0.Name), + prop1 => Assert.Equal("B", prop1.Name)); + } + + [ConditionalFact] + 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); + } + + [InlineData(typeof(EntityWithInvalidNullIndexProperty))] + [InlineData(typeof(EntityWithInvalidEmptyIndexProperty))] + [InlineData(typeof(EntityWithInvalidWhiteSpaceIndexProperty))] + [ConditionalTheory] + 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); + } + + [ConditionalFact] + public void IndexAttribute_can_be_applied_more_than_once_per_entity_type() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + var entityBuilder = modelBuilder.Entity(); + modelBuilder.Model.FinalizeModel(); + + var indexes = entityBuilder.Metadata.GetIndexes(); + Assert.Equal(2, indexes.Count()); + + var index0 = (Metadata.Internal.Index)indexes.First(); + Assert.Equal(ConfigurationSource.DataAnnotation, index0.GetConfigurationSource()); + Assert.Equal("IndexOnAAndB", index0.Name); + Assert.Equal(ConfigurationSource.DataAnnotation, index0.GetNameConfigurationSource()); + 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 = (Metadata.Internal.Index)indexes.Skip(1).First(); + Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetConfigurationSource()); + Assert.Equal("IndexOnBAndC", index1.Name); + Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetNameConfigurationSource()); + Assert.False(index1.IsUnique); + Assert.Equal(ConfigurationSource.DataAnnotation, index1.GetIsUniqueConfigurationSource()); + Assert.Collection(index1.Properties, + prop0 => Assert.Equal("B", prop0.Name), + prop1 => Assert.Equal("C", prop1.Name)); + } + + [ConditionalFact] + public void IndexAttribute_can_be_inherited_from_base_entity_type() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + var entityBuilder = modelBuilder.Entity(); + modelBuilder.Model.FinalizeModel(); + + // assert that the base type is not part of the model + Assert.Empty(modelBuilder.Model.GetEntityTypes() + .Where(e => e.ClrType == typeof(BaseUnmappedEntityWithIndex))); + + // assert that we see the index anyway + var index = (Metadata.Internal.Index)entityBuilder.Metadata.GetIndexes().Single(); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetConfigurationSource()); + Assert.Equal("IndexOnAAndB", index.Name); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetNameConfigurationSource()); + Assert.True(index.IsUnique); + Assert.Equal(ConfigurationSource.DataAnnotation, index.GetIsUniqueConfigurationSource()); + Assert.Collection(index.Properties, + prop0 => Assert.Equal("A", prop0.Name), + prop1 => Assert.Equal("B", prop1.Name)); + } + + #endregion + + private void RunConvention(InternalModelBuilder modelBuilder) + { + var context = new ConventionContext(modelBuilder.Metadata.ConventionDispatcher); + + new IndexAttributeConvention(CreateDependencies()) + .ProcessModelFinalizing(modelBuilder, context); + } + + private ProviderConventionSetBuilderDependencies CreateDependencies() + => InMemoryTestHelpers.Instance.CreateContextServices().GetRequiredService(); + + [Index(nameof(A), nameof(B), Name = "IndexOnAAndB", IsUnique = true)] + private class EntityWithIndex + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + [Index(nameof(A), nameof(B), Name = "IndexOnAAndB", IsUnique = true)] + [Index(nameof(B), nameof(C), Name = "IndexOnBAndC", IsUnique = false)] + private class EntityWithTwoIndexes + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + public int C { get; set; } + } + + [Index(nameof(A), nameof(B), Name = "IndexOnAAndB", IsUnique = true)] + [NotMapped] + private class BaseUnmappedEntityWithIndex + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + private class EntityWithIndexFromBaseType : BaseUnmappedEntityWithIndex + { + public int C { get; set; } + public int D { get; set; } + } + + [Index] + private class EntityWithInvalidEmptyIndex + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + [Index(nameof(A), null, Name = "IndexOnAAndNull")] + private class EntityWithInvalidNullIndexProperty + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + [Index(nameof(A), "", Name = "IndexOnAAndEmpty")] + private class EntityWithInvalidEmptyIndexProperty + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + + [Index(nameof(A), " \r\n\t", Name = "IndexOnAAndWhiteSpace")] + private class EntityWithInvalidWhiteSpaceIndexProperty + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } + } +} From 32e4f9f13a0c79f129edfcf77d990f0b6bc0d4d1 Mon Sep 17 00:00:00 2001 From: lajones Date: Tue, 26 May 2020 21:05:51 -0700 Subject: [PATCH 3/6] Small updates. --- src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs | 1 + .../Extensions/Internal}/TupleExtensions.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) rename src/{EFCore/Extensions => EFCore.Relational/Extensions/Internal}/TupleExtensions.cs (96%) diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs index c14f5d26c7e..a0cd54e10a1 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs @@ -15,6 +15,7 @@ using System.Transactions; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Diagnostics.Internal; +using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Migrations; using Microsoft.EntityFrameworkCore.Migrations.Internal; diff --git a/src/EFCore/Extensions/TupleExtensions.cs b/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs similarity index 96% rename from src/EFCore/Extensions/TupleExtensions.cs rename to src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs index 843f1c56709..67adc4df9fd 100644 --- a/src/EFCore/Extensions/TupleExtensions.cs +++ b/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs @@ -6,7 +6,7 @@ using JetBrains.Annotations; // ReSharper disable once CheckNamespace -namespace Microsoft.EntityFrameworkCore +namespace Microsoft.EntityFrameworkCore.Internal { /// /// Extension methods for tuples. From 709927581e027568c47b5815a7e8ab811eb0c5de Mon Sep 17 00:00:00 2001 From: lajones Date: Wed, 27 May 2020 15:04:02 -0700 Subject: [PATCH 4/6] Add in info message for all index properties unmapped. Update error to only apply if there's a mixture of mapped and unmapped. Updating IndexAttributeConvention back to throw instead of logging. --- .../Diagnostics/IndexEventData.cs | 54 +++++++++++ .../IndexInvalidPropertiesEventData.cs | 2 +- .../IndexInvalidPropertyEventData.cs | 2 +- .../Diagnostics/RelationalEventId.cs | 20 +++- .../Diagnostics/RelationalLoggerExtensions.cs | 54 ++++++++++- .../RelationalLoggingDefinitions.cs | 11 ++- .../RelationalModelValidator.cs | 44 +++++++-- .../Properties/RelationalStrings.Designer.cs | 50 ++++++---- .../Properties/RelationalStrings.resx | 13 +-- src/EFCore/Diagnostics/CoreEventId.cs | 28 ------ .../Diagnostics/CoreLoggerExtensions.cs | 97 ------------------- src/EFCore/Diagnostics/LoggingDefinitions.cs | 18 ---- .../Conventions/IndexAttributeConvention.cs | 46 ++++----- src/EFCore/Properties/CoreStrings.Designer.cs | 64 +++--------- src/EFCore/Properties/CoreStrings.resx | 6 +- .../RelationalModelValidatorTest.cs | 53 +++++++++- .../DataAnnotationTestBase.cs | 55 +---------- .../IndexAttributeConventionTest.cs | 49 ++++++++++ 18 files changed, 344 insertions(+), 322 deletions(-) create mode 100644 src/EFCore.Relational/Diagnostics/IndexEventData.cs rename src/{EFCore => EFCore.Relational}/Diagnostics/IndexInvalidPropertyEventData.cs (95%) diff --git a/src/EFCore.Relational/Diagnostics/IndexEventData.cs b/src/EFCore.Relational/Diagnostics/IndexEventData.cs new file mode 100644 index 00000000000..8bd79f289ff --- /dev/null +++ b/src/EFCore.Relational/Diagnostics/IndexEventData.cs @@ -0,0 +1,54 @@ +// 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.Collections.Generic; +using System.Diagnostics; +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Metadata; + +namespace Microsoft.EntityFrameworkCore.Diagnostics +{ + /// + /// A event payload class for + /// the events involving an invalid index. + /// + public class IndexEventData : EventData + { + /// + /// Constructs the event payload for events involving an invalid index. + /// + /// The event definition. + /// A delegate that generates a log message for this event. + /// The entity type on which the index is defined. + /// The name of the index. + /// The names of the properties which define the index. + public IndexEventData( + [NotNull] EventDefinitionBase eventDefinition, + [NotNull] Func messageGenerator, + [NotNull] IEntityType entityType, + [CanBeNull] string indexName, + [NotNull] List indexPropertyNames) + : base(eventDefinition, messageGenerator) + { + EntityType = entityType; + Name = indexName; + PropertyNames = indexPropertyNames; + } + + /// + /// The entity type on which the index is defined. + /// + public virtual IEntityType EntityType { get; } + + /// + /// The name of the index. + /// + public virtual string Name { get; } + + /// + /// The list of properties which define the index. + /// + public virtual List PropertyNames { get; } + } +} diff --git a/src/EFCore.Relational/Diagnostics/IndexInvalidPropertiesEventData.cs b/src/EFCore.Relational/Diagnostics/IndexInvalidPropertiesEventData.cs index e8699cc08cb..f55a60f5fc9 100644 --- a/src/EFCore.Relational/Diagnostics/IndexInvalidPropertiesEventData.cs +++ b/src/EFCore.Relational/Diagnostics/IndexInvalidPropertiesEventData.cs @@ -16,7 +16,7 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics public class IndexInvalidPropertiesEventData : EventData { /// - /// Constructs the event payload for the event. + /// Constructs the event payload for the event. /// /// The event definition. /// A delegate that generates a log message for this event. diff --git a/src/EFCore/Diagnostics/IndexInvalidPropertyEventData.cs b/src/EFCore.Relational/Diagnostics/IndexInvalidPropertyEventData.cs similarity index 95% rename from src/EFCore/Diagnostics/IndexInvalidPropertyEventData.cs rename to src/EFCore.Relational/Diagnostics/IndexInvalidPropertyEventData.cs index f8dad5769d6..cbaaf9d87b6 100644 --- a/src/EFCore/Diagnostics/IndexInvalidPropertyEventData.cs +++ b/src/EFCore.Relational/Diagnostics/IndexInvalidPropertyEventData.cs @@ -16,7 +16,7 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics public class IndexInvalidPropertyEventData : EventData { /// - /// Constructs the event payload for the event. + /// Constructs the event payload for indexes with a invalid property. /// /// The event definition. /// A delegate that generates a log message for this event. diff --git a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs index 43fdd18198b..59d6799b24c 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs @@ -75,7 +75,8 @@ private enum Id // Model validation events ModelValidationKeyDefaultValueWarning = CoreEventId.RelationalBaseId + 600, BoolWithDefaultWarning, - IndexPropertyNotMappedToAnyTable, + AllIndexPropertiesNotToMappedToAnyTable, + IndexPropertiesBothMappedAndNotMappedToTable, IndexPropertiesMappedToNonOverlappingTables, // Update events @@ -557,7 +558,20 @@ private enum Id /// /// - /// An index specifies a property which is not mapped to a column in any table. + /// An index specifies properties all of which are not mapped to a column in any table. + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId AllIndexPropertiesNotToMappedToAnyTable = MakeValidationId(Id.AllIndexPropertiesNotToMappedToAnyTable); + + /// + /// + /// An index specifies properties some of which are mapped and some of which are not mapped to a column in a table. /// /// /// This event is in the category. @@ -566,7 +580,7 @@ private enum Id /// This event uses the payload when used with a . /// /// - public static readonly EventId IndexPropertyNotMappedToAnyTable = MakeValidationId(Id.IndexPropertyNotMappedToAnyTable); + public static readonly EventId IndexPropertiesBothMappedAndNotMappedToTable = MakeValidationId(Id.IndexPropertiesBothMappedAndNotMappedToTable); /// /// diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs index a0cd54e10a1..95874e6831d 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs @@ -3604,19 +3604,63 @@ private static string BatchSmallerThanMinBatchSize(EventDefinitionBase definitio } /// - /// Logs the event. + /// Logs the event. + /// + /// The diagnostics logger to use. + /// The entity type on which the index is defined. + /// The index on the entity type. + public static void AllIndexPropertiesNotToMappedToAnyTable( + [NotNull] this IDiagnosticsLogger diagnostics, + [NotNull] IEntityType entityType, + [NotNull] IIndex index) + { + var definition = RelationalResources.LogAllIndexPropertiesNotToMappedToAnyTable(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + index.Name, + entityType.DisplayName(), + index.Properties.Format()); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexEventData( + definition, + AllIndexPropertiesNotToMappedToAnyTable, + entityType, + index.Name, + index.Properties.Select(p => p.Name).ToList()); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string AllIndexPropertiesNotToMappedToAnyTable(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (IndexEventData)payload; + return d.GenerateMessage( + p.Name, + p.EntityType.DisplayName(), + p.PropertyNames.Format()); + } + + /// + /// Logs the event. /// /// The diagnostics logger to use. /// The entity type on which the index is defined. /// The index on the entity type. /// The name of the property which is not mapped. - public static void IndexPropertyNotMappedToAnyTable( + public static void IndexPropertiesBothMappedAndNotMappedToTable( [NotNull] this IDiagnosticsLogger diagnostics, [NotNull] IEntityType entityType, [NotNull] IIndex index, [NotNull] string unmappedPropertyName) { - var definition = RelationalResources.LogIndexPropertyNotMappedToAnyTable(diagnostics); + var definition = RelationalResources.LogIndexPropertiesBothMappedAndNotMappedToTable(diagnostics); if (diagnostics.ShouldLog(definition)) { @@ -3631,7 +3675,7 @@ public static void IndexPropertyNotMappedToAnyTable( { var eventData = new IndexInvalidPropertyEventData( definition, - IndexPropertyNotMappedToAnyTable, + IndexPropertiesBothMappedAndNotMappedToTable, entityType, index.Name, index.Properties.Select(p => p.Name).ToList(), @@ -3641,7 +3685,7 @@ public static void IndexPropertyNotMappedToAnyTable( } } - private static string IndexPropertyNotMappedToAnyTable(EventDefinitionBase definition, EventData payload) + private static string IndexPropertiesBothMappedAndNotMappedToTable(EventDefinitionBase definition, EventData payload) { var d = (EventDefinition)definition; var p = (IndexInvalidPropertyEventData)payload; diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs index b51ec167613..955861b171d 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs @@ -374,7 +374,16 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] - public EventDefinitionBase LogIndexPropertyNotMappedToAnyTable; + public EventDefinitionBase LogAllIndexPropertiesNotToMappedToAnyTable; + + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase LogIndexPropertiesBothMappedAndNotMappedToTable; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 5070f396775..115fd1a83e6 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -930,6 +930,7 @@ protected virtual void ValidateIndexProperties( foreach (var index in entityType.GetIndexes() .Where(i => ConfigurationSource.Convention != ((IConventionIndex)i).GetConfigurationSource())) { + IProperty propertyNotMappedToAnyTable = null; Tuple> firstPropertyTables = null; Tuple> lastPropertyTables = null; HashSet<(string Table, string Schema)> overlappingTables = null; @@ -942,13 +943,17 @@ protected virtual void ValidateIndexProperties( .ToList<(string Table, string Schema)>(); if (tablesMappedToProperty.Count == 0) { - logger.IndexPropertyNotMappedToAnyTable( - entityType, - index, - property.Name); - + propertyNotMappedToAnyTable = property; overlappingTables = null; - break; + + if (firstPropertyTables != null) + { + // Property is not mapped but we already found + // a property that is mapped. + break; + } + + continue; } if (firstPropertyTables == null) @@ -964,6 +969,14 @@ protected virtual void ValidateIndexProperties( new Tuple>(property.Name, tablesMappedToProperty); } + if (propertyNotMappedToAnyTable != null) + { + // Property is mapped but we already found + // a property that is not mapped. + overlappingTables = null; + break; + } + if (overlappingTables == null) { overlappingTables = new HashSet<(string Table, string Schema)>(tablesMappedToProperty); @@ -978,8 +991,23 @@ protected virtual void ValidateIndexProperties( } } - if (overlappingTables != null - && overlappingTables.Count == 0) + if (overlappingTables == null) + { + if (firstPropertyTables == null) + { + logger.AllIndexPropertiesNotToMappedToAnyTable( + entityType, + index); + } + else + { + logger.IndexPropertiesBothMappedAndNotMappedToTable( + entityType, + index, + propertyNotMappedToAnyTable.Name); + } + } + else if (overlappingTables.Count == 0) { Debug.Assert(firstPropertyTables != null, nameof(firstPropertyTables)); Debug.Assert(lastPropertyTables != null, nameof(lastPropertyTables)); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 76879fea376..9b4f46b73ec 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -865,14 +865,6 @@ public static string PropertyNotMappedToTable([CanBeNull] object property, [CanB GetString("PropertyNotMappedToTable", nameof(property), nameof(entityType), nameof(table)), property, entityType, table); - /// - /// An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field with name '{memberName}' is not mapped to a column in any table. Please use only members mapped to a column. - /// - public static string IndexMemberNotMappedToAnyTable([CanBeNull] object entityType, [CanBeNull] object indexMemberList, [CanBeNull] object memberName) - => string.Format( - GetString("IndexMemberNotMappedToAnyTable", nameof(entityType), nameof(indexMemberList), nameof(memberName)), - entityType, indexMemberList, memberName); - private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); @@ -1812,31 +1804,55 @@ public static EventDefinition LogMigrationAttributeMissingWarning([NotNu } /// - /// The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. But the property '{propertyName}' is not mapped to a column in any table. Please use only properties mapped to a column. + /// The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. None of these properties are mapped to a column in any table. This index will not be created in the database. + /// + public static EventDefinition LogAllIndexPropertiesNotToMappedToAnyTable([NotNull] IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogAllIndexPropertiesNotToMappedToAnyTable; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogAllIndexPropertiesNotToMappedToAnyTable, + () => new EventDefinition( + logger.Options, + RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable, + LogLevel.Information, + "RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable", + level => LoggerMessage.Define( + level, + RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable, + _resourceManager.GetString("LogAllIndexPropertiesNotToMappedToAnyTable")))); + } + + return (EventDefinition)definition; + } + + /// + /// The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. Some properties are mapped to a column in a table, but the property '{propertyName}' is not. Either all or none of the properties should be mapped. /// - public static EventDefinition LogIndexPropertyNotMappedToAnyTable([NotNull] IDiagnosticsLogger logger) + public static EventDefinition LogIndexPropertiesBothMappedAndNotMappedToTable([NotNull] IDiagnosticsLogger logger) { - var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertyNotMappedToAnyTable; + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertiesBothMappedAndNotMappedToTable; if (definition == null) { definition = LazyInitializer.EnsureInitialized( - ref ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertyNotMappedToAnyTable, + ref ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertiesBothMappedAndNotMappedToTable, () => new EventDefinition( logger.Options, - RelationalEventId.IndexPropertyNotMappedToAnyTable, + RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable, LogLevel.Error, - "RelationalEventId.IndexPropertyNotMappedToAnyTable", + "RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable", level => LoggerMessage.Define( level, - RelationalEventId.IndexPropertyNotMappedToAnyTable, - _resourceManager.GetString("LogIndexPropertyNotMappedToAnyTable")))); + RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable, + _resourceManager.GetString("LogIndexPropertiesBothMappedAndNotMappedToTable")))); } return (EventDefinition)definition; } /// - /// The index named '{indexName}' on the entity type '{entityType}' specifies members {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. + /// The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. /// public static FallbackEventDefinition LogIndexPropertiesMappedToNonOverlappingTables([NotNull] IDiagnosticsLogger logger) { diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 147219f72e5..ae065d26ebc 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -606,15 +606,16 @@ The property '{property}' on entity type '{entityType}' is not mapped to the table '{table}'. - - An index on the entity type '{entityType}' specifies members {indexMemberList}. But the property or field with name '{memberName}' is not mapped to a column in any table. Please use only members mapped to a column. + + The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. None of these properties are mapped to a column in any table. This index will not be created in the database. + Information RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable string string string - - The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. But the property '{propertyName}' is not mapped to a column in any table. Please use only properties mapped to a column. - Error RelationalEventId.IndexPropertyNotMappedToAnyTable string string string string + + The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. Some properties are mapped to a column in a table, but the property '{propertyName}' is not. Either all or none of the properties should be mapped. + Error RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable string string string string - The index named '{indexName}' on the entity type '{entityType}' specifies members {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. + The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. Error RelationalEventId.IndexPropertiesMappedToNonOverlappingTables string string string string string string string \ No newline at end of file diff --git a/src/EFCore/Diagnostics/CoreEventId.cs b/src/EFCore/Diagnostics/CoreEventId.cs index 9af974dc6ee..11bdd2cd170 100644 --- a/src/EFCore/Diagnostics/CoreEventId.cs +++ b/src/EFCore/Diagnostics/CoreEventId.cs @@ -105,8 +105,6 @@ private enum Id CollectionWithoutComparer, ConflictingKeylessAndKeyAttributesWarning, PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning, - IndexDefinedOnIgnoredProperty, - IndexDefinedOnNonExistentProperty, // ChangeTracking events DetectChangesStarting = CoreBaseId + 800, @@ -665,32 +663,6 @@ public static readonly EventId InvalidIncludePathError public static readonly EventId PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning = MakeModelValidationId(Id.PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning); - /// - /// - /// An index has specified a property which has been marked [NotMapped] or Ignore(). - /// - /// - /// This event is in the category. - /// - /// - /// This event uses the payload when used with a . - /// - /// - public static readonly EventId IndexDefinedOnIgnoredProperty = MakeModelId(Id.IndexDefinedOnIgnoredProperty); - - /// - /// - /// An index has specified a property which does not exist on the entity type or any of its base types. - /// - /// - /// This event is in the category. - /// - /// - /// This event uses the payload when used with a . - /// - /// - public static readonly EventId IndexDefinedOnNonExistentProperty = MakeModelId(Id.IndexDefinedOnNonExistentProperty); - private static readonly string _changeTrackingPrefix = DbLoggerCategory.ChangeTracking.Name + "."; private static EventId MakeChangeTrackingId(Id id) => new EventId((int)id, _changeTrackingPrefix + id); diff --git a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs index 3339ddfb9e4..ed75b4623a8 100644 --- a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs +++ b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs @@ -3035,102 +3035,5 @@ private static string PossibleIncorrectRequiredNavigationWithQueryFilterInteract p.ForeignKey.PrincipalEntityType.DisplayName(), p.ForeignKey.DeclaringEntityType.DisplayName()); } - - /// - /// Logs the event. - /// - /// The diagnostics logger to use. - /// The entity type on which the index is defined. - /// The on the entity type. - /// The name of the property which is ignored. - public static void IndexDefinedOnIgnoredProperty( - [NotNull] this IDiagnosticsLogger diagnostics, - [NotNull] IEntityType entityType, - [NotNull] IndexAttribute indexAttribute, - [NotNull] string ignoredPropertyName) - { - var definition = CoreResources.LogIndexDefinedOnIgnoredProperty(diagnostics); - - if (diagnostics.ShouldLog(definition)) - { - definition.Log(diagnostics, - indexAttribute.Name, - entityType.DisplayName(), - indexAttribute.PropertyNames.Format(), - ignoredPropertyName); - } - - if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) - { - var eventData = new IndexInvalidPropertyEventData( - definition, - IndexDefinedOnIgnoredProperty, - entityType, - indexAttribute.Name, - indexAttribute.PropertyNames, - ignoredPropertyName); - - diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); - } - } - private static string IndexDefinedOnIgnoredProperty(EventDefinitionBase definition, EventData payload) - { - var d = (EventDefinition)definition; - var p = (IndexInvalidPropertyEventData)payload; - return d.GenerateMessage( - p.Name, - p.EntityType.DisplayName(), - p.PropertyNames.Format(), - p.InvalidPropertyName); - } - - /// - /// Logs the event. - /// - /// The diagnostics logger to use. - /// The entity type on which the index is defined. - /// The on the entity type. - /// The name of the property which does not exist. - public static void IndexDefinedOnNonExistentProperty( - [NotNull] this IDiagnosticsLogger diagnostics, - [NotNull] IEntityType entityType, - [NotNull] IndexAttribute indexAttribute, - [NotNull] string nonExistentPropertyName) - { - var definition = CoreResources.LogIndexDefinedOnNonExistentProperty(diagnostics); - - if (diagnostics.ShouldLog(definition)) - { - definition.Log(diagnostics, - indexAttribute.Name, - entityType.DisplayName(), - indexAttribute.PropertyNames.Format(), - nonExistentPropertyName); - } - - if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) - { - var eventData = new IndexInvalidPropertyEventData( - definition, - IndexDefinedOnNonExistentProperty, - entityType, - indexAttribute.Name, - indexAttribute.PropertyNames, - nonExistentPropertyName); - - diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); - } - } - - private static string IndexDefinedOnNonExistentProperty(EventDefinitionBase definition, EventData payload) - { - var d = (EventDefinition)definition; - var p = (IndexInvalidPropertyEventData)payload; - return d.GenerateMessage( - p.Name, - p.EntityType.DisplayName(), - p.PropertyNames.Format(), - p.InvalidPropertyName); - } } } diff --git a/src/EFCore/Diagnostics/LoggingDefinitions.cs b/src/EFCore/Diagnostics/LoggingDefinitions.cs index 5960d64db0a..149a7606de6 100644 --- a/src/EFCore/Diagnostics/LoggingDefinitions.cs +++ b/src/EFCore/Diagnostics/LoggingDefinitions.cs @@ -663,23 +663,5 @@ public abstract class LoggingDefinitions /// [EntityFrameworkInternal] public EventDefinitionBase LogPossibleIncorrectRequiredNavigationWithQueryFilterInteraction; - - /// - /// 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. - /// - [EntityFrameworkInternal] - public EventDefinitionBase LogIndexDefinedOnIgnoredProperty; - - /// - /// 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. - /// - [EntityFrameworkInternal] - public EventDefinitionBase LogIndexDefinedOnNonExistentProperty; } } diff --git a/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs b/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs index eea98f3df90..e5b9d5a2c39 100644 --- a/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs @@ -42,50 +42,44 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, foreach (var indexAttribute in entityType.ClrType.GetCustomAttributes(true)) { - var shouldGenerate = true; var indexProperties = new List(); foreach (var propertyName in indexAttribute.PropertyNames) { if (ignoredMembers.Contains(propertyName)) { - Dependencies.Logger - .IndexDefinedOnIgnoredProperty( - entityType, - indexAttribute, - propertyName); - shouldGenerate = false; - break; + throw new InvalidOperationException( + CoreStrings.IndexDefinedOnIgnoredProperty( + indexAttribute.Name, + entityType.DisplayName(), + indexAttribute.PropertyNames.Format(), + propertyName)); } var property = entityType.FindProperty(propertyName); if (property == null) { - Dependencies.Logger - .IndexDefinedOnNonExistentProperty( - entityType, - indexAttribute, - propertyName); - shouldGenerate = false; - break; + throw new InvalidOperationException( + CoreStrings.IndexDefinedOnNonExistentProperty( + indexAttribute.Name, + entityType.DisplayName(), + indexAttribute.PropertyNames.Format(), + propertyName)); } indexProperties.Add(property); } - if (shouldGenerate) + var indexBuilder = entityType.Builder.HasIndex(indexProperties, fromDataAnnotation: true); + if (indexBuilder != null) { - var indexBuilder = entityType.Builder.HasIndex(indexProperties, fromDataAnnotation: true); - if (indexBuilder != null) + if (indexAttribute.Name != null) { - if (indexAttribute.Name != null) - { - indexBuilder.HasName(indexAttribute.Name, fromDataAnnotation: true); - } + indexBuilder.HasName(indexAttribute.Name, fromDataAnnotation: true); + } - if (indexAttribute.GetIsUnique().HasValue) - { - indexBuilder.IsUnique(indexAttribute.GetIsUnique().Value, fromDataAnnotation: true); - } + if (indexAttribute.GetIsUnique().HasValue) + { + indexBuilder.IsUnique(indexAttribute.GetIsUnique().Value, fromDataAnnotation: true); } } } diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 56f68b45084..db5e055a713 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -2658,6 +2658,22 @@ public static string MissingInverseManyToManyNavigation([CanBeNull] object princ public static string QueryContextAlreadyInitializedStateManager => GetString("QueryContextAlreadyInitializedStateManager"); + /// + /// The index named '{indexName}' on the entity type '{entityType}' with properties {indexPropertyList} is invalid. The property '{propertyName}' has been marked NotMapped or Ignore(). An index cannot use such properties. + /// + public static string IndexDefinedOnIgnoredProperty([CanBeNull] object indexName, [CanBeNull] object entityType, [CanBeNull] object indexPropertyList, [CanBeNull] object propertyName) + => string.Format( + GetString("IndexDefinedOnIgnoredProperty", nameof(indexName), nameof(entityType), nameof(indexPropertyList), nameof(propertyName)), + indexName, entityType, indexPropertyList, propertyName); + + /// + /// An index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertyList}. But no property with name '{propertyName}' exists on that entity type or any of its base types. + /// + public static string IndexDefinedOnNonExistentProperty([CanBeNull] object indexName, [CanBeNull] object entityType, [CanBeNull] object indexPropertyList, [CanBeNull] object propertyName) + => string.Format( + GetString("IndexDefinedOnNonExistentProperty", nameof(indexName), nameof(entityType), nameof(indexPropertyList), nameof(propertyName)), + indexName, entityType, indexPropertyList, propertyName); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); @@ -4267,53 +4283,5 @@ public static EventDefinition LogPossibleIncorrectRequiredNaviga return (EventDefinition)definition; } - - /// - /// The index named '{indexName}' on the entity type '{entityType}' with properties {indexPropertyList} is invalid. The property '{propertyName}' has been marked NotMapped or Ignore(). An index cannot use such properties. - /// - public static EventDefinition LogIndexDefinedOnIgnoredProperty([NotNull] IDiagnosticsLogger logger) - { - var definition = ((LoggingDefinitions)logger.Definitions).LogIndexDefinedOnIgnoredProperty; - if (definition == null) - { - definition = LazyInitializer.EnsureInitialized( - ref ((LoggingDefinitions)logger.Definitions).LogIndexDefinedOnIgnoredProperty, - () => new EventDefinition( - logger.Options, - CoreEventId.IndexDefinedOnIgnoredProperty, - LogLevel.Error, - "CoreEventId.IndexDefinedOnIgnoredProperty", - level => LoggerMessage.Define( - level, - CoreEventId.IndexDefinedOnIgnoredProperty, - _resourceManager.GetString("LogIndexDefinedOnIgnoredProperty")))); - } - - return (EventDefinition)definition; - } - - /// - /// An index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertyList}. But no property with name '{propertyName}' exists on that entity type or any of its base types. - /// - public static EventDefinition LogIndexDefinedOnNonExistentProperty([NotNull] IDiagnosticsLogger logger) - { - var definition = ((LoggingDefinitions)logger.Definitions).LogIndexDefinedOnNonExistentProperty; - if (definition == null) - { - definition = LazyInitializer.EnsureInitialized( - ref ((LoggingDefinitions)logger.Definitions).LogIndexDefinedOnNonExistentProperty, - () => new EventDefinition( - logger.Options, - CoreEventId.IndexDefinedOnNonExistentProperty, - LogLevel.Error, - "CoreEventId.IndexDefinedOnNonExistentProperty", - level => LoggerMessage.Define( - level, - CoreEventId.IndexDefinedOnNonExistentProperty, - _resourceManager.GetString("LogIndexDefinedOnNonExistentProperty")))); - } - - return (EventDefinition)definition; - } } } diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index e86dd9d6dbd..8e5f53b893e 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1403,12 +1403,10 @@ InitializeStateManager method has been called multiple times on current query context. This method is intended to be called only once before query enumeration starts. - + The index named '{indexName}' on the entity type '{entityType}' with properties {indexPropertyList} is invalid. The property '{propertyName}' has been marked NotMapped or Ignore(). An index cannot use such properties. - Error CoreEventId.IndexDefinedOnIgnoredProperty string string string string - + An index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertyList}. But no property with name '{propertyName}' exists on that entity type or any of its base types. - Error CoreEventId.IndexDefinedOnNonExistentProperty string string string string \ No newline at end of file diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 46f8d894f8f..6f685101767 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -1249,22 +1249,65 @@ var methodInfo } [ConditionalFact] - public void Detects_index_property_not_mapped_to_any_table() + public void Passes_for_all_index_properties_not_mapped_to_any_table() { var modelBuilder = CreateConventionalModelBuilder(); modelBuilder.Entity().ToTable(null); - modelBuilder.Entity().HasIndex(nameof(Animal.Name)); + modelBuilder.Entity().HasIndex(nameof(Animal.Id), nameof(Animal.Name)); var definition = RelationalResources - .LogIndexPropertyNotMappedToAnyTable( + .LogAllIndexPropertiesNotToMappedToAnyTable( new TestLogger()); VerifyWarning( definition.GenerateMessage( "(null)", nameof(Animal), - "{'Name'}", - "Name"), + "{'Id', 'Name'}"), + modelBuilder.Model, + LogLevel.Information); + } + + [ConditionalFact(Skip = "Needs TPT (PR 20938) to run")] //TODO - awaiting PR 20938 + public void Detects_mix_of_index_property_mapped_and_not_mapped_to_any_table_unmapped_first() + { + var modelBuilder = CreateConventionalModelBuilder(); + + modelBuilder.Entity().ToTable(null); + modelBuilder.Entity().ToTable("Cats"); + modelBuilder.Entity().HasIndex(nameof(Animal.Id), nameof(Cat.Identity)); + + var definition = RelationalResources + .LogIndexPropertiesBothMappedAndNotMappedToTable( + new TestLogger()); + VerifyWarning( + definition.GenerateMessage( + "(null)", + nameof(Cat), + "{'Id', 'Identity'}", + "Id"), + modelBuilder.Model, + LogLevel.Error); + } + + [ConditionalFact(Skip = "Needs TPT (PR 20938) to run")] //TODO - awaiting PR 20938 + public void Detects_mix_of_index_property_mapped_and_not_mapped_to_any_table_mapped_first() + { + var modelBuilder = CreateConventionalModelBuilder(); + + modelBuilder.Entity().ToTable(null); + modelBuilder.Entity().ToTable("Cats"); + modelBuilder.Entity().HasIndex(nameof(Cat.Identity), nameof(Animal.Id)); + + var definition = RelationalResources + .LogIndexPropertiesBothMappedAndNotMappedToTable( + new TestLogger()); + VerifyWarning( + definition.GenerateMessage( + "(null)", + nameof(Cat), + "{'Identity', 'Id'}", + "Id"), modelBuilder.Model, LogLevel.Error); } diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs index 72e06c796d2..74cbdcf95ff 100644 --- a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs +++ b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs @@ -808,40 +808,6 @@ public virtual void Key_fluent_api_and_keyless_attribute_do_not_cause_warning() Assert.Empty(Fixture.ListLoggerFactory.Log); } - [ConditionalFact] - public virtual void IndexAttribute_with_an_ignored_property_causes_error() - { - var modelBuilder = CreateModelBuilder(); - var entity = modelBuilder.Entity(); - modelBuilder.Model.FinalizeModel(); - - var logEntry = Fixture.ListLoggerFactory.Log.Single(); - Assert.Equal(LogLevel.Error, logEntry.Level); - Assert.Equal( - CoreResources.LogIndexDefinedOnIgnoredProperty(new TestLogger()) - .GenerateMessage("(null)", nameof(EntityWithIgnoredProperty), "{'A', 'B'}", "B"), - logEntry.Message); - } - - [ConditionalFact] - public virtual void IndexAttribute_with_a_non_existent_property_causes_error() - { - var modelBuilder = CreateModelBuilder(); - var entity = modelBuilder.Entity(); - modelBuilder.Model.FinalizeModel(); - - var logEntry = Fixture.ListLoggerFactory.Log.Single(); - Assert.Equal(LogLevel.Error, logEntry.Level); - Assert.Equal( - CoreResources.LogIndexDefinedOnNonExistentProperty(new TestLogger()) - .GenerateMessage( - "IndexOnAAndNonExistentProperty", - nameof(EntityWithNonExistentProperty), - "{'A', 'DoesNotExist'}", - "DoesNotExist"), - logEntry.Message); - } - private class CompositeKeyAttribute { [Key] @@ -2407,9 +2373,7 @@ public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder build .Log(CoreEventId.ConflictingForeignKeyAttributesOnNavigationAndPropertyWarning) .Log(CoreEventId.ForeignKeyAttributesOnBothNavigationsWarning) .Log(CoreEventId.ForeignKeyAttributesOnBothPropertiesWarning) - .Log(CoreEventId.ConflictingKeylessAndKeyAttributesWarning) - .Log(CoreEventId.IndexDefinedOnIgnoredProperty) - .Log(CoreEventId.IndexDefinedOnNonExistentProperty)); + .Log(CoreEventId.ConflictingKeylessAndKeyAttributesWarning)); protected override bool ShouldLogCategory(string logCategory) => logCategory == DbLoggerCategory.Model.Name; @@ -2506,22 +2470,5 @@ protected class KeyFluentApiAndKeylessAttribute { public int MyKey { get; set; } } - - [Index(nameof(A), nameof(B))] - private class EntityWithIgnoredProperty - { - public int Id { get; set; } - public int A { get; set; } - [NotMapped] - public int B { get; set; } - } - - [Index(nameof(A), "DoesNotExist", Name = "IndexOnAAndNonExistentProperty")] - private class EntityWithNonExistentProperty - { - public int Id { get; set; } - public int A { get; set; } - public int B { get; set; } - } } } diff --git a/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs index 4675ecd9762..fac541df31d 100644 --- a/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/IndexAttributeConventionTest.cs @@ -155,6 +155,38 @@ public void IndexAttribute_can_be_inherited_from_base_entity_type() prop1 => Assert.Equal("B", prop1.Name)); } + [ConditionalFact] + public virtual void IndexAttribute_with_an_ignored_property_causes_error() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + var entity = modelBuilder.Entity(); + + Assert.Equal( + CoreStrings.IndexDefinedOnIgnoredProperty( + "", + nameof(EntityWithIgnoredProperty), + "{'A', 'B'}", + "B"), + Assert.Throws( + () => modelBuilder.Model.FinalizeModel()).Message); + } + + [ConditionalFact] + public virtual void IndexAttribute_with_a_non_existent_property_causes_error() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + var entity = modelBuilder.Entity(); + + Assert.Equal( + CoreStrings.IndexDefinedOnNonExistentProperty( + "IndexOnAAndNonExistentProperty", + nameof(EntityWithNonExistentProperty), + "{'A', 'DoesNotExist'}", + "DoesNotExist"), + Assert.Throws( + () => modelBuilder.Model.FinalizeModel()).Message); + } + #endregion private void RunConvention(InternalModelBuilder modelBuilder) @@ -232,5 +264,22 @@ private class EntityWithInvalidWhiteSpaceIndexProperty public int A { get; set; } public int B { get; set; } } + + [Index(nameof(A), nameof(B))] + private class EntityWithIgnoredProperty + { + public int Id { get; set; } + public int A { get; set; } + [NotMapped] + public int B { get; set; } + } + + [Index(nameof(A), "DoesNotExist", Name = "IndexOnAAndNonExistentProperty")] + private class EntityWithNonExistentProperty + { + public int Id { get; set; } + public int A { get; set; } + public int B { get; set; } + } } } From e1230b6cd7d788dfdbc18463bc52d59dc433b36d Mon Sep 17 00:00:00 2001 From: lajones Date: Wed, 27 May 2020 18:51:37 -0700 Subject: [PATCH 5/6] Rebase to updated master. --- .../Extensions/Internal/TupleExtensions.cs | 4 +++- .../Extensions/RelationalIndexExtensions.cs | 4 +--- .../RelationalModelValidator.cs | 5 ++-- .../Conventions/SharedTableConvention.cs | 2 +- .../Internal/RelationalIndexExtensions.cs | 4 ++-- .../Metadata/Internal/RelationalModel.cs | 2 +- .../Internal/SqlServerIndexExtensions.cs | 2 +- .../RelationalModelValidatorTest.cs | 23 ++++++++++--------- .../Infrastructure/CoreEventIdTest.cs | 3 +-- 9 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs b/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs index 67adc4df9fd..6263ffd59cd 100644 --- a/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs +++ b/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs @@ -31,6 +31,8 @@ public static string FormatTables([NotNull] this IEnumerable<(string Table, stri /// The (Table, Schema) tuple to format. /// The string representation. public static string FormatTable(this (string Table, string Schema) table) - => table.Schema == null ? table.Table : table.Schema + "." + table.Table; + => "'" + + (table.Schema == null ? table.Table : table.Schema + "." + table.Table) + + "'"; } } diff --git a/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs b/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs index a272dd9ef03..5dfa848382e 100644 --- a/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs @@ -8,7 +8,6 @@ using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; -using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Utilities; // ReSharper disable once CheckNamespace @@ -47,8 +46,7 @@ public static string GetName( [NotNull] this IIndex index, [NotNull] string tableName, [CanBeNull] string schema) - => (string)index[RelationalAnnotationNames.Name] - ?? index.GetDefaultName(tableName, schema); + => index.Name ?? index.GetDefaultName(tableName, schema); /// /// Returns the default name that would be used for this index. diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 115fd1a83e6..892d5335a96 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -693,7 +693,7 @@ protected virtual void ValidateSharedIndexesCompatibility( foreach (var index in mappedTypes.SelectMany(et => et.GetDeclaredIndexes())) { - var indexName = index.GetDatabaseName(); + var indexName = index.GetName(tableName, schema); if (!indexMappings.TryGetValue(indexName, out var duplicateIndex)) { indexMappings[indexName] = index; @@ -936,10 +936,9 @@ protected virtual void ValidateIndexProperties( HashSet<(string Table, string Schema)> overlappingTables = null; foreach (var property in index.Properties) { - //TODO - update to use table-schema override of GetColumn() below when PR #20938 is checked in var tablesMappedToProperty = property.DeclaringEntityType.GetDerivedTypesInclusive() .Select(t => (t.GetTableName(), t.GetSchema())).Distinct() - .Where(n => n.Item1 != null && property.GetColumnName(/* n.Item1, n.Item2 */) != null) + .Where(n => n.Item1 != null && property.GetColumnName(n.Item1, n.Item2) != null) .ToList<(string Table, string Schema)>(); if (tablesMappedToProperty.Count == 0) { diff --git a/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs b/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs index 8ea5f81f439..effa880329f 100644 --- a/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs @@ -303,7 +303,7 @@ private void TryUniquifyIndexNames( { foreach (var index in entityType.GetDeclaredIndexes()) { - var indexName = index.GetDatabaseName(); + var indexName = index.GetName(tableName, schema); if (!indexes.TryGetValue(indexName, out var otherIndex)) { indexes[indexName] = index; diff --git a/src/EFCore.Relational/Metadata/Internal/RelationalIndexExtensions.cs b/src/EFCore.Relational/Metadata/Internal/RelationalIndexExtensions.cs index e86c4d0ced1..5ca8c309b64 100644 --- a/src/EFCore.Relational/Metadata/Internal/RelationalIndexExtensions.cs +++ b/src/EFCore.Relational/Metadata/Internal/RelationalIndexExtensions.cs @@ -42,7 +42,7 @@ public static bool AreCompatible( duplicateIndex.Properties.Format(), duplicateIndex.DeclaringEntityType.DisplayName(), index.DeclaringEntityType.GetSchemaQualifiedTableName(), - index.GetDatabaseName(), + index.GetName(tableName, schema), index.Properties.FormatColumns(tableName, schema), duplicateIndex.Properties.FormatColumns(tableName, schema))); } @@ -61,7 +61,7 @@ public static bool AreCompatible( duplicateIndex.Properties.Format(), duplicateIndex.DeclaringEntityType.DisplayName(), index.DeclaringEntityType.GetSchemaQualifiedTableName(), - index.GetDatabaseName())); + index.GetName(tableName, schema))); } return false; diff --git a/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs b/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs index 4f2416ce101..17288196e23 100644 --- a/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs +++ b/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs @@ -475,7 +475,7 @@ private static void PopulateConstraints(Table table) foreach (var index in entityType.GetIndexes()) { - var name = index.GetDatabaseName(); + var name = index.GetName(table.Name, table.Schema); if (!table.Indexes.TryGetValue(name, out var tableIndex)) { var columns = new Column[index.Properties.Count]; diff --git a/src/EFCore.SqlServer/Metadata/Internal/SqlServerIndexExtensions.cs b/src/EFCore.SqlServer/Metadata/Internal/SqlServerIndexExtensions.cs index c021bffe362..2640419f7af 100644 --- a/src/EFCore.SqlServer/Metadata/Internal/SqlServerIndexExtensions.cs +++ b/src/EFCore.SqlServer/Metadata/Internal/SqlServerIndexExtensions.cs @@ -51,7 +51,7 @@ public static bool AreCompatibleForSqlServer( FormatInclude(duplicateIndex, tableName, schema))); } - return false; + return false; } } diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 6f685101767..3de014a2d88 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -1268,14 +1268,14 @@ public void Passes_for_all_index_properties_not_mapped_to_any_table() LogLevel.Information); } - [ConditionalFact(Skip = "Needs TPT (PR 20938) to run")] //TODO - awaiting PR 20938 + [ConditionalFact] public void Detects_mix_of_index_property_mapped_and_not_mapped_to_any_table_unmapped_first() { var modelBuilder = CreateConventionalModelBuilder(); modelBuilder.Entity().ToTable(null); modelBuilder.Entity().ToTable("Cats"); - modelBuilder.Entity().HasIndex(nameof(Animal.Id), nameof(Cat.Identity)); + modelBuilder.Entity().HasIndex(nameof(Animal.Name), nameof(Cat.Identity)); var definition = RelationalResources .LogIndexPropertiesBothMappedAndNotMappedToTable( @@ -1284,20 +1284,20 @@ public void Detects_mix_of_index_property_mapped_and_not_mapped_to_any_table_unm definition.GenerateMessage( "(null)", nameof(Cat), - "{'Id', 'Identity'}", - "Id"), + "{'Name', 'Identity'}", + "Name"), modelBuilder.Model, LogLevel.Error); } - [ConditionalFact(Skip = "Needs TPT (PR 20938) to run")] //TODO - awaiting PR 20938 + [ConditionalFact] public void Detects_mix_of_index_property_mapped_and_not_mapped_to_any_table_mapped_first() { var modelBuilder = CreateConventionalModelBuilder(); modelBuilder.Entity().ToTable(null); modelBuilder.Entity().ToTable("Cats"); - modelBuilder.Entity().HasIndex(nameof(Cat.Identity), nameof(Animal.Id)); + modelBuilder.Entity().HasIndex(nameof(Cat.Identity), nameof(Animal.Name)); var definition = RelationalResources .LogIndexPropertiesBothMappedAndNotMappedToTable( @@ -1306,13 +1306,13 @@ public void Detects_mix_of_index_property_mapped_and_not_mapped_to_any_table_map definition.GenerateMessage( "(null)", nameof(Cat), - "{'Identity', 'Id'}", - "Id"), + "{'Identity', 'Name'}", + "Name"), modelBuilder.Model, LogLevel.Error); } - [ConditionalFact(Skip = "Needs TPT (PR 20938) to run")] //TODO - awaiting PR 20938 + [ConditionalFact] public void Passes_for_index_properties_mapped_to_same_table_in_TPT_hierarchy() { var modelBuilder = CreateConventionalModelBuilder(); @@ -1323,10 +1323,11 @@ public void Passes_for_index_properties_mapped_to_same_table_in_TPT_hierarchy() Validate(modelBuilder.Model); - Assert.Empty(LoggerFactory.Log); + Assert.Empty(LoggerFactory.Log + .Where(l => l.Level != LogLevel.Trace && l.Level != LogLevel.Debug)); } - [ConditionalFact(Skip = "Needs TPT (PR 20938) to run")] //TODO - awaiting PR 20938 + [ConditionalFact] public void Detects_index_properties_mapped_to_different_tables_in_TPT_hierarchy() { var modelBuilder = CreateConventionalModelBuilder(); diff --git a/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs b/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs index 415b7ae167b..23dd6850ce5 100644 --- a/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs +++ b/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs @@ -66,8 +66,7 @@ public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() typeof(IList>), () => new List> { new Dictionary { { "A", "B" } } } }, - { typeof(IDictionary), () => new Dictionary() }, - { typeof(IndexAttribute), () => new IndexAttribute("FakeProperty")} + { typeof(IDictionary), () => new Dictionary() } }; TestEventLogging( From 7d061cd20c7f6788e0ee26f3640d5d3b830559c6 Mon Sep 17 00:00:00 2001 From: lajones Date: Thu, 28 May 2020 13:56:02 -0700 Subject: [PATCH 6/6] Updated based on last review comments. --- .../Design/CSharpSnapshotGenerator.cs | 23 +- .../Internal/CSharpModelGenerator.cs | 2 +- .../Diagnostics/RelationalLoggerExtensions.cs | 243 +++++++++++++----- .../RelationalLoggingDefinitions.cs | 33 ++- .../Extensions/Internal/TupleExtensions.cs | 21 +- .../RelationalModelValidator.cs | 2 +- .../Properties/RelationalStrings.Designer.cs | 98 ++++++- .../Properties/RelationalStrings.resx | 20 +- src/EFCore/Extensions/PropertyExtensions.cs | 13 - .../Conventions/IndexAttributeConvention.cs | 1 + src/Shared/EnumerableExtensions.cs | 7 + .../RelationalModelValidatorTest.cs | 70 ++++- 12 files changed, 416 insertions(+), 117 deletions(-) diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index 44fc7a2147f..1dd58982534 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs @@ -789,7 +789,7 @@ protected virtual void GenerateIndexAnnotations( IgnoreAnnotations( annotations, - CSharpModelGenerator.IgnoredIndexAnnotations.ToArray()); + CSharpModelGenerator.IgnoredIndexAnnotations); GenerateFluentApiForAnnotation( ref annotations, RelationalAnnotationNames.Filter, nameof(RelationalIndexBuilderExtensions.HasFilter), stringBuilder); @@ -1223,6 +1223,27 @@ protected virtual void IgnoreAnnotations( } } + /// + /// Removes ignored annotations. + /// + /// The annotations to remove from. + /// The ignored annotation names. + protected virtual void IgnoreAnnotations( + [NotNull] IList annotations, [NotNull] IReadOnlyList annotationNames) + { + Check.NotNull(annotations, nameof(annotations)); + Check.NotNull(annotationNames, nameof(annotationNames)); + + foreach (var annotationName in annotationNames) + { + var annotation = annotations.FirstOrDefault(a => a.Name == annotationName); + if (annotation != null) + { + annotations.Remove(annotation); + } + } + } + /// /// Removes ignored annotations. /// diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpModelGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpModelGenerator.cs index c0346f7bce4..c3254f6bd21 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpModelGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpModelGenerator.cs @@ -133,7 +133,7 @@ public override ScaffoldedModel GenerateModel( /// /// The set of annotations ignored for the purposes of code generation for indexes. /// - public static IEnumerable IgnoredIndexAnnotations + public static IReadOnlyList IgnoredIndexAnnotations => new List { RelationalAnnotationNames.TableIndexMappings }; } } diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs index 95874e6831d..79a53c893c0 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs @@ -22,6 +22,7 @@ using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Storage.Internal; using Microsoft.EntityFrameworkCore.Update; +using Microsoft.EntityFrameworkCore.Utilities; using Microsoft.Extensions.Logging; using IsolationLevel = System.Data.IsolationLevel; @@ -3614,30 +3615,65 @@ public static void AllIndexPropertiesNotToMappedToAnyTable( [NotNull] IEntityType entityType, [NotNull] IIndex index) { - var definition = RelationalResources.LogAllIndexPropertiesNotToMappedToAnyTable(diagnostics); - - if (diagnostics.ShouldLog(definition)) + if (index.Name == null) { - definition.Log(diagnostics, - index.Name, - entityType.DisplayName(), - index.Properties.Format()); - } + var definition = RelationalResources.LogUnnamedIndexAllPropertiesNotToMappedToAnyTable(diagnostics); - if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + entityType.DisplayName(), + index.Properties.Format()); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexEventData( + definition, + UnnamedIndexAllPropertiesNotToMappedToAnyTable, + entityType, + null, + index.Properties.Select(p => p.Name).ToList()); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + else { - var eventData = new IndexEventData( - definition, - AllIndexPropertiesNotToMappedToAnyTable, - entityType, - index.Name, - index.Properties.Select(p => p.Name).ToList()); + var definition = RelationalResources.LogNamedIndexAllPropertiesNotToMappedToAnyTable(diagnostics); - diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + index.Name, + entityType.DisplayName(), + index.Properties.Format()); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexEventData( + definition, + NamedIndexAllPropertiesNotToMappedToAnyTable, + entityType, + index.Name, + index.Properties.Select(p => p.Name).ToList()); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } } } - private static string AllIndexPropertiesNotToMappedToAnyTable(EventDefinitionBase definition, EventData payload) + private static string UnnamedIndexAllPropertiesNotToMappedToAnyTable(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (IndexEventData)payload; + return d.GenerateMessage( + p.EntityType.DisplayName(), + p.PropertyNames.Format()); + } + + private static string NamedIndexAllPropertiesNotToMappedToAnyTable(EventDefinitionBase definition, EventData payload) { var d = (EventDefinition)definition; var p = (IndexEventData)payload; @@ -3660,32 +3696,70 @@ public static void IndexPropertiesBothMappedAndNotMappedToTable( [NotNull] IIndex index, [NotNull] string unmappedPropertyName) { - var definition = RelationalResources.LogIndexPropertiesBothMappedAndNotMappedToTable(diagnostics); - - if (diagnostics.ShouldLog(definition)) + if (index.Name == null) { - definition.Log(diagnostics, - index.Name, - entityType.DisplayName(), - index.Properties.Format(), - unmappedPropertyName); - } + var definition = RelationalResources.LogUnnamedIndexPropertiesBothMappedAndNotMappedToTable(diagnostics); - if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + entityType.DisplayName(), + index.Properties.Format(), + unmappedPropertyName); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexInvalidPropertyEventData( + definition, + UnnamedIndexPropertiesBothMappedAndNotMappedToTable, + entityType, + null, + index.Properties.Select(p => p.Name).ToList(), + unmappedPropertyName); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + else { - var eventData = new IndexInvalidPropertyEventData( - definition, - IndexPropertiesBothMappedAndNotMappedToTable, - entityType, - index.Name, - index.Properties.Select(p => p.Name).ToList(), - unmappedPropertyName); + var definition = RelationalResources.LogNamedIndexPropertiesBothMappedAndNotMappedToTable(diagnostics); - diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + index.Name, + entityType.DisplayName(), + index.Properties.Format(), + unmappedPropertyName); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexInvalidPropertyEventData( + definition, + NamedIndexPropertiesBothMappedAndNotMappedToTable, + entityType, + index.Name, + index.Properties.Select(p => p.Name).ToList(), + unmappedPropertyName); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } } } - private static string IndexPropertiesBothMappedAndNotMappedToTable(EventDefinitionBase definition, EventData payload) + private static string UnnamedIndexPropertiesBothMappedAndNotMappedToTable(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (IndexInvalidPropertyEventData)payload; + return d.GenerateMessage( + p.EntityType.DisplayName(), + p.PropertyNames.Format(), + p.InvalidPropertyName); + } + + private static string NamedIndexPropertiesBothMappedAndNotMappedToTable(EventDefinitionBase definition, EventData payload) { var d = (EventDefinition)definition; var p = (IndexInvalidPropertyEventData)payload; @@ -3715,42 +3789,89 @@ public static void IndexPropertiesMappedToNonOverlappingTables( [NotNull] string property2Name, [NotNull] List<(string Table, string Schema)> tablesMappedToProperty2) { - var definition = RelationalResources.LogIndexPropertiesMappedToNonOverlappingTables(diagnostics); - - if (diagnostics.ShouldLog(definition)) + if (index.Name == null) { - definition.Log(diagnostics, - l => l.Log( - definition.Level, - definition.EventId, - definition.MessageFormat, - index.Name, + var definition = RelationalResources.LogUnnamedIndexPropertiesMappedToNonOverlappingTables(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, entityType.DisplayName(), index.Properties.Format(), + property1Name, + tablesMappedToProperty1.FormatTables(), + property2Name, + tablesMappedToProperty2.FormatTables()); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexInvalidPropertiesEventData( + definition, + UnnamedIndexPropertiesMappedToNonOverlappingTables, + entityType, + null, + index.Properties.Select(p => p.Name).ToList(), property1Name, - tablesMappedToProperty1.FormatTables(), + tablesMappedToProperty1, property2Name, - tablesMappedToProperty2.FormatTables())); - } + tablesMappedToProperty2); - if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + else { - var eventData = new IndexInvalidPropertiesEventData( - definition, - IndexPropertiesMappedToNonOverlappingTables, - entityType, - index.Name, - index.Properties.Select(p => p.Name).ToList(), - property1Name, - tablesMappedToProperty1, - property2Name, - tablesMappedToProperty2); + var definition = RelationalResources.LogNamedIndexPropertiesMappedToNonOverlappingTables(diagnostics); - diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, + l => l.Log( + definition.Level, + definition.EventId, + definition.MessageFormat, + index.Name, + entityType.DisplayName(), + index.Properties.Format(), + property1Name, + tablesMappedToProperty1.FormatTables(), + property2Name, + tablesMappedToProperty2.FormatTables())); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new IndexInvalidPropertiesEventData( + definition, + NamedIndexPropertiesMappedToNonOverlappingTables, + entityType, + index.Name, + index.Properties.Select(p => p.Name).ToList(), + property1Name, + tablesMappedToProperty1, + property2Name, + tablesMappedToProperty2); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } } } - private static string IndexPropertiesMappedToNonOverlappingTables(EventDefinitionBase definition, EventData payload) + private static string UnnamedIndexPropertiesMappedToNonOverlappingTables(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (IndexInvalidPropertiesEventData)payload; + return d.GenerateMessage( + p.EntityType.DisplayName(), + p.PropertyNames.Format(), + p.Property1Name, + p.TablesMappedToProperty1.FormatTables(), + p.Property2Name, + p.TablesMappedToProperty2.FormatTables()); + } + + private static string NamedIndexPropertiesMappedToNonOverlappingTables(EventDefinitionBase definition, EventData payload) { var d = (FallbackEventDefinition)definition; var p = (IndexInvalidPropertiesEventData)payload; diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs index 955861b171d..eb528cc2a3d 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs @@ -374,7 +374,7 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] - public EventDefinitionBase LogAllIndexPropertiesNotToMappedToAnyTable; + public EventDefinitionBase LogNamedIndexAllPropertiesNotToMappedToAnyTable; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -383,7 +383,7 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] - public EventDefinitionBase LogIndexPropertiesBothMappedAndNotMappedToTable; + public EventDefinitionBase LogUnnamedIndexAllPropertiesNotToMappedToAnyTable; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -392,6 +392,33 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] - public EventDefinitionBase LogIndexPropertiesMappedToNonOverlappingTables; + public EventDefinitionBase LogNamedIndexPropertiesBothMappedAndNotMappedToTable; + + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase LogUnnamedIndexPropertiesBothMappedAndNotMappedToTable; + + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase LogNamedIndexPropertiesMappedToNonOverlappingTables; + + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase LogUnnamedIndexPropertiesMappedToNonOverlappingTables; } } diff --git a/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs b/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs index 6263ffd59cd..1f320c94255 100644 --- a/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs +++ b/src/EFCore.Relational/Extensions/Internal/TupleExtensions.cs @@ -9,27 +9,30 @@ namespace Microsoft.EntityFrameworkCore.Internal { /// - /// Extension methods for tuples. + /// 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. /// public static class TupleExtensions { /// - /// Creates a formatted string representation of the given tables such as is useful - /// when throwing exceptions. + /// 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. /// - /// The (Table, Schema) tuples to format. - /// The string representation. public static string FormatTables([NotNull] this IEnumerable<(string Table, string Schema)> tables) => "{" + string.Join(", ", tables.Select(FormatTable)) + "}"; /// - /// Creates a formatted string representation of the given table such as is useful - /// when throwing exceptions. + /// 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. /// - /// The (Table, Schema) tuple to format. - /// The string representation. public static string FormatTable(this (string Table, string Schema) table) => "'" + (table.Schema == null ? table.Table : table.Schema + "." + table.Table) diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 892d5335a96..e25de156ffa 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -927,7 +927,7 @@ protected virtual void ValidateIndexProperties( foreach (var entityType in model.GetEntityTypes()) { - foreach (var index in entityType.GetIndexes() + foreach (var index in entityType.GetDeclaredIndexes() .Where(i => ConfigurationSource.Convention != ((IConventionIndex)i).GetConfigurationSource())) { IProperty propertyNotMappedToAnyTable = null; diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 9b4f46b73ec..55ecbf8ed08 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1806,13 +1806,13 @@ public static EventDefinition LogMigrationAttributeMissingWarning([NotNu /// /// The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. None of these properties are mapped to a column in any table. This index will not be created in the database. /// - public static EventDefinition LogAllIndexPropertiesNotToMappedToAnyTable([NotNull] IDiagnosticsLogger logger) + public static EventDefinition LogNamedIndexAllPropertiesNotToMappedToAnyTable([NotNull] IDiagnosticsLogger logger) { - var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogAllIndexPropertiesNotToMappedToAnyTable; + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogNamedIndexAllPropertiesNotToMappedToAnyTable; if (definition == null) { definition = LazyInitializer.EnsureInitialized( - ref ((RelationalLoggingDefinitions)logger.Definitions).LogAllIndexPropertiesNotToMappedToAnyTable, + ref ((RelationalLoggingDefinitions)logger.Definitions).LogNamedIndexAllPropertiesNotToMappedToAnyTable, () => new EventDefinition( logger.Options, RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable, @@ -1821,22 +1821,46 @@ public static EventDefinition LogAllIndexPropertiesNotTo level => LoggerMessage.Define( level, RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable, - _resourceManager.GetString("LogAllIndexPropertiesNotToMappedToAnyTable")))); + _resourceManager.GetString("LogNamedIndexAllPropertiesNotToMappedToAnyTable")))); } return (EventDefinition)definition; } /// - /// The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. Some properties are mapped to a column in a table, but the property '{propertyName}' is not. Either all or none of the properties should be mapped. + /// The unnamed index on the entity type '{entityType}' specifies properties {indexPropertiesList}. None of these properties are mapped to a column in any table. This index will not be created in the database. /// - public static EventDefinition LogIndexPropertiesBothMappedAndNotMappedToTable([NotNull] IDiagnosticsLogger logger) + public static EventDefinition LogUnnamedIndexAllPropertiesNotToMappedToAnyTable([NotNull] IDiagnosticsLogger logger) { - var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertiesBothMappedAndNotMappedToTable; + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogUnnamedIndexAllPropertiesNotToMappedToAnyTable; if (definition == null) { definition = LazyInitializer.EnsureInitialized( - ref ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertiesBothMappedAndNotMappedToTable, + ref ((RelationalLoggingDefinitions)logger.Definitions).LogUnnamedIndexAllPropertiesNotToMappedToAnyTable, + () => new EventDefinition( + logger.Options, + RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable, + LogLevel.Information, + "RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable", + level => LoggerMessage.Define( + level, + RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable, + _resourceManager.GetString("LogUnnamedIndexAllPropertiesNotToMappedToAnyTable")))); + } + + return (EventDefinition)definition; + } + + /// + /// The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. Some properties are mapped to a column in a table, but the property '{propertyName}' is not. All of the properties should be mapped for the index to be created in the database. + /// + public static EventDefinition LogNamedIndexPropertiesBothMappedAndNotMappedToTable([NotNull] IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogNamedIndexPropertiesBothMappedAndNotMappedToTable; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogNamedIndexPropertiesBothMappedAndNotMappedToTable, () => new EventDefinition( logger.Options, RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable, @@ -1845,31 +1869,79 @@ public static EventDefinition LogIndexProperties level => LoggerMessage.Define( level, RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable, - _resourceManager.GetString("LogIndexPropertiesBothMappedAndNotMappedToTable")))); + _resourceManager.GetString("LogNamedIndexPropertiesBothMappedAndNotMappedToTable")))); } return (EventDefinition)definition; } + /// + /// The unnamed index on the entity type '{entityType}' specifies properties {indexPropertiesList}. Some properties are mapped to a column in a table, but the property '{propertyName}' is not. All of the properties should be mapped for the index to be created in the database. + /// + public static EventDefinition LogUnnamedIndexPropertiesBothMappedAndNotMappedToTable([NotNull] IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogUnnamedIndexPropertiesBothMappedAndNotMappedToTable; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogUnnamedIndexPropertiesBothMappedAndNotMappedToTable, + () => new EventDefinition( + logger.Options, + RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable, + LogLevel.Error, + "RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable", + level => LoggerMessage.Define( + level, + RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable, + _resourceManager.GetString("LogUnnamedIndexPropertiesBothMappedAndNotMappedToTable")))); + } + + return (EventDefinition)definition; + } + /// /// The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. /// - public static FallbackEventDefinition LogIndexPropertiesMappedToNonOverlappingTables([NotNull] IDiagnosticsLogger logger) + public static FallbackEventDefinition LogNamedIndexPropertiesMappedToNonOverlappingTables([NotNull] IDiagnosticsLogger logger) { - var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertiesMappedToNonOverlappingTables; + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogNamedIndexPropertiesMappedToNonOverlappingTables; if (definition == null) { definition = LazyInitializer.EnsureInitialized( - ref ((RelationalLoggingDefinitions)logger.Definitions).LogIndexPropertiesMappedToNonOverlappingTables, + ref ((RelationalLoggingDefinitions)logger.Definitions).LogNamedIndexPropertiesMappedToNonOverlappingTables, () => new FallbackEventDefinition( logger.Options, RelationalEventId.IndexPropertiesMappedToNonOverlappingTables, LogLevel.Error, "RelationalEventId.IndexPropertiesMappedToNonOverlappingTables", - _resourceManager.GetString("LogIndexPropertiesMappedToNonOverlappingTables"))); + _resourceManager.GetString("LogNamedIndexPropertiesMappedToNonOverlappingTables"))); } return (FallbackEventDefinition)definition; } + + /// + /// The unnamed index on the entity type '{entityType}' specifies properties {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. + /// + public static EventDefinition LogUnnamedIndexPropertiesMappedToNonOverlappingTables([NotNull] IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogUnnamedIndexPropertiesMappedToNonOverlappingTables; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogUnnamedIndexPropertiesMappedToNonOverlappingTables, + () => new EventDefinition( + logger.Options, + RelationalEventId.IndexPropertiesMappedToNonOverlappingTables, + LogLevel.Error, + "RelationalEventId.IndexPropertiesMappedToNonOverlappingTables", + level => LoggerMessage.Define( + level, + RelationalEventId.IndexPropertiesMappedToNonOverlappingTables, + _resourceManager.GetString("LogUnnamedIndexPropertiesMappedToNonOverlappingTables")))); + } + + return (EventDefinition)definition; + } } } diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index ae065d26ebc..b59a504cf75 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -606,16 +606,28 @@ The property '{property}' on entity type '{entityType}' is not mapped to the table '{table}'. - + The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. None of these properties are mapped to a column in any table. This index will not be created in the database. Information RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable string string string - - The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. Some properties are mapped to a column in a table, but the property '{propertyName}' is not. Either all or none of the properties should be mapped. + + The unnamed index on the entity type '{entityType}' specifies properties {indexPropertiesList}. None of these properties are mapped to a column in any table. This index will not be created in the database. + Information RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable string string + + + The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. Some properties are mapped to a column in a table, but the property '{propertyName}' is not. All of the properties should be mapped for the index to be created in the database. Error RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable string string string string - + + The unnamed index on the entity type '{entityType}' specifies properties {indexPropertiesList}. Some properties are mapped to a column in a table, but the property '{propertyName}' is not. All of the properties should be mapped for the index to be created in the database. + Error RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable string string string + + The index named '{indexName}' on the entity type '{entityType}' specifies properties {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. Error RelationalEventId.IndexPropertiesMappedToNonOverlappingTables string string string string string string string + + The unnamed index on the entity type '{entityType}' specifies properties {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table. + Error RelationalEventId.IndexPropertiesMappedToNonOverlappingTables string string string string string string + \ No newline at end of file diff --git a/src/EFCore/Extensions/PropertyExtensions.cs b/src/EFCore/Extensions/PropertyExtensions.cs index a3232053cc4..f96879def4b 100644 --- a/src/EFCore/Extensions/PropertyExtensions.cs +++ b/src/EFCore/Extensions/PropertyExtensions.cs @@ -400,19 +400,6 @@ public static string Format([NotNull] this IEnumerable properties p => "'" + p.Name + "'" + (includeTypes ? " : " + p.ClrType.DisplayName(fullName: false) : ""))) + "}"; - /// - /// Creates a formatted string representation of the given - /// property names such as is useful when throwing exceptions. - /// - /// The names of the properties to format. - /// The string representation. - public static string Format([NotNull] this IEnumerable propertyNames) - => "{" - + string.Join( - ", ", - propertyNames.Select(name => "'" + name + "'")) - + "}"; - /// /// /// Creates a human-readable representation of the given metadata. diff --git a/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs b/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs index e5b9d5a2c39..8e25c1b804c 100644 --- a/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs @@ -9,6 +9,7 @@ using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; +using Microsoft.EntityFrameworkCore.Utilities; namespace Microsoft.EntityFrameworkCore.Metadata.Conventions { diff --git a/src/Shared/EnumerableExtensions.cs b/src/Shared/EnumerableExtensions.cs index 54beb347d9d..9f08ef12189 100644 --- a/src/Shared/EnumerableExtensions.cs +++ b/src/Shared/EnumerableExtensions.cs @@ -137,5 +137,12 @@ public static async Task> ToListAsync( public static List ToList(this IEnumerable source) => source.OfType().ToList(); + + public static string Format([NotNull] this IEnumerable strings) + => "{" + + string.Join( + ", ", + strings.Select(s => "'" + s + "'")) + + "}"; } } diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 3de014a2d88..1541d52ed5f 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -1249,7 +1249,7 @@ var methodInfo } [ConditionalFact] - public void Passes_for_all_index_properties_not_mapped_to_any_table() + public void Passes_for_unnamed_index_with_all_properties_not_mapped_to_any_table() { var modelBuilder = CreateConventionalModelBuilder(); @@ -1257,11 +1257,32 @@ public void Passes_for_all_index_properties_not_mapped_to_any_table() modelBuilder.Entity().HasIndex(nameof(Animal.Id), nameof(Animal.Name)); var definition = RelationalResources - .LogAllIndexPropertiesNotToMappedToAnyTable( + .LogUnnamedIndexAllPropertiesNotToMappedToAnyTable( new TestLogger()); VerifyWarning( definition.GenerateMessage( - "(null)", + nameof(Animal), + "{'Id', 'Name'}"), + modelBuilder.Model, + LogLevel.Information); + } + + [ConditionalFact] + public void Passes_for_named_index_with_all_properties_not_mapped_to_any_table() + { + var modelBuilder = CreateConventionalModelBuilder(); + + modelBuilder.Entity().ToTable(null); + modelBuilder.Entity() + .HasIndex(nameof(Animal.Id), nameof(Animal.Name)) + .HasName("IX_AllPropertiesNotMapped"); + + var definition = RelationalResources + .LogNamedIndexAllPropertiesNotToMappedToAnyTable( + new TestLogger()); + VerifyWarning( + definition.GenerateMessage( + "IX_AllPropertiesNotMapped", nameof(Animal), "{'Id', 'Name'}"), modelBuilder.Model, @@ -1278,11 +1299,10 @@ public void Detects_mix_of_index_property_mapped_and_not_mapped_to_any_table_unm modelBuilder.Entity().HasIndex(nameof(Animal.Name), nameof(Cat.Identity)); var definition = RelationalResources - .LogIndexPropertiesBothMappedAndNotMappedToTable( + .LogUnnamedIndexPropertiesBothMappedAndNotMappedToTable( new TestLogger()); VerifyWarning( definition.GenerateMessage( - "(null)", nameof(Cat), "{'Name', 'Identity'}", "Name"), @@ -1297,14 +1317,16 @@ public void Detects_mix_of_index_property_mapped_and_not_mapped_to_any_table_map modelBuilder.Entity().ToTable(null); modelBuilder.Entity().ToTable("Cats"); - modelBuilder.Entity().HasIndex(nameof(Cat.Identity), nameof(Animal.Name)); + modelBuilder.Entity() + .HasIndex(nameof(Cat.Identity), nameof(Animal.Name)) + .HasName("IX_MixOfMappedAndUnmappedProperties"); var definition = RelationalResources - .LogIndexPropertiesBothMappedAndNotMappedToTable( + .LogNamedIndexPropertiesBothMappedAndNotMappedToTable( new TestLogger()); VerifyWarning( definition.GenerateMessage( - "(null)", + "IX_MixOfMappedAndUnmappedProperties", nameof(Cat), "{'Identity', 'Name'}", "Name"), @@ -1328,7 +1350,7 @@ public void Passes_for_index_properties_mapped_to_same_table_in_TPT_hierarchy() } [ConditionalFact] - public void Detects_index_properties_mapped_to_different_tables_in_TPT_hierarchy() + public void Detects_unnamed_index_properties_mapped_to_different_tables_in_TPT_hierarchy() { var modelBuilder = CreateConventionalModelBuilder(); @@ -1337,7 +1359,33 @@ public void Detects_index_properties_mapped_to_different_tables_in_TPT_hierarchy modelBuilder.Entity().HasIndex(nameof(Animal.Name), nameof(Cat.Identity)); var definition = RelationalResources - .LogIndexPropertiesMappedToNonOverlappingTables( + .LogUnnamedIndexPropertiesMappedToNonOverlappingTables( + new TestLogger()); + VerifyWarning( + definition.GenerateMessage( + nameof(Cat), + "{'Name', 'Identity'}", + nameof(Animal.Name), + "{'Animals'}", + nameof(Cat.Identity), + "{'Cats'}"), + modelBuilder.Model, + LogLevel.Error); + } + + [ConditionalFact] + public void Detects_named_index_properties_mapped_to_different_tables_in_TPT_hierarchy() + { + var modelBuilder = CreateConventionalModelBuilder(); + + modelBuilder.Entity().ToTable("Animals"); + modelBuilder.Entity().ToTable("Cats"); + modelBuilder.Entity() + .HasIndex(nameof(Animal.Name), nameof(Cat.Identity)) + .HasName("IX_MappedToDifferentTables"); + + var definition = RelationalResources + .LogNamedIndexPropertiesMappedToNonOverlappingTables( new TestLogger()); VerifyWarning( definition.GenerateMessage( @@ -1345,7 +1393,7 @@ public void Detects_index_properties_mapped_to_different_tables_in_TPT_hierarchy definition.Level, definition.EventId, definition.MessageFormat, - "(null)", + "IX_MappedToDifferentTables", nameof(Cat), "{'Name', 'Identity'}", nameof(Animal.Name),