diff --git a/apps/dav/appinfo/Migrations/Version20170116150538.php b/apps/dav/appinfo/Migrations/Version20170116150538.php index 47dda3e3ad2c..8d362676914b 100644 --- a/apps/dav/appinfo/Migrations/Version20170116150538.php +++ b/apps/dav/appinfo/Migrations/Version20170116150538.php @@ -1,6 +1,6 @@ + * @author Viktar Dubiniuk * * @copyright Copyright (c) 2017, ownCloud GmbH * @license AGPL-3.0 diff --git a/apps/dav/appinfo/Migrations/Version20170116170538.php b/apps/dav/appinfo/Migrations/Version20170116170538.php index ab9f3298dd9c..c1ac8949af3c 100644 --- a/apps/dav/appinfo/Migrations/Version20170116170538.php +++ b/apps/dav/appinfo/Migrations/Version20170116170538.php @@ -1,6 +1,6 @@ + * @author Viktar Dubiniuk * * @copyright Copyright (c) 2017, ownCloud GmbH * @license AGPL-3.0 diff --git a/apps/dav/appinfo/Migrations/Version20170202213905.php b/apps/dav/appinfo/Migrations/Version20170202213905.php index b530921f697b..622b0b1e0cbc 100644 --- a/apps/dav/appinfo/Migrations/Version20170202213905.php +++ b/apps/dav/appinfo/Migrations/Version20170202213905.php @@ -1,6 +1,6 @@ + * @author Viktar Dubiniuk * * @copyright Copyright (c) 2017, ownCloud GmbH * @license AGPL-3.0 diff --git a/apps/dav/appinfo/Migrations/Version20170202220512.php b/apps/dav/appinfo/Migrations/Version20170202220512.php index 0ef8ed8c1390..4b93a65b25b1 100644 --- a/apps/dav/appinfo/Migrations/Version20170202220512.php +++ b/apps/dav/appinfo/Migrations/Version20170202220512.php @@ -1,6 +1,6 @@ + * @author Viktar Dubiniuk * * @copyright Copyright (c) 2017, ownCloud GmbH * @license AGPL-3.0 diff --git a/apps/dav/appinfo/Migrations/Version20170427182800.php b/apps/dav/appinfo/Migrations/Version20170427182800.php index 8d0af1cff145..7af9a80755f2 100644 --- a/apps/dav/appinfo/Migrations/Version20170427182800.php +++ b/apps/dav/appinfo/Migrations/Version20170427182800.php @@ -1,6 +1,6 @@ + * @author Viktar Dubiniuk * * @copyright Copyright (c) 2017, ownCloud GmbH * @license AGPL-3.0 diff --git a/apps/dav/appinfo/Migrations/Version20170927201245.php b/apps/dav/appinfo/Migrations/Version20170927201245.php new file mode 100644 index 000000000000..d459e6a324cb --- /dev/null +++ b/apps/dav/appinfo/Migrations/Version20170927201245.php @@ -0,0 +1,66 @@ + + * + * @copyright Copyright (c) 2017, 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\Migrations; + +use OCP\Migration\ISchemaMigration; +use Doctrine\DBAL\Schema\Schema; +use Doctrine\DBAL\Types\Type; + +/* + * Create dav_properties table that stores properties + * of non-fs items (calendar/contacts) by path + */ + +class Version20170927201245 implements ISchemaMigration { + + /** + * @param Schema $schema + * @param array $options + */ + public function changeSchema(Schema $schema, array $options) { + $prefix = $options['tablePrefix']; + if (!$schema->hasTable("${prefix}dav_properties")){ + $table = $schema->createTable("${prefix}dav_properties"); + $table->addColumn('id', Type::BIGINT, [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 20, + ]); + $table->addColumn('propertypath', Type::STRING, [ + 'notnull' => true, + 'length' => 255, + 'default' => '', + ]); + $table->addColumn('propertyname', Type::STRING, [ + 'notnull' => true, + 'length' => 255, + 'default' => '', + ]); + $table->addColumn('propertyvalue', Type::STRING, [ + 'notnull' => true, + 'length' => 255, + ]); + $table->setPrimaryKey(['id']); + $table->addIndex(['propertypath'], 'propertypath_index'); + } + } +} diff --git a/apps/dav/appinfo/info.xml b/apps/dav/appinfo/info.xml index ce934e68b236..4e7c6d88d1ae 100644 --- a/apps/dav/appinfo/info.xml +++ b/apps/dav/appinfo/info.xml @@ -5,7 +5,7 @@ ownCloud WebDAV endpoint AGPL owncloud.org - 0.3.1 + 0.3.2 true diff --git a/apps/dav/lib/Connector/Sabre/CustomPropertiesBackend.php b/apps/dav/lib/Connector/Sabre/CustomPropertiesBackend.php deleted file mode 100644 index 765db62b284f..000000000000 --- a/apps/dav/lib/Connector/Sabre/CustomPropertiesBackend.php +++ /dev/null @@ -1,358 +0,0 @@ - - * @author Martin Mattel - * @author Thomas Müller - * @author Vincent Petry - * - * @copyright Copyright (c) 2017, 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\Connector\Sabre; - -use OCP\IDBConnection; -use OCP\IUser; -use Sabre\DAV\PropertyStorage\Backend\BackendInterface; -use Sabre\DAV\PropFind; -use Sabre\DAV\PropPatch; -use Sabre\DAV\Tree; -use Sabre\Dav\Exception\Forbidden; -use Sabre\DAV\Exception\NotFound; -use Sabre\DAV\Exception\ServiceUnavailable; - -class CustomPropertiesBackend implements BackendInterface { - - /** - * Ignored properties - * - * @var array - */ - private $ignoredProperties = [ - '{DAV:}getcontentlength', - '{DAV:}getcontenttype', - '{DAV:}getetag', - '{DAV:}quota-used-bytes', - '{DAV:}quota-available-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', - ]; - - /** - * @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 (Forbidden $e) { - // might happen for excluded 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(), - ['app' => 'files'] - ); - return; - } - - $requestedProps = $propFind->get404Properties(); - - // these might appear - $requestedProps = array_diff( - $requestedProps, - $this->ignoredProperties - ); - - if (empty($requestedProps)) { - return; - } - - if ($node instanceof Directory - && $propFind->getDepth() !== 0 - ) { - // note: pre-fetching only supported for depth <= 1 - $this->loadChildrenProperties($node, $requestedProps); - } - - $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) { - $fileId =$this->getFileIdByPath($path); - $statement = $this->connection->prepare( - 'DELETE FROM `*PREFIX*properties` WHERE `fileid` = ?' - ); - $statement->execute([$fileId]); - $statement->closeCursor(); - - unset($this->cache[$fileId]); - } - - /** - * This method is called after a successful MOVE - * - * @param string $source - * @param string $destination - * - * @return void - */ - public function move($source, $destination) { - // Do nothing - } - - /** - * 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) { - $fileId = $node->getId(); - if (isset($this->cache[$fileId])) { - return $this->cache[$fileId]; - } - - // TODO: chunking if more than 1000 properties - $sql = 'SELECT * FROM `*PREFIX*properties` WHERE `fileid` = ?'; - - $whereValues = [$fileId]; - $whereTypes = [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[$fileId] = $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) { - $fileId = $node->getId(); - - $deleteStatement = 'DELETE FROM `*PREFIX*properties`' . - ' WHERE `fileid` = ? AND `propertyname` = ?'; - - $insertStatement = 'INSERT INTO `*PREFIX*properties`' . - ' (`fileid`,`propertyname`,`propertyvalue`) VALUES(?,?,?)'; - - $updateStatement = 'UPDATE `*PREFIX*properties` SET `propertyvalue` = ?' . - ' WHERE `fileid` = ? AND `propertyname` = ?'; - - // TODO: use "insert or update" strategy ? - $existing = $this->getProperties($node, []); - $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, - [ - $fileId, - $propertyName - ] - ); - } - } else { - if (!array_key_exists($propertyName, $existing)) { - $this->connection->executeUpdate($insertStatement, - [ - $fileId, - $propertyName, - $propertyValue - ] - ); - } else { - $this->connection->executeUpdate($updateStatement, - [ - $propertyValue, - $fileId, - $propertyName - ] - ); - } - } - } - - $this->connection->commit(); - unset($this->cache[$fileId]); - - return true; - } - - /** - * Bulk load properties for directory children - * - * @param Directory $node - * @param array $requestedProperties requested properties - * - * @return void - */ - private function loadChildrenProperties(Directory $node, $requestedProperties) { - $fileId = $node->getId(); - - if (isset($this->cache[$fileId])) { - // we already loaded them at some point - return; - } - - $childNodes = $node->getChildren(); - $childrenIds = []; - // pre-fill cache - foreach ($childNodes as $childNode) { - $childId = $childNode->getId(); - $childrenIds[] = $childId; - $this->cache[$childId] = []; - } - - $sql = 'SELECT * FROM `*PREFIX*properties` WHERE `fileid` IN (?)'; - $sql .= ' AND `propertyname` in (?) ORDER BY `propertyname`'; - - $result = $this->connection->executeQuery( - $sql, - [$childrenIds, $requestedProperties], - [\Doctrine\DBAL\Connection::PARAM_STR_ARRAY, \Doctrine\DBAL\Connection::PARAM_STR_ARRAY] - ); - - $props = []; - while ($row = $result->fetch()) { - $props[$row['propertyname']] = $row['propertyvalue']; - $this->cache[$row['fileid']] = $props; - } - - $result->closeCursor(); - } - - /** - * @param string $filePath - * @return int - */ - private function getFileIdByPath($filePath){ - if (!$this->tree->nodeExists($filePath)) { - return; - } - $node = $this->tree->getNodeForPath($filePath); - if ($node instanceof Node) { - return $node->getId(); - } - } -} diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 00c828095f9f..0f89d16205ff 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -28,6 +28,7 @@ namespace OCA\DAV\Connector\Sabre; +use OCA\DAV\DAV\FileCustomPropertiesBackend; use OCA\DAV\Files\BrowserErrorPagePlugin; use OCP\Files\Mount\IMountManager; use OCP\IConfig; @@ -177,10 +178,11 @@ public function createServer($baseUri, \OC::$server->getGroupManager(), $userFolder )); + // custom properties plugin must be the last one $server->addPlugin( new \Sabre\DAV\PropertyStorage\Plugin( - new \OCA\DAV\Connector\Sabre\CustomPropertiesBackend( + new FileCustomPropertiesBackend( $objectTree, $this->databaseConnection, $this->userSession->getUser() diff --git a/apps/dav/lib/Files/CustomPropertiesBackend.php b/apps/dav/lib/DAV/AbstractCustomPropertiesBackend.php similarity index 50% rename from apps/dav/lib/Files/CustomPropertiesBackend.php rename to apps/dav/lib/DAV/AbstractCustomPropertiesBackend.php index ff1fb4250b34..c95f3fac1d55 100644 --- a/apps/dav/lib/Files/CustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/AbstractCustomPropertiesBackend.php @@ -19,25 +19,27 @@ * */ -namespace OCA\DAV\Files; +namespace OCA\DAV\DAV; -use Doctrine\DBAL\Connection; -use OCA\DAV\Connector\Sabre\Node; use OCP\IDBConnection; use OCP\IUser; +use Sabre\DAV\INode; use Sabre\DAV\PropertyStorage\Backend\BackendInterface; use Sabre\DAV\PropFind; use Sabre\DAV\PropPatch; use Sabre\DAV\Tree; +use Sabre\Dav\Exception\Forbidden; +use Sabre\DAV\Exception\NotFound; +use Sabre\DAV\Exception\ServiceUnavailable; -class CustomPropertiesBackend implements BackendInterface { +abstract class AbstractCustomPropertiesBackend implements BackendInterface { /** * Ignored properties * * @var array */ - private $ignoredProperties = [ + protected $ignoredProperties = [ '{DAV:}getcontentlength', '{DAV:}getcontenttype', '{DAV:}getetag', @@ -52,24 +54,23 @@ class CustomPropertiesBackend implements BackendInterface { /** * @var Tree */ - private $tree; + protected $tree; /** * @var IDBConnection */ - private $connection; + protected $connection; /** * @var string */ - private $user; + protected $user; /** - * Properties cache - * + * Property cache for the filesystem items * @var array */ - private $cache = []; + protected $cache; /** * @param Tree $tree node tree @@ -85,6 +86,73 @@ public function __construct( $this->user = $user->getUID(); } + /** + * Returns a list of properties for these nodes + * + * @param string $path path + * @param INode $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 + */ + abstract protected function getProperties($path, INode $node, array $requestedProperties); + + /** + * Update properties + * + * @param string $path + * @param INode $node node for which to update properties + * @param array $changedProperties array of properties to update + * @return bool + */ + abstract protected function updateProperties($path, INode $node, $changedProperties); + + /** + * Bulk load properties for children + * + * @param INode $node + * @param array $requestedProperties requested properties + * + * @return void + */ + abstract protected function loadChildrenProperties(INode $node, $requestedProperties); + + /** + * This method is called after a node is deleted. + * + * @param string $path path of node for which to delete properties + */ + abstract public function delete($path); + + /** + * @param string|int $key + * @return array|null + */ + protected function offsetGet($key){ + if (!isset($this->cache[$key])){ + return null; + } + return $this->cache[$key]; + } + + /** + * @param string|int $key + * @param array $value + */ + protected function offsetSet($key, $value){ + $this->cache[$key] = $value; + } + + /** + * @param string|int $key + */ + protected function offsetUnset($key){ + unset($this->cache[$key]); + } + /** * Fetches properties for a path. * @@ -93,6 +161,10 @@ public function __construct( * @return void */ public function propFind($path, PropFind $propFind) { + $node = $this->getNodeForPath($path); + if (is_null($node)) { + return; + } $requestedProps = $propFind->get404Properties(); @@ -106,8 +178,11 @@ public function propFind($path, PropFind $propFind) { return; } - $fileId = $this->getFileIdByPath($path); - $props = $this->getProperties($fileId, $requestedProps); + if ($propFind->getDepth() !== 0) { + $this->loadChildrenProperties($node, $requestedProps); + } + + $props = $this->getProperties($path, $node, $requestedProps); foreach ($props as $propName => $propValue) { $propFind->set($propName, $propValue); } @@ -122,68 +197,23 @@ public function propFind($path, PropFind $propFind) { * @return void */ public function propPatch($path, PropPatch $propPatch) { - $propPatch->handleRemaining(function($changedProps) use ($path) { - $fileId = $this->getFileIdByPath($path); - return $this->updateProperties($fileId, $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) { - $fileId = $this->getFileIdByPath($path); - $statement = $this->connection->prepare( - 'DELETE FROM `*PREFIX*properties` WHERE `fileid` = ?' - ); - $statement->execute([$fileId]); - $statement->closeCursor(); - - unset($this->cache[$fileId]); - } + $node = $this->getNodeForPath($path); + if (is_null($node)) { + return; + } - /** - * This method is called after a successful MOVE - * - * @param string $source - * @param string $destination - * - * @return void - */ - public function move($source, $destination) { - // Do nothing + $propPatch->handleRemaining(function($changedProps) use ($path, $node) { + return $this->updateProperties($path, $node, $changedProps); + }); } /** - * Returns a list of properties for this nodes.; - * @param int $fileId - * @param array $requestedProperties requested properties or empty array for "all" + * @param string $sql + * @param array $whereValues + * @param array $whereTypes * @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($fileId, array $requestedProperties) { - if (isset($this->cache[$fileId])) { - return $this->cache[$fileId]; - } - - // TODO: chunking if more than 1000 properties - $sql = 'SELECT * FROM `*PREFIX*properties` WHERE `fileid` = ?'; - - $whereValues = [$fileId]; - $whereTypes = [null]; - - if (!empty($requestedProperties)) { - // request only a subset - $sql .= ' AND `propertyname` in (?)'; - $whereValues[] = $requestedProperties; - $whereTypes[] = Connection::PARAM_STR_ARRAY; - } - + protected function fetchProperties($sql, $whereValues, $whereTypes) { $result = $this->connection->executeQuery( $sql, $whereValues, @@ -196,83 +226,35 @@ private function getProperties($fileId, array $requestedProperties) { } $result->closeCursor(); - - $this->cache[$fileId] = $props; return $props; } /** - * Update properties - * - * @param int $fileId node for which to update properties - * @param array $properties array of properties to update - * - * @return bool - */ - private function updateProperties($fileId, $properties) { - - $deleteStatement = 'DELETE FROM `*PREFIX*properties`' . - ' WHERE `fileid` = ? AND `propertyname` = ?'; - - $insertStatement = 'INSERT INTO `*PREFIX*properties`' . - ' (`fileid`,`propertyname`,`propertyvalue`) VALUES(?,?,?)'; - - $updateStatement = 'UPDATE `*PREFIX*properties` SET `propertyvalue` = ?' . - ' WHERE `fileid` = ? AND `propertyname` = ?'; - - // TODO: use "insert or update" strategy ? - $existing = $this->getProperties($fileId, []); - $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, - [ - $fileId, - $propertyName - ] - ); - } - } else { - if (!array_key_exists($propertyName, $existing)) { - $this->connection->executeUpdate($insertStatement, - [ - $fileId, - $propertyName, - $propertyValue - ] - ); - } else { - $this->connection->executeUpdate($updateStatement, - [ - $propertyValue, - $fileId, - $propertyName - ] - ); - } - } - } - - $this->connection->commit(); - unset($this->cache[$fileId]); - - return true; - } - - /** - * @param string $filePath - * @return int + * @param string $path + * @return INode|null */ - private function getFileIdByPath($filePath){ - if (!$this->tree->nodeExists($filePath)) { - return; - } - $node = $this->tree->getNodeForPath($filePath); - if ($node instanceof Node) { - return $node->getId(); + protected function getNodeForPath($path){ + try { + $node = $this->tree->getNodeForPath($path); + return $node; + } catch (ServiceUnavailable $e) { + // might happen for unavailable mount points, skip + } catch (Forbidden $e) { + // might happen for excluded mount points, skip + } 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}" : {$message}', + [ + 'app' => 'dav', + 'path' => $path, + 'message' => $e->getMessage(), + ] + ); } + return null; } } diff --git a/apps/dav/lib/DAV/FileCustomPropertiesBackend.php b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php new file mode 100644 index 000000000000..6a3031f89f57 --- /dev/null +++ b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php @@ -0,0 +1,204 @@ + + * @author Vincent Petry + * @author Viktar Dubiniuk + * + * @copyright Copyright (c) 2017, 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\DAV; + +use Doctrine\DBAL\Connection; +use OCA\DAV\Connector\Sabre\Directory; +use Sabre\DAV\INode; + +/** + * Class FileCustomPropertiesBackend + * + * Provides ability to store/retrieve custom file properties via DAV + * into oc_properties DB table using fileId as a reference to the file + * + * @package OCA\DAV\DAV + */ +class FileCustomPropertiesBackend extends AbstractCustomPropertiesBackend { + + const SELECT_BY_ID_STMT = 'SELECT * FROM `*PREFIX*properties` WHERE `fileid` = ?'; + const INSERT_BY_ID_STMT = 'INSERT INTO `*PREFIX*properties`' + . ' (`fileid`,`propertyname`,`propertyvalue`) VALUES(?,?,?)'; + const UPDATE_BY_ID_AND_NAME_STMT = 'UPDATE `*PREFIX*properties`' + . ' SET `propertyvalue` = ? WHERE `fileid` = ? AND `propertyname` = ?'; + const DELETE_BY_ID_STMT = 'DELETE FROM `*PREFIX*properties` WHERE `fileid` = ?'; + const DELETE_BY_ID_AND_NAME_STMT = 'DELETE FROM `*PREFIX*properties`' + . ' WHERE `fileid` = ? AND `propertyname` = ?'; + + /** + * This method is called after a node is deleted. + * + * @param string $path path of node for which to delete properties + */ + public function delete($path) { + $node = $this->getNodeForPath($path); + if (is_null($node)) { + return; + } + + $fileId = $node->getId(); + $statement = $this->connection->prepare(self::DELETE_BY_ID_STMT); + $statement->execute([$fileId]); + $this->offsetUnset($fileId); + $statement->closeCursor(); + } + + /** + * This method is called after a successful MOVE + * + * @param string $source + * @param string $destination + * + * @return void + */ + public function move($source, $destination) { + // Part of interface. We don't care about move because it doesn't affect fileId + } + + /** + * @inheritdoc + */ + protected function getProperties($path, INode $node, array $requestedProperties) { + $fileId = $node->getId(); + if (is_null($this->offsetGet($fileId))) { + // TODO: chunking if more than 1000 properties + $sql = self::SELECT_BY_ID_STMT; + $whereValues = [$fileId]; + $whereTypes = [null]; + + if (!empty($requestedProperties)) { + // request only a subset + $sql .= ' AND `propertyname` in (?)'; + $whereValues[] = $requestedProperties; + $whereTypes[] = Connection::PARAM_STR_ARRAY; + } + + $props = $this->fetchProperties($sql, $whereValues, $whereTypes); + $this->offsetSet($fileId, $props); + } + return $this->offsetGet($fileId); + } + + /** + * @inheritdoc + */ + protected function updateProperties($path, INode $node, $changedProperties) { + $existingProperties = $this->getProperties($path, $node, []); + $fileId = $node->getId(); + $deleteStatement = self::DELETE_BY_ID_AND_NAME_STMT; + $insertStatement = self::INSERT_BY_ID_STMT; + $updateStatement = self::UPDATE_BY_ID_AND_NAME_STMT; + + // TODO: use "insert or update" strategy ? + $this->connection->beginTransaction(); + foreach ($changedProperties as $propertyName => $propertyValue) { + $propertyExists = array_key_exists($propertyName, $existingProperties); + // If it was null, we need to delete the property + if (is_null($propertyValue)) { + if ($propertyExists) { + $this->connection->executeUpdate($deleteStatement, + [ + $fileId, + $propertyName + ] + ); + } + } else { + if (!$propertyExists) { + $this->connection->executeUpdate($insertStatement, + [ + $fileId, + $propertyName, + $propertyValue + ] + ); + } else { + $this->connection->executeUpdate($updateStatement, + [ + $propertyValue, + $fileId, + $propertyName + ] + ); + } + } + } + + $this->connection->commit(); + $this->offsetUnset($fileId); + + return true; + } + + /** + * Bulk load properties for directory children + * + * @param INode $node + * @param array $requestedProperties requested properties + * + * @return void + */ + protected function loadChildrenProperties(INode $node, $requestedProperties) { + // note: pre-fetching only supported for depth <= 1 + if (!($node instanceof Directory)){ + return; + } + + $fileId = $node->getId(); + if (!is_null($this->offsetGet($fileId))) { + // we already loaded them at some point + return; + } + + $childNodes = $node->getChildren(); + $childrenIds = []; + // pre-fill cache + foreach ($childNodes as $childNode) { + $childId = $childNode->getId(); + if ($childId) { + $childrenIds[] = $childId; + $this->offsetSet($childId, []); + } + } + + // TODO: use query builder + $sql = 'SELECT * FROM `*PREFIX*properties` WHERE `fileid` IN (?)'; + $sql .= ' AND `propertyname` in (?) ORDER BY `propertyname`'; + + $result = $this->connection->executeQuery( + $sql, + [$childrenIds, $requestedProperties], + [Connection::PARAM_STR_ARRAY, Connection::PARAM_STR_ARRAY] + ); + + $props = []; + while ($row = $result->fetch()) { + $props[$row['propertyname']] = $row['propertyvalue']; + $this->offsetSet($row['fileid'], $props); + } + + $result->closeCursor(); + } + +} diff --git a/apps/dav/lib/DAV/MiscCustomPropertiesBackend.php b/apps/dav/lib/DAV/MiscCustomPropertiesBackend.php new file mode 100644 index 000000000000..9d0a033a9f60 --- /dev/null +++ b/apps/dav/lib/DAV/MiscCustomPropertiesBackend.php @@ -0,0 +1,171 @@ + + * + * @copyright Copyright (c) 2017, 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\DAV; + +use Doctrine\DBAL\Connection; +use Sabre\DAV\INode; + +/** + * Class MiscCustomPropertiesBackend + * + * Provides ability to store/retrieve custom properties + * for card/calendar/whatever (excluding files) + * via DAV server into oc_dav_properties DB table using propertypath as a reference to the item + * + * @package OCA\DAV\DAV + */ +class MiscCustomPropertiesBackend extends AbstractCustomPropertiesBackend { + + const SELECT_BY_PATH_STMT = 'SELECT * FROM `*PREFIX*dav_properties` WHERE `propertypath` = ?'; + const INSERT_BY_PATH_STMT = 'INSERT INTO `*PREFIX*dav_properties`' + . ' (`propertypath`, `propertyname`, `propertyvalue`) VALUES(?,?,?)'; + const UPDATE_BY_PATH_STMT = 'UPDATE `*PREFIX*dav_properties`' + . ' SET `propertypath` = ? WHERE `propertypath` = ?'; + const UPDATE_BY_PATH_AND_NAME_STMT = 'UPDATE `*PREFIX*dav_properties` ' + . 'SET `propertyvalue` = ? WHERE `propertypath` = ? AND `propertyname` = ?'; + const DELETE_BY_PATH_STMT = 'DELETE FROM `*PREFIX*dav_properties` WHERE `propertypath` = ?'; + const DELETE_BY_PATH_AND_NAME_STMT = 'DELETE FROM `*PREFIX*dav_properties`' + . ' WHERE `propertypath` = ? AND `propertyname` = ?'; + + /** + * This method is called after a node is deleted. + * + * @param string $path path of node for which to delete properties + */ + public function delete($path) { + $node = $this->getNodeForPath($path); + if (is_null($node)) { + return; + } + + $statement = $this->connection->prepare(self::DELETE_BY_PATH_STMT); + $statement->execute([$path]); + $statement->closeCursor(); + $this->offsetUnset($path); + } + + /** + * This method is called after a successful MOVE + * + * @param string $source + * @param string $destination + * + * @return void + */ + public function move($source, $destination) { + $node = $this->getNodeForPath($source); + if (is_null($node)) { + return; + } + + $statement = $this->connection->prepare(self::UPDATE_BY_PATH_STMT); + $statement->execute([$destination, $source]); + $statement->closeCursor(); + } + + /** + * @inheritdoc + */ + protected function getProperties($path, INode $node, array $requestedProperties) { + if (is_null($this->offsetGet($path))) { + // TODO: chunking if more than 1000 properties + $sql = self::SELECT_BY_PATH_STMT; + + $whereValues = [$path]; + $whereTypes = [null]; + + if (!empty($requestedProperties)) { + // request only a subset + $sql .= ' AND `propertyname` in (?)'; + $whereValues[] = $requestedProperties; + $whereTypes[] = Connection::PARAM_STR_ARRAY; + } + + $props = $this->fetchProperties($sql, $whereValues, $whereTypes); + $this->offsetSet($path, $props); + } + return $this->offsetGet($path); + } + + /** + * @inheritdoc + */ + protected function updateProperties($path, INode $node, $changedProperties) { + $existingProperties = $this->getProperties($path, $node, []); + $deleteStatement = self::DELETE_BY_PATH_AND_NAME_STMT; + $insertStatement = self::INSERT_BY_PATH_STMT; + $updateStatement = self::UPDATE_BY_PATH_AND_NAME_STMT; + + // TODO: use "insert or update" strategy ? + $this->connection->beginTransaction(); + foreach ($changedProperties as $propertyName => $propertyValue) { + $propertyExists = array_key_exists($propertyName, $existingProperties); + // If it was null, we need to delete the property + if (is_null($propertyValue)) { + if ($propertyExists) { + $this->connection->executeUpdate($deleteStatement, + [ + $path, + $propertyName + ] + ); + } + } else { + if (!$propertyExists) { + $this->connection->executeUpdate($insertStatement, + [ + $path, + $propertyName, + $propertyValue + ] + ); + } else { + $this->connection->executeUpdate($updateStatement, + [ + $propertyValue, + $path, + $propertyName + ] + ); + } + } + } + + $this->connection->commit(); + $this->offsetUnset($path); + + return true; + } + + /** + * Bulk load properties for children + * + * @param INode $node + * @param array $requestedProperties requested properties + * + * @return void + */ + protected function loadChildrenProperties(INode $node, $requestedProperties) { + // Not supported + } + +} diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 9ab1a1cddf37..7fd765b5c0b8 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -42,7 +42,8 @@ use OCA\DAV\DAV\PublicAuth; use OCA\DAV\Connector\Sabre\QuotaPlugin; use OCA\DAV\Files\BrowserErrorPagePlugin; -use OCA\DAV\Files\CustomPropertiesBackend; +use OCA\DAV\DAV\FileCustomPropertiesBackend; +use OCA\DAV\DAV\MiscCustomPropertiesBackend; use OCA\DAV\SystemTag\SystemTagPlugin; use OCA\DAV\Upload\ChunkingPlugin; use OCP\IRequest; @@ -59,6 +60,9 @@ class Server { /** @var IRequest */ private $request; + /** @var Connector\Sabre\Server */ + public $server; + public function __construct(IRequest $request, $baseUri) { $this->request = $request; $this->baseUri = $baseUri; @@ -175,15 +179,31 @@ public function __construct(IRequest $request, $baseUri) { ) ); - $this->server->addPlugin( - new \Sabre\DAV\PropertyStorage\Plugin( - new CustomPropertiesBackend( - $this->server->tree, - \OC::$server->getDatabaseConnection(), - \OC::$server->getUserSession()->getUser() - ) + $filePropertiesPlugin = new \Sabre\DAV\PropertyStorage\Plugin( + new FileCustomPropertiesBackend( + $this->server->tree, + \OC::$server->getDatabaseConnection(), + \OC::$server->getUserSession()->getUser() + ) + ); + $filePropertiesPlugin->pathFilter = function($path) { + // oh yes, we could set custom properties on the user's storage root + return strpos($path, 'files/') === 0; + }; + $this->server->addPlugin($filePropertiesPlugin); + + $miscPropertiesPlugin = new \Sabre\DAV\PropertyStorage\Plugin( + new MiscCustomPropertiesBackend( + $this->server->tree, + \OC::$server->getDatabaseConnection(), + \OC::$server->getUserSession()->getUser() ) ); + $miscPropertiesPlugin->pathFilter = function($path) { + return strpos($path, 'files/') !== 0; + }; + $this->server->addPlugin($miscPropertiesPlugin); + if (!is_null($view)) { $this->server->addPlugin( new QuotaPlugin($view)); diff --git a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php similarity index 93% rename from apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php rename to apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php index a52e47d0b7a4..dfc922b077dc 100644 --- a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php @@ -20,23 +20,19 @@ * along with this program. If not, see * */ -namespace OCA\DAV\Tests\unit\Connector\Sabre; -/** - * Copyright (c) 2015 Vincent Petry - * This file is licensed under the Affero General Public License version 3 or - * later. - * See the COPYING-README file. - */ +namespace OCA\DAV\Tests\unit\DAV; + +use \OCA\DAV\DAV\FileCustomPropertiesBackend; /** - * Class CustomPropertiesBackend + * Class FileCustomPropertiesBackendTest * * @group DB * - * @package OCA\DAV\Tests\unit\Connector\Sabre + * @package OCA\DAV\Tests\unit\DAV */ -class CustomPropertiesBackendTest extends \Test\TestCase { +class FileCustomPropertiesBackendTest extends \Test\TestCase { /** * @var \Sabre\DAV\Server @@ -49,7 +45,7 @@ class CustomPropertiesBackendTest extends \Test\TestCase { private $tree; /** - * @var \OCA\DAV\Connector\Sabre\CustomPropertiesBackend + * @var FileCustomPropertiesBackend */ private $plugin; @@ -75,7 +71,7 @@ public function setUp() { ->method('getUID') ->will($this->returnValue($userId)); - $this->plugin = new \OCA\DAV\Connector\Sabre\CustomPropertiesBackend( + $this->plugin = new FileCustomPropertiesBackend( $this->tree, \OC::$server->getDatabaseConnection(), $this->user diff --git a/apps/dav/tests/unit/DAV/MiscCustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/MiscCustomPropertiesBackendTest.php new file mode 100644 index 000000000000..539061168a11 --- /dev/null +++ b/apps/dav/tests/unit/DAV/MiscCustomPropertiesBackendTest.php @@ -0,0 +1,241 @@ + + * @author Thomas Müller + * @author Vincent Petry + * + * @copyright Copyright (c) 2017, 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\Tests\unit\DAV; + +use \OCA\DAV\DAV\MiscCustomPropertiesBackend; +use \OCP\IUser; +use Sabre\CalDAV\Calendar; + +/** + * Class FileCustomPropertiesBackendTest + * + * @group DB + * + * @package OCA\DAV\Tests\unit\DAV + */ +class MiscCustomPropertiesBackendTest extends \Test\TestCase { + + /** + * @var \Sabre\DAV\Server + */ + private $server; + + /** + * @var \Sabre\DAV\Tree + */ + private $tree; + + /** + * @var MiscCustomPropertiesBackend + */ + private $plugin; + + /** + * @var \OCP\IUser + */ + private $user; + + /** @var int */ + private $maxId; + + public function setUp() { + parent::setUp(); + $this->server = new \Sabre\DAV\Server(); + $this->tree = $this->getMockBuilder('\Sabre\DAV\Tree') + ->disableOriginalConstructor() + ->getMock(); + + $userId = $this->getUniqueID('testcustompropertiesuser'); + + $this->user = $this->createMock(IUser::class); + $this->user->expects($this->any()) + ->method('getUID') + ->will($this->returnValue($userId)); + + $this->plugin = new MiscCustomPropertiesBackend( + $this->tree, + \OC::$server->getDatabaseConnection(), + $this->user + ); + + + $connection = \OC::$server->getDatabaseConnection(); + $qb = $connection->getQueryBuilder(); + $maxFunction = $qb->createFunction( + "MAX(`id`)" + ); + $this->maxId = (int) $qb->select($maxFunction) + ->from('dav_properties') + ->execute()->fetchColumn(); + } + + public function tearDown() { + $connection = \OC::$server->getDatabaseConnection(); + $deleteStatement = $connection->prepare( + 'DELETE FROM `*PREFIX*dav_properties`' . + ' WHERE `id` > ?' + ); + $deleteStatement->execute( + [ + $this->maxId, + ] + ); + $deleteStatement->closeCursor(); + } + + /** + * 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', + [ + 'customprop', + 'customprop2', + 'unsetprop', + ], + 0 + ); + + $this->plugin->propFind( + '/dummypath', + $propFind + ); + + $this->plugin->propFind( + '/dummypath', + $propFind + ); + + // no exception, soft fail + $this->assertTrue(true); + } + + /** + * Test setting/getting properties + */ + public function testSetGetPropertiesForCalendar() { + $testPath = "calendars/{$this->user->getUID()}/personal"; + $node = $this->createTestNode(Calendar::class); + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with($testPath) + ->will($this->returnValue($node)); + + $this->applyDefaultProps($testPath); + + $propFind = new \Sabre\DAV\PropFind( + $testPath, + [ + 'customprop', + 'customprop2', + 'unsetprop', + ], + 0 + ); + + $this->plugin->propFind( + $testPath, + $propFind + ); + + $this->assertEquals('value1', $propFind->get('customprop')); + $this->assertEquals('value2', $propFind->get('customprop2')); + $this->assertEquals(['unsetprop'], $propFind->get404Properties()); + } + + + /** + * Test delete property + */ + public function testDeleteProperty() { + $testPath = "calendars/{$this->user->getUID()}/personal"; + $node = $this->createTestNode(Calendar::class); + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with($testPath) + ->will($this->returnValue($node)); + + $this->applyDefaultProps($testPath); + + $propPatch = new \Sabre\DAV\PropPatch([ + 'customprop' => null, + ]); + + $this->plugin->propPatch( + $testPath, + $propPatch + ); + + $propPatch->commit(); + + $this->assertEmpty($propPatch->getRemainingMutations()); + + $result = $propPatch->getResult(); + $this->assertEquals(204, $result['customprop']); + } + + private function createTestNode($class) { + $node = $this->getMockBuilder($class) + ->disableOriginalConstructor() + ->getMock(); + + $node->expects($this->any()) + ->method('getName') + ->will($this->returnValue('dummycal')); + + return $node; + } + + private function applyDefaultProps($path = '/dummypath') { + // properties to set + $propPatch = new \Sabre\DAV\PropPatch([ + 'customprop' => 'value1', + 'customprop2' => 'value2', + ]); + + $this->plugin->propPatch( + $path, + $propPatch + ); + + $propPatch->commit(); + + $this->assertEmpty($propPatch->getRemainingMutations()); + + $result = $propPatch->getResult(); + $this->assertEquals(200, $result['customprop']); + $this->assertEquals(200, $result['customprop2']); + } +} diff --git a/tests/integration/features/webdav-related-new-endpoint.feature b/tests/integration/features/webdav-related-new-endpoint.feature index c1600fe090e7..ced94057c215 100644 --- a/tests/integration/features/webdav-related-new-endpoint.feature +++ b/tests/integration/features/webdav-related-new-endpoint.feature @@ -454,7 +454,7 @@ Feature: webdav-related-new-endpoint And user "user0" moved file "/testcustompropwithmove.txt" to "/catchmeifyoucan.txt" When as "user0" gets a custom property "{http://whatever.org/ns}very-custom-prop" of file "/catchmeifyoucan.txt" Then the response should contain a custom "{http://whatever.org/ns}very-custom-prop" property with "valueForMovetest" - + Scenario: Setting custom DAV property on a shared file as an owner and reading as a recipient Given using new dav path And user "user0" exists @@ -471,6 +471,16 @@ Feature: webdav-related-new-endpoint When as "user1" gets a custom property "{http://whatever.org/ns}very-custom-prop" of file "/testcustompropshared.txt" Then the response should contain a custom "{http://whatever.org/ns}very-custom-prop" property with "valueForSharetest" + Scenario: Setting custom DAV property using a new endpoint and reading it using an old endpoint + Given using new dav path + And user "user0" exists + And as an "user0" + And user "user0" uploads file "data/textfile.txt" to "/testnewold.txt" + And "user0" sets property "{http://whatever.org/ns}very-custom-prop" of file "/testnewold.txt" to "lucky" + And using old dav path + When as "user0" gets a custom property "{http://whatever.org/ns}very-custom-prop" of file "/testnewold.txt" + Then the response should contain a custom "{http://whatever.org/ns}very-custom-prop" property with "lucky" + ## Specific scenarios for new endpoint Scenario: Upload chunked file asc with new chunking diff --git a/tests/integration/features/webdav-related-old-endpoint.feature b/tests/integration/features/webdav-related-old-endpoint.feature index e1f23955687c..e11159af71a4 100644 --- a/tests/integration/features/webdav-related-old-endpoint.feature +++ b/tests/integration/features/webdav-related-old-endpoint.feature @@ -459,6 +459,16 @@ Feature: webdav-related-old-endpoint When as "user1" gets a custom property "{http://whatever.org/ns}very-custom-prop" of file "/testcustompropshared.txt" Then the response should contain a custom "{http://whatever.org/ns}very-custom-prop" property with "valueForSharetest" + Scenario: Setting custom DAV property using an old endpoint and reading it using a new endpoint + Given using old dav path + And user "user0" exists + And as an "user0" + And user "user0" uploads file "data/textfile.txt" to "/testoldnew.txt" + And "user0" sets property "{http://whatever.org/ns}very-custom-prop" of file "/testoldnew.txt" to "constant" + And using new dav path + When as "user0" gets a custom property "{http://whatever.org/ns}very-custom-prop" of file "/testoldnew.txt" + Then the response should contain a custom "{http://whatever.org/ns}very-custom-prop" property with "constant" + ### Scenarios specific to old endpoint Scenario: Upload chunked file asc