Address review comments

src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs

@@ -996,28 +996,34 @@ protected virtual void GenerateTrigger(
         ITrigger trigger,
         IndentedStringBuilder stringBuilder)
     {
-        stringBuilder
+        var triggerBuilderNameStringBuilder = new StringBuilder();
+        triggerBuilderNameStringBuilder
             .Append(tableBuilderName)
             .Append(".HasTrigger(")
             .Append(Code.Literal(trigger.ModelName))
             .Append(")");
+        var triggerBuilderName = triggerBuilderNameStringBuilder.ToString();
+
+        stringBuilder.Append(triggerBuilderName);
+
+        // Note that GenerateAnnotations below does the corresponding decrement
+        stringBuilder.IncrementIndent();
 
         if (trigger.Name != null
             && trigger.Name != (trigger.GetDefaultName() ?? trigger.ModelName))
         {
-            using (stringBuilder.Indent())
-            {
-                stringBuilder
-                    .AppendLine()
-                    .Append(".HasName(")
-                    .Append(Code.Literal(trigger.Name))
-                    .Append(")");
-            }
+            stringBuilder
+                .AppendLine()
+                .Append(".HasName(")
+                .Append(Code.Literal(trigger.Name))
+                .Append(")");
         }
 
-        // TODO: Generate annotations - but decide if we want a nested builder or a returned one
+        var annotations = Dependencies.AnnotationCodeGenerator
+            .FilterIgnoredAnnotations(trigger.GetAnnotations())
+            .ToDictionary(a => a.Name, a => a);
 
-        stringBuilder.AppendLine(";");
+        GenerateAnnotations(triggerBuilderName, trigger, stringBuilder, annotations, inChainedCall: true);
     }
 
     /// <summary>

src/EFCore.Relational/Design/IAnnotationCodeGenerator.cs

@@ -223,6 +223,17 @@ IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
         IDictionary<string, IAnnotation> annotations)
         => Array.Empty<MethodCallCodeFragment>();
 
+    /// <summary>
+    ///     For the given annotations which have corresponding fluent API calls, returns those fluent API calls
+    ///     and removes the annotations.
+    /// </summary>
+    /// <param name="trigger">The trigger to which the annotations are applied.</param>
+    /// <param name="annotations">The set of annotations from which to generate fluent API calls.</param>
+    IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
+        ITrigger trigger,
+        IDictionary<string, IAnnotation> annotations)
+        => Array.Empty<MethodCallCodeFragment>();
+
     /// <summary>
     ///     For the given annotations which have corresponding fluent API calls, returns those fluent API calls
     ///     and removes the annotations.
@@ -240,6 +251,8 @@ IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(IAnnotatable annota
             INavigation navigation => GenerateFluentApiCalls(navigation, annotations),
             ISkipNavigation skipNavigation => GenerateFluentApiCalls(skipNavigation, annotations),
             IIndex index => GenerateFluentApiCalls(index, annotations),
+            ITrigger trigger => GenerateFluentApiCalls(trigger, annotations),
+
             _ => throw new ArgumentException(RelationalStrings.UnhandledAnnotatableType(annotatable.GetType()))
         };
 

src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs

@@ -1773,4 +1773,55 @@ public static bool CanSetComment(
             RelationalAnnotationNames.Comment,
             comment,
             fromDataAnnotation);
+
+    /// <summary>
+    ///     Configures a database trigger when targeting a relational database.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
+    /// </remarks>
+    /// <param name="entityTypeBuilder">The entity type builder.</param>
+    /// <param name="name">The name of the trigger.</param>
+    /// <param name="tableName">The name of the table on which this trigger is defined.</param>
+    /// <param name="tableSchema">The schema of the table on which this trigger is defined.</param>
+    /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
+    /// <returns>The same builder instance if the check constraint was configured, <see langword="null" /> otherwise.</returns>
+    public static IConventionTriggerBuilder? HasTrigger(
+        this IConventionEntityTypeBuilder entityTypeBuilder,
+        string name,
+        string? tableName,
+        string? tableSchema,
+        bool fromDataAnnotation = false)
+        => InternalTriggerBuilder.HasTrigger(
+                entityTypeBuilder.Metadata,
+                name,
+                tableName,
+                tableSchema,
+                fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention)
+            ?.Builder;
+
+    /// <summary>
+    ///     Returns a value indicating whether the trigger can be configured.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
+    /// </remarks>
+    /// <param name="entityTypeBuilder">The builder for the entity type being configured.</param>
+    /// <param name="name">The name of the trigger.</param>
+    /// <param name="tableName">The name of the table on which this trigger is defined.</param>
+    /// <param name="tableSchema">The schema of the table on which this trigger is defined.</param>
+    /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
+    /// <returns><see langword="true" /> if the configuration can be applied.</returns>
+    public static bool CanHaveTrigger(
+        this IConventionEntityTypeBuilder entityTypeBuilder,
+        string name,
+        string? tableName,
+        string? tableSchema,
+        bool fromDataAnnotation = false)
+        => InternalTriggerBuilder.CanHaveTrigger(
+            entityTypeBuilder.Metadata,
+            name,
+            tableName,
+            tableSchema,
+            fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
 }

