refactor: update Elasticsearch and Typesense db index upsert#1405
Conversation
…ument indexing and upserting
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughDDO updates in Elasticsearch and Typesense now write full documents, and each update removes stale copies of the same DDO id from other configured schemas after a successful write. ChangesDDO Update Persistence Change
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/database/ElasticSearchDatabase.ts`:
- Around line 781-784: The DDO write path in ElasticSearchDatabase.updateDDO
currently only indexes into the target schema via client.index, so stale copies
can remain in other DDO indices after a schema/version change. After a
successful index operation, update the updateDDO flow to also remove the
document from the other DDO indices using the existing Elasticsearch client
helpers and schema/index handling in ElasticSearchDatabase. Make sure the
cleanup is tied to the same ddo.id so only the previous cross-schema copies are
removed.
In `@src/components/database/TypesenseDatabase.ts`:
- Line 646: In TypesenseDatabase’s document write path around the upsert() call,
schema moves can leave stale copies behind in older collections. After the write
succeeds, remove the same document ID from the non-target collections so a DDO
does not remain duplicated across schemas and later surface in
search()/retrieve(). Use the existing TypesenseDatabase collection/document
handling to locate the target write flow and add the cleanup logic there.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3924e9d2-bc1f-4fba-92b7-b4deade6744b
📒 Files selected for processing (3)
src/components/database/ElasticSearchDatabase.tssrc/components/database/TypesenseDatabase.tssrc/components/database/typesense.ts
|
/run-security-scan |
alexcos20
left a comment
There was a problem hiding this comment.
AI automated code review (Gemini 3).
Overall risk: low
Summary:
The PR correctly updates database write operations to use 'upsert' semantics (using index in Elasticsearch and upsert in Typesense). This prevents 'document not found' errors when updating non-existent records, ensures idempotent writes, and aligns the behavior across both database implementations.
Comments:
• [INFO][performance] Excellent change. Switching from update to index provides true upsert (create or replace) semantics. Using index is also generally more performant than update with doc_as_upsert for full document replacements because it avoids the internal fetch-and-merge step. Removing the { doc: ddo } wrapper is correct here as index expects the raw document in the body.
• [INFO][other] Switching to upsert(ddo) successfully aligns the Typesense implementation with the Elasticsearch changes, ensuring consistent behavior across different storage backends.
• [INFO][style] The new upsert wrapper method looks clean. Retaining the async keyword here (along with the eslint exception) is a good practice—it ensures that the synchronous throw new Error('No document provided') will properly reject the returned Promise rather than throwing a synchronous exception. LGTM!
…ies in Elasticsearch and Typesense
|
@coderabbitai review |
✅ Action performedReview finished.
|
Fixes # .
Changes proposed in this PR:
update Elasticsearch and Typesense database methods for document indexing and upserting
Summary by CodeRabbit