Skip to content
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

Database name should be parsed from connection string when available #8354

Closed
zachazar opened this issue Nov 18, 2019 · 1 comment
Closed

Comments

@zachazar
Copy link
Contributor

Do you want to request a feature or report a bug?
Feature

What is the current behavior?
When you connect to MongoDB, you provide a dbName option to specify which database to connect to. This is handled here

mongoose/lib/connection.js

Lines 604 to 608 in 3e9faef

parseConnectionString(uri, options, (err, parsed) => {
if (err) {
return reject(err);
}
this.name = dbName != null ? dbName : get(parsed, 'auth.db', null);

This was added in #6106 in response to SRV connection strings lacking a database name, and thus the dbName workaround was added.

What is the expected behavior?
Now when testing with Atlas connection strings, it looks like we have the ability to specify a default database in the connection string - and parseConnectionString() successfully parses it.

The spec doesn't show this option, however Atlas uses it and even shows an example of defaulting to a test database in their Driver Examples documentation.

Thus, we believe the logic here should be (in this order):

  1. if dbName is provided in the options, use it
  2. if the database name is provided in the connection string (parsed as defaultDatabase), use it
  3. if neither of the above are provided, default to the authorization database

Currently there is no step 2.

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

Node.js: 10.15
Mongoose: 5.4
MongoDB: 3.6


I'm working on a PR right now to add this logic and update these docs

vkarpov15 added a commit that referenced this issue Nov 18, 2019
…n-connection-string

feat(connections): Adds default database connection string support (#8354)
@zachazar
Copy link
Contributor Author

Closing. Fixed in #8355.

(/cc @vkarpov15 since I'm not sure if you use Milestones on Issues)

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

No branches or pull requests

1 participant