src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs

@@ -1,6 +1,7 @@
 // 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.Internal;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 
 namespace Microsoft.EntityFrameworkCore.Infrastructure;
@@ -56,6 +57,7 @@ public override void Validate(IModel model, IDiagnosticsLogger<DbLoggerCategory.
         ValidateDefaultValuesOnKeys(model, logger);
         ValidateBoolsWithDefaults(model, logger);
         ValidateIndexProperties(model, logger);
+        ValidateTriggers(model, logger);
     }
 
     /// <summary>
@@ -291,6 +293,7 @@ protected virtual void ValidateSharedTableCompatibility(
             ValidateSharedForeignKeysCompatibility(mappedTypes, table, logger);
             ValidateSharedIndexesCompatibility(mappedTypes, table, logger);
             ValidateSharedCheckConstraintCompatibility(mappedTypes, table, logger);
+            ValidateSharedTriggerCompatibility(mappedTypes, table, logger);
 
             // Validate optional dependents
             if (mappedTypes.Count == 1)
@@ -1186,7 +1189,7 @@ protected virtual void ValidateCompatible(
         => key.AreCompatible(duplicateKey, storeObject, shouldThrow: true);
 
     /// <summary>
-    ///     Validates the compatibility of check constraint in a given shared table.
+    ///     Validates the compatibility of check constraints in a given shared table.
     /// </summary>
     /// <param name="mappedTypes">The mapped entity types.</param>
     /// <param name="storeObject">The identifier of the store object.</param>
@@ -1218,8 +1221,8 @@ protected virtual void ValidateSharedCheckConstraintCompatibility(
     /// <summary>
     ///     Validates the compatibility of two check constraints with the same name.
     /// </summary>
-    /// <param name="checkConstraint">An check constraints.</param>
-    /// <param name="duplicateCheckConstraint">Another check constraints.</param>
+    /// <param name="checkConstraint">A check constraint.</param>
+    /// <param name="duplicateCheckConstraint">Another check constraint.</param>
     /// <param name="indexName">The name of the check constraint.</param>
     /// <param name="storeObject">The identifier of the store object.</param>
     /// <param name="logger">The logger to use.</param>
@@ -1231,6 +1234,53 @@ protected virtual void ValidateCompatible(
         IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
         => CheckConstraint.AreCompatible(checkConstraint, duplicateCheckConstraint, storeObject, shouldThrow: true);
 
+    /// <summary>
+    ///     Validates the compatibility of triggers in a given shared table.
+    /// </summary>
+    /// <param name="mappedTypes">The mapped entity types.</param>
+    /// <param name="storeObject">The identifier of the store object.</param>
+    /// <param name="logger">The logger to use.</param>
+    protected virtual void ValidateSharedTriggerCompatibility(
+        IReadOnlyList<IEntityType> mappedTypes,
+        in StoreObjectIdentifier storeObject,
+        IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
+    {
+        var triggerMappings = new Dictionary<string, ITrigger>();
+        foreach (var trigger in mappedTypes.SelectMany(et => et.GetDeclaredTriggers()))
+        {
+            var triggerName = trigger.GetName(storeObject);
+            if (triggerName == null)
+            {
+                continue;
+            }
+
+            if (!triggerMappings.TryGetValue(triggerName, out var duplicateTrigger))
+            {
+                triggerMappings[triggerName] = trigger;
+                continue;
+            }
+
+            ValidateCompatible(trigger, duplicateTrigger, triggerName, storeObject, logger);
+        }
+    }
+
+    /// <summary>
+    ///     Validates the compatibility of two trigger with the same name.
+    /// </summary>
+    /// <param name="trigger">A trigger.</param>
+    /// <param name="duplicateTrigger">Another trigger.</param>
+    /// <param name="indexName">The name of the trigger.</param>
+    /// <param name="storeObject">The identifier of the store object.</param>
+    /// <param name="logger">The logger to use.</param>
+    protected virtual void ValidateCompatible(
+        ITrigger trigger,
+        ITrigger duplicateTrigger,
+        string indexName,
+        in StoreObjectIdentifier storeObject,
+        IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
+    {
+    }
+
     /// <summary>
     ///     Validates the mapping/configuration of inheritance in the model.
     /// </summary>
@@ -1405,8 +1455,7 @@ protected virtual void ValidatePropertyOverrides(
     }
 
     /// <summary>
-    ///     Validates that the properties of any one index are
-    ///     all mapped to columns on at least one common table.
+    ///     Validates that the properties of any one index are all mapped to columns on at least one common table.
     /// </summary>
     /// <param name="model">The model to validate.</param>
     /// <param name="logger">The logger to use.</param>
@@ -1508,6 +1557,43 @@ protected virtual void ValidateIndexProperties(
         }
     }
 
+    /// <summary>
+    ///     Validates that the triggers are unambiguously mapped to exactly one table.
+    /// </summary>
+    /// <param name="model">The model to validate.</param>
+    /// <param name="logger">The logger to use.</param>
+    protected virtual void ValidateTriggers(
+        IModel model,
+        IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
+    {
+        foreach (var entityType in model.GetEntityTypes())
+        {
+            var tableName = entityType.GetTableName();
+            var tableSchema = entityType.GetSchema();
+
+            foreach (var trigger in entityType.GetDeclaredTriggers())
+            {
+                if (tableName is null)
+                {
+                    throw new InvalidOperationException(
+                        RelationalStrings.TriggerOnUnmappedEntityType(trigger.ModelName, entityType.DisplayName()));
+                }
+
+                if ((trigger.TableName is not null && trigger.TableName != tableName)
+                    || (trigger.TableSchema is not null && trigger.TableSchema != tableSchema))
+                {
+                    throw new InvalidOperationException(
+                        RelationalStrings.TriggerWithMismatchedTable(
+                            trigger.ModelName,
+                            (trigger.TableName!, trigger.TableSchema).FormatTable(),
+                            entityType.DisplayName(),
+                            entityType.GetSchemaQualifiedTableName())
+                    );
+                }
+            }
+        }
+    }
+
     /// <summary>
     ///     Throws an <see cref="InvalidOperationException" /> with a message containing provider-specific information, when
     ///     available, indicating possible reasons why the property cannot be mapped.

src/EFCore.Relational/Metadata/Builders/TableBuilder.cs

@@ -66,22 +66,16 @@ public virtual TableBuilder ExcludeFromMigrations(bool excluded = true)
     /// </summary>
     /// <param name="name">The name of the trigger.</param>
     /// <returns>A builder that can be used to configure the database trigger.</returns>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
+    /// </remarks>
     public virtual TriggerBuilder HasTrigger(string name)
-        => new(HasTrigger(Metadata, name, ConfigurationSource.Explicit));
-
-    private Trigger HasTrigger(IMutableEntityType entityType, string name, ConfigurationSource configurationSource)
-    {
-        Check.NotEmpty(name, nameof(name));
-
-        var trigger = (Trigger?)Trigger.FindTrigger(entityType, name);
-        if (trigger != null)
-        {
-            trigger.UpdateConfigurationSource(configurationSource);
-            return trigger;
-        }
-
-        return new Trigger(entityType, name, Name, Schema, configurationSource);
-    }
+        => new((Trigger)InternalTriggerBuilder.HasTrigger(
+            (IConventionEntityType)Metadata,
+            name,
+            Name,
+            Schema,
+            ConfigurationSource.Explicit)!);
 
     #region Hidden System.Object members
 

src/EFCore.Relational/Metadata/Builders/TriggerBuilder.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.ComponentModel;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 
 namespace Microsoft.EntityFrameworkCore.Metadata.Builders;
@@ -56,4 +57,51 @@ public virtual TriggerBuilder HasName(string name)
 
         return this;
     }
+
+    /// <summary>
+    ///     Adds or updates an annotation on the trigger. If an annotation with the key specified in <paramref name="annotation" />
+    ///     already exists, its value will be updated.
+    /// </summary>
+    /// <param name="annotation">The key of the annotation to be added or updated.</param>
+    /// <param name="value">The value to be stored in the annotation.</param>
+    /// <returns>The same builder instance so that multiple configuration calls can be chained.</returns>
+    public virtual TriggerBuilder HasAnnotation(string annotation, object? value)
+    {
+        Check.NotEmpty(annotation, nameof(annotation));
+
+        Builder.HasAnnotation(annotation, value, ConfigurationSource.Explicit);
+
+        return this;
+    }
+
+    #region Hidden System.Object members
+
+    /// <summary>
+    ///     Returns a string that represents the current object.
+    /// </summary>
+    /// <returns>A string that represents the current object.</returns>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public override string? ToString()
+        => base.ToString();
+
+    /// <summary>
+    ///     Determines whether the specified object is equal to the current object.
+    /// </summary>
+    /// <param name="obj">The object to compare with the current object.</param>
+    /// <returns><see langword="true" /> if the specified object is equal to the current object; otherwise, <see langword="false" />.</returns>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    // ReSharper disable once BaseObjectEqualsIsObjectEquals
+    public override bool Equals(object? obj)
+        => base.Equals(obj);
+
+    /// <summary>
+    ///     Serves as the default hash function.
+    /// </summary>
+    /// <returns>A hash code for the current object.</returns>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    // ReSharper disable once BaseObjectGetHashCodeCallInGetHashCode
+    public override int GetHashCode()
+        => base.GetHashCode();
+
+    #endregion
 }

src/EFCore.Relational/Metadata/Conventions/CheckConstraintConvention.cs

@@ -6,8 +6,8 @@
 namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
 
 /// <summary>
-///     A convention that ensures that the check constraints on the derived types are compatible with
-///     the check constraints on the base type. And also ensures that the declaring type is current.
+///     A convention that ensures that the check constraints on the derived types are compatible with the check constraints on the base
+///     type. And also ensures that the declaring type is current.
 /// </summary>
 /// <remarks>
 ///     See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see> for more information and examples.