Merge pull request #4004 from nextcloud/backport-27172

Remove SharedCache::getNumericStorageId to let CacheWrapper do it

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()

apps/files_sharing/lib/SharedStorage.php

@@ -350,6 +350,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;

apps/files_sharing/tests/CacheTest.php

@@ -30,6 +30,8 @@
 
 namespace OCA\Files_Sharing\Tests;
 
+use OCA\Files_Sharing\SharedStorage;
+
 /**
  * Class CacheTest
  *
@@ -525,4 +527,29 @@ 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'));
+		/** @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());
+	}
 }

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

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

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` = ?';