Address review comments

dotnet · Aug 3, 2022 · 9d706b5 · 9d706b5
1 parent 209bbbe
commit 9d706b5
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 20 deletions.
diff --git a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs
@@ -496,9 +496,7 @@ public virtual DbParameter CreateParameter(
         parameter.Direction = direction;
         parameter.ParameterName = name;
 
-        var isInputParameter = direction is ParameterDirection.Input or ParameterDirection.InputOutput;
-
-        if (isInputParameter)
+        if (direction.HasFlag(ParameterDirection.Input))
         {
             value = NormalizeEnumValue(value);
 
@@ -512,7 +510,10 @@ public virtual DbParameter CreateParameter(
 
         if (nullable.HasValue)
         {
-            Check.DebugAssert(nullable.Value || !isInputParameter || value != null, "Null value in a non-nullable input parameter");
+            Check.DebugAssert(nullable.Value
+                || !direction.HasFlag(ParameterDirection.Input)
+                || value != null,
+                "Null value in a non-nullable input parameter");
 
             parameter.IsNullable = nullable.Value;
         }

diff --git a/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs b/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs
@@ -97,7 +97,7 @@ void HandleOutputParameters()
                 {
                     if (requiresResultPropagation)
                     {
-                        command.PropagateOutputParameters(reader);
+                        command.PropagateOutputParameters(reader.DbCommand.Parameters);
                     }
 
                     // TODO: Rows affected via output parameter
@@ -164,26 +164,32 @@ protected override async Task ConsumeAsync(
                     // TODO: Maybe record if there's any sprocs in the batch, and if not, skip this
                     while (true)
                     {
-                        if (command.StoredProcedure is not null)
+                        if (resultSetMapping.HasFlag(ResultSetMapping.HasOutputParameters))
                         {
-                            command.PropagateOutputParameters(reader);
-                            // TODO: Rows affected via output parameter
+                            HandleOutputParameters();
                         }
 
                         if (commandIndex == lastHandledCommandIndex)
                         {
                             break;
                         }
 
-                        command = ModificationCommands[++commandIndex];
+                        resultSetMapping = CommandResultSet[++commandIndex];
+                        command = ModificationCommands[commandIndex];
+                        requiresResultPropagation = command.RequiresResultPropagation;
                     }
                 }
                 else if (resultSetMapping.HasFlag(ResultSetMapping.HasOutputParameters))
                 {
                     // Handle output parameters for commands which have no result rows. Commands with result rows are handled above.
+                    HandleOutputParameters();
+                }
+
+                void HandleOutputParameters()
+                {
                     if (requiresResultPropagation)
                     {
-                        command.PropagateOutputParameters(reader);
+                        command.PropagateOutputParameters(reader.DbCommand.Parameters);
                     }
 
                     // TODO: Rows affected via output parameter

diff --git a/src/EFCore.Relational/Update/IReadOnlyModificationCommand.cs b/src/EFCore.Relational/Update/IReadOnlyModificationCommand.cs
@@ -69,9 +69,9 @@ public interface IReadOnlyModificationCommand
     public void PropagateResults(RelationalDataReader relationalReader);
 
     /// <summary>
-    ///     Reads output parameters returned from the database in the given <paramref name="relationalReader" /> and propagates them back to
-    ///     into the appropriate <see cref="IColumnModification" /> from which the values can be propagated on to tracked entities.
+    ///     Reads output parameters returned from the database in the given <paramref name="parameterCollection" /> and propagates them back
+    ///     to into the appropriate <see cref="IColumnModification" /> from which the values can be propagated on to tracked entities.
     /// </summary>
-    /// <param name="relationalReader">The relational reader containing the values read from the database.</param>
-    public void PropagateOutputParameters(RelationalDataReader relationalReader);
+    /// <param name="parameterCollection">The parameter collection from which to propagate output values.</param>
+    public void PropagateOutputParameters(DbParameterCollection parameterCollection);
 }
diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs
@@ -500,7 +500,7 @@ public virtual void PropagateResults(RelationalDataReader relationalReader)
     }
 
     /// <inheritdoc />
-    public virtual void PropagateOutputParameters(RelationalDataReader relationalReader)
+    public virtual void PropagateOutputParameters(DbParameterCollection parameterCollection)
     {
         // Note that this call sets the value into a sidecar and will only commit to the actual entity
         // if SaveChanges is successful.
@@ -528,7 +528,7 @@ public virtual void PropagateOutputParameters(RelationalDataReader relationalRea
             // Another option is to put the invariant name in the DbParameterCollection (without the prefix); the prefix isn't needed
             // there AFAIK (but should check this is OK across databases.
             var prefixedName = "@" + columnModification.ParameterName;
-            var dbParameter = relationalReader.DbCommand.Parameters[prefixedName];
+            var dbParameter = parameterCollection[prefixedName];
             columnModification.Value = dbParameter.Value;
         }
     }

diff --git a/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs b/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs
@@ -85,11 +85,10 @@ protected virtual void ValidateNoStoredProcedures(
     {
         foreach (var entityType in model.GetEntityTypes())
         {
-            if (entityType.GetInsertStoredProcedureMappings().Any()
-                || entityType.GetUpdateStoredProcedureMappings().Any()
-                || entityType.GetDeleteStoredProcedureMappings().Any())
+            if (entityType.GetInsertStoredProcedure() is not null
+                || entityType.GetUpdateStoredProcedure() is not null
+                || entityType.GetDeleteStoredProcedure() is not null)
             {
-                // TODO: Make this throw and not warn (like other validations) since updates won't work
                 throw new InvalidOperationException(SqliteStrings.StoredProceduresNotSupported(entityType.DisplayName()));
             }
         }