Skip to content

Commit

Permalink
Merge pull request #37172 from owncloud/bugfix/chunk-dav-props-query
Browse files Browse the repository at this point in the history
Perform chunked requests in any case on any sql platoform + php 7.1 s…
  • Loading branch information
phil-davis authored Mar 27, 2020
2 parents 0785f95 + 41f8b30 commit 83133c5
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 44 deletions.
27 changes: 12 additions & 15 deletions apps/dav/lib/DAV/FileCustomPropertiesBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -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` = ?';

/**
Expand All @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
41 changes: 12 additions & 29 deletions apps/dav/tests/unit/DAV/FileCustomPropertiesBackendTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -471,54 +471,37 @@ 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)],
];
}

/**
* @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,
$this->user,
\OC::$server->getRootFolder()
);

$actual = $this->invokePrivate(
$actual = self::invokePrivate(
$this->backend,
'getChunks',
[$toSlice, $otherPlaceholdersCount]
Expand Down
3 changes: 3 additions & 0 deletions changelog/unreleased/37172
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Change: Query on oc_properties is now always chunked

https://github.com/owncloud/core/pull/37172

0 comments on commit 83133c5

Please sign in to comment.