[CLEANUP] Adjust usage of ContentObjectRenderer->getTreeList() 92/23092/10
authorAnja Leichsenring <aleichsenring@ab-softlab.de>
Wed, 14 Aug 2013 09:19:44 +0000 (11:19 +0200)
committerChristian Kuhn <lolli@schwarzbu.ch>
Thu, 29 Aug 2013 13:28:11 +0000 (15:28 +0200)
In ContentObjectRenderer->getQuery() the pid where clause is build
using getTreeList() to fetch recursive pids. But for the inclusion
of the uppermost pid, string concatenation is used.
The method getTreeList() offers the option to include the uppermost
pid into the returned result by passing it as a negative value. Method
getQuery() should use this opportunity instead of doing its own magic.

Additionally some cleanup is done for getTreeList, optimizing the
returned string, and unittests were added.

Resolves: #51067
Resolves: #51503
Releases: 6.2
Change-Id: Ie72103ca66d847cb0eb0f3ef5e33a1896d86de5b
Reviewed-on: https://review.typo3.org/23092
Reviewed-by: Kai Ole Hartwig
Tested-by: Kai Ole Hartwig
Reviewed-by: Christian Kuhn
Tested-by: Christian Kuhn
NEWS.txt
typo3/sysext/extbase/Classes/Configuration/FrontendConfigurationManager.php
typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
typo3/sysext/indexed_search/Classes/Controller/SearchFormController.php
typo3/sysext/indexed_search/Classes/Domain/Repository/IndexSearchRepository.php
typo3/sysext/indexed_search/Classes/Hook/MysqlFulltextIndexHook.php

index 33e4c84..ac01349 100644 (file)
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -36,14 +36,33 @@ General
 Logging
 -------------------------------------------------------------------------------
 
-* The logger of the Logging API now complies with the PSR-3 standard of the
-  PHP Framework Interop Group: http://www.php-fig.org/psr/3/
+* Logging API PSR-3 compliance
+
+The logger of the Logging API now complies with the PSR-3 standard of the
+PHP Framework Interop Group: http://www.php-fig.org/psr/3/
 
 -------------------------------------------------------------------------------
 Backend
 -------------------------------------------------------------------------------
 
 -------------------------------------------------------------------------------
+Frontend
+-------------------------------------------------------------------------------
+
+* Minor API change in \TYPO3\CMS\Frontend\ContentObjectRenderer->getTreeList()
+
+getTreeList() got some cleanup and slightly changed its return result. Former
+versions sometimes returned a trailing comma which is not the case anymore.
+
+Before:
+getTreeList(42, 4) // get pids for pageId 42, 4 levels deep
+result: '0, 22, 11, 4,'
+
+After:
+getTreeList(42, 4)
+result: '0, 22, 11, 4'
+
+-------------------------------------------------------------------------------
 Administration / Customization
 -------------------------------------------------------------------------------
 
