feat: add MongoDB support #10
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds MongoDB support to the rtc-visualizer application alongside the existing AWS DynamoDB/S3 implementation. The refactoring introduces an adapter pattern to support multiple database and file storage backends, allowing users to choose between AWS services and MongoDB/GridFS based on configuration.
- Refactors database and file storage services into adapter pattern with abstract base classes
- Implements MongoDB adapter for metadata storage and GridFS adapter for file storage
- Adds service setup configuration to dynamically initialize adapters based on environment variables
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/services/setup.mjs | New service setup module that initializes adapters based on RTCSTATS_SERVICE_TYPE |
| src/server/services/mongodb.mjs | MongoDB adapter implementation for metadata storage using Mongoose |
| src/server/services/gridfs.mjs | GridFS adapter implementation for file storage |
| src/server/services/dynamodb.mjs | Refactored DynamoDB service into adapter pattern |
| src/server/services/s3.mjs | Refactored S3 service into adapter pattern |
| src/server/services/database-adapter.mjs | Abstract base class for database adapters |
| src/server/services/file-storage-adapter.mjs | Abstract base class for file storage adapters |
| src/server/routes/search.mjs | Updated to use dependency injection pattern with database adapter |
| src/server/routes/files.mjs | Updated to use dependency injection pattern with file storage adapter |
| src/server/routes/download.mjs | Updated to use dependency injection pattern with file storage adapter |
| src/server/index.mjs | Modified to initialize services and inject adapters into routes |
| scripts/create-mongodb.mjs | New script to setup MongoDB database with proper indexes and test data |
| package.json | Added MongoDB and Mongoose dependencies |
| .env | Added MongoDB configuration variables |
src/server/services/mongodb.mjs
Outdated
|
|
||
| async connect () { | ||
| await mongoose.connect(RTCSTATS_MONGODB_URI, { dbName: RTCSTATS_MONGODB_NAME }) | ||
| console.log(`Successfully connected to MongoDB database: ${RTCSTATS_MONGODB_NAME}`) |
There was a problem hiding this comment.
Use the imported logger instead of console.log for consistency with the rest of the codebase.
| console.log(`Successfully connected to MongoDB database: ${RTCSTATS_MONGODB_NAME}`) | |
| logger.info(`Successfully connected to MongoDB database: ${RTCSTATS_MONGODB_NAME}`) |
There was a problem hiding this comment.
@Taukon please check thisone.
src/server/services/dynamodb.mjs
Outdated
| if (endpoint) { | ||
| dynamoose.aws.ddb.local(endpoint) | ||
| } | ||
| console.log('DynamoDB connection configured.') |
There was a problem hiding this comment.
Use the imported logger instead of console.log for consistency with the rest of the codebase.
| console.log('DynamoDB connection configured.') | |
| logger.info('DynamoDB connection configured.') |
src/server/services/gridfs.mjs
Outdated
| } | ||
|
|
||
| async connect () { | ||
| return Promise.resolve() | ||
| } | ||
|
|
||
| async fileExists (key) { | ||
| if (!this._bucket) { |
There was a problem hiding this comment.
The bucket is initialized in the constructor, so this check will never be true. The constructor throws an error if db is null, so _bucket will always be defined. This check should be removed or replaced with a check for the connection state.
| } | |
| async connect () { | |
| return Promise.resolve() | |
| } | |
| async fileExists (key) { | |
| if (!this._bucket) { | |
| this._connected = false | |
| } | |
| async connect () { | |
| this._connected = true | |
| return Promise.resolve() | |
| } | |
| async fileExists (key) { | |
| if (!this._connected) { |
There was a problem hiding this comment.
@Taukon this also makes sense.
src/server/services/gridfs.mjs
Outdated
| } | ||
|
|
||
| getFileStream (key) { | ||
| if (!this._bucket) { |
There was a problem hiding this comment.
The bucket is initialized in the constructor, so this check will never be true. The constructor throws an error if db is null, so _bucket will always be defined. This check should be removed or replaced with a check for the connection state.
| if (!this._bucket) { | |
| if (!this._connected) { |
| AWS.config.update(config) | ||
| export class S3Adapter extends FileStorageAdapter { | ||
| async connect () { | ||
| AWS.config.update(config) |
There was a problem hiding this comment.
I don't think connect makes a lot of sense in this interface, the s3 connector, doesn't have any async logic,neither does the gridfs one, at least from what I saw.
Also gridfs.mjs seems to have the setup logic in the ctor, I'd suggest moving the setup logic in the c'tor on both servies and discard the connect function alltogether.
2 participants
No description provided.