Simplify IsDescending values

roji · roji · commit de5bf7c41366 · 2022-01-19T12:16:33.000+01:00
If IsDescending is specified, it must explicitly specify all values for all properties.
diff --git a/src/EFCore.Abstractions/IndexAttribute.cs b/src/EFCore.Abstractions/IndexAttribute.cs
@@ -55,8 +55,7 @@ public bool IsUnique
     }
 
     /// <summary>
-    ///     Gets a set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
+    ///     A set of values indicating whether each corresponding index column has descending sort order.
     /// </summary>
     public bool[]? IsDescending { get; set; }
 
diff --git a/src/EFCore.Design/Migrations/Design/CSharpMigrationOperationGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpMigrationOperationGenerator.cs
@@ -911,7 +911,7 @@ protected virtual void Generate(CreateIndexOperation operation, IndentedStringBu
                     .Append("unique: true");
             }
 
-            if (operation.IsDescending.Length > 0)
+            if (operation.IsDescending is not null)
             {
                 builder
                     .AppendLine(",")
diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
@@ -634,7 +634,7 @@ protected virtual void GenerateIndex(
                 .Append(".IsUnique()");
         }
 
-        if (index.IsDescending.Count > 0)
+        if (index.IsDescending is not null)
         {
             stringBuilder
                 .AppendLine()
diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
@@ -567,46 +567,9 @@ private void GenerateIndex(IIndex index)
             lines.Add($".{nameof(IndexBuilder.IsUnique)}()");
         }
 
-        // Ascending/descending:
-        // If all ascending (IsDescending.Count == 0), no need to scaffold IsDescending() call (default)
-        // If all descending (IsDescending is all true), scaffold parameterless IsDescending()
-        // Otherwise, scaffold IsDescending() with values up until the last true (unspecified values default to false)
-        if (index.IsDescending.Count > 0)
+        if (index.IsDescending is not null)
         {
-            var descendingBuilder = new StringBuilder()
-                .Append('.')
-                .Append(nameof(IndexBuilder.IsDescending))
-                .Append('(');
-
-            var isAnyAscending = false;
-            var lastDescending = -1;
-            for (var i = 0; i < index.IsDescending.Count; i++)
-            {
-                if (index.IsDescending[i])
-                {
-                    lastDescending = i;
-                }
-                else
-                {
-                    isAnyAscending = true;
-                }
-            }
-
-            if (isAnyAscending)
-            {
-                for (var i = 0; i <= lastDescending; i++)
-                {
-                    if (i > 0)
-                    {
-                        descendingBuilder.Append(", ");
-                    }
-
-                    descendingBuilder.Append(_code.Literal(index.IsDescending[i]));
-                }
-            }
-
-            descendingBuilder.Append(')');
-            lines.Add(descendingBuilder.ToString());
+            lines.Add($".{nameof(IndexBuilder.IsDescending)}({string.Join(", ", index.IsDescending.Select(d => _code.Literal(d)))})");
         }
 
         GenerateAnnotations(index, annotations, lines);
diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs
@@ -212,7 +212,7 @@ private void GenerateIndexAttributes(IEntityType entityType)
                     indexAttribute.AddParameter($"{nameof(IndexAttribute.IsUnique)} = {_code.Literal(index.IsUnique)}");
                 }
 
-                if (index.IsDescending.Count > 0)
+                if (index.IsDescending is not null)
                 {
                     indexAttribute.AddParameter($"{nameof(IndexAttribute.IsDescending)} = {_code.UnknownLiteral(index.IsDescending)}");
                 }
diff --git a/src/EFCore.Relational/Metadata/ITableIndex.cs b/src/EFCore.Relational/Metadata/ITableIndex.cs
@@ -41,9 +41,8 @@ public interface ITableIndex : IAnnotatable
 
     /// <summary>
     ///     A set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
     /// </summary>
-    IReadOnlyList<bool> IsDescending { get; }
+    IReadOnlyList<bool>? IsDescending { get; }
 
     /// <summary>
     ///     Gets the expression used as the index filter.
@@ -80,7 +79,9 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt
             .AppendJoin(
                 ", ",
                 Enumerable.Range(0, Columns.Count)
-                    .Select(i => $"'{Columns[i].Name}'{(i < IsDescending.Count && IsDescending[i] ? " Desc" : "")}"))
+                    .Select(
+                        i =>
+                            $"'{Columns[i].Name}'{(IsDescending is not null && i < IsDescending.Count && IsDescending[i] ? " Desc" : "")}"))
             .Append('}');
 
         if (IsUnique)
