Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions projects/kit/offline/src/lib/offline-sync.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,100 @@ describe('OfflineSyncService', () => {
});
});

it('同じaggregateの競合commandと後続intentを一transactionで再materializeする', async () => {
const firstId = await service.enqueue(
{
scopeId: '10',
aggregateType: 'documents',
identity: { kind: 'generated', localId: 'replace-chain' },
operation: 'documents.update',
payload: { title: 'stocktake' },
optimisticValue: { id: 15, title: 'stocktake' },
baseRevision: 1,
},
{ flush: false },
);
await service.enqueue(
{
scopeId: '10',
aggregateType: 'documents',
identity: { kind: 'generated', localId: 'replace-chain' },
operation: 'documents.update',
payload: { title: 'later delta' },
optimisticValue: { id: 15, title: 'later delta' },
baseRevision: 1,
},
{ flush: false },
);
commands[0] = { ...commands[0]!, state: 'conflict' };
const originalIds = commands.map((command) => command.commandId);
const originalCreatedAt = commands.map((command) => command.createdAt);

const replacementIds = await service.replacePreparedAggregate(
firstId,
async (_repository, chain) =>
chain.map((command, index) => ({
request: {
scopeId: command.scopeId,
aggregateType: command.aggregateType,
identity: command.identity,
operation: command.operation,
payload: command.payload,
optimisticValue: { id: 15, title: index === 0 ? 'new stocktake' : 'new stocktake plus delta' },
baseRevision: 2,
},
})),
{ flush: false },
);

expect(replacementIds).toHaveLength(2);
expect(replacementIds).not.toEqual(originalIds);
expect(commands.map((command) => command.commandId)).toEqual(replacementIds);
expect(commands.map((command) => command.state)).toEqual(['pending', 'pending']);
expect(commands.map((command) => command.createdAt)).toEqual(originalCreatedAt);
expect(rows.find((row) => row.identity.kind === 'generated' && row.identity.localId === 'replace-chain')?.values).toEqual({
id: 15,
title: 'new stocktake plus delta',
});
});

it('aggregate chainの準備失敗では元commandとprojectionを一切変更しない', async () => {
const firstId = await service.enqueue(
{
scopeId: '10',
aggregateType: 'documents',
identity: { kind: 'generated', localId: 'replace-chain-failure' },
operation: 'documents.update',
payload: { title: 'first' },
optimisticValue: { id: 16, title: 'first' },
},
{ flush: false },
);
await service.enqueue(
{
scopeId: '10',
aggregateType: 'documents',
identity: { kind: 'generated', localId: 'replace-chain-failure' },
operation: 'documents.update',
payload: { title: 'second' },
optimisticValue: { id: 16, title: 'second' },
},
{ flush: false },
);
commands[0] = { ...commands[0]!, state: 'conflict' };
const beforeCommands = structuredClone(commands);
const beforeRows = structuredClone(rows);

await expect(
service.replacePreparedAggregate(firstId, async () => {
throw new Error('chain preparation failed');
}),
).rejects.toThrow('chain preparation failed');

expect(commands).toEqual(beforeCommands);
expect(rows).toEqual(beforeRows);
});

it('companionの対象集合を変えるreplacementを元commandと楽観値を残して拒否する', async () => {
const companion: OfflineReplicaRow = {
userId: 1,
Expand Down
79 changes: 72 additions & 7 deletions projects/kit/offline/src/lib/offline-sync.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,67 @@ export class OfflineSyncService {
});
}

/**
* Atomically rematerializes every unresolved intent for one aggregate.
*
* This is the conflict-recovery boundary for ordered intent chains: the old
* chain remains durable until every replacement has been prepared and the
* complete chain can be committed in one replica transaction.
*/
replacePreparedAggregate<T>(
commandId: string,
prepare: (
repository: OfflineRepository,
commands: readonly OfflineCommand[],
) => Promise<readonly PreparedOfflineCommand<T>[]>,
options: PreparedOfflineBatchOptions = {},
): Promise<readonly string[]> {
const generation = this.#generation;
return this.#serializeReplicaMutation(async () => {
await this.initialize();
if (!this.#isCurrent(generation)) throw new Error('Offline session changed before prepared aggregate replacement.');
const knownCommands = await this.#readKnownCommands();
const selected = knownCommands.find((command) => command.commandId === commandId);
if (!selected) throw new Error(`Offline command ${commandId} no longer exists.`);
const aggregateKey = this.#aggregateKey(selected);
const replaced = knownCommands.filter((command) => this.#aggregateKey(command) === aggregateKey);
this.#assertDiscardable(replaced);
const prepared = await prepare(this.#repository, replaced);
if (prepared.length !== replaced.length) {
throw new Error('Offline aggregate replacement must preserve the ordered intent count.');
}
const session = await this.#beginEnqueueSession(generation);
const materializations: MaterializedOfflineEnqueue[] = [];
for (const [index, entry] of prepared.entries()) {
this.#assertEnqueueScope(session, entry.request.scopeId);
materializations.push(
await this.#materializeEnqueue(
session.userId,
entry.request,
entry.replicaTransaction,
replaced[index],
),
);
}
const retained = knownCommands.filter((command) => !replaced.some((item) => item.commandId === command.commandId));
this.#assertDistinctBatchFootprints(materializations, retained, true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 連鎖置換で一度消してから作り直したはずの関連データが消えたままになる

集約チェーンの置換をまとめて書き込む際、後続intentが復活させたはずの関連行が先行intentの削除指示によって上書き削除されます(#commitMaterializedEnqueuesremoveRows 生成、projects/kit/offline/src/lib/offline-sync.service.ts:660)。
Impact: 置換後に本来残るはずのローカルデータが消え、画面表示や後続同期が実際の意図と食い違います。

putRows→removeRows の適用順とチェーン内重複キー許可の組み合わせ

replacePreparedAggregate#assertDistinctBatchFootprints(materializations, retained, true)projects/kit/offline/src/lib/offline-sync.service.ts:354)で allowOneAggregate = true を渡すため、チェーン内の同一 replica キー重複チェック(同ファイル 622-625 行)が無効化されます。その結果、entry0 の companion が after = null(削除)、entry1 の同じ companion が after = row(再作成)というチェーンが許容されます。

しかし #commitMaterializedEnqueues は全 entry の put を putRows に、全 entry の削除を removeRows にフラットに集約します(同ファイル 656-660 行)。両リポジトリ実装とも putRows を全件適用した後に removeRows を適用するため(projects/kit/offline/src/lib/offline-repository.ts:686-698projects/kit/offline/src/lib/sqlite-offline-repository.ts:411-420)、entry1 が書いた行が entry0 の削除で消え、チェーン最終状態と一致しません。ベース行は putRows のみなので順序勝ちで正しく、影響は companion 行に限られます。

enqueuePreparedBatch 経路では重複 footprint が例外になるため、この経路は本 PR で新規に生じたものです。

Prompt for agents
replacePreparedAggregate では #assertDistinctBatchFootprints に allowOneAggregate=true を渡してチェーン内の replica footprint 重複チェックを無効化しているため、同一 companion キーに対して先行 intent が削除、後続 intent が再作成という組み合わせが通ってしまいます。一方 #commitMaterializedEnqueues は全 entry の put/remove をまとめて渡し、両リポジトリ実装(offline-repository.ts の #applyReplicaTransaction、sqlite-offline-repository.ts の transactReplica)は putRows を全て適用した後に removeRows を適用するため、最終状態がチェーンの意図と食い違い、本来残るべき companion 行が削除されます。対策としては、コミット前にチェーン内で同一 replica キーの最終状態(最後の entry の after)だけを put/remove に畳み込む、もしくは同一キーに対する削除と再作成が混在するチェーンを検証時に拒否することが考えられます。
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

await this.#assertOutboxCapacity(
session.userId,
materializations.map((item) => item.command),
replaced.map((command) => command.commandId),
knownCommands,
);
options.assertCurrent?.();
await this.#commitMaterializedEnqueues(
materializations,
generation,
options,
replaced.map((command) => command.commandId),
);
return materializations.map((item) => item.command.commandId);
});
}

