diff --git a/apps/dav/lib/BaseServer.php b/apps/dav/lib/BaseServer.php new file mode 100644 index 000000000000..0b6d92276933 --- /dev/null +++ b/apps/dav/lib/BaseServer.php @@ -0,0 +1,84 @@ + + * + * @copyright Copyright (c) 2018, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + + +namespace OCA\DAV; + +use OC\Cache\CappedMemoryCache; +use OCA\DAV\Connector\Sabre\Directory; +use OCA\DAV\Connector\Sabre\File; + +/** + * Class BaseSever + * + * Provides ability to store/retrieve deleted file ids by path + * + * @package OCA\DAV + */ +abstract class BaseServer extends \Sabre\DAV\Tree { + + /** + * @var CappedMemoryCache + */ + protected $deletedItemsCache; + + /** + * Get fileId by path + * + * @param string $path + * + * @return int|false + */ + public function getDeletedItemFileId($path) { + if ($this->deletedItemsCache === null) { + return false; + } + $fileId = $this->deletedItemsCache->get($path); + if ($fileId === null) { + return false; + } + return $fileId; + } + + /** + * Store fileId before deletion + * + * @param string $path + * + * @return void + */ + public function beforeDelete($path) { + try { + $node = $this->getNodeForPath($path); + if (($node instanceof Directory + || $node instanceof File) + && $node->getId() + ) { + if ($this->deletedItemsCache === null) { + $this->deletedItemsCache = new CappedMemoryCache(); + } + $this->deletedItemsCache->set($path, $node->getId()); + } + } catch (\Exception $e) { + // do nothing, delete will throw the same exception anyway + } + } +} diff --git a/apps/dav/lib/Connector/Sabre/ObjectTree.php b/apps/dav/lib/Connector/Sabre/ObjectTree.php index 97d70cf7dfc0..919a0a44ddc7 100644 --- a/apps/dav/lib/Connector/Sabre/ObjectTree.php +++ b/apps/dav/lib/Connector/Sabre/ObjectTree.php @@ -31,12 +31,13 @@ use OCA\DAV\Connector\Sabre\Exception\FileLocked; use OCA\DAV\Connector\Sabre\Exception\Forbidden; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; +use OCA\DAV\BaseServer; use OCP\Files\ForbiddenException; use OCP\Files\StorageInvalidException; use OCP\Files\StorageNotAvailableException; use OCP\Lock\LockedException; -class ObjectTree extends \Sabre\DAV\Tree { +class ObjectTree extends BaseServer { /** * @var \OC\Files\View diff --git a/apps/dav/lib/DAV/FileCustomPropertiesBackend.php b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php index 3434b9784f6c..2988f5d1c2d4 100644 --- a/apps/dav/lib/DAV/FileCustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php @@ -71,16 +71,13 @@ public function delete($path) { return; } - $node = $this->getNodeForPath($path); - if (\is_null($node)) { - return; + $fileId = $this->tree->getDeletedItemFileId($path); + if ($fileId !== false) { + $statement = $this->connection->prepare(self::DELETE_BY_ID_STMT); + $statement->execute([$fileId]); + $this->offsetUnset($fileId); + $statement->closeCursor(); } - - $fileId = $node->getId(); - $statement = $this->connection->prepare(self::DELETE_BY_ID_STMT); - $statement->execute([$fileId]); - $this->offsetUnset($fileId); - $statement->closeCursor(); } /** diff --git a/apps/dav/lib/Tree.php b/apps/dav/lib/Tree.php index f85d25130f19..63b4c1f1081e 100644 --- a/apps/dav/lib/Tree.php +++ b/apps/dav/lib/Tree.php @@ -30,11 +30,23 @@ * Provides a shortcut when accessing the "files/" subtree to avoid * having to walk through every node and trigger unnecessary extra queries. */ -class Tree extends \Sabre\DAV\Tree { +class Tree extends BaseServer { + + /** + * @param string $path + * + * @return void + */ + public function delete($path) { + $this->beforeDelete($path); + parent::delete($path); + } + /** * Returns the INode object for the requested path * * @param string $path + * * @return \Sabre\DAV\INode * @throws NotFound */ diff --git a/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php index 50bb264176ee..658df144abd7 100644 --- a/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php @@ -27,6 +27,7 @@ use Doctrine\DBAL\Platforms\MySqlPlatform; use Doctrine\DBAL\Platforms\SqlitePlatform; use OCA\DAV\DAV\FileCustomPropertiesBackend; +use OCA\DAV\Tree; use Sabre\DAV\PropFind; use Sabre\DAV\SimpleCollection; @@ -45,7 +46,7 @@ class FileCustomPropertiesBackendTest extends \Test\TestCase { private $server; /** - * @var \Sabre\DAV\Tree + * @var Tree */ private $tree; @@ -65,7 +66,7 @@ class FileCustomPropertiesBackendTest extends \Test\TestCase { public function setUp() { parent::setUp(); $this->server = new \Sabre\DAV\Server(); - $this->tree = $this->getMockBuilder('\Sabre\DAV\Tree') + $this->tree = $this->getMockBuilder(Tree::class) ->disableOriginalConstructor() ->getMock(); @@ -337,29 +338,47 @@ public function testGetPropertiesForDirectory() { * Test delete property */ public function testDeleteProperty() { + $path = '/dummypath'; $node = $this->createTestNode('\OCA\DAV\Connector\Sabre\File'); + $this->tree->expects($this->any()) + ->method('getDeletedItemFileId') + ->with($path) + ->will($this->returnValue($node->getId())); + $this->tree->expects($this->any()) ->method('getNodeForPath') - ->with('/dummypath') + ->with($path) ->will($this->returnValue($node)); $this->applyDefaultProps(); $propPatch = new \Sabre\DAV\PropPatch([ - 'customprop' => null, + 'customprop' => 'propvalue', ]); - - $this->plugin->propPatch( - '/dummypath', - $propPatch - ); - + $this->plugin->propPatch($path, $propPatch); $propPatch->commit(); - $this->assertEmpty($propPatch->getRemainingMutations()); $result = $propPatch->getResult(); - $this->assertEquals(204, $result['customprop']); + $this->assertEquals(200, $result['customprop']); + + $propFindBefore = new PropFind( + $path, + [ 'customprop' ], + 0 + ); + $this->plugin->propFind($path, $propFindBefore); + $this->assertEquals('propvalue', $propFindBefore->get('customprop')); + + $this->plugin->delete($path); + + $propFindAfter = new PropFind( + $path, + [ 'customprop' ], + 0 + ); + $this->plugin->propFind($path, $propFindAfter); + $this->assertNull($propFindAfter->get('customprop')); } public function slicesProvider() { diff --git a/apps/dav/tests/unit/TreeTest.php b/apps/dav/tests/unit/TreeTest.php index 6d02ea2da482..208a397ae5de 100644 --- a/apps/dav/tests/unit/TreeTest.php +++ b/apps/dav/tests/unit/TreeTest.php @@ -84,4 +84,31 @@ public function testInFilesWithoutTree() { $node = $this->tree->getNodeForPath($path); $this->assertEquals($file, $node); } + + public function testDelete() { + $path = 'files/user1'; + $fileId = 7755; + + $file = $this->createMock(\OCA\DAV\Connector\Sabre\File::class); + $file->expects($this->any()) + ->method('getId') + ->willReturn($fileId); + + $folder = $this->createMock(ICollection::class); + $folder->expects($this->any()) + ->method('getChild') + ->willReturn($file); + + $this->rootNode->expects($this->any()) + ->method('getChild') + ->willReturnMap( + [ + ['files', $folder] + ] + ); + + $this->tree->delete($path); + $deletedFileId = $this->tree->getDeletedItemFileId($path); + $this->assertEquals($fileId, $deletedFileId); + } }