[Pkg-owncloud-commits] [owncloud] 54/123: Added rmdir to trashbin storage wrapper
David Prévot
taffit at moszumanska.debian.org
Tue May 19 23:55:15 UTC 2015
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch stable8
in repository owncloud.
commit beb6a38d8519ab75109e95e1ae33e5f212a8f9bf
Author: Vincent Petry <pvince81 at owncloud.com>
Date: Mon May 11 17:53:21 2015 +0200
Added rmdir to trashbin storage wrapper
This makes sure that folders are moved to trash when deleted with
rmdir() instead of unlink().
This happens for example when deleting a folder over WebDAV.
The web UI uses unlink() so it wasn't affected.
---
apps/files_trashbin/lib/storage.php | 35 ++++++-
apps/files_trashbin/tests/storage.php | 179 ++++++++++++++++++++++++++++++++--
2 files changed, 199 insertions(+), 15 deletions(-)
diff --git a/apps/files_trashbin/lib/storage.php b/apps/files_trashbin/lib/storage.php
index 418d7d2..006971f 100644
--- a/apps/files_trashbin/lib/storage.php
+++ b/apps/files_trashbin/lib/storage.php
@@ -81,14 +81,39 @@ class Storage extends Wrapper {
/**
* Deletes the given file by moving it into the trashbin.
*
- * @param string $path
+ * @param string $path path of file or folder to delete
+ *
+ * @return bool true if the operation succeeded, false otherwise
*/
public function unlink($path) {
+ return $this->doDelete($path, 'unlink');
+ }
+
+ /**
+ * Deletes the given folder by moving it into the trashbin.
+ *
+ * @param string $path path of folder to delete
+ *
+ * @return bool true if the operation succeeded, false otherwise
+ */
+ public function rmdir($path) {
+ return $this->doDelete($path, 'rmdir');
+ }
+
+ /**
+ * Run the delete operation with the given method
+ *
+ * @param string $path path of file or folder to delete
+ * @param string $method either "unlink" or "rmdir"
+ *
+ * @return bool true if the operation succeeded, false otherwise
+ */
+ private function doDelete($path, $method) {
if (self::$disableTrash
|| !\OC_App::isEnabled('files_trashbin')
|| (pathinfo($path, PATHINFO_EXTENSION) === 'part')
) {
- return $this->storage->unlink($path);
+ return call_user_func_array([$this->storage, $method], [$path]);
}
$normalized = Filesystem::normalizePath($this->mountPoint . '/' . $path);
$result = true;
@@ -101,14 +126,14 @@ class Storage extends Wrapper {
// in cross-storage cases the file will be copied
// but not deleted, so we delete it here
if ($result) {
- $this->storage->unlink($path);
+ call_user_func_array([$this->storage, $method], [$path]);
}
} else {
- $result = $this->storage->unlink($path);
+ $result = call_user_func_array([$this->storage, $method], [$path]);
}
unset($this->deletedFiles[$normalized]);
} else if ($this->storage->file_exists($path)) {
- $result = $this->storage->unlink($path);
+ $result = call_user_func_array([$this->storage, $method], [$path]);
}
return $result;
diff --git a/apps/files_trashbin/tests/storage.php b/apps/files_trashbin/tests/storage.php
index d146852..7415aba 100644
--- a/apps/files_trashbin/tests/storage.php
+++ b/apps/files_trashbin/tests/storage.php
@@ -62,6 +62,8 @@ class Storage extends \Test\TestCase {
$this->userView = new \OC\Files\View('/' . $this->user . '/files/');
$this->userView->file_put_contents('test.txt', 'foo');
+ $this->userView->mkdir('folder');
+ $this->userView->file_put_contents('folder/inside.txt', 'bar');
}
protected function tearDown() {
@@ -75,7 +77,7 @@ class Storage extends \Test\TestCase {
/**
* Test that deleting a file puts it into the trashbin.
*/
- public function testSingleStorageDelete() {
+ public function testSingleStorageDeleteFile() {
$this->assertTrue($this->userView->file_exists('test.txt'));
$this->userView->unlink('test.txt');
list($storage,) = $this->userView->resolvePath('test.txt');
@@ -90,12 +92,34 @@ class Storage extends \Test\TestCase {
}
/**
+ * Test that deleting a folder puts it into the trashbin.
+ */
+ public function testSingleStorageDeleteFolder() {
+ $this->assertTrue($this->userView->file_exists('folder/inside.txt'));
+ $this->userView->rmdir('folder');
+ list($storage,) = $this->userView->resolvePath('folder/inside.txt');
+ $storage->getScanner()->scan(''); // make sure we check the storage
+ $this->assertFalse($this->userView->getFileInfo('folder'));
+
+ // check if folder is in trashbin
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/');
+ $this->assertEquals(1, count($results));
+ $name = $results[0]->getName();
+ $this->assertEquals('folder', substr($name, 0, strrpos($name, '.')));
+
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/' . $name . '/');
+ $this->assertEquals(1, count($results));
+ $name = $results[0]->getName();
+ $this->assertEquals('inside.txt', $name);
+ }
+
+ /**
* Test that deleting a file from another mounted storage properly
* lands in the trashbin. This is a cross-storage situation because
* the trashbin folder is in the root storage while the mounted one
* isn't.
*/
- public function testCrossStorageDelete() {
+ public function testCrossStorageDeleteFile() {
$storage2 = new Temporary(array());
\OC\Files\Filesystem::mount($storage2, array(), $this->user . '/files/substorage');
@@ -116,9 +140,41 @@ class Storage extends \Test\TestCase {
}
/**
+ * Test that deleting a folder from another mounted storage properly
+ * lands in the trashbin. This is a cross-storage situation because
+ * the trashbin folder is in the root storage while the mounted one
+ * isn't.
+ */
+ public function testCrossStorageDeleteFolder() {
+ $storage2 = new Temporary(array());
+ \OC\Files\Filesystem::mount($storage2, array(), $this->user . '/files/substorage');
+
+ $this->userView->mkdir('substorage/folder');
+ $this->userView->file_put_contents('substorage/folder/subfile.txt', 'bar');
+ $storage2->getScanner()->scan('');
+ $this->assertTrue($storage2->file_exists('folder/subfile.txt'));
+ $this->userView->rmdir('substorage/folder');
+
+ $storage2->getScanner()->scan('');
+ $this->assertFalse($this->userView->getFileInfo('substorage/folder'));
+ $this->assertFalse($storage2->file_exists('folder/subfile.txt'));
+
+ // check if folder is in trashbin
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files');
+ $this->assertEquals(1, count($results));
+ $name = $results[0]->getName();
+ $this->assertEquals('folder', substr($name, 0, strrpos($name, '.')));
+
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/' . $name . '/');
+ $this->assertEquals(1, count($results));
+ $name = $results[0]->getName();
+ $this->assertEquals('subfile.txt', $name);
+ }
+
+ /**
* Test that deleted versions properly land in the trashbin.
*/
- public function testDeleteVersions() {
+ public function testDeleteVersionsOfFile() {
\OCA\Files_Versions\Hooks::connectHooks();
// trigger a version (multiple would not work because of the expire logic)
@@ -137,7 +193,38 @@ class Storage extends \Test\TestCase {
$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/versions');
$this->assertEquals(1, count($results));
$name = $results[0]->getName();
- $this->assertEquals('test.txt', substr($name, 0, strlen('test.txt')));
+ $this->assertEquals('test.txt.v', substr($name, 0, strlen('test.txt.v')));
+ }
+
+ /**
+ * Test that deleted versions properly land in the trashbin.
+ */
+ public function testDeleteVersionsOfFolder() {
+ \OCA\Files_Versions\Hooks::connectHooks();
+
+ // trigger a version (multiple would not work because of the expire logic)
+ $this->userView->file_put_contents('folder/inside.txt', 'v1');
+
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_versions/folder/');
+ $this->assertEquals(1, count($results));
+
+ $this->userView->rmdir('folder');
+
+ // rescan trash storage
+ list($rootStorage,) = $this->rootView->resolvePath($this->user . '/files_trashbin');
+ $rootStorage->getScanner()->scan('');
+
+ // check if versions are in trashbin
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/versions');
+ $this->assertEquals(1, count($results));
+ $name = $results[0]->getName();
+ $this->assertEquals('folder.d', substr($name, 0, strlen('folder.d')));
+
+ // check if versions are in trashbin
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/versions/' . $name . '/');
+ $this->assertEquals(1, count($results));
+ $name = $results[0]->getName();
+ $this->assertEquals('inside.txt.v', substr($name, 0, strlen('inside.txt.v')));
}
/**
@@ -145,7 +232,7 @@ class Storage extends \Test\TestCase {
* storages. This is because rename() between storages would call
* unlink() which should NOT trigger the version deletion logic.
*/
- public function testKeepFileAndVersionsWhenMovingBetweenStorages() {
+ public function testKeepFileAndVersionsWhenMovingFileBetweenStorages() {
\OCA\Files_Versions\Hooks::connectHooks();
$storage2 = new Temporary(array());
@@ -162,7 +249,7 @@ class Storage extends \Test\TestCase {
// move to another storage
$this->userView->rename('test.txt', 'substorage/test.txt');
- $this->userView->file_exists('substorage/test.txt');
+ $this->assertTrue($this->userView->file_exists('substorage/test.txt'));
// rescan trash storage
list($rootStorage,) = $this->rootView->resolvePath($this->user . '/files_trashbin');
@@ -182,9 +269,50 @@ class Storage extends \Test\TestCase {
}
/**
+ * Test that versions are not auto-trashed when moving a file between
+ * storages. This is because rename() between storages would call
+ * unlink() which should NOT trigger the version deletion logic.
+ */
+ public function testKeepFileAndVersionsWhenMovingFolderBetweenStorages() {
+ \OCA\Files_Versions\Hooks::connectHooks();
+
+ $storage2 = new Temporary(array());
+ \OC\Files\Filesystem::mount($storage2, array(), $this->user . '/files/substorage');
+
+ // trigger a version (multiple would not work because of the expire logic)
+ $this->userView->file_put_contents('folder/inside.txt', 'v1');
+
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files');
+ $this->assertEquals(0, count($results));
+
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_versions/folder/');
+ $this->assertEquals(1, count($results));
+
+ // move to another storage
+ $this->userView->rename('folder', 'substorage/folder');
+ $this->assertTrue($this->userView->file_exists('substorage/folder/inside.txt'));
+
+ // rescan trash storage
+ list($rootStorage,) = $this->rootView->resolvePath($this->user . '/files_trashbin');
+ $rootStorage->getScanner()->scan('');
+
+ // versions were moved too
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_versions/substorage/folder/');
+ $this->assertEquals(1, count($results));
+
+ // check that nothing got trashed by the rename's unlink() call
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files');
+ $this->assertEquals(0, count($results));
+
+ // check that versions were moved and not trashed
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/versions/');
+ $this->assertEquals(0, count($results));
+ }
+
+ /**
* Delete should fail is the source file cant be deleted
*/
- public function testSingleStorageDeleteFail() {
+ public function testSingleStorageDeleteFileFail() {
/**
* @var \OC\Files\Storage\Temporary | \PHPUnit_Framework_MockObject_MockObject $storage
*/
@@ -194,9 +322,6 @@ class Storage extends \Test\TestCase {
->getMock();
$storage->expects($this->any())
- ->method('rename')
- ->will($this->returnValue(false));
- $storage->expects($this->any())
->method('unlink')
->will($this->returnValue(false));
@@ -214,4 +339,38 @@ class Storage extends \Test\TestCase {
$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/');
$this->assertEquals(0, count($results));
}
+
+ /**
+ * Delete should fail is the source folder cant be deleted
+ */
+ public function testSingleStorageDeleteFolderFail() {
+ /**
+ * @var \OC\Files\Storage\Temporary | \PHPUnit_Framework_MockObject_MockObject $storage
+ */
+ $storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+ ->setConstructorArgs([[]])
+ ->setMethods(['rename', 'unlink', 'rmdir'])
+ ->getMock();
+
+ $storage->expects($this->any())
+ ->method('rmdir')
+ ->will($this->returnValue(false));
+
+ $cache = $storage->getCache();
+
+ Filesystem::mount($storage, [], '/' . $this->user);
+ $storage->mkdir('files');
+ $this->userView->mkdir('folder');
+ $this->userView->file_put_contents('folder/test.txt', 'foo');
+ $this->assertTrue($storage->file_exists('files/folder/test.txt'));
+ $this->assertFalse($this->userView->rmdir('files/folder'));
+ $this->assertTrue($storage->file_exists('files/folder'));
+ $this->assertTrue($storage->file_exists('files/folder/test.txt'));
+ $this->assertTrue($cache->inCache('files/folder'));
+ $this->assertTrue($cache->inCache('files/folder/test.txt'));
+
+ // file should not be in the trashbin
+ $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/');
+ $this->assertEquals(0, count($results));
+ }
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list