Skip to content

Commit

Permalink
feat: getByDocId can optionally throw
Browse files Browse the repository at this point in the history
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]: #188
  • Loading branch information
EvanHahn committed Nov 12, 2024
1 parent 6a3bb5b commit d260252
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 49 deletions.
6 changes: 5 additions & 1 deletion src/datatype/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,12 @@ export class DataType<

getByDocId(
docId: string,
opts?: { lang?: string }
opts?: { mustBeFound?: true; 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>

Expand Down
31 changes: 22 additions & 9 deletions src/datatype/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<TDoc & { forks: string[] }>}
*/
/**
* @param {string} docId
* @param {object} [options]
* @param {boolean} [options.mustBeFound]
* @param {string} [options.lang]
* @returns {Promise<null | (TDoc & { forks: string[] })>}
*/
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
}
}

/**
Expand All @@ -186,7 +200,7 @@ export class DataType extends TypedEmitter {
}

/**
* @param {MapeoDoc} doc
* @param {any} doc
* @param {{ lang?: string }} [opts]
*/
async #translate(doc, { lang } = {}) {
Expand Down Expand Up @@ -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,
}
Expand Down
38 changes: 17 additions & 21 deletions src/mapeo-project.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export const kClearDataIfLeft = Symbol('clear data if left project')
export const kSetIsArchiveDevice = Symbol('set isArchiveDevice')
export const kIsArchiveDevice = Symbol('isArchiveDevice (temp - test only)')

/** @type {EditableProjectSettings} */
const EMPTY_PROJECT_SETTINGS = Object.freeze({})

/**
Expand Down Expand Up @@ -599,14 +600,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(
Expand All @@ -629,13 +625,13 @@ export class MapeoProject extends TypedEmitter {
* @returns {Promise<EditableProjectSettings>}
*/
async $getProjectSettings() {
try {
return extractEditableProjectSettings(
await this.#dataTypes.projectSettings.getByDocId(this.#projectId)
)
} catch (e) {
return /** @type {EditableProjectSettings} */ (EMPTY_PROJECT_SETTINGS)
}
const projectSettingsDoc = await this.#dataTypes.projectSettings.getByDocId(
this.#projectId,
{ mustBeFound: false }
)
return projectSettingsDoc
? extractEditableProjectSettings(projectSettingsDoc)
: EMPTY_PROJECT_SETTINGS
}

/**
Expand Down Expand Up @@ -714,14 +710,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 */
Expand Down
18 changes: 9 additions & 9 deletions src/roles.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,10 @@ export class Roles extends TypedEmitter {
* @returns {Promise<Role>}
*/
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) {
Expand All @@ -280,6 +278,8 @@ export class Roles extends TypedEmitter {
return NO_ROLE
}
}

const { roleId } = roleAssignment
if (!isRoleId(roleId)) {
return ROLES[BLOCKED_ROLE_ID]
}
Expand Down Expand Up @@ -381,9 +381,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(
Expand Down
13 changes: 4 additions & 9 deletions src/translation-api.js
Original file line number Diff line number Diff line change
@@ -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' */
Expand Down Expand Up @@ -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)
}
}

Expand Down
8 changes: 8 additions & 0 deletions test/data-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,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 })
Expand Down

0 comments on commit d260252

Please sign in to comment.