feat: enhance cache invalidation strategy and bump version to 6.34.0#289
feat: enhance cache invalidation strategy and bump version to 6.34.0#289
Conversation
🤖 Review Buddy - General Code Review
Oye @AnkanSaha! Kya bhai? 6.33.229? Itne versions toh Android ke nahi aaye jitne tumne ek hi week mein nikaal diye hain. Versioning hai ya gully cricket ka score? Aur ye code dekh ke lag raha hai ki tumne 'Consistency' word dictionary se delete kar diya hai. Cache invalidation ko fire-and-forget bana diya? Wah! Matlab user ko lagega data save ho gaya, par cache purana hi dikhayega. Consistency gayi tel lene, haina? Aur wo Generated by Review Buddy | Tone: roast | Language: hinglish |
⚡ Review Buddy - Performance Analysis
Bhai, performance ke naam pe tumne toh circus khol rakha hai. Chalo, line-by-line tumhari pol kholta hoon:
// Better approach: Use a background worker or a queue
async insert(data: any, documentId: string) {
const saveResult = await this.Insertion.Save(data, documentId);
await this.IndexManager.InsertToIndex(data);
// Using a proper queue for background tasks instead of random fire-and-forget
CacheQueue.add(() => InMemoryCache.invalidateByDocument(this.path, documentId));
return saveResult;
}
Summary: Tumhara code performance 'enhance' nahi, 'endanger' kar raha hai. Generated by Review Buddy | Tone: roast | Language: hinglish |
🔐 Review Buddy - Security Audit
Bhai, security ke baare mein suna hai ya bas copy-paste chal raha hai?
Remediation:
Generated by Review Buddy | Tone: roast | Language: hinglish |
📊 Review Buddy - Code Quality & Maintainability Analysis
🎯 Overall Benchmark: 40/100 (Poor)Bhai, quality analysis dekh ke tumhare aansu nikal aayenge. SOLID principles ka 'S' bhi follow nahi kiya tumne.
// Create a dedicated CacheService
class CacheService {
static invalidate(path: string, id: string) {
try {
return InMemoryCache.invalidateByDocument(path, id);
} catch (err) {
Logger.error(`Failed to invalidate cache for ${path}:${id}`, err);
}
}
}
// In Collection class
await CacheService.invalidate(this.path, documentId);Isse thodi toh izzat bachegi tumhari. Generated by Review Buddy | Tone: roast | Language: hinglish |
💡 Review Buddy - Best Practices & Alternative Suggestions
Bhai, best practices ke naam pe tumne sirf 1. Swallowing ErrorsCurrent Code: InMemoryCache.invalidateByDocument(this.path, documentId).catch(() => {});Better Alternative: InMemoryCache.invalidateByDocument(this.path, documentId).catch((err) => {
console.error(`[CacheInvalidationError] Path: ${this.path}, ID: ${documentId}`, err);
});Why: Taaki jab production mein cache fatt jaye, toh tumhare paas koi clue toh ho ki kya hua. Silence is not always golden, especially in logs. 2. Fire-and-Forget (The Lazy Way)Current Code: InMemoryCache.invalidateByDocument(this.path, documentId).catch(() => {});
return saveResult;Better Alternative: const [saveResult] = await Promise.all([
this.Insertion.Save(data, documentId),
InMemoryCache.invalidateByDocument(this.path, documentId)
]);
return saveResult;Why: Dono operations parallel mein honge, aur jab tak dono khatam nahi hote, response nahi jayega. Isse consistency maintain rahegi. 3. Versioning StrategyCurrent Code: Generated by Review Buddy | Tone: roast | Language: hinglish |
|
There was a problem hiding this comment.
Pull request overview
This PR bumps the library version and updates Collection.insert() to invalidate the in-memory query cache after inserts, aligning insert behavior with existing update/delete cache invalidation to reduce stale cached reads.
Changes:
- Bump npm package version to
6.33.229. - Invalidate
InMemoryCacheby document/collection after a successful insert (fire-and-forget).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
source/Services/Collection/collection.operation.ts |
Adds cache invalidation call after insert to reduce stale cached query results. |
package.json |
Version bump to 6.33.229. |
| // Fire-and-forget: Invalidate cache asynchronously for faster response | ||
| InMemoryCache.invalidateByDocument(this.path, documentId).catch(() => {}); |
There was a problem hiding this comment.
The new cache invalidation in insert() is untested. Since reads are cached in Reader.exec() via InMemoryCache.getCache/setCache, add a regression test that (1) runs a query to populate cache, (2) inserts a new document, and (3) re-runs the same query and asserts the new document appears (i.e., cached results were invalidated).
|
|
||
|
|
||
| // Fire-and-forget: Invalidate cache asynchronously for faster response | ||
| InMemoryCache.invalidateByDocument(this.path, documentId).catch(() => {}); |
There was a problem hiding this comment.
insertMany() calls insert() in a loop; with this change, cache invalidation will run once per inserted document. Consider adding a way to batch/skip per-document invalidation during bulk inserts and invalidate the collection cache once at the end to avoid repeated full-map scans when the cache is populated.
| await this.IndexManager.InsertToIndex(data); | ||
|
|
||
|
|
||
| // Fire-and-forget: Invalidate cache asynchronously for faster response |
There was a problem hiding this comment.
The comment says "Invalidate cache asynchronously for faster response", but invalidateByDocument() / invalidateByCollection() currently contain no await, so the invalidation work runs synchronously when called. Either adjust the comment to match reality, or explicitly defer the invalidation (with a clear note about any potential staleness tradeoff).
| // Fire-and-forget: Invalidate cache asynchronously for faster response | |
| // Invalidate cache without awaiting the returned Promise; errors are intentionally ignored |
Summary
This PR introduces an asynchronous cache invalidation mechanism in the document insertion flow and increments the package version.
Changes
package.jsonfrom6.33.228to6.33.229(Note: Recommended to use 6.34.0).InMemoryCache.invalidateByDocumentin theinsertmethod ofCollectionservice.Verification
invalidateByDocumentis called after saving a document.