Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make StoreObjectIdentifier a readonly struct #21889

Merged
1 commit merged into from
Aug 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics
/// A <see cref="DiagnosticSource" /> event payload class for the
/// <see cref="RelationalEventId.IndexPropertiesMappedToNonOverlappingTables"/> event.
/// </summary>
public class IndexInvalidPropertiesEventData : EventData
public class IndexWithPropertiesEventData : EventData
{
/// <summary>
/// Constructs the event payload for the <see cref="RelationalEventId.IndexPropertiesMappedToNonOverlappingTables" /> event.
Expand All @@ -27,7 +27,7 @@ public class IndexInvalidPropertiesEventData : EventData
/// <param name="tablesMappedToProperty1"> The tables mapped to the first property. </param>
/// <param name="property2Name"> The name of the second property name which causes this event. </param>
/// <param name="tablesMappedToProperty2"> The tables mapped to the second property. </param>
public IndexInvalidPropertiesEventData(
public IndexWithPropertiesEventData(
[NotNull] EventDefinitionBase eventDefinition,
[NotNull] Func<EventDefinitionBase, EventData, string> messageGenerator,
[NotNull] IEntityType entityType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics
/// A <see cref="DiagnosticSource" /> event payload class for
/// the events involving an invalid property name on an index.
/// </summary>
public class IndexInvalidPropertyEventData : EventData
public class IndexWithPropertyEventData : EventData
{
/// <summary>
/// Constructs the event payload for indexes with a invalid property.
Expand All @@ -24,7 +24,7 @@ public class IndexInvalidPropertyEventData : EventData
/// <param name="indexName"> The name of the index. </param>
/// <param name="indexPropertyNames"> The names of the properties which define the index. </param>
/// <param name="invalidPropertyName"> The property name which is invalid. </param>
public IndexInvalidPropertyEventData(
public IndexWithPropertyEventData(
[NotNull] EventDefinitionBase eventDefinition,
[NotNull] Func<EventDefinitionBase, EventData, string> messageGenerator,
[NotNull] IEntityType entityType,
Expand All @@ -36,7 +36,7 @@ public IndexInvalidPropertyEventData(
EntityType = entityType;
Name = indexName;
PropertyNames = indexPropertyNames;
InvalidPropertyName = invalidPropertyName;
PropertyName = invalidPropertyName;
}

/// <summary>
Expand All @@ -55,8 +55,8 @@ public IndexInvalidPropertyEventData(
public virtual List<string> PropertyNames { get; }

/// <summary>
/// The name of the invalid property.
/// The name of the specific property.
/// </summary>
public virtual string InvalidPropertyName { get; }
public virtual string PropertyName { get; }
}
}
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Diagnostics/RelationalEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ private enum Id
/// This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="IndexInvalidPropertyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// This event uses the <see cref="IndexWithPropertyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId IndexPropertiesBothMappedAndNotMappedToTable = MakeValidationId(Id.IndexPropertiesBothMappedAndNotMappedToTable);
Expand All @@ -687,7 +687,7 @@ private enum Id
/// This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="IndexInvalidPropertiesEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// This event uses the <see cref="IndexWithPropertiesEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId IndexPropertiesMappedToNonOverlappingTables = MakeValidationId(Id.IndexPropertiesMappedToNonOverlappingTables);
Expand Down
20 changes: 10 additions & 10 deletions src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4460,7 +4460,7 @@ public static void IndexPropertiesBothMappedAndNotMappedToTable(

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new IndexInvalidPropertyEventData(
var eventData = new IndexWithPropertyEventData(
definition,
UnnamedIndexPropertiesBothMappedAndNotMappedToTable,
entityType,
Expand All @@ -4486,7 +4486,7 @@ public static void IndexPropertiesBothMappedAndNotMappedToTable(

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new IndexInvalidPropertyEventData(
var eventData = new IndexWithPropertyEventData(
definition,
NamedIndexPropertiesBothMappedAndNotMappedToTable,
entityType,
Expand All @@ -4502,22 +4502,22 @@ public static void IndexPropertiesBothMappedAndNotMappedToTable(
private static string UnnamedIndexPropertiesBothMappedAndNotMappedToTable(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string, string>)definition;
var p = (IndexInvalidPropertyEventData)payload;
var p = (IndexWithPropertyEventData)payload;
return d.GenerateMessage(
p.EntityType.DisplayName(),
p.PropertyNames.Format(),
p.InvalidPropertyName);
p.PropertyName);
}

private static string NamedIndexPropertiesBothMappedAndNotMappedToTable(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string, string, string>)definition;
var p = (IndexInvalidPropertyEventData)payload;
var p = (IndexWithPropertyEventData)payload;
return d.GenerateMessage(
p.Name,
p.EntityType.DisplayName(),
p.PropertyNames.Format(),
p.InvalidPropertyName);
p.PropertyName);
}

/// <summary>
Expand Down Expand Up @@ -4556,7 +4556,7 @@ public static void IndexPropertiesMappedToNonOverlappingTables(

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new IndexInvalidPropertiesEventData(
var eventData = new IndexWithPropertiesEventData(
definition,
UnnamedIndexPropertiesMappedToNonOverlappingTables,
entityType,
Expand Down Expand Up @@ -4592,7 +4592,7 @@ public static void IndexPropertiesMappedToNonOverlappingTables(

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new IndexInvalidPropertiesEventData(
var eventData = new IndexWithPropertiesEventData(
definition,
NamedIndexPropertiesMappedToNonOverlappingTables,
entityType,
Expand All @@ -4611,7 +4611,7 @@ public static void IndexPropertiesMappedToNonOverlappingTables(
private static string UnnamedIndexPropertiesMappedToNonOverlappingTables(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string, string, string, string, string>)definition;
var p = (IndexInvalidPropertiesEventData)payload;
var p = (IndexWithPropertiesEventData)payload;
return d.GenerateMessage(
p.EntityType.DisplayName(),
p.PropertyNames.Format(),
Expand All @@ -4624,7 +4624,7 @@ private static string UnnamedIndexPropertiesMappedToNonOverlappingTables(EventDe
private static string NamedIndexPropertiesMappedToNonOverlappingTables(EventDefinitionBase definition, EventData payload)
{
var d = (FallbackEventDefinition)definition;
var p = (IndexInvalidPropertiesEventData)payload;
var p = (IndexWithPropertiesEventData)payload;
return d.GenerateMessage(
l => l.Log(
d.Level,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ public static IEnumerable<IForeignKey> FindRowInternalForeignKeys(
/// <param name="entityType"> The entity type. </param>
/// <param name="storeObject"> The identifier of the store object. </param>
public static IEnumerable<IMutableForeignKey> FindRowInternalForeignKeys(
[NotNull] this IMutableEntityType entityType, StoreObjectIdentifier storeObject)
[NotNull] this IMutableEntityType entityType, in StoreObjectIdentifier storeObject)
=> ((IEntityType)entityType).FindRowInternalForeignKeys(storeObject).Cast<IMutableForeignKey>();

/// <summary>
Expand All @@ -776,7 +776,7 @@ public static IEnumerable<IMutableForeignKey> FindRowInternalForeignKeys(
/// <param name="entityType"> The entity type. </param>
/// <param name="storeObject"> The identifier of the store object. </param>
public static IEnumerable<IConventionForeignKey> FindRowInternalForeignKeys(
[NotNull] this IConventionEntityType entityType, StoreObjectIdentifier storeObject)
[NotNull] this IConventionEntityType entityType, in StoreObjectIdentifier storeObject)
=> ((IEntityType)entityType).FindRowInternalForeignKeys(storeObject).Cast<IConventionForeignKey>();

/// <summary>
Expand Down
60 changes: 39 additions & 21 deletions src/EFCore.Relational/Extensions/RelationalForeignKeyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public static string GetConstraintName([NotNull] this IForeignKey foreignKey)
/// <param name="principalStoreObject"> The identifier of the principal store object. </param>
/// <returns> The foreign key constraint name. </returns>
public static string GetConstraintName(
[NotNull] this IForeignKey foreignKey, StoreObjectIdentifier storeObject, StoreObjectIdentifier principalStoreObject)
[NotNull] this IForeignKey foreignKey, in StoreObjectIdentifier storeObject, in StoreObjectIdentifier principalStoreObject)
{
var annotation = foreignKey.FindAnnotation(RelationalAnnotationNames.Name);
return annotation != null
Expand Down Expand Up @@ -75,24 +75,32 @@ public static string GetDefaultName([NotNull] this IForeignKey foreignKey)
/// <returns> The default constraint name that would be used for this foreign key. </returns>
public static string GetDefaultName(
[NotNull] this IForeignKey foreignKey,
StoreObjectIdentifier storeObject,
StoreObjectIdentifier principalStoreObject)
in StoreObjectIdentifier storeObject,
in StoreObjectIdentifier principalStoreObject)
{
var propertyNames = foreignKey.Properties.Select(p => p.GetColumnName(storeObject)).ToList();
var principalPropertyNames = foreignKey.PrincipalKey.Properties.Select(p => p.GetColumnName(storeObject)).ToList();
var propertyNames = foreignKey.Properties.GetColumnNames(storeObject);
var principalPropertyNames = foreignKey.PrincipalKey.Properties.GetColumnNames(principalStoreObject);
var rootForeignKey = foreignKey;

// Limit traversal to avoid getting stuck in a cycle (validation will throw for these later)
// Using a hashset is detrimental to the perf when there are no cycles
for (var i = 0; i < Metadata.Internal.RelationalEntityTypeExtensions.MaxEntityTypesSharingTable; i++)
{
var linkedForeignKey = rootForeignKey.DeclaringEntityType
.FindRowInternalForeignKeys(storeObject)
.SelectMany(fk => fk.PrincipalEntityType.GetForeignKeys())
.FirstOrDefault(k => principalStoreObject.Name == k.PrincipalEntityType.GetTableName()
&& principalStoreObject.Schema == k.PrincipalEntityType.GetSchema()
&& propertyNames.SequenceEqual(k.Properties.Select(p => p.GetColumnName(storeObject)))
&& principalPropertyNames.SequenceEqual(k.PrincipalKey.Properties.Select(p => p.GetColumnName(storeObject))));
IForeignKey linkedForeignKey = null;
foreach (var otherForeignKey in rootForeignKey.DeclaringEntityType
.FindRowInternalForeignKeys(storeObject)
.SelectMany(fk => fk.PrincipalEntityType.GetForeignKeys()))
{
if (principalStoreObject.Name == otherForeignKey.PrincipalEntityType.GetTableName()
&& principalStoreObject.Schema == otherForeignKey.PrincipalEntityType.GetSchema()
&& propertyNames.SequenceEqual(otherForeignKey.Properties.GetColumnNames(storeObject))
&& principalPropertyNames.SequenceEqual(otherForeignKey.PrincipalKey.Properties.GetColumnNames(principalStoreObject)))
{
linkedForeignKey = otherForeignKey;
break;
}
}

if (linkedForeignKey == null)
{
break;
Expand All @@ -112,7 +120,7 @@ public static string GetDefaultName(
.Append("_")
.Append(principalStoreObject.Name)
.Append("_")
.AppendJoin(foreignKey.Properties.Select(p => p.GetColumnName(storeObject)), "_")
.AppendJoin(propertyNames, "_")
.ToString();

return Uniquifier.Truncate(baseName, foreignKey.DeclaringEntityType.Model.GetMaxIdentifierLength());
Expand Down Expand Up @@ -176,7 +184,7 @@ public static IEnumerable<IForeignKeyConstraint> GetMappedConstraints([NotNull]
/// <param name="foreignKey"> The foreign key. </param>
/// <param name="storeObject"> The identifier of the containing store object. </param>
/// <returns> The foreign key if found, or <see langword="null" /> if none was found.</returns>
public static IForeignKey FindSharedObjectRootForeignKey([NotNull] this IForeignKey foreignKey, StoreObjectIdentifier storeObject)
public static IForeignKey FindSharedObjectRootForeignKey([NotNull] this IForeignKey foreignKey, in StoreObjectIdentifier storeObject)
{
Check.NotNull(foreignKey, nameof(foreignKey));

Expand All @@ -188,12 +196,22 @@ public static IForeignKey FindSharedObjectRootForeignKey([NotNull] this IForeign
// Using a hashset is detrimental to the perf when there are no cycles
for (var i = 0; i < Metadata.Internal.RelationalEntityTypeExtensions.MaxEntityTypesSharingTable; i++)
{
var linkedKey = rootForeignKey.DeclaringEntityType
IForeignKey linkedKey = null;
foreach (var otherForeignKey in rootForeignKey.DeclaringEntityType
.FindRowInternalForeignKeys(storeObject)
.SelectMany(fk => fk.PrincipalEntityType.GetForeignKeys())
.FirstOrDefault(k => k.GetConstraintName(storeObject,
StoreObjectIdentifier.Table(k.PrincipalEntityType.GetTableName(), k.PrincipalEntityType.GetSchema()))
== foreignKeyName);
.SelectMany(fk => fk.PrincipalEntityType.GetForeignKeys()))
{
if (otherForeignKey.GetConstraintName(storeObject,
StoreObjectIdentifier.Table(
otherForeignKey.PrincipalEntityType.GetTableName(),
otherForeignKey.PrincipalEntityType.GetSchema()))
== foreignKeyName)
{
linkedKey = otherForeignKey;
break;
}
}

if (linkedKey == null)
{
break;
Expand All @@ -218,7 +236,7 @@ public static IForeignKey FindSharedObjectRootForeignKey([NotNull] this IForeign
/// <param name="storeObject"> The identifier of the containing store object. </param>
/// <returns> The foreign key if found, or <see langword="null" /> if none was found.</returns>
public static IMutableForeignKey FindSharedObjectRootForeignKey(
[NotNull] this IMutableForeignKey foreignKey, StoreObjectIdentifier storeObject)
[NotNull] this IMutableForeignKey foreignKey, in StoreObjectIdentifier storeObject)
=> (IMutableForeignKey)((IForeignKey)foreignKey).FindSharedObjectRootForeignKey(storeObject);

/// <summary>
Expand All @@ -234,7 +252,7 @@ public static IMutableForeignKey FindSharedObjectRootForeignKey(
/// <param name="storeObject"> The identifier of the containing store object. </param>
/// <returns> The foreign key if found, or <see langword="null" /> if none was found.</returns>
public static IConventionForeignKey FindSharedObjectRootForeignKey(
[NotNull] this IConventionForeignKey foreignKey, StoreObjectIdentifier storeObject)
[NotNull] this IConventionForeignKey foreignKey, in StoreObjectIdentifier storeObject)
=> (IConventionForeignKey)((IForeignKey)foreignKey).FindSharedObjectRootForeignKey(storeObject);
}
}
Loading