Make Phalcon\Logger\Adapter compatible with PSR-3

[ghost]

This includes:

• all debug/warning/etc methods accept optional $context parameter (array $context = array())
• log() method has its arguments backwards: first $level, then $message. We can accept both this and the old way
• Psr\Log\LogLevel constants are string, therefore our methods accept both string and integer values

[ghost]

Apparently we cannot implement Psr\Log\LoggerInterface because that interface is created at runtime

[ghost]

OK to merge :-)

[phalcon]

Great

[ghost]

@phalcon Do you think it is worth adding this to Phalcon: https://github.com/sjinks/psr3logger

My concern is that Phalcon\Logger cannot be used where Psr\Log\LoggerInterface is required — just because that interface is created after Phalcon is initialized and destroyed when the request finishes.

However, when that interface is loaded from a PHP extension, we can use it safely.

I was thinking about a php.ini option, say, phalcon.register_psr3_classes (off by default) which controls whether Phalcon registers all those PSR-3 classes (and in this case, Logger can implement LoggerInterface); if the value is off, nothing changes.

What do you think?

[phalcon]

It's ok, it would not break existing applications unless developers are explicitly requiring those interfaces instead of using an autoloader.

[temuri416]

Guys,

I've just recompiled Phalcon and my log device is now completely broken.

I understand reasons for making it PSR-3 compatible, but this a drastic change.

You'd veto a change because it may break existing code, but what about this one here?

App\Log\Stub contains 3 abstract methods and must therefore be declared abstract or implement the remaining methods (Phalcon\Logger\Adapter::logInternal, Phalcon\Logger\AdapterInterface::getFormatter, Phalcon\Logger\AdapterInterface::close)

What are those methods supposed to do?

[temuri416]

To give you more info. I have a class that acts as log stub:

class Stub extends \Phalcon\Logger\Adapter
{
    /**
     * Sends/Writes an emergence message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function emergence($message, array $context=null){ }


    /**
     * Sends/Writes a debug message to the log
     *
     * @param string $message
     * @param ing $type
     * @return \Phalcon\Logger\Adapter
     */
    public function debug($message, array $context = NULL){ }


    /**
     * Sends/Writes an error message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function error($message, array $context = NULL){ }


    /**
     * Sends/Writes an info message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function info($message, array $context = NULL){ }


    /**
     * Sends/Writes a notice message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function notice($message, array $context = NULL){ }


    /**
     * Sends/Writes a warning message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function warning($message, array $context = NULL){ }


    /**
     * Sends/Writes an alert message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function alert($message, array $context = NULL){ }


    /**
     * Logs messages to the internal loggger. Appends logs to the
     *
     * @param string $message
     * @param int $type
     * @return \Phalcon\Logger\Adapter
     */
    public function log($type, $message, array $context = NULL){ }
}

So now Phalcon complains about unimplemented abstract methods. How do I implement them?

Thanks.

[ghost]

@temuri416 those methods have nothing to do with PSR-3. PSR-3 changes were adding array $context to all log methods.

close() and getFormatter() were always there, nothing has changed: sjinks@32beceb#diff-91283c398e56077e0a3b0f0975719670L61

logInternal() is (and was) invoked by LoggerAdapter::commit() and log().

Are you sure you are not missing anything?

[temuri416]

Hmmmm. Can you see whether close() and getFormatter() were there in 1.2.4 branch?

[ghost]

Yes.

• getFormatter(): https://github.com/sjinks/cphalcon/blob/1.2.4/ext/logger/adapterinterface.c#L53
• close(): https://github.com/sjinks/cphalcon/blob/1.2.4/ext/logger/adapterinterface.c#L105

[ghost]

If you inherit from \Phalcon\Logger\Adapter all you need is 2 functions:

class Stub extends \Phalcon\Logger\Adapter
{
    protected function logInternal($message, $type, $time, $context) {}
    public function getFormatter() { return new \Phalcon\Logger\Formatter\Line(); } // return null; will probably work either
}

[temuri416]

Something isn't right then. I had the following class working fine under Phalcon 1.2.4:

class Stub extends \Phalcon\Logger\Adapter
{
    /**
     * Sends/Writes an emergence message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function emergence($message){ }


    /**
     * Sends/Writes a debug message to the log
     *
     * @param string $message
     * @param ing $type
     * @return \Phalcon\Logger\Adapter
     */
    public function debug($message){ }


    /**
     * Sends/Writes an error message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function error($message){ }


    /**
     * Sends/Writes an info message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function info($message){ }


    /**
     * Sends/Writes a notice message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function notice($message){ }


    /**
     * Sends/Writes a warning message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function warning($message){ }


    /**
     * Sends/Writes an alert message to the log
     *
     * @param string $message
     * @return \Phalcon\Logger\Adapter
     */
    public function alert($message){ }


    /**
     * Logs messages to the internal loggger. Appends logs to the
     *
     * @param string $message
     * @param int $type
     * @return \Phalcon\Logger\Adapter
     */
    public function log($message, $type=null){ }
}

(as a side note, I added , array $context = NULL to Stub class today, having upgraded to 1.3.0.).

It was not supposed to log anything, I was using it as a substitute when I wanted to globally disable log devices.

How could it have worked then if those abstract methods that were there in 1.2.4 are missing from my stub class?

[ghost]

How could it have worked then if those abstract methods that were there in 1.2.4 are missing from my stub class?

That's because of this one: #1852

In 1.2.4 Adapters never implemented AdapterInterface.

[temuri416]

1. OK, so I assume [BUG] $form->add() stopped working, latest Phalcon 1.3.0 #1852 also went into 1.2.4 branch
2. Thanks for the snippet.
   a) What should Phalcon\Logger\AdapterInterface::close() be doing?
   b) For my Stub class it's enough, as it isn't logging anything. However, if I were to implement my custom log device, would the contents of those methods differ?

[ghost]

OK, so I assume #1852 also went into 1.2.4 branch

Don't think so — it is only in 1.3.0

What should Phalcon\Logger\AdapterInterface::close() be doing?

Perform cleanup if necessary. For example, for Phalcon\Logger\Adapter\File close() just closes the file. For Phalcon\Logger\Adapter\FirePhp it does nothing and just returns true.

However, if I were to implement my custom log device, would the contents of those methods differ?

Yes, you would want to implement logInternal() as this is the method that performs logging. See, for example, https://github.com/phalcon/incubator/blob/1.3.0/Library/Phalcon/Logger/Adapter/Database.php#L67

[temuri416]

ok, thanks for your time. I'll just update my Stub class, fix signatures, upgrade to 1.3.0 and stop thinking about it.

Cheers.

[ghost]

BTW, if you use emergence(), this will spit E_DEPRECATED; the correct method name is emergency().