georgringer / news

TYPO3 Extension news
GNU General Public License v2.0
264 stars 355 forks source link

Cache clear on backend save should respect `TCEMAIN.clearCache_disable` (or provide alternatives) #1782

Open sbuerk opened 2 years ago

sbuerk commented 2 years ago

Feature Request

Is your feature request related to a problem? Please describe.

In a few installations(instances) we use the strategy to avoid cache clear on backend actions. This is mainly done by using proper configuration option TCEMAIN.clearCache_disable. This prevents cache clear on page and content element changes. (We have a proper warmup stragety in place).

However, editing a news will clear the news cache ignoring the core setting.

Describe the solution you'd like

DataHandler hook calling cache clear should respect the core setting OR provide one/two own settings to avoid this. (uid / pid / both tags clear etc).

Describe alternatives you've considered

Alternative would be to extend the hook class and overriding clearCachePostProc() in instance, which we are currently using to mitigate this issue.

As described as above, either core option `` should be respected or custom setting/option (PageTS) or as extension configuration would be nice.

Teachability, Documentation, Adoption, Migration Strategy

Uses which want to avoid cache clear on news writing, they can simply configure the corresponding flag in PageTS/UserTS - either globally or in correpsonding page trees.

# PageTS
TCEMAIN.clearCache_disable = 1

# UserTS
page.TCEMAIN.clearCache_disable = 1

or corresponding custom option

# PageTS
TCEMAIN.news_clearCache_disable = 1

# UserTS
page.TCEMAIN.news_clearCache_disable = 1

If generally accepted or noted if another approach may be more suiting the extension need, I would take the time and oppurtunity to provide a proper pull request.

Created as Feature request as do not wanted to emerge this as "BUG" (even if it may consided as bug).

georgringer commented 2 years ago

Thanks for creating the issue, never have seen this setting. Is it possible to check this setting during the hook? Otherwise I propose a setting in extension settings

sbuerk commented 2 years ago

I would say, should be doable with something like that:

$TSConfig = BackendUtility::getPagesTSconfig($pid)['TCEMAIN.'] ?? [];
if (!empty($TSConfig['clearCache_disable'])) {
  $clearCacheEnabled = false;
}

at least that is what core datahandler do to check this: https://github.com/TYPO3/typo3/blob/c648baf05d8fc51fdc6d9dc6a7cc4dd1a551d1a7/typo3/sysext/core/Classes/DataHandling/DataHandler.php#L8896

However, will look into this deeper the next days and create a PR for it.

georgringer commented 2 years ago

I merged some stuff adding events, e.g. https://github.com/georgringer/news/pull/1800. maybe you can use that now?

sbuerk commented 2 years ago

Thanks @georgringer, I had a look. The events added are nice and helps with manipulating "which" tags may or may not be set on frontend when displayed (detail, list).

This issue is about the backend datahandler hook which clears caches pased on uid and pid. Have not seen in current main branch on latest commit a "event" there (which may be weired anyway). So I still thing at least respecting the core setting before clearing could be a good thing - sadly not found time yet for a PR.

It's about here -> https://github.com/georgringer/news/blob/ec1847a0cc7aefe09ac505a2e15ac6fe7d18a6df/Classes/Hooks/DataHandler.php#L44

This should not be executed if "clearCache_disable" is set to true (at least this is my thinking). What do you think ?

georgringer commented 2 years ago

absolutly true. waiting for your PR - just wanted to share some recent related changes