[TASK] Revert "Remove runtime cache and early return from TemplatePaths" 45/54145/3
authorBenni Mack <benni@typo3.org>
Thu, 14 Sep 2017 13:13:54 +0000 (15:13 +0200)
committerBenni Mack <benni@typo3.org>
Thu, 14 Sep 2017 14:50:03 +0000 (16:50 +0200)
This reverts commit 6e02927806218e7b9b84310ea735ded578433c02
due to several side effects.

Change-Id: I8d58ebef93c6bb4c81ea894a4a815c118d95d1dc
Resolves: #82487
Reverts: #82196
Reverts: #82181
Releases: master, 8.7
Reviewed-on: https://review.typo3.org/54145
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
typo3/sysext/fluid/Classes/View/TemplatePaths.php
typo3/sysext/fluid/Tests/Unit/View/TemplatePathsTest.php

index 7bba39f..1a61b56 100644 (file)
@@ -14,6 +14,8 @@ namespace TYPO3\CMS\Fluid\View;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Cache\CacheManager;
+use TYPO3\CMS\Core\Cache\Frontend\VariableFrontend;
 use TYPO3\CMS\Core\Utility\ArrayUtility;
 use TYPO3\CMS\Core\Utility\ExtensionManagementUtility;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -77,8 +79,23 @@ class TemplatePaths extends \TYPO3Fluid\Fluid\View\TemplatePaths
         if (empty($extensionKey)) {
             return [];
         }
+        $cache = $this->getRuntimeCache();
+        $cacheIdentifier = 'viewpaths_' . $extensionKey;
+        $configuration = $cache->get($cacheIdentifier);
+        if (is_array($configuration)) {
+            return $configuration;
+        }
 
         $configuredPaths = [];
+        if (!empty($this->templateRootPaths) || !empty($this->partialRootPaths) || !empty($this->layoutRootPaths)) {
+            // The view was manually configured - early return to avoid caching this configuration.
+            return [
+                self::CONFIG_TEMPLATEROOTPATHS => $this->templateRootPaths,
+                self::CONFIG_PARTIALROOTPATHS => $this->partialRootPaths,
+                self::CONFIG_LAYOUTROOTPATHS => $this->layoutRootPaths,
+            ];
+        }
+
         $resources = $this->getExtensionPrivateResourcesPath($extensionKey);
         $paths = [
             self::CONFIG_TEMPLATEROOTPATHS => [$resources . 'Templates/'],
@@ -104,6 +121,8 @@ class TemplatePaths extends \TYPO3Fluid\Fluid\View\TemplatePaths
             }
         }
 
+        $cache->set($cacheIdentifier, $paths);
+
         return $paths;
     }
 
@@ -212,4 +231,12 @@ class TemplatePaths extends \TYPO3Fluid\Fluid\View\TemplatePaths
     {
         return TYPO3_MODE === 'FE';
     }
+
+    /**
+     * @return VariableFrontend
+     */
+    protected function getRuntimeCache()
+    {
+        return GeneralUtility::makeInstance(CacheManager::class)->getCache('cache_runtime');
+    }
 }
index fa6518e..58a7643 100644 (file)
@@ -14,6 +14,7 @@ namespace TYPO3\CMS\Fluid\Tests\Unit\View;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Cache\Frontend\VariableFrontend;
 use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
 use TYPO3\CMS\Fluid\View\TemplatePaths;
 
@@ -101,312 +102,9 @@ class TemplatePathsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     }
 
     /**
-     * Bulk test to confirm that configuration is returned correctly based on
-     * different TypoScript and FE/BE mode, and mixed sorting results in a
-     * correctly sorted set of template paths.
-     *
-     * @param bool $frontendMode
-     * @param array $typoScript
-     * @param array $expectedPaths
      * @test
-     * @dataProvider getContextSpecificViewConfigurationTestValues
      */
