Commit 15932f95 authored by Alexander Schnitzler's avatar Alexander Schnitzler Committed by Frank Nägler
Browse files

[TASK] Harden \TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbBackend

- Use strict type mode
- Use type hints whereever possible
- Use type casts according to phpstan results

Releases: master
Resolves: #88666
Change-Id: I4d93c3e3c75f5f6ca6b53bd73acffe73d7b6a58e
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61205


Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Andreas Fernandez's avatarAndreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Frank Nägler's avatarFrank Naegler <frank.naegler@typo3.org>
Reviewed-by: Andreas Fernandez's avatarAndreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Oliver Klee's avatarOliver Klee <typo3-coding@oliverklee.de>
Reviewed-by: Frank Nägler's avatarFrank Naegler <frank.naegler@typo3.org>
parent 66302a14
......@@ -619,8 +619,8 @@ class Backend implements \TYPO3\CMS\Extbase\Persistence\Generic\BackendInterface
{
if ($object instanceof \TYPO3\CMS\Extbase\DomainObject\AbstractValueObject) {
$result = $this->getUidOfAlreadyPersistedValueObject($object);
if ($result !== false) {
$object->_setProperty('uid', (int)$result);
if ($result !== null) {
$object->_setProperty('uid', $result);
return;
}
}
......@@ -708,7 +708,7 @@ class Backend implements \TYPO3\CMS\Extbase\Persistence\Generic\BackendInterface
* Tests, if the given Value Object already exists in the storage backend and if so, it returns the uid.
*
* @param \TYPO3\CMS\Extbase\DomainObject\AbstractValueObject $object The object to be tested
* @return mixed The matching uid if an object was found, else FALSE
* @return int|null The matching uid if an object was found, else null
*/
protected function getUidOfAlreadyPersistedValueObject(\TYPO3\CMS\Extbase\DomainObject\AbstractValueObject $object)
{
......@@ -776,11 +776,11 @@ class Backend implements \TYPO3\CMS\Extbase\Persistence\Generic\BackendInterface
if (is_array($relationTableMatchFields)) {
$row = array_merge($relationTableMatchFields, $row);
}
$res = $this->storageBackend->updateRelationTableRow(
$this->storageBackend->updateRelationTableRow(
$relationTableName,
$row
);
return $res;
return true;
}
/**
......@@ -802,8 +802,8 @@ class Backend implements \TYPO3\CMS\Extbase\Persistence\Generic\BackendInterface
if (is_array($relationTableMatchFields)) {
$relationMatchFields = array_merge($relationTableMatchFields, $relationMatchFields);
}
$res = $this->storageBackend->removeRow($relationTableName, $relationMatchFields, false);
return $res;
$this->storageBackend->removeRow($relationTableName, $relationMatchFields, false);
return true;
}
/**
......@@ -827,8 +827,8 @@ class Backend implements \TYPO3\CMS\Extbase\Persistence\Generic\BackendInterface
if (is_array($relationTableMatchFields)) {
$relationMatchFields = array_merge($relationTableMatchFields, $relationMatchFields);
}
$res = $this->storageBackend->removeRow($relationTableName, $relationMatchFields, false);
return $res;
$this->storageBackend->removeRow($relationTableName, $relationMatchFields, false);
return true;
}
/**
......@@ -905,15 +905,14 @@ class Backend implements \TYPO3\CMS\Extbase\Persistence\Generic\BackendInterface
$row['uid'] = $object->_getProperty('_localizedUid');
}
}
$res = $this->storageBackend->updateRow($dataMap->getTableName(), $row);
if ($res === true) {
$this->storageBackend->updateRow($dataMap->getTableName(), $row);
$this->emitAfterUpdateObjectSignal($object);
}
$frameworkConfiguration = $this->configurationManager->getConfiguration(\TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK);
if ($frameworkConfiguration['persistence']['updateReferenceIndex'] === '1') {
$this->referenceIndex->updateRefIndexTable($dataMap->getTableName(), $row['uid']);
}
return $res;
return true;
}
/**
......@@ -1003,13 +1002,12 @@ class Backend implements \TYPO3\CMS\Extbase\Persistence\Generic\BackendInterface
$deletedColumnName => 1
];
$this->addCommonDateFieldsToRow($object, $row);
$res = $this->storageBackend->updateRow($tableName, $row);
$this->storageBackend->updateRow($tableName, $row);
} else {
$res = $this->storageBackend->removeRow($tableName, ['uid' => $object->getUid()]);
$this->storageBackend->removeRow($tableName, ['uid' => $object->getUid()]);
}
if ($res === true) {
$this->emitAfterRemoveObjectSignal($object);
}
$this->removeRelatedObjects($object);
$frameworkConfiguration = $this->configurationManager->getConfiguration(\TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK);
if ($frameworkConfiguration['persistence']['updateReferenceIndex'] === '1') {
......
<?php
declare(strict_types = 1);
namespace TYPO3\CMS\Extbase\Persistence\Generic\Storage;
/*
......@@ -27,7 +29,7 @@ interface BackendInterface
* @param bool $isRelation TRUE if we are currently inserting into a relation table, FALSE by default
* @return int the UID of the inserted row
*/
public function addRow($tableName, array $fieldValues, $isRelation = false);
public function addRow(string $tableName, array $fieldValues, bool $isRelation = false): int;
/**
* Updates a row in the storage
......@@ -35,18 +37,16 @@ interface BackendInterface
* @param string $tableName The database table name
* @param array $fieldValues The fieldValues to update
* @param bool $isRelation TRUE if we are currently inserting into a relation table, FALSE by default
* @return mixed|void
*/
public function updateRow($tableName, array $fieldValues, $isRelation = false);
public function updateRow(string $tableName, array $fieldValues, bool $isRelation = false): void;
/**
* Updates a relation row in the storage
*
* @param string $tableName The database relation table name
* @param array $fieldValues The fieldValues to be updated
* @return bool
*/
public function updateRelationTableRow($tableName, array $fieldValues);
public function updateRelationTableRow(string $tableName, array $fieldValues): void;
/**
* Deletes a row in the storage
......@@ -54,9 +54,8 @@ interface BackendInterface
* @param string $tableName The database table name
* @param array $where An array of where array('fieldname' => value). This array will be transformed to a WHERE clause
* @param bool $isRelation TRUE if we are currently inserting into a relation table, FALSE by default
* @return mixed|void
*/
public function removeRow($tableName, array $where, $isRelation = false);
public function removeRow(string $tableName, array $where, bool $isRelation = false): void;
/**
* Fetches maximal value for given table column
......@@ -66,7 +65,7 @@ interface BackendInterface
* @param string $columnName column name to get the max value from
* @return mixed the max value
*/
public function getMaxValueFromTable($tableName, array $where, $columnName);
public function getMaxValueFromTable(string $tableName, array $where, string $columnName);
/**
* Returns the number of items matching the query.
......@@ -74,7 +73,7 @@ interface BackendInterface
* @param \TYPO3\CMS\Extbase\Persistence\QueryInterface $query
* @return int
*/
public function getObjectCountByQuery(\TYPO3\CMS\Extbase\Persistence\QueryInterface $query);
public function getObjectCountByQuery(\TYPO3\CMS\Extbase\Persistence\QueryInterface $query): int;
/**
* Returns the object data matching the $query.
......@@ -82,14 +81,14 @@ interface BackendInterface
* @param \TYPO3\CMS\Extbase\Persistence\QueryInterface $query
* @return array
*/
public function getObjectDataByQuery(\TYPO3\CMS\Extbase\Persistence\QueryInterface $query);
public function getObjectDataByQuery(\TYPO3\CMS\Extbase\Persistence\QueryInterface $query): array;
/**
* Checks if a Value Object equal to the given Object exists in the data base
*
* @param \TYPO3\CMS\Extbase\DomainObject\AbstractValueObject $object The Value Object
* @return mixed The matching uid if an object was found, else FALSE
* @return int|null The matching uid if an object was found, else null
* @todo this is the last monster in this persistence series. refactor!
*/
public function getUidOfAlreadyPersistedValueObject(\TYPO3\CMS\Extbase\DomainObject\AbstractValueObject $object);
public function getUidOfAlreadyPersistedValueObject(\TYPO3\CMS\Extbase\DomainObject\AbstractValueObject $object): ?int;
}
<?php
declare(strict_types = 1);
namespace TYPO3\CMS\Extbase\Persistence\Generic\Storage;
/*
......@@ -81,7 +83,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
/**
* @param ConfigurationManagerInterface $configurationManager
*/
public function injectConfigurationManager(ConfigurationManagerInterface $configurationManager)
public function injectConfigurationManager(ConfigurationManagerInterface $configurationManager): void
{
$this->configurationManager = $configurationManager;
}
......@@ -89,7 +91,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
/**
* @param CacheService $cacheService
*/
public function injectCacheService(CacheService $cacheService)
public function injectCacheService(CacheService $cacheService): void
{
$this->cacheService = $cacheService;
}
......@@ -97,7 +99,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
/**
* @param EnvironmentService $environmentService
*/
public function injectEnvironmentService(EnvironmentService $environmentService)
public function injectEnvironmentService(EnvironmentService $environmentService): void
{
$this->environmentService = $environmentService;
}
......@@ -105,7 +107,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
/**
* @param ObjectManagerInterface $objectManager
*/
public function injectObjectManager(ObjectManagerInterface $objectManager)
public function injectObjectManager(ObjectManagerInterface $objectManager): void
{
$this->objectManager = $objectManager;
}
......@@ -127,7 +129,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* @return int The uid of the inserted row
* @throws SqlErrorException
*/
public function addRow($tableName, array $fieldValues, $isRelation = false)
public function addRow(string $tableName, array $fieldValues, bool $isRelation = false): int
{
if (isset($fieldValues['uid'])) {
unset($fieldValues['uid']);
......@@ -153,10 +155,10 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
$uid = 0;
if (!$isRelation) {
// Relation tables have no auto_increment column, so no retrieval must be tried.
$uid = $connection->lastInsertId($tableName);
$uid = (int)$connection->lastInsertId($tableName);
$this->clearPageCache($tableName, $uid);
}
return (int)$uid;
return $uid;
}
/**
......@@ -165,11 +167,10 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* @param string $tableName The database table name
* @param array $fieldValues The row to be updated
* @param bool $isRelation TRUE if we are currently inserting into a relation table, FALSE by default
* @return bool
* @throws \InvalidArgumentException
* @throws SqlErrorException
*/
public function updateRow($tableName, array $fieldValues, $isRelation = false)
public function updateRow(string $tableName, array $fieldValues, bool $isRelation = false): void
{
if (!isset($fieldValues['uid'])) {
throw new \InvalidArgumentException('The given row must contain a value for "uid".', 1476045164);
......@@ -187,6 +188,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
// mssql needs to set proper PARAM_LOB and others to update fields
$tableDetails = $connection->getSchemaManager()->listTableDetails($tableName);
foreach ($fieldValues as $columnName => $columnValue) {
$columnName = (string)$columnName;
$types[$columnName] = $tableDetails->getColumn($columnName)->getType()->getBindingType();
}
}
......@@ -199,9 +201,6 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
if (!$isRelation) {
$this->clearPageCache($tableName, $uid);
}
// always returns true
return true;
}
/**
......@@ -209,11 +208,10 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
*
* @param string $tableName The database relation table name
* @param array $fieldValues The row to be updated
* @return bool
* @throws SqlErrorException
* @throws \InvalidArgumentException
*/
public function updateRelationTableRow($tableName, array $fieldValues)
public function updateRelationTableRow(string $tableName, array $fieldValues): void
{
if (!isset($fieldValues['uid_local']) && !isset($fieldValues['uid_foreign'])) {
throw new \InvalidArgumentException(
......@@ -222,6 +220,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
);
}
$where = [];
$where['uid_local'] = (int)$fieldValues['uid_local'];
$where['uid_foreign'] = (int)$fieldValues['uid_foreign'];
unset($fieldValues['uid_local']);
......@@ -241,9 +240,6 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
} catch (DBALException $e) {
throw new SqlErrorException($e->getPrevious()->getMessage(), 1470230768, $e);
}
// always returns true
return true;
}
/**
......@@ -252,10 +248,9 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* @param string $tableName The database table name
* @param array $where An array of where array('fieldname' => value).
* @param bool $isRelation TRUE if we are currently manipulating a relation table, FALSE by default
* @return bool
* @throws SqlErrorException
*/
public function removeRow($tableName, array $where, $isRelation = false)
public function removeRow(string $tableName, array $where, bool $isRelation = false): void
{
try {
$this->connectionPool->getConnectionForTable($tableName)->delete($tableName, $where);
......@@ -264,11 +259,8 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
}
if (!$isRelation && isset($where['uid'])) {
$this->clearPageCache($tableName, $where['uid']);
$this->clearPageCache($tableName, (int)$where['uid']);
}
// always returns true
return true;
}
/**
......@@ -280,7 +272,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* @return mixed the max value
* @throws SqlErrorException
*/
public function getMaxValueFromTable($tableName, array $where, $columnName)
public function getMaxValueFromTable(string $tableName, array $where, string $columnName)
{
try {
$queryBuilder = $this->connectionPool->getQueryBuilderForTable($tableName);
......@@ -312,8 +304,10 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* @return array|bool
* @throws SqlErrorException
*/
public function getRowByIdentifier($tableName, array $where)
public function getRowByIdentifier(string $tableName, array $where)
{
// todo: this method is not in use, consider removing it.
try {
$queryBuilder = $this->connectionPool->getQueryBuilderForTable($tableName);
$queryBuilder->getRestrictions()->removeAll();
......@@ -328,10 +322,10 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
}
$row = $queryBuilder->execute()->fetch();
return is_array($row) ? $row : false;
} catch (DBALException $e) {
throw new SqlErrorException($e->getPrevious()->getMessage(), 1470230771, $e);
}
return $row ?: false;
}
/**
......@@ -341,14 +335,16 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* @return array
* @throws SqlErrorException
*/
public function getObjectDataByQuery(QueryInterface $query)
public function getObjectDataByQuery(QueryInterface $query): array
{
$statement = $query->getStatement();
// todo: remove instanceof checks as soon as getStatement() strictly returns Qom\Statement only
if ($statement instanceof Qom\Statement
&& !$statement->getStatement() instanceof QueryBuilder
) {
$rows = $this->getObjectDataByRawQuery($statement);
} else {
/** @var Typo3DbQueryParser $queryParser */
$queryParser = $this->objectManager->get(Typo3DbQueryParser::class);
if ($statement instanceof Qom\Statement
&& $statement->getStatement() instanceof QueryBuilder
......@@ -389,7 +385,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* @return array
* @throws SqlErrorException when the raw SQL statement fails in the database
*/
protected function getObjectDataByRawQuery(Qom\Statement $statement)
protected function getObjectDataByRawQuery(Qom\Statement $statement): array
{
$realStatement = $statement->getStatement();
$parameters = $statement->getBoundVariables();
......@@ -435,7 +431,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* @throws BadConstraintException
* @throws SqlErrorException
*/
public function getObjectCountByQuery(QueryInterface $query)
public function getObjectCountByQuery(QueryInterface $query): int
{
if ($query->getConstraint() instanceof Qom\Statement) {
throw new BadConstraintException('Could not execute count on queries with a constraint of type TYPO3\\CMS\\Extbase\\Persistence\\Generic\\Qom\\Statement', 1256661045);
......@@ -448,6 +444,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
$rows = $this->getObjectDataByQuery($query);
$count = count($rows);
} else {
/** @var Typo3DbQueryParser $queryParser */
$queryParser = $this->objectManager->get(Typo3DbQueryParser::class);
$queryBuilder = $queryParser
->convertQueryToDoctrineQueryBuilder($query)
......@@ -483,11 +480,12 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* Checks if a Value Object equal to the given Object exists in the database
*
* @param AbstractValueObject $object The Value Object
* @return mixed The matching uid if an object was found, else FALSE
* @return int|null The matching uid if an object was found, else FALSE
* @throws SqlErrorException
*/
public function getUidOfAlreadyPersistedValueObject(AbstractValueObject $object)
public function getUidOfAlreadyPersistedValueObject(AbstractValueObject $object): ?int
{
/** @var DataMapper $dataMapper */
$dataMapper = $this->objectManager->get(DataMapper::class);
$dataMap = $dataMapper->getDataMap(get_class($object));
$tableName = $dataMap->getTableName();
......@@ -499,6 +497,8 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
// loop over all properties of the object to exactly set the values of each database field
$properties = $object->_getProperties();
foreach ($properties as $propertyName => $propertyValue) {
$propertyName = (string)$propertyName;
// @todo We couple the Backend to the Entity implementation (uid, isClone); changes there breaks this method
if ($dataMap->isPersistableProperty($propertyName) && $propertyName !== 'uid' && $propertyName !== 'pid' && $propertyName !== 'isClone') {
$fieldName = $dataMap->getColumnMap($propertyName)->getColumnName();
......@@ -521,7 +521,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
if ($uid > 0) {
return $uid;
}
return false;
return null;
} catch (DBALException $e) {
throw new SqlErrorException($e->getPrevious()->getMessage(), 1470231748, $e);
}
......@@ -550,15 +550,16 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
return $rows;
}
/** @var Context $context */
$context = $this->objectManager->get(Context::class);
if ($workspaceUid === null) {
$workspaceUid = $context->getPropertyFromAspect('workspace', 'id');
$workspaceUid = (int)$context->getPropertyFromAspect('workspace', 'id');
} else {
// A custom query is needed, so a custom context is cloned
$workspaceUid = (int)$workspaceUid;
$context = clone $context;
$context->setAspect('workspace', $this->objectManager->get(WorkspaceAspect::class, $workspaceUid));
}
/** @var PageRepository $pageRepository */
$pageRepository = $this->objectManager->get(PageRepository::class, $context);
// Fetches the move-placeholder in case it is supported
......@@ -595,6 +596,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
if ($tableName === 'pages') {
$row = $pageRepository->getPageOverlay($row, $querySettings->getLanguageUid());
} else {
// todo: remove type cast once getLanguageUid strictly returns an int
$languageUid = (int)$querySettings->getLanguageUid();
if (!$querySettings->getRespectSysLanguage()
&& isset($row[$GLOBALS['TCA'][$tableName]['ctrl']['languageField']])
......@@ -632,7 +634,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
* @param string $tableName Table name of the record
* @param int $uid UID of the record
*/
protected function clearPageCache($tableName, $uid)
protected function clearPageCache(string $tableName, int $uid): void
{
$frameworkConfiguration = $this->configurationManager->getConfiguration(ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK);
if (empty($frameworkConfiguration['persistence']['enableAutomaticCacheClearing'])) {
......@@ -643,7 +645,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
// As determining the table columns is a costly operation this is done only once per table during runtime and cached then
if (!isset($this->hasPidColumn[$tableName])) {
$columns = GeneralUtility::makeInstance(ConnectionPool::class)
$columns = $this->connectionPool
->getConnectionForTable($tableName)
->getSchemaManager()
->listTableColumns($tableName);
......@@ -696,7 +698,7 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
/**
* @return TypoScriptFrontendController|null
*/
protected function getTSFE()
protected function getTSFE(): ?TypoScriptFrontendController
{
return $GLOBALS['TSFE'] ?? null;
}
......
......@@ -64,6 +64,10 @@ class BackendTest extends UnitTestCase
'' => 0
];
$columnMap
->expects($this->once())
->method('getRelationTableName')
->will($this->returnValue('myTable'));
$columnMap
->expects($this->once())
->method('getRelationTableMatchFields')
......@@ -83,7 +87,7 @@ class BackendTest extends UnitTestCase
$storageBackend
->expects($this->once())
->method('addRow')
->with(null, $expectedRow, true);
->with('myTable', $expectedRow, true);
$fixture->_set('dataMapFactory', $dataMapFactory);
$fixture->_set('storageBackend', $storageBackend);
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment