Skip to content
Snippets Groups Projects
Commit e1923bac authored by Joas Schilling's avatar Joas Schilling
Browse files

Merge pull request #16247 from owncloud/fixrmdirtodeletefolderstotrash 

Added rmdir to trashbin storage wrapper
parents 937306b4 beb6a38d
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
apps/files_trashbin/lib/storage.php
+ 30
− 5
View file @ e1923bac
......@@ -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;
......
apps/files_trashbin/tests/storage.php
+ 169
− 10
View file @ e1923bac
......@@ -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');
......@@ -89,13 +91,35 @@ class Storage extends \Test\TestCase {
$this->assertEquals('test.txt', substr($name, 0, strrpos($name, '.')));
}
/**
* 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');
......@@ -115,10 +139,42 @@ class Storage extends \Test\TestCase {
$this->assertEquals('subfile.txt', substr($name, 0, strrpos($name, '.')));
}
/**
* 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');
......@@ -181,10 +268,51 @@ class Storage extends \Test\TestCase {
$this->assertEquals(0, count($results));
}
/**
* 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
*/
......@@ -193,9 +321,6 @@ class Storage extends \Test\TestCase {
->setMethods(['rename', 'unlink'])
->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));
}
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment