From 4dcc2cbf1ad4a604696632fc8ae67838d577e612 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Mon, 2 Oct 2017 23:23:34 +0300 Subject: [PATCH 1/7] Add integration tests for setting properties via old endpoind and getting them via new one and vice versa --- .../features/webdav-related-new-endpoint.feature | 12 +++++++++++- .../features/webdav-related-old-endpoint.feature | 10 ++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) 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 From e0936236bfc5c96081d0eac29075a399ba911d79 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Wed, 4 Oct 2017 22:07:33 +0300 Subject: [PATCH 2/7] Use single CustomPropertiesBackend for old and new dav endpoints --- .../Sabre/CustomPropertiesBackend.php | 358 ------------------ .../dav/lib/Connector/Sabre/ServerFactory.php | 3 +- .../dav/lib/Files/CustomPropertiesBackend.php | 124 +++++- .../CustomPropertiesBackendTest.php | 8 +- 4 files changed, 111 insertions(+), 382 deletions(-) delete mode 100644 apps/dav/lib/Connector/Sabre/CustomPropertiesBackend.php rename apps/dav/tests/unit/{Connector/Sabre => Files}/CustomPropertiesBackendTest.php (97%) 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..cbc1cd9ae4e4 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -29,6 +29,7 @@ namespace OCA\DAV\Connector\Sabre; use OCA\DAV\Files\BrowserErrorPagePlugin; +use OCA\DAV\Files\CustomPropertiesBackend; use OCP\Files\Mount\IMountManager; use OCP\IConfig; use OCP\IDBConnection; @@ -180,7 +181,7 @@ public function createServer($baseUri, // custom properties plugin must be the last one $server->addPlugin( new \Sabre\DAV\PropertyStorage\Plugin( - new \OCA\DAV\Connector\Sabre\CustomPropertiesBackend( + new CustomPropertiesBackend( $objectTree, $this->databaseConnection, $this->userSession->getUser() diff --git a/apps/dav/lib/Files/CustomPropertiesBackend.php b/apps/dav/lib/Files/CustomPropertiesBackend.php index ff1fb4250b34..af0bfaa3d47b 100644 --- a/apps/dav/lib/Files/CustomPropertiesBackend.php +++ b/apps/dav/lib/Files/CustomPropertiesBackend.php @@ -22,6 +22,7 @@ namespace OCA\DAV\Files; use Doctrine\DBAL\Connection; +use OCA\DAV\Connector\Sabre\Directory; use OCA\DAV\Connector\Sabre\Node; use OCP\IDBConnection; use OCP\IUser; @@ -29,6 +30,9 @@ 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 { @@ -93,6 +97,31 @@ public function __construct( * @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}" : {$message}', + [ + 'app' => 'dav', + 'path' => $path, + 'message' => $e->getMessage(), + ] + ); + return; + } $requestedProps = $propFind->get404Properties(); @@ -106,8 +135,14 @@ public function propFind($path, PropFind $propFind) { return; } - $fileId = $this->getFileIdByPath($path); - $props = $this->getProperties($fileId, $requestedProps); + 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); } @@ -122,9 +157,13 @@ 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); + $node = $this->tree->getNodeForPath($path); + if (!($node instanceof Node)) { + return; + } + + $propPatch->handleRemaining(function($changedProps) use ($node) { + return $this->updateProperties($node, $changedProps); }); } @@ -135,13 +174,14 @@ public function propPatch($path, PropPatch $propPatch) { */ 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]); + if ($fileId) { + $statement = $this->connection->prepare( + 'DELETE FROM `*PREFIX*properties` WHERE `fileid` = ?' + ); + $statement->execute([$fileId]); + $statement->closeCursor(); + unset($this->cache[$fileId]); + } } /** @@ -158,7 +198,7 @@ public function move($source, $destination) { /** * Returns a list of properties for this nodes.; - * @param int $fileId + * @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 @@ -166,7 +206,8 @@ public function move($source, $destination) { * http://www.example.org/namespace#author If the array is empty, all * properties should be returned */ - private function getProperties($fileId, array $requestedProperties) { + private function getProperties(Node $node, array $requestedProperties) { + $fileId = $node->getId(); if (isset($this->cache[$fileId])) { return $this->cache[$fileId]; } @@ -204,13 +245,13 @@ private function getProperties($fileId, array $requestedProperties) { /** * Update properties * - * @param int $fileId node for which to update properties + * @param Node $node node for which to update properties * @param array $properties array of properties to update * * @return bool */ - private function updateProperties($fileId, $properties) { - + private function updateProperties($node, $properties) { + $fileId = $node->getId(); $deleteStatement = 'DELETE FROM `*PREFIX*properties`' . ' WHERE `fileid` = ? AND `propertyname` = ?'; @@ -221,7 +262,7 @@ private function updateProperties($fileId, $properties) { ' WHERE `fileid` = ? AND `propertyname` = ?'; // TODO: use "insert or update" strategy ? - $existing = $this->getProperties($fileId, []); + $existing = $this->getProperties($node, []); $this->connection->beginTransaction(); foreach ($properties as $propertyName => $propertyValue) { // If it was null, we need to delete the property @@ -261,9 +302,54 @@ private function updateProperties($fileId, $properties) { 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(); + if ($childId) { + $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], + [Connection::PARAM_STR_ARRAY, 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 + * @return int | null */ private function getFileIdByPath($filePath){ if (!$this->tree->nodeExists($filePath)) { diff --git a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php similarity index 97% rename from apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php rename to apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php index a52e47d0b7a4..93c12d30a618 100644 --- a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php @@ -20,7 +20,7 @@ * along with this program. If not, see * */ -namespace OCA\DAV\Tests\unit\Connector\Sabre; +namespace OCA\DAV\Tests\unit\Files; /** * Copyright (c) 2015 Vincent Petry @@ -34,7 +34,7 @@ * * @group DB * - * @package OCA\DAV\Tests\unit\Connector\Sabre + * @package OCA\DAV\Tests\unit\Files */ class CustomPropertiesBackendTest extends \Test\TestCase { @@ -49,7 +49,7 @@ class CustomPropertiesBackendTest extends \Test\TestCase { private $tree; /** - * @var \OCA\DAV\Connector\Sabre\CustomPropertiesBackend + * @var \OCA\DAV\Files\CustomPropertiesBackend */ private $plugin; @@ -75,7 +75,7 @@ public function setUp() { ->method('getUID') ->will($this->returnValue($userId)); - $this->plugin = new \OCA\DAV\Connector\Sabre\CustomPropertiesBackend( + $this->plugin = new \OCA\DAV\Files\CustomPropertiesBackend( $this->tree, \OC::$server->getDatabaseConnection(), $this->user From 9b6dcfe9a1be7f3ef3a525fe53d87a0ac6889ff9 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Mon, 9 Oct 2017 23:06:36 +0300 Subject: [PATCH 3/7] Store non-fs items data into dav_properties table --- .../Migrations/Version20170927201245.php | 66 +++ apps/dav/appinfo/info.xml | 2 +- .../dav/lib/Connector/Sabre/ServerFactory.php | 2 +- apps/dav/lib/DAV/CustomPropertiesBackend.php | 524 ++++++++++++++++++ .../dav/lib/Files/CustomPropertiesBackend.php | 364 ------------ apps/dav/lib/Server.php | 2 +- .../CustomPropertiesBackendTest.php | 16 +- 7 files changed, 599 insertions(+), 377 deletions(-) create mode 100644 apps/dav/appinfo/Migrations/Version20170927201245.php create mode 100644 apps/dav/lib/DAV/CustomPropertiesBackend.php delete mode 100644 apps/dav/lib/Files/CustomPropertiesBackend.php rename apps/dav/tests/unit/{Files => DAV}/CustomPropertiesBackendTest.php (95%) diff --git a/apps/dav/appinfo/Migrations/Version20170927201245.php b/apps/dav/appinfo/Migrations/Version20170927201245.php new file mode 100644 index 000000000000..1d872427d2f1 --- /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/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index cbc1cd9ae4e4..e02ad3ee8043 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -29,7 +29,7 @@ namespace OCA\DAV\Connector\Sabre; use OCA\DAV\Files\BrowserErrorPagePlugin; -use OCA\DAV\Files\CustomPropertiesBackend; +use OCA\DAV\DAV\CustomPropertiesBackend; use OCP\Files\Mount\IMountManager; use OCP\IConfig; use OCP\IDBConnection; diff --git a/apps/dav/lib/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php new file mode 100644 index 000000000000..c2aa384f95f3 --- /dev/null +++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php @@ -0,0 +1,524 @@ + + * + * @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 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 { + + 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` = ?'; + + 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` = ?'; + + /** + * Ignored properties + * + * @var array + */ + private $ignoredProperties = [ + '{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', + ]; + + /** + * @var Tree + */ + private $tree; + + /** + * @var IDBConnection + */ + private $connection; + + /** + * @var string + */ + private $user; + + /** + * Property cache for the filesystem items + * + * @var array + */ + private $cacheByFileId = []; + + /** + * Property cache for the non-filesystem items + * + * @var array + */ + private $cacheByPath = []; + + /** + * @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) { + $node = $this->getNodeForPath($path); + if (is_null($node)) { + 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($path, $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->getNodeForPath($path); + if (is_null($node)) { + return; + } + + $propPatch->handleRemaining(function($changedProps) use ($path, $node) { + return $this->updateProperties($path, $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) { + $node = $this->getNodeForPath($path); + if (is_null($node)) { + return; + } + + if ($node instanceof Node) { + $fileId = $node->getId(); + $statement = $this->connection->prepare(self::DELETE_BY_ID_STMT); + $statement->execute([$fileId]); + unset($this->cacheByFileId[$fileId]); + } else { + $statement = $this->connection->prepare(self::DELETE_BY_PATH_STMT); + $statement->execute([$path]); + unset($this->cacheByPath[$path]); + } + + $statement->closeCursor(); + } + + /** + * 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; + } + + if ($node instanceof Node) { + // Do nothing + } else { + $statement = $this->connection->prepare(self::UPDATE_BY_PATH_STMT); + $statement->execute([$destination, $source]); + $statement->closeCursor(); + } + } + + /** + * 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 + */ + private function getProperties($path, INode $node, array $requestedProperties) { + if ($node instanceof Node) { + $fileId = $node->getId(); + return $this->getPropertiesById($fileId, $requestedProperties); + } else { + return $this->getPropertiesByPath($path, $requestedProperties); + } + } + + /** + * Returns a list of properties for nodes; + * @param int $fileId + * @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 getPropertiesById($fileId, array $requestedProperties) { + if (!isset($this->cacheByFileId[$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->cacheByFileId[$fileId] = $props; + } + return $this->cacheByFileId[$fileId]; + } + + /** + * Returns a list of properties for nodes + * @param string $path + * @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 getPropertiesByPath($path, array $requestedProperties) { + if (!isset($this->cacheByPath[$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->cacheByPath[$path] = $props; + } + return $this->cacheByPath[$path]; + } + + /** + * @param string $sql + * @param array $whereValues + * @param array $whereTypes + * @return array + */ + private function fetchProperties($sql, $whereValues, $whereTypes) { + $result = $this->connection->executeQuery( + $sql, + $whereValues, + $whereTypes + ); + + $props = []; + while ($row = $result->fetch()) { + $props[$row['propertyname']] = $row['propertyvalue']; + } + + $result->closeCursor(); + return $props; + } + + /** + * Update properties + * + * @param string $path + * @param INode $node node for which to update properties + * @param array $changedProperties array of properties to update + * @return bool + */ + private function updateProperties($path, INode $node, $changedProperties) { + $existingProperties = $this->getProperties($path, $node, []); + if ($node instanceof Node) { + $fileId = $node->getId(); + return $this->updatePropertiesByFileId($fileId, $changedProperties, $existingProperties); + } else { + return $this->updatePropertiesByPath($path, $changedProperties, $existingProperties); + } + } + + /** + * @param int $fileId + * @param array $changedProperties + * @param array $existingProperties + * @return bool + */ + private function updatePropertiesByFileId($fileId, $changedProperties, $existingProperties) { + $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(); + unset($this->cacheByFileId[$fileId]); + + return true; + } + + /** + * @param string $path + * @param array $changedProperties + * @param array $existingProperties + * @return bool + */ + private function updatePropertiesByPath($path, $changedProperties, $existingProperties) { + $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(); + unset($this->cacheByPath[$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) { + $fileId = $node->getId(); + + if (isset($this->cacheByFileId[$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->cacheByFileId[$childId] = []; + } + } + + $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->cacheByFileId[$row['fileid']] = $props; + } + + $result->closeCursor(); + } + + /** + * @param string $path + * @return INode|null + */ + private 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/Files/CustomPropertiesBackend.php b/apps/dav/lib/Files/CustomPropertiesBackend.php deleted file mode 100644 index af0bfaa3d47b..000000000000 --- a/apps/dav/lib/Files/CustomPropertiesBackend.php +++ /dev/null @@ -1,364 +0,0 @@ - - * - * @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\Files; - -use Doctrine\DBAL\Connection; -use OCA\DAV\Connector\Sabre\Directory; -use OCA\DAV\Connector\Sabre\Node; -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', - '{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 string - */ - 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}" : {$message}', - [ - 'app' => 'dav', - 'path' => $path, - 'message' => $e->getMessage(), - ] - ); - 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); - if ($fileId) { - $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[] = 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(); - if ($childId) { - $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], - [Connection::PARAM_STR_ARRAY, 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 | null - */ - 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/Server.php b/apps/dav/lib/Server.php index 9ab1a1cddf37..4f467b7981e5 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -42,7 +42,7 @@ 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\CustomPropertiesBackend; use OCA\DAV\SystemTag\SystemTagPlugin; use OCA\DAV\Upload\ChunkingPlugin; use OCP\IRequest; diff --git a/apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php similarity index 95% rename from apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php rename to apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php index 93c12d30a618..4d179bbb8a30 100644 --- a/apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php @@ -20,21 +20,17 @@ * along with this program. If not, see * */ -namespace OCA\DAV\Tests\unit\Files; -/** - * 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\CustomPropertiesBackend; /** * Class CustomPropertiesBackend * * @group DB * - * @package OCA\DAV\Tests\unit\Files + * @package OCA\DAV\Tests\unit\DAV */ class CustomPropertiesBackendTest extends \Test\TestCase { @@ -49,7 +45,7 @@ class CustomPropertiesBackendTest extends \Test\TestCase { private $tree; /** - * @var \OCA\DAV\Files\CustomPropertiesBackend + * @var CustomPropertiesBackend */ private $plugin; @@ -75,7 +71,7 @@ public function setUp() { ->method('getUID') ->will($this->returnValue($userId)); - $this->plugin = new \OCA\DAV\Files\CustomPropertiesBackend( + $this->plugin = new CustomPropertiesBackend( $this->tree, \OC::$server->getDatabaseConnection(), $this->user From c15f74ae11b645ff8f939a5e0ce031dbe549db72 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Tue, 10 Oct 2017 23:34:23 +0300 Subject: [PATCH 4/7] Refactored into a base class and two subclasses --- .../Migrations/Version20170116150538.php | 2 +- .../Migrations/Version20170116170538.php | 2 +- .../Migrations/Version20170202213905.php | 2 +- .../Migrations/Version20170202220512.php | 2 +- .../Migrations/Version20170427182800.php | 2 +- .../Migrations/Version20170927201245.php | 2 +- .../dav/lib/Connector/Sabre/ServerFactory.php | 35 +- .../DAV/AbstractCustomPropertiesBackend.php | 260 +++++++++ apps/dav/lib/DAV/CustomPropertiesBackend.php | 524 ------------------ .../lib/DAV/FileCustomPropertiesBackend.php | 196 +++++++ .../lib/DAV/MiscCustomPropertiesBackend.php | 162 ++++++ apps/dav/lib/Server.php | 36 +- ...hp => FileCustomPropertiesBackendTest.php} | 10 +- 13 files changed, 684 insertions(+), 551 deletions(-) create mode 100644 apps/dav/lib/DAV/AbstractCustomPropertiesBackend.php delete mode 100644 apps/dav/lib/DAV/CustomPropertiesBackend.php create mode 100644 apps/dav/lib/DAV/FileCustomPropertiesBackend.php create mode 100644 apps/dav/lib/DAV/MiscCustomPropertiesBackend.php rename apps/dav/tests/unit/DAV/{CustomPropertiesBackendTest.php => FileCustomPropertiesBackendTest.php} (96%) 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 index 1d872427d2f1..d459e6a324cb 100644 --- a/apps/dav/appinfo/Migrations/Version20170927201245.php +++ b/apps/dav/appinfo/Migrations/Version20170927201245.php @@ -1,6 +1,6 @@ + * @author Viktar Dubiniuk * * @copyright Copyright (c) 2017, ownCloud GmbH * @license AGPL-3.0 diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index e02ad3ee8043..1dd369909b59 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -28,8 +28,9 @@ namespace OCA\DAV\Connector\Sabre; +use OCA\DAV\DAV\FileCustomPropertiesBackend; +use OCA\DAV\DAV\MiscCustomPropertiesBackend; use OCA\DAV\Files\BrowserErrorPagePlugin; -use OCA\DAV\DAV\CustomPropertiesBackend; use OCP\Files\Mount\IMountManager; use OCP\IConfig; use OCP\IDBConnection; @@ -178,16 +179,34 @@ public function createServer($baseUri, \OC::$server->getGroupManager(), $userFolder )); + // custom properties plugin must be the last one - $server->addPlugin( - new \Sabre\DAV\PropertyStorage\Plugin( - new CustomPropertiesBackend( - $objectTree, - $this->databaseConnection, - $this->userSession->getUser() - ) + $filePropertiesPlugin = new \Sabre\DAV\PropertyStorage\Plugin( + new FileCustomPropertiesBackend( + $objectTree, + $this->databaseConnection, + $this->userSession->getUser() + ) + ); + $filePropertiesPlugin->pathFilter = function($path) { + $pathInfo = $this->request->getPathInfo(); + // matches '/webdav', '/webdav/', '/webdav/adcd' but not '/webdavabcd' + return preg_match('!^/webdav(/|\z)!', $pathInfo); + }; + $server->addPlugin($filePropertiesPlugin); + + $miscPropertiesPlugin = new \Sabre\DAV\PropertyStorage\Plugin( + new MiscCustomPropertiesBackend( + $objectTree, + $this->databaseConnection, + $this->userSession->getUser() ) ); + $miscPropertiesPlugin->pathFilter = function($path) { + $pathInfo = $this->request->getPathInfo(); + return preg_match('!^/webdav(/|\z)!', $pathInfo); + }; + $server->addPlugin($miscPropertiesPlugin); } $server->addPlugin(new \OCA\DAV\Connector\Sabre\CopyEtagHeaderPlugin()); }, 30); // priority 30: after auth (10) and acl(20), before lock(50) and handling the request diff --git a/apps/dav/lib/DAV/AbstractCustomPropertiesBackend.php b/apps/dav/lib/DAV/AbstractCustomPropertiesBackend.php new file mode 100644 index 000000000000..c95f3fac1d55 --- /dev/null +++ b/apps/dav/lib/DAV/AbstractCustomPropertiesBackend.php @@ -0,0 +1,260 @@ + + * + * @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 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; + +abstract class AbstractCustomPropertiesBackend implements BackendInterface { + + /** + * Ignored properties + * + * @var array + */ + protected $ignoredProperties = [ + '{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', + ]; + + /** + * @var Tree + */ + protected $tree; + + /** + * @var IDBConnection + */ + protected $connection; + + /** + * @var string + */ + protected $user; + + /** + * Property cache for the filesystem items + * @var array + */ + protected $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(); + } + + /** + * 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. + * + * @param string $path + * @param PropFind $propFind + * @return void + */ + public function propFind($path, PropFind $propFind) { + $node = $this->getNodeForPath($path); + if (is_null($node)) { + return; + } + + $requestedProps = $propFind->get404Properties(); + + // these might appear + $requestedProps = array_diff( + $requestedProps, + $this->ignoredProperties + ); + + if (empty($requestedProps)) { + return; + } + + if ($propFind->getDepth() !== 0) { + $this->loadChildrenProperties($node, $requestedProps); + } + + $props = $this->getProperties($path, $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->getNodeForPath($path); + if (is_null($node)) { + return; + } + + $propPatch->handleRemaining(function($changedProps) use ($path, $node) { + return $this->updateProperties($path, $node, $changedProps); + }); + } + + /** + * @param string $sql + * @param array $whereValues + * @param array $whereTypes + * @return array + */ + protected function fetchProperties($sql, $whereValues, $whereTypes) { + $result = $this->connection->executeQuery( + $sql, + $whereValues, + $whereTypes + ); + + $props = []; + while ($row = $result->fetch()) { + $props[$row['propertyname']] = $row['propertyvalue']; + } + + $result->closeCursor(); + return $props; + } + + /** + * @param string $path + * @return INode|null + */ + 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/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php deleted file mode 100644 index c2aa384f95f3..000000000000 --- a/apps/dav/lib/DAV/CustomPropertiesBackend.php +++ /dev/null @@ -1,524 +0,0 @@ - - * - * @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 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 { - - 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` = ?'; - - 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` = ?'; - - /** - * Ignored properties - * - * @var array - */ - private $ignoredProperties = [ - '{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', - ]; - - /** - * @var Tree - */ - private $tree; - - /** - * @var IDBConnection - */ - private $connection; - - /** - * @var string - */ - private $user; - - /** - * Property cache for the filesystem items - * - * @var array - */ - private $cacheByFileId = []; - - /** - * Property cache for the non-filesystem items - * - * @var array - */ - private $cacheByPath = []; - - /** - * @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) { - $node = $this->getNodeForPath($path); - if (is_null($node)) { - 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($path, $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->getNodeForPath($path); - if (is_null($node)) { - return; - } - - $propPatch->handleRemaining(function($changedProps) use ($path, $node) { - return $this->updateProperties($path, $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) { - $node = $this->getNodeForPath($path); - if (is_null($node)) { - return; - } - - if ($node instanceof Node) { - $fileId = $node->getId(); - $statement = $this->connection->prepare(self::DELETE_BY_ID_STMT); - $statement->execute([$fileId]); - unset($this->cacheByFileId[$fileId]); - } else { - $statement = $this->connection->prepare(self::DELETE_BY_PATH_STMT); - $statement->execute([$path]); - unset($this->cacheByPath[$path]); - } - - $statement->closeCursor(); - } - - /** - * 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; - } - - if ($node instanceof Node) { - // Do nothing - } else { - $statement = $this->connection->prepare(self::UPDATE_BY_PATH_STMT); - $statement->execute([$destination, $source]); - $statement->closeCursor(); - } - } - - /** - * 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 - */ - private function getProperties($path, INode $node, array $requestedProperties) { - if ($node instanceof Node) { - $fileId = $node->getId(); - return $this->getPropertiesById($fileId, $requestedProperties); - } else { - return $this->getPropertiesByPath($path, $requestedProperties); - } - } - - /** - * Returns a list of properties for nodes; - * @param int $fileId - * @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 getPropertiesById($fileId, array $requestedProperties) { - if (!isset($this->cacheByFileId[$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->cacheByFileId[$fileId] = $props; - } - return $this->cacheByFileId[$fileId]; - } - - /** - * Returns a list of properties for nodes - * @param string $path - * @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 getPropertiesByPath($path, array $requestedProperties) { - if (!isset($this->cacheByPath[$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->cacheByPath[$path] = $props; - } - return $this->cacheByPath[$path]; - } - - /** - * @param string $sql - * @param array $whereValues - * @param array $whereTypes - * @return array - */ - private function fetchProperties($sql, $whereValues, $whereTypes) { - $result = $this->connection->executeQuery( - $sql, - $whereValues, - $whereTypes - ); - - $props = []; - while ($row = $result->fetch()) { - $props[$row['propertyname']] = $row['propertyvalue']; - } - - $result->closeCursor(); - return $props; - } - - /** - * Update properties - * - * @param string $path - * @param INode $node node for which to update properties - * @param array $changedProperties array of properties to update - * @return bool - */ - private function updateProperties($path, INode $node, $changedProperties) { - $existingProperties = $this->getProperties($path, $node, []); - if ($node instanceof Node) { - $fileId = $node->getId(); - return $this->updatePropertiesByFileId($fileId, $changedProperties, $existingProperties); - } else { - return $this->updatePropertiesByPath($path, $changedProperties, $existingProperties); - } - } - - /** - * @param int $fileId - * @param array $changedProperties - * @param array $existingProperties - * @return bool - */ - private function updatePropertiesByFileId($fileId, $changedProperties, $existingProperties) { - $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(); - unset($this->cacheByFileId[$fileId]); - - return true; - } - - /** - * @param string $path - * @param array $changedProperties - * @param array $existingProperties - * @return bool - */ - private function updatePropertiesByPath($path, $changedProperties, $existingProperties) { - $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(); - unset($this->cacheByPath[$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) { - $fileId = $node->getId(); - - if (isset($this->cacheByFileId[$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->cacheByFileId[$childId] = []; - } - } - - $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->cacheByFileId[$row['fileid']] = $props; - } - - $result->closeCursor(); - } - - /** - * @param string $path - * @return INode|null - */ - private 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..a86deb28b2ed --- /dev/null +++ b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php @@ -0,0 +1,196 @@ + + * @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 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..01cd748463b6 --- /dev/null +++ b/apps/dav/lib/DAV/MiscCustomPropertiesBackend.php @@ -0,0 +1,162 @@ + + * + * @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 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 4f467b7981e5..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\DAV\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/DAV/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php similarity index 96% rename from apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php rename to apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php index 4d179bbb8a30..dfc922b077dc 100644 --- a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php @@ -23,16 +23,16 @@ namespace OCA\DAV\Tests\unit\DAV; -use \OCA\DAV\DAV\CustomPropertiesBackend; +use \OCA\DAV\DAV\FileCustomPropertiesBackend; /** - * Class CustomPropertiesBackend + * Class FileCustomPropertiesBackendTest * * @group DB * * @package OCA\DAV\Tests\unit\DAV */ -class CustomPropertiesBackendTest extends \Test\TestCase { +class FileCustomPropertiesBackendTest extends \Test\TestCase { /** * @var \Sabre\DAV\Server @@ -45,7 +45,7 @@ class CustomPropertiesBackendTest extends \Test\TestCase { private $tree; /** - * @var CustomPropertiesBackend + * @var FileCustomPropertiesBackend */ private $plugin; @@ -71,7 +71,7 @@ public function setUp() { ->method('getUID') ->will($this->returnValue($userId)); - $this->plugin = new CustomPropertiesBackend( + $this->plugin = new FileCustomPropertiesBackend( $this->tree, \OC::$server->getDatabaseConnection(), $this->user From d8f4dc6a4b6c04a1026c43a05dac4b3b8ed70b53 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Wed, 11 Oct 2017 17:03:15 +0300 Subject: [PATCH 5/7] Add FilePoperties plugin only, server as this is a files endpoint --- .../dav/lib/Connector/Sabre/ServerFactory.php | 32 ++++--------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 1dd369909b59..0f89d16205ff 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -29,7 +29,6 @@ namespace OCA\DAV\Connector\Sabre; use OCA\DAV\DAV\FileCustomPropertiesBackend; -use OCA\DAV\DAV\MiscCustomPropertiesBackend; use OCA\DAV\Files\BrowserErrorPagePlugin; use OCP\Files\Mount\IMountManager; use OCP\IConfig; @@ -181,32 +180,15 @@ public function createServer($baseUri, )); // custom properties plugin must be the last one - $filePropertiesPlugin = new \Sabre\DAV\PropertyStorage\Plugin( - new FileCustomPropertiesBackend( - $objectTree, - $this->databaseConnection, - $this->userSession->getUser() + $server->addPlugin( + new \Sabre\DAV\PropertyStorage\Plugin( + new FileCustomPropertiesBackend( + $objectTree, + $this->databaseConnection, + $this->userSession->getUser() + ) ) ); - $filePropertiesPlugin->pathFilter = function($path) { - $pathInfo = $this->request->getPathInfo(); - // matches '/webdav', '/webdav/', '/webdav/adcd' but not '/webdavabcd' - return preg_match('!^/webdav(/|\z)!', $pathInfo); - }; - $server->addPlugin($filePropertiesPlugin); - - $miscPropertiesPlugin = new \Sabre\DAV\PropertyStorage\Plugin( - new MiscCustomPropertiesBackend( - $objectTree, - $this->databaseConnection, - $this->userSession->getUser() - ) - ); - $miscPropertiesPlugin->pathFilter = function($path) { - $pathInfo = $this->request->getPathInfo(); - return preg_match('!^/webdav(/|\z)!', $pathInfo); - }; - $server->addPlugin($miscPropertiesPlugin); } $server->addPlugin(new \OCA\DAV\Connector\Sabre\CopyEtagHeaderPlugin()); }, 30); // priority 30: after auth (10) and acl(20), before lock(50) and handling the request From 756e89e2aa4a4a5f83920861e9f8cfc8669f5494 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Fri, 13 Oct 2017 20:10:35 +0300 Subject: [PATCH 6/7] Add classes description --- apps/dav/lib/DAV/FileCustomPropertiesBackend.php | 8 ++++++++ apps/dav/lib/DAV/MiscCustomPropertiesBackend.php | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/apps/dav/lib/DAV/FileCustomPropertiesBackend.php b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php index a86deb28b2ed..6a3031f89f57 100644 --- a/apps/dav/lib/DAV/FileCustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php @@ -27,6 +27,14 @@ 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` = ?'; diff --git a/apps/dav/lib/DAV/MiscCustomPropertiesBackend.php b/apps/dav/lib/DAV/MiscCustomPropertiesBackend.php index 01cd748463b6..9d0a033a9f60 100644 --- a/apps/dav/lib/DAV/MiscCustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/MiscCustomPropertiesBackend.php @@ -24,6 +24,15 @@ 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` = ?'; From 3c13fca13d24f344eda515de8b1ea2dd522338a2 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Mon, 16 Oct 2017 21:51:12 +0300 Subject: [PATCH 7/7] Test for MiscCustomPropertiesBackend --- .../DAV/MiscCustomPropertiesBackendTest.php | 241 ++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 apps/dav/tests/unit/DAV/MiscCustomPropertiesBackendTest.php 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']); + } +}