Bug #63550
closedMenu configuration caching disabled by ineffective type checks
100%
Description
We just encountered a bug that under certain circumstances prevented the caching of multi-level menu configurations.
We encountered this issue due to massive performance problems after a TYPO3 6.2 migration. In our case, we have a nested menu with 3 levels; however, only a small subset of menu items actually has a 3rd menu level. For every other 2nd level menu item without sub menu items, the entire menu configuration was completely rebuilt from scratch, because the (empty) menu configuration was not cached by the caching framework.
This is actually due to two issues in the TYPO3\Core\Frontend\ContentObject\Menu\TextMenuContentObject
and TYPO3\Core\Frontend\Page\PageRepository
class.
First of all, the TextMenuContentObject
class generates a NULL
menu configuration when no pages are rendered with in the menu. The AbstractMenuContentObject
class then tries to cache this NULL
value. However, when rendering the NEXT menu item, the menu configuration will be re-built when the cached value is !is_array(...)
(i.e. always, when the cached value is NULL
).
Secondly, the TYPO3\Core\Frontend\Page\PageRepository
class contains a typing error that effectively prevents empty arrays from being cached by the caching framework.
Both of these bugs in conjunction not only effectively disable the caching of the menu configuration, but also put a lot of load on the database, because the AbstractMenuContentObject
actually tries to read the cache AND rebuild it for each single menu item.
Proposed solution:
- Store an (empty) array in cache for empty menus instead of NULL
.
- Allow the TYPO3\Core\Frontend\Page\PageRepository
class to cache empty arrays (this class actually wraps TYPO3's caching framework, which in itself handles those values correctly).
Caveats:
- Not sure if the proposed change is backwards-compatible. Comments are welcome.
We propose the following patch as a solution (change request on Gerrit will follow shortly):
diff --git a/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php b/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php index 44098de..779e5ad 100644 --- a/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php +++ b/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php @@ -33,7 +33,10 @@ class TextMenuContentObject extends \TYPO3\CMS\Frontend\ContentObject\Menu\Abstr $splitCount = count($this->menuArr); if ($splitCount) { list($NOconf) = $this->procesItemStates($splitCount); + } else { + $NOconf = array(); } + if ($this->mconf['debugItemConf']) { echo '<h3>$NOconf:</h3>'; debug($NOconf); diff --git a/typo3/sysext/frontend/Classes/Page/PageRepository.php b/typo3/sysext/frontend/Classes/Page/PageRepository.php index c876d34..beb3f39 100644 --- a/typo3/sysext/frontend/Classes/Page/PageRepository.php +++ b/typo3/sysext/frontend/Classes/Page/PageRepository.php @@ -879,7 +879,7 @@ class PageRepository { $hashContent = NULL; $contentHashCache = GeneralUtility::makeInstance('TYPO3\\CMS\\Core\\Cache\\CacheManager')->getCache('cache_hash'); $cacheEntry = $contentHashCache->get($hash); - if ($cacheEntry) { + if ($cacheEntry !== FALSE) { $hashContent = $cacheEntry; } return $hashContent;