[stable31] Port away from IQueryBuilder::execute in lib/ and tests/ and fix invalid usage of UniqueConstraintViolationException

lib/private/Authentication/Token/Manager.php

@@ -7,14 +7,14 @@
  */
 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;
 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 @@ public function generateToken(string $token,
 				$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

lib/private/Collaboration/Resources/Manager.php

@@ -8,14 +8,14 @@
  */
 namespace OC\Collaboration\Resources;
 
-use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
 use OCP\Collaboration\Resources\CollectionException;
 use OCP\Collaboration\Resources\ICollection;
 use OCP\Collaboration\Resources\IManager;
 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 @@ public function cacheAccessForResource(IResource $resource, ?IUser $user, bool $
 			]);
 		try {
 			$query->execute();
-		} catch (UniqueConstraintViolationException $e) {
+		} catch (Exception $e) {
+			if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
+				throw $e;
+			}
 		}
 	}
 
@@ -367,7 +370,10 @@ public function cacheAccessForCollection(ICollection $collection, ?IUser $user,
 			]);
 		try {
 			$query->execute();
-		} catch (UniqueConstraintViolationException $e) {
+		} catch (Exception $e) {
+			if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
+				throw $e;
+			}
 		}
 	}
 

lib/private/Comments/Manager.php

@@ -7,8 +7,6 @@
  */
 namespace OC\Comments;
 
-use Doctrine\DBAL\Exception\DriverException;
-use OCA\DAV\Connector\Sabre\File;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Comments\CommentsEvent;
 use OCP\Comments\IComment;
@@ -855,20 +853,23 @@ public function delete($id) {
 
 		try {
 			$comment = $this->get($id);
-		} catch (\Exception $e) {
+		} catch (\Exception) {
 			// Ignore exceptions, we just don't fire a hook then
 			$comment = null;
 		}
 
+		if (!is_numeric($id)) {
+			return false;
+		}
+
 		$qb = $this->dbConn->getQueryBuilder();
 		$query = $qb->delete('comments')
-			->where($qb->expr()->eq('id', $qb->createParameter('id')))
-			->setParameter('id', $id);
+			->where($qb->expr()->eq('id', $qb->createNamedParameter((int)$id, IQueryBuilder::PARAM_INT)));
 
 		try {
 			$affectedRows = $query->execute();
 			$this->uncache($id);
-		} catch (DriverException $e) {
+		} catch (\OCP\DB\Exception $e) {
 			$this->logger->error($e->getMessage(), [
 				'exception' => $e,
 				'app' => 'core_comments',
@@ -1325,7 +1326,7 @@ public function deleteReadMarksFromUser(IUser $user) {
 
 		try {
 			$affectedRows = $query->execute();
-		} catch (DriverException $e) {
+		} catch (\OCP\DB\Exception $e) {
 			$this->logger->error($e->getMessage(), [
 				'exception' => $e,
 				'app' => 'core_comments',
@@ -1432,7 +1433,7 @@ public function deleteReadMarksOnObject($objectType, $objectId) {
 
 		try {
 			$affectedRows = $query->execute();
-		} catch (DriverException $e) {
+		} catch (\OCP\DB\Exception $e) {
 			$this->logger->error($e->getMessage(), [
 				'exception' => $e,
 				'app' => 'core_comments',

lib/private/Files/Cache/Cache.php

@@ -7,13 +7,13 @@
  */
 namespace OC\Files\Cache;
 
-use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
 use OC\DB\Exceptions\DbalException;
 use OC\DB\QueryBuilder\Sharded\ShardDefinition;
 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;
@@ -247,7 +247,7 @@ public function put($file, array $data) {
 	 * @param array $data
 	 *
 	 * @return int file id
-	 * @throws \RuntimeException
+	 * @throws \RuntimeException|Exception
 	 */
 	public function insert($file, array $data) {
 		// normalize file
@@ -307,15 +307,19 @@ public function insert($file, array $data) {
 				$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;
@@ -375,7 +379,10 @@ public function update($id, array $data) {
 				}
 
 				$query->execute();
-			} 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)

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 @@ public function createGroup(string $name): ?string {
 				->setValue('gid', $builder->createNamedParameter($gid))
 				->setValue('displayname', $builder->createNamedParameter($name))
 				->execute();
-		} catch (UniqueConstraintViolationException $e) {
-			return null;
+		} catch (Exception $e) {
+			if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
+				return null;
+			} else {
+				throw $e;
+			}
 		}
 
 		// Add to cache

lib/private/SystemTag/SystemTagManager.php

@@ -8,7 +8,7 @@
  */
 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 @@ public function createTag(string $tagName, bool $userVisible, bool $userAssignab
 
 		try {
 			$query->execute();
-		} 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 @@ public function updateTag(
 					'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(

lib/private/SystemTag/SystemTagObjectMapper.php

@@ -9,7 +9,7 @@
  */
 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 @@ public function assignTags(string $objId, string $objectType, $tagIds): void {
 				$query->setParameter('tagid', $tagId);
 				$query->execute();
 				$tagsAssigned[] = $tagId;
-			} catch (UniqueConstraintViolationException $e) {
+			} catch (Exception $e) {
+				if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
+					throw $e;
+				}
 				// ignore existing relations
 			}
 		}

lib/public/IDBConnection.php

@@ -141,7 +141,7 @@ public function lastInsertId(string $table): int;
 	 * @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);
 

tests/lib/Authentication/Token/ManagerTest.php

@@ -9,12 +9,12 @@
 
 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 @@ public function testGenerateToken(): void {
 	}
 
 	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');