From 1a0af6daf1562c3c40c054d5807c37f9fa234dab Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Mon, 18 Nov 2024 22:13:15 +0000 Subject: [PATCH 1/3] revert: remove `getByDocId` optional parameter This reverts commit 56f7e18ae618e6d36dcfdf478ffe42f879aecd2e and replaces it with an internal-only function, `getByDocIdIfExists`. --- src/datatype/get-if-exists.js | 22 +++++ src/datatype/index.d.ts | 8 +- src/datatype/index.js | 31 ++---- src/mapeo-project.js | 9 +- src/roles.js | 13 +-- src/translation-api.js | 3 +- test/data-type/get-if-exists.js | 20 ++++ test/{data-type.js => data-type/index.js} | 110 +++------------------- test/data-type/test-helpers.js | 93 ++++++++++++++++++ 9 files changed, 169 insertions(+), 140 deletions(-) create mode 100644 src/datatype/get-if-exists.js create mode 100644 test/data-type/get-if-exists.js rename test/{data-type.js => data-type/index.js} (73%) create mode 100644 test/data-type/test-helpers.js diff --git a/src/datatype/get-if-exists.js b/src/datatype/get-if-exists.js new file mode 100644 index 000000000..b8cd7130d --- /dev/null +++ b/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} dataType + * @param {string} docId + * @returns {Promise} + */ +export async function getByDocIdIfExists(dataType, docId) { + try { + return await dataType.getByDocId(docId) + } catch (err) { + if (err instanceof NotFoundError) return null + throw err + } +} diff --git a/src/datatype/index.d.ts b/src/datatype/index.d.ts index 39922b265..f7f36a8a9 100644 --- a/src/datatype/index.d.ts +++ b/src/datatype/index.d.ts @@ -18,7 +18,7 @@ import TranslationApi from '../translation-api.js' type MapeoDocTableName = `${MapeoDoc['schemaName']}Table` type GetMapeoDocTables = 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 | GetMapeoDocTables type MapeoDocTablesMap = { @@ -87,12 +87,8 @@ export class DataType< getByDocId( docId: string, - opts?: { mustBeFound?: true; lang?: string } + opts?: { 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 1784987cd..82459ca2c 100644 --- a/src/datatype/index.js +++ b/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} - */ - /** - * @param {string} docId - * @param {object} [options] - * @param {boolean} [options.mustBeFound] - * @param {string} [options.lang] - * @returns {Promise} + * @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, } diff --git a/src/mapeo-project.js b/src/mapeo-project.js index dcb719373..68e5b92fe 100644 --- a/src/mapeo-project.js +++ b/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 { diff --git a/src/roles.js b/src/roles.js index a7fc8dfb3..261eaf5d5 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 { 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} */ 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( diff --git a/src/translation-api.js b/src/translation-api.js index 636a8a20f..a57ec835a 100644 --- a/src/translation-api.js +++ b/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 { diff --git a/test/data-type/get-if-exists.js b/test/data-type/get-if-exists.js new file mode 100644 index 000000000..f34021e06 --- /dev/null +++ b/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)) + }) +}) diff --git a/test/data-type.js b/test/data-type/index.js similarity index 73% rename from test/data-type.js rename to test/data-type/index.js index 1559f7028..00746b449 100644 --- a/test/data-type.js +++ b/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 } -} diff --git a/test/data-type/test-helpers.js b/test/data-type/test-helpers.js new file mode 100644 index 000000000..ea361c1d0 --- /dev/null +++ b/test/data-type/test-helpers.js @@ -0,0 +1,93 @@ +import assert from 'node:assert/strict' +import RAM from 'random-access-memory' +import { DataStore } from '../../src/datastore/index.js' +import { DataType } from '../../src/datatype/index.js' +import { IndexWriter } from '../../src/index-writer/index.js' +import { observationTable, translationTable } from '../../src/schema/project.js' +import { createCoreManager } from '../helpers/core-manager.js' + +import { decode, decodeBlockPrefix } from '@comapeo/schema' +import Database from 'better-sqlite3' +import { drizzle } from 'drizzle-orm/better-sqlite3' +import { migrate } from 'drizzle-orm/better-sqlite3/migrator' +import TranslationApi from '../../src/translation-api.js' + +/** + * @param {object} [opts={}] + * @param {Buffer} [opts.projectKey] + */ +export 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 } +} From a652b0570e7d9efc6d5fd1d11f0d60867ca09b23 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Wed, 20 Nov 2024 16:19:25 +0000 Subject: [PATCH 2/3] nullIfNotFound instead --- src/datatype/get-if-exists.js | 22 ----- src/errors.js | 9 ++ src/mapeo-project.js | 11 ++- src/roles.js | 10 ++- src/translation-api.js | 4 +- test/{data-type/index.js => data-type.js} | 102 +++++++++++++++++++--- test/data-type/get-if-exists.js | 20 ----- test/data-type/test-helpers.js | 93 -------------------- test/errors.js | 12 ++- 9 files changed, 125 insertions(+), 158 deletions(-) delete mode 100644 src/datatype/get-if-exists.js rename test/{data-type/index.js => data-type.js} (75%) delete mode 100644 test/data-type/get-if-exists.js delete mode 100644 test/data-type/test-helpers.js diff --git a/src/datatype/get-if-exists.js b/src/datatype/get-if-exists.js deleted file mode 100644 index b8cd7130d..000000000 --- a/src/datatype/get-if-exists.js +++ /dev/null @@ -1,22 +0,0 @@ -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} dataType - * @param {string} docId - * @returns {Promise} - */ -export async function getByDocIdIfExists(dataType, docId) { - try { - return await dataType.getByDocId(docId) - } catch (err) { - if (err instanceof NotFoundError) return null - throw err - } -} diff --git a/src/errors.js b/src/errors.js index 65bd6c968..38bc87640 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 68e5b92fe..da236e91a 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -56,8 +56,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 { getByDocIdIfExists } from './datatype/get-if-exists.js' +import { NotFoundError, nullIfNotFound } from './errors.js' /** @import { ProjectSettingsValue } from '@comapeo/schema' */ /** @import { CoreStorage, KeyPair, Namespace, ReplicationStream } from './types.js' */ @@ -601,7 +600,9 @@ export class MapeoProject extends TypedEmitter { async $setProjectSettings(settings) { const { projectSettings } = this.#dataTypes - const existing = await getByDocIdIfExists(projectSettings, this.#projectId) + const existing = await projectSettings + .getByDocId(this.#projectId) + .catch(nullIfNotFound) if (existing) { return extractEditableProjectSettings( @@ -709,7 +710,9 @@ export class MapeoProject extends TypedEmitter { schemaName: /** @type {const} */ ('deviceInfo'), } - const existingDoc = await getByDocIdIfExists(deviceInfo, configCoreId) + 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 261eaf5d5..596f8a456 100644 --- a/src/roles.js +++ b/src/roles.js @@ -2,7 +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 { nullIfNotFound } from './errors.js' import { TypedEmitter } from 'tiny-typed-emitter' /** @import { Namespace } from './types.js' */ @@ -270,7 +270,9 @@ export class Roles extends TypedEmitter { * @returns {Promise} */ async getRole(deviceId) { - const roleRecord = await getByDocIdIfExists(this.#dataType, deviceId) + 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') @@ -385,7 +387,9 @@ export class Roles extends TypedEmitter { } } - const existingRoleDoc = await getByDocIdIfExists(this.#dataType, deviceId) + 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 a57ec835a..e32e6fe09 100644 --- a/src/translation-api.js +++ b/src/translation-api.js @@ -1,7 +1,7 @@ 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 { nullIfNotFound } from './errors.js' import { omit } from './lib/omit.js' /** @import { Translation, TranslationValue } from '@comapeo/schema' */ /** @import { SetOptional } from 'type-fest' */ @@ -50,7 +50,7 @@ export default class TranslationApi { async put(value) { const identifiers = omit(value, ['message']) const docId = hashObject(identifiers) - const doc = await getByDocIdIfExists(this.#dataType, docId) + 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/index.js b/test/data-type.js similarity index 75% rename from test/data-type/index.js rename to test/data-type.js index 00746b449..cc709a2fb 100644 --- a/test/data-type/index.js +++ b/test/data-type.js @@ -1,26 +1,25 @@ 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 } 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, 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 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 { parseVersionId } from '@comapeo/schema' - -import { testenv } from './test-helpers.js' +import { decode, decodeBlockPrefix, parseVersionId } from '@comapeo/schema' /** @type {import('@comapeo/schema').ObservationValue} */ const obsFixture = { @@ -46,8 +45,7 @@ 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() @@ -83,8 +81,7 @@ 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() @@ -306,3 +303,82 @@ 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 } +} diff --git a/test/data-type/get-if-exists.js b/test/data-type/get-if-exists.js deleted file mode 100644 index f34021e06..000000000 --- a/test/data-type/get-if-exists.js +++ /dev/null @@ -1,20 +0,0 @@ -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)) - }) -}) diff --git a/test/data-type/test-helpers.js b/test/data-type/test-helpers.js deleted file mode 100644 index ea361c1d0..000000000 --- a/test/data-type/test-helpers.js +++ /dev/null @@ -1,93 +0,0 @@ -import assert from 'node:assert/strict' -import RAM from 'random-access-memory' -import { DataStore } from '../../src/datastore/index.js' -import { DataType } from '../../src/datatype/index.js' -import { IndexWriter } from '../../src/index-writer/index.js' -import { observationTable, translationTable } from '../../src/schema/project.js' -import { createCoreManager } from '../helpers/core-manager.js' - -import { decode, decodeBlockPrefix } from '@comapeo/schema' -import Database from 'better-sqlite3' -import { drizzle } from 'drizzle-orm/better-sqlite3' -import { migrate } from 'drizzle-orm/better-sqlite3/migrator' -import TranslationApi from '../../src/translation-api.js' - -/** - * @param {object} [opts={}] - * @param {Buffer} [opts.projectKey] - */ -export 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 } -} diff --git a/test/errors.js b/test/errors.js index 2e68dee80..5bfbd003f 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' }) + }) +}) From 7cd7e7087d5ce83901d1fee96c8173f0df7417b7 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Wed, 20 Nov 2024 16:21:49 +0000 Subject: [PATCH 3/3] More reversions --- src/datatype/index.d.ts | 8 ++++++-- src/datatype/index.js | 31 ++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/datatype/index.d.ts b/src/datatype/index.d.ts index f7f36a8a9..39922b265 100644 --- a/src/datatype/index.d.ts +++ b/src/datatype/index.d.ts @@ -18,7 +18,7 @@ import TranslationApi from '../translation-api.js' type MapeoDocTableName = `${MapeoDoc['schemaName']}Table` type GetMapeoDocTables = T[keyof T & MapeoDocTableName] /** Union of Drizzle schema tables that correspond to MapeoDoc types (e.g. excluding backlink tables and other utility tables) */ -export type MapeoDocTables = +type MapeoDocTables = | GetMapeoDocTables | GetMapeoDocTables type MapeoDocTablesMap = { @@ -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, }