Comments legacy Ticket-comment migration

MigrateTicketComments moves the Ticket aggregate’s CommentsJson into SysComment. It is an in-process Command without an HTTP endpoint or IAuthorizedRequest, and the source contains no scheduler or host caller. Operations must supply a controlled trigger and the correct tenant context.

1. Migration data flow

Every migrated row is a root: EntityType=Ticket, EntityId is the Ticket persistence Id, and ParentCommentId=null. CommentId, AuthorId, and Body are copied. The legacy shape has no reply chain.

2. Trigger boundary

The Command has no [GenerateEndpoint] and no IAuthorizedRequest. Avoiding public HTTP is a good starting point, but any in-process caller able to send the Command can choose a TenantId.

A production trigger should:

• live only in a controlled JobHost or migration tool;
• require platform-operator authorization and any required dual approval;
• push the target EffectiveTenant context;
• assert Command TenantId == EffectiveTenantId;
• support dry-run, a maximum slice, deadline, and cancellation;
• write immutable migration audit data without comment bodies.

3. The transaction is not per Ticket

A source comment describes per-ticket atomic migration, but the handler creates no per-ticket transaction. Its message implements ICommand<Result<...>> and not INonTransactionalCommand, so the default TransactionPipelineBehavior is expected to wrap the entire handler, including every while-loop iteration.

Consequences:

• one late exception may roll back every Ticket handled by the Command;
• a large tenant can create a long transaction, locks, log growth, and timeouts;
• BatchSize controls one in-memory iteration, not a commit boundary;
• the current source does not prove that a completed batch is resumable after a later failure.

A safer job sends stable-cursor slices, each with a bounded number of Tickets and an explicit per-ticket or per-slice transaction contract.

4. What BatchSize actually controls

TicketCommentMigrationSource first calls ListAsync for every matching Ticket and only then applies .Take(batchSize) in memory. BatchSize therefore does not bound the database read. It is not validated: zero or a negative value returns an apparently successful empty report.

public static Result<MigrationSlice> CreateSlice(
    string requestedTenant,
    string effectiveTenant,
    int batchSize)
{
    // Prevent a split between the tenant read from and the tenant written to.
    if (!string.Equals(requestedTenant, effectiveTenant, StringComparison.Ordinal))
        return Result.Failure<MigrationSlice>(MigrationErrors.TenantMismatch);

    // Bound transaction, memory, and database pressure; tune from capacity tests.
    if (batchSize is < 1 or > 500)
        return Result.Failure<MigrationSlice>(MigrationErrors.InvalidBatchSize);

    return new MigrationSlice(effectiveTenant, batchSize);
}

Push Take/cursor into the store query and impose a deterministic order such as Ticket Id.

5. Idempotency and partial failure

For every legacy comment, migration calls GetByCommentId and skips an existing key. It marks CommentsJson=[] only after processing the Ticket. Under the current outer transaction, an Insert followed by a Mark failure normally rolls back together. With future small transactions, a replay can skip a committed target row and finish clearing the source.

Remaining boundaries:

• two migration instances can race between lookup and insert and hit the unique index;
• CommentStore lookup requires batch TenantId to equal EffectiveTenantId;
• migration bypasses Create validation, including body/id length checks;
• an existing key with different content is skipped without fingerprint comparison;
• the migratedAt argument reaches the source but is not persisted; the Ticket stores only [].

6. Corrupt JSON and misleading report fields

When deserialization throws InvalidOperationException, the source logs a warning and skips that Ticket. The handler’s skippedTickets variable is never incremented, so SkippedTickets remains zero and TotalTickets excludes corrupt rows.

If every remaining row is corrupt or deserializes to no comments, the source returns an empty batch and the loop exits while CommentsJson remains non-empty.

{
  "totalTickets": 120,
  "migratedTickets": 118,
  "totalComments": 842,
  "skippedTickets": 0,
  "alreadyMigrated": 2
}

skippedTickets=0 is not evidence that the source is clean. The release gate must independently count remaining non-[] values and quarantine TicketId plus an error category.

7. Meaning of AlreadyMigrated

When every CommentId from a non-empty source already exists, commentCount is zero, the handler clears the legacy JSON, and increments AlreadyMigrated. It means “no new row was inserted while this pass completed source cleanup,” not necessarily that a previous migration was formally recorded.

The report calculates TotalTickets as MigratedTickets + AlreadyMigrated + SkippedTickets. Because skipped never changes, it counts only valid batches returned to the handler.

8. Dependency-isolation problem

Application’s migration port directly uses TicketComment from Tickets.Contracts. Infrastructure directly references Tickets.Infrastructure and queries the Ticket aggregate. Yet CommentsModule declares only [DependsOn("Authorization")]. The source therefore contradicts the governance dependency and is unsuitable for independent deployment without the Tickets implementation.

A physical migration exception is better isolated in a dedicated migration assembly/tool and removed after completion. At minimum:

• declare the temporary Tickets dependency and its removal condition;
• delete the adapter/project reference after reconciliation;
• isolate the legacy shape through an export DTO or database migration script;
• retain recovery evidence instead of leaving a permanent dual-model bridge.

9. Safe migration runbook

1. Back up Ticket CommentsJson and SysComment; prove restore works.
2. Inventory Ticket/comment counts, maximum body/id lengths, and corrupt JSON per tenant.
3. stop writes to the legacy field or install an explicit dual-write fence.
4. Dry-run in the target EffectiveTenant and assert tenant equality.
5. Migrate stable-cursor slices, commit each slice, and retain checkpoints.
6. Reconcile CommentId, AuthorId, Body hashes, Ticket counts, and remaining non-empty JSON.
7. Sample GET results and verify subject authorization.
8. After the rollback window, remove legacy reads/writes and cross-module references.

10. Required test cases

• Command TenantId differs from EffectiveTenantId;
• BatchSize zero, negative, excessive, and database pushdown;
• failure on Ticket N and the actual transaction boundary;
• two migration workers running concurrently;
• Insert success/Mark failure followed by replay;
• corrupt JSON, empty JSON, null, whitespace, and non-canonical [];
• legacy body/id longer than the new table allows;
• existing CommentId with different body;
• source remainder zero and target count/hash equality;
• equivalent semantics under both ORM providers.

Back to Comments