Skip to content

Commit 0caf2c4

Browse files
committed
fix(wallet): preserve pouchdb collection documents when bulkDocs fails
PouchDbCollectionStore.setAll used to 1. delete all documents 2. insert the new documents This approach has a problem that when step 2. fails, all documents from db are gone and not re-created. For some collections such as transactions or utxo this is not a problem, because it can recover, but for it is a big problem for WalletRepository, because there is no way to recover the wallets if they are lost. This fix updates setAll to perform the following steps: 1. delete *only* the documents that are intended to be deleted 2. upsert all new documents
1 parent e0d6005 commit 0caf2c4

File tree

6 files changed

+52
-10
lines changed

6 files changed

+52
-10
lines changed

packages/wallet/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@
8181
"pouchdb": "^7.3.0",
8282
"rxjs": "^7.4.0",
8383
"ts-custom-error": "^3.2.0",
84-
"ts-log": "^2.2.3"
84+
"ts-log": "^2.2.3",
85+
"uuid": "^8.3.2"
8586
},
8687
"files": [
8788
"dist/*",

packages/wallet/src/persistence/pouchDbStores/PouchDbCollectionStore.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Logger } from 'ts-log';
44
import { PouchDbStore } from './PouchDbStore';
55
import { observeAll } from '../util';
66
import { sanitizePouchDbDoc } from './util';
7+
import { v4 } from 'uuid';
78

89
export type ComputePouchDbDocId<T> = (doc: T) => string;
910

@@ -14,7 +15,7 @@ export interface PouchDbCollectionStoreProps<T> {
1415

1516
/** PouchDB database that implements CollectionStore. Supports sorting by custom document _id */
1617
export class PouchDbCollectionStore<T extends {}> extends PouchDbStore<T> implements CollectionStore<T> {
17-
readonly #computeDocId: ComputePouchDbDocId<T> | undefined;
18+
readonly #computeDocId: ComputePouchDbDocId<T>;
1819
readonly #updates$ = new Subject<T[]>();
1920

2021
observeAll: CollectionStore<T>['observeAll'];
@@ -29,7 +30,7 @@ export class PouchDbCollectionStore<T extends {}> extends PouchDbStore<T> implem
2930
// Using a db per collection
3031
super(dbName, logger);
3132
this.observeAll = observeAll(this, this.#updates$);
32-
this.#computeDocId = computeDocId;
33+
this.#computeDocId = computeDocId ?? (() => v4());
3334
}
3435

3536
getAll(): Observable<T[]> {
@@ -50,13 +51,34 @@ export class PouchDbCollectionStore<T extends {}> extends PouchDbStore<T> implem
5051
return from(
5152
(this.idle = this.idle.then(async (): Promise<void> => {
5253
try {
53-
await this.clearDB();
54+
const newDocsWithId = docs.map((doc) => ({
55+
...this.toPouchDbDoc(doc),
56+
_id: this.#computeDocId(doc)
57+
}));
58+
const existingDocs = await this.fetchAllDocs();
59+
const newDocsWithRev = newDocsWithId.map((newDoc): T & { _id: string; _rev?: string } => {
60+
const existingDoc = existingDocs.find((doc) => doc.id === newDoc._id);
61+
if (!existingDoc) return newDoc;
62+
return {
63+
...newDoc,
64+
_rev: existingDoc.value.rev
65+
};
66+
});
67+
const docsToDelete = existingDocs.filter(
68+
(existingDoc) => !newDocsWithId.some((newDoc) => newDoc._id === existingDoc.id)
69+
);
5470
await this.db.bulkDocs(
55-
docs.map((doc) => ({
56-
...this.toPouchDbDoc(doc),
57-
_id: this.#computeDocId?.(doc)
58-
}))
71+
docsToDelete.map(
72+
({ id, value: { rev } }) =>
73+
({
74+
_deleted: true,
75+
_id: id,
76+
_rev: rev
77+
} as unknown as T)
78+
)
5979
);
80+
await this.db.bulkDocs(newDocsWithRev);
81+
6082
this.#updates$.next(docs);
6183
} catch (error) {
6284
this.logger.error(`PouchDbCollectionStore(${this.dbName}): failed to setAll`, docs, error);

packages/wallet/src/persistence/pouchDbStores/PouchDbStore.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export abstract class PouchDbStore<T extends {}> {
99
destroyed = false;
1010
protected idle: Promise<void> = Promise.resolve();
1111
protected readonly logger: Logger;
12-
protected readonly db: PouchDB.Database<T>;
12+
public readonly db: PouchDB.Database<T>;
1313

1414
constructor(public dbName: string, logger: Logger) {
1515
this.logger = logger;

packages/wallet/src/persistence/pouchDbStores/pouchDbWalletStores.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,11 @@ export const createPouchDbWalletStores = (
8585
// However it is extremely unlikely that it would have inline datums,
8686
// and renaming this store is not an option as it's being used
8787
// for storing collateral settings
88-
unspendableUtxo: new PouchDbUtxoStore({ dbName: `${baseDbName}UnspendableUtxo` }, logger),
88+
unspendableUtxo: new PouchDbUtxoStore(
89+
// random doc id; setAll will always delete and re-insert all docs
90+
{ dbName: `${baseDbName}UnspendableUtxo` },
91+
logger
92+
),
8993
utxo: new PouchDbUtxoStore({ dbName: `${baseDbName}Utxo_v2` }, logger),
9094
volatileTransactions: new PouchDbVolatileTransactionsStore(docsDbName, 'volatileTransactions_v3', logger)
9195
};

packages/wallet/test/persistence/pouchDbStores.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,20 @@ describe('pouchDbStores', () => {
8282
expect(await firstValueFrom(store1.getAll())).toEqual([doc2]);
8383
});
8484

85+
it('setAll preserves existing documents if bulkDocs fails', async () => {
86+
await firstValueFrom(store1.setAll([doc1]));
87+
const originalBulkDocs = store1.db.bulkDocs.bind(store1.db);
88+
// 1st call used to clearAll, 2nd call was used to insert new docs.
89+
// after the fix, 1st only deletes the documents that are intended to be deleted
90+
store1.db.bulkDocs = jest.fn().mockImplementationOnce(originalBulkDocs).mockRejectedValueOnce(new Error('fail'));
91+
92+
// attempting to insert doc2
93+
await firstValueFrom(store1.setAll([doc1, doc2]));
94+
95+
const docs = await firstValueFrom(store1.getAll());
96+
expect(docs.length).toBeGreaterThan(0);
97+
});
98+
8599
it('simultaneous setAll() calls are resolved in series - last value is always persisted', async () => {
86100
await firstValueFrom(
87101
combineLatest([store1.setAll([doc1]), timer(1).pipe(mergeMap(() => store1.setAll([doc2])))])

yarn.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4085,6 +4085,7 @@ __metadata:
40854085
ts-log: ^2.2.3
40864086
tsc-alias: ^1.8.10
40874087
typescript: ^4.7.4
4088+
uuid: ^8.3.2
40884089
wait-on: ^6.0.1
40894090
webextension-polyfill: ^0.9.0
40904091
languageName: unknown

0 commit comments

Comments
 (0)