add cache block - #357
Conversation
Ibochkarev
left a comment
Ibochkarev
left a comment
There was a problem hiding this comment.
Идея полезной — {cache} в Fenom закрывает реальный use case. В текущем виде мержить рано: есть расхождение с microMODXCacheManager и баг с falsy-контентом.
До merge:
- Убрать
optionsизset()(в обёртке их сознательно нет из-за security). - Cache miss проверять через
=== false/=== null, не через!. - Либо убрать
optionsсовсем, либо аккуратно оставить только дляget()с безопасным default ('array()'как code-string). - Привести стиль к остальному файлу.
На шаблоны без {cache} PR сам по себе не влияет.
| $cname = $scope->tpl->parsePlainArg($tokens, $name); | ||
| $params = [ | ||
| 'lifetime'=>0, | ||
| 'options'=>[], |
There was a problem hiding this comment.
options default
Дефолт 'options' => [] — PHP-массив, а Fenom parseParams обычно ждёт code-string'и. Для пустого дефолта Compiler::toArray([]) ещё ок, но при options=$foo в toArray() попадёт строка выражения и foreach разберёт её по символам.
Лучше: убрать options совсем, либо default как 'options' => 'array()' и явно валидировать значение.
| ]; | ||
| $params = $scope->tpl->parseParams($tokens, $params); | ||
|
|
||
| if (!$name) { |
There was a problem hiding this comment.
Только static name
parsePlainArg + if (!$name) означает, что {cache $key} не скомпилируется. Ок как ограничение, но стоит явно описать в docs/PR: имя только строковый литерал.
| $scope['var'] = $scope->tpl->tmpVar(); | ||
|
|
||
| return "{$scope['var']} = \$var[\"_modx\"]->cacheManager->get({$scope['name']},".Fenom\Compiler::toArray($scope['params']['options']).");\n | ||
| if(!{$scope['var']} ) { \n |
There was a problem hiding this comment.
Cache miss
if (!{$scope['var']}) ломает кэш пустого/нулевого контента ('', '0' и т.п.) — каждый раз будет miss.
Нужно что-то вроде:
if ({$scope['var']} === false || {$scope['var']} === null) {(как обычно отдаёт miss у MODX cacheManager).
| function($tokens, $scope){ | ||
| return " | ||
| {$scope['var']} = ob_get_clean(); \n | ||
| \$var[\"_modx\"]->cacheManager->set({$scope['name']},{$scope['var']},{$scope['params']['lifetime']},".Fenom\Compiler::toArray($scope['params']['options']).");\n |
There was a problem hiding this comment.
set() + options — blocker
microMODXCacheManager::set($key, &$var, $lifetime = 0) принимает только 3 аргумента. Четвёртый options сознательно убран:
// $options is not used due to security reasons
return $this->cacheManager->set($key, $var, $lifetime);Здесь options снова протаскиваются в set(). Нужно:
\$var[\"_modx\"]->cacheManager->set({$scope['name']}, {$scope['var']}, {$scope['params']['lifetime']});без 4-го аргумента.
2 participants
{cache 'app/block1' lifetime=60}
content to be cached for one minute
{/cache}