From 06f54e84d1bc98a13b513526eb8e4151fd3edb67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 14 Dec 2021 17:56:23 +0100 Subject: [PATCH 1/4] Extract PDO connection logic to DatabaseConnectionManager. This should ease the implementation of PDO in other DAOs. Besides, it will allow to reuse the connection for all DAO objects instead of creating a new one every time. --- .../dao/TemplateDAO/PostgreSQLTemplateDAO.php | 31 +------- util/DatabaseConnectionManager.php | 74 +++++++++++++++++++ 2 files changed, 78 insertions(+), 27 deletions(-) create mode 100644 util/DatabaseConnectionManager.php diff --git a/model/dao/TemplateDAO/PostgreSQLTemplateDAO.php b/model/dao/TemplateDAO/PostgreSQLTemplateDAO.php index 73c7bd7c0..21d570217 100644 --- a/model/dao/TemplateDAO/PostgreSQLTemplateDAO.php +++ b/model/dao/TemplateDAO/PostgreSQLTemplateDAO.php @@ -27,10 +27,9 @@ * @package PhpReport * @subpackage DAO */ -include_once(PHPREPORT_ROOT . '/util/DBPostgres.php'); include_once(PHPREPORT_ROOT . '/model/vo/TemplateVO.php'); include_once(PHPREPORT_ROOT . '/model/dao/TemplateDAO/TemplateDAO.php'); -include_once(PHPREPORT_ROOT . '/util/ConfigurationParametersManager.php'); +include_once(PHPREPORT_ROOT . '/util/DatabaseConnectionManager.php'); /** DAO for Templates in PostgreSQL * @@ -53,36 +52,14 @@ class PostgreSQLTemplateDAO extends TemplateDAO{ /** Template DAO for PostgreSQL constructor. * * This is the constructor of the implementation for PostgreSQL of - * {@link TemplateDAO}. It sets up everything for database connection, using - * the parameters read from {@link config.php} and saving the open - * connection in {@link $pdo}. + * {@link TemplateDAO}. It sets up the database connection via + * DatabaseConnectionManager. * Notice this DAO connects to the DB through PDO, unlike the rest of the * application. * - * @throws {@link DBConnectionErrorException} */ function __construct() { - // Call parent to initialize non-PDO database access, while we don't - // migrate all the methods here. - parent::__construct(); - - // TODO: EXTRA_DB_CONNECTION_PARAMETERS used to expect pg_connect - // parameters, which were space-separated, but PDO requires semicolons - $connectionString = sprintf("pgsql:host=%s;port=%d;user=%s;dbname=%s;password=%s;%s", - ConfigurationParametersManager::getParameter('DB_HOST'), - ConfigurationParametersManager::getParameter('DB_PORT'), - ConfigurationParametersManager::getParameter('DB_USER'), - ConfigurationParametersManager::getParameter('DB_NAME'), - ConfigurationParametersManager::getParameter('DB_PASSWORD'), - ConfigurationParametersManager::getParameter('EXTRA_DB_CONNECTION_PARAMETERS')); - - try { - $this->pdo = new PDO($connectionString); - $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); - } catch (PDOException $e) { - error_log('Connection failed: ' . $e->getMessage()); - throw new DBConnectionErrorException($connectionString); - } + $this->pdo = DatabaseConnectionManager::getPDO(); } /** diff --git a/util/DatabaseConnectionManager.php b/util/DatabaseConnectionManager.php new file mode 100644 index 000000000..a21f6e437 --- /dev/null +++ b/util/DatabaseConnectionManager.php @@ -0,0 +1,74 @@ + + * + * This file is part of PhpReport. + * + * PhpReport is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * PhpReport is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with PhpReport. If not, see . + */ + +include_once(PHPREPORT_ROOT . '/util/ConfigurationParametersManager.php'); + +class DatabaseConnectionManager { + + /** The connection to DB. + * + * PDO object with an open connection to the database. + * + * @var resource + * @see __construct() + */ + private static PDO $pdo; + + /** Setup database connection via PDO. + * + * It sets up everything for database connection via the PDO API, using th + * parameters read from config.php, and saves the open connection in the + * static property $pdo. + * + * @throws {@link DBConnectionErrorException} + */ + private static function setupPDOConnection() { + // TODO: EXTRA_DB_CONNECTION_PARAMETERS used to expect pg_connect + // parameters, which were space-separated, but PDO requires semicolons + $connectionString = sprintf("pgsql:host=%s;port=%d;user=%s;dbname=%s;password=%s;%s", + ConfigurationParametersManager::getParameter('DB_HOST'), + ConfigurationParametersManager::getParameter('DB_PORT'), + ConfigurationParametersManager::getParameter('DB_USER'), + ConfigurationParametersManager::getParameter('DB_NAME'), + ConfigurationParametersManager::getParameter('DB_PASSWORD'), + ConfigurationParametersManager::getParameter('EXTRA_DB_CONNECTION_PARAMETERS')); + + try { + self::$pdo = new PDO($connectionString); + self::$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + } catch (PDOException $e) { + error_log('Connection failed: ' . $e->getMessage()); + throw new DBConnectionErrorException($connectionString); + } + } + + /** Get or create a PDO object for database connection. + * + * Retrieve a PDO object for database connection. The first time this method + * is called, it will create the object, and it will be reused in subsequent + * calls. + */ + public static function getPDO(): PDO { + if (!isset(self::$pdo)) { + self::setupPDOConnection(); + } + return self::$pdo; + } +} From 1b83e5c8566647cdf31fd61470fb99377b5b5051 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 14 Dec 2021 18:29:02 +0100 Subject: [PATCH 2/4] Move PDO object up in the DAO hierarchy to BaseDAO. This makes it accessible to other DAOs extending BaseDAO. --- model/dao/BaseDAO.php | 20 ++++++++++++++++- .../dao/TemplateDAO/PostgreSQLTemplateDAO.php | 22 ++++--------------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/model/dao/BaseDAO.php b/model/dao/BaseDAO.php index 3218956d6..b2d8e3cf3 100644 --- a/model/dao/BaseDAO.php +++ b/model/dao/BaseDAO.php @@ -31,6 +31,7 @@ include_once(PHPREPORT_ROOT . '/util/ConfigurationParametersManager.php'); include_once(PHPREPORT_ROOT . '/model/dao/DAOFactory.php'); +include_once(PHPREPORT_ROOT . '/util/DatabaseConnectionManager.php'); include_once(PHPREPORT_ROOT . '/util/DBConnectionErrorException.php'); include_once(PHPREPORT_ROOT . '/util/SQLQueryErrorException.php'); @@ -53,9 +54,23 @@ abstract class BaseDAO { */ protected $connect; + /** The connection to DB. + * + * PDO object with an open connection to the database, initialized in the + * class constructor. + * + * @var resource + * @see __construct() + */ + protected PDO $pdo; + /** Base constructor. * - * This is the base constructor of all simple DAOs, and it just creates the connection with the parameters read from {@link config.php}, storing it in {@link $connect}. + * This is the base constructor of all simple DAOs, and it just creates the + * connection with the parameters read from {@link config.php}, + * storing it in {@link $connect}. + * It also sets up a connection using the PDO API, via + * DatabaseConnectionManager. Eventually, all DAOs should use it. * * @see ConfigurationParametersManager, config.php * @throws {@link ConnectionErrorException} @@ -75,6 +90,9 @@ protected function __construct() { if ($this->connect == NULL) throw new DBConnectionErrorException($connectionString); pg_set_error_verbosity($this->connect, PGSQL_ERRORS_VERBOSE); + + // PDO setup for the DAOs that need it. + $this->pdo = DatabaseConnectionManager::getPDO(); } /** Value object constructor. diff --git a/model/dao/TemplateDAO/PostgreSQLTemplateDAO.php b/model/dao/TemplateDAO/PostgreSQLTemplateDAO.php index 21d570217..bd96d3e13 100644 --- a/model/dao/TemplateDAO/PostgreSQLTemplateDAO.php +++ b/model/dao/TemplateDAO/PostgreSQLTemplateDAO.php @@ -29,7 +29,6 @@ */ include_once(PHPREPORT_ROOT . '/model/vo/TemplateVO.php'); include_once(PHPREPORT_ROOT . '/model/dao/TemplateDAO/TemplateDAO.php'); -include_once(PHPREPORT_ROOT . '/util/DatabaseConnectionManager.php'); /** DAO for Templates in PostgreSQL * @@ -39,27 +38,14 @@ */ class PostgreSQLTemplateDAO extends TemplateDAO{ - /** The connection to DB. + /** Template DAO constructor. * - * PDO object with an open connection to the database, initialized in the - * class constructor. - * - * @var resource - * @see __construct() - */ - protected PDO $pdo; - - /** Template DAO for PostgreSQL constructor. - * - * This is the constructor of the implementation for PostgreSQL of - * {@link TemplateDAO}. It sets up the database connection via - * DatabaseConnectionManager. - * Notice this DAO connects to the DB through PDO, unlike the rest of the - * application. + * Default constructor of TemplateDAO, it just calls parent constructor. * + * @see BaseDAO::__construct() */ function __construct() { - $this->pdo = DatabaseConnectionManager::getPDO(); + parent::__construct(); } /** From c868e06b519e1a9cf96b277ed5cdb8eb96c1144f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 14 Dec 2021 18:35:45 +0100 Subject: [PATCH 3/4] Remove unused operation ConfigDAO::getVersionNumber(). --- model/dao/ConfigDAO/ConfigDAO.php | 8 -------- model/dao/ConfigDAO/PostgreSQLConfigDAO.php | 16 ---------------- 2 files changed, 24 deletions(-) diff --git a/model/dao/ConfigDAO/ConfigDAO.php b/model/dao/ConfigDAO/ConfigDAO.php index 5ca60f51c..a026969ed 100644 --- a/model/dao/ConfigDAO/ConfigDAO.php +++ b/model/dao/ConfigDAO/ConfigDAO.php @@ -49,14 +49,6 @@ protected function __construct() { parent::__construct(); } - /** Get version number. - * - * Get database version number. - * - * @return String a string containing the version number - */ - public abstract function getVersionNumber(); - /** Query PhpReport task block configuration. * * Check if PhpReport configuration allows writing tasks on the specified diff --git a/model/dao/ConfigDAO/PostgreSQLConfigDAO.php b/model/dao/ConfigDAO/PostgreSQLConfigDAO.php index 8dc150b0b..d0908e57d 100644 --- a/model/dao/ConfigDAO/PostgreSQLConfigDAO.php +++ b/model/dao/ConfigDAO/PostgreSQLConfigDAO.php @@ -49,22 +49,6 @@ function __construct() { parent::__construct(); } - /** Get version number. - * - * Get database version number. - * - * @return String a string containing the version number - */ - public function getVersionNumber() { - $sql = "SELECT version FROM config"; - - $result = $this->execute($sql); - - if (!is_null($result[0])) { - return $result[0]; - } - } - /** Query PhpReport task block configuration. * * Check if PhpReport configuration allows writing tasks on the specified From 81a521dd6d12721fb1849f34dbb8ba9201fd507f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 14 Dec 2021 18:54:58 +0100 Subject: [PATCH 4/4] Port PostgreSQLConfigDAO to use PDO. --- model/dao/ConfigDAO/PostgreSQLConfigDAO.php | 52 ++++++++++++--------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/model/dao/ConfigDAO/PostgreSQLConfigDAO.php b/model/dao/ConfigDAO/PostgreSQLConfigDAO.php index d0908e57d..58309aa3c 100644 --- a/model/dao/ConfigDAO/PostgreSQLConfigDAO.php +++ b/model/dao/ConfigDAO/PostgreSQLConfigDAO.php @@ -101,20 +101,19 @@ public function getTaskBlockConfiguration() { "block_tasks_by_date_enabled,". "block_tasks_by_date_date ". "FROM config"; - $res = pg_query($this->connect, $sql); - if ($res == NULL) throw new SQLQueryErrorException(pg_last_error()); - $config = array(); - if(pg_num_rows($res) > 0) { - $config = pg_fetch_array($res); + try { + $statement = $this->pdo->prepare($sql); + $statement->execute(); + $config = $statement->fetch(PDO::FETCH_ASSOC); + } catch (PDOException $e) { + error_log('Query failed: ' . $e->getMessage()); + throw new SQLQueryErrorException($e->getMessage()); } - - pg_freeresult($res); - return array( - "dayLimitEnabled" => (strtolower($config["block_tasks_by_day_limit_enabled"]) == "t"), - "dateLimitEnabled" => (strtolower($config["block_tasks_by_date_enabled"]) == "t"), + "dayLimitEnabled" => $config["block_tasks_by_day_limit_enabled"], + "dateLimitEnabled" => $config["block_tasks_by_date_enabled"], "numberOfDays" => $config["block_tasks_by_day_limit_number_of_days"], "date" => is_null($config["block_tasks_by_date_date"])? NULL : date_create($config["block_tasks_by_date_date"])); } @@ -136,19 +135,28 @@ public function getTaskBlockConfiguration() { */ public function setTaskBlockConfiguration($dayLimitEnabled, $numberOfDays, $dateLimitEnabled, $date) { + $affectedRows = 0; + $sql = "UPDATE config SET " . - "block_tasks_by_day_limit_enabled = " . - DBPostgres::boolToString($dayLimitEnabled) . "," . - "block_tasks_by_day_limit_number_of_days =" . - DBPostgres::checkNull($numberOfDays) . "," . - "block_tasks_by_date_enabled = " . - DBPostgres::boolToString($dateLimitEnabled) . "," . - "block_tasks_by_date_date = " . - DBPostgres::formatDate($date); - - $res = pg_query($this->connect, $sql); - - return ($res != NULL); + "block_tasks_by_day_limit_enabled = :dayLimitEnabled, " . + "block_tasks_by_day_limit_number_of_days = :numberOfDays, " . + "block_tasks_by_date_enabled = :dateLimitEnabled, " . + "block_tasks_by_date_date = :date"; + + try { + $statement = $this->pdo->prepare($sql); + $statement->bindValue(":dayLimitEnabled", $dayLimitEnabled, PDO::PARAM_BOOL); + $statement->bindValue(":numberOfDays", $numberOfDays, PDO::PARAM_INT); + $statement->bindValue(":dateLimitEnabled", $dateLimitEnabled, PDO::PARAM_BOOL); + $statement->bindValue(":date", DBPostgres::formatDate($date), PDO::PARAM_STR); + $statement->execute(); + + $affectedRows = $statement->rowCount(); + } catch (PDOException $e) { + error_log('Query failed: ' . $e->getMessage()); + throw new SQLQueryErrorException($e->getMessage()); + } + return $affectedRows != 0; } }