Skip to content

Commit

Permalink
Remove TypeMapping annotation
Browse files Browse the repository at this point in the history
Sort columns and column mappings for PK first

Fixes #21772
  • Loading branch information
AndriySvyryd committed Aug 4, 2020
1 parent 40d99f0 commit 3975629
Show file tree
Hide file tree
Showing 23 changed files with 301 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private ColumnMappingBaseComparer()
/// </summary>
public int Compare(IColumnMappingBase x, IColumnMappingBase y)
{
var result = y.Column.IsNullable.CompareTo(x.Column.IsNullable);
var result = y.Property.IsPrimaryKey().CompareTo(x.Property.IsPrimaryKey());
if (result != 0)
{
return result;
Expand Down
77 changes: 77 additions & 0 deletions src/EFCore.Relational/Metadata/Internal/ColumnNameComparer.cs
Original file line number Diff line number Diff line change
@@ -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.Diagnostics.CodeAnalysis;

namespace Microsoft.EntityFrameworkCore.Metadata.Internal
{
/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public sealed class ColumnNameComparer : IComparer<string>
{
private readonly Table _table;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public ColumnNameComparer([NotNull] Table table)
{
_table = table;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public int Compare(string x, string y)
{
var xIndex = -1;
var yIndex = -1;

var columns = _table.PrimaryKey?.Columns;
if (columns != null)
{
for (var i = 0; i < columns.Count; i++)
{
var name = columns[i].Name;
if (name == x)
{
xIndex = i;
}

if (name == y)
{
yIndex = i;
}
}
}

if (xIndex == -1
&& yIndex == -1)
{
return StringComparer.Ordinal.Compare(x, y);
}

if (xIndex > -1
&& yIndex > -1)
{
return xIndex - yIndex;
}

return xIndex > yIndex
? -1
: 1;
}
}
}
46 changes: 44 additions & 2 deletions src/EFCore.Relational/Metadata/Internal/Table.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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 System.Linq;
Expand All @@ -18,6 +17,8 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Internal
/// </summary>
public class Table : TableBase, ITable
{
private UniqueConstraint _primaryKey;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand All @@ -27,6 +28,7 @@ public class Table : TableBase, ITable
public Table([NotNull] string name, [CanBeNull] string schema, [NotNull] RelationalModel model)
: base(name, schema, model)
{
Columns = new SortedDictionary<string, IColumnBase>(new ColumnNameComparer(this));
}

/// <summary>
Expand All @@ -44,7 +46,47 @@ public Table([NotNull] string name, [CanBeNull] string schema, [NotNull] Relatio
/// 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 UniqueConstraint PrimaryKey { get; [param: NotNull] set; }
public virtual UniqueConstraint PrimaryKey
{
get => _primaryKey; [param: NotNull]
set
{
var oldPrimaryKey = _primaryKey;
if (oldPrimaryKey != null)
{
foreach (var column in oldPrimaryKey.Columns)
{
Columns.Remove(column.Name);
}
}

if (value != null)
{
foreach (var column in value.Columns)
{
Columns.Remove(column.Name);
}
}

_primaryKey = value;

if (oldPrimaryKey != null)
{
foreach (var column in oldPrimaryKey.Columns)
{
Columns.TryAdd(column.Name, column);
}
}

if (value != null)
{
foreach (var column in value.Columns)
{
Columns.TryAdd(column.Name, column);
}
}
}
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Metadata/Internal/TableBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public TableBase([NotNull] string name, [CanBeNull] string schema, [NotNull] Rel
/// 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 SortedDictionary<string, IColumnBase> Columns { get; }
public virtual SortedDictionary<string, IColumnBase> Columns { get; [param: NotNull] protected set; }
= new SortedDictionary<string, IColumnBase>(StringComparer.Ordinal);

/// <inheritdoc/>
Expand Down
24 changes: 17 additions & 7 deletions src/EFCore.Relational/Update/ModificationCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -284,17 +284,27 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
&& (entry.EntityState == EntityState.Deleted
|| entry.EntityState == EntityState.Added);

foreach (var property in entry.EntityType.GetProperties())
ITableMappingBase tableMapping = null;
foreach (var mapping in entry.EntityType.GetTableMappings())
{
var columnMapping = property.GetTableColumnMappings()
.Where(m => m.TableMapping.Table.Name == TableName && m.TableMapping.Table.Schema == Schema)
.FirstOrDefault();
if (columnMapping == null)
var table = ((ITableMappingBase)mapping).Table;
if (table.Name == TableName
&& table.Schema == Schema)
{
continue;
tableMapping = mapping;
break;
}
}

var column = columnMapping.Column;
if (tableMapping == null)
{
continue;
}

foreach (var columnMapping in tableMapping.ColumnMappings)
{
var property = columnMapping.Property;
var column = (IColumn)columnMapping.Column;
var isKey = property.IsPrimaryKey();
var isCondition = !adding && (isKey || property.IsConcurrencyToken);
var readValue = state != EntityState.Deleted && entry.IsStoreGenerated(property);
Expand Down
20 changes: 17 additions & 3 deletions src/EFCore/Extensions/ConventionPropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.EntityFrameworkCore.ChangeTracking;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Microsoft.EntityFrameworkCore.ValueGeneration;

Expand Down Expand Up @@ -81,13 +82,26 @@ public static IConventionKey FindContainingPrimaryKey([NotNull] this IConvention
public static IEnumerable<IConventionKey> GetContainingKeys([NotNull] this IConventionProperty property)
=> ((Property)property).GetContainingKeys();

/// <summary>
/// Sets the <see cref="CoreTypeMapping" /> for the given property
/// </summary>
/// <param name="property"> The property. </param>
/// <param name="typeMapping"> The <see cref="CoreTypeMapping" /> for this property. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
public static CoreTypeMapping SetTypeMapping(
[NotNull] this IConventionProperty property,
[NotNull] CoreTypeMapping typeMapping,
bool fromDataAnnotation = false)
=> ((Property)property).SetTypeMapping(
typeMapping, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <summary>
/// Gets the <see cref="ConfigurationSource" /> for <see cref="PropertyExtensions.FindTypeMapping(IProperty)" />.
/// </summary>
/// <param name="property"> The property. </param>
/// <returns> The <see cref="ConfigurationSource" /> for <see cref="PropertyExtensions.FindTypeMapping(IProperty)" />. </returns>
public static ConfigurationSource? GetTypeMappingConfigurationSource([NotNull] this IConventionProperty property)
=> property.FindAnnotation(CoreAnnotationNames.TypeMapping)?.GetConfigurationSource();
=> ((Property)property).GetTypeMappingConfigurationSource();

/// <summary>
/// Sets the maximum length of data that is allowed in this property. For example, if the property is a <see cref="string" /> '
Expand Down Expand Up @@ -115,7 +129,7 @@ public static IEnumerable<IConventionKey> GetContainingKeys([NotNull] this IConv
/// For example, if the property is a <see cref="decimal" />
/// then this is the maximum number of digits.
/// </summary>
/// <param name="property"> The property to get the precision of. </param>
/// <param name="property"> The property. </param>
/// <param name="precision"> The maximum number of digits that is allowed in this property. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
public static int? SetPrecision([NotNull] this IConventionProperty property, int? precision, bool fromDataAnnotation = false)
Expand All @@ -135,7 +149,7 @@ public static IEnumerable<IConventionKey> GetContainingKeys([NotNull] this IConv
/// For example, if the property is a <see cref="decimal" />
/// then this is the maximum number of decimal places.
/// </summary>
/// <param name="property"> The property to get the precision of. </param>
/// <param name="property"> The property. </param>
/// <param name="scale"> The maximum number of decimal places that is allowed in this property. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
public static int? SetScale([NotNull] this IConventionProperty property, int? scale, bool fromDataAnnotation = false)
Expand Down
11 changes: 11 additions & 0 deletions src/EFCore/Extensions/MutablePropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.EntityFrameworkCore.ChangeTracking;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Microsoft.EntityFrameworkCore.ValueGeneration;

Expand Down Expand Up @@ -198,6 +199,16 @@ public static void SetValueConverter([NotNull] this IMutableProperty property, [
public static void SetProviderClrType([NotNull] this IMutableProperty property, [CanBeNull] Type providerClrType)
=> property.AsProperty().SetProviderClrType(providerClrType, ConfigurationSource.Explicit);

/// <summary>
/// Sets the <see cref="CoreTypeMapping" /> for the given property
/// </summary>
/// <param name="property"> The property. </param>
/// <param name="typeMapping"> The <see cref="CoreTypeMapping" /> for this property. </param>
public static CoreTypeMapping SetTypeMapping(
[NotNull] this IMutableProperty property,
[NotNull] CoreTypeMapping typeMapping)
=> ((Property)property).SetTypeMapping(typeMapping, ConfigurationSource.Explicit);

/// <summary>
/// Sets the custom <see cref="ValueComparer" /> for this property.
/// </summary>
Expand Down
19 changes: 9 additions & 10 deletions src/EFCore/Extensions/PropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ public static class PropertyExtensions
/// <returns> The type mapping. </returns>
public static CoreTypeMapping GetTypeMapping([NotNull] this IProperty property)
{
var mapping = (CoreTypeMapping)property[CoreAnnotationNames.TypeMapping];

var mapping = ((Property)property).TypeMapping;
if (mapping == null)
{
throw new InvalidOperationException(
Expand All @@ -50,7 +49,7 @@ public static CoreTypeMapping GetTypeMapping([NotNull] this IProperty property)
/// <param name="property"> The property. </param>
/// <returns> The type mapping, or <see langword="null"/> if none was found. </returns>
public static CoreTypeMapping FindTypeMapping([NotNull] this IProperty property)
=> (CoreTypeMapping)property[CoreAnnotationNames.TypeMapping];
=> ((Property)property).TypeMapping;

/// <summary>
/// Finds the first principal property that the given property is constrained by
Expand Down Expand Up @@ -124,15 +123,15 @@ private static void AddPrincipals(IProperty property, List<IProperty> visited)
/// <param name="property"> The property to check. </param>
/// <returns> <see langword="true"/> if the property is used as a foreign key, otherwise <see langword="false"/>. </returns>
public static bool IsForeignKey([NotNull] this IProperty property)
=> Check.NotNull(property, nameof(property)).AsProperty().ForeignKeys != null;
=> Check.NotNull((Property)property, nameof(property)).ForeignKeys != null;

/// <summary>
/// Gets a value indicating whether this property is used as an index (or part of a composite index).
/// </summary>
/// <param name="property"> The property to check. </param>
/// <returns> <see langword="true"/> if the property is used as an index, otherwise <see langword="false"/>. </returns>
public static bool IsIndex([NotNull] this IProperty property)
=> Check.NotNull(property, nameof(property)).AsProperty().Indexes != null;
=> Check.NotNull((Property)property, nameof(property)).Indexes != null;

/// <summary>
/// Gets a value indicating whether this property is used as a unique index (or part of a unique composite index).
Expand All @@ -157,7 +156,7 @@ public static bool IsPrimaryKey([NotNull] this IProperty property)
/// <param name="property"> The property to check. </param>
/// <returns> <see langword="true"/> if the property is used as a key, otherwise <see langword="false"/>. </returns>
public static bool IsKey([NotNull] this IProperty property)
=> Check.NotNull(property, nameof(property)).AsProperty().Keys != null;
=> Check.NotNull((Property)property, nameof(property)).Keys != null;

/// <summary>
/// Gets all foreign keys that use this property (including composite foreign keys in which this property
Expand All @@ -166,7 +165,7 @@ public static bool IsKey([NotNull] this IProperty property)
/// <param name="property"> The property to get foreign keys for. </param>
/// <returns> The foreign keys that use this property. </returns>
public static IEnumerable<IForeignKey> GetContainingForeignKeys([NotNull] this IProperty property)
=> Check.NotNull(property, nameof(property)).AsProperty().GetContainingForeignKeys();
=> Check.NotNull((Property)property, nameof(property)).GetContainingForeignKeys();

/// <summary>
/// Gets all indexes that use this property (including composite indexes in which this property
Expand All @@ -175,7 +174,7 @@ public static IEnumerable<IForeignKey> GetContainingForeignKeys([NotNull] this I
/// <param name="property"> The property to get indexes for. </param>
/// <returns> The indexes that use this property. </returns>
public static IEnumerable<IIndex> GetContainingIndexes([NotNull] this IProperty property)
=> Check.NotNull(property, nameof(property)).AsProperty().GetContainingIndexes();
=> Check.NotNull((Property)property, nameof(property)).GetContainingIndexes();

/// <summary>
/// Gets the primary key that uses this property (including a composite primary key in which this property
Expand All @@ -184,7 +183,7 @@ public static IEnumerable<IIndex> GetContainingIndexes([NotNull] this IProperty
/// <param name="property"> The property to get primary key for. </param>
/// <returns> The primary that use this property, or <see langword="null"/> if it is not part of the primary key. </returns>
public static IKey FindContainingPrimaryKey([NotNull] this IProperty property)
=> Check.NotNull(property, nameof(property)).AsProperty().PrimaryKey;
=> Check.NotNull((Property)property, nameof(property)).PrimaryKey;

/// <summary>
/// Gets all primary or alternate keys that use this property (including composite keys in which this property
Expand All @@ -193,7 +192,7 @@ public static IKey FindContainingPrimaryKey([NotNull] this IProperty property)
/// <param name="property"> The property to get primary and alternate keys for. </param>
/// <returns> The primary and alternate keys that use this property. </returns>
public static IEnumerable<IKey> GetContainingKeys([NotNull] this IProperty property)
=> Check.NotNull(property, nameof(property)).AsProperty().GetContainingKeys();
=> Check.NotNull((Property)property, nameof(property)).GetContainingKeys();

/// <summary>
/// Gets the maximum length of data that is allowed in this property. For example, if the property is a <see cref="string" /> '
Expand Down
Loading

0 comments on commit 3975629

Please sign in to comment.