Skip to content

Commit

Permalink
fix: DataType.prototype.getByVersionId could return wrong type
Browse files Browse the repository at this point in the history
We have many `DataType`s backed by the same `DataStore`. For example,
observations and tracks are both in the "data" store.

`DataType.prototype.getByVersionId` would pull documents from the store
without checking the type, which could cause issues. Simple code
example:

    const track = await trackDataType.create({ /* ... */ })

    await observationDataType.getByVersionId(track.versionId)
    // => the track, unexpectedly!

Now, it throws a `NotFoundError`.
  • Loading branch information
EvanHahn committed Dec 12, 2024
1 parent f073ca9 commit 1c7b7cd
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/datatype/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ export class DataType extends TypedEmitter {
*/
async getByVersionId(versionId, { lang } = {}) {
const result = await this.#dataStore.read(versionId)
if (result.schemaName !== this.#schemaName) throw new NotFoundError()
return this.#translate(result, { lang })
}

Expand Down
48 changes: 44 additions & 4 deletions test/data-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import {
} 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 {
observationTable,
trackTable,
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'
Expand All @@ -19,7 +23,13 @@ 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 {
decode,
decodeBlockPrefix,
parseVersionId,
valueOf,
} from '@comapeo/schema'
import { generate } from '@mapeo/mock-data'

/** @type {import('@comapeo/schema').ObservationValue} */
const obsFixture = {
Expand All @@ -41,6 +51,9 @@ const newObsFixture = {
metadata: { manualLocation: false },
}

/** @type {import('@comapeo/schema').TrackValue} */
const trackFixture = valueOf(generate('track')[0])

test('private createWithDocId() method', async () => {
const sqlite = new Database(':memory:')
const db = drizzle(sqlite)
Expand Down Expand Up @@ -123,6 +136,33 @@ test('getByVersionId fetches docs by their version ID', async () => {
assert.equal(created.docId, fetched.docId)
})

test('getByVersionId returns null if fetching a version ID in the same store, but with a different type', async () => {
const {
dataType: observationDataType,
dataStore,
db,
translationApi,
} = await testenv()
const trackDataType = new DataType({
dataStore,
table: trackTable,
db,
getTranslations: translationApi.get.bind(translationApi),
})

const observation = await observationDataType.create(obsFixture)
const track = await trackDataType.create(trackFixture)

await assert.rejects(
() => observationDataType.getByVersionId(track.versionId),
NotFoundError
)
await assert.rejects(
() => trackDataType.getByVersionId(observation.versionId),
NotFoundError
)
})

test('`originalVersionId` field', async () => {
const { dataType, dataStore } = await testenv()

Expand Down Expand Up @@ -318,7 +358,7 @@ async function testenv(opts = {}) {
const coreManager = createCoreManager({ ...opts, db })

const indexWriter = new IndexWriter({
tables: [observationTable, translationTable],
tables: [observationTable, trackTable, translationTable],
sqlite,
})

Expand Down Expand Up @@ -380,5 +420,5 @@ async function testenv(opts = {}) {
getTranslations: translationApi.get.bind(translationApi),
})

return { coreManager, dataType, dataStore, translationApi }
return { coreManager, dataType, dataStore, db, translationApi }
}

0 comments on commit 1c7b7cd

Please sign in to comment.