[PM-28555] Add idempotent sproc to create My Items collections (#6801)

* Add sproc to create multiple default collections. 
  SqlBulkCopy implementation is overkill for most cases.
  This provides a lighter weight sproc implementation for smaller
  data sets.
* DRY up collection arrangement
* DRY up tests because bulk and non-bulk share same behavior
* use EF native AddRange instead of bulk insert, because
  we expect smaller data sizes on self-host

test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUsers/AutomaticallyConfirmUsersCommandTests.cs

@@ -10,7 +10,6 @@ using Bit.Core.AdminConsole.Utilities.v2;
 using Bit.Core.AdminConsole.Utilities.v2.Validation;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
-using Bit.Core.Models.Data;
 using Bit.Core.Platform.Push;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
@@ -204,14 +203,10 @@ public class AutomaticallyConfirmUsersCommandTests
 
         await sutProvider.GetDependency<ICollectionRepository>()
             .Received(1)
-            .CreateAsync(
-                Arg.Is<Collection>(c =>
-                    c.OrganizationId == organization.Id &&
-                    c.Name == defaultCollectionName &&
-                    c.Type == CollectionType.DefaultUserCollection),
-                Arg.Is<IEnumerable<CollectionAccessSelection>>(groups => groups == null),
-                Arg.Is<IEnumerable<CollectionAccessSelection>>(access =>
-                    access.FirstOrDefault(x => x.Id == organizationUser.Id && x.Manage) != null));
+            .CreateDefaultCollectionsAsync(
+                organization.Id,
+                Arg.Is<IEnumerable<Guid>>(ids => ids.Single() == organizationUser.Id),
+                defaultCollectionName);
     }
 
     [Theory]
@@ -253,9 +248,7 @@ public class AutomaticallyConfirmUsersCommandTests
 
         await sutProvider.GetDependency<ICollectionRepository>()
             .DidNotReceive()
-            .CreateAsync(Arg.Any<Collection>(),
-                Arg.Any<IEnumerable<CollectionAccessSelection>>(),
-                Arg.Any<IEnumerable<CollectionAccessSelection>>());
+            .CreateDefaultCollectionsAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
     }
 
     [Theory]
@@ -291,9 +284,7 @@ public class AutomaticallyConfirmUsersCommandTests
 
         var collectionException = new Exception("Collection creation failed");
         sutProvider.GetDependency<ICollectionRepository>()
-            .CreateAsync(Arg.Any<Collection>(),
-                Arg.Any<IEnumerable<CollectionAccessSelection>>(),
-                Arg.Any<IEnumerable<CollectionAccessSelection>>())
+            .CreateDefaultCollectionsAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>())
             .ThrowsAsync(collectionException);
 
         // Act

test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs

@@ -13,7 +13,6 @@ using Bit.Core.Billing.Enums;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
 using Bit.Core.Exceptions;
-using Bit.Core.Models.Data;
 using Bit.Core.Models.Data.Organizations.OrganizationUsers;
 using Bit.Core.Platform.Push;
 using Bit.Core.Repositories;
@@ -493,15 +492,10 @@ public class ConfirmOrganizationUserCommandTests
 
         await sutProvider.GetDependency<ICollectionRepository>()
             .Received(1)
-            .CreateAsync(
-                Arg.Is<Collection>(c =>
-                    c.Name == collectionName &&
-                    c.OrganizationId == organization.Id &&
-                    c.Type == CollectionType.DefaultUserCollection),
-                Arg.Any<IEnumerable<CollectionAccessSelection>>(),
-                Arg.Is<IEnumerable<CollectionAccessSelection>>(cu =>
-                    cu.Single().Id == orgUser.Id &&
-                    cu.Single().Manage));
+            .CreateDefaultCollectionsAsync(
+                organization.Id,
+                Arg.Is<IEnumerable<Guid>>(ids => ids.Single() == orgUser.Id),
+                collectionName);
     }
 
     [Theory, BitAutoData]
@@ -522,7 +516,7 @@ public class ConfirmOrganizationUserCommandTests
 
         await sutProvider.GetDependency<ICollectionRepository>()
             .DidNotReceive()
-            .UpsertDefaultCollectionsAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
+            .CreateDefaultCollectionsAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
     }
 
     [Theory, BitAutoData]
@@ -539,24 +533,15 @@ public class ConfirmOrganizationUserCommandTests
         sutProvider.GetDependency<IOrganizationUserRepository>().GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser });
         sutProvider.GetDependency<IUserRepository>().GetManyAsync(default).ReturnsForAnyArgs(new[] { user });
 
-        var policyDetails = new PolicyDetails
-        {
-            OrganizationId = org.Id,
-            OrganizationUserId = orgUser.Id,
-            IsProvider = false,
-            OrganizationUserStatus = orgUser.Status,
-            OrganizationUserType = orgUser.Type,
-            PolicyType = PolicyType.OrganizationDataOwnership
-        };
         sutProvider.GetDependency<IPolicyRequirementQuery>()
             .GetAsync<OrganizationDataOwnershipPolicyRequirement>(orgUser.UserId!.Value)
-            .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Disabled, [policyDetails]));
+            .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Disabled, []));
 
         await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, collectionName);
 
         await sutProvider.GetDependency<ICollectionRepository>()
             .DidNotReceive()
-            .UpsertDefaultCollectionsAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
+            .CreateDefaultCollectionsAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
     }
 
     [Theory, BitAutoData]

test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/OrganizationDataOwnershipPolicyValidatorTests.cs

@@ -38,7 +38,7 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Assert
         await sutProvider.GetDependency<ICollectionRepository>()
             .DidNotReceive()
-            .UpsertDefaultCollectionsAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
+            .CreateDefaultCollectionsBulkAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
     }
 
     [Theory, BitAutoData]
@@ -60,7 +60,7 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Assert
         await sutProvider.GetDependency<ICollectionRepository>()
             .DidNotReceive()
-            .UpsertDefaultCollectionsAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
+            .CreateDefaultCollectionsBulkAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
     }
 
     [Theory, BitAutoData]
@@ -86,7 +86,7 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Assert
         await collectionRepository
             .DidNotReceive()
-            .UpsertDefaultCollectionsAsync(
+            .CreateDefaultCollectionsBulkAsync(
                 Arg.Any<Guid>(),
                 Arg.Any<IEnumerable<Guid>>(),
                 Arg.Any<string>());
@@ -172,10 +172,10 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Act
         await sut.ExecuteSideEffectsAsync(policyRequest, postUpdatedPolicy, previousPolicyState);
 
-        // Assert
+        // Assert - Should call with all user IDs (repository does internal filtering)
         await collectionRepository
             .Received(1)
-            .UpsertDefaultCollectionsAsync(
+            .CreateDefaultCollectionsBulkAsync(
                 policyUpdate.OrganizationId,
                 Arg.Is<IEnumerable<Guid>>(ids => ids.Count() == 3),
                 _defaultUserCollectionName);
@@ -210,7 +210,7 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Assert
         await sutProvider.GetDependency<ICollectionRepository>()
             .DidNotReceive()
-            .UpsertDefaultCollectionsAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
+            .CreateDefaultCollectionsBulkAsync(Arg.Any<Guid>(), Arg.Any<IEnumerable<Guid>>(), Arg.Any<string>());
     }
 
     private static IPolicyRepository ArrangePolicyRepository(IEnumerable<OrganizationPolicyDetails> policyDetails)
@@ -251,7 +251,7 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Assert
         await sutProvider.GetDependency<ICollectionRepository>()
             .DidNotReceiveWithAnyArgs()
-            .UpsertDefaultCollectionsAsync(default, default, default);
+            .CreateDefaultCollectionsBulkAsync(default, default, default);
     }
 
     [Theory, BitAutoData]
@@ -273,7 +273,7 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Assert
         await sutProvider.GetDependency<ICollectionRepository>()
             .DidNotReceiveWithAnyArgs()
-            .UpsertDefaultCollectionsAsync(default, default, default);
+            .CreateDefaultCollectionsBulkAsync(default, default, default);
     }
 
     [Theory, BitAutoData]
@@ -299,7 +299,7 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Assert
         await collectionRepository
             .DidNotReceiveWithAnyArgs()
