From 3d767a4d25c1b8bf2d4776bf75afe231693b8e57 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Tue, 16 Jul 2019 17:39:58 +0300 Subject: [PATCH] Flex objects no longer return temporary key if they do not have one; empty key is returned instead --- CHANGELOG.md | 1 + .../Grav/Framework/Flex/FlexCollection.php | 9 ++-- .../src/Grav/Framework/Flex/FlexFormFlash.php | 2 +- system/src/Grav/Framework/Flex/FlexObject.php | 50 +++++++++++++------ .../Storage/AbstractFilesystemStorage.php | 2 +- .../Framework/Flex/Storage/FolderStorage.php | 2 +- .../Framework/Flex/Storage/SimpleStorage.php | 4 +- 7 files changed, 47 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8672ca34c..68d885a11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Better support for Symfony local server `symfony server:start` * Make `Route` objects immutable * `FlexDirectory::getObject()` can now be called without any parameters to create a new object + * Flex objects no longer return temporary key if they do not have one; empty key is returned instead 1. [](#bugfix) * Fixed `Form` not to use deleted flash object until the end of the request fixing issues with reset * Fixed `FlexForm` to allow multiple form instances with non-existing objects diff --git a/system/src/Grav/Framework/Flex/FlexCollection.php b/system/src/Grav/Framework/Flex/FlexCollection.php index 3add23698..42f4775b6 100644 --- a/system/src/Grav/Framework/Flex/FlexCollection.php +++ b/system/src/Grav/Framework/Flex/FlexCollection.php @@ -301,16 +301,19 @@ class FlexCollection extends ObjectCollection implements FlexCollectionInterface $debugger = $grav['debugger']; $debugger->startTimer('flex-collection-' . ($debugKey = uniqid($type, false)), 'Render Collection ' . $type . ' (' . $layout . ')'); - $cache = $key = null; + $key = null; foreach ($context as $value) { if (!\is_scalar($value)) { $key = false; + break; } } if ($key !== false) { $key = md5($this->getCacheKey() . '.' . $layout . json_encode($context)); $cache = $this->getCache('render'); + } else { + $cache = null; } try { @@ -331,9 +334,9 @@ class FlexCollection extends ObjectCollection implements FlexCollectionInterface } if (!$block) { - $block = HtmlBlock::create($key); + $block = HtmlBlock::create($key ?: null); $block->setChecksum($checksum); - if ($key === false) { + if (!$key) { $block->disableCache(); } diff --git a/system/src/Grav/Framework/Flex/FlexFormFlash.php b/system/src/Grav/Framework/Flex/FlexFormFlash.php index c0d280104..8dc157885 100644 --- a/system/src/Grav/Framework/Flex/FlexFormFlash.php +++ b/system/src/Grav/Framework/Flex/FlexFormFlash.php @@ -37,7 +37,7 @@ class FlexFormFlash extends FormFlash if ($object) { $serialized['object'] = [ 'type' => $object->getFlexType(), - 'key' => $object->hasKey() ? $object->getKey() : null, + 'key' => $object->getKey() ?: null, 'storage_key' => $object->exists() ? $object->getStorageKey() : null, 'timestamp' => $object->getTimestamp(), 'serialized' => $object->jsonSerialize() diff --git a/system/src/Grav/Framework/Flex/FlexObject.php b/system/src/Grav/Framework/Flex/FlexObject.php index f0aebba17..0c5e7ad93 100644 --- a/system/src/Grav/Framework/Flex/FlexObject.php +++ b/system/src/Grav/Framework/Flex/FlexObject.php @@ -171,7 +171,7 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface */ public function getCacheKey(): string { - return $this->getTypePrefix() . $this->getFlexType() . '.' . $this->getStorageKey(); + return $this->hasKey() ? $this->getTypePrefix() . $this->getFlexType() . '.' . $this->getKey() : ''; } /** @@ -213,7 +213,7 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface */ public function getKey() { - return $this->_key ?: $this->getFlexType() . '@@' . spl_object_hash($this); + return (string)$this->_key; } /** @@ -222,7 +222,13 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface */ public function getFlexKey(): string { - return $this->_storage['flex_key'] ?? $this->_flexDirectory->getFlexType() . '.obj:' . $this->getStorageKey(); + $key = $this->_storage['flex_key'] ?? null; + + if (!$key && $key = $this->getStorageKey()) { + $key = $this->_flexDirectory->getFlexType() . '.obj:' . $key; + } + + return (string)$key; } /** @@ -231,7 +237,7 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface */ public function getStorageKey(): string { - return $this->_storage['storage_key'] ?? $this->getTypePrefix() . $this->getFlexType() . '@@' . spl_object_hash($this); + return (string)($this->_storage['storage_key'] ?? null); } /** @@ -373,7 +379,7 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface */ public function setStorageKey($key = null) { - $this->_storage['storage_key'] = $key; + $this->_storage['storage_key'] = $key ?? ''; return $this; } @@ -407,20 +413,28 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface $debugger = $grav['debugger']; $debugger->startTimer('flex-object-' . ($debugKey = uniqid($type, false)), 'Render Object ' . $type . ' (' . $layout . ')'); - $cache = $key = null; - foreach ($context as $value) { - if (!\is_scalar($value)) { - $key = false; + $key = $this->getCacheKey(); + + // Disable caching if context isn't all scalars. + if ($key) { + foreach ($context as $value) { + if (!\is_scalar($value)) { + $key = ''; + break; + } } } - if ($key !== false) { - $key = md5($this->getCacheKey() . '.' . $layout . json_encode($context)); + if ($key) { + // Create a new key which includes layout and context. + $key = md5($key . '.' . $layout . json_encode($context)); $cache = $this->getCache('render'); + } else { + $cache = null; } try { - $data = $cache && $key ? $cache->get($key) : null; + $data = $cache ? $cache->get($key) : null; $block = $data ? HtmlBlock::fromArray($data) : null; } catch (InvalidArgumentException $e) { @@ -441,7 +455,7 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface if (!$block) { $block = HtmlBlock::create($key ?: null); $block->setChecksum($checksum); - if ($key === false) { + if (!$cache) { $block->disableCache(); } @@ -463,7 +477,7 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface $block->setContent($output); try { - $cache && $key && $block->isCached() && $cache->set($key, $block->toArray()); + $cache && $block->isCached() && $cache->set($key, $block->toArray()); } catch (InvalidArgumentException $e) { $debugger->addException($e); } @@ -567,7 +581,9 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface { $this->triggerEvent('onBeforeSave'); - $result = $this->getFlexDirectory()->getStorage()->replaceRows([$this->getStorageKey() => $this->prepareStorage()]); + $result = $this->getFlexDirectory()->getStorage()->replaceRows( + [$this->getStorageKey() ?: '@@' . spl_object_hash($this) => $this->prepareStorage()] + ); $value = reset($result); $storageKey = (string)key($result); @@ -615,6 +631,10 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface */ public function delete() { + if (!$this->exists()) { + return $this; + } + $this->triggerEvent('onBeforeDelete'); $this->getFlexDirectory()->getStorage()->deleteRows([$this->getStorageKey() => $this->prepareStorage()]); diff --git a/system/src/Grav/Framework/Flex/Storage/AbstractFilesystemStorage.php b/system/src/Grav/Framework/Flex/Storage/AbstractFilesystemStorage.php index cc460ccd0..e49284b93 100644 --- a/system/src/Grav/Framework/Flex/Storage/AbstractFilesystemStorage.php +++ b/system/src/Grav/Framework/Flex/Storage/AbstractFilesystemStorage.php @@ -149,6 +149,6 @@ abstract class AbstractFilesystemStorage implements FlexStorageInterface */ protected function validateKey(string $key): bool { - return (bool) preg_match('/^[^\\/\\?\\*:;{}\\\\\\n]+$/u', $key); + return $key && (bool) preg_match('/^[^\\/\\?\\*:;{}\\\\\\n]+$/u', $key); } } diff --git a/system/src/Grav/Framework/Flex/Storage/FolderStorage.php b/system/src/Grav/Framework/Flex/Storage/FolderStorage.php index 76397c235..7599f3334 100644 --- a/system/src/Grav/Framework/Flex/Storage/FolderStorage.php +++ b/system/src/Grav/Framework/Flex/Storage/FolderStorage.php @@ -179,7 +179,7 @@ class FolderStorage extends AbstractFilesystemStorage $list = []; foreach ($rows as $key => $row) { $key = (string)$key; - if (strpos($key, '@@')) { + if (strpos($key, '@@') !== false) { $key = $this->getNewKey(); } $path = $this->getPathFromKey($key); diff --git a/system/src/Grav/Framework/Flex/Storage/SimpleStorage.php b/system/src/Grav/Framework/Flex/Storage/SimpleStorage.php index 088b9527e..07a244d7d 100644 --- a/system/src/Grav/Framework/Flex/Storage/SimpleStorage.php +++ b/system/src/Grav/Framework/Flex/Storage/SimpleStorage.php @@ -192,7 +192,7 @@ class SimpleStorage extends AbstractFilesystemStorage $list = []; foreach ($rows as $key => $row) { - if (strpos($key, '@@')) { + if (strpos($key, '@@') !== false) { $key = $this->getNewKey(); } $this->data[$key] = $list[$key] = $row; @@ -255,7 +255,7 @@ class SimpleStorage extends AbstractFilesystemStorage return sprintf('%s/%s/%s', $this->dataFolder, basename($this->dataPattern, $this->dataFormatter->getDefaultFileExtension()), $key); } - protected function save() : void + protected function save(): void { if (null === $this->data) { $this->buildIndex();