Skip to content

Commit

Permalink
Merge pull request #28284 from owncloud/stable9-cross-storage-moveinfo
Browse files Browse the repository at this point in the history
[stable9]  Fix cross-storage move info when moving between two received shares
  • Loading branch information
Vincent Petry authored Jul 6, 2017
2 parents 9b38c9d + 55864e6 commit eb9bf10
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 8 deletions.
8 changes: 0 additions & 8 deletions apps/files_sharing/lib/cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,6 @@ public function __construct($storage, IStorage $sourceStorage, ICacheEntry $sour
);
}

public function getNumericStorageId() {
if (isset($this->numericId)) {
return $this->numericId;
} else {
return false;
}
}

protected function formatCacheEntry($entry) {
$path = $entry['path'];
$entry = parent::formatCacheEntry($entry);
Expand Down
24 changes: 24 additions & 0 deletions apps/files_sharing/tests/cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -505,4 +505,28 @@ public function testGetPathByIdShareSubFolder() {
$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());
}
}
40 changes: 40 additions & 0 deletions build/integration/features/bootstrap/WebDav.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ trait WebDav {
private $response;
/** @var map with user as key and another map as value, which has path as key and etag as value */
private $storedETAG = NULL;
/** @var integer */
private $storedFileID = NULL;

/**
* @Given /^using dav path "([^"]*)"$/
Expand Down Expand Up @@ -652,4 +654,42 @@ public function thereAreNoDuplicateHeaders() {
}
}
}

/**
* @param string $user
* @param string $path
* @return int
*/
private function getFileIdForPath($user, $path) {
$propertiesTable = new \Behat\Gherkin\Node\TableNode([["{http://owncloud.org/ns}fileid"]]);
$this->asGetsPropertiesOfFolderWith($user, 'file', $path, $propertiesTable);
if (is_array($this->response)) {
return (int) $this->response['{http://owncloud.org/ns}fileid'];
} else {
return null;
}
}

/**
* @Given /^User "([^"]*)" stores id of file "([^"]*)"$/
* @param string $user
* @param string $path
* @param string $fileid
* @return int
*/
public function userStoresFileIdForPath($user, $path) {
$this->storedFileID = $this->getFileIdForPath($user, $path);
}

/**
* @Given /^User "([^"]*)" checks id of file "([^"]*)"$/
* @param string $user
* @param string $path
* @param string $fileid
* @return int
*/
public function userChecksFileIdForPath($user, $path) {
$currentFileID = $this->getFileIdForPath($user, $path);
PHPUnit_Framework_Assert::assertEquals($currentFileID, $this->storedFileID);
}
}
23 changes: 23 additions & 0 deletions build/integration/features/external-storage.feature
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,27 @@ Feature: external-storage
| token | A_TOKEN |
| mimetype | httpd/unix-directory |

@local_storage
@no_encryption
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

@local_storage
@no_encryption
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

10 changes: 10 additions & 0 deletions build/integration/features/sharing-v1.feature
Original file line number Diff line number Diff line change
Expand Up @@ -977,3 +977,13 @@ Feature: sharing
And the HTTP status code should be "200"
And last share_id is not included in the answer

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

20 changes: 20 additions & 0 deletions build/integration/features/webdav-related.feature
Original file line number Diff line number Diff line change
Expand Up @@ -386,3 +386,23 @@ Feature: webdav-related
When as "user0" gets properties of folder "/test_folder:5" with
|{DAV:}resourcetype|
Then the single response should contain a property "{DAV:}resourcetype" with value "{DAV:}collection"

Scenario: Checking file id after a move between received shares
Given using old dav path
And user "user0" exists
And user "user1" exists
And user "user0" created a folder "/folderA"
And user "user0" created a folder "/folderB"
And folder "/folderA" of user "user0" is shared with user "user1"
And folder "/folderB" of user "user0" is shared with user "user1"
And user "user1" created a folder "/folderA/ONE"
And User "user1" stores id of file "/folderA/ONE"
And user "user1" created a folder "/folderA/ONE/TWO"
When User "user1" moves folder "/folderA/ONE" to "/folderB/ONE"
Then as "user1" the folder "/folderA" exists
And as "user1" the folder "/folderA/ONE" does not exist
# yes, a weird bug used to make this one fail
And as "user1" the folder "/folderA/ONE/TWO" does not exist
And as "user1" the folder "/folderB/ONE" exists
And as "user1" the folder "/folderB/ONE/TWO" exists
And User "user1" checks id of file "/folderB/ONE"
3 changes: 3 additions & 0 deletions build/integration/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ $OCC files_external:delete -y $ID_STORAGE
#Disable external storage app
$OCC app:disable files_external

# Clear storage folder
rm -Rf work/local_storage/*

if test "$OC_TEST_ALT_HOME" = "1"; then
env_alt_home_clear
fi
Expand Down
8 changes: 8 additions & 0 deletions lib/private/files/cache/cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ protected function getMoveInfo($path) {
* @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) {
Expand All @@ -505,6 +506,13 @@ public function moveFromCache(ICache $sourceCache, $sourcePath, $targetPath) {
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` = ?';

Expand Down
20 changes: 20 additions & 0 deletions lib/private/files/cache/wrapper/cachejail.php
Original file line number Diff line number Diff line change
Expand Up @@ -292,4 +292,24 @@ public function getPathById($id) {
$path = $this->cache->getPathById($id);
return $this->getJailedPath($path);
}

/**
* Move a file or folder in the cache
*
* Note that this should make sure the entries are removed from the source cache
*
* @param \OCP\Files\Cache\ICache $sourceCache
* @param string $sourcePath
* @param string $targetPath
*/
public function moveFromCache(\OCP\Files\Cache\ICache $sourceCache, $sourcePath, $targetPath) {
if ($sourceCache === $this) {
return $this->move($sourcePath, $targetPath);
}
return $this->cache->moveFromCache($sourceCache, $sourcePath, $this->getSourcePath($targetPath));
}

protected function getMoveInfo($path) {
return [$this->getNumericStorageId(), $this->getSourcePath($path)];
}
}

0 comments on commit eb9bf10

Please sign in to comment.