-            .UpsertDefaultCollectionsAsync(
+            .CreateDefaultCollectionsBulkAsync(
                 default,
                 default,
                 default);
@@ -336,10 +336,10 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Act
         await sut.ExecutePostUpsertSideEffectAsync(policyRequest, postUpdatedPolicy, previousPolicyState);
 
-        // Assert
+        // Assert - Should call with all user IDs (repository does internal filtering)
         await collectionRepository
             .Received(1)
-            .UpsertDefaultCollectionsAsync(
+            .CreateDefaultCollectionsBulkAsync(
                 policyUpdate.OrganizationId,
                 Arg.Is<IEnumerable<Guid>>(ids => ids.Count() == 3),
                 _defaultUserCollectionName);
@@ -367,6 +367,6 @@ public class OrganizationDataOwnershipPolicyValidatorTests
         // Assert
         await sutProvider.GetDependency<ICollectionRepository>()
             .DidNotReceiveWithAnyArgs()
-            .UpsertDefaultCollectionsAsync(default, default, default);
+            .CreateDefaultCollectionsBulkAsync(default, default, default);
     }
 }

test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsBulkTests.cs

@@ -0,0 +1,53 @@
+using Bit.Core.Repositories;
+using Xunit;
+
+namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories.CollectionRepository;
+
+
+public class CreateDefaultCollectionsBulkAsyncTests
+{
+    [Theory, DatabaseData]
+    public async Task CreateDefaultCollectionsBulkAsync_CreatesDefaultCollections_Success(
+        IOrganizationRepository organizationRepository,
+        IUserRepository userRepository,
+        IOrganizationUserRepository organizationUserRepository,
+        ICollectionRepository collectionRepository)
+    {
+        await CreateDefaultCollectionsSharedTests.CreatesDefaultCollections_Success(
+            collectionRepository.CreateDefaultCollectionsBulkAsync,
+            organizationRepository,
+            userRepository,
+            organizationUserRepository,
+            collectionRepository);
+    }
+
+    [Theory, DatabaseData]
+    public async Task CreateDefaultCollectionsBulkAsync_CreatesForNewUsersOnly_AndIgnoresExistingUsers(
+        IOrganizationRepository organizationRepository,
+        IUserRepository userRepository,
+        IOrganizationUserRepository organizationUserRepository,
+        ICollectionRepository collectionRepository)
+    {
+        await CreateDefaultCollectionsSharedTests.CreatesForNewUsersOnly_AndIgnoresExistingUsers(
+            collectionRepository.CreateDefaultCollectionsBulkAsync,
+            organizationRepository,
+            userRepository,
+            organizationUserRepository,
+            collectionRepository);
+    }
+
+    [Theory, DatabaseData]
+    public async Task CreateDefaultCollectionsBulkAsync_IgnoresAllExistingUsers(
+        IOrganizationRepository organizationRepository,
+        IUserRepository userRepository,
+        IOrganizationUserRepository organizationUserRepository,
+        ICollectionRepository collectionRepository)
+    {
+        await CreateDefaultCollectionsSharedTests.IgnoresAllExistingUsers(
+            collectionRepository.CreateDefaultCollectionsBulkAsync,
+            organizationRepository,
+            userRepository,
+            organizationUserRepository,
+            collectionRepository);
+    }
+}

test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsSharedTests.cs

@@ -6,10 +6,14 @@ using Xunit;
 
 namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories.CollectionRepository;
 
-public class UpsertDefaultCollectionsTests
+/// <summary>
+/// Shared tests for CreateDefaultCollections methods - both bulk and non-bulk implementations,
+/// as they share the same behavior. Both test suites call the tests in this class.
+/// </summary>
+public static class CreateDefaultCollectionsSharedTests
 {
-    [Theory, DatabaseData]
-    public async Task UpsertDefaultCollectionsAsync_ShouldCreateDefaultCollection_WhenUsersDoNotHaveDefaultCollection(
+    public static async Task CreatesDefaultCollections_Success(
+        Func<Guid, IEnumerable<Guid>, string, Task> createDefaultCollectionsFunc,
         IOrganizationRepository organizationRepository,
         IUserRepository userRepository,
         IOrganizationUserRepository organizationUserRepository,
@@ -21,14 +25,13 @@ public class UpsertDefaultCollectionsTests
         var resultOrganizationUsers = await Task.WhenAll(
             CreateUserForOrgAsync(userRepository, organizationUserRepository, organization),
             CreateUserForOrgAsync(userRepository, organizationUserRepository, organization)
-            );
+        );
 
-
-        var affectedOrgUserIds = resultOrganizationUsers.Select(organizationUser => organizationUser.Id);
+        var affectedOrgUserIds = resultOrganizationUsers.Select(organizationUser => organizationUser.Id).ToList();
         var defaultCollectionName = $"default-name-{organization.Id}";
 
         // Act
-        await collectionRepository.UpsertDefaultCollectionsAsync(organization.Id, affectedOrgUserIds, defaultCollectionName);
+        await createDefaultCollectionsFunc(organization.Id, affectedOrgUserIds, defaultCollectionName);
 
         // Assert
         await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, resultOrganizationUsers, organization.Id);
@@ -36,8 +39,8 @@ public class UpsertDefaultCollectionsTests
         await CleanupAsync(organizationRepository, userRepository, organization, resultOrganizationUsers);
     }
 
