revert: remove getByDocId optional parameter

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

src/datatype/get-if-exists.js

@@ -0,0 +1,22 @@
+import { NotFoundError } from '../errors.js'
+/** @import { DataStore } from '../datastore/index.js' */
+/** @import { MapeoDocMap, MapeoValueMap } from '../types.js' */
+/** @import { DataType, MapeoDocTables } from './index.js' */
+
+/**
+ * @template {MapeoDocTables} TTable
+ * @template {TTable['_']['name']} TSchemaName
+ * @template {MapeoDocMap[TSchemaName]} TDoc
+ * @template {MapeoValueMap[TSchemaName]} TValue
+ * @param {DataType<DataStore, TTable, TSchemaName, TDoc, TValue>} dataType
+ * @param {string} docId
+ * @returns {Promise<null | TDoc & { forks: string[] }>}
+ */
+export async function getByDocIdIfExists(dataType, docId) {
+  try {
+    return await dataType.getByDocId(docId)
+  } catch (err) {
+    if (err instanceof NotFoundError) return null
+    throw err
+  }
+}

src/datatype/index.d.ts

@@ -18,7 +18,7 @@ import TranslationApi from '../translation-api.js'
 type MapeoDocTableName = `${MapeoDoc['schemaName']}Table`
 type GetMapeoDocTables<T> = T[keyof T & MapeoDocTableName]
 /** Union of Drizzle schema tables that correspond to MapeoDoc types (e.g. excluding backlink tables and other utility tables) */
