Skip to content

Commit 575bd35

Browse files
lolli42lolli42
committed
[BUGFIX] Avoid storing FE "last changed" in register
The "last updated" mechanism in FE is and has always been flawed. It is however hard to substitute with something more reliable. The patch fixes a single long standing issue and documents the other shortcomings. Storing "SYS_LASTCHANGED" in the FE "register" cripples the approach of gathering the "highest" timestamp from content elements: The register is a stack, an updated value on the stack vanishes on pop. When the "final" value is used after rendering all content elements, any changed "last update" values from content elements are usually gone due to RESTORE_REGISTER cObj calls. The patch thus moves the "last updated" information gathering away from "register" and stores it as separated information in the recently established "frontent.page.parts" request attribute. TS data/getText key "register:SYS_LASTCHANGED" is kept as b/w compat layer to not break hard to debug FE TS. Resolves: #82528 Related: #107578 Releases: main Change-Id: I477b486837bfccc2c6ca36a686b7424b3c41741e Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/90913 Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Stefan Bürk <stefan@buerk.tech> Tested-by: core-ci <typo3@b13.com> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Garvin Hicking <garvin@hick.ing> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Garvin Hicking <garvin@hick.ing>
1 parent c387285 commit 575bd35

File tree

6 files changed

+57
-20
lines changed

6 files changed

+57
-20
lines changed

