diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_3_0/RetrustForeignKeyAndCheckConstraints.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_3_0/RetrustForeignKeyAndCheckConstraints.cs index 3039e2ebf1bd..c75682cb2cd6 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_3_0/RetrustForeignKeyAndCheckConstraints.cs +++ b/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; /// public class RetrustForeignKeyAndCheckConstraints : AsyncMigrationBase { + private readonly IUmbracoDatabaseFactory _databaseFactory; + /// /// Initializes a new instance of the class. /// /// The migration context. + [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()) { } + /// + /// Initializes a new instance of the class. + /// + /// The migration context. + /// The database factory used to create separate connections for constraint validation. + public RetrustForeignKeyAndCheckConstraints(IMigrationContext context, IUmbracoDatabaseFactory databaseFactory) + : base(context) => _databaseFactory = databaseFactory; + /// 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(sql); + RetrustResultDto result = db.Single(sql); if (result.Success) {