-type MapeoDocTables =
+export type MapeoDocTables =
   | GetMapeoDocTables<typeof import('../schema/project.js')>
   | GetMapeoDocTables<typeof import('../schema/client.js')>
 type MapeoDocTablesMap = {
@@ -87,12 +87,8 @@ export class DataType<
 
   getByDocId(
     docId: string,
-    opts?: { mustBeFound?: true; lang?: string }
+    opts?: { lang?: string }
   ): Promise<TDoc & { forks: string[] }>
-  getByDocId(
-    docId: string,
-    opts?: { mustBeFound?: boolean; lang?: string }
-  ): Promise<null | (TDoc & { forks: string[] })>
 
   getByVersionId(versionId: string, opts?: { lang?: string }): Promise<TDoc>
 

src/datatype/index.js

@@ -164,30 +164,16 @@ export class DataType extends TypedEmitter {
   }
 
   /**
-   * @overload
    * @param {string} docId
-   * @param {object} [options]
-   * @param {true} [options.mustBeFound]
-   * @param {string} [options.lang]
-   * @returns {Promise<TDoc & { forks: string[] }>}
-   */
-  /**
-   * @param {string} docId
-   * @param {object} [options]
-   * @param {boolean} [options.mustBeFound]
-   * @param {string} [options.lang]
-   * @returns {Promise<null | (TDoc & { forks: string[] })>}
+   * @param {{ lang?: string }} [opts]
    */
-  async getByDocId(docId, { mustBeFound = true, lang } = {}) {
+  async getByDocId(docId, { lang } = {}) {
     await this.#dataStore.indexer.idle()
-    const result = this.#sql.getByDocId.get({ docId })
-    if (result) {
-      return this.#translate(deNullify(result), { lang })
-    } else if (mustBeFound) {
-      throw new NotFoundError()
-    } else {
-      return null
-    }
+    const result = /** @type {undefined | MapeoDoc} */ (
+      this.#sql.getByDocId.get({ docId })
+    )
+    if (!result) throw new NotFoundError()
+    return this.#translate(deNullify(result), { lang })
   }
 
   /**
@@ -200,7 +186,7 @@ export class DataType extends TypedEmitter {
   }
 
   /**
-   * @param {any} doc
+   * @param {MapeoDoc} doc
    * @param {{ lang?: string }} [opts]
    */
   async #translate(doc, { lang } = {}) {
@@ -292,6 +278,7 @@ 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,
     }

src/mapeo-project.js

@@ -57,6 +57,7 @@ import { IconApi } from './icon-api.js'
 import { readConfig } from './config-import.js'
 import TranslationApi from './translation-api.js'
 import { NotFoundError } from './errors.js'
+import { getByDocIdIfExists } from './datatype/get-if-exists.js'
 /** @import { ProjectSettingsValue } from '@comapeo/schema' */
 /** @import { CoreStorage, KeyPair, Namespace, ReplicationStream } from './types.js' */
 
@@ -600,9 +601,7 @@ export class MapeoProject extends TypedEmitter {
   async $setProjectSettings(settings) {
     const { projectSettings } = this.#dataTypes
 
-    const existing = await projectSettings.getByDocId(this.#projectId, {
-      mustBeFound: false,
-    })
+    const existing = await getByDocIdIfExists(projectSettings, this.#projectId)
 
     if (existing) {
       return extractEditableProjectSettings(
@@ -710,9 +709,7 @@ export class MapeoProject extends TypedEmitter {
       schemaName: /** @type {const} */ ('deviceInfo'),
     }
 
-    const existingDoc = await deviceInfo.getByDocId(configCoreId, {
-      mustBeFound: false,
-    })
+    const existingDoc = await getByDocIdIfExists(deviceInfo, configCoreId)
     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 { getByDocIdIfExists } from './datatype/get-if-exists.js'
 import { TypedEmitter } from 'tiny-typed-emitter'
 /** @import { Namespace } from './types.js' */
 
@@ -269,10 +270,8 @@ 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 getByDocIdIfExists(this.#dataType, deviceId)
+    if (!roleRecord) {
       // The project creator will have the creator role
       const authCoreId = await this.#coreOwnership.getCoreId(deviceId, 'auth')
       if (authCoreId === this.#projectCreatorAuthCoreId) {
@@ -284,7 +283,7 @@ export class Roles extends TypedEmitter {
       }
     }
 
-    const { roleId } = roleAssignment
+    const { roleId } = roleRecord
     if (!isRoleId(roleId)) {
       return BLOCKED_ROLE
     }
@@ -386,9 +385,7 @@ export class Roles extends TypedEmitter {
       }
     }
 
-    const existingRoleDoc = await this.#dataType.getByDocId(deviceId, {
-      mustBeFound: false,
-    })
+    const existingRoleDoc = await getByDocIdIfExists(this.#dataType, deviceId)
 
     if (existingRoleDoc) {
       await this.#dataType.update(

src/translation-api.js

@@ -1,5 +1,6 @@
 import { and, sql } from 'drizzle-orm'
 import { kCreateWithDocId, kSelect } from './datatype/index.js'
+import { getByDocIdIfExists } from './datatype/get-if-exists.js'
 import { hashObject } from './utils.js'
 import { omit } from './lib/omit.js'
 /** @import { Translation, TranslationValue } from '@comapeo/schema' */
@@ -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 getByDocIdIfExists(this.#dataType, docId)
     if (doc) {
       return await this.#dataType.update(doc.versionId, value)
     } else {

test/data-type/get-if-exists.js

@@ -0,0 +1,20 @@
+import { testenv } from './test-helpers.js'
+import test, { describe } from 'node:test'
+import assert from 'node:assert/strict'
+import { getByDocIdIfExists } from '../../src/datatype/get-if-exists.js'
+import { valueOf } from '@comapeo/schema'
+import { generate } from '@mapeo/mock-data'
+
+describe('getByDocIdIfExists', () => {
+  test('resolves with null if no document exists with that ID', async () => {
+    const { dataType } = await testenv()
+    assert.equal(await getByDocIdIfExists(dataType, 'foo bar'), null)
+  })
+
+  test('resolves with the document if it exists', async () => {
+    const { dataType } = await testenv()
+    const fixture = valueOf(generate('observation')[0])
+    const observation = await dataType.create(fixture)
+    assert(await getByDocIdIfExists(dataType, observation.docId))
+  })
+})

test/data-type.js → test/data-type/index.js

@@ -1,25 +1,26 @@
 import test from 'node:test'
 import assert from 'node:assert/strict'
-import { DataStore } from '../src/datastore/index.js'
+import { DataStore } from '../../src/datastore/index.js'
 import {
   createCoreManager,
   waitForCores,
   replicate,
-} from './helpers/core-manager.js'
+} from '../helpers/core-manager.js'
 import RAM from 'random-access-memory'
 import crypto from 'hypercore-crypto'
-import { observationTable, translationTable } from '../src/schema/project.js'
-import { DataType, kCreateWithDocId } from '../src/datatype/index.js'
-import { IndexWriter } from '../src/index-writer/index.js'
-import { NotFoundError } from '../src/errors.js'
+import { observationTable } from '../../src/schema/project.js'
+import { DataType, kCreateWithDocId } from '../../src/datatype/index.js'
+import { IndexWriter } from '../../src/index-writer/index.js'
+import { NotFoundError } from '../../src/errors.js'
 
 import Database from 'better-sqlite3'
 import { drizzle } from 'drizzle-orm/better-sqlite3'
 import { migrate } from 'drizzle-orm/better-sqlite3/migrator'
 import { randomBytes } from 'crypto'
-import TranslationApi from '../src/translation-api.js'
 import { getProperty } from 'dot-prop'
-import { decode, decodeBlockPrefix, parseVersionId } from '@comapeo/schema'
+import { parseVersionId } from '@comapeo/schema'
+
+import { testenv } from './test-helpers.js'
 
 /** @type {import('@comapeo/schema').ObservationValue} */
 const obsFixture = {
@@ -45,7 +46,8 @@ test('private createWithDocId() method', async () => {
   const sqlite = new Database(':memory:')
   const db = drizzle(sqlite)
   migrate(db, {
-    migrationsFolder: new URL('../drizzle/project', import.meta.url).pathname,
+    migrationsFolder: new URL('../../drizzle/project', import.meta.url)
+      .pathname,
   })
 
   const coreManager = createCoreManager()
@@ -81,7 +83,8 @@ test('private createWithDocId() method throws when doc exists', async () => {
   const sqlite = new Database(':memory:')
   const db = drizzle(sqlite)
   migrate(db, {
-    migrationsFolder: new URL('../drizzle/project', import.meta.url).pathname,
+    migrationsFolder: new URL('../../drizzle/project', import.meta.url)
+      .pathname,
   })
 
   const coreManager = createCoreManager()
@@ -209,14 +212,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 })
@@ -311,82 +306,3 @@ test('translation', async () => {
     `not passing a a language code returns the untranslated message`
   )
 })
-
-/**
- * @param {object} [opts={}]
- * @param {Buffer} [opts.projectKey]
- */
-async function testenv(opts = {}) {
-  const sqlite = new Database(':memory:')
-  const db = drizzle(sqlite)
-  migrate(db, {
-    migrationsFolder: new URL('../drizzle/project', import.meta.url).pathname,
-  })
-
-  const coreManager = createCoreManager({ ...opts, db })
-
-  const indexWriter = new IndexWriter({
-    tables: [observationTable, translationTable],
-    sqlite,
-  })
-
-  const dataStore = new DataStore({
-    coreManager,
-    namespace: 'data',
-    batch: async (entries) => indexWriter.batch(entries),
-    storage: () => new RAM(),
-    reindex: false,
-  })
-
-  const configDataStore = new DataStore({
-    coreManager,
-    namespace: 'config',
-    batch: async (entries) => {
-      /** @type {import('multi-core-indexer').Entry[]} */
-      const entriesToIndex = []
-      for (const entry of entries) {
-        const { schemaName } = decodeBlockPrefix(entry.block)
-        try {
-          if (schemaName === 'translation') {
-            const doc = decode(entry.block, {
-              coreDiscoveryKey: entry.key,
-              index: entry.index,
-            })
-            assert(
-              doc.schemaName === 'translation',
-              'expected a translation doc'
-            )
-            translationApi.index(doc)
-          }
-          entriesToIndex.push(entry)
-        } catch {
-          // Ignore errors thrown by values that can't be decoded for now
-        }
-      }
-      const indexed = await indexWriter.batch(entriesToIndex)
-      return indexed
-    },
-    storage: () => new RAM(),
-    reindex: false,
-  })
-
-  const translationDataType = new DataType({
-    dataStore: configDataStore,
-    table: translationTable,
-    db,
-    getTranslations: () => {
-      throw new Error('Cannot get translations for translations')
-    },
-  })
-
-  const translationApi = new TranslationApi({ dataType: translationDataType })
-
-  const dataType = new DataType({
-    dataStore,
-    table: observationTable,
-    db,
-    getTranslations: translationApi.get.bind(translationApi),
-  })
-
-  return { coreManager, dataType, dataStore, translationApi }
-}