From 9cace533cf466ccdfcb215629545c07bffb00933 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Wed, 1 Nov 2017 00:31:48 +0300 Subject: [PATCH] Beyond the 1000 fileId limit --- .../lib/DAV/FileCustomPropertiesBackend.php | 46 ++++++++++---- .../DAV/FileCustomPropertiesBackendTest.php | 62 ++++++++++++++++++- 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/apps/dav/lib/DAV/FileCustomPropertiesBackend.php b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php index 6d3da2819cfe..d677632e9531 100644 --- a/apps/dav/lib/DAV/FileCustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/FileCustomPropertiesBackend.php @@ -24,6 +24,8 @@ namespace OCA\DAV\DAV; use Doctrine\DBAL\Connection; +use Doctrine\DBAL\Platforms\OraclePlatform; +use Doctrine\DBAL\Platforms\SqlitePlatform; use OCA\DAV\Connector\Sabre\Directory; use OCA\DAV\Connector\Sabre\Node; use Sabre\DAV\INode; @@ -187,19 +189,20 @@ protected function loadChildrenProperties(INode $node, $requestedProperties) { $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] - ); - + $fileIdChunks = $this->getChunks($childrenIds, count($requestedProperties)); $props = []; - while ($row = $result->fetch()) { - $props[$row['propertyname']] = $row['propertyvalue']; - $this->offsetSet($row['fileid'], $props); + foreach ($fileIdChunks as $chunk) { + $result = $this->connection->executeQuery( + $sql, + [$chunk, $requestedProperties], + [Connection::PARAM_STR_ARRAY, Connection::PARAM_STR_ARRAY] + ); + while ($row = $result->fetch()) { + $props[$row['propertyname']] = $row['propertyvalue']; + $this->offsetSet($row['fileid'], $props); + } + $result->closeCursor(); } - - $result->closeCursor(); } /** @@ -214,4 +217,25 @@ protected function getNodeForPath($path){ return $node; } + /** + * Chunk items from a single dimension array into a bidimensional array + * that has a limit of items for the second dimension + * This limit equals to 999 by default + * and is decreased by $otherPlaceholdersCount + * + * @param int[] $toSlice + * @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; + } + } diff --git a/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php index b55baf71242f..23c853c68951 100644 --- a/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php @@ -23,6 +23,9 @@ namespace OCA\DAV\Tests\unit\DAV; +use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\Platforms\MySqlPlatform; +use Doctrine\DBAL\Platforms\SqlitePlatform; use OCA\DAV\DAV\FileCustomPropertiesBackend; use Sabre\DAV\PropFind; use Sabre\DAV\SimpleCollection; @@ -79,7 +82,6 @@ public function setUp() { $this->user ); - $connection = \OC::$server->getDatabaseConnection(); $qb = $connection->getQueryBuilder(); $maxFunction = $qb->createFunction( @@ -359,4 +361,62 @@ public function testDeleteProperty() { $result = $propPatch->getResult(); $this->assertEquals(204, $result['customprop']); } + + public function slicesProvider() { + $emptyFileIds = []; + $fiveFileIds = [1, 2, 3, 4, 5]; + $thousandFileIds = []; + for ($i=0;$i<1000;$i++){ + $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]], + ]; + } + + /** + * @dataProvider slicesProvider + * @param $toSlice + * @param $otherPlaceholdersCount + * @param AbstractPlatform $platform + * @param $expected + */ + public function testGetChunks($toSlice, $otherPlaceholdersCount, AbstractPlatform $platform, $expected) { + $dbConnectionMock = $this->getMockBuilder(\OCP\IDBConnection::class) + ->disableOriginalConstructor() + ->getMock(); + + $dbConnectionMock->expects($this->any()) + ->method('getDatabasePlatform') + ->will($this->returnValue($platform)); + + $this->plugin = new FileCustomPropertiesBackend( + $this->tree, + $dbConnectionMock, + $this->user + ); + + $actual = $this->invokePrivate($this->plugin, 'getChunks', [$toSlice, $otherPlaceholdersCount]); + $this->assertEquals($expected, $actual); + } }