diff --git a/src/EFCore.Design/Metadata/Internal/ScaffoldingAnnotationNames.cs b/src/EFCore.Design/Metadata/Internal/ScaffoldingAnnotationNames.cs index 5ba3449b0db..d2be29a1ef5 100644 --- a/src/EFCore.Design/Metadata/Internal/ScaffoldingAnnotationNames.cs +++ b/src/EFCore.Design/Metadata/Internal/ScaffoldingAnnotationNames.cs @@ -19,14 +19,6 @@ public static class ScaffoldingAnnotationNames /// public const string Prefix = "Scaffolding:"; - /// - /// 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 const string ColumnOrdinal = Prefix + "ColumnOrdinal"; - /// /// 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.Design/Metadata/Internal/ScaffoldingPropertyExtensions.cs b/src/EFCore.Design/Metadata/Internal/ScaffoldingPropertyExtensions.cs deleted file mode 100644 index af53e2a46e1..00000000000 --- a/src/EFCore.Design/Metadata/Internal/ScaffoldingPropertyExtensions.cs +++ /dev/null @@ -1,34 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Microsoft.EntityFrameworkCore.Metadata.Internal -{ - /// - /// 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 ScaffoldingPropertyExtensions - { - /// - /// 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 int GetColumnOrdinal(this IReadOnlyProperty property) - => (int?)property[ScaffoldingAnnotationNames.ColumnOrdinal] ?? -1; - - /// - /// 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 void SetColumnOrdinal(this IMutableProperty property, int? ordinal) - => property.SetOrRemoveAnnotation( - ScaffoldingAnnotationNames.ColumnOrdinal, - ordinal); - } -} diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs index c0aacc2a032..5a09c3d454d 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs @@ -600,7 +600,7 @@ private void GenerateProperty(IProperty property) .FilterIgnoredAnnotations(property.GetAnnotations()) .ToDictionary(a => a.Name, a => a); _annotationCodeGenerator.RemoveAnnotationsHandledByConventions(property, annotations); - annotations.Remove(ScaffoldingAnnotationNames.ColumnOrdinal); + annotations.Remove(RelationalAnnotationNames.ColumnOrder); if (_useDataAnnotations) { @@ -879,7 +879,7 @@ private void GenerateManyToMany(ISkipNavigation skipNavigation) .FilterIgnoredAnnotations(property.GetAnnotations()) .ToDictionary(a => a.Name, a => a); _annotationCodeGenerator.RemoveAnnotationsHandledByConventions(property, propertyAnnotations); - propertyAnnotations.Remove(ScaffoldingAnnotationNames.ColumnOrdinal); + propertyAnnotations.Remove(RelationalAnnotationNames.ColumnOrder); if ((!_useNullableReferenceTypes || property.ClrType.IsValueType) && !property.IsNullable diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs index 2269e313aa5..2ddd507fedd 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs @@ -272,7 +272,7 @@ protected virtual void GenerateProperties(IEntityType entityType) { Check.NotNull(entityType, nameof(entityType)); - foreach (var property in entityType.GetProperties().OrderBy(p => p.GetColumnOrdinal())) + foreach (var property in entityType.GetProperties().OrderBy(p => p.GetColumnOrder() ?? -1)) { GenerateComment(property.GetComment()); diff --git a/src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs b/src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs index 57d3deefe53..39db30a020e 100644 --- a/src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs +++ b/src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs @@ -519,7 +519,7 @@ protected virtual EntityTypeBuilder VisitColumns(EntityTypeBuilder builder, ICol property.IsConcurrencyToken(); } - property.Metadata.SetColumnOrdinal(column.Table.Columns.IndexOf(column)); + property.Metadata.SetColumnOrder(column.Table.Columns.IndexOf(column)); property.Metadata.AddAnnotations( column.GetAnnotations().Where( diff --git a/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs b/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs index 93548874e1d..f5dd8e73498 100644 --- a/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs +++ b/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs @@ -55,6 +55,10 @@ private static readonly MethodInfo _propertyHasColumnNameMethodInfo = typeof(RelationalPropertyBuilderExtensions).GetRequiredRuntimeMethod( nameof(RelationalPropertyBuilderExtensions.HasColumnName), typeof(PropertyBuilder), typeof(string)); + private static readonly MethodInfo _propertyHasColumnOrderMethodInfo + = typeof(RelationalPropertyBuilderExtensions).GetRequiredRuntimeMethod( + nameof(RelationalPropertyBuilderExtensions.HasColumnOrder), typeof(PropertyBuilder), typeof(int?)); + private static readonly MethodInfo _propertyHasDefaultValueSqlMethodInfo1 = typeof(RelationalPropertyBuilderExtensions).GetRequiredRuntimeMethod( nameof(RelationalPropertyBuilderExtensions.HasDefaultValueSql), typeof(PropertyBuilder)); @@ -221,6 +225,10 @@ public virtual IReadOnlyList GenerateFluentApiCalls( annotations, RelationalAnnotationNames.ColumnName, _propertyHasColumnNameMethodInfo, methodCallCodeFragments); + GenerateSimpleFluentApiCall( + annotations, + RelationalAnnotationNames.ColumnOrder, _propertyHasColumnOrderMethodInfo, methodCallCodeFragments); + if (TryGetAndRemove(annotations, RelationalAnnotationNames.DefaultValueSql, out string? defaultValueSql)) { methodCallCodeFragments.Add( diff --git a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs index 165fb5283e9..0789a825c32 100644 --- a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs +++ b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs @@ -367,6 +367,7 @@ public override void Generate(IProperty property, CSharpRuntimeAnnotationCodeGen } else { + annotations.Remove(RelationalAnnotationNames.ColumnOrder); annotations.Remove(RelationalAnnotationNames.Comment); annotations.Remove(RelationalAnnotationNames.Collation); diff --git a/src/EFCore.Relational/Diagnostics/ColumnsEventData.cs b/src/EFCore.Relational/Diagnostics/ColumnsEventData.cs new file mode 100644 index 00000000000..592979376ea --- /dev/null +++ b/src/EFCore.Relational/Diagnostics/ColumnsEventData.cs @@ -0,0 +1,46 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using Microsoft.EntityFrameworkCore.Metadata; + +namespace Microsoft.EntityFrameworkCore.Diagnostics +{ + /// + /// A event payload class for events that have columns. + /// + public class ColumnsEventData : EventData + { + /// + /// Constructs the event payload. + /// + /// The event definition. + /// A delegate that generates a log message for this event. + /// The table. + /// The columns. + public ColumnsEventData( + EventDefinitionBase eventDefinition, + Func messageGenerator, + StoreObjectIdentifier storeObject, + IReadOnlyList columns) + : base(eventDefinition, messageGenerator) + { + StoreObject = storeObject; + Columns = columns; + } + + /// + /// Gets the table. + /// + /// The table. + public virtual StoreObjectIdentifier StoreObject { get; } + + /// + /// Gets the columns. + /// + /// The columns. + public virtual IReadOnlyList Columns { get; } + } +} diff --git a/src/EFCore.Relational/Diagnostics/MigrationColumnOperationEventData.cs b/src/EFCore.Relational/Diagnostics/MigrationColumnOperationEventData.cs new file mode 100644 index 00000000000..e94b37fa0b4 --- /dev/null +++ b/src/EFCore.Relational/Diagnostics/MigrationColumnOperationEventData.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using Microsoft.EntityFrameworkCore.Migrations.Operations; + +namespace Microsoft.EntityFrameworkCore.Diagnostics +{ + /// + /// The event payload for events that reference a Migrations column operation. + /// + public class MigrationColumnOperationEventData : EventData + { + /// + /// Initializes a new instance of the class. + /// + /// The event definition. + /// A delegate that generates a log message for this event. + /// The column operation. + public MigrationColumnOperationEventData( + EventDefinitionBase eventDefinition, + Func messageGenerator, + ColumnOperation columnOperation) + : base(eventDefinition, messageGenerator) + => ColumnOperation = columnOperation; + + /// + /// Gets the column operation. + /// + /// The column operation. + public virtual ColumnOperation ColumnOperation { get; } + } +} diff --git a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs index 0edfb562e47..04fdbf13ac8 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs @@ -72,6 +72,7 @@ private enum Id MigrationsNotApplied, MigrationsNotFound, MigrationAttributeMissingWarning, + ColumnOrderIgnoredWarning, // Query events QueryClientEvaluationWarning = CoreEventId.RelationalBaseId + 500, @@ -88,6 +89,7 @@ private enum Id IndexPropertiesMappedToNonOverlappingTables, ForeignKeyPropertiesMappedToUnrelatedTables, OptionalDependentWithoutIdentifyingPropertyWarning, + DuplicateColumnOrders, // Update events BatchReadyForExecution = CoreEventId.RelationalBaseId + 700, @@ -598,6 +600,19 @@ private static EventId MakeMigrationsId(Id id) /// public static readonly EventId MigrationAttributeMissingWarning = MakeMigrationsId(Id.MigrationAttributeMissingWarning); + /// + /// + /// Column order was ignored. + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId ColumnOrderIgnoredWarning = MakeMigrationsId(Id.ColumnOrderIgnoredWarning); + private static readonly string _queryPrefix = DbLoggerCategory.Query.Name + "."; private static EventId MakeQueryId(Id id) @@ -739,6 +754,19 @@ private static EventId MakeValidationId(Id id) public static readonly EventId OptionalDependentWithoutIdentifyingPropertyWarning = MakeValidationId(Id.OptionalDependentWithoutIdentifyingPropertyWarning); + /// + /// + /// The configured column orders for a table contains duplicates. + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId DuplicateColumnOrders = MakeValidationId(Id.DuplicateColumnOrders); + private static readonly string _updatePrefix = DbLoggerCategory.Update.Name + "."; private static EventId MakeUpdateId(Id id) diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs index 60c550ddbf2..c754ee114f2 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs @@ -6,7 +6,6 @@ using System.Data; using System.Data.Common; using System.Diagnostics; -using System.Globalization; using System.Linq; using System.Reflection; using System.Threading; @@ -18,9 +17,9 @@ using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Migrations; using Microsoft.EntityFrameworkCore.Migrations.Internal; +using Microsoft.EntityFrameworkCore.Migrations.Operations; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.Storage; -using Microsoft.EntityFrameworkCore.Storage.Internal; using Microsoft.EntityFrameworkCore.Update; using Microsoft.EntityFrameworkCore.Utilities; using Microsoft.Extensions.Logging; @@ -3033,5 +3032,78 @@ private static string OptionalDependentWithAllNullPropertiesWarningSensitive(Eve var p = (UpdateEntryEventData)payload; return d.GenerateMessage(p.EntityEntry.EntityType.DisplayName(), p.EntityEntry.BuildCurrentValuesString(p.EntityEntry.EntityType.FindPrimaryKey()!.Properties)); } + + /// + /// Logs the event. + /// + /// The diagnostics logger to use. + /// The table. + /// The columns. + public static void DuplicateColumnOrders( + this IDiagnosticsLogger diagnostics, + StoreObjectIdentifier storeObject, + IReadOnlyList columns) + { + var definition = RelationalResources.LogDuplicateColumnOrders(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, storeObject.DisplayName(), string.Join(", ", columns.Select(c => "'" + c + "'"))); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new ColumnsEventData( + definition, + DuplicateColumnOrders, + storeObject, + columns); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string DuplicateColumnOrders(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (ColumnsEventData)payload; + + return d.GenerateMessage(p.StoreObject.DisplayName(), string.Join(", ", p.Columns.Select(c => "'" + c + "'"))); + } + + /// + /// 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 void ColumnOrderIgnoredWarning( + this IDiagnosticsLogger diagnostics, + ColumnOperation operation) + { + var definition = RelationalResources.LogColumnOrderIgnoredWarning(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, (operation.Table, operation.Schema).FormatTable(), operation.Name); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new MigrationColumnOperationEventData( + definition, + ColumnOrderIgnoredWarning, + operation); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string ColumnOrderIgnoredWarning(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (MigrationColumnOperationEventData)payload; + return d.GenerateMessage((p.ColumnOperation.Table, p.ColumnOperation.Schema).FormatTable(), p.ColumnOperation.Name); + } } } diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs index d79b4e03445..ff254134937 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs @@ -530,5 +530,23 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions /// [EntityFrameworkInternal] public EventDefinitionBase? LogOptionalDependentWithAllNullPropertiesSensitive; + + /// + /// 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? LogDuplicateColumnOrders; + + /// + /// 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? LogColumnOrderIgnoredWarning; } } diff --git a/src/EFCore.Relational/Extensions/RelationalPropertyBuilderExtensions.cs b/src/EFCore.Relational/Extensions/RelationalPropertyBuilderExtensions.cs index 83e4f2ef92a..d37f5f2eedc 100644 --- a/src/EFCore.Relational/Extensions/RelationalPropertyBuilderExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalPropertyBuilderExtensions.cs @@ -129,6 +129,62 @@ public static bool CanSetColumnName( || overrides.ColumnName == name; } + /// + /// Configures the order of the column the property is mapped to. + /// + /// The builder of the property being configured. + /// The column order. + /// The same builder instance so that multiple calls can be chained. + public static PropertyBuilder HasColumnOrder(this PropertyBuilder propertyBuilder, int? order) + { + Check.NotNull(propertyBuilder, nameof(propertyBuilder)); + + propertyBuilder.Metadata.SetColumnOrder(order); + + return propertyBuilder; + } + + /// + /// Configures the order of the column the property is mapped to. + /// + /// The builder of the property being configured. + /// The column order. + /// The same builder instance so that multiple calls can be chained. + public static PropertyBuilder HasColumnOrder(this PropertyBuilder propertyBuilder, int? order) + => (PropertyBuilder)HasColumnOrder((PropertyBuilder)propertyBuilder, order); + + /// + /// Configures the order of the column the property is mapped to. + /// + /// The builder of the property being configured. + /// The column order. + /// A value indicating whether the configuration was specified using a data annotation. + /// The same builder instance if the configuration was applied, otherwise. + public static IConventionPropertyBuilder? HasColumnOrder( + this IConventionPropertyBuilder propertyBuilder, + int? order, + bool fromDataAnnotation = false) + { + if (!propertyBuilder.CanSetColumnOrder(order, fromDataAnnotation)) + { + return null; + } + + propertyBuilder.Metadata.SetColumnOrder(order, fromDataAnnotation); + + return propertyBuilder; + } + + /// + /// Gets a value indicating whether the given column order can be set for the property. + /// + /// The builder of the property being configured. + /// The column order. + /// A value indicating whether the configuration was specified using a data annotation. + /// if the column order can be set for the property. + public static bool CanSetColumnOrder(this IConventionPropertyBuilder propertyBuilder, int? order, bool fromDataAnnotation = false) + => propertyBuilder.CanSetAnnotation(RelationalAnnotationNames.ColumnOrder, order, fromDataAnnotation); + /// /// Configures the data type of the column that the property maps to when targeting a relational database. /// This should be the complete type name, including precision, scale, length, etc. diff --git a/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs b/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs index c8174eb0649..87184726308 100644 --- a/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs @@ -277,6 +277,74 @@ public static void SetColumnName( in StoreObjectIdentifier storeObject) => ((RelationalPropertyOverrides?)RelationalPropertyOverrides.Find(property, storeObject))?.GetColumnNameConfigurationSource(); + /// + /// Returns the order of the column this property is mapped to. + /// + /// The property. + /// The column order. + public static int? GetColumnOrder(this IReadOnlyProperty property) + => property is RuntimeProperty + ? throw new InvalidOperationException(CoreStrings.RuntimeModelMissingData) + : (int?)property.FindAnnotation(RelationalAnnotationNames.ColumnOrder)?.Value; + + /// + /// Returns the order of the column this property is mapped to for a particular table. + /// + /// The property. + /// The identifier of the table-like store object containing the column. + /// The column order. + public static int? GetColumnOrder(this IReadOnlyProperty property, in StoreObjectIdentifier storeObject) + { + if (property is RuntimeProperty) + { + throw new InvalidOperationException(CoreStrings.RuntimeModelMissingData); + } + + var annotation = property.FindAnnotation(RelationalAnnotationNames.ColumnOrder); + if (annotation != null) + { + return (int?)annotation.Value; + } + + var sharedTableRootProperty = property.FindSharedStoreObjectRootProperty(storeObject); + if (sharedTableRootProperty != null) + { + return GetColumnOrder(sharedTableRootProperty, storeObject); + } + + return null; + } + + /// + /// Sets the order of the column the property is mapped to. + /// + /// The property. + /// The column order. + public static void SetColumnOrder(this IMutableProperty property, int? order) + => property.SetOrRemoveAnnotation(RelationalAnnotationNames.ColumnOrder, order); + + /// + /// Sets the order of the column the property is mapped to. + /// + /// The property. + /// The column order. + /// A value indicating whether the configuration was specified using a data annotation. + /// The configured value. + public static int? SetColumnOrder(this IConventionProperty property, int? order, bool fromDataAnnotation = false) + { + property.SetOrRemoveAnnotation(RelationalAnnotationNames.ColumnOrder, order, fromDataAnnotation); + + return order; + } + + /// + /// Gets the of the column order. + /// + /// The property. + /// The . + public static ConfigurationSource? GetColumnOrderConfigurationSource(this IConventionProperty property) + => property.FindAnnotation(RelationalAnnotationNames.ColumnName)?.GetConfigurationSource(); + /// /// Returns the database type of the column to which the property is mapped, or if the database type /// could not be found. diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 69ea9a40dad..166e8b99063 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -738,6 +738,26 @@ protected virtual void ValidateSharedColumnsCompatibility( } } } + + var columnOrders = new Dictionary>(); + foreach (var property in propertyMappings.Values) + { + var columnOrder = property.GetColumnOrder(storeObject); + if (!columnOrder.HasValue) + { + continue; + } + + var columns = columnOrders.GetOrAddNew(columnOrder.Value); + columns.Add(property.GetColumnName(storeObject)!); + } + + if (columnOrders.Any(g => g.Value.Count > 1)) + { + logger.DuplicateColumnOrders( + storeObject, + columnOrders.Where(g => g.Value.Count > 1).SelectMany(g => g.Value).ToList()); + } } /// @@ -972,6 +992,22 @@ protected virtual void ValidateCompatible( previousCollation, currentCollation)); } + + var currentColumnOrder = property.GetColumnOrder(storeObject); + var previousColumnOrder = duplicateProperty.GetColumnOrder(storeObject); + if (currentColumnOrder != previousColumnOrder) + { + throw new InvalidOperationException( + RelationalStrings.DuplicateColumnNameOrderMismatch( + duplicateProperty.DeclaringEntityType.DisplayName(), + duplicateProperty.Name, + property.DeclaringEntityType.DisplayName(), + property.Name, + columnName, + storeObject.DisplayName(), + previousColumnOrder, + currentColumnOrder)); + } } /// diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalColumnAttributeConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalColumnAttributeConvention.cs index 38a2727d04e..02e7d426187 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalColumnAttributeConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalColumnAttributeConvention.cs @@ -56,6 +56,11 @@ protected override void ProcessPropertyAdded( { propertyBuilder.HasColumnType(attribute.TypeName, fromDataAnnotation: true); } + + if (attribute.Order >= 0) + { + propertyBuilder.HasColumnOrder(attribute.Order, fromDataAnnotation: true); + } } } } diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs index 0a6cae53807..a959d3b1cea 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs @@ -255,6 +255,7 @@ protected override void ProcessPropertyAnnotations( } else { + annotations.Remove(RelationalAnnotationNames.ColumnOrder); annotations.Remove(RelationalAnnotationNames.Comment); annotations.Remove(RelationalAnnotationNames.Collation); diff --git a/src/EFCore.Relational/Metadata/IColumn.cs b/src/EFCore.Relational/Metadata/IColumn.cs index f9ea1081fa4..413f8f74631 100644 --- a/src/EFCore.Relational/Metadata/IColumn.cs +++ b/src/EFCore.Relational/Metadata/IColumn.cs @@ -64,6 +64,13 @@ bool IsRowVersion => PropertyMappings.First().Property.IsConcurrencyToken && PropertyMappings.First().Property.ValueGenerated == ValueGenerated.OnAddOrUpdate; + /// + /// Gets the column order. + /// + /// The column order. + public virtual int? Order + => PropertyMappings.First().Property.GetColumnOrder(StoreObjectIdentifier.Table(Table.Name, Table.Schema)); + /// /// Returns the object that is used as the default value for this column. /// diff --git a/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs b/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs index e1b3b0ecd1d..eca1b00e452 100644 --- a/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs +++ b/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs @@ -21,6 +21,11 @@ public static class RelationalAnnotationNames /// public const string ColumnName = Prefix + "ColumnName"; + /// + /// The name for column order annotations. + /// + public const string ColumnOrder = Prefix + "ColumnOrder"; + /// /// The name for column type annotations. /// diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs index ab09750a5ef..8952357e1ce 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs @@ -739,7 +739,11 @@ private static IEnumerable GetSortedColumns(ITable table) } } - return sortedColumns; + Check.DebugAssert(columns.Count == 0, "columns is not empty"); + + return sortedColumns.Where(c => c.Order.HasValue).OrderBy(c => c.Order) + .Concat(sortedColumns.Where(c => !c.Order.HasValue)) + .Concat(columns); } private static IEnumerable GetSortedProperties(IEntityType entityType, ITable table) @@ -1030,6 +1034,7 @@ protected virtual IEnumerable Diff( || !Equals(sourceDefault, targetDefault) || source.Comment != target.Comment || source.Collation != target.Collation + || source.Order != target.Order || HasDifferences(sourceMigrationsAnnotations, targetMigrationsAnnotations)) { var isDestructiveChange = isNullableChanged && source.IsNullable @@ -1055,6 +1060,19 @@ protected virtual IEnumerable Diff( alterColumnOperation.OldColumn, source, sourceTypeMapping, source.IsNullable, sourceMigrationsAnnotations, inline: true); + if (source.Order != target.Order) + { + if (source.Order.HasValue) + { + alterColumnOperation.OldColumn.AddAnnotation(RelationalAnnotationNames.ColumnOrder, source.Order.Value); + } + + if (target.Order.HasValue) + { + alterColumnOperation.AddAnnotation(RelationalAnnotationNames.ColumnOrder, target.Order.Value); + } + } + yield return alterColumnOperation; } } @@ -1086,6 +1104,11 @@ protected virtual IEnumerable Add( operation, target, targetTypeMapping, target.IsNullable, target.GetAnnotations(), inline); + if (!inline && target.Order.HasValue) + { + operation.AddAnnotation(RelationalAnnotationNames.ColumnOrder, target.Order.Value); + } + yield return operation; } diff --git a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs index 2b9b4ae03e1..8ebacab078a 100644 --- a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs +++ b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs @@ -193,6 +193,11 @@ protected virtual void Generate( Check.NotNull(operation, nameof(operation)); Check.NotNull(builder, nameof(builder)); + if (operation[RelationalAnnotationNames.ColumnOrder] != null) + { + Dependencies.MigrationsLogger.ColumnOrderIgnoredWarning(operation); + } + builder .Append("ALTER TABLE ") .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 4244c78f8db..34c87273ba9 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1,7 +1,9 @@ // using System; +using System.Reflection; using System.Resources; +using System.Threading; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.Extensions.Logging; @@ -403,6 +405,14 @@ public static string DuplicateColumnNameNullabilityMismatch(object? entityType1, GetString("DuplicateColumnNameNullabilityMismatch", nameof(entityType1), nameof(property1), nameof(entityType2), nameof(property2), nameof(columnName), nameof(table)), entityType1, property1, entityType2, property2, columnName, table); + /// + /// '{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured to use different column orders ('{columnOrder1}' and '{columnOrder2}'). + /// + public static string DuplicateColumnNameOrderMismatch(object? entityType1, object? property1, object? entityType2, object? property2, object? columnName, object? table, object? columnOrder1, object? columnOrder2) + => string.Format( + GetString("DuplicateColumnNameOrderMismatch", nameof(entityType1), nameof(property1), nameof(entityType2), nameof(property2), nameof(columnName), nameof(table), nameof(columnOrder1), nameof(columnOrder2)), + entityType1, property1, entityType2, property2, columnName, table, columnOrder1, columnOrder2); + /// /// '{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured with different precisions ('{precision1}' and '{precision2}'). /// @@ -483,6 +493,14 @@ public static string DuplicateIndexColumnMismatch(object? indexProperties1, obje GetString("DuplicateIndexColumnMismatch", nameof(indexProperties1), nameof(entityType1), nameof(indexProperties2), nameof(entityType2), nameof(table), nameof(indexName), nameof(columnNames1), nameof(columnNames2)), indexProperties1, entityType1, indexProperties2, entityType2, table, indexName, columnNames1, columnNames2); + /// + /// The indexes {indexProperties1} on '{entityType1}' and {indexProperties2} on '{entityType2}' are both mapped to '{table}.{indexName}', but with different filters ('{filter1}' and '{filter2}'). + /// + public static string DuplicateIndexFiltersMismatch(object? indexProperties1, object? entityType1, object? indexProperties2, object? entityType2, object? table, object? indexName, object? filter1, object? filter2) + => string.Format( + GetString("DuplicateIndexFiltersMismatch", nameof(indexProperties1), nameof(entityType1), nameof(indexProperties2), nameof(entityType2), nameof(table), nameof(indexName), nameof(filter1), nameof(filter2)), + indexProperties1, entityType1, indexProperties2, entityType2, table, indexName, filter1, filter2); + /// /// The indexes {indexProperties1} on '{entityType1}' and {indexProperties2} on '{entityType2}' are both mapped to '{indexName}', but are declared on different tables ('{table1}' and '{table2}'). /// @@ -499,14 +517,6 @@ public static string DuplicateIndexUniquenessMismatch(object? indexProperties1, GetString("DuplicateIndexUniquenessMismatch", nameof(indexProperties1), nameof(entityType1), nameof(indexProperties2), nameof(entityType2), nameof(table), nameof(indexName)), indexProperties1, entityType1, indexProperties2, entityType2, table, indexName); - /// - /// The indexes {indexProperties1} on '{entityType1}' and {indexProperties2} on '{entityType2}' are both mapped to '{table}.{indexName}', but with different filters ('{filter1}' and '{filter2}'). - /// - public static string DuplicateIndexFiltersMismatch(object? indexProperties1, object? entityType1, object? indexProperties2, object? entityType2, object? table, object? indexName, object? filter1, object? filter2) - => string.Format( - GetString("DuplicateIndexFiltersMismatch", nameof(indexProperties1), nameof(entityType1), nameof(indexProperties2), nameof(entityType2), nameof(table), nameof(indexName), nameof(filter1), nameof(filter2)), - indexProperties1, entityType1, indexProperties2, entityType2, table, indexName, filter1, filter2); - /// /// The keys {keyProperties1} on '{entityType1}' and {keyProperties2} on '{entityType2}' are both mapped to '{table}.{keyName}', but with different columns ({columnNames1} and {columnNames2}). /// @@ -1116,10 +1126,10 @@ public static string UnableToBindMemberToEntityProjection(object? memberType, ob /// /// Unhandled annotatable type '{annotatableType}'. /// - public static string UnhandledAnnotatableType(object? expressionType) + public static string UnhandledAnnotatableType(object? annotatableType) => string.Format( - GetString("UnhandledAnnotatableType", nameof(expressionType)), - expressionType); + GetString("UnhandledAnnotatableType", nameof(annotatableType)), + annotatableType); /// /// Unhandled expression '{expression}' of type '{expressionType}' encountered in '{visitor}'. @@ -1579,6 +1589,31 @@ public static EventDefinition LogClosingConnection(IDiagnosticsL return (EventDefinition)definition; } + /// + /// The order of column '{table}.{column}' was ignored. Column orders are only used when the table is first created. + /// + public static EventDefinition LogColumnOrderIgnoredWarning(IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogColumnOrderIgnoredWarning; + if (definition == null) + { + definition = NonCapturingLazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogColumnOrderIgnoredWarning, + logger, + static logger => new EventDefinition( + logger.Options, + RelationalEventId.ColumnOrderIgnoredWarning, + LogLevel.Warning, + "RelationalEventId.ColumnOrderIgnoredWarning", + level => LoggerMessage.Define( + level, + RelationalEventId.ColumnOrderIgnoredWarning, + _resourceManager.GetString("LogColumnOrderIgnoredWarning")!))); + } + + return (EventDefinition)definition; + } + /// /// Created DbCommand for '{executionType}' ({elapsed}ms). /// @@ -1854,6 +1889,31 @@ public static EventDefinition LogDisposingTransaction(IDiagnosticsLogger logger) return (EventDefinition)definition; } + /// + /// The configured column orders for the table '{table}' contains duplicates. Ensure the specified column order values are distinct. Conflicting columns: {columns} + /// + public static EventDefinition LogDuplicateColumnOrders(IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogDuplicateColumnOrders; + if (definition == null) + { + definition = NonCapturingLazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogDuplicateColumnOrders, + logger, + static logger => new EventDefinition( + logger.Options, + RelationalEventId.DuplicateColumnOrders, + LogLevel.Error, + "RelationalEventId.DuplicateColumnOrders", + level => LoggerMessage.Define( + level, + RelationalEventId.DuplicateColumnOrders, + _resourceManager.GetString("LogDuplicateColumnOrders")!))); + } + + return (EventDefinition)definition; + } + /// /// Executed DbCommand ({elapsed}ms) [Parameters=[{parameters}], CommandType='{commandType}', CommandTimeout='{commandTimeout}']{newLine}{commandText} /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index b0a1e6bada0..30ceccf692e 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -267,6 +267,9 @@ '{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured with different nullability settings. + + '{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured to use different column orders ('{columnOrder1}' and '{columnOrder2}'). + '{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured with different precisions ('{precision1}' and '{precision2}'). @@ -297,15 +300,15 @@ The indexes {indexProperties1} on '{entityType1}' and {indexProperties2} on '{entityType2}' are both mapped to '{table}.{indexName}', but with different columns ({columnNames1} and {columnNames2}). + + The indexes {indexProperties1} on '{entityType1}' and {indexProperties2} on '{entityType2}' are both mapped to '{table}.{indexName}', but with different filters ('{filter1}' and '{filter2}'). + The indexes {indexProperties1} on '{entityType1}' and {indexProperties2} on '{entityType2}' are both mapped to '{indexName}', but are declared on different tables ('{table1}' and '{table2}'). The indexes {indexProperties1} on '{entityType1}' and {indexProperties2} on '{entityType2}' are both mapped to '{table}.{indexName}', but with different uniqueness configurations. - - The indexes {indexProperties1} on '{entityType1}' and {indexProperties2} on '{entityType2}' are both mapped to '{table}.{indexName}', but with different filters ('{filter1}' and '{filter2}'). - The keys {keyProperties1} on '{entityType1}' and {keyProperties2} on '{entityType2}' are both mapped to '{table}.{keyName}', but with different columns ({columnNames1} and {columnNames2}). @@ -456,6 +459,10 @@ Closing connection to database '{database}' on server '{server}'. Debug RelationalEventId.ConnectionClosing string string + + The order of column '{table}.{column}' was ignored. Column orders are only used when the table is first created. + Warning RelationalEventId.ColumnOrderIgnoredWarning string string + Created DbCommand for '{executionType}' ({elapsed}ms). Debug RelationalEventId.CommandCreated string int @@ -500,6 +507,10 @@ Disposing transaction. Debug RelationalEventId.TransactionDisposed + + The configured column orders for the table '{table}' contains duplicates. Ensure the specified column order values are distinct. Conflicting columns: {columns} + Error RelationalEventId.DuplicateColumnOrders string string + Executed DbCommand ({elapsed}ms) [Parameters=[{parameters}], CommandType='{commandType}', CommandTimeout='{commandTimeout}']{newLine}{commandText} Information RelationalEventId.CommandExecuted string string System.Data.CommandType int string string diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index f0221440284..19289767eba 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -9,6 +9,7 @@ using System.Linq; using System.Text; using System.Text.RegularExpressions; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Migrations.Operations; @@ -244,6 +245,11 @@ protected override void Generate( Check.NotNull(operation, nameof(operation)); Check.NotNull(builder, nameof(builder)); + if (operation[RelationalAnnotationNames.ColumnOrder] != operation.OldColumn[RelationalAnnotationNames.ColumnOrder]) + { + Dependencies.MigrationsLogger.ColumnOrderIgnoredWarning(operation); + } + IEnumerable? indexesToRebuild = null; var column = model?.GetRelationalModel().FindTable(operation.Table, operation.Schema) ?.Columns.FirstOrDefault(c => c.Name == operation.Name); diff --git a/src/EFCore.Sqlite.Core/Migrations/SqliteMigrationsSqlGenerator.cs b/src/EFCore.Sqlite.Core/Migrations/SqliteMigrationsSqlGenerator.cs index 8f85d47e404..0dc4ed4c3e9 100644 --- a/src/EFCore.Sqlite.Core/Migrations/SqliteMigrationsSqlGenerator.cs +++ b/src/EFCore.Sqlite.Core/Migrations/SqliteMigrationsSqlGenerator.cs @@ -312,7 +312,8 @@ private IReadOnlyList RewriteOperations( createTableOperation.PrimaryKey = AddPrimaryKeyOperation.CreateFrom(primaryKey); } - foreach (var column in table.Columns) + foreach (var column in table.Columns.Where(c => c.Order.HasValue).OrderBy(c => c.Order.Value) + .Concat(table.Columns.Where(c => !c.Order.HasValue))) { if (!column.TryGetDefaultValue(out var defaultValue)) { diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs index bcd377ce3ac..7f25686fcce 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs @@ -64,6 +64,7 @@ public void Test_new_annotations_handled_for_entity_types() CoreAnnotationNames.EagerLoaded, CoreAnnotationNames.DuplicateServiceProperties, RelationalAnnotationNames.ColumnName, + RelationalAnnotationNames.ColumnOrder, RelationalAnnotationNames.ColumnType, RelationalAnnotationNames.TableColumnMappings, RelationalAnnotationNames.ViewColumnMappings, @@ -245,6 +246,10 @@ public void Test_new_annotations_handled_for_properties() RelationalAnnotationNames.ColumnName, ("MyColumn", $@"{columnMapping}{_nl}.{nameof(RelationalPropertyBuilderExtensions.HasColumnName)}(""MyColumn"")") }, + { + RelationalAnnotationNames.ColumnOrder, + (1, $@"{columnMapping}{_nl}.{nameof(RelationalPropertyBuilderExtensions.HasColumnOrder)}(1)") + }, { RelationalAnnotationNames.ColumnType, ("int", $@"{_nl}.{nameof(RelationalPropertyBuilderExtensions.HasColumnType)}(""int"")") diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index c031d6f994d..ecc0398c898 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -3763,6 +3763,37 @@ public virtual void Property_with_identity_column_custom_seed_increment() }); } + [ConditionalFact] + public virtual void Property_column_order_annotation_is_stored_in_snapshot_as_fluent_api() + { + Test( + builder => + { + builder.Entity().Property("AlternateId").HasColumnOrder(1); + builder.Ignore(); + }, + AddBoilerPlate( + GetHeading() + + @" + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithTwoProperties"", b => + { + b.Property(""Id"") + .ValueGeneratedOnAdd() + .HasColumnType(""int""); + + SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property(""Id""), 1L, 1); + + b.Property(""AlternateId"") + .HasColumnType(""int"") + .HasColumnOrder(1); + + b.HasKey(""Id""); + + b.ToTable(""EntityWithTwoProperties""); + });"), + o => Assert.Equal(1, o.GetEntityTypes().First().FindProperty("AlternateId").GetColumnOrder())); + } + #endregion #region HasKey diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpModelGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpModelGeneratorTest.cs index 29066bf0bf8..216db0bb068 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpModelGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpModelGeneratorTest.cs @@ -29,7 +29,7 @@ public void WriteCode_works() { var generator = CreateGenerator(); var modelBuilder = RelationalTestHelpers.Instance.CreateConventionBuilder(); - modelBuilder.Entity("TestEntity").Property("Id").HasAnnotation(ScaffoldingAnnotationNames.ColumnOrdinal, 0); + modelBuilder.Entity("TestEntity").Property("Id"); var result = generator.GenerateModel( modelBuilder.FinalizeModel(designTime: true), diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs index 2cc88a9c5ef..c93b47dedc1 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs @@ -1929,6 +1929,10 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) Assert.Equal( CoreStrings.RuntimeModelMissingData, Assert.Throws(() => rowid.GetCollation()).Message); + Assert.Null(rowid[RelationalAnnotationNames.ColumnOrder]); + Assert.Equal( + CoreStrings.RuntimeModelMissingData, + Assert.Throws(() => rowid.GetColumnOrder()).Message); Assert.Null(rowid.GetValueConverter()); Assert.NotNull(rowid.GetValueComparer()); Assert.NotNull(rowid.GetKeyValueComparer()); @@ -2115,7 +2119,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) jb.Property("rowid") .IsRowVersion() .HasComment("RowVersion") - .UseCollation("ri"); + .UseCollation("ri") + .HasColumnOrder(1); }); eb.Navigation(e => e.Principals).AutoInclude(); diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs index f09d8645523..c2a84daa5a8 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs @@ -422,9 +422,9 @@ public void Column_ordinal_annotation() var property2 = (Property)entityTypeA.FindProperty("Col2"); var property3 = (Property)entityTypeA.FindProperty("Col3"); - Assert.Equal(0, property1.GetColumnOrdinal()); - Assert.Equal(1, property2.GetColumnOrdinal()); - Assert.Equal(2, property3.GetColumnOrdinal()); + Assert.Equal(0, property1.GetColumnOrder()); + Assert.Equal(1, property2.GetColumnOrder()); + Assert.Equal(2, property3.GetColumnOrder()); } [ConditionalTheory] diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 9049e58ca6b..add186ec9a9 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -820,6 +820,20 @@ public virtual void Detects_duplicate_column_names_within_hierarchy_with_differe modelBuilder); } + [ConditionalFact] + public virtual void Detects_duplicate_column_names_within_hierarchy_with_different_orders() + { + var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity(); + modelBuilder.Entity().Property(c => c.Breed).HasColumnName("Breed").HasColumnOrder(0); + modelBuilder.Entity().Property(c => c.Breed).HasColumnName("Breed"); + + VerifyError( + RelationalStrings.DuplicateColumnNameOrderMismatch( + nameof(Cat), nameof(Cat.Breed), nameof(Dog), nameof(Dog.Breed), nameof(Cat.Breed), nameof(Animal), 0, null), + modelBuilder); + } + [ConditionalFact] public virtual void Detects_duplicate_column_names_within_hierarchy_with_different_precision() { @@ -2120,6 +2134,21 @@ public virtual void Passes_for_relational_override_without_inheritance() Assert.DoesNotContain(LoggerFactory.Log, l => l.Level == LogLevel.Warning); } + [ConditionalFact] + public virtual void Detects_duplicate_column_orders() + { + var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity( + x => + { + x.Property(a => a.Id).HasColumnOrder(0); + x.Property(a => a.Name).HasColumnOrder(0); + }); + + var definition = RelationalResources.LogDuplicateColumnOrders(new TestLogger()); + VerifyWarning(definition.GenerateMessage("Animal", "'Id', 'Name'"), modelBuilder, LogLevel.Error); + } + protected override void SetBaseType(IMutableEntityType entityType, IMutableEntityType baseEntityType) { base.SetBaseType(entityType, baseEntityType); diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalPropertyAttributeConventionTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalPropertyAttributeConventionTest.cs index 30e9f0775b8..1152e4c2777 100644 --- a/test/EFCore.Relational.Tests/Metadata/RelationalPropertyAttributeConventionTest.cs +++ b/test/EFCore.Relational.Tests/Metadata/RelationalPropertyAttributeConventionTest.cs @@ -25,6 +25,7 @@ public void ColumnAttribute_sets_column_name_and_type_with_conventional_builder( Assert.Equal("Post Name", entityBuilder.Property(e => e.Name).Metadata.GetColumnBaseName()); Assert.Equal("DECIMAL", entityBuilder.Property(e => e.Name).Metadata.GetColumnType()); + Assert.Equal(1, entityBuilder.Property(e => e.Name).Metadata.GetColumnOrder()); } [ConditionalFact] @@ -46,6 +47,7 @@ public void ColumnAttribute_on_field_sets_column_name_and_type_with_conventional Assert.Equal("Post Name", entityBuilder.Property(nameof(F.Name)).Metadata.GetColumnBaseName()); Assert.Equal("DECIMAL", entityBuilder.Property(nameof(F.Name)).Metadata.GetColumnType()); + Assert.Equal(1, entityBuilder.Property(nameof(F.Name)).Metadata.GetColumnOrder()); } [ConditionalFact] @@ -67,12 +69,14 @@ public void ColumnAttribute_overrides_configuration_from_convention_source() propertyBuilder.HasAnnotation(RelationalAnnotationNames.ColumnName, "ConventionalName", ConfigurationSource.Convention); propertyBuilder.HasAnnotation(RelationalAnnotationNames.ColumnType, "BYTE", ConfigurationSource.Convention); + propertyBuilder.HasAnnotation(RelationalAnnotationNames.ColumnOrder, 2, ConfigurationSource.Convention); propertyBuilder.HasAnnotation(RelationalAnnotationNames.Comment, "ConventionalName", ConfigurationSource.Convention); RunConvention(propertyBuilder); Assert.Equal("Post Name", propertyBuilder.Metadata.GetColumnBaseName()); Assert.Equal("DECIMAL", propertyBuilder.Metadata.GetColumnType()); + Assert.Equal(1, propertyBuilder.Metadata.GetColumnOrder()); Assert.Equal("Test column comment", propertyBuilder.Metadata.GetComment()); } @@ -99,12 +103,14 @@ public void ColumnAttribute_does_not_override_configuration_from_explicit_source propertyBuilder.HasAnnotation(RelationalAnnotationNames.ColumnName, "ExplicitName", ConfigurationSource.Explicit); propertyBuilder.HasAnnotation(RelationalAnnotationNames.ColumnType, "BYTE", ConfigurationSource.Explicit); + propertyBuilder.HasAnnotation(RelationalAnnotationNames.ColumnOrder, 2, ConfigurationSource.Explicit); propertyBuilder.HasAnnotation(RelationalAnnotationNames.Comment, "ExplicitComment", ConfigurationSource.Explicit); RunConvention(propertyBuilder); Assert.Equal("ExplicitName", propertyBuilder.Metadata.GetColumnBaseName()); Assert.Equal("BYTE", propertyBuilder.Metadata.GetColumnType()); + Assert.Equal(2, propertyBuilder.Metadata.GetColumnOrder()); Assert.Equal("ExplicitComment", propertyBuilder.Metadata.GetComment()); } diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs index 45aec7aaa3b..d96a079aad8 100644 --- a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs @@ -736,6 +736,34 @@ public void Create_table_columns_handles_self_referencing_one_to_one() }); } + [ConditionalFact] + public void Create_table_columns_use_explicit_order() + { + Execute( + _ => { }, + modelBuilder => modelBuilder.Entity( + b => + { + b.Property(e => e.C).HasColumnOrder(3); + b.Property(e => e.B).HasColumnOrder(1); + b.Property(e => e.A).HasColumnOrder(2); + }), + operations => + { + var operation = Assert.IsType(Assert.Single(operations)); + Assert.Collection( + operation.Columns, + x => + { + Assert.Equal("B", x.Name); + Assert.Null(operation.FindAnnotation(RelationalAnnotationNames.ColumnOrder)); + }, + x => Assert.Equal("A", x.Name), + x => Assert.Equal("C", x.Name), + x => Assert.Equal("Id", x.Name)); + }); + } + [ConditionalFact] public void Create_FK_to_excluded_principal() { @@ -1566,6 +1594,31 @@ public void Add_column_with_seed_data() })); } + [ConditionalFact] + public void Add_column_with_order() + { + Execute( + source => source.Entity("Peacock").Property("Id"), + target => target.Entity( + "Peacock", + x => + { + x.Property("Id"); + x.Property("Name") + .HasColumnOrder(1); + }), + operations => + { + Assert.Equal(1, operations.Count); + + var operation = Assert.IsType(operations[0]); + Assert.Equal("Peacock", operation.Table); + Assert.Equal("Name", operation.Name); + Assert.Equal(typeof(string), operation.ClrType); + Assert.Equal(1, operation[RelationalAnnotationNames.ColumnOrder]); + }); + } + [ConditionalFact] public void Add_seed_data_with_non_writable_column_insert_operations_with_batching() { @@ -2665,6 +2718,71 @@ public void Alter_column_comment() }); } + [ConditionalFact] + public void Alter_column_order() + { + Execute( + source => source.Entity( + "Pangolin", + x => + { + x.Property("Id"); + x.Property("Name") + .HasColumnOrder(1); + }), + target => target.Entity( + "Pangolin", + x => + { + x.Property("Id"); + x.Property("Name") + .HasColumnOrder(2); + }), + operations => + { + Assert.Equal(1, operations.Count); + + var operation = Assert.IsType(operations[0]); + Assert.Equal("Pangolin", operation.Table); + Assert.Equal("Name", operation.Name); + Assert.Equal(2, operation[RelationalAnnotationNames.ColumnOrder]); + Assert.Equal(1, operation.OldColumn[RelationalAnnotationNames.ColumnOrder]); + }); + } + + [ConditionalFact] + public void Alter_column_but_not_order() + { + Execute( + source => source.Entity( + "Crane", + x => + { + x.Property("Id"); + x.Property("Name") + .HasColumnOrder(1); + }), + target => target.Entity( + "Crane", + x => + { + x.Property("Id"); + x.Property("Name") + .HasColumnOrder(1) + .IsUnicode(false); + }), + operations => + { + Assert.Equal(1, operations.Count); + + var operation = Assert.IsType(operations[0]); + Assert.Equal("Crane", operation.Table); + Assert.Equal("Name", operation.Name); + Assert.Null(operation.FindAnnotation(RelationalAnnotationNames.ColumnOrder)); + Assert.Null(operation.OldColumn.FindAnnotation(RelationalAnnotationNames.ColumnOrder)); + }); + } + [ConditionalFact] public void Add_unique_constraint() { diff --git a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs index 5cfdbcd2531..12cd97a2969 100644 --- a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs +++ b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs @@ -17,6 +17,7 @@ using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Migrations.Operations; using Microsoft.EntityFrameworkCore.Query; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.Storage; @@ -46,11 +47,13 @@ public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() var index = new Index(new List { property }, "IndexName", entityType, ConfigurationSource.Convention); var contextServices = RelationalTestHelpers.Instance.CreateContextServices(model.FinalizeModel()); var updateEntry = new InternalEntityEntry(contextServices.GetRequiredService(), entityType, new object()); + var columnOperation = new AddColumnOperation { Name = "Column1", Table = "Table1", ClrType = typeof(int) }; var fakeFactories = new Dictionary> { { typeof(string), () => "Fake" }, { typeof(IList), () => new List { "Fake1", "Fake2" } }, + { typeof(IReadOnlyList), () => new List { "Fake1", "Fake2" } }, { typeof(IEnumerable), () => new List { @@ -81,7 +84,8 @@ public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() { typeof(ValueConverter), () => new BoolToZeroOneConverter() }, { typeof(DbContext), () => new FakeDbContext() }, { typeof(SqlExpression), () => new FakeSqlExpression() }, - { typeof(IUpdateEntry), () => updateEntry} + { typeof(IUpdateEntry), () => updateEntry }, + { typeof(ColumnOperation), () => columnOperation } }; TestEventLogging( diff --git a/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs b/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs index 03097b2f08b..2aaa778ef3e 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs @@ -1027,6 +1027,44 @@ public virtual void RenameTable_preserves_pending_rebuilds() ALTER TABLE ""ef_temp_Blog"" RENAME TO ""Blog""; GO +PRAGMA foreign_keys = 1; +"); + } + + [ConditionalFact] + public virtual void Rebuild_preserves_column_order() + { + Generate( + modelBuilder => modelBuilder.Entity( + "Ordinal", + e => + { + e.Property("B").HasColumnOrder(0); + e.Property("A").HasColumnOrder(1); + }), + migrationBuilder => migrationBuilder.DropColumn(name: "C", table: "Ordinal")); + + AssertSql( + @"CREATE TABLE ""ef_temp_Ordinal"" ( + ""B"" TEXT NULL, + ""A"" TEXT NULL +); +GO + +INSERT INTO ""ef_temp_Ordinal"" (""A"", ""B"") +SELECT ""A"", ""B"" +FROM ""Ordinal""; +GO + +PRAGMA foreign_keys = 0; +GO + +DROP TABLE ""Ordinal""; +GO + +ALTER TABLE ""ef_temp_Ordinal"" RENAME TO ""Ordinal""; +GO + PRAGMA foreign_keys = 1; "); }