Skip to content

Commit

Permalink
Updated per review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
lajones committed May 26, 2020
1 parent 55bdff6 commit c6dfbe0
Show file tree
Hide file tree
Showing 51 changed files with 1,237 additions and 470 deletions.
36 changes: 26 additions & 10 deletions src/EFCore.Abstractions/IndexAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// 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.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Utilities;

Expand All @@ -11,32 +13,46 @@ namespace Microsoft.EntityFrameworkCore
/// Specifies an index to be generated in the database.
/// </summary>
[AttributeUsage(AttributeTargets.Class, AllowMultiple = true)]
public class IndexAttribute : Attribute
public sealed class IndexAttribute : Attribute
{
private static readonly bool DefaultIsUnique = false;
private bool? _isUnique;

/// <summary>
/// Initializes a new instance of the <see cref="IndexAttribute" /> class.
/// </summary>
/// <param name="memberNames"> The members which constitute the index, in order (there must be at least one). </param>
public IndexAttribute(params string[] memberNames)
/// <param name="propertyNames"> The properties which constitute the index, in order (there must be at least one). </param>
public IndexAttribute(params string[] propertyNames)
{
Check.NotEmpty(memberNames, nameof(memberNames));
MemberNames = memberNames;
Check.NotEmpty(propertyNames, nameof(propertyNames));
Check.HasNoEmptyElements(propertyNames, nameof(propertyNames));
PropertyNames = propertyNames.ToList();
}

/// <summary>
/// The members which constitute the index, in order.
/// The properties which constitute the index, in order.
/// </summary>
public virtual string[] MemberNames { get; }
public List<string> PropertyNames { get; }

/// <summary>
/// The name of the index in the database.
/// The name of the index.
/// </summary>
public virtual string Name { get; [param: NotNull] set; }
public string Name { get; [param: NotNull] set; }


/// <summary>
/// Whether the index is unique.
/// </summary>
public virtual bool IsUnique { get; set; }
public bool IsUnique
{
get => _isUnique ?? DefaultIsUnique;
set => _isUnique = value;
}

/// <summary>
/// Use this method if you want to know the uniqueness of
/// the index or <see langword="null"/> if it was not specified.
/// </summary>
public bool? GetIsUnique() => _isUnique;
}
}

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

