Skip to content

allow to override default Yii2 Logger class via DI container #1

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

Closed
wants to merge 1 commit into from
Closed

Conversation

albertborsos
Copy link

I have an enhanced Logger class which implements my LoggerInterface. All of my tests are failing because the default Logger class of Codeception is not implements this interface. I made a custom Logger class which extends the Codeception's Logger class and implements my LoggerInterface, but the Logger class is hardcoded.

With this PR it is possible to override in the test application via the configuration of the DI container, with the following way:

return [
     'components' => [
          ...
     ],
     'container' => [
          'definitions' => [
               \Codeception\Lib\Connector\Yii2\Logger::class => \tests\codeception\support\Logger::class,
          ],
     ],
];

@albertborsos
Copy link
Author

previous PR: Codeception/Codeception#5771

@SamMousa
Copy link
Collaborator

SamMousa commented Nov 28, 2019

I think that your tests should be failing.

    /**
     * @return Logger message logger
     */
    public static function getLogger()
    {
        if (self::$_logger !== null) {
            return self::$_logger;
        }

        return self::$_logger = static::createObject('yii\log\Logger');
    }
    /**
     * Sets the logger object.
     * @param Logger $logger the logger object.
     */
    public static function setLogger($logger)
    {
        self::$_logger = $logger;
    }

While the code does not use type hinting because Yii2 supports older PHP versions, the PHPDoc clearly indicates that the param of setLogger() must be an instance of Logger.
You are obviously free to tighten the return type of getLogger(), in this case to a subclass of Logger that implements LoggerInterface you are responsible for dealing with that when setLogger() is called.

If we merge this PR and allow for custom implementations that do not (properly) override the init function we will reintroduce memory leaks due to the use of register_shutdown_function.

Given that you have basically changed the contract in your code:

class Yii {

    public static function getLogger(): LoggerInterface 
    {
        ...
    }

I'm inclined to recommend actually fixing your code using composition instead of inheritance:

    public static function setLogger(Logger $logger) 
    {
         if ($logger instanceof LoggerInterface) {
            self::$_logger = $logger;
        } else {
            // Deal with a logger that does not implement your interface
        }
    }

I'm not a fan of static instantiation and would gladly replace it.
But I feel this is not the way to go about it.
I've actually worked on an alternative a while ago here: https://github.com/SamMousa/Codeception/tree/yii2-log-handling

That implementation doesn't replace your logger at all, so no issue there. All it does is add an extra log target for Codeception, which is what we want.

Also, it mocks register_shutdown_function in the yii\log namespace to prevent a memory leak

@albertborsos
Copy link
Author

I understand what you said, but in my case it is a different use case. I just extended the default Logger with some helper method. For example created($id, $name, $type) etc.

But now, I decoupled these functions into a new component, so I do not need this feature anymore.

@SamMousa
Copy link
Collaborator

Okay, thanks for spending your time making Codeception better!
Please go ahead and close this PR.

sunnyphp added a commit to sunnyphp/module-yii2 that referenced this pull request Aug 22, 2022
Fix last commit error:
```shell
Exception 'Error' with message 'Call to a member function getAndClearLog() on null' in /srv/www/app/vendor/codeception/module-yii2/src/Codeception/Module/Yii2.php:400

Stack trace:
#0 /srv/www/app/vendor/codeception/codeception/src/Codeception/Subscriber/Module.php(90): Codeception\Module\Yii2->_failed(Object(Codeception\Test\TestCaseWrapper), Object(PHPUnit\Framework\ExpectationFailedException))
Codeception#1 /srv/www/app/vendor/symfony/event-dispatcher/EventDispatcher.php(230): Codeception\Subscriber\Module->failed(Object(Codeception\Event\FailEvent), 'test.fail', Object(Symfony\Component\EventDispatcher\EventDispatcher))
...
```
samdark pushed a commit that referenced this pull request Aug 22, 2022
Fix last commit error:
```shell
Exception 'Error' with message 'Call to a member function getAndClearLog() on null' in /srv/www/app/vendor/codeception/module-yii2/src/Codeception/Module/Yii2.php:400

Stack trace:
#0 /srv/www/app/vendor/codeception/codeception/src/Codeception/Subscriber/Module.php(90): Codeception\Module\Yii2->_failed(Object(Codeception\Test\TestCaseWrapper), Object(PHPUnit\Framework\ExpectationFailedException))
#1 /srv/www/app/vendor/symfony/event-dispatcher/EventDispatcher.php(230): Codeception\Subscriber\Module->failed(Object(Codeception\Event\FailEvent), 'test.fail', Object(Symfony\Component\EventDispatcher\EventDispatcher))
...
```
uaoleg added a commit to uaoleg/module-yii2 that referenced this pull request Jan 8, 2023
SamMousa added a commit that referenced this pull request Jan 14, 2023
PHP 8.1: parse_str(): Passing null to parameter #1 ($string) of type string is deprecated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants