Skip to content

Commit c4e6fbd

Browse files
Carl SchwanCarlSchwan
authored andcommitted
fix(query-builder): Don't catch UniqueConstraintViolationException
UniqueConstraintViolationException is no longer throw directly but instead is now wrapped inside a \OCP\DB\Exception. So check the exception reason. Signed-off-by: Carl Schwan <[email protected]>
1 parent c21b816 commit c4e6fbd

8 files changed

Lines changed: 66 additions & 36 deletions

File tree

lib/private/Authentication/Token/Manager.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@
77
*/
88
namespace OC\Authentication\Token;
99

10-
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
1110
use OC\Authentication\Exceptions\InvalidTokenException as OcInvalidTokenException;
1211
use OC\Authentication\Exceptions\PasswordlessTokenException;
1312
use OCP\Authentication\Exceptions\ExpiredTokenException;
1413
use OCP\Authentication\Exceptions\InvalidTokenException;
1514
use OCP\Authentication\Exceptions\WipeTokenException;
1615
use OCP\Authentication\Token\IProvider as OCPIProvider;
1716
use OCP\Authentication\Token\IToken as OCPIToken;
17+
use OCP\DB\Exception;
1818

1919
class Manager implements IProvider, OCPIProvider {
2020
/** @var PublicKeyTokenProvider */
@@ -60,7 +60,10 @@ public function generateToken(string $token,
6060
$remember,
6161
$scope,
6262
);
63-
} catch (UniqueConstraintViolationException $e) {
63+
} catch (Exception $e) {
64+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
65+
throw $e;
66+
}
6467
// It's rare, but if two requests of the same session (e.g. env-based SAML)
6568
// try to create the session token they might end up here at the same time
6669
// because we use the session ID as token and the db token is created anew

lib/private/Collaboration/Resources/Manager.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@
88
*/
99
namespace OC\Collaboration\Resources;
1010

11-
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
1211
use OCP\Collaboration\Resources\CollectionException;
1312
use OCP\Collaboration\Resources\ICollection;
1413
use OCP\Collaboration\Resources\IManager;
1514
use OCP\Collaboration\Resources\IProvider;
1615
use OCP\Collaboration\Resources\IProviderManager;
1716
use OCP\Collaboration\Resources\IResource;
1817
use OCP\Collaboration\Resources\ResourceException;
18+
use OCP\DB\Exception;
1919
use OCP\DB\QueryBuilder\IQueryBuilder;
2020
use OCP\IDBConnection;
2121
use OCP\IUser;
@@ -351,7 +351,10 @@ public function cacheAccessForResource(IResource $resource, ?IUser $user, bool $
351351
]);
352352
try {
353353
$query->executeStatement();
354-
} catch (UniqueConstraintViolationException $e) {
354+
} catch (Exception $e) {
355+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
356+
throw $e;
357+
}
355358
}
356359
}
357360

@@ -367,7 +370,10 @@ public function cacheAccessForCollection(ICollection $collection, ?IUser $user,
367370
]);
368371
try {
369372
$query->executeStatement();
370-
} catch (UniqueConstraintViolationException $e) {
373+
} catch (Exception $e) {
374+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
375+
throw $e;
376+
}
371377
}
372378
}
373379

lib/private/Files/Cache/Cache.php

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88
namespace OC\Files\Cache;
99

10-
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
1110
use OC\DB\Exceptions\DbalException;
1211
use OC\DB\QueryBuilder\Sharded\ShardDefinition;
1312
use OC\Files\Cache\Wrapper\CacheJail;
@@ -16,6 +15,7 @@
1615
use OC\Files\Search\SearchQuery;
1716
use OC\Files\Storage\Wrapper\Encryption;
1817
use OC\SystemConfig;
18+
use OCP\DB\Exception;
1919
use OCP\DB\QueryBuilder\IQueryBuilder;
2020
use OCP\EventDispatcher\IEventDispatcher;
2121
use OCP\Files\Cache\CacheEntryInsertedEvent;
@@ -249,7 +249,7 @@ public function put($file, array $data) {
249249
* @param array $data
250250
*
251251
* @return int file id
252-
* @throws \RuntimeException
252+
* @throws \RuntimeException|Exception
253253
*/
254254
public function insert($file, array $data) {
255255
// normalize file
@@ -309,15 +309,19 @@ public function insert($file, array $data) {
309309
$this->eventDispatcher->dispatchTyped($event);
310310
return $fileId;
311311
}
312-
} catch (UniqueConstraintViolationException $e) {
313-
// entry exists already
314-
if ($this->connection->inTransaction()) {
315-
$this->connection->commit();
316-
$this->connection->beginTransaction();
312+
} catch (Exception $e) {
313+
if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
314+
// entry exists already
315+
if ($this->connection->inTransaction()) {
316+
$this->connection->commit();
317+
$this->connection->beginTransaction();
318+
}
319+
} else {
320+
throw $e;
317321
}
318322
}
319323

320-
// The file was created in the mean time
324+
// The file was created in the meantime
321325
if (($id = $this->getId($file)) > -1) {
322326
$this->update($id, $data);
323327
return $id;
@@ -377,7 +381,10 @@ public function update($id, array $data) {
377381
}
378382

379383
$query->executeStatement();
380-
} catch (UniqueConstraintViolationException $e) {
384+
} catch (Exception $e) {
385+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
386+
throw $e;
387+
}
381388
$query = $this->getQueryBuilder();
382389
$query->update('filecache_extended')
383390
->whereFileId($id)

lib/private/Group/Database.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
*/
88
namespace OC\Group;
99

10-
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
1110
use OC\User\LazyUser;
11+
use OCP\DB\Exception;
1212
use OCP\DB\QueryBuilder\IQueryBuilder;
1313
use OCP\Group\Backend\ABackend;
1414
use OCP\Group\Backend\IAddToGroupBackend;
@@ -75,8 +75,12 @@ public function createGroup(string $name): ?string {
7575
->setValue('gid', $builder->createNamedParameter($gid))
7676
->setValue('displayname', $builder->createNamedParameter($name))
7777
->executeStatement();
78-
} catch (UniqueConstraintViolationException $e) {
79-
return null;
78+
} catch (Exception $e) {
79+
if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
80+
return null;
81+
} else {
82+
throw $e;
83+
}
8084
}
8185

