[TASK] Do not rely on sys_page when storing cached data in cObj 81/52181/3
authorBenni Mack <benni@typo3.org>
Mon, 27 Mar 2017 13:53:17 +0000 (15:53 +0200)
committerSusanne Moog <susanne.moog@typo3.org>
Tue, 28 Mar 2017 14:41:44 +0000 (16:41 +0200)
ContentObject renderer has an unnecessary call to $TSFE->sys_page
for calling a static method in a non-static way.

It should directly use the caching framework instead.

Resolves: #80482
Releases: master
Change-Id: Iaa93de3c1e98cf5e639605dcf49cb5715a28b5d7
Reviewed-on: https://review.typo3.org/52181
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
typo3/sysext/frontend/Tests/Unit/ContentObject/Fixtures/PageRepositoryFixture.php [deleted file]

index cdf99b7..aa49db4 100644 (file)
@@ -1664,7 +1664,8 @@ class ContentObjectRenderer
             $storeArr = $this->substMarkerCache[$storeKey];
             $timeTracker->setTSlogMessage('Cached', 0);
         } else {
-            $storeArrDat = $this->getTypoScriptFrontendController()->sys_page->getHash($storeKey);
+            $cache = $this->getCache();
+            $storeArrDat = $cache->get($storeKey);
             if (is_array($storeArrDat)) {
                 $storeArr = $storeArrDat;
                 // Setting cache:
@@ -1702,8 +1703,8 @@ class ContentObjectRenderer
                     $storeArr['k'] = $wrappedKeys; // contains all markers incl. ###
                     // Setting cache:
                     $this->substMarkerCache[$storeKey] = $storeArr;
-                    // Storing the cached data:
-                    $this->getTypoScriptFrontendController()->sys_page->storeHash($storeKey, $storeArr, 'substMarkArrayCached');
+                    // Storing the cached data
+                    $cache->set($storeKey, $storeArr, ['substMarkArrayCached'], 0);
                 }
                 $timeTracker->setTSlogMessage('Parsing', 0);
             }
@@ -8372,4 +8373,12 @@ class ContentObjectRenderer
     {
         return $this->typoScriptFrontendController ?: $GLOBALS['TSFE'];
     }
+
+    /**
+     * @return \TYPO3\CMS\Core\Cache\Frontend\FrontendInterface
+     */
+    protected function getCache()
+    {
+        return GeneralUtility::makeInstance(CacheManager::class)->getCache('cache_hash');
+    }
 }
index fe7c963..b2ca3f7 100644 (file)
@@ -49,7 +49,7 @@ use TYPO3\CMS\Frontend\ContentObject\TextContentObject;
 use TYPO3\CMS\Frontend\ContentObject\UserContentObject;
 use TYPO3\CMS\Frontend\ContentObject\UserInternalContentObject;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
-use TYPO3\CMS\Frontend\Tests\Unit\ContentObject\Fixtures\PageRepositoryFixture;
+use TYPO3\CMS\Frontend\Page\PageRepository;
 
 /**
  * Testcase for TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer
@@ -73,6 +73,11 @@ class ContentObjectRendererTest extends \TYPO3\TestingFramework\Core\Unit\UnitTe
     protected $frontendControllerMock = null;
 
     /**
+     * @var \PHPUnit_Framework_MockObject_MockObject|CacheFrontendInterface
+     */
+    protected $cacheFrontendMock = null;
+
+    /**
      * @var \PHPUnit_Framework_MockObject_MockObject|TemplateService
      */
     protected $templateServiceMock = null;
@@ -118,8 +123,7 @@ class ContentObjectRendererTest extends \TYPO3\TestingFramework\Core\Unit\UnitTe
             $this->getMockBuilder(TemplateService::class)
             ->setMethods(['getFileName', 'linkData'])->getMock();
         $pageRepositoryMock =
-            $this->getMockBuilder(PageRepositoryFixture::class)
-            ->setMethods(['getRawRecord', 'getMountPointInfo'])->getMock();
+            $this->getAccessibleMock(PageRepository::class, ['getRawRecord', 'getMountPointInfo']);
         $this->frontendControllerMock =
             $this->getAccessibleMock(TypoScriptFrontendController::class,
             ['dummy'], [], '', false);
@@ -3108,224 +3112,6 @@ class ContentObjectRendererTest extends \TYPO3\TestingFramework\Core\Unit\UnitTe
     }
 
     /**
-     * @return array
-     */
-    public function substituteMarkerArrayCachedReturnsExpectedContentDataProvider()
-    {
-        return [
-            'no markers defined' => [
-                'dummy content with ###UNREPLACED### marker',
-                [],
-                [],
-                [],
-                'dummy content with ###UNREPLACED### marker',
-                false,
-                false
-            ],
-            'no markers used' => [
-                'dummy content with no marker',
-                [
-                    '###REPLACED###' => '_replaced_'
-                ],
-                [],
-                [],
-                'dummy content with no marker',
-                true,
-                false
-            ],
-            'one marker' => [
-                'dummy content with ###REPLACED### marker',
-                [
-                    '###REPLACED###' => '_replaced_'
-                ],
-                [],
-                [],
-                'dummy content with _replaced_ marker'
-            ],
-            'one marker with lots of chars' => [
-                'dummy content with ###RE.:##-=_()LACED### marker',
-                [
-                    '###RE.:##-=_()LACED###' => '_replaced_'
-                ],
-                [],
-                [],
-                'dummy content with _replaced_ marker'
-            ],
-            'markers which are special' => [
-                'dummy ###aa##.#######A### ######',
-                [
-                    '###aa##.###' => 'content ',
-                    '###A###' => 'is',
-                    '######' => '-is not considered-'
-                ],
-                [],
-                [],
-                'dummy content #is ######'
-            ],
-            'two markers in content, but more defined' => [
-                'dummy ###CONTENT### with ###REPLACED### marker',
-                [
-                    '###REPLACED###' => '_replaced_',
-                    '###CONTENT###' => 'content',
-                    '###NEVERUSED###' => 'bar'
-                ],
-                [],
-                [],
-                'dummy content with _replaced_ marker'
-            ],
-            'one subpart' => [
-                'dummy content with ###ASUBPART### around some text###ASUBPART###.',
-                [],
-                [
-                    '###ASUBPART###' => 'some other text'
-                ],
-                [],
-                'dummy content with some other text.'
-            ],
-            'one wrapped subpart' => [
-                'dummy content with ###AWRAPPEDSUBPART### around some text###AWRAPPEDSUBPART###.',
-                [],
-                [],
-                [
-                    '###AWRAPPEDSUBPART###' => [
-                        'more content',
-                        'content'
-                    ]
-                ],
-                'dummy content with more content around some textcontent.'
-            ],
-            'one subpart with markers, not replaced recursively' => [
-                'dummy ###CONTENT### with ###ASUBPART### around ###SOME### text###ASUBPART###.',
-                [
-                    '###CONTENT###' => 'content',
-                    '###SOME###' => '-this should never make it into output-',
-                    '###OTHER_NOT_REPLACED###' => '-this should never make it into output-'
-                ],
-                [
-                    '###ASUBPART###' => 'some ###OTHER_NOT_REPLACED### text'
-                ],
-                [],
-                'dummy content with some ###OTHER_NOT_REPLACED### text.'
-            ],
-        ];
-    }
-
-    /**
-     * @test
-     * @dataProvider substituteMarkerArrayCachedReturnsExpectedContentDataProvider
-     *
-     * @param string $content
-     * @param array $markContentArray
-     * @param array $subpartContentArray
-     * @param array $wrappedSubpartContentArray
-     * @param string $expectedContent
-     * @param bool $shouldQueryCache
-     * @param bool $shouldStoreCache
-     */
-    public function substituteMarkerArrayCachedReturnsExpectedContent($content, array $markContentArray, array $subpartContentArray, array $wrappedSubpartContentArray, $expectedContent, $shouldQueryCache = true, $shouldStoreCache = true)
-    {
-        /** @var PageRepositoryFixture|\PHPUnit_Framework_MockObject_MockObject $pageRepo */
-        $pageRepo = $this->frontendControllerMock->sys_page;
-        $pageRepo->resetCallCount();
-
-        $resultContent = $this->subject->substituteMarkerArrayCached($content, $markContentArray, $subpartContentArray, $wrappedSubpartContentArray);
-
-        $this->assertSame((int)$shouldQueryCache, $pageRepo::$getHashCallCount, 'getHash call count mismatch');
-        $this->assertSame((int)$shouldStoreCache, $pageRepo::$storeHashCallCount, 'storeHash call count mismatch');
-        $this->assertSame($expectedContent, $resultContent);
-    }
-
-    /**
-     * @test
-     */
-    public function substituteMarkerArrayCachedRetrievesCachedValueFromRuntimeCache()
-    {
-        /** @var PageRepositoryFixture|\PHPUnit_Framework_MockObject_MockObject $pageRepo */
-        $pageRepo = $this->frontendControllerMock->sys_page;
-        $pageRepo->resetCallCount();
-
-        $content = 'Please tell me this ###FOO###.';
-        $markContentArray = [
-            '###FOO###' => 'foo',
-            '###NOTUSED###' => 'blub'
-        ];
-        $storeKey = md5('substituteMarkerArrayCached_storeKey:' . serialize([$content, array_keys($markContentArray)]));
-        $this->subject->substMarkerCache[$storeKey] = [
-            'c' => [
-                'Please tell me this ',
-                '.'
-            ],
-            'k' => [
-                '###FOO###'
-            ],
-        ];
-        $resultContent = $this->subject->substituteMarkerArrayCached($content, $markContentArray);
-        $this->assertSame(0, $pageRepo::$getHashCallCount);
-        $this->assertSame('Please tell me this foo.', $resultContent);
-    }
-
-    /**
-     * @test
-     */
-    public function substituteMarkerArrayCachedRetrievesCachedValueFromDbCache()
-    {
-        /** @var PageRepositoryFixture|\PHPUnit_Framework_MockObject_MockObject $pageRepo */
-        $pageRepo = $this->frontendControllerMock->sys_page;
-        $pageRepo->resetCallCount();
-
-        $content = 'Please tell me this ###FOO###.';
-        $markContentArray = [
-            '###FOO###' => 'foo',
-            '###NOTUSED###' => 'blub'
-        ];
-        $pageRepo::$dbCacheContent = [
-            'c' => [
-                'Please tell me this ',
-                '.'
-            ],
-            'k' => [
-                '###FOO###'
-            ],
-        ];
-        $resultContent = $this->subject->substituteMarkerArrayCached($content, $markContentArray);
-        $this->assertSame(1, $pageRepo::$getHashCallCount, 'getHash call count mismatch');
-        $this->assertSame(0, $pageRepo::$storeHashCallCount, 'storeHash call count mismatch');
-        $this->assertSame('Please tell me this foo.', $resultContent);
-    }
-
-    /**
-     * @test
-     */
-    public function substituteMarkerArrayCachedStoresResultInCaches()
-    {
-        /** @var PageRepositoryFixture|\PHPUnit_Framework_MockObject_MockObject $pageRepo */
-        $pageRepo = $this->frontendControllerMock->sys_page;
-        $pageRepo->resetCallCount();
-
-        $content = 'Please tell me this ###FOO###.';
-        $markContentArray = [
-            '###FOO###' => 'foo',
-            '###NOTUSED###' => 'blub'
-        ];
-        $resultContent = $this->subject->substituteMarkerArrayCached($content, $markContentArray);
-
-        $storeKey = md5('substituteMarkerArrayCached_storeKey:' . serialize([$content, array_keys($markContentArray)]));
-        $storeArr = [
-            'c' => [
-                'Please tell me this ',
-                '.'
-            ],
-            'k' => [
-                '###FOO###'
-            ],
-        ];
-        $this->assertSame(1, $pageRepo::$getHashCallCount);
-        $this->assertSame('Please tell me this foo.', $resultContent);
-        $this->assertSame($storeArr, $this->subject->substMarkerCache[$storeKey]);
-        $this->assertSame(1, $pageRepo::$storeHashCallCount);
-    }
-
-    /**
      * Check if calculateCacheKey works properly.
      *
      * @return array Order: expect, conf, times, with, withWrap, will
diff --git a/typo3/sysext/frontend/Tests/Unit/ContentObject/Fixtures/PageRepositoryFixture.php b/typo3/sysext/frontend/Tests/Unit/ContentObject/Fixtures/PageRepositoryFixture.php
deleted file mode 100644 (file)
index da8a5ff..0000000
+++ /dev/null
@@ -1,56 +0,0 @@
-<?php
-namespace TYPO3\CMS\Frontend\Tests\Unit\ContentObject\Fixtures;
-
-/*
- * This file is part of the TYPO3 CMS project.
- *
- * It is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License, either version 2
- * of the License, or any later version.
- *
- * For the full copyright and license information, please read the
- * LICENSE.txt file that was distributed with this source code.
- *
- * The TYPO3 project - inspiring people to share!
- */
-
-use TYPO3\CMS\Frontend\Page\PageRepository;
-
-/**
- * Fixture for TYPO3\CMS\Core\Utility\GeneralUtility
- */
-class PageRepositoryFixture extends PageRepository
-{
-    /**
-     * @var int
-     */
-    public static $getHashCallCount = 0;
-
-    /**
-     * @var int
-     */
-    public static $storeHashCallCount = 0;
-
-    /**
-     * @var mixed
-     */
-    public static $dbCacheContent = null;
-
-    public static function getHash($hash)
-    {
-        static::$getHashCallCount++;
-        return static::$dbCacheContent;
-    }
-
-    public static function storeHash($hash, $data, $ident, $lifetime = 0)
-    {
-        static::$storeHashCallCount++;
-    }
-
-    public static function resetCallCount()
-    {
-        static::$getHashCallCount = 0;
-        static::$storeHashCallCount = 0;
-        static::$dbCacheContent = null;
-    }
-}