From c4e6fbdae7d31fe2331e896829457a4d0eb8021e Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 26 Aug 2025 14:40:39 +0200 Subject: [PATCH] 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 --- lib/private/Authentication/Token/Manager.php | 7 ++-- .../Collaboration/Resources/Manager.php | 12 +++++-- lib/private/Files/Cache/Cache.php | 25 +++++++++------ lib/private/Group/Database.php | 10 ++++-- lib/private/SystemTag/SystemTagManager.php | 32 +++++++++++-------- .../SystemTag/SystemTagObjectMapper.php | 7 ++-- lib/public/IDBConnection.php | 2 +- .../lib/Authentication/Token/ManagerTest.php | 7 ++-- 8 files changed, 66 insertions(+), 36 deletions(-) diff --git a/lib/private/Authentication/Token/Manager.php b/lib/private/Authentication/Token/Manager.php index 6953f47b004..b55970a4979 100644 --- a/lib/private/Authentication/Token/Manager.php +++ b/lib/private/Authentication/Token/Manager.php @@ -7,7 +7,6 @@ declare(strict_types=1); */ namespace OC\Authentication\Token; -use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OC\Authentication\Exceptions\InvalidTokenException as OcInvalidTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException; use OCP\Authentication\Exceptions\ExpiredTokenException; @@ -15,6 +14,7 @@ use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Exceptions\WipeTokenException; use OCP\Authentication\Token\IProvider as OCPIProvider; use OCP\Authentication\Token\IToken as OCPIToken; +use OCP\DB\Exception; class Manager implements IProvider, OCPIProvider { /** @var PublicKeyTokenProvider */ @@ -60,7 +60,10 @@ class Manager implements IProvider, OCPIProvider { $remember, $scope, ); - } catch (UniqueConstraintViolationException $e) { + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } // It's rare, but if two requests of the same session (e.g. env-based SAML) // try to create the session token they might end up here at the same time // because we use the session ID as token and the db token is created anew diff --git a/lib/private/Collaboration/Resources/Manager.php b/lib/private/Collaboration/Resources/Manager.php index c5b0d8bd339..943deb50607 100644 --- a/lib/private/Collaboration/Resources/Manager.php +++ b/lib/private/Collaboration/Resources/Manager.php @@ -8,7 +8,6 @@ declare(strict_types=1); */ namespace OC\Collaboration\Resources; -use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OCP\Collaboration\Resources\CollectionException; use OCP\Collaboration\Resources\ICollection; use OCP\Collaboration\Resources\IManager; @@ -16,6 +15,7 @@ use OCP\Collaboration\Resources\IProvider; use OCP\Collaboration\Resources\IProviderManager; use OCP\Collaboration\Resources\IResource; use OCP\Collaboration\Resources\ResourceException; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IUser; @@ -351,7 +351,10 @@ class Manager implements IManager { ]); try { $query->executeStatement(); - } catch (UniqueConstraintViolationException $e) { + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } } } @@ -367,7 +370,10 @@ class Manager implements IManager { ]); try { $query->executeStatement(); - } catch (UniqueConstraintViolationException $e) { + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } } } diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index eec146531e7..b6b550150f1 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -7,7 +7,6 @@ */ namespace OC\Files\Cache; -use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OC\DB\Exceptions\DbalException; use OC\DB\QueryBuilder\Sharded\ShardDefinition; use OC\Files\Cache\Wrapper\CacheJail; @@ -16,6 +15,7 @@ use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchQuery; use OC\Files\Storage\Wrapper\Encryption; use OC\SystemConfig; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Cache\CacheEntryInsertedEvent; @@ -249,7 +249,7 @@ class Cache implements ICache { * @param array $data * * @return int file id - * @throws \RuntimeException + * @throws \RuntimeException|Exception */ public function insert($file, array $data) { // normalize file @@ -309,15 +309,19 @@ class Cache implements ICache { $this->eventDispatcher->dispatchTyped($event); return $fileId; } - } catch (UniqueConstraintViolationException $e) { - // entry exists already - if ($this->connection->inTransaction()) { - $this->connection->commit(); - $this->connection->beginTransaction(); + } catch (Exception $e) { + if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + // entry exists already + if ($this->connection->inTransaction()) { + $this->connection->commit(); + $this->connection->beginTransaction(); + } + } else { + throw $e; } } - // The file was created in the mean time + // The file was created in the meantime if (($id = $this->getId($file)) > -1) { $this->update($id, $data); return $id; @@ -377,7 +381,10 @@ class Cache implements ICache { } $query->executeStatement(); - } catch (UniqueConstraintViolationException $e) { + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } $query = $this->getQueryBuilder(); $query->update('filecache_extended') ->whereFileId($id) diff --git a/lib/private/Group/Database.php b/lib/private/Group/Database.php index 5744b33d5b2..bf2f9dfbcc9 100644 --- a/lib/private/Group/Database.php +++ b/lib/private/Group/Database.php @@ -7,8 +7,8 @@ */ namespace OC\Group; -use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OC\User\LazyUser; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Group\Backend\ABackend; use OCP\Group\Backend\IAddToGroupBackend; @@ -75,8 +75,12 @@ class Database extends ABackend implements ->setValue('gid', $builder->createNamedParameter($gid)) ->setValue('displayname', $builder->createNamedParameter($name)) ->executeStatement(); - } catch (UniqueConstraintViolationException $e) { - return null; + } catch (Exception $e) { + if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + return null; + } else { + throw $e; + } } // Add to cache diff --git a/lib/private/SystemTag/SystemTagManager.php b/lib/private/SystemTag/SystemTagManager.php index 9b1e9fb9edd..2b8992f0aef 100644 --- a/lib/private/SystemTag/SystemTagManager.php +++ b/lib/private/SystemTag/SystemTagManager.php @@ -8,7 +8,7 @@ declare(strict_types=1); */ namespace OC\SystemTag; -use Doctrine\DBAL\Exception\UniqueConstraintViolationException; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\IAppConfig; @@ -177,12 +177,15 @@ class SystemTagManager implements ISystemTagManager { try { $query->executeStatement(); - } catch (UniqueConstraintViolationException $e) { - throw new TagAlreadyExistsException( - 'Tag ("' . $truncatedTagName . '", ' . $userVisible . ', ' . $userAssignable . ') already exists', - 0, - $e - ); + } catch (Exception $e) { + if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw new TagAlreadyExistsException( + 'Tag ("' . $truncatedTagName . '", ' . $userVisible . ', ' . $userAssignable . ') already exists', + 0, + $e + ); + } + throw $e; } $tagId = $query->getLastInsertId(); @@ -262,12 +265,15 @@ class SystemTagManager implements ISystemTagManager { 'Tag does not exist', 0, null, [$tagId] ); } - } catch (UniqueConstraintViolationException $e) { - throw new TagAlreadyExistsException( - 'Tag ("' . $newName . '", ' . $userVisible . ', ' . $userAssignable . ') already exists', - 0, - $e - ); + } catch (Exception $e) { + if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw new TagAlreadyExistsException( + 'Tag ("' . $newName . '", ' . $userVisible . ', ' . $userAssignable . ') already exists', + 0, + $e + ); + } + throw $e; } $this->dispatcher->dispatch(ManagerEvent::EVENT_UPDATE, new ManagerEvent( diff --git a/lib/private/SystemTag/SystemTagObjectMapper.php b/lib/private/SystemTag/SystemTagObjectMapper.php index 42f57fcca6a..59da1a15939 100644 --- a/lib/private/SystemTag/SystemTagObjectMapper.php +++ b/lib/private/SystemTag/SystemTagObjectMapper.php @@ -9,7 +9,7 @@ declare(strict_types=1); */ namespace OC\SystemTag; -use Doctrine\DBAL\Exception\UniqueConstraintViolationException; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; @@ -151,7 +151,10 @@ class SystemTagObjectMapper implements ISystemTagObjectMapper { $query->setParameter('tagid', $tagId); $query->executeStatement(); $tagsAssigned[] = $tagId; - } catch (UniqueConstraintViolationException $e) { + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } // ignore existing relations } } diff --git a/lib/public/IDBConnection.php b/lib/public/IDBConnection.php index ea9b71d8958..05ac0da2d7a 100644 --- a/lib/public/IDBConnection.php +++ b/lib/public/IDBConnection.php @@ -146,7 +146,7 @@ interface IDBConnection { * @return int number of inserted rows * @throws Exception used to be the removed dbal exception, since 21.0.0 it's \OCP\DB\Exception * @since 6.0.0 - parameter $compare was added in 8.1.0, return type changed from boolean in 8.1.0 - * @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 + * @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 */ public function insertIfNotExist(string $table, array $input, ?array $compare = null); diff --git a/tests/lib/Authentication/Token/ManagerTest.php b/tests/lib/Authentication/Token/ManagerTest.php index 58bbe236248..8c238fb1587 100644 --- a/tests/lib/Authentication/Token/ManagerTest.php +++ b/tests/lib/Authentication/Token/ManagerTest.php @@ -9,12 +9,12 @@ declare(strict_types=1); namespace Test\Authentication\Token; -use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Token\IToken; use OC\Authentication\Token\Manager; use OC\Authentication\Token\PublicKeyToken; use OC\Authentication\Token\PublicKeyTokenProvider; +use OCP\DB\Exception; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -62,8 +62,9 @@ class ManagerTest extends TestCase { } public function testGenerateConflictingToken(): void { - /** @var MockObject|UniqueConstraintViolationException $exception */ - $exception = $this->createMock(UniqueConstraintViolationException::class); + /** @var MockObject|Exception $exception */ + $exception = $this->createMock(Exception::class); + $exception->method('getReason')->willReturn(Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION); $token = new PublicKeyToken(); $token->setUid('uid');