Skip to content

Commit

Permalink
fix: correctly handle mongo replica set driver option (typeorm#7908)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
klassm authored and imnotjames committed Jul 21, 2021
1 parent fff19f2 commit 423aef5
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 21 deletions.
7 changes: 4 additions & 3 deletions src/driver/mongodb/MongoDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
54 changes: 54 additions & 0 deletions test/functional/driver/MongoDriver.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});

});
38 changes: 38 additions & 0 deletions test/functional/driver/driver-utils.ts
Original file line number Diff line number Diff line change
@@ -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"
});
});
});
});
34 changes: 16 additions & 18 deletions test/functional/mongodb/basic/mongo-repository/mongo-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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");

})));

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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 => {
Expand Down

0 comments on commit 423aef5

Please sign in to comment.