Skip to content

Commit b7f1313

Browse files
authored
🔎 fix: Specify Explicit Primary Key for Meilisearch Document Operations (danny-avila#12542)
* fix: pass explicit primaryKey to Meilisearch addDocuments/updateDocuments calls Meilisearch v1.0+ refuses to auto-infer the primary key when a document contains multiple fields ending with 'id'. The messages index has both conversationId and messageId, causing addDocuments to silently fail with index_primary_key_multiple_candidates_found, leaving message search empty. Pass { primaryKey } to addDocumentsInBatches, addDocuments, and updateDocuments — the variable was already in scope. Also replace raw this.collection.updateMany with Mongoose Model.updateMany to satisfy the no-restricted-syntax ESLint rule (tenant isolation guard). Closes danny-avila#12538 * fix: resolve additional Meilisearch plugin bugs found in review Address review findings from PR danny-avila#12542: - Fix deleteObjectFromMeili using MongoDB _id instead of the Meilisearch primary key (conversationId/messageId), causing post-remove cleanup to silently no-op and leave orphaned documents in the index. - Pass options.primaryKey explicitly to createMeiliMongooseModel factory instead of deriving it from attributesToIndex[0] (schema field order), eliminating a fragile implicit contract. - Fix updateObjectToMeili skipping preprocessObjectForIndex, which meant updates bypassed content array-to-text conversion and conversationId pipe character escaping. - Change collection.updateMany to collection.updateOne in addObjectToMeili since _id is unique (semantic correctness). - Add primaryKey to validateOptions required keys. - Strengthen test assertions to verify { primaryKey } argument is passed to addDocuments, addDocumentsInBatches, and updateDocuments. Add tests for the update path including preprocessObjectForIndex pipe escaping. * fix: add regression tests for delete and message update paths Address follow-up review findings: - Add test for deleteObjectFromMeili verifying it uses messageId (not MongoDB _id) when calling index.deleteDocument, guarding against regression of the silent orphaned-document bug. - Add test for message model update path asserting { primaryKey: 'messageId' } is passed to updateDocuments (previously only the conversation model update path was tested). - Add @param config.primaryKey to createMeiliMongooseModel JSDoc.
1 parent c171fd8 commit b7f1313

2 files changed

Lines changed: 105 additions & 15 deletions

File tree

‎packages/data-schemas/src/models/plugins/mongoMeili.spec.ts‎

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,10 @@ describe('Meilisearch Mongoose plugin', () => {
7373
title: 'Test Conversation',
7474
endpoint: EModelEndpoint.openAI,
7575
});
76-
expect(mockAddDocuments).toHaveBeenCalled();
76+
expect(mockAddDocuments).toHaveBeenCalledWith(
77+
[expect.objectContaining({ conversationId: expect.anything() })],
78+
{ primaryKey: 'conversationId' },
79+
);
7780
});
7881

7982
test('saving conversation indexes with expiredAt=null w/ meilisearch', async () => {
@@ -105,7 +108,10 @@ describe('Meilisearch Mongoose plugin', () => {
105108
user: new mongoose.Types.ObjectId(),
106109
isCreatedByUser: true,
107110
});
108-
expect(mockAddDocuments).toHaveBeenCalled();
111+
expect(mockAddDocuments).toHaveBeenCalledWith(
112+
[expect.objectContaining({ messageId: expect.anything() })],
113+
{ primaryKey: 'messageId' },
114+
);
109115
});
110116

