Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions lib/connection_string.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ function matchesParentDomain(srvAddress, parentDomain) {
function parseSrvConnectionString(uri, options, callback) {
const result = URL.parse(uri, true);

if (options.directConnection || options.directconnection) {
Copy link
Member

Choose a reason for hiding this comment

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

is it possible to have both forms here? I was hoping we wouldn't be introducing more cases where we had to check the upper and lowercase version of URI options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During testing it came in as both, I think it performs differently when in URL and passed in, this covers all our bases.

Screen Shot 2020-05-11 at 3 52 25 PM

Is there a helper I'm missing? It's also in CASE_TRANSLATION. 😫

Copy link
Member

Choose a reason for hiding this comment

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

so case translation comes into play in applyConnectionStringOption which is in turn called by parseQueryString. Your change here moves the call to parseSrvConnectionString to after parseQueryString is called, which means that we should be in a world where all known values are camelCased.

Copy link
Member

Choose a reason for hiding this comment

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

@reggi I think we're waiting on you here

return callback(new MongoParseError('directConnection not supported with SRV URI'));
}

if (result.hostname.split('.').length < 3) {
return callback(new MongoParseError('URI does not have hostname, domain name and tld'));
}
Expand Down Expand Up @@ -217,7 +221,8 @@ const CASE_TRANSLATION = {
tlscertificatekeyfile: 'tlsCertificateKeyFile',
tlscertificatekeyfilepassword: 'tlsCertificateKeyFilePassword',
wtimeout: 'wTimeoutMS',
j: 'journal'
j: 'journal',
directconnection: 'directConnection'
};

/**
Expand Down Expand Up @@ -594,10 +599,6 @@ function parseConnectionString(uri, options, callback) {
return callback(new MongoParseError('Invalid protocol provided'));
}

if (protocol === PROTOCOL_MONGODB_SRV) {
return parseSrvConnectionString(uri, options, callback);
}

const dbAndQuery = cap[4].split('?');
const db = dbAndQuery.length > 0 ? dbAndQuery[0] : null;
const query = dbAndQuery.length > 1 ? dbAndQuery[1] : null;
Expand All @@ -613,6 +614,12 @@ function parseConnectionString(uri, options, callback) {
return callback(parseError);
}

parsedOptions = Object.assign({}, parsedOptions, options);

if (protocol === PROTOCOL_MONGODB_SRV) {
return parseSrvConnectionString(uri, parsedOptions, callback);
}

const auth = { username: null, password: null, db: db && db !== '' ? qs.unescape(db) : null };
if (parsedOptions.auth) {
// maintain support for legacy options passed into `MongoClient`
Expand Down Expand Up @@ -706,6 +713,22 @@ function parseConnectionString(uri, options, callback) {
return callback(new MongoParseError('No hostname or hostnames provided in connection string'));
}

const directConnection = !!parsedOptions.directConnection;
if (directConnection && hosts.length !== 1) {
// If the option is set to true, the driver MUST validate that there is exactly one host given
// in the host list in the URI, and fail client creation otherwise.
return callback(new MongoParseError('directConnection option requires exactly one host'));
}

// NOTE: this behavior will go away in v4.0, we will always auto discover there
if (
parsedOptions.directConnection == null &&
hosts.length === 1 &&
parsedOptions.replicaSet == null
) {
parsedOptions.directConnection = true;
}

const result = {
hosts: hosts,
auth: auth.db || auth.username ? auth : null,
Expand Down
2 changes: 2 additions & 0 deletions lib/mongo_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ const { connect, validOptions } = require('./operations/connect');
* @param {boolean} [options.useNewUrlParser=true] Determines whether or not to use the new url parser. Enables the new, spec-compliant, url parser shipped in the core driver. This url parser fixes a number of problems with the original parser, and aims to outright replace that parser in the near future. Defaults to true, and must be explicitly set to false to use the legacy url parser.
* @param {DriverInfoOptions} [options.driverInfo] Allows a wrapping driver to amend the client metadata generated by the driver to include information about the wrapping driver
* @param {AutoEncrypter~AutoEncryptionOptions} [options.autoEncryption] Optionally enable client side auto encryption
* @param {boolean} [options.directConnection=false] Enable directConnection
Copy link
Member

Choose a reason for hiding this comment

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

can we improve this doc string? "Indicates that a client should directly connect to a node without attempting to discover its topology type" or something like that

* @returns {MongoClient} a MongoClient instance
*/
function MongoClient(url, options) {
Expand Down Expand Up @@ -388,6 +389,7 @@ MongoClient.prototype.isConnected = function(options) {
* @param {number} [options.numberOfRetries=5] The number of retries for a tailable cursor
* @param {boolean} [options.auto_reconnect=true] Enable auto reconnecting for single server instances
* @param {number} [options.minSize] If present, the connection pool will be initialized with minSize connections, and will never dip below minSize connections
* @param {boolean} [options.directConnection=false] Enable directConnection
Copy link
Member

Choose a reason for hiding this comment

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

same comment

* @param {MongoClient~connectCallback} [callback] The command result callback
* @returns {Promise<MongoClient>} returns Promise if no callback passed
*/
Expand Down
3 changes: 2 additions & 1 deletion lib/operations/connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ const validOptionNames = [
'tlsCertificateKeyFilePassword',
'minHeartbeatFrequencyMS',
'heartbeatFrequencyMS',
'waitQueueTimeoutMS'
'waitQueueTimeoutMS',
'directConnection'
];

const ignoreOptionNames = ['native_parser'];
Expand Down
12 changes: 9 additions & 3 deletions lib/sdam/topology.js
Original file line number Diff line number Diff line change
Expand Up @@ -798,10 +798,16 @@ function parseStringSeedlist(seedlist) {
}

function topologyTypeFromSeedlist(seedlist, options) {
if (options.directConnection) {
return TopologyType.Single;
}

const replicaSet = options.replicaSet || options.setName || options.rs_name;
if (seedlist.length === 1 && !replicaSet) return TopologyType.Single;
if (replicaSet) return TopologyType.ReplicaSetNoPrimary;
return TopologyType.Unknown;
if (replicaSet == null) {
return TopologyType.Unknown;
}

return TopologyType.ReplicaSetNoPrimary;
}

function randomSelection(array) {
Expand Down
20 changes: 17 additions & 3 deletions lib/sdam/topology_description.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class TopologyDescription {
}

if (topologyType === TopologyType.Unknown) {
if (serverType === ServerType.Standalone) {
if (serverType === ServerType.Standalone && this.servers.size !== 1) {
serverDescriptions.delete(address);
} else {
topologyType = topologyTypeForServerType(serverType);
Expand Down Expand Up @@ -277,8 +277,22 @@ class TopologyDescription {
}

function topologyTypeForServerType(serverType) {
if (serverType === ServerType.Mongos) return TopologyType.Sharded;
if (serverType === ServerType.RSPrimary) return TopologyType.ReplicaSetWithPrimary;
if (serverType === ServerType.Standalone) {
return TopologyType.Single;
}

if (serverType === ServerType.Mongos) {
return TopologyType.Sharded;
}

if (serverType === ServerType.RSPrimary) {
return TopologyType.ReplicaSetWithPrimary;
}

if (serverType === ServerType.RSGhost || serverType === ServerType.Unknown) {
return TopologyType.Unknown;
}

Copy link
Contributor

@emadum emadum May 5, 2020

Choose a reason for hiding this comment

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

I personally think switches are cleaner for this sort of thing.

switch (serverType) {
  case ServerType.Standalone:
    return TopologyType.single;
  case ServerType.Mongos:
    return TopologyType.Sharded;
  case ServerType.RSPrimary:
    return TopologyType.ReplicaSetWithPrimary;
  default:
    return TopologyType.Unknown;
}

Or if we want to keep it exactly the same (void return for unrecognized serverType), replace default: with

  case ServerType.RSGhost:
  case ServerType.Unknown:
    return TopologyType.Unknown;

Thoughts?

return TopologyType.ReplicaSetNoPrimary;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"uri": "mongodb+srv://test3.test.build.10gen.cc/?directConnection=false",
"seeds": [
"localhost.test.build.10gen.cc:27017"
],
"hosts": [
"localhost:27017",
"localhost:27018",
"localhost:27019"
],
"options": {
"ssl": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
uri: "mongodb+srv://test3.test.build.10gen.cc/?directConnection=false"
seeds:
- localhost.test.build.10gen.cc:27017
hosts:
- localhost:27017
- localhost:27018
- localhost:27019
options:
ssl: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"uri": "mongodb+srv://test3.test.build.10gen.cc/?directConnection=true",
"seeds": [],
"hosts": [],
"error": true,
"comment": "Should fail because directConnection=true is incompatible with SRV URIs."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
uri: "mongodb+srv://test3.test.build.10gen.cc/?directConnection=true"
seeds: []
hosts: []
error: true
comment: Should fail because directConnection=true is incompatible with SRV URIs.
2 changes: 2 additions & 0 deletions test/spec/server-discovery-and-monitoring/rs/compatible.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
description: "Replica set member with large maxWireVersion"

uri: "mongodb://a,b/?replicaSet=rs"

phases: [
{
responses: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
description: "Replica set member and an unknown server"

uri: "mongodb://a,b/?replicaSet=rs"

phases: [
{
responses: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"description": "Discover arbiters",
"uri": "mongodb://a/?replicaSet=rs",
"description": "Discover arbiters with directConnection URI option",
"uri": "mongodb://a/?directConnection=false",
"phases": [
{
"responses": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
description: "Discover arbiters"
description: "Discover arbiters with directConnection URI option"

uri: "mongodb://a/?replicaSet=rs"
uri: "mongodb://a/?directConnection=false"

phases: [

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"description": "Discover arbiters with replicaSet URI option",
"uri": "mongodb://a/?replicaSet=rs",
"phases": [
{
"responses": [
[
"a:27017",
{
"ok": 1,
"ismaster": true,
"hosts": [
"a:27017"
],
"arbiters": [
"b:27017"
],
"setName": "rs",
"minWireVersion": 0,
"maxWireVersion": 6
}
]
],
"outcome": {
"servers": {
"a:27017": {
"type": "RSPrimary",
"setName": "rs"
},
"b:27017": {
"type": "Unknown",
"setName": null
}
},
"topologyType": "ReplicaSetWithPrimary",
"logicalSessionTimeoutMinutes": null,
"setName": "rs"
}
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
description: "Discover arbiters with replicaSet URI option"

uri: "mongodb://a/?replicaSet=rs"

phases: [

{
responses: [

["a:27017", {

ok: 1,
ismaster: true,
hosts: ["a:27017"],
arbiters: ["b:27017"],
setName: "rs",
minWireVersion: 0,
maxWireVersion: 6
}]
],

outcome: {

servers: {

"a:27017": {

type: "RSPrimary",
setName: "rs"
},

"b:27017": {

type: "Unknown",
setName:
}
},
topologyType: "ReplicaSetWithPrimary",
logicalSessionTimeoutMinutes: null,
setName: "rs"
}
}
]
31 changes: 31 additions & 0 deletions test/spec/server-discovery-and-monitoring/rs/discover_ghost.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"description": "Discover ghost with directConnection URI option",
"uri": "mongodb://b/?directConnection=false",
"phases": [
{
"responses": [
[
"b:27017",
{
"ok": 1,
"ismaster": false,
"isreplicaset": true,
"minWireVersion": 0,
"maxWireVersion": 6
}
]
],
"outcome": {
"servers": {
"b:27017": {
"type": "RSGhost",
"setName": null
}
},
"topologyType": "Unknown",
"logicalSessionTimeoutMinutes": null,
"setName": null
}
}
]
}
35 changes: 35 additions & 0 deletions test/spec/server-discovery-and-monitoring/rs/discover_ghost.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
description: "Discover ghost with directConnection URI option"

uri: "mongodb://b/?directConnection=false"

phases: [

{
responses: [

["b:27017", {

ok: 1,
ismaster: false,
isreplicaset: true,
minWireVersion: 0,
maxWireVersion: 6
}]
],

outcome: {

servers: {

"b:27017": {

type: "RSGhost",
setName:
}
},
topologyType: "Unknown",
logicalSessionTimeoutMinutes: null,
setName:
}
}
]
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"description": "Ghost discovered",
"description": "Discover ghost with replicaSet URI option",
"uri": "mongodb://a,b/?replicaSet=rs",
"phases": [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
description: "Ghost discovered"
description: "Discover ghost with replicaSet URI option"

uri: "mongodb://a,b/?replicaSet=rs"

Expand Down
Loading