add monolog logger

cyklokoalicia · Mar 2, 2024 · 1431e59 · 1431e59
1 parent 7de6a53
commit 1431e59
Show file tree

Hide file tree

Showing 10 changed files with 106 additions and 38 deletions.
diff --git a/admin.php b/admin.php
@@ -2,26 +2,28 @@
 
 use BikeShare\Authentication\Auth;
 use BikeShare\Db\DbInterface;
-use BikeShare\Db\MysqliDb;
 use BikeShare\User\User;
+use Psr\Log\LoggerInterface;
 
 require_once 'vendor/autoload.php';
 require("config.php");
 require('actions-web.php');
 
 /**
  * @var DbInterface $db
+ * @var LoggerInterface $logger
  */
-$db=new MysqliDb($dbserver,$dbuser,$dbpassword,$dbname);
-$db->connect();
 $user = new User($db);
 $auth = new Auth($db);
 
 $auth->refreshSession();
 
 $userid = $auth->getUserId();
 
-if ($user->findPrivileges($userid)<=0) exit(_('You need admin privileges to access this page.'));
+if ($user->findPrivileges($userid)<=0) {
+    $logger->error('User has no privileges to access this page', ['userid' => $userid]);
+    exit(_('You need admin privileges to access this page.'));
+}
 ?>
 <!DOCTYPE html>
 <html lang="en">

diff --git a/common.php b/common.php
@@ -14,16 +14,23 @@
 use BikeShare\SmsConnector\DebugConnector;
 use BikeShare\SmsConnector\SmsConnectorFactory;
 use BikeShare\User\User;
+use Monolog\ErrorHandler;
+use Monolog\Handler\RotatingFileHandler;
+use Monolog\Logger;
 
 require_once 'vendor/autoload.php';
 
+$logger = new Logger('BikeShare');
+$logger->pushHandler(new RotatingFileHandler( __DIR__ . '/var/log/log.log', 30, Logger::WARNING));
+ErrorHandler::register($logger);
+
 $locale = $systemlang . ".utf8";
 setlocale(LC_ALL, $locale);
 putenv("LANG=" . $locale);
 bindtextdomain("messages", dirname(__FILE__) . '/languages');
 textdomain("messages");
 
