From a8f9d1a36f8a0028a5d60c702410598ef84676d8 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Mon, 18 Nov 2024 14:42:18 -0600 Subject: [PATCH] feat: `getByDocId` can optionally throw (#959) This adds a new option to `DataType.prototype.getByDocId()`, `mustBeFound`. If set to `false` and the document doesn't exist, it will resolve with `null` instead of throwing. If the option is missing or set to `true`, the current behavior is maintained (throwing if the document is missing). I think this is a useful feature on its own but will also hopefully make at least one [upcoming change][0] a little easier. [0]: https://github.com/digidem/comapeo-core/issues/188 --- src/datatype/index.d.ts | 6 +++++- src/datatype/index.js | 31 ++++++++++++++++++++++--------- src/mapeo-project.js | 23 +++++++++-------------- src/roles.js | 18 +++++++++--------- src/translation-api.js | 13 ++++--------- test/data-type.js | 8 ++++++++ 6 files changed, 57 insertions(+), 42 deletions(-) diff --git a/src/datatype/index.d.ts b/src/datatype/index.d.ts index 2a6f7d367..39922b265 100644 --- a/src/datatype/index.d.ts +++ b/src/datatype/index.d.ts @@ -87,8 +87,12 @@ export class DataType< getByDocId( docId: string, - opts?: { lang?: string } + opts?: { mustBeFound?: true; lang?: string } ): Promise + getByDocId( + docId: string, + opts?: { mustBeFound?: boolean; lang?: string } + ): Promise getByVersionId(versionId: string, opts?: { lang?: string }): Promise diff --git a/src/datatype/index.js b/src/datatype/index.js index 82459ca2c..1784987cd 100644 --- a/src/datatype/index.js +++ b/src/datatype/index.js @@ -164,16 +164,30 @@ export class DataType extends TypedEmitter { } /** + * @overload * @param {string} docId - * @param {{ lang?: string }} [opts] + * @param {object} [options] + * @param {true} [options.mustBeFound] + * @param {string} [options.lang] + * @returns {Promise} + */ + /** + * @param {string} docId + * @param {object} [options] + * @param {boolean} [options.mustBeFound] + * @param {string} [options.lang] + * @returns {Promise} */ - async getByDocId(docId, { lang } = {}) { + async getByDocId(docId, { mustBeFound = true, lang } = {}) { await this.#dataStore.indexer.idle() - const result = /** @type {undefined | MapeoDoc} */ ( - this.#sql.getByDocId.get({ docId }) - ) - if (!result) throw new NotFoundError() - return this.#translate(deNullify(result), { lang }) + const result = this.#sql.getByDocId.get({ docId }) + if (result) { + return this.#translate(deNullify(result), { lang }) + } else if (mustBeFound) { + throw new NotFoundError() + } else { + return null + } } /** @@ -186,7 +200,7 @@ export class DataType extends TypedEmitter { } /** - * @param {MapeoDoc} doc + * @param {any} doc * @param {{ lang?: string }} [opts] */ async #translate(doc, { lang } = {}) { @@ -278,7 +292,6 @@ export class DataType extends TypedEmitter { const doc = { ...existingDoc, updatedAt: new Date().toISOString(), - // @ts-expect-error - TS just doesn't work in this class links: [existingDoc.versionId, ...existingDoc.forks], deleted: true, } diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 118cbd34e..541995bc5 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -599,14 +599,9 @@ export class MapeoProject extends TypedEmitter { async $setProjectSettings(settings) { const { projectSettings } = this.#dataTypes - // We only want to catch the error to the getByDocId call - // Using try/catch for this is a little verbose when dealing with TS types - const existing = await projectSettings - .getByDocId(this.#projectId) - .catch(() => { - // project does not exist so return null - return null - }) + const existing = await projectSettings.getByDocId(this.#projectId, { + mustBeFound: false, + }) if (existing) { return extractEditableProjectSettings( @@ -714,14 +709,14 @@ export class MapeoProject extends TypedEmitter { schemaName: /** @type {const} */ ('deviceInfo'), } - let existingDoc - try { - existingDoc = await deviceInfo.getByDocId(configCoreId) - } catch (err) { + const existingDoc = await deviceInfo.getByDocId(configCoreId, { + mustBeFound: false, + }) + if (existingDoc) { + return await deviceInfo.update(existingDoc.versionId, doc) + } else { return await deviceInfo[kCreateWithDocId](configCoreId, doc) } - - return deviceInfo.update(existingDoc.versionId, doc) } /** @param {boolean} isArchiveDevice */ diff --git a/src/roles.js b/src/roles.js index f037dc5e6..a7fc8dfb3 100644 --- a/src/roles.js +++ b/src/roles.js @@ -269,12 +269,10 @@ export class Roles extends TypedEmitter { * @returns {Promise} */ async getRole(deviceId) { - /** @type {string} */ - let roleId - try { - const roleAssignment = await this.#dataType.getByDocId(deviceId) - roleId = roleAssignment.roleId - } catch (e) { + const roleAssignment = await this.#dataType.getByDocId(deviceId, { + mustBeFound: false, + }) + if (!roleAssignment) { // The project creator will have the creator role const authCoreId = await this.#coreOwnership.getCoreId(deviceId, 'auth') if (authCoreId === this.#projectCreatorAuthCoreId) { @@ -285,6 +283,8 @@ export class Roles extends TypedEmitter { return NO_ROLE } } + + const { roleId } = roleAssignment if (!isRoleId(roleId)) { return BLOCKED_ROLE } @@ -386,9 +386,9 @@ export class Roles extends TypedEmitter { } } - const existingRoleDoc = await this.#dataType - .getByDocId(deviceId) - .catch(() => null) + const existingRoleDoc = await this.#dataType.getByDocId(deviceId, { + mustBeFound: false, + }) if (existingRoleDoc) { await this.#dataType.update( diff --git a/src/translation-api.js b/src/translation-api.js index 05d0a92eb..636a8a20f 100644 --- a/src/translation-api.js +++ b/src/translation-api.js @@ -1,7 +1,6 @@ import { and, sql } from 'drizzle-orm' import { kCreateWithDocId, kSelect } from './datatype/index.js' import { hashObject } from './utils.js' -import { NotFoundError } from './errors.js' import { omit } from './lib/omit.js' /** @import { Translation, TranslationValue } from '@comapeo/schema' */ /** @import { SetOptional } from 'type-fest' */ @@ -50,15 +49,11 @@ export default class TranslationApi { async put(value) { const identifiers = omit(value, ['message']) const docId = hashObject(identifiers) - try { - const doc = await this.#dataType.getByDocId(docId) + const doc = await this.#dataType.getByDocId(docId, { mustBeFound: false }) + if (doc) { return await this.#dataType.update(doc.versionId, value) - } catch (e) { - if (e instanceof NotFoundError) { - return await this.#dataType[kCreateWithDocId](docId, value) - } else { - throw new Error(`Error on translation ${e}`) - } + } else { + return await this.#dataType[kCreateWithDocId](docId, value) } } diff --git a/test/data-type.js b/test/data-type.js index cc709a2fb..1559f7028 100644 --- a/test/data-type.js +++ b/test/data-type.js @@ -209,6 +209,14 @@ test('getByDocId() throws if no document exists with that ID', async () => { await assert.rejects(() => dataType.getByDocId('foo bar'), NotFoundError) }) +test('getByDocId() can return null if no document exists with that ID', async () => { + const { dataType } = await testenv({ projectKey: randomBytes(32) }) + assert.equal( + await dataType.getByDocId('foo bar', { mustBeFound: false }), + null + ) +}) + test('delete()', async () => { const projectKey = randomBytes(32) const { dataType } = await testenv({ projectKey })