-    public function getContextSpecificViewConfigurationMergesAndSortsPaths($frontendMode, array $typoScript, array $expectedPaths)
-    {
-        $configurationManager = $this->getMockBuilder(ConfigurationManagerInterface::class)->getMockForAbstractClass();
-        $configurationManager->expects($this->once())->method('getConfiguration')->willReturn($typoScript);
-        $subject = $this->getMockBuilder(TemplatePaths::class)->setMethods(['getConfigurationManager', 'getExtensionPrivateResourcesPath', 'isBackendMode', 'isFrontendMode'])->getMock();
-        $subject->expects($this->once())->method('getExtensionPrivateResourcesPath')->with('test')->willReturn('test/');
-        $subject->expects($this->once())->method('getConfigurationManager')->willReturn($configurationManager);
-        $subject->expects($this->any())->method('isBackendMode')->willReturn(!$frontendMode);
-        $subject->expects($this->any())->method('isFrontendMode')->willReturn($frontendMode);
-        $result = $this->callInaccessibleMethod($subject, 'getContextSpecificViewConfiguration', 'test');
-        $this->assertSame($expectedPaths, $result);
-    }
-
-    /**
-     * Confirmation that if an empty extension key context is passed,
-     * TemplatePaths will be unable to return even fallback paths and
-     * will instead return an empty array and never call any methods
-     * to get TypoScript etc.
-     *
-     * @test
-     */
-    public function getContextSpecificViewConfigurationReturnsEmptyPathsForEmptyExtensionKey()
-    {
-        $subject = $this->getMockBuilder(TemplatePaths::class)->setMethods(['getConfigurationManager'])->getMock();
-        $subject->expects($this->never())->method('getConfigurationManager');
-        $this->callInaccessibleMethod($subject, 'getContextSpecificViewConfiguration', '');
-    }
-
-    /**
-     * @return array
-     */
-    public function getContextSpecificViewConfigurationTestValues()
-    {
-        return [
-            'complete TypoScript configuration with mixed-sorting paths get sorted correctly in backend mode' => [
-                false,
-                [
-                    'module.' => [
-                        'tx_test.' => [
-                            'view.' => [
-                                'templateRootPaths.' => [
-                                    '30' => 'third',
-                                    '10' => 'first',
-                                    '20' => 'second'
-                                ],
-                                'partialRootPaths.' => [
-                                    '20' => '2',
-                                    '30' => '3',
-                                    '10' => '1'
-                                ],
-                                'layoutRootPaths.' => [
-                                    '130' => '3.',
-                                    '10' => '1.',
-                                    '120' => '2.'
-                                ],
-                            ]
-                        ]
-                    ]
-                ],
-                [
-                    'templateRootPaths' => [
-                        'test/Templates/',
-                        'first',
-                        'second',
-                        'third'
-                    ],
-                    'partialRootPaths' => [
-                        'test/Partials/',
-                        '1',
-                        '2',
-                        '3'
-                    ],
-                    'layoutRootPaths' => [
-                        'test/Layouts/',
-                        '1.',
-                        '2.',
-                        '3.'
-                    ]
-                ]
-            ],
-            'complete TypoScript configuration with mixed-sorting paths get sorted correctly in frontend mode' => [
-                true,
-                [
-                    'plugin.' => [
-                        'tx_test.' => [
-                            'view.' => [
-                                'templateRootPaths.' => [
-                                    '30' => 'third',
-                                    '10' => 'first',
-                                    '20' => 'second'
-                                ],
-                                'partialRootPaths.' => [
-                                    '20' => '2',
-                                    '30' => '3',
-                                    '10' => '1'
-                                ],
-                                'layoutRootPaths.' => [
-                                    '130' => '3.',
-                                    '10' => '1.',
-                                    '120' => '2.'
-                                ],
-                            ]
-                        ]
-                    ]
-                ],
-                [
-                    'templateRootPaths' => [
-                        'test/Templates/',
-                        'first',
-                        'second',
-                        'third'
-                    ],
-                    'partialRootPaths' => [
-                        'test/Partials/',
-                        '1',
-                        '2',
-                        '3'
-                    ],
-                    'layoutRootPaths' => [
-                        'test/Layouts/',
-                        '1.',
-                        '2.',
-                        '3.'
-                    ]
-                ]
-            ],
-            'partial TypoScript configuration merges fallback templateRootPaths' => [
-                true,
-                [
-                    'plugin.' => [
-                        'tx_test.' => [
-                            'view.' => [
-                                'partialRootPaths.' => [
-                                    '20' => '2',
-                                    '30' => '3',
-                                    '10' => '1'
-                                ],
-                                'layoutRootPaths.' => [
-                                    '130' => '3.',
-                                    '10' => '1.',
-                                    '120' => '2.'
-                                ],
-                            ]
-                        ]
-                    ]
-                ],
-                [
-                    'templateRootPaths' => [
-                        'test/Templates/'
-                    ],
-                    'partialRootPaths' => [
-                        'test/Partials/',
-                        '1',
-                        '2',
-                        '3'
-                    ],
-                    'layoutRootPaths' => [
-                        'test/Layouts/',
-                        '1.',
-                        '2.',
-                        '3.'
-                    ]
-                ]
-            ],
-            'partial TypoScript configuration merges fallback partialRootPaths' => [
-                true,
-                [
-                    'plugin.' => [
-                        'tx_test.' => [
-                            'view.' => [
-                                'templateRootPaths.' => [
-                                    '20' => '2',
-                                    '30' => '3',
-                                    '10' => '1'
-                                ],
-                                'layoutRootPaths.' => [
-                                    '130' => '3.',
-                                    '10' => '1.',
-                                    '120' => '2.'
-                                ],
-                            ]
-                        ]
-                    ]
-                ],
-                [
-                    'templateRootPaths' => [
-                        'test/Templates/',
-                        '1',
-                        '2',
-                        '3'
-                    ],
-                    'partialRootPaths' => [
-                        'test/Partials/'
-                    ],
-                    'layoutRootPaths' => [
-                        'test/Layouts/',
-                        '1.',
-                        '2.',
-                        '3.'
-                    ]
-                ]
-            ],
-            'partial TypoScript configuration merges fallback layoutRootPaths' => [
-                true,
-                [
-                    'plugin.' => [
-                        'tx_test.' => [
-                            'view.' => [
-                                'templateRootPaths.' => [
-                                    '20' => '2',
-                                    '30' => '3',
-                                    '10' => '1'
-                                ],
-                                'partialRootPaths.' => [
-                                    '130' => '3.',
-                                    '10' => '1.',
-                                    '120' => '2.'
-                                ],
-                            ]
-                        ]
-                    ]
-                ],
-                [
-                    'templateRootPaths' => [
-                        'test/Templates/',
-                        '1',
-                        '2',
-                        '3'
-                    ],
-                    'partialRootPaths' => [
-                        'test/Partials/',
-                        '1.',
-                        '2.',
-                        '3.'
-                    ],
-                    'layoutRootPaths' => [
-                        'test/Layouts/'
-                    ]
-                ]
-            ],
-            'partial TypoScript configuration merges fallback layoutRootPaths and partialRootPaths' => [
-                true,
-                [
-                    'plugin.' => [
-                        'tx_test.' => [
-                            'view.' => [
-                                'templateRootPaths.' => [
-                                    '20' => '2',
-                                    '30' => '3',
-                                    '10' => '1'
-                                ]
-                            ]
-                        ]
-                    ]
-                ],
-                [
-                    'templateRootPaths' => [
-                        'test/Templates/',
-                        '1',
-                        '2',
-                        '3'
-                    ],
-                    'partialRootPaths' => [
-                        'test/Partials/'
-                    ],
-                    'layoutRootPaths' => [
-                        'test/Layouts/'
-                    ]
-                ]
-            ],
-            'missing TypoScript configuration returns fallback path collection' => [
-                true,
-                [],
-                [
-                    'templateRootPaths' => [
-                        'test/Templates/'
-                    ],
-                    'partialRootPaths' => [
-                        'test/Partials/'
-                    ],
-                    'layoutRootPaths' => [
-                        'test/Layouts/'
-                    ]
-                ]
-            ],
-        ];
-    }
-
-    /**
-     * Test to confirm that regardless of TypoScript configuration, an unknown FE/BE mode
-     * results in purely fallback template paths being returned (since not knowing the
-     * mode means inability to resolve correct TS TLO plugin./module.)
-     *
-     * @test
-     */
-    public function getContextSpecificViewConfigurationDoesNotResolveFromTypoScriptAndDoesNotSortInUnspecifiedMode()
+    public function getContextSpecificViewConfigurationSortsTypoScriptConfiguredPathsCorrectlyInFrontendMode()
     {
         $configurationManager = $this->getMockBuilder(ConfigurationManagerInterface::class)->getMockForAbstractClass();
         $configurationManager->expects($this->once())->method('getConfiguration')->willReturn([
@@ -432,40 +130,47 @@ class TemplatePathsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
                 ]
             ]
         ]);
