Skip to content

Commit

Permalink
fix: handle Undefined values in driver URL options (#6925)
Browse files Browse the repository at this point in the history
the `buildDriverOptions` parses the URL & will have undefined values
which it ends up overwriting non-undefined values with - which isn't great
  • Loading branch information
imnotjames committed Oct 17, 2020
1 parent a09fb7f commit 6fa2df5
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 25 deletions.
22 changes: 11 additions & 11 deletions src/driver/DriverUtils.ts
Expand Up @@ -16,18 +16,18 @@ export class DriverUtils {
*/
static buildDriverOptions(options: any, buildOptions?: { useSid: boolean }): any {
if (options.url) {
const parsedUrl = this.parseConnectionUrl(options.url);
let urlDriverOptions: any = {
type: parsedUrl.type,
host: parsedUrl.host,
username: parsedUrl.username,
password: parsedUrl.password,
port: parsedUrl.port,
database: parsedUrl.database
};
if (buildOptions && buildOptions.useSid) {
urlDriverOptions.sid = parsedUrl.database;
const urlDriverOptions = this.parseConnectionUrl(options.url) as { [key: string]: any };

if (buildOptions && buildOptions.useSid && urlDriverOptions.database) {
urlDriverOptions.sid = urlDriverOptions.database;
}

for (const key of Object.keys(urlDriverOptions)) {
if (typeof urlDriverOptions[key] === 'undefined') {
delete urlDriverOptions[key];
}
}

return Object.assign({}, options, urlDriverOptions);
}
return Object.assign({}, options);
Expand Down
28 changes: 14 additions & 14 deletions src/driver/mongodb/MongoDriver.ts
Expand Up @@ -17,6 +17,7 @@ import {EntityMetadata} from "../../metadata/EntityMetadata";
import {ObjectUtils} from "../../util/ObjectUtils";
import {ApplyValueTransformers} from "../../util/ApplyValueTransformers";
import {ReplicationMode} from "../types/ReplicationMode";
import {DriverUtils} from "../DriverUtils";

/**
* Organizes communication with MongoDB.
Expand Down Expand Up @@ -223,9 +224,11 @@ export class MongoDriver implements Driver {
*/
connect(): Promise<void> {
return new Promise<void>((ok, fail) => {
const options = DriverUtils.buildDriverOptions(this.options);

this.mongodb.MongoClient.connect(
this.buildConnectionUrl(),
this.buildConnectionOptions(),
this.buildConnectionUrl(options),
this.buildConnectionOptions(options),
(err: any, client: any) => {
if (err) return fail(err);

Expand Down Expand Up @@ -434,30 +437,27 @@ export class MongoDriver implements Driver {
/**
* Builds connection url that is passed to underlying driver to perform connection to the mongodb database.
*/
protected buildConnectionUrl(): string {
if (this.options.url)
return this.options.url;

const credentialsUrlPart = (this.options.username && this.options.password)
? `${this.options.username}:${this.options.password}@`
protected buildConnectionUrl(options: { [key: string]: any }): string {
const credentialsUrlPart = (options.username && options.password)
? `${options.username}:${options.password}@`
: "";

return `mongodb://${credentialsUrlPart}${this.options.host || "127.0.0.1"}:${this.options.port || "27017"}/${this.options.database}`;
return `mongodb://${credentialsUrlPart}${options.host || "127.0.0.1"}:${options.port || "27017"}/${options.database || ""}`;
}

/**
* Build connection options from MongoConnectionOptions
*/
protected buildConnectionOptions(): any {
protected buildConnectionOptions(options: { [key: string]: any }): any {
const mongoOptions: any = {};

for (let index = 0; index < this.validOptionNames.length; index++) {
const optionName = this.validOptionNames[index];

if (this.options.extra && optionName in this.options.extra) {
mongoOptions[optionName] = this.options.extra[optionName];
} else if (optionName in this.options) {
mongoOptions[optionName] = (this.options as any)[optionName];
if (options.extra && optionName in options.extra) {
mongoOptions[optionName] = options.extra[optionName];
} else if (optionName in options) {
mongoOptions[optionName] = options[optionName];
}
}

Expand Down
22 changes: 22 additions & 0 deletions test/github-issues/6900/entity/Warn.ts
@@ -0,0 +1,22 @@
import { Entity, ObjectID, ObjectIdColumn, Column } from "../../../../src";

@Entity("warnings")
export class Warn {
@ObjectIdColumn()
id!: ObjectID

@Column()
guild!: string;

@Column()
user!: string;

@Column()
moderator!: string;

@Column()
reason!: string;

@Column()
createdAt!: Date;
}
75 changes: 75 additions & 0 deletions test/github-issues/6900/issue-6900.ts
@@ -0,0 +1,75 @@
import { expect } from "chai";
import {
closeTestingConnections, reloadTestingDatabases,
setupTestingConnections
} from "../../utils/test-utils";
import {MongoDriver} from "../../../src/driver/mongodb/MongoDriver";
import {Connection, ConnectionOptions, createConnection, MongoClient} from "../../../src";
import {Warn} from "./entity/Warn";

describe("github issues > #6900 MongoDB ConnectionManager doesn't select given database, creates new database \"test\" instead", () => {
let connections: Connection[] = [];
afterEach(async () => {
await closeTestingConnections(connections);
connections.length = 0;
});

it("should connect to the expected database", async () => {
const options = setupTestingConnections({ enabledDrivers: ["mongodb"] });

if (options.length === 0) {
// Skip if we can't grab the mongodb
return;
}

const connection = await createConnection({
...options[0],
url: 'mongodb://localhost',
database: 'foo'
} as ConnectionOptions);
connections.push(connection);

await reloadTestingDatabases(connections);

const mongoDriver = connection.driver as MongoDriver ;
const client = (mongoDriver.queryRunner!.databaseConnection as any) as MongoClient;

expect(client.db().databaseName).to.be.equal('foo');
expect(mongoDriver.database).to.be.equal('foo');
});

it("should write data to the correct database", async () => {
const options = setupTestingConnections({ enabledDrivers: ["mongodb"] });

if (options.length === 0) {
// Skip if we can't grab the mongodb
return;
}

const connection = await createConnection({
...options[0],
entities: [ Warn ],
url: 'mongodb://localhost',
database: 'foo'
} as ConnectionOptions);
connections.push(connection);

await reloadTestingDatabases(connections);

const repo = connection.getRepository(Warn)

await repo.insert({
id: Math.floor(Math.random() * 1000000),
guild: "Hello",
user: "WORLD",
moderator: "Good Moderator",
reason: "For Mongo not writing correctly to the databsae!",
createdAt: new Date()
});

const mongoDriver = connection.driver as MongoDriver ;
const client = (mongoDriver.queryRunner!.databaseConnection as any) as MongoClient;

expect(await client.db('foo').collection('warnings').count({})).to.be.greaterThan(0);
})
});

0 comments on commit 6fa2df5

Please sign in to comment.