From fe6fc8cd97e55d6c738bc97d10a83f014c46331b Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Thu, 21 Nov 2024 09:14:35 -0600 Subject: [PATCH] revert: remove `getByDocId` optional parameter (#968) This reverts commit 56f7e18ae618e6d36dcfdf478ffe42f879aecd2e and replaces it with an internal-only function, `nullIfNotFound`. --- src/errors.js | 9 +++++++++ src/mapeo-project.js | 14 +++++++------- src/roles.js | 17 +++++++++-------- src/translation-api.js | 3 ++- test/data-type.js | 8 -------- test/errors.js | 12 +++++++++++- 6 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/errors.js b/src/errors.js index 65bd6c96..38bc8764 100644 --- a/src/errors.js +++ b/src/errors.js @@ -3,3 +3,12 @@ export class NotFoundError extends Error { super(message) } } + +/** + * @param {unknown} err + * @returns {null} + */ +export function nullIfNotFound(err) { + if (err instanceof NotFoundError) return null + throw err +} diff --git a/src/mapeo-project.js b/src/mapeo-project.js index c8d70331..f81e386f 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -59,7 +59,7 @@ import { Logger } from './logger.js' import { IconApi } from './icon-api.js' import { readConfig } from './config-import.js' import TranslationApi from './translation-api.js' -import { NotFoundError } from './errors.js' +import { NotFoundError, nullIfNotFound } from './errors.js' import { getErrorCode, getErrorMessage } from './lib/error.js' /** @import { ProjectSettingsValue } from '@comapeo/schema' */ /** @import { CoreStorage, BlobFilter, BlobStoreEntriesStream, KeyPair, Namespace, ReplicationStream } from './types.js' */ @@ -663,9 +663,9 @@ export class MapeoProject extends TypedEmitter { async $setProjectSettings(settings) { const { projectSettings } = this.#dataTypes - const existing = await projectSettings.getByDocId(this.#projectId, { - mustBeFound: false, - }) + const existing = await projectSettings + .getByDocId(this.#projectId) + .catch(nullIfNotFound) if (existing) { return extractEditableProjectSettings( @@ -773,9 +773,9 @@ export class MapeoProject extends TypedEmitter { schemaName: /** @type {const} */ ('deviceInfo'), } - const existingDoc = await deviceInfo.getByDocId(configCoreId, { - mustBeFound: false, - }) + const existingDoc = await deviceInfo + .getByDocId(configCoreId) + .catch(nullIfNotFound) if (existingDoc) { return await deviceInfo.update(existingDoc.versionId, doc) } else { diff --git a/src/roles.js b/src/roles.js index a7fc8dfb..596f8a45 100644 --- a/src/roles.js +++ b/src/roles.js @@ -2,6 +2,7 @@ import { currentSchemaVersions } from '@comapeo/schema' import mapObject from 'map-obj' import { kCreateWithDocId, kDataStore } from './datatype/index.js' import { assert, setHas } from './utils.js' +import { nullIfNotFound } from './errors.js' import { TypedEmitter } from 'tiny-typed-emitter' /** @import { Namespace } from './types.js' */ @@ -269,10 +270,10 @@ export class Roles extends TypedEmitter { * @returns {Promise} */ async getRole(deviceId) { - const roleAssignment = await this.#dataType.getByDocId(deviceId, { - mustBeFound: false, - }) - if (!roleAssignment) { + const roleRecord = await this.#dataType + .getByDocId(deviceId) + .catch(nullIfNotFound) + if (!roleRecord) { // The project creator will have the creator role const authCoreId = await this.#coreOwnership.getCoreId(deviceId, 'auth') if (authCoreId === this.#projectCreatorAuthCoreId) { @@ -284,7 +285,7 @@ export class Roles extends TypedEmitter { } } - const { roleId } = roleAssignment + const { roleId } = roleRecord if (!isRoleId(roleId)) { return BLOCKED_ROLE } @@ -386,9 +387,9 @@ export class Roles extends TypedEmitter { } } - const existingRoleDoc = await this.#dataType.getByDocId(deviceId, { - mustBeFound: false, - }) + const existingRoleDoc = await this.#dataType + .getByDocId(deviceId) + .catch(nullIfNotFound) if (existingRoleDoc) { await this.#dataType.update( diff --git a/src/translation-api.js b/src/translation-api.js index 636a8a20..e32e6fe0 100644 --- a/src/translation-api.js +++ b/src/translation-api.js @@ -1,6 +1,7 @@ import { and, sql } from 'drizzle-orm' import { kCreateWithDocId, kSelect } from './datatype/index.js' import { hashObject } from './utils.js' +import { nullIfNotFound } from './errors.js' import { omit } from './lib/omit.js' /** @import { Translation, TranslationValue } from '@comapeo/schema' */ /** @import { SetOptional } from 'type-fest' */ @@ -49,7 +50,7 @@ export default class TranslationApi { async put(value) { const identifiers = omit(value, ['message']) const docId = hashObject(identifiers) - const doc = await this.#dataType.getByDocId(docId, { mustBeFound: false }) + const doc = await this.#dataType.getByDocId(docId).catch(nullIfNotFound) if (doc) { return await this.#dataType.update(doc.versionId, value) } else { diff --git a/test/data-type.js b/test/data-type.js index 1559f702..cc709a2f 100644 --- a/test/data-type.js +++ b/test/data-type.js @@ -209,14 +209,6 @@ 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 }) diff --git a/test/errors.js b/test/errors.js index 2e68dee8..5bfbd003 100644 --- a/test/errors.js +++ b/test/errors.js @@ -1,6 +1,6 @@ import test, { describe } from 'node:test' import assert from 'node:assert/strict' -import { NotFoundError } from '../src/errors.js' +import { NotFoundError, nullIfNotFound } from '../src/errors.js' describe('NotFoundError', () => { test('subclasses Error', () => { @@ -15,3 +15,13 @@ describe('NotFoundError', () => { assert.equal(new NotFoundError('foo').message, 'foo') }) }) + +describe('nullIfNotFound', () => { + test('returns null if passed a NotFoundError', () => { + assert.equal(nullIfNotFound(new NotFoundError()), null) + }) + + test('throws if passed something other than a NotFoundError', () => { + assert.throws(() => nullIfNotFound(new Error('foo')), { message: 'foo' }) + }) +})