-    [Theory, DatabaseData]
-    public async Task UpsertDefaultCollectionsAsync_ShouldUpsertCreateDefaultCollection_ForUsersWithAndWithoutDefaultCollectionsExist(
+    public static async Task CreatesForNewUsersOnly_AndIgnoresExistingUsers(
+        Func<Guid, IEnumerable<Guid>, string, Task> createDefaultCollectionsFunc,
         IOrganizationRepository organizationRepository,
         IUserRepository userRepository,
         IOrganizationUserRepository organizationUserRepository,
@@ -51,31 +54,30 @@ public class UpsertDefaultCollectionsTests
             CreateUserForOrgAsync(userRepository, organizationUserRepository, organization)
         );
 
-        var arrangedOrgUserIds = arrangedOrganizationUsers.Select(organizationUser => organizationUser.Id);
+        var arrangedOrgUserIds = arrangedOrganizationUsers.Select(organizationUser => organizationUser.Id).ToList();
         var defaultCollectionName = $"default-name-{organization.Id}";
 
+        await CreateUsersWithExistingDefaultCollectionsAsync(createDefaultCollectionsFunc, collectionRepository, organization.Id, arrangedOrgUserIds, defaultCollectionName, arrangedOrganizationUsers);
 
-        await CreateUsersWithExistingDefaultCollectionsAsync(collectionRepository, organization.Id, arrangedOrgUserIds, defaultCollectionName, arrangedOrganizationUsers);
-
-        var newOrganizationUsers = new List<OrganizationUser>()
+        var newOrganizationUsers = new List<OrganizationUser>
         {
             await CreateUserForOrgAsync(userRepository, organizationUserRepository, organization)
         };
 
-        var affectedOrgUsers = newOrganizationUsers.Concat(arrangedOrganizationUsers);
-        var affectedOrgUserIds = affectedOrgUsers.Select(organizationUser => organizationUser.Id);
+        var affectedOrgUsers = newOrganizationUsers.Concat(arrangedOrganizationUsers).ToList();
+        var affectedOrgUserIds = affectedOrgUsers.Select(organizationUser => organizationUser.Id).ToList();
 
         // Act
-        await collectionRepository.UpsertDefaultCollectionsAsync(organization.Id, affectedOrgUserIds, defaultCollectionName);
+        await createDefaultCollectionsFunc(organization.Id, affectedOrgUserIds, defaultCollectionName);
 
         // Assert
-        await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, arrangedOrganizationUsers, organization.Id);
+        await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, affectedOrgUsers, organization.Id);
 
         await CleanupAsync(organizationRepository, userRepository, organization, affectedOrgUsers);
     }
 
-    [Theory, DatabaseData]
-    public async Task UpsertDefaultCollectionsAsync_ShouldNotCreateDefaultCollection_WhenUsersAlreadyHaveOne(
+    public static async Task IgnoresAllExistingUsers(
+        Func<Guid, IEnumerable<Guid>, string, Task> createDefaultCollectionsFunc,
         IOrganizationRepository organizationRepository,
         IUserRepository userRepository,
         IOrganizationUserRepository organizationUserRepository,
@@ -89,26 +91,29 @@ public class UpsertDefaultCollectionsTests
             CreateUserForOrgAsync(userRepository, organizationUserRepository, organization)
         );
 
-        var affectedOrgUserIds = resultOrganizationUsers.Select(organizationUser => organizationUser.Id);
+        var affectedOrgUserIds = resultOrganizationUsers.Select(organizationUser => organizationUser.Id).ToList();
         var defaultCollectionName = $"default-name-{organization.Id}";
 
+        await CreateUsersWithExistingDefaultCollectionsAsync(createDefaultCollectionsFunc, collectionRepository, organization.Id, affectedOrgUserIds, defaultCollectionName, resultOrganizationUsers);
 
-        await CreateUsersWithExistingDefaultCollectionsAsync(collectionRepository, organization.Id, affectedOrgUserIds, defaultCollectionName, resultOrganizationUsers);
+        // Act - Try to create again, should silently filter and not create duplicates
+        await createDefaultCollectionsFunc(organization.Id, affectedOrgUserIds, defaultCollectionName);
 
-        // Act
-        await collectionRepository.UpsertDefaultCollectionsAsync(organization.Id, affectedOrgUserIds, defaultCollectionName);
-
-        // Assert
+        // Assert - Original collections should remain unchanged, still only one per user
         await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, resultOrganizationUsers, organization.Id);
 
         await CleanupAsync(organizationRepository, userRepository, organization, resultOrganizationUsers);
     }
 
