fix: correctly handle mongo replica set driver option (#7908)

* test: migrates should syntax to expect

So all testcase use a uniform expectation library

* fix: adds replicaSet and tls option to mongo connection url

fixes 7130

* test: add a unit test for the connection string building

src/driver/mongodb/MongoDriver.ts

@@ -448,15 +448,16 @@ export class MongoDriver implements Driver {
             ? `${options.username}:${options.password}@`
             : "";
 
-        let connectionString = undefined;
+
         const portUrlPart = (schemaUrlPart === "mongodb+srv")
             ? ""
             : `:${options.port || "27017"}`;
 
+        let connectionString: string;
         if(options.replicaSet) {
-            connectionString = `${schemaUrlPart}://${credentialsUrlPart}${options.hostReplicaSet || options.host + portUrlPart || "127.0.0.1" + portUrlPart}/${options.database || ""}`;
+            connectionString = `${schemaUrlPart}://${credentialsUrlPart}${options.hostReplicaSet || options.host + portUrlPart || "127.0.0.1" + portUrlPart}/${options.database || ""}?replicaSet=${options.replicaSet}${options.tls ? "&tls=true" : ""}`;
         } else {
-            connectionString = `${schemaUrlPart}://${credentialsUrlPart}${options.host || "127.0.0.1"}${portUrlPart}/${options.database || ""}`;
+            connectionString = `${schemaUrlPart}://${credentialsUrlPart}${options.host || "127.0.0.1"}${portUrlPart}/${options.database || ""}${options.tls ? "?tls=true" : ""}`;
         }
 
         return connectionString;

test/functional/driver/MongoDriver.ts

@@ -0,0 +1,54 @@
+import { expect } from "chai";
+import sinon from "sinon";
+import { Connection } from "../../../src";
+import { DriverUtils } from "../../../src/driver/DriverUtils";
+import { MongoDriver } from "../../../src/driver/mongodb/MongoDriver";
+
+describe("MongoDriver", () => {
+    async function getConnectionUrlFromFakedMongoClient(url: string): Promise<string> {
+        const options = DriverUtils.buildMongoDBDriverOptions({ url});
+
+        // Setup a MongoDriver with a mocked connect method, so we can get the connection
+        // url from the actual call afterwards.
+        const driver = new MongoDriver({
+            options
+        } as Connection);
+        const connect = sinon.fake();
+        driver.mongodb = {
+            ...driver.mongodb,
+            MongoClient: {
+                connect
+            }
+        };
+
+        const connectPromise = driver.connect();
+
+        // Take the promise parameter that we receive in the callback, call it, so the underlying promise gets resolved.
+        const firstMethodCall = connect.args[0];
+        const callback = firstMethodCall[2];
+        callback(undefined, {});
+        await connectPromise;
+
+        return firstMethodCall[0];
+    }
+
+    describe("connection string", () => {
+
+        it("should create a connection string for replica sets", async () => {
+            const url = "mongodb://username:password@someHost1:27017,someHost2:27018/myDatabase?replicaSet=abc&tls=true";
+
+            const connectionUrl = await getConnectionUrlFromFakedMongoClient(url);
+
+            expect(connectionUrl).to.eql(url);
+        });
+
+        it("should create a connection string for non replica sets", async() => {
+            const url = "mongodb://username:password@someHost1:27017/myDatabase?tls=true";
+
+            const connectionUrl = await getConnectionUrlFromFakedMongoClient(url);
+
+            expect(connectionUrl).to.eql(url);
+        });
+    });
+
+});

test/functional/driver/driver-utils.ts

@@ -0,0 +1,38 @@
+import { expect } from "chai";
+import { DriverUtils } from "../../../src/driver/DriverUtils";
+
+describe("DriverUtils", () => {
+    describe("parse mongo url", () => {
+        it("should return a mongo url with a replica set", () => {
+            const url = "mongodb://username:password@someHost1:27017,someHost2:27018/myDatabase?replicaSet=abc&tls=true";
+            const result = DriverUtils.buildMongoDBDriverOptions({ url});
+
+            expect(result).to.eql({
+                database: "myDatabase",
+                hostReplicaSet: "someHost1:27017,someHost2:27018",
+                password: "password",
+                replicaSet: "abc",
+                tls: "true",
+                type: "mongodb",
+                url,
+                username: "username"
+            });
+        });
+
+        it("should return a mongo url without a replica set", () => {
+            const url = "mongodb://username:password@someHost1:27017/myDatabase?tls=true";
+            const result = DriverUtils.buildMongoDBDriverOptions({ url});
+
+            expect(result).to.eql({
+                database: "myDatabase",
+                host: "someHost1",
+                port: 27017,
+                password: "password",
+                tls: "true",
+                type: "mongodb",
+                url,
+                username: "username"
+            });
+        });
+    });
+});

test/functional/mongodb/basic/mongo-repository/mongo-repository.ts

@@ -17,12 +17,12 @@ describe("mongodb > MongoRepository", () => {
 
     it("connection should return mongo repository when requested", () => Promise.all(connections.map(async connection => {
         const postRepository = connection.getMongoRepository(Post);
-        postRepository.should.be.instanceOf(MongoRepository);
+        expect(postRepository).to.be.instanceOf(MongoRepository);
     })));
 
     it("entity manager should return mongo repository when requested", () => Promise.all(connections.map(async connection => {
         const postRepository = connection.manager.getMongoRepository(Post);
-        postRepository.should.be.instanceOf(MongoRepository);
+        expect(postRepository).to.be.instanceOf(MongoRepository);
     })));
 
     it("should be able to use entity cursor which will return instances of entity classes", () => Promise.all(connections.map(async connection => {
@@ -44,11 +44,11 @@ describe("mongodb > MongoRepository", () => {
         });
 
         const loadedPosts = await cursor.toArray();
-        loadedPosts.length.should.be.equal(1);
-        loadedPosts[0].should.be.instanceOf(Post);
-        loadedPosts[0].id.should.be.eql(firstPost.id);
-        loadedPosts[0].title.should.be.equal("Post #1");
-        loadedPosts[0].text.should.be.equal("Everything about post #1");
+        expect(loadedPosts).to.have.length(1);
+        expect(loadedPosts[0]).to.be.instanceOf(Post);
+        expect(loadedPosts[0].id).to.eql(firstPost.id);
+        expect(loadedPosts[0].title).to.eql("Post #1");
+        expect(loadedPosts[0].text).to.eql("Everything about post #1");
 
     })));
 
@@ -79,14 +79,13 @@ describe("mongodb > MongoRepository", () => {
             }
         });
 
-        loadedPosts.length.should.be.equal(1);
-        loadedPosts[0].should.be.instanceOf(Post);
-        loadedPosts[0].id.should.be.eql(firstPost.id);
-        loadedPosts[0].title.should.be.equal("Post #1");
-        loadedPosts[0].text.should.be.equal("Everything about post #1");
-
+        expect(loadedPosts).to.have.length(1);
+        expect(loadedPosts[0]).to.be.instanceOf(Post);
+        expect(loadedPosts[0].id).to.eql(firstPost.id);
+        expect(loadedPosts[0].title).to.eql("Post #1");
+        expect(loadedPosts[0].text).to.eql("Everything about post #1");
     })));
-  
+
     it("should be able to use findByIds with both objectId and strings", () => Promise.all(connections.map(async connection => {
         const postRepository = connection.getMongoRepository(Post);
 
@@ -128,10 +127,9 @@ describe("mongodb > MongoRepository", () => {
 
         const loadedPosts = await postRepository.find();
 
-        loadedPosts.length.should.be.equal(2);
-        loadedPosts[0].text.should.be.equal("Everything and more about post #1");
-        loadedPosts[1].text.should.be.equal("Everything about post #2");
-
+        expect(loadedPosts).to.have.length(2);
+        expect(loadedPosts[0].text).to.eql("Everything and more about post #1");
+        expect(loadedPosts[1].text).to.eql("Everything about post #2");
     })));
 
     it("should ignore non-column properties", () => Promise.all(connections.map(async connection => {