Merge pull request #37520 from nextcloud/fix/noid/fix-dbal-exception-handling

fix DBAL exception handling in setValues

Author: blizzz

lib/private/DB/Connection.php

@@ -40,8 +40,6 @@
 use Doctrine\DBAL\Configuration;
 use Doctrine\DBAL\Driver;
 use Doctrine\DBAL\Exception;
-use Doctrine\DBAL\Exception\ConstraintViolationException;
-use Doctrine\DBAL\Exception\NotNullConstraintViolationException;
 use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
@@ -381,10 +379,10 @@ private function getType($value) {
 	 * @param array $values (column name => value)
 	 * @param array $updatePreconditionValues ensure values match preconditions (column name => value)
 	 * @return int number of new rows
-	 * @throws \Doctrine\DBAL\Exception
+	 * @throws \OCP\DB\Exception
 	 * @throws PreConditionNotMetException
 	 */
-	public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []) {
+	public function setValues(string $table, array $keys, array $values, array $updatePreconditionValues = []): int {
 		try {
 			$insertQb = $this->getQueryBuilder();
 			$insertQb->insert($table)
@@ -394,9 +392,15 @@ public function setValues($table, array $keys, array $values, array $updatePreco
 					}, array_merge($keys, $values))
 				);
 			return $insertQb->executeStatement();
-		} catch (NotNullConstraintViolationException $e) {
-			throw $e;
-		} catch (ConstraintViolationException $e) {
+		} catch (\OCP\DB\Exception $e) {
+			if (!in_array($e->getReason(), [
+				\OCP\DB\Exception::REASON_CONSTRAINT_VIOLATION,
+				\OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION,
+			])
+			) {
+				throw $e;
+			}
+
 			// value already exists, try update
 			$updateQb = $this->getQueryBuilder();
 			$updateQb->update($table);

tests/lib/Security/CredentialsManagerTest.php

@@ -24,6 +24,9 @@
 
 namespace Test\Security;
 
+use OCP\Security\ICredentialsManager;
+use OCP\Server;
+
 /**
  * @group DB
  */
@@ -32,7 +35,7 @@ class CredentialsManagerTest extends \Test\TestCase {
 	 * @dataProvider credentialsProvider
 	 */
 	public function testWithDB($userId, $identifier) {
-		$credentialsManager = \OC::$server->getCredentialsManager();
+		$credentialsManager = Server::get(ICredentialsManager::class);
 
 		$secrets = 'Open Sesame';
 
@@ -45,7 +48,23 @@ public function testWithDB($userId, $identifier) {
 		$this->assertSame(1, $removedRows);
 	}
 
-	public function credentialsProvider() {
+	/**
+	 * @dataProvider credentialsProvider
+	 */
+	public function testUpdate($userId, $identifier): void {
+		$credentialsManager = Server::get(ICredentialsManager::class);
+
+		$secrets = 'Open Sesame';
+		$secretsRev = strrev($secrets);
+
+		$credentialsManager->store($userId, $identifier, $secrets);
+		$credentialsManager->store($userId, $identifier, $secretsRev);
+		$received = $credentialsManager->retrieve($userId, $identifier);
+
+		$this->assertSame($secretsRev, $received);
+	}
+
+	public function credentialsProvider(): array {
 		return [
 			[
 				'alice',