-$sms = (new SmsConnectorFactory())->getConnector(
+$sms = (new SmsConnectorFactory($logger))->getConnector(
     !empty($connectors["sms"]) ? $connectors["sms"] : 'disabled',
     !empty($connectors["config"][$connectors["sms"]]) ? json_decode($connectors["config"][$connectors["sms"]], true) : array(),
     DEBUG
@@ -32,7 +39,7 @@
 /**
  * @var DbInterface $db
  */
-$db = new MysqliDb($dbserver, $dbuser, $dbpassword, $dbname);
+$db = new MysqliDb($dbserver, $dbuser, $dbpassword, $dbname, $logger);
 $db->connect();
 
 /**

diff --git a/composer.json b/composer.json
@@ -26,10 +26,11 @@
     "ext-json": "*",
     "ext-mysqli": "*",
     "ext-gettext": "*",
-    "ext-curl": "*"
+    "ext-curl": "*",
+    "monolog/monolog": "^1.27"
   },
   "require-dev": {
     "squizlabs/php_codesniffer": "^3.0",
     "phpunit/phpunit": "^5.7"
   }
-}
+}
diff --git a/receive.php b/receive.php
@@ -1,30 +1,25 @@
 <?php
 
 use BikeShare\Db\DbInterface;
-use BikeShare\Db\MysqliDb;
 use BikeShare\SmsConnector\SmsConnectorInterface;
+use Psr\Log\LoggerInterface;
 
 require_once 'vendor/autoload.php';
 require("config.php");
-
-/**
- * @var DbInterface $db
- */
-$db=new MysqliDb($dbserver,$dbuser,$dbpassword,$dbname);
-$db->connect();
-
 require("actions-sms.php");
 
 /**
  * @var SmsConnectorInterface $sms
+ * @var LoggerInterface $logger
+ * @var DbInterface $db
  */
 log_sms($sms->getUUID(),$sms->getNumber(),$sms->getTime(),$sms->getMessage(),$sms->getIPAddress());
 
 $args=preg_split("/\s+/",$sms->getProcessedMessage());//preg_split must be used instead of explode because of multiple spaces
 
 if(!validateNumber($sms->getNumber()))
    {
-       trigger_error("Invalid number: ".$sms->getNumber(), E_USER_WARNING);
+       $logger->error("Invalid number", ["number" => $sms->getNumber(), 'sms' => $sms]);
    ####
    #$smsSender->send($sms->getNumber(),_('Your number is not registered.'));
    }
@@ -121,5 +116,3 @@
 
 $db->commit();
 $sms->respond();
-
-?>
diff --git a/src/Db/MysqliDb.php b/src/Db/MysqliDb.php
@@ -2,6 +2,8 @@
 
 namespace BikeShare\Db;
 
+use Psr\Log\LoggerInterface;
+
 class MysqliDb implements DbInterface
 {
     /**
@@ -24,31 +26,50 @@ class MysqliDb implements DbInterface
      * @var string
      */
     private $dbname;
+    /**
+     * @var LoggerInterface|null
+     */
+    private $logger;
     /**
      * @var false
      */
     private $throwException;
 
-    public function __construct($dbserver, $dbuser, $dbpassword, $dbname, $throwException = false)
-    {
+    public function __construct(
+        $dbserver,
+        $dbuser,
+        $dbpassword,
+        $dbname,
+        LoggerInterface $logger,
+        $throwException = false
+    ) {
         $this->dbserver = $dbserver;
         $this->dbuser = $dbuser;
         $this->dbpassword = $dbpassword;
         $this->dbname = $dbname;
+        $this->logger = $logger;
         $this->throwException = $throwException;
     }
 
     public function connect()
     {
+        //in future exception should be thrown
+        //mysqli_report(MYSQLI_REPORT_ERROR|MYSQLI_REPORT_STRICT);
         $this->conn = new \mysqli($this->dbserver, $this->dbuser, $this->dbpassword, $this->dbname);
         if (!$this->conn || $this->conn->connect_errno) {
+            $this->logger->error(
+                'DB connection error!',
+                [
+                    'error' => $this->conn->connect_error,
+                    'errno' => $this->conn->connect_errno,
+                ]
+            );
             if ($this->throwException) {
                 throw new \RuntimeException(
                     'DB connection error!',
                     !empty($this->conn->connect_errno) ? $this->conn->connect_errno : 0
                 );
             } else {
-                trigger_error('DB connection error!', E_USER_ERROR);
                 die(_('DB connection error!'));
             }
         }
@@ -60,11 +81,20 @@ public function query($query, $params = array())
     {
         $result = $this->conn->query($query);
         if (!$result) {
+            $this->logger->error(
+                'DB query error',
+                [
+                    'query' => $query,
+                    'params' => $params,
+                    'error' => $this->conn->get_connection_stats() ? $this->conn->error : 'unknown',
+                    'errno' => $this->conn->get_connection_stats() ? $this->conn->errno : 'unknown',
+                ]
+            );
             $this->conn->rollback();
+
             if ($this->throwException) {
                 throw new \RuntimeException('DB error in : ' . $query);
             } else {
-                trigger_error('DB error' . ' ' . $this->conn->error . ' ' . _('in') . ': ' . $query, E_USER_ERROR);
                 die(_('DB error') . ' ' . $this->conn->error . ' ' . _('in') . ': ' . $query);
             }
         }

diff --git a/src/SmsConnector/SmsConnectorFactory.php b/src/SmsConnector/SmsConnectorFactory.php
@@ -2,8 +2,21 @@
 
 namespace BikeShare\SmsConnector;
 
+use Psr\Log\LoggerInterface;
+
 class SmsConnectorFactory
 {
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
+    public function __construct(
+        LoggerInterface $logger
+    ) {
+        $this->logger = $logger;
+    }
+
     /**
      * @param string $connector
      * @param array $config
@@ -24,9 +37,9 @@ public function getConnector($connector, array $config, $debugMode = false)
                 default:
                     return new DisabledConnector();
             }
-        } catch (\Exception $e) {
-            // TODO add logging instead of triggering error
-            trigger_error($e->getMessage(), E_USER_WARNING);
+        } catch (\Exception $exception) {
+            $this->logger->error('Error creating SMS connector', compact('connector', 'exception'));
+
             return new DisabledConnector();
         }
     }

diff --git a/src/SmsConnector/SmsGatewayConnector.php b/src/SmsConnector/SmsGatewayConnector.php
@@ -61,7 +61,7 @@ public function checkConfig(array $config)
             return;
         }
         if (empty($config['gatewayEmail']) || empty($config['gatewayPassword']) || empty($config['gatewaySecret'])) {
-            exit('Please, configure SMS API gateway access in config.php!');
+            throw new \RuntimeException('Invalid SmsGateway configuration');
         }
         $this->gatewayEmail = $config['gatewayEmail'];
         $this->gatewayPassword = $config['gatewayPassword'];

diff --git a/src/SmsConnector/TextmagicSmsConnector.php b/src/SmsConnector/TextmagicSmsConnector.php
@@ -55,7 +55,7 @@ public function checkConfig(array $config)
             || empty($config['gatewayPassword'])
             || empty($config['gatewaySenderNumber'])
         ) {
-            exit('Please, configure SMS API gateway access in ' . __FILE__ . '!');
+            throw new \RuntimeException('Invalid Textmagic configuration');
         }
         $this->gatewayUser = $config['gatewayUser'];
         $this->gatewayPassword = $config['gatewayPassword'];

diff --git a/tests/Db/MysqliDbTest.php b/tests/Db/MysqliDbTest.php
@@ -4,12 +4,19 @@
 
 use BikeShare\Db\MysqliDb;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
 
 class MysqliDbTest extends TestCase
 {
+    /**
+     * @var LoggerInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $logger;
+
     public function setUp()
     {
-        $db = new MysqliDb('server', 'user', 'password', 'dbname', true);
+        $this->logger = $this->createMock(LoggerInterface::class);
+        $db = new MysqliDb('server', 'user', 'password', 'dbname', $this->logger, true);
         $mysqliMock = $this->createMock(\mysqli::class);
 
         $reflection = new \ReflectionClass($db);
@@ -43,6 +50,10 @@ public function testQueryError()
         $this->conn->expects($this->once())
             ->method('rollback');
 
+        $this->logger->expects($this->once())
+            ->method('error')
+            ->with('DB query error', $this->callback(function () { return true; }));
+
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionMessage('DB error in : ' . $query);
 

diff --git a/tests/SmsConnector/SmsConnectorFactory.php → .../SmsConnector/SmsConnectorFactoryTest.php b/tests/SmsConnector/SmsConnectorFactory.php → .../SmsConnector/SmsConnectorFactoryTest.php
@@ -5,12 +5,13 @@
 use BikeShare\SmsConnector\DisabledConnector;
 use BikeShare\SmsConnector\EuroSmsConnector;
 use BikeShare\SmsConnector\LoopbackConnector;
-use BikeShare\SmsConnector\SmsConnectorFactory as SmsConnectorFactoryAlias;
+use BikeShare\SmsConnector\SmsConnectorFactory;
 use BikeShare\SmsConnector\SmsGatewayConnector;
 use BikeShare\SmsConnector\TextmagicSmsConnector;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
 
-class SmsConnectorFactory extends TestCase
+class SmsConnectorFactoryTest extends TestCase
 {
     /**
      * @param string $connector
@@ -27,13 +28,24 @@ public function testGetConnector(
         $expectedInstance,
         $expectedExceptionMessage = null
     ) {
-        $smsConnectorFactory = new SmsConnectorFactoryAlias();
-        try {
-            $result = $smsConnectorFactory->getConnector($connector, $config, $debugMode);
-            $this->assertInstanceOf($expectedInstance, $result);
-        } catch (\PHPUnit_Framework_Error_Warning $e) {
-            $this->assertEquals($expectedExceptionMessage, $e->getMessage());
+        $logger = $this->createMock(LoggerInterface::class);
+        $smsConnectorFactory = new SmsConnectorFactory($logger);
+
+        if ($expectedExceptionMessage) {
+            $logger
+                ->expects($this->once())
+                ->method('error')
+                ->with(
+                    'Error creating SMS connector',
+                    $this->callback(function ($context) use ($connector, $expectedExceptionMessage) {
+                        return $context['connector'] === $connector
+                            && $context['exception'] instanceof \Exception
+                            && $context['exception']->getMessage() === $expectedExceptionMessage;
+                    })
+                );
         }
+        $result = $smsConnectorFactory->getConnector($connector, $config, $debugMode);
+        $this->assertInstanceOf($expectedInstance, $result);
     }
 
     public function getConnectorDataProvider()
@@ -70,7 +82,6 @@ public function getConnectorDataProvider()
             'expectedInstance' => DisabledConnector::class,
         ];
 
-        //PHPUNIT configured to convert warnings to exceptions so we test for exception message
         yield 'throwException' => [
             'connector' => 'eurosms',
             'config' => [],