Merge pull request #371 from noplanman/simplify_logging

Make logger more flexible

examples/Commands/ShortenerCommand.php

@@ -40,7 +40,7 @@ class ShortenerCommand extends UserCommand
         $data = [];
         $data['chat_id'] = $chat_id;
 
-        $text = Botan::shortenUrl("https://github.com/akalongman/php-telegram-bot", $user_id);
+        $text = Botan::shortenUrl('https://github.com/akalongman/php-telegram-bot', $user_id);
 
         $data['text'] = $text;
 

src/Botan.php

@@ -191,7 +191,7 @@ class Botan
         $responseData = json_decode($result, true);
 
         if (!$responseData || $responseData['status'] !== 'accepted') {
-            TelegramLog::debug('Botan.io stats report failed: ' . ($result ?: 'empty response'));
+            TelegramLog::debug('Botan.io stats report failed: %s', $result ?: 'empty response');
 
             return false;
         }
@@ -238,7 +238,7 @@ class Botan
         }
 
         if (filter_var($result, FILTER_VALIDATE_URL) === false) {
-            TelegramLog::debug('Botan.io URL shortening failed for "' . $url . '": ' . ($result ?: 'empty response'));
+            TelegramLog::debug('Botan.io URL shortening failed for "%s": %s', $url, $result ?: 'empty response');
 
             return $url;
         }

src/Entities/Message.php

@@ -174,13 +174,13 @@ class Message extends Entity
             return $command;
         }
 
-        $cmd = $this->getFullCommand();
+        $full_command = $this->getFullCommand();
 
