Connecting without the database name yields an Authentication error

[rchaput]

I am using y-mongodb-provider in a small project in which I already have a working Mongo connection. When reusing the same exact connection string, y-mongodb-provider cannot access the database (any attempt to get or write a YDoc yields an "Authentication error").
I have been able to pinpoint this problem to the fact that y-mongodb-provider removes the database name when creating a MongoClient, in the MongoAdapter (and especially its parseMongoDBConnectionString helper function).
When commenting the url.pathname = '/'; line, everything works perfectly fine.

While I could probably solve this by adding more permissions to my mongo user (perhaps logging to the default db instead of my specific project db?), I'd rather keep the users and dbs neatly contained, each user having access to a specific db. Is there a rationale for removing the database name from the connection string?

On a related note, it would be useful to be able to give y-mongodb-provider an existing instance of MongoClient, instead of forcing to create a new instance (and thus a new connection to the database).
I've also been able to do this, by replacing the signatures of your constructors, and putting client and db optional options inside the dictionary. When these variables are set, MongoAdapter uses them; otherwise, we expect connectionString to be set, and create a new connection as before.

See for example:

	constructor({ connectionString, client, db, collection, multipleCollections }) {
		this.collection = collection;
		this.multipleCollections = multipleCollections;

		if (client && db) {
			this.client = client;
			this.db = db;
		} else {
			const connectionParams = parseMongoDBConnectionString(location);
			this.client = new MongoClient(connectionParams.linkWithoutDatabase);
			this.db = this.client.db(connectionParams.database);
		}
	}

I can send a PR fairly quickly, but since it would be a breaking change (because of the different signature), I prefer asking first what you think of this idea.

[MaxNoetzold]

Hey,

Thank you for bringing up this issue.

Is there a rationale for removing the database name from the connection string?

I initially thought it was necessary to not specify the database in the client creation when using:
this.db = this.client.db(this.databaseName);
In my mind, it didn't make sense to connect to a database with new MongoClient() and then connect to the database again. Since this isn't the case, we can definitely remove this part.

On a related note, it would be useful to be able to give y-mongodb-provider an existing instance of MongoClient

I completely agree, this is a great idea. My only concern is how we should modify the constructor parameters to avoid breaking existing code.

So perhaps we can change from:
constructor(connectionString, { collection, multipleCollections })
to sth like:
constructor(connectionObj, { collection, multipleCollections })
where connectionObj can be either the connection string or an object with client and db.

I would like to know your thoughts on this, @raineorshine, if you have time.

[rchaput]

I initially thought it was necessary to not specify the database in the client creation when using: this.db = this.client.db(this.databaseName); In my mind, it didn't make sense to connect to a database with new MongoClient() and then connect to the database again. Since this isn't the case, we can definitely remove this part.

My understanding is that mongo allows to authentify the user with respect to a certain "authentication database" (see the "defaultauthdb" option); the user may then open one or several databases for read/write operations, which may be the same as the auth db, or not.

So perhaps we can change from: constructor(connectionString, { collection, multipleCollections }) to sth like: constructor(connectionObj, { collection, multipleCollections }) where connectionObj can be either the connection string or an object with client and db.

Love this idea, much better than mine. This way we can keep compatibility with previous versions. And it will be clearer in the documentation that exactly one of connectionString or { client, db } must be set.

[MaxNoetzold]

I resolved the issue as we discussed and released a new version on npm. so i close this issue