Improve tests (#41)

SebastianStehle · web-flow · commit 0644b4b068c5 · 2025-05-25T23:40:34.000+02:00
* Fix tests.

* Drop table.

* another fix.

* Fix tests
diff --git a/src/PhenX.EntityFrameworkCore.BulkInsert.Sqlite/SqliteBulkInsertProvider.cs b/src/PhenX.EntityFrameworkCore.BulkInsert.Sqlite/SqliteBulkInsertProvider.cs
@@ -20,10 +20,12 @@ internal class SqliteBulkInsertProvider(ILogger<SqliteBulkInsertProvider>? logge
     /// <inheritdoc />
     protected override string BulkInsertId => "rowid";
 
-    //language=sql
     /// <inheritdoc />
     protected override string AddTableCopyBulkInsertId => "--"; // No need to add an ID column in SQLite
 
+    /// <inheritdoc />
+    protected override string GetTempTableName(string tableName) => $"_temp_bulk_insert_test_entity_{Guid.NewGuid():N}";
+
     /// <inheritdoc />
     protected override BulkInsertOptions CreateDefaultOptions() => new()
     {
@@ -116,6 +118,12 @@ private static DbCommand GetInsertCommand(
         return command;
     }
 
+    /// <inheritdoc />
+    protected override Task DropTempTableAsync(bool sync, DbContext dbContext, string tableName)
+    {
+        return ExecuteAsync(sync, dbContext, $"DROP TABLE IF EXISTS {tableName}", default);
+    }
+
     /// <inheritdoc />
     protected override async Task BulkInsert<T>(
         bool sync,
diff --git a/src/PhenX.EntityFrameworkCore.BulkInsert/BulkInsertProviderBase.cs b/src/PhenX.EntityFrameworkCore.BulkInsert/BulkInsertProviderBase.cs
@@ -1,4 +1,5 @@
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Storage;
@@ -43,14 +44,20 @@ protected override async IAsyncEnumerable<T> BulkInsertReturnEntities<T>(
             }
 
             var tableName = await PerformBulkInsertAsync(sync, context, tableInfo, entities, options, tempTableRequired: true, ctk: ctk);
+            try
+            {
+                var result =
+                    await CopyFromTempTableAsync<T, T>(sync, context, tableInfo, tableName, true, options, onConflict, ctk: ctk)
+                        ?? throw new InvalidOperationException("Copy returns null enumerable.");
 
-            var result =
-                await CopyFromTempTableAsync<T, T>(sync, context, tableInfo, tableName, true, options, onConflict, ctk: ctk)
-                    ?? throw new InvalidOperationException("Copy returns null enumerable.");
-
-            await foreach (var item in result.WithCancellation(ctk))
+                await foreach (var item in result.WithCancellation(ctk))
+                {
+                    yield return item;
+                }
+            }
+            finally
             {
-                yield return item;
+                await PerformDropTempTableAsync(sync, context, tableName);
             }
 
             // Commit the transaction if we own them.
@@ -71,6 +78,11 @@ protected override async Task BulkInsert<T>(
         OnConflictOptions<T>? onConflict,
         CancellationToken ctk) where T : class
     {
+        if (entities.TryGetNonEnumeratedCount(out var count) && count == 0)
+        {
+            throw new InvalidOperationException("No entities to insert.");
+        }
+
         using var activity = Telemetry.ActivitySource.StartActivity("BulkInsert");
         activity?.AddTag("tableName", tableInfo.TableName);
         activity?.AddTag("synchronous", sync);
@@ -86,8 +98,14 @@ protected override async Task BulkInsert<T>(
                 }
 
                 var tableName = await PerformBulkInsertAsync(sync, context, tableInfo, entities, options, tempTableRequired: true, ctk: ctk);
-
-                await CopyFromTempTableAsync<T, T>(sync, context, tableInfo, tableName, false, options, onConflict, ctk);
+                try
+                {
+                    await CopyFromTempTableAsync<T, T>(sync, context, tableInfo, tableName, false, options, onConflict, ctk);
+                }
+                finally
+                {
+                    await PerformDropTempTableAsync(sync, context, tableName);
+                }
             }
             else
             {
@@ -107,7 +125,6 @@ protected override async Task BulkInsert<T>(
             await connection.Close(sync, ctk);
         }
     }
-
     private async Task<string> PerformBulkInsertAsync<T>(
         bool sync,
         DbContext context,
@@ -117,11 +134,6 @@ private async Task<string> PerformBulkInsertAsync<T>(
         bool tempTableRequired,
         CancellationToken ctk) where T : class
     {
-        if (entities.TryGetNonEnumeratedCount(out var count) && count == 0)
-        {
-            throw new InvalidOperationException("No entities to insert.");
-        }
-
         var tableName = tempTableRequired
             ? await CreateTableCopyAsync<T>(sync, context, options, tableInfo, ctk)
             : tableInfo.QuotedTableName;
@@ -208,6 +220,33 @@ protected virtual async Task AddBulkInsertIdColumn<T>(
         return null;
     }
 
+    private async Task PerformDropTempTableAsync(bool sync, DbContext dbContext, string tableName)
+    {
+        try
+        {
+            await DropTempTableAsync(sync, dbContext, tableName);
+        }
+        catch (Exception ex)
+        {
+            // The drop operation is not mandatory, therefore never fail the actual operation.
+            if (logger != null)
+            {
+                Log.DropTemporaryTableFailed(logger, ex);
+            }
+        }
+    }
+
+    /// <summary>
+    /// Drops the temporary table manually if needed.
+    /// </summary>
+    /// <param name="sync">Indicates if the operation is synchronous.</param>
+    /// <param name="dbContext">The context.</param>
+    /// <param name="tableName">The table name.</param>
+    protected virtual Task DropTempTableAsync(bool sync, DbContext dbContext, string tableName)
+    {
+        return Task.CompletedTask;
+    }
+
     protected static async Task ExecuteAsync(
         bool sync,
         DbContext context,
diff --git a/src/PhenX.EntityFrameworkCore.BulkInsert/BulkInsertProviderUntyped.cs b/src/PhenX.EntityFrameworkCore.BulkInsert/BulkInsertProviderUntyped.cs
@@ -36,6 +36,11 @@ public IAsyncEnumerable<T> BulkInsertReturnEntities<T>(
             throw new InvalidOperationException($"Invalid options type: {options.GetType().Name}. Expected: {typeof(TOptions).Name}");
         }
 
+        if (entities.TryGetNonEnumeratedCount(out var count) && count == 0)
+        {
+            throw new InvalidOperationException("No entities to insert.");
+        }
+
         return BulkInsertReturnEntities(sync, context, tableInfo, entities, providerOptions, onConflict, ctk);
     }
 
@@ -62,6 +67,11 @@ public Task BulkInsert<T>(
             throw new InvalidOperationException($"Invalid options type: {options.GetType().Name}. Expected: {typeof(TOptions).Name}");
         }
 
+        if (entities.TryGetNonEnumeratedCount(out var count) && count == 0)
+        {
+            throw new InvalidOperationException("No entities to insert.");
+        }
+
         return BulkInsert(sync, context, tableInfo, entities, providerOptions, onConflict, ctk);
     }
 
diff --git a/src/PhenX.EntityFrameworkCore.BulkInsert/Dialect/SqlDialectBuilder.cs b/src/PhenX.EntityFrameworkCore.BulkInsert/Dialect/SqlDialectBuilder.cs
@@ -84,7 +84,7 @@ public virtual string BuildMoveDataSql<T>(
 
         if (returnedColumns.Count != 0)
         {
-            q.Append("RETURNING ");
+            q.Append(" RETURNING ");
             q.AppendJoin(", ", returnedColumns.Select(p => p.QuotedColumName));
             q.AppendLine();
         }
diff --git a/src/PhenX.EntityFrameworkCore.BulkInsert/Log.cs b/src/PhenX.EntityFrameworkCore.BulkInsert/Log.cs
@@ -21,4 +21,10 @@ internal static partial class Log
         Level = LogLevel.Trace,
         Message = "Insert to table directly")]
     public static partial void UsingDirectInsert(ILogger logger);
+
+    [LoggerMessage(
+        EventId = 1003,
+        Level = LogLevel.Error,
+        Message = "Failed to drop temporary table.")]
+    public static partial void DropTemporaryTableFailed(ILogger logger, Exception exception);
 }
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntity.cs b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntity.cs
@@ -8,7 +8,7 @@ namespace PhenX.EntityFrameworkCore.BulkInsert.Tests.DbContext;
 [PrimaryKey(nameof(Id))]
 [Index(nameof(Name), IsUnique = true)]
 [Table("test_entity")]
-public class TestEntity
+public class TestEntity : TestEntityBase
 {
     public int Id { get; set; }
 
@@ -19,9 +19,6 @@ public class TestEntity
     [Column("some_price")]
     public decimal Price { get; set; }
 
-    [Column("test_run")]
-    public Guid TestRun { get; set; }
-
     [Column("the_identifier")]
     public Guid Identifier { get; set; }
 
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityBase.cs b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityBase.cs
@@ -0,0 +1,9 @@
+using System.ComponentModel.DataAnnotations.Schema;
+
+namespace PhenX.EntityFrameworkCore.BulkInsert.Tests.DbContext;
+
+public abstract class TestEntityBase
+{
+    [Column("test_run")]
+    public Guid TestRun { get; set; }
+}
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityWithConverters.cs b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityWithConverters.cs
@@ -4,7 +4,7 @@
 namespace PhenX.EntityFrameworkCore.BulkInsert.Tests.DbContext;
 
 [Table("test_entity_with_converters")]
-public class TestEntityWithConverters
+public class TestEntityWithConverters : TestEntityBase
 {
     public int Id { get; set; }
 
@@ -14,8 +14,5 @@ public class TestEntityWithConverters
 
     [Column("created_at")]
     public DateTime CreatedAt { get; set; }
-
-    [Column("test_run")]
-    public Guid TestRun { get; set; }
 }
 
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityWithGeo.cs b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityWithGeo.cs
@@ -1,18 +1,15 @@
-﻿using System.ComponentModel.DataAnnotations;
+using System.ComponentModel.DataAnnotations;
 using System.ComponentModel.DataAnnotations.Schema;
 
 using NetTopologySuite.Geometries;
 
 namespace PhenX.EntityFrameworkCore.BulkInsert.Tests.DbContext;
 
 [Table("test_entity_geo")]
-public class TestEntityWithGeo
+public class TestEntityWithGeo : TestEntityBase
 {
     [Key]
     public int Id { get; set; }
 
     public Geometry GeoObject { get; set; } = null!;
-
-    [Column("test_run")]
-    public Guid TestRun { get; set; }
 }
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityWithGuidId.cs b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityWithGuidId.cs
@@ -4,15 +4,12 @@
 namespace PhenX.EntityFrameworkCore.BulkInsert.Tests.DbContext;
 
 [Table("test_entity_guids")]
-public class TestEntityWithGuidId
+public class TestEntityWithGuidId : TestEntityBase
 {
     [Key]
     public Guid Id { get; set; }
 
     [Column("name")]
     [MaxLength(100)]
     public string Name { get; set; } = string.Empty;
-
-    [Column("test_run")]
-    public Guid TestRun { get; set; }
 }
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityWithJson.cs b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestEntityWithJson.cs
@@ -4,13 +4,10 @@
 namespace PhenX.EntityFrameworkCore.BulkInsert.Tests.DbContext;
 
 [Table("test_entity_json")]
-public class TestEntityWithJson
+public class TestEntityWithJson : TestEntityBase
 {
     [Key]
     public int Id { get; set; }
 
     public List<int> Json { get; set; } = [];
-
-    [Column("test_run")]
-    public Guid TestRun { get; set; }
 }
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/PhenX.EntityFrameworkCore.BulkInsert.Tests.csproj b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/PhenX.EntityFrameworkCore.BulkInsert.Tests.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+﻿<Project Sdk="Microsoft.NET.Sdk">
 
     <PropertyGroup>
         <TargetFrameworks>net8.0;net9.0</TargetFrameworks>
@@ -13,8 +13,10 @@
           <PrivateAssets>all</PrivateAssets>
           <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
         </PackageReference>
+        <PackageReference Include="FluentAssertions" Version="7.2.0" />
         <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.13.0" />
         <PackageReference Include="xunit" Version="2.9.3" />
+        <PackageReference Include="Xunit.Combinatorial" Version="1.6.24" />
         <PackageReference Include="xunit.runner.visualstudio" Version="3.0.2">
           <PrivateAssets>all</PrivateAssets>
           <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/TestHelpers.cs b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/TestHelpers.cs
@@ -0,0 +1,72 @@
+using Microsoft.EntityFrameworkCore;
+
+using PhenX.EntityFrameworkCore.BulkInsert.Enums;
+using PhenX.EntityFrameworkCore.BulkInsert.Extensions;
+using PhenX.EntityFrameworkCore.BulkInsert.Options;
+using PhenX.EntityFrameworkCore.BulkInsert.Tests.DbContext;
+
+using Xunit;
+
+namespace PhenX.EntityFrameworkCore.BulkInsert.Tests;
+
+public enum InsertStrategy
+{
+    Insert,
+    InsertReturn,
+    InsertAsync,
+    InsertReturnAsync
+}
+
+public static class TestHelpers
+{
+    public static async Task<List<T>> InsertWithStrategyAsync<T>(
+        this TestDbContextBase dbContext,
+        InsertStrategy strategy,
+        List<T> entities,
+        Action<BulkInsertOptions>? configure = null,
+        OnConflictOptions<T>? onConflict = null)
+        where T : TestEntityBase
+    {
+        Skip.If(strategy is InsertStrategy.InsertReturn or InsertStrategy.InsertReturnAsync && dbContext.IsProvider(ProviderType.MySql));
+
+        var runId = Guid.NewGuid();
+        if (entities.Any(x => x.TestRun == default))
+        {
+            foreach (var entity in entities)
+            {
+                if (entity.TestRun == default)
+                {
+                    entity.TestRun = runId;
+                }
+            }
+        }
+        else if (entities.Count > 0)
+        {
+            runId = entities[0].TestRun;
+        }
+
+        var actualConfigure = configure ?? (_ => { });
+        try
+        {
+            switch (strategy)
+            {
+                case InsertStrategy.InsertReturn:
+                    return dbContext.ExecuteBulkInsertReturnEntities(entities, actualConfigure, onConflict);
+                case InsertStrategy.InsertReturnAsync:
+                    return await dbContext.ExecuteBulkInsertReturnEntitiesAsync(entities, actualConfigure, onConflict);
+                case InsertStrategy.Insert:
+                    dbContext.ExecuteBulkInsert(entities, actualConfigure, onConflict);
+                    return dbContext.Set<T>().Where(x => x.TestRun == runId).ToList();
+                case InsertStrategy.InsertAsync:
+                    await dbContext.ExecuteBulkInsertAsync(entities, actualConfigure, onConflict);
+                    return await dbContext.Set<T>().Where(x => x.TestRun == runId).ToListAsync();
+                default:
+                    throw new NotImplementedException();
+            }
+        }
+        finally
+        {
+            dbContext.ChangeTracker.Clear();
+        }
+    }
+}
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/Tests/Basic/BasicTestsBase.cs b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/Tests/Basic/BasicTestsBase.cs
diff --git a/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/Tests/Geo/GeoTestsBase.cs b/tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/Tests/Geo/GeoTestsBase.cs

Original file line number	Diff line number	Diff line change
`@@ -36,6 +36,11 @@ public IAsyncEnumerable<T> BulkInsertReturnEntities<T>(`
`36`	`36`	`throw new InvalidOperationException($"Invalid options type: {options.GetType().Name}. Expected: {typeof(TOptions).Name}");`
`37`	`37`	`}`
`38`	`38`
	`39`	`+ if (entities.TryGetNonEnumeratedCount(out var count) && count == 0)`
	`40`	`+ {`
	`41`	`+ throw new InvalidOperationException("No entities to insert.");`
	`42`	`+ }`
	`43`	`+`
`39`	`44`	`return BulkInsertReturnEntities(sync, context, tableInfo, entities, providerOptions, onConflict, ctk);`
`40`	`45`	`}`
`41`	`46`
`@@ -62,6 +67,11 @@ public Task BulkInsert<T>(`
`62`	`67`	`throw new InvalidOperationException($"Invalid options type: {options.GetType().Name}. Expected: {typeof(TOptions).Name}");`
`63`	`68`	`}`
`64`	`69`
	`70`	`+ if (entities.TryGetNonEnumeratedCount(out var count) && count == 0)`
	`71`	`+ {`
	`72`	`+ throw new InvalidOperationException("No entities to insert.");`
	`73`	`+ }`
	`74`	`+`
`65`	`75`	`return BulkInsert(sync, context, tableInfo, entities, providerOptions, onConflict, ctk);`
`66`	`76`	`}`
`67`	`77`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ public virtual string BuildMoveDataSql<T>(`
`84`	`84`
`85`	`85`	`if (returnedColumns.Count != 0)`
`86`	`86`	`{`
`87`		`- q.Append("RETURNING ");`
	`87`	`+ q.Append(" RETURNING ");`
`88`	`88`	`q.AppendJoin(", ", returnedColumns.Select(p => p.QuotedColumName));`
`89`	`89`	`q.AppendLine();`
`90`	`90`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,7 @@`
`4`	`4`	`namespace PhenX.EntityFrameworkCore.BulkInsert.Tests.DbContext;`
`5`	`5`
`6`	`6`	`[Table("test_entity_with_converters")]`
`7`		`-public class TestEntityWithConverters`
	`7`	`+public class TestEntityWithConverters : TestEntityBase`
`8`	`8`	`{`
`9`	`9`	`public int Id { get; set; }`
`10`	`10`
`@@ -14,8 +14,5 @@ public class TestEntityWithConverters`
`14`	`14`
`15`	`15`	`[Column("created_at")]`
`16`	`16`	`public DateTime CreatedAt { get; set; }`
`17`		`-`
`18`		`- [Column("test_run")]`
`19`		`- public Guid TestRun { get; set; }`
`20`	`17`	`}`
`21`	`18`