Merge pull request #19242 from nextcloud/dav-long-properties

Handle long dav property paths by hashing them

7 files changed, 182 insertions, 504 deletions

diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php
index ecf51164e80..ab0c7ee63bd 100644
--- a/apps/dav/composer/composer/autoload_classmap.php
+++ b/apps/dav/composer/composer/autoload_classmap.php
@@ -124,7 +124,6 @@ return array(
     'OCA\\DAV\\Connector\\Sabre\\ChecksumList' => $baseDir . '/../lib/Connector/Sabre/ChecksumList.php',
     'OCA\\DAV\\Connector\\Sabre\\CommentPropertiesPlugin' => $baseDir . '/../lib/Connector/Sabre/CommentPropertiesPlugin.php',
     'OCA\\DAV\\Connector\\Sabre\\CopyEtagHeaderPlugin' => $baseDir . '/../lib/Connector/Sabre/CopyEtagHeaderPlugin.php',
-    'OCA\\DAV\\Connector\\Sabre\\CustomPropertiesBackend' => $baseDir . '/../lib/Connector/Sabre/CustomPropertiesBackend.php',
     'OCA\\DAV\\Connector\\Sabre\\DavAclPlugin' => $baseDir . '/../lib/Connector/Sabre/DavAclPlugin.php',
     'OCA\\DAV\\Connector\\Sabre\\Directory' => $baseDir . '/../lib/Connector/Sabre/Directory.php',
     'OCA\\DAV\\Connector\\Sabre\\DummyGetResponsePlugin' => $baseDir . '/../lib/Connector/Sabre/DummyGetResponsePlugin.php',
diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php
index 4df92c174e2..b142b93d793 100644
--- a/apps/dav/composer/composer/autoload_static.php
+++ b/apps/dav/composer/composer/autoload_static.php
@@ -139,7 +139,6 @@ class ComposerStaticInitDAV
         'OCA\\DAV\\Connector\\Sabre\\ChecksumList' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ChecksumList.php',
         'OCA\\DAV\\Connector\\Sabre\\CommentPropertiesPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/CommentPropertiesPlugin.php',
         'OCA\\DAV\\Connector\\Sabre\\CopyEtagHeaderPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/CopyEtagHeaderPlugin.php',
-        'OCA\\DAV\\Connector\\Sabre\\CustomPropertiesBackend' => __DIR__ . '/..' . '/../lib/Connector/Sabre/CustomPropertiesBackend.php',
         'OCA\\DAV\\Connector\\Sabre\\DavAclPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/DavAclPlugin.php',
         'OCA\\DAV\\Connector\\Sabre\\Directory' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Directory.php',
         'OCA\\DAV\\Connector\\Sabre\\DummyGetResponsePlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/DummyGetResponsePlugin.php',
diff --git a/apps/dav/lib/Connector/Sabre/CustomPropertiesBackend.php b/apps/dav/lib/Connector/Sabre/CustomPropertiesBackend.php
deleted file mode 100644
index daa71dac070..00000000000
--- a/apps/dav/lib/Connector/Sabre/CustomPropertiesBackend.php
+++ /dev/null
@@ -1,351 +0,0 @@
-<?php
-/**
- * @copyright Copyright (c) 2016, ownCloud, Inc.
- *
- * @author Aaron Wood <aaronjwood@gmail.com>
- * @author Lukas Reschke <lukas@statuscode.ch>
- * @author Roeland Jago Douma <roeland@famdouma.nl>
- * @author Thomas Müller <thomas.mueller@tmit.eu>
- * @author Vincent Petry <pvince81@owncloud.com>
- *
- * @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\Connector\Sabre;
-
-use OCP\IDBConnection;
-use OCP\IUser;
-use Sabre\DAV\Exception\NotFound;
-use Sabre\DAV\Exception\ServiceUnavailable;
-use Sabre\DAV\PropertyStorage\Backend\BackendInterface;
-use Sabre\DAV\PropFind;
-use Sabre\DAV\PropPatch;
-use Sabre\DAV\Tree;
-
-class CustomPropertiesBackend implements BackendInterface {
-
-	/**
-	 * Ignored properties
-	 *
-	 * @var array
-	 */
-	private $ignoredProperties = array(
-		'{DAV:}getcontentlength',
-		'{DAV:}getcontenttype',
-		'{DAV:}getetag',
-		'{DAV:}quota-used-bytes',
-		'{DAV:}quota-available-bytes',
-		'{http://owncloud.org/ns}permissions',
-		'{http://owncloud.org/ns}downloadURL',
-		'{http://owncloud.org/ns}dDC',
-		'{http://owncloud.org/ns}size',
-		'{http://nextcloud.org/ns}is-encrypted',
-	);
-
-	/**
-	 * @var Tree
-	 */
-	private $tree;
-
-	/**
-	 * @var IDBConnection
-	 */
-	private $connection;
-
-	/**
-	 * @var IUser
-	 */
-	private $user;
-
-	/**
-	 * Properties cache
-	 *
-	 * @var array
-	 */
-	private $cache = [];
-
-	/**
-	 * @param Tree $tree node tree
-	 * @param IDBConnection $connection database connection
-	 * @param IUser $user owner of the tree and properties
-	 */
-	public function __construct(
-		Tree $tree,
-		IDBConnection $connection,
-		IUser $user) {
-		$this->tree = $tree;
-		$this->connection = $connection;
-		$this->user = $user->getUID();
-	}
-
-	/**
-	 * Fetches properties for a path.
-	 *
-	 * @param string $path
-	 * @param PropFind $propFind
-	 * @return void
-	 */
-	public function propFind($path, PropFind $propFind) {
-		try {
-			$node = $this->tree->getNodeForPath($path);
-			if (!($node instanceof Node)) {
-				return;
-			}
-		} catch (ServiceUnavailable $e) {
-			// might happen for unavailable mount points, skip
-			return;
-		} catch (NotFound $e) {
-			// in some rare (buggy) cases the node might not be found,
-			// we catch the exception to prevent breaking the whole list with a 404
-			// (soft fail)
-			\OC::$server->getLogger()->warning(
-				'Could not get node for path: \"' . $path . '\" : ' . $e->getMessage(),
-				array('app' => 'files')
-			);
-			return;
-		}
-
-		$requestedProps = $propFind->get404Properties();
-
-		// these might appear
-		$requestedProps = array_diff(
-			$requestedProps,
-			$this->ignoredProperties
-		);
-
-		if (empty($requestedProps)) {
-			return;
-		}
-
-		$props = $this->getProperties($node, $requestedProps);
-		foreach ($props as $propName => $propValue) {
-			$propFind->set($propName, $propValue);
-		}
-	}
-
-	/**
-	 * Updates properties for a path
-	 *
-	 * @param string $path
-	 * @param PropPatch $propPatch
-	 *
-	 * @return void
-	 */
-	public function propPatch($path, PropPatch $propPatch) {
-		$node = $this->tree->getNodeForPath($path);
-		if (!($node instanceof Node)) {
-			return;
-		}
-
-		$propPatch->handleRemaining(function($changedProps) use ($node) {
-			return $this->updateProperties($node, $changedProps);
-		});
-	}
-
-	/**
-	 * This method is called after a node is deleted.
-	 *
-	 * @param string $path path of node for which to delete properties
-	 */
-	public function delete($path) {
-		$statement = $this->connection->prepare(
-			'DELETE FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?'
-		);
-		$statement->execute(array($this->user, '/' . $path));
-		$statement->closeCursor();
-
-		unset($this->cache[$path]);
-	}
-
-	/**
-	 * This method is called after a successful MOVE
-	 *
-	 * @param string $source
-	 * @param string $destination
-	 *
-	 * @return void
-	 */
-	public function move($source, $destination) {
-		$statement = $this->connection->prepare(
-			'UPDATE `*PREFIX*properties` SET `propertypath` = ?' .
-			' WHERE `userid` = ? AND `propertypath` = ?'
-		);
-		$statement->execute(array('/' . $destination, $this->user, '/' . $source));
-		$statement->closeCursor();
-	}
-
-	/**
-	 * Returns a list of properties for this nodes.;
-	 * @param Node $node
-	 * @param array $requestedProperties requested properties or empty array for "all"
-	 * @return array
-	 * @note The properties list is a list of propertynames the client
-	 * requested, encoded as xmlnamespace#tagName, for example:
-	 * http://www.example.org/namespace#author If the array is empty, all
-	 * properties should be returned
-	 */
-	private function getProperties(Node $node, array $requestedProperties) {
-		$path = $node->getPath();
-		if (isset($this->cache[$path])) {
-			return $this->cache[$path];
-		}
-
-		// TODO: chunking if more than 1000 properties
-		$sql = 'SELECT * FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?';
-
-		$whereValues = array($this->user, $path);
-		$whereTypes = array(null, null);
-
-		if (!empty($requestedProperties)) {
-			// request only a subset
-			$sql .= ' AND `propertyname` in (?)';
-			$whereValues[] = $requestedProperties;
-			$whereTypes[] = \Doctrine\DBAL\Connection::PARAM_STR_ARRAY;
-		}
-
-		$result = $this->connection->executeQuery(
-			$sql,
-			$whereValues,
-			$whereTypes
-		);
-
-		$props = [];
-		while ($row = $result->fetch()) {
-			$props[$row['propertyname']] = $row['propertyvalue'];
-		}
-
-		$result->closeCursor();
-
-		$this->cache[$path] = $props;
-		return $props;
-	}
-
-	/**
-	 * Update properties
-	 *
-	 * @param Node $node node for which to update properties
-	 * @param array $properties array of properties to update
-	 *
-	 * @return bool
-	 */
-	private function updateProperties($node, $properties) {
-		$path = $node->getPath();
-
-		$deleteStatement = 'DELETE FROM `*PREFIX*properties`' .
-			' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?';
-
-		$insertStatement = 'INSERT INTO `*PREFIX*properties`' .
-			' (`userid`,`propertypath`,`propertyname`,`propertyvalue`) VALUES(?,?,?,?)';
-
-		$updateStatement = 'UPDATE `*PREFIX*properties` SET `propertyvalue` = ?' .
-			' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?';
-
-		// TODO: use "insert or update" strategy ?
-		$existing = $this->getProperties($node, array());
-		$this->connection->beginTransaction();
-		foreach ($properties as $propertyName => $propertyValue) {
-			// If it was null, we need to delete the property
-			if (is_null($propertyValue)) {
-				if (array_key_exists($propertyName, $existing)) {
-					$this->connection->executeUpdate($deleteStatement,
-						array(
-							$this->user,
-							$path,
-							$propertyName
-						)
-					);
-				}
-			} else {
-				if (!array_key_exists($propertyName, $existing)) {
-					$this->connection->executeUpdate($insertStatement,
-						array(
-							$this->user,
-							$path,
-							$propertyName,
-							$propertyValue
-						)
-					);
-				} else {
-					$this->connection->executeUpdate($updateStatement,
-						array(
-							$propertyValue,
-							$this->user,
-							$path,
-							$propertyName
-						)
-					);
-				}
-			}
-		}
-
-		$this->connection->commit();
-		unset($this->cache[$path]);
-
-		return true;
-	}
-
-	/**
-	 * Bulk load properties for directory children
-	 *
-	 * @param Directory $node
-	 * @param array $requestedProperties requested properties
-	 *
-	 * @return void
-	 */
-	private function loadChildrenProperties(Directory $node, $requestedProperties) {
-		$path = $node->getPath();
-		if (isset($this->cache[$path])) {
-			// we already loaded them at some point
-			return;
-		}
-
-		$childNodes = $node->getChildren();
-		// pre-fill cache
-		foreach ($childNodes as $childNode) {
-			$this->cache[$childNode->getPath()] = [];
-		}
-
-		$sql = 'SELECT * FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` LIKE ?';
-		$sql .= ' AND `propertyname` in (?) ORDER BY `propertypath`, `propertyname`';
-
-		$result = $this->connection->executeQuery(
-			$sql,
-			array($this->user, $this->connection->escapeLikeParameter(rtrim($path, '/')) . '/%', $requestedProperties),
-			array(null, null, \Doctrine\DBAL\Connection::PARAM_STR_ARRAY)
-		);
-
-		$oldPath = null;
-		$props = [];
-		while ($row = $result->fetch()) {
-			$path = $row['propertypath'];
-			if ($oldPath !== $path) {
-				// save previously gathered props
-				$this->cache[$oldPath] = $props;
-				$oldPath = $path;
-				// prepare props for next path
-				$props = [];
-			}
-			$props[$row['propertyname']] = $row['propertyvalue'];
-		}
-		if (!is_null($oldPath)) {
-			// save props from last run
-			$this->cache[$oldPath] = $props;
-		}
-
-		$result->closeCursor();
-	}
-
-}
diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php
index 595e3b734b5..9efd9ba7b56 100644
--- a/apps/dav/lib/Connector/Sabre/ServerFactory.php
+++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php
@@ -195,7 +195,7 @@ class ServerFactory {
 				// custom properties plugin must be the last one
 				$server->addPlugin(
 					new \Sabre\DAV\PropertyStorage\Plugin(
-						new \OCA\DAV\Connector\Sabre\CustomPropertiesBackend(
+						new \OCA\DAV\DAV\CustomPropertiesBackend(
 							$objectTree,
 							$this->databaseConnection,
 							$this->userSession->getUser()
diff --git a/apps/dav/lib/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php
index 35b3daa8a4d..8b0408a461b 100644
--- a/apps/dav/lib/DAV/CustomPropertiesBackend.php
+++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php
@@ -3,9 +3,11 @@
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  * @copyright Copyright (c) 2017, Georg Ehrke <oc.list@georgehrke.com>
  *
- * @author Georg Ehrke <oc.list@georgehrke.com>
- * @author Robin Appelman <robin@icewind.nl>
+ * @author Aaron Wood <aaronjwood@gmail.com>
+ * @author Lukas Reschke <lukas@statuscode.ch>
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
  * @author Thomas Müller <thomas.mueller@tmit.eu>
+ * @author Vincent Petry <pvince81@owncloud.com>
  *
  * @license AGPL-3.0
  *
@@ -25,8 +27,12 @@
 
 namespace OCA\DAV\DAV;
 
+use OCA\DAV\Connector\Sabre\Node;
 use OCP\IDBConnection;
 use OCP\IUser;
+use Sabre\DAV\Exception\NotFound;
+use Sabre\DAV\Exception\ServiceUnavailable;
+use Sabre\DAV\INode;
 use Sabre\DAV\PropertyStorage\Backend\BackendInterface;
 use Sabre\DAV\PropFind;
 use Sabre\DAV\PropPatch;
@@ -39,7 +45,7 @@ class CustomPropertiesBackend implements BackendInterface {
 	 *
 	 * @var array
 	 */
-	private $ignoredProperties = array(
+	private $ignoredProperties = [
 		'{DAV:}getcontentlength',
 		'{DAV:}getcontenttype',
 		'{DAV:}getetag',
@@ -50,7 +56,7 @@ class CustomPropertiesBackend implements BackendInterface {
 		'{http://owncloud.org/ns}dDC',
 		'{http://owncloud.org/ns}size',
 		'{http://nextcloud.org/ns}is-encrypted',
-	);
+	];
 
 	/**
 	 * @var Tree
@@ -63,7 +69,7 @@ class CustomPropertiesBackend implements BackendInterface {
 	private $connection;
 
 	/**
-	 * @var string
+	 * @var IUser
 	 */
 	private $user;
 
@@ -85,7 +91,7 @@ class CustomPropertiesBackend implements BackendInterface {
 		IUser $user) {
 		$this->tree = $tree;
 		$this->connection = $connection;
-		$this->user = $user->getUID();
+		$this->user = $user;
 	}
 
 	/**
@@ -96,7 +102,6 @@ class CustomPropertiesBackend implements BackendInterface {
 	 * @return void
 	 */
 	public function propFind($path, PropFind $propFind) {
-
 		$requestedProps = $propFind->get404Properties();
 
 		// these might appear
@@ -144,7 +149,7 @@ class CustomPropertiesBackend implements BackendInterface {
 	 * @return void
 	 */
 	public function propPatch($path, PropPatch $propPatch) {
-		$propPatch->handleRemaining(function($changedProps) use ($path) {
+		$propPatch->handleRemaining(function ($changedProps) use ($path) {
 			return $this->updateProperties($path, $changedProps);
 		});
 	}
@@ -158,7 +163,7 @@ class CustomPropertiesBackend implements BackendInterface {
 		$statement = $this->connection->prepare(
 			'DELETE FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?'
 		);
-		$statement->execute(array($this->user, $path));
+		$statement->execute([$this->user->getUID(), $this->formatPath($path)]);
 		$statement->closeCursor();
 
 		unset($this->cache[$path]);
@@ -177,12 +182,13 @@ class CustomPropertiesBackend implements BackendInterface {
 			'UPDATE `*PREFIX*properties` SET `propertypath` = ?' .
 			' WHERE `userid` = ? AND `propertypath` = ?'
 		);
-		$statement->execute(array($destination, $this->user, $source));
+		$statement->execute([$this->formatPath($destination), $this->user->getUID(), $this->formatPath($source)]);
 		$statement->closeCursor();
 	}
 
 	/**
 	 * Returns a list of properties for this nodes.;
+	 *
 	 * @param string $path
 	 * @param array $requestedProperties requested properties or empty array for "all"
 	 * @return array
@@ -191,7 +197,7 @@ class CustomPropertiesBackend implements BackendInterface {
 	 * http://www.example.org/namespace#author If the array is empty, all
 	 * properties should be returned
 	 */
-	private function getProperties($path, array $requestedProperties) {
+	private function getProperties(string $path, array $requestedProperties) {
 		if (isset($this->cache[$path])) {
 			return $this->cache[$path];
 		}
@@ -199,8 +205,8 @@ class CustomPropertiesBackend implements BackendInterface {
 		// TODO: chunking if more than 1000 properties
 		$sql = 'SELECT * FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?';
 
-		$whereValues = array($this->user, $path);
-		$whereTypes = array(null, null);
+		$whereValues = [$this->user->getUID(), $this->formatPath($path)];
+		$whereTypes = [null, null];
 
 		if (!empty($requestedProperties)) {
 			// request only a subset
@@ -229,12 +235,12 @@ class CustomPropertiesBackend implements BackendInterface {
 	/**
 	 * Update properties
 	 *
-	 * @param string $path node for which to update properties
+	 * @param string $path path for which to update properties
 	 * @param array $properties array of properties to update
 	 *
 	 * @return bool
 	 */
-	private function updateProperties($path, $properties) {
+	private function updateProperties(string $path, array $properties) {
 
 		$deleteStatement = 'DELETE FROM `*PREFIX*properties`' .
 			' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?';
@@ -246,38 +252,38 @@ class CustomPropertiesBackend implements BackendInterface {
 			' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?';
 
 		// TODO: use "insert or update" strategy ?
-		$existing = $this->getProperties($path, array());
+		$existing = $this->getProperties($path, []);
 		$this->connection->beginTransaction();
 		foreach ($properties as $propertyName => $propertyValue) {
 			// If it was null, we need to delete the property
 			if (is_null($propertyValue)) {
 				if (array_key_exists($propertyName, $existing)) {
 					$this->connection->executeUpdate($deleteStatement,
-						array(
-							$this->user,
-							$path,
-							$propertyName
-						)
+						[
+							$this->user->getUID(),
+							$this->formatPath($path),
+							$propertyName,
+						]
 					);
 				}
 			} else {
 				if (!array_key_exists($propertyName, $existing)) {
 					$this->connection->executeUpdate($insertStatement,
-						array(
-							$this->user,
-							$path,
+						[
+							$this->user->getUID(),
+							$this->formatPath($path),
 							$propertyName,
-							$propertyValue
-						)
+							$propertyValue,
+						]
 					);
 				} else {
 					$this->connection->executeUpdate($updateStatement,
-						array(
+						[
 							$propertyValue,
-							$this->user,
-							$path,
-							$propertyName
-						)
+							$this->user->getUID(),
+							$this->formatPath($path),
+							$propertyName,
+						]
 					);
 				}
 			}
@@ -289,4 +295,17 @@ class CustomPropertiesBackend implements BackendInterface {
 		return true;
 	}
 
+	/**
+	 * long paths are hashed to ensure they fit in the database
+	 *
+	 * @param string $path
+	 * @return string
+	 */
+	private function formatPath(string $path): string {
+		if (strlen($path) > 250) {
+			return sha1($path);
+		} else {
+			return $path;
+		}
+	}
 }
diff --git a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php
index 495efefa79b..a78d64f9c94 100644
--- a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php
@@ -58,7 +58,7 @@ class CustomPropertiesBackendTest extends \Test\TestCase {
 	private $tree;
 
 	/**
-	 * @var \OCA\DAV\Connector\Sabre\CustomPropertiesBackend
+	 * @var \OCA\DAV\DAV\CustomPropertiesBackend
 	 */
 	private $plugin;
 
@@ -83,7 +83,7 @@ class CustomPropertiesBackendTest extends \Test\TestCase {
 			->method('getUID')
 			->will($this->returnValue($userId));
 
-		$this->plugin = new \OCA\DAV\Connector\Sabre\CustomPropertiesBackend(
+		$this->plugin = new \OCA\DAV\DAV\CustomPropertiesBackend(
 			$this->tree,
 			\OC::$server->getDatabaseConnection(),
 			$this->user
@@ -144,16 +144,6 @@ class CustomPropertiesBackendTest extends \Test\TestCase {
 	 * Test that propFind on a missing file soft fails
 	 */
 	public function testPropFindMissingFileSoftFail() {
-		$this->tree->expects($this->at(0))
-			->method('getNodeForPath')
-			->with('/dummypath')
-			->will($this->throwException(new \Sabre\DAV\Exception\NotFound()));
-
-		$this->tree->expects($this->at(1))
-			->method('getNodeForPath')
-			->with('/dummypath')
-			->will($this->throwException(new \Sabre\DAV\Exception\ServiceUnavailable()));
-
 		$propFind = new \Sabre\DAV\PropFind(
 			'/dummypath',
 			array(
@@ -174,20 +164,14 @@ class CustomPropertiesBackendTest extends \Test\TestCase {
 			$propFind
 		);
 
-		// no exception, soft fail
-		$this->addToAssertionCount(1);
+		// assert that the above didn't throw exceptions
+		$this->assertTrue(true);
 	}
 
 	/**
 	 * Test setting/getting properties
 	 */
 	public function testSetGetPropertiesForFile() {
-		$node = $this->createTestNode(File::class);
-		$this->tree->expects($this->any())
-			->method('getNodeForPath')
-			->with('/dummypath')
-			->will($this->returnValue($node));
-
 		$this->applyDefaultProps();
 
 		$propFind = new \Sabre\DAV\PropFind(
@@ -214,39 +198,6 @@ class CustomPropertiesBackendTest extends \Test\TestCase {
 	 * Test getting properties from directory
 	 */
 	public function testGetPropertiesForDirectory() {
-		$rootNode = $this->createTestNode(Directory::class);
-
-		$nodeSub = $this->getMockBuilder(File::class)
-			->disableOriginalConstructor()
-			->getMock();
-		$nodeSub->expects($this->any())
-			->method('getId')
-			->will($this->returnValue(456));
-
-		$nodeSub->expects($this->any())
-			->method('getPath')
-			->will($this->returnValue('/dummypath/test.txt'));
-
-		$this->tree->expects($this->at(0))
-			->method('getNodeForPath')
-			->with('/dummypath')
-			->will($this->returnValue($rootNode));
-
-		$this->tree->expects($this->at(1))
-			->method('getNodeForPath')
-			->with('/dummypath/test.txt')
-			->will($this->returnValue($nodeSub));
-
-		$this->tree->expects($this->at(2))
-			->method('getNodeForPath')
-			->with('/dummypath')
-			->will($this->returnValue($rootNode));
-
-		$this->tree->expects($this->at(3))
-			->method('getNodeForPath')
-			->with('/dummypath/test.txt')
-			->will($this->returnValue($nodeSub));
-
 		$this->applyDefaultProps('/dummypath');
 		$this->applyDefaultProps('/dummypath/test.txt');
 
@@ -294,12 +245,6 @@ class CustomPropertiesBackendTest extends \Test\TestCase {
 	 * Test delete property
 	 */
 	public function testDeleteProperty() {
-		$node = $this->createTestNode(File::class);
-		$this->tree->expects($this->any())
-			->method('getNodeForPath')
-			->with('/dummypath')
-			->will($this->returnValue($node));
-
 		$this->applyDefaultProps();
 
 		$propPatch = new \Sabre\DAV\PropPatch(array(
diff --git a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
index 44e5c6464a2..45aab9af1f5 100644
--- a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
+++ b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
@@ -24,20 +24,26 @@
 
 namespace OCA\DAV\Tests\DAV;
 
+use OCA\DAV\Connector\Sabre\Node;
 use OCA\DAV\DAV\CustomPropertiesBackend;
 use OCP\IDBConnection;
 use OCP\IUser;
+use Sabre\DAV\Exception\NotFound;
+use Sabre\DAV\INode;
 use Sabre\DAV\PropFind;
 use Sabre\DAV\PropPatch;
 use Sabre\DAV\Tree;
 use Test\TestCase;
 
+/**
+ * @group DB
+ */
 class CustomPropertiesBackendTest extends TestCase {
 
 	/** @var Tree | \PHPUnit_Framework_MockObject_MockObject */
 	private $tree;
 
-	/** @var  IDBConnection | \PHPUnit_Framework_MockObject_MockObject */
+	/** @var  IDBConnection */
 	private $dbConnection;
 
 	/** @var IUser | \PHPUnit_Framework_MockObject_MockObject */
@@ -50,18 +56,71 @@ class CustomPropertiesBackendTest extends TestCase {
 		parent::setUp();
 
 		$this->tree = $this->createMock(Tree::class);
-		$this->dbConnection = $this->createMock(IDBConnection::class);
 		$this->user = $this->createMock(IUser::class);
-		$this->user->expects($this->once())
-			->method('getUID')
+		$this->user->method('getUID')
 			->with()
 			->will($this->returnValue('dummy_user_42'));
+		$this->dbConnection = \OC::$server->getDatabaseConnection();
+
+		$this->backend = new CustomPropertiesBackend(
+			$this->tree,
+			$this->dbConnection,
+			$this->user
+		);
 
-		$this->backend = new CustomPropertiesBackend($this->tree,
-			$this->dbConnection, $this->user);
+	}
+
+	protected function tearDown(): void {
+		$query = $this->dbConnection->getQueryBuilder();
+		$query->delete('properties');
+		$query->execute();
+
+		parent::tearDown();
+	}
+
+	private function formatPath(string $path): string {
+		if (strlen($path) > 250) {
+			return sha1($path);
+		} else {
+			return $path;
+		}
+	}
+
+	protected function insertProps(string $user, string $path, array $props) {
+		foreach ($props as $name => $value) {
+			$this->insertProp($user, $path, $name, $value);
+		}
+	}
+
+	protected function insertProp(string $user, string $path, string $name, string $value) {
+		$query = $this->dbConnection->getQueryBuilder();
+		$query->insert('properties')
+			->values([
+				'userid' => $query->createNamedParameter($user),
+				'propertypath' => $query->createNamedParameter($this->formatPath($path)),
+				'propertyname' => $query->createNamedParameter($name),
+				'propertyvalue' => $query->createNamedParameter($value),
+			]);
+		$query->execute();
+	}
+
+	protected function getProps(string $user, string $path) {
+		$query = $this->dbConnection->getQueryBuilder();
+		$query->select('propertyname', 'propertyvalue')
+			->from('properties')
+			->where($query->expr()->eq('userid', $query->createNamedParameter($user)))
+			->where($query->expr()->eq('propertypath', $query->createNamedParameter($this->formatPath($path))));
+		return $query->execute()->fetchAll(\PDO::FETCH_KEY_PAIR);
 	}
 
 	public function testPropFindNoDbCalls() {
+		$db = $this->createMock(IDBConnection::class);
+		$backend = new CustomPropertiesBackend(
+			$this->tree,
+			$db,
+			$this->user
+		);
+
 		$propFind = $this->createMock(PropFind::class);
 		$propFind->expects($this->at(0))
 			->method('get404Properties')
@@ -73,26 +132,24 @@ class CustomPropertiesBackendTest extends TestCase {
 				'{http://owncloud.org/ns}size',
 			]));
 
-		$this->dbConnection->expects($this->never())
+		$db->expects($this->never())
 			->method($this->anything());
 
-		$this->backend->propFind('foo_bar_path_1337_0', $propFind);
+		$backend->propFind('foo_bar_path_1337_0', $propFind);
 	}
 
 	public function testPropFindCalendarCall() {
 		$propFind = $this->createMock(PropFind::class);
-		$propFind->expects($this->at(0))
-			->method('get404Properties')
+		$propFind->method('get404Properties')
 			->with()
 			->will($this->returnValue([
 				'{DAV:}getcontentlength',
 				'{DAV:}getcontenttype',
 				'{DAV:}getetag',
-				'{abc}def'
+				'{abc}def',
 			]));
 
-		$propFind->expects($this->at(1))
-			->method('getRequestedProperties')
+		$propFind->method('getRequestedProperties')
 			->with()
 			->will($this->returnValue([
 				'{DAV:}getcontentlength',
@@ -101,72 +158,82 @@ class CustomPropertiesBackendTest extends TestCase {
 				'{DAV:}displayname',
 				'{urn:ietf:params:xml:ns:caldav}calendar-description',
 				'{urn:ietf:params:xml:ns:caldav}calendar-timezone',
-				'{abc}def'
+				'{abc}def',
 			]));
 
-		$statement = $this->createMock('\Doctrine\DBAL\Driver\Statement');
-		$this->dbConnection->expects($this->once())
-			->method('executeQuery')
-			->with('SELECT * FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` in (?)',
-				['dummy_user_42', 'calendars/foo/bar_path_1337_0', [
-					3 => '{abc}def',
-					4 => '{DAV:}displayname',
-					5 => '{urn:ietf:params:xml:ns:caldav}calendar-description',
-					6 => '{urn:ietf:params:xml:ns:caldav}calendar-timezone']],
-				[null, null, 102])
-			->will($this->returnValue($statement));
+		$props = [
+			'{abc}def' => 'a',
+			'{DAV:}displayname' => 'b',
+			'{urn:ietf:params:xml:ns:caldav}calendar-description' => 'c',
+			'{urn:ietf:params:xml:ns:caldav}calendar-timezone' => 'd',
+		];
+
+		$this->insertProps('dummy_user_42', 'calendars/foo/bar_path_1337_0', $props);
+
+		$setProps = [];
+		$propFind->method('set')
+			->willReturnCallback(function ($name, $value, $status) use (&$setProps) {
+				$setProps[$name] = $value;
+			});
 
 		$this->backend->propFind('calendars/foo/bar_path_1337_0', $propFind);
+		$this->assertEquals($props, $setProps);
 	}
 
 	/**
 	 * @dataProvider propPatchProvider
 	 */
-	public function testPropPatch($path, $propPatch) {
-		$propPatch->expects($this->once())
-			->method('handleRemaining');
+	public function testPropPatch(string $path, array $existing, array $props, array $result) {
+		$this->insertProps($this->user->getUID(), $path, $existing);
+		$propPatch = new PropPatch($props);
 
 		$this->backend->propPatch($path, $propPatch);
+		$propPatch->commit();
+
+		$storedProps = $this->getProps($this->user->getUID(), $path);
+		$this->assertEquals($result, $storedProps);
 	}
 
 	public function propPatchProvider() {
-		$propPatchMock = $this->createMock(PropPatch::class);
+		$longPath = str_repeat('long_path', 100);
+		return [
+			['foo_bar_path_1337', [], ['{DAV:}displayname' => 'anything'], ['{DAV:}displayname' => 'anything']],
+			['foo_bar_path_1337', ['{DAV:}displayname' => 'foo'], ['{DAV:}displayname' => 'anything'], ['{DAV:}displayname' => 'anything']],
+			['foo_bar_path_1337', ['{DAV:}displayname' => 'foo'], ['{DAV:}displayname' => null], []],
+			[$longPath, [], ['{DAV:}displayname' => 'anything'], ['{DAV:}displayname' => 'anything']],
+		];
+	}
+
+	/**
+	 * @dataProvider deleteProvider
+	 */
+	public function testDelete(string $path) {
+		$this->insertProps('dummy_user_42', $path, ['foo' => 'bar']);
+		$this->backend->delete($path);
+		$this->assertEquals([], $this->getProps('dummy_user_42', $path));
+	}
+
+	public function deleteProvider() {
 		return [
-			['foo_bar_path_1337', $propPatchMock],
+			['foo_bar_path_1337'],
+			[str_repeat('long_path', 100)]
 		];
 	}
 
-	public function testDelete() {
-		$statement = $this->createMock('\Doctrine\DBAL\Driver\Statement');
-		$statement->expects($this->at(0))
-			->method('execute')
-			->with(['dummy_user_42', 'foo_bar_path_1337']);
-		$statement->expects($this->at(1))
-			->method('closeCursor')
-			->with();
-
-		$this->dbConnection->expects($this->at(0))
-			->method('prepare')
-			->with('DELETE FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?')
-			->will($this->returnValue($statement));
-
-		$this->backend->delete('foo_bar_path_1337');
+	/**
+	 * @dataProvider moveProvider
+	 */
+	public function testMove(string $source, string $target) {
+		$this->insertProps('dummy_user_42', $source, ['foo' => 'bar']);
+		$this->backend->move($source, $target);
+		$this->assertEquals([], $this->getProps('dummy_user_42', $source));
+		$this->assertEquals(['foo' => 'bar'], $this->getProps('dummy_user_42', $target));
 	}
 
-	public function testMove() {
-		$statement = $this->createMock('\Doctrine\DBAL\Driver\Statement');
-		$statement->expects($this->at(0))
-			->method('execute')
-			->with(['bar_foo_path_7331', 'dummy_user_42', 'foo_bar_path_1337']);
-		$statement->expects($this->at(1))
-			->method('closeCursor')
-			->with();
-
-		$this->dbConnection->expects($this->at(0))
-			->method('prepare')
-			->with('UPDATE `*PREFIX*properties` SET `propertypath` = ? WHERE `userid` = ? AND `propertypath` = ?')
-			->will($this->returnValue($statement));
-
-		$this->backend->move('foo_bar_path_1337', 'bar_foo_path_7331');
+	public function moveProvider() {
+		return [
+			['foo_bar_path_1337', 'foo_bar_path_7333'],
+			[str_repeat('long_path1', 100), str_repeat('long_path2', 100)]
+		];
 	}
 }