From 0c5d0fd47fae3ca2849be031e93f9466c738ddc9 Mon Sep 17 00:00:00 2001 From: Hemant Mann Date: Fri, 22 Sep 2017 02:24:53 +0530 Subject: [PATCH 1/4] New Cacheable Flysystem Adapter --- .../Files/Storage/CacheableFlysystem.php | 160 ++++++++++++++++++ .../Storage/CacheableFlysystemAdapter.php | 46 +++++ 2 files changed, 206 insertions(+) create mode 100644 lib/private/Files/Storage/CacheableFlysystem.php create mode 100644 lib/public/Files/Storage/CacheableFlysystemAdapter.php diff --git a/lib/private/Files/Storage/CacheableFlysystem.php b/lib/private/Files/Storage/CacheableFlysystem.php new file mode 100644 index 000000000000..ac8b2dba693d --- /dev/null +++ b/lib/private/Files/Storage/CacheableFlysystem.php @@ -0,0 +1,160 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Files\Storage; + +use League\Flysystem\FileNotFoundException; +use Icewind\Streams\IteratorDirectory; + +/** + * Generic Cacheable adapter between flysystem adapters and owncloud's storage system + * + * To use: subclass and call $this->buildFlysystem with the flysystem adapter of choice + */ +abstract class CacheableFlysystem extends \OC\Files\Storage\Flysystem { + /** + * Stores the results in cache for the current request to prevent multiple requests to the API + * @var array + */ + protected $cacheContents = []; + + /** + * Get the location which will be used as a key in cache + * If Storage is not case sensitive then convert the key to lowercase + * @param string $path Path to file/folder + * @return string + */ + public function getCacheLocation($path) { + $location = $this->buildPath($path); + if ($location === '') { + $location = '/'; + } + + if ($this->isCaseInsensitiveStorage) { + $location = strtolower($location); + } + return $location; + } + + /** + * Check if Cache Contains the data for given path, if not then get the data + * from flysystem and store it in cache for this request lifetime + * @param string $path Path to file/folder + * @return array|boolean + */ + public function getFlysystemMetadata($path) { + $location = $this->getCacheLocation($path); + if (!isset($this->cacheContents[$location])) { + try { + $this->cacheContents[$location] = $this->flysystem->getMetadata($location); + } catch (FileNotFoundException $e) { + $this->cacheContents[$location] = false; + } + } + return $this->cacheContents[$location]; + } + + /** + * Store the list of files/folders in the cache so that subsequent requests in the + * same request life cycle does not call the flysystem API + * @param array $contents Return value of $this->flysystem->listContents + */ + public function updateCache($contents) { + foreach ($contents as $object) { + $path = $object['path']; + $this->cacheContents[$path] = $object; + } + } + + /** + * Check for existence of file/folder for the given path + * @param string $path Path to file/folder + * @return boolean + */ + public function hasPath($path) { + return (bool) $this->getFlysystemMetadata($path); + } + + /** + * {@inheritdoc} + */ + public function opendir($path) { + try { + $content = $this->flysystem->listContents($this->buildPath($path)); + $this->updateCache($content); + } catch (FileNotFoundException $e) { + return false; + } + $names = array_map(function ($object) { + return $object['basename']; + }, $content); + return IteratorDirectory::wrap($names); + } + + /** + * {@inheritdoc} + */ + public function file_exists($path) { + return $this->hasPath($path); + } + + /** + * {@inheritdoc} + */ + public function filesize($path) { + if ($this->is_dir($path)) { + return 0; + } else { + $info = $this->getFlysystemMetadata($path); + return $info['size']; + } + } + + /** + * {@inheritdoc} + */ + public function filemtime($path) { + $info = $this->getFlysystemMetadata($path); + return $info['timestamp']; + } + + /** + * {@inheritdoc} + */ + public function stat($path) { + $info = $this->getFlysystemMetadata($path); + return [ + 'mtime' => $info['timestamp'], + 'size' => $info['size'] + ]; + } + + /** + * {@inheritdoc} + */ + public function filetype($path) { + if ($path === '' or $path === '/' or $path === '.') { + return 'dir'; + } + $info = $this->getFlysystemMetadata($path); + return $info['type']; + } +} diff --git a/lib/public/Files/Storage/CacheableFlysystemAdapter.php b/lib/public/Files/Storage/CacheableFlysystemAdapter.php new file mode 100644 index 000000000000..61afe37f60b9 --- /dev/null +++ b/lib/public/Files/Storage/CacheableFlysystemAdapter.php @@ -0,0 +1,46 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCP\Files\Storage; + +/** + * Public Cacheable storage adapter to be extended to connect to + * flysystem adapters + * + * @since 10.0.x + */ +abstract class CacheableFlysystemAdapter extends \OC\Files\Storage\CacheableFlysystem { + /** + * Check that if storage is case insensitive or not + * @var boolean + */ + protected $isCaseInsensitiveStorage = false; + + /** + * Get the identifier for the storage, + * the returned id should be the same for every storage object that is created with the same parameters + * and two storage objects with the same id should refer to two storages that display the same files. + * + * @return string storage id + * @since 10.0 + */ + abstract public function getId(); +} From 15b068afcc6143f9438bc2b85629208577edd050 Mon Sep 17 00:00:00 2001 From: Hemant Mann Date: Sat, 23 Sep 2017 19:58:28 +0530 Subject: [PATCH 2/4] Fix some edge cases when using cache results --- .../Files/Storage/CacheableFlysystem.php | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/lib/private/Files/Storage/CacheableFlysystem.php b/lib/private/Files/Storage/CacheableFlysystem.php index ac8b2dba693d..d7112522d3c5 100644 --- a/lib/private/Files/Storage/CacheableFlysystem.php +++ b/lib/private/Files/Storage/CacheableFlysystem.php @@ -47,7 +47,11 @@ public function getCacheLocation($path) { if ($location === '') { $location = '/'; } + return $location; + } + public function buildPath($path) { + $location = parent::buildPath($path); if ($this->isCaseInsensitiveStorage) { $location = strtolower($location); } @@ -60,13 +64,14 @@ public function getCacheLocation($path) { * @param string $path Path to file/folder * @return array|boolean */ - public function getFlysystemMetadata($path) { + public function getFlysystemMetadata($path, $overRideCache = false) { $location = $this->getCacheLocation($path); - if (!isset($this->cacheContents[$location])) { + if (!isset($this->cacheContents[$location]) || $overRideCache) { try { $this->cacheContents[$location] = $this->flysystem->getMetadata($location); } catch (FileNotFoundException $e) { - $this->cacheContents[$location] = false; + // do not store this info in cache as it might interfere with Upload process + return false; } } return $this->cacheContents[$location]; @@ -84,21 +89,13 @@ public function updateCache($contents) { } } - /** - * Check for existence of file/folder for the given path - * @param string $path Path to file/folder - * @return boolean - */ - public function hasPath($path) { - return (bool) $this->getFlysystemMetadata($path); - } - /** * {@inheritdoc} */ public function opendir($path) { try { - $content = $this->flysystem->listContents($this->buildPath($path)); + $location = $this->buildPath($path); + $content = $this->flysystem->listContents($location); $this->updateCache($content); } catch (FileNotFoundException $e) { return false; @@ -112,8 +109,17 @@ public function opendir($path) { /** * {@inheritdoc} */ - public function file_exists($path) { - return $this->hasPath($path); + public function fopen($path, $mode) { + switch ($mode) { + case 'r': + case 'rb': + return parent::fopen($path, $mode); + + default: + unset($this->cacheContents[$this->getCacheLocation($path)]); + return parent::fopen($path, $mode); + } + return false; } /** From 2ed4728a70fd03ef02e32328182e095c82e4397d Mon Sep 17 00:00:00 2001 From: Hemant Mann Date: Tue, 10 Oct 2017 00:06:16 +0530 Subject: [PATCH 3/4] Fix indentation --- lib/private/Files/Storage/CacheableFlysystem.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/Storage/CacheableFlysystem.php b/lib/private/Files/Storage/CacheableFlysystem.php index d7112522d3c5..7fcd8a22c7c0 100644 --- a/lib/private/Files/Storage/CacheableFlysystem.php +++ b/lib/private/Files/Storage/CacheableFlysystem.php @@ -32,6 +32,7 @@ abstract class CacheableFlysystem extends \OC\Files\Storage\Flysystem { /** * Stores the results in cache for the current request to prevent multiple requests to the API + * * @var array */ protected $cacheContents = []; @@ -39,6 +40,7 @@ abstract class CacheableFlysystem extends \OC\Files\Storage\Flysystem { /** * Get the location which will be used as a key in cache * If Storage is not case sensitive then convert the key to lowercase + * * @param string $path Path to file/folder * @return string */ @@ -61,6 +63,7 @@ public function buildPath($path) { /** * Check if Cache Contains the data for given path, if not then get the data * from flysystem and store it in cache for this request lifetime + * * @param string $path Path to file/folder * @return array|boolean */ @@ -71,7 +74,7 @@ public function getFlysystemMetadata($path, $overRideCache = false) { $this->cacheContents[$location] = $this->flysystem->getMetadata($location); } catch (FileNotFoundException $e) { // do not store this info in cache as it might interfere with Upload process - return false; + return false; } } return $this->cacheContents[$location]; @@ -80,6 +83,7 @@ public function getFlysystemMetadata($path, $overRideCache = false) { /** * Store the list of files/folders in the cache so that subsequent requests in the * same request life cycle does not call the flysystem API + * * @param array $contents Return value of $this->flysystem->listContents */ public function updateCache($contents) { From b37abc8773a59d0e525d9efb216b1c9a2eeb8234 Mon Sep 17 00:00:00 2001 From: Hemant Mann Date: Tue, 10 Oct 2017 22:13:17 +0530 Subject: [PATCH 4/4] Update Requested Changes --- lib/private/Files/Storage/CacheableFlysystem.php | 3 ++- .../Files/Storage/CacheableFlysystemAdapter.php | 12 +----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/private/Files/Storage/CacheableFlysystem.php b/lib/private/Files/Storage/CacheableFlysystem.php index 7fcd8a22c7c0..1fc3e93b1afd 100644 --- a/lib/private/Files/Storage/CacheableFlysystem.php +++ b/lib/private/Files/Storage/CacheableFlysystem.php @@ -29,7 +29,7 @@ * * To use: subclass and call $this->buildFlysystem with the flysystem adapter of choice */ -abstract class CacheableFlysystem extends \OC\Files\Storage\Flysystem { +abstract class CacheableFlysystem extends \OCP\Files\Storage\FlysystemStorageAdapter { /** * Stores the results in cache for the current request to prevent multiple requests to the API * @@ -65,6 +65,7 @@ public function buildPath($path) { * from flysystem and store it in cache for this request lifetime * * @param string $path Path to file/folder + * @param boolean $overRideCache Whether to fetch the contents from Cache or directly from the API * @return array|boolean */ public function getFlysystemMetadata($path, $overRideCache = false) { diff --git a/lib/public/Files/Storage/CacheableFlysystemAdapter.php b/lib/public/Files/Storage/CacheableFlysystemAdapter.php index 61afe37f60b9..ebe9ad352b5c 100644 --- a/lib/public/Files/Storage/CacheableFlysystemAdapter.php +++ b/lib/public/Files/Storage/CacheableFlysystemAdapter.php @@ -25,7 +25,7 @@ * Public Cacheable storage adapter to be extended to connect to * flysystem adapters * - * @since 10.0.x + * @since 10.0.4 */ abstract class CacheableFlysystemAdapter extends \OC\Files\Storage\CacheableFlysystem { /** @@ -33,14 +33,4 @@ abstract class CacheableFlysystemAdapter extends \OC\Files\Storage\CacheableFlys * @var boolean */ protected $isCaseInsensitiveStorage = false; - - /** - * Get the identifier for the storage, - * the returned id should be the same for every storage object that is created with the same parameters - * and two storage objects with the same id should refer to two storages that display the same files. - * - * @return string storage id - * @since 10.0 - */ - abstract public function getId(); }