From c6803fabb82a48231c0f4faff83b7655c7fae45c Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Wed, 10 Aug 2016 06:56:28 +0200
Subject: [PATCH] [TASK] Doctrine: Migrate Extbase DB backend Part 2
The method getUidOfAlreadyPersistedValueObject() is migrated
to doctrine.
During the change, it was noticable that the calls to the deprecated
protected methods addVisibilityConstraintStatement(),
getBackendConstraintStatement() and getFrontendConstraintStatement()
are not needed at all, and are removed, as the RestrictionBuilder
is taking care of that functionality transparently.
Resolves: #77476
Releases: master
Change-Id: Ib89abd6e5155f5caeb9d06b452aba77ac39444b1
Reviewed-on: https://review.typo3.org/49436
Tested-by: Bamboo TYPO3com <info@typo3.com>
Reviewed-by: Morton Jonuschat <m.jonuschat@mojocode.de>
Tested-by: Morton Jonuschat <m.jonuschat@mojocode.de>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
---
.../Generic/Storage/Typo3DbBackend.php | 171 +++---------------
.../Generic/Storage/Typo3DbBackendTest.php | 81 +++++++--
2 files changed, 94 insertions(+), 158 deletions(-)
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbBackend.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbBackend.php
index c802e1cc2da3..37e6afcde77a 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbBackend.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbBackend.php
@@ -17,10 +17,12 @@ namespace TYPO3\CMS\Extbase\Persistence\Generic\Storage;
use Doctrine\DBAL\DBALException;
use TYPO3\CMS\Backend\Utility\BackendUtility;
use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Database\Query\Restriction\FrontendRestrictionContainer;
use TYPO3\CMS\Core\SingletonInterface;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\MathUtility;
use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
+use TYPO3\CMS\Extbase\DomainObject\AbstractValueObject;
use TYPO3\CMS\Extbase\Persistence\Generic\Qom;
use TYPO3\CMS\Extbase\Persistence\Generic\Storage\Exception\SqlErrorException;
use TYPO3\CMS\Extbase\Persistence\QueryInterface;
@@ -523,166 +525,51 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
}
/**
- * Checks if a Value Object equal to the given Object exists in the data base
+ * Checks if a Value Object equal to the given Object exists in the database
*
- * @param \TYPO3\CMS\Extbase\DomainObject\AbstractValueObject $object The Value Object
+ * @param AbstractValueObject $object The Value Object
* @return mixed The matching uid if an object was found, else FALSE
- * @todo this is the last monster in this persistence series. refactor!
+ * @throws SqlErrorException
*/
- public function getUidOfAlreadyPersistedValueObject(\TYPO3\CMS\Extbase\DomainObject\AbstractValueObject $object)
+ public function getUidOfAlreadyPersistedValueObject(AbstractValueObject $object)
{
- $fields = array();
- $parameters = array();
$dataMap = $this->dataMapper->getDataMap(get_class($object));
+ $tableName = $dataMap->getTableName();
+ $queryBuilder = $this->connectionPool->getQueryBuilderForTable($tableName);
+ if ($this->environmentService->isEnvironmentInFrontendMode()) {
+ $queryBuilder->setRestrictions(GeneralUtility::makeInstance(FrontendRestrictionContainer::class));
+ }
+ $whereClause = [];
+ // loop over all properties of the object to exactly set the values of each database field
$properties = $object->_getProperties();
foreach ($properties as $propertyName => $propertyValue) {
// @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();
if ($propertyValue === null) {
- $fields[] = $dataMap->getColumnMap($propertyName)->getColumnName() . ' IS NULL';
+ $whereClause[] = $queryBuilder->expr()->isNull($fieldName);
} else {
- $fields[] = $dataMap->getColumnMap($propertyName)->getColumnName() . '=?';
- $parameters[] = $this->dataMapper->getPlainValue($propertyValue);
+ $whereClause[] = $queryBuilder->expr()->eq($fieldName, $queryBuilder->createNamedParameter($this->dataMapper->getPlainValue($propertyValue)));
}
}
}
- $sql = array();
- $sql['additionalWhereClause'] = array();
- $tableName = $dataMap->getTableName();
- $this->addVisibilityConstraintStatement(new \TYPO3\CMS\Extbase\Persistence\Generic\Typo3QuerySettings(), $tableName, $sql);
- $statement = 'SELECT * FROM ' . $tableName;
- $statement .= ' WHERE ' . implode(' AND ', $fields);
- if (!empty($sql['additionalWhereClause'])) {
- $statement .= ' AND ' . implode(' AND ', $sql['additionalWhereClause']);
- }
- $this->replacePlaceholders($statement, $parameters, $tableName);
- // debug($statement,-2);
- $res = $this->databaseHandle->sql_query($statement);
- $this->checkSqlErrors($statement);
- $row = $this->databaseHandle->sql_fetch_assoc($res);
- if ($row !== false) {
- return (int)$row['uid'];
- } else {
- return false;
- }
- }
-
- /**
- * Replace query placeholders in a query part by the given
- * parameters.
- *
- * @param string &$sqlString The query part with placeholders
- * @param array $parameters The parameters
- * @param string $tableName
- *
- * @throws \TYPO3\CMS\Extbase\Persistence\Generic\Exception
- * @deprecated since 6.2, will be removed two versions later
- * @todo add deprecation notice after getUidOfAlreadyPersistedValueObject is adjusted
- */
- protected function replacePlaceholders(&$sqlString, array $parameters, $tableName = 'foo')
- {
- // @todo profile this method again
- if (substr_count($sqlString, '?') !== count($parameters)) {
- throw new \TYPO3\CMS\Extbase\Persistence\Generic\Exception('The number of question marks to replace must be equal to the number of parameters.', 1460975513);
- }
- $offset = 0;
- foreach ($parameters as $parameter) {
- $markPosition = strpos($sqlString, '?', $offset);
- if ($markPosition !== false) {
- if ($parameter === null) {
- $parameter = 'NULL';
- } elseif (is_array($parameter) || $parameter instanceof \ArrayAccess || $parameter instanceof \Traversable) {
- $items = array();
- foreach ($parameter as $item) {
- $items[] = $this->databaseHandle->fullQuoteStr($item, $tableName);
- }
- $parameter = '(' . implode(',', $items) . ')';
- } else {
- $parameter = $this->databaseHandle->fullQuoteStr($parameter, $tableName);
- }
- $sqlString = substr($sqlString, 0, $markPosition) . $parameter . substr($sqlString, ($markPosition + 1));
- }
- $offset = $markPosition + strlen($parameter);
- }
- }
-
- /**
- * Adds enableFields and deletedClause to the query if necessary
- *
- * @param \TYPO3\CMS\Extbase\Persistence\Generic\QuerySettingsInterface $querySettings
- * @param string $tableName The database table name
- * @param array &$sql The query parts
- * @return void
- * @todo remove after getUidOfAlreadyPersistedValueObject is adjusted, this was moved to queryParser
- */
- protected function addVisibilityConstraintStatement(\TYPO3\CMS\Extbase\Persistence\Generic\QuerySettingsInterface $querySettings, $tableName, array &$sql)
- {
- $statement = '';
- if (is_array($GLOBALS['TCA'][$tableName]['ctrl'])) {
- $ignoreEnableFields = $querySettings->getIgnoreEnableFields();
- $enableFieldsToBeIgnored = $querySettings->getEnableFieldsToBeIgnored();
- $includeDeleted = $querySettings->getIncludeDeleted();
- if ($this->environmentService->isEnvironmentInFrontendMode()) {
- $statement .= $this->getFrontendConstraintStatement($tableName, $ignoreEnableFields, $enableFieldsToBeIgnored, $includeDeleted);
- } else {
- // TYPO3_MODE === 'BE'
- $statement .= $this->getBackendConstraintStatement($tableName, $ignoreEnableFields, $includeDeleted);
- }
- if (!empty($statement)) {
- $statement = strtolower(substr($statement, 1, 3)) === 'and' ? substr($statement, 5) : $statement;
- $sql['additionalWhereClause'][] = $statement;
- }
- }
- }
+ $queryBuilder
+ ->select('uid')
+ ->from($tableName)
+ ->where(...$whereClause);
- /**
- * Returns constraint statement for frontend context
- *
- * @param string $tableName
- * @param bool $ignoreEnableFields A flag indicating whether the enable fields should be ignored
- * @param array $enableFieldsToBeIgnored If $ignoreEnableFields is true, this array specifies enable fields to be ignored. If it is NULL or an empty array (default) all enable fields are ignored.
- * @param bool $includeDeleted A flag indicating whether deleted records should be included
- * @return string
- * @throws \TYPO3\CMS\Extbase\Persistence\Generic\Exception\InconsistentQuerySettingsException
- * @todo remove after getUidOfAlreadyPersistedValueObject is adjusted, this was moved to queryParser
- */
- protected function getFrontendConstraintStatement($tableName, $ignoreEnableFields, array $enableFieldsToBeIgnored = array(), $includeDeleted)
- {
- $statement = '';
- if ($ignoreEnableFields && !$includeDeleted) {
- if (!empty($enableFieldsToBeIgnored)) {
- // array_combine() is necessary because of the way \TYPO3\CMS\Frontend\Page\PageRepository::enableFields() is implemented
- $statement .= $this->getPageRepository()->enableFields($tableName, -1, array_combine($enableFieldsToBeIgnored, $enableFieldsToBeIgnored));
+ try {
+ $uid = (int)$queryBuilder
+ ->execute()
+ ->fetchColumn(0);
+ if ($uid > 0) {
+ return $uid;
} else {
- $statement .= $this->getPageRepository()->deleteClause($tableName);
+ return false;
}
- } elseif (!$ignoreEnableFields && !$includeDeleted) {
- $statement .= $this->getPageRepository()->enableFields($tableName);
- } elseif (!$ignoreEnableFields && $includeDeleted) {
- throw new \TYPO3\CMS\Extbase\Persistence\Generic\Exception\InconsistentQuerySettingsException('Query setting "ignoreEnableFields=FALSE" can not be used together with "includeDeleted=TRUE" in frontend context.', 1327678173);
- }
- return $statement;
- }
-
- /**
- * Returns constraint statement for backend context
- *
- * @param string $tableName
- * @param bool $ignoreEnableFields A flag indicating whether the enable fields should be ignored
- * @param bool $includeDeleted A flag indicating whether deleted records should be included
- * @return string
- * @todo remove after getUidOfAlreadyPersistedValueObject is adjusted, this was moved to queryParser
- */
- protected function getBackendConstraintStatement($tableName, $ignoreEnableFields, $includeDeleted)
- {
- $statement = '';
- if (!$ignoreEnableFields) {
- $statement .= BackendUtility::BEenableFields($tableName);
- }
- if (!$includeDeleted) {
- $statement .= BackendUtility::deleteClause($tableName);
+ } catch (DBALException $e) {
+ throw new SqlErrorException($e->getPrevious()->getMessage(), 1470231748);
}
- return $statement;
}
/**
diff --git a/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Storage/Typo3DbBackendTest.php b/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Storage/Typo3DbBackendTest.php
index d7fd8d5a554c..a961bf440442 100644
--- a/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Storage/Typo3DbBackendTest.php
+++ b/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Storage/Typo3DbBackendTest.php
@@ -14,11 +14,18 @@ namespace TYPO3\CMS\Extbase\Tests\Unit\Persistence\Generic\Storage;
* The TYPO3 project - inspiring people to share!
*/
+use Doctrine\DBAL\Driver\Statement;
+use Prophecy\Argument;
+use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder;
+use TYPO3\CMS\Core\Database\Query\QueryBuilder;
+use TYPO3\CMS\Core\Database\Query\Restriction\FrontendRestrictionContainer;
use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper;
use TYPO3\CMS\Extbase\Persistence\Generic\QuerySettingsInterface;
use TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbBackend;
use TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbQueryParser;
use TYPO3\CMS\Extbase\Persistence\QueryInterface;
+use TYPO3\CMS\Extbase\Service\EnvironmentService;
/**
* Test case
@@ -30,6 +37,13 @@ class Typo3DbBackendTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
*/
protected static $dataMapper;
+ public function setUp()
+ {
+ parent::setUp();
+ $GLOBALS['TSFE'] = new \stdClass();
+ $GLOBALS['TSFE']->gr_list = '';
+ }
+
/**
* Setup DataMapper
*/
@@ -38,16 +52,29 @@ class Typo3DbBackendTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
self::$dataMapper = new DataMapper();
}
+ /**
+ * @return array
+ */
+ public function uidOfAlreadyPersistedValueObjectIsDeterminedCorrectlyDataProvider(): array
+ {
+ return [
+ 'isFrontendEnvironment' => [true],
+ 'isBackendEnvironment' => [false],
+ ];
+ }
+
/**
* @test
+ * @dataProvider uidOfAlreadyPersistedValueObjectIsDeterminedCorrectlyDataProvider
*/
- public function uidOfAlreadyPersistedValueObjectIsDeterminedCorrectly()
+ public function uidOfAlreadyPersistedValueObjectIsDeterminedCorrectly(bool $isFrontendEnvironment)
{
$mockValueObject = $this->getMockBuilder(\TYPO3\CMS\Extbase\DomainObject\AbstractValueObject::class)
->setMethods(array('_getProperties'))
->disableOriginalConstructor()
->getMock();
- $mockValueObject->expects($this->once())->method('_getProperties')->will($this->returnValue(array('propertyName' => 'propertyValue')));
+ $mockValueObject->expects($this->once())->method('_getProperties')
+ ->will($this->returnValue(['propertyName' => 'propertyValue']));
$mockColumnMap = $this->getMockBuilder(\TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMap::class)
->setMethods(array('isPersistableProperty', 'getColumnName'))
->disableOriginalConstructor()
@@ -65,22 +92,44 @@ class Typo3DbBackendTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
->setMethods(array('getDataMap', 'getPlainValue'))
->disableOriginalConstructor()
->getMock();
- $mockDataMapper->expects($this->once())->method('getDataMap')->will($this->returnValue($mockDataMap));
- $mockDataMapper->expects($this->once())->method('getPlainValue')->will($this->returnValue('plainPropertyValue'));
- $expectedStatement = 'SELECT * FROM tx_foo_table WHERE column_name=?';
- $expectedParameters = array('plainPropertyValue');
+ $mockDataMapper->expects($this->once())->method('getDataMap')
+ ->will($this->returnValue($mockDataMap));
+ $mockDataMapper->expects($this->once())->method('getPlainValue')
+ ->will($this->returnValue('plainPropertyValue'));
$expectedUid = 52;
- $mockDataBaseHandle = $this->getMockBuilder(\TYPO3\CMS\Core\Database\DatabaseConnection::class)
- ->setMethods(array('sql_query', 'sql_fetch_assoc'))
- ->disableOriginalConstructor()
- ->getMock();
- $mockDataBaseHandle->expects($this->once())->method('sql_query')->will($this->returnValue('resource'));
- $mockDataBaseHandle->expects($this->any())->method('sql_fetch_assoc')->with('resource')->will($this->returnValue(array('uid' => $expectedUid)));
- $mockTypo3DbBackend = $this->getAccessibleMock(\TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbBackend::class, array('checkSqlErrors', 'replacePlaceholders', 'addVisibilityConstraintStatement'), array(), '', false);
- $mockTypo3DbBackend->expects($this->once())->method('addVisibilityConstraintStatement')->with($this->isInstanceOf(\TYPO3\CMS\Extbase\Persistence\Generic\QuerySettingsInterface::class), $tableName, $this->isType('array'));
- $mockTypo3DbBackend->expects($this->once())->method('replacePlaceholders')->with($expectedStatement, $expectedParameters);
+
+ $expressionBuilderProphet = $this->prophesize(ExpressionBuilder::class);
+ $expressionBuilderProphet->eq(Argument::cetera())->willReturn('1 = 1');
+ $queryResultProphet = $this->prophesize(Statement::class);
+ $queryResultProphet->fetchColumn(Argument::cetera())->willReturn($expectedUid);
+ $queryBuilderProphet = $this->prophesize(QueryBuilder::class);
+ $queryBuilderProphet->execute()->willReturn($queryResultProphet->reveal());
+ $queryBuilderProphet->expr()->willReturn($expressionBuilderProphet->reveal());
+ $queryBuilderProphet->createNamedParameter(Argument::cetera())->willReturnArgument(0);
+ $queryBuilderProphet->select('uid')->willReturn($queryBuilderProphet->reveal());
+ $queryBuilderProphet->from($tableName)->willReturn($queryBuilderProphet->reveal());
+ $queryBuilderProphet->where(Argument::cetera())->willReturn($queryBuilderProphet->reveal());
+ $connectionPoolProphet = $this->prophesize(ConnectionPool::class);
+ $connectionPoolProphet->getQueryBuilderForTable(Argument::cetera())->willReturn($queryBuilderProphet->reveal());
+
+ $environmentServiceProphet = $this->prophesize(EnvironmentService::class);
+ $environmentServiceProphet->isEnvironmentInFrontendMode()->willReturn($isFrontendEnvironment);
+
+ if ($isFrontendEnvironment) {
+ $queryBuilderProphet->setRestrictions(Argument::type(FrontendRestrictionContainer::class))
+ ->shouldBeCalled();
+ }
+
+ $mockTypo3DbBackend = $this->getAccessibleMock(
+ \TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbBackend::class,
+ ['dummy'],
+ [],
+ '',
+ false
+ );
$mockTypo3DbBackend->_set('dataMapper', $mockDataMapper);
- $mockTypo3DbBackend->_set('databaseHandle', $mockDataBaseHandle);
+ $mockTypo3DbBackend->_set('connectionPool', $connectionPoolProphet->reveal());
+ $mockTypo3DbBackend->_set('environmentService', $environmentServiceProphet->reveal());
$result = $mockTypo3DbBackend->_callRef('getUidOfAlreadyPersistedValueObject', $mockValueObject);
$this->assertSame($expectedUid, $result);
}
--
GitLab