Support sproc input/output parameters on non-concurrency-token proper…

…ties

Closes dotnet#28704

src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs

@@ -436,12 +436,69 @@ private static void ValidateSproc(IStoredProcedure sproc, string mappingStrategy
                             RelationalStrings.StoredProcedureParameterNotFound(
                                 parameter.PropertyName, entityType.DisplayName(), storeObjectIdentifier.DisplayName()));
                     }
+
+                    if (storeObjectIdentifier.StoreObjectType == StoreObjectType.InsertStoredProcedure)
+                    {
+                        throw new InvalidOperationException(
+                            RelationalStrings.StoredProcedureOriginalValueParameterOnInsert(
+                                parameter.Name, storeObjectIdentifier.DisplayName()));
+                    }
                 }
-                else if (!properties.TryGetAndRemove(parameter.PropertyName, out property))
+                else
                 {
-                    throw new InvalidOperationException(
-                        RelationalStrings.StoredProcedureParameterNotFound(
-                            parameter.PropertyName, entityType.DisplayName(), storeObjectIdentifier.DisplayName()));
+                    if (!properties.TryGetAndRemove(parameter.PropertyName, out property))
+                    {
+                        throw new InvalidOperationException(
+                            RelationalStrings.StoredProcedureParameterNotFound(
+                                parameter.PropertyName, entityType.DisplayName(), storeObjectIdentifier.DisplayName()));
+                    }
+
+                    if (storeObjectIdentifier.StoreObjectType == StoreObjectType.DeleteStoredProcedure)
+                    {
+                        throw new InvalidOperationException(
+                            RelationalStrings.StoredProcedureCurrentValueParameterOnDelete(
+                                parameter.Name, storeObjectIdentifier.DisplayName()));
+                    }
+
+                    if (parameter.Direction.HasFlag(ParameterDirection.Input))
+                    {
+                        switch (storeObjectIdentifier.StoreObjectType)
+                        {
+                            case StoreObjectType.InsertStoredProcedure:
+                                if (property.GetBeforeSaveBehavior() != PropertySaveBehavior.Save)
+                                {
+                                    throw new InvalidOperationException(
+                                        RelationalStrings.StoredProcedureInputParameterForInsertNonSaveProperty(
+                                            parameter.Name,
+                                            storeObjectIdentifier.DisplayName(),
+                                            parameter.PropertyName,
+                                            entityType.DisplayName(),
+                                            property.GetBeforeSaveBehavior()));
+                                }
+                                break;
+
+                            case StoreObjectType.UpdateStoredProcedure:
+                                if (property.GetAfterSaveBehavior() != PropertySaveBehavior.Save)
+                                {
+                                    throw new InvalidOperationException(
+                                        RelationalStrings.StoredProcedureInputParameterForUpdateNonSaveProperty(
+                                            parameter.Name,
+                                            storeObjectIdentifier.DisplayName(),
+                                            parameter.PropertyName,
+                                            entityType.DisplayName(),
+                                            property.GetAfterSaveBehavior()));
+                                }
+
+                                break;
+
+                            case StoreObjectType.DeleteStoredProcedure:
+                                break;
+
+                            default:
+                                Check.DebugFail("Unexpected stored procedure type: " + storeObjectIdentifier.StoreObjectType);
+                                break;
+                        }
+                    }
                 }
             }
 
