neos / flow-development-collection

The unified repository containing the Flow core packages, used for Flow development.
https://flow.neos.io/
MIT License
137 stars 189 forks source link

Confusing cache configuration behavior!? #1212

Open kdambekalns opened 6 years ago

kdambekalns commented 6 years ago

Description

The Flow caching documentation states regarding configuration:

If no frontend, backend or options are specified for a cache, these values will be taken to create the cache.

Expected behavior

Well, what is the expected behavior? I assumed the default values were taken, unless overridden. Much like context-specific values override generic ones on Flow configuration generally.

Admittedly, the sentence cited above does not state any merging takes palce. But this behavior was brought to my attention by a client, so I'm not the only one assuming the above as expected.

Actual behavior

If any option is set, the default options are not used.

This leads to e.g. no defaultLifetime option being passed on, which then makes any default defined in a backend active. This may lead to caches expiring after an hour, even though an unlimited lifetime may be desired.

Affected Versions

Neos: any Flow: any

kdambekalns commented 6 years ago

The question now is: how should this be solved? Introduce merging? Clarify documentation?

markuspfeifenberger commented 6 years ago

The Default->backendOptions->defaultLifetime: 0 should be considered and added as fallaback, if the value is not set in the 'backendOptions'. Than Default is a real default value.

This can be done in createCache($identifier) by adding

if (!isset($backendOptions['defaultLifetime'])) {
    $backendOptions['defaultLifetime'] = $this->cacheConfigurations['Default']['backendOptions']['defaultLifetime'];
}
kitsunet commented 6 years ago

So, the major problem is that any option not actually available for a backend automatically means an uncaught exception. This would be pretty ugly if we started to merge these options IMHO. Either we give up the former or we don't merge to avoid this. I would be a fan of introducing "presets" at some point that you can apply to a cache. Those might then be merged with any local configuration of the specific cache.

markuspfeifenberger commented 6 years ago

Presets would be a good idea, as long as they are easy to understand. The only crucial setting for a cache is the defaultLivetime, which should exist for every cache and may be handled especially.

albe commented 6 years ago

I once upon a time already started implementing some kind of Cache-Configuration Preset system... maybe worth digging that up again. Otherwise, what @kitsunet said, backends erroring out due to not supported options is not so nice.