moleculer-db
moleculer-db copied to clipboard
Correct Database per service + mongoose V8
This PR will correct the "database per service" pattern . So, running multiples services on the same node, can use different databases connections (one per service) .
Because this can include some breaking changes, I choose to upgrade mongoose to V7 (and so drop old code supporting older versions)
I will pass this PR in ready when I think it will be good . For the moment, I will just to tests . Don't hesitate to do some discussions
A note about virtual tests .
Actually, they are using global models ( from global mongoose connection ) .
So, they doesn't work, because the default connection is never connected, and so, we can't use the models .
Virtuals seems to come from this PR : https://github.com/moleculerjs/moleculer-db/pull/354 . But, the logic seems not working with the "one database per service" pattern, because mongoose can't load the other object ... I need to investigate how to handle this, and what is the problem resolved by the other PR
@icebob I have a choice to do .
By default, did we create a new connection, or use the global one ? Pros :
- people using the global connection will not be impacted by a breaking change on this
Cons :
- this will be "first come first served", so sometimes the global connection will not be the same connection (and will become harder to debug)
I think, don't using the global connection is better, because everyone need to adapt following the breaking change .
( about the rest of the code, it doesn't change so much )
Also, did you remember why of this : https://github.com/moleculerjs/moleculer-db/blame/master/packages/moleculer-db-adapter-mongoose/src/index.js#L326-L330 ( and why not reusing the objectIDToString function ? )
By default, did we create a new connection, or use the global one ? I think, don't using the global connection is better, because everyone need to adapt following the breaking change.
Since a Mongoose 7 upgrade already causes a breaking change, we can adapt the new connection way, but we should write a migration help for mongoose adapter users on how they should migrate their codes to work with the new version.
( about the rest of the code, it doesn't change so much )
Also, did you remember why of this : https://github.com/moleculerjs/moleculer-db/blame/master/packages/moleculer-db-adapter-mongoose/src/index.js#L326-L330 ( and why not reusing the objectIDToString function ? )
Hmm, strange, I have no idea why I implemented it this complicated way. :) You can rewrite this method.
Hey! Mongoose 8.0.0-rc0 was released two days ago.
The stable version will be out any time soon.
As the PR is not acheived yet, you may consider skipping Mongoose 7.x.x in favor of 8.x.x
Keep up the good work !
I'm doing some tests with V8 .
And, first, I think I will need to do something different to handle user using global connection .
not sure if I add an option "useGlobalConnection", or a method that will be called with the current connection, I think I prefer the second one to respect "database per service" . But option 1 will maybe be easier for users .
Also, it seems that mongoose V8 remove/rename some functions ( findByIdAndRemove disapear )