fix: Indexes with prefix should match the prefix (#537)

When we create new index after forking a client we create the indexes by
iterating over the value BTree. When there is a prefix present we start
from the prefix but we forgot to end when the key no longer matches the
prefix, leading the index map to contain items that do not match the
index constraints.

src/db/write.ts

@@ -1,39 +1,39 @@
 import type {LogContext} from '@rocicorp/logger';
-import type * as dag from '../dag/mod.js';
+import {assert} from '../asserts.js';
 import * as btree from '../btree/mod.js';
-import * as sync from '../sync/mod.js';
+import {BTreeRead, BTreeWrite} from '../btree/mod.js';
+import type {InternalDiff} from '../btree/node.js';
+import {allEntriesAsDiff} from '../btree/read.js';
+import type * as dag from '../dag/mod.js';
+import {Hash, emptyHash} from '../hash.js';
+import type {IndexDefinition, IndexDefinitions} from '../index-defs.js';
+import type {FrozenJSONValue} from '../json.js';
+import {lazy} from '../lazy.js';
+import type {DiffComputationConfig} from '../sync/diff.js';
 import type {ClientID} from '../sync/mod.js';
+import * as sync from '../sync/mod.js';
 import {
+  ChunkIndexDefinition,
   Commit,
   Meta as CommitMeta,
   IndexRecord,
+  Meta,
+  MetaType,
+  chunkIndexDefinitionEqualIgnoreName,
   newIndexChange as commitNewIndexChange,
-  newLocalSDD as commitNewLocalSDD,
   newLocalDD31 as commitNewLocalDD31,
-  newSnapshotSDD as commitNewSnapshotSDD,
+  newLocalSDD as commitNewLocalSDD,
   newSnapshotDD31 as commitNewSnapshotDD31,
-  MetaType,
+  newSnapshotSDD as commitNewSnapshotSDD,
   toChunkIndexDefinition,
-  ChunkIndexDefinition,
-  chunkIndexDefinitionEqualIgnoreName,
-  Meta,
 } from './commit.js';
+import {IndexOperation, IndexRead, IndexWrite, indexValue} from './index.js';
 import {
   Read,
+  Whence,
   readCommitForBTreeWrite,
   readIndexesForRead,
-  Whence,
 } from './read.js';
-import {IndexWrite, IndexOperation, indexValue, IndexRead} from './index.js';
-import {BTreeRead, BTreeWrite} from '../btree/mod.js';
-import {lazy} from '../lazy.js';
-import {emptyHash, Hash} from '../hash.js';
-import type {InternalDiff} from '../btree/node.js';
-import {allEntriesAsDiff} from '../btree/read.js';
-import {assert} from '../asserts.js';
-import type {IndexDefinition, IndexDefinitions} from '../index-defs.js';
-import type {DiffComputationConfig} from '../sync/diff.js';
-import type {FrozenJSONValue} from '../json.js';
 
 export class Write extends Read {
   private readonly _dagWrite: dag.Write;
@@ -619,11 +619,15 @@ export async function createIndexBTree(
 ): Promise<BTreeWrite> {
   const indexMap = new BTreeWrite(dagWrite);
   for await (const entry of valueMap.scan(prefix)) {
+    const key = entry[0];
+    if (!key.startsWith(prefix)) {
+      break;
+    }
     await indexValue(
       lc,
       indexMap,
       IndexOperation.Add,
-      entry[0],
+      key,
       entry[1],
       jsonPointer,
       allowEmpty,

src/replicache.test.ts

@@ -1,57 +1,57 @@
+import {assert as chaiAssert, expect} from '@esm-bundle/chai';
+import {
+  LicenseStatus,
+  PROD_LICENSE_SERVER_URL,
+  TEST_LICENSE_KEY,
+} from '@rocicorp/licensing/src/client';
+import {
+  LICENSE_ACTIVE_PATH,
+  LICENSE_STATUS_PATH,
+} from '@rocicorp/licensing/src/server/api-types.js';
+import type {LogLevel} from '@rocicorp/logger';
+import * as sinon from 'sinon';
+import {assert} from './asserts.js';
+import {asyncIterableToArray} from './async-iterable-to-array.js';
+import * as db from './db/mod.js';
+import {Hash, emptyHash} from './hash.js';
+import type {JSONValue, ReadonlyJSONValue} from './json.js';
+import {TestMemStore} from './kv/test-mem-store.js';
+import type {ReadTransaction, WriteTransaction} from './mod.js';
+import {
+  ClientID,
+  MutatorDefs,
+  PatchOperation,
+  Poke,
+  Replicache,
+  TransactionClosedError,
+} from './mod.js';
+import {deleteClientForTesting} from './persist/clients-test-helpers.js';
+import {PullResponse, defaultPuller} from './puller.js';
+import {defaultPusher} from './pusher.js';
+import type {ReplicacheOptions} from './replicache-options.js';
 import {httpStatusUnauthorized} from './replicache.js';
+import type {ScanOptions} from './scan-options.js';
+import {sleep} from './sleep.js';
+import type {MutationSDD} from './sync/push.js';
 import {
+  MemStoreWithCounters,
+  ReplicacheTest,
   addData,
   clock,
   expectAsyncFuncToThrow,
   expectLogContext,
   expectPromiseToReject,
   initReplicacheTesting,
   makePullResponse,
-  MemStoreWithCounters,
   replicacheForTesting,
-  ReplicacheTest,
   tickAFewTimes,
   tickUntil,
 } from './test-util.js';
-import {
-  ClientID,
-  MutatorDefs,
-  PatchOperation,
-  Poke,
-  Replicache,
-  TransactionClosedError,
-} from './mod.js';
-import type {ReadTransaction, WriteTransaction} from './mod.js';
-import type {JSONValue} from './json.js';
-import {assert as chaiAssert, expect} from '@esm-bundle/chai';
-import * as sinon from 'sinon';
-import type {ScanOptions} from './scan-options.js';
-import {asyncIterableToArray} from './async-iterable-to-array.js';
-import {sleep} from './sleep.js';
-import * as db from './db/mod.js';
-import {TestMemStore} from './kv/test-mem-store.js';
-import {emptyHash, Hash} from './hash.js';
-import {defaultPuller, PullResponse} from './puller.js';
-import {defaultPusher} from './pusher.js';
-import {
-  PROD_LICENSE_SERVER_URL,
-  TEST_LICENSE_KEY,
-  LicenseStatus,
-} from '@rocicorp/licensing/src/client';
 
 // fetch-mock has invalid d.ts file so we removed that on npm install.
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-expect-error
 import fetchMock from 'fetch-mock/esm/client';
-import type {MutationSDD} from './sync/push.js';
-import type {ReplicacheOptions} from './replicache-options.js';
-import {deleteClientForTesting} from './persist/clients-test-helpers.js';
-import type {LogLevel} from '@rocicorp/logger';
-import {
-  LICENSE_ACTIVE_PATH,
-  LICENSE_STATUS_PATH,
-} from '@rocicorp/licensing/src/server/api-types.js';
-import {assert} from './asserts.js';
 
 const {fail} = chaiAssert;
 
@@ -1304,37 +1304,46 @@ test('index in options', async () => {
   ]);
 });
 
+async function populateDataUsingPull<
+  // eslint-disable-next-line @typescript-eslint/ban-types
+  MD extends MutatorDefs = {},
+>(rep: ReplicacheTest<MD>, data: Record<string, ReadonlyJSONValue>) {
+  fetchMock.postOnce(rep.pullURL, {
+    cookie: '',
+    lastMutationID: 2,
+    patch: Object.entries(data).map(([key, value]) => ({
+      op: 'put',
+      key,
+      value,
+    })),
+  });
+
+  rep.pull();
+
+  // Allow pull to finish (larger than PERSIST_TIMEOUT)
+  // await clock.tickAsync(2 * 1000);
+  await tickAFewTimes(20, 100);
+}
+
 test('allow redefinition of indexes', async () => {
   if (DD31) {
     // TODO(DD31): Without persist we cannot test this.
     return;
   }
 
-  const pullURL = 'https://diff.com/pull';
   const rep = await replicacheForTesting('index-redefinition', {
-    pullURL,
     indexes: {
       aIndex: {jsonPointer: '/a'},
     },
   });
 
-  fetchMock.postOnce(pullURL, {
-    cookie: '',
-    lastMutationID: 2,
-    patch: [
-      {op: 'put', key: 'a/0', value: {a: '0'}},
-      {op: 'put', key: 'a/1', value: {a: '1'}},
-      {op: 'put', key: 'b/2', value: {a: '2'}},
-      {op: 'put', key: 'b/3', value: {a: '3'}},
-    ],
+  await populateDataUsingPull(rep, {
+    'a/0': {a: '0'},
+    'a/1': {a: '1'},
+    'b/2': {a: '2'},
+    'b/3': {a: '3'},
   });
 
-  rep.pull();
-
-  // Allow pull to finish (larger than PERSIST_TIMEOUT)
-  // await clock.tickAsync(2 * 1000);
-  await tickAFewTimes(20, 100);
-
   await testScanResult(rep, {indexName: 'aIndex'}, [
     [['0', 'a/0'], {a: '0'}],
     [['1', 'a/1'], {a: '1'}],
@@ -1361,6 +1370,88 @@ test('allow redefinition of indexes', async () => {
 
   await rep2.close();
 });
+test('add index definition with prefix', async () => {
+  if (DD31) {
+    // TODO(DD31): Without persist we cannot test this.
+    return;
+  }
+
+  const rep = await replicacheForTesting('index-add-more', {
+    mutators: {addData},
+  });
+
+  await populateDataUsingPull(rep, {
+    'a/0': {a: '0'},
+    'a/1': {b: '1'},
+    'b/2': {a: '2'},
+    'b/3': {b: '3'},
+  });
+
+  await rep.close();
+
+  const rep2 = await replicacheForTesting(
+    rep.name,
+    {
+      mutators: {addData},
+      indexes: {
+        aIndex: {jsonPointer: '/a', prefix: 'a'},
+      },
+    },
+    {useUniqueName: false},
+  );
+
+  await testScanResult(rep2, {indexName: 'aIndex'}, [[['0', 'a/0'], {a: '0'}]]);
+
+  await rep2.close();
+});
+
+test('rename indexes', async () => {
+  const rep = await replicacheForTesting('index-add-more', {
+    mutators: {addData},
+    indexes: {
+      aIndex: {jsonPointer: '/a'},
+      bIndex: {jsonPointer: '/b'},
+    },
+  });
+  await populateDataUsingPull(rep, {
+    'a/0': {a: '0'},
+    'b/1': {a: '1'},
+    'b/2': {a: '2'},
+    'b/3': {b: '3'},
+  });
+
+  await testScanResult(rep, {indexName: 'aIndex'}, [
+    [['0', 'a/0'], {a: '0'}],
+    [['1', 'b/1'], {a: '1'}],
+    [['2', 'b/2'], {a: '2'}],
+  ]);
+
+  await testScanResult(rep, {indexName: 'bIndex'}, [[['3', 'b/3'], {b: '3'}]]);
+
+  await rep.close();
+
+  const rep2 = await replicacheForTesting(
+    rep.name,
+    {
+      mutators: {addData},
+      indexes: {
+        bIndex: {jsonPointer: '/a'},
+        aIndex: {jsonPointer: '/b'},
+      },
+    },
+    {useUniqueName: false},
+  );
+
+  await testScanResult(rep2, {indexName: 'bIndex'}, [
+    [['0', 'a/0'], {a: '0'}],
+    [['1', 'b/1'], {a: '1'}],
+    [['2', 'b/2'], {a: '2'}],
+  ]);
+
+  await testScanResult(rep2, {indexName: 'aIndex'}, [[['3', 'b/3'], {b: '3'}]]);
+
+  await rep2.close();
+});
 
 test('index array', async () => {
   const rep = await replicacheForTesting('test-index', {