Skip to content

Commit 810cc4e

Browse files
rdlabordlabo
committed
fix(offline): adopt known server identity safely
1 parent 60082a6 commit 810cc4e

5 files changed

Lines changed: 371 additions & 29 deletions

File tree

‎projects/kit/offline/src/lib/offline-replica-schema.types.fixture.ts‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,6 @@ type TextIdSelect = {
2323
title: string;
2424
};
2525

26-
defineReplicaEntity<TextIdSelect>()({
27-
table: 'text_items',
28-
sourceKey: 'text_items',
29-
scope: 'user',
30-
fields: {
31-
id: serverId('text'),
32-
title: text(),
33-
},
34-
});
35-
3626
type LiteralSelect = {
3727
id: number;
3828
kind: 0 | 1;
@@ -79,15 +69,3 @@ defineReplicaEntity<TextIdSelect>()({
7969
title: text(),
8070
},
8171
});
82-
83-
defineReplicaEntity<NumericSelect>()({
84-
table: 'numeric_mismatch_items',
85-
sourceKey: 'numeric_mismatch_items',
86-
scope: 'user',
87-
fields: {
88-
// @ts-expect-error — serverId("text") cannot map a numeric property.
89-
id: serverId('text'),
90-
title: text(),
91-
notes: nullable(text()),
92-
},
93-
});

‎projects/kit/offline/src/lib/offline-repository.spec.ts‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,159 @@ describe('IonicOfflineRepository', () => {
10721072
});
10731073
});
10741074