111117
test('saving messages with expiredAt=null indexes w/ meilisearch', async () => {
@@ -130,6 +136,87 @@ describe('Meilisearch Mongoose plugin', () => {
130136
expect(mockAddDocuments).not.toHaveBeenCalled();
131137
});
132138

139+
test('updating an indexed conversation calls updateDocuments with primaryKey', async () => {
140+
const conversationModel = createConversationModel(mongoose);
141+
const convo = await conversationModel.create({
142+
conversationId: new mongoose.Types.ObjectId().toString(),
143+
user: new mongoose.Types.ObjectId(),
144+
title: 'Original Title',
145+
endpoint: EModelEndpoint.openAI,
146+
});
147+
mockUpdateDocuments.mockClear();
148+
149+
convo._meiliIndex = true;
150+
convo.title = 'Updated Title';
151+
await convo.save();
152+
153+
expect(mockUpdateDocuments).toHaveBeenCalledWith(
154+
[expect.objectContaining({ conversationId: expect.anything() })],
155+
{ primaryKey: 'conversationId' },
156+
);
157+
});
158+
159+
test('updating an indexed message calls updateDocuments with primaryKey: messageId', async () => {
160+
const messageModel = createMessageModel(mongoose);
161+
const msg = await messageModel.create({
162+
messageId: new mongoose.Types.ObjectId().toString(),
163+
conversationId: new mongoose.Types.ObjectId(),
164+
user: new mongoose.Types.ObjectId(),
165+
isCreatedByUser: true,
166+
});
167+
mockUpdateDocuments.mockClear();
168+
169+
msg._meiliIndex = true;
170+
msg.text = 'Updated text';
171+
await msg.save();
172+
173+
expect(mockUpdateDocuments).toHaveBeenCalledWith(
174+
[expect.objectContaining({ messageId: expect.anything() })],
175+
{ primaryKey: 'messageId' },
176+
);
177+
});
178+
179+
test('deleteObjectFromMeili calls deleteDocument with messageId, not _id', async () => {
180+
const messageModel = createMessageModel(mongoose);
181+
const msgId = new mongoose.Types.ObjectId().toString();
182+
const msg = await messageModel.create({
183+
messageId: msgId,
184+
conversationId: new mongoose.Types.ObjectId(),
185+
user: new mongoose.Types.ObjectId(),
186+
isCreatedByUser: true,
187+
});
188+
mockDeleteDocument.mockClear();
189+
190+
const typedMsg = msg as unknown as import('./mongoMeili').DocumentWithMeiliIndex;
191+
await new Promise<void>((resolve, reject) => {
192+
typedMsg.deleteObjectFromMeili!((err) => (err ? reject(err) : resolve()));
193+
});
194+
195+
expect(mockDeleteDocument).toHaveBeenCalledWith(msgId);
196+
expect(mockDeleteDocument).not.toHaveBeenCalledWith(String(msg._id));
197+
});
198+
199+
test('updateDocuments receives preprocessed data with primaryKey', async () => {
200+
const conversationModel = createConversationModel(mongoose);
201+
const conversationId = 'abc|def|ghi';
202+
const convo = await conversationModel.create({
203+
conversationId,
204+
user: new mongoose.Types.ObjectId(),
205+
title: 'Pipe Test',
206+
endpoint: EModelEndpoint.openAI,
207+
});
208+
mockUpdateDocuments.mockClear();
209+
210+
convo._meiliIndex = true;
211+
convo.title = 'Updated Pipe Test';
212+
await convo.save();
213+
214+
expect(mockUpdateDocuments).toHaveBeenCalledWith(
215+
[expect.objectContaining({ conversationId: 'abc--def--ghi' })],
216+
{ primaryKey: 'conversationId' },
217+
);
218+
});
219+
133220
test('sync w/ meili does not include TTL documents', async () => {
134221
const conversationModel = createConversationModel(mongoose) as SchemaWithMeiliMethods;
135222
await conversationModel.create({
@@ -299,8 +386,10 @@ describe('Meilisearch Mongoose plugin', () => {
299386
// Run sync which should call processSyncBatch internally
300387
await conversationModel.syncWithMeili();
301388

302-
// Verify addDocumentsInBatches was called (new batch method)
303-
expect(mockAddDocumentsInBatches).toHaveBeenCalled();
389+
// Verify addDocumentsInBatches was called with explicit primaryKey
390+
expect(mockAddDocumentsInBatches).toHaveBeenCalledWith(expect.any(Array), undefined, {
391+
primaryKey: 'conversationId',
392+
});
304393
});
305394

306395
test('addObjectToMeili retries on failure', async () => {

‎packages/data-schemas/src/models/plugins/mongoMeili.ts‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ const getSyncConfig = () => ({
9494
* Validates the required options for configuring the mongoMeili plugin.
9595
*/
9696
const validateOptions = (options: Partial<MongoMeiliOptions>): void => {
97-
const requiredKeys: (keyof MongoMeiliOptions)[] = ['host', 'apiKey', 'indexName'];
97+
const requiredKeys: (keyof MongoMeiliOptions)[] = ['host', 'apiKey', 'indexName', 'primaryKey'];
9898
requiredKeys.forEach((key) => {
9999
if (!options[key]) {
100100
throw new Error(`Missing mongoMeili Option: ${key}`);
@@ -130,19 +130,21 @@ const processBatch = async <T>(
130130
* @param config - Configuration object.
131131
* @param config.index - The MeiliSearch index object.
132132
* @param config.attributesToIndex - List of attributes to index.
133+
* @param config.primaryKey - The primary key field for MeiliSearch document operations.
133134
* @param config.syncOptions - Sync configuration options.
134135
* @returns A class definition that will be loaded into the Mongoose schema.
135136
*/
136137
const createMeiliMongooseModel = ({
137138
index,
138139
attributesToIndex,
140+
primaryKey,
139141
syncOptions,
140142
}: {
141143
index: Index<MeiliIndexable>;
142144
attributesToIndex: string[];
145+
primaryKey: string;
143146
syncOptions: { batchSize: number; delayMs: number };
144147
}) => {
145-
const primaryKey = attributesToIndex[0];
146148
const syncConfig = { ...getSyncConfig(), ...syncOptions };
147149

148150
class MeiliMongooseModel {
@@ -255,7 +257,7 @@ const createMeiliMongooseModel = ({
255257

256258
try {
257259
// Add documents to MeiliSearch
258-
await index.addDocumentsInBatches(formattedDocs);
260+
await index.addDocumentsInBatches(formattedDocs, undefined, { primaryKey });
259261

260262
// Update MongoDB to mark documents as indexed.
261263
// { timestamps: false } prevents Mongoose from touching updatedAt, preserving
@@ -422,7 +424,7 @@ const createMeiliMongooseModel = ({
422424

423425
while (retryCount < maxRetries) {
424426
try {
425-
await index.addDocuments([object]);
427+
await index.addDocuments([object], { primaryKey });
426428
break;
427429
} catch (error) {
428430
retryCount++;
@@ -436,7 +438,8 @@ const createMeiliMongooseModel = ({
436438
}
437439

438440
try {
439-
await this.collection.updateMany(
441+
// eslint-disable-next-line no-restricted-syntax -- _meiliIndex is an internal bookkeeping flag, not tenant-scoped data
442+
await this.collection.updateOne(
440443
{ _id: this._id as Types.ObjectId },
441444
{ $set: { _meiliIndex: true } },
442445
);
@@ -456,10 +459,8 @@ const createMeiliMongooseModel = ({
456459
next: CallbackWithoutResultAndOptionalError,
457460
): Promise<void> {
458461
try {
459-
const object = _.omitBy(_.pick(this.toJSON(), attributesToIndex), (v, k) =>
460-
k.startsWith('$'),
461-
);
462-
await index.updateDocuments([object]);
462+
const object = this.preprocessObjectForIndex!();
463+
await index.updateDocuments([object], { primaryKey });
463464
next();
464465
} catch (error) {
465466
logger.error('[updateObjectToMeili] Error updating document in Meili:', error);
@@ -477,7 +478,7 @@ const createMeiliMongooseModel = ({
477478
next: CallbackWithoutResultAndOptionalError,
478479
): Promise<void> {
479480
try {
480-
await index.deleteDocument(this._id as string);
481+
await index.deleteDocument(String(this[primaryKey as keyof DocumentWithMeiliIndex]));
481482
next();
482483
} catch (error) {
483484
logger.error('[deleteObjectFromMeili] Error deleting document from Meili:', error);
@@ -643,7 +644,7 @@ export default function mongoMeili(schema: Schema, options: MongoMeiliOptions):
643644
logger.debug(`[mongoMeili] Added 'user' field to ${indexName} index attributes`);
644645
}
645646

646-
schema.loadClass(createMeiliMongooseModel({ index, attributesToIndex, syncOptions }));
647+
schema.loadClass(createMeiliMongooseModel({ index, attributesToIndex, primaryKey, syncOptions }));
647648

648649
// Register Mongoose hooks
649650
schema.post('save', function (doc: DocumentWithMeiliIndex, next) {

0 commit comments

Comments
 (0)