Migrations: Fix RetrustForeignKeyAndCheckConstraints failing when data violates a constraint

src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_3_0/RetrustForeignKeyAndCheckConstraints.cs

@@ -1,6 +1,8 @@
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using NPoco;
-using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.DependencyInjection;
+using Umbraco.Cms.Infrastructure.Persistence;
 
 namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_17_3_0;
 
@@ -12,15 +14,28 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_17_3_0;
 /// </summary>
 public class RetrustForeignKeyAndCheckConstraints : AsyncMigrationBase
 {
+    private readonly IUmbracoDatabaseFactory _databaseFactory;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="RetrustForeignKeyAndCheckConstraints"/> class.
     /// </summary>
     /// <param name="context">The migration context.</param>
+    [Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 18.")]
     public RetrustForeignKeyAndCheckConstraints(IMigrationContext context)
-        : base(context)
+        : this(
+            context,
+            StaticServiceProvider.Instance.GetRequiredService<IUmbracoDatabaseFactory>())
     {
     }
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="RetrustForeignKeyAndCheckConstraints"/> class.
+    /// </summary>
+    /// <param name="context">The migration context.</param>
+    /// <param name="databaseFactory">The database factory used to create separate connections for constraint validation.</param>
+    public RetrustForeignKeyAndCheckConstraints(IMigrationContext context, IUmbracoDatabaseFactory databaseFactory)
+        : base(context) => _databaseFactory = databaseFactory;
+
     /// <inheritdoc />
     protected override Task MigrateAsync()
     {
@@ -67,17 +82,21 @@ FROM sys.check_constraints cc
 
         Logger.LogInformation("Found {Count} untrusted constraint(s) to re-trust.", untrustedConstraints.Count);
 
-        // Ensure we have a long command timeout, in case the migration targets a huge table.
-        EnsureLongCommandTimeout(Database);
-
+        // ALTER TABLE ... WITH CHECK CHECK CONSTRAINT is executed on a separate database connection
+        // to isolate failures from the migration scope's transaction. When constraint validation fails
+        // (e.g. orphaned FK rows), the error can zombie the .NET SqlTransaction even when caught by
+        // T-SQL TRY...CATCH, because the transaction state change propagates through the TDS (Tabular
+        // Data Stream) protocol layer that underlies SQL Server client-server communication.
+        // Using a separate connection (which has no explicit transaction) avoids this entirely —
+        // TRY...CATCH works correctly and no SqlException is thrown to the .NET layer.
         var retrusted = 0;
         var failed = 0;
 
+        using IUmbracoDatabase db = _databaseFactory.CreateDatabase();
+        EnsureLongCommandTimeout(db);
+
         foreach (UntrustedConstraintDto constraint in untrustedConstraints)
         {
-            // Use T-SQL TRY...CATCH to handle errors at the SQL level. This prevents a constraint
-            // validation failure from dooming the .NET SqlTransaction, which would cause all
-            // subsequent operations to fail with "This SqlTransaction has completed".
             // Leading semicolon prevents NPoco's auto-select from prepending
             // "SELECT ... FROM []" based on the empty [TableName("")] attribute.
             var sql = $@";
@@ -89,7 +108,7 @@ BEGIN CATCH
     SELECT CAST(0 AS BIT) AS Success, ERROR_MESSAGE() AS ErrorMessage;
 END CATCH";
 
-            RetrustResultDto result = Database.Single<RetrustResultDto>(sql);
+            RetrustResultDto result = db.Single<RetrustResultDto>(sql);
 
             if (result.Success)
             {