SierraSoftworks / Iridium

A high performance MongoDB ORM for Node.js
http://sierrasoftworks.github.io/Iridium/
Other
570 stars 25 forks source link

Feature Request: Auto-Connect #109

Open CatGuardian opened 6 years ago

CatGuardian commented 6 years ago

I thought of a useful idea which came about from a problem I am struggling with. The problem is I don't think I should have to do 'await myCore.connect()' everywhere I want to do something that requires the Core to be already connected. For example: I shouldn't have to ' await db.connect()' before doing this:

// db is an instance of something that extends Core and db.Users is an instance of Model
const user = new User(db.Users, {
  name: 'First User',
});

const savedUser = await user.save();

The save() method should behind the scenes do an 'await db.connect()' before proceeding on with whatever save() normally does.

This should apply to all methods needing the database to be connected. Examples: find, get, select, etc...

This type of 'lazy connection' grabbing is done by another popular database library pg-promise. http://vitaly-t.github.io/pg-promise/index.html See Database section.

notheotherben commented 6 years ago

Hi @CatGuardian,

This was something I did consider when creating Iridium, however I opted for the explicit approach for a number of reasons. Most notably, people tend to be bad at handling a wide range of error cases in their code (often just assuming the most obvious failure mode is the only one to deal with). In practice, an error thrown by the .save() method usually indicates an issue with the data being saved or a transient upstream issue. On the other hand, a failure to .connect() probably indicates that your application isn't configured correctly (if it's the first time it's being called) or your environment is not set up to spec.

By having an explicit .connect() method, it encourages people to handle errors relating to connection failures in a sensible manner, without polluting the various other interactions with error handling for connection issues.

Touching on the second (implicit) part of your question, the "best practice" for using Iridium's core is not to instantiate (or maintain an instance of it) for each request, but rather to have a singleton core for your application's lifecycle, which is instantiated and configured when the application starts and lasts until the application is shutdown.

This approach allows you to manage tasks such as schema migrations, index creation and data bootstrapping in a single, centralized location and ties well into the notion of an application "readiness" healthcheck (which only succeeds once the application has started and is ready to serve requests correctly).

It also takes advantage of the connection pooling support provided by the MongoDB client driver, as well as its robust support for re-connection and live topology changes.

import {MyCore, bootstrap} from "./db"
import {Application} from "./Application"

const config = require("./config.json")

const core = new MyCore(config.db)
console.log("Connecting to DB")
await core.connect()

console.log("Bootstrapping DB")
await bootstrap(core)

const app = new Application(core, config.app)

console.log(`Listening on ${config.app.port}`)
await app.listen()

Let me know if you still feel there's a strong motivation for including auto-connect in spite of this, or if the approach above solves your requirements.

Regards,

CatGuardian commented 6 years ago

Thank you for your detailed answer. And yes a solution where I could import it once and await on the connect once and then export the singleton would be fantastic! And that is what I tried doing. However typescript gave me an error saying I cannot await on something that isn't in an "async" method.

So your example doesn't really work. I use express if that helps you to help me. But if u can think of a way to export a singleton that is already connected to the database then that would help me tremendously.

Also I am trying to write Mocha unit tests so I was hoping for a clean way where I didn't need to set up the database every time in each individual mocha file. That could be accomplished by having the save() method await connect() for me and then I could just await save() in the unit test and wouldn't have to worry about 'await connect()' first. But this isn't as big of a deal I suppose I could await connect in every file.

I'm really more concerned with how to make a singleton and exporting it so I can import the already awaited and connected db.

Oh it might help to understand that I have my database layer in its own npm package and I am importing my database npm package into my API node.js app. So ideally I'm looking for something like "import my various Instance or Model classes" instead of importing the entire 'MyDatabase extends Core' kind of thing because I feel like we shouldn't import more than necessary in files.