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: Usage of static::methodName in CodeIgniter\Config\Services prevents Service overriding #2376

Closed
dafriend opened this issue Oct 30, 2019 · 3 comments · Fixed by #2381
Closed
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@dafriend
Copy link
Contributor

dafriend commented Oct 30, 2019

The Bug

There are multiple methods in CodeIgniter\Config\Services that make calls to other methods in the class using the form static::someMethod().

For instance, see CodeIgniter\Config\Services::renderer() where the last line in the method is

return new \CodeIgniter\View\View(
            $config, 
            $viewPath, 
            static::locator(true), 
            CI_DEBUG, 
            static::logger(true) //  local scope!!!
);

The problem is that any overriding service defined in APPPATH/Config/Services.php is not in scope so the core class is delivered and not the extended one.

The following services are all called using the static::someMethod() form and so cannot be extended.

  • logger
  • request
  • response
  • locator
  • routes
  • cache
  • renderer

And I might have missed some. There are quite a few CodeIgniter\Config\Services that make static calls to those services so the ramifications would conceivably multiply.

CodeIgniter version
CI 4.0.0-rc.3 at commit 2c04e1c (10/28/19)

Affected module
Services

Reproducing the Problem

Extend Logger

<?php
namespace App\Log;

use Psr\Log\LoggerInterface;
use CodeIgniter\Log\Logger;

class Logger extends Logger implements LoggerInterface
{
    public function myNewMethod($message)
    {
        echo "New Logger Method says: $message";
    }
}

In APPPATH/Config/Services.php

public static function logger(bool $getShared = true)
{
    if ($getShared)
    {
        return static::getSharedInstance('logger');
    }

    return new \App\Log\Logger(new \Config\Logger\Logger());
}

In a controller (of your choice)

$this->logger->myNewMethod('There be Dragons!');

Produces the following error

Call to undefined method CodeIgniter\Log\Logger::myNewMethod()

Context

  • OS: Linux (Debian 9)
  • Web server Apache version 2.4.25
  • PHP version 7.2.17
@dafriend dafriend added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 30, 2019
@lonnieezell
Copy link
Member

I've overridden the renderer service previously and never hit any problems. Though looking at that code I'm calling self so I'm surprised it works. :)

The static keyword, unlike self, is bound the the calling class instead of the class it's defined in due to late static binding, IIRC.

Your problem seems to be that the myNewMethod method is protected and cannot be called from outside of the class.

@dafriend
Copy link
Contributor Author

The problem is not that myNewMethod is protected. The error message would be "Call to protected method" if that was the problem.

myNewMethod was marked as protected because I ran into this problem trying to get an extended version of Logger to work. It was protected there and I missed changing it to public when writing the "Reproducing the Problem" example. Even when defined as public the "Call to undefined method" error is given. So clearly the extended class never gets instantiated - only the core version does.

I'm not really certain, but could it be that the toolbar is getting to Services first thereby binding us to the core class?

I'm not seeing a call to Services::logger() (every call has been static::logger()) until CodeIgniter::createController() when the execution path does lead to APPPATH/Config/Services However, we already have an instance of Logger which getSharedInstance dutifully returns. Sadly, that instance doesn't have the method we're calling.

@dafriend
Copy link
Contributor Author

dafriend commented Nov 2, 2019

@lonnieezell
I have found the problem and it the following statement used by all the Toolbar Collectors

use CodeIgniter\Config\Services;

Which results in the App\Config\Services being bypassed.

By changing the use statement in all the Toolbar Collectors (Events, Logs, Routes, Timers, and Views) to:

use Config\Services;

Then CodeIgniter\Config\Services::Logger() actually gets overridden and Logger::myNewMethod() runs as expected.

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
Development

Successfully merging a pull request may close this issue.

2 participants