Skip to content

Commit 2c9cdc1

Browse files
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>
1 parent c8cbb73 commit 2c9cdc1

File tree

10 files changed

+439
-27
lines changed

10 files changed

+439
-27
lines changed

lib/composer/composer/autoload_classmap.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@
161161
'OCP\\Contacts\\ContactsMenu\\IProvider' => $baseDir . '/lib/public/Contacts/ContactsMenu/IProvider.php',
162162
'OCP\\Contacts\\Events\\ContactInteractedWithEvent' => $baseDir . '/lib/public/Contacts/Events/ContactInteractedWithEvent.php',
163163
'OCP\\Contacts\\IManager' => $baseDir . '/lib/public/Contacts/IManager.php',
164+
'OCP\\DB\\Exception' => $baseDir . '/lib/public/DB/Exception.php',
164165
'OCP\\DB\\IPreparedStatement' => $baseDir . '/lib/public/DB/IPreparedStatement.php',
165166
'OCP\\DB\\IResult' => $baseDir . '/lib/public/DB/IResult.php',
166167
'OCP\\DB\\ISchemaWrapper' => $baseDir . '/lib/public/DB/ISchemaWrapper.php',
@@ -952,6 +953,7 @@
952953
'OC\\DB\\Connection' => $baseDir . '/lib/private/DB/Connection.php',
953954
'OC\\DB\\ConnectionAdapter' => $baseDir . '/lib/private/DB/ConnectionAdapter.php',
954955
'OC\\DB\\ConnectionFactory' => $baseDir . '/lib/private/DB/ConnectionFactory.php',
956+
'OC\\DB\\Exceptions\\DbalException' => $baseDir . '/lib/private/DB/Exceptions/DbalException.php',
955957
'OC\\DB\\MDB2SchemaManager' => $baseDir . '/lib/private/DB/MDB2SchemaManager.php',
956958
'OC\\DB\\MDB2SchemaReader' => $baseDir . '/lib/private/DB/MDB2SchemaReader.php',
957959
'OC\\DB\\MigrationException' => $baseDir . '/lib/private/DB/MigrationException.php',

lib/composer/composer/autoload_static.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
190190
'OCP\\Contacts\\ContactsMenu\\IProvider' => __DIR__ . '/../../..' . '/lib/public/Contacts/ContactsMenu/IProvider.php',
191191
'OCP\\Contacts\\Events\\ContactInteractedWithEvent' => __DIR__ . '/../../..' . '/lib/public/Contacts/Events/ContactInteractedWithEvent.php',
192192
'OCP\\Contacts\\IManager' => __DIR__ . '/../../..' . '/lib/public/Contacts/IManager.php',
193+
'OCP\\DB\\Exception' => __DIR__ . '/../../..' . '/lib/public/DB/Exception.php',
193194
'OCP\\DB\\IPreparedStatement' => __DIR__ . '/../../..' . '/lib/public/DB/IPreparedStatement.php',
194195
'OCP\\DB\\IResult' => __DIR__ . '/../../..' . '/lib/public/DB/IResult.php',
195196
'OCP\\DB\\ISchemaWrapper' => __DIR__ . '/../../..' . '/lib/public/DB/ISchemaWrapper.php',
@@ -981,6 +982,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
981982
'OC\\DB\\Connection' => __DIR__ . '/../../..' . '/lib/private/DB/Connection.php',
982983
'OC\\DB\\ConnectionAdapter' => __DIR__ . '/../../..' . '/lib/private/DB/ConnectionAdapter.php',
983984
'OC\\DB\\ConnectionFactory' => __DIR__ . '/../../..' . '/lib/private/DB/ConnectionFactory.php',
985+
'OC\\DB\\Exceptions\\DbalException' => __DIR__ . '/../../..' . '/lib/private/DB/Exceptions/DbalException.php',
984986
'OC\\DB\\MDB2SchemaManager' => __DIR__ . '/../../..' . '/lib/private/DB/MDB2SchemaManager.php',
985987
'OC\\DB\\MDB2SchemaReader' => __DIR__ . '/../../..' . '/lib/private/DB/MDB2SchemaReader.php',
986988
'OC\\DB\\MigrationException' => __DIR__ . '/../../..' . '/lib/private/DB/MigrationException.php',

