[FEATURE] Cleanup buildQueryParameters hook 63/53263/10
authorFrank Naegler <frank.naegler@typo3.org>
Mon, 19 Jun 2017 20:09:20 +0000 (22:09 +0200)
committerChristian Kuhn <lolli@schwarzbu.ch>
Thu, 29 Jun 2017 11:59:56 +0000 (13:59 +0200)
Add the QueryBuilder object to list module buildQueryParameters
hook allowing direct operation on this object including proper
named parameter usage and quoting.
The old array/string based parameter handling still works but
is deprecated.

Resolves: #81651
Releases: master
Change-Id: Ib17ba9383c29b5b48540203e6952b9670c6244f3
Reviewed-on: https://review.typo3.org/53263
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Reviewed-by: Henrik Elsner <helsner@dfau.de>
Tested-by: Henrik Elsner <helsner@dfau.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
typo3/sysext/core/Documentation/Changelog/master/Deprecation-81651-ArgumentParametersInListModuleHook.rst [new file with mode: 0644]
typo3/sysext/core/Documentation/Changelog/master/Feature-81651-QueryBuilderObjectAsArgumentInListModuleHook.rst [new file with mode: 0644]
typo3/sysext/recordlist/Classes/RecordList/AbstractDatabaseRecordList.php

diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-81651-ArgumentParametersInListModuleHook.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-81651-ArgumentParametersInListModuleHook.rst
new file mode 100644 (file)
index 0000000..7362d6b
--- /dev/null
@@ -0,0 +1,34 @@
+.. include:: ../../Includes.txt
+
+=============================================================
+Deprecation: #81651 - Argument parameters in list module hook
+=============================================================
+
+See :issue:`81651`
+
+Description
+===========
+
+The parameter array :php:`$parameters` of :php:`$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS'][DatabaseRecordList::class]['buildQueryParameters']`
+has been marked as deprecated.
+
+
+Impact
+======
+
+Changing the array content array within a hook triggers a deprecation log entry.
+
+
+Affected Installations
+======================
+
+Any installation using third-party extension that use this array to modify the query.
+
+
+Migration
+=========
+
+Use new argument :php:`$queryBuilder` that hands over the query builder instance
+to modify the list module query.
+
+.. index:: Backend, Database, PHP-API
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Feature-81651-QueryBuilderObjectAsArgumentInListModuleHook.rst b/typo3/sysext/core/Documentation/Changelog/master/Feature-81651-QueryBuilderObjectAsArgumentInListModuleHook.rst
new file mode 100644 (file)
index 0000000..e82f660
--- /dev/null
@@ -0,0 +1,24 @@
+.. include:: ../../Includes.txt
+
+======================================================================
+Feature: #81651 - Query builder object as argument in list module hook
+======================================================================
+
+See :issue:`81651`
+
+Description
+===========
+
+A new parameter :php:`$queryBuilder` has been added to
+:php:`$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS'][DatabaseRecordList::class]['buildQueryParameters']` hook.
+The QueryBuilder object can be used to modify the main list module query.
+The QueryBuilder instance is passed by reference, it allows any query modification.
+
+
+Impact
+======
+
+The old :php:`$parameters` array has been marked as deprecated.
+Existing hooks must be adjusted.
+
+.. index:: Backend, Database, PHP-API
index aef9c35..2436281 100644 (file)
@@ -20,6 +20,7 @@ use TYPO3\CMS\Backend\Routing\UriBuilder;
 use TYPO3\CMS\Backend\Tree\View\PageTreeView;
 use TYPO3\CMS\Backend\Utility\BackendUtility;
 use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
+use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\QueryBuilder;
 use TYPO3\CMS\Core\Database\Query\QueryHelper;
@@ -500,9 +501,9 @@ class AbstractDatabaseRecordList extends AbstractRecordList
                     ->removeAll()
                     ->add(GeneralUtility::makeInstance(DeletedRestriction::class))
                     ->add(GeneralUtility::makeInstance(BackendWorkspaceRestriction::class));
+                $queryBuilder = $this->addPageIdConstraint($tableName, $queryBuilder);
                 $firstRow = $queryBuilder->select('uid')
                     ->from($tableName)
