Skip to content

Commit

Permalink
Merge pull request #1518 from input-output-hk/fix/set-all-drop-db
Browse files Browse the repository at this point in the history
fix(wallet): preserve pouchdb collection documents when bulkDocs fails
  • Loading branch information
mkazlauskas authored Oct 25, 2024
2 parents e0d6005 + 0caf2c4 commit 6acb1e6
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 10 deletions.
3 changes: 2 additions & 1 deletion packages/wallet/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@
"pouchdb": "^7.3.0",
"rxjs": "^7.4.0",
"ts-custom-error": "^3.2.0",
"ts-log": "^2.2.3"
"ts-log": "^2.2.3",
"uuid": "^8.3.2"
},
"files": [
"dist/*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Logger } from 'ts-log';
import { PouchDbStore } from './PouchDbStore';
import { observeAll } from '../util';
import { sanitizePouchDbDoc } from './util';
import { v4 } from 'uuid';

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

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

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

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

getAll(): Observable<T[]> {
Expand All @@ -50,13 +51,34 @@ export class PouchDbCollectionStore<T extends {}> extends PouchDbStore<T> implem
return from(
(this.idle = this.idle.then(async (): Promise<void> => {
try {
await this.clearDB();
const newDocsWithId = docs.map((doc) => ({
...this.toPouchDbDoc(doc),
_id: this.#computeDocId(doc)
}));
const existingDocs = await this.fetchAllDocs();
const newDocsWithRev = newDocsWithId.map((newDoc): T & { _id: string; _rev?: string } => {
const existingDoc = existingDocs.find((doc) => doc.id === newDoc._id);
if (!existingDoc) return newDoc;
return {
...newDoc,
_rev: existingDoc.value.rev
};
});
const docsToDelete = existingDocs.filter(
(existingDoc) => !newDocsWithId.some((newDoc) => newDoc._id === existingDoc.id)
);
await this.db.bulkDocs(
docs.map((doc) => ({
...this.toPouchDbDoc(doc),
_id: this.#computeDocId?.(doc)
}))
docsToDelete.map(
({ id, value: { rev } }) =>
({
_deleted: true,
_id: id,
_rev: rev
} as unknown as T)
)
);
await this.db.bulkDocs(newDocsWithRev);

this.#updates$.next(docs);
} catch (error) {
this.logger.error(`PouchDbCollectionStore(${this.dbName}): failed to setAll`, docs, error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export abstract class PouchDbStore<T extends {}> {
destroyed = false;
protected idle: Promise<void> = Promise.resolve();
protected readonly logger: Logger;
protected readonly db: PouchDB.Database<T>;
public readonly db: PouchDB.Database<T>;

constructor(public dbName: string, logger: Logger) {
this.logger = logger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,11 @@ export const createPouchDbWalletStores = (
// However it is extremely unlikely that it would have inline datums,
// and renaming this store is not an option as it's being used
// for storing collateral settings
unspendableUtxo: new PouchDbUtxoStore({ dbName: `${baseDbName}UnspendableUtxo` }, logger),
unspendableUtxo: new PouchDbUtxoStore(
// random doc id; setAll will always delete and re-insert all docs
{ dbName: `${baseDbName}UnspendableUtxo` },
logger
),
utxo: new PouchDbUtxoStore({ dbName: `${baseDbName}Utxo_v2` }, logger),
volatileTransactions: new PouchDbVolatileTransactionsStore(docsDbName, 'volatileTransactions_v3', logger)
};
Expand Down
14 changes: 14 additions & 0 deletions packages/wallet/test/persistence/pouchDbStores.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,20 @@ describe('pouchDbStores', () => {
expect(await firstValueFrom(store1.getAll())).toEqual([doc2]);
});

it('setAll preserves existing documents if bulkDocs fails', async () => {
await firstValueFrom(store1.setAll([doc1]));
const originalBulkDocs = store1.db.bulkDocs.bind(store1.db);
// 1st call used to clearAll, 2nd call was used to insert new docs.
// after the fix, 1st only deletes the documents that are intended to be deleted
store1.db.bulkDocs = jest.fn().mockImplementationOnce(originalBulkDocs).mockRejectedValueOnce(new Error('fail'));

// attempting to insert doc2
await firstValueFrom(store1.setAll([doc1, doc2]));

const docs = await firstValueFrom(store1.getAll());
expect(docs.length).toBeGreaterThan(0);
});

it('simultaneous setAll() calls are resolved in series - last value is always persisted', async () => {
await firstValueFrom(
combineLatest([store1.setAll([doc1]), timer(1).pipe(mergeMap(() => store1.setAll([doc2])))])
Expand Down
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4085,6 +4085,7 @@ __metadata:
ts-log: ^2.2.3
tsc-alias: ^1.8.10
typescript: ^4.7.4
uuid: ^8.3.2
wait-on: ^6.0.1
webextension-polyfill: ^0.9.0
languageName: unknown
Expand Down

0 comments on commit 6acb1e6

Please sign in to comment.