diff options
author | Christoph Wurst <christoph@winzerhof-wurst.at> | 2021-01-12 12:24:36 +0100 |
---|---|---|
committer | Christoph Wurst <christoph@winzerhof-wurst.at> | 2021-01-12 16:38:23 +0100 |
commit | 2c9cdc1cdbf900a8578c80075f5ea2da479c4f80 (patch) | |
tree | d215a64989be204099d3be926bdd390c9f11c359 /lib/private/DB | |
parent | c8cbb73c05714f035eb63fe91873fc43e0557f1c (diff) | |
download | nextcloud-server-2c9cdc1cdbf900a8578c80075f5ea2da479c4f80.tar.gz nextcloud-server-2c9cdc1cdbf900a8578c80075f5ea2da479c4f80.zip |
Add our own DB exception abstraction
Right now our API exports the Doctrine/dbal exception. As we've seen
with the dbal 3 upgrade, the leakage of 3rdparty types is problematic as
a dependency update means lots of work in apps, due to the direct
dependency of what Nextcloud ships. This breaks this dependency so that
apps only need to depend on our public API. That API can then be vendor
(db lib) agnostic and we can work around future deprecations/removals in
dbal more easily.
Right now the type of exception thrown is transported as "reason". For
the more popular types of errors we can extend the new exception class
and allow apps to catch specific errors only. Right now they have to
catch-check-rethrow. This is not ideal, but better than the dependnecy
on dbal.
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Diffstat (limited to 'lib/private/DB')
-rw-r--r-- | lib/private/DB/Adapter.php | 10 | ||||
-rw-r--r-- | lib/private/DB/Connection.php | 25 | ||||
-rw-r--r-- | lib/private/DB/ConnectionAdapter.php | 118 | ||||
-rw-r--r-- | lib/private/DB/Exceptions/DbalException.php | 136 | ||||
-rw-r--r-- | lib/private/DB/Migrator.php | 7 |
5 files changed, 272 insertions, 24 deletions
diff --git a/lib/private/DB/Adapter.php b/lib/private/DB/Adapter.php index 49b831301be..62514e7d83a 100644 --- a/lib/private/DB/Adapter.php +++ b/lib/private/DB/Adapter.php @@ -30,6 +30,7 @@ namespace OC\DB; +use Doctrine\DBAL\Exception; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; /** @@ -49,7 +50,9 @@ class Adapter { /** * @param string $table name + * * @return int id of last insert statement + * @throws Exception */ public function lastInsertId($table) { return (int) $this->conn->realLastInsertId($table); @@ -67,6 +70,7 @@ class Adapter { * Create an exclusive read+write lock on a table * * @param string $tableName + * @throws Exception * @since 9.1.0 */ public function lockTable($tableName) { @@ -77,6 +81,7 @@ class Adapter { /** * Release a previous acquired lock again * + * @throws Exception * @since 9.1.0 */ public function unlockTable() { @@ -94,7 +99,7 @@ class Adapter { * If this is null or an empty array, all keys of $input will be compared * Please note: text fields (clob) must not be used in the compare array * @return int number of inserted rows - * @throws \Doctrine\DBAL\Exception + * @throws Exception * @deprecated 15.0.0 - use unique index and "try { $db->insert() } catch (UniqueConstraintViolationException $e) {}" instead, because it is more reliable and does not have the risk for deadlocks - see https://github.com/nextcloud/server/pull/12371 */ public function insertIfNotExist($table, $input, array $compare = null) { @@ -130,6 +135,9 @@ class Adapter { } } + /** + * @throws \OCP\DB\Exception + */ public function insertIgnoreConflict(string $table,array $values) : int { try { $builder = $this->conn->getQueryBuilder(); diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index c67c6df0826..cb7af4d51e2 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -74,6 +74,9 @@ class Connection extends ReconnectWrapper { /** @var int */ protected $queriesExecuted = 0; + /** + * @throws Exception + */ public function connect() { try { return parent::connect(); @@ -183,7 +186,9 @@ class Connection extends ReconnectWrapper { * @param string $statement The SQL statement to prepare. * @param int $limit * @param int $offset + * * @return Statement The prepared statement. + * @throws Exception */ public function prepare($statement, $limit = null, $offset = null): Statement { if ($limit === -1) { @@ -221,6 +226,9 @@ class Connection extends ReconnectWrapper { return parent::executeQuery($sql, $params, $types, $qcp); } + /** + * @throws Exception + */ public function executeUpdate(string $sql, array $params = [], array $types = []): int { $sql = $this->replaceTablePrefix($sql); $sql = $this->adapter->fixupStatement($sql); @@ -258,7 +266,9 @@ class Connection extends ReconnectWrapper { * columns or sequences. * * @param string $seqName Name of the sequence object from which the ID should be returned. + * * @return string the last inserted ID. + * @throws Exception */ public function lastInsertId($seqName = null) { if ($seqName) { @@ -267,7 +277,10 @@ class Connection extends ReconnectWrapper { return $this->adapter->lastInsertId($seqName); } - // internal use + /** + * @internal + * @throws Exception + */ public function realLastInsertId($seqName = null) { return parent::lastInsertId($seqName); } @@ -364,7 +377,9 @@ class Connection extends ReconnectWrapper { * Create an exclusive read+write lock on a table * * @param string $tableName + * * @throws \BadMethodCallException When trying to acquire a second lock + * @throws Exception * @since 9.1.0 */ public function lockTable($tableName) { @@ -380,6 +395,7 @@ class Connection extends ReconnectWrapper { /** * Release a previous acquired lock again * + * @throws Exception * @since 9.1.0 */ public function unlockTable() { @@ -415,6 +431,8 @@ class Connection extends ReconnectWrapper { * Drop a table from the database if it exists * * @param string $table table name without the prefix + * + * @throws Exception */ public function dropTable($table) { $table = $this->tablePrefix . trim($table); @@ -428,7 +446,9 @@ class Connection extends ReconnectWrapper { * Check if a table exists * * @param string $table table name without the prefix + * * @return bool + * @throws Exception */ public function tableExists($table) { $table = $this->tablePrefix . trim($table); @@ -483,6 +503,7 @@ class Connection extends ReconnectWrapper { * Create the schema of the connected database * * @return Schema + * @throws Exception */ public function createSchema() { $schemaManager = new MDB2SchemaManager($this); @@ -494,6 +515,8 @@ class Connection extends ReconnectWrapper { * Migrate the database to the given schema * * @param Schema $toSchema + * + * @throws Exception */ public function migrateToSchema(Schema $toSchema) { $schemaManager = new MDB2SchemaManager($this); diff --git a/lib/private/DB/ConnectionAdapter.php b/lib/private/DB/ConnectionAdapter.php index 97a0b60044d..62c013e4dcf 100644 --- a/lib/private/DB/ConnectionAdapter.php +++ b/lib/private/DB/ConnectionAdapter.php @@ -25,8 +25,10 @@ declare(strict_types=1); namespace OC\DB; +use Doctrine\DBAL\Exception; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Schema\Schema; +use OC\DB\Exceptions\DbalException; use OCP\DB\IPreparedStatement; use OCP\DB\IResult; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -49,51 +51,95 @@ class ConnectionAdapter implements IDBConnection { } public function prepare($sql, $limit = null, $offset = null): IPreparedStatement { - return new PreparedStatement( - $this->inner->prepare($sql, $limit, $offset) - ); + try { + return new PreparedStatement( + $this->inner->prepare($sql, $limit, $offset) + ); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function executeQuery(string $sql, array $params = [], $types = []): IResult { - return new ResultAdapter( - $this->inner->executeQuery($sql, $params, $types) - ); + try { + return new ResultAdapter( + $this->inner->executeQuery($sql, $params, $types) + ); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function executeUpdate(string $sql, array $params = [], array $types = []): int { - return $this->inner->executeUpdate($sql, $params, $types); + try { + return $this->inner->executeUpdate($sql, $params, $types); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function executeStatement($sql, array $params = [], array $types = []): int { - return $this->inner->executeStatement($sql, $params, $types); + try { + return $this->inner->executeStatement($sql, $params, $types); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function lastInsertId(string $table): int { - return (int) $this->inner->lastInsertId($table); + try { + return (int)$this->inner->lastInsertId($table); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function insertIfNotExist(string $table, array $input, array $compare = null) { - return $this->inner->insertIfNotExist($table, $input, $compare); + try { + return $this->inner->insertIfNotExist($table, $input, $compare); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function insertIgnoreConflict(string $table, array $values): int { - return $this->inner->insertIgnoreConflict($table, $values); + try { + return $this->inner->insertIgnoreConflict($table, $values); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []): int { - return $this->inner->setValues($table, $keys, $values, $updatePreconditionValues); + try { + return $this->inner->setValues($table, $keys, $values, $updatePreconditionValues); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function lockTable($tableName): void { - $this->inner->lockTable($tableName); + try { + $this->inner->lockTable($tableName); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function unlockTable(): void { - $this->inner->unlockTable(); + try { + $this->inner->unlockTable(); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function beginTransaction(): void { - $this->inner->beginTransaction(); + try { + $this->inner->beginTransaction(); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function inTransaction(): bool { @@ -101,11 +147,19 @@ class ConnectionAdapter implements IDBConnection { } public function commit(): void { - $this->inner->commit(); + try { + $this->inner->commit(); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function rollBack(): void { - $this->inner->rollBack(); + try { + $this->inner->rollBack(); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function getError(): string { @@ -121,7 +175,11 @@ class ConnectionAdapter implements IDBConnection { } public function connect(): bool { - return $this->inner->connect(); + try { + return $this->inner->connect(); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function close(): void { @@ -140,11 +198,19 @@ class ConnectionAdapter implements IDBConnection { } public function dropTable(string $table): void { - $this->inner->dropTable($table); + try { + $this->inner->dropTable($table); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function tableExists(string $table): bool { - return $this->inner->tableExists($table); + try { + return $this->inner->tableExists($table); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function escapeLikeParameter(string $param): string { @@ -159,11 +225,19 @@ class ConnectionAdapter implements IDBConnection { * @todo leaks a 3rdparty type */ public function createSchema(): Schema { - return $this->inner->createSchema(); + try { + return $this->inner->createSchema(); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function migrateToSchema(Schema $toSchema): void { - $this->inner->migrateToSchema($toSchema); + try { + $this->inner->migrateToSchema($toSchema); + } catch (Exception $e) { + throw DbalException::wrap($e); + } } public function getInner(): Connection { diff --git a/lib/private/DB/Exceptions/DbalException.php b/lib/private/DB/Exceptions/DbalException.php new file mode 100644 index 00000000000..4e0e1517048 --- /dev/null +++ b/lib/private/DB/Exceptions/DbalException.php @@ -0,0 +1,136 @@ +<?php + +declare(strict_types=1); + +/* + * @copyright 2021 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2021 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OC\DB\Exceptions; + +use Doctrine\DBAL\ConnectionException; +use Doctrine\DBAL\Exception\ConstraintViolationException; +use Doctrine\DBAL\Exception\DatabaseObjectExistsException; +use Doctrine\DBAL\Exception\DatabaseObjectNotFoundException; +use Doctrine\DBAL\Exception\DeadlockException; +use Doctrine\DBAL\Exception\DriverException; +use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException; +use Doctrine\DBAL\Exception\InvalidArgumentException; +use Doctrine\DBAL\Exception\InvalidFieldNameException; +use Doctrine\DBAL\Exception\NonUniqueFieldNameException; +use Doctrine\DBAL\Exception\NotNullConstraintViolationException; +use Doctrine\DBAL\Exception\ServerException; +use Doctrine\DBAL\Exception\SyntaxErrorException; +use Doctrine\DBAL\Exception\UniqueConstraintViolationException; +use OCP\DB\Exception; + +/** + * Wrapper around the raw dbal exception, so we can pass it to apps that catch + * our OCP db exception + * + * @psalm-immutable + */ +class DbalException extends Exception { + + /** @var \Doctrine\DBAL\Exception */ + private $original; + + /** + * @param \Doctrine\DBAL\Exception $original + * @param int $code + * @param string $message + */ + private function __construct(\Doctrine\DBAL\Exception $original, int $code, string $message) { + parent::__construct( + $message, + $code, + $original + ); + $this->original = $original; + } + + public static function wrap(\Doctrine\DBAL\Exception $original, string $message = ''): self { + return new self( + $original, + is_int($original->getCode()) ? $original->getCode() : 0, + empty($message) ? $original->getMessage() : $message + ); + } + + public function getReason(): ?int { + /** + * Generic errors + */ + if ($this->original instanceof ConnectionException) { + return parent::REASON_CONNECTION_LOST; + } + if ($this->original instanceof DriverException) { + return parent::REASON_DRIVER; + } + if ($this->original instanceof InvalidArgumentException) { + return parent::REASON_INVALID_ARGUMENT; + } + + /** + * Constraint errors + */ + if ($this->original instanceof ForeignKeyConstraintViolationException) { + return parent::REASON_FOREIGN_KEY_VIOLATION; + } + if ($this->original instanceof NotNullConstraintViolationException) { + return parent::REASON_NOT_NULL_CONSTRAINT_VIOLATION; + } + if ($this->original instanceof UniqueConstraintViolationException) { + return parent::REASON_UNIQUE_CONSTRAINT_VIOLATION; + } + // The base exception comes last + if ($this->original instanceof ConstraintViolationException) { + return parent::REASON_CONSTRAINT_VIOLATION; + } + + /** + * Other server errors + */ + if ($this->original instanceof DatabaseObjectExistsException) { + return parent::REASON_DATABASE_OBJECT_EXISTS; + } + if ($this->original instanceof DatabaseObjectNotFoundException) { + return parent::REASON_DATABASE_OBJECT_NOT_FOUND; + } + if ($this->original instanceof DeadlockException) { + return parent::REASON_DEADLOCK; + } + if ($this->original instanceof InvalidFieldNameException) { + return parent::REASON_INVALID_FIELD_NAME; + } + if ($this->original instanceof NonUniqueFieldNameException) { + return parent::REASON_NON_UNIQUE_FIELD_NAME; + } + if ($this->original instanceof SyntaxErrorException) { + return parent::REASON_SYNTAX_ERROR; + } + // The base server exception class comes last + if ($this->original instanceof ServerException) { + return parent::REASON_SERVER; + } + + return null; + } +} diff --git a/lib/private/DB/Migrator.php b/lib/private/DB/Migrator.php index e50927f620b..dcf0db89f72 100644 --- a/lib/private/DB/Migrator.php +++ b/lib/private/DB/Migrator.php @@ -82,6 +82,8 @@ class Migrator { /** * @param \Doctrine\DBAL\Schema\Schema $targetSchema + * + * @throws Exception */ public function migrate(Schema $targetSchema) { $this->noEmit = true; @@ -171,6 +173,9 @@ class Migrator { return new Table($newName, $table->getColumns(), $newIndexes, [], [], $table->getOptions()); } + /** + * @throws Exception + */ public function createSchema() { $this->connection->getConfiguration()->setSchemaAssetsFilter(function ($asset) { /** @var string|AbstractAsset $asset */ @@ -231,6 +236,8 @@ class Migrator { /** * @param \Doctrine\DBAL\Schema\Schema $targetSchema * @param \Doctrine\DBAL\Connection $connection + * + * @throws Exception */ protected function applySchema(Schema $targetSchema, \Doctrine\DBAL\Connection $connection = null) { if (is_null($connection)) { |