3 changes: 3 additions & 0 deletions src/EFCore.Abstractions/Properties/AbstractionsStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,7 @@
<data name="CollectionArgumentIsEmpty" xml:space="preserve">
<value>The collection argument '{argumentName}' must contain at least one element.</value>
</data>
<data name="CollectionArgumentHasEmptyElements" xml:space="preserve">
<value>The collection argument '{argumentName}' must not contain any empty elements.</value>
</data>
</root>
17 changes: 7 additions & 10 deletions src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Scaffolding.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Microsoft.EntityFrameworkCore.Utilities;
Expand Down Expand Up @@ -742,12 +743,12 @@ protected virtual void GenerateIndex(
Check.NotNull(index, nameof(index));
Check.NotNull(stringBuilder, nameof(stringBuilder));

// Note - method names below are meant to be hard-coded
// because old snapshot files will fail if they are changed
stringBuilder
.AppendLine()
.Append(builderName)
.Append(".")
.Append(nameof(EntityTypeBuilder.HasIndex))
.Append("(")
.Append(".HasIndex(")
.Append(string.Join(", ", index.Properties.Select(p => Code.Literal(p.Name))))
.Append(")");

Expand All @@ -757,9 +758,7 @@ protected virtual void GenerateIndex(
{
stringBuilder
.AppendLine()
.Append(".")
.Append(nameof(IndexBuilder.HasName))
.Append("(")
.Append(".HasName(")
.Append(Code.Literal(index.Name))
.Append(")");
}
Expand All @@ -768,9 +767,7 @@ protected virtual void GenerateIndex(
{
stringBuilder
.AppendLine()
.Append(".")
.Append(nameof(IndexBuilder.IsUnique))
.Append("()");
.Append(".IsUnique()");
}

GenerateIndexAnnotations(index, stringBuilder);
Expand All @@ -791,7 +788,7 @@ protected virtual void GenerateIndexAnnotations(

IgnoreAnnotations(
annotations,
RelationalAnnotationNames.TableIndexMappings);
CSharpModelGenerator.IgnoredIndexAnnotations.ToArray());

GenerateFluentApiForAnnotation(
ref annotations, RelationalAnnotationNames.Filter, nameof(RelationalIndexBuilderExtensions.HasFilter), stringBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ private void GenerateEntityType(IEntityType entityType, bool useDataAnnotations)
// if useDataAnnotations is true.
if (!useDataAnnotations
|| index.GetAnnotations().Any(
a => !RelationalAnnotationNames.TableIndexMappings.Equals(a.Name, StringComparison.Ordinal)))
a => !CSharpModelGenerator.IgnoredIndexAnnotations.Contains(a.Name)))
{
GenerateIndex(index);
}
Expand Down Expand Up @@ -584,13 +584,16 @@ private void GenerateIndex(IIndex index)

var annotations = index.GetAnnotations().ToList();

RemoveAnnotation(ref annotations, RelationalAnnotationNames.TableIndexMappings);
foreach (var annotation in CSharpModelGenerator.IgnoredIndexAnnotations)
{
RemoveAnnotation(ref annotations, annotation);
}

if (index.Name != null)
{
lines.Add(
$".{nameof(IndexBuilder.HasName)}" +
$"({_code.Literal(index.GetName())})");
$"({_code.Literal(index.GetDatabaseName())})");
}

if (index.IsUnique)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ private void GenerateIndexAttributes(IEntityType entityType)
// If there are annotations that cannot be represented
// using an IndexAttribute then use fluent API instead.
if (!index.GetAnnotations().Any(
a => !RelationalAnnotationNames.TableIndexMappings.Equals(a.Name, StringComparison.Ordinal)))
a => !CSharpModelGenerator.IgnoredIndexAnnotations.Contains(a.Name)))
{
var indexAttribute = new AttributeWriter(nameof(IndexAttribute));
foreach (var property in index.Properties)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// 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.IO;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
Expand Down Expand Up @@ -127,5 +128,11 @@ public override ScaffoldedModel GenerateModel(

return resultingFiles;
}

/// <summary>
/// The set of annotations ignored for the purposes of code generation for indexes.
/// </summary>
public static IEnumerable<string> IgnoredIndexAnnotations
=> new List<string> { RelationalAnnotationNames.TableIndexMappings };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ protected virtual IMutableForeignKey VisitForeignKey([NotNull] ModelBuilder mode
_reporter.WriteWarning(
DesignStrings.ForeignKeyPrincipalEndContainsNullableColumns(
foreignKey.DisplayName(),
index.GetName(),
index.GetDatabaseName(),
nullablePrincipalProperties.Select(tuple => tuple.column.DisplayName()).ToList()
.Aggregate((a, b) => a + "," + b)));

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// 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
/// <see cref="RelationalEventId.IndexPropertiesMappedToNonOverlappingTables"/> event.
/// </summary>
public class IndexInvalidPropertiesEventData : EventData
{
/// <summary>
/// Constructs the event payload for the <see cref="CoreEventId.IndexDefinedOnIgnoredProperty" /> event.
/// </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>
/// <param name="property1Name"> The name of the first property name which causes this event. </param>
/// <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(
[NotNull] EventDefinitionBase eventDefinition,
[NotNull] Func<EventDefinitionBase, EventData, string> messageGenerator,
[NotNull] IEntityType entityType,
[CanBeNull] string indexName,
[NotNull] List<string> indexPropertyNames,
[NotNull] string property1Name,
[NotNull] List<(string Table, string Schema)> tablesMappedToProperty1,
[NotNull] string property2Name,
[NotNull] List<(string Table, string Schema)> tablesMappedToProperty2)
: base(eventDefinition, messageGenerator)
{
EntityType = entityType;
Name = indexName;
PropertyNames = indexPropertyNames;
Property1Name = property1Name;
TablesMappedToProperty1 = tablesMappedToProperty1;
Property2Name = property2Name;
TablesMappedToProperty2 = tablesMappedToProperty2;
}

/// <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; }

/// <summary>
/// The name of the first property.
/// </summary>
public virtual string Property1Name { get; }

/// <summary>
/// The tables mapped to the first property.
/// </summary>
public virtual List<(string Table, string Schema)> TablesMappedToProperty1 { get; }

/// <summary>
/// The name of the second property.
/// </summary>
public virtual string Property2Name { get; }

/// <summary>
/// The tables mapped to the second property.
/// </summary>
public virtual List<(string Table, string Schema)> TablesMappedToProperty2 { get; }
}
}
28 changes: 28 additions & 0 deletions src/EFCore.Relational/Diagnostics/RelationalEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ private enum Id
// Model validation events
ModelValidationKeyDefaultValueWarning = CoreEventId.RelationalBaseId + 600,
BoolWithDefaultWarning,
IndexPropertyNotMappedToAnyTable,
IndexPropertiesMappedToNonOverlappingTables,

// Update events
BatchReadyForExecution = CoreEventId.RelationalBaseId + 700,
Expand Down Expand Up @@ -553,6 +555,32 @@ private enum Id
/// </summary>
public static readonly EventId BoolWithDefaultWarning = MakeValidationId(Id.BoolWithDefaultWarning);

/// <summary>
/// <para>
/// An index specifies a property which is 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="IndexInvalidPropertyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId IndexPropertyNotMappedToAnyTable = MakeValidationId(Id.IndexPropertyNotMappedToAnyTable);

/// <summary>
/// <para>
/// An index specifies properties which map to columns on non-overlapping tables.
/// </para>
/// <para>
/// 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" />.
/// </para>
/// </summary>
public static readonly EventId IndexPropertiesMappedToNonOverlappingTables = MakeValidationId(Id.IndexPropertiesMappedToNonOverlappingTables);

private static readonly string _updatePrefix = DbLoggerCategory.Update.Name + ".";
private static EventId MakeUpdateId(Id id) => new EventId((int)id, _updatePrefix + id);

Expand Down
Loading

0 comments on commit c6dfbe0

Please sign in to comment.