Igalia/phpreport

Adopt PDO for database access

jaragunde opened this issue · 11 comments

PHP Data Objects (PDO) define a consistent interface for accessing databases in PHP: https://www.php.net/manual/en/intro.pdo.php.

It can access different database providers with the same interface, saving ourselves the hassle to code DB access different implementations with pg_* or mysql_* methods if we need to support multiple backends.

More importantly, PDO provides a defense against SQL injection using parameter binding for DB queries. We currently have some protections using the DBAdapter code that we imported from the Propel project, but we must not forget to use them when coding queries and we probably failed to do so here and there.

Let's figure a way to adopt PDO incrementally in our code base!

A lesson learned when coding the first patches: if we want to use PDO::FETCH_CLASS, we need to modify the VOs to have the internal properties match the column names.

I already attempted to match column names in the query with the internal properties in the VOs using AS like SELECT usrid AS userId, but it returns column names in lower case. The result is it creates a new userid property in TemplateVO, instead of assigning the correct value to the existing userId property.

Reviewed in PR #534:

09a5efa6 [#513] Port Template DAO to use PDO. [Jacobo Aragunde Pérez]
9460159d Use PDO::FETCH_CLASS to retrieve TemplateVO objects. [Jacobo Aragunde Pérez]
f74cf305 Docs: Add PDO to system requirements. [Jacobo Aragunde Pérez]
01afc795 Port PostgreSQLTemplateDAO::delete() to PDO. [Jacobo Aragunde Pérez]
a4ea9bd5 Port PostgreSQLTemplateDAO::create() to PDO. [Jacobo Aragunde Pérez]

From #539:

f134ab10 [#513] Port Config DAO to use PDO. [Jacobo Aragunde Pérez]
81a521dd Port PostgreSQLConfigDAO to use PDO. [Jacobo Aragunde Pérez]
c868e06b Remove unused operation ConfigDAO::getVersionNumber(). [Jacobo Aragunde Pérez]
1b83e5c8 Move PDO object up in the DAO hierarchy to BaseDAO. [Jacobo Aragunde Pérez]
06f54e84 Extract PDO connection logic to DatabaseConnectionManager. [Jacobo Aragunde Pérez]

Reviewed in #551:

edd6c61b [#513] Port Customer DAO to use PDO. [Jacobo Aragunde Pérez]
4839173e Delete unused PostgreSQLCustomerDAO::setValues. [Jacobo Aragunde Pérez]
e4f3dbe9 Remove commented test lines in PostgreSQLCustomerDAO. [Jacobo Aragunde Pérez]
06a1e578 [#431] Fix PHP warnings in customer management page. [Jacobo Aragunde Pérez]
04bd1841 Migrate CustomerDAO create/update/delete operations to PDO. [Jacobo Aragunde Pérez]
0424e4b5 Remove unused CustomerDAO::getBySectorId and related code. [Jacobo Aragunde Pérez]
011987c6 Migrate CustomerDAO::getByProjectUserLogin to PDO. [Jacobo Aragunde Pérez]
6141862c Migrate CustomerDAO::getById and getAll to PDO. [Jacobo Aragunde Pérez]
5bd78abe Remove redundant CustomerDAO constructor declaration. [Jacobo Aragunde Pérez]
df4e6f2f Move runSelectQuery() up to BaseDAO. [Jacobo Aragunde Pérez]

Reviewed in #553:

edb994c7 [#513] Port Sector DAO to use PDO. [Jacobo Aragunde Pérez]
b8f9d25c Remove commented test lines in PostgreSQLSectorDAO. [Jacobo Aragunde Pérez]
d8b73be8 [#431] Fix 'undefined variable: string' in sector services. [Jacobo Aragunde Pérez]
06c20138 Migrate SectorDAO create/update/delete operations to PDO. [Jacobo Aragunde Pérez]
c2e78a38 Migrate SectorDAO::getById and getAll to PDO. [Jacobo Aragunde Pérez]
79b6d7d2 Remove redundant SectorDAO constructor. [Jacobo Aragunde Pérez]

Reviewed in PR #560:

ff8fc346 [#513] Port Area DAO to use PDO. [Jacobo Aragunde Pérez]
66486a9c Remove placeholder implementations of setValues in PDO DAOs. [Jacobo Aragunde Pérez]
3efef683 Remove commented test lines in PostgreSQLareaDAO. [Jacobo Aragunde Pérez]
bc8dbff6 [#431] Fix warnings in area-related services. [Jacobo Aragunde Pérez]
af485803 Migrate AreaDAO create/update/delete operations to PDO. [Jacobo Aragunde Pérez]
9ac65148 Remove unused AreaDAO::getByName(). [Jacobo Aragunde Pérez]
5dae0d54 Migrate AreaDAO getter operations to PDO. [Jacobo Aragunde Pérez]
f251a936 Remove redundant AreaDAO constructor. [Jacobo Aragunde Pérez]

Merged in #575:

0faa34dd [#513] Port Extra Hour DAO to use PDO. [Jacobo Aragunde Pérez]
1976c0e9 [#431] Fix PHP warnings in hour compensation management page. [Jacobo Aragunde Pérez]
1079ec7e Remove unused test lines. [Jacobo Aragunde Pérez]
e9a86663 Migrate ExtraHourDAO create/update/delete operations to PDO. [Jacobo Aragunde Pérez]
3ffd3638 Remove ExtraHourDAO::setValues(). [Jacobo Aragunde Pérez]
8ed8ba07 Migrate ExtraHourDAO::getLastByUserId and getAll to PDO. [Jacobo Aragunde Pérez]
575992f8 Remove unused ExtraHourDAO::getByUserId(). [Jacobo Aragunde Pérez]
77a6fa8e Migrate ExtraHourDAO::getById to PDO and adjust VO. [Jacobo Aragunde Pérez]
6b91b7c7 Remove unused constructor for ExtraHourDAO. [Jacobo Aragunde Pérez]

As part of PR #608, the create operation in ProjectDAO was migrated:

a968bd17 Merge pull request #608 from Igalia/bubble-up-sql-errors [Danielle M]
7ec2b2fe Fix string construction in service [Danielle Mayabb]
207ea879 (origin/bubble-up-sql-errors) Clean up formatting, use getters [Danielle Mayabb]
5bfa360e Clean up some xml so that messages are returned correctly [Danielle Mayabb]
a4175f5b Fix formatting/naming, return error code on error [Danielle Mayabb]
cfcdc447 Reload data store if creation fails [Danielle Mayabb]
a15d8cae Update save of dates, facade, and service [Danielle Mayabb]
d1940ea5 Add error handling for not null violation [Danielle Mayabb]
7762bffd Update object metadata and fix date conversion [Danielle Mayabb]
b9d4be4a Update copyright and remove trailing whitespace [Danielle Mayabb]
6c080269 Update var convention and delete whitespace [Danielle Mayabb]
90337f62 Expand exception handling in projectManagement.js [Danielle Mayabb]
6ad9871c Update createProjectsService to check new property isSuccessful [Danielle Mayabb]
074632f4 Update Project action, facade, and DAO [Danielle Mayabb]
d58e254f Update ProjectDAO create to use PDO and return OperationResult [Danielle Mayabb]
441c31a4 Add OperationResult class [Danielle Mayabb]

In PR #614 I migrated the delete operation in ProjectDAO:

f0ae97d8 Delete unused test code. [Jacobo Aragunde Pérez]
3145b3b3 [#172] Delete user assignment when deleting a project. [Jacobo Aragunde Pérez]
55d87e67 [#496,#513] Return more specific errors on project deletion. [Jacobo Aragunde Pérez]
1b1aa84c Remove unused constructor for ProjectDAO. [Jacobo Aragunde Pérez]

In PR #615 I migrated update operations in ProjectDAO:

0aeaf6e6 [#496, #513] Replace project partial update operation. [Jacobo Aragunde Pérez]
3fe93733 [#496,#513] Return more specific errors on project update. [Jacobo Aragunde Pérez]
b7b339bd Simplify project create operation. [Jacobo Aragunde Pérez]

Merged in #618:

82d4d972 [#496] Review error response codes in task operations. [Jacobo Aragunde Pérez]
ddbeb195 [#173] Make error messages more noticeable. [Jacobo Aragunde Pérez]
13e9c2b2 [#496,#513] Return more detailed errors on task delete. [Jacobo Aragunde Pérez]
e49f20e5 [#496] Return more detailed errors on task update. [Jacobo Aragunde Pérez]