🔎 fix: Specify Explicit Primary Key for Meilisearch Document Operations#12542
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes MeiliSearch message indexing/search on MeiliSearch v1.0+ by explicitly passing the index primaryKey to all document mutation calls in the mongoMeili Mongoose plugin, avoiding primary-key inference failures when multiple *id fields exist (e.g., messageId + conversationId).
Changes:
- Pass
{ primaryKey }toaddDocumentsInBatchesin the bulk sync path (processSyncBatch). - Pass
{ primaryKey }toaddDocuments(post-save hook) andupdateDocuments(post-update hook). - Replace
this.collection.updateManywithModel.updateMany(..., { timestamps: false })to avoid raw driver calls that bypass Mongoose middleware (tenant isolation).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| // Add documents to MeiliSearch | ||
| await index.addDocumentsInBatches(formattedDocs); | ||
| await index.addDocumentsInBatches(formattedDocs, undefined, { primaryKey }); |
There was a problem hiding this comment.
Test coverage: the new behavior relies on passing { primaryKey } into Meili document mutation calls. The existing mongoMeili.spec.ts assertions only check that addDocuments*/updateDocuments were called, so this regression could reappear without failing tests. Please extend the spec(s) to assert that addDocumentsInBatches, addDocuments, and updateDocuments are invoked with the expected primaryKey option (e.g., messageId for messages, conversationId for convos).
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ents 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 #12538
Address review findings from PR #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.
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.
92e5c11 to
3e6f90e
Compare
…ns (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.
…ns (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.
Summary
Fixed message search returning empty results on Meilisearch v1.0+ and resolved several pre-existing bugs in the
mongoMeiliMongoose plugin discovered during review.Meilisearch v1.0 tightened primary key inference: when a document contains multiple fields ending with
id, it refuses to auto-infer and requires the primary key to be specified explicitly. Themessagesindex contains bothconversationIdandmessageId, causing everyaddDocuments/addDocumentsInBatches/updateDocumentscall to silently fail withindex_primary_key_multiple_candidates_found. Documents get marked as_meiliIndex: truein MongoDB but never actually reach Meilisearch, leaving message search empty while conversation search continues working.{ primaryKey }toindex.addDocumentsInBatches()inprocessSyncBatch(bulk sync path).{ primaryKey }toindex.addDocuments()inaddObjectToMeili(per-save hook path).{ primaryKey }toindex.updateDocuments()inupdateObjectToMeili(per-update hook path).deleteObjectFromMeiliusing MongoDB_idinstead of the Meilisearch primary key (conversationId/messageId), which caused post-remove cleanup to silently no-op and leave orphaned documents in the index.options.primaryKeyexplicitly to thecreateMeiliMongooseModelfactory function instead of deriving it fromattributesToIndex[0](schema field order), eliminating a fragile implicit contract.updateObjectToMeiliskippingpreprocessObjectForIndex(), which meant updates bypassedcontentarray-to-text conversion andconversationIdpipe character escaping.collection.updateManytocollection.updateOneinaddObjectToMeilisince_idis unique.primaryKeytovalidateOptionsrequired keys to fail fast if omitted.{ primaryKey }argument is passed toaddDocuments,addDocumentsInBatches, andupdateDocuments. Add tests for the update path includingpreprocessObjectForIndexpipe escaping.Closes #12538
Change Type
Testing
All 43 tests pass in the
mongoMeili.spec.tssuite.To verify the primary fix on a live deployment:
messagesindex (or start with a fresh volume)._meiliIndexflags in MongoDB:db.messages.updateMany({}, { $set: { _meiliIndex: false } }).addDocumentstasks should now succeed instead of failing withindex_primary_key_multiple_candidates_found.Checklist