Skip to content

Commit b1e7305

Browse files
ChristophWurstChristophWurst
committed
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 <[email protected]>
1 parent c8cbb73 commit b1e7305

File tree

10 files changed

+433
-28
lines changed

10 files changed

+433
-28
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: 26 additions & 2 deletions
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,11 @@ public function lastInsertId($seqName = null) {
267277
return $this->adapter->lastInsertId($seqName);
268278
}
269279

270-
// internal use
280+
/**
281+
* @internal
282+
* @return string
283+
* @throws Exception
284+
*/
271285
public function realLastInsertId($seqName = null) {
272286
return parent::lastInsertId($seqName);
273287
}
@@ -364,7 +378,9 @@ public function setValues($table, array $keys, array $values, array $updatePreco
364378
* Create an exclusive read+write lock on a table
365379
*
366380
* @param string $tableName
367-
* @throws \BadMethodCallException When trying to acquire a second lock
381+
*
382+
* @throws \BadMethodCallException When trying to acquire a second lock*@throws Exception
383+
* @throws Exception
368384
* @since 9.1.0
369385
*/
370386
public function lockTable($tableName) {
@@ -380,6 +396,7 @@ public function lockTable($tableName) {
380396
/**
381397
* Release a previous acquired lock again
382398
*
399+
* @throws Exception
383400
* @since 9.1.0
384401
*/
385402
public function unlockTable() {
@@ -415,6 +432,8 @@ public function errorInfo() {
415432
* Drop a table from the database if it exists
416433
*
417434
* @param string $table table name without the prefix
435+
*
436+
* @throws Exception
418437
*/
419438
public function dropTable($table) {
420439
$table = $this->tablePrefix . trim($table);
@@ -428,7 +447,9 @@ public function dropTable($table) {
428447
* Check if a table exists
429448
*
430449
* @param string $table table name without the prefix
450+
*
431451
* @return bool
452+
* @throws Exception
432453
*/
433454
public function tableExists($table) {
434455
$table = $this->tablePrefix . trim($table);
@@ -483,6 +504,7 @@ public function supports4ByteText() {
483504
* Create the schema of the connected database
484505
*
485506
* @return Schema
507+
* @throws Exception
486508
*/
487509
public function createSchema() {
488510
$schemaManager = new MDB2SchemaManager($this);
@@ -494,6 +516,8 @@ public function createSchema() {
494516
* Migrate the database to the given schema
495517
*
496518
* @param Schema $toSchema
519+
*
520+
* @throws Exception
497521
*/
498522
public function migrateToSchema(Schema $toSchema) {
499523
$schemaManager = new MDB2SchemaManager($this);

‎lib/private/DB/ConnectionAdapter.php‎

Lines changed: 97 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,15 @@
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;
3335
use OCP\IDBConnection;
36+
use OCP\PreConditionNotMetException;
3437

3538
/**
3639
* Adapts the public API to our internal DBAL connection wrapper
@@ -49,63 +52,115 @@ public function getQueryBuilder(): IQueryBuilder {
4952
}
5053

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

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

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

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

7190
public function lastInsertId(string $table): int {
72-
return (int) $this->inner->lastInsertId($table);
91+
try {
92+
return (int)$this->inner->lastInsertId($table);
93+
} catch (Exception $e) {
94+
throw DbalException::wrap($e);
95+
}
7396
}
7497

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

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

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

87122
public function lockTable($tableName): void {
88-
$this->inner->lockTable($tableName);
123+
try {
124+
$this->inner->lockTable($tableName);
125+
} catch (Exception $e) {
126+
throw DbalException::wrap($e);
127+
}
89128
}
90129

91130
public function unlockTable(): void {
92-
$this->inner->unlockTable();
131+
try {
132+
$this->inner->unlockTable();
133+
} catch (Exception $e) {
134+
throw DbalException::wrap($e);
135+
}
93136
}
94137

95138
public function beginTransaction(): void {
96-
$this->inner->beginTransaction();
139+
try {
140+
$this->inner->beginTransaction();
141+
} catch (Exception $e) {
142+
throw DbalException::wrap($e);
143+
}
97144
}
98145

99146
public function inTransaction(): bool {
100147
return $this->inner->inTransaction();
101148
}
102149

103150
public function commit(): void {
104-
$this->inner->commit();
151+
try {
152+
$this->inner->commit();
153+
} catch (Exception $e) {
154+
throw DbalException::wrap($e);
155+
}
105156
}
106157

107158
public function rollBack(): void {
108-
$this->inner->rollBack();
159+
try {
160+
$this->inner->rollBack();
161+
} catch (Exception $e) {
162+
throw DbalException::wrap($e);
163+
}
109164
}
110165

111166
public function getError(): string {
@@ -121,7 +176,11 @@ public function errorInfo() {
121176
}
122177

123178
public function connect(): bool {
124-
return $this->inner->connect();
179+
try {
180+
return $this->inner->connect();
181+
} catch (Exception $e) {
182+
throw DbalException::wrap($e);
183+
}
125184
}
126185

127186
public function close(): void {
@@ -140,11 +199,19 @@ public function getDatabasePlatform(): AbstractPlatform {
140199
}
141200

142201
public function dropTable(string $table): void {
143-
$this->inner->dropTable($table);
202+
try {
203+
$this->inner->dropTable($table);
204+
} catch (Exception $e) {
205+
throw DbalException::wrap($e);
206+
}
144207
}
145208

146209
public function tableExists(string $table): bool {
147-
return $this->inner->tableExists($table);
210+
try {
211+
return $this->inner->tableExists($table);
212+
} catch (Exception $e) {
213+
throw DbalException::wrap($e);
214+
}
148215
}
149216

150217
public function escapeLikeParameter(string $param): string {
@@ -159,11 +226,19 @@ public function supports4ByteText(): bool {
159226
* @todo leaks a 3rdparty type
160227
*/
161228
public function createSchema(): Schema {
162-
return $this->inner->createSchema();
229+
try {
230+
return $this->inner->createSchema();
231+
} catch (Exception $e) {
232+
throw DbalException::wrap($e);
233+
}
163234
}
164235

165236
public function migrateToSchema(Schema $toSchema): void {
166-
$this->inner->migrateToSchema($toSchema);
237+
try {
238+
$this->inner->migrateToSchema($toSchema);
239+
} catch (Exception $e) {
240+
throw DbalException::wrap($e);
241+
}
167242
}
168243

169244
public function getInner(): Connection {

0 commit comments

Comments
 (0)