‎typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,20 +1044,19 @@ public function imageLinkWrap($string, $imageFile, $conf)
10441044
* Therefore you should call this function with the last-changed timestamp of any element you display.
10451045
*
10461046
* @param RecordInterface|int|string|float|null $item a record objet or a Unix timestamp (number of seconds since 1970)
1047-
* @see TypoScriptFrontendController::setSysLastChanged()
10481047
*/
1049-
public function lastChanged(RecordInterface|int|string|float|null $item)
1048+
public function lastChanged(RecordInterface|int|string|float|null $item): void
10501049
{
10511050
if (MathUtility::canBeInterpretedAsInteger($item)) {
10521051
$item = (int)$item;
10531052
} elseif ($item instanceof Record) {
10541053
$item = $item->getSystemProperties()->getLastUpdatedAt()->getTimestamp();
10551054
} else {
1056-
$item = 0;
1055+
return;
10571056
}
1058-
$tsfe = $this->getTypoScriptFrontendController();
1059-
if ($item > (int)($tsfe->register['SYS_LASTCHANGED'] ?? 0)) {
1060-
$tsfe->register['SYS_LASTCHANGED'] = $item;
1057+
$pageParts = $this->getRequest()->getAttribute('frontend.page.parts');
1058+
if ($item > $pageParts->getLastChanged()) {
1059+
$pageParts->setLastChanged($item);
10611060
}
10621061
}
10631062

@@ -3921,8 +3920,14 @@ public function getData($string, $fieldArray = null)
39213920
$retVal = $this->parameters[$key] ?? null;
39223921
break;
39233922
case 'register':
3924-
$tsfe = $this->getTypoScriptFrontendController();
3925-
$retVal = $tsfe->register[$key] ?? null;
3923+
if ($key === 'SYS_LASTCHANGED') {
3924+
// b/w compat layer: SYS_LASTCHANGED has been a register entry until TYPO3 v14. It is now part
3925+
// of a request attribute. The register access via TS should continue to work, though.
3926+
$retVal = $this->getRequest()->getAttribute('frontend.page.parts')->getLastChanged();
3927+
} else {
3928+
$tsfe = $this->getTypoScriptFrontendController();
3929+
$retVal = $tsfe->register[$key] ?? null;
3930+
}
39263931
break;
39273932
case 'global':
39283933
$retVal = $this->getGlobal($key);

‎typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,14 +338,15 @@ protected function setSysLastChanged(ServerRequestInterface $request): void
338338
return;
339339
}
340340
$pageInformation = $request->getAttribute('frontend.page.information');
341+
$pageParts = $request->getAttribute('frontend.page.parts');
341342
$pageRecord = $pageInformation->getPageRecord();
342-
if ($pageRecord['SYS_LASTCHANGED'] < (int)($this->register['SYS_LASTCHANGED'] ?? 0)) {
343+
if ($pageRecord['SYS_LASTCHANGED'] < $pageParts->getLastChanged()) {
343344
$connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable('pages');
344345
$pageId = $pageRecord['_LOCALIZED_UID'] ?? $pageInformation->getId();
345346
$connection->update(
346347
'pages',
347348
[
348-
'SYS_LASTCHANGED' => (int)$this->register['SYS_LASTCHANGED'],
349+
'SYS_LASTCHANGED' => $pageParts->getLastChanged(),
349350
],
350351
[
351352
'uid' => (int)$pageId,

‎typo3/sysext/frontend/Classes/Middleware/TypoScriptFrontendInitialization.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
107107

108108
$controller = GeneralUtility::makeInstance(TypoScriptFrontendController::class);
109109
$controller->initializePageRenderer($request);
110-
// Update SYS_LASTCHANGED at the very last, when page record was finally resolved.
111-
// Is also called when a translated page is in use, so the register reflects
112-
// the state of the translated page, not the page in the default language.
113-
$controller->register['SYS_LASTCHANGED'] = (int)$pageInformation->getPageRecord()['tstamp'];
114-
if ($controller->register['SYS_LASTCHANGED'] < (int)$pageInformation->getPageRecord()['SYS_LASTCHANGED']) {
115-
$controller->register['SYS_LASTCHANGED'] = (int)$pageInformation->getPageRecord()['SYS_LASTCHANGED'];
116-
}
117110
$request = $request->withAttribute('frontend.controller', $controller);
118111
$pageParts = new PageParts();
112+
// Init "last changed" with "tstamp" of the page record, or SYS_LASTCHANGED if it is lower.
113+
// See the attribute property comment for details.
114+
$lastChanged = (int)$pageInformation->getPageRecord()['tstamp'];
115+
if ($lastChanged < (int)$pageInformation->getPageRecord()['SYS_LASTCHANGED']) {
116+
$lastChanged = (int)$pageInformation->getPageRecord()['SYS_LASTCHANGED'];
117+
}
118+
$pageParts->setLastChanged($lastChanged);
119119
$request = $request->withAttribute('frontend.page.parts', $pageParts);
120120
$GLOBALS['TYPO3_REQUEST'] = $request;
121121
$GLOBALS['TSFE'] = $controller;

‎typo3/sysext/frontend/Classes/Page/PageParts.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,25 @@ final class PageParts
4646
*/
4747
private string $content = '';
4848

49+
/**
50+
* "Last change" is page record "SYS_LASTCHANGED", initialized with pages record "tstamp", whichever is higher.
51+
* This value is later modified by ContentObjectRenderer when records are rendered. The pages record column
52+
* "SYS_LASTCHANGED" is then written to the highest value at the end of FE rendering.
53+
* The main goal of pages "SYS_LASTCHANGED" is to have a DB field on pages that "knows" when a record that is
54+
* displayed on the page is last changed. This information is for instance used in the ext:seo sitemap XML.
55+
*
56+
* This approach is flawed for multimple reasons: The "last updated" value can only be "gathered" during FE
57+
* rendering since BE does not necessarily know all elements rendered on a page when for example a news plugin
58+
* fetches record from "elsewhere", e.g. a record storage page. This means the "final" value of pages "SYS_LASTCHANGED"
59+
* is only ready after all content elements have been rendered. It also relies on such a plugin actively taking
60+
* care of updating $lastChanged here (@see ContentObjectRenderer->lastChanged()). Rendering a "Last updated"
61+
* value on a page thus only works when it is output at the end (or as USER_INT which are calculated in the end).
62+
* Additionally, a plugin like the ext:seo pages sitemap (which only gives a list of pages but does not actually
63+
* render all pages) can only render a correct value after a page containing a "just changed" content element
64+
* has been FE rendered at least once to have the newest "SYS_LASTCHANGED" value in DB.
65+
*/
66+
private int $lastChanged = 0;
67+
4968
public function setContent(string $content): void
5069
{
5170
$this->content = $content;
@@ -55,4 +74,14 @@ public function getContent(): string
5574
{
5675
return $this->content;
5776
}
77+
78+
public function setLastChanged(int $timestamp): void
79+
{
80+
$this->lastChanged = $timestamp;
81+
}
82+
83+
public function getLastChanged(): int
84+
{
85+
return $this->lastChanged;
86+
}
5887
}

‎typo3/sysext/frontend/Tests/Functional/ContentObject/ContentContentObjectTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use TYPO3\CMS\Frontend\ContentObject\Event\ModifyRecordsAfterFetchingContentEvent;
2828
use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
2929
use TYPO3\CMS\Frontend\Page\PageInformation;
30+
use TYPO3\CMS\Frontend\Page\PageParts;
3031
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
3132

3233
final class ContentContentObjectTest extends FunctionalTestCase
@@ -59,7 +60,8 @@ static function (ModifyRecordsAfterFetchingContentEvent $event) use (&$modifyRec
5960
$pageInformation->setContentFromPid(1);
6061
$request = (new ServerRequest())
6162
->withAttribute('frontend.page.information', $pageInformation)
62-
->withAttribute('frontend.cache.collector', new CacheDataCollector());
63+
->withAttribute('frontend.cache.collector', new CacheDataCollector())
64+
->withAttribute('frontend.page.parts', new PageParts());
6365
$contentObjectRenderer->setRequest($request);
6466
$subject = $contentObjectRenderer->getContentObject('CONTENT');
6567
$result = $subject->render(['table' => 'tt_content']);

‎typo3/sysext/indexed_search/Classes/EventListener/FrontendGenerationPageIndexingTrigger.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ public function indexPageContent(AfterCacheableContentIsGeneratedEvent $event):
5858
$typoScriptConfigArray = $request->getAttribute('frontend.typoscript')->getConfigArray();
5959
$pageArguments = $request->getAttribute('routing');
6060
$pageInformation = $request->getAttribute('frontend.page.information');
61+
$pageParts = $request->getAttribute('frontend.page.parts');
6162
$pageRecord = $pageInformation->getPageRecord();
62-
$tsfe = $request->getAttribute('frontend.controller');
6363

6464
// Determine if page should be indexed, and if so, configure and initialize indexer
6565
if (!($typoScriptConfigArray['index_enable'] ?? false)) {
@@ -111,7 +111,7 @@ public function indexPageContent(AfterCacheableContentIsGeneratedEvent $event):
111111
// Alternative title for indexing
112112
'indexedDocTitle' => $this->pageTitleProviderManager->getTitle($request),
113113
// Most recent modification time (seconds) of the content on the page. Used to evaluate whether it should be re-indexed.
114-
'mtime' => $tsfe->register['SYS_LASTCHANGED'] ?? $pageRecord['SYS_LASTCHANGED'],
114+
'mtime' => $pageParts->getLastChanged(),
115115
// Whether to index external documents like PDF, DOC etc.
116116
'index_externals' => $typoScriptConfigArray['index_externals'] ?? true,
117117
// Length of description text (max 250, default 200)

0 commit comments

Comments
 (0)