8286
// Add to cache

lib/private/SystemTag/SystemTagManager.php

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99
namespace OC\SystemTag;
1010

11-
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
11+
use OCP\DB\Exception;
1212
use OCP\DB\QueryBuilder\IQueryBuilder;
1313
use OCP\EventDispatcher\IEventDispatcher;
1414
use OCP\IAppConfig;
@@ -177,12 +177,15 @@ public function createTag(string $tagName, bool $userVisible, bool $userAssignab
177177

178178
try {
179179
$query->executeStatement();
180-
} catch (UniqueConstraintViolationException $e) {
181-
throw new TagAlreadyExistsException(
182-
'Tag ("' . $truncatedTagName . '", ' . $userVisible . ', ' . $userAssignable . ') already exists',
183-
0,
184-
$e
185-
);
180+
} catch (Exception $e) {
181+
if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
182+
throw new TagAlreadyExistsException(
183+
'Tag ("' . $truncatedTagName . '", ' . $userVisible . ', ' . $userAssignable . ') already exists',
184+
0,
185+
$e
186+
);
187+
}
188+
throw $e;
186189
}
187190

188191
$tagId = $query->getLastInsertId();
@@ -262,12 +265,15 @@ public function updateTag(
262265
'Tag does not exist', 0, null, [$tagId]
263266
);
264267
}
265-
} catch (UniqueConstraintViolationException $e) {
266-
throw new TagAlreadyExistsException(
267-
'Tag ("' . $newName . '", ' . $userVisible . ', ' . $userAssignable . ') already exists',
268-
0,
269-
$e
270-
);
268+
} catch (Exception $e) {
269+
if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
270+
throw new TagAlreadyExistsException(
271+
'Tag ("' . $newName . '", ' . $userVisible . ', ' . $userAssignable . ') already exists',
272+
0,
273+
$e
274+
);
275+
}
276+
throw $e;
271277
}
272278

273279
$this->dispatcher->dispatch(ManagerEvent::EVENT_UPDATE, new ManagerEvent(

lib/private/SystemTag/SystemTagObjectMapper.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*/
1010
namespace OC\SystemTag;
1111

12-
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
12+
use OCP\DB\Exception;
1313
use OCP\DB\QueryBuilder\IQueryBuilder;
1414
use OCP\EventDispatcher\IEventDispatcher;
1515
use OCP\IDBConnection;
@@ -151,7 +151,10 @@ public function assignTags(string $objId, string $objectType, $tagIds): void {
151151
$query->setParameter('tagid', $tagId);
152152
$query->executeStatement();
153153
$tagsAssigned[] = $tagId;
154-
} catch (UniqueConstraintViolationException $e) {
154+
} catch (Exception $e) {
155+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
156+
throw $e;
157+
}
155158
// ignore existing relations
156159
}
157160
}

lib/public/IDBConnection.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ public function lastInsertId(string $table): int;
146146
* @return int number of inserted rows
147147
* @throws Exception used to be the removed dbal exception, since 21.0.0 it's \OCP\DB\Exception
148148
* @since 6.0.0 - parameter $compare was added in 8.1.0, return type changed from boolean in 8.1.0
149-
* @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
149+
* @deprecated 15.0.0 - use unique index and "try { $db->insert() } catch (\OCP\DB\Exception $e) { if ($e->getReason() === \OCP\DB\Exception::REASON_CONSTRAINT_VIOLATION) {} }" instead, because it is more reliable and does not have the risk for deadlocks - see https://github.com/nextcloud/server/pull/12371
150150
*/
151151
public function insertIfNotExist(string $table, array $input, ?array $compare = null);
152152

tests/lib/Authentication/Token/ManagerTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99

1010
namespace Test\Authentication\Token;
1111

12-
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
1312
use OC\Authentication\Exceptions\InvalidTokenException;
1413
use OC\Authentication\Token\IToken;
1514
use OC\Authentication\Token\Manager;
1615
use OC\Authentication\Token\PublicKeyToken;
1716
use OC\Authentication\Token\PublicKeyTokenProvider;
17+
use OCP\DB\Exception;
1818
use PHPUnit\Framework\MockObject\MockObject;
1919
use Test\TestCase;
2020

@@ -62,8 +62,9 @@ public function testGenerateToken(): void {
6262
}
6363

6464
public function testGenerateConflictingToken(): void {
65-
/** @var MockObject|UniqueConstraintViolationException $exception */
66-
$exception = $this->createMock(UniqueConstraintViolationException::class);
65+
/** @var MockObject|Exception $exception */
66+
$exception = $this->createMock(Exception::class);
67+
$exception->method('getReason')->willReturn(Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION);
6768

6869
$token = new PublicKeyToken();
6970
$token->setUid('uid');

0 commit comments

Comments
 (0)