lib/private/DB/Adapter.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
namespace OC\DB;
3232

33+
use Doctrine\DBAL\Exception;
3334
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
3435

3536
/**
@@ -49,7 +50,9 @@ public function __construct($conn) {
4950

5051
/**
5152
* @param string $table name
53+
*
5254
* @return int id of last insert statement
55+
* @throws Exception
5356
*/
5457
public function lastInsertId($table) {
5558
return (int) $this->conn->realLastInsertId($table);
@@ -67,6 +70,7 @@ public function fixupStatement($statement) {
6770
* Create an exclusive read+write lock on a table
6871
*
6972
* @param string $tableName
73+
* @throws Exception
7074
* @since 9.1.0
7175
*/
7276
public function lockTable($tableName) {
@@ -77,6 +81,7 @@ public function lockTable($tableName) {
7781
/**
7882
* Release a previous acquired lock again
7983
*
84+
* @throws Exception
8085
* @since 9.1.0
8186
*/
8287
public function unlockTable() {
@@ -94,7 +99,7 @@ public function unlockTable() {
9499
* If this is null or an empty array, all keys of $input will be compared
95100
* Please note: text fields (clob) must not be used in the compare array
96101
* @return int number of inserted rows
97-
* @throws \Doctrine\DBAL\Exception
102+
* @throws Exception
98103
* @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
99104
*/
100105
public function insertIfNotExist($table, $input, array $compare = null) {
@@ -130,6 +135,9 @@ public function insertIfNotExist($table, $input, array $compare = null) {
130135
}
131136
}
132137

138+
/**
139+
* @throws \OCP\DB\Exception
140+
*/
133141
public function insertIgnoreConflict(string $table,array $values) : int {
134142
try {
135143
$builder = $this->conn->getQueryBuilder();

lib/private/DB/Connection.php

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ class Connection extends ReconnectWrapper {
7474
/** @var int */
7575
protected $queriesExecuted = 0;
7676

77+
/**
78+
* @throws Exception
79+
*/
7780
public function connect() {
7881
try {
7982
return parent::connect();
@@ -183,7 +186,9 @@ public function __construct(array $params, Driver $driver, Configuration $config
183186
* @param string $statement The SQL statement to prepare.
184187
* @param int $limit
185188
* @param int $offset
189+
*
186190
* @return Statement The prepared statement.
191+
* @throws Exception
187192
*/
188193
public function prepare($statement, $limit = null, $offset = null): Statement {
189194
if ($limit === -1) {
@@ -221,6 +226,9 @@ public function executeQuery(string $sql, array $params = [], $types = [], Query
221226
return parent::executeQuery($sql, $params, $types, $qcp);
222227
}
223228

229+
/**
230+
* @throws Exception
231+
*/
224232
public function executeUpdate(string $sql, array $params = [], array $types = []): int {
225233
$sql = $this->replaceTablePrefix($sql);
226234
$sql = $this->adapter->fixupStatement($sql);
@@ -258,7 +266,9 @@ public function executeStatement($sql, array $params = [], array $types = []): i
258266
* columns or sequences.
259267
*
260268
* @param string $seqName Name of the sequence object from which the ID should be returned.
269+
*
261270
* @return string the last inserted ID.
271+
* @throws Exception
262272
*/
263273
public function lastInsertId($seqName = null) {
264274
if ($seqName) {
@@ -267,7 +277,10 @@ public function lastInsertId($seqName = null) {
267277
return $this->adapter->lastInsertId($seqName);
268278
}
269279

270-
// internal use
280+
/**
281+
* @internal
282+
* @throws Exception
283+
*/
271284
public function realLastInsertId($seqName = null) {
272285
return parent::lastInsertId($seqName);
273286
}
@@ -364,7 +377,9 @@ public function setValues($table, array $keys, array $values, array $updatePreco
364377
* Create an exclusive read+write lock on a table
365378
*
366379
* @param string $tableName
380+
*
367381
* @throws \BadMethodCallException When trying to acquire a second lock
382+
* @throws Exception
368383
* @since 9.1.0
369384
*/
370385
public function lockTable($tableName) {
@@ -380,6 +395,7 @@ public function lockTable($tableName) {
380395
/**
381396
* Release a previous acquired lock again
382397
*
398+
* @throws Exception
383399
* @since 9.1.0
384400
*/
385401
public function unlockTable() {
@@ -415,6 +431,8 @@ public function errorInfo() {
415431
* Drop a table from the database if it exists
416432
*
417433
* @param string $table table name without the prefix
434+
*
435+
* @throws Exception
418436
*/
419437
public function dropTable($table) {
420438
$table = $this->tablePrefix . trim($table);
@@ -428,7 +446,9 @@ public function dropTable($table) {
428446
* Check if a table exists
429447
*
430448
* @param string $table table name without the prefix
449+
*
431450
* @return bool
451+
* @throws Exception
432452
*/
433453
public function tableExists($table) {
434454
$table = $this->tablePrefix . trim($table);
@@ -483,6 +503,7 @@ public function supports4ByteText() {
483503
* Create the schema of the connected database
484504
*
485505
* @return Schema
506+
* @throws Exception
486507
*/
487508
public function createSchema() {
488509
$schemaManager = new MDB2SchemaManager($this);
@@ -494,6 +515,8 @@ public function createSchema() {
494515
* Migrate the database to the given schema
495516
*
496517
* @param Schema $toSchema
518+
*
519+
* @throws Exception
497520
*/
498521
public function migrateToSchema(Schema $toSchema) {
499522
$schemaManager = new MDB2SchemaManager($this);

lib/private/DB/ConnectionAdapter.php

Lines changed: 96 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@
2525

2626
namespace OC\DB;
2727

28+
use Doctrine\DBAL\Exception;
2829
use Doctrine\DBAL\Platforms\AbstractPlatform;
2930
use Doctrine\DBAL\Schema\Schema;
31+
use OC\DB\Exceptions\DbalException;
3032
use OCP\DB\IPreparedStatement;
3133
use OCP\DB\IResult;
3234
use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -49,63 +51,115 @@ public function getQueryBuilder(): IQueryBuilder {
4951
}
5052

5153
public function prepare($sql, $limit = null, $offset = null): IPreparedStatement {
52-
return new PreparedStatement(
53-
$this->inner->prepare($sql, $limit, $offset)
54-
);
54+
try {
55+
return new PreparedStatement(
56+
$this->inner->prepare($sql, $limit, $offset)
57+
);
58+
} catch (Exception $e) {
59+
throw DbalException::wrap($e);
60+
}
5561
}
5662

5763
public function executeQuery(string $sql, array $params = [], $types = []): IResult {
58-
return new ResultAdapter(
59-
$this->inner->executeQuery($sql, $params, $types)
60-
);
64+
try {
65+
return new ResultAdapter(
66+
$this->inner->executeQuery($sql, $params, $types)
67+
);
68+
} catch (Exception $e) {
69+
throw DbalException::wrap($e);
70+
}
6171
}
6272

6373
public function executeUpdate(string $sql, array $params = [], array $types = []): int {
64-
return $this->inner->executeUpdate($sql, $params, $types);
74+
try {
75+
return $this->inner->executeUpdate($sql, $params, $types);
76+
} catch (Exception $e) {
77+
throw DbalException::wrap($e);
78+
}
6579
}
6680

6781
public function executeStatement($sql, array $params = [], array $types = []): int {
68-
return $this->inner->executeStatement($sql, $params, $types);
82+
try {
83+
return $this->inner->executeStatement($sql, $params, $types);
84+
} catch (Exception $e) {
85+
throw DbalException::wrap($e);
86+
}
6987
}
7088

7189
public function lastInsertId(string $table): int {
72-
return (int) $this->inner->lastInsertId($table);
90+
try {
91+
return (int)$this->inner->lastInsertId($table);
92+
} catch (Exception $e) {
93+
throw DbalException::wrap($e);
94+
}
7395
}
7496

7597
public function insertIfNotExist(string $table, array $input, array $compare = null) {
76-
return $this->inner->insertIfNotExist($table, $input, $compare);
98+
try {
99+
return $this->inner->insertIfNotExist($table, $input, $compare);
100+
} catch (Exception $e) {
101+
throw DbalException::wrap($e);
102+
}
77103
}
78104

79105
public function insertIgnoreConflict(string $table, array $values): int {
80-
return $this->inner->insertIgnoreConflict($table, $values);
106+
try {
107+
return $this->inner->insertIgnoreConflict($table, $values);
108+
} catch (Exception $e) {
109+
throw DbalException::wrap($e);
110+
}
81111
}
82112

83113
public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []): int {
84-
return $this->inner->setValues($table, $keys, $values, $updatePreconditionValues);
114+
try {
115+
return $this->inner->setValues($table, $keys, $values, $updatePreconditionValues);
116+
} catch (Exception $e) {
117+
throw DbalException::wrap($e);
118+
}
85119
}
86120

87121
public function lockTable($tableName): void {
88-
$this->inner->lockTable($tableName);
122+
try {
123+
$this->inner->lockTable($tableName);
124+
} catch (Exception $e) {
125+
throw DbalException::wrap($e);
126+
}
89127
}
90128

91129
public function unlockTable(): void {
92-
$this->inner->unlockTable();
130+
try {
131+
$this->inner->unlockTable();
132+
} catch (Exception $e) {
133+
throw DbalException::wrap($e);
134+
}
93135
}
94136

95137
public function beginTransaction(): void {
96-
$this->inner->beginTransaction();
138+
try {
139+
$this->inner->beginTransaction();
140+
} catch (Exception $e) {
141+
throw DbalException::wrap($e);
142+
}
97143
}
98144

99145
public function inTransaction(): bool {
100146
return $this->inner->inTransaction();
101147
}
102148

103149
public function commit(): void {
104-
$this->inner->commit();
150+
try {
151+
$this->inner->commit();
152+
} catch (Exception $e) {
153+
throw DbalException::wrap($e);
154+
}
105155
}
106156

107157
public function rollBack(): void {
108-
$this->inner->rollBack();
158+
try {
159+
$this->inner->rollBack();
160+
} catch (Exception $e) {
161+
throw DbalException::wrap($e);
162+
}
109163
}
110164

111165
public function getError(): string {
@@ -121,7 +175,11 @@ public function errorInfo() {
121175
}
122176

123177
public function connect(): bool {
124-
return $this->inner->connect();
178+
try {
179+
return $this->inner->connect();
180+
} catch (Exception $e) {
181+
throw DbalException::wrap($e);
182+
}
125183
}
126184

127185
public function close(): void {
@@ -140,11 +198,19 @@ public function getDatabasePlatform(): AbstractPlatform {
140198
}
141199

142200
public function dropTable(string $table): void {
143-
$this->inner->dropTable($table);
201+
try {
202+
$this->inner->dropTable($table);
203+
} catch (Exception $e) {
204+
throw DbalException::wrap($e);
205+
}
144206
}
145207

146208
public function tableExists(string $table): bool {
147-
return $this->inner->tableExists($table);
209+
try {
210+
return $this->inner->tableExists($table);
211+
} catch (Exception $e) {
212+
throw DbalException::wrap($e);
213+
}
148214
}
149215

150216
public function escapeLikeParameter(string $param): string {
@@ -159,11 +225,19 @@ public function supports4ByteText(): bool {
159225
* @todo leaks a 3rdparty type
160226
*/
161227
public function createSchema(): Schema {
162-
return $this->inner->createSchema();
228+
try {
229+
return $this->inner->createSchema();
230+
} catch (Exception $e) {
231+
throw DbalException::wrap($e);
232+
}
163233
}
164234

165235
public function migrateToSchema(Schema $toSchema): void {
166-
$this->inner->migrateToSchema($toSchema);
236+
try {
237+
$this->inner->migrateToSchema($toSchema);
238+
} catch (Exception $e) {
239+
throw DbalException::wrap($e);
240+
}
167241
}
168242

169243
public function getInner(): Connection {

0 commit comments

Comments
 (0)