Skip to content

feat: add mongodb connector#128

Merged
andrei-gavrilescu merged 12 commits intojitsi:masterfrom
Taukon:feat/mongodb-connector
Aug 25, 2025
Merged

feat: add mongodb connector#128
andrei-gavrilescu merged 12 commits intojitsi:masterfrom
Taukon:feat/mongodb-connector

Conversation

@Taukon
Copy link

@Taukon Taukon commented Jul 9, 2025

This PR adds support for MongoDB and GridFS as alternatives to the existing AWS services (DynamoDB and S3).

This comment was marked as outdated.

@andrei-gavrilescu
Copy link

@Taukon look good, make sure you run npm run integration, and that those tests pass.

@Taukon Taukon marked this pull request as ready for review July 30, 2025 12:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds MongoDB and GridFS support as alternatives to AWS services (DynamoDB and S3), implementing a service abstraction layer to switch between storage backends.

  • Introduces MongoDB connection handling and data storage classes (MongoDataSender, GridFSManager)
  • Creates a service factory pattern that initializes either AWS or MongoDB services based on configuration
  • Refactors the application initialization to use the new service abstraction layer

Reviewed Changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/store/MongoDataSender.js Implements MongoDB data storage using Mongoose with schema definition and error handling
src/store/GridFSManager.js Provides GridFS file storage functionality with gzip compression
src/services/index.js Service factory that initializes either AWS or MongoDB services based on configuration
src/services/MongoDB.js MongoDB service setup functions for connection and service initialization
src/services/AWS.js Extracted AWS service setup functions from main application
src/app.js Refactored to use service factory instead of direct service initialization
src/RTCStatsServer.js Moved server setup functions to separate module and updated service injection
src/ServerSetup.js Extracted HTTP/HTTPS server setup logic from RTCStatsServer
config files Added MongoDB configuration options and service type selection
package.json Added mongodb and mongoose dependencies
test files Comprehensive test coverage for new MongoDB components with mocking

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

*/
constructor(collectionName) {

assert(collectionName, 'Collection Name is required when initializing MongoDB');
Copy link

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message is inconsistent with the constructor parameter name. It should say 'Collection Name is required' to match the parameter name 'collectionName'.

Suggested change
assert(collectionName, 'Collection Name is required when initializing MongoDB');
assert(collectionName, "'collectionName' is required when initializing MongoDB");

Copilot uses AI. Check for mistakes.
versionKey: false
});

this._model = mongoose.models.Metadata || mongoose.model('Metadata', metadataSchema);
Copy link

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded model name 'Metadata' should be configurable or derived from the collection name to avoid conflicts when multiple instances use different collections.

Suggested change
this._model = mongoose.models.Metadata || mongoose.model('Metadata', metadataSchema);
// Derive a unique model name from the collection name to avoid conflicts
const modelName = `Metadata_${collectionName.replace(/[^a-zA-Z0-9]/g, '_')}`;
this._model = mongoose.models[modelName] || mongoose.model(modelName, metadataSchema);

Copilot uses AI. Check for mistakes.
@Taukon
Copy link
Author

Taukon commented Aug 22, 2025

@andrei-gavrilescu I pushed the fixes for Copilot's suggestions. Both npm run test and npm run integration are passing locally.

@andrei-gavrilescu andrei-gavrilescu merged commit c977bdb into jitsi:master Aug 25, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants