From 8f72322ef56bb73966eae4c2a6b1b31c39cb5c78 Mon Sep 17 00:00:00 2001 From: Cy Okeke Date: Wed, 1 Jul 2026 13:37:47 +0100 Subject: [PATCH 1/5] [PM-36965] feat: Add SeederApi scene for migration cohort CSV-export testing Adds MigrationCohortExportScene, the HTTP/API equivalent of the PM-36965 SQL seed script: it creates a migration cohort plus N inert placeholder organizations and their cohort assignments, so the Admin Portal cohort-management table has a populated cohort ready to Export CSV. Organizations and assignments are bulk-inserted via LinqToDB BulkCopy (the same path BulkCommitter uses) to stay fast at scale. Because BulkCopy bypasses the repository CreateAsync tracking hook, the scene records PlayItem rows itself so DELETE /seed/{playId} can tear the seeded organizations down; the empty cohort row is not play-id tracked and is removed by name. Covered by integration tests asserting database persistence, cohort reuse-by-name, and play-id tracking. --- .../MigrationCohortExportSceneCleanupTests.cs | 115 ++++++++++ .../SeedControllerTests.cs | 75 ++++++ .../Scenes/MigrationCohortExportScene.cs | 215 ++++++++++++++++++ 3 files changed, 405 insertions(+) create mode 100644 test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs create mode 100644 util/Seeder/Scenes/MigrationCohortExportScene.cs diff --git a/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs b/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs new file mode 100644 index 000000000000..dd72729b2419 --- /dev/null +++ b/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs @@ -0,0 +1,115 @@ +using System.Text.Json; +using Bit.Core.Services; +using Bit.Seeder.Scenes; +using Bit.SeederApi.Models.Request; +using Duende.IdentityModel.Client; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Xunit; + +namespace Bit.SeederApi.IntegrationTest; + +/// +/// Verifies the play-id cleanup path for . The scene inserts +/// its organizations via BulkCopy, which bypasses the repository tracking decorators, so the scene +/// records the PlayItem rows itself. These tests confirm that DELETE /seed/{playId} can +/// therefore tear the bulk-seeded organizations down. +/// +/// +/// Uses a dedicated factory that enables a real (fixed) -- the shared +/// registers NeverPlayIdServices, under which the +/// scene's play-id recording is a deliberate no-op. +/// +public class MigrationCohortExportSceneCleanupTests + : IClassFixture, IAsyncLifetime +{ + private readonly HttpClient _client; + private readonly PlayIdEnabledSeederApiApplicationFactory _factory; + private const string _username = "username"; + private const string _password = "pass"; + + public MigrationCohortExportSceneCleanupTests(PlayIdEnabledSeederApiApplicationFactory factory) + { + _factory = factory; + factory.ConfigureAuth(_username, _password); + _client = _factory.CreateClient(); + _client.SetBasicAuthentication(_username, _password); + } + + public Task InitializeAsync() => Task.CompletedTask; + + public Task DisposeAsync() + { + _client.Dispose(); + return Task.CompletedTask; + } + + [Fact] + public async Task Seed_WithPlayId_RecordsOnePlayItemPerOrganizationLinkingThemForCleanup() + { + const int orgCount = 4; + var cohortName = $"PM-36965 Cleanup {Guid.NewGuid()}"; + var namePrefix = $"pm36965-{Guid.NewGuid():N}-"; + var playId = _factory.PlayId; + + var seedResponse = await _client.PostAsJsonAsync("/seed", new SeedRequestModel + { + Template = "MigrationCohortExportScene", + Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request + { + CohortName = cohortName, + OrgCount = orgCount, + NamePrefix = namePrefix + }) + }, playId); + seedResponse.EnsureSuccessStatusCode(); + + var db = _factory.GetDatabaseContext(); + + var orgIds = await db.Organizations + .Where(o => o.Name.StartsWith(namePrefix)) + .Select(o => o.Id) + .ToListAsync(); + Assert.Equal(orgCount, orgIds.Count); + + // The scene bulk-inserts a PlayItem per organization (BulkCopy bypasses the repository + // tracking decorator), which is exactly what lets DELETE /seed/{playId} find and remove them. + var trackedOrgIds = await db.PlayItem + .Where(p => p.PlayId == playId && p.OrganizationId != null) + .Select(p => p.OrganizationId!.Value) + .ToListAsync(); + + Assert.Equal(orgCount, trackedOrgIds.Count); + Assert.Equal(orgIds.ToHashSet(), trackedOrgIds.ToHashSet()); + } +} + +/// +/// Factory variant that registers a fixed so play-id tracking is active +/// for the cleanup tests (the base factory registers NeverPlayIdServices). +/// +public class PlayIdEnabledSeederApiApplicationFactory : SeederApiApplicationFactory +{ + public string PlayId { get; } = $"cleanup-{Guid.NewGuid()}"; + + public PlayIdEnabledSeederApiApplicationFactory() + { + _configureTestServices.Add(services => + { + services.RemoveAll(); + services.AddSingleton(new FixedPlayIdService(PlayId)); + }); + } + + private sealed class FixedPlayIdService(string playId) : IPlayIdService + { + public string? PlayId { get; set; } = playId; + + public bool InPlay(out string playId) + { + playId = PlayId ?? string.Empty; + return !string.IsNullOrEmpty(playId); + } + } +} diff --git a/test/SeederApi.IntegrationTest/SeedControllerTests.cs b/test/SeederApi.IntegrationTest/SeedControllerTests.cs index a409741ed1d5..54bd9bd44b27 100644 --- a/test/SeederApi.IntegrationTest/SeedControllerTests.cs +++ b/test/SeederApi.IntegrationTest/SeedControllerTests.cs @@ -1,8 +1,10 @@ using System.Net; +using System.Text.Json; using Bit.Seeder.Scenes; using Bit.SeederApi.Models.Request; using Bit.SeederApi.Models.Response; using Duende.IdentityModel.Client; +using Microsoft.EntityFrameworkCore; using Xunit; namespace Bit.SeederApi.IntegrationTest; @@ -222,6 +224,79 @@ public async Task SeedEndpoint_VerifyResponseContainsMangleMapAndResult() Assert.Contains("result", jsonString, StringComparison.OrdinalIgnoreCase); } + [Fact] + public async Task SeedEndpoint_MigrationCohortExportScene_PersistsCohortOrgsAndAssignmentsToDatabase() + { + const int orgCount = 5; + var cohortName = $"PM-36965 Export Test {Guid.NewGuid()}"; + var namePrefix = $"pm36965-{Guid.NewGuid():N}-"; + + var response = await _client.PostAsJsonAsync("/seed", new SeedRequestModel + { + Template = "MigrationCohortExportScene", + Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request + { + CohortName = cohortName, + OrgCount = orgCount, + NamePrefix = namePrefix + }) + }, Guid.NewGuid().ToString()); + + response.EnsureSuccessStatusCode(); + + // Read the data back straight from the database -- proving the rows actually landed, + // not merely that the API reported success. + var db = _factory.GetDatabaseContext(); + + var cohort = await db.OrganizationPlanMigrationCohorts + .SingleOrDefaultAsync(c => c.Name == cohortName); + Assert.NotNull(cohort); + Assert.True(cohort.IsActive); + + var orgs = await db.Organizations + .Where(o => o.Name.StartsWith(namePrefix)) + .ToListAsync(); + Assert.Equal(orgCount, orgs.Count); + Assert.All(orgs, o => Assert.False(o.Enabled)); // seeded orgs are inert + + var assignments = await db.OrganizationPlanMigrationCohortAssignments + .Where(a => a.CohortId == cohort.Id) + .ToListAsync(); + Assert.Equal(orgCount, assignments.Count); + + // Every seeded org has exactly one assignment to this cohort. + var orgIds = orgs.Select(o => o.Id).ToHashSet(); + Assert.Equal(orgIds, assignments.Select(a => a.OrganizationId).ToHashSet()); + + // This factory registers NeverPlayIdServices, so play-id recording is a no-op: no PlayItem + // rows are written for a seed that carries no active play id. + Assert.Equal(0, await db.PlayItem.CountAsync(p => orgIds.Contains(p.OrganizationId!.Value))); + } + + [Fact] + public async Task SeedEndpoint_MigrationCohortExportScene_ReusesExistingCohortByName() + { + var cohortName = $"PM-36965 Reuse {Guid.NewGuid()}"; + + async Task SeedAsync() => + (await (await _client.PostAsJsonAsync("/seed", new SeedRequestModel + { + Template = "MigrationCohortExportScene", + Arguments = JsonSerializer.SerializeToElement( + new MigrationCohortExportScene.Request { CohortName = cohortName, OrgCount = 2 }) + }, Guid.NewGuid().ToString())) + .EnsureSuccessStatusCode() + .Content.ReadFromJsonAsync()); + + var first = (await SeedAsync()).GetProperty("result"); + var second = (await SeedAsync()).GetProperty("result"); + + // Second run finds the cohort by name and reuses it rather than creating a duplicate. + Assert.True(first.GetProperty("cohortCreated").GetBoolean()); + Assert.False(second.GetProperty("cohortCreated").GetBoolean()); + Assert.Equal(first.GetProperty("cohortId").GetGuid(), second.GetProperty("cohortId").GetGuid()); + } + private class BatchDeleteResponse { public string? Message { get; set; } diff --git a/util/Seeder/Scenes/MigrationCohortExportScene.cs b/util/Seeder/Scenes/MigrationCohortExportScene.cs new file mode 100644 index 000000000000..62a9a1a2746c --- /dev/null +++ b/util/Seeder/Scenes/MigrationCohortExportScene.cs @@ -0,0 +1,215 @@ +using System.ComponentModel.DataAnnotations; +using System.Globalization; +using Bit.Core.Billing.Enums; +using Bit.Core.Billing.Organizations.PlanMigration.Repositories; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Services; +using Bit.Core.Utilities; +using Bit.Infrastructure.EntityFramework.Repositories; +using LinqToDB.EntityFrameworkCore; +using AutoMapper; +using EfCohortAssignment = Bit.Infrastructure.EntityFramework.Billing.Models.OrganizationPlanMigrationCohortAssignment; +using EfOrganization = Bit.Infrastructure.EntityFramework.AdminConsole.Models.Organization; +using EfPlayItem = Bit.Infrastructure.EntityFramework.Models.PlayItem; +using CoreCohort = Bit.Core.Billing.Organizations.PlanMigration.Entities.OrganizationPlanMigrationCohort; +using CoreCohortAssignment = Bit.Core.Billing.Organizations.PlanMigration.Entities.OrganizationPlanMigrationCohortAssignment; +using CoreOrganization = Bit.Core.AdminConsole.Entities.Organization; +using MigrationPath = Bit.Core.Billing.Organizations.PlanMigration.Enums.MigrationPathId; + +namespace Bit.Seeder.Scenes; + +public readonly struct MigrationCohortExportSceneResult +{ + public Guid CohortId { get; init; } + public bool CohortCreated { get; init; } + public int OrganizationsCreated { get; init; } + public int AssignmentsCreated { get; init; } +} + +/// +/// Seeds a migration cohort plus N inert organizations, each assigned to the cohort, so the +/// Admin Portal cohort-management table has a populated cohort ready to "Export CSV". +/// +/// +/// The HTTP/API equivalent of the PM-36965 SQL seed script. The organizations are throwaway +/// placeholders (Disabled, no billing, no users) and exist only because +/// OrganizationPlanMigrationCohortAssignment has a FK to Organization with a UNIQUE +/// constraint on OrganizationId -- every assignment needs its own real organization row. +/// +/// Organizations and assignments are bulk-inserted via LinqToDB BulkCopy (the same path +/// uses) so counts in the tens of thousands stay fast. Each +/// organization's CreationDate is spread across distinct seconds. The export, however, orders +/// on the assignment's (CreationDate, Id), and the assignment's CreationDate +/// has an internal setter unreachable from this assembly, so every assignment shares a near-identical +/// DateTime.UtcNow and the cursor advances on the Id tiebreaker. That is exactly the +/// bulk-loaded case the export's stored procedure is documented to handle, so it remains correct. +/// +/// Cleanup: BulkCopy bypasses the repository CreateAsync hook that normally records +/// play-id tracking rows, so this scene bulk-inserts the rows itself when the +/// request carries an x-play-id. That lets DELETE /seed/{playId} tear the seeded +/// organizations down (their assignments cascade away via the FK). The cohort row is NOT play-id +/// tracked -- DestroySceneCommand only deletes users/organizations -- so an emptied cohort is +/// left behind after a play-id delete; remove it by name if desired +/// (DELETE FROM dbo.OrganizationPlanMigrationCohort WHERE Name = '<CohortName>'). +/// +/// LOCAL / NON-PROD ONLY -- the SeederApi refuses to run in production. +/// +public class MigrationCohortExportScene( + DatabaseContext db, + IMapper mapper, + IPlayIdService playIdService, + IOrganizationPlanMigrationCohortRepository cohortRepository) + : IScene +{ + /// + /// Inert plan applied to seeded placeholder organizations. Arbitrary -- these orgs never + /// behave like live organizations (Enabled = false, no billing, no users). + /// + private const PlanType InertPlanType = PlanType.EnterpriseAnnually; + + /// Anchor for the spread CreationDate, matching the SQL seed script's baseline. + private static readonly DateTime _creationAnchor = new(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); + + public class Request + { + [Required] + public required string CohortName { get; set; } + + [Required] + [Range(1, 100_000)] + public required int OrgCount { get; set; } + + /// + /// Migration path for the cohort. Null produces a Churn-only cohort. Defaults to + /// to mirror the SQL seed script. + /// + public MigrationPath? MigrationPathId { get; set; } = MigrationPath.Enterprise2020AnnualToCurrent; + + /// + /// Distinctive name prefix for the seeded organizations so they can be identified and + /// cleaned up. Must stay short enough that the suffixed name fits Organization.Name. + /// + [MaxLength(40)] + public string NamePrefix { get; set; } = "pm36965-seed-"; + } + + public async Task> SeedAsync(Request request) + { + var (cohort, cohortCreated) = await GetOrCreateCohortAsync(request); + + var organizations = BuildOrganizations(request); + var assignments = BuildAssignments(organizations, cohort.Id); + + // BulkCopy maps Core entities to their EF counterparts (navigation properties are ignored). + db.BulkCopy(organizations.Select(mapper.Map)); + db.BulkCopy(assignments.Select(mapper.Map)); + + // Record play-id tracking rows for the organizations so DELETE /seed/{playId} can clean them + // up. BulkCopy bypasses the repository CreateAsync hook that normally does this, so the scene + // inserts the PlayItem rows directly -- and in bulk, to keep the fast path fast. + RecordOrganizationsForPlayId(organizations); + + return new SceneResult( + result: new MigrationCohortExportSceneResult + { + CohortId = cohort.Id, + CohortCreated = cohortCreated, + OrganizationsCreated = organizations.Count, + AssignmentsCreated = assignments.Count, + }, + mangleMap: []); + } + + private async Task<(CoreCohort Cohort, bool Created)> GetOrCreateCohortAsync(Request request) + { + var existing = await cohortRepository.GetByNameAsync(request.CohortName); + if (existing is not null) + { + return (existing, false); + } + + var cohort = new CoreCohort + { + Name = request.CohortName, + MigrationPathId = request.MigrationPathId, + IsActive = true, + }; + cohort.SetNewId(); + + var created = await cohortRepository.CreateAsync(cohort); + return (created, true); + } + + private static List BuildOrganizations(Request request) + { + var organizations = new List(request.OrgCount); + + for (var n = 1; n <= request.OrgCount; n++) + { + // Spread CreationDate across distinct seconds so the export keyset cursor advances over + // real dates, exactly as the SQL seed script does. + var creationDate = _creationAnchor.AddSeconds(n); + var suffix = n.ToString("D6", CultureInfo.InvariantCulture); + + organizations.Add(new CoreOrganization + { + Id = CoreHelpers.GenerateComb(), + Name = $"{request.NamePrefix}{suffix}", + BillingEmail = $"{request.NamePrefix}{n}@example.com", + Plan = "Enterprise (Annually)", + PlanType = InertPlanType, + Status = OrganizationStatusType.Created, + Enabled = false, // never behaves like a live organization + CreationDate = creationDate, + RevisionDate = creationDate, + }); + } + + return organizations; + } + + private static List BuildAssignments( + IEnumerable organizations, Guid cohortId) + { + var assignments = new List(); + + foreach (var org in organizations) + { + // CreationDate has an internal setter and defaults to DateTime.UtcNow at construction; + // it cannot be set from this assembly. RevisionDate is settable, so align it for tidiness. + var assignment = new CoreCohortAssignment + { + OrganizationId = org.Id, + CohortId = cohortId, + }; + assignment.RevisionDate = assignment.CreationDate; + assignment.SetNewId(); + assignments.Add(assignment); + } + + return assignments; + } + + /// + /// Bulk-inserts a row per organization when the request is part of a play + /// session, mirroring what the repository tracking decorators do on CreateAsync. No-op when + /// no x-play-id is set. Deleting an organization cascade-removes its + /// row, so DELETE /seed/{playId} stays self-consistent. + /// + private void RecordOrganizationsForPlayId(IEnumerable organizations) + { + if (!playIdService.InPlay(out var playId)) + { + return; + } + + var playItems = organizations.Select(org => + { + var playItem = PlayItem.Create(org, playId); + playItem.SetNewId(); // repository normally assigns the COMB id; BulkCopy bypasses it + return playItem; + }); + db.BulkCopy(playItems.Select(mapper.Map)); + } +} From 3623d12dd1e635dc8706b840c98f46184fe216a9 Mon Sep 17 00:00:00 2001 From: Cy Okeke Date: Wed, 1 Jul 2026 14:06:49 +0100 Subject: [PATCH 2/5] Address review: wrap writes in a transaction, enforce OrgCount range, add churn-path and range tests - Wrap cohort + orgs + assignments + PlayItem inserts in one transaction so a mid-sequence failure rolls back instead of leaving partly-seeded state. - Move cohort creation into the transactional BulkCopy path. - Enforce OrgCount range explicitly in SeedAsync (SceneExecutor does not run DataAnnotations on scene requests), returning BadRequest out of range. - Reframe the CreationDate comment around observed batch behavior (rot-hardening). - Add tests: null MigrationPathId => churn cohort, OrgCount range validation (0/-1/100001 rejected, 1 accepted), and assert persisted MigrationPathId. --- .../SeedControllerTests.cs | 72 +++++++++++++++++++ .../Scenes/MigrationCohortExportScene.cs | 65 +++++++++++------ 2 files changed, 116 insertions(+), 21 deletions(-) diff --git a/test/SeederApi.IntegrationTest/SeedControllerTests.cs b/test/SeederApi.IntegrationTest/SeedControllerTests.cs index 54bd9bd44b27..c77c22628887 100644 --- a/test/SeederApi.IntegrationTest/SeedControllerTests.cs +++ b/test/SeederApi.IntegrationTest/SeedControllerTests.cs @@ -1,5 +1,6 @@ using System.Net; using System.Text.Json; +using Bit.Core.Billing.Organizations.PlanMigration.Enums; using Bit.Seeder.Scenes; using Bit.SeederApi.Models.Request; using Bit.SeederApi.Models.Response; @@ -252,6 +253,8 @@ public async Task SeedEndpoint_MigrationCohortExportScene_PersistsCohortOrgsAndA .SingleOrDefaultAsync(c => c.Name == cohortName); Assert.NotNull(cohort); Assert.True(cohort.IsActive); + // Default request => a migration cohort on the default path. + Assert.Equal(MigrationPathId.Enterprise2020AnnualToCurrent, cohort.MigrationPathId); var orgs = await db.Organizations .Where(o => o.Name.StartsWith(namePrefix)) @@ -297,6 +300,75 @@ async Task SeedAsync() => Assert.Equal(first.GetProperty("cohortId").GetGuid(), second.GetProperty("cohortId").GetGuid()); } + [Fact] + public async Task SeedEndpoint_MigrationCohortExportScene_NullMigrationPathId_CreatesChurnOnlyCohort() + { + var cohortName = $"PM-36965 Churn {Guid.NewGuid()}"; + var namePrefix = $"pm36965-{Guid.NewGuid():N}-"; + + var response = await _client.PostAsJsonAsync("/seed", new SeedRequestModel + { + Template = "MigrationCohortExportScene", + Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request + { + CohortName = cohortName, + OrgCount = 2, + NamePrefix = namePrefix, + MigrationPathId = null + }) + }, Guid.NewGuid().ToString()); + + response.EnsureSuccessStatusCode(); + + var db = _factory.GetDatabaseContext(); + var cohort = await db.OrganizationPlanMigrationCohorts.SingleAsync(c => c.Name == cohortName); + + // A null path persists as null: a churn-only cohort, not the default migration path. + Assert.Null(cohort.MigrationPathId); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + [InlineData(100_001)] + public async Task SeedEndpoint_MigrationCohortExportScene_OrgCountOutOfRange_ReturnsBadRequest(int orgCount) + { + var response = await _client.PostAsJsonAsync("/seed", new SeedRequestModel + { + Template = "MigrationCohortExportScene", + Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request + { + CohortName = $"PM-36965 Range {Guid.NewGuid()}", + OrgCount = orgCount + }) + }, Guid.NewGuid().ToString()); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task SeedEndpoint_MigrationCohortExportScene_OrgCountLowerBound_Succeeds() + { + var cohortName = $"PM-36965 Min {Guid.NewGuid()}"; + var namePrefix = $"pm36965-{Guid.NewGuid():N}-"; + + var response = await _client.PostAsJsonAsync("/seed", new SeedRequestModel + { + Template = "MigrationCohortExportScene", + Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request + { + CohortName = cohortName, + OrgCount = 1, + NamePrefix = namePrefix + }) + }, Guid.NewGuid().ToString()); + + response.EnsureSuccessStatusCode(); + + var db = _factory.GetDatabaseContext(); + Assert.Equal(1, await db.Organizations.CountAsync(o => o.Name.StartsWith(namePrefix))); + } + private class BatchDeleteResponse { public string? Message { get; set; } diff --git a/util/Seeder/Scenes/MigrationCohortExportScene.cs b/util/Seeder/Scenes/MigrationCohortExportScene.cs index 62a9a1a2746c..d54595f73d97 100644 --- a/util/Seeder/Scenes/MigrationCohortExportScene.cs +++ b/util/Seeder/Scenes/MigrationCohortExportScene.cs @@ -9,6 +9,7 @@ using Bit.Infrastructure.EntityFramework.Repositories; using LinqToDB.EntityFrameworkCore; using AutoMapper; +using EfCohort = Bit.Infrastructure.EntityFramework.Billing.Models.OrganizationPlanMigrationCohort; using EfCohortAssignment = Bit.Infrastructure.EntityFramework.Billing.Models.OrganizationPlanMigrationCohortAssignment; using EfOrganization = Bit.Infrastructure.EntityFramework.AdminConsole.Models.Organization; using EfPlayItem = Bit.Infrastructure.EntityFramework.Models.PlayItem; @@ -37,13 +38,14 @@ public readonly struct MigrationCohortExportSceneResult /// OrganizationPlanMigrationCohortAssignment has a FK to Organization with a UNIQUE /// constraint on OrganizationId -- every assignment needs its own real organization row. /// -/// Organizations and assignments are bulk-inserted via LinqToDB BulkCopy (the same path -/// uses) so counts in the tens of thousands stay fast. Each -/// organization's CreationDate is spread across distinct seconds. The export, however, orders -/// on the assignment's (CreationDate, Id), and the assignment's CreationDate -/// has an internal setter unreachable from this assembly, so every assignment shares a near-identical -/// DateTime.UtcNow and the cursor advances on the Id tiebreaker. That is exactly the -/// bulk-loaded case the export's stored procedure is documented to handle, so it remains correct. +/// The cohort, organizations, and assignments are bulk-inserted via LinqToDB BulkCopy (the same +/// path uses) so counts in the tens of thousands stay fast, all +/// inside a single transaction so a mid-sequence failure leaves the database untouched rather than +/// partly seeded. Each organization's CreationDate is spread across distinct seconds. The +/// export, however, orders on the assignment's (CreationDate, Id), and assignments +/// created in one bulk batch share a near-identical CreationDate, so the cursor advances on the +/// Id tiebreaker. That is exactly the bulk-loaded case the export's stored procedure is +/// documented to handle, so it remains correct. /// /// Cleanup: BulkCopy bypasses the repository CreateAsync hook that normally records /// play-id tracking rows, so this scene bulk-inserts the rows itself when the @@ -71,13 +73,19 @@ public class MigrationCohortExportScene( /// Anchor for the spread CreationDate, matching the SQL seed script's baseline. private static readonly DateTime _creationAnchor = new(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); + private const int MinOrgCount = 1; + private const int MaxOrgCount = 100_000; + public class Request { [Required] public required string CohortName { get; set; } + // The scene enforces this range explicitly in SeedAsync -- the SeederApi executes scenes by + // deserializing arguments and calling SeedAsync directly, so DataAnnotations here are not run + // by the framework. The attribute documents the contract and applies if validation is wired up. [Required] - [Range(1, 100_000)] + [Range(MinOrgCount, MaxOrgCount)] public required int OrgCount { get; set; } /// @@ -96,12 +104,32 @@ public class Request public async Task> SeedAsync(Request request) { - var (cohort, cohortCreated) = await GetOrCreateCohortAsync(request); + if (request.OrgCount is < MinOrgCount or > MaxOrgCount) + { + throw new ArgumentOutOfRangeException( + nameof(request), request.OrgCount, + $"{nameof(request.OrgCount)} must be between {MinOrgCount} and {MaxOrgCount}."); + } + + var existingCohort = await cohortRepository.GetByNameAsync(request.CohortName); + var cohortCreated = existingCohort is null; + var cohort = existingCohort ?? BuildCohort(request); var organizations = BuildOrganizations(request); var assignments = BuildAssignments(organizations, cohort.Id); + // Wrap every write in one transaction so the scene is all-or-nothing: a mid-sequence failure + // (e.g. a timeout on a large assignment insert) rolls back the cohort and organizations rather + // than leaving orphaned rows or organizations without their play-id tracking. LinqToDB's + // BulkCopy honors the EF transaction opened here (see UserRepository for the same pattern). + await using var transaction = await db.Database.BeginTransactionAsync(); + // BulkCopy maps Core entities to their EF counterparts (navigation properties are ignored). + if (cohortCreated) + { + db.BulkCopy([mapper.Map(cohort)]); + } + db.BulkCopy(organizations.Select(mapper.Map)); db.BulkCopy(assignments.Select(mapper.Map)); @@ -110,6 +138,8 @@ public async Task> SeedAsync(Reque // inserts the PlayItem rows directly -- and in bulk, to keep the fast path fast. RecordOrganizationsForPlayId(organizations); + await transaction.CommitAsync(); + return new SceneResult( result: new MigrationCohortExportSceneResult { @@ -121,14 +151,8 @@ public async Task> SeedAsync(Reque mangleMap: []); } - private async Task<(CoreCohort Cohort, bool Created)> GetOrCreateCohortAsync(Request request) + private static CoreCohort BuildCohort(Request request) { - var existing = await cohortRepository.GetByNameAsync(request.CohortName); - if (existing is not null) - { - return (existing, false); - } - var cohort = new CoreCohort { Name = request.CohortName, @@ -136,9 +160,7 @@ public async Task> SeedAsync(Reque IsActive = true, }; cohort.SetNewId(); - - var created = await cohortRepository.CreateAsync(cohort); - return (created, true); + return cohort; } private static List BuildOrganizations(Request request) @@ -176,8 +198,9 @@ private static List BuildAssignments( foreach (var org in organizations) { - // CreationDate has an internal setter and defaults to DateTime.UtcNow at construction; - // it cannot be set from this assembly. RevisionDate is settable, so align it for tidiness. + // CreationDate defaults to DateTime.UtcNow at construction and is not settable here, so + // assignments in one bulk batch share a near-identical value. RevisionDate is settable, so + // align it for tidiness. var assignment = new CoreCohortAssignment { OrganizationId = org.Id, From b7d3f69f748a7dc1a7cf3b5282143a3590b5ac34 Mon Sep 17 00:00:00 2001 From: Cy Okeke Date: Wed, 1 Jul 2026 14:27:17 +0100 Subject: [PATCH 3/5] Fix formatting: add UTF-8 BOM, order imports, drop unused using (dotnet format) --- .../MigrationCohortExportSceneCleanupTests.cs | 3 +-- util/Seeder/Scenes/MigrationCohortExportScene.cs | 10 +++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs b/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs index dd72729b2419..b2abcd752731 100644 --- a/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs +++ b/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs @@ -1,10 +1,9 @@ -using System.Text.Json; +using System.Text.Json; using Bit.Core.Services; using Bit.Seeder.Scenes; using Bit.SeederApi.Models.Request; using Duende.IdentityModel.Client; using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Xunit; diff --git a/util/Seeder/Scenes/MigrationCohortExportScene.cs b/util/Seeder/Scenes/MigrationCohortExportScene.cs index d54595f73d97..4562347f4cb4 100644 --- a/util/Seeder/Scenes/MigrationCohortExportScene.cs +++ b/util/Seeder/Scenes/MigrationCohortExportScene.cs @@ -1,5 +1,6 @@ -using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations; using System.Globalization; +using AutoMapper; using Bit.Core.Billing.Enums; using Bit.Core.Billing.Organizations.PlanMigration.Repositories; using Bit.Core.Entities; @@ -8,14 +9,13 @@ using Bit.Core.Utilities; using Bit.Infrastructure.EntityFramework.Repositories; using LinqToDB.EntityFrameworkCore; -using AutoMapper; +using CoreCohort = Bit.Core.Billing.Organizations.PlanMigration.Entities.OrganizationPlanMigrationCohort; +using CoreCohortAssignment = Bit.Core.Billing.Organizations.PlanMigration.Entities.OrganizationPlanMigrationCohortAssignment; +using CoreOrganization = Bit.Core.AdminConsole.Entities.Organization; using EfCohort = Bit.Infrastructure.EntityFramework.Billing.Models.OrganizationPlanMigrationCohort; using EfCohortAssignment = Bit.Infrastructure.EntityFramework.Billing.Models.OrganizationPlanMigrationCohortAssignment; using EfOrganization = Bit.Infrastructure.EntityFramework.AdminConsole.Models.Organization; using EfPlayItem = Bit.Infrastructure.EntityFramework.Models.PlayItem; -using CoreCohort = Bit.Core.Billing.Organizations.PlanMigration.Entities.OrganizationPlanMigrationCohort; -using CoreCohortAssignment = Bit.Core.Billing.Organizations.PlanMigration.Entities.OrganizationPlanMigrationCohortAssignment; -using CoreOrganization = Bit.Core.AdminConsole.Entities.Organization; using MigrationPath = Bit.Core.Billing.Organizations.PlanMigration.Enums.MigrationPathId; namespace Bit.Seeder.Scenes; From bbec0549d54b3b94fa57903f62039f0fbbb945c4 Mon Sep 17 00:00:00 2001 From: Cy Okeke Date: Wed, 1 Jul 2026 14:33:53 +0100 Subject: [PATCH 4/5] Address review: build assignments with Select projection instead of foreach --- util/Seeder/Scenes/MigrationCohortExportScene.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/util/Seeder/Scenes/MigrationCohortExportScene.cs b/util/Seeder/Scenes/MigrationCohortExportScene.cs index 4562347f4cb4..ecbc7f632069 100644 --- a/util/Seeder/Scenes/MigrationCohortExportScene.cs +++ b/util/Seeder/Scenes/MigrationCohortExportScene.cs @@ -192,11 +192,8 @@ private static List BuildOrganizations(Request request) } private static List BuildAssignments( - IEnumerable organizations, Guid cohortId) - { - var assignments = new List(); - - foreach (var org in organizations) + IEnumerable organizations, Guid cohortId) => + organizations.Select(org => { // CreationDate defaults to DateTime.UtcNow at construction and is not settable here, so // assignments in one bulk batch share a near-identical value. RevisionDate is settable, so @@ -208,11 +205,8 @@ private static List BuildAssignments( }; assignment.RevisionDate = assignment.CreationDate; assignment.SetNewId(); - assignments.Add(assignment); - } - - return assignments; - } + return assignment; + }).ToList(); /// /// Bulk-inserts a row per organization when the request is part of a play From 49ba2ea4487d4b48e1ea8d1200bcdca861486284 Mon Sep 17 00:00:00 2001 From: Cy Okeke Date: Wed, 1 Jul 2026 16:00:25 +0100 Subject: [PATCH 5/5] Address review: remove scene-specific tests per Scenes owner guidance SeedControllerTests covers scene discovery/naming and delete generically, not per-scene end-to-end behavior; and cleanup is handled by DeleteOldPlayDataJob (verified running clean locally against SQL Server with no Command errors), so the dedicated cleanup tests are redundant. Restores SeedControllerTests to its original state and removes MigrationCohortExportSceneCleanupTests. --- .../MigrationCohortExportSceneCleanupTests.cs | 114 -------------- .../SeedControllerTests.cs | 147 ------------------ 2 files changed, 261 deletions(-) delete mode 100644 test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs diff --git a/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs b/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs deleted file mode 100644 index b2abcd752731..000000000000 --- a/test/SeederApi.IntegrationTest/MigrationCohortExportSceneCleanupTests.cs +++ /dev/null @@ -1,114 +0,0 @@ -using System.Text.Json; -using Bit.Core.Services; -using Bit.Seeder.Scenes; -using Bit.SeederApi.Models.Request; -using Duende.IdentityModel.Client; -using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.DependencyInjection.Extensions; -using Xunit; - -namespace Bit.SeederApi.IntegrationTest; - -/// -/// Verifies the play-id cleanup path for . The scene inserts -/// its organizations via BulkCopy, which bypasses the repository tracking decorators, so the scene -/// records the PlayItem rows itself. These tests confirm that DELETE /seed/{playId} can -/// therefore tear the bulk-seeded organizations down. -/// -/// -/// Uses a dedicated factory that enables a real (fixed) -- the shared -/// registers NeverPlayIdServices, under which the -/// scene's play-id recording is a deliberate no-op. -/// -public class MigrationCohortExportSceneCleanupTests - : IClassFixture, IAsyncLifetime -{ - private readonly HttpClient _client; - private readonly PlayIdEnabledSeederApiApplicationFactory _factory; - private const string _username = "username"; - private const string _password = "pass"; - - public MigrationCohortExportSceneCleanupTests(PlayIdEnabledSeederApiApplicationFactory factory) - { - _factory = factory; - factory.ConfigureAuth(_username, _password); - _client = _factory.CreateClient(); - _client.SetBasicAuthentication(_username, _password); - } - - public Task InitializeAsync() => Task.CompletedTask; - - public Task DisposeAsync() - { - _client.Dispose(); - return Task.CompletedTask; - } - - [Fact] - public async Task Seed_WithPlayId_RecordsOnePlayItemPerOrganizationLinkingThemForCleanup() - { - const int orgCount = 4; - var cohortName = $"PM-36965 Cleanup {Guid.NewGuid()}"; - var namePrefix = $"pm36965-{Guid.NewGuid():N}-"; - var playId = _factory.PlayId; - - var seedResponse = await _client.PostAsJsonAsync("/seed", new SeedRequestModel - { - Template = "MigrationCohortExportScene", - Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request - { - CohortName = cohortName, - OrgCount = orgCount, - NamePrefix = namePrefix - }) - }, playId); - seedResponse.EnsureSuccessStatusCode(); - - var db = _factory.GetDatabaseContext(); - - var orgIds = await db.Organizations - .Where(o => o.Name.StartsWith(namePrefix)) - .Select(o => o.Id) - .ToListAsync(); - Assert.Equal(orgCount, orgIds.Count); - - // The scene bulk-inserts a PlayItem per organization (BulkCopy bypasses the repository - // tracking decorator), which is exactly what lets DELETE /seed/{playId} find and remove them. - var trackedOrgIds = await db.PlayItem - .Where(p => p.PlayId == playId && p.OrganizationId != null) - .Select(p => p.OrganizationId!.Value) - .ToListAsync(); - - Assert.Equal(orgCount, trackedOrgIds.Count); - Assert.Equal(orgIds.ToHashSet(), trackedOrgIds.ToHashSet()); - } -} - -/// -/// Factory variant that registers a fixed so play-id tracking is active -/// for the cleanup tests (the base factory registers NeverPlayIdServices). -/// -public class PlayIdEnabledSeederApiApplicationFactory : SeederApiApplicationFactory -{ - public string PlayId { get; } = $"cleanup-{Guid.NewGuid()}"; - - public PlayIdEnabledSeederApiApplicationFactory() - { - _configureTestServices.Add(services => - { - services.RemoveAll(); - services.AddSingleton(new FixedPlayIdService(PlayId)); - }); - } - - private sealed class FixedPlayIdService(string playId) : IPlayIdService - { - public string? PlayId { get; set; } = playId; - - public bool InPlay(out string playId) - { - playId = PlayId ?? string.Empty; - return !string.IsNullOrEmpty(playId); - } - } -} diff --git a/test/SeederApi.IntegrationTest/SeedControllerTests.cs b/test/SeederApi.IntegrationTest/SeedControllerTests.cs index c77c22628887..a409741ed1d5 100644 --- a/test/SeederApi.IntegrationTest/SeedControllerTests.cs +++ b/test/SeederApi.IntegrationTest/SeedControllerTests.cs @@ -1,11 +1,8 @@ using System.Net; -using System.Text.Json; -using Bit.Core.Billing.Organizations.PlanMigration.Enums; using Bit.Seeder.Scenes; using Bit.SeederApi.Models.Request; using Bit.SeederApi.Models.Response; using Duende.IdentityModel.Client; -using Microsoft.EntityFrameworkCore; using Xunit; namespace Bit.SeederApi.IntegrationTest; @@ -225,150 +222,6 @@ public async Task SeedEndpoint_VerifyResponseContainsMangleMapAndResult() Assert.Contains("result", jsonString, StringComparison.OrdinalIgnoreCase); } - [Fact] - public async Task SeedEndpoint_MigrationCohortExportScene_PersistsCohortOrgsAndAssignmentsToDatabase() - { - const int orgCount = 5; - var cohortName = $"PM-36965 Export Test {Guid.NewGuid()}"; - var namePrefix = $"pm36965-{Guid.NewGuid():N}-"; - - var response = await _client.PostAsJsonAsync("/seed", new SeedRequestModel - { - Template = "MigrationCohortExportScene", - Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request - { - CohortName = cohortName, - OrgCount = orgCount, - NamePrefix = namePrefix - }) - }, Guid.NewGuid().ToString()); - - response.EnsureSuccessStatusCode(); - - // Read the data back straight from the database -- proving the rows actually landed, - // not merely that the API reported success. - var db = _factory.GetDatabaseContext(); - - var cohort = await db.OrganizationPlanMigrationCohorts - .SingleOrDefaultAsync(c => c.Name == cohortName); - Assert.NotNull(cohort); - Assert.True(cohort.IsActive); - // Default request => a migration cohort on the default path. - Assert.Equal(MigrationPathId.Enterprise2020AnnualToCurrent, cohort.MigrationPathId); - - var orgs = await db.Organizations - .Where(o => o.Name.StartsWith(namePrefix)) - .ToListAsync(); - Assert.Equal(orgCount, orgs.Count); - Assert.All(orgs, o => Assert.False(o.Enabled)); // seeded orgs are inert - - var assignments = await db.OrganizationPlanMigrationCohortAssignments - .Where(a => a.CohortId == cohort.Id) - .ToListAsync(); - Assert.Equal(orgCount, assignments.Count); - - // Every seeded org has exactly one assignment to this cohort. - var orgIds = orgs.Select(o => o.Id).ToHashSet(); - Assert.Equal(orgIds, assignments.Select(a => a.OrganizationId).ToHashSet()); - - // This factory registers NeverPlayIdServices, so play-id recording is a no-op: no PlayItem - // rows are written for a seed that carries no active play id. - Assert.Equal(0, await db.PlayItem.CountAsync(p => orgIds.Contains(p.OrganizationId!.Value))); - } - - [Fact] - public async Task SeedEndpoint_MigrationCohortExportScene_ReusesExistingCohortByName() - { - var cohortName = $"PM-36965 Reuse {Guid.NewGuid()}"; - - async Task SeedAsync() => - (await (await _client.PostAsJsonAsync("/seed", new SeedRequestModel - { - Template = "MigrationCohortExportScene", - Arguments = JsonSerializer.SerializeToElement( - new MigrationCohortExportScene.Request { CohortName = cohortName, OrgCount = 2 }) - }, Guid.NewGuid().ToString())) - .EnsureSuccessStatusCode() - .Content.ReadFromJsonAsync()); - - var first = (await SeedAsync()).GetProperty("result"); - var second = (await SeedAsync()).GetProperty("result"); - - // Second run finds the cohort by name and reuses it rather than creating a duplicate. - Assert.True(first.GetProperty("cohortCreated").GetBoolean()); - Assert.False(second.GetProperty("cohortCreated").GetBoolean()); - Assert.Equal(first.GetProperty("cohortId").GetGuid(), second.GetProperty("cohortId").GetGuid()); - } - - [Fact] - public async Task SeedEndpoint_MigrationCohortExportScene_NullMigrationPathId_CreatesChurnOnlyCohort() - { - var cohortName = $"PM-36965 Churn {Guid.NewGuid()}"; - var namePrefix = $"pm36965-{Guid.NewGuid():N}-"; - - var response = await _client.PostAsJsonAsync("/seed", new SeedRequestModel - { - Template = "MigrationCohortExportScene", - Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request - { - CohortName = cohortName, - OrgCount = 2, - NamePrefix = namePrefix, - MigrationPathId = null - }) - }, Guid.NewGuid().ToString()); - - response.EnsureSuccessStatusCode(); - - var db = _factory.GetDatabaseContext(); - var cohort = await db.OrganizationPlanMigrationCohorts.SingleAsync(c => c.Name == cohortName); - - // A null path persists as null: a churn-only cohort, not the default migration path. - Assert.Null(cohort.MigrationPathId); - } - - [Theory] - [InlineData(0)] - [InlineData(-1)] - [InlineData(100_001)] - public async Task SeedEndpoint_MigrationCohortExportScene_OrgCountOutOfRange_ReturnsBadRequest(int orgCount) - { - var response = await _client.PostAsJsonAsync("/seed", new SeedRequestModel - { - Template = "MigrationCohortExportScene", - Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request - { - CohortName = $"PM-36965 Range {Guid.NewGuid()}", - OrgCount = orgCount - }) - }, Guid.NewGuid().ToString()); - - Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); - } - - [Fact] - public async Task SeedEndpoint_MigrationCohortExportScene_OrgCountLowerBound_Succeeds() - { - var cohortName = $"PM-36965 Min {Guid.NewGuid()}"; - var namePrefix = $"pm36965-{Guid.NewGuid():N}-"; - - var response = await _client.PostAsJsonAsync("/seed", new SeedRequestModel - { - Template = "MigrationCohortExportScene", - Arguments = JsonSerializer.SerializeToElement(new MigrationCohortExportScene.Request - { - CohortName = cohortName, - OrgCount = 1, - NamePrefix = namePrefix - }) - }, Guid.NewGuid().ToString()); - - response.EnsureSuccessStatusCode(); - - var db = _factory.GetDatabaseContext(); - Assert.Equal(1, await db.Organizations.CountAsync(o => o.Name.StartsWith(namePrefix))); - } - private class BatchDeleteResponse { public string? Message { get; set; }