Skip to content

Commit

Permalink
Add in info message for all index properties unmapped.
Browse files Browse the repository at this point in the history
Update error to only apply if there's a mixture of mapped and unmapped.
Updating IndexAttributeConvention back to throw instead of logging.
  • Loading branch information
lajones committed May 27, 2020
1 parent 6299d14 commit ba42972
Show file tree
Hide file tree
Showing 18 changed files with 345 additions and 313 deletions.
54 changes: 54 additions & 0 deletions src/EFCore.Relational/Diagnostics/IndexEventData.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata;

namespace Microsoft.EntityFrameworkCore.Diagnostics
{
/// <summary>
/// A <see cref="DiagnosticSource" /> event payload class for
/// the events involving an invalid index.
/// </summary>
public class IndexEventData : EventData
{
/// <summary>
/// Constructs the event payload for events involving an invalid index.
/// </summary>
/// <param name="eventDefinition"> The event definition. </param>
/// <param name="messageGenerator"> A delegate that generates a log message for this event. </param>
/// <param name="entityType"> The entity type on which the index is defined. </param>
/// <param name="indexName"> The name of the index. </param>
/// <param name="indexPropertyNames"> The names of the properties which define the index. </param>
public IndexEventData(
[NotNull] EventDefinitionBase eventDefinition,
[NotNull] Func<EventDefinitionBase, EventData, string> messageGenerator,
[NotNull] IEntityType entityType,
[CanBeNull] string indexName,
[NotNull] List<string> indexPropertyNames)
: base(eventDefinition, messageGenerator)
{
EntityType = entityType;
Name = indexName;
PropertyNames = indexPropertyNames;
}

/// <summary>
/// The entity type on which the index is defined.
/// </summary>
public virtual IEntityType EntityType { get; }

/// <summary>
/// The name of the index.
/// </summary>
public virtual string Name { get; }

/// <summary>
/// The list of properties which define the index.
/// </summary>
public virtual List<string> PropertyNames { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics
public class IndexInvalidPropertiesEventData : EventData
{
/// <summary>
/// Constructs the event payload for the <see cref="CoreEventId.IndexDefinedOnIgnoredProperty" /> event.
/// Constructs the event payload for the <see cref="RelationalEventId.IndexPropertiesMappedToNonOverlappingTables" /> event.
/// </summary>
/// <param name="eventDefinition"> The event definition. </param>
/// <param name="messageGenerator"> A delegate that generates a log message for this event. </param>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics
public class IndexInvalidPropertyEventData : EventData
{
/// <summary>
/// Constructs the event payload for the <see cref="CoreEventId.IndexDefinedOnIgnoredProperty" /> event.
/// Constructs the event payload for indexes with a invalid property.
/// </summary>
/// <param name="eventDefinition"> The event definition. </param>
/// <param name="messageGenerator"> A delegate that generates a log message for this event. </param>
Expand Down
20 changes: 17 additions & 3 deletions src/EFCore.Relational/Diagnostics/RelationalEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ private enum Id
// Model validation events
ModelValidationKeyDefaultValueWarning = CoreEventId.RelationalBaseId + 600,
BoolWithDefaultWarning,
IndexPropertyNotMappedToAnyTable,
AllIndexPropertiesNotToMappedToAnyTable,
IndexPropertiesBothMappedAndNotMappedToTable,
IndexPropertiesMappedToNonOverlappingTables,

// Update events
Expand Down Expand Up @@ -557,7 +558,20 @@ private enum Id

/// <summary>
/// <para>
/// An index specifies a property which is not mapped to a column in any table.
/// An index specifies properties all of which are not mapped to a column in any table.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="IndexEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId AllIndexPropertiesNotToMappedToAnyTable = MakeValidationId(Id.AllIndexPropertiesNotToMappedToAnyTable);

/// <summary>
/// <para>
/// An index specifies properties some of which are mapped and some of which are not mapped to a column in a table.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
Expand All @@ -566,7 +580,7 @@ private enum Id
/// This event uses the <see cref="IndexInvalidPropertyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId IndexPropertyNotMappedToAnyTable = MakeValidationId(Id.IndexPropertyNotMappedToAnyTable);
public static readonly EventId IndexPropertiesBothMappedAndNotMappedToTable = MakeValidationId(Id.IndexPropertiesBothMappedAndNotMappedToTable);

/// <summary>
/// <para>
Expand Down
54 changes: 49 additions & 5 deletions src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3604,19 +3604,63 @@ private static string BatchSmallerThanMinBatchSize(EventDefinitionBase definitio
}

/// <summary>
/// Logs the <see cref="RelationalEventId.IndexPropertyNotMappedToAnyTable" /> event.
/// Logs the <see cref="RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="entityType"> The entity type on which the index is defined. </param>
/// <param name="index"> The index on the entity type. </param>
public static void AllIndexPropertiesNotToMappedToAnyTable(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
[NotNull] IEntityType entityType,
[NotNull] IIndex index)
{
var definition = RelationalResources.LogAllIndexPropertiesNotToMappedToAnyTable(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics,
index.Name,
entityType.DisplayName(),
index.Properties.Format());
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new IndexEventData(
definition,
AllIndexPropertiesNotToMappedToAnyTable,
entityType,
index.Name,
index.Properties.Select(p => p.Name).ToList());

diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
}
}

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

/// <summary>
/// Logs the <see cref="RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="entityType"> The entity type on which the index is defined. </param>
/// <param name="index"> The index on the entity type. </param>
/// <param name="unmappedPropertyName"> The name of the property which is not mapped. </param>
public static void IndexPropertyNotMappedToAnyTable(
public static void IndexPropertiesBothMappedAndNotMappedToTable(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
[NotNull] IEntityType entityType,
[NotNull] IIndex index,
[NotNull] string unmappedPropertyName)
{
var definition = RelationalResources.LogIndexPropertyNotMappedToAnyTable(diagnostics);
var definition = RelationalResources.LogIndexPropertiesBothMappedAndNotMappedToTable(diagnostics);

if (diagnostics.ShouldLog(definition))
{
Expand All @@ -3631,7 +3675,7 @@ public static void IndexPropertyNotMappedToAnyTable(
{
var eventData = new IndexInvalidPropertyEventData(
definition,
IndexPropertyNotMappedToAnyTable,
IndexPropertiesBothMappedAndNotMappedToTable,
entityType,
index.Name,
index.Properties.Select(p => p.Name).ToList(),
Expand All @@ -3641,7 +3685,7 @@ public static void IndexPropertyNotMappedToAnyTable(
}
}

private static string IndexPropertyNotMappedToAnyTable(EventDefinitionBase definition, EventData payload)
private static string IndexPropertiesBothMappedAndNotMappedToTable(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string, string, string>)definition;
var p = (IndexInvalidPropertyEventData)payload;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,16 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
public EventDefinitionBase LogIndexPropertyNotMappedToAnyTable;
public EventDefinitionBase LogAllIndexPropertiesNotToMappedToAnyTable;

/// <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>
[EntityFrameworkInternal]
public EventDefinitionBase LogIndexPropertiesBothMappedAndNotMappedToTable;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
44 changes: 36 additions & 8 deletions src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ protected virtual void ValidateIndexProperties(
foreach (var index in entityType.GetIndexes()
.Where(i => ConfigurationSource.Convention != ((IConventionIndex)i).GetConfigurationSource()))
{
IProperty propertyNotMappedToAnyTable = null;
Tuple<string, List<(string Table, string Schema)>> firstPropertyTables = null;
Tuple<string, List<(string Table, string Schema)>> lastPropertyTables = null;
HashSet<(string Table, string Schema)> overlappingTables = null;
Expand All @@ -779,13 +780,17 @@ protected virtual void ValidateIndexProperties(
.ToList<(string Table, string Schema)>();
if (tablesMappedToProperty.Count == 0)
{
logger.IndexPropertyNotMappedToAnyTable(
entityType,
index,
property.Name);

propertyNotMappedToAnyTable = property;
overlappingTables = null;
break;

if (firstPropertyTables != null)
{
// Property is not mapped but we already found
// a property that is mapped.
break;
}

continue;
}

if (firstPropertyTables == null)
Expand All @@ -801,6 +806,14 @@ protected virtual void ValidateIndexProperties(
new Tuple<string, List<(string Table, string Schema)>>(property.Name, tablesMappedToProperty);
}

if (propertyNotMappedToAnyTable != null)
{
// Property is mapped but we already found
// a property that is not mapped.
overlappingTables = null;
break;
}

if (overlappingTables == null)
{
overlappingTables = new HashSet<(string Table, string Schema)>(tablesMappedToProperty);
Expand All @@ -815,8 +828,23 @@ protected virtual void ValidateIndexProperties(
}
}

if (overlappingTables != null
&& overlappingTables.Count == 0)
if (overlappingTables == null)
{
if (firstPropertyTables == null)
{
logger.AllIndexPropertiesNotToMappedToAnyTable(
entityType,
index);
}
else
{
logger.IndexPropertiesBothMappedAndNotMappedToTable(
entityType,
index,
propertyNotMappedToAnyTable.Name);
}
}
else if (overlappingTables.Count == 0)
{
Debug.Assert(firstPropertyTables != null, nameof(firstPropertyTables));
Debug.Assert(lastPropertyTables != null, nameof(lastPropertyTables));
Expand Down
42 changes: 33 additions & 9 deletions src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit ba42972

Please sign in to comment.