chore: remove left over oplog code#38313
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughRemoves oplog-based observer recovery and metrics, disables oplog unconditionally, and introduces a token-driven reactivity system that monkey-patches observeChanges to manage per-user token callbacks and session invalidation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Observer
participant Connection as Connection._observeChanges
participant UserReactivity as userReactivity
participant Mongo as Mongo Users Collection
participant Service as Meteor Service
Client->>Connection: start observeChanges(users, fields:{tokens})
Connection->>UserReactivity: delegate observe hook
UserReactivity->>Mongo: initial fetch matching records
Mongo-->>UserReactivity: return records
UserReactivity->>UserReactivity: store per-user hashed-token callbacks
UserReactivity-->>Client: emit added/ready events
Service->>UserReactivity: processOnChange(diff, userId)
UserReactivity->>UserReactivity: detect removed tokens for userId
UserReactivity-->>Client: emit removed callbacks (session invalidation)
Client->>Service: client handles invalidation
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38313 +/- ##
===========================================
- Coverage 70.39% 70.39% -0.01%
===========================================
Files 3161 3162 +1
Lines 110654 110650 -4
Branches 19892 19920 +28
===========================================
- Hits 77895 77889 -6
- Misses 30731 30735 +4
+ Partials 2028 2026 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/server/services/meteor/userReactivity.ts`:
- Around line 17-72: The override
MongoInternals.Connection.prototype._observeChanges must be synchronous and
return a handle immediately (not an async function); change the async function
to a normal function that starts the initial DB query asynchronously,
immediately returns an object with stop(), isReady (initially false) and
isReadyPromise (a Promise resolved when the initial query completes), and when
the async query finishes set isReady=true and resolve isReadyPromise and invoke
callbacks.added as currently implemented; also forward additional query options
(e.g., sort, limit, skip, hint, etc.) from the options param when calling
mongo.rawCollection(...).find(...) so you don't only pass projection/fields;
keep the userCallbacks and serviceConfigCallbacks logic but ensure stop() can
cancel/cleanup even if called before the initial query resolves.
🧹 Nitpick comments (3)
apps/meteor/packages/rocketchat-mongo-config/server/index.js (1)
7-8: Drop the inline comment to match repo guidance.
The assignment is self-explanatory; keep the rationale out of the implementation.As per coding guidelines, Avoid code comments in the implementation.🧹 Suggested change
-// we always want Meteor to disable oplog tailing Package['disable-oplog'] = {};apps/meteor/server/services/meteor/userReactivity.ts (2)
11-16: Remove inline comments and the commented-out debug line.
These are implementation notes; keep code comment-free per repo guidance.As per coding guidelines, Avoid code comments in the implementation.🧹 Suggested change
-// Stores the callbacks for the disconnection reactivity bellow const userCallbacks = new Map(); -// Overrides the native observe changes to prevent database polling and stores the callbacks -// for the users' tokens to re-implement the reactivity based on our database listeners const { mongo } = MongoInternals.defaultRemoteCollectionDriver(); @@ - // console.error('Connection.Collection.prototype._observeChanges', collectionName, selector, options); @@ -// Re-implement meteor's reactivity that uses observe to disconnect sessions when the token -// associated was removed export const processOnChange = (diff: Record<string, any>, id: string): void => {Also applies to: 33-33, 74-76
83-90: Clean up empty per-user callback sets.
After removals, empty sets remain inuserCallbacks; deleting them avoids unnecessary retention.🧹 Suggested change
if (cbs) { [...cbs] .filter(({ hashedToken }) => tokens === undefined || !tokens.includes(hashedToken)) .forEach((item) => { item.callbacks.removed(id); cbs.delete(item); }); + if (cbs.size === 0) { + userCallbacks.delete(id); + } }
Co-authored-by: Kevin Aleman <kaleman960@gmail.com>
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.