From cbeedbfb83efc12947651fd34fc10940bf135951 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 25 Jan 2017 12:22:09 +0100 Subject: [PATCH 1/4] Ignore NoUserException for shares from ghosts Add unit tests for FailedStorage init from SharedStorage Signed-off-by: Morris Jobke --- apps/files_sharing/lib/SharedStorage.php | 8 +++- .../files_sharing/tests/SharedStorageTest.php | 38 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index 888cbfda143..560c195d8ba 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -32,15 +32,14 @@ namespace OCA\Files_Sharing; use OC\Files\Filesystem; -use OC\Files\Cache\FailedCache; use OC\Files\Storage\Wrapper\PermissionsMask; -use OCA\Files_Sharing\ISharedStorage; use OC\Files\Storage\FailedStorage; use OCP\Constants; use OCP\Files\Cache\ICacheEntry; use OCP\Files\NotFoundException; use OCP\Files\Storage\IStorage; use OCP\Lock\ILockingProvider; +use OC\User\NoUserException; /** * Convert target path to source path and pass the function call to the correct storage provider @@ -121,6 +120,11 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto 'mask' => $this->superShare->getPermissions() ]); } catch (NotFoundException $e) { + // original file not accessible or deleted, set FailedStorage + $this->storage = new FailedStorage(['exception' => $e]); + $this->rootPath = ''; + } catch (NoUserException $e) { + // sharer user deleted, set FailedStorage $this->storage = new FailedStorage(['exception' => $e]); $this->rootPath = ''; } catch (\Exception $e) { diff --git a/apps/files_sharing/tests/SharedStorageTest.php b/apps/files_sharing/tests/SharedStorageTest.php index eaa138b0f70..ad6243c46b1 100644 --- a/apps/files_sharing/tests/SharedStorageTest.php +++ b/apps/files_sharing/tests/SharedStorageTest.php @@ -28,6 +28,11 @@ namespace OCA\Files_Sharing\Tests; +use OCA\Files_Sharing\SharedStorage; +use OCP\Share\IShare; +use OC\Files\View; +use OCP\Files\NotFoundException; + /** * Class SharedStorageTest * @@ -559,4 +564,37 @@ class SharedStorageTest extends TestCase { $this->shareManager->deleteShare($share); } + + public function testInitWithNonExistingUser() { + $share = $this->createMock(IShare::class); + $share->method('getShareOwner')->willReturn('unexist'); + $ownerView = $this->createMock(View::class); + $storage = new SharedStorage([ + 'ownerView' => $ownerView, + 'superShare' => $share, + 'groupedShares' => [$share], + 'user' => 'user1', + ]); + + // trigger init + $this->assertInstanceOf(\OC\Files\Cache\FailedCache::class, $storage->getCache()); + $this->assertInstanceOf(\OC\Files\Storage\FailedStorage::class, $storage->getSourceStorage()); + } + + public function testInitWithNotFoundSource() { + $share = $this->createMock(IShare::class); + $share->method('getShareOwner')->willReturn(self::TEST_FILES_SHARING_API_USER1); + $ownerView = $this->createMock(View::class); + $ownerView->method('getPath')->will($this->throwException(new NotFoundException())); + $storage = new SharedStorage([ + 'ownerView' => $ownerView, + 'superShare' => $share, + 'groupedShares' => [$share], + 'user' => 'user1', + ]); + + // trigger init + $this->assertInstanceOf(\OC\Files\Cache\FailedCache::class, $storage->getCache()); + $this->assertInstanceOf(\OC\Files\Storage\FailedStorage::class, $storage->getSourceStorage()); + } } From 4652923011ee30ea28b94ab4bda202964b01040f Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 20 Mar 2017 18:02:21 +0100 Subject: [PATCH 2/4] also set nonmaskedstorage in error cases Signed-off-by: Robin Appelman --- apps/files_sharing/lib/SharedStorage.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index 560c195d8ba..77bbb2f227b 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -98,6 +98,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto private function getSourceRootInfo() { if (is_null($this->sourceRootInfo)) { if (is_null($this->superShare->getNodeCacheEntry())) { + $this->init(); $this->sourceRootInfo = $this->nonMaskedStorage->getCache()->get($this->rootPath); } else { $this->sourceRootInfo = $this->superShare->getNodeCacheEntry(); @@ -132,6 +133,10 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto $this->rootPath = ''; $this->logger->logException($e); } + + if (!$this->nonMaskedStorage) { + $this->nonMaskedStorage = $this->storage; + } } /** From 0d20865548283035a43d8d9c7775437458178c28 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 20 Mar 2017 18:31:34 +0100 Subject: [PATCH 3/4] return failed cache if the shared storage failed to setup Signed-off-by: Robin Appelman --- apps/files_sharing/lib/SharedStorage.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index 77bbb2f227b..2cfba16c686 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -31,6 +31,7 @@ namespace OCA\Files_Sharing; +use OC\Files\Cache\FailedCache; use OC\Files\Filesystem; use OC\Files\Storage\Wrapper\PermissionsMask; use OC\Files\Storage\FailedStorage; @@ -353,6 +354,9 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto if (!$storage) { $storage = $this; } + if ($this->storage instanceof FailedStorage) { + return new FailedCache(); + } $this->cache = new \OCA\Files_Sharing\Cache($storage, $this->getSourceRootInfo(), $this->superShare); return $this->cache; } From 1fd6369a600054db8e46517c4efa77826174182c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 23 Mar 2017 15:01:23 +0100 Subject: [PATCH 4/4] set cache to failedcache when shared storage init fails Signed-off-by: Robin Appelman --- apps/files_sharing/lib/SharedStorage.php | 3 +++ apps/files_sharing/tests/SharedStorageTest.php | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index 2cfba16c686..ddbc9b8a898 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -124,13 +124,16 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto } catch (NotFoundException $e) { // original file not accessible or deleted, set FailedStorage $this->storage = new FailedStorage(['exception' => $e]); + $this->cache = new FailedCache(); $this->rootPath = ''; } catch (NoUserException $e) { // sharer user deleted, set FailedStorage $this->storage = new FailedStorage(['exception' => $e]); + $this->cache = new FailedCache(); $this->rootPath = ''; } catch (\Exception $e) { $this->storage = new FailedStorage(['exception' => $e]); + $this->cache = new FailedCache(); $this->rootPath = ''; $this->logger->logException($e); } diff --git a/apps/files_sharing/tests/SharedStorageTest.php b/apps/files_sharing/tests/SharedStorageTest.php index ad6243c46b1..7d007cb6414 100644 --- a/apps/files_sharing/tests/SharedStorageTest.php +++ b/apps/files_sharing/tests/SharedStorageTest.php @@ -577,8 +577,8 @@ class SharedStorageTest extends TestCase { ]); // trigger init - $this->assertInstanceOf(\OC\Files\Cache\FailedCache::class, $storage->getCache()); $this->assertInstanceOf(\OC\Files\Storage\FailedStorage::class, $storage->getSourceStorage()); + $this->assertInstanceOf(\OC\Files\Cache\FailedCache::class, $storage->getCache()); } public function testInitWithNotFoundSource() { @@ -594,7 +594,7 @@ class SharedStorageTest extends TestCase { ]); // trigger init - $this->assertInstanceOf(\OC\Files\Cache\FailedCache::class, $storage->getCache()); $this->assertInstanceOf(\OC\Files\Storage\FailedStorage::class, $storage->getSourceStorage()); + $this->assertInstanceOf(\OC\Files\Cache\FailedCache::class, $storage->getCache()); } }