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

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

Open
zyz954489346 opened this issue Jan 29, 2024 · 6 comments

Comments

@zyz954489346
Copy link

zyz954489346 commented Jan 29, 2024

  • Laravel-mongodb Version: 4.1
  • PHP Version: 8.3
  • Database Driver & Version: 4

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
Copy link
Member

GromNaN commented Jan 29, 2024

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

@alcaeus
Copy link
Member

alcaeus commented Jan 29, 2024

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
Copy link
Author

zyz954489346 commented Jan 30, 2024

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
Copy link
Author

zyz954489346 commented Jan 30, 2024

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.

@zyz954489346 zyz954489346 changed the title Register the Mongo driver through the make function instead of resolving functiong Register the Mongo driver through the make function instead of resolving function Jan 30, 2024
@alcaeus
Copy link
Member

alcaeus commented Feb 1, 2024

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants