-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: Add support for more MongoDB driver options #9911
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add support for more MongoDB driver options #9911
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughAdds a broad set of MongoDB driver configuration options to DatabaseOptions across option declarations, documentation, Flow types, and TypeScript definitions — covering authentication, TLS/SSL, proxy, SRV, compression, read preferences, and connection tuning. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9911 +/- ##
==========================================
- Coverage 93.06% 93.05% -0.02%
==========================================
Files 187 187
Lines 15187 15187
Branches 177 177
==========================================
- Hits 14134 14132 -2
- Misses 1041 1043 +2
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Options/docs.js (1)
243-296: LGTM! Documentation is comprehensive and accurate.The JSDoc documentation for the new MongoDB driver options is well-structured and aligns with the official MongoDB driver documentation. The descriptions clearly explain the purpose and behavior of each option.
One minor suggestion: For
zlibCompressionLevel(line 296), consider adding a note that values outside the 0-9 range will be rejected by the MongoDB driver.src/Options/Definitions.js (1)
1120-1124: Consider adding a parser for the compressors option.The
compressorsoption (lines 1120-1124) can accept either a string array or a comma-delimited string according to its type definition (string[] | string). However, no parser action is specified.Consider adding a parser to handle both input formats consistently, especially when the value comes from an environment variable (which would be a string). For example, an
arrayParsercould split comma-delimited strings into arrays.This would ensure consistent handling regardless of whether the option is provided as:
- An array:
['snappy', 'zlib']- A string:
'snappy,zlib'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Options/Definitions.js(7 hunks)src/Options/docs.js(2 hunks)src/Options/index.js(1 hunks)types/Options/index.d.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Options/Definitions.js (1)
resources/buildConfigDefinitions.js (1)
parsers(12-12)
🪛 Biome (2.1.2)
src/Options/index.js
[error] 641-641: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 643-643: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 645-645: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 647-647: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 649-649: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 651-651: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 653-653: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 655-655: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 657-657: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 659-659: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 661-661: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 663-663: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 665-665: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 667-667: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 669-669: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 671-671: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 673-673: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 675-675: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 677-677: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 679-679: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 681-681: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 683-683: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 685-685: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 687-687: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 689-689: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 691-691: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 693-693: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 695-695: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 697-697: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 699-699: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 701-701: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
[error] 703-703: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: Node 22
- GitHub Check: Redis Cache
- GitHub Check: Node 20
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: Docker Build
🔇 Additional comments (7)
src/Options/index.js (4)
641-656: LGTM! Connection pool and topology options look correct.The newly added MongoDB driver options for connection pool management (
maxConnecting,waitQueueTimeoutMS) and topology configuration (replicaSet,directConnection,loadBalanced,localThresholdMS,srvMaxHosts,srvServiceName) are properly typed and documented.
657-672: Verify security implications of TLS bypass options.The TLS/SSL configuration options include several that bypass security validations:
tlsAllowInvalidCertificates(line 668): bypasses certificate validationtlsAllowInvalidHostnames(line 670): disables hostname validationtlsInsecure(line 672): disables various certificate validationsThese options are legitimate MongoDB driver options but can create security vulnerabilities if misused. Consider:
- Adding warnings in the documentation about production use
- Documenting when these options might be legitimately needed (e.g., development/testing environments)
Also note:
ssl(line 660) andtls(line 658) are equivalent options per MongoDB documentation. Having both provides flexibility but users should be aware they serve the same purpose.
697-704: Note: Proxy configuration includes sensitive credentials.The proxy configuration options include
proxyPassword(line 704) which contains sensitive credentials. Ensure that:
- This value is properly protected when logged or displayed
- Environment variable handling follows security best practices
- Documentation warns users about secure credential management
The proxy options themselves are correctly typed and align with MongoDB driver requirements.
641-704: Static analysis errors are false positives.The Biome static analysis tool is reporting parse errors on all the newly added optional properties (lines 641-704), claiming it "Expected a statement but instead found '?'". These are false positives - the syntax is valid Flow notation for optional interface properties (e.g.,
maxConnecting: ?number).However, per the PR objectives, tests for these new options have not been added yet. Please ensure comprehensive tests are added to verify:
- Options are correctly passed to the MongoDB driver
- Environment variables are properly parsed
- Invalid values are rejected appropriately
- Security-sensitive options behave as expected
types/Options/index.d.ts (1)
230-278: LGTM! TypeScript definitions are consistent and well-organized.The TypeScript interface definitions correctly mirror the Flow types in
src/Options/index.js. The added organizational comments ("Parse Server custom options" and "MongoDB driver options") improve code readability by clearly delineating the two categories of options.All types are consistent:
- Optional properties use
?:notation- Primitive types match Flow definitions
- Union types (like
compressors: string[] | string) are correctly defined- The
anytype is used sparingly and only where appropriate (e.g.,authMechanismProperties,readPreferenceTags)src/Options/Definitions.js (2)
1180-1330: LGTM! Parser configuration is correct.The configuration metadata for the MongoDB driver options is well-structured:
- Boolean options correctly use
booleanParser- Numeric options correctly use
numberParserwith proper field names- Array options correctly use
arrayParser- String options appropriately have no parser (passed through as-is)
- Environment variable names consistently follow the
PARSE_SERVER_DATABASE_*pattern- Help text matches the documentation in
docs.js
1331-1394: LGTM! TLS/SSL and compression options are properly configured.The TLS/SSL configuration options and compression settings have appropriate parsers:
- Boolean flags use
booleanParser- Numeric values (timeouts, compression level) use
numberParser- File paths and passwords are passed through as strings without parsing
- Environment variable names are consistent
The configuration correctly supports all MongoDB driver TLS/SSL options for secure connections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
spec/ParseConfigKey.spec.js (2)
126-127: Usepath.join()for cross-platform file path construction.String concatenation for file paths can cause issues on Windows. Use Node.js
path.join()instead.Apply this diff:
+const path = require('path'); + const Config = require('../lib/Config');Then update the file paths:
- tlsCAFile: __dirname + '/support/cert/cert.pem', - tlsCertificateKeyFile: __dirname + '/support/cert/cert.pem', + tlsCAFile: path.join(__dirname, 'support', 'cert', 'cert.pem'), + tlsCertificateKeyFile: path.join(__dirname, 'support', 'cert', 'cert.pem'),
133-141: Clarify the driver config rules fortlsInsecure.The comment mentions "driver config rules" requiring separate testing for
tlsInsecure. Per MongoDB documentation,tlsInsecureis mutually exclusive withtlsAllowInvalidCertificatesandtlsAllowInvalidHostnames(present in lines 124-125). Consider making the comment more specific to help future maintainers understand the constraint.databaseOptions: { - // The following option needs to be tested separately due to driver config rules + // tlsInsecure is mutually exclusive with tlsAllowInvalidCertificates/tlsAllowInvalidHostnames tlsInsecure: false, },Also note that this test may not be meaningful if TLS is disabled in the first configuration block (lines 122-123).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/ParseConfigKey.spec.js(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
spec/ParseConfigKey.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Applied to files:
spec/ParseConfigKey.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Applied to files:
spec/ParseConfigKey.spec.js
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.
Applied to files:
spec/ParseConfigKey.spec.js
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.
Applied to files:
spec/ParseConfigKey.spec.js
🧬 Code graph analysis (1)
spec/ParseConfigKey.spec.js (1)
spec/helper.js (1)
reconfigureServer(180-214)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Check Types
- GitHub Check: Node 22
- GitHub Check: Node 18
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: Node 20
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Redis Cache
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Docker Build
🔇 Additional comments (3)
spec/ParseConfigKey.spec.js (3)
76-97: LGTM! MongoDB connection options look correct.This segment properly tests basic MongoDB driver options with reasonable values. The inline comment explaining why
authMechanismoptions can't be tested is helpful.
99-116: LGTM! Read preferences and retry options configured correctly.The commented-out proxy and replica set options are properly documented. The read preference and retry options use valid values.
126-128: Certificate files exist at the specified path.The certificate file
spec/support/cert/cert.pemhas been verified to exist in the repository at the expected location. The test configuration references are valid.
# [8.5.0-alpha.3](8.5.0-alpha.2...8.5.0-alpha.3) (2025-11-07) ### Features * Add support for more MongoDB driver options ([#9911](#9911)) ([cff451e](cff451e))
|
🎉 This change has been released in version 8.5.0-alpha.3 |
Pull Request
Issue
There are ~40 MongoDB driver connection options that are currently not recognized by Parse Server.
See https://www.mongodb.com/docs/drivers/node/current/connect/connection-options/
Approach
Add missing options.
Tasks
Summary by CodeRabbit
New Features
Documentation