Collection parameters: by default, the collections is serialized imme…

…diately instead of deferred This should be the expected behavior, because I can't image why I would create a parameter, then modify the underlying (in-memory-)collection and in the end execute the query expecting it has the last modifications. And there is a regression in 6.0.2 (dotnet/efcore#27427)
PawelGerr · Feb 10, 2022 · a18899c · a18899c
1 parent 4563034
commit a18899c
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 12 deletions.
diff --git a/...rkCore.SqlServer/EntityFrameworkCore/Infrastructure/SqlServerDbContextOptionsExtension.cs b/...rkCore.SqlServer/EntityFrameworkCore/Infrastructure/SqlServerDbContextOptionsExtension.cs
@@ -103,6 +103,7 @@ public bool AddCustomQuerySqlGeneratorFactory
    private JsonSerializerOptions? _collectionParameterJsonSerializerOptions;
    private bool _addCollectionParameterSupport;
    internal bool ConfigureCollectionParametersForPrimitiveTypes { get; private set; }
+   internal bool UseDeferredCollectionParameterSerialization { get; private set; }
 
    /// <summary>
    /// Changes the implementation of <see cref="IMigrationsSqlGenerator"/> to <see cref="ThinktectureSqlServerMigrationsSqlGenerator"/>.
@@ -158,9 +159,7 @@ public void ApplyServices(IServiceCollection services)
       {
          var jsonSerializerOptions = _collectionParameterJsonSerializerOptions ?? new JsonSerializerOptions();
 
-         services.AddSingleton<ICollectionParameterFactory>(serviceProvider => new SqlServerCollectionParameterFactory(jsonSerializerOptions,
-                                                                                                                       serviceProvider.GetRequiredService<ObjectPool<StringBuilder>>(),
-                                                                                                                       serviceProvider.GetRequiredService<ISqlGenerationHelper>()));
+         services.AddSingleton<ICollectionParameterFactory>(serviceProvider => ActivatorUtilities.CreateInstance<SqlServerCollectionParameterFactory>(serviceProvider, jsonSerializerOptions));
          services.Add<IConventionSetPlugin, SqlServerCollectionParameterConventionSetPlugin>(GetLifetime<IConventionSetPlugin>());
       }
 
@@ -200,11 +199,13 @@ public void Register(Type serviceType, object implementationInstance)
    public SqlServerDbContextOptionsExtension AddCollectionParameterSupport(
       bool addCollectionParameterSupport,
       JsonSerializerOptions? jsonSerializerOptions,
-      bool configureCollectionParametersForPrimitiveTypes)
+      bool configureCollectionParametersForPrimitiveTypes,
+      bool useDeferredSerialization)
    {
       _addCollectionParameterSupport = addCollectionParameterSupport;
       _collectionParameterJsonSerializerOptions = jsonSerializerOptions;
       ConfigureCollectionParametersForPrimitiveTypes = addCollectionParameterSupport && configureCollectionParametersForPrimitiveTypes;
+      UseDeferredCollectionParameterSerialization = addCollectionParameterSupport && useDeferredSerialization;
 
       return this;
    }
@@ -261,6 +262,7 @@ public override int GetServiceProviderHashCode()
          hashCode.Add(_extension.ConfigureTempTablesForPrimitiveTypes);
          hashCode.Add(_extension._addCollectionParameterSupport);
          hashCode.Add(_extension.ConfigureCollectionParametersForPrimitiveTypes);
+         hashCode.Add(_extension.UseDeferredCollectionParameterSerialization);
          hashCode.Add(_extension._collectionParameterJsonSerializerOptions);
          hashCode.Add(_extension.AddTenantDatabaseSupport);
          hashCode.Add(_extension.AddTableHintSupport);
@@ -280,6 +282,7 @@ public override bool ShouldUseSameServiceProvider(DbContextOptionsExtensionInfo
                 && _extension.ConfigureTempTablesForPrimitiveTypes == otherSqlServerInfo._extension.ConfigureTempTablesForPrimitiveTypes
                 && _extension._addCollectionParameterSupport == otherSqlServerInfo._extension._addCollectionParameterSupport
                 && _extension.ConfigureCollectionParametersForPrimitiveTypes == otherSqlServerInfo._extension.ConfigureCollectionParametersForPrimitiveTypes
+                && _extension.UseDeferredCollectionParameterSerialization == otherSqlServerInfo._extension.UseDeferredCollectionParameterSerialization
                 && _extension._collectionParameterJsonSerializerOptions == otherSqlServerInfo._extension._collectionParameterJsonSerializerOptions
                 && _extension.AddTenantDatabaseSupport == otherSqlServerInfo._extension.AddTenantDatabaseSupport
                 && _extension.AddTableHintSupport == otherSqlServerInfo._extension.AddTableHintSupport

diff --git a/...re.SqlServer/EntityFrameworkCore/Infrastructure/SqliteDbContextOptionsExtensionOptions.cs b/...re.SqlServer/EntityFrameworkCore/Infrastructure/SqliteDbContextOptionsExtensionOptions.cs
@@ -16,13 +16,19 @@ public class SqlServerDbContextOptionsExtensionOptions : IBulkOperationsDbContex
    /// </summary>
    public bool ConfigureCollectionParametersForPrimitiveTypes { get; private set; }
 
+   /// <summary>
+   /// Indication whether to use deferred serialization or not.
+   /// </summary>
+   public bool UseDeferredCollectionParameterSerialization { get; private set; }
+
    /// <inheritdoc />
    public void Initialize(IDbContextOptions options)
    {
       var extension = GetExtension(options);
 
       ConfigureTempTablesForPrimitiveTypes = extension.ConfigureTempTablesForPrimitiveTypes;
       ConfigureCollectionParametersForPrimitiveTypes = extension.ConfigureCollectionParametersForPrimitiveTypes;
+      UseDeferredCollectionParameterSerialization = extension.UseDeferredCollectionParameterSerialization;
    }
 
    /// <inheritdoc />
@@ -35,6 +41,9 @@ public void Validate(IDbContextOptions options)
 
       if (extension.ConfigureCollectionParametersForPrimitiveTypes != ConfigureCollectionParametersForPrimitiveTypes)
          throw new InvalidOperationException($"The setting '{nameof(SqlServerDbContextOptionsExtension.ConfigureCollectionParametersForPrimitiveTypes)}' has been changed.");
+
+      if (extension.UseDeferredCollectionParameterSerialization != UseDeferredCollectionParameterSerialization)
+         throw new InvalidOperationException($"The setting '{nameof(SqlServerDbContextOptionsExtension.UseDeferredCollectionParameterSerialization)}' has been changed.");
    }
 
    private static SqlServerDbContextOptionsExtension GetExtension(IDbContextOptions options)

diff --git a/...eworkCore.SqlServer/EntityFrameworkCore/Parameters/SqlServerCollectionParameterFactory.cs b/...eworkCore.SqlServer/EntityFrameworkCore/Parameters/SqlServerCollectionParameterFactory.cs
@@ -9,6 +9,7 @@
 using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
 using Microsoft.Extensions.ObjectPool;
+using Thinktecture.EntityFrameworkCore.Infrastructure;
 using Thinktecture.Internal;
 
 namespace Thinktecture.EntityFrameworkCore.Parameters;
@@ -20,22 +21,26 @@ public class SqlServerCollectionParameterFactory : ICollectionParameterFactory
    private readonly JsonSerializerOptions _jsonSerializerOptions;
    private readonly ObjectPool<StringBuilder> _stringBuilderPool;
    private readonly ISqlGenerationHelper _sqlGenerationHelper;
+   private readonly SqlServerDbContextOptionsExtensionOptions _options;
 
    /// <summary>
    /// Initializes new instance of <see cref="SqlServerCollectionParameterFactory"/>.
    /// </summary>
    /// <param name="jsonSerializerOptions">JSON serialization options.</param>
    /// <param name="stringBuilderPool">String builder pool.</param>
-   /// <param name="sqlGenerationHelper"></param>
+   /// <param name="sqlGenerationHelper">SQL generation helper.</param>
+   /// <param name="options">Options.</param>
    /// <exception cref="ArgumentNullException">If <paramref name="jsonSerializerOptions"/> is <c>null</c>.</exception>
    public SqlServerCollectionParameterFactory(
       JsonSerializerOptions jsonSerializerOptions,
       ObjectPool<StringBuilder> stringBuilderPool,
-      ISqlGenerationHelper sqlGenerationHelper)
+      ISqlGenerationHelper sqlGenerationHelper,
+      SqlServerDbContextOptionsExtensionOptions options)
    {
       _jsonSerializerOptions = jsonSerializerOptions ?? throw new ArgumentNullException(nameof(jsonSerializerOptions));
       _stringBuilderPool = stringBuilderPool ?? throw new ArgumentNullException(nameof(stringBuilderPool));
       _sqlGenerationHelper = sqlGenerationHelper ?? throw new ArgumentNullException(nameof(sqlGenerationHelper));
+      _options = options ?? throw new ArgumentNullException(nameof(options));
       _cache = new ConcurrentDictionary<IEntityType, CollectionParameterInfo>();
    }
 
@@ -73,23 +78,29 @@ public IQueryable<T> CreateComplexQuery<T>(DbContext ctx, IReadOnlyCollection<T>
                             CreateJsonParameter(parameterValue));
    }
 