index debded9..81c3755 100644 (file)
@@ -120,7 +120,7 @@ class FrontendConfigurationManager extends \TYPO3\CMS\Extbase\Configuration\Abst
                        if ($this->contentObject->data['recursive'] > 0) {
                                $explodedPages = \TYPO3\CMS\Core\Utility\GeneralUtility::trimExplode(',', $pages);
                                foreach ($explodedPages as $pid) {
-                                       $list[] = trim($this->contentObject->getTreeList($pid, $this->contentObject->data['recursive']), ',');
+                                       $list[] = $this->contentObject->getTreeList($pid, $this->contentObject->data['recursive']);
                                }
                        }
                        if (count($list) > 0) {
index 6fc75eb..55f16ea 100644 (file)
@@ -7209,69 +7209,87 @@ class ContentObjectRenderer {
         * @param string $moreWhereClauses Additional where clauses. Syntax: " AND [fieldname]=[value] AND ...
         * @param array $prevId_array array of IDs from previous recursions. In order to prevent infinite loops with mount pages.
         * @param integer $recursionLevel Internal: Zero for the first recursion, incremented for each recursive call.
-        * @return string Returns the list with a comma in the end (if any pages selected and not if $id is negative and $id is added itself) - which means the input page id can comfortably be appended to the output string if you need it to.
+        * @return string Returns the list of ids as a comma seperated string
         * @see \TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController::checkEnableFields(), \TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController::checkPagerecordForIncludeSection()
         */
        public function getTreeList($id, $depth, $begin = 0, $dontCheckEnableFields = FALSE, $addSelectFields = '', $moreWhereClauses = '', array $prevId_array = array(), $recursionLevel = 0) {
+               $id = intval($id);
+               if (!$id) {
+                       return '';
+               }
+
                // Init vars:
                $allFields = 'uid,hidden,starttime,endtime,fe_group,extendToSubpages,doktype,php_tree_stop,mount_pid,mount_pid_ol,t3ver_state' . $addSelectFields;
                $depth = intval($depth);
                $begin = intval($begin);
-               $id = intval($id);
-               $theList = '';
+               $theList = array();
                $addId = 0;
                $requestHash = '';
-               if ($id) {
-                       // First level, check id (second level, this is done BEFORE the recursive call)
-                       if (!$recursionLevel) {
-                               // Check tree list cache
-                               // First, create the hash for this request - not sure yet whether we need all these parameters though
-                               $parameters = array(
-                                       $id,
-                                       $depth,
-                                       $begin,
-                                       $dontCheckEnableFields,
-                                       $addSelectFields,
-                                       $moreWhereClauses,
-                                       $prevId_array,
-                                       $GLOBALS['TSFE']->gr_list
-                               );
-                               $requestHash = md5(serialize($parameters));
-                               $cacheEntry = $GLOBALS['TYPO3_DB']->exec_SELECTgetSingleRow('treelist', 'cache_treelist', 'md5hash = \'' . $requestHash . '\' AND ( expires > ' . $GLOBALS['EXEC_TIME'] . ' OR expires = 0 )');
-                               if (is_array($cacheEntry)) {
-                                       // Cache hit
-                                       return $cacheEntry['treelist'];
-                               }
-                               // If Id less than zero it means we should add the real id to list:
-                               if ($id < 0) {
-                                       $addId = ($id = abs($id));
-                               }
-                               // Check start page:
-                               if ($GLOBALS['TSFE']->sys_page->getRawRecord('pages', $id, 'uid')) {
-                                       // Find mount point if any:
-                                       $mount_info = $GLOBALS['TSFE']->sys_page->getMountPointInfo($id);
-                                       if (is_array($mount_info)) {
-                                               $id = $mount_info['mount_pid'];
-                                               // In Overlay mode, use the mounted page uid as added ID!:
-                                               if ($addId && $mount_info['overlay']) {
-                                                       $addId = $id;
-                                               }
+
+               // First level, check id (second level, this is done BEFORE the recursive call)
+               if (!$recursionLevel) {
+                       // Check tree list cache
+                       // First, create the hash for this request - not sure yet whether we need all these parameters though
+                       $parameters = array(
+                               $id,
+                               $depth,
+                               $begin,
+                               $dontCheckEnableFields,
+                               $addSelectFields,
+                               $moreWhereClauses,
+                               $prevId_array,
+                               $GLOBALS['TSFE']->gr_list
+                       );
+                       $requestHash = md5(serialize($parameters));
+                       $cacheEntry = $GLOBALS['TYPO3_DB']->exec_SELECTgetSingleRow(
+                               'treelist',
+                               'cache_treelist',
+                               'md5hash = \'' . $requestHash . '\' AND ( expires > ' . $GLOBALS['EXEC_TIME'] . ' OR expires = 0 )'
+                       );
+                       if (is_array($cacheEntry)) {
+                               // Cache hit
+                               return $cacheEntry['treelist'];
+                       }
+                       // If Id less than zero it means we should add the real id to list:
+                       if ($id < 0) {
+                               $addId = ($id = abs($id));
+                       }
+                       // Check start page:
+                       if ($GLOBALS['TSFE']->sys_page->getRawRecord('pages', $id, 'uid')) {
+                               // Find mount point if any:
+                               $mount_info = $GLOBALS['TSFE']->sys_page->getMountPointInfo($id);
+                               if (is_array($mount_info)) {
+                                       $id = $mount_info['mount_pid'];
+                                       // In Overlay mode, use the mounted page uid as added ID!:
+                                       if ($addId && $mount_info['overlay']) {
+                                               $addId = $id;
                                        }
-                               } else {
-                                       // Return blank if the start page was NOT found at all!
-                                       return '';
                                }
-                       }
-                       // Add this ID to the array of IDs
-                       if ($begin <= 0) {
-                               $prevId_array[] = $id;
-                       }
-                       // Select sublevel:
-                       if ($depth > 0) {
-                               $res = $GLOBALS['TYPO3_DB']->exec_SELECTquery($allFields, 'pages', 'pid = ' . intval($id) . ' AND deleted = 0 ' . $moreWhereClauses, '', 'sorting');
-                               while ($row = $GLOBALS['TYPO3_DB']->sql_fetch_assoc($res)) {
+                       } else {
+                               // Return blank if the start page was NOT found at all!
+                               return '';
+                       }
+               }
+               // Add this ID to the array of IDs
+               if ($begin <= 0) {
+                       $prevId_array[] = $id;
+               }
+               // Select sublevel:
+               if ($depth > 0) {
+                       $rows = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows(
+                               $allFields,
+                               'pages',
+                               'pid = ' . intval($id) . ' AND deleted = 0 ' . $moreWhereClauses,
+                               '',
+                               'sorting'
+                       );
+                       if (is_array($rows)) {
+                               foreach ($rows as $row) {
                                        $GLOBALS['TSFE']->sys_page->versionOL('pages', $row);
-                                       if ($row['doktype'] == \TYPO3\CMS\Frontend\Page\PageRepository::DOKTYPE_RECYCLER || $row['doktype'] == \TYPO3\CMS\Frontend\Page\PageRepository::DOKTYPE_BE_USER_SECTION || $row['t3ver_state'] > 0) {
+                                       if ($row['doktype'] == \TYPO3\CMS\Frontend\Page\PageRepository::DOKTYPE_RECYCLER
+                                               || $row['doktype'] == \TYPO3\CMS\Frontend\Page\PageRepository::DOKTYPE_BE_USER_SECTION
+                                               || $row['t3ver_state'] > 0
+                                       ) {
                                                // Doing this after the overlay to make sure changes
                                                // in the overlay are respected.
                                                // However, we do not process pages below of and
@@ -7284,11 +7302,18 @@ class ContentObjectRenderer {
                                        // Overlay mode:
                                        if (is_array($mount_info) && $mount_info['overlay']) {
                                                $next_id = $mount_info['mount_pid'];
-                                               $res2 = $GLOBALS['TYPO3_DB']->exec_SELECTquery($allFields, 'pages', 'uid = ' . intval($next_id) . ' AND deleted = 0 ' . $moreWhereClauses, '', 'sorting');
-                                               $row = $GLOBALS['TYPO3_DB']->sql_fetch_assoc($res2);
-                                               $GLOBALS['TYPO3_DB']->sql_free_result($res2);
+                                               $row = $GLOBALS['TYPO3_DB']->exec_SELECTgetSingleRow(
+                                                       $allFields,
+                                                       'pages',
+                                                       'uid = ' . intval($next_id) . ' AND deleted = 0 ' . $moreWhereClauses,
+                                                       '',
+                                                       'sorting'
+                                               );
                                                $GLOBALS['TSFE']->sys_page->versionOL('pages', $row);
-                                               if ($row['doktype'] == \TYPO3\CMS\Frontend\Page\PageRepository::DOKTYPE_RECYCLER || $row['doktype'] == \TYPO3\CMS\Frontend\Page\PageRepository::DOKTYPE_BE_USER_SECTION || $row['t3ver_state'] > 0) {
+                                               if ($row['doktype'] == \TYPO3\CMS\Frontend\Page\PageRepository::DOKTYPE_RECYCLER
+                                                       || $row['doktype'] == \TYPO3\CMS\Frontend\Page\PageRepository::DOKTYPE_BE_USER_SECTION
+                                                       || $row['t3ver_state'] > 0
+                                               ) {
                                                        // Doing this after the overlay to make sure
                                                        // changes in the overlay are respected.
                                                        // see above
@@ -7300,7 +7325,7 @@ class ContentObjectRenderer {
                                                // Add ID to list:
                                                if ($begin <= 0) {
                                                        if ($dontCheckEnableFields || $GLOBALS['TSFE']->checkEnableFields($row)) {
-                                                               $theList .= $next_id . ',';
+                                                               $theList[] = $next_id;
                                                        }
                                                }
                                                // Next level:
@@ -7311,32 +7336,38 @@ class ContentObjectRenderer {
                                                        }
                                                        // Call recursively, if the id is not in prevID_array:
                                                        if (!in_array($next_id, $prevId_array)) {
-                                                               $theList .= self::getTreeList($next_id, $depth - 1, $begin - 1, $dontCheckEnableFields, $addSelectFields, $moreWhereClauses, $prevId_array, $recursionLevel + 1);
+                                                               $theList = array_merge(
+                                                                       GeneralUtility::intExplode(
+                                                                               ',',
+                                                                               self::getTreeList($next_id, $depth - 1, $begin - 1, $dontCheckEnableFields,
+                                                                                       $addSelectFields, $moreWhereClauses, $prevId_array, $recursionLevel + 1)
+                                                                       ),
+                                                                       $theList
+                                                               );
                                                        }
                                                }
                                        }
                                }
-                               $GLOBALS['TYPO3_DB']->sql_free_result($res);
                        }
-                       // If first run, check if the ID should be returned:
-                       if (!$recursionLevel) {
-                               if ($addId) {
-                                       if ($begin > 0) {
-                                               $theList .= 0;
-                                       } else {
-                                               $theList .= $addId;
-                                       }
+               }
+               // If first run, check if the ID should be returned:
+               if (!$recursionLevel) {
+                       if ($addId) {
+                               if ($begin > 0) {
+                                       $theList[] = 0;
+                               } else {
+                                       $theList[] = $addId;
                                }
-                               $GLOBALS['TYPO3_DB']->exec_INSERTquery('cache_treelist', array(
-                                       'md5hash' => $requestHash,
-                                       'pid' => $id,
-                                       'treelist' => $theList,
-                                       'tstamp' => $GLOBALS['EXEC_TIME']
-                               ));
                        }
+                       $GLOBALS['TYPO3_DB']->exec_INSERTquery('cache_treelist', array(
+                               'md5hash' => $requestHash,
+                               'pid' => $id,
+                               'treelist' => implode(',', $theList),
+                               'tstamp' => $GLOBALS['EXEC_TIME']
+                       ));
                }
-               // Return list:
-               return $theList;
+
+               return implode(',', $theList);
        }
 
        /**
@@ -7491,14 +7522,25 @@ class ContentObjectRenderer {
                if (isset($conf['recursive'])) {
                        $conf['recursive'] = intval($conf['recursive']);
                        if ($conf['recursive'] > 0) {
-                               $pidList = '';
-                               foreach (explode(',', $conf['pidInList']) as $value) {
-                                       if ($value === 'this') {
-                                               $value = $GLOBALS['TSFE']->id;
+                               $pidList = GeneralUtility::trimExplode(',', $conf['pidInList'], TRUE);
+                               array_walk($pidList, function (&$storagePid) {
+                                       if ($storagePid === 'this') {
+                                               $storagePid = $GLOBALS['TSFE']->id;
+                                       }
+                                       if ($storagePid > 0) {
+                                               $storagePid = -$storagePid;
                                        }
-                                       $pidList .= $value . ',' . $this->getTreeList($value, $conf['recursive']);
+                               });
+                               $expandedPidList = array();
+                               foreach ($pidList as $value) {
+                                       // Implementation of getTreeList allows to pass the id negative to include
+                                       // it into the result otherwise only childpages are returned
+                                       $expandedPidList = array_merge(
+                                               GeneralUtility::intExplode(',', $this->getTreeList($value, $conf['recursive'])),
+                                               $expandedPidList
+                                       );
                                }
-                               $conf['pidInList'] = trim($pidList, ',');
+                               $conf['pidInList'] = implode(',', $expandedPidList);
                        }
                }
                if (!strcmp($conf['pidInList'], '')) {
index 13b6727..0221d50 100644 (file)
@@ -50,6 +50,7 @@ class ContentObjectRendererTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
         * Set up
         */
        public function setUp() {
+               $GLOBALS['TYPO3_DB'] = $this->getMock('TYPO3\\CMS\\Core\\Database\\DatabaseConnection', array());
                $this->template = $this->getMock('TYPO3\\CMS\\Core\\TypoScript\\TemplateService', array('getFileName', 'linkData'));
                $this->tsfe = $this->getAccessibleMock('TYPO3\\CMS\\Frontend\\Controller\\TypoScriptFrontendController', array('dummy'), array(), '', FALSE);
                $this->tsfe->tmpl = $this->template;
@@ -1225,6 +1226,42 @@ class ContentObjectRendererTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
        }
 
        /**
+        * @test
+        */
+       public function getQueryCallsGetTreeListWithNegativeValuesIfRecursiveIsSet() {
+               $this->cObj = $this->getAccessibleMock('\\TYPO3\\CMS\\Frontend\\ContentObject\\ContentObjectRenderer', array('getTreeList'));
+               $this->cObj->start(array(), 'tt_content');
+               $conf = array(
+                       'recursive' => '15',
+                       'pidInList' => '16, -35'
+               );
+               $this->cObj->expects($this->at(0))
+                       ->method('getTreeList')
+                       ->with(-16, 15);
+               $this->cObj->expects($this->at(1))
+                       ->method('getTreeList')
+                       ->with(-35, 15);
+               $this->cObj->getQuery('tt_content', $conf, TRUE);
+       }
+
+       /**
+        * @test
+        */
+       public function getQueryCallsGetTreeListWithCurrentPageIfThisIsSet() {
+               $this->cObj = $this->getAccessibleMock('\\TYPO3\\CMS\\Frontend\\ContentObject\\ContentObjectRenderer', array('getTreeList'));
+               $GLOBALS['TSFE']->id = 27;
+               $this->cObj->start(array(), 'tt_content');
+               $conf = array(
+                       'pidInList' => 'this',
+                       'recursive' => '4'
+               );
+               $this->cObj->expects($this->once())
+                       ->method('getTreeList')
+                       ->with(-27);
+               $this->cObj->getQuery('tt_content', $conf, TRUE);
+       }
+
+       /**
         * Data provider for the stdWrap_strftime test
         *
         * @return array multi-dimensional array with the second level like this:
@@ -1853,6 +1890,88 @@ class ContentObjectRendererTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
 
                $this->assertEquals($expectedResult, $cleanedResult);
        }
+
+       /**
+        * @test
+        */
+       public function getTreeListReturnsChildPageUids() {
+               $GLOBALS['TYPO3_DB']->expects($this->any())->method('exec_SELECTgetSingleRow')->with('treelist')->will($this->returnValue(NULL));
+               $GLOBALS['TSFE']->sys_page
+                       ->expects($this->any())
+                       ->method('getRawRecord')
+                       ->will(
+                               $this->onConsecutiveCalls(
+                                       array('uid' => 17),
+                                       array('uid' => 321),
+                                       array('uid' => 719),
+                                       array('uid' => 42)
+                               )
+                       );
+
+               $GLOBALS['TSFE']->sys_page->expects($this->any())->method('getMountPointInfo')->will($this->returnValue(NULL));
+               $GLOBALS['TYPO3_DB']
+                       ->expects($this->any())
+                       ->method('exec_SELECTgetRows')
+                       ->will(
+                               $this->onConsecutiveCalls(
+                                       array(
+                                               array('uid' => 321)
+                                       ),
+                                       array(
+                                               array('uid' => 719)
+                                       ),
+                                       array(
+                                               array('uid' => 42)
+                                       )
+                               )
+                       );
+               // 17 = pageId, 5 = recursionLevel, 0 = begin (entry to recursion, internal), TRUE = do not check enable fields
+               // 17 is positive, we expect 17 NOT to be included in result
+               $result = $this->cObj->getTreeList(17, 5, 0, TRUE);
+               $expectedResult = '0,42,719,321';
+               $this->assertEquals($expectedResult, $result);
+       }
+
+       /**
+        * @test
+        */
+       public function getTreeListReturnsChildPageUidsAndOriginalPidForNegativeValue() {
+               $GLOBALS['TYPO3_DB']->expects($this->any())->method('exec_SELECTgetSingleRow')->with('treelist')->will($this->returnValue(NULL));
+               $GLOBALS['TSFE']->sys_page
+                       ->expects($this->any())
+                       ->method('getRawRecord')
+                       ->will(
+                               $this->onConsecutiveCalls(
+                                       array('uid' => 17),
+                                       array('uid' => 321),
+                                       array('uid' => 719),
+                                       array('uid' => 42)
+                               )
+                       );
+
+               $GLOBALS['TSFE']->sys_page->expects($this->any())->method('getMountPointInfo')->will($this->returnValue(NULL));
+               $GLOBALS['TYPO3_DB']
+                       ->expects($this->any())
+                       ->method('exec_SELECTgetRows')
+                       ->will(
+                               $this->onConsecutiveCalls(
+                                       array(
+                                               array('uid' => 321)
+                                       ),
+                                       array(
+                                               array('uid' => 719)
+                                       ),
+                                       array(
+                                               array('uid' => 42)
+                                       )
+                               )
+                       );
+               // 17 = pageId, 5 = recursionLevel, 0 = begin (entry to recursion, internal), TRUE = do not check enable fields
+               // 17 is negative, we expect 17 to be included in result
+               $result = $this->cObj->getTreeList(-17, 5, 0, TRUE);
+               $expectedResult = '0,42,719,321,17';
+               $this->assertEquals($expectedResult, $result);
+       }
 }
 
 ?>
index 2741524..0e7a9b9 100644 (file)
@@ -1113,7 +1113,7 @@ class SearchFormController extends \TYPO3\CMS\Frontend\Plugin\AbstractPlugin {
                        $siteIdNumbers = GeneralUtility::intExplode(',', $this->wholeSiteIdList);
                        $id_list = array();
                        foreach ($siteIdNumbers as $rootId) {
-                               $id_list[] = $this->cObj->getTreeList($rootId, 9999, 0, 0, '', '') . $rootId;
+                               $id_list[] = $this->cObj->getTreeList(-1 * $rootId, 9999);
                        }
                        $page_where = ' ISEC.page_id IN (' . implode(',', $id_list) . ')';
                } else {
index 33b08c8..e394001 100644 (file)
@@ -579,7 +579,7 @@ class IndexSearchRepository {
                        $siteIdNumbers = \TYPO3\CMS\Core\Utility\GeneralUtility::intExplode(',', $this->searchRootPageIdList);
                        $pageIdList = array();
                        foreach ($siteIdNumbers as $rootId) {
-                               $pageIdList[] = \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::getTreeList($rootId, 9999, 0, 0, '', '') . $rootId;
+                               $pageIdList[] = \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::getTreeList(-1 * $rootId, 9999);
                        }
                        $page_where = ' AND ISEC.page_id IN (' . implode(',', $pageIdList) . ')';
                }
index 1068956..25dc675 100644 (file)
@@ -175,7 +175,7 @@ class MysqlFulltextIndexHook {
                        foreach ($siteIdNumbers as $rootId) {
                                $cObj = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Frontend\\ContentObject\\ContentObjectRenderer');
                                /** @var \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer $cObj */
-                               $idList[] = $cObj->getTreeList($rootId, 9999, 0, 0, '', '') . $rootId;
+                               $idList[] = $cObj->getTreeList( -1 * $rootId, 9999);
                        }
                        $pageWhere = ' ISEC.page_id IN (' . implode(',', $idList) . ')';
                } else {