From 9c5903946fe849c070e0945a89d8eb9b8576cf2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 10 Jan 2023 13:41:04 +0100 Subject: [PATCH 1/9] Migrate CityDAO create/update/delete operations to PDO. We also remove unnecessary checks for row id in update and delete operations. --- model/dao/CityDAO/PostgreSQLCityDAO.php | 79 +++++++++++-------------- 1 file changed, 36 insertions(+), 43 deletions(-) diff --git a/model/dao/CityDAO/PostgreSQLCityDAO.php b/model/dao/CityDAO/PostgreSQLCityDAO.php index 526a9cd78..14ca384d1 100644 --- a/model/dao/CityDAO/PostgreSQLCityDAO.php +++ b/model/dao/CityDAO/PostgreSQLCityDAO.php @@ -147,25 +147,21 @@ public function getAll() { * @throws {@link SQLQueryErrorException}, {@link SQLUniqueViolationException} */ public function update(CityVO $cityVO) { - $affectedRows = 0; - - if($cityVO->getId() >= 0) { - $currcityVO = $this->getById($cityVO->getId()); - } - - // If the query returned a row then update - if(sizeof($currcityVO) > 0) { - - $sql = "UPDATE city SET name=" . DBPostgres::checkStringNull($cityVO->getName()) . " WHERE id=".$cityVO->getId(); - - $res = pg_query($this->connect, $sql); - - if ($res == NULL) - if (strpos(pg_last_error(), "unique_city_name")) - throw new SQLUniqueViolationException(pg_last_error()); - else throw new SQLQueryErrorException(pg_last_error()); + $affectedRows = 0; - $affectedRows = pg_affected_rows($res); + $sql = "UPDATE city SET name=:name WHERE id=:id"; + try { + $statement = $this->pdo->prepare($sql); + $statement->bindValue(":name", $cityVO->getName(), PDO::PARAM_STR); + $statement->bindValue(":id", $cityVO->getId(), PDO::PARAM_INT); + $statement->execute(); + + $affectedRows = $statement->rowCount(); + } catch (PDOException $e) { + error_log('Query failed: ' . $e->getMessage()); + if (strpos($e->getMessage(), "unique_city_name")) + throw new SQLUniqueViolationException($e->getMessage()); + else throw new SQLQueryErrorException($e->getMessage()); } return $affectedRows; @@ -180,24 +176,25 @@ public function update(CityVO $cityVO) { * @throws {@link SQLQueryErrorException}, {@link SQLUniqueViolationException} */ public function create(CityVO $cityVO) { - $affectedRows = 0; - $sql = "INSERT INTO city (name) VALUES (" . DBPostgres::checkStringNull($cityVO->getName()) . ")"; + $sql = "INSERT INTO city (name) VALUES (:name)"; + try { + $statement = $this->pdo->prepare($sql); + $statement->bindValue(":name", $cityVO->getName(), PDO::PARAM_STR); + $statement->execute(); - $res = pg_query($this->connect, $sql); + $cityVO->setId($this->pdo->lastInsertId('city_id_seq')); - if ($res == NULL) - if (strpos(pg_last_error(), "unique_city_name")) - throw new SQLUniqueViolationException(pg_last_error()); - else throw new SQLQueryErrorException(pg_last_error()); - - $cityVO->setId(DBPostgres::getId($this->connect, "city_id_seq")); - - $affectedRows = pg_affected_rows($res); + $affectedRows = $statement->rowCount(); + } catch (PDOException $e) { + error_log('Query failed: ' . $e->getMessage()); + if (strpos($e->getMessage(), "unique_city_name")) + throw new SQLUniqueViolationException($e->getMessage()); + else throw new SQLQueryErrorException($e->getMessage()); + } return $affectedRows; - } /** City deleter. @@ -211,20 +208,16 @@ public function create(CityVO $cityVO) { public function delete(CityVO $cityVO) { $affectedRows = 0; - // Check for a city ID. - if($cityVO->getId() >= 0) { - $currcityVO = $this->getById($cityVO->getId()); - } - - // Delete a city. - if(sizeof($currcityVO) > 0) { - $sql = "DELETE FROM city WHERE id=".$cityVO->getId(); - - $res = pg_query($this->connect, $sql); - - if ($res == NULL) throw new SQLQueryErrorException(pg_last_error()); + $sql = "DELETE FROM city WHERE id=:id"; + try { + $statement = $this->pdo->prepare($sql); + $statement->bindValue(":id", $cityVO->getId(), PDO::PARAM_INT); + $statement->execute(); - $affectedRows = pg_affected_rows($res); + $affectedRows = $statement->rowCount(); + } catch (PDOException $e) { + error_log('Query failed: ' . $e->getMessage()); + throw new SQLQueryErrorException($e->getMessage()); } return $affectedRows; From 19dc8ace302a321e032f31f24a9d16ad2ea84b18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 10 Jan 2023 15:49:30 +0100 Subject: [PATCH 2/9] Delete unused CityDAO::getById. This was used only inside the DAO, and we remove any uses of it. We would re-add the function if we ever need it. --- model/dao/CityDAO/CityDAO.php | 10 ---------- model/dao/CityDAO/PostgreSQLCityDAO.php | 16 ---------------- 2 files changed, 26 deletions(-) diff --git a/model/dao/CityDAO/CityDAO.php b/model/dao/CityDAO/CityDAO.php index 5a532509f..947147b8d 100644 --- a/model/dao/CityDAO/CityDAO.php +++ b/model/dao/CityDAO/CityDAO.php @@ -51,16 +51,6 @@ protected function __construct() { parent::__construct(); } - /** City retriever by id. - * - * This function retrieves the row from City table with the id $cityId and creates a {@link CityVO} with its data. - * - * @param int $cityId the id of the row we want to retrieve. - * @return CityVO a value object {@link CityVO} with its properties set to the values from the row. - * @throws {@link OperationErrorException} - */ - public abstract function getById($cityId); - /** City Histories retriever by City id. * * This function retrieves the rows from City History table that are assigned to the City with diff --git a/model/dao/CityDAO/PostgreSQLCityDAO.php b/model/dao/CityDAO/PostgreSQLCityDAO.php index 14ca384d1..304511443 100644 --- a/model/dao/CityDAO/PostgreSQLCityDAO.php +++ b/model/dao/CityDAO/PostgreSQLCityDAO.php @@ -73,22 +73,6 @@ protected function setValues($row) return $cityVO; } - /** Cities retriever by id. - * - * This function retrieves the row from City table with the id $cityId and creates a {@link CityVO} with its data. - * - * @param int $cityId the id of the row we want to retrieve. - * @return CityVO a value object {@link CityVO} with its properties set to the values from the row. - * @throws {@link SQLQueryErrorException} - */ - public function getById($cityId) { - if (!is_numeric($cityId)) - throw new SQLIncorrectTypeException($cityId); - $sql = "SELECT * FROM city WHERE id=" . $cityId; - $result = $this->execute($sql); - return $result[0]; - } - /** City Histories retriever by City id. * * This function retrieves the rows from CityHistory table that are assigned to the City with From 04109d0133ab8f1180105c1653b651d1d6d189fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 10 Jan 2023 15:59:04 +0100 Subject: [PATCH 3/9] Fix class not found. --- model/dao/CityDAO/PostgreSQLCityDAO.php | 1 + 1 file changed, 1 insertion(+) diff --git a/model/dao/CityDAO/PostgreSQLCityDAO.php b/model/dao/CityDAO/PostgreSQLCityDAO.php index 304511443..f2c864e84 100644 --- a/model/dao/CityDAO/PostgreSQLCityDAO.php +++ b/model/dao/CityDAO/PostgreSQLCityDAO.php @@ -30,6 +30,7 @@ */ include_once(PHPREPORT_ROOT . '/util/SQLIncorrectTypeException.php'); +include_once(PHPREPORT_ROOT . '/util/SQLUniqueViolationException.php'); include_once(PHPREPORT_ROOT . '/util/DBPostgres.php'); include_once(PHPREPORT_ROOT . '/model/vo/CityVO.php'); include_once(PHPREPORT_ROOT . '/model/dao/CityDAO/CityDAO.php'); From b4cbd02628419297b953fb71b5cdc94655cbf7cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 10 Jan 2023 16:01:25 +0100 Subject: [PATCH 4/9] Remove unneeded constructor for CityDAO. If constructor is missing, parent's will be called. No need to do that explicitly. In PostgreSQLCityDAO the constructor is still necessary because it needs to overwrite the BaseDAO constructor visibility. --- model/dao/CityDAO/CityDAO.php | 13 +------------ model/dao/CityDAO/PostgreSQLCityDAO.php | 10 ++++++---- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/model/dao/CityDAO/CityDAO.php b/model/dao/CityDAO/CityDAO.php index 947147b8d..6347a6765 100644 --- a/model/dao/CityDAO/CityDAO.php +++ b/model/dao/CityDAO/CityDAO.php @@ -38,18 +38,7 @@ * * @see DAOFactory::getCityDAO(), CityVO */ -abstract class CityDAO extends BaseDAO{ - - /** City DAO constructor. - * - * This is the base constructor of City DAOs, and it just calls its parent's constructor. - * - * @throws {@link ConnectionErrorException} - * @see BaseDAO::__construct() - */ - protected function __construct() { - parent::__construct(); - } +abstract class CityDAO extends BaseDAO { /** City Histories retriever by City id. * diff --git a/model/dao/CityDAO/PostgreSQLCityDAO.php b/model/dao/CityDAO/PostgreSQLCityDAO.php index f2c864e84..a2a06dc0c 100644 --- a/model/dao/CityDAO/PostgreSQLCityDAO.php +++ b/model/dao/CityDAO/PostgreSQLCityDAO.php @@ -43,17 +43,19 @@ * * @see CityDAO, CityVO */ -class PostgreSQLCityDAO extends CityDAO{ +class PostgreSQLCityDAO extends CityDAO { /** City DAO for PostgreSQL constructor. * - * This is the constructor of the implementation for PostgreSQL of {@link CityDAO}, and it just calls its parent's constructor. + * This constructor just calls its parent's constructor. It's necessary + * to overwrite the visibility of the BaseDAO constructor, which is set + * to `protected`. * * @throws {@link DBConnectionErrorException} - * @see CityDAO::__construct() + * @see BaseDAO::__construct() */ function __construct() { - parent::__construct(); + parent::__construct(); } /** City value object constructor for PostgreSQL. From 2f9d2a41da280aafe902750fbc9fc9d28fd4c101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 10 Jan 2023 18:41:40 +0100 Subject: [PATCH 5/9] Migrate CityDAO::getAll() to PDO. Remove setValues() too, which was the last used in getAll() (through BaseDAO::execute()). --- model/dao/CityDAO/PostgreSQLCityDAO.php | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/model/dao/CityDAO/PostgreSQLCityDAO.php b/model/dao/CityDAO/PostgreSQLCityDAO.php index a2a06dc0c..bd0b917a3 100644 --- a/model/dao/CityDAO/PostgreSQLCityDAO.php +++ b/model/dao/CityDAO/PostgreSQLCityDAO.php @@ -58,24 +58,6 @@ function __construct() { parent::__construct(); } - /** City value object constructor for PostgreSQL. - * - * This function creates a new {@link CityVO} with data retrieved from database. - * - * @param array $row an array with the City values from a row. - * @return CityVO a {@link CityVO} with its properties set to the values from $row. - * @see CityVO - */ - protected function setValues($row) - { - $cityVO = new CityVO(); - - $cityVO->setId($row['id']); - $cityVO->setName($row['name']); - - return $cityVO; - } - /** City Histories retriever by City id. * * This function retrieves the rows from CityHistory table that are assigned to the City with @@ -121,8 +103,8 @@ public function getCommonEvents($cityId) { * @throws {@link SQLQueryErrorException} */ public function getAll() { - $sql = "SELECT * FROM city ORDER BY id ASC"; - return $this->execute($sql); + return $this->runSelectQuery( + "SELECT * FROM city ORDER BY id ASC", [], 'CityVO'); } /** City updater. From 92cee66f3aec5080891a91ee402e4fffbfe51368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 10 Jan 2023 19:03:46 +0100 Subject: [PATCH 6/9] Delete unused CityDAO::getCommonEvents. This operation is never used; upper layers use CommonEventDAO::getByCityId instead. --- model/dao/CityDAO/CityDAO.php | 13 ------------- model/dao/CityDAO/PostgreSQLCityDAO.php | 18 ------------------ 2 files changed, 31 deletions(-) diff --git a/model/dao/CityDAO/CityDAO.php b/model/dao/CityDAO/CityDAO.php index 6347a6765..6ec53955e 100644 --- a/model/dao/CityDAO/CityDAO.php +++ b/model/dao/CityDAO/CityDAO.php @@ -53,19 +53,6 @@ abstract class CityDAO extends BaseDAO { */ public abstract function getCityHistories($cityId); - /** Common Events retriever by City id. - * - * This function retrieves the rows from Common Event table that are assigned to the City with - * the id $cityId and creates a {@link CommonEventVO} with data from each row. - * - * @param int $cityId the id of the City whose Common Events we want to retrieve. - * @return array an array with value objects {@link CommonEventVO} with their properties set to the values from the rows - * and ordered ascendantly by their database internal identifier. - * @see CommonEventDAO - * @throws {@link OperationErrorException} - */ - public abstract function getCommonEvents($cityId); - /** Cities retriever. * * This function retrieves all rows from City table and creates a {@link CityVO} with data from each row. diff --git a/model/dao/CityDAO/PostgreSQLCityDAO.php b/model/dao/CityDAO/PostgreSQLCityDAO.php index bd0b917a3..b89185f67 100644 --- a/model/dao/CityDAO/PostgreSQLCityDAO.php +++ b/model/dao/CityDAO/PostgreSQLCityDAO.php @@ -76,24 +76,6 @@ public function getCityHistories($cityId) { } - /** Common Events retriever by City id. - * - * This function retrieves the rows from Common Event table that are assigned to the City with - * the id $cityId and creates a {@link CommonEventVO} with data from each row. - * - * @param int $cityId the id of the City whose Common Events we want to retrieve. - * @return array an array with value objects {@link CommonEventVO} with their properties set to the values from the rows - * and ordered ascendantly by their database internal identifier. - * @see CommonEventDAO - * @throws {@link SQLQueryErrorException} - */ - public function getCommonEvents($cityId) { - - $dao = DAOFactory::getCommonEventDAO(); - return $dao->getByCityId($cityId); - - } - /** City retriever. * * This function retrieves all rows from City table and creates a {@link CityVO} with data from each row. From 1e8464905d68caf20a1bff4aeaf59a5dd8c56fcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 10 Jan 2023 19:05:50 +0100 Subject: [PATCH 7/9] Remove unused CityDAO::getCityHistories. --- model/dao/CityDAO/CityDAO.php | 13 ------------- model/dao/CityDAO/PostgreSQLCityDAO.php | 18 ------------------ 2 files changed, 31 deletions(-) diff --git a/model/dao/CityDAO/CityDAO.php b/model/dao/CityDAO/CityDAO.php index 6ec53955e..4083aee38 100644 --- a/model/dao/CityDAO/CityDAO.php +++ b/model/dao/CityDAO/CityDAO.php @@ -40,19 +40,6 @@ */ abstract class CityDAO extends BaseDAO { - /** City Histories retriever by City id. - * - * This function retrieves the rows from City History table that are assigned to the City with - * the id $cityId and creates a {@link CityHistoryVO} with data from each row. - * - * @param int $cityId the id of the City whose City Histories we want to retrieve. - * @return array an array with value objects {@link CityHistoryVO} with their properties set to the values from the rows - * and ordered ascendantly by their database internal identifier. - * @see CityHistoryDAO - * @throws {@link OperationErrorException} - */ - public abstract function getCityHistories($cityId); - /** Cities retriever. * * This function retrieves all rows from City table and creates a {@link CityVO} with data from each row. diff --git a/model/dao/CityDAO/PostgreSQLCityDAO.php b/model/dao/CityDAO/PostgreSQLCityDAO.php index b89185f67..8da742e13 100644 --- a/model/dao/CityDAO/PostgreSQLCityDAO.php +++ b/model/dao/CityDAO/PostgreSQLCityDAO.php @@ -58,24 +58,6 @@ function __construct() { parent::__construct(); } - /** City Histories retriever by City id. - * - * This function retrieves the rows from CityHistory table that are assigned to the City with - * the id $cityId and creates a {@link CityHistoryVO} with data from each row. - * - * @param int $cityId the id of the City whose City Histories we want to retrieve. - * @return array an array with value objects {@link CityHistoryVO} with their properties set to the values from the rows - * and ordered ascendantly by their database internal identifier. - * @see CityHistoryDAO - * @throws {@link SQLQueryErrorException} - */ - public function getCityHistories($cityId) { - - $dao = DAOFactory::getCityHistoryDAO(); - return $dao->getByCityId($cityId); - - } - /** City retriever. * * This function retrieves all rows from City table and creates a {@link CityVO} with data from each row. From d1f0e6c6c6451757bc7b0633e1e455c939c14f30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Wed, 11 Jan 2023 10:06:24 +0100 Subject: [PATCH 8/9] [#431] Fix PHP warnings in city-related services. --- web/services/createCitiesService.php | 4 +--- web/services/deleteCitiesService.php | 4 +--- web/services/getAllCitiesService.php | 2 +- web/services/updateCitiesService.php | 4 +--- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/web/services/createCitiesService.php b/web/services/createCitiesService.php index 3fc66091f..639cdedc9 100644 --- a/web/services/createCitiesService.php +++ b/web/services/createCitiesService.php @@ -115,9 +115,7 @@ } - - - if (!$string) + if (!isset($string)) { $string = "Operation Success!"; diff --git a/web/services/deleteCitiesService.php b/web/services/deleteCitiesService.php index ae406c9cf..886b17a39 100644 --- a/web/services/deleteCitiesService.php +++ b/web/services/deleteCitiesService.php @@ -116,9 +116,7 @@ } - - - if (!$string) + if (!isset($string)) $string = "Operation Success!"; } while (false); diff --git a/web/services/getAllCitiesService.php b/web/services/getAllCitiesService.php index a021c9c4e..f53412799 100644 --- a/web/services/getAllCitiesService.php +++ b/web/services/getAllCitiesService.php @@ -32,7 +32,7 @@ include_once(PHPREPORT_ROOT . '/model/facade/AdminFacade.php'); include_once(PHPREPORT_ROOT . '/model/vo/AreaVO.php'); - $sid = $_GET['sid']; + $sid = $_GET['sid'] ?? NULL; do { /* We check authentication and authorization */ diff --git a/web/services/updateCitiesService.php b/web/services/updateCitiesService.php index 76670e68b..f6cb95cda 100644 --- a/web/services/updateCitiesService.php +++ b/web/services/updateCitiesService.php @@ -125,9 +125,7 @@ } - - - if (!$string) + if (!isset($string)) { $string = "Operation Success!"; From 88604ed1b61e67063c91dd8729caa9b801532505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Wed, 11 Jan 2023 12:21:38 +0100 Subject: [PATCH 9/9] Remove unused test lines. --- model/dao/CityDAO/PostgreSQLCityDAO.php | 41 ------------------------- 1 file changed, 41 deletions(-) diff --git a/model/dao/CityDAO/PostgreSQLCityDAO.php b/model/dao/CityDAO/PostgreSQLCityDAO.php index 8da742e13..9a706af65 100644 --- a/model/dao/CityDAO/PostgreSQLCityDAO.php +++ b/model/dao/CityDAO/PostgreSQLCityDAO.php @@ -156,44 +156,3 @@ public function delete(CityVO $cityVO) { return $affectedRows; } } - - - - -/*//Uncomment these lines in order to do a simple test of the Dao - - - -$dao = new PostgreSQLcityDAO(); - -// We create a new city - -$city = new cityVO(); - -$city->setName("Shanghai"); - -$dao->create($city); - -print ("New city Id is ". $city->getId() ."\n"); - -// We search for the new Id - -$city = $dao->getById($city->getId()); - -print ("New city Id found is ". $city->getId() ."\n"); - -// We update the city with a differente name - -$city->setName("Laos"); - -$dao->update($city); - -// We search for the new name - -$city = $dao->getById($city->getId()); - -print ("New city name found is ". $city->getName() ."\n"); - -// We delete the new city - -$dao->delete($city);*/