@@ -578,8 +635,14 @@ private static void ValidateSproc(IStoredProcedure sproc, string mappingStrategy
             || sproc.FindRowsAffectedParameter() != null
             || sproc.FindRowsAffectedResultColumn() != null)
         {
-            if (originalValueProperties.Values.FirstOrDefault(p => p.IsConcurrencyToken) is { } missedConcurrencyToken
-                && storeObjectIdentifier.StoreObjectType != StoreObjectType.InsertStoredProcedure)
+            if (storeObjectIdentifier.StoreObjectType == StoreObjectType.InsertStoredProcedure)
+            {
+                throw new InvalidOperationException(
+                    RelationalStrings.StoredProcedureRowsAffectedForInsert(
+                        storeObjectIdentifier.DisplayName()));
+            }
+
+            if (originalValueProperties.Values.FirstOrDefault(p => p.IsConcurrencyToken) is { } missedConcurrencyToken)
             {
                 throw new InvalidOperationException(
                     RelationalStrings.StoredProcedureConcurrencyTokenNotMapped(

src/EFCore.Relational/Properties/RelationalStrings.resx

@@ -979,6 +979,9 @@
   <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>
   <data name="StoredProcedureDeleteNonKeyProperty" xml:space="preserve">
     <value>The property '{entityType}.{property}' is mapped to a parameter of the stored procedure '{sproc}', but only concurrency token and key properties are supported for Delete stored procedures.</value>
   </data>
@@ -1006,12 +1009,21 @@
   <data name="StoredProcedureGeneratedPropertiesNotMapped" xml:space="preserve">
     <value>The entity type '{entityType}' is mapped to the stored procedure '{sproc}', however the store-generated properties {properties} are not mapped to any output parameter or result column.</value>
   </data>
+  <data name="StoredProcedureInputParameterForInsertNonSaveProperty" xml:space="preserve">
+    <value>Input parameter '{parameter}' of insert stored procedure '{sproc}' is mapped to property '{property}' of entity type '{entityType}', but that property is configured with BeforeSaveBehavior '{behavior}', and so cannot be saved on insert.</value>
+  </data>
+  <data name="StoredProcedureInputParameterForUpdateNonSaveProperty" xml:space="preserve">
+    <value>Input parameter '{parameter}' of update stored procedure '{sproc}' is mapped to property '{property}' of entity type '{entityType}', but that property is configured with AfterSaveBehavior '{behavior}', and so cannot be saved on update. You may need to use HasOriginalValueParameter() instead of HasParameter().</value>
+  </data>
   <data name="StoredProcedureKeyless" xml:space="preserve">
     <value>The keyless entity type '{entityType}' was configured to use '{sproc}'. An entity type requires a primary key to be able to be mapped to a stored procedure.</value>
   </data>
   <data name="StoredProcedureNoName" xml:space="preserve">
     <value>The entity type '{entityType}' was configured to use '{sproc}', but the store name was not specified. Configure the stored procedure name explicitly.</value>
   </data>
+  <data name="StoredProcedureOriginalValueParameterOnInsert" xml:space="preserve">
+    <value>Original value parameter '{parameter}' is not allowed on insert stored procedure '{sproc}'. Use HasParameter() instead.</value>
+  </data>
   <data name="StoredProcedureOutputParameterConflict" xml:space="preserve">
     <value>The property '{entityType}.{property}' is mapped to an output parameter of the stored procedure '{sproc}', but it is also mapped to an output original value output parameter. A store-generated property can only be mapped to one output parameter.</value>
   </data>
@@ -1048,6 +1060,9 @@
   <data name="StoredProcedureRowsAffectedNotPopulated" xml:space="preserve">
     <value>Stored procedure '{sproc}' was configured with a rows affected output parameter or return value, but a valid value was not found when executing the procedure.</value>
   </data>
+  <data name="StoredProcedureRowsAffectedForInsert" xml:space="preserve">
+    <value>A rows affected parameter, result column or return value cannot be configured on stored procedure '{sproc}' because it is used for insertion. Rows affected values are only allowed on stored procedures performing updating or deletion.</value>
+  </data>
   <data name="StoredProcedureRowsAffectedReturnConflictingParameter" xml:space="preserve">
     <value>The stored procedure '{sproc}' cannot be configured to return the rows affected because a rows affected parameter or a rows affected result column for this stored procedure already exists.</value>
   </data>

src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs

@@ -44,7 +44,7 @@ protected override void Consume(RelationalDataReader reader)
             bool? onResultSet = null;
             var hasOutputParameters = false;
 
-            for (; commandIndex < ResultSetMappings.Count; commandIndex++)
+            while (commandIndex < ResultSetMappings.Count)
             {
                 var resultSetMapping = ResultSetMappings[commandIndex];
 
@@ -63,10 +63,14 @@ protected override void Consume(RelationalDataReader reader)
                         ? lastHandledCommandIndex == commandIndex
                         : lastHandledCommandIndex > commandIndex, "Bad handling of ResultSetMapping and command indexing");
 
-                    commandIndex = lastHandledCommandIndex;
+                    commandIndex = lastHandledCommandIndex + 1;
 
                     onResultSet = reader.DbDataReader.NextResult();
                 }
+                else
+                {
+                    commandIndex++;
+                }
 
                 if (resultSetMapping.HasFlag(ResultSetMapping.HasOutputParameters))
                 {
@@ -158,7 +162,7 @@ protected override async Task ConsumeAsync(
             bool? onResultSet = null;
             var hasOutputParameters = false;
 
-            for (; commandIndex < ResultSetMappings.Count; commandIndex++)
+            while (commandIndex < ResultSetMappings.Count)
             {
                 var resultSetMapping = ResultSetMappings[commandIndex];
 
@@ -177,10 +181,14 @@ protected override async Task ConsumeAsync(
                         ? lastHandledCommandIndex == commandIndex
                         : lastHandledCommandIndex > commandIndex, "Bad handling of ResultSetMapping and command indexing");
 
-                    commandIndex = lastHandledCommandIndex;
+                    commandIndex = lastHandledCommandIndex + 1;
 
                     onResultSet = await reader.DbDataReader.NextResultAsync(cancellationToken).ConfigureAwait(false);
                 }
+                else
+                {
+                    commandIndex++;
+                }
 
                 if (resultSetMapping.HasFlag(ResultSetMapping.HasOutputParameters))
                 {

src/EFCore.Relational/Update/ModificationCommand.cs

@@ -523,9 +523,12 @@ void HandleColumnModification(IColumnMappingBase columnMapping)
                     && (isKey
                         || storedProcedureParameter is { ForOriginalValue: true }
                         || (property.IsConcurrencyToken && storedProcedureParameter is null));
+
+                // Store-generated properties generally need to be read back (unless we're deleting).
+                // One exception is if the property is mapped to a non-output parameter.
                 var readValue = state != EntityState.Deleted
                     && entry.IsStoreGenerated(property)
-                    && storedProcedureParameter is null or { ForOriginalValue: false };
+                    && (storedProcedureParameter is null || storedProcedureParameter.Direction.HasFlag(ParameterDirection.Output));
 
                 ColumnValuePropagator? columnPropagator = null;
                 sharedTableColumnMap?.TryGetValue(column.Name, out columnPropagator);

src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs

@@ -269,7 +269,13 @@ protected virtual void AddParameters(IReadOnlyModificationCommand modificationCo
         Check.DebugAssert(!modificationCommand.ColumnModifications.Any(m => m.Column is IStoreStoredProcedureReturnValue)
             || modificationCommand.ColumnModifications[0].Column is IStoreStoredProcedureReturnValue,
             "ResultValue column modification in non-first position");
-        foreach (var columnModification in modificationCommand.ColumnModifications)
+
+        var modifications = modificationCommand.StoreStoredProcedure is null
+            ? modificationCommand.ColumnModifications
+            : modificationCommand.ColumnModifications.Where(
+                c => c.Column is IStoreStoredProcedureParameter or IStoreStoredProcedureReturnValue);
+
+        foreach (var columnModification in modifications)
         {
             AddParameter(columnModification);
         }
@@ -288,13 +294,17 @@ protected virtual void AddParameter(IColumnModification columnModification)
             _ => ParameterDirection.Input
         };
 
-        // For in/out parameters, both UseCurrentValueParameter and UseOriginalValueParameter are true, but we only want to add a single
-        // parameter. This will happen below.
-        if (columnModification.UseCurrentValueParameter && direction != ParameterDirection.InputOutput)
+        // For the case where the same modification has both current and original value parameters, and corresponds to an in/out parameter,
+        // we only want to add a single parameter. This will happen below.
+        if (columnModification.UseCurrentValueParameter
+            && !(columnModification.UseOriginalValueParameter && direction == ParameterDirection.InputOutput))
         {
-            AddParameterCore(columnModification.ParameterName, direction == ParameterDirection.Output
-                ? null
-                : columnModification.Value);
+            AddParameterCore(
+                columnModification.ParameterName, columnModification.UseCurrentValue
+                    ? columnModification.Value
+                    : direction == ParameterDirection.InputOutput
+                        ? DBNull.Value
+                        : null);
         }
 
         if (columnModification.UseOriginalValueParameter)
@@ -313,8 +323,6 @@ void AddParameterCore(string name, object? value)
                 columnModification.IsNullable,
                 direction);
 
-            // TODO: As an alternative, don't add output-only parameters to ParameterValues at all.
-            // But that means we can't check values exist for input parameters in RelationalParameterBase.AddDbParameter
             ParameterValues.Add(name, value);
 
             _pendingParameters++;