Skip to content

Commit

Permalink
Metadata changes from the API review
Browse files Browse the repository at this point in the history
Revert 'Add a custom convention which adds a blank trigger to all tables'

Part of #27588
  • Loading branch information
AndriySvyryd authored Aug 19, 2022
1 parent 3ad27d0 commit 4cfbb5f
Show file tree
Hide file tree
Showing 29 changed files with 70 additions and 191 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1263,7 +1263,7 @@ protected virtual void GenerateTriggerAnnotations(
{
stringBuilder
.AppendLine()
.Append(".HasName(")
.Append(".HasDatabaseName(")
.Append(Code.Literal((string?)nameAnnotation.Value))
.Append(")");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ public static class RelationalTriggerBuilderExtensions
/// <param name="name">The database name of the trigger.</param>
/// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
/// <returns>The same builder instance if the configuration was applied, <see langword="null" /> otherwise.</returns>
public static IConventionTriggerBuilder? HasName(
public static IConventionTriggerBuilder? HasDatabaseName(
this IConventionTriggerBuilder triggerBuilder, string? name, bool fromDataAnnotation = false)
{
if (!triggerBuilder.CanSetName(name, fromDataAnnotation))
if (!triggerBuilder.CanSetDatabaseName(name, fromDataAnnotation))
{
return null;
}

triggerBuilder.Metadata.SetName(name, fromDataAnnotation);
triggerBuilder.Metadata.SetDatabaseName(name, fromDataAnnotation);
return triggerBuilder;
}

Expand All @@ -38,7 +38,7 @@ public static class RelationalTriggerBuilderExtensions
/// <param name="name">The database name of the trigger.</param>
/// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
/// <returns><see langword="true" /> if the database name can be set for the trigger.</returns>
public static bool CanSetName(this IConventionTriggerBuilder triggerBuilder, string? name, bool fromDataAnnotation = false)
public static bool CanSetDatabaseName(this IConventionTriggerBuilder triggerBuilder, string? name, bool fromDataAnnotation = false)
=> triggerBuilder.CanSetAnnotation(RelationalAnnotationNames.Name, name, fromDataAnnotation);

/// <summary>
Expand Down
20 changes: 10 additions & 10 deletions src/EFCore.Relational/Extensions/RelationalTriggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,26 @@ public static class RelationalTriggerExtensions
/// </summary>
/// <param name="trigger">The trigger.</param>
/// <returns>The name of the trigger in the database.</returns>
public static string? GetName(this IReadOnlyTrigger trigger)
public static string? GetDatabaseName(this IReadOnlyTrigger trigger)
{
if (trigger.EntityType.GetTableName() == null)
{
return null;
}

var annotation = trigger.FindAnnotation(RelationalAnnotationNames.Name);
return annotation != null ? (string?)annotation.Value : trigger.GetDefaultName();
return annotation != null ? (string?)annotation.Value : trigger.GetDefaultDatabaseName();
}

/// <summary>
/// Returns the default name that would be used for this trigger in the database.
/// </summary>
/// <param name="trigger">The trigger.</param>
/// <returns>The default name that would be used for this trigger in the database.</returns>
public static string? GetDefaultName(this IReadOnlyTrigger trigger)
public static string? GetDefaultDatabaseName(this IReadOnlyTrigger trigger)
{
var table = StoreObjectIdentifier.Create(trigger.EntityType, StoreObjectType.Table);
return !table.HasValue ? null : trigger.GetDefaultName(table.Value);
return !table.HasValue ? null : trigger.GetDefaultDatabaseName(table.Value);
}

/// <summary>
Expand All @@ -45,7 +45,7 @@ public static class RelationalTriggerExtensions
/// <param name="trigger">The trigger.</param>
/// <param name="storeObject">The identifier of the store object.</param>
/// <returns>The database name of the trigger for the given store object.</returns>
public static string? GetName(this IReadOnlyTrigger trigger, in StoreObjectIdentifier storeObject)
public static string? GetDatabaseName(this IReadOnlyTrigger trigger, in StoreObjectIdentifier storeObject)
{
var triggerTable = trigger.GetTableName();
if (storeObject.StoreObjectType != StoreObjectType.Table
Expand All @@ -57,7 +57,7 @@ public static class RelationalTriggerExtensions
}

var annotation = trigger.FindAnnotation(RelationalAnnotationNames.Name);
return annotation != null ? (string?)annotation.Value : trigger.GetDefaultName(storeObject);
return annotation != null ? (string?)annotation.Value : trigger.GetDefaultDatabaseName(storeObject);
}

/// <summary>
Expand All @@ -66,7 +66,7 @@ public static class RelationalTriggerExtensions
/// <param name="trigger">The trigger.</param>
/// <param name="storeObject">The identifier of the store object.</param>
/// <returns>The default name that would be used for this trigger.</returns>
public static string? GetDefaultName(this IReadOnlyTrigger trigger, in StoreObjectIdentifier storeObject)
public static string? GetDefaultDatabaseName(this IReadOnlyTrigger trigger, in StoreObjectIdentifier storeObject)
=> storeObject.StoreObjectType == StoreObjectType.Table
? Uniquifier.Truncate(trigger.ModelName, trigger.EntityType.Model.GetMaxIdentifierLength())
: null;
Expand All @@ -76,7 +76,7 @@ public static class RelationalTriggerExtensions
/// </summary>
/// <param name="trigger">The trigger.</param>
/// <param name="name">The name of the trigger in the database.</param>
public static void SetName(this IMutableTrigger trigger, string? name)
public static void SetDatabaseName(this IMutableTrigger trigger, string? name)
=> trigger.SetOrRemoveAnnotation(
RelationalAnnotationNames.Name,
Check.NullButNotEmpty(name, nameof(name)));
Expand All @@ -88,7 +88,7 @@ public static void SetName(this IMutableTrigger trigger, string? name)
/// <param name="name">The name of the trigger in the database.</param>
/// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
/// <returns>The configured value.</returns>
public static string? SetName(this IConventionTrigger trigger, string? name, bool fromDataAnnotation = false)
public static string? SetDatabaseName(this IConventionTrigger trigger, string? name, bool fromDataAnnotation = false)
=> (string?)trigger.SetOrRemoveAnnotation(
RelationalAnnotationNames.Name,
Check.NullButNotEmpty(name, nameof(name)),
Expand All @@ -99,7 +99,7 @@ public static void SetName(this IMutableTrigger trigger, string? name)
/// </summary>
/// <param name="trigger">The trigger.</param>
/// <returns>The configuration source for the database name.</returns>
public static ConfigurationSource? GetNameConfigurationSource(this IConventionTrigger trigger)
public static ConfigurationSource? GetDatabaseNameConfigurationSource(this IConventionTrigger trigger)
=> trigger.FindAnnotation(RelationalAnnotationNames.Name)?.GetConfigurationSource();

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1647,7 +1647,7 @@ protected virtual void ValidateSharedTriggerCompatibility(
var triggerMappings = new Dictionary<string, ITrigger>();
foreach (var trigger in mappedTypes.SelectMany(et => et.GetDeclaredTriggers()))
{
var triggerName = trigger.GetName(storeObject);
var triggerName = trigger.GetDatabaseName(storeObject);
if (triggerName == null)
{
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public virtual OwnedNavigationSplitTableBuilder ExcludeFromMigrations(bool exclu
/// </remarks>
public virtual TableTriggerBuilder HasTrigger(string modelName)
{
var trigger = EntityTypeBuilder.HasTrigger(modelName, OwnedNavigationBuilder.OwnedEntityType).Metadata;
var trigger = EntityTypeBuilder.HasTrigger(OwnedNavigationBuilder.OwnedEntityType, modelName).Metadata;
trigger.SetTableName(Name);
trigger.SetTableSchema(Schema);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public virtual OwnedNavigationTableBuilder ExcludeFromMigrations(bool excluded =
/// </remarks>
public virtual TableTriggerBuilder HasTrigger(string modelName)
{
var trigger = EntityTypeBuilder.HasTrigger(modelName, Metadata).Metadata;
var trigger = EntityTypeBuilder.HasTrigger(Metadata, modelName).Metadata;
if (Name != null)
{
trigger.SetTableName(Name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public virtual SplitTableBuilder ExcludeFromMigrations(bool excluded = true)
/// </remarks>
public virtual TableTriggerBuilder HasTrigger(string modelName)
{
var trigger = EntityTypeBuilder.HasTrigger(modelName, EntityTypeBuilder.Metadata).Metadata;
var trigger = EntityTypeBuilder.HasTrigger(EntityTypeBuilder.Metadata, modelName).Metadata;
trigger.SetTableName(Name);
trigger.SetTableSchema(Schema);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ protected virtual PropertyBuilder CreatePropertyBuilder(string propertyName)
property = entityType.GetDerivedTypes().SelectMany(et => et.GetDeclaredProperties())
.FirstOrDefault(p => p.Name == propertyName);
}

if (property == null)
{
throw new InvalidOperationException(CoreStrings.PropertyNotFound(propertyName, entityType.DisplayName()));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.EntityFrameworkCore.Metadata.Internal;

namespace Microsoft.EntityFrameworkCore.Metadata.Builders;

/// <summary>
Expand Down Expand Up @@ -73,7 +71,7 @@ public virtual StoredProcedureBuilder<TEntity> HasParameter<TDerivedEntity, TPro
Builder.HasParameter(propertyExpression, ConfigurationSource.Explicit);
return this;
}

/// <summary>
/// Configures a new parameter if no parameter mapped to the given property exists.
/// </summary>
Expand Down Expand Up @@ -154,7 +152,7 @@ public virtual StoredProcedureBuilder<TEntity> HasOriginalValueParameter<TDerive
Builder.HasOriginalValueParameter(propertyExpression, ConfigurationSource.Explicit);
return this;
}

/// <summary>
/// Configures a new parameter that holds the original value if no parameter mapped to the given property exists.
/// </summary>
Expand Down Expand Up @@ -222,7 +220,7 @@ public virtual StoredProcedureBuilder<TEntity> HasOriginalValueParameter<TDerive
public new virtual StoredProcedureBuilder<TEntity> HasResultColumn(
string propertyName, Action<StoredProcedureResultColumnBuilder> buildAction)
=> (StoredProcedureBuilder<TEntity>)base.HasResultColumn(propertyName, buildAction);

/// <summary>
/// Configures a new column of the result for this stored procedure. This is used for database generated columns.
/// </summary>
Expand Down Expand Up @@ -251,7 +249,7 @@ public virtual StoredProcedureBuilder<TEntity> HasResultColumn<TDerivedEntity, T
Builder.HasResultColumn(propertyExpression, ConfigurationSource.Explicit);
return this;
}

/// <summary>
/// Configures a new column of the result for this stored procedure. This is used for database generated columns.
/// </summary>
Expand Down Expand Up @@ -285,15 +283,15 @@ public virtual StoredProcedureBuilder<TEntity> HasResultColumn<TDerivedEntity, T
buildAction(new(resultColumnBuilder, CreatePropertyBuilder(propertyExpression)));
return this;
}

/// <summary>
/// Configures a new column of the result that returns the rows affected for this stored procedure
/// if no such column exists.
/// </summary>
/// <returns>The same builder instance so that multiple configuration calls can be chained.</returns>
public new virtual StoredProcedureBuilder<TEntity> HasRowsAffectedResultColumn()
=> (StoredProcedureBuilder<TEntity>)base.HasRowsAffectedResultColumn();

/// <summary>
/// Configures a new column of the result that returns the rows affected for this stored procedure
/// if no such column exists.
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Metadata/Builders/TableBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public virtual TableBuilder ExcludeFromMigrations(bool excluded = true)
/// </remarks>
public virtual TableTriggerBuilder HasTrigger(string modelName)
{
var trigger = EntityTypeBuilder.HasTrigger(modelName, Metadata).Metadata;
var trigger = EntityTypeBuilder.HasTrigger(Metadata, modelName).Metadata;
if (Name != null)
{
trigger.SetTableName(Name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.ComponentModel;
using System.Xml.Linq;
using Microsoft.EntityFrameworkCore.Metadata.Internal;

namespace Microsoft.EntityFrameworkCore.Metadata.Builders;

Expand Down Expand Up @@ -32,9 +30,9 @@ public TableTriggerBuilder(IMutableTrigger trigger)
/// </remarks>
/// <param name="name">The database name of the trigger.</param>
/// <returns>The same builder instance so that multiple configuration calls can be chained.</returns>
public virtual TableTriggerBuilder HasName(string? name)
public virtual TableTriggerBuilder HasDatabaseName(string? name)
{
Metadata.SetName(name);
Metadata.SetDatabaseName(name);

return this;
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ private void TryUniquifyTriggerNames(
{
foreach (var trigger in entityType.GetDeclaredTriggers())
{
var triggerName = trigger.GetName(storeObject);
var triggerName = trigger.GetDatabaseName(storeObject);
if (triggerName == null)
{
continue;
Expand Down Expand Up @@ -659,10 +659,10 @@ protected virtual bool AreCompatible(
Dictionary<string, T> triggers,
int maxLength)
{
if (trigger.Builder.CanSetName(null))
if (trigger.Builder.CanSetDatabaseName(null))
{
triggerName = Uniquifier.Uniquify(triggerName, triggers, n => n, maxLength);
trigger.Builder.HasName(triggerName);
trigger.Builder.HasDatabaseName(triggerName);
return triggerName;
}

Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Metadata/Internal/RelationalModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1503,7 +1503,7 @@ private static void PopulateTableConfiguration(Table table, bool designTime)
// Triggers are not inherited
foreach (var trigger in entityType.GetDeclaredTriggers())
{
var name = trigger.GetName(storeObject);
var name = trigger.GetDatabaseName(storeObject);
if (name == null)
{
continue;
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/Metadata/Builders/EntityTypeBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,10 +1093,10 @@ public virtual EntityTypeBuilder HasNoDiscriminator()
/// <summary>
/// Configures a trigger for the the entity type.
/// </summary>
/// <param name="modelName">The name of the trigger.</param>
/// <param name="entityType">The entity type.</param>
/// <param name="modelName">The name of the trigger.</param>
/// <returns>A builder that can be used to configure the trigger.</returns>
public static TriggerBuilder HasTrigger(string modelName, IMutableEntityType entityType)
public static TriggerBuilder HasTrigger(IMutableEntityType entityType, string modelName)
=> new(((EntityType)entityType).Builder.HasTrigger(modelName, ConfigurationSource.Explicit)!.Metadata);

#region Hidden System.Object members
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ protected override void Down(MigrationBuilder migrationBuilder)
migrationCode,
ignoreLineEndingDifferences: true);

var modelBuilder = SqlServerTestHelpers.Instance.CreateConventionBuilder(configureModel: c => c.RemoveAllConventions());
var modelBuilder = SqlServerTestHelpers.Instance.CreateConventionBuilder(configureConventions: c => c.RemoveAllConventions());
modelBuilder.HasAnnotation("Some:EnumValue", RegexOptions.Multiline);
modelBuilder.HasAnnotation(RelationalAnnotationNames.DbFunctions, new SortedDictionary<string, IDbFunction>());
modelBuilder.Entity(
Expand Down
Loading

0 comments on commit 4cfbb5f

Please sign in to comment.