From 41f8b3035829c53bc5cb30f7aa8c52f54e057959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 26 Mar 2020 17:17:50 +0100 Subject: [PATCH] Perform chunked requests in any case on any sql platoform + php 7.1 syntax cleanup --- .../lib/DAV/FileCustomPropertiesBackend.php | 27 ++++++------ .../DAV/FileCustomPropertiesBackendTest.php | 41 ++++++------------- changelog/unreleased/37172 | 3 ++ 3 files changed, 27 insertions(+), 44 deletions(-) create mode 100644 changelog/unreleased/37172 diff --git a/apps/dav/lib/DAV/FileCustomPropertiesBackend.php b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php index 96d34ff8215b..57bd587647b9 100644 --- a/apps/dav/lib/DAV/FileCustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php @@ -40,13 +40,13 @@ * @package OCA\DAV\DAV */ class FileCustomPropertiesBackend extends AbstractCustomPropertiesBackend { - const SELECT_BY_ID_STMT = 'SELECT * FROM `*PREFIX*properties` WHERE `fileid` = ?'; - const INSERT_BY_ID_STMT = 'INSERT INTO `*PREFIX*properties`' + public const SELECT_BY_ID_STMT = 'SELECT * FROM `*PREFIX*properties` WHERE `fileid` = ?'; + public const INSERT_BY_ID_STMT = 'INSERT INTO `*PREFIX*properties`' . ' (`fileid`,`propertyname`,`propertyvalue`) VALUES(?,?,?)'; - const UPDATE_BY_ID_AND_NAME_STMT = 'UPDATE `*PREFIX*properties`' + public 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`' + public const DELETE_BY_ID_STMT = 'DELETE FROM `*PREFIX*properties` WHERE `fileid` = ?'; + public const DELETE_BY_ID_AND_NAME_STMT = 'DELETE FROM `*PREFIX*properties`' . ' WHERE `fileid` = ? AND `propertyname` = ?'; /** @@ -68,7 +68,7 @@ class FileCustomPropertiesBackend extends AbstractCustomPropertiesBackend { * * @return void */ - public function beforeDelete($path) { + public function beforeDelete($path): void { try { $node = $this->getNodeForPath($path); '@phan-var \OCA\DAV\Connector\Sabre\Node $node'; @@ -89,6 +89,7 @@ public function beforeDelete($path) { * @param string $path path of node for which to delete properties * * @return void + * @throws \OCP\Files\NotFoundException */ public function delete($path) { $moveSource = $this->moveSource; @@ -218,6 +219,8 @@ protected function updateProperties($path, INode $node, $changedProperties) { * @param array $requestedProperties requested properties * * @return void + * @throws \OCA\DAV\Connector\Sabre\Exception\Forbidden + * @throws \Sabre\DAV\Exception\Locked */ protected function loadChildrenProperties(INode $node, $requestedProperties) { // note: pre-fetching only supported for depth <= 1 @@ -285,14 +288,8 @@ protected function getNodeForPath($path) { * @param int $otherPlaceholdersCount * @return array */ - private function getChunks($toSlice, $otherPlaceholdersCount = 0) { - $databasePlatform = $this->connection->getDatabasePlatform(); - if ($databasePlatform instanceof OraclePlatform || $databasePlatform instanceof SqlitePlatform) { - $slicer = 999 - $otherPlaceholdersCount; - $slices = \array_chunk($toSlice, $slicer); - } else { - $slices = \count($toSlice) ? [ 0 => $toSlice] : []; - } - return $slices; + private function getChunks($toSlice, $otherPlaceholdersCount = 0): array { + $slicer = 999 - $otherPlaceholdersCount; + return \array_chunk($toSlice, $slicer); } } diff --git a/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php index 693a47253964..5ee8bd6e4f4e 100644 --- a/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php @@ -101,7 +101,7 @@ public function setUp(): void { $this->rootFolder ); $this->plugin = new FileCustomPropertiesPlugin($this->backend); - + $connection = \OC::$server->getDatabaseConnection(); $qb = $connection->getQueryBuilder(); $maxFunction = $qb->createFunction( @@ -174,7 +174,7 @@ public function testGetPropertiesForCollection() { $node->expects($this->any()) ->method('getName') ->will($this->returnValue('dummypath')); - + $this->tree->expects($this->any()) ->method('getNodeForPath') ->with('/dummypath') @@ -471,27 +471,15 @@ public function slicesProvider() { $thousandFileIds[] = $i; } - $sqlitePlatform = new SqlitePlatform(); - $mysqlPlatform = new MySqlPlatform(); - return [ - [$emptyFileIds, 0, $sqlitePlatform, []], - [$emptyFileIds, 5, $sqlitePlatform, []], - [$fiveFileIds, 0, $sqlitePlatform, [ 0 => $fiveFileIds]], - [$fiveFileIds, 5, $sqlitePlatform, [ 0 => $fiveFileIds]], - [$fiveFileIds, 994, $sqlitePlatform, [ 0 => $fiveFileIds]], - [$fiveFileIds, 995, $sqlitePlatform, [ 0 => [1,2,3,4] , 1 => [5]]], - [$thousandFileIds, 0, $sqlitePlatform, \array_chunk($thousandFileIds, 999)], - [$thousandFileIds, 5, $sqlitePlatform, \array_chunk($thousandFileIds, 994)], - - [$emptyFileIds, 0, $mysqlPlatform, []], - [$emptyFileIds, 5, $mysqlPlatform, []], - [$fiveFileIds, 0, $mysqlPlatform, [ 0 => $fiveFileIds]], - [$fiveFileIds, 5, $mysqlPlatform, [ 0 => $fiveFileIds]], - [$fiveFileIds, 994, $mysqlPlatform, [ 0 => $fiveFileIds]], - [$fiveFileIds, 995, $mysqlPlatform, [0 => $fiveFileIds]], - [$thousandFileIds, 0, $mysqlPlatform, [0 => $thousandFileIds]], - [$thousandFileIds, 5, $mysqlPlatform, [0 => $thousandFileIds]], + [$emptyFileIds, 0, []], + [$emptyFileIds, 5, []], + [$fiveFileIds, 0, [ 0 => $fiveFileIds]], + [$fiveFileIds, 5, [ 0 => $fiveFileIds]], + [$fiveFileIds, 994, [ 0 => $fiveFileIds]], + [$fiveFileIds, 995, [ 0 => [1,2,3,4] , 1 => [5]]], + [$thousandFileIds, 0, \array_chunk($thousandFileIds, 999)], + [$thousandFileIds, 5, \array_chunk($thousandFileIds, 994)], ]; } @@ -499,18 +487,13 @@ public function slicesProvider() { * @dataProvider slicesProvider * @param $toSlice * @param $otherPlaceholdersCount - * @param AbstractPlatform $platform * @param $expected */ - public function testGetChunks($toSlice, $otherPlaceholdersCount, AbstractPlatform $platform, $expected) { + public function testGetChunks($toSlice, $otherPlaceholdersCount, $expected) { $dbConnectionMock = $this->getMockBuilder(\OCP\IDBConnection::class) ->disableOriginalConstructor() ->getMock(); - $dbConnectionMock->expects($this->any()) - ->method('getDatabasePlatform') - ->will($this->returnValue($platform)); - $this->backend = new FileCustomPropertiesBackend( $this->tree, $dbConnectionMock, @@ -518,7 +501,7 @@ public function testGetChunks($toSlice, $otherPlaceholdersCount, AbstractPlatfor \OC::$server->getRootFolder() ); - $actual = $this->invokePrivate( + $actual = self::invokePrivate( $this->backend, 'getChunks', [$toSlice, $otherPlaceholdersCount] diff --git a/changelog/unreleased/37172 b/changelog/unreleased/37172 new file mode 100644 index 000000000000..d0d14487f8f9 --- /dev/null +++ b/changelog/unreleased/37172 @@ -0,0 +1,3 @@ +Change: Query on oc_properties is now always chunked + +https://github.com/owncloud/core/pull/37172