revert: remove getByDocId optional parameter (#968)

This reverts commit 56f7e18 and
replaces it with an internal-only function, `nullIfNotFound`.

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
+}

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 {

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<Role>}
    */
   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(

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 {

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 })

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' })
+  })
+})