mongodb / laravel-mongodb

A MongoDB based Eloquent model and Query builder for Laravel (Moloquent)
https://www.mongodb.com/docs/drivers/php/laravel-mongodb/
MIT License
7.02k stars 1.43k forks source link

Register the Mongo driver through the make function instead of resolving function #2715

Open zyz954489346 opened 9 months ago

zyz954489346 commented 9 months ago

Description:

Register the Mongo driver through the make function instead of resolving functiong

If db is instantiated in another package, then the callback function registered through resolving function will not be executed, which is a hidden BUG.

This problem does not occur stably, only if a package is sorted by the composer and executes its own provider before the laravel-mongodb provider, and exactly instantiates db.

The core code is: image

 In my project, the following code exists in other packages, which can lead to problems. Because db is instantiated in advance: image

Problems caused by,there is an error: Unsupported driver [mongodb]. - InvalidArgumentException

GromNaN commented 9 months ago

Hello @zyz954489346, do you have any idea how we can fix this issue?

alcaeus commented 9 months ago

Hi @zyz954489346, I believe the issue is actually in your code. Quoting from the Service Providers documentation:

Within the register method, you should only bind things into the service container. You should never attempt to register any event listeners, routes, or any other piece of functionality within the register method.

In your code, you are registering a listener, which you shouldn't be doing in register, but rather in boot. Again, quoting from the documentation:

So, what if we need to register a view composer within our service provider? This should be done within the boot method. This method is called after all other service providers have been registered, meaning you have access to all other services that have been registered by the framework.

zyz954489346 commented 9 months ago

Hi @zyz954489346, I believe the issue is actually in your code. Quoting from the Service Providers documentation:

Within the register method, you should only bind things into the service container. You should never attempt to register any event listeners, routes, or any other piece of functionality within the register method.

In your code, you are registering a listener, which you shouldn't be doing in register, but rather in boot. Again, quoting from the documentation:

So, what if we need to register a view composer within our service provider? This should be done within the boot method. This method is called after all other service providers have been registered, meaning you have access to all other services that have been registered by the framework.

@alcaeus You're right, but there's no way to let other package writers to do that. As far as I can see, there are actually some packages that don't quite conform to the specification. This leads to problems. As a user of the package, there is no other way but to override the Provider. 

Another thing I don't understand is why you register through the resolving function instead of the make function. I read other similar packages, most of which are register through the make function.

zyz954489346 commented 9 months ago

Hello @zyz954489346, do you have any idea how we can fix this issue?

@GromNaN I temporarily solved the problem by rewriting Provider, but this is really not an elegant solution.

alcaeus commented 9 months ago

As far as I can see, there are actually some packages that don't quite conform to the specification. This leads to problems.

I agree. The specification says to not request dependencies in register, as not all services have been registered at that point. If other tools ignore that, that's on them and should be reported (and fixed) there.

Another thing I don't understand is why you register through the resolving function instead of the make function.

The make method actually resolves a dependency, which isn't what we need. We need to call the extend method on the DatabaseManager service to inject a custom extension for the mongodb database driver. We can't use the extend method of the service container for this, but instead need to inject this when the db service is resolved. The only way to achieve this is by using resolving.

Like I said: if tools break because do what a bolded sentence in the docs tells them not to do, that's not on us, that's on them. I'm closing this as won't fix.

maojindao55 commented 2 months ago

i hit the same problem until i close the laravel-sql-logger package, thx