-        $subject = $this->getMockBuilder(TemplatePaths::class)->setMethods(['getConfigurationManager', 'getExtensionPrivateResourcesPath', 'isBackendMode', 'isFrontendMode'])->getMock();
+        $cache = $this->getMockBuilder(VariableFrontend::class)->setMethods(['get', 'set'])->disableOriginalConstructor()->getMock();
+        $cache->expects($this->once())->method('get')->willReturn(false);
+        $cache->expects($this->once())->method('set');
+        $subject = $this->getMockBuilder(TemplatePaths::class)->setMethods(['getConfigurationManager', 'getExtensionPrivateResourcesPath', 'getRuntimeCache', 'isBackendMode', 'isFrontendMode'])->getMock();
         $subject->expects($this->once())->method('getExtensionPrivateResourcesPath')->with('test')->willReturn('test/');
         $subject->expects($this->once())->method('getConfigurationManager')->willReturn($configurationManager);
+        $subject->expects($this->once())->method('getRuntimeCache')->willReturn($cache);
         $subject->expects($this->once())->method('isBackendMode')->willReturn(false);
-        $subject->expects($this->once())->method('isFrontendMode')->willReturn(false);
+        $subject->expects($this->once())->method('isFrontendMode')->willReturn(true);
         $result = $this->callInaccessibleMethod($subject, 'getContextSpecificViewConfiguration', 'test');
         $this->assertSame([
             'templateRootPaths' => [
-                'test/Templates/'
+                'test/Templates/',
+                'first',
+                'second',
+                'third'
             ],
             'partialRootPaths' => [
-                'test/Partials/'
+                'test/Partials/',
+                '1',
+                '2',
+                '3'
             ],
             'layoutRootPaths' => [
-                'test/Layouts/'
+                'test/Layouts/',
+                '1.',
+                '2.',
+                '3.'
             ]
         ], $result);
     }
 
     /**
-     * Test to confirm that regardless which setter methods for path collections
-     * were called, the getContextSpecificViewConfiguration() method always returns
-     * the paths that were configured in TypoScript and does not change any of the
-     * paths set via the setter methods.
-     *
-     * @dataProvider getContextSpecificViewConfigurationIgnoresValues
      * @test
      */