-        if (strpos($cmd, '/') === 0) {
-            $cmd = substr($cmd, 1);
+        if (strpos($full_command, '/') === 0) {
+            $full_command = substr($full_command, 1);
 
             //check if command is follow by botname
-            $split_cmd = explode('@', $cmd);
+            $split_cmd = explode('@', $full_command);
             if (isset($split_cmd[1])) {
                 //command is followed by name check if is addressed to me
                 if (strtolower($split_cmd[1]) === strtolower($this->getBotName())) {
@@ -188,7 +188,7 @@ class Message extends Entity
                 }
             } else {
                 //command is not followed by name
-                return $cmd;
+                return $full_command;
             }
         }
 
@@ -208,10 +208,10 @@ class Message extends Entity
 
         if ($without_cmd && $command = $this->getFullCommand()) {
             if (strlen($command) + 1 < strlen($text)) {
-                $text = substr($text, strlen($command) + 1);
-            } else {
-                $text = '';
+                return substr($text, strlen($command) + 1);
             }
+
+            return '';
         }
 
         return $text;

src/Telegram.php

@@ -499,7 +499,7 @@ class Telegram
         if (is_int($admin_id) && $admin_id > 0 && !in_array($admin_id, $this->admins_list, true)) {
             $this->admins_list[] = $admin_id;
         } else {
-            TelegramLog::error('Invalid value "' . $admin_id . '" for admin.');
+            TelegramLog::error('Invalid value "%s" for admin.', $admin_id);
         }
 
         return $this;
@@ -590,7 +590,7 @@ class Telegram
     public function addCommandsPath($path, $before = true)
     {
         if (!is_dir($path)) {
-            TelegramLog::error('Commands path "' . $path . '" does not exist.');
+            TelegramLog::error('Commands path "%s" does not exist.', $path);
         } elseif (!in_array($path, $this->commands_paths, true)) {
             if ($before) {
                 array_unshift($this->commands_paths, $path);

src/TelegramLog.php

@@ -165,12 +165,7 @@ class TelegramLog
     {
         if (is_resource(self::$debug_log_temp_stream_handle)) {
             rewind(self::$debug_log_temp_stream_handle);
-            self::debug(
-                sprintf(
-                    $message,
-                    stream_get_contents(self::$debug_log_temp_stream_handle)
-                )
-            );
+            self::debug($message, stream_get_contents(self::$debug_log_temp_stream_handle));
             fclose(self::$debug_log_temp_stream_handle);
             self::$debug_log_temp_stream_handle = null;
         }
@@ -218,7 +213,7 @@ class TelegramLog
      */
     public static function isErrorLogActive()
     {
-        return (self::$error_log_path !== null);
+        return self::$error_log_path !== null;
     }
 
     /**
@@ -228,7 +223,7 @@ class TelegramLog
      */
     public static function isDebugLogActive()
     {
-        return (self::$debug_log_path !== null);
+        return self::$debug_log_path !== null;
     }
 
     /**
@@ -238,7 +233,7 @@ class TelegramLog
      */
     public static function isUpdateLogActive()
     {
-        return (self::$update_log_path !== null);
+        return self::$update_log_path !== null;
     }
 
     /**
@@ -249,6 +244,7 @@ class TelegramLog
     public static function error($text)
     {
         if (self::isErrorLogActive()) {
+            $text = self::getLogText($text, func_get_args());
             self::$monolog->error($text);
         }
     }
@@ -261,6 +257,7 @@ class TelegramLog
     public static function debug($text)
     {
         if (self::isDebugLogActive()) {
+            $text = self::getLogText($text, func_get_args());
             self::$monolog->debug($text);
         }
     }
@@ -273,7 +270,25 @@ class TelegramLog
     public static function update($text)
     {
         if (self::isUpdateLogActive()) {
+            $text = self::getLogText($text, func_get_args());
             self::$monolog_update->info($text);
         }
     }
+
+    /**
+     * Applies vsprintf to the text if placeholder replacements are passed along.
+     *
+     * @param string $text
+     * @param array  $args
+     *
+     * @return string
+     */
+    protected static function getLogText($text, array $args = [])
+    {
+        // Pop the $text off the array, as it gets passed via func_get_args().
+        array_shift($args);
+
+        // Suppress warning if placeholders don't match out.
+        return @vsprintf($text, $args) ?: $text;
+    }
 }

tests/unit/TelegramLogTest.php

@@ -26,7 +26,7 @@ class TelegramLogTest extends TestCase
     /**
      * @var array Dummy logfile paths
      */
-    private $logfiles = [
+    private static $logfiles = [
         'error'    => '/tmp/php-telegram-bot-errorlog.log',
         'debug'    => '/tmp/php-telegram-bot-debuglog.log',
         'update'   => '/tmp/php-telegram-bot-updatelog.log',
@@ -48,7 +48,7 @@ class TelegramLogTest extends TestCase
     protected function tearDown()
     {
         // Make sure no logfiles exist.
-        foreach ($this->logfiles as $file) {
+        foreach (self::$logfiles as $file) {
             file_exists($file) && unlink($file);
         }
     }
@@ -79,37 +79,46 @@ class TelegramLogTest extends TestCase
 
     public function testErrorStream()
     {
-        $file = $this->logfiles['error'];
+        $file = self::$logfiles['error'];
         $this->assertFileNotExists($file);
         TelegramLog::initErrorLog($file);
         TelegramLog::error('my error');
+        TelegramLog::error('my %s error', 'placeholder');
         $this->assertFileExists($file);
-        $this->assertContains('bot_log.ERROR: my error', file_get_contents($file));
+        $error_log = file_get_contents($file);
+        $this->assertContains('bot_log.ERROR: my error', $error_log);
+        $this->assertContains('bot_log.ERROR: my placeholder error', $error_log);
     }
 
     public function testDebugStream()
     {
-        $file = $this->logfiles['debug'];
+        $file = self::$logfiles['debug'];
         $this->assertFileNotExists($file);
         TelegramLog::initDebugLog($file);
         TelegramLog::debug('my debug');
+        TelegramLog::debug('my %s debug', 'placeholder');
         $this->assertFileExists($file);
-        $this->assertContains('bot_log.DEBUG: my debug', file_get_contents($file));
+        $debug_log = file_get_contents($file);
+        $this->assertContains('bot_log.DEBUG: my debug', $debug_log);
+        $this->assertContains('bot_log.DEBUG: my placeholder debug', $debug_log);
     }
 
     public function testUpdateStream()
     {
-        $file = $this->logfiles['update'];
+        $file = self::$logfiles['update'];
         $this->assertFileNotExists($file);
         TelegramLog::initUpdateLog($file);
         TelegramLog::update('my update');
+        TelegramLog::update('my %s update', 'placeholder');
         $this->assertFileExists($file);
-        $this->assertContains('my update', file_get_contents($file));
+        $debug_log = file_get_contents($file);
+        $this->assertContains('my update', $debug_log);
+        $this->assertContains('my placeholder update', $debug_log);
     }
 
     public function testExternalStream()
     {
-        $file = $this->logfiles['external'];
+        $file = self::$logfiles['external'];
         $this->assertFileNotExists($file);
 
         $external_monolog = new Logger('bot_update_log');
@@ -118,11 +127,15 @@ class TelegramLogTest extends TestCase
 
         TelegramLog::initialize($external_monolog);
         TelegramLog::error('my error');
+        TelegramLog::error('my %s error', 'placeholder');
         TelegramLog::debug('my debug');
+        TelegramLog::debug('my %s debug', 'placeholder');
 
         $this->assertFileExists($file);
         $file_contents = file_get_contents($file);
         $this->assertContains('bot_update_log.ERROR: my error', $file_contents);
+        $this->assertContains('bot_update_log.ERROR: my placeholder error', $file_contents);
         $this->assertContains('bot_update_log.DEBUG: my debug', $file_contents);
+        $this->assertContains('bot_update_log.DEBUG: my placeholder debug', $file_contents);
     }
 }