Keep fileId for deletedItems in a separate cache

apps/dav/lib/BaseServer.php

@@ -0,0 +1,84 @@
+<?php
+/**
+ * @author Viktar Dubiniuk <[email protected]>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+
+
+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
+		}
+	}
+}

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

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();
 	}
 
 	/**

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
 	 */

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