From 2fc0257d1dda19a0ff1c28c636a3ee1e9e562fc6 Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Sat, 3 Jan 2026 13:49:38 +1000 Subject: [PATCH] Move transaction up to code layer --- .../Repositories/CollectionRepository.cs | 17 ++- .../Repositories/CollectionRepository.cs | 1 - .../Collection_CreateDefaultCollections.sql | 117 ++++++++---------- ...01_Collection_CreateDefaultCollections.sql | 117 ++++++++---------- 4 files changed, 119 insertions(+), 133 deletions(-) diff --git a/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs b/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs index 36f4f914a3..469747fdbb 100644 --- a/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs @@ -375,6 +375,7 @@ public class CollectionRepository : Repository, ICollectionRep await using var connection = new SqlConnection(ConnectionString); await connection.OpenAsync(); + await using var transaction = connection.BeginTransaction(); try { @@ -386,12 +387,20 @@ public class CollectionRepository : Repository, ICollectionRep DefaultCollectionName = defaultCollectionName, OrganizationUserCollectionIds = organizationUserCollectionIds }, - commandType: CommandType.StoredProcedure); + commandType: CommandType.StoredProcedure, + transaction: transaction); + + await transaction.CommitAsync(); } catch (Exception ex) when (DatabaseExceptionHelpers.IsDuplicateKeyException(ex)) { + await transaction.RollbackAsync(); throw new DuplicateDefaultCollectionException(); } + catch + { + await transaction.RollbackAsync(); + } } public async Task CreateDefaultCollectionsBulkAsync(Guid organizationId, IEnumerable organizationUserIds, string defaultCollectionName) @@ -417,16 +426,16 @@ public class CollectionRepository : Repository, ICollectionRep await BulkResourceCreationService.CreateCollectionsAsync(connection, transaction, collections); await BulkResourceCreationService.CreateCollectionsUsersAsync(connection, transaction, collectionUsers); - transaction.Commit(); + await transaction.CommitAsync(); } catch (Exception ex) when (DatabaseExceptionHelpers.IsDuplicateKeyException(ex)) { - transaction.Rollback(); + await transaction.RollbackAsync(); throw new DuplicateDefaultCollectionException(); } catch { - transaction.Rollback(); + await transaction.RollbackAsync(); throw; } } diff --git a/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs b/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs index 736c1e9775..65bdb68312 100644 --- a/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs @@ -819,7 +819,6 @@ public class CollectionRepository : Repository>(collections)); await dbContext.BulkCopyAsync(Mapper.Map>(collectionUsers)); - await dbContext.SaveChangesAsync(); await transaction.CommitAsync(); } catch (Exception ex) when (DatabaseExceptionHelpers.IsDuplicateKeyException(ex)) diff --git a/src/Sql/dbo/AdminConsole/Stored Procedures/Collection_CreateDefaultCollections.sql b/src/Sql/dbo/AdminConsole/Stored Procedures/Collection_CreateDefaultCollections.sql index b7424dd348..f751b8776e 100644 --- a/src/Sql/dbo/AdminConsole/Stored Procedures/Collection_CreateDefaultCollections.sql +++ b/src/Sql/dbo/AdminConsole/Stored Procedures/Collection_CreateDefaultCollections.sql @@ -1,5 +1,6 @@ -- Creates default user collections for organization users -- Uses semaphore table to prevent duplicate default collections at database level +-- NOTE: this MUST be executed in a single transaction to obtain semaphore protection CREATE PROCEDURE [dbo].[Collection_CreateDefaultCollections] @OrganizationId UNIQUEIDENTIFIER, @DefaultCollectionName VARCHAR(MAX), @@ -10,70 +11,58 @@ BEGIN DECLARE @Now DATETIME2(7) = GETUTCDATE() - BEGIN TRANSACTION; + -- Insert semaphore entries first to obtain the "lock" + -- If this fails due to duplicate key, the entire transaction will be rolled back + INSERT INTO [dbo].[DefaultCollectionSemaphore] + ( + [OrganizationUserId], + [CreationDate] + ) + SELECT + ids.[Id1], -- OrganizationUserId + @Now + FROM + @OrganizationUserCollectionIds ids; - BEGIN TRY - -- Insert semaphore entries first to obtain the "lock" - -- If this fails due to duplicate key, the entire transaction will be rolled back - INSERT INTO [dbo].[DefaultCollectionSemaphore] - ( - [OrganizationUserId], - [CreationDate] - ) - SELECT - ids.[Id1], -- OrganizationUserId - @Now - FROM - @OrganizationUserCollectionIds ids; + -- Insert collections for users who obtained semaphore entries + INSERT INTO [dbo].[Collection] + ( + [Id], + [OrganizationId], + [Name], + [CreationDate], + [RevisionDate], + [Type], + [ExternalId], + [DefaultUserCollectionEmail] + ) + SELECT + ids.[Id2], -- CollectionId + @OrganizationId, + @DefaultCollectionName, + @Now, + @Now, + 1, -- CollectionType.DefaultUserCollection + NULL, + NULL + FROM + @OrganizationUserCollectionIds ids; - -- Insert collections for users who obtained semaphore entries - INSERT INTO [dbo].[Collection] - ( - [Id], - [OrganizationId], - [Name], - [CreationDate], - [RevisionDate], - [Type], - [ExternalId], - [DefaultUserCollectionEmail] - ) - SELECT - ids.[Id2], -- CollectionId - @OrganizationId, - @DefaultCollectionName, - @Now, - @Now, - 1, -- CollectionType.DefaultUserCollection - NULL, - NULL - FROM - @OrganizationUserCollectionIds ids; - - -- Insert collection user mappings - INSERT INTO [dbo].[CollectionUser] - ( - [CollectionId], - [OrganizationUserId], - [ReadOnly], - [HidePasswords], - [Manage] - ) - SELECT - ids.[Id2], -- CollectionId - ids.[Id1], -- OrganizationUserId - 0, -- ReadOnly = false - 0, -- HidePasswords = false - 1 -- Manage = true - FROM - @OrganizationUserCollectionIds ids; - - COMMIT TRANSACTION; - END TRY - BEGIN CATCH - IF @@TRANCOUNT > 0 - ROLLBACK TRANSACTION; - - THROW; - END CATCH + -- Insert collection user mappings + INSERT INTO [dbo].[CollectionUser] + ( + [CollectionId], + [OrganizationUserId], + [ReadOnly], + [HidePasswords], + [Manage] + ) + SELECT + ids.[Id2], -- CollectionId + ids.[Id1], -- OrganizationUserId + 0, -- ReadOnly = false + 0, -- HidePasswords = false + 1 -- Manage = true + FROM + @OrganizationUserCollectionIds ids; END diff --git a/util/Migrator/DbScripts/2025-12-30_01_Collection_CreateDefaultCollections.sql b/util/Migrator/DbScripts/2025-12-30_01_Collection_CreateDefaultCollections.sql index 0d08dcf4f2..a03040c571 100644 --- a/util/Migrator/DbScripts/2025-12-30_01_Collection_CreateDefaultCollections.sql +++ b/util/Migrator/DbScripts/2025-12-30_01_Collection_CreateDefaultCollections.sql @@ -1,5 +1,6 @@ -- Creates default user collections for organization users -- Uses semaphore table to prevent duplicate default collections at database level +-- NOTE: this MUST be executed in a single transaction to obtain semaphore protection CREATE OR ALTER PROCEDURE [dbo].[Collection_CreateDefaultCollections] @OrganizationId UNIQUEIDENTIFIER, @DefaultCollectionName VARCHAR(MAX), @@ -10,71 +11,59 @@ BEGIN DECLARE @Now DATETIME2(7) = GETUTCDATE() - BEGIN TRANSACTION; + -- Insert semaphore entries first to obtain the "lock" + -- If this fails due to duplicate key, the entire transaction will be rolled back + INSERT INTO [dbo].[DefaultCollectionSemaphore] + ( + [OrganizationUserId], + [CreationDate] + ) + SELECT + ids.[Id1], -- OrganizationUserId + @Now + FROM + @OrganizationUserCollectionIds ids; - BEGIN TRY - -- Insert semaphore entries first to obtain the "lock" - -- If this fails due to duplicate key, the entire transaction will be rolled back - INSERT INTO [dbo].[DefaultCollectionSemaphore] - ( - [OrganizationUserId], - [CreationDate] - ) - SELECT - ids.[Id1], -- OrganizationUserId - @Now - FROM - @OrganizationUserCollectionIds ids; + -- Insert collections for users who obtained semaphore entries + INSERT INTO [dbo].[Collection] + ( + [Id], + [OrganizationId], + [Name], + [CreationDate], + [RevisionDate], + [Type], + [ExternalId], + [DefaultUserCollectionEmail] + ) + SELECT + ids.[Id2], -- CollectionId + @OrganizationId, + @DefaultCollectionName, + @Now, + @Now, + 1, -- CollectionType.DefaultUserCollection + NULL, + NULL + FROM + @OrganizationUserCollectionIds ids; - -- Insert collections for users who obtained semaphore entries - INSERT INTO [dbo].[Collection] - ( - [Id], - [OrganizationId], - [Name], - [CreationDate], - [RevisionDate], - [Type], - [ExternalId], - [DefaultUserCollectionEmail] - ) - SELECT - ids.[Id2], -- CollectionId - @OrganizationId, - @DefaultCollectionName, - @Now, - @Now, - 1, -- CollectionType.DefaultUserCollection - NULL, - NULL - FROM - @OrganizationUserCollectionIds ids; - - -- Insert collection user mappings - INSERT INTO [dbo].[CollectionUser] - ( - [CollectionId], - [OrganizationUserId], - [ReadOnly], - [HidePasswords], - [Manage] - ) - SELECT - ids.[Id2], -- CollectionId - ids.[Id1], -- OrganizationUserId - 0, -- ReadOnly = false - 0, -- HidePasswords = false - 1 -- Manage = true - FROM - @OrganizationUserCollectionIds ids; - - COMMIT TRANSACTION; - END TRY - BEGIN CATCH - IF @@TRANCOUNT > 0 - ROLLBACK TRANSACTION; - - THROW; - END CATCH + -- Insert collection user mappings + INSERT INTO [dbo].[CollectionUser] + ( + [CollectionId], + [OrganizationUserId], + [ReadOnly], + [HidePasswords], + [Manage] + ) + SELECT + ids.[Id2], -- CollectionId + ids.[Id1], -- OrganizationUserId + 0, -- ReadOnly = false + 0, -- HidePasswords = false + 1 -- Manage = true + FROM + @OrganizationUserCollectionIds ids; END GO