/**
* Serializes a product-owned replica projection with enqueue and command ACK
* reconciliation. For read/derive/write cache updates, prefer
Expand Down Expand Up @@ -352,7 +413,7 @@ export class OfflineSyncService {
const currentCommands = await this.#commandsForUser(session.userId);
const retainedCommands = replaced ? currentCommands.filter((command) => command.commandId !== replaced.commandId) : currentCommands;
this.#assertDistinctBatchFootprints([materialization], retainedCommands);
await this.#assertOutboxCapacity(session.userId, [materialization.command], replaced?.commandId, currentCommands);
await this.#assertOutboxCapacity(session.userId, [materialization.command], replaced ? [replaced.commandId] : undefined, currentCommands);
await this.#commitMaterializedEnqueues([materialization], generation, options, replaced ? [replaced.commandId] : undefined);
return materialization.command.commandId;
}
Expand Down Expand Up @@ -538,7 +599,11 @@ export class OfflineSyncService {
return { command, optimisticRow, optimisticCompanions };
}

#assertDistinctBatchFootprints(entries: readonly MaterializedOfflineEnqueue[], existingCommands: readonly OfflineCommand[]): void {
#assertDistinctBatchFootprints(
entries: readonly MaterializedOfflineEnqueue[],
existingCommands: readonly OfflineCommand[],
allowOneAggregate = false,
): void {
const aggregates = new Set<string>();
const replicaKeys = new Set<string>();
const existingFootprints = new Map<string, string>();
Expand All @@ -548,15 +613,15 @@ export class OfflineSyncService {
}
for (const entry of entries) {
const aggregate = this.#aggregateKey(entry.command);
if (aggregates.has(aggregate)) {
if (aggregates.has(aggregate) && !allowOneAggregate) {
throw new Error('Prepared offline batch contains overlapping aggregate intents.');
}
aggregates.add(aggregate);
for (const key of [
this.#replicaRowKey(entry.optimisticRow),
...entry.optimisticCompanions.map((companion) => this.#replicaRowKey(companion.key)),
]) {
if (replicaKeys.has(key)) {
if (replicaKeys.has(key) && !allowOneAggregate) {
throw new Error('Prepared offline batch contains overlapping replica footprints.');
}
const existingAggregate = existingFootprints.get(key);
Expand Down Expand Up @@ -700,12 +765,12 @@ export class OfflineSyncService {
async #assertOutboxCapacity(
userId: OfflinePrincipalId,
newCommands: readonly OfflineCommand[],
excludingCommandId?: string,
excludingCommandIds?: readonly string[],
knownCommands?: readonly OfflineCommand[],
): Promise<void> {
const currentCommands = knownCommands ?? (await this.#commandsForUser(userId));
const commands = excludingCommandId
? currentCommands.filter((candidate) => candidate.commandId !== excludingCommandId)
const commands = excludingCommandIds
? currentCommands.filter((candidate) => !excludingCommandIds.includes(candidate.commandId))
: currentCommands;
const maxCommands = this.#options.outboxLimits?.maxCommandsPerUser ?? DEFAULT_MAX_OUTBOX_COMMANDS_PER_USER;
const maxBytes = this.#options.outboxLimits?.maxBytesPerUser ?? DEFAULT_MAX_OUTBOX_BYTES_PER_USER;
Expand Down