-    public function getContextSpecificViewConfigurationIgnoresValuesSetInPathSetterMethods($frontendMode, array $paths)
+    public function getContextSpecificViewConfigurationSortsTypoScriptConfiguredPathsCorrectlyInBackendMode()
     {
         $configurationManager = $this->getMockBuilder(ConfigurationManagerInterface::class)->getMockForAbstractClass();
         $configurationManager->expects($this->once())->method('getConfiguration')->willReturn([
-            'plugin.' => [
-                'tx_test.' => [
+            'module.' => [
+                'tx_test2.' => [
                     'view.' => [
                         'templateRootPaths.' => [
                             '30' => 'third',
@@ -486,20 +191,16 @@ class TemplatePathsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
                 ]
             ]
         ]);
-        $subject = $this->getMockBuilder(TemplatePaths::class)->setMethods(['getConfigurationManager', 'getExtensionPrivateResourcesPath', 'isBackendMode', 'isFrontendMode', 'sanitizePath'])->getMock();
-        $subject->expects($this->once())->method('getExtensionPrivateResourcesPath')->with('test')->willReturn('test/');
+        $cache = $this->getMockBuilder(VariableFrontend::class)->setMethods(['get', 'set'])->disableOriginalConstructor()->getMock();
+        $cache->expects($this->once())->method('get')->willReturn(false);
+        $cache->expects($this->once())->method('set');
+        $subject = $this->getMockBuilder(TemplatePaths::class)->setMethods(['getConfigurationManager', 'getExtensionPrivateResourcesPath', 'getRuntimeCache', 'isBackendMode', 'isFrontendMode'])->getMock();
+        $subject->expects($this->once())->method('getExtensionPrivateResourcesPath')->with('test2')->willReturn('test/');
         $subject->expects($this->once())->method('getConfigurationManager')->willReturn($configurationManager);
-        $subject->expects($this->once())->method('isBackendMode')->willReturn(!$frontendMode);
-        $subject->expects($this->once())->method('isFrontendMode')->willReturn($frontendMode);
-
-        // Set paths and expect sanitizePath() to be called, emulate return from sanitizePath()
-        $subject->expects($this->atLeastOnce())->method('sanitizePath')->willReturnArgument(0);
-        foreach ($paths as $key => $pathSet) {
-            $setter = 'set' . ucfirst($key);
-            $subject->$setter($pathSet);
-        }
-
-        $result = $this->callInaccessibleMethod($subject, 'getContextSpecificViewConfiguration', 'test');
+        $subject->expects($this->once())->method('getRuntimeCache')->willReturn($cache);
+        $subject->expects($this->once())->method('isBackendMode')->willReturn(true);
+        $subject->expects($this->never())->method('isFrontendMode');
+        $result = $this->callInaccessibleMethod($subject, 'getContextSpecificViewConfiguration', 'test2');
         $this->assertSame([
             'templateRootPaths' => [
                 'test/Templates/',
@@ -520,44 +221,57 @@ class TemplatePathsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
                 '3.'
             ]
         ], $result);
-        foreach ($paths as $key => $pathSet) {
-            $this->assertAttributeSame($pathSet, $key, $subject);
-        }
     }
 
     /**
-     * @return array
+     * @test
      */
-    public function getContextSpecificViewConfigurationIgnoresValues()
+    public function getContextSpecificViewConfigurationDoesNotResolveFromTypoScriptAndDoesNotSortInUnspecifiedMode()
     {
-        return [
-            'has templateRootPaths set with setter method' => [
-                true,
-                [
-                    'templateRootPaths' => [
-                        '1.',
-                        '2.'
-                    ]
-                ]
-            ],
-            'has partialRootPaths set with setter method' => [
-                true,
-                [
-                    'partialRootPaths' => [
-                        '1.',
-                        '2.'
+        $configurationManager = $this->getMockBuilder(ConfigurationManagerInterface::class)->getMockForAbstractClass();
+        $configurationManager->expects($this->once())->method('getConfiguration')->willReturn([
+            'plugin.' => [
+                'tx_test.' => [
+                    'view.' => [
+                        'templateRootPaths.' => [
+                            '30' => 'third',
+                            '10' => 'first',
+                            '20' => 'second'
+                        ],
+                        'partialRootPaths.' => [
+                            '20' => '2',
+                            '30' => '3',
+                            '10' => '1'
+                        ],
+                        'layoutRootPaths.' => [
+                            '130' => '3.',
+                            '10' => '1.',
+                            '120' => '2.'
+                        ],
                     ]
                 ]
+            ]
+        ]);
+        $cache = $this->getMockBuilder(VariableFrontend::class)->setMethods(['get', 'set'])->disableOriginalConstructor()->getMock();
+        $cache->expects($this->once())->method('get')->willReturn(false);
+        $cache->expects($this->once())->method('set');
+        $subject = $this->getMockBuilder(TemplatePaths::class)->setMethods(['getConfigurationManager', 'getExtensionPrivateResourcesPath', 'getRuntimeCache', 'isBackendMode', 'isFrontendMode'])->getMock();
+        $subject->expects($this->once())->method('getExtensionPrivateResourcesPath')->with('test')->willReturn('test/');
+        $subject->expects($this->once())->method('getConfigurationManager')->willReturn($configurationManager);
+        $subject->expects($this->once())->method('getRuntimeCache')->willReturn($cache);
+        $subject->expects($this->once())->method('isBackendMode')->willReturn(false);
+        $subject->expects($this->once())->method('isFrontendMode')->willReturn(false);
+        $result = $this->callInaccessibleMethod($subject, 'getContextSpecificViewConfiguration', 'test');
+        $this->assertSame([
+            'templateRootPaths' => [
+                'test/Templates/'
             ],
-            'has layoutRootPaths set with setter method' => [
-                true,
-                [
-                    'layoutRootPaths' => [
-                        '1.',
-                        '2.'
-                    ]
-                ]
+            'partialRootPaths' => [
+                'test/Partials/'
             ],
-        ];
+            'layoutRootPaths' => [
+                'test/Layouts/'
+            ]
+        ], $result);
     }
 }