Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perform chunked requests in any case on any sql platoform + php 7.1 s… #37172

Merged
merged 1 commit into from
Mar 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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