diff --git a/src/EFCore.Relational/Metadata/Internal/TableIndex.cs b/src/EFCore.Relational/Metadata/Internal/TableIndex.cs
@@ -22,7 +22,7 @@ public TableIndex(
         Table table,
         IReadOnlyList<Column> columns,
         bool unique,
-        IReadOnlyList<bool> isDescending)
+        IReadOnlyList<bool>? isDescending)
     {
         Name = name;
         Table = table;
@@ -71,7 +71,7 @@ public override bool IsReadOnly
     public virtual bool IsUnique { get; }
 
     /// <inheritdoc />
-    public virtual IReadOnlyList<bool> IsDescending { get; }
+    public virtual IReadOnlyList<bool>? IsDescending { get; }
 
     /// <inheritdoc />
     public virtual string? Filter
diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
@@ -1393,7 +1393,11 @@ protected virtual IEnumerable<MigrationOperation> Diff(
 
     private bool IndexStructureEquals(ITableIndex source, ITableIndex target, DiffContext diffContext)
         => source.IsUnique == target.IsUnique
-            && source.IsDescending.SequenceEqual(target.IsDescending)
+            && (
+                source.IsDescending is null == target.IsDescending is null
+                || source.IsDescending is not null
+                && target.IsDescending is not null
+                && source.IsDescending.SequenceEqual(target.IsDescending))
             && source.Filter == target.Filter
             && !HasDifferences(source.GetAnnotations(), target.GetAnnotations())
             && source.Columns.Select(p => p.Name).SequenceEqual(
diff --git a/src/EFCore.Relational/Migrations/MigrationBuilder.cs b/src/EFCore.Relational/Migrations/MigrationBuilder.cs
@@ -603,7 +603,6 @@ public virtual AlterOperationBuilder<AlterTableOperation> AlterTable(
     /// <param name="filter">The filter to apply to the index, or <see langword="null" /> for no filter.</param>
     /// <param name="descending">
     ///     A set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
     ///     If <see langword="null" />, all columns will have ascending order.
     /// </param>
     /// <returns>A builder to allow annotations to be added to the operation.</returns>
@@ -638,7 +637,6 @@ public virtual OperationBuilder<CreateIndexOperation> CreateIndex(
     /// <param name="filter">The filter to apply to the index, or <see langword="null" /> for no filter.</param>
     /// <param name="descending">
     ///     A set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
     ///     If <see langword="null" />, all columns will have ascending order.
     /// </param>
     /// <returns>A builder to allow annotations to be added to the operation.</returns>
diff --git a/src/EFCore.Relational/Migrations/Operations/CreateIndexOperation.cs b/src/EFCore.Relational/Migrations/Operations/CreateIndexOperation.cs
@@ -39,9 +39,8 @@ public class CreateIndexOperation : MigrationOperation, ITableMigrationOperation
 
     /// <summary>
     ///     A set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
     /// </summary>
-    public virtual bool[] IsDescending { get; set; } = Array.Empty<bool>();
+    public virtual bool[]? IsDescending { get; set; }
 
     /// <summary>
     ///     An expression to use as the index filter.
@@ -64,7 +63,7 @@ public static CreateIndexOperation CreateFrom(ITableIndex index)
             Table = index.Table.Name,
             Columns = index.Columns.Select(p => p.Name).ToArray(),
             IsUnique = index.IsUnique,
-            IsDescending = index.IsDescending.ToArray(),
+            IsDescending = index.IsDescending?.ToArray(),
             Filter = index.Filter
         };
         operation.AddAnnotations(index.GetAnnotations());
diff --git a/src/EFCore.Relational/Scaffolding/Metadata/DatabaseIndex.cs b/src/EFCore.Relational/Scaffolding/Metadata/DatabaseIndex.cs
@@ -34,7 +34,6 @@ public class DatabaseIndex : Annotatable
 
     /// <summary>
     ///     A set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
     /// </summary>
     public virtual IList<bool> IsDescending { get; set; } = new List<bool>();
 
diff --git a/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs b/src/EFCore/Metadata/Builders/IConventionIndexBuilder.cs
@@ -41,21 +41,15 @@ public interface IConventionIndexBuilder : IConventionAnnotatableBuilder
     /// <summary>
     ///     Configures the sort order(s) for the columns of this index (ascending or descending).
     /// </summary>
-    /// <param name="descending">
-    ///     A set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
-    /// </param>
+    /// <param name="descending">A set of values indicating whether each corresponding index column has descending sort order.</param>
     /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
     /// <returns>The same builder instance if the uniqueness was configured, <see langword="null" /> otherwise.</returns>
     IConventionIndexBuilder? IsDescending(IReadOnlyList<bool>? descending, bool fromDataAnnotation = false);
 
     /// <summary>
     ///     Returns a value indicating whether this index sort order can be configured from the current configuration source.
     /// </summary>
-    /// <param name="descending">
-    ///     A set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
-    /// </param>
+    /// <param name="descending">A set of values indicating whether each corresponding index column has descending sort order.</param>
     /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
     /// <returns><see langword="true" /> if the index uniqueness can be configured.</returns>
     bool CanSetIsDescending(IReadOnlyList<bool>? descending, bool fromDataAnnotation = false);
diff --git a/src/EFCore/Metadata/Builders/IndexBuilder.cs b/src/EFCore/Metadata/Builders/IndexBuilder.cs
@@ -80,20 +80,10 @@ public virtual IndexBuilder IsUnique(bool unique = true)
     /// <summary>
     ///     Configures the sort order(s) for the columns of this index (ascending or descending).
     /// </summary>
-    /// <param name="descending">
-    ///     If empty, all index columns have descending sort order. Otherwise, each value determines whether the corresponding index
-    ///     column has descending sort order. If less sort order values are provided than there are columns, the remaining columns will have
-    ///     ascending order.
-    /// </param>
+    /// <param name="descending">A set of values indicating whether each corresponding index column has descending sort order.</param>
     /// <returns>The same builder instance so that multiple configuration calls can be chained.</returns>
     public virtual IndexBuilder IsDescending(params bool[] descending)
     {
-        if (descending.Length == 0)
-        {
-            descending = new bool[Builder.Metadata.Properties.Count];
-            Array.Fill(descending, true);
-        }
-
         Builder.IsDescending(descending, ConfigurationSource.Explicit);
 
         return this;
diff --git a/src/EFCore/Metadata/Builders/IndexBuilder`.cs b/src/EFCore/Metadata/Builders/IndexBuilder`.cs
@@ -53,11 +53,7 @@ public IndexBuilder(IMutableIndex index)
     /// <summary>
     ///     Configures the sort order(s) for the columns of this index (ascending or descending).
     /// </summary>
-    /// <param name="descending">
-    ///     If empty, all index columns have descending sort order. Otherwise, each value determines whether the corresponding index
-    ///     column has descending sort order. If less sort order values are provided than there are columns, the remaining columns will have
-    ///     ascending order.
-    /// </param>
+    /// <param name="descending">A set of values indicating whether each corresponding index column has descending sort order.</param>
     /// <returns>The same builder instance so that multiple configuration calls can be chained.</returns>
     public new virtual IndexBuilder<T> IsDescending(params bool[] descending)
         => (IndexBuilder<T>)base.IsDescending(descending);
diff --git a/src/EFCore/Metadata/IConventionIndex.cs b/src/EFCore/Metadata/IConventionIndex.cs
@@ -58,10 +58,7 @@ public interface IConventionIndex : IReadOnlyIndex, IConventionAnnotatable
     /// <summary>
     ///     Sets the sort order(s) for this index (ascending or descending).
     /// </summary>
-    /// <param name="descending">
-    ///     A set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
-    /// </param>
+    /// <param name="descending">A set of values indicating whether each corresponding index column has descending sort order.</param>
     /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
     /// <returns>The configured sort order(s).</returns>
     IReadOnlyList<bool>? SetIsDescending(IReadOnlyList<bool>? descending, bool fromDataAnnotation = false);
diff --git a/src/EFCore/Metadata/IMutableIndex.cs b/src/EFCore/Metadata/IMutableIndex.cs
@@ -25,9 +25,8 @@ public interface IMutableIndex : IReadOnlyIndex, IMutableAnnotatable
 
     /// <summary>
     ///     A set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
     /// </summary>
-    new IReadOnlyList<bool> IsDescending { get; set; }
+    new IReadOnlyList<bool>? IsDescending { get; set; }
 
     /// <summary>
     ///     Gets the properties that this index is defined on.
diff --git a/src/EFCore/Metadata/IReadOnlyIndex.cs b/src/EFCore/Metadata/IReadOnlyIndex.cs
@@ -29,10 +29,9 @@ public interface IReadOnlyIndex : IReadOnlyAnnotatable
     bool IsUnique { get; }
 
     /// <summary>
-    ///     Gets a set of values indicating whether each corresponding index column has descending sort order.
-    ///     If less sort order values are provided than there are columns, the remaining columns will have ascending order.
+    ///     A set of values indicating whether each corresponding index column has descending sort order.
     /// </summary>
-    IReadOnlyList<bool> IsDescending { get; }
+    IReadOnlyList<bool>? IsDescending { get; }
 
     /// <summary>
     ///     Gets the entity type the index is defined on. This may be different from the type that <see cref="Properties" />
diff --git a/src/EFCore/Metadata/Internal/Index.cs b/src/EFCore/Metadata/Internal/Index.cs
@@ -202,7 +202,7 @@ private void UpdateIsUniqueConfigurationSource(ConfigurationSource configuration
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual IReadOnlyList<bool> IsDescending
+    public virtual IReadOnlyList<bool>? IsDescending
     {
         get => _isDescending ?? DefaultIsDescending;
         set => SetIsDescending(value, ConfigurationSource.Explicit);
@@ -218,21 +218,16 @@ public virtual IReadOnlyList<bool> IsDescending
     {
         EnsureMutable();
 
-        if (descending is not null && descending.Count > Properties.Count)
+        if (descending is not null && descending.Count != Properties.Count)
         {
             throw new ArgumentException(
-                CoreStrings.TooManyIndexSortOrderValues(descending.Count, Properties.Format(), Properties.Count), nameof(descending));
-        }
-
-        // Normalize all-false array to the simpler empty array representation
-        if (descending is not null && descending.All(d => d == false))
-        {
-            descending = DefaultIsDescending;
+                CoreStrings.InvalidNumberOfIndexSortOrderValues(
+                    Properties.Format(), descending.Count, Properties.Count), nameof(descending));
         }
 
         var oldIsDescending = IsDescending;
         var isChanging = _isDescending is null != descending is null
-            || descending is not null && !oldIsDescending.SequenceEqual(descending);
+            || descending is not null && oldIsDescending is not null && !oldIsDescending.SequenceEqual(descending);
         _isDescending = descending;
 
         if (descending == null)
@@ -249,8 +244,8 @@ public virtual IReadOnlyList<bool> IsDescending
             : oldIsDescending;
     }
 
-    private static readonly bool[] DefaultIsDescending
-        = Array.Empty<bool>();
+    private static readonly bool[]? DefaultIsDescending
+        = null;
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
diff --git a/src/EFCore/Metadata/Internal/InternalIndexBuilder.cs b/src/EFCore/Metadata/Internal/InternalIndexBuilder.cs
@@ -73,8 +73,8 @@ public virtual bool CanSetIsUnique(bool? unique, ConfigurationSource? configurat
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual bool CanSetIsDescending(IReadOnlyList<bool>? descending, ConfigurationSource? configurationSource)
-        => descending is null
-            || Metadata.IsDescending.SequenceEqual(descending)
+        => descending is null && Metadata.IsDescending is null
+            || descending is not null && Metadata.IsDescending is not null && Metadata.IsDescending.SequenceEqual(descending)
             || configurationSource.Overrides(Metadata.GetIsDescendingConfigurationSource());
 
     /// <summary>
diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs
diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx
diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationOperationGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationOperationGeneratorTest.cs
diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs
diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs
diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs
diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs
diff --git a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs
diff --git a/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs b/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs

Original file line number	Diff line number	Diff line change
`@@ -55,8 +55,7 @@ public bool IsUnique`
`55`	`55`	`}`
`56`	`56`
`57`	`57`	`/// <summary>`
`58`		`- /// Gets a set of values indicating whether each corresponding index column has descending sort order.`
`59`		`- /// If less sort order values are provided than there are columns, the remaining columns will have ascending order.`
	`58`	`+ /// A set of values indicating whether each corresponding index column has descending sort order.`
`60`	`59`	`/// </summary>`
`61`	`60`	`public bool[]? IsDescending { get; set; }`
`62`	`61`
Original file line number	Diff line number	Diff line change
`@@ -911,7 +911,7 @@ protected virtual void Generate(CreateIndexOperation operation, IndentedStringBu`
`911`	`911`	`.Append("unique: true");`
`912`	`912`	`}`
`913`	`913`
`914`		`- if (operation.IsDescending.Length > 0)`
	`914`	`+ if (operation.IsDescending is not null)`
`915`	`915`	`{`
`916`	`916`	`builder`
`917`	`917`	`.AppendLine(",")`
Original file line number	Diff line number	Diff line change
`@@ -634,7 +634,7 @@ protected virtual void GenerateIndex(`
`634`	`634`	`.Append(".IsUnique()");`
`635`	`635`	`}`
`636`	`636`
`637`		`- if (index.IsDescending.Count > 0)`
	`637`	`+ if (index.IsDescending is not null)`
`638`	`638`	`{`
`639`	`639`	`stringBuilder`
`640`	`640`	`.AppendLine()`
Original file line number	Diff line number	Diff line change
`@@ -212,7 +212,7 @@ private void GenerateIndexAttributes(IEntityType entityType)`
`212`	`212`	`indexAttribute.AddParameter($"{nameof(IndexAttribute.IsUnique)} = {_code.Literal(index.IsUnique)}");`
`213`	`213`	`}`
`214`	`214`
`215`		`- if (index.IsDescending.Count > 0)`
	`215`	`+ if (index.IsDescending is not null)`
`216`	`216`	`{`
`217`	`217`	`indexAttribute.AddParameter($"{nameof(IndexAttribute.IsDescending)} = {_code.UnknownLiteral(index.IsDescending)}");`
`218`	`218`	`}`