1075+
describe('replica serverId uniqueness', () => {
1076+
const scope = { userId: 1, groupId: 10 };
1077+
const groupRow = {
1078+
sourceKey: 'test_group_items' as const,
1079+
serverId: 55,
1080+
confirmedValues: null,
1081+
serverRevision: null,
1082+
fetchedAt: 1,
1083+
syncState: 'confirmed' as const,
1084+
};
1085+
const userRow = {
1086+
sourceKey: 'test_items' as const,
1087+
serverId: 42,
1088+
confirmedValues: null,
1089+
serverRevision: null,
1090+
fetchedAt: 1,
1091+
syncState: 'confirmed' as const,
1092+
};
1093+
1094+
it('group-scopedで別localIdに同じserverIdを割り当てるとrejectする', async () => {
1095+
await repository.transactReplica({
1096+
putRows: [
1097+
{
1098+
...groupRow,
1099+
userId: 1,
1100+
groupId: 10,
1101+
localId: '019d-aaaa',
1102+
values: { id: 55, name: 'A' },
1103+
},
1104+
],
1105+
});
1106+
await expect(
1107+
repository.transactReplica({
1108+
putRows: [
1109+
{
1110+
...groupRow,
1111+
userId: 1,
1112+
groupId: 10,
1113+
localId: '019d-bbbb',
1114+
values: { id: 55, name: 'B' },
1115+
},
1116+
],
1117+
}),
1118+
).rejects.toThrow('Offline replica serverId 55 is already mapped to localId 019d-aaaa.');
1119+
});
1120+
1121+
it('user-scopedで別localIdに同じserverIdを割り当てるとrejectする', async () => {
1122+
await repository.transactReplica({
1123+
putRows: [
1124+
{
1125+
...userRow,
1126+
userId: 1,
1127+
groupId: 10,
1128+
localId: '019d-aaaa',
1129+
values: { id: 42, title: 'A' },
1130+
},
1131+
],
1132+
});
1133+
await expect(
1134+
repository.transactReplica({
1135+
putRows: [
1136+
{
1137+
...userRow,
1138+
userId: 1,
1139+
groupId: 10,
1140+
localId: '019d-bbbb',
1141+
values: { id: 42, title: 'B' },
1142+
},
1143+
],
1144+
}),
1145+
).rejects.toThrow('Offline replica serverId 42 is already mapped to localId 019d-aaaa.');
1146+
});
1147+
1148+
it('同一transaction内のserverId重複は部分永続化せずrejectする', async () => {
1149+
await expect(
1150+
repository.transactReplica({
1151+
putRows: [
1152+
{
1153+
...groupRow,
1154+
userId: 1,
1155+
groupId: 10,
1156+
localId: '019d-aaaa',
1157+
values: { id: 55, name: 'A' },
1158+
},
1159+
{
1160+
...groupRow,
1161+
userId: 1,
1162+
groupId: 10,
1163+
localId: '019d-bbbb',
1164+
values: { id: 55, name: 'B' },
1165+
},
1166+
],
1167+
}),
1168+
).rejects.toThrow('Offline replica serverId 55 is already mapped to localId 019d-aaaa.');
1169+
expect(await repository.getReplicaRow(scope, 'test_group_items', '019d-aaaa')).toBeNull();
1170+
expect(await repository.getReplicaRow(scope, 'test_group_items', '019d-bbbb')).toBeNull();
1171+
});
1172+
1173+
it('group-scopedは別groupなら同じserverIdを許容する', async () => {
1174+
await repository.transactReplica({
1175+
putRows: [
1176+
{
1177+
...groupRow,
1178+
userId: 1,
1179+
groupId: 10,
1180+
localId: '019d-aaaa',
1181+
values: { id: 55, name: 'G10' },
1182+
},
1183+
{
1184+
...groupRow,
1185+
userId: 1,
1186+
groupId: 11,
1187+
localId: '019d-bbbb',
1188+
values: { id: 55, name: 'G11' },
1189+
},
1190+
],
1191+
});
1192+
await expect(repository.getReplicaRowByServerId(scope, 'test_group_items', 55)).resolves.toMatchObject({
1193+
localId: '019d-aaaa',
1194+
});
1195+
await expect(repository.getReplicaRowByServerId({ userId: 1, groupId: 11 }, 'test_group_items', 55)).resolves.toMatchObject({
1196+
localId: '019d-bbbb',
1197+
});
1198+
});
1199+
1200+
it('user-scopedは別groupでも同じserverIdをrejectする', async () => {
1201+
await repository.transactReplica({
1202+
putRows: [
1203+
{
1204+
...userRow,
1205+
userId: 1,
1206+
groupId: 10,
1207+
localId: '019d-aaaa',
1208+
values: { id: 42, title: 'G10' },
1209+
},
1210+
],
1211+
});
1212+
await expect(
1213+
repository.transactReplica({
1214+
putRows: [
1215+
{
1216+
...userRow,
1217+
userId: 1,
1218+
groupId: 11,
1219+
localId: '019d-bbbb',
1220+
values: { id: 42, title: 'G11' },
1221+
},
1222+
],
1223+
}),
1224+
).rejects.toThrow('Offline replica serverId 42 is already mapped to localId 019d-aaaa.');
1225+
});
1226+
});
1227+
10751228
describe('getReplicaRows', () => {
10761229
const baseRow = {
10771230
sourceKey: 'test_items',

‎projects/kit/offline/src/lib/offline-repository.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,12 +510,17 @@ export class IonicOfflineRepository implements OfflineRepository {
510510
async #applyReplicaTransaction(transaction: OfflineReplicaTransaction, journal: boolean): Promise<void> {
511511
await this.#assertReplicaSchemaLocked();
512512
for (const row of transaction.putRows ?? []) this.#validateReplicaRow(row);
513-
if (journal) await this.#storage.set(REPLICA_TRANSACTION_KEY, transaction);
514513
const [rows, commands, cursors] = await Promise.all([
515514
this.#readRecord<OfflineReplicaRow>(ROWS_KEY),
516515
this.#readRecord<OfflineCommand>(OUTBOX_KEY),
517516
this.#readRecord<string>(CURSORS_KEY),
518517
]);
518+
const identityCheckRows = { ...rows };
519+
for (const row of transaction.putRows ?? []) {
520+
this.#assertUniqueReplicaServerId(identityCheckRows, row);
521+
identityCheckRows[this.#rowKey(row, row.sourceKey, row.localId)] = row;
522+
}
523+
if (journal) await this.#storage.set(REPLICA_TRANSACTION_KEY, transaction);
519524
for (const row of transaction.putRows ?? []) {
520525
const schema = this.#resolveReplicaEntitySchema(row.sourceKey);
521526
rows[this.#rowKey(row, row.sourceKey, row.localId)] = {
@@ -598,6 +603,24 @@ export class IonicOfflineRepository implements OfflineRepository {
598603
if (row.confirmedValues !== null) encodeOfflineReplicaValues(schema, row.confirmedValues);
599604
}
600605

606+
#assertUniqueReplicaServerId(rows: Record<string, OfflineReplicaRow>, incoming: OfflineReplicaRow): void {
607+
if (incoming.serverId === null) return;
608+
const schema = this.#resolveReplicaEntitySchema(incoming.sourceKey);
609+
const incomingKey = this.#rowKey(incoming, incoming.sourceKey, incoming.localId);
610+
const collision = Object.entries(rows).find(([key, row]) => {
611+
if (key === incomingKey) return false;
612+
if (row.userId !== incoming.userId || row.sourceKey !== incoming.sourceKey || row.serverId !== incoming.serverId) {
613+
return false;
614+
}
615+
return schema.scope === 'user' || row.groupId === incoming.groupId;
616+
});
617+
if (collision) {
618+
throw new Error(
619+
`Offline replica serverId ${String(incoming.serverId)} is already mapped to localId ${collision[1].localId}.`,
620+
);
621+
}
622+
}
623+
601624
#resolveReplicaEntitySchema(sourceKey: string): OfflineReplicaEntitySchema<Record<string, unknown>> {
602625
const schema = this.#options.replicaSchema.entities.find((entity) => entity.sourceKey === sourceKey);
603626
if (!schema) throw new Error(`Unknown offline replica source key "${sourceKey}".`);

0 commit comments

Comments
 (0)