From 4e0a981b43e118440deeab01ad494f89b9a7e48e Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Wed, 3 Jul 2024 11:45:44 +1000 Subject: [PATCH] [AC-2809] Remove unused FlexibleCollections feature flag from Cipher Repository (#4282) Remove FlexibleCollections feature flag logic for repository methods: * CiphersController.GetByIdAsync * CipherRepository.DeleteAsync * CipherRepository.MoveAsync * RestoreAsync * SoftDeleteAsync This feature flag was never turned on and we will update the sprocs directly as required. --- .../Vault/Controllers/CiphersController.cs | 2 +- .../Implementations/EmergencyAccessService.cs | 2 +- .../Vault/Repositories/ICipherRepository.cs | 10 ++--- .../Services/Implementations/CipherService.cs | 8 ++-- src/Events/Controllers/CollectController.cs | 7 +--- .../Vault/Repositories/CipherRepository.cs | 40 +++++-------------- .../Vault/Repositories/CipherRepository.cs | 24 +++++------ .../Controllers/CiphersControllerTests.cs | 10 ++--- .../Vault/Services/CipherServiceTests.cs | 4 +- 9 files changed, 42 insertions(+), 65 deletions(-) diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 6017513aec..2caeb95cc1 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -1261,7 +1261,7 @@ public class CiphersController : Controller private async Task GetByIdAsync(Guid cipherId, Guid userId) { - return await _cipherRepository.GetByIdAsync(cipherId, userId, UseFlexibleCollections); + return await _cipherRepository.GetByIdAsync(cipherId, userId); } private bool UseFlexibleCollectionsV1() diff --git a/src/Core/Auth/Services/Implementations/EmergencyAccessService.cs b/src/Core/Auth/Services/Implementations/EmergencyAccessService.cs index 6936ce3036..db14db7feb 100644 --- a/src/Core/Auth/Services/Implementations/EmergencyAccessService.cs +++ b/src/Core/Auth/Services/Implementations/EmergencyAccessService.cs @@ -411,7 +411,7 @@ public class EmergencyAccessService : IEmergencyAccessService throw new BadRequestException("Emergency Access not valid."); } - var cipher = await _cipherRepository.GetByIdAsync(cipherId, emergencyAccess.GrantorId, UseFlexibleCollections); + var cipher = await _cipherRepository.GetByIdAsync(cipherId, emergencyAccess.GrantorId); return await _cipherService.GetAttachmentDownloadDataAsync(cipher, attachmentId); } diff --git a/src/Core/Vault/Repositories/ICipherRepository.cs b/src/Core/Vault/Repositories/ICipherRepository.cs index 80a1e260d6..630778e84b 100644 --- a/src/Core/Vault/Repositories/ICipherRepository.cs +++ b/src/Core/Vault/Repositories/ICipherRepository.cs @@ -8,7 +8,7 @@ namespace Bit.Core.Vault.Repositories; public interface ICipherRepository : IRepository { - Task GetByIdAsync(Guid id, Guid userId, bool useFlexibleCollections); + Task GetByIdAsync(Guid id, Guid userId); Task GetOrganizationDetailsByIdAsync(Guid id); Task> GetManyOrganizationDetailsByOrganizationIdAsync(Guid organizationId); Task GetCanEditByIdAsync(Guid userId, Guid cipherId); @@ -24,18 +24,18 @@ public interface ICipherRepository : IRepository Task UpdatePartialAsync(Guid id, Guid userId, Guid? folderId, bool favorite); Task UpdateAttachmentAsync(CipherAttachment attachment); Task DeleteAttachmentAsync(Guid cipherId, string attachmentId); - Task DeleteAsync(IEnumerable ids, Guid userId, bool useFlexibleCollections); + Task DeleteAsync(IEnumerable ids, Guid userId); Task DeleteByIdsOrganizationIdAsync(IEnumerable ids, Guid organizationId); - Task MoveAsync(IEnumerable ids, Guid? folderId, Guid userId, bool useFlexibleCollections); + Task MoveAsync(IEnumerable ids, Guid? folderId, Guid userId); Task DeleteByUserIdAsync(Guid userId); Task DeleteByOrganizationIdAsync(Guid organizationId); Task UpdateCiphersAsync(Guid userId, IEnumerable ciphers); Task CreateAsync(IEnumerable ciphers, IEnumerable folders); Task CreateAsync(IEnumerable ciphers, IEnumerable collections, IEnumerable collectionCiphers, IEnumerable collectionUsers); - Task SoftDeleteAsync(IEnumerable ids, Guid userId, bool useFlexibleCollections); + Task SoftDeleteAsync(IEnumerable ids, Guid userId); Task SoftDeleteByIdsOrganizationIdAsync(IEnumerable ids, Guid organizationId); - Task RestoreAsync(IEnumerable ids, Guid userId, bool useFlexibleCollections); + Task RestoreAsync(IEnumerable ids, Guid userId); Task RestoreByIdsOrganizationIdAsync(IEnumerable ids, Guid organizationId); Task DeleteDeletedAsync(DateTime deletedDateBefore); diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index 9feaab9cbb..a643a7c4e0 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -433,7 +433,7 @@ public class CipherService : ICipherService var ciphers = await _cipherRepository.GetManyByUserIdAsync(deletingUserId, useFlexibleCollections: UseFlexibleCollections); deletingCiphers = ciphers.Where(c => cipherIdsSet.Contains(c.Id) && c.Edit).Select(x => (Cipher)x).ToList(); - await _cipherRepository.DeleteAsync(deletingCiphers.Select(c => c.Id), deletingUserId, UseFlexibleCollections); + await _cipherRepository.DeleteAsync(deletingCiphers.Select(c => c.Id), deletingUserId); } var events = deletingCiphers.Select(c => @@ -485,7 +485,7 @@ public class CipherService : ICipherService } } - await _cipherRepository.MoveAsync(cipherIds, destinationFolderId, movingUserId, UseFlexibleCollections); + await _cipherRepository.MoveAsync(cipherIds, destinationFolderId, movingUserId); // push await _pushService.PushSyncCiphersAsync(movingUserId); } @@ -878,7 +878,7 @@ public class CipherService : ICipherService var ciphers = await _cipherRepository.GetManyByUserIdAsync(deletingUserId, useFlexibleCollections: UseFlexibleCollections); deletingCiphers = ciphers.Where(c => cipherIdsSet.Contains(c.Id) && c.Edit).Select(x => (Cipher)x).ToList(); - await _cipherRepository.SoftDeleteAsync(deletingCiphers.Select(c => c.Id), deletingUserId, UseFlexibleCollections); + await _cipherRepository.SoftDeleteAsync(deletingCiphers.Select(c => c.Id), deletingUserId); } var events = deletingCiphers.Select(c => @@ -944,7 +944,7 @@ public class CipherService : ICipherService var ciphers = await _cipherRepository.GetManyByUserIdAsync(restoringUserId, useFlexibleCollections: UseFlexibleCollections); restoringCiphers = ciphers.Where(c => cipherIdsSet.Contains(c.Id) && c.Edit).Select(c => (CipherOrganizationDetails)c).ToList(); - revisionDate = await _cipherRepository.RestoreAsync(restoringCiphers.Select(c => c.Id), restoringUserId, UseFlexibleCollections); + revisionDate = await _cipherRepository.RestoreAsync(restoringCiphers.Select(c => c.Id), restoringUserId); } var events = restoringCiphers.Select(c => diff --git a/src/Events/Controllers/CollectController.cs b/src/Events/Controllers/CollectController.cs index 38781c1854..9e4ff531f6 100644 --- a/src/Events/Controllers/CollectController.cs +++ b/src/Events/Controllers/CollectController.cs @@ -1,5 +1,4 @@ -using Bit.Core; -using Bit.Core.Context; +using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Repositories; using Bit.Core.Services; @@ -73,10 +72,8 @@ public class CollectController : Controller } else { - var useFlexibleCollections = _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections); cipher = await _cipherRepository.GetByIdAsync(eventModel.CipherId.Value, - _currentContext.UserId.Value, - useFlexibleCollections); + _currentContext.UserId.Value); } if (cipher == null) { diff --git a/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs b/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs index 9e767844e4..ca496b4a16 100644 --- a/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs +++ b/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs @@ -24,16 +24,12 @@ public class CipherRepository : Repository, ICipherRepository : base(connectionString, readOnlyConnectionString) { } - public async Task GetByIdAsync(Guid id, Guid userId, bool useFlexibleCollections) + public async Task GetByIdAsync(Guid id, Guid userId) { - var sprocName = useFlexibleCollections - ? $"[{Schema}].[CipherDetails_ReadByIdUserId_V2]" - : $"[{Schema}].[CipherDetails_ReadByIdUserId]"; - using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.QueryAsync( - sprocName, + $"[{Schema}].[CipherDetails_ReadByIdUserId]", new { Id = id, UserId = userId }, commandType: CommandType.StoredProcedure); @@ -249,16 +245,12 @@ public class CipherRepository : Repository, ICipherRepository } } - public async Task DeleteAsync(IEnumerable ids, Guid userId, bool useFlexibleCollections) + public async Task DeleteAsync(IEnumerable ids, Guid userId) { - var sprocName = useFlexibleCollections - ? $"[{Schema}].[Cipher_Delete_V2]" - : $"[{Schema}].[Cipher_Delete]"; - using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.ExecuteAsync( - sprocName, + $"[{Schema}].[Cipher_Delete]", new { Ids = ids.ToGuidIdArrayTVP(), UserId = userId }, commandType: CommandType.StoredProcedure); } @@ -286,16 +278,12 @@ public class CipherRepository : Repository, ICipherRepository } } - public async Task MoveAsync(IEnumerable ids, Guid? folderId, Guid userId, bool useFlexibleCollections) + public async Task MoveAsync(IEnumerable ids, Guid? folderId, Guid userId) { - var sprocName = useFlexibleCollections - ? $"[{Schema}].[Cipher_Move_V2]" - : $"[{Schema}].[Cipher_Move]"; - using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.ExecuteAsync( - sprocName, + $"[{Schema}].[Cipher_Move]", new { Ids = ids.ToGuidIdArrayTVP(), FolderId = folderId, UserId = userId }, commandType: CommandType.StoredProcedure); } @@ -579,31 +567,23 @@ public class CipherRepository : Repository, ICipherRepository } } - public async Task SoftDeleteAsync(IEnumerable ids, Guid userId, bool useFlexibleCollections) + public async Task SoftDeleteAsync(IEnumerable ids, Guid userId) { - var sprocName = useFlexibleCollections - ? $"[{Schema}].[Cipher_SoftDelete_V2]" - : $"[{Schema}].[Cipher_SoftDelete]"; - using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.ExecuteAsync( - sprocName, + $"[{Schema}].[Cipher_SoftDelete]", new { Ids = ids.ToGuidIdArrayTVP(), UserId = userId }, commandType: CommandType.StoredProcedure); } } - public async Task RestoreAsync(IEnumerable ids, Guid userId, bool useFlexibleCollections) + public async Task RestoreAsync(IEnumerable ids, Guid userId) { - var sprocName = useFlexibleCollections - ? $"[{Schema}].[Cipher_Restore_V2]" - : $"[{Schema}].[Cipher_Restore]"; - using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.ExecuteScalarAsync( - sprocName, + $"[{Schema}].[Cipher_Restore]", new { Ids = ids.ToGuidIdArrayTVP(), UserId = userId }, commandType: CommandType.StoredProcedure); diff --git a/src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs b/src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs index 05f660ca4e..dd03d4b7c5 100644 --- a/src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs +++ b/src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs @@ -200,9 +200,9 @@ public class CipherRepository : Repository ids, Guid userId, bool useFlexibleCollections) + public async Task DeleteAsync(IEnumerable ids, Guid userId) { - await ToggleCipherStates(ids, userId, CipherStateAction.HardDelete, useFlexibleCollections); + await ToggleCipherStates(ids, userId, CipherStateAction.HardDelete); } public async Task DeleteAttachmentAsync(Guid cipherId, string attachmentId) @@ -303,12 +303,12 @@ public class CipherRepository : Repository GetByIdAsync(Guid id, Guid userId, bool useFlexibleCollections) + public async Task GetByIdAsync(Guid id, Guid userId) { using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); - var userCipherDetails = new UserCipherDetailsQuery(userId, useFlexibleCollections); + var userCipherDetails = new UserCipherDetailsQuery(userId, false); var data = await userCipherDetails.Run(dbContext).FirstOrDefaultAsync(c => c.Id == id); return data; } @@ -407,13 +407,13 @@ public class CipherRepository : Repository ids, Guid? folderId, Guid userId, bool useFlexibleCollections) + public async Task MoveAsync(IEnumerable ids, Guid? folderId, Guid userId) { using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); var cipherEntities = dbContext.Ciphers.Where(c => ids.Contains(c.Id)); - var userCipherDetails = new UserCipherDetailsQuery(userId, useFlexibleCollections).Run(dbContext); + var userCipherDetails = new UserCipherDetailsQuery(userId, false).Run(dbContext); var idsToMove = from ucd in userCipherDetails join c in cipherEntities on ucd.Id equals c.Id @@ -644,9 +644,9 @@ public class CipherRepository : Repository RestoreAsync(IEnumerable ids, Guid userId, bool useFlexibleCollections) + public async Task RestoreAsync(IEnumerable ids, Guid userId) { - return await ToggleCipherStates(ids, userId, CipherStateAction.Restore, useFlexibleCollections); + return await ToggleCipherStates(ids, userId, CipherStateAction.Restore); } public async Task RestoreByIdsOrganizationIdAsync(IEnumerable ids, Guid organizationId) @@ -674,12 +674,12 @@ public class CipherRepository : Repository ids, Guid userId, bool useFlexibleCollections) + public async Task SoftDeleteAsync(IEnumerable ids, Guid userId) { - await ToggleCipherStates(ids, userId, CipherStateAction.SoftDelete, useFlexibleCollections); + await ToggleCipherStates(ids, userId, CipherStateAction.SoftDelete); } - private async Task ToggleCipherStates(IEnumerable ids, Guid userId, CipherStateAction action, bool useFlexibleCollections) + private async Task ToggleCipherStates(IEnumerable ids, Guid userId, CipherStateAction action) { static bool FilterDeletedDate(CipherStateAction action, CipherDetails ucd) { @@ -694,7 +694,7 @@ public class CipherRepository : Repository ids.Contains(c.Id))).ToListAsync(); var query = from ucd in await (userCipherDetailsQuery.Run(dbContext)).ToListAsync() join c in cipherEntitiesToCheck diff --git a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs index f0eff2c46a..b0452abec1 100644 --- a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs @@ -45,7 +45,7 @@ public class CiphersControllerTests }; sutProvider.GetDependency() - .GetByIdAsync(cipherId, userId, Arg.Any()) + .GetByIdAsync(cipherId, userId) .Returns(Task.FromResult(cipherDetails)); var result = await sutProvider.Sut.PutPartial(cipherId, new CipherPartialRequestModel { Favorite = isFavorite, FolderId = folderId.ToString() }); @@ -60,7 +60,7 @@ public class CiphersControllerTests { sutProvider.GetDependency().GetProperUserId(default).Returns(userId); sutProvider.GetDependency().OrganizationUser(Guid.NewGuid()).Returns(false); - sutProvider.GetDependency().GetByIdAsync(id, userId, true).ReturnsNull(); + sutProvider.GetDependency().GetByIdAsync(id, userId).ReturnsNull(); var requestAction = async () => await sutProvider.Sut.PutCollections_vNext(id, model); @@ -72,7 +72,7 @@ public class CiphersControllerTests { SetupUserAndOrgMocks(id, userId, sutProvider); var cipherDetails = CreateCipherDetailsMock(id, userId); - sutProvider.GetDependency().GetByIdAsync(id, userId, true).ReturnsForAnyArgs(cipherDetails); + sutProvider.GetDependency().GetByIdAsync(id, userId).ReturnsForAnyArgs(cipherDetails); sutProvider.GetDependency().GetManyByUserIdCipherIdAsync(userId, id, Arg.Any()).Returns((ICollection)new List()); var cipherService = sutProvider.GetDependency(); @@ -87,7 +87,7 @@ public class CiphersControllerTests { SetupUserAndOrgMocks(id, userId, sutProvider); var cipherDetails = CreateCipherDetailsMock(id, userId); - sutProvider.GetDependency().GetByIdAsync(id, userId, true).ReturnsForAnyArgs(cipherDetails); + sutProvider.GetDependency().GetByIdAsync(id, userId).ReturnsForAnyArgs(cipherDetails); sutProvider.GetDependency().GetManyByUserIdCipherIdAsync(userId, id, Arg.Any()).Returns((ICollection)new List()); @@ -102,7 +102,7 @@ public class CiphersControllerTests { SetupUserAndOrgMocks(id, userId, sutProvider); var cipherDetails = CreateCipherDetailsMock(id, userId); - sutProvider.GetDependency().GetByIdAsync(id, userId, true).ReturnsForAnyArgs(cipherDetails, [(CipherDetails)null]); + sutProvider.GetDependency().GetByIdAsync(id, userId).ReturnsForAnyArgs(cipherDetails, [(CipherDetails)null]); sutProvider.GetDependency().GetManyByUserIdCipherIdAsync(userId, id, Arg.Any()).Returns((ICollection)new List()); diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index b070eb2e90..fa3d219540 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -735,7 +735,7 @@ public class CipherServiceTests sutProvider.GetDependency().GetManyByUserIdAsync(restoringUserId, useFlexibleCollections: Arg.Any()).Returns(ciphers); var revisionDate = previousRevisionDate + TimeSpan.FromMinutes(1); - sutProvider.GetDependency().RestoreAsync(Arg.Any>(), restoringUserId, Arg.Any()).Returns(revisionDate); + sutProvider.GetDependency().RestoreAsync(Arg.Any>(), restoringUserId).Returns(revisionDate); await sutProvider.Sut.RestoreManyAsync(cipherIds, restoringUserId); @@ -848,7 +848,7 @@ public class CipherServiceTests await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreByIdsOrganizationIdAsync(default, default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreByIdsOrganizationIdAsync(default, default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyByUserIdAsync(default, useFlexibleCollections: default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreAsync(default, default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreAsync(default, default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCipherEventsAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCiphersAsync(default); }