From 1c7b7cd9a70384f8ef6843089066b4971cef2c82 Mon Sep 17 00:00:00 2001
From: Evan Hahn <me@evanhahn.com>
Date: Thu, 14 Nov 2024 18:55:23 +0000
Subject: [PATCH] fix: `DataType.prototype.getByVersionId` could return wrong
 type

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`.
---
 src/datatype/index.js |  1 +
 test/data-type.js     | 48 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/src/datatype/index.js b/src/datatype/index.js
index 1784987c..85ea6361 100644
--- a/src/datatype/index.js
+++ b/src/datatype/index.js
@@ -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 })
   }
 
diff --git a/test/data-type.js b/test/data-type.js
index cc709a2f..c546c455 100644
--- a/test/data-type.js
+++ b/test/data-type.js
@@ -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'
@@ -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 = {
@@ -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)
@@ -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()
 
@@ -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,
   })
 
@@ -380,5 +420,5 @@ async function testenv(opts = {}) {
     getTranslations: translationApi.get.bind(translationApi),
   })
 
-  return { coreManager, dataType, dataStore, translationApi }
+  return { coreManager, dataType, dataStore, db, translationApi }
 }