Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to provide partialConfig object when using DB URI in constructor #196

Closed
joezappie opened this issue Jun 12, 2022 · 7 comments · Fixed by #214
Closed

Ability to provide partialConfig object when using DB URI in constructor #196

joezappie opened this issue Jun 12, 2022 · 7 comments · Fixed by #214
Labels
area/core Refers to Mongo Seeding library 🚀 enhancement New feature or request
Milestone

Comments

@joezappie
Copy link

joezappie commented Jun 12, 2022

There is an old issue from 2019 #73 about being able to switch between databases. I'm currently trying to do something similar in a multi tenant approach where I want to seed a bunch of different DB's.

The issue I have is being able to change the database when using a URI. My project reuses a single config file which has my URI string in it as every library seems to support URI's. This however makes it difficult to change the DB name.

Other libraries I use will connect with the URI to my default DB. Then there typically is a function do call .db(name) on the mongo client to switch the database.

I think this could be added as an additional parameter to .import(collections, database) or as a separate function to switch DB.

There was some talk in #73 about how to handle databases with different credentials and I think that roadblocked the feature. In that case, if you wanted to use a DB with different user then you could still create a new seeder with that. For development, I think most people are going to have it all under one user and that wouldn't be an issue.

@pkosiec
Copy link
Owner

pkosiec commented Jul 26, 2022

Hi @jrj2211,

If I understood you correctly:

  • you use Mongo Seeding TS/JS library
  • you create Seeder with database URI
  • you want to change the database while having the same Seeder instance

Is that correct? If so, then you should be able to use the import function with different partialConfig provided as the optional parameter: https://github.com/pkosiec/mongo-seeding/tree/main/core#importcollections-partialconfig.
But, looking at the current code, merging won't work in such case - is that right?

If this is the case, then it could be easily done - while loading db URI as the base config, Mongo Seeding could convert that to DB object internally. That would enable merging DB configs regardless it's string or object form.

Let me know if that helps!

EDIT: Edited the comment after second thought. I think I understand the problem now.

@joezappie
Copy link
Author

joezappie commented Jul 26, 2022

I totally missed the partialConfig option in the readme! Thats correct though, I want to connect with a URI string, then change the DB later on. Just to clarify, what your suggestion to switch to 'my-second-db' is provide the object form of the database in the partial config but this would require a change to mongo-seeding? If you don't have the time I'd be willing to provide a PR to add this in.

  // Initial connection
  const seeder = new Seeder({
    database: mongodb://localhost:27017/initialdb
  });


  const collections = seeder.readCollectionsFromPath(path);
  
  // Switch to alternative DB and seed
  await seeder.import(collections, {
    database: {
      name: 'my-second-db'
    }
  });

I had been doing a string replace of my original URI with my alternate database and creating a whole new seeder instance so thank you for help.

@pkosiec
Copy link
Owner

pkosiec commented Jul 31, 2022

Hey @jrj2211,
Exactly 🙂 - my suggestion is to change the Mongo Seeding behavior to be able to provide string URI in constructor, and then provide partial config object to e.g. change DB. The snippet you provided would work after the change.

I would love to accept such PR! I think the best way would be to do it inside mergeSeederConfig:

export const mergeSeederConfig = (
. Before running extend function, we should check if database is string - if so, we could convert it to SeederDatabaseConfigObject object. Maybe there are already some helpers to parse MongoDB URI inside mongodb library? 🤔 That's an open question.

Let me know if you'd like to take care of this 🙂 Cheers!

@pkosiec pkosiec changed the title Ability to change database Ability to provide partialConfig object when using DB URI in constructor Jul 31, 2022
@pkosiec pkosiec added 🚀 enhancement New feature or request area/core Refers to Mongo Seeding library and removed question labels Jul 31, 2022
@joezappie
Copy link
Author

joezappie commented Aug 2, 2022

Hey @pkosiec, I'll give it a shot this week.

@joezappie
Copy link
Author

joezappie commented Feb 18, 2023

Turns out that week I was busy and then totally forgot about this 😄

I've forked the project and am looking at this now if your still interested in including this feature.

I've done a little digging and while Mongo does have a URL parser that can take in a URI and break it up into all of its components, theres no good way to collapse it back into a string. Nor does the MongoClient allow you to pass in the configuration object directly.

The object that the URL parser returns is much more complex than the SeederConfig, so taking a URI, parsing it, and then populating it into the SeederConfig would probably be a big challenge. It also would probably be hard to maintain, ensuring no properties are accidentally left out. Heres an example (not sure how this object would look with more complex URI's such as sharding):

{
  usingSrv: false,
  auth: { user: 'myDBReader', password: 'D1fficultP@ssw0rd' },
  server_options: { socketOptions: {} },
  db_options: {
    read_preference_tags: null,
    authSource: 'admin',
    read_preference: 'primary'
  },
  rs_options: { socketOptions: {} },
  mongos_options: {},
  dbName: 'admin',
  servers: [ { host: 'mongodb0.example.com', port: 27017 } ]
}

While it would be nice to support partial config to merge in any part of the URI, I'm not sure the feasibility of this if a URI is used. The other option is to only allow the "database.name" in a partial config if a URI is given. This is easily accomplished by switching out the db after connecting, as Mongo handles that well out of the box:

import = async (
    collections: SeederCollection[],
    partialConfig?: DeepPartial<SeederConfig>,
  ) => {
    
   ...

    let database: Database | undefined;
    try {
      database = await databaseConnector.connect(config.database);
  
      // Switch DB's if a new DB name is given
      if(config.database?.hasOwnProperty('uri') && config.database?.hasOwnProperty('name')) {
        database = database.db(config.database);
      }

Only issue there is merging "name" from a partial config when "database" is a string. I think SeederConfig would have to be updated to always convert a string being provided for database to an object with a uri property so name can be merged into it:

database = {
   "uri": "mongodb://localhost:27017",
   "name": "second-db"
}

Does this give you any ideas? I'll continue playing around with this but wanted to share my initial findings.

@joezappie
Copy link
Author

I think what might be best is to use a library like:
https://www.npmjs.com/package/connection-string

Supports both parsing and building of URI's. Reading through some comments, it should fully support parsing of mongo URI's.

If a partialConfig is provided and the config is a string, it could parse it, update it from the SeederConfig params, then rebuild the URI string?

@pkosiec
Copy link
Owner

pkosiec commented Feb 25, 2023

Hey @jrj2211,
I'm sorry for a little delay.
Thank you very much for reaching out and for the very valuable research! Yes, I think using connection-string would make sense 👍 The solution you suggested sounds great, as long as we will keep the SeederConfig fully typed 👍

@pkosiec pkosiec added this to the 4.0.0 milestone Dec 9, 2023
pkosiec added a commit that referenced this issue Dec 9, 2023
- Ability to provide partialConfig object when using DB URI in constructor (Resolves: #196)
- Fix encoding password in MongoDB connection URI (Resolves: #209)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core Refers to Mongo Seeding library 🚀 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants