Make original value parameter check for sproc into a warning (#29061)

src/EFCore.Relational/Diagnostics/RelationalEventId.cs

@@ -102,6 +102,7 @@ private enum Id
         ForeignKeyTpcPrincipalWarning,
         TpcStoreGeneratedIdentityWarning,
         KeyPropertiesNotMappedToTable,
+        StoredProcedureConcurrencyTokenNotMapped,
 
         // Update events
         BatchReadyForExecution = CoreEventId.RelationalBaseId + 700,
@@ -872,6 +873,20 @@ private static EventId MakeValidationId(Id id)
     public static readonly EventId KeyPropertiesNotMappedToTable =
         MakeValidationId(Id.KeyPropertiesNotMappedToTable);
 
+    /// <summary>
+    ///     An entity type is mapped to the stored procedure with a concurrency token not mapped to any original value parameter.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
+    ///     </para>
+    ///     <para>
+    ///         This event uses the <see cref="StoredProcedurePropertyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
+    ///     </para>
+    /// </remarks>
+    public static readonly EventId StoredProcedureConcurrencyTokenNotMapped =
+        MakeValidationId(Id.StoredProcedureConcurrencyTokenNotMapped);
+
     /// <summary>
     ///     A foreign key specifies properties which don't map to the related tables.
     /// </summary>

src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs

@@ -3122,6 +3122,46 @@ private static string OptionalDependentWithoutIdentifyingPropertyWarning(EventDe
         return d.GenerateMessage(p.EntityType.DisplayName());
     }
 
+    /// <summary>
+    ///     Logs the <see cref="RelationalEventId.StoredProcedureConcurrencyTokenNotMapped" /> event.
+    /// </summary>
+    /// <param name="diagnostics">The diagnostics logger to use.</param>
+    /// <param name="entityType">The entity type that the stored procedure is mapped to.</param>
+    /// <param name="concurrencyProperty">The property which represents the concurrency token.</param>
+    /// <param name="storedProcedureName">The stored procedure name.</param>
+    public static void StoredProcedureConcurrencyTokenNotMapped(
+        this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
+        IEntityType entityType,
+        IProperty concurrencyProperty,
+        string storedProcedureName)
+    {
+        var definition = RelationalResources.LogStoredProcedureConcurrencyTokenNotMapped(diagnostics);
+
+        if (diagnostics.ShouldLog(definition))
+        {
+            definition.Log(diagnostics, entityType.DisplayName(), storedProcedureName, concurrencyProperty.Name);
+        }
+
+        if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+        {
+            var eventData = new StoredProcedurePropertyEventData(
+                definition,
+                StoredProcedureConcurrencyTokenNotMapped,
+                entityType,
+                concurrencyProperty,
+                storedProcedureName);
+
+            diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+        }
+    }
+
+    private static string StoredProcedureConcurrencyTokenNotMapped(EventDefinitionBase definition, EventData payload)
+    {
+        var d = (EventDefinition<string, string, string>)definition;
+        var p = (StoredProcedurePropertyEventData)payload;
+        return d.GenerateMessage(p.EntityType.DisplayName(), p.StoredProcedureName, p.Property.Name);
+    }
+
     /// <summary>
     ///     Logs for the <see cref="RelationalEventId.BatchExecutorFailedToRollbackToSavepoint" /> event.
     /// </summary>

src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs

@@ -286,6 +286,15 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
     [EntityFrameworkInternal]
     public EventDefinitionBase? LogPossibleUnintendedUseOfEquals;
 
+    /// <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? LogStoredProcedureConcurrencyTokenNotMapped;
+
     /// <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

src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs

@@ -221,23 +221,23 @@ protected virtual void ValidateStoredProcedures(
             if (deleteStoredProcedure != null)
             {
                 AddSproc(StoreObjectType.DeleteStoredProcedure, entityType, storedProcedures);
-                ValidateSproc(deleteStoredProcedure, mappingStrategy);
+                ValidateSproc(deleteStoredProcedure, mappingStrategy, logger);
                 sprocCount++;
             }
 
             var insertStoredProcedure = entityType.GetInsertStoredProcedure();
             if (insertStoredProcedure != null)
             {
                 AddSproc(StoreObjectType.InsertStoredProcedure, entityType, storedProcedures);
-                ValidateSproc(insertStoredProcedure, mappingStrategy);
+                ValidateSproc(insertStoredProcedure, mappingStrategy, logger);
                 sprocCount++;
             }
 
             var updateStoredProcedure = entityType.GetUpdateStoredProcedure();
             if (updateStoredProcedure != null)
             {
                 AddSproc(StoreObjectType.UpdateStoredProcedure, entityType, storedProcedures);
-                ValidateSproc(updateStoredProcedure, mappingStrategy);
+                ValidateSproc(updateStoredProcedure, mappingStrategy, logger);
                 sprocCount++;
             }
 
@@ -288,7 +288,10 @@ static void AddSproc(
         }
     }
 
-    private static void ValidateSproc(IStoredProcedure sproc, string mappingStrategy)
+    private static void ValidateSproc(
+        IStoredProcedure sproc,
+        string mappingStrategy,
+        IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
     {
         var entityType = sproc.EntityType;
         var storeObjectIdentifier = sproc.GetStoreIdentifier();
@@ -646,11 +649,7 @@ private static void ValidateSproc(IStoredProcedure sproc, string mappingStrategy
 
             if (originalValueProperties.Values.FirstOrDefault(p => p.IsConcurrencyToken) is { } missedConcurrencyToken)
             {
-                throw new InvalidOperationException(
-                    RelationalStrings.StoredProcedureConcurrencyTokenNotMapped(
-                        entityType.DisplayName(),
-                        storeObjectIdentifier.DisplayName(),
-                        missedConcurrencyToken.Name));
+                logger.StoredProcedureConcurrencyTokenNotMapped(entityType, missedConcurrencyToken, storeObjectIdentifier.DisplayName());
             }
 
             if (sproc.ResultColumns.Any(c => c != sproc.FindRowsAffectedResultColumn()))

src/EFCore.Relational/Infrastructure/RelationalOptionsExtension.cs

@@ -386,7 +386,8 @@ public static CoreOptionsExtension WithDefaultWarningConfiguration(CoreOptionsEx
                 .TryWithExplicit(RelationalEventId.AmbientTransactionWarning, WarningBehavior.Throw)
                 .TryWithExplicit(RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable, WarningBehavior.Throw)
                 .TryWithExplicit(RelationalEventId.IndexPropertiesMappedToNonOverlappingTables, WarningBehavior.Throw)
-                .TryWithExplicit(RelationalEventId.ForeignKeyPropertiesMappedToUnrelatedTables, WarningBehavior.Throw));
+                .TryWithExplicit(RelationalEventId.ForeignKeyPropertiesMappedToUnrelatedTables, WarningBehavior.Throw)
+                .TryWithExplicit(RelationalEventId.StoredProcedureConcurrencyTokenNotMapped, WarningBehavior.Throw));
 
     /// <summary>
     ///     Information/metadata for a <see cref="RelationalOptionsExtension" />.

src/EFCore.Relational/Properties/RelationalStrings.resx

@@ -769,6 +769,10 @@
     <value>The entity type '{entityType}' is an optional dependent using table sharing without any required non shared property that could be used to identify whether the entity exists. If all nullable properties contain a null value in database then an object instance won't be created in the query. Add a required property to create instances with null values for other properties or mark the incoming navigation as required to always create an instance.</value>
     <comment>Warning RelationalEventId.OptionalDependentWithoutIdentifyingPropertyWarning string</comment>
   </data>
+  <data name="LogStoredProcedureConcurrencyTokenNotMapped" xml:space="preserve">
+    <value>The entity type '{entityType}' is mapped to the stored procedure '{sproc}', but the concurrency token '{token}' is not mapped to any original value parameter.</value>
+    <comment>Warning RelationalEventId.StoredProcedureConcurrencyTokenNotMapped string string string</comment>
+  </data>
   <data name="LogPossibleUnintendedUseOfEquals" xml:space="preserve">
     <value>Possible unintended use of method 'Equals' for arguments '{left}' and '{right}' of different types in a query. This comparison will always return false.</value>
     <comment>Warning RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning string string</comment>
@@ -976,9 +980,6 @@
   <data name="SqlQueryUnmappedType" xml:space="preserve">
     <value>The element type '{elementType}' used in 'SqlQuery' method is not natively supported by your database provider. Either use a supported element type, or use ModelConfigurationBuilder.DefaultTypeMapping to define a mapping for your type.</value>
   </data>
-  <data name="StoredProcedureConcurrencyTokenNotMapped" xml:space="preserve">
-    <value>The entity type '{entityType}' is mapped to the stored procedure '{sproc}', but the concurrency token '{token}' is not mapped to any original value parameter.</value>
-  </data>
   <data name="StoredProcedureCurrentValueParameterOnDelete" xml:space="preserve">
     <value>Current value parameter '{parameter}' is not allowed on delete stored procedure '{sproc}'. Use HasOriginalValueParameter() instead.</value>
   </data>

src/EFCore/Diagnostics/StoredProcedurePropertyEventData.cs

@@ -0,0 +1,43 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Diagnostics;
+
+/// <summary>
+///     A <see cref="DiagnosticSource" /> event payload class for events that have involving mapping of a property to a stored procedure.
+/// </summary>
+/// <remarks>
+///     See <see href="https://aka.ms/efcore-docs-diagnostics">Logging, events, and diagnostics</see> for more information and examples.
+/// </remarks>
+public class StoredProcedurePropertyEventData : PropertyEventData
+{
+    /// <summary>
+    ///     Constructs the event payload.
+    /// </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 that the stored procedure is mapped to.</param>
+    /// <param name="property">The property.</param>
+    /// <param name="storedProcedureName">The stored procedure name.</param>
+    public StoredProcedurePropertyEventData(
+        EventDefinitionBase eventDefinition,
+        Func<EventDefinitionBase, EventData, string> messageGenerator,
+        IEntityType entityType,
+        IProperty property,
+        string storedProcedureName)
+        : base(eventDefinition, messageGenerator, property)
+    {
+        EntityType = entityType;
+        StoredProcedureName = storedProcedureName;
+    }
+
+    /// <summary>
+    ///     The entity type that the stored procedure is mapped to.
+    /// </summary>
+    public virtual IEntityType EntityType { get; }
+
+    /// <summary>
+    ///     The stored procedure name.
+    /// </summary>
+    public virtual string StoredProcedureName { get; }
+}

test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs

@@ -2960,9 +2960,9 @@ public virtual void Detects_unmapped_concurrency_token()
                     .HasRowsAffectedReturnValue())
             .Property(a => a.Name).IsRowVersion();
 
-        VerifyError(
-            RelationalStrings.StoredProcedureConcurrencyTokenNotMapped(nameof(Animal), "Animal_Update", nameof(Animal.Name)),
-            modelBuilder);
+        VerifyWarning(
+            RelationalResources.LogStoredProcedureConcurrencyTokenNotMapped(new TestLogger<TestRelationalLoggingDefinitions>())
+                .GenerateMessage(nameof(Animal), "Animal_Update", nameof(Animal.Name)), modelBuilder);
     }
 
     [ConditionalFact]

test/EFCore.SqlServer.FunctionalTests/LoggingSqlServerTest.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.Diagnostics.Internal;
 using Microsoft.EntityFrameworkCore.SqlServer.Diagnostics.Internal;
 using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal;
 
@@ -9,6 +10,44 @@ namespace Microsoft.EntityFrameworkCore;
 
 public class LoggingSqlServerTest : LoggingRelationalTestBase<SqlServerDbContextOptionsBuilder, SqlServerOptionsExtension>
 {
+    [ConditionalFact]
+    public virtual void StoredProcedureConcurrencyTokenNotMapped_throws_by_default()
+    {
+        using var context = new StoredProcedureConcurrencyTokenNotMappedContext(CreateOptionsBuilder(new ServiceCollection()));
+
+        var definition = RelationalResources.LogStoredProcedureConcurrencyTokenNotMapped(CreateTestLogger());
+        Assert.Equal(
+            CoreStrings.WarningAsErrorTemplate(
+                RelationalEventId.StoredProcedureConcurrencyTokenNotMapped.ToString(),
+                definition.GenerateMessage(nameof(Animal), "Animal_Update", nameof(Animal.Name)),
+                "RelationalEventId.StoredProcedureConcurrencyTokenNotMapped"),
+            Assert.Throws<InvalidOperationException>(
+                () => context.Model).Message);
+    }
+
+    protected class StoredProcedureConcurrencyTokenNotMappedContext : DbContext
+    {
+        public StoredProcedureConcurrencyTokenNotMappedContext(DbContextOptionsBuilder optionsBuilder)
+            : base(optionsBuilder.Options)
+        {
+        }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+            => modelBuilder.Entity<Animal>(
+                b =>
+                {
+                    b.Ignore(a => a.FavoritePerson);
+                    b.Property(e => e.Name).IsRowVersion();
+                    b.UpdateUsingStoredProcedure(
+                        b =>
+                        {
+                            b.HasOriginalValueParameter(e => e.Id);
+                            b.HasParameter(e => e.Name, p => p.IsOutput());
+                            b.HasRowsAffectedReturnValue();
+                        });
+                });
+    }
+
     protected override DbContextOptionsBuilder CreateOptionsBuilder(
         IServiceCollection services,
         Action<RelationalDbContextOptionsBuilder<SqlServerDbContextOptionsBuilder, SqlServerOptionsExtension>> relationalAction)

test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs

@@ -127,6 +127,14 @@ public override void Passes_on_derived_entity_type_not_mapped_to_a_stored_proced
         Assert.Equal(SqliteStrings.StoredProceduresNotSupported(nameof(Animal)), exception.Message);
     }
 
+    public override void Detects_unmapped_concurrency_token()
+    {
+        var exception =
+            Assert.Throws<InvalidOperationException>(() => base.Detects_unmapped_concurrency_token());
+
+        Assert.Equal(SqliteStrings.StoredProceduresNotSupported(nameof(Animal)), exception.Message);
+    }
+
     public override void Store_generated_in_composite_key()
     {
         var modelBuilder = CreateConventionModelBuilder();