From f4fd68da4cf50346e08c384a34c02c3836bdb6cc Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 1 Jul 2022 22:44:20 +0200 Subject: [PATCH 1/2] DAV custom props: catch Exception and rollback transaction in case - before exceptions were not caught, a started transaction might not have been finished - also resolve depractions and use IQueryBuilder Signed-off-by: Arthur Schiwon --- apps/dav/lib/DAV/CustomPropertiesBackend.php | 132 +++++++++++-------- 1 file changed, 76 insertions(+), 56 deletions(-) diff --git a/apps/dav/lib/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php index acee65cd00d9e..3f7a4a1b9e873 100644 --- a/apps/dav/lib/DAV/CustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php @@ -24,7 +24,8 @@ */ namespace OCA\DAV\DAV; -use OCA\DAV\Connector\Sabre\Node; +use OCP\DB\Exception; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IUser; use Sabre\DAV\PropertyStorage\Backend\BackendInterface; @@ -292,68 +293,56 @@ private function getUserProperties(string $path, array $requestedProperties) { } /** - * Update properties - * - * @param string $path path for which to update properties - * @param array $properties array of properties to update - * - * @return bool + * @throws Exception */ - private function updateProperties(string $path, array $properties) { - $deleteStatement = 'DELETE FROM `*PREFIX*properties`' . - ' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?'; - - $insertStatement = 'INSERT INTO `*PREFIX*properties`' . - ' (`userid`,`propertypath`,`propertyname`,`propertyvalue`) VALUES(?,?,?,?)'; - - $updateStatement = 'UPDATE `*PREFIX*properties` SET `propertyvalue` = ?' . - ' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?'; - + private function updateProperties(string $path, array $properties): bool { // TODO: use "insert or update" strategy ? $existing = $this->getUserProperties($path, []); - $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, - [ - $this->user->getUID(), - $this->formatPath($path), - $propertyName, - ] - ); - } - } else { - if ($propertyValue instanceOf \Sabre\DAV\Xml\Property\Complex) { - $propertyValue = $propertyValue->getXml(); - } elseif (!is_string($propertyValue)) { - $propertyValue = (string)$propertyValue; - } - if (!array_key_exists($propertyName, $existing)) { - $this->connection->executeUpdate($insertStatement, - [ - $this->user->getUID(), - $this->formatPath($path), - $propertyName, - $propertyValue, - ] - ); + try { + $this->connection->beginTransaction(); + foreach ($properties as $propertyName => $propertyValue) { + // common parameters for all queries + $dbParameters = [ + 'userid' => $this->user->getUID(), + 'propertyPath' => $this->formatPath($path), + 'propertyName' => $propertyName + ]; + + // If it was null, we need to delete the property + if (is_null($propertyValue)) { + if (array_key_exists($propertyName, $existing)) { + $deleteQuery = $deleteQuery ?? $this->createDeleteQuery(); + $deleteQuery + ->setParameters($dbParameters) + ->executeStatement(); + } } else { - $this->connection->executeUpdate($updateStatement, - [ - $propertyValue, - $this->user->getUID(), - $this->formatPath($path), - $propertyName, - ] - ); + if ($propertyValue instanceOf \Sabre\DAV\Xml\Property\Complex) { + $dbParameters['propertyValue'] = $propertyValue->getXml(); + } else { + $dbParameters['propertyValue'] = (string)$propertyValue; + } + + if (!array_key_exists($propertyName, $existing)) { + $insertQuery = $insertQuery ?? $this->createInsertQuery(); + $insertQuery + ->setParameters($dbParameters) + ->executeStatement(); + } else { + $updateQuery = $updateQuery ?? $this->createUpdateQuery(); + $updateQuery + ->setParameters($dbParameters) + ->executeStatement(); + } } } - } - $this->connection->commit(); - unset($this->userCache[$path]); + $this->connection->commit(); + unset($this->userCache[$path]); + } catch (Exception $e) { + $this->connection->rollBack(); + throw $e; + } return true; } @@ -371,4 +360,35 @@ private function formatPath(string $path): string { return $path; } } + + private function createDeleteQuery(): IQueryBuilder { + $deleteQuery = $this->connection->getQueryBuilder(); + $deleteQuery->delete('properties') + ->where($deleteQuery->expr()->eq('userid', $deleteQuery->createParameter('userid'))) + ->andWhere($deleteQuery->expr()->eq('propertypath', $deleteQuery->createParameter('propertyPath'))) + ->andWhere($deleteQuery->expr()->eq('propertyname', $deleteQuery->createParameter('propertyName'))); + return $deleteQuery; + } + + private function createInsertQuery(): IQueryBuilder { + $insertQuery = $this->connection->getQueryBuilder(); + $insertQuery->insert('properties') + ->values([ + 'userid' => $insertQuery->createParameter('userid'), + 'propertypath' => $insertQuery->createParameter('propertyPath'), + 'propertyname' => $insertQuery->createParameter('propertyName'), + 'propertyvalue' => $insertQuery->createParameter('propertyValue'), + ]); + return $insertQuery; + } + + private function createUpdateQuery(): IQueryBuilder { + $updateQuery = $this->connection->getQueryBuilder(); + $updateQuery->update('properties') + ->set('propertyvalue', $updateQuery->createParameter('propertyValue')) + ->where($updateQuery->expr()->eq('userid', $updateQuery->createParameter('userid'))) + ->andWhere($updateQuery->expr()->eq('propertypath', $updateQuery->createParameter('propertyPath'))) + ->andWhere($updateQuery->expr()->eq('propertyname', $updateQuery->createParameter('propertyName'))); + return $updateQuery; + } } From b2475b606ff4451c90bf82c4a332bff7871febeb Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 6 Jul 2022 16:13:59 +0200 Subject: [PATCH 2/2] catch any exception for transaction control Signed-off-by: Arthur Schiwon --- apps/dav/lib/DAV/CustomPropertiesBackend.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php index 3f7a4a1b9e873..b74a9d4d6089d 100644 --- a/apps/dav/lib/DAV/CustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php @@ -24,7 +24,7 @@ */ namespace OCA\DAV\DAV; -use OCP\DB\Exception; +use Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IUser;