feat: allow running QueryBuilder queries on different connections

Signed-off-by: Robin Appelman <robin@icewind.nl>

lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php

@@ -11,6 +11,7 @@ namespace OC\DB\QueryBuilder;
 use OC\DB\Exceptions\DbalException;
 use OCP\DB\IResult;
 use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IDBConnection;
 
 /**
  * Base class for creating classes that extend the builtin query builder
@@ -46,12 +47,12 @@ abstract class ExtendedQueryBuilder implements IQueryBuilder {
 		return $this->builder->getState();
 	}
 
-	public function execute() {
+	public function execute(?IDBConnection $connection = null) {
 		try {
 			if ($this->getType() === \Doctrine\DBAL\Query\QueryBuilder::SELECT) {
-				return $this->executeQuery();
+				return $this->executeQuery($connection);
 			} else {
-				return $this->executeStatement();
+				return $this->executeStatement($connection);
 			}
 		} catch (DBALException $e) {
 			// `IQueryBuilder->execute` never wrapped the exception, but `executeQuery` and `executeStatement` do
@@ -280,11 +281,11 @@ abstract class ExtendedQueryBuilder implements IQueryBuilder {
 		return $this->builder->getColumnName($column, $tableAlias);
 	}
 
-	public function executeQuery(): IResult {
+	public function executeQuery(?IDBConnection $connection = null): IResult {
-		return $this->builder->executeQuery();
+		return $this->builder->executeQuery($connection);
 	}
 
-	public function executeStatement(): int {
+	public function executeStatement(?IDBConnection $connection = null): int {
-		return $this->builder->executeStatement();
+		return $this->builder->executeStatement($connection);
 	}
 }

lib/private/DB/QueryBuilder/QueryBuilder.php

@@ -13,6 +13,7 @@ use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Query\QueryException;
 use OC\DB\ConnectionAdapter;
+use OC\DB\Exceptions\DbalException;
 use OC\DB\QueryBuilder\ExpressionBuilder\ExpressionBuilder;
 use OC\DB\QueryBuilder\ExpressionBuilder\MySqlExpressionBuilder;
 use OC\DB\QueryBuilder\ExpressionBuilder\OCIExpressionBuilder;
@@ -22,7 +23,6 @@ use OC\DB\QueryBuilder\FunctionBuilder\FunctionBuilder;
 use OC\DB\QueryBuilder\FunctionBuilder\OCIFunctionBuilder;
 use OC\DB\QueryBuilder\FunctionBuilder\PgSqlFunctionBuilder;
 use OC\DB\QueryBuilder\FunctionBuilder\SqliteFunctionBuilder;
-use OC\DB\ResultAdapter;
 use OC\SystemConfig;
 use OCP\DB\IResult;
 use OCP\DB\QueryBuilder\ICompositeExpression;
@@ -30,6 +30,7 @@ use OCP\DB\QueryBuilder\ILiteral;
 use OCP\DB\QueryBuilder\IParameter;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\DB\QueryBuilder\IQueryFunction;
+use OCP\IDBConnection;
 use Psr\Log\LoggerInterface;
 
 class QueryBuilder implements IQueryBuilder {
@@ -168,15 +169,7 @@ class QueryBuilder implements IQueryBuilder {
 		return $this->queryBuilder->getState();
 	}
 
-	/**
+	private function prepareForExecute() {
-	 * Executes this query using the bound parameters and their types.
-	 *
-	 * Uses {@see Connection::executeQuery} for select statements and {@see Connection::executeUpdate}
-	 * for insert, update and delete statements.
-	 *
-	 * @return IResult|int
-	 */
-	public function execute() {
 		if ($this->systemConfig->getValue('log_query', false)) {
 			try {
 				$params = [];
@@ -253,48 +246,63 @@ class QueryBuilder implements IQueryBuilder {
 				'exception' => $exception,
 			]);
 		}
+	}
 