-                    ->where($this->getPageIdConstraint($tableName))
                     ->execute()
                     ->fetch();
                 if (!is_array($firstRow)) {
@@ -674,100 +675,90 @@ class AbstractDatabaseRecordList extends AbstractRecordList
         array $additionalConstraints = [],
         array $fields = ['*']
     ): QueryBuilder {
-        $queryParameters = $this->buildQueryParameters($table, $pageId, $fields, $additionalConstraints);
-
         $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
-            ->getQueryBuilderForTable($queryParameters['table']);
+            ->getQueryBuilderForTable($table);
         $queryBuilder->getRestrictions()
             ->removeAll()
             ->add(GeneralUtility::makeInstance(DeletedRestriction::class))
             ->add(GeneralUtility::makeInstance(BackendWorkspaceRestriction::class));
         $queryBuilder
-            ->select(...$queryParameters['fields'])
-            ->from($queryParameters['table'])
-            ->where(...$queryParameters['where']);
+            ->select(...$fields)
+            ->from($table);
 
-        if (!empty($queryParameters['orderBy'])) {
-            foreach ($queryParameters['orderBy'] as $fieldNameAndSorting) {
-                list($fieldName, $sorting) = $fieldNameAndSorting;
-                $queryBuilder->addOrderBy($fieldName, $sorting);
-            }
+        if (!empty($additionalConstraints)) {
+            $queryBuilder->andWhere(...$additionalConstraints);
         }
 
-        if (!empty($queryParameters['firstResult'])) {
-            $queryBuilder->setFirstResult((int)$queryParameters['firstResult']);
-        }
-
-        if (!empty($queryParameters['maxResults'])) {
-            $queryBuilder->setMaxResults((int)$queryParameters['maxResults']);
-        }
-
-        if (!empty($queryParameters['groupBy'])) {
-            $queryBuilder->groupBy($queryParameters['groupBy']);
-        }
+        $queryBuilder = $this->prepareQueryBuilder($table, $pageId, $fields, $additionalConstraints, $queryBuilder);
 
         return $queryBuilder;
     }
 
     /**
-     * Return the query parameters to select the records from a table $table with pid = $this->pidList
+     * Return the modified QueryBuilder object ($queryBuilder) which will be
+     * used to select the records from a table $table with pid = $this->pidList
      *
      * @param string $table Table name
      * @param int $pageId Page id Only used to build the search constraints, $this->pidList is used for restrictions
      * @param string[] $fieldList List of fields to select from the table
      * @param string[] $additionalConstraints Additional part for where clause
-     * @return array
+     * @param QueryBuilder $queryBuilder
+     * @return QueryBuilder
      */
-    protected function buildQueryParameters(
+    protected function prepareQueryBuilder(
         string $table,
         int $pageId,
         array $fieldList = ['*'],
-        array $additionalConstraints = []
-    ): array {
-        $expressionBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
-            ->getQueryBuilderForTable($table)
-            ->expr();
-
+        array $additionalConstraints = [],
+        QueryBuilder $queryBuilder
+    ): QueryBuilder {
         $parameters = [
             'table' => $table,
             'fields' => $fieldList,
             'groupBy' => null,
             'orderBy' => null,
             'firstResult' => $this->firstElementNumber ?: null,
-            'maxResults' => $this->iLimit ? $this->iLimit : null,
+            'maxResults' => $this->iLimit ?: null
         ];
 
+        if ($this->iLimit !== null) {
+            $queryBuilder->setMaxResults($this->iLimit);
+        }
+
         if ($this->sortField && in_array($this->sortField, $this->makeFieldList($table, 1))) {
-            $parameters['orderBy'][] = $this->sortRev ? [$this->sortField, 'DESC'] : [$this->sortField, 'ASC'];
+            $queryBuilder->orderBy($this->sortField, $this->sortRev ? 'DESC' : 'ASC');
         } else {
             $orderBy = $GLOBALS['TCA'][$table]['ctrl']['sortby'] ?: $GLOBALS['TCA'][$table]['ctrl']['default_sortby'];
-            $parameters['orderBy'] = QueryHelper::parseOrderBy((string)$orderBy);
+            $orderBys = QueryHelper::parseOrderBy((string)$orderBy);
+            foreach ($orderBys as $orderBy) {
+                $queryBuilder->orderBy($orderBy[0], $orderBy[1]);
+            }
         }
 
         // Build the query constraints
-        $constraints = [
-            'pidSelect' => $this->getPageIdConstraint($table),
-            'search' => $this->makeSearchString($table, $pageId)
-        ];
+        $queryBuilder = $this->addPageIdConstraint($table, $queryBuilder);
+        $searchWhere = $this->makeSearchString($table, $pageId);
+        if (!empty($searchWhere)) {
+            $queryBuilder->andWhere($searchWhere);
+        }
 
         // Filtering on displayable pages (permissions):
         if ($table === 'pages' && $this->perms_clause) {
-            $constraints['pagePermsClause'] = $this->perms_clause;
+            $queryBuilder->andWhere($this->perms_clause);
         }
 
         // Filter out records that are translated, if TSconfig mod.web_list.hideTranslations is set
-        if ((GeneralUtility::inList($this->hideTranslations, $table) || $this->hideTranslations === '*')
+        if (
+            $table !== 'pages_language_overlay'
             && !empty($GLOBALS['TCA'][$table]['ctrl']['transOrigPointerField'])
-            && $table !== 'pages_language_overlay'
+            && (GeneralUtility::inList($this->hideTranslations, $table) || $this->hideTranslations === '*')
         ) {
-            $constraints['transOrigPointerField'] = $expressionBuilder->eq(
+            $queryBuilder->andWhere($queryBuilder->expr()->eq(
                 $GLOBALS['TCA'][$table]['ctrl']['transOrigPointerField'],
                 0
-            );
+            ));
         }
 
-        $parameters['where'] = array_merge($constraints, $additionalConstraints);
-
         $hookName = DatabaseRecordList::class;
         if (is_array($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS'][$hookName]['buildQueryParameters'])) {
             foreach ($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS'][$hookName]['buildQueryParameters'] as $className) {
@@ -779,7 +770,8 @@ class AbstractDatabaseRecordList extends AbstractRecordList
                         $pageId,
                         $additionalConstraints,
                         $fieldList,
-                        $this
+                        $this,
+                        $queryBuilder
                     );
                 }
             }
@@ -788,13 +780,40 @@ class AbstractDatabaseRecordList extends AbstractRecordList
         // array_unique / array_filter used to eliminate empty and duplicate constraints
         // the array keys are eliminated by this as well to facilitate argument unpacking
         // when used with the querybuilder.
-        $parameters['where'] = array_unique(array_filter(array_values($parameters['where'])));
+        // @deprecated since TYPO3 v9, will be removed in TYPO3 v10
+        if (!empty($parameters['where'])) {
+            $parameters['where'] = array_unique(array_filter(array_values($parameters['where'])));
+        }
+        if (!empty($parameters['where'])) {
+            $this->logDeprecation('where');
+            $queryBuilder->where(...$parameters['where']);
+        }
+        if (!empty($parameters['orderBy'])) {
+            $this->logDeprecation('orderBy');
+            foreach ($parameters['orderBy'] as $fieldNameAndSorting) {
+                list($fieldName, $sorting) = $fieldNameAndSorting;
+                $queryBuilder->addOrderBy($fieldName, $sorting);
+            }
+        }
+        if (!empty($parameters['firstResult'])) {
+            $this->logDeprecation('firstResult');
+            $queryBuilder->setFirstResult((int)$parameters['firstResult']);
+        }
+        if (!empty($parameters['maxResults']) && $parameters['maxResults'] !== $this->iLimit) {
+            $this->logDeprecation('maxResults');
+            $queryBuilder->setMaxResults((int)$parameters['maxResults']);
+        }
+        if (!empty($parameters['groupBy'])) {
+            $this->logDeprecation('groupBy');
+            $queryBuilder->groupBy($parameters['groupBy']);
+        }
 
-        return $parameters;
+        return $queryBuilder;
     }
 
     /**
-     * Set the total items for the record list
+     * Executed a query to set $this->totalItems to the number of total
+     * items, eg. for pagination
      *
      * @param string $table Table name
      * @param int $pageId Only used to build the search constraints, $this->pidList is used for restrictions
@@ -802,16 +821,21 @@ class AbstractDatabaseRecordList extends AbstractRecordList
      */
     public function setTotalItems(string $table, int $pageId, array $constraints)
     {
-        $queryParameters = $this->buildQueryParameters($table, $pageId, ['*'], $constraints);
         $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
-            ->getQueryBuilderForTable($queryParameters['table']);
+            ->getQueryBuilderForTable($table);
+
         $queryBuilder->getRestrictions()
             ->removeAll()
             ->add(GeneralUtility::makeInstance(DeletedRestriction::class))
             ->add(GeneralUtility::makeInstance(BackendWorkspaceRestriction::class));
         $queryBuilder
-            ->from($queryParameters['table'])
-            ->where(...$queryParameters['where']);
+            ->from($table);
+
+        if (!empty($constraints)) {
+            $queryBuilder->andWhere(...$constraints);
+        }
+
+        $queryBuilder = $this->prepareQueryBuilder($table, $pageId, ['*'], $constraints, $queryBuilder);
 
         $this->totalItems = (int)$queryBuilder->count('*')
             ->execute()
@@ -1114,7 +1138,6 @@ class AbstractDatabaseRecordList extends AbstractRecordList
 
     /**
      * Returns "requestUri" - which is basically listURL
-     *
      * @return string Content of ->listURL()
      */
     public function requestUri()
@@ -1320,13 +1343,14 @@ class AbstractDatabaseRecordList extends AbstractRecordList
     }
 
     /**
-     * Build SQL fragment to limit a query to a list of page IDs based on
-     * the current search level setting.
+     * Add conditions to the QueryBuilder object ($queryBuilder) to limit a
+     * query to a list of page IDs based on the current search level setting.
      *
      * @param string $tableName
-     * @return string
+     * @param QueryBuilder $queryBuilder
+     * @return QueryBuilder Modified QueryBuilder object
      */
-    protected function getPageIdConstraint(string $tableName): string
+    protected function addPageIdConstraint(string $tableName, QueryBuilder $queryBuilder): QueryBuilder
     {
         // Set search levels:
         $searchLevels = $this->searchLevels;
@@ -1337,28 +1361,44 @@ class AbstractDatabaseRecordList extends AbstractRecordList
             $searchLevels = 999;
         }
 
-        // Default is to search everywhere
-        $constraint = '1=1';
-
-        $expressionBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
-            ->getConnectionForTable($tableName)
-            ->getExpressionBuilder();
-
         if ($searchLevels === 0) {
-            $constraint = $expressionBuilder->eq($tableName . '.pid', (int)$this->id);
+            $queryBuilder->andWhere(
+                $queryBuilder->expr()->eq(
+                    $tableName . '.pid',
+                    $queryBuilder->createNamedParameter($this->id, \PDO::PARAM_INT)
+                )
+            );
         } elseif ($searchLevels > 0) {
             $allowedMounts = $this->getSearchableWebmounts($this->id, $searchLevels, $this->perms_clause);
-            $constraint = $expressionBuilder->in($tableName . '.pid', array_map('intval', $allowedMounts));
+            $queryBuilder->andWhere(
+                $queryBuilder->expr()->in(
+                    $tableName . '.pid',
+                    $queryBuilder->createNamedParameter($allowedMounts, Connection::PARAM_INT_ARRAY)
+                )
+            );
         }
 
         if (!empty($this->getOverridePageIdList())) {
-            $constraint = $expressionBuilder->in(
-                $tableName . '.pid',
-                $this->getOverridePageIdList()
+            $queryBuilder->andWhere(
+                $queryBuilder->expr()->in(
+                    $tableName . '.pid',
+                    $queryBuilder->createNamedParameter($this->getOverridePageIdList(), Connection::PARAM_INT_ARRAY)
+                )
             );
         }
 
-        return (string)$constraint;
+        return $queryBuilder;
+    }
+
+    /**
+     * Method used to log deprecated usage of old buildQueryParametersPostProcess hook arguments
+     *
+     * @param string $index
+     * @deprecated since TYPO3 v9, will be removed in TYPO3 v10 - see method usages
+     */
+    protected function logDeprecation(string $index)
+    {
+        GeneralUtility::deprecationLog('[index: ' . $index . '] $parameters in "buildQueryParameters"-Hook has been deprecated in v9 and will be remove in v10, use $queryBuilder instead');
     }
 
     /**