-    private static async Task CreateUsersWithExistingDefaultCollectionsAsync(ICollectionRepository collectionRepository,
-        Guid organizationId, IEnumerable<Guid> affectedOrgUserIds, string defaultCollectionName,
+    private static async Task CreateUsersWithExistingDefaultCollectionsAsync(
+        Func<Guid, IEnumerable<Guid>, string, Task> createDefaultCollectionsFunc,
+        ICollectionRepository collectionRepository,
+        Guid organizationId,
+        IEnumerable<Guid> affectedOrgUserIds,
+        string defaultCollectionName,
         OrganizationUser[] resultOrganizationUsers)
     {
-        await collectionRepository.UpsertDefaultCollectionsAsync(organizationId, affectedOrgUserIds, defaultCollectionName);
+        await createDefaultCollectionsFunc(organizationId, affectedOrgUserIds, defaultCollectionName);
 
         await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, resultOrganizationUsers, organizationId);
     }
@@ -131,7 +136,6 @@ public class UpsertDefaultCollectionsTests
     private static async Task<OrganizationUser> CreateUserForOrgAsync(IUserRepository userRepository,
         IOrganizationUserRepository organizationUserRepository, Organization organization)
     {
-
         var user = await userRepository.CreateTestUserAsync();
         var orgUser = await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user);
 

test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsTests.cs

@@ -0,0 +1,52 @@
+using Bit.Core.Repositories;
+using Xunit;
+
+namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories.CollectionRepository;
+
+public class CreateDefaultCollectionsAsyncTests
+{
+    [Theory, DatabaseData]
+    public async Task CreateDefaultCollectionsAsync_CreatesDefaultCollections_Success(
+        IOrganizationRepository organizationRepository,
+        IUserRepository userRepository,
+        IOrganizationUserRepository organizationUserRepository,
+        ICollectionRepository collectionRepository)
+    {
+        await CreateDefaultCollectionsSharedTests.CreatesDefaultCollections_Success(
+            collectionRepository.CreateDefaultCollectionsAsync,
+            organizationRepository,
+            userRepository,
+            organizationUserRepository,
+            collectionRepository);
+    }
+
+    [Theory, DatabaseData]
+    public async Task CreateDefaultCollectionsAsync_CreatesForNewUsersOnly_AndIgnoresExistingUsers(
+        IOrganizationRepository organizationRepository,
+        IUserRepository userRepository,
+        IOrganizationUserRepository organizationUserRepository,
+        ICollectionRepository collectionRepository)
+    {
+        await CreateDefaultCollectionsSharedTests.CreatesForNewUsersOnly_AndIgnoresExistingUsers(
+            collectionRepository.CreateDefaultCollectionsAsync,
+            organizationRepository,
+            userRepository,
+            organizationUserRepository,
+            collectionRepository);
+    }
+
+    [Theory, DatabaseData]
+    public async Task CreateDefaultCollectionsAsync_IgnoresAllExistingUsers(
+        IOrganizationRepository organizationRepository,
+        IUserRepository userRepository,
+        IOrganizationUserRepository organizationUserRepository,
+        ICollectionRepository collectionRepository)
+    {
+        await CreateDefaultCollectionsSharedTests.IgnoresAllExistingUsers(
+            collectionRepository.CreateDefaultCollectionsAsync,
+            organizationRepository,
+            userRepository,
+            organizationUserRepository,
+            collectionRepository);
+    }
+}