-		$result = $this->queryBuilder->execute();
+	/**
-		if (is_int($result)) {
+	 * Executes this query using the bound parameters and their types.
-			return $result;
+	 *
+	 * Uses {@see Connection::executeQuery} for select statements and {@see Connection::executeUpdate}
+	 * for insert, update and delete statements.
+	 *
+	 * @return IResult|int
+	 */
+	public function execute(?IDBConnection $connection = null) {
+		try {
+			if ($this->getType() === \Doctrine\DBAL\Query\QueryBuilder::SELECT) {
+				return $this->executeQuery($connection);
+			} else {
+				return $this->executeStatement($connection);
+			}
+		} catch (DBALException $e) {
+			// `IQueryBuilder->execute` never wrapped the exception, but `executeQuery` and `executeStatement` do
+			/** @var \Doctrine\DBAL\Exception $previous */
+			$previous = $e->getPrevious();
+			throw $previous;
 		}
-		return new ResultAdapter($result);
 	}
 
-	public function executeQuery(): IResult {
+	public function executeQuery(?IDBConnection $connection = null): IResult {
 		if ($this->getType() !== \Doctrine\DBAL\Query\QueryBuilder::SELECT) {
 			throw new \RuntimeException('Invalid query type, expected SELECT query');
 		}
 
-		try {
+		$this->prepareForExecute();
-			$result = $this->execute();
+		if (!$connection) {
-		} catch (\Doctrine\DBAL\Exception $e) {
+			$connection = $this->connection;
-			throw \OC\DB\Exceptions\DbalException::wrap($e);
 		}
 
-		if ($result instanceof IResult) {
+		return $connection->executeQuery(
-			return $result;
+			$this->getSQL(),
-		}
+			$this->getParameters(),
-
+			$this->getParameterTypes(),
-		throw new \RuntimeException('Invalid return type for query');
+		);
 	}
 
-	public function executeStatement(): int {
+	public function executeStatement(?IDBConnection $connection = null): int {
 		if ($this->getType() === \Doctrine\DBAL\Query\QueryBuilder::SELECT) {
 			throw new \RuntimeException('Invalid query type, expected INSERT, DELETE or UPDATE statement');
 		}
 
-		try {
+		$this->prepareForExecute();
-			$result = $this->execute();
+		if (!$connection) {
-		} catch (\Doctrine\DBAL\Exception $e) {
+			$connection = $this->connection;
-			throw \OC\DB\Exceptions\DbalException::wrap($e);
-		}
-
-		if (!is_int($result)) {
-			throw new \RuntimeException('Invalid return type for statement');
 		}
 
-		return $result;
+		return $connection->executeStatement(
+			$this->getSQL(),
+			$this->getParameters(),
+			$this->getParameterTypes(),
+		);
 	}
 
 

lib/public/DB/QueryBuilder/IQueryBuilder.php

@@ -12,6 +12,7 @@ use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\ParameterType;
 use OCP\DB\Exception;
 use OCP\DB\IResult;
+use OCP\IDBConnection;
 
 /**
  * This class provides a wrapper around Doctrine's QueryBuilder
@@ -146,34 +147,37 @@ interface IQueryBuilder {
 	 *          that interface changed in a breaking way the adapter \OCP\DB\QueryBuilder\IStatement is returned
 	 *          to bridge old code to the new API
 	 *
+	 * @param ?IDBConnection $connection (optional) the connection to run the query against. since 30.0
 	 * @return IResult|int
 	 * @throws Exception since 21.0.0
 	 * @since 8.2.0
 	 * @deprecated 22.0.0 Use executeQuery or executeStatement
 	 */
-	public function execute();
+	public function execute(?IDBConnection $connection = null);
 
 	/**
 	 * Execute for select statements
 	 *
+	 * @param ?IDBConnection $connection (optional) the connection to run the query against. since 30.0
 	 * @return IResult
 	 * @since 22.0.0
 	 *
 	 * @throws Exception
 	 * @throws \RuntimeException in case of usage with non select query
 	 */
-	public function executeQuery(): IResult;
+	public function executeQuery(?IDBConnection $connection = null): IResult;
 
 	/**
 	 * Execute insert, update and delete statements
 	 *
+	 * @param ?IDBConnection $connection (optional) the connection to run the query against. since 30.0
 	 * @return int the number of affected rows
 	 * @since 22.0.0
 	 *
 	 * @throws Exception
 	 * @throws \RuntimeException in case of usage with select query
 	 */
-	public function executeStatement(): int;
+	public function executeStatement(?IDBConnection $connection = null): int;
 
 	/**
 	 * Gets the complete SQL string formed by the current specifications of this QueryBuilder.

tests/lib/DB/QueryBuilder/QueryBuilderTest.php

@@ -9,11 +9,12 @@ namespace Test\DB\QueryBuilder;
 
 use Doctrine\DBAL\Query\Expression\CompositeExpression;
 use Doctrine\DBAL\Query\QueryException;
-use Doctrine\DBAL\Result;
 use OC\DB\QueryBuilder\Literal;
 use OC\DB\QueryBuilder\Parameter;
 use OC\DB\QueryBuilder\QueryBuilder;
 use OC\SystemConfig;
+use OCP\DB\IResult;
+use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\DB\QueryBuilder\IQueryFunction;
 use OCP\IDBConnection;
 use Psr\Log\LoggerInterface;
@@ -1253,16 +1254,29 @@ class QueryBuilderTest extends \Test\TestCase {
 		);
 	}
 
+	private function getConnection(): IDBConnection {
+		$connection = $this->createMock(IDBConnection::class);
+		$connection->method('executeStatement')
+			->willReturn(3);
+		$connection->method('executeQuery')
+			->willReturn($this->createMock(IResult::class));
+		return $connection;
+	}
+
 	public function testExecuteWithoutLogger() {
 		$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
 		$queryBuilder
-			->expects($this->once())
+			->method('getSQL')
-			->method('execute')
+			->willReturn('');
-			->willReturn(3);
 		$queryBuilder
-			->expects($this->any())
 			->method('getParameters')
 			->willReturn([]);
+		$queryBuilder
+			->method('getParameterTypes')
+			->willReturn([]);
+		$queryBuilder
+			->method('getType')
+			->willReturn(\Doctrine\DBAL\Query\QueryBuilder::UPDATE);
 		$this->logger
 			->expects($this->never())
 			->method('debug');
@@ -1273,6 +1287,7 @@ class QueryBuilderTest extends \Test\TestCase {
 			->willReturn(false);
 
 		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->invokePrivate($this->queryBuilder, 'connection', [$this->getConnection()]);
 		$this->assertEquals(3, $this->queryBuilder->execute());
 	}
 
@@ -1285,21 +1300,26 @@ class QueryBuilderTest extends \Test\TestCase {
 				'foo' => 'bar',
 				'key' => 'value',
 			]);
+		$queryBuilder
+			->method('getParameterTypes')
+			->willReturn([
+				'foo' => IQueryBuilder::PARAM_STR,
+				'key' => IQueryBuilder::PARAM_STR,
+			]);
+		$queryBuilder
+			->method('getType')
+			->willReturn(\Doctrine\DBAL\Query\QueryBuilder::UPDATE);
 		$queryBuilder
 			->expects($this->any())
 			->method('getSQL')
-			->willReturn('SELECT * FROM FOO WHERE BAR = ?');
+			->willReturn('UPDATE FOO SET bar = 1 WHERE BAR = ?');
-		$queryBuilder
-			->expects($this->once())
-			->method('execute')
-			->willReturn(3);
 		$this->logger
 			->expects($this->once())
 			->method('debug')
 			->with(
 				'DB QueryBuilder: \'{query}\' with parameters: {params}',
 				[
-					'query' => 'SELECT * FROM FOO WHERE BAR = ?',
+					'query' => 'UPDATE FOO SET bar = 1 WHERE BAR = ?',
 					'params' => 'foo => \'bar\', key => \'value\'',
 					'app' => 'core',
 				]
@@ -1311,6 +1331,7 @@ class QueryBuilderTest extends \Test\TestCase {
 			->willReturn(true);
 
 		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->invokePrivate($this->queryBuilder, 'connection', [$this->getConnection()]);
 		$this->assertEquals(3, $this->queryBuilder->execute());
 	}
 
@@ -1320,21 +1341,23 @@ class QueryBuilderTest extends \Test\TestCase {
 			->expects($this->any())
 			->method('getParameters')
 			->willReturn(['Bar']);
+		$queryBuilder
+			->method('getParameterTypes')
+			->willReturn([IQueryBuilder::PARAM_STR]);
+		$queryBuilder
+			->method('getType')
+			->willReturn(\Doctrine\DBAL\Query\QueryBuilder::UPDATE);
 		$queryBuilder
 			->expects($this->any())
 			->method('getSQL')
-			->willReturn('SELECT * FROM FOO WHERE BAR = ?');
+			->willReturn('UPDATE FOO SET bar = false WHERE BAR = ?');
-		$queryBuilder
-			->expects($this->once())
-			->method('execute')
-			->willReturn(3);
 		$this->logger
 			->expects($this->once())
 			->method('debug')
 			->with(
 				'DB QueryBuilder: \'{query}\' with parameters: {params}',
 				[
-					'query' => 'SELECT * FROM FOO WHERE BAR = ?',
+					'query' => 'UPDATE FOO SET bar = false WHERE BAR = ?',
 					'params' => '0 => \'Bar\'',
 					'app' => 'core',
 				]
@@ -1346,6 +1369,7 @@ class QueryBuilderTest extends \Test\TestCase {
 			->willReturn(true);
 
 		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->invokePrivate($this->queryBuilder, 'connection', [$this->getConnection()]);
 		$this->assertEquals(3, $this->queryBuilder->execute());
 	}
 
@@ -1355,21 +1379,23 @@ class QueryBuilderTest extends \Test\TestCase {
 			->expects($this->any())
 			->method('getParameters')
 			->willReturn([]);
+		$queryBuilder
+			->method('getParameterTypes')
+			->willReturn([]);
+		$queryBuilder
+			->method('getType')
+			->willReturn(\Doctrine\DBAL\Query\QueryBuilder::UPDATE);
 		$queryBuilder
 			->expects($this->any())
 			->method('getSQL')
-			->willReturn('SELECT * FROM FOO WHERE BAR = ?');
+			->willReturn('UPDATE FOO SET bar = false WHERE BAR = ?');
-		$queryBuilder
-			->expects($this->once())
-			->method('execute')
-			->willReturn(3);
 		$this->logger
 			->expects($this->once())
 			->method('debug')
 			->with(
 				'DB QueryBuilder: \'{query}\'',
 				[
-					'query' => 'SELECT * FROM FOO WHERE BAR = ?',
+					'query' => 'UPDATE FOO SET bar = false WHERE BAR = ?',
 					'app' => 'core',
 				]
 			);
@@ -1380,6 +1406,7 @@ class QueryBuilderTest extends \Test\TestCase {
 			->willReturn(true);
 
 		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->invokePrivate($this->queryBuilder, 'connection', [$this->getConnection()]);
 		$this->assertEquals(3, $this->queryBuilder->execute());
 	}
 
@@ -1390,14 +1417,13 @@ class QueryBuilderTest extends \Test\TestCase {
 			->expects($this->any())
 			->method('getParameters')
 			->willReturn([$p]);
+		$queryBuilder
+			->method('getParameterTypes')
+			->willReturn([IQueryBuilder::PARAM_STR_ARRAY]);
 		$queryBuilder
 			->expects($this->any())
 			->method('getSQL')
 			->willReturn('SELECT * FROM FOO WHERE BAR IN (?)');
-		$queryBuilder
-			->expects($this->once())
-			->method('execute')
-			->willReturn($this->createMock(Result::class));
 		$this->logger
 			->expects($this->once())
 			->method('error')
@@ -1415,6 +1441,7 @@ class QueryBuilderTest extends \Test\TestCase {
 			->willReturn(false);
 
 		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->invokePrivate($this->queryBuilder, 'connection', [$this->getConnection()]);
 		$this->queryBuilder->execute();
 	}
 
@@ -1425,14 +1452,13 @@ class QueryBuilderTest extends \Test\TestCase {
 			->expects($this->any())
 			->method('getParameters')
 			->willReturn(array_fill(0, 66, $p));
+		$queryBuilder
+			->method('getParameterTypes')
+			->willReturn([IQueryBuilder::PARAM_STR_ARRAY]);
 		$queryBuilder
 			->expects($this->any())
 			->method('getSQL')
 			->willReturn('SELECT * FROM FOO WHERE BAR IN (?) OR BAR IN (?)');
-		$queryBuilder
-			->expects($this->once())
-			->method('execute')
-			->willReturn($this->createMock(Result::class));
 		$this->logger
 			->expects($this->once())
 			->method('error')
@@ -1450,6 +1476,7 @@ class QueryBuilderTest extends \Test\TestCase {
 			->willReturn(false);
 
 		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->invokePrivate($this->queryBuilder, 'connection', [$this->getConnection()]);
 		$this->queryBuilder->execute();
 	}
 }