-   private static SqlParameter CreateJsonParameter(JsonCollectionParameter parameterValue)
+   private object CreateJsonParameter(JsonCollectionParameter parameter)
    {
+      if (!_options.UseDeferredCollectionParameterSerialization)
+         return parameter.ToString(null);
+
       return new SqlParameter
              {
                 DbType = DbType.String,
                 SqlDbType = SqlDbType.NVarChar,
-                Value = parameterValue
+                Value = parameter
              };
    }
 
-   private static SqlParameter CreateTopParameter(JsonCollectionParameter jsonCollection)
+   private object CreateTopParameter(JsonCollectionParameter parameter)
    {
+      if (!_options.UseDeferredCollectionParameterSerialization)
+         return parameter.ToInt64(null);
+
       return new SqlParameter
              {
                 DbType = DbType.Int64,
                 SqlDbType = SqlDbType.BigInt,
-                Value = jsonCollection
+                Value = parameter
              };
    }
 

diff --git a/...re.EntityFrameworkCore.SqlServer/Extensions/SqlServerDbContextOptionsBuilderExtensions.cs b/...re.EntityFrameworkCore.SqlServer/Extensions/SqlServerDbContextOptionsBuilderExtensions.cs
@@ -41,14 +41,21 @@ public static SqlServerDbContextOptionsBuilder AddBulkOperationSupport(
    /// <param name="jsonSerializerOptions">JSON serialization options.</param>
    /// <param name="addCollectionParameterSupport">Indication whether to enable or disable the feature.</param>
    /// <param name="configureCollectionParametersForPrimitiveTypes">Indication whether to configure collection parameters for primitive types.</param>
+   /// <param name="useDeferredSerialization">
+   /// If <c>true</c> then the provided collection will be serialized when the query is executed.
+   /// If <c>false</c> then the collection is going to be serialized when "collection parameter" is created,
+   /// i.e. when calling <see cref="BulkOperationsDbContextExtensions.CreateScalarCollectionParameter{T}"/> pr <see cref="BulkOperationsDbContextExtensions.CreateComplexCollectionParameter{T}"/>.
+   /// Default is <c>false</c>.
+   /// </param>
    /// <returns>Provided <paramref name="sqlServerOptionsBuilder"/>.</returns>
    public static SqlServerDbContextOptionsBuilder AddCollectionParameterSupport(
       this SqlServerDbContextOptionsBuilder sqlServerOptionsBuilder,
       JsonSerializerOptions? jsonSerializerOptions = null,
       bool addCollectionParameterSupport = true,
-      bool configureCollectionParametersForPrimitiveTypes = true)
+      bool configureCollectionParametersForPrimitiveTypes = true,
+      bool useDeferredSerialization = false)
    {
-      return AddOrUpdateExtension(sqlServerOptionsBuilder, extension => extension.AddCollectionParameterSupport(addCollectionParameterSupport, jsonSerializerOptions, configureCollectionParametersForPrimitiveTypes));
+      return AddOrUpdateExtension(sqlServerOptionsBuilder, extension => extension.AddCollectionParameterSupport(addCollectionParameterSupport, jsonSerializerOptions, configureCollectionParametersForPrimitiveTypes, useDeferredSerialization));
    }
 
    /// <summary>

diff --git a/...re.SqlServer.Tests/Extensions/DbContextExtensionsTests/CreateScalarCollectionParameter.cs b/...re.SqlServer.Tests/Extensions/DbContextExtensionsTests/CreateScalarCollectionParameter.cs
@@ -82,4 +82,34 @@ public async Task Should_work_with_contains(bool applyDistinct)
                                               Count = 42
                                            });
    }
+
+   [Fact]
+   public async Task Should_work_GroupBy_and_aggregate()
+   {
+      var testEntity = new TestEntity
+                       {
+                          Id = new Guid("7F8B0E79-2C91-4682-9F61-6FC86B4E5244"),
+                          Name = "Name",
+                          RequiredName = "RequiredName",
+                          Count = 42
+                       };
+      await ArrangeDbContext.AddAsync(testEntity);
+      await ArrangeDbContext.SaveChangesAsync();
+
+      var collectionParameter = ActDbContext.CreateScalarCollectionParameter(new[] { testEntity.Id });
+      var loadedEntities = await ActDbContext.TestEntities
+                                             .Where(e => collectionParameter.Contains(e.Id))
+                                             .GroupBy(e => e.Id)
+                                             .Select(g => new { g.Key, Aggregate = g.Count() })
+                                             .ToListAsync();
+
+      loadedEntities.Should().BeEquivalentTo(new[]
+                                             {
+                                                new
+                                                {
+                                                   Key = new Guid("7F8B0E79-2C91-4682-9F61-6FC86B4E5244"),
+                                                   Aggregate = 1
+                                                }
+                                             });
+   }
 }