Problem with page cache variations

chemezov commented 2 years ago

What steps will reproduce the problem?

Action page/index has cache with dependency by timestamp_update column, and key by navItem ID. But if page has block with random (or other dependency) content. For example, slider with images in random order. Block, thats output text related to user's city or something. Page will be cached with static content, and dynamic content will be ignored.

Probmel here: https://github.com/luyadev/luya-headless-cms-api/blob/master/src/controllers/PageController.php#L71

What is the expected result?

Need some way for cache variations. May by add GET params to cache key? Like that:

return $this->getOrSetHasCache(array_merge(['headlessapi', 'navItem', $navItem->id], $this->request->get()), function() use ($navItem) {
    return [
        'page' => $navItem->toArray(['id', 'nav_id', 'lang_id', 'title', 'alias', 'description', 'keywords', 'title_tag']),
        'placeholders' => $navItem->currentPage->getContent(),
        'layout' => $navItem->currentPage->layout->toArray(['id', 'name']),
        'properties' => $navItem->nav->formatedProperties(),
    ];
}, 0,  new DbDependency(['sql' => 'select max(timestamp_update) from cms_nav_item']));

What do you get instead? (A Screenshot can help us a lot!)

LUYA Check ouput (run this script and post the result: luyacheck.php)

Additional infos

Q	A
LUYA Version	dev-master
PHP Version	7.4
Platform	Apache, OpenServer.
Operating system	Windows

nadar commented 2 years ago

A get param which can burst the cache is always a bit of a risk. There should be a attribute whiich defines whether this page allows full page chache or not, we should choose this one whether caching is enabled or not: https://github.com/luyadev/luya-module-cms/blob/master/src/models/NavItem.php#L41

chemezov commented 2 years ago

I completely agree with you about risks. What about Block $cacheEnabled property? Can we calculate getCacheEnabled() in page by NavItem::is_cacheable and all blocks inside the page? What do you think about it? If at least one block has $cacheEnabled = false, than disable cache for whole page.

nadar commented 2 years ago

If at least one block has $cacheEnabled = false, than disable cache for whole page.

This is absolut the best approach! 👍 Then we can ignore is_cacheable. Would you like to send a PR?

chemezov commented 2 years ago

Yes, I will try to implement it!

luyadev / luya-headless-cms-api