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

Bug: New Service replacement fails at service provider precedence on core factory implementations #4483

Closed
element-code opened this issue Mar 24, 2021 · 5 comments · Fixed by #4593
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@element-code
Copy link
Contributor

element-code commented Mar 24, 2021

When a core service factory implementation calls another service (which should be overwritten by an app or module service), the service provider precedence doesn't work.

When CodeIgniter\Config\Services::exceptions() is called it internally calls static::response().
As of #3943 this should again check for response service factories in all service providers. Instead, the core factory static::response is used.
Tl;dr: Right now we can't replace services when they are called in core factories.
Workaround: Let Config\Services extend CodeIgniter\Config\Services , as done before.

CodeIgniter 4 version
development

Affected module(s)
Services

Expected behavior, and steps to reproduce if appropriate
All service providers should never call static::foobar() as this ignores the service provider precedence.
Instead, there should be a simple way to fetch the right service provider / service.

Reproduce

  1. Provide an own response service via any service provider
  2. Call in your controller:
dd(
	$this->response, 
	Services::response(NULL, FALSE), 
	Services::response(NULL,TRUE)
); 
  1. the shared instance is the system instance (since it got created while CI boot through the exceptions service).
    When creating a new instance from the config service provider we get the right service.
    grafik
@element-code element-code added the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 24, 2021
@element-code
Copy link
Contributor Author

element-code commented Mar 24, 2021

@MGatner is the right way to get a service always to call \App\Config\Service::foobar() \Config\Service::foobar()?

@MGatner
Copy link
Member

MGatner commented Mar 24, 2021

Right idea but it is actually \Config\Services::service() because of Config's special namespace. Also theoretically I could see wanting to access a service without lookup from System services directly, but we should never do that in the framework.

The helper function service() is basically a convenient way to do the former.

@element-code
Copy link
Contributor Author

So then i replace all parent::foobar() calls in CodeIgniter\Config\Services?

@MGatner
Copy link
Member

MGatner commented Mar 25, 2021

I think it should be static::foobar() that is replaced with Services::foobar().

@element-code
Copy link
Contributor Author

Yeah 🤦‍♂️🤦‍♂️🤦‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
2 participants