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

…ties

Closes dotnet#28704

src/EFCore.Relational/Update/ModificationCommand.cs

@@ -549,14 +549,14 @@ void HandleColumnModification(IColumnMappingBase columnMapping)
                         || storedProcedureParameter is { ForOriginalValue: true }
                         || (property.IsConcurrencyToken && storedProcedureParameter is null));
                 var readValue = state != EntityState.Deleted
-                    && entry.IsStoreGenerated(property)
+                    && (entry.IsStoreGenerated(property) || storedProcedureParameter?.Direction.HasFlag(ParameterDirection.Output) == true)
                     && storedProcedureParameter is null or { ForOriginalValue: false };
 
                 ColumnValuePropagator? columnPropagator = null;
                 sharedTableColumnMap?.TryGetValue(column.Name, out columnPropagator);
 
                 var writeValue = false;
-                if (!readValue)
+                if (!readValue || storedProcedureParameter is { Direction: ParameterDirection.InputOutput})
                 {
                     if (adding)
                     {

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,16 @@ 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)
+        // If in/out parameters, stored-generated concurrency tokens have both original (the value to be written as the condition) and
+        // current (the new generated value, to be read back). For this case, we want only a single parameter - that's done below.
+        // For other cases of in/out parameters, there's no original value and we just add it here.
+        if (columnModification.UseCurrentValueParameter
+            && !(direction == ParameterDirection.InputOutput && columnModification.UseOriginalValueParameter))
         {
-            AddParameterCore(columnModification.ParameterName, direction == ParameterDirection.Output
-                ? null
-                : columnModification.Value);
+            AddParameterCore(
+                columnModification.ParameterName, direction == ParameterDirection.Output
+                    ? null
+                    : columnModification.Value);
         }
 
         if (columnModification.UseOriginalValueParameter)

test/EFCore.Relational.Specification.Tests/TestModels/StoredProcedureUpdateModel/StoredProcedureUpdateContext.cs

@@ -49,8 +49,8 @@ public DbSet<EntityWithAdditionalProperty> WithUserManagedConcurrencyToken
     public DbSet<Entity> WithOriginalAndCurrentValueOnNonConcurrencyToken
         => Set<Entity>(nameof(WithOriginalAndCurrentValueOnNonConcurrencyToken));
 
-    public DbSet<Entity> WithInputOutputParameterOnNonConcurrencyToken
-        => Set<Entity>(nameof(WithInputOutputParameterOnNonConcurrencyToken));
+    public DbSet<Entity> WithInputOutputParameter
+        => Set<Entity>(nameof(WithInputOutputParameter));
 
     public DbSet<TphParent> TphParent { get; set; }
     public DbSet<TphChild1> TphChild { get; set; }

test/EFCore.Relational.Specification.Tests/Update/StoredProcedureUpdateFixtureBase.cs

@@ -151,15 +151,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                     .HasOriginalValueParameter(w => w.Name, pb => pb.HasName("NameOriginal")));
 
         modelBuilder.SharedTypeEntity<Entity>(
-            nameof(StoredProcedureUpdateContext.WithInputOutputParameterOnNonConcurrencyToken),
+            nameof(StoredProcedureUpdateContext.WithInputOutputParameter),
             b =>
             {
-                b.Property(w => w.Name).ValueGeneratedOnAddOrUpdate();
+                b.Property(w => w.Name).ValueGeneratedOnAdd();
 
-                b.UpdateUsingStoredProcedure(
-                    nameof(StoredProcedureUpdateContext.WithInputOutputParameterOnNonConcurrencyToken) + "_Update",
+                b.InsertUsingStoredProcedure(
+                    nameof(StoredProcedureUpdateContext.WithInputOutputParameter) + "_Insert",
                     spb => spb
-                        .HasParameter(w => w.Id)
+                        .HasParameter(w => w.Id, pb => pb.IsOutput())
                         .HasParameter(w => w.Name, pb => pb.IsInputOutput()));
             });
 
@@ -275,7 +275,7 @@ public virtual void CleanData()
         context.WithStoreGeneratedConcurrencyTokenAsTwoParameters.RemoveRange(context.WithStoreGeneratedConcurrencyTokenAsTwoParameters);
         context.WithUserManagedConcurrencyToken.RemoveRange(context.WithUserManagedConcurrencyToken);
         context.WithOriginalAndCurrentValueOnNonConcurrencyToken.RemoveRange(context.WithOriginalAndCurrentValueOnNonConcurrencyToken);
-        context.WithInputOutputParameterOnNonConcurrencyToken.RemoveRange(context.WithInputOutputParameterOnNonConcurrencyToken);
+        context.WithInputOutputParameter.RemoveRange(context.WithInputOutputParameter);
         context.TphParent.RemoveRange(context.TphParent);
         context.TphChild.RemoveRange(context.TphChild);
         context.TptParent.RemoveRange(context.TptParent);

test/EFCore.Relational.Specification.Tests/Update/StoredProcedureUpdateTestBase.cs

@@ -454,27 +454,20 @@ public virtual async Task Original_and_current_value_on_non_concurrency_token(bo
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual async Task Input_output_parameter_on_non_concurrency_token(bool async)
+    public virtual async Task Input_output_parameter(bool async)
     {
         await using var context = CreateContext();
 
-        var entity = new Entity { Name = "Initial", };
-        context.WithInputOutputParameterOnNonConcurrencyToken.Add(entity);
-        await context.SaveChangesAsync();
-
-        entity.Name = "Updated";
-
-        ClearLog();
-
+        var entity = new Entity { Name = "Initial" };
+        context.WithInputOutputParameter.Add(entity);
         await SaveChanges(context, async);
 
-        // TODO: This (and below) should be UpdatedWithSuffix. Reference issue tracking this.
-        Assert.Equal("Updated", entity.Name);
+        Assert.Equal("InitialWithSuffix", entity.Name);
 
         using (Fixture.TestSqlLoggerFactory.SuspendRecordingEvents())
         {
-            Assert.Equal(
-                "Updated", (await context.WithInputOutputParameterOnNonConcurrencyToken.SingleAsync(w => w.Id == entity.Id)).Name);
+            Assert.Same(
+                entity, await context.WithInputOutputParameter.SingleAsync(w => w.Id == entity.Id && w.Name == "InitialWithSuffix"));
         }
     }
 

test/EFCore.SqlServer.FunctionalTests/Update/StoredProcedureUpdateSqlServerTest.cs

@@ -353,11 +353,16 @@ public override async Task Tpc(bool async)
 EXEC [TpcChild_Insert] @p0 OUTPUT, @p1, @p2;");
     }
 
-    public override async Task Input_output_parameter_on_non_concurrency_token(bool async)
+    public override async Task Input_output_parameter(bool async)
     {
-        await base.Input_output_parameter_on_non_concurrency_token(async);
+        await base.Input_output_parameter(async);
 
-        AssertSql();
+        AssertSql(
+            @"@p0='1' (Direction = Output)
+@p1='InitialWithSuffix' (Size = 4000) (Direction = InputOutput)
+
+SET NOCOUNT ON;
+EXEC [WithInputOutputParameter_Insert] @p0 OUTPUT, @p1 OUTPUT;");
     }
 
     [ConditionalFact]
@@ -380,7 +385,7 @@ public override void CleanData()
 
         private const string CleanDataSql = @"
 -- Regular tables without foreign keys
-TRUNCATE TABLE [WithInputOutputParameterOnNonConcurrencyToken];
+TRUNCATE TABLE [WithInputOutputParameter];
 TRUNCATE TABLE [WithOriginalAndCurrentValueOnNonConcurrencyToken];
 TRUNCATE TABLE [WithOutputParameter];
 TRUNCATE TABLE [WithOutputParameterAndResultColumn];
@@ -520,10 +525,11 @@ IF @NameCurrent <> @NameOriginal
 
 GO
 
-CREATE PROCEDURE WithInputOutputParameterOnNonConcurrencyToken_Update(@Id int, @Name varchar(max) OUT)
+CREATE PROCEDURE WithInputOutputParameter_Insert(@Id int OUT, @Name varchar(max) OUT)
 AS BEGIN
     SET @Name = @Name + 'WithSuffix';
-    UPDATE [WithInputOutputParameterOnNonConcurrencyToken] SET [Name] = @Name WHERE [Id] = @Id;
+    INSERT INTO [WithInputOutputParameter] ([Name]) VALUES (@Name);
+    SET @Id = SCOPE_IDENTITY();
 END;
 
 GO