mirror of
https://github.com/bitwarden/server
synced 2026-02-17 18:09:11 +00:00
First pass at reverting semaphore approach
This commit is contained in:
Thomas Rittson
@@ -1,5 +1,4 @@
|
||||
|
||||
using Bit.Core.AdminConsole.Collections;
|
||||
using Bit.Core.AdminConsole.Entities;
|
||||
using Bit.Core.Entities;
|
||||
using Bit.Core.Enums;
|
||||
@@ -33,13 +32,12 @@ public class CreateDefaultCollectionsBulkTests
|
||||
|
||||
// Assert
|
||||
await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, resultOrganizationUsers, organization.Id);
|
||||
await AssertSempahoresCreatedAsync(collectionRepository, affectedOrgUserIds);
|
||||
|
||||
await CleanupAsync(organizationRepository, userRepository, organization, resultOrganizationUsers);
|
||||
}
|
||||
|
||||
[Theory, DatabaseData]
|
||||
public async Task CreateDefaultCollectionsBulkAsync_CreatesForNewUsersOnly_WhenCallerFiltersExisting(
|
||||
public async Task CreateDefaultCollectionsBulkAsync_CreatesForNewUsersOnly_AutoFiltersExisting(
|
||||
IOrganizationRepository organizationRepository,
|
||||
IUserRepository userRepository,
|
||||
IOrganizationUserRepository organizationUserRepository,
|
||||
@@ -66,20 +64,17 @@ public class CreateDefaultCollectionsBulkTests
|
||||
var affectedOrgUsers = newOrganizationUsers.Concat(arrangedOrganizationUsers);
|
||||
var affectedOrgUserIds = affectedOrgUsers.Select(organizationUser => organizationUser.Id).ToList();
|
||||
|
||||
// Act - Caller filters out existing users (new pattern)
|
||||
var existingSemaphores = await collectionRepository.GetDefaultCollectionSemaphoresAsync(affectedOrgUserIds);
|
||||
var usersNeedingCollections = affectedOrgUserIds.Except(existingSemaphores).ToList();
|
||||
await collectionRepository.CreateDefaultCollectionsBulkAsync(organization.Id, usersNeedingCollections, defaultCollectionName);
|
||||
// Act - Pass all user IDs, method should auto-filter existing users
|
||||
await collectionRepository.CreateDefaultCollectionsBulkAsync(organization.Id, affectedOrgUserIds, defaultCollectionName);
|
||||
|
||||
// Assert - All users now have exactly one collection
|
||||
await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, affectedOrgUsers, organization.Id);
|
||||
await AssertSempahoresCreatedAsync(collectionRepository, affectedOrgUserIds);
|
||||
|
||||
await CleanupAsync(organizationRepository, userRepository, organization, affectedOrgUsers);
|
||||
}
|
||||
|
||||
[Theory, DatabaseData]
|
||||
public async Task CreateDefaultCollectionsBulkAsync_ThrowsException_WhenUsersAlreadyHaveOne(
|
||||
public async Task CreateDefaultCollectionsBulkAsync_DoesNotCreateDuplicates_WhenUsersAlreadyHaveOne(
|
||||
IOrganizationRepository organizationRepository,
|
||||
IUserRepository userRepository,
|
||||
IOrganizationUserRepository organizationUserRepository,
|
||||
@@ -98,19 +93,17 @@ public class CreateDefaultCollectionsBulkTests
|
||||
|
||||
await CreateUsersWithExistingDefaultCollectionsAsync(collectionRepository, organization.Id, affectedOrgUserIds, defaultCollectionName, resultOrganizationUsers);
|
||||
|
||||
// Act - Try to create again, should throw specific duplicate collection exception
|
||||
await Assert.ThrowsAsync<DuplicateDefaultCollectionException>(() =>
|
||||
collectionRepository.CreateDefaultCollectionsBulkAsync(organization.Id, affectedOrgUserIds, defaultCollectionName));
|
||||
// Act - Try to create again, should silently filter and not create duplicates
|
||||
await collectionRepository.CreateDefaultCollectionsBulkAsync(organization.Id, affectedOrgUserIds, defaultCollectionName);
|
||||
|
||||
// Assert - Original collections should remain unchanged
|
||||
// Assert - Original collections should remain unchanged, still only one per user
|
||||
await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, resultOrganizationUsers, organization.Id);
|
||||
await AssertSempahoresCreatedAsync(collectionRepository, affectedOrgUserIds);
|
||||
|
||||
await CleanupAsync(organizationRepository, userRepository, organization, resultOrganizationUsers);
|
||||
}
|
||||
|
||||
[Theory, DatabaseData]
|
||||
public async Task CreateDefaultCollectionsBulkAsync_ThrowsException_WhenDuplicatesNotFiltered(
|
||||
public async Task CreateDefaultCollectionsBulkAsync_AutoFilters_WhenMixedUsersProvided(
|
||||
IOrganizationRepository organizationRepository,
|
||||
IUserRepository userRepository,
|
||||
IOrganizationUserRepository organizationUserRepository,
|
||||
@@ -126,24 +119,24 @@ public class CreateDefaultCollectionsBulkTests
|
||||
// Create collection for existing user
|
||||
await collectionRepository.CreateDefaultCollectionsBulkAsync(organization.Id, [existingUser.Id], defaultCollectionName);
|
||||
|
||||
// Act - Try to create for both without filtering (incorrect usage)
|
||||
await Assert.ThrowsAsync<DuplicateDefaultCollectionException>(() =>
|
||||
collectionRepository.CreateDefaultCollectionsBulkAsync(
|
||||
organization.Id,
|
||||
[existingUser.Id, newUser.Id],
|
||||
defaultCollectionName));
|
||||
// Act - Pass both users, method should auto-filter and only create for new user
|
||||
await collectionRepository.CreateDefaultCollectionsBulkAsync(
|
||||
organization.Id,
|
||||
[existingUser.Id, newUser.Id],
|
||||
defaultCollectionName);
|
||||
|
||||
// Assert - Verify existing user still has collection
|
||||
// Assert - Verify existing user still has exactly one collection
|
||||
var existingUserCollections = await collectionRepository.GetManyByUserIdAsync(existingUser.UserId!.Value);
|
||||
var existingUserDefaultCollection = existingUserCollections
|
||||
.SingleOrDefault(c => c.OrganizationId == organization.Id && c.Type == CollectionType.DefaultUserCollection);
|
||||
Assert.NotNull(existingUserDefaultCollection);
|
||||
var existingUserDefaultCollections = existingUserCollections
|
||||
.Where(c => c.OrganizationId == organization.Id && c.Type == CollectionType.DefaultUserCollection)
|
||||
.ToList();
|
||||
Assert.Single(existingUserDefaultCollections);
|
||||
|
||||
// Verify new user does NOT have collection (transaction rolled back)
|
||||
// Verify new user now has collection (was created)
|
||||
var newUserCollections = await collectionRepository.GetManyByUserIdAsync(newUser.UserId!.Value);
|
||||
var newUserDefaultCollection = newUserCollections
|
||||
.FirstOrDefault(c => c.OrganizationId == organization.Id && c.Type == CollectionType.DefaultUserCollection);
|
||||
Assert.Null(newUserDefaultCollection);
|
||||
.SingleOrDefault(c => c.OrganizationId == organization.Id && c.Type == CollectionType.DefaultUserCollection);
|
||||
Assert.NotNull(newUserDefaultCollection);
|
||||
|
||||
await CleanupAsync(organizationRepository, userRepository, organization, [existingUser, newUser]);
|
||||
}
|
||||
@@ -181,14 +174,6 @@ public class CreateDefaultCollectionsBulkTests
|
||||
return orgUser;
|
||||
}
|
||||
|
||||
private static async Task AssertSempahoresCreatedAsync(ICollectionRepository collectionRepository,
|
||||
IEnumerable<Guid> organizationUserIds)
|
||||
{
|
||||
var organizationUserIdHashSet = organizationUserIds.ToHashSet();
|
||||
var semaphores = await collectionRepository.GetDefaultCollectionSemaphoresAsync(organizationUserIdHashSet);
|
||||
Assert.Equal(organizationUserIdHashSet, semaphores);
|
||||
}
|
||||
|
||||
private static async Task CleanupAsync(IOrganizationRepository organizationRepository,
|
||||
IUserRepository userRepository,
|
||||
Organization organization,
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
using Bit.Core.AdminConsole.Collections;
|
||||
using Bit.Core.Enums;
|
||||
using Bit.Core.Enums;
|
||||
using Bit.Core.Repositories;
|
||||
using Xunit;
|
||||
|
||||
@@ -41,9 +40,6 @@ public class CreateDefaultCollectionsTests
|
||||
Assert.All(defaultCollections, c => Assert.Equal("My Items", c.Item1.Name));
|
||||
Assert.All(defaultCollections, c => Assert.Equal(organization.Id, c.Item1.OrganizationId));
|
||||
|
||||
var semaphores = await collectionRepository.GetDefaultCollectionSemaphoresAsync([orgUser1.Id, orgUser2.Id]);
|
||||
Assert.Equal([orgUser1.Id, orgUser2.Id], semaphores);
|
||||
|
||||
// Verify each user has exactly 1 collection with correct permissions
|
||||
var orgUser1Collection = Assert.Single(defaultCollections,
|
||||
c => c.Item2.Users.FirstOrDefault()?.Id == orgUser1.Id);
|
||||
@@ -71,7 +67,7 @@ public class CreateDefaultCollectionsTests
|
||||
/// Test that calling CreateDefaultCollectionsAsync multiple times does NOT create duplicates
|
||||
/// </summary>
|
||||
[Theory, DatabaseData]
|
||||
public async Task CreateDefaultCollectionsAsync_CalledMultipleTimesForSameOrganizationUser_Throws(
|
||||
public async Task CreateDefaultCollectionsAsync_CalledMultipleTimesForSameOrganizationUser_DoesNotCreateDuplicates(
|
||||
IUserRepository userRepository,
|
||||
IOrganizationRepository organizationRepository,
|
||||
ICollectionRepository collectionRepository,
|
||||
@@ -88,12 +84,11 @@ public class CreateDefaultCollectionsTests
|
||||
[orgUser.Id],
|
||||
"My Items");
|
||||
|
||||
// Second call should throw specific exception and should not create duplicate
|
||||
await Assert.ThrowsAsync<DuplicateDefaultCollectionException>(() =>
|
||||
collectionRepository.CreateDefaultCollectionsAsync(
|
||||
organization.Id,
|
||||
[orgUser.Id],
|
||||
"My Items Duplicate"));
|
||||
// Second call should silently filter and not create duplicate
|
||||
await collectionRepository.CreateDefaultCollectionsAsync(
|
||||
organization.Id,
|
||||
[orgUser.Id],
|
||||
"My Items Duplicate");
|
||||
|
||||
// Assert - Only one collection should exist
|
||||
var collections = await collectionRepository.GetManyByOrganizationIdAsync(organization.Id);
|
||||
@@ -101,9 +96,6 @@ public class CreateDefaultCollectionsTests
|
||||
|
||||
Assert.Single(defaultCollections);
|
||||
|
||||
var semaphores = await collectionRepository.GetDefaultCollectionSemaphoresAsync([orgUser.Id]);
|
||||
Assert.Equal([orgUser.Id], semaphores);
|
||||
|
||||
var access = await collectionRepository.GetManyUsersByIdAsync(defaultCollections.Single().Id);
|
||||
var userAccess = Assert.Single(access);
|
||||
Assert.Equal(orgUser.Id, userAccess.Id);
|
||||
|
||||
@@ -1,72 +0,0 @@
|
||||
using Bit.Core.Repositories;
|
||||
using Xunit;
|
||||
|
||||
namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories.CollectionRepository;
|
||||
|
||||
/// <summary>
|
||||
/// Tests for DefaultCollectionSemaphore table behavior including cascade deletions
|
||||
/// </summary>
|
||||
public class DefaultCollectionSemaphoreTests
|
||||
{
|
||||
[Theory, DatabaseData]
|
||||
public async Task DeleteOrganizationUser_CascadeDeletesSemaphore(
|
||||
IUserRepository userRepository,
|
||||
IOrganizationRepository organizationRepository,
|
||||
ICollectionRepository collectionRepository,
|
||||
IOrganizationUserRepository organizationUserRepository)
|
||||
{
|
||||
// Arrange
|
||||
var user = await userRepository.CreateTestUserAsync();
|
||||
var organization = await organizationRepository.CreateTestOrganizationAsync();
|
||||
var orgUser = await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user);
|
||||
|
||||
await collectionRepository.CreateDefaultCollectionsAsync(
|
||||
organization.Id,
|
||||
[orgUser.Id],
|
||||
"My Items");
|
||||
|
||||
// Verify semaphore exists
|
||||
var semaphoreBefore = await collectionRepository.GetDefaultCollectionSemaphoresAsync([orgUser.Id]);
|
||||
Assert.Single(semaphoreBefore, s => s == orgUser.Id);
|
||||
|
||||
// Act - Delete organization user
|
||||
await organizationUserRepository.DeleteAsync(orgUser);
|
||||
|
||||
// Assert - Semaphore should be cascade deleted
|
||||
var semaphoreAfter = await collectionRepository.GetDefaultCollectionSemaphoresAsync([orgUser.Id]);
|
||||
Assert.Empty(semaphoreAfter);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Test that deleting an Organization cascades through OrganizationUser to DefaultCollectionSemaphore
|
||||
/// Note: Cascade path is Organization -> OrganizationUser -> DefaultCollectionSemaphore (not direct)
|
||||
/// </summary>
|
||||
[Theory, DatabaseData]
|
||||
public async Task DeleteOrganization_CascadeDeletesSemaphore_ThroughOrganizationUser(
|
||||
IUserRepository userRepository,
|
||||
IOrganizationRepository organizationRepository,
|
||||
ICollectionRepository collectionRepository,
|
||||
IOrganizationUserRepository organizationUserRepository)
|
||||
{
|
||||
// Arrange
|
||||
var user = await userRepository.CreateTestUserAsync();
|
||||
var organization = await organizationRepository.CreateTestOrganizationAsync();
|
||||
var orgUser = await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user);
|
||||
|
||||
await collectionRepository.CreateDefaultCollectionsAsync(
|
||||
organization.Id,
|
||||
[orgUser.Id],
|
||||
"My Items");
|
||||
|
||||
// Verify semaphore exists
|
||||
var semaphoreBefore = await collectionRepository.GetDefaultCollectionSemaphoresAsync([orgUser.Id]);
|
||||
Assert.Single(semaphoreBefore, s => s == orgUser.Id);
|
||||
|
||||
// Act - Delete organization (which cascades to OrganizationUser, which cascades to semaphore)
|
||||
await organizationRepository.DeleteAsync(organization);
|
||||
|
||||
// Assert - Semaphore should be cascade deleted via OrganizationUser
|
||||
var semaphoreAfter = await collectionRepository.GetDefaultCollectionSemaphoresAsync([orgUser.Id]);
|
||||
Assert.Empty(semaphoreAfter);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user