From 89c42a76c34aba7dd4c23649f97495283cf023ba Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 16 Feb 2017 11:47:16 +0100 Subject: [PATCH 1/3] Remove SharedCache::getNumericStorageId to let CacheWrapper do it The CacheWrapper will properly forward the call to the wrapped cache. Signed-off-by: Morris Jobke --- apps/files_sharing/lib/Cache.php | 8 ------- apps/files_sharing/tests/CacheTest.php | 24 +++++++++++++++++++ .../features/external-storage.feature | 21 +++++++++++++++- build/integration/features/sharing-v1.feature | 10 ++++++++ lib/private/Files/Cache/Cache.php | 8 +++++++ 5 files changed, 62 insertions(+), 9 deletions(-) diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php index bc10ddbd94f..000cbf1baa5 100644 --- a/apps/files_sharing/lib/Cache.php +++ b/apps/files_sharing/lib/Cache.php @@ -71,14 +71,6 @@ class Cache extends CacheJail { return $this->cache; } - public function getNumericStorageId() { - if (isset($this->numericId)) { - return $this->numericId; - } else { - return false; - } - } - public function get($file) { if ($this->rootUnchanged && ($file === '' || $file === $this->sourceRootInfo->getId())) { return $this->formatCacheEntry(clone $this->sourceRootInfo); diff --git a/apps/files_sharing/tests/CacheTest.php b/apps/files_sharing/tests/CacheTest.php index ae0247a84e2..10db4104aae 100644 --- a/apps/files_sharing/tests/CacheTest.php +++ b/apps/files_sharing/tests/CacheTest.php @@ -525,4 +525,28 @@ class CacheTest extends TestCase { $this->assertEquals('', $sharedCache->getPathById($folderInfo->getId())); $this->assertEquals('bar/test.txt', $sharedCache->getPathById($fileInfo->getId())); } + + public function testNumericStorageId() { + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + \OC\Files\Filesystem::mkdir('foo'); + + $rootFolder = \OC::$server->getUserFolder(self::TEST_FILES_SHARING_API_USER1); + $node = $rootFolder->get('foo'); + $share = $this->shareManager->newShare(); + $share->setNode($node) + ->setShareType(\OCP\Share::SHARE_TYPE_USER) + ->setSharedWith(self::TEST_FILES_SHARING_API_USER2) + ->setSharedBy(self::TEST_FILES_SHARING_API_USER1) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + $this->shareManager->createShare($share); + \OC_Util::tearDownFS(); + + list($sourceStorage) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER1 . '/files/foo'); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + $this->assertTrue(\OC\Files\Filesystem::file_exists('/foo')); + list($sharedStorage) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/foo'); + + $this->assertEquals($sourceStorage->getCache()->getNumericStorageId(), $sharedStorage->getCache()->getNumericStorageId()); + } } diff --git a/build/integration/features/external-storage.feature b/build/integration/features/external-storage.feature index da085d9e983..09ffdb29803 100644 --- a/build/integration/features/external-storage.feature +++ b/build/integration/features/external-storage.feature @@ -23,7 +23,6 @@ Feature: external-storage | token | A_TOKEN | | mimetype | httpd/unix-directory | - @local_storage Scenario: Shares dont overwrite external storages Given user "user0" exists And user "user1" exists @@ -40,3 +39,23 @@ Feature: external-storage And folder "/test" of user "user1" is shared with user "user0" And As an "user0" Then as "user0" the file "/test/textfile1.txt" does not exist + + Scenario: Move a file into storage works + Given user "user0" exists + And user "user1" exists + And As an "user0" + And user "user0" created a folder "/local_storage/foo1" + When User "user0" moved file "/textfile0.txt" to "/local_storage/foo1/textfile0.txt" + Then as "user1" the file "/local_storage/foo1/textfile0.txt" exists + And as "user0" the file "/local_storage/foo1/textfile0.txt" exists + + Scenario: Move a file out of the storage works + Given user "user0" exists + And user "user1" exists + And As an "user0" + And user "user0" created a folder "/local_storage/foo2" + And User "user0" moved file "/textfile0.txt" to "/local_storage/foo2/textfile0.txt" + When User "user1" moved file "/local_storage/foo2/textfile0.txt" to "/local.txt" + Then as "user1" the file "/local_storage/foo2/textfile0.txt" does not exist + And as "user0" the file "/local_storage/foo2/textfile0.txt" does not exist + And as "user1" the file "/local.txt" exists diff --git a/build/integration/features/sharing-v1.feature b/build/integration/features/sharing-v1.feature index 4ce32654ba4..74579f63527 100644 --- a/build/integration/features/sharing-v1.feature +++ b/build/integration/features/sharing-v1.feature @@ -988,3 +988,13 @@ Feature: sharing And Updating last share with | publicUpload | true | Then the OCS status code should be "404" + + Scenario: moving a file into a share as recipient + Given As an "admin" + And user "user0" exists + And user "user1" exists + And user "user0" created a folder "/shared" + And folder "/shared" of user "user0" is shared with user "user1" + When User "user1" moved file "/textfile0.txt" to "/shared/shared_file.txt" + Then as "user1" the file "/shared/shared_file.txt" exists + And as "user0" the file "/shared/shared_file.txt" exists diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 9c3b786ae87..dfe368d9d22 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -500,6 +500,7 @@ class Cache implements ICache { * @param string $sourcePath * @param string $targetPath * @throws \OC\DatabaseException + * @throws \Exception if the given storages have an invalid id */ public function moveFromCache(ICache $sourceCache, $sourcePath, $targetPath) { if ($sourceCache instanceof Cache) { @@ -514,6 +515,13 @@ class Cache implements ICache { list($sourceStorageId, $sourcePath) = $sourceCache->getMoveInfo($sourcePath); list($targetStorageId, $targetPath) = $this->getMoveInfo($targetPath); + if (is_null($sourceStorageId) || $sourceStorageId === false) { + throw new \Exception('Invalid source storage id: ' . $sourceStorageId); + } + if (is_null($targetStorageId) || $targetStorageId === false) { + throw new \Exception('Invalid target storage id: ' . $targetStorageId); + } + // sql for final update $moveSql = 'UPDATE `*PREFIX*filecache` SET `storage` = ?, `path` = ?, `path_hash` = ?, `name` = ?, `parent` =? WHERE `fileid` = ?'; From 869ba16273ededa4637c5ddeaab11533fb4e455f Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 23 Mar 2017 13:10:43 -0600 Subject: [PATCH 2/3] do not remove the method and only keep the tests Signed-off-by: Morris Jobke --- apps/files_sharing/lib/Cache.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php index 000cbf1baa5..bc10ddbd94f 100644 --- a/apps/files_sharing/lib/Cache.php +++ b/apps/files_sharing/lib/Cache.php @@ -71,6 +71,14 @@ class Cache extends CacheJail { return $this->cache; } + public function getNumericStorageId() { + if (isset($this->numericId)) { + return $this->numericId; + } else { + return false; + } + } + public function get($file) { if ($this->rootUnchanged && ($file === '' || $file === $this->sourceRootInfo->getId())) { return $this->formatCacheEntry(clone $this->sourceRootInfo); From ae3016959e6cbcadcd5169fa9e35a332abcc4495 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 27 Mar 2017 14:05:01 +0200 Subject: [PATCH 3/3] fix shared storage numeric id Signed-off-by: Robin Appelman --- apps/files_sharing/lib/Cache.php | 3 +++ apps/files_sharing/lib/SharedStorage.php | 5 +++++ apps/files_sharing/tests/CacheTest.php | 3 +++ 3 files changed, 11 insertions(+) diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php index bc10ddbd94f..d7dcb426d85 100644 --- a/apps/files_sharing/lib/Cache.php +++ b/apps/files_sharing/lib/Cache.php @@ -51,6 +51,8 @@ class Cache extends CacheJail { private $ownerDisplayName; + private $numericId; + /** * @param \OCA\Files_Sharing\SharedStorage $storage * @param ICacheEntry $sourceRootInfo @@ -58,6 +60,7 @@ class Cache extends CacheJail { public function __construct($storage, ICacheEntry $sourceRootInfo) { $this->storage = $storage; $this->sourceRootInfo = $sourceRootInfo; + $this->numericId = $sourceRootInfo->getStorageId(); parent::__construct( null, $this->sourceRootInfo->getPath() diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index 888cbfda143..4efea477e96 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -337,6 +337,11 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto return $this->superShare->getNodeType(); } + /** + * @param string $path + * @param null $storage + * @return Cache + */ public function getCache($path = '', $storage = null) { if ($this->cache) { return $this->cache; diff --git a/apps/files_sharing/tests/CacheTest.php b/apps/files_sharing/tests/CacheTest.php index 10db4104aae..26ba5b21e46 100644 --- a/apps/files_sharing/tests/CacheTest.php +++ b/apps/files_sharing/tests/CacheTest.php @@ -30,6 +30,8 @@ namespace OCA\Files_Sharing\Tests; +use OCA\Files_Sharing\SharedStorage; + /** * Class CacheTest * @@ -545,6 +547,7 @@ class CacheTest extends TestCase { self::loginHelper(self::TEST_FILES_SHARING_API_USER2); $this->assertTrue(\OC\Files\Filesystem::file_exists('/foo')); + /** @var SharedStorage $sharedStorage */ list($sharedStorage) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/foo'); $this->assertEquals($sourceStorage->getCache()->getNumericStorageId(), $sharedStorage->getCache()->getNumericStorageId());