refactor(webui): Integrate existing Mongo SocketIO plugin into new Fastify architecture; Pass logger into plugin instead of entire Fastify Instance (resolves #885). #1027
+77
−99
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as off-topic.
This comment was marked as off-topic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
components/log-viewer-webui/server/src/app.ts(0 hunks)components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts(7 hunks)components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts(13 hunks)components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/typings.ts(1 hunks)components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/utils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- components/log-viewer-webui/server/src/app.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/typings.tscomponents/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/utils.tscomponents/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.tscomponents/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts
🪛 Biome (1.9.4)
components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
[error] 328-329: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (5)
components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/utils.ts (1)
1-79: LGTM!The utility functions are well-implemented and the removal of
initializeMongoClientaligns with the refactoring goals.components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/typings.ts (1)
14-14: Import path correctly updated.The import path adjustment reflects the new directory structure after moving the plugin to
fastify-v2.components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts (1)
1-216: Excellent refactoring to use injected logger.The migration from console logging to structured logging via the injected FastifyBaseLogger is well-executed. All error objects are properly passed as the first parameter to the logger methods, following structured logging best practices.
components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts (2)
67-102: Well-structured dependency injection refactoring.The refactoring to inject specific dependencies (io, logger, mongoDb) instead of the entire Fastify instance follows the principle of least privilege and makes the class more testable. The synchronous
createmethod with proper error handling for missing MongoDB is a good improvement.
376-390: Clean plugin registration pattern.The synchronous plugin registration with proper TypeScript module augmentation and Fastify decoration is well-implemented. This pattern aligns with Fastify best practices for plugins that don't require async initialization.
...nents/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts (1)
333-334: Use template literal for consistency.This issue was already identified in a previous review. Replace string concatenation with a template literal.
- this.#logger.debug(`Collection:${queryParams.collectionName}` + - " deallocated from server."); + this.#logger.debug(`Collection:${queryParams.collectionName} deallocated from server.`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts(13 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
🪛 Biome (1.9.4)
components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
[error] 328-329: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (4)
components/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts (4)
62-77: Constructor refactoring looks good.The constructor now properly accepts individual dependencies (
io,logger,mongoDb) instead of the entire Fastify instance, which improves testability and follows the single responsibility principle.
85-102: Static create method refactoring is well implemented.The synchronous implementation is appropriate, and the error handling for missing MongoDB database is clear. The method correctly extracts dependencies from the Fastify instance and passes them to the constructor.
209-215: MongoWatcherCollection instantiation correctly updated.The constructor call properly passes the logger instance, maintaining consistency with the refactoring to decouple from the Fastify instance.
376-389: Module augmentation and plugin export are well implemented.The TypeScript module augmentation properly extends the FastifyInstance interface, and the synchronous plugin pattern with the decorator is the correct approach for this refactoring.
...nents/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
Show resolved
Hide resolved
...nents/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
Show resolved
Hide resolved
...nents/log-viewer-webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
Show resolved
Hide resolved
3 tasks
added 2 commits
June 30, 2025 16:55
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
components/webui/server/src/app.ts(0 hunks)components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts(7 hunks)components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts(13 hunks)components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/typings.ts(1 hunks)components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/utils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- components/webui/server/src/app.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/typings.tscomponents/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/utils.tscomponents/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.tscomponents/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:79-83
Timestamp: 2025-05-05T18:03:50.472Z
Learning: In MongoWatcherCollection, the unsubscribe method is intentionally designed to remove only one instance of a connection ID from the subscribers array using removeItemFromArray. This allows for multiple subscriptions from the same connection, with each unsubscribe call removing only one subscription at a time.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:196-200
Timestamp: 2025-05-05T18:08:23.313Z
Learning: In the MongoSocketIoServer plugin, change streams are designed to be closed only through explicit client unsubscribe actions rather than having automatic cleanup on error or unexpected close events. This design choice puts the responsibility on the client to properly unsubscribe.
Learnt from: junhaoliao
PR: y-scope/clp#899
File: components/log-viewer-webui/server/src/main.ts:50-58
Timestamp: 2025-05-13T19:38:31.164Z
Learning: When using Fastify's `app.close()` method, always wrap it in a try/catch block to properly handle shutdown errors. The close() method returns a Promise that can reject if errors occur during the shutdown process (especially with plugins using onClose hooks), and unhandled rejections could cause the application to hang.
components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/typings.ts (6)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: davemarco
PR: y-scope/clp#1015
File: components/log-viewer-webui/server/src/routes/static.ts:65-70
Timestamp: 2025-06-16T13:05:27.349Z
Learning: In components/log-viewer-webui/server/src/routes/static.ts, when decorateReply is set to true in fastifyStatic configuration, the reply.sendFile() method automatically uses the root directory configured in the static plugin registration, eliminating the need to pass the root directory as a second parameter.
Learnt from: junhaoliao
PR: y-scope/clp#647
File: components/log-viewer-webui/server/src/main.ts:8-8
Timestamp: 2024-12-31T19:19:55.032Z
Learning: When "moduleResolution": "node16" is used in a TypeScript project's tsconfig.json, the import file extension is typically expected to be ".js" (or ".cjs"/".mjs"), even if the source code is originally in TypeScript (.ts). This is a part of node16’s resolution behavior.
components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/utils.ts (5)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:196-200
Timestamp: 2025-05-05T18:08:23.313Z
Learning: In the MongoSocketIoServer plugin, change streams are designed to be closed only through explicit client unsubscribe actions rather than having automatic cleanup on error or unexpected close events. This design choice puts the responsibility on the client to properly unsubscribe.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts (6)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:79-83
Timestamp: 2025-05-05T18:03:50.472Z
Learning: In MongoWatcherCollection, the unsubscribe method is intentionally designed to remove only one instance of a connection ID from the subscribers array using removeItemFromArray. This allows for multiple subscriptions from the same connection, with each unsubscribe call removing only one subscription at a time.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:196-200
Timestamp: 2025-05-05T18:08:23.313Z
Learning: In the MongoSocketIoServer plugin, change streams are designed to be closed only through explicit client unsubscribe actions rather than having automatic cleanup on error or unexpected close events. This design choice puts the responsibility on the client to properly unsubscribe.
components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts (10)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:196-200
Timestamp: 2025-05-05T18:08:23.313Z
Learning: In the MongoSocketIoServer plugin, change streams are designed to be closed only through explicit client unsubscribe actions rather than having automatic cleanup on error or unexpected close events. This design choice puts the responsibility on the client to properly unsubscribe.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#1015
File: components/log-viewer-webui/server/src/routes/static.ts:65-70
Timestamp: 2025-06-16T13:05:27.349Z
Learning: In components/log-viewer-webui/server/src/routes/static.ts, when decorateReply is set to true in fastifyStatic configuration, the reply.sendFile() method automatically uses the root directory configured in the static plugin registration, eliminating the need to pass the root directory as a second parameter.
Learnt from: junhaoliao
PR: y-scope/clp#899
File: components/log-viewer-webui/server/src/main.ts:50-58
Timestamp: 2025-05-13T19:38:31.164Z
Learning: When using Fastify's `app.close()` method, always wrap it in a try/catch block to properly handle shutdown errors. The close() method returns a Promise that can reject if errors occur during the shutdown process (especially with plugins using onClose hooks), and unhandled rejections could cause the application to hang.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:79-83
Timestamp: 2025-05-05T18:03:50.472Z
Learning: In MongoWatcherCollection, the unsubscribe method is intentionally designed to remove only one instance of a connection ID from the subscribers array using removeItemFromArray. This allows for multiple subscriptions from the same connection, with each unsubscribe call removing only one subscription at a time.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:25-43
Timestamp: 2025-05-09T18:06:33.042Z
Learning: Socket.IO connections in JavaScript require explicit disconnection by calling socket.disconnect() and are not automatically cleaned up when the object is garbage collected.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:25-43
Timestamp: 2025-05-09T18:21:05.843Z
Learning: Socket.IO has a built-in ping/pong mechanism that detects connection failures, but it doesn't automatically disconnect truly inactive but connected clients. By default, it sends pings every 25 seconds and expects pongs within 20 seconds, but this is for connection health monitoring, not application inactivity detection.
🪛 Biome (1.9.4)
components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
[error] 328-329: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (5)
components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/typings.ts (1)
9-14: LGTM!The import path adjustment correctly reflects the new location of this file within the
fastify-v2directory structure.components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/utils.ts (1)
1-7: Removal of MongoDB initialization aligns with PR objectives.The removal of
initializeMongoClientand related imports is correct, as the plugin now uses the shared MongoDB client from the Fastify instance (fastify.mongo.db) instead of creating its own connection.components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts (1)
1-39: Excellent refactoring to use injected logger.The migration from console logging to using the injected
FastifyBaseLoggeris well implemented. All logging calls have been properly converted with appropriate log levels.components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts (2)
7-102: Well-structured refactoring to support dependency injection.The changes successfully:
- Replace the Fastify instance dependency with explicit
ioandloggerparameters- Retrieve the MongoDB database from the shared Fastify MongoDB plugin
- Convert the static factory method from async to sync (appropriate since no async operations are needed)
- Add proper error handling for missing MongoDB database
376-390: Clean Fastify plugin implementation.The plugin export correctly:
- Uses
fastifyPluginfor proper encapsulation- Decorates the Fastify instance with the
MongoSocketIoServerproperty- Includes TypeScript module augmentation for type safety
- Provides a descriptive plugin name
components/webui/server/src/fastify-v2/plugins/app/socket/MongoSocketIoServer/index.ts
Show resolved
Hide resolved
3 tasks
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
Author
|
@hoophalab i merged main back into this. Please rereview |
hoophalab
approved these changes
Jul 10, 2025
Contributor
There was a problem hiding this comment.
lgtm
I merged the main branch to check for conflicts, so when you merge the PR, feel free to remove me as a co-author since I didn't contribute to the actual code.
Validation steps:
-
run
clp-text- Compressed several files in hive-24hr
- Executed multiple queries, all of which returned the expected results.
-
run
clp-json- Compressed
spark-event-logsandpostgresql - Executed multiple queries, all of which returned the expected results.
- Compressed
Contributor
|
I just remembered: after the release, we're planning to name PRs |
davemarco
changed the title
Contributor
Author
|
okay tagging @junhaoliao for write access |
davemarco
changed the title
junhaoliao
approved these changes
Jul 10, 2025
Member
There was a problem hiding this comment.
nice work refactoring this
deferring to @hoophalab 's review
This was referenced Jul 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
PR #880 added new mongo plugin, but this was complete prior to #899 (fastify architecture refactor). This PR begins the process of migrating old plugins/routes from old architecture to new architecture. There will be several more PRs. When all the PRs are finished, the
fastify-v2directory can be flattened(its files/dir brought into src)Specifically, this PR moves plugin to fastify-v2 directory, modifies the plugin to use shared mongo client, and passes the logger to plugin instead of entire fastify instance (which happens to resolve #885)
Checklist
breaking change.
Validation performed
Tested a query, and still ran in webui
Summary by CodeRabbit
Refactor
Chores