From df19cabb44f9106484290a269ef6d4cb07b059d7 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Fri, 23 Jan 2015 21:53:21 +0000 Subject: [PATCH 1/2] Store storage availability in database Storage status is saved in the database. Failed storages are rechecked every 10 minutes, while working storages are rechecked every request. Using the files_external app will recheck all external storages when the settings page is viewed, or whenever an external storage is saved. --- apps/files_external/lib/config.php | 12 +- db_structure.xml | 12 + lib/private/files/cache/storage.php | 48 +- lib/private/files/storage/common.php | 19 + .../files/storage/wrapper/availability.php | 462 ++++++++++++++++++ lib/private/files/storage/wrapper/wrapper.php | 18 + lib/private/util.php | 8 + lib/public/files/storage.php | 20 + .../files/storage/wrapper/availability.php | 149 ++++++ version.php | 2 +- 10 files changed, 737 insertions(+), 13 deletions(-) create mode 100644 lib/private/files/storage/wrapper/availability.php create mode 100644 tests/lib/files/storage/wrapper/availability.php diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index 5dc6d06ae065..e384512468b8 100644 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -494,8 +494,16 @@ public static function getBackendStatus($class, $options, $isPersonal) { if (class_exists($class)) { try { $storage = new $class($options); - if ($storage->test($isPersonal)) { - return self::STATUS_SUCCESS; + + try { + $result = $storage->test($isPersonal); + $storage->setAvailability($result); + if ($result) { + return self::STATUS_SUCCESS; + } + } catch (\Exception $e) { + $storage->setAvailability(false); + throw $e; } } catch (Exception $exception) { \OCP\Util::logException('files_external', $exception); diff --git a/db_structure.xml b/db_structure.xml index 6d1cf6973c55..870c0ab018d7 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -102,6 +102,18 @@ 4 + + available + boolean + true + true + + + + last_checked + integer + + storages_id_index true diff --git a/lib/private/files/cache/storage.php b/lib/private/files/cache/storage.php index ebef245f3991..338d8308281c 100644 --- a/lib/private/files/cache/storage.php +++ b/lib/private/files/cache/storage.php @@ -43,9 +43,10 @@ class Storage { /** * @param \OC\Files\Storage\Storage|string $storage + * @param bool $isAvailable * @throws \RuntimeException */ - public function __construct($storage) { + public function __construct($storage, $isAvailable = true) { if ($storage instanceof \OC\Files\Storage\Storage) { $this->storageId = $storage->getId(); } else { @@ -53,17 +54,14 @@ public function __construct($storage) { } $this->storageId = self::adjustStorageId($this->storageId); - $sql = 'SELECT `numeric_id` FROM `*PREFIX*storages` WHERE `id` = ?'; - $result = \OC_DB::executeAudited($sql, array($this->storageId)); - if ($row = $result->fetchRow()) { + if ($row = self::getStorageById($this->storageId)) { $this->numericId = $row['numeric_id']; } else { $connection = \OC_DB::getConnection(); - if ($connection->insertIfNotExist('*PREFIX*storages', ['id' => $this->storageId])) { + if ($connection->insertIfNotExist('*PREFIX*storages', ['id' => $this->storageId, 'available' => $isAvailable])) { $this->numericId = \OC_DB::insertid('*PREFIX*storages'); } else { - $result = \OC_DB::executeAudited($sql, array($this->storageId)); - if ($row = $result->fetchRow()) { + if ($row = self::getStorageById($this->storageId)) { $this->numericId = $row['numeric_id']; } else { throw new \RuntimeException('Storage could neither be inserted nor be selected from the database'); @@ -72,6 +70,16 @@ public function __construct($storage) { } } + /** + * @param string $storageId + * @return array|null + */ + public static function getStorageById($storageId) { + $sql = 'SELECT * FROM `*PREFIX*storages` WHERE `id` = ?'; + $result = \OC_DB::executeAudited($sql, array($storageId)); + return $result->fetchRow(); + } + /** * Adjusts the storage id to use md5 if too long * @param string $storageId storage id @@ -120,15 +128,35 @@ public static function getStorageId($numericId) { public static function getNumericStorageId($storageId) { $storageId = self::adjustStorageId($storageId); - $sql = 'SELECT `numeric_id` FROM `*PREFIX*storages` WHERE `id` = ?'; - $result = \OC_DB::executeAudited($sql, array($storageId)); - if ($row = $result->fetchRow()) { + if ($row = self::getStorageById($storageId)) { return $row['numeric_id']; } else { return null; } } + /** + * @return array|null [ available, last_checked ] + */ + public function getAvailability() { + if ($row = self::getStorageById($this->storageId)) { + return [ + 'available' => $row['available'], + 'last_checked' => $row['last_checked'] + ]; + } else { + return null; + } + } + + /** + * @param bool $isAvailable + */ + public function setAvailability($isAvailable) { + $sql = 'UPDATE `*PREFIX*storages` SET `available` = ?, `last_checked` = ? WHERE `id` = ?'; + \OC_DB::executeAudited($sql, array($isAvailable, time(), $this->storageId)); + } + /** * Check if a string storage id is known * diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 847cb8492fef..d006f1fb4862 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -404,6 +404,11 @@ public function cleanPath($path) { return implode('/', $output); } + /** + * Test a storage for availability + * + * @return bool + */ public function test() { if ($this->stat('')) { return true; @@ -650,4 +655,18 @@ public function releaseLock($path, $type, ILockingProvider $provider) { public function changeLock($path, $type, ILockingProvider $provider) { $provider->changeLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type); } + + /** + * @return array [ available, last_checked ] + */ + public function getAvailability() { + return $this->getStorageCache()->getAvailability(); + } + + /** + * @param bool $isAvailable + */ + public function setAvailability($isAvailable) { + $this->getStorageCache()->setAvailability($isAvailable); + } } diff --git a/lib/private/files/storage/wrapper/availability.php b/lib/private/files/storage/wrapper/availability.php new file mode 100644 index 000000000000..37319a8f7d11 --- /dev/null +++ b/lib/private/files/storage/wrapper/availability.php @@ -0,0 +1,462 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @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\Wrapper; + +/** + * Availability checker for storages + * + * Throws a StorageNotAvailableException for storages with known failures + */ +class Availability extends Wrapper { + const RECHECK_TTL_SEC = 600; // 10 minutes + + /** + * @return bool + */ + private function updateAvailability() { + try { + $result = $this->test(); + } catch (\Exception $e) { + $result = false; + } + $this->setAvailability($result); + return $result; + } + + /** + * @return bool + */ + private function isAvailable() { + $availability = $this->getAvailability(); + if (!$availability['available']) { + // trigger a recheck if TTL reached + if ((time() - $availability['last_checked']) > self::RECHECK_TTL_SEC) { + return $this->updateAvailability(); + } + } + return $availability['available']; + } + + /** + * @throws \OCP\Files\StorageNotAvailableException + */ + private function checkAvailability() { + if (!$this->isAvailable()) { + throw new \OCP\Files\StorageNotAvailableException(); + } + } + + /** {@inheritdoc} */ + public function mkdir($path) { + $this->checkAvailability(); + try { + return parent::mkdir($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function rmdir($path) { + $this->checkAvailability(); + try { + return parent::rmdir($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function opendir($path) { + $this->checkAvailability(); + try { + return parent::opendir($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function is_dir($path) { + $this->checkAvailability(); + try { + return parent::is_dir($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function is_file($path) { + $this->checkAvailability(); + try { + return parent::is_file($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function stat($path) { + $this->checkAvailability(); + try { + return parent::stat($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function filetype($path) { + $this->checkAvailability(); + try { + return parent::filetype($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function filesize($path) { + $this->checkAvailability(); + try { + return parent::filesize($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function isCreatable($path) { + $this->checkAvailability(); + try { + return parent::isCreatable($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function isReadable($path) { + $this->checkAvailability(); + try { + return parent::isReadable($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function isUpdatable($path) { + $this->checkAvailability(); + try { + return parent::isUpdatable($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function isDeletable($path) { + $this->checkAvailability(); + try { + return parent::isDeletable($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function isSharable($path) { + $this->checkAvailability(); + try { + return parent::isSharable($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function getPermissions($path) { + $this->checkAvailability(); + try { + return parent::getPermissions($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function file_exists($path) { + $this->checkAvailability(); + try { + return parent::file_exists($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function filemtime($path) { + $this->checkAvailability(); + try { + return parent::filemtime($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function file_get_contents($path) { + $this->checkAvailability(); + try { + return parent::file_get_contents($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function file_put_contents($path, $data) { + $this->checkAvailability(); + try { + return parent::file_put_contents($path, $data); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function unlink($path) { + $this->checkAvailability(); + try { + return parent::unlink($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function rename($path1, $path2) { + $this->checkAvailability(); + try { + return parent::rename($path1, $path2); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function copy($path1, $path2) { + $this->checkAvailability(); + try { + return parent::copy($path1, $path2); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function fopen($path, $mode) { + $this->checkAvailability(); + try { + return parent::fopen($path, $mode); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function getMimeType($path) { + $this->checkAvailability(); + try { + return parent::getMimeType($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function hash($type, $path, $raw = false) { + $this->checkAvailability(); + try { + return parent::hash($type, $path, $raw); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function free_space($path) { + $this->checkAvailability(); + try { + return parent::free_space($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function search($query) { + $this->checkAvailability(); + try { + return parent::search($query); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function touch($path, $mtime = null) { + $this->checkAvailability(); + try { + return parent::touch($path, $mtime); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function getLocalFile($path) { + $this->checkAvailability(); + try { + return parent::getLocalFile($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function getLocalFolder($path) { + $this->checkAvailability(); + try { + return parent::getLocalFolder($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function hasUpdated($path, $time) { + $this->checkAvailability(); + try { + return parent::hasUpdated($path, $time); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function getOwner($path) { + $this->checkAvailability(); + try { + return parent::getOwner($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function getETag($path) { + $this->checkAvailability(); + try { + return parent::getETag($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function getDirectDownload($path) { + $this->checkAvailability(); + try { + return parent::getDirectDownload($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + $this->checkAvailability(); + try { + return parent::copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + $this->checkAvailability(); + try { + return parent::moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } + + /** {@inheritdoc} */ + public function getMetaData($path) { + $this->checkAvailability(); + try { + return parent::getMetaData($path); + } catch (\OCP\Files\StorageNotAvailableException $e) { + $this->setAvailability(false); + throw $e; + } + } +} diff --git a/lib/private/files/storage/wrapper/wrapper.php b/lib/private/files/storage/wrapper/wrapper.php index d1414880beb0..b43dd4fe1420 100644 --- a/lib/private/files/storage/wrapper/wrapper.php +++ b/lib/private/files/storage/wrapper/wrapper.php @@ -497,6 +497,24 @@ public function getDirectDownload($path) { return $this->storage->getDirectDownload($path); } + /** + * Get availability of the storage + * + * @return array [ available, last_checked ] + */ + public function getAvailability() { + return $this->storage->getAvailability(); + } + + /** + * Set availability of the storage + * + * @param bool $isAvailable + */ + public function setAvailability($isAvailable) { + $this->storage->setAvailability($isAvailable); + } + /** * @param string $path the path of the target folder * @param string $fileName the name of the file itself diff --git a/lib/private/util.php b/lib/private/util.php index 39d64952dc68..8a85a8a5f0d3 100644 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -143,6 +143,14 @@ public static function setupFS($user = '') { return $storage; }); + // install storage availability wrapper, before most other wrappers + \OC\Files\Filesystem::addStorageWrapper('oc_availability', function ($mountPoint, $storage) { + if (!$storage->isLocal()) { + return new \OC\Files\Storage\Wrapper\Availability(['storage' => $storage]); + } + return $storage; + }); + \OC\Files\Filesystem::addStorageWrapper('oc_quota', function ($mountPoint, $storage) { // set up quota for home storages, even for other users // which can happen when using sharing diff --git a/lib/public/files/storage.php b/lib/public/files/storage.php index 41218996382c..ac3603e48d46 100644 --- a/lib/public/files/storage.php +++ b/lib/public/files/storage.php @@ -439,4 +439,24 @@ public function releaseLock($path, $type, ILockingProvider $provider); * @since 8.1.0 */ public function changeLock($path, $type, ILockingProvider $provider); + + /** + * Test a storage for availability + * + * @since 8.2.0 + * @return bool + */ + public function test(); + + /** + * @since 8.2.0 + * @return array [ available, last_checked ] + */ + public function getAvailability(); + + /** + * @since 8.2.0 + * @param bool $isAvailable + */ + public function setAvailability($isAvailable); } diff --git a/tests/lib/files/storage/wrapper/availability.php b/tests/lib/files/storage/wrapper/availability.php new file mode 100644 index 000000000000..9b394df8ca33 --- /dev/null +++ b/tests/lib/files/storage/wrapper/availability.php @@ -0,0 +1,149 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @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 Test\Files\Storage\Wrapper; + +class Availability extends \Test\TestCase { + protected function getWrapperInstance() { + $storage = $this->getMockBuilder('\OC\Files\Storage\Temporary') + ->disableOriginalConstructor() + ->getMock(); + $wrapper = new \OC\Files\Storage\Wrapper\Availability(['storage' => $storage]); + return [$storage, $wrapper]; + } + + /** + * Storage is available + */ + public function testAvailable() { + list($storage, $wrapper) = $this->getWrapperInstance(); + $storage->expects($this->once()) + ->method('getAvailability') + ->willReturn(['available' => true, 'last_checked' => 0]); + $storage->expects($this->never()) + ->method('test'); + $storage->expects($this->once()) + ->method('mkdir'); + + $wrapper->mkdir('foobar'); + } + + /** + * Storage marked unavailable, TTL not expired + * + * @expectedException \OCP\Files\StorageNotAvailableException + */ + public function testUnavailable() { + list($storage, $wrapper) = $this->getWrapperInstance(); + $storage->expects($this->once()) + ->method('getAvailability') + ->willReturn(['available' => false, 'last_checked' => time()]); + $storage->expects($this->never()) + ->method('test'); + $storage->expects($this->never()) + ->method('mkdir'); + + $wrapper->mkdir('foobar'); + } + + /** + * Storage marked unavailable, TTL expired + */ + public function testUnavailableRecheck() { + list($storage, $wrapper) = $this->getWrapperInstance(); + $storage->expects($this->once()) + ->method('getAvailability') + ->willReturn(['available' => false, 'last_checked' => 0]); + $storage->expects($this->once()) + ->method('test') + ->willReturn(true); + $storage->expects($this->once()) + ->method('setAvailability') + ->with($this->equalTo(true)); + $storage->expects($this->once()) + ->method('mkdir'); + + $wrapper->mkdir('foobar'); + } + + /** + * Storage marked available, but throws StorageNotAvailableException + * + * @expectedException \OCP\Files\StorageNotAvailableException + */ + public function testAvailableThrowStorageNotAvailable() { + list($storage, $wrapper) = $this->getWrapperInstance(); + $storage->expects($this->once()) + ->method('getAvailability') + ->willReturn(['available' => true, 'last_checked' => 0]); + $storage->expects($this->never()) + ->method('test'); + $storage->expects($this->once()) + ->method('mkdir') + ->will($this->throwException(new \OCP\Files\StorageNotAvailableException())); + $storage->expects($this->once()) + ->method('setAvailability') + ->with($this->equalTo(false)); + + $wrapper->mkdir('foobar'); + } + + /** + * Storage available, but call fails + * Method failure does not indicate storage unavailability + */ + public function testAvailableFailure() { + list($storage, $wrapper) = $this->getWrapperInstance(); + $storage->expects($this->once()) + ->method('getAvailability') + ->willReturn(['available' => true, 'last_checked' => 0]); + $storage->expects($this->never()) + ->method('test'); + $storage->expects($this->once()) + ->method('mkdir') + ->willReturn(false); + $storage->expects($this->never()) + ->method('setAvailability'); + + $wrapper->mkdir('foobar'); + } + + /** + * Storage available, but throws exception + * Standard exception does not indicate storage unavailability + * + * @expectedException \Exception + */ + public function testAvailableThrow() { + list($storage, $wrapper) = $this->getWrapperInstance(); + $storage->expects($this->once()) + ->method('getAvailability') + ->willReturn(['available' => true, 'last_checked' => 0]); + $storage->expects($this->never()) + ->method('test'); + $storage->expects($this->once()) + ->method('mkdir') + ->will($this->throwException(new \Exception())); + $storage->expects($this->never()) + ->method('setAvailability'); + + $wrapper->mkdir('foobar'); + } +} diff --git a/version.php b/version.php index 03593c471605..7ccd2e6b5488 100644 --- a/version.php +++ b/version.php @@ -22,7 +22,7 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version=array(8, 2, 0, 2); +$OC_Version=array(8, 2, 0, 3); // The human readable string $OC_VersionString='8.2 pre alpha'; From 75a5e6e12b18a9f5b7b113cd7e2c9c56c204084d Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Sat, 7 Mar 2015 00:12:46 +0000 Subject: [PATCH 2/2] Fix mounting wrapped storages resulting in many-layered wrapping This usually doesn't cause issues, but in unit tests sometimes a wrapped storage is passed to Filesystem::mount() and gets rewrapped, hitting the XDebug function nesting level limit when used. --- lib/private/files/mount/mountpoint.php | 7 ++++++- tests/lib/files/mount/mountpoint.php | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/private/files/mount/mountpoint.php b/lib/private/files/mount/mountpoint.php index 2871bbd90835..5e4949aa9ddb 100644 --- a/lib/private/files/mount/mountpoint.php +++ b/lib/private/files/mount/mountpoint.php @@ -29,6 +29,7 @@ use \OC\Files\Filesystem; use OC\Files\Storage\StorageFactory; use OC\Files\Storage\Storage; +use OC\Files\Storage\Wrapper\Wrapper; use OCP\Files\Mount\IMountPoint; class MountPoint implements IMountPoint { @@ -92,7 +93,11 @@ public function __construct($storage, $mountpoint, $arguments = null, $loader = $this->mountPoint = $mountpoint; if ($storage instanceof Storage) { $this->class = get_class($storage); - $this->storage = $this->loader->wrap($this, $storage); + $this->storage = $storage; + // only wrap if not already wrapped + if (!($this->storage instanceof Wrapper)) { + $this->storage = $this->loader->wrap($this, $this->storage); + } } else { // Update old classes to new namespace if (strpos($storage, 'OC_Filestorage_') !== false) { diff --git a/tests/lib/files/mount/mountpoint.php b/tests/lib/files/mount/mountpoint.php index 29610e6058da..d758c1b8d4d7 100644 --- a/tests/lib/files/mount/mountpoint.php +++ b/tests/lib/files/mount/mountpoint.php @@ -70,4 +70,25 @@ public function testInvalidStorage() { // storage wrapper never called $this->assertFalse($called); } + + public function testWrappedStorage() { + $storage = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Wrapper') + ->disableOriginalConstructor() + ->getMock(); + + $loader = $this->getMock('\OCP\Files\Storage\IStorageFactory'); + $loader->expects($this->never()) + ->method('getInstance'); + $loader->expects($this->never()) + ->method('wrap'); + + $mountPoint = new \OC\Files\Mount\MountPoint( + $storage, + '/mountpoint', + null, + $loader + ); + + $this->assertEquals($storage, $mountPoint->getStorage()); + } }