Replace Core Services

[MGatner]

Right now the only way to replace a core service is to create a new version in the extended version app/Config/Services.php. This means that modules and other namespaces cannot offer service replacements.

The problem is twofold:

1. All the system static methods are public which means they will match without using __callStatic’s lookup
2. The Services config discovery appends classes and then match the first instance, so the default system class will always match first

This PR provides a test case demonstrating the failure. If we want this as a "feature" (namespace replacement of core services) I could come back later and add it to this PR.

---

EDIT: This PR now applies the changes necessary to replace core services from any namespace. I took a different approach than outlined above because method_exists() is still true for protected static methods yet is_callable() is always true when there is a __callStatic() definition. This PR changes Config\Services to extend BaseServices which will skip anything defined in CodeIgniter\Config\Services and thus force discovery. Class precedence (number 2 above) has also been reworked to this priority: App, any other namespace, CodeIgniter (system).

[najdanovicivan]

This will be very useful feature.

It will be great if we can also add the support to load Common.php from Code Modules as well.

In my example I have the Template module which is used to manage frontend templates. It relies on overriding the view() method to load the appropriate view depending on currently set template.

So i had to use App/Common.php but this will be much better if the file can be located inside the code module. Of course in such case only the method from first loaded file will be used as this will be handled with function_exists() method

The implementation of this should be quite simple. We just need to check fi Common.php exist in any of the namespaces defined in Autoload $psr4 and just load the file from there

[michalsn]

I like this idea 👍

[MGatner]

I'm a little uncomfortable building on top of Common.php. It has always seemed to me like a bit of a catch-all that should have better ways of handling. For example, we have a very nice helper structure for loading functions from any namespace - why reinvent that just because helpers are not pre-loaded? Could we add a Config property (maybe to Autoload?) that would load a list of helpers super early? Just a thought.

[michalsn]

To be more precise - I like the idea of extending services functionality.

[MGatner]

People in this thread check out my edit to the PR above, and new commits.

[najdanovicivan]

The goal behind loading Common.php is to override the core function from framework. So this preloaded helper will have to be loaded before CI Common php.

Also the idea is to have it autolad if it existi in module. So it might be better to have config for it in modules confguration. And then load helper from each module where it exist

[MGatner]

I understand the goal but the part I am wary of is scanning all namespaces for such a core file, some of whose functions might be used in scanning namespaces for files. Since Common.php amounts to a list of function definitions (identical to Helpers) I am proposing that we have a way to bootstrap pre-defined Helpers instead, and they can trump definitions in system/Common.php. We do need to maintain precedence though, so app/Common.php should always come first.

Regardless of the implementation, I believe it is beyond the scope of this PR. I would recommend opening a new Issue (or PR if you have the time) to continue these thoughts.

[MGatner]

Thanks @michalsn

@najdanovicivan Anything else for this PR?

[najdanovicivan]

@MGatner I'm ok with all the changes here.

Will this one be a BC as it requires changing the parent class in App/Config/Services ?

[MGatner]

@najdanovicivan I don't believe so. We aren't actually instantiating the class since the methods are static, so there should be no concerns about instanceof. Because the methods are now inaccessible they will trigger __callStatic() which will trigger discovery and locate the methods from App.

Anyone else think of how this might break? @michalsn ?

[najdanovicivan]

@MGatner what will happen if App\Config\Services is not updated so it still extends Services instead of BaseService ?

[MGatner]

Everything will work fine. It will preclude third-party namespaces from superseding core services, since the methods will be accessible to Config\Services now, but since this is the developer App space it is probably a good thing that this new behavior is "opt-in".

[michalsn]

We should be good.

[najdanovicivan]

Ok then, It should all be good.

[kenjis]

@MGatner I just want to know what's the problem when modules and other namespaces cannot offer core service replacements?

If the only way to replace a core service is to create a new version in the extended version app/Config/Services.php, then we control all the object creation of core services in it. It is easy to find the method where a core service is created, and easy to know what the service class type is.

[MGatner]

@kenjis I don't understand your question. The point of this PR was to allow third-party namespaces to provide overriding core services. For example, prior to this PR if you wanted to use your own Router class for the service you would need to define it in app/Config/Services.php. But since this PR a developer could add a module (say, via Composer) that had its own Config/Services.php file to provide an alternative Router service.

[element-code]

Im not 100% sure but 99 :D
when CodeIgniter\Config\Services::exceptions() is called it internally calls static::response().
This leads to a problem since it won't any more check if there is an implementation in App\Config\Services::response() and other service providers and therefore won't use service replacements.
This did work fine as long as App\Config\Services did extend CodeIgniter\Config\Services since static was always the App\Config\Services class.
How to get it running again?

[kenjis]

@element-code You can use App\Config\Services that extends CodeIgniter\Config\Services.

[element-code]

Yeah @kenjis thats my current workaround but this wont help with modules.
@MGatner what about the static::response() call in CodeIgniter\Config\Services::exceptions(). Should this one work as expected or is there something that needs to be tweaked to work in the future?

[MGatner]

I believe this is a valid point. @element-code have you confirmed the issue? I think the solution is for CodeIgniter\Config\Services to replace all static::method() calls with \Config\Services::method() to enforce the proper entry point.

[element-code